This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


* Rich Felker:

>> 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,

Major file servers use this today, so it's supported as far as glibc is
concerned.  Probably the kernel considers it supported, too.

> and (2) whether the standard set*id functions should preserve such a
> state rather than clearing it.

If the process in a mixed state and set*id is called from a thread that
doesn't have CAP_SYS_ADMIN and needs to change some IDs, and then
encounters IDs that it cannot change, it's not safe to keep the process
going because we've just run a partial set*id, with no way to
communicate that to the caller.  The glibc implementation intends to
abort the process in this case.

> 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.

But setuid (getuid ()) does not fully drop privileges even without this
change.  If there are open file descriptors, it is not verified that the
new IDs are still able to re-create these descriptors with the same
semantics.  Similarly, existing file mappings are not checked, either.
This means that a library already needs to be aware of the overall state
of the process if it wants to safely drop 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.

I think there are two different scenarios:

* Dropping privileges to run user code.  For doing this to be safe in a
  multi-threaded process, it's necessary to give up all privileges and
  then call fork, set*id, and execve (with all the magic that is needed
  to disassociate the new subprocess: closing file descriptors, terminal
  and session management, etc.)  All other threads are gone at the
  set*id call, so there's no functionality change for this scenario.

* Temporarily switching privileges, to perform a set of carefully
  limited actions on behalf of a user with their permissions, instead of
  reimplementing the permission checks in userspace (which generally
  leads to more security bugs than it avoids).  This typically retains
  the privileged user ID in some fashion (via setresuid), so that the
  code can switch back, so there's no fundamental change, either.  (In
  fact, the main risk is doing this stuff in a multi-threaded process
  without libc support, so that other threads access files with the
  wrong permissions.)

>> > 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.

If the return value of the second call is not the expected one, then
what?  You still don't know whether the second call has succeeded.

> 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.

I think POSIX calls them effective IDs, and it just took Linux a while
to recognize that and adjust the signal delivery semantics accordingly.

Thanks,
Florian


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]