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