Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753146AbbHDBic (ORCPT ); Mon, 3 Aug 2015 21:38:32 -0400 Received: from mail-io0-f175.google.com ([209.85.223.175]:32916 "EHLO mail-io0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752968AbbHDBi0 convert rfc822-to-8bit (ORCPT ); Mon, 3 Aug 2015 21:38:26 -0400 MIME-Version: 1.0 In-Reply-To: <1438601680.2111.3.camel@samsung.com> References: <1437732285-11524-1-git-send-email-l.pawelczyk@samsung.com> <1437732285-11524-2-git-send-email-l.pawelczyk@samsung.com> <20150730213059.GA13589@mail.hallyn.com> <1438334936.2081.6.camel@samsung.com> <20150801034816.GA5541@mail.hallyn.com> <1438601680.2111.3.camel@samsung.com> Date: Mon, 3 Aug 2015 18:38:25 -0700 X-Google-Sender-Auth: xcGzADJHlD9sjwHObsBYDAmxi1U Message-ID: Subject: Re: [PATCH v3 01/11] user_ns: 3 new LSM hooks for user namespace operations From: Kees Cook To: Lukasz Pawelczyk Cc: "Serge E. Hallyn" , "Eric W. Biederman" , Al Viro , Alexey Dobriyan , Andrew Morton , Andy Lutomirski , Arnd Bergmann , Casey Schaufler , David Howells , Eric Dumazet , Eric Paris , Fabian Frederick , Greg KH , James Morris , Jiri Slaby , Joe Perches , John Johansen , Jonathan Corbet , Mauro Carvalho Chehab , NeilBrown , Oleg Nesterov , Paul Moore , Stephen Smalley , Tetsuo Handa , Zefan Li , "linux-doc@vger.kernel.org" , "linux-fsdevel@vger.kernel.org" , LKML , linux-security-module , SE Linux , havner@gmail.com Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3166 Lines: 80 On Mon, Aug 3, 2015 at 4:34 AM, Lukasz Pawelczyk wrote: > On pią, 2015-07-31 at 22:48 -0500, Serge E. Hallyn wrote: >> On Fri, Jul 31, 2015 at 11:28:56AM +0200, Lukasz Pawelczyk wrote: >> > On czw, 2015-07-30 at 16:30 -0500, Serge E. Hallyn wrote: >> > > On Fri, Jul 24, 2015 at 12:04:35PM +0200, Lukasz Pawelczyk wrote: >> > > > @@ -969,6 +982,7 @@ static int userns_install(struct nsproxy >> > > > *nsproxy, struct ns_common *ns) >> > > > { >> > > > struct user_namespace *user_ns = to_user_ns(ns); >> > > > struct cred *cred; >> > > > + int err; >> > > > >> > > > /* Don't allow gaining capabilities by reentering >> > > > * the same user namespace. >> > > > @@ -986,6 +1000,10 @@ static int userns_install(struct nsproxy >> > > > *nsproxy, struct ns_common *ns) >> > > > if (!ns_capable(user_ns, CAP_SYS_ADMIN)) >> > > > return -EPERM; >> > > > >> > > > + err = security_userns_setns(nsproxy, user_ns); >> > > > + if (err) >> > > > + return err; >> > > >> > > So at this point the LSM thinks current is in the new ns. If >> > > prepare_creds() fails below, should it be informed of that? >> > > (Or am I over-thinking this?) >> > > >> > > > + >> > > > cred = prepare_creds(); >> > > > if (!cred) >> > > > return -ENOMEM; >> > >> > Hmm, the use case for this hook I had in mind was just to allow or >> > disallow the operation based on the information passed in >> > arguments. >> > Not to register the current in any way so LSM can think it is or >> > isn't >> > in the new namespace. >> > >> > I think that any other LSM check that would like to know in what >> > namespace the current is, would just check that from current's >> > creds. >> > Not use some stale and duplicated information the above hook could >> > have >> > registered. >> > >> > I see no reason for this hook to change the LSM state, only to >> > answer >> > the question: allowed/disallowed (eventually return an error cause >> > it >> > is unable to give an answer which falls into the disallow >> > category). >> >> How about renaming it "security_userns_may_setns()" for clarity? > > I personally have nothing against it. However looking at already > existing hooks only one of them has "may" in the name (unix_may_send) > while a lot clearly have exactly this purpose (e.g. most of inode_* > family, some from file_* and task_*). So it seems the trend is against > it. > > What do you think? Anyone else has an opinion? Personally, I prefer that hooks be named as closely to their caller, or calling context, as possible. In this case, it seems like "may" is implied. It's an LSM like all the others, so it can fail, which would cause the caller to fail too, so "may" tends to be implicit. I would leave it as-is, but I could be convinced otherwise. -Kees -- Kees Cook Chrome OS Security -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/