Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sat, 12 Jul 2014 13:12:40 -0700
From: Isaac Dunham <ibid.ag@...il.com>
To: musl@...ts.openwall.com
Cc: Brent Cook <bcook@...nbsd.org>
Subject: Re: [PATCH] implement issetugid(2)

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
> 

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.