linux-kernel - Re: [PATCH v2 2/3] Teach SELinux about anonymous inodes

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: <b5999b89-6921-5667-9eb2-662b14d5f730@tycho.nsa.gov>
Date:   Thu, 26 Mar 2020 09:58:05 -0400
From:   Stephen Smalley <sds@...ho.nsa.gov>
To:     Daniel Colascione <dancol@...gle.com>, timmurray@...gle.com,
        selinux@...r.kernel.org, linux-security-module@...r.kernel.org,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        kvm@...r.kernel.org, viro@...iv.linux.org.uk, paul@...l-moore.com,
        nnk@...gle.com, lokeshgidra@...gle.com
Subject: Re: [PATCH v2 2/3] Teach SELinux about anonymous inodes

On 3/25/20 7:02 PM, Daniel Colascione wrote:
> This change uses the anon_inodes and LSM infrastructure introduced in
> the previous patch to give SELinux the ability to control
> anonymous-inode files that are created using the new _secure()
> anon_inodes functions.
> 
> A SELinux policy author detects and controls these anonymous inodes by
> adding a name-based type_transition rule that assigns a new security
> type to anonymous-inode files created in some domain. The name used
> for the name-based transition is the name associated with the
> anonymous inode for file listings --- e.g., "[userfaultfd]" or
> "[perf_event]".
> 
> Example:
> 
> type uffd_t;
> type_transition sysadm_t sysadm_t : file uffd_t "[userfaultfd]";
> allow sysadm_t uffd_t:file { create };
> 
> (The next patch in this series is necessary for making userfaultfd
> support this new interface.  The example above is just
> for exposition.)
> 
> Signed-off-by: Daniel Colascione <dancol@...gle.com>
> ---
>   security/selinux/hooks.c            | 54 +++++++++++++++++++++++++++++
>   security/selinux/include/classmap.h |  2 ++
>   2 files changed, 56 insertions(+)
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 1659b59fb5d7..b9eb45c2e4e5 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2915,6 +2915,59 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
>   	return 0;
>   }
>   
> +static int selinux_inode_init_security_anon(struct inode *inode,
> +					    const struct qstr *name,
> +					    const struct file_operations *fops,
> +					    const struct inode *context_inode)
> +{
> +	const struct task_security_struct *tsec = selinux_cred(current_cred());
> +	struct common_audit_data ad;
> +	struct inode_security_struct *isec;
> +	int rc;
> +
> +	if (unlikely(!selinux_state.initialized))
> +		return 0;

This leaves secure anon inodes created before first policy load with the 
unlabeled SID rather than defaulting to the SID of the creating task 
(kernel SID in that situation).  Is that what you want?  Alternatively 
you can just remove this test and let it proceed; nothing should be 
break and the anon inodes will get the kernel SID.

> +
> +	isec = selinux_inode(inode);
> +
> +	/*
> +	 * We only get here once per ephemeral inode.  The inode has
> +	 * been initialized via inode_alloc_security but is otherwise
> +	 * untouched.
> +	 */
> +
> +	if (context_inode) {
> +		struct inode_security_struct *context_isec =
> +			selinux_inode(context_inode);
> +		isec->sclass = context_isec->sclass;
> +		isec->sid = context_isec->sid;
> +	} else {
> +		isec->sclass = SECCLASS_ANON_INODE;
> +		rc = security_transition_sid(
> +			&selinux_state, tsec->sid, tsec->sid,
> +			SECCLASS_FILE, name, &isec->sid);
> +		if (rc)
> +			return rc;
> +	}
> +
> +	isec->initialized = LABEL_INITIALIZED;
> +
> +	/*
> +	 * Now that we've initialized security, check whether we're
> +	 * allowed to actually create this type of anonymous inode.
> +	 */
> +
> +	ad.type = LSM_AUDIT_DATA_INODE;
> +	ad.u.inode = inode;
> +
> +	return avc_has_perm(&selinux_state,
> +			    tsec->sid,
> +			    isec->sid,
> +			    isec->sclass,
> +			    FILE__CREATE,
> +			    &ad);
> +}
> +
>   static int selinux_inode_create(struct inode *dir, struct dentry *dentry, umode_t mode)
>   {
>   	return may_create(dir, dentry, SECCLASS_FILE);
> @@ -6923,6 +6976,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
>   
>   	LSM_HOOK_INIT(inode_free_security, selinux_inode_free_security),
>   	LSM_HOOK_INIT(inode_init_security, selinux_inode_init_security),
> +	LSM_HOOK_INIT(inode_init_security_anon, selinux_inode_init_security_anon),
>   	LSM_HOOK_INIT(inode_create, selinux_inode_create),
>   	LSM_HOOK_INIT(inode_link, selinux_inode_link),
>   	LSM_HOOK_INIT(inode_unlink, selinux_inode_unlink),
> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
> index 986f3ac14282..263750b6aaac 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -248,6 +248,8 @@ struct security_class_mapping secclass_map[] = {
>   	  {"open", "cpu", "kernel", "tracepoint", "read", "write"} },
>   	{ "lockdown",
>   	  { "integrity", "confidentiality", NULL } },
> +	{ "anon_inode",
> +	  { COMMON_FILE_PERMS, NULL } },
>   	{ NULL }
>     };
>   
>