From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751986AbcETSkE (ORCPT ); Fri, 20 May 2016 14:40:04 -0400 Received: from out02.mta.xmission.com ([166.70.13.232]:44223 "EHLO out02.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751240AbcETSj7 (ORCPT ); Fri, 20 May 2016 14:39:59 -0400 From: ebiederm@xmission.com (Eric W. Biederman) To: Mimi Zohar Cc: "Serge E. Hallyn" , LKML , Jann Horn , Seth Forshee , LSM , "Andrew G. Morgan" , Kees Cook , Michael Kerrisk-manpages , "Serge E. Hallyn" , Linux API , Andy Lutomirski , Linux Containers References: <87h9egp2oq.fsf@x220.int.ebiederm.org> <20160503051921.GA31551@mail.hallyn.com> <87bn4nhejj.fsf@x220.int.ebiederm.org> <20160507231012.GA11076@pc.thejh.net> <20160511210221.GA24015@mail.hallyn.com> <20160516211523.GA5282@mail.hallyn.com> <20160516214804.GA5926@mail.hallyn.com> <20160518215752.GA9187@mail.hallyn.com> <1463691236.2465.74.camel@linux.vnet.ibm.com> <20160520034048.GA31216@mail.hallyn.com> <1463743150.2465.100.camel@linux.vnet.ibm.com> Date: Fri, 20 May 2016 13:28:55 -0500 In-Reply-To: <1463743150.2465.100.camel@linux.vnet.ibm.com> (Mimi Zohar's message of "Fri, 20 May 2016 07:19:10 -0400") Message-ID: <87mvnklh20.fsf@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-AID: U2FsdGVkX1/YK1tWXQOD33AqTLHyO6SPr8wJEznw+zM= X-SA-Exim-Connect-IP: 97.119.107.188 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.7 XMSubLong Long Subject * 0.0 TVD_RCVD_IP Message was received from an IP address * 0.0 T_TM2_M_HEADER_IN_MSG BODY: No description available. * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa06 1397; Body=1 Fuz1=1 Fuz2=1] X-Spam-DCC: XMission; sa06 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ;Mimi Zohar X-Spam-Relay-Country: X-Spam-Timing: total 778 ms - load_scoreonly_sql: 0.05 (0.0%), signal_user_changed: 3.7 (0.5%), b_tie_ro: 2.7 (0.3%), parse: 0.80 (0.1%), extract_message_metadata: 4.2 (0.5%), get_uri_detail_list: 2.4 (0.3%), tests_pri_-1000: 4.0 (0.5%), tests_pri_-950: 1.23 (0.2%), tests_pri_-900: 1.08 (0.1%), tests_pri_-400: 27 (3.5%), check_bayes: 26 (3.3%), b_tokenize: 8 (1.1%), b_tok_get_all: 9 (1.1%), b_comp_prob: 2.5 (0.3%), b_tok_touch_all: 3.7 (0.5%), b_finish: 0.71 (0.1%), tests_pri_0: 723 (92.9%), check_dkim_signature: 0.56 (0.1%), check_dkim_adsp: 3.2 (0.4%), tests_pri_500: 3.5 (0.4%), rewrite_mail: 0.00 (0.0%) Subject: Re: [PATCH RFC] user-namespaced file capabilities - now with more magic X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in01.mta.xmission.com) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Mimi Zohar writes: > On Thu, 2016-05-19 at 22:40 -0500, Serge E. Hallyn wrote: >> Quoting Mimi Zohar (zohar@linux.vnet.ibm.com): >> > On Wed, 2016-05-18 at 16:57 -0500, Serge E. Hallyn wrote: > >> > > diff --git a/fs/xattr.c b/fs/xattr.c >> > > index 4861322..5c0e7ae 100644 >> > > --- a/fs/xattr.c >> > > +++ b/fs/xattr.c >> > > @@ -94,11 +94,26 @@ int __vfs_setxattr_noperm(struct dentry *dentry, const char *name, >> > > { >> > > struct inode *inode = dentry->d_inode; >> > > int error = -EOPNOTSUPP; >> > > + void *wvalue = NULL; >> > > + size_t wsize = 0; >> > > int issec = !strncmp(name, XATTR_SECURITY_PREFIX, >> > > XATTR_SECURITY_PREFIX_LEN); >> > > >> > > - if (issec) >> > > + if (issec) { >> > > inode->i_flags &= ~S_NOSEC; >> > > + /* if root in a non-init user_ns tries to set >> > > + * security.capability, write a security.nscapability >> > > + * in its place */ >> > > + if (!strcmp(name, "security.capability") && >> > > + current_user_ns() != &init_user_ns) { >> > > + cap_setxattr_make_nscap(dentry, value, size, &wvalue, &wsize); >> > > + if (!wvalue) >> > > + return -EPERM; >> > > + value = wvalue; >> > > + size = wsize; >> > > + name = "security.nscapability"; >> > > + } >> > >> > The call to capable_wrt_inode_uidgid() is hidden behind >> > cap_setxattr_make_nscap(). Does it make sense to call it here instead, >> > before the security.capability test? This would lay the foundation for >> > doing something similar for IMA. >> >> Might make sense to move that. Though looking at it with fresh eyes I wonder >> whether adding less code here at __vfs_setxattr_noperm(), i.e. >> >> if (!cap_setxattr_makenscap(dentry, &value, &size, &name)) >> return -EPERM; >> >> would be cleaner. > > Yes, it would be cleaner, but I'm suggesting you do all the hard work > making it generic. Then the rest of us can follow your lead. Its more > likely that you'll get it right. At a high level, it might look like: > > /* Permit root in a non-init user_ns to modify the security > * namespace xattr equivalents (eg. nscapability, ns_ima, etc). > */ > if ((current_user_ns() != &init_user_ns) && > capable_wrt_inode_uidgid(inode, CAP_SETFCAP)) { > > if security..capability > call capability /* set nscapability? */ > > else if security.ima > call ima /* set ns_ima? */ > } Hmm. I am confused about this part of the strategy. I don't understand the capability vs nscapability distinction. It seems to add complexity without benefit. If I am in a nested user namespace and I try to write a capability on a file and it has nscapability I can't be allowed to (as a more privileged user namespace already wrote it). Not rewriting an existing attribute seems to be the only benefit I can see to have both a capability and a nscapability attribute vs having a new version of the capability attribute. Am I missing something here? Mimi as for generalizing the code for handling IMA I expect it makes sense to refactor the code to have shared library functions (or whatever it takes to generalize the code) when you are ready to implement this kind of IMA attribute. That way in the first implementation we can concentrate on getting the code clean and the sematics correct. That alone seems to be taking a while. Eric