Re: [PATCH] Linux: Implement per-thread user and group IDs

[Rich Felker <dalias at libc dot org>]

On Mon, Jul 08, 2019 at 10:26:04AM +0200, Florian Weimer wrote:
> * Rich Felker:
> 
> > On Fri, Jun 28, 2019 at 08:54:05PM +0200, Florian Weimer wrote:
> >> This commit adds the functions pthread_attr_setperthreadids_np and
> >> pthread_attr_getperthreadids_np.  Threads created with the new flag
> >> will be exempted from the setxid broadcast.  setuid and related
> >> functions will only update the credentials for the current thread.
> >> 
> >> Multi-threaded file servers typically need this functionality and
> >> call the system calls directly to implement this.
> >
> > From a security standpoint, this is a really bad idea. Given a set of
> > tasks sharing virtual address space, the individual privilege of each
> > is effectively the union of the privileges of them all, simply because
> > each has the capacity to undermine the execution integrity of all the
> > others.
> 
> If a task has the capability to call setresuid, it can already achieve
> the same thing by calling the system call directly.  All system calls
> operate on a per-task credential structure, not a process-wide one.

Right. The questions are (1) whether such usage should be treated as
officially supported, as opposed to a hack producing invalid state,
and (2) whether the standard set*id functions should preserve such a
state rather than clearing it.

I'm strongly of the opinion that it's reasonable for application-level
code to assume that, after a successful setuid(getuid()) or similar
"privilege dropping" operation returns, privilges been fully dropped
and are not recoverable in the event that someone gains
(inaadvertently or intentionally) control of execution. If obscure
library code has left around threads marked not to be affected by
set*id(), this invariant is broken. For example, some awful PAM module
might make it so that ssh session processes end up keeping around root
privileges.

> Even if tasks with different credentials share the same address space,
> this does not make those credentials equivalent.  It is still necessary
> to affect execution in such a way that one task performs an action on
> behalf of another task which has different credentials.  Whether this is

Generally the whole point of "dropping suid" or "switching from root
to a user on whose behalf you're acting" is that you *assume* the user
can achieve code execution. Sometimes this is by design; other times
it's just a matter of the code being sufficiently complex that trying
to preclude it would be impractical.

> > For fsuid/fsgid, we already have per-thread behavior, and it's
> > somewhat reasonable because there's an understanding that this is
> > *not* restricting the privilege of the thread, just performing fs
> > access "as if" by another user/group (you always have the privilege to
> > revert fsuid/fsgid changes anyway). The useful part of the new
> > functionality your patch adds seems to just duplicate this, and the
> > remainder of the new functionality all seems actively dangerous,
> > creating a false impression that you can make isolated security
> > contexts as threads within a process.
> 
> setfsuid and setfsgid do not allow any error checking, so you have to
> parse /proc to see if the kernel gave you the requested values.

It seems like you can just call them a second time to determine
whether they succeeded, no? I agree this is an awkward interface
problem though and I wasn't aware of it.

> Originally, they were introduced because at the time, the kernel would
> allow sending signals to processes whose effective user ID was the same
> as the current process.  That is obviously bad if a file server changes
> effective IDs to get permission checking as that user from the kernel
> because it allows the user to terminate the server process.  Today,
> effective IDs are no longer used for signal permission checks, and
> fsuid/fsgid is obsolete.
> 
> And on most systems, fsuid/fsgid alone allows full escalation to the
> target user anyway, via dotfiles and similar mechanisms.

As I just said, the fs[ug]id operations are not privilege-restricting.
Rather they're a mechanism for still-fully-privileged code to perform
operations "as if" by another user/group. My understanding of what you
want to add is that it has effectively the same (lack of, in a
multithreaded setting) security properties as fs[ug]id, but with a
layer of added confusion and unsafety over what properties it
provides.

Rich