From patchwork Tue Jul 21 20:35:50 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Seth Forshee X-Patchwork-Id: 6838131 Return-Path: X-Original-To: patchwork-linux-fsdevel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 11E4BC05AC for ; Tue, 21 Jul 2015 20:35:58 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id F2C4820648 for ; Tue, 21 Jul 2015 20:35:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id B06B5205F9 for ; Tue, 21 Jul 2015 20:35:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932804AbbGUUfy (ORCPT ); Tue, 21 Jul 2015 16:35:54 -0400 Received: from mail-oi0-f42.google.com ([209.85.218.42]:34093 "EHLO mail-oi0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932116AbbGUUfx (ORCPT ); Tue, 21 Jul 2015 16:35:53 -0400 Received: by oigd21 with SMTP id d21so88985337oig.1 for ; Tue, 21 Jul 2015 13:35:52 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-type:content-disposition:in-reply-to :user-agent; bh=kN5SBVvCA9N3OJS4FPnDkvhRewYsQCTWu0bh8WRe1HI=; b=TOTfqVpakAsu/wWG0t9xVYXfZG3S89NthIKUMjmmRrd63x5o8YF32mWcOHL37KvNOI XLY7ZK5ZvMLOYwe3GWKd50vAu4F/LbYv+M4dqvMcluqkzzZDKwz9knAKESfMWuxAf5zK aVFoVgkrRx8HQ1XTUBXgJiMKMpGVkb2wZNHouHGu6RCMEY47XyfTC1svV+37bq/eKcJL LIWYrne+u3l43KVahI68cZ9U3KdD9eb+E+LGBfuvre7o6CsKz8dOMNcxsRhprEK+yRHj FMrqa2JAhUPgu3ASSqUu/uM2iPkry8CaEL5e6J7zSkvoCp6Fjm3sIopDBFrEWhkhgxRk VEeg== X-Gm-Message-State: ALoCoQk//D0yVawowaPuzrFarbdKI2BhRxeBFWaDkqZ+azwPdyYYOo4TaE+aFIje49sgt+tSxyi/ X-Received: by 10.202.54.3 with SMTP id d3mr31639183oia.103.1437510952456; Tue, 21 Jul 2015 13:35:52 -0700 (PDT) Received: from localhost (199-87-125-144.dyn.kc.surewest.net. [199.87.125.144]) by smtp.gmail.com with ESMTPSA id c3sm14811778obo.5.2015.07.21.13.35.51 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Tue, 21 Jul 2015 13:35:51 -0700 (PDT) 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-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Spam-Status: No, score=-8.1 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 --- -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html 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