[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <82ACF49A-E44A-4C21-8961-AE5F7BC60AC7@openbsd.org>
Date: Tue, 15 Jul 2014 04:06:35 +0200
From: Brent Cook <busterb@...il.com>
To: musl@...ts.openwall.com
Cc: Bob Beck <beck@...nbsd.org>
Subject: Re: [PATCH] implement issetugid(2)
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@...ndary.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@...nbsd.org> wrote:
>
>> From: Brent Cook <brent@...ndary.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
>>
>
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.
