[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <88E37E71-33ED-4D8A-9188-E6522518F7B7@openbsd.org>
Date: Sat, 12 Jul 2014 23:23:14 +0200
From: Brent Cook <busterb@...il.com>
To: Isaac Dunham <ibid.ag@...il.com>
Cc: musl@...ts.openwall.com,
Bob Beck <beck@...nbsd.org>
Subject: Re: [PATCH] implement issetugid(2)
On Jul 12, 2014, at 10:12 PM, Isaac Dunham <ibid.ag@...il.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
>>
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
