[PATCH] Cygwin: add secure_getenv

[Eric Blake]

On 2/19/19 10:58 AM, Yaakov Selkowitz wrote:

>>> "Secure execution is required if one of the following conditions was
>>>  true when the program run by the calling process was loaded: [...]"
>>>
>>> Do we ever have this situation?  We don't have any capability to make
>>> real and effective user ID different at process startup.  But from that
>>> description it seems secure_getenv does not trigger secure mode if the
>>> process calls seteuid() or setreuid() later in the process.

It says it may also be triggered by some Linux security modules (for
which I'll assume that can include states that were not present at
startup).  The main reason it was invented was to ensure that a setgid
application CANNOT be negatively impacted by LD_PRELOAD and friends
prior to main(), because all of the startup code in the dynamic loader
was switched to use secure_getenv() for any place where the loader can
normally be influenced by the environment.  But the wording sounds vague
enough about what other situations may be considered as security that it
is easy enough to just state that you should always be prepared for a
NULL return when using the function.

That said, while it is ideal to avoid squashing to NULL in situations
that are not security boundaries (as with your STC displaying HOME even
after seteuid() on Linux), I'm also okay if we filter too aggressively
(the way gnulib's fallback implementation does when neither
__secure_getenv() nor issetugid() available).


>> So I wonder if secure_getenv isn't just a synonym for getenv
>> in our case.
> 
> Or could it be the STC?  glibc's test is a bit more complicated:
> 
> https://sourceware.org/git/?p=glibc.git;a=blob;f=stdlib/tst-secure-getenv.c;hb=HEAD
> 
> And, looking now, FWIW gnulib's implementation is practically similar:
> 
> https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/secure_getenv.c;hb=HEAD

Gnulib argued that for native windows, being a synonym for getenv() is
okay because you have to opt in to running as administrator, and that
there is no native setuid/setgid binaries where you can otherwise gain
privileges by influencing the environment presented to a binary.  Of
course, if Cygwin is able to emulate setgid binaries where native
Windows can't, then we need secure_getenv() to reflect that emulation.

> 
> So if there is something wrong with the patch, then AFAIK gnulib is
> wrong too.  Eric?

The patch may be overly strict (returning NULL where it doesn't have
to), but that does not make it wrong in my eyes.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org