From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jim Lieb Subject: Re: Re: [PATCH 1/3] switch_creds: Syscall to switch creds for file server ops Date: Thu, 24 Oct 2013 13:24:10 -0700 Message-ID: <5068709.xK6KPeP0iH@jlieb-e6410> References: <1381960919-4542-1-git-send-email-jlieb@panasas.com> <8761snb44l.fsf@xmission.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: "Eric W. Biederman" , Al Viro , "Theodore Ts'o" , "linux-kernel@vger.kernel.org" , , Jeff Layton To: Andy Lutomirski , Linux FS Devel Return-path: Received: from static-209-166-131-148.expedient.com ([209.166.131.148]:59725 "EHLO natasha.panasas.com" rhost-flags-OK-FAIL-OK-OK) by vger.kernel.org with ESMTP id S1754025Ab3JXUYd convert rfc822-to-8bit (ORCPT ); Thu, 24 Oct 2013 16:24:33 -0400 In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Thursday, October 24, 2013 12:28:15 Andy Lutomirski wrote: > On Wed, Oct 23, 2013 at 10:59 PM, Eric W. Biederman >=20 > wrote: > > Andy Lutomirski writes: > >> On 10/16/2013 08:52 PM, Eric W. Biederman wrote: > >>> Al Viro writes: > >>>> On Wed, Oct 16, 2013 at 06:18:16PM -0700, Eric W. Biederman wrot= e: > >>>>> That doesn't look bad but it does need capable(CAP_SETUID) && > >>>>> capable(CAP_SETGID) or possibly something a little more refined= =2E > >>>>=20 > >>>> D'oh > >>>>=20 > >>>>> I don't think we want file descriptor passing to all of a sudde= n > >>>>> become > >>>>> a grant of privilege, beyond what the passed fd can do. > >>>>=20 > >>>> Definitely. And an extra ) to make it compile wouldn't hurt eit= her... > >>>=20 > >>> There also appears to need to be a check that we don't gain any > >>> capabilities. > >>>=20 > >>> We also need a check so that you don't gain any capabilities, and > >>> possibly a few other things. > >>=20 > >> Why? I like the user_ns part, but I'm not immediately seeing the = issue > >> with capabilities. > >=20 > > My reasoning was instead of making this syscall as generic as possi= ble > > start it out by only allowing the cases Jim cares about and working= with > > a model where you can't gain any permissions you couldn't gain > > otherwise. > >=20 > > Although the fd -1 trick to revert to your other existing cred seem= s > > reasonable. > >=20 > >>> So I suspect we want a check something like: > >>>=20 > >>> if ((new_cred->securebits !=3D current_cred->securebits) || > >>>=20 > >>> (new_cred->cap_inheritable !=3D current_cred->cap_inheritable= ) || > >>> (new_cred->cap_permitted !=3D current_cred->cap_permitted) || > >>> (new_cred->cap_effective !=3D current_cred->cap_effective) || > >>> (new_cred->cap_bset !=3D current_cred->cap_bset) || > >>> (new_cred->jit_keyring !=3D current_cred->jit_keyring) || > >>> (new_cred->session_keyring !=3D current_cred->session_keyring= ) || > >>> (new_cred->process_keyring !=3D current_cred->process_keyring= ) || > >>> (new_cred->thread_keyring !=3D current_cred->thread_keyring) = || > >>> (new_cred->request_keyring !=3D current_cred->request_keyring= ) || > >>> (new_cred->security !=3D current_cred->security) || > >>> (new_cred->user_ns !=3D current_cred->user_ns)) { > >>> =20 > >>> return -EPERM; > >>>=20 > >>> } > >>=20 > >> I *really* don't like the idea of being able to use any old file > >> descriptor. I barely care what rights the caller needs to have to > >> invoke this -- if you're going to pass an fd that grants a capabil= ity > >> (in the non-Linux sense of the work), please make sure that the se= nder > >> actually wants that behavior. > >>=20 > >> IOW, have a syscall to generate a special fd for this purpose. It= 's > >> only a couple lines of code, and I think we'll really regret it if= we > >> fsck this up. > >>=20 > >> (I will take it as a personal challenge to find at least one explo= itable > >> privilege escalation in this if an arbitrary fd works.) > >=20 > > If you can't switch to a uid or a gid you couldn't switch to otherw= ise > > then the worst that can happen is an information leak. And informa= tion > > leaks are rarely directly exploitable. >=20 > Here's the attack: >=20 > Suppose there's a daemon that uses this in conjunction with > SCM_RIGHTS. The daemon is effectively root (under the current > proposal, it has to be). So a client connects, sends a credential fd= , > and the daemon impersonates those credentials. >=20 > Now a malicious user obtains *any* fd opened by root. It sends that > fd to the daemon. The daemon then impersonates root. We lose. (It > can't possibly be hard to obtain an fd with highly privileged f_cred > -- I bet that most console programs have stdin like that, for example= =2E > There are probably many setuid programs that will happily open > /dev/null for you, too.) In my reply to Eric, I note that I need to add a check that the fd pass= ed is=20 one from switch_creds. With that test, not any fd will do and the one t= hat=20 does has only been able to set fsuid, fsgid, altgroups, and reduced (th= e nfsd=20 set) caps. They can do no more damage than what the original switch_cr= eds=20 allowed. The any fd by root no longer applies so use doesn't get much = (no=20 escalation). >=20 > >> Also... real_cred looks confusing. AFAICS it is used *only* for k= nfsd > >> and faccessat. That is, current userspace can't see it. But now = you'll > >> expose various oddities. For example, AFAICS a capability-less pr= ocess > >> that's a userns owner can always use setuid. This will *overwrite= * > >> real_cred. Then you're screwed, especially if this happens by > >> accident. > >=20 > > And doing in userland what faccessat, and knfsd do in the kernel is > > exactly what is desired here. But maybe there are issues with that= =2E > >=20 > >> That being said, Windows has had functions like this for a long ti= me. > >> Processes have a primary token and possibly an impersonation token= =2E Any > >> process can call ImpersonateLoggedOnUser (no privilege required) t= o > >> impersonate the credentials of a token (which is special kind of f= d). > >> Similarly, any process can call RevertToSelf to undo it. > >>=20 > >> Is there any actual problem with allowing completely unprivileged = tasks > >> to switch to one of these magic cred fds? That would avoid needin= g a > >> "revert" operation. > >=20 > > If the permission model is this switching of credentials doesn't ge= t you > > anything you couldn't get some other way. That would seem to total= ly > > rules out unprivileged processes switching these things. >=20 > IMO, there are two reasonable models that involve fds carrying some > kind of credential. >=20 > 1. The fd actually carries the ability to use the credentials. You > need to be very careful whom you send these to. The security > implications are obvious (which is good) and the receiver doesn't nee= d > privilege (which is good, as long as the receiver is careful). I test for caps. I think using switch_creds in all forms should requir= e=20 privs. I thought of the revert case and although it does simply return= to=20 "real" creds, you still need CAP_SETUID and CAP_SETGID to do it. This = means,=20 of course, if you have really messed up things, you may not be able to = get=20 back home which, although a bad thing for the process, is a good thing = for the=20 system as a whole. >=20 > 2. The fd is for identification only. But this means that the fd > carries the ability to identify as a user. So you *still* have to be > very careful about whom you send it to. What you need is an operatio= n > that allows you to identify using the fd without transitively grantin= g > the recipient the same ability. On networks, this is done by signing > some kind of challenge. The kernel could work the same way, or there > could be a new CMSG_IDENTITY that you need an identity fd to send but > that does not copy that fd to the recipient. I am not sure I understand this. CMSG only applies to UNIX_DOMAIN sock= ets=20 which means that the switch_creds fd test still applies here. It is=20 identification but only for within the same kernel. As for namespaces,= the=20 translation was done when the creds fd was created. I suppose if it wa= s=20 passed across namespace boundaries there could be a problem but what is= in the=20 creds structure is the translated fduid,fsgid etc., not the untranslate= d one. =20 We are still doing access checks and quota stuff with the translated cr= eds. If=20 one namespace has "fred" as uid 52 and another has "mary" as 52, the qu= ota=20 will only be credited to whoever "fred" really is only if "fred" gets t= o be=20 "mary" in the second alternate universe... >=20 > >> Another note: I think that there may be issues if the creator of a= token > >> has no_new_privs set and the user doesn't. Imagine a daemon that > >> accepts one of these fds, impersonates it, and calls exec. This c= ould > >> be used to escape from no_new_privs land. > >=20 > > Which is why I was suggesting that we don't allow changing any fiel= d in > > the cred except for uids and gids. >=20 > If the daemon impersonates and execs, we still lose. I answered this. You only get to impersonate for purposes of file acce= ss with=20 reduced caps to prevent you from being root as well. Also, since these= are=20 O_CLOEXEC and switch_creds "magic" fds, this can't happen because the f= d is=20 gone post-exec. >=20 >=20 > --Andy --=20 Jim Lieb Linux Systems Engineer Panasas Inc. "If ease of use was the only requirement, we would all be riding tricyc= les" - Douglas Engelbart 1925=E2=80=932013 -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel= " in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html