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
