Re: Re: Thoughts on credential switching

From: Andy Lutomirski
Date: Thu Mar 27 2014 - 15:46:03 EST


On Thu, Mar 27, 2014 at 12:30 PM, Jim Lieb <jlieb@xxxxxxxxxxx> wrote:
> Rather than inline, I'm responding in the context of Jeremy's comments but I
> have to answer others as well. It is Jeremy after all who said my baby was
> ugly ;).
>
> Jeremy is right about overloading "fd". Maybe I can call it something else
> but an fd (in implementation) has merit because a creds struct hangs off of
> either/or/both task_struct and file but nothing else. Coming up with a creds
> token struct adds/intros another struct to the kernel that has to justify its
> existence. This fd points to an anon inode so the resource consumption is
> pretty low and all the paths for managing it are well known and *work*. I'm
> trying to get across the swamp, not build a new Golden Gate bridge...
>
> As for POSIX, all of the pthreads API was based on CDE threads whose initial
> implementation was on Ultrix (BSD 4.2). Process wide was assumed because
> utheeads was a user space hack and setuid was process wide because a proc was
> a just that. Good grief, that kernel was UP... When OSF/1 appeared, the Mach
> 2.5 kernel just carried that forward with its proc + thread(s) model to be
> compatible with the old world. In other words, we incrementally backed
> ourselves into a corner. Declaring POSIX now avoids admitting that we didn't
> see all that far into the future. Enough said. These calls are *outside*
> POSIX. Pthreads in 2014 is broken/obsolete.
>
> For the interface, I changed it from what is in the cited lkml below. It is
> now:
>
> int switch_creds(struct user_creds *creds);

What is struct user_creds? And why is this called switch_creds? It
doesn't switch anything.

>
> Behavior is the same except the mux'd syscall idea is gone. Adding a flags arg
> to this is a useful idea both for current use and future proofing the API. But
> this requires a second call
>
> int use_creds(int fd);
>
> This call does the "use an fd" case but adds -1 to revert to real creds. Its
> guts are either an override_creds(fd->file->creds) or a revert_creds(). Nice
> and quick. Note that use_creds(fd) is used if I have cached the fd/token from
> switch_creds(). Also nice and quick.
>
> Given that I've done the checking in switch_creds and I don't pass anything
> back other than the token/fd and this token/fd is/will be confined to the
> thread group, use_creds(fd) only needs to validate that it is a switch_creds
> one, not from an arbitrary open(). I do this.

Not so fast... what if you start privileged, create a cred fd, call
unshare, do a dyntransition, chroot, drop privileges, and call
use_creds? I don't immediately see why this is insecure, but having
it be secure seems to be more or less the same condition as having my
credfd_activate be secure.

And I still don't see why you need revert at all. Just create a
second token/fd/whatever representing your initial creds and switch to
that.

>
> I have focused this down to "fsuid" because I intend this for ops that file
> perms. Other stuff is out of scope unless someone can come up with a use case
> and add flag defs... The other variations on the theme uid, euid, and that
> other one I don't understand the use for, are for long lasting creds change,
> i.e. become user "bob" and go off an fork a shell... I am wrapping I/O.

Isn't there euid for that?

>
> I do not like the idea of spinning off a separate proc to do creds work. It
> doesn't buy anything in performance (everybody is a task to the kernel) but it
> does open a door to scary places. Jeremy and I agree that this token/fd must
> stay within the thread group, aka, process. I have already (in the newer
> patchset) tied off inheritance by opening the anon fd with close-on-exec. I
> think I tied off passing the fd thru a unix socket but if not, I will in my
> next submission. This fd/token should stay within the thread group, period.

Maybe I'm uniquely not scared of adding sane interfaces. setuid(2) is
insane. Impersonating a token is quite sane and even has lots of
prior art.

>
> As to the "get an fd and then do set*id", you have to do this twice because
> that fd gets the creds at the time of open, not after fooling around. I am
> trying to avoid multiple RCU cycles, not add more. Second, the above path
> makes the creds switch atomic because I use the creds swapping infrastructure.
> Popping back up to user space before that *all* happens opens all kinds of
> ptrace+signal+??? holes.

I assume you're planning on caching these things. So spending some
cycles setting this thing up shouldn't matter much.

If you want to add a totally separate syscall
setresfsuidgidgroupscaps, be my guest :) It would actually be
generally useful.

>
> Note also that I mask caps as well in the override creds including the caps to
> do things like set*id. That is intentional. This whole idea is to constrain
> the thread, not open a door *but* still provide a way to get back home
> (safely). That is via use_creds(-1).
>
> Some have proposed that we personalize a worker thread (the rpc op processor)
> to the creds of the client user right off. Ganesha only needs to do this user
> creds work for VFS (kernel local) filesystems. Most of our cluster filesystems
> have apis that allow us to pass creds directly on calls. We only need this
> for that local mounted namespace. The server core owns rpc and ops, the
> backend (FSAL) owns the shim layer. User creds are backend... Having a
> personalized thread complicates the core.
>
> As I mentioned at LSF, I have another set pending that needs a bit more polish
> to answer issues from the last cycle. I need to fix the issue of handling
> syscalls that would do their own creds swapping inside the switch_creds ...
> use_creds(-1) region. The patch causes these syscalls, e.g. setuid() to
> EPERM. Again, I like this because I want the client creds impersonating
> thread to only be able to do I/O, not escape into the wild.

Eek! You want this for I/O. What if someone else wants it for
something else? Any where does the actual list of what syscalls get
blocked come from?

I think that your patches will get a *lot* simpler if you get rid of
this override_creds and revert stuff and just switch the entire set of
creds. No setuid blocking, no BUG, and no need to audit the whole
tree for odd real_creds uses.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/