Linux-Security-Module Archive on lore.kernel.org
 help / color / Atom feed
From: Stephen Smalley <sds@tycho.nsa.gov>
To: Casey Schaufler <casey@schaufler-ca.com>,
	casey.schaufler@intel.com, jmorris@namei.org,
	linux-security-module@vger.kernel.org, selinux@vger.kernel.org
Cc: keescook@chromium.org, john.johansen@canonical.com,
	penguin-kernel@i-love.sakura.ne.jp, paul@paul-moore.com
Subject: Re: [PATCH v8 22/28] SELinux: Verify LSM display sanity in binder
Date: Wed, 18 Sep 2019 13:43:48 -0400
Message-ID: <972d4dfd-74fd-0747-8c2d-ad74842ed067@tycho.nsa.gov> (raw)
In-Reply-To: <20190829232935.7099-23-casey@schaufler-ca.com>

On 8/29/19 7:29 PM, Casey Schaufler wrote:
> Verify that the tasks on the ends of a binder transaction
> use LSM display values that don't cause SELinux contexts
> to be interpreted by another LSM or another LSM's context
> to be interpreted by SELinux. No judgement is made in cases
> that where SELinux contexts are not used in the binder
> transaction.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
>   include/linux/security.h |  1 +
>   security/selinux/hooks.c | 34 ++++++++++++++++++++++++++++++++++
>   2 files changed, 35 insertions(+)
> 
> diff --git a/include/linux/security.h b/include/linux/security.h
> index c16aea55be97..7fcc94ec8e04 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -125,6 +125,7 @@ struct lsmblob {
>   	u32     secid[LSMBLOB_ENTRIES];
>   };
>   
> +#define LSMBLOB_FIRST		0	/* First valid LSM slot number */
>   #define LSMBLOB_INVALID		-1	/* Not a valid LSM slot number */
>   #define LSMBLOB_NEEDED		-2	/* Slot requested on initialization */
>   #define LSMBLOB_NOT_NEEDED	-3	/* Slot not requested */
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 352be16a887d..2844f2ab7706 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2009,6 +2009,28 @@ static inline u32 open_file_to_av(struct file *file)
>   	return av;
>   }
>   
> +/*
> + * Verify that if the "display" LSM is SELinux for either task
> + * that it is for both tasks.
> + */
> +static inline bool compatible_task_displays(struct task_struct *here,
> +					    struct task_struct *there)
> +{
> +	int h = lsm_task_display(here);
> +	int t = lsm_task_display(there);
> +
> +	if (h == t)
> +		return true;
> +
> +	/* unspecified is only ok if SELinux isn't going to be involved */
> +	if (selinux_lsmid.slot == LSMBLOB_FIRST)
> +		return ((h == LSMBLOB_FIRST && t == LSMBLOB_INVALID) ||
> +			(t == LSMBLOB_FIRST && h == LSMBLOB_INVALID));
> +
> +	/* it's ok only if neither display is SELinux */
> +	return (h != selinux_lsmid.slot && t != selinux_lsmid.slot);
> +}
> +
>   /* Hook functions begin here. */
>   
>   static int selinux_binder_set_context_mgr(struct task_struct *mgr)
> @@ -2016,6 +2038,9 @@ static int selinux_binder_set_context_mgr(struct task_struct *mgr)
>   	u32 mysid = current_sid();
>   	u32 mgrsid = task_sid(mgr);
>   
> +	if (!compatible_task_displays(current, mgr))
> +		return -EINVAL;
> +
>   	return avc_has_perm(&selinux_state,
>   			    mysid, mgrsid, SECCLASS_BINDER,
>   			    BINDER__SET_CONTEXT_MGR, NULL);
> @@ -2029,6 +2054,9 @@ static int selinux_binder_transaction(struct task_struct *from,
>   	u32 tosid = task_sid(to);
>   	int rc;
>   
> +	if (!compatible_task_displays(from, to))
> +		return -EINVAL;
> +
>   	if (mysid != fromsid) {
>   		rc = avc_has_perm(&selinux_state,
>   				  mysid, fromsid, SECCLASS_BINDER,
> @@ -2048,6 +2076,9 @@ static int selinux_binder_transfer_binder(struct task_struct *from,
>   	u32 fromsid = task_sid(from);
>   	u32 tosid = task_sid(to);
>   
> +	if (!compatible_task_displays(from, to))
> +		return -EINVAL;
> +
>   	return avc_has_perm(&selinux_state,
>   			    fromsid, tosid, SECCLASS_BINDER, BINDER__TRANSFER,
>   			    NULL);
> @@ -2064,6 +2095,9 @@ static int selinux_binder_transfer_file(struct task_struct *from,
>   	struct common_audit_data ad;
>   	int rc;
>   
> +	if (!compatible_task_displays(from, to))
> +		return -EINVAL;
> +
>   	ad.type = LSM_AUDIT_DATA_PATH;
>   	ad.u.path = file->f_path;
>   
> 

The only hook that approximates where/when you want to do this check is 
security_binder_transaction(), so you don't need to insert the check 
into the other hooks.  Further, the check is only needed if the server 
has requested peer contexts, which is indicated by the flag bit 
target_node->txn_security_ctx, so if you amended the hook interface to 
also pass that flag value, then you could avoid the check except in that 
situation.

As soon as any other LSM implements the binder hooks, it will also 
likely want an equivalent check, so sooner or later you'll probably want 
to take the logic to the framework and not just the security module.


  reply index

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-29 23:29 [PATCH v8 00/28] LSM: Module stacking for AppArmor Casey Schaufler
2019-08-29 23:29 ` [PATCH v8 01/28] LSM: Infrastructure management of the superblock Casey Schaufler
2019-09-16 18:19   ` Stephen Smalley
2019-08-29 23:29 ` [PATCH v8 02/28] LSM: Infrastructure management of the sock security Casey Schaufler
2019-09-16 18:42   ` Stephen Smalley
2019-09-18  7:19     ` John Johansen
2019-08-29 23:29 ` [PATCH v8 03/28] LSM: Infrastructure management of the key blob Casey Schaufler
2019-09-16 18:47   ` Stephen Smalley
2019-08-29 23:29 ` [PATCH v8 04/28] LSM: Create and manage the lsmblob data structure Casey Schaufler
2019-09-16 19:15   ` Stephen Smalley
2019-09-23 15:56     ` Casey Schaufler
2019-08-29 23:29 ` [PATCH v8 05/28] LSM: Use lsmblob in security_audit_rule_match Casey Schaufler
2019-08-29 23:29 ` [PATCH v8 06/28] LSM: Use lsmblob in security_kernel_act_as Casey Schaufler
2019-08-29 23:29 ` [PATCH v8 07/28] net: Prepare UDS for security module stacking Casey Schaufler
2019-08-29 23:29 ` [PATCH v8 08/28] LSM: Use lsmblob in security_secctx_to_secid Casey Schaufler
2019-08-29 23:29 ` [PATCH v8 09/28] LSM: Use lsmblob in security_secid_to_secctx Casey Schaufler
2019-08-29 23:29 ` [PATCH v8 10/28] LSM: Use lsmblob in security_ipc_getsecid Casey Schaufler
2019-08-29 23:29 ` [PATCH v8 11/28] LSM: Use lsmblob in security_task_getsecid Casey Schaufler
2019-08-29 23:29 ` [PATCH v8 12/28] LSM: Use lsmblob in security_inode_getsecid Casey Schaufler
2019-08-29 23:29 ` [PATCH v8 13/28] LSM: Use lsmblob in security_cred_getsecid Casey Schaufler
2019-08-29 23:29 ` [PATCH v8 14/28] IMA: Change internal interfaces to use lsmblobs Casey Schaufler
2019-08-29 23:29 ` [PATCH v8 15/28] LSM: Specify which LSM to display Casey Schaufler
2019-08-29 23:29 ` [PATCH v8 16/28] LSM: Ensure the correct LSM context releaser Casey Schaufler
2019-08-29 23:29 ` [PATCH v8 17/28] LSM: Use lsmcontext in security_secid_to_secctx Casey Schaufler
2019-08-29 23:29 ` [PATCH v8 18/28] LSM: Use lsmcontext in security_dentry_init_security Casey Schaufler
2019-08-29 23:29 ` [PATCH v8 19/28] LSM: Use lsmcontext in security_inode_getsecctx Casey Schaufler
2019-08-29 23:29 ` [PATCH v8 20/28] LSM: security_secid_to_secctx in netlink netfilter Casey Schaufler
2019-08-29 23:29 ` [PATCH v8 21/28] NET: Store LSM netlabel data in a lsmblob Casey Schaufler
2019-08-29 23:29 ` [PATCH v8 22/28] SELinux: Verify LSM display sanity in binder Casey Schaufler
2019-09-18 17:43   ` Stephen Smalley [this message]
2019-08-29 23:29 ` [PATCH v8 23/28] Audit: Add subj_LSM fields when necessary Casey Schaufler
2019-08-29 23:29 ` [PATCH v8 24/28] Audit: Include object data for all security modules Casey Schaufler
2019-08-29 23:29 ` [PATCH v8 25/28] LSM: Provide an user space interface for the default display Casey Schaufler
2019-08-29 23:29 ` [PATCH v8 26/28] NET: Add SO_PEERCONTEXT for multiple LSMs Casey Schaufler
2019-08-29 23:29 ` [PATCH v8 27/28] LSM: Add /proc attr entry for full LSM context Casey Schaufler
2019-08-29 23:29 ` [PATCH v8 28/28] AppArmor: Remove the exclusive flag Casey Schaufler
2019-09-04 19:13 ` [PATCH v8 00/28] LSM: Module stacking for AppArmor Casey Schaufler
2019-09-06 13:46   ` Stephen Smalley

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=972d4dfd-74fd-0747-8c2d-ad74842ed067@tycho.nsa.gov \
    --to=sds@tycho.nsa.gov \
    --cc=casey.schaufler@intel.com \
    --cc=casey@schaufler-ca.com \
    --cc=jmorris@namei.org \
    --cc=john.johansen@canonical.com \
    --cc=keescook@chromium.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=paul@paul-moore.com \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --cc=selinux@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Security-Module Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-security-module/0 linux-security-module/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-security-module linux-security-module/ https://lore.kernel.org/linux-security-module \
		linux-security-module@vger.kernel.org
	public-inbox-index linux-security-module

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-security-module


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git