Re: dropping setuid/setgid privileges

[Bruno Haible]

James Youngman wrote:
> >  /* Drop the gid privilege first, because in some cases dropping the gid
> 
> I think you need to delete the word "dropping" in the line above.

Oops, corrected.

> I think it's also worth briefly stating into the introductory comment
> that the caller should also worry about things that dropping
> privileges doesn't directly solve, such as holding in-memory copies of
> sensitive data or open FDs.

Agreed. I'm adding this wording:

      Note: There may still be security issues if the privileged task puts
      sensitive data into the process memory or opens communication channels
      to restricted facilities.

> >     <http://www.usenix.org/events/sec02/full_papers/chen/chen.pdf>
> >     section 8.1.3 also recommends to use a setreuid call as a probe, but
> >     this call would unexpectedly succeed (and the verification thus fail)
> >     on Linux if the process has the CAP_SETUID capability.  */
> 
> I'm not convinced this approach is correct.   If we hold CAP_SETUID, I
> think we should drop that too.  Otherwise, our privilege drop is
> reversible.

I don't agree that this module should deal with Linux specific capabilities.
The module is meant for the cases when 'make install' has added a setuid or
setgid bit to the executable. Additional Linux capabilities are not needed
in this case. In other words, if the Makefile's 'install' rule sets a setuid
bit, use this module; if the Makefile's 'install' rule sets a capability,
use another code.

Additionally, capabilities like CAP_SETUID can be granted to a process even
after it has relinquished them, through setcap(). Therefore there is no point
in verifying that a process does not have CAP_SETUID.

> We also need to cope with the case where the attacker invokes the
> setuid proram with the ability to call setuid() turned off, which on
> some systems could cause the attempt to drop privileges to fail.

Yes. The proposed code does this: it verifies the return value of all
system calls.

> For this reason locate currently does this:
> 
>               /* Check that we can no longer switch back to root */
>               if (0 == setuid (0))
>                 {
>                   what = _("Failed to fully drop privileges");

I think that's overkill if you used the right system call to drop
the privilege. But it's a good verification in a unit test (to verify
that the right system call was used).

> It's possible that one of the process's supplementary groups is
> privileged.   So we may also need to do something like this:
> 
> #if HAVE_SETGROUPS
>   /* Use of setgroups() is restricted to root only. */
>   if (0 == geteuid())
>     {
>       /* We're either root or running setuid-root. */
>       gid_t groups[1];
>       groups[0] = gid;
>       if (0 != setgroups(1u, groups))
>         {
>            return -1;
>         }
>     }
> #endif

I think this is overkill as well. If the program needs to access a database
and is therefore setgid to group 'dbadmin', then:
  - If it is running on behalf of a user who is in the 'dbadmin' group,
    why would you make the program fail? The user has access to the database.
  - If it is running on behalf of 'root', then what is the point of not
    allowing 'root' to access the database? root can access all files in the
    system anyway.

Or did I misunderstand the way in which the program is installed when this
happens?

Bruno