Re: [PATCH] Cygwin: add secure_getenv

[Corinna Vinschen <corinna-cygwin at cygwin dot com>]

On Feb 19 11:14, Eric Blake wrote:
> 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).

In fact, gnulib's implementation would chose the

   if (issetugid ())
     return NULL;
   return getenv (name);

branch on Cygwin right now, just as on BSDs.  If that's the right thing
to do for BSD, it's not... *really* wrong for Cygwin either, regardless
what Linux is doing.

That in turn means Yaakov's patch is perfeclty fine since it's equivalent
to the above gnulib code.

Agreed?


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

Attachment: signature.asc
Description: PGP signature