linux-kernel - Re: [PATCH v3 15/15] selinux: delay sid population for rootfs till init is complete

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <1519152994.14218.15.camel@tycho.nsa.gov>
Date:   Tue, 20 Feb 2018 13:56:34 -0500
From:   Stephen Smalley <sds@...ho.nsa.gov>
To:     Taras Kondratiuk <takondra@...co.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        Al Viro <viro@...iv.linux.org.uk>,
        Arnd Bergmann <arnd@...db.de>, Rob Landley <rob@...dley.net>,
        Mimi Zohar <zohar@...ux.vnet.ibm.com>,
        Jonathan Corbet <corbet@....net>,
        James McMechan <james.w.mcmechan@...il.com>
Cc:     initramfs@...r.kernel.org, Victor Kamensky <kamensky@...co.com>,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-security-module@...r.kernel.org, xe-linux-external@...co.com,
        Paul Moore <paul@...l-moore.com>,
        Eric Paris <eparis@...isplace.org>
Subject: Re: [PATCH v3 15/15] selinux: delay sid population for rootfs till
 init is complete

On Fri, 2018-02-16 at 20:33 +0000, Taras Kondratiuk wrote:
> From: Victor Kamensky <kamensky@...co.com>
> 
> With initramfs cpio format that supports extended attributes
> we need to skip sid population on sys_lsetxattr call from
> initramfs for rootfs if security server is not initialized yet.
> 
> Otherwise callback in selinux_inode_post_setxattr will try to
> translate give security.selinux label into sid context and since
> security server is not available yet inode will receive default
> sid (typically kernel_t). Note that in the same time proper
> label will be stored in inode xattrs. Later, since inode sid
> would be already populated system will never look back at
> actual xattrs. But if we skip sid population for rootfs and
> we have policy that direct use of xattrs for rootfs, proper
> sid will be filled in from extended attributes one node is
> accessed and server is initialized.
> 
> Note new DELAYAFTERINIT_MNT super block flag is introduced
> to only mark rootfs for such behavior. For other types of
> tmpfs original logic is still used.

(cc selinux maintainers)

Wondering if we shouldn't just do this always, for all filesystem
types.  Also, I think this should likely also be done in
selinux_inode_setsecurity() for consistency.

> 
> Signed-off-by: Victor Kamensky <kamensky@...co.com>
> ---
>  security/selinux/hooks.c            | 9 ++++++++-
>  security/selinux/include/security.h | 1 +
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index f3fe65589f02..bb25268f734e 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -716,7 +716,7 @@ static int selinux_set_mnt_opts(struct
> super_block *sb,
>  			 */
>  			if (!strncmp(sb->s_type->name, "rootfs",
>  				     sizeof("rootfs")))
> -				sbsec->flags |= SBLABEL_MNT;
> +				sbsec->flags |=
> SBLABEL_MNT|DELAYAFTERINIT_MNT;
>  
>  			/* Defer initialization until
> selinux_complete_init,
>  			   after the initial policy is loaded and
> the security
> @@ -3253,6 +3253,7 @@ static void selinux_inode_post_setxattr(struct
> dentry *dentry, const char *name,
>  {
>  	struct inode *inode = d_backing_inode(dentry);
>  	struct inode_security_struct *isec;
> +	struct superblock_security_struct *sbsec;
>  	u32 newsid;
>  	int rc;
>  
> @@ -3261,6 +3262,12 @@ static void selinux_inode_post_setxattr(struct
> dentry *dentry, const char *name,
>  		return;
>  	}
>  
> +	if (!ss_initialized) {
> +		sbsec = inode->i_sb->s_security;
> +		if (sbsec->flags & DELAYAFTERINIT_MNT)
> +			return;
> +	}
> +
>  	rc = security_context_to_sid_force(value, size, &newsid);
>  	if (rc) {
>  		printk(KERN_ERR "SELinux:  unable to map context to
> SID"
> diff --git a/security/selinux/include/security.h
> b/security/selinux/include/security.h
> index 02f0412d42f2..585acfd6cbcf 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -52,6 +52,7 @@
>  #define ROOTCONTEXT_MNT	0x04
>  #define DEFCONTEXT_MNT	0x08
>  #define SBLABEL_MNT	0x10
> +#define DELAYAFTERINIT_MNT 0x20
>  /* Non-mount related flags */
>  #define SE_SBINITIALIZED	0x0100
>  #define SE_SBPROC		0x0200