Comment 0 for bug 1628360

Revision history for this message
Seth Arnold (seth-arnold) wrote :

Hello, I'm conducting a super-quick review of oslo.privsep as part of the
Ubuntu Main Inclusion process.

I noticed a few odd things with some of the privilege dropping code that
may or may not represent bugs:

    def _drop_privs(self):
        try:
            # Keep current capabilities across setuid away from root.
            capabilities.set_keepcaps(True)

            if self.group is not None:
                try:
                    os.setgroups([])
                except OSError:
                    msg = _('Failed to remove supplemental groups')
                    LOG.critical(msg)
                    raise FailedToDropPrivileges(msg)

            if self.user is not None:
                setuid(self.user)

            if self.group is not None:
                setgid(self.group)

        finally:
            capabilities.set_keepcaps(False)

First, if the transition is to a non-root user, the only setgid()
transitions that will work are to the real group ID or saved set-group-ID.
If the setgid() and setuid() lines are swapped, all group IDs will work.
Is this intentional?

Second, I don't understand why supplementary groups aren't dropped always.
The 'is not None' check doesn't make sense to me. Why not drop the
supplementary groups unconditionally?

Third, and most troubling, the setuid() and setgid() functions throw
exceptions when they fail but this function ignores all exceptions. These
calls can fail and when they do, it can have catastrophic consequences.
The error returns from the system calls must be checked. Does oslo.privsep
die properly when these functions fail?

Thanks