From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753304AbbHCLeu (ORCPT ); Mon, 3 Aug 2015 07:34:50 -0400 Received: from mailout2.w1.samsung.com ([210.118.77.12]:15709 "EHLO mailout2.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753267AbbHCLep (ORCPT ); Mon, 3 Aug 2015 07:34:45 -0400 MIME-version: 1.0 Content-type: text/plain; charset=UTF-8 X-AuditID: cbfec7f4-f79c56d0000012ee-b0-55bf51d2c96d Content-transfer-encoding: 8BIT Message-id: <1438601680.2111.3.camel@samsung.com> Subject: Re: [PATCH v3 01/11] user_ns: 3 new LSM hooks for user namespace operations From: Lukasz Pawelczyk To: "Serge E. Hallyn" Cc: "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 , Kees Cook , Mauro Carvalho Chehab , NeilBrown , Oleg Nesterov , Paul Moore , Stephen Smalley , Tetsuo Handa , Zefan Li , linux-doc@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-security-module@vger.kernel.org, selinux@tycho.nsa.gov, havner@gmail.com Date: Mon, 03 Aug 2015 13:34:40 +0200 In-reply-to: <20150801034816.GA5541@mail.hallyn.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> X-Mailer: Evolution 3.16.4 (3.16.4-2.fc22) X-Brightmail-Tracker: H4sIAAAAAAAAA02SS0wTURiFczvT6UCsXKrVG1Q0VXwQRRpYXJUY3Y2ukIgExMeoE1BawI4Q cGGKAkpVMK0RaEUoUBc8GwOoYMUUQn1CQdT4gsjDGKTIy4CWWlvQ6O7kP19yvsVPE5KLwgD6 ZPIZTpXMKmSUL/nsl613S8++1ujQPPtiXJi1F9+sr6GwS9shwn1NPyk89OgiwGPnnSR2N2WL 8HDHgAjnfXpBYI3zB4UvVNRT2P11M84fjMKG/kESP57OofB187gAP7+sxMbcShI/sDwh8cvm mxQevzJAYe2lqyJcXasW4urPGdhSqiZxY2sOwO+1hSQ2ZI8JcVtLBYF1Q1YSd9k7RbjLZRPu WsP0j7pIxvlTCxi9+irFGNTdJHNf/1HElN1JY163xDLZ7Q4hc7+2SsA8LnKSTGtJjYipyNcJ mYnhdyTTUOBBX3yPY2otX0BkQJxvxAlOcTKdU23dedQ30dA+KUgtX5JRZzcRatC2WAN8aATD 0dj0I2IhL0P2vnpKA3xpCTQBpOmvJr2FGPqjWV2fJ9M0AVej9p4k75mAG5D2VjmxwE8BlGvt n2fEUI6m3/p4mSXwAKprrBJ4MwVD0YzdMr+1FG5ExtHZ+S0CTtDI9MFNeQsSBqHi4aL57OOR mzSZ/wipBUije0N5BxAMRrmjp64BqP9PT/9PT/+fXhkgqoCUSzueyh9LUMpDeFbJpyUnhBxP Ud4BC+8xfQ9UdGy3AkgD2SLxeNnDaImQTeczlVaAaEK2VDzr9JzEJ9jMs5wq5YgqTcHxVrCC JmXLxSXN3/ZLYAJ7hkviuFRO9bcV0D4BapASfU67ig7PYk1BsTGHKpNCc/b0AjF/SBa/DfqX Zq97FX848PvTmAA/P0NEZIFD2himyzfqXXkNJqPDab13Wy54Zttx98KIc8vcB0XwQbf0RljQ dvPsxtMjNXNR+QDO7LD9eF+T5+julK4t3uy0buoyB06t3F2y3vg2XtFzOkFG8omsPJhQ8exv pqo8zRoDAAA= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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? -- Lukasz Pawelczyk Samsung R&D Institute Poland Samsung Electronics