[PATCH] implement issetugid(2) (v3)

[PATCH] implement issetugid(2) (v3)

[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       | 4 ++++
 src/unistd/issetugid.c | 7 +++++++
 2 files changed, 11 insertions(+)
 create mode 100644 src/unistd/issetugid.c

diff --git a/include/unistd.h b/include/unistd.h
index bb19cd8..fc21114 100644
--- a/include/unistd.h
+++ b/include/unistd.h
@@ -192,6 +192,10 @@ int euidaccess(const char *, int);
 int eaccess(const char *, int);
 #endif
 
+#ifdef _BSD_SOURCE
+int issetugid(void);
+#endif
+
 #if defined(_LARGEFILE64_SOURCE) || defined(_GNU_SOURCE)
 #define lseek64 lseek
 #define pread64 pread
diff --git a/src/unistd/issetugid.c b/src/unistd/issetugid.c
new file mode 100644
index 0000000..6ffd930
--- /dev/null
+++ b/src/unistd/issetugid.c
@@ -0,0 +1,7 @@
+#include <unistd.h>
+#include "libc.h"
+
+int issetugid(void)
+{
+	return libc.secure;
+}
-- 
1.9.1

Re: [PATCH] implement issetugid(2) (v3)

[Szabolcs Nagy]

* Brent Cook <busterb@gmail.com> [2014-07-15 15:40:46 +0000]:
> --- a/include/unistd.h
> +++ b/include/unistd.h
> @@ -192,6 +192,10 @@ int euidaccess(const char *, int);
>  int eaccess(const char *, int);
>  #endif
>  
> +#ifdef _BSD_SOURCE
> +int issetugid(void);
> +#endif
> +

in musl _BSD_SOURCE is a subset of _GNU_SOURCE
so it should be used with || defined(_GNU_SOURCE)

(_GNU_SOURCE actually means 'everything' instead of
'gnu', and _BSD_SOURCE means 'default', but they are
very close to the usual gnu/bsd feature set applications
expect

there is no distict bsd/gnu/posix behaviour in musl, an
interface always behaves the same way if it is visible)

otherwise the patch looks good

>  #if defined(_LARGEFILE64_SOURCE) || defined(_GNU_SOURCE)
>  #define lseek64 lseek
>  #define pread64 pread
> diff --git a/src/unistd/issetugid.c b/src/unistd/issetugid.c
> new file mode 100644
> index 0000000..6ffd930
> --- /dev/null
> +++ b/src/unistd/issetugid.c
> @@ -0,0 +1,7 @@
> +#include <unistd.h>
> +#include "libc.h"
> +
> +int issetugid(void)
> +{
> +	return libc.secure;
> +}
> -- 
> 1.9.1

Re: [PATCH] implement issetugid(2) (v3)

[Brent Cook]

On Jul 15, 2014, at 11:16 AM, Szabolcs Nagy <nsz@port70.net> wrote:

> * Brent Cook <busterb@gmail.com> [2014-07-15 15:40:46 +0000]:
>> --- a/include/unistd.h
>> +++ b/include/unistd.h
>> @@ -192,6 +192,10 @@ int euidaccess(const char *, int);
>> int eaccess(const char *, int);
>> #endif
>> 
>> +#ifdef _BSD_SOURCE
>> +int issetugid(void);
>> +#endif
>> +
> 
> in musl _BSD_SOURCE is a subset of _GNU_SOURCE
> so it should be used with || defined(_GNU_SOURCE)
> 
> (_GNU_SOURCE actually means 'everything' instead of
> 'gnu', and _BSD_SOURCE means 'default', but they are
> very close to the usual gnu/bsd feature set applications
> expect
> 
> there is no distict bsd/gnu/posix behaviour in musl, an
> interface always behaves the same way if it is visible)
> 
> otherwise the patch looks good

OK, I’ll just move it to the existing block for that and resend.

>> #if defined(_LARGEFILE64_SOURCE) || defined(_GNU_SOURCE)
>> #define lseek64 lseek
>> #define pread64 pread
>> diff --git a/src/unistd/issetugid.c b/src/unistd/issetugid.c
>> new file mode 100644
>> index 0000000..6ffd930
>> --- /dev/null
>> +++ b/src/unistd/issetugid.c
>> @@ -0,0 +1,7 @@
>> +#include <unistd.h>
>> +#include "libc.h"
>> +
>> +int issetugid(void)
>> +{
>> +	return libc.secure;
>> +}
>> -- 
>> 1.9.1

Re: [PATCH] implement issetugid(2) (v3)

[Rich Felker]

On Tue, Jul 15, 2014 at 06:16:57PM +0200, Szabolcs Nagy wrote:
> * Brent Cook <busterb@gmail.com> [2014-07-15 15:40:46 +0000]:
> > --- a/include/unistd.h
> > +++ b/include/unistd.h
> > @@ -192,6 +192,10 @@ int euidaccess(const char *, int);
> >  int eaccess(const char *, int);
> >  #endif
> >  
> > +#ifdef _BSD_SOURCE
> > +int issetugid(void);
> > +#endif
> > +
> 
> in musl _BSD_SOURCE is a subset of _GNU_SOURCE
> so it should be used with || defined(_GNU_SOURCE)
> 
> (_GNU_SOURCE actually means 'everything' instead of
> 'gnu', and _BSD_SOURCE means 'default', but they are
> very close to the usual gnu/bsd feature set applications
> expect

Yes. I think we should go ahead and change the names to
_DEFAULT_SOURCE and _ALL_SOURCE and put the remapping from _BSD_SOURCE
and _GNU_SOURCE into features.h. This would eliminate confusion about
whether something should be BSD or BSD|GNU.

Rich