| Message ID | 55AFFC32.6070701@tycho.nsa.gov (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/22/2015 04:25 PM, Stephen Smalley wrote: > On 07/22/2015 12:14 PM, Seth Forshee wrote: >> On Wed, Jul 22, 2015 at 12:02:13PM -0400, Stephen Smalley wrote: >>> On 07/16/2015 09:23 AM, Stephen Smalley wrote: >>>> On 07/15/2015 03:46 PM, Seth Forshee wrote: >>>>> Unprivileged users should not be able to supply security labels >>>>> in filesystems, nor should they be able to supply security >>>>> contexts in unprivileged mounts. For any mount where s_user_ns is >>>>> not init_user_ns, force the use of SECURITY_FS_USE_NONE behavior >>>>> and return EPERM if any contexts are supplied in the mount >>>>> options. >>>>> >>>>> Signed-off-by: Seth Forshee <seth.forshee@canonical.com> >>>> >>>> I think this is obsoleted by the subsequent discussion, but just for the >>>> record: this patch would cause the files in the userns mount to be left >>>> with the "unlabeled" label, and therefore under typical policies, >>>> completely inaccessible to any process in a confined domain. >>> >>> The right way to handle this for SELinux would be to automatically use >>> mountpoint labeling (SECURITY_FS_USE_MNTPOINT, normally set by >>> specifying a context= mount option), with the sbsec->mntpoint_sid set >>> from some related object (e.g. the block device file context, as in your >>> patches for Smack). That will cause SELinux to use that value instead >>> of any xattr value from the filesystem and will cause attempts by >>> userspace to set the security.selinux xattr to fail on that filesystem. >>> That is how SELinux normally deals with untrusted filesystems, except >>> that it is normally specified as a mount option by a trusted mounting >>> process, whereas in your case you need to automatically set it. >> >> Excellent, thank you for the advice. I'll start on this when I've >> finished with Smack. > > Not tested, but something like this should work. Note that it should > come after the call to security_fs_use() so we know whether SELinux > would even try to use xattrs supplied by the filesystem in the first place. > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 564079c..84da3a2 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -745,6 +745,30 @@ static int selinux_set_mnt_opts(struct super_block *sb, > goto out; > } > } > + > + /* > + * If this is a user namespace mount, no contexts are allowed > + * on the command line and security labels must be ignored. > + */ > + if (sb->s_user_ns != &init_user_ns) { > + if (context_sid || fscontext_sid || rootcontext_sid || > + defcontext_sid) { > + rc = -EACCES; > + goto out; > + } > + if (sbsec->behavior == SECURITY_FS_USE_XATTR) { > + struct block_device *bdev = sb->s_bdev; > + sbsec->behavior = SECURITY_FS_USE_MNTPOINT; > + if (bdev) { > + struct inode_security_struct *isec = > bdev->bd_inode; That should be bdev->bd_inode->i_security. > + sbsec->mntpoint_sid = isec->sid; > + } else { > + sbsec->mntpoint_sid = current_sid(); > + } > + } > + goto out_set_opts; > + } > + > /* sets the context of the superblock for the fs being mounted. */ > if (fscontext_sid) { > rc = may_context_mount_sb_relabel(fscontext_sid, sbsec, > cred); > @@ -813,6 +837,7 @@ static int selinux_set_mnt_opts(struct super_block *sb, > sbsec->def_sid = defcontext_sid; > } > > +out_set_opts: > rc = sb_finish_set_opts(sb); > out: > mutex_unlock(&sbsec->lock); > > _______________________________________________ > Selinux mailing list > Selinux@tycho.nsa.gov > To unsubscribe, send email to Selinux-leave@tycho.nsa.gov. > To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov. > -- 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
On 07/22/2015 04:40 PM, Stephen Smalley wrote: > On 07/22/2015 04:25 PM, Stephen Smalley wrote: >> On 07/22/2015 12:14 PM, Seth Forshee wrote: >>> On Wed, Jul 22, 2015 at 12:02:13PM -0400, Stephen Smalley wrote: >>>> On 07/16/2015 09:23 AM, Stephen Smalley wrote: >>>>> On 07/15/2015 03:46 PM, Seth Forshee wrote: >>>>>> Unprivileged users should not be able to supply security labels >>>>>> in filesystems, nor should they be able to supply security >>>>>> contexts in unprivileged mounts. For any mount where s_user_ns is >>>>>> not init_user_ns, force the use of SECURITY_FS_USE_NONE behavior >>>>>> and return EPERM if any contexts are supplied in the mount >>>>>> options. >>>>>> >>>>>> Signed-off-by: Seth Forshee <seth.forshee@canonical.com> >>>>> >>>>> I think this is obsoleted by the subsequent discussion, but just for the >>>>> record: this patch would cause the files in the userns mount to be left >>>>> with the "unlabeled" label, and therefore under typical policies, >>>>> completely inaccessible to any process in a confined domain. >>>> >>>> The right way to handle this for SELinux would be to automatically use >>>> mountpoint labeling (SECURITY_FS_USE_MNTPOINT, normally set by >>>> specifying a context= mount option), with the sbsec->mntpoint_sid set >>>> from some related object (e.g. the block device file context, as in your >>>> patches for Smack). That will cause SELinux to use that value instead >>>> of any xattr value from the filesystem and will cause attempts by >>>> userspace to set the security.selinux xattr to fail on that filesystem. >>>> That is how SELinux normally deals with untrusted filesystems, except >>>> that it is normally specified as a mount option by a trusted mounting >>>> process, whereas in your case you need to automatically set it. >>> >>> Excellent, thank you for the advice. I'll start on this when I've >>> finished with Smack. >> >> Not tested, but something like this should work. Note that it should >> come after the call to security_fs_use() so we know whether SELinux >> would even try to use xattrs supplied by the filesystem in the first place. >> >> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c >> index 564079c..84da3a2 100644 >> --- a/security/selinux/hooks.c >> +++ b/security/selinux/hooks.c >> @@ -745,6 +745,30 @@ static int selinux_set_mnt_opts(struct super_block *sb, >> goto out; >> } >> } >> + >> + /* >> + * If this is a user namespace mount, no contexts are allowed >> + * on the command line and security labels must be ignored. >> + */ >> + if (sb->s_user_ns != &init_user_ns) { >> + if (context_sid || fscontext_sid || rootcontext_sid || >> + defcontext_sid) { >> + rc = -EACCES; >> + goto out; >> + } >> + if (sbsec->behavior == SECURITY_FS_USE_XATTR) { >> + struct block_device *bdev = sb->s_bdev; >> + sbsec->behavior = SECURITY_FS_USE_MNTPOINT; >> + if (bdev) { >> + struct inode_security_struct *isec = >> bdev->bd_inode; > > That should be bdev->bd_inode->i_security. Sorry, this won't work. bd_inode is not the inode of the block device file that was passed to mount, and it isn't labeled in any way. It will just be unlabeled. So I guess the only real option here as a fallback is sbsec->mntpoint_sid = current_sid(). Which isn't great either, as the only case where we currently assign task labels to files is for their /proc/pid inodes, and no current policy will therefore allow create permission to such files. > >> + sbsec->mntpoint_sid = isec->sid; >> + } else { >> + sbsec->mntpoint_sid = current_sid(); >> + } >> + } >> + goto out_set_opts; >> + } >> + >> /* sets the context of the superblock for the fs being mounted. */ >> if (fscontext_sid) { >> rc = may_context_mount_sb_relabel(fscontext_sid, sbsec, >> cred); >> @@ -813,6 +837,7 @@ static int selinux_set_mnt_opts(struct super_block *sb, >> sbsec->def_sid = defcontext_sid; >> } >> >> +out_set_opts: >> rc = sb_finish_set_opts(sb); >> out: >> mutex_unlock(&sbsec->lock); >> >> _______________________________________________ >> Selinux mailing list >> Selinux@tycho.nsa.gov >> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov. >> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov. >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- 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
On Thu, Jul 23, 2015 at 09:57:20AM -0400, Stephen Smalley wrote: > On 07/22/2015 04:40 PM, Stephen Smalley wrote: > > On 07/22/2015 04:25 PM, Stephen Smalley wrote: > >> On 07/22/2015 12:14 PM, Seth Forshee wrote: > >>> On Wed, Jul 22, 2015 at 12:02:13PM -0400, Stephen Smalley wrote: > >>>> On 07/16/2015 09:23 AM, Stephen Smalley wrote: > >>>>> On 07/15/2015 03:46 PM, Seth Forshee wrote: > >>>>>> Unprivileged users should not be able to supply security labels > >>>>>> in filesystems, nor should they be able to supply security > >>>>>> contexts in unprivileged mounts. For any mount where s_user_ns is > >>>>>> not init_user_ns, force the use of SECURITY_FS_USE_NONE behavior > >>>>>> and return EPERM if any contexts are supplied in the mount > >>>>>> options. > >>>>>> > >>>>>> Signed-off-by: Seth Forshee <seth.forshee@canonical.com> > >>>>> > >>>>> I think this is obsoleted by the subsequent discussion, but just for the > >>>>> record: this patch would cause the files in the userns mount to be left > >>>>> with the "unlabeled" label, and therefore under typical policies, > >>>>> completely inaccessible to any process in a confined domain. > >>>> > >>>> The right way to handle this for SELinux would be to automatically use > >>>> mountpoint labeling (SECURITY_FS_USE_MNTPOINT, normally set by > >>>> specifying a context= mount option), with the sbsec->mntpoint_sid set > >>>> from some related object (e.g. the block device file context, as in your > >>>> patches for Smack). That will cause SELinux to use that value instead > >>>> of any xattr value from the filesystem and will cause attempts by > >>>> userspace to set the security.selinux xattr to fail on that filesystem. > >>>> That is how SELinux normally deals with untrusted filesystems, except > >>>> that it is normally specified as a mount option by a trusted mounting > >>>> process, whereas in your case you need to automatically set it. > >>> > >>> Excellent, thank you for the advice. I'll start on this when I've > >>> finished with Smack. > >> > >> Not tested, but something like this should work. Note that it should > >> come after the call to security_fs_use() so we know whether SELinux > >> would even try to use xattrs supplied by the filesystem in the first place. > >> > >> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > >> index 564079c..84da3a2 100644 > >> --- a/security/selinux/hooks.c > >> +++ b/security/selinux/hooks.c > >> @@ -745,6 +745,30 @@ static int selinux_set_mnt_opts(struct super_block *sb, > >> goto out; > >> } > >> } > >> + > >> + /* > >> + * If this is a user namespace mount, no contexts are allowed > >> + * on the command line and security labels must be ignored. > >> + */ > >> + if (sb->s_user_ns != &init_user_ns) { > >> + if (context_sid || fscontext_sid || rootcontext_sid || > >> + defcontext_sid) { > >> + rc = -EACCES; > >> + goto out; > >> + } > >> + if (sbsec->behavior == SECURITY_FS_USE_XATTR) { > >> + struct block_device *bdev = sb->s_bdev; > >> + sbsec->behavior = SECURITY_FS_USE_MNTPOINT; > >> + if (bdev) { > >> + struct inode_security_struct *isec = > >> bdev->bd_inode; > > > > That should be bdev->bd_inode->i_security. > > Sorry, this won't work. bd_inode is not the inode of the block device > file that was passed to mount, and it isn't labeled in any way. It will > just be unlabeled. > > So I guess the only real option here as a fallback is > sbsec->mntpoint_sid = current_sid(). Which isn't great either, as the > only case where we currently assign task labels to files is for their > /proc/pid inodes, and no current policy will therefore allow create > permission to such files. Darn, you're right, that isn't the inode we want. There really doesn't seem to be any way to get back to the one we want from the LSM, short of adding a new hook. 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
On 07/23/2015 10:39 AM, Seth Forshee wrote: > On Thu, Jul 23, 2015 at 09:57:20AM -0400, Stephen Smalley wrote: >> On 07/22/2015 04:40 PM, Stephen Smalley wrote: >>> On 07/22/2015 04:25 PM, Stephen Smalley wrote: >>>> On 07/22/2015 12:14 PM, Seth Forshee wrote: >>>>> On Wed, Jul 22, 2015 at 12:02:13PM -0400, Stephen Smalley wrote: >>>>>> On 07/16/2015 09:23 AM, Stephen Smalley wrote: >>>>>>> On 07/15/2015 03:46 PM, Seth Forshee wrote: >>>>>>>> Unprivileged users should not be able to supply security labels >>>>>>>> in filesystems, nor should they be able to supply security >>>>>>>> contexts in unprivileged mounts. For any mount where s_user_ns is >>>>>>>> not init_user_ns, force the use of SECURITY_FS_USE_NONE behavior >>>>>>>> and return EPERM if any contexts are supplied in the mount >>>>>>>> options. >>>>>>>> >>>>>>>> Signed-off-by: Seth Forshee <seth.forshee@canonical.com> >>>>>>> >>>>>>> I think this is obsoleted by the subsequent discussion, but just for the >>>>>>> record: this patch would cause the files in the userns mount to be left >>>>>>> with the "unlabeled" label, and therefore under typical policies, >>>>>>> completely inaccessible to any process in a confined domain. >>>>>> >>>>>> The right way to handle this for SELinux would be to automatically use >>>>>> mountpoint labeling (SECURITY_FS_USE_MNTPOINT, normally set by >>>>>> specifying a context= mount option), with the sbsec->mntpoint_sid set >>>>>> from some related object (e.g. the block device file context, as in your >>>>>> patches for Smack). That will cause SELinux to use that value instead >>>>>> of any xattr value from the filesystem and will cause attempts by >>>>>> userspace to set the security.selinux xattr to fail on that filesystem. >>>>>> That is how SELinux normally deals with untrusted filesystems, except >>>>>> that it is normally specified as a mount option by a trusted mounting >>>>>> process, whereas in your case you need to automatically set it. >>>>> >>>>> Excellent, thank you for the advice. I'll start on this when I've >>>>> finished with Smack. >>>> >>>> Not tested, but something like this should work. Note that it should >>>> come after the call to security_fs_use() so we know whether SELinux >>>> would even try to use xattrs supplied by the filesystem in the first place. >>>> >>>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c >>>> index 564079c..84da3a2 100644 >>>> --- a/security/selinux/hooks.c >>>> +++ b/security/selinux/hooks.c >>>> @@ -745,6 +745,30 @@ static int selinux_set_mnt_opts(struct super_block *sb, >>>> goto out; >>>> } >>>> } >>>> + >>>> + /* >>>> + * If this is a user namespace mount, no contexts are allowed >>>> + * on the command line and security labels must be ignored. >>>> + */ >>>> + if (sb->s_user_ns != &init_user_ns) { >>>> + if (context_sid || fscontext_sid || rootcontext_sid || >>>> + defcontext_sid) { >>>> + rc = -EACCES; >>>> + goto out; >>>> + } >>>> + if (sbsec->behavior == SECURITY_FS_USE_XATTR) { >>>> + struct block_device *bdev = sb->s_bdev; >>>> + sbsec->behavior = SECURITY_FS_USE_MNTPOINT; >>>> + if (bdev) { >>>> + struct inode_security_struct *isec = >>>> bdev->bd_inode; >>> >>> That should be bdev->bd_inode->i_security. >> >> Sorry, this won't work. bd_inode is not the inode of the block device >> file that was passed to mount, and it isn't labeled in any way. It will >> just be unlabeled. >> >> So I guess the only real option here as a fallback is >> sbsec->mntpoint_sid = current_sid(). Which isn't great either, as the >> only case where we currently assign task labels to files is for their >> /proc/pid inodes, and no current policy will therefore allow create >> permission to such files. > > Darn, you're right, that isn't the inode we want. There really doesn't > seem to be any way to get back to the one we want from the LSM, short of > adding a new hook. Maybe list_first_entry(&sb->s_bdev->bd_inodes, struct inode, i_devices)? Feels like a layering violation though... -- 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
On Thu, Jul 23, 2015 at 11:36:03AM -0400, Stephen Smalley wrote: > On 07/23/2015 10:39 AM, Seth Forshee wrote: > > On Thu, Jul 23, 2015 at 09:57:20AM -0400, Stephen Smalley wrote: > >> On 07/22/2015 04:40 PM, Stephen Smalley wrote: > >>> On 07/22/2015 04:25 PM, Stephen Smalley wrote: > >>>> On 07/22/2015 12:14 PM, Seth Forshee wrote: > >>>>> On Wed, Jul 22, 2015 at 12:02:13PM -0400, Stephen Smalley wrote: > >>>>>> On 07/16/2015 09:23 AM, Stephen Smalley wrote: > >>>>>>> On 07/15/2015 03:46 PM, Seth Forshee wrote: > >>>>>>>> Unprivileged users should not be able to supply security labels > >>>>>>>> in filesystems, nor should they be able to supply security > >>>>>>>> contexts in unprivileged mounts. For any mount where s_user_ns is > >>>>>>>> not init_user_ns, force the use of SECURITY_FS_USE_NONE behavior > >>>>>>>> and return EPERM if any contexts are supplied in the mount > >>>>>>>> options. > >>>>>>>> > >>>>>>>> Signed-off-by: Seth Forshee <seth.forshee@canonical.com> > >>>>>>> > >>>>>>> I think this is obsoleted by the subsequent discussion, but just for the > >>>>>>> record: this patch would cause the files in the userns mount to be left > >>>>>>> with the "unlabeled" label, and therefore under typical policies, > >>>>>>> completely inaccessible to any process in a confined domain. > >>>>>> > >>>>>> The right way to handle this for SELinux would be to automatically use > >>>>>> mountpoint labeling (SECURITY_FS_USE_MNTPOINT, normally set by > >>>>>> specifying a context= mount option), with the sbsec->mntpoint_sid set > >>>>>> from some related object (e.g. the block device file context, as in your > >>>>>> patches for Smack). That will cause SELinux to use that value instead > >>>>>> of any xattr value from the filesystem and will cause attempts by > >>>>>> userspace to set the security.selinux xattr to fail on that filesystem. > >>>>>> That is how SELinux normally deals with untrusted filesystems, except > >>>>>> that it is normally specified as a mount option by a trusted mounting > >>>>>> process, whereas in your case you need to automatically set it. > >>>>> > >>>>> Excellent, thank you for the advice. I'll start on this when I've > >>>>> finished with Smack. > >>>> > >>>> Not tested, but something like this should work. Note that it should > >>>> come after the call to security_fs_use() so we know whether SELinux > >>>> would even try to use xattrs supplied by the filesystem in the first place. > >>>> > >>>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > >>>> index 564079c..84da3a2 100644 > >>>> --- a/security/selinux/hooks.c > >>>> +++ b/security/selinux/hooks.c > >>>> @@ -745,6 +745,30 @@ static int selinux_set_mnt_opts(struct super_block *sb, > >>>> goto out; > >>>> } > >>>> } > >>>> + > >>>> + /* > >>>> + * If this is a user namespace mount, no contexts are allowed > >>>> + * on the command line and security labels must be ignored. > >>>> + */ > >>>> + if (sb->s_user_ns != &init_user_ns) { > >>>> + if (context_sid || fscontext_sid || rootcontext_sid || > >>>> + defcontext_sid) { > >>>> + rc = -EACCES; > >>>> + goto out; > >>>> + } > >>>> + if (sbsec->behavior == SECURITY_FS_USE_XATTR) { > >>>> + struct block_device *bdev = sb->s_bdev; > >>>> + sbsec->behavior = SECURITY_FS_USE_MNTPOINT; > >>>> + if (bdev) { > >>>> + struct inode_security_struct *isec = > >>>> bdev->bd_inode; > >>> > >>> That should be bdev->bd_inode->i_security. > >> > >> Sorry, this won't work. bd_inode is not the inode of the block device > >> file that was passed to mount, and it isn't labeled in any way. It will > >> just be unlabeled. > >> > >> So I guess the only real option here as a fallback is > >> sbsec->mntpoint_sid = current_sid(). Which isn't great either, as the > >> only case where we currently assign task labels to files is for their > >> /proc/pid inodes, and no current policy will therefore allow create > >> permission to such files. > > > > Darn, you're right, that isn't the inode we want. There really doesn't > > seem to be any way to get back to the one we want from the LSM, short of > > adding a new hook. > > Maybe list_first_entry(&sb->s_bdev->bd_inodes, struct inode, i_devices)? > Feels like a layering violation though... Yeah, and even though that probably works out to be the inode we want in most cases I don't think we can be absolutely certain that it is. Maybe there's some way we could walk the list and be sure we've found the right inode, but I'm not seeing it. -- 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/security/selinux/hooks.c b/security/selinux/hooks.c index 564079c..84da3a2 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -745,6 +745,30 @@ static int selinux_set_mnt_opts(struct super_block *sb, goto out; } } + + /* + * If this is a user namespace mount, no contexts are allowed + * on the command line and security labels must be ignored. + */ + if (sb->s_user_ns != &init_user_ns) { + if (context_sid || fscontext_sid || rootcontext_sid || + defcontext_sid) { + rc = -EACCES; + goto out; + } + if (sbsec->behavior == SECURITY_FS_USE_XATTR) { + struct block_device *bdev = sb->s_bdev; + sbsec->behavior = SECURITY_FS_USE_MNTPOINT; + if (bdev) { + struct inode_security_struct *isec = bdev->bd_inode; + sbsec->mntpoint_sid = isec->sid; + } else { + sbsec->mntpoint_sid = current_sid(); + } + } + goto out_set_opts; + } + /* sets the context of the superblock for the fs being mounted. */ if (fscontext_sid) {