Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757288AbdLWDdm (ORCPT ); Fri, 22 Dec 2017 22:33:42 -0500 Received: from h2.hallyn.com ([78.46.35.8]:54014 "EHLO h2.hallyn.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757161AbdLWDdi (ORCPT ); Fri, 22 Dec 2017 22:33:38 -0500 Date: Fri, 22 Dec 2017 21:33:36 -0600 From: "Serge E. Hallyn" To: Dongsu Park Cc: linux-kernel@vger.kernel.org, containers@lists.linux-foundation.org, Alban Crequy , "Eric W . Biederman" , Miklos Szeredi , Seth Forshee , Sargun Dhillon , linux-security-module@vger.kernel.org, James Morris , Serge Hallyn Subject: Re: [PATCH 06/11] capabilities: Allow privileged user in s_user_ns to set security.* xattrs Message-ID: <20171223033336.GF6837@mail.hallyn.com> References: <5adc5e31c25beb987798ecc219df79671547a9ac.1512041070.git.dongsu@kinvolk.io> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5adc5e31c25beb987798ecc219df79671547a9ac.1512041070.git.dongsu@kinvolk.io> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2987 Lines: 77 On Fri, Dec 22, 2017 at 03:32:30PM +0100, Dongsu Park wrote: > From: Seth Forshee > > A privileged user in s_user_ns will generally have the ability to > manipulate the backing store and insert security.* xattrs into > the filesystem directly. Therefore the kernel must be prepared to > handle these xattrs from unprivileged mounts, and it makes little > sense for commoncap to prevent writing these xattrs to the > filesystem. The capability and LSM code have already been updated > to appropriately handle xattrs from unprivileged mounts, so it > is safe to loosen this restriction on setting xattrs. > > The exception to this logic is that writing xattrs to a mounted > filesystem may also cause the LSM inode_post_setxattr or > inode_setsecurity callbacks to be invoked. SELinux will deny the > xattr update by virtue of applying mountpoint labeling to > unprivileged userns mounts, and Smack will deny the writes for > any user without global CAP_MAC_ADMIN, so loosening the > capability check in commoncap is safe in this respect as well. > > Patch v4 is available: https://patchwork.kernel.org/patch/8944641/ > > Cc: linux-security-module@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: James Morris > Cc: Serge Hallyn Reviewed-by: Serge Hallyn > Signed-off-by: Seth Forshee > Signed-off-by: Dongsu Park > --- > security/commoncap.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/security/commoncap.c b/security/commoncap.c > index 4f8e0934..dd0afef9 100644 > --- a/security/commoncap.c > +++ b/security/commoncap.c > @@ -920,6 +920,8 @@ int cap_bprm_set_creds(struct linux_binprm *bprm) > int cap_inode_setxattr(struct dentry *dentry, const char *name, > const void *value, size_t size, int flags) > { > + struct user_namespace *user_ns = dentry->d_sb->s_user_ns; > + > /* Ignore non-security xattrs */ > if (strncmp(name, XATTR_SECURITY_PREFIX, > sizeof(XATTR_SECURITY_PREFIX) - 1) != 0) > @@ -932,7 +934,7 @@ int cap_inode_setxattr(struct dentry *dentry, const char *name, > if (strcmp(name, XATTR_NAME_CAPS) == 0) > return 0; > > - if (!capable(CAP_SYS_ADMIN)) > + if (!ns_capable(user_ns, CAP_SYS_ADMIN)) > return -EPERM; > return 0; > } > @@ -950,6 +952,8 @@ int cap_inode_setxattr(struct dentry *dentry, const char *name, > */ > int cap_inode_removexattr(struct dentry *dentry, const char *name) > { > + struct user_namespace *user_ns = dentry->d_sb->s_user_ns; > + > /* Ignore non-security xattrs */ > if (strncmp(name, XATTR_SECURITY_PREFIX, > sizeof(XATTR_SECURITY_PREFIX) - 1) != 0) > @@ -965,7 +969,7 @@ int cap_inode_removexattr(struct dentry *dentry, const char *name) > return 0; > } > > - if (!capable(CAP_SYS_ADMIN)) > + if (!ns_capable(user_ns, CAP_SYS_ADMIN)) > return -EPERM; > return 0; > } > -- > 2.13.6