linux-kernel - Re: [PATCH 6/7] selinux: Ignore security labels on user namespace mounts

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 23 Jul 2015 11:36:03 -0400
From:	Stephen Smalley <sds@...ho.nsa.gov>
To:	Seth Forshee <seth.forshee@...onical.com>
CC:	Serge Hallyn <serge.hallyn@...onical.com>, selinux@...ho.nsa.gov,
	linux-kernel@...r.kernel.org,
	Andy Lutomirski <luto@...capital.net>,
	linux-security-module@...r.kernel.org,
	Alexander Viro <viro@...iv.linux.org.uk>,
	James Morris <james.l.morris@...cle.com>,
	linux-fsdevel@...r.kernel.org,
	Casey Schaufler <casey@...aufler-ca.com>
Subject: Re: [PATCH 6/7] selinux: Ignore security labels on user namespace
 mounts

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@...onical.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-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/