Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933001AbbGUUfz (ORCPT ); Tue, 21 Jul 2015 16:35:55 -0400 Received: from mail-ob0-f181.google.com ([209.85.214.181]:34016 "EHLO mail-ob0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932503AbbGUUfx (ORCPT ); Tue, 21 Jul 2015 16:35:53 -0400 Date: Tue, 21 Jul 2015 15:35:50 -0500 From: Seth Forshee To: Casey Schaufler , Andy Lutomirski Cc: "Eric W. Biederman" , Alexander Viro , Linux FS Devel , LSM List , SELinux-NSA , Serge Hallyn , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH 0/7] Initial support for user namespace owned mounts Message-ID: <20150721203550.GA80838@ubuntu-hedt> References: <87615k7pyu.fsf@x220.int.ebiederm.org> <20150716135947.GC77715@ubuntu-hedt> <55A7C920.7090206@schaufler-ca.com> <20150716185750.GB51751@ubuntu-hedt> <55A8253E.3000407@schaufler-ca.com> <55A8398A.3000802@schaufler-ca.com> <55A85041.2070301@schaufler-ca.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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: 5155 Lines: 134 On Thu, Jul 16, 2015 at 05:59:22PM -0700, Andy Lutomirski wrote: > On Thu, Jul 16, 2015 at 5:45 PM, Casey Schaufler wrote: > > On 7/16/2015 4:29 PM, Andy Lutomirski wrote: > >> I really don't see the benefit of making up extra rules that apply to > >> users outside a userns who try to access specifically a filesystem > >> with backing store. They wouldn't make sense for filesystems without > >> backing store. > > > > Sure it would. For Smack, it would be the label a file would be > > created with, which would be the label of the process creating > > the memory based filesystem. For SELinux the rules are more a > > touch more sophisticated, but I'm sure that Paul or Stephen could > > come up with how to determine it. > > > > The point, looping all the way back to the beginning, where we > > were talking about just ignoring the labels on the filesystem, > > is that if you use the same Smack label on the files in the > > filesystem as the backing store file has, we'll all be happy. > > If that label isn't something user can write to, he won't be > > able to write to the mounted objects, either. If there is no > > backing store then use the label of the process creating the > > filesystem, which will be the user, which will mean everything > > will work hunky dory. > > > > Yes, there's work involved, but I doubt there's a lot. Getting > > the label from the backing store or the creating process is > > simple enough. > > So something like the diff below (untested)? All I'm really doing is setting smk_default as you describe above and then using it instead of smk_of_current() in smack_inode_alloc_security() and instead of the label from the disk in smack_d_instantiate(). Since a user currently needs CAP_MAC_ADMIN in init_user_ns to store security labels it looks like this should be sufficient. I'm not even sure that the inode_alloc_security hook changes are needed. We could allow privileged users in s_user_ns to write security labels to disk since they already control the backing store, as long as Smack didn't subsequently import them. I didn't do that here. > So what if Smack used the label of the user creating the filesystem > even for filesystems with backing store? IMO this ought to be doable > with the LSM hooks -- it certainly seems reasonable for the LSM to be > aware of who created a filesystem. In fact, I'd argue that if Smack > can't do this with the proposed LSM hooks, then the hooks are > insufficient. It would be very simple to use the label of the task instead. Seth --- diff --git a/include/linux/fs.h b/include/linux/fs.h index 32f598db0b0d..4597420ab933 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1486,6 +1486,10 @@ static inline void sb_start_intwrite(struct super_block *sb) __sb_start_write(sb, SB_FREEZE_FS, true); } +static inline bool sb_in_userns(struct super_block *sb) +{ + return sb->s_user_ns != &init_user_ns; +} extern bool inode_owner_or_capable(const struct inode *inode); diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index a143328f75eb..591fd19294e7 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -255,6 +255,10 @@ static struct smack_known *smk_fetch(const char *name, struct inode *ip, char *buffer; struct smack_known *skp = NULL; + /* Should never fetch xattrs from untrusted mounts */ + if (WARN_ON(sb_in_userns(ip->i_sb))) + return ERR_PTR(-EPERM); + if (ip->i_op->getxattr == NULL) return ERR_PTR(-EOPNOTSUPP); @@ -656,10 +660,14 @@ static int smack_sb_kern_mount(struct super_block *sb, int flags, void *data) */ if (specified) return -EPERM; + /* - * Unprivileged mounts get root and default from the caller. + * User namespace mounts get root and default from the backing + * store, if there is one. Other unprivileged mounts get them + * from the caller. */ - skp = smk_of_current(); + skp = (sb_in_userns(sb) && sb->s_bdev) ? + smk_of_inode(sb->s_bdev->bd_inode) : smk_of_current(); sp->smk_root = skp; sp->smk_default = skp; } @@ -792,7 +800,12 @@ static int smack_bprm_secureexec(struct linux_binprm *bprm) */ static int smack_inode_alloc_security(struct inode *inode) { - struct smack_known *skp = smk_of_current(); + struct smack_known *skp; + + if (sb_in_userns(inode->i_sb)) + skp = ((struct superblock_smack *)(inode->i_sb->s_security))->smk_default; + else + skp = smk_of_current(); inode->i_security = new_inode_smack(skp); if (inode->i_security == NULL) @@ -3175,6 +3188,11 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode) break; } /* + * Don't use labels from xattrs for unprivileged mounts. + */ + if (sb_in_userns(inode->i_sb)) + break; + /* * No xattr support means, alas, no SMACK label. * Use the aforeapplied default. * It would be curious if the label of the task -- 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/