[PATCH] implement issetugid(2)

[PATCH] implement issetugid(2)

[Brent Cook]

From OpenBSD 2.0 and later
http://www.openbsd.org/cgi-bin/man.cgi?query=issetugid&sektion=2
---
 include/unistd.h       | 1 +
 src/unistd/issetugid.c | 9 +++++++++
 2 files changed, 10 insertions(+)
 create mode 100644 src/unistd/issetugid.c

diff --git a/include/unistd.h b/include/unistd.h
index bb19cd8..30290c3 100644
--- a/include/unistd.h
+++ b/include/unistd.h
@@ -109,6 +109,7 @@ uid_t geteuid(void);
 gid_t getgid(void);
 gid_t getegid(void);
 int getgroups(int, gid_t []);
+int issetugid(void);
 int setuid(uid_t);
 int setreuid(uid_t, uid_t);
 int seteuid(uid_t);
diff --git a/src/unistd/issetugid.c b/src/unistd/issetugid.c
new file mode 100644
index 0000000..8c81336
--- /dev/null
+++ b/src/unistd/issetugid.c
@@ -0,0 +1,9 @@
+#include <errno.h>
+#include <unistd.h>
+#include <sys/auxv.h>
+
+int issetugid(void)
+{
+	errno = 0;
+	return !(getauxval(AT_SECURE) == 0 && errno != ENOENT);
+}
--
1.9.1

Re: [PATCH] implement issetugid(2)

[Rich Felker]

On Sat, Jul 12, 2014 at 11:55:06AM -0600, Brent Cook wrote:
> >From OpenBSD 2.0 and later
> http://www.openbsd.org/cgi-bin/man.cgi?query=issetugid&sektion=2
> ---
>  include/unistd.h       | 1 +
>  src/unistd/issetugid.c | 9 +++++++++
>  2 files changed, 10 insertions(+)
>  create mode 100644 src/unistd/issetugid.c
> 
> diff --git a/include/unistd.h b/include/unistd.h
> index bb19cd8..30290c3 100644
> --- a/include/unistd.h
> +++ b/include/unistd.h
> @@ -109,6 +109,7 @@ uid_t geteuid(void);
>  gid_t getgid(void);
>  gid_t getegid(void);
>  int getgroups(int, gid_t []);
> +int issetugid(void);
>  int setuid(uid_t);
>  int setreuid(uid_t, uid_t);
>  int seteuid(uid_t);
> diff --git a/src/unistd/issetugid.c b/src/unistd/issetugid.c
> new file mode 100644
> index 0000000..8c81336
> --- /dev/null
> +++ b/src/unistd/issetugid.c
> @@ -0,0 +1,9 @@
> +#include <errno.h>
> +#include <unistd.h>
> +#include <sys/auxv.h>
> +
> +int issetugid(void)
> +{
> +	errno = 0;
> +	return !(getauxval(AT_SECURE) == 0 && errno != ENOENT);
> +}
> --
> 1.9.1

If this interface is to be added, it should be consistent with the
internal logic and use libc.secure, not getauxval(AT_SECURE). The
proposed code above presumably gives false positives for old kernels
where AT_SECURE did not exist, whereas the internal libc logic also
checks AT_E?[UG]ID.

Rich

Re: [PATCH] implement issetugid(2)

[Isaac Dunham]

On Sat, Jul 12, 2014 at 11:55:06AM -0600, Brent Cook wrote:
> From OpenBSD 2.0 and later
> http://www.openbsd.org/cgi-bin/man.cgi?query=issetugid&sektion=2
> ---
>  include/unistd.h       | 1 +
>  src/unistd/issetugid.c | 9 +++++++++
>  2 files changed, 10 insertions(+)
>  create mode 100644 src/unistd/issetugid.c

Hello Brent Cook,
Would you mind giving an explanation of the rationale when you 
add functions?

My understanding is that this patch arose from an issue with
libressl-portable. libressl needs to check if it's running in
privileged code; while getauxval() is the simplest way to check on
Linux systems, it's unreliable.
Specifically, getauxval() on glibc < 2.19, klibc, and on bionic will not
set errno if it cannot check AT_SECURE, but only returns 0.
uclibc does not implement this, and I see no reference to dietlibc
or newlib implementations.
Given the number of known-bad versions, expecting an unknown version to
set errno is excessivey optimistic.
So libressl cannot trust getauxval() unless it's known to be a version
that sets errno; this currently means glibc 2.19+ or musl.
glibc did not version getauxval when they fixed it to set errno, so relying
on symbol versioning will not prevent running against lower versions.

(But might other versioned functions that libressl uses prevent
using older glibc versions with libressl built against glibc 2.19+?)

My guess is that the logic here is that a system providing issetugid()
can be assumed to have a working version, unlike getauxval().

Is that the reason for this?

> diff --git a/include/unistd.h b/include/unistd.h
> index bb19cd8..30290c3 100644
> --- a/include/unistd.h
> +++ b/include/unistd.h
> @@ -109,6 +109,7 @@ uid_t geteuid(void);
>  gid_t getgid(void);
>  gid_t getegid(void);
>  int getgroups(int, gid_t []);
> +int issetugid(void);

This part is a namespace violation.
issetugid() should be conditional on _BSD_SOURCE if it is added, since
unistd.h is in POSIX.

>  int setuid(uid_t);
>  int setreuid(uid_t, uid_t);
>  int seteuid(uid_t);
> diff --git a/src/unistd/issetugid.c b/src/unistd/issetugid.c
> new file mode 100644
> index 0000000..8c81336
> --- /dev/null
> +++ b/src/unistd/issetugid.c
> @@ -0,0 +1,9 @@
> +#include <errno.h>
> +#include <unistd.h>
> +#include <sys/auxv.h>
> +
> +int issetugid(void)
> +{
> +	errno = 0;
> +	return !(getauxval(AT_SECURE) == 0 && errno != ENOENT);
> +}
> --
> 1.9.1
> 

Re: [PATCH] implement issetugid(2)

[Brent Cook]

On Jul 12, 2014, at 10:12 PM, Isaac Dunham <ibid.ag@gmail.com> wrote:

> On Sat, Jul 12, 2014 at 11:55:06AM -0600, Brent Cook wrote:
>> From OpenBSD 2.0 and later
>> http://www.openbsd.org/cgi-bin/man.cgi?query=issetugid&sektion=2
>> ---
>> include/unistd.h       | 1 +
>> src/unistd/issetugid.c | 9 +++++++++
>> 2 files changed, 10 insertions(+)
>> create mode 100644 src/unistd/issetugid.c
> 
> Hello Brent Cook,
> Would you mind giving an explanation of the rationale when you 
> add functions?
> 
> My understanding is that this patch arose from an issue with
> libressl-portable. libressl needs to check if it's running in
> privileged code; while getauxval() is the simplest way to check on
> Linux systems, it's unreliable.
> Specifically, getauxval() on glibc < 2.19, klibc, and on bionic will not
> set errno if it cannot check AT_SECURE, but only returns 0.
> uclibc does not implement this, and I see no reference to dietlibc
> or newlib implementations.
> Given the number of known-bad versions, expecting an unknown version to
> set errno is excessivey optimistic.
> So libressl cannot trust getauxval() unless it's known to be a version
> that sets errno; this currently means glibc 2.19+ or musl.
> glibc did not version getauxval when they fixed it to set errno, so relying
> on symbol versioning will not prevent running against lower versions.
> 
> (But might other versioned functions that libressl uses prevent
> using older glibc versions with libressl built against glibc 2.19+?)

This is a fair point.

Compile-time tests were ruled out because static libraries can be built against a safe libc, then linked to an app that uses an unsafe libc, causing a vulnerability.

For example, here I have built a static libressl library with musl-gcc, then a link a program with the resulting library using glibc:

$ touch crypto/compat/arc4random.c

$ make V=1
Making all in crypto
make[1]: Entering directory `/home/bcook/libressl-2.0.0/crypto'
/bin/bash ../libtool  --tag=CC   --mode=compile musl-gcc -DPACKAGE_NAME=\"libressl\" -DPACKAGE_TARNAME=\"libressl\" -DPACKAGE_VERSION=\"2.0.0\" -DPACKAGE_STRING=\"libressl\ 2.0.0\" -DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" -DPACKAGE=\"libressl\" -DVERSION=\"2.0.0\" -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 -DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 -DLT_OBJDIR=\".libs/\" -DHAVE_GETAUXVAL=1 -I.  -I../include -DOPENSSL_NO_ASM -DHAVE_CRYPTODEV -DLIBRESSL_INTERNAL -I../crypto/asn1 -I../crypto/evp -I../crypto/modes  -Wall -std=c99 -g -D_BSD_SOURCE -D_POSIX_SOURCE -D_GNU_SOURCE -O2  -Wall -std=c99 -g -D_BSD_SOURCE -D_POSIX_SOURCE -D_GNU_SOURCE -MT compat/libcompat_la-arc4random.lo -MD -MP -MF compat/.deps/libcompat_la-arc4random.Tpo -c -o compat/libcompat_la-arc4random.lo `test -f 'compat/arc4random.c' || echo './'`compat/arc4random.c

..library builds..

$:~/libressl-2.0.0$ cat test.c
#include <stdio.h>
int main()
{
	printf("arc4random %u\n", arc4random());
}

$:~/libressl-2.0.0$ gcc test.c crypto/.libs/libcompat.a crypto/.libs/libcrypto.a

$:~/libressl-2.0.0$ ./a.out
arc4random 565490330

$:~/libressl-2.0.0$ ldd a.out
	linux-vdso.so.1 =>  (0x00007fffff492000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f2fde3b3000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f2fde78e000)

With the requirement of issetugid, we get a link-time failure instead:

$ cat test.c
#include <stdio.h>
int main()
{
	printf("issetugid %u\n", issetugid());
}

$ gcc test.c ./crypto/.libs/libcompat.a
/tmp/ccPenDq1.o: In function `main':
test.c:(.text+0xa): undefined reference to `issetugid'
collect2: error: ld returned 1 exit status

$ musl-gcc test.c ./crypto/.libs/libcompat.a
$ ./a.out
issetugid 0


> 
> My guess is that the logic here is that a system providing issetugid()
> can be assumed to have a working version, unlike getauxval().
> 
> Is that the reason for this?

Yes, this is an excellent summary of the issue. I should have been clearer in the initial presentation.

>> diff --git a/include/unistd.h b/include/unistd.h
>> index bb19cd8..30290c3 100644
>> --- a/include/unistd.h
>> +++ b/include/unistd.h
>> @@ -109,6 +109,7 @@ uid_t geteuid(void);
>> gid_t getgid(void);
>> gid_t getegid(void);
>> int getgroups(int, gid_t []);
>> +int issetugid(void);
> 
> This part is a namespace violation.
> issetugid() should be conditional on _BSD_SOURCE if it is added, since
> unistd.h is in POSIX.

OK, I do not think that this would be a problem.

>> int setuid(uid_t);
>> int setreuid(uid_t, uid_t);
>> int seteuid(uid_t);
>> diff --git a/src/unistd/issetugid.c b/src/unistd/issetugid.c
>> new file mode 100644
>> index 0000000..8c81336
>> --- /dev/null
>> +++ b/src/unistd/issetugid.c
>> @@ -0,0 +1,9 @@
>> +#include <errno.h>
>> +#include <unistd.h>
>> +#include <sys/auxv.h>
>> +
>> +int issetugid(void)
>> +{
>> +	errno = 0;
>> +	return !(getauxval(AT_SECURE) == 0 && errno != ENOENT);
>> +}
>> --
>> 1.9.1
>> 

Re: [PATCH] implement issetugid(2)

[Brent Cook]

On Jul 12, 2014, at 9:20 PM, Rich Felker <dalias@libc.org> wrote:

> On Sat, Jul 12, 2014 at 11:55:06AM -0600, Brent Cook wrote:
>>> From OpenBSD 2.0 and later
>> http://www.openbsd.org/cgi-bin/man.cgi?query=issetugid&sektion=2
>> ---
>> include/unistd.h       | 1 +
>> src/unistd/issetugid.c | 9 +++++++++
>> 2 files changed, 10 insertions(+)
>> create mode 100644 src/unistd/issetugid.c
>> 
>> diff --git a/include/unistd.h b/include/unistd.h
>> index bb19cd8..30290c3 100644
>> --- a/include/unistd.h
>> +++ b/include/unistd.h
>> @@ -109,6 +109,7 @@ uid_t geteuid(void);
>> gid_t getgid(void);
>> gid_t getegid(void);
>> int getgroups(int, gid_t []);
>> +int issetugid(void);
>> int setuid(uid_t);
>> int setreuid(uid_t, uid_t);
>> int seteuid(uid_t);
>> diff --git a/src/unistd/issetugid.c b/src/unistd/issetugid.c
>> new file mode 100644
>> index 0000000..8c81336
>> --- /dev/null
>> +++ b/src/unistd/issetugid.c
>> @@ -0,0 +1,9 @@
>> +#include <errno.h>
>> +#include <unistd.h>
>> +#include <sys/auxv.h>
>> +
>> +int issetugid(void)
>> +{
>> +	errno = 0;
>> +	return !(getauxval(AT_SECURE) == 0 && errno != ENOENT);
>> +}
>> --
>> 1.9.1
> 
> If this interface is to be added, it should be consistent with the
> internal logic and use libc.secure, not getauxval(AT_SECURE).

OK, that makes sense.

> The
> proposed code above presumably gives false positives for old kernels
> where AT_SECURE did not exist, whereas the internal libc logic also
> checks AT_E?[UG]ID.
> 
> Rich

Yes, the intent is that the function will fail securely if AT_SECURE is not present.

EUID/EGID checks alone do not provide the same guarantee that AT_SECURE does, since it does not consider capabilities:

http://lxr.free-electrons.com/source/security/commoncap.c#L590

There does not seem to be a good reason for a security mechanism to fail in a weaker way than it would if it succeeded.

 - Brent

Re: [PATCH] implement issetugid(2)

[Isaac Dunham]

On Sun, Jul 13, 2014 at 12:18:25AM +0200, Brent Cook wrote:
> 
> On Jul 12, 2014, at 9:20 PM, Rich Felker <dalias@libc.org> wrote:
> 
> > On Sat, Jul 12, 2014 at 11:55:06AM -0600, Brent Cook wrote:
> >>> From OpenBSD 2.0 and later
> >> http://www.openbsd.org/cgi-bin/man.cgi?query=issetugid&sektion=2
> >> ---
> >> include/unistd.h       | 1 +
> >> src/unistd/issetugid.c | 9 +++++++++
> >> 2 files changed, 10 insertions(+)
> >> create mode 100644 src/unistd/issetugid.c
> >> 
> >> diff --git a/include/unistd.h b/include/unistd.h
> >> index bb19cd8..30290c3 100644
> >> --- a/include/unistd.h
> >> +++ b/include/unistd.h
> >> @@ -109,6 +109,7 @@ uid_t geteuid(void);
> >> gid_t getgid(void);
> >> gid_t getegid(void);
> >> int getgroups(int, gid_t []);
> >> +int issetugid(void);
> >> int setuid(uid_t);
> >> int setreuid(uid_t, uid_t);
> >> int seteuid(uid_t);
> >> diff --git a/src/unistd/issetugid.c b/src/unistd/issetugid.c
> >> new file mode 100644
> >> index 0000000..8c81336
> >> --- /dev/null
> >> +++ b/src/unistd/issetugid.c
> >> @@ -0,0 +1,9 @@
> >> +#include <errno.h>
> >> +#include <unistd.h>
> >> +#include <sys/auxv.h>
> >> +
> >> +int issetugid(void)
> >> +{
> >> +	errno = 0;
> >> +	return !(getauxval(AT_SECURE) == 0 && errno != ENOENT);
> >> +}
> >> --
> >> 1.9.1
> > 
> > If this interface is to be added, it should be consistent with the
> > internal logic and use libc.secure, not getauxval(AT_SECURE).
> 
> OK, that makes sense.
> 
> > The
> > proposed code above presumably gives false positives for old kernels
> > where AT_SECURE did not exist, whereas the internal libc logic also
> > checks AT_E?[UG]ID.
> > 
> > Rich
> 
> Yes, the intent is that the function will fail securely if AT_SECURE is not present.
> 
> EUID/EGID checks alone do not provide the same guarantee that AT_SECURE does, since it does not consider capabilities:
> 
> http://lxr.free-electrons.com/source/security/commoncap.c#L590
> 
> There does not seem to be a good reason for a security mechanism to fail in a weaker way than it would if it succeeded.

As far as I can tell, no kernels supported a way to gain capabilities at
binary load time other than setuid/setgid while not supporting AT_SECURE.
The original patch to add AT_SECURE simply tested for
euid != uid || egid != gid, and had this comment:

	/* If/when this module is enhanced to incorporate capability
	   bits on files, the test below should be extended to also perform a 
	   test between the old and new capability sets.  For now,
	   it simply preserves the legacy decision algorithm used by
	   the old userland. */

The patch adding this was against kernel 2.5.x, so no supported kernel
precedes this. But 2.4.x kernels may still be used on rare occasions.

Note: while it's probably possible to reset euid/egid and drop 
privileges while keeping some capabilities in older kernels,
the values in question tell the status at load time--
before the application can do that.

Hope this helps,
Isaac Dunham

Re: [PATCH] implement issetugid(2)

[Szabolcs Nagy]

* Brent Cook <busterb@gmail.com> [2014-07-12 23:23:14 +0200]:
> Compile-time tests were ruled out because static libraries can be built against a safe libc, then linked to an app that uses an unsafe libc, causing a vulnerability.
> 

in general a static lib cannot verify the safety of the libc
that will be used with it so while i understand the concern
i think it's futile trying to work this around in the lib

i see that issetugid is needed because there are many getenv
calls in openssl, glibc has secure_getenv for this (which
can be added to musl too i think) so that might be another
approach that works on linux

Re: [PATCH] implement issetugid(2)

[Brent Cook]

On Jul 14, 2014, at 9:40 PM, Isaac Dunham <ibid.ag@gmail.com> wrote:

> On Sat, Jul 12, 2014 at 05:54:51PM -0600, Brent Cook wrote:
>> From: Brent Cook <brent@boundary.com>
>> 
>> From OpenBSD 2.0 and later, NetBSD, FreeBSD, OS X and Solaris
>> http://www.openbsd.org/cgi-bin/man.cgi?query=issetugid&sektion=2
> <snip>
>> The fix is to implement the BSD issetugid(2) interface so that a
>> portable library can use its presence to determine if the underlying C
>> library has a reliable way of determining the value of AT_SECURE, and by
>> extension if the library is running with elevated privileges. If the
>> call fails, it assumes secure mode rather than falling back to an
>> insecure result.
> 
> My previous response to your last email didn't get sent to you, for which
> I apologize. But to summarize:
> -auxval is initialized at ELF load time, so a setuid/setgid binary will
> always show up if privileges were gained.
> -AT_SECURE was added before filesystem capabilities (prior to kernel 2.6.0,
> I believe), so any system where checking AT_SECURE fails and auxval is properly
> initialized cannot obtain privileges except by setuid/setgid*
> -If auxval is not properly initialized (I'm not aware of any such cases),
> it cannot be detected if getauxval() is broken, but looking up AT_E?[UG]ID
> will also fail.

This makes sense, I have sent v3 of the patch. I am pretty sure we have not yet begun looking at some other aspects of running libressl on a 2.4 kernel, but this is a fine start.

I went ahead and subscribed anyway with my gmail account, so you don’t have to CC me directly necessarily (though please CC beck@).

Musl looks like a very nice project in general, and I can definitely see more uses for it @ the day job. We’ve had some customers that apparently build our code on musl for a while (via Alpine linux), and given that they never needed any patches or sent any bug reports, I’m going to assume it’s been 100% compatible.

Thank you!

 - Brent

> In other words, for the fallback used to set libc.secure to "fail open",
> you would have to have a 2.4 kernel, the 2.6.x filesystem code 
> (including filesystem capabilities), AND no backport of AT_SECURE.
> 
> [*] Unless we start talking about rootkits; I suspect detecting rootkits
> to avoid privilege escalation attacks on the rootkit via environmental
> variables doesn't make that much sense. ;)
> 
> See below for further comments.
> 
>> ---
>> include/unistd.h       |  3 +++
>> src/unistd/issetugid.c | 10 ++++++++++
>> 2 files changed, 13 insertions(+)
>> create mode 100644 src/unistd/issetugid.c
>> 
>> diff --git a/include/unistd.h b/include/unistd.h
>> index bb19cd8..3990c1e 100644
>> --- a/include/unistd.h
>> +++ b/include/unistd.h
>> @@ -109,6 +109,9 @@ uid_t geteuid(void);
>> gid_t getgid(void);
>> gid_t getegid(void);
>> int getgroups(int, gid_t []);
>> +#if defined(_BSD_SOURCE)
>> +int issetugid(void);
>> +#endif
>> int setuid(uid_t);
>> int setreuid(uid_t, uid_t);
>> int seteuid(uid_t);
> 
> As a point of style, #ifdef sections stand in separate blocks, after all the
> non-ifdef stuff.
> 
>> diff --git a/src/unistd/issetugid.c b/src/unistd/issetugid.c
>> new file mode 100644
>> index 0000000..f538626
>> --- /dev/null
>> +++ b/src/unistd/issetugid.c
>> @@ -0,0 +1,10 @@
>> +#include <sys/auxv.h>
>> +#include "libc.h"
>> +
>> +int issetugid(void)
>> +{
>> +	size_t *auxv = libc.auxv;
>> +	for (; *auxv; auxv+=2)
>> +		if (*auxv==AT_SECURE) return auxv[1] != 0;
>> +	return 1;
>> +}
> 
> This can be "return libc.secure;" unless you're concerned about the possibility
> that someone backported filesystem capabilities to a 2.4.x kernel without
> bothering to add AT_SECURE to auxval.
> 
> Thanks and hope this helps,
> Isaac Dunham

Re: [PATCH] implement issetugid(2)

[Rich Felker]

On Mon, Jul 14, 2014 at 07:40:31PM -0700, Isaac Dunham wrote:
> On Sat, Jul 12, 2014 at 05:54:51PM -0600, Brent Cook wrote:
> > From: Brent Cook <brent@boundary.com>
> > 
> > From OpenBSD 2.0 and later, NetBSD, FreeBSD, OS X and Solaris
> > http://www.openbsd.org/cgi-bin/man.cgi?query=issetugid&sektion=2
> <snip>
> > The fix is to implement the BSD issetugid(2) interface so that a
> > portable library can use its presence to determine if the underlying C
> > library has a reliable way of determining the value of AT_SECURE, and by
> > extension if the library is running with elevated privileges. If the
> > call fails, it assumes secure mode rather than falling back to an
> > insecure result.
> 
> My previous response to your last email didn't get sent to you, for which
> I apologize. But to summarize:
> -auxval is initialized at ELF load time, so a setuid/setgid binary will
> always show up if privileges were gained.
> -AT_SECURE was added before filesystem capabilities (prior to kernel 2.6.0,
> I believe), so any system where checking AT_SECURE fails and auxval is properly
> initialized cannot obtain privileges except by setuid/setgid*
> -If auxval is not properly initialized (I'm not aware of any such cases),
> it cannot be detected if getauxval() is broken, but looking up AT_E?[UG]ID
> will also fail.
> 
> In other words, for the fallback used to set libc.secure to "fail open",
> you would have to have a 2.4 kernel, the 2.6.x filesystem code 
> (including filesystem capabilities), AND no backport of AT_SECURE.

In that case you're already screwed since you can LD_PRELOAD anything
you want. (And even for static linking, there are some issues like
processing of arbitrary zoneinfo and soon arbitrary locale files.) So
there's no point in worrying about whether issetugid() has false
negatives on such a broken system.

> [*] Unless we start talking about rootkits; I suspect detecting rootkits
> to avoid privilege escalation attacks on the rootkit via environmental
> variables doesn't make that much sense. ;)

Agreed.

> See below for further comments.
> 
> > ---
> >  include/unistd.h       |  3 +++
> >  src/unistd/issetugid.c | 10 ++++++++++
> >  2 files changed, 13 insertions(+)
> >  create mode 100644 src/unistd/issetugid.c
> > 
> > diff --git a/include/unistd.h b/include/unistd.h
> > index bb19cd8..3990c1e 100644
> > --- a/include/unistd.h
> > +++ b/include/unistd.h
> > @@ -109,6 +109,9 @@ uid_t geteuid(void);
> >  gid_t getgid(void);
> >  gid_t getegid(void);
> >  int getgroups(int, gid_t []);
> > +#if defined(_BSD_SOURCE)
> > +int issetugid(void);
> > +#endif
> >  int setuid(uid_t);
> >  int setreuid(uid_t, uid_t);
> >  int seteuid(uid_t);
> 
> As a point of style, #ifdef sections stand in separate blocks, after all the
> non-ifdef stuff.

Yes, and it's preferable not to add new ones when such a block already exists.

> > diff --git a/src/unistd/issetugid.c b/src/unistd/issetugid.c
> > new file mode 100644
> > index 0000000..f538626
> > --- /dev/null
> > +++ b/src/unistd/issetugid.c
> > @@ -0,0 +1,10 @@
> > +#include <sys/auxv.h>
> > +#include "libc.h"
> > +
> > +int issetugid(void)
> > +{
> > +	size_t *auxv = libc.auxv;
> > +	for (; *auxv; auxv+=2)
> > +		if (*auxv==AT_SECURE) return auxv[1] != 0;
> > +	return 1;
> > +}
> 
> This can be "return libc.secure;" unless you're concerned about the possibility
> that someone backported filesystem capabilities to a 2.4.x kernel without
> bothering to add AT_SECURE to auxval.

And it should be. We don't want false positives for issetugid on 2.4
kernels. libc.secure already has to be correct or it's game over at a
much earlier/deeper level.

Rich

Re: [PATCH] implement issetugid(2)

[Isaac Dunham]

On Sat, Jul 12, 2014 at 05:54:51PM -0600, Brent Cook wrote:
> From: Brent Cook <brent@boundary.com>
> 
> From OpenBSD 2.0 and later, NetBSD, FreeBSD, OS X and Solaris
> http://www.openbsd.org/cgi-bin/man.cgi?query=issetugid&sektion=2
<snip>
> The fix is to implement the BSD issetugid(2) interface so that a
> portable library can use its presence to determine if the underlying C
> library has a reliable way of determining the value of AT_SECURE, and by
> extension if the library is running with elevated privileges. If the
> call fails, it assumes secure mode rather than falling back to an
> insecure result.

My previous response to your last email didn't get sent to you, for which
I apologize. But to summarize:
-auxval is initialized at ELF load time, so a setuid/setgid binary will
always show up if privileges were gained.
-AT_SECURE was added before filesystem capabilities (prior to kernel 2.6.0,
I believe), so any system where checking AT_SECURE fails and auxval is properly
initialized cannot obtain privileges except by setuid/setgid*
-If auxval is not properly initialized (I'm not aware of any such cases),
it cannot be detected if getauxval() is broken, but looking up AT_E?[UG]ID
will also fail.

In other words, for the fallback used to set libc.secure to "fail open",
you would have to have a 2.4 kernel, the 2.6.x filesystem code 
(including filesystem capabilities), AND no backport of AT_SECURE.

[*] Unless we start talking about rootkits; I suspect detecting rootkits
to avoid privilege escalation attacks on the rootkit via environmental
variables doesn't make that much sense. ;)

See below for further comments.

> ---
>  include/unistd.h       |  3 +++
>  src/unistd/issetugid.c | 10 ++++++++++
>  2 files changed, 13 insertions(+)
>  create mode 100644 src/unistd/issetugid.c
> 
> diff --git a/include/unistd.h b/include/unistd.h
> index bb19cd8..3990c1e 100644
> --- a/include/unistd.h
> +++ b/include/unistd.h
> @@ -109,6 +109,9 @@ uid_t geteuid(void);
>  gid_t getgid(void);
>  gid_t getegid(void);
>  int getgroups(int, gid_t []);
> +#if defined(_BSD_SOURCE)
> +int issetugid(void);
> +#endif
>  int setuid(uid_t);
>  int setreuid(uid_t, uid_t);
>  int seteuid(uid_t);

As a point of style, #ifdef sections stand in separate blocks, after all the
non-ifdef stuff.

> diff --git a/src/unistd/issetugid.c b/src/unistd/issetugid.c
> new file mode 100644
> index 0000000..f538626
> --- /dev/null
> +++ b/src/unistd/issetugid.c
> @@ -0,0 +1,10 @@
> +#include <sys/auxv.h>
> +#include "libc.h"
> +
> +int issetugid(void)
> +{
> +	size_t *auxv = libc.auxv;
> +	for (; *auxv; auxv+=2)
> +		if (*auxv==AT_SECURE) return auxv[1] != 0;
> +	return 1;
> +}

This can be "return libc.secure;" unless you're concerned about the possibility
that someone backported filesystem capabilities to a 2.4.x kernel without
bothering to add AT_SECURE to auxval.

Thanks and hope this helps,
Isaac Dunham

Re: [PATCH] implement issetugid(2)

[Brent Cook]

In glibc secure_getenv code appears to be overridable as easily as this, since __libc_enable_secure is exported:

Ubuntu 14.04 x64
$ cat secure_getenv.c
#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>

extern int __libc_enable_secure;
int main()
{
	char *foo;

	foo = secure_getenv("FOO");
	printf("%s\n", foo ? foo : "secure");

	__libc_enable_secure = 0;

	foo = secure_getenv("FOO");
	printf("%s\n", foo ? foo : "secure");
}
$ sudo chown root:root a.out
$ sudo chmod u+s a.out
$ FOO=insecure ./a.out
secure
insecure

It doesn’t take much searching to find examples like this in the wild as well:

http://git.yoctoproject.org/cgit.cgi/poky/plain/meta/recipes-core/systemd/systemd/optional_secure_getenv.patch

Granted, there are quite a few bad ‘portable’ issetugid emulations out there as well, which is all the better reason for the C library to implement correctly instead.

Regards
 - Brent

On Jul 15, 2014, at 2:57 AM, Brent Cook <brent@boundary.com> wrote:

> Sorry, I am not subscribed to the musl development list, so I missed the reply about secure_getenv() and static libraries. I did find it on the list archives while looking for a follow up.
> 
> The fact of the mater is, OpenSSL and LibreSSL will likely be used quite frequently in static libraries, and it is often bundled into standalone and commercial software. The chance for mistakes is quite high.
> 
> Additionally, there are some concerns about secure_getenv() that echo the concerns about getauxval, in that it is difficult to reason if it is really a secure interface, or that there are not libc or kernel-specific bugs that were not versioned into the interface. It contains fallback code in current versions of glibc that also does not provide the same guarantee as AT_SECURE. I filed a bug report earlier about it:
> 
> https://mail.google.com/mail/u/0/#search/secure_getenv/146e6918d408735c
> 
> On Jul 13, 2014, at 1:54 AM, Brent Cook <bcook@openbsd.org> wrote:
> 
>> From: Brent Cook <brent@boundary.com>
>> 
>> From OpenBSD 2.0 and later, NetBSD, FreeBSD, OS X and Solaris
>> http://www.openbsd.org/cgi-bin/man.cgi?query=issetugid&sektion=2
>> 
>> While getauxval(AT_SECURE) might have been able to provide comparable
>> functionality on the libc versions that support it, several Linux libc
>> versions implement it in a way such that the results cannot be trusted,
>> since there is no way to tell if it has failed. Worse, the result of '0'
>> returned on failures effectively causes the security mechanism to fail
>> 'open'.
>> 
>> There is also no simultaneously reliable and portable way for a
>> library to identify if the C library has a usable version of getauxval,
>> since the symbol is unversioned. Compile-time checks for usability are
>> also unfeasible, since static libraries built with a 'good' version can
>> be linked to a 'bad' version of getauxval.
>> 
>> The fix is to implement the BSD issetugid(2) interface so that a
>> portable library can use its presence to determine if the underlying C
>> library has a reliable way of determining the value of AT_SECURE, and by
>> extension if the library is running with elevated privileges. If the
>> call fails, it assumes secure mode rather than falling back to an
>> insecure result.
>> ---
>> include/unistd.h       |  3 +++
>> src/unistd/issetugid.c | 10 ++++++++++
>> 2 files changed, 13 insertions(+)
>> create mode 100644 src/unistd/issetugid.c
>> 
>> diff --git a/include/unistd.h b/include/unistd.h
>> index bb19cd8..3990c1e 100644
>> --- a/include/unistd.h
>> +++ b/include/unistd.h
>> @@ -109,6 +109,9 @@ uid_t geteuid(void);
>> gid_t getgid(void);
>> gid_t getegid(void);
>> int getgroups(int, gid_t []);
>> +#if defined(_BSD_SOURCE)
>> +int issetugid(void);
>> +#endif
>> int setuid(uid_t);
>> int setreuid(uid_t, uid_t);
>> int seteuid(uid_t);
>> diff --git a/src/unistd/issetugid.c b/src/unistd/issetugid.c
>> new file mode 100644
>> index 0000000..f538626
>> --- /dev/null
>> +++ b/src/unistd/issetugid.c
>> @@ -0,0 +1,10 @@
>> +#include <sys/auxv.h>
>> +#include "libc.h"
>> +
>> +int issetugid(void)
>> +{
>> +	size_t *auxv = libc.auxv;
>> +	for (; *auxv; auxv+=2)
>> +		if (*auxv==AT_SECURE) return auxv[1] != 0;
>> +	return 1;
>> +}
>> --
>> 1.9.1
>> 
> 

Re: [PATCH] implement issetugid(2)

[Brent Cook]

Whoops, wrong link:

https://sourceware.org/bugzilla/show_bug.cgi?id=17100

On Jul 15, 2014, at 2:57 AM, Brent Cook <brent@boundary.com> wrote:

> Sorry, I am not subscribed to the musl development list, so I missed the reply about secure_getenv() and static libraries. I did find it on the list archives while looking for a follow up.
> 
> The fact of the mater is, OpenSSL and LibreSSL will likely be used quite frequently in static libraries, and it is often bundled into standalone and commercial software. The chance for mistakes is quite high.
> 
> Additionally, there are some concerns about secure_getenv() that echo the concerns about getauxval, in that it is difficult to reason if it is really a secure interface, or that there are not libc or kernel-specific bugs that were not versioned into the interface. It contains fallback code in current versions of glibc that also does not provide the same guarantee as AT_SECURE. I filed a bug report earlier about it:
> 
> https://mail.google.com/mail/u/0/#search/secure_getenv/146e6918d408735c
> 
> On Jul 13, 2014, at 1:54 AM, Brent Cook <bcook@openbsd.org> wrote:
> 
>> From: Brent Cook <brent@boundary.com>
>> 
>> From OpenBSD 2.0 and later, NetBSD, FreeBSD, OS X and Solaris
>> http://www.openbsd.org/cgi-bin/man.cgi?query=issetugid&sektion=2
>> 
>> While getauxval(AT_SECURE) might have been able to provide comparable
>> functionality on the libc versions that support it, several Linux libc
>> versions implement it in a way such that the results cannot be trusted,
>> since there is no way to tell if it has failed. Worse, the result of '0'
>> returned on failures effectively causes the security mechanism to fail
>> 'open'.
>> 
>> There is also no simultaneously reliable and portable way for a
>> library to identify if the C library has a usable version of getauxval,
>> since the symbol is unversioned. Compile-time checks for usability are
>> also unfeasible, since static libraries built with a 'good' version can
>> be linked to a 'bad' version of getauxval.
>> 
>> The fix is to implement the BSD issetugid(2) interface so that a
>> portable library can use its presence to determine if the underlying C
>> library has a reliable way of determining the value of AT_SECURE, and by
>> extension if the library is running with elevated privileges. If the
>> call fails, it assumes secure mode rather than falling back to an
>> insecure result.
>> ---
>> include/unistd.h       |  3 +++
>> src/unistd/issetugid.c | 10 ++++++++++
>> 2 files changed, 13 insertions(+)
>> create mode 100644 src/unistd/issetugid.c
>> 
>> diff --git a/include/unistd.h b/include/unistd.h
>> index bb19cd8..3990c1e 100644
>> --- a/include/unistd.h
>> +++ b/include/unistd.h
>> @@ -109,6 +109,9 @@ uid_t geteuid(void);
>> gid_t getgid(void);
>> gid_t getegid(void);
>> int getgroups(int, gid_t []);
>> +#if defined(_BSD_SOURCE)
>> +int issetugid(void);
>> +#endif
>> int setuid(uid_t);
>> int setreuid(uid_t, uid_t);
>> int seteuid(uid_t);
>> diff --git a/src/unistd/issetugid.c b/src/unistd/issetugid.c
>> new file mode 100644
>> index 0000000..f538626
>> --- /dev/null
>> +++ b/src/unistd/issetugid.c
>> @@ -0,0 +1,10 @@
>> +#include <sys/auxv.h>
>> +#include "libc.h"
>> +
>> +int issetugid(void)
>> +{
>> +	size_t *auxv = libc.auxv;
>> +	for (; *auxv; auxv+=2)
>> +		if (*auxv==AT_SECURE) return auxv[1] != 0;
>> +	return 1;
>> +}
>> --
>> 1.9.1
>> 
> 

Re: [PATCH] implement issetugid(2)

[Brent Cook]

Sorry, I am not subscribed to the musl development list, so I missed the reply about secure_getenv() and static libraries. I did find it on the list archives while looking for a follow up.

The fact of the mater is, OpenSSL and LibreSSL will likely be used quite frequently in static libraries, and it is often bundled into standalone and commercial software. The chance for mistakes is quite high.

Additionally, there are some concerns about secure_getenv() that echo the concerns about getauxval, in that it is difficult to reason if it is really a secure interface, or that there are not libc or kernel-specific bugs that were not versioned into the interface. It contains fallback code in current versions of glibc that also does not provide the same guarantee as AT_SECURE. I filed a bug report earlier about it:

https://mail.google.com/mail/u/0/#search/secure_getenv/146e6918d408735c

On Jul 13, 2014, at 1:54 AM, Brent Cook <bcook@openbsd.org> wrote:

> From: Brent Cook <brent@boundary.com>
> 
> From OpenBSD 2.0 and later, NetBSD, FreeBSD, OS X and Solaris
> http://www.openbsd.org/cgi-bin/man.cgi?query=issetugid&sektion=2
> 
> While getauxval(AT_SECURE) might have been able to provide comparable
> functionality on the libc versions that support it, several Linux libc
> versions implement it in a way such that the results cannot be trusted,
> since there is no way to tell if it has failed. Worse, the result of '0'
> returned on failures effectively causes the security mechanism to fail
> 'open'.
> 
> There is also no simultaneously reliable and portable way for a
> library to identify if the C library has a usable version of getauxval,
> since the symbol is unversioned. Compile-time checks for usability are
> also unfeasible, since static libraries built with a 'good' version can
> be linked to a 'bad' version of getauxval.
> 
> The fix is to implement the BSD issetugid(2) interface so that a
> portable library can use its presence to determine if the underlying C
> library has a reliable way of determining the value of AT_SECURE, and by
> extension if the library is running with elevated privileges. If the
> call fails, it assumes secure mode rather than falling back to an
> insecure result.
> ---
> include/unistd.h       |  3 +++
> src/unistd/issetugid.c | 10 ++++++++++
> 2 files changed, 13 insertions(+)
> create mode 100644 src/unistd/issetugid.c
> 
> diff --git a/include/unistd.h b/include/unistd.h
> index bb19cd8..3990c1e 100644
> --- a/include/unistd.h
> +++ b/include/unistd.h
> @@ -109,6 +109,9 @@ uid_t geteuid(void);
> gid_t getgid(void);
> gid_t getegid(void);
> int getgroups(int, gid_t []);
> +#if defined(_BSD_SOURCE)
> +int issetugid(void);
> +#endif
> int setuid(uid_t);
> int setreuid(uid_t, uid_t);
> int seteuid(uid_t);
> diff --git a/src/unistd/issetugid.c b/src/unistd/issetugid.c
> new file mode 100644
> index 0000000..f538626
> --- /dev/null
> +++ b/src/unistd/issetugid.c
> @@ -0,0 +1,10 @@
> +#include <sys/auxv.h>
> +#include "libc.h"
> +
> +int issetugid(void)
> +{
> +	size_t *auxv = libc.auxv;
> +	for (; *auxv; auxv+=2)
> +		if (*auxv==AT_SECURE) return auxv[1] != 0;
> +	return 1;
> +}
> --
> 1.9.1
> 

[PATCH] implement issetugid(2)

[Brent Cook]

From: Brent Cook <brent@boundary.com>

From OpenBSD 2.0 and later, NetBSD, FreeBSD, OS X and Solaris
http://www.openbsd.org/cgi-bin/man.cgi?query=issetugid&sektion=2

While getauxval(AT_SECURE) might have been able to provide comparable
functionality on the libc versions that support it, several Linux libc
versions implement it in a way such that the results cannot be trusted,
since there is no way to tell if it has failed. Worse, the result of '0'
returned on failures effectively causes the security mechanism to fail
'open'.

There is also no simultaneously reliable and portable way for a
library to identify if the C library has a usable version of getauxval,
since the symbol is unversioned. Compile-time checks for usability are
also unfeasible, since static libraries built with a 'good' version can
be linked to a 'bad' version of getauxval.

The fix is to implement the BSD issetugid(2) interface so that a
portable library can use its presence to determine if the underlying C
library has a reliable way of determining the value of AT_SECURE, and by
extension if the library is running with elevated privileges. If the
call fails, it assumes secure mode rather than falling back to an
insecure result.
---
 include/unistd.h       |  3 +++
 src/unistd/issetugid.c | 10 ++++++++++
 2 files changed, 13 insertions(+)
 create mode 100644 src/unistd/issetugid.c

diff --git a/include/unistd.h b/include/unistd.h
index bb19cd8..3990c1e 100644
--- a/include/unistd.h
+++ b/include/unistd.h
@@ -109,6 +109,9 @@ uid_t geteuid(void);
 gid_t getgid(void);
 gid_t getegid(void);
 int getgroups(int, gid_t []);
+#if defined(_BSD_SOURCE)
+int issetugid(void);
+#endif
 int setuid(uid_t);
 int setreuid(uid_t, uid_t);
 int seteuid(uid_t);
diff --git a/src/unistd/issetugid.c b/src/unistd/issetugid.c
new file mode 100644
index 0000000..f538626
--- /dev/null
+++ b/src/unistd/issetugid.c
@@ -0,0 +1,10 @@
+#include <sys/auxv.h>
+#include "libc.h"
+
+int issetugid(void)
+{
+	size_t *auxv = libc.auxv;
+	for (; *auxv; auxv+=2)
+		if (*auxv==AT_SECURE) return auxv[1] != 0;
+	return 1;
+}
--
1.9.1