[v4,15/23] LSM: Specify which LSM to display
diff mbox series

Message ID 20190626192234.11725-16-casey@schaufler-ca.com
State Superseded
Headers show
Series
  • LSM: Module stacking for AppArmor
Related show

Commit Message

Casey Schaufler June 26, 2019, 7:22 p.m. UTC
Create a new entry "display" in /proc/.../attr for controlling
which LSM security information is displayed for a process.
The name of an active LSM that supplies hooks for human readable
data may be written to "display" to set the value. The name of
the LSM currently in use can be read from "display".
At this point there can only be one LSM capable of display
active.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 fs/proc/base.c      |   1 +
 security/security.c | 129 ++++++++++++++++++++++++++++++++++++++------
 2 files changed, 113 insertions(+), 17 deletions(-)

Comments

Kees Cook June 26, 2019, 11:12 p.m. UTC | #1
On Wed, Jun 26, 2019 at 12:22:26PM -0700, Casey Schaufler wrote:
> Create a new entry "display" in /proc/.../attr for controlling
> which LSM security information is displayed for a process.
> The name of an active LSM that supplies hooks for human readable
> data may be written to "display" to set the value. The name of
> the LSM currently in use can be read from "display".
> At this point there can only be one LSM capable of display
> active.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  fs/proc/base.c      |   1 +
>  security/security.c | 129 ++++++++++++++++++++++++++++++++++++++------
>  2 files changed, 113 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index ddef482f1334..7bf70e041315 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -2618,6 +2618,7 @@ static const struct pid_entry attr_dir_stuff[] = {
>  	ATTR(NULL, "fscreate",		0666),
>  	ATTR(NULL, "keycreate",		0666),
>  	ATTR(NULL, "sockcreate",	0666),
> +	ATTR(NULL, "display",		0666),
>  #ifdef CONFIG_SECURITY_SMACK
>  	DIR("smack",			0555,
>  	    proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops),
> diff --git a/security/security.c b/security/security.c
> index 3180a6f30625..82e29c477fa4 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -46,7 +46,9 @@ static struct kmem_cache *lsm_file_cache;
>  static struct kmem_cache *lsm_inode_cache;
>  
>  char *lsm_names;
> -static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init;
> +static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init = {
> +	.lbs_task = sizeof(int),	/* slot number for the "display" LSM */
> +};
>  
>  /* Boot-time LSM user choice */
>  static __initdata const char *chosen_lsm_order;
> @@ -423,8 +425,10 @@ static int lsm_append(const char *new, char **result)
>  
>  /*
>   * Current index to use while initializing the lsmblob secid list.
> + * Pointers to the LSM id structures for local use.
>   */
>  static int lsm_slot;
> +static struct lsm_id *lsm_slotlist[LSMBLOB_ENTRIES];
>  
>  /**
>   * security_add_hooks - Add a modules hooks to the hook lists.
> @@ -444,6 +448,7 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
>  	if (lsmid->slot == LSMBLOB_NEEDED) {
>  		if (lsm_slot >= LSMBLOB_ENTRIES)
>  			panic("%s Too many LSMs registered.\n", __func__);
> +		lsm_slotlist[lsm_slot] = lsmid;
>  		lsmid->slot = lsm_slot++;
>  		init_debug("%s assigned lsmblob slot %d\n", lsmid->lsm,
>  			   lsmid->slot);
> @@ -564,6 +569,8 @@ int lsm_inode_alloc(struct inode *inode)
>   */
>  static int lsm_task_alloc(struct task_struct *task)
>  {
> +	int *display;
> +
>  	if (blob_sizes.lbs_task == 0) {
>  		task->security = NULL;
>  		return 0;
> @@ -572,6 +579,15 @@ static int lsm_task_alloc(struct task_struct *task)
>  	task->security = kzalloc(blob_sizes.lbs_task, GFP_KERNEL);
>  	if (task->security == NULL)
>  		return -ENOMEM;
> +
> +	/*
> +	 * The start of the task blob contains the "display" LSM slot number.
> +	 * Start with it set to the invalid slot number, indicating that the
> +	 * default first registered LSM be displayed.
> +	 */
> +	display = task->security;
> +	*display = LSMBLOB_INVALID;
> +
>  	return 0;
>  }
>  
> @@ -1563,14 +1579,24 @@ int security_file_open(struct file *file)
>  
>  int security_task_alloc(struct task_struct *task, unsigned long clone_flags)
>  {
> +	int *odisplay = current->security;
> +	int *ndisplay;
>  	int rc = lsm_task_alloc(task);
>  
> -	if (rc)
> +	if (unlikely(rc))
>  		return rc;
> +
>  	rc = call_int_hook(task_alloc, 0, task, clone_flags);
> -	if (unlikely(rc))
> +	if (unlikely(rc)) {
>  		security_task_free(task);
> -	return rc;
> +		return rc;
> +	}
> +
> +	ndisplay = task->security;
> +	if (ndisplay && odisplay)
> +		*ndisplay = *odisplay;
> +
> +	return 0;
>  }
>  
>  void security_task_free(struct task_struct *task)
> @@ -1967,10 +1993,29 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
>  				char **value)
>  {
>  	struct security_hook_list *hp;
> +	int *display = current->security;
> +	int slot = 0;
> +
> +	if (!strcmp(name, "display")) {
> +		/*
> +		 * lsm_slot will be 0 if there are no displaying modules.
> +		 */
> +		if (lsm_slot == 0)
> +			return -EINVAL;
> +		if (*display != LSMBLOB_INVALID)
> +			slot = *display;
> +		*value = kstrdup(lsm_slotlist[slot]->lsm, GFP_KERNEL);
> +		if (*value)
> +			return strlen(*value);
> +		return -ENOMEM;
> +	}
>  
>  	hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) {
>  		if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
>  			continue;
> +		if (lsm == NULL && *display != LSMBLOB_INVALID &&
> +		    *display != hp->lsmid->slot)
> +			continue;
>  		return hp->hook.getprocattr(p, name, value);
>  	}
>  	return -EINVAL;
> @@ -1980,10 +2025,46 @@ int security_setprocattr(const char *lsm, const char *name, void *value,
>  			 size_t size)
>  {
>  	struct security_hook_list *hp;
> +	char *term;
> +	char *cp;
> +	int *display = current->security;
> +	int rc = -EINVAL;
> +	int slot = 0;
> +
> +	if (!strcmp(name, "display")) {
> +		/*
> +		 * lsm_slot will be 0 if there are no displaying modules.
> +		 */
> +		if (lsm_slot == 0 || size == 0)
> +			return -EINVAL;
> +		cp = kzalloc(size + 1, GFP_KERNEL);
> +		if (cp == NULL)
> +			return -ENOMEM;
> +		memcpy(cp, value, size);
> +
> +		term = strchr(cp, ' ');
> +		if (term == NULL)
> +			term = strchr(cp, '\n');
> +		if (term != NULL)
> +			*term = '\0';
> +
> +		for (slot = 0; slot < lsm_slot; slot++)
> +			if (!strcmp(cp, lsm_slotlist[slot]->lsm)) {
> +				*display = lsm_slotlist[slot]->slot;
> +				rc = size;
> +				break;
> +			}
> +
> +		kfree(cp);
> +		return rc;
> +	}
>  
>  	hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) {
>  		if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
>  			continue;
> +		if (lsm == NULL && *display != LSMBLOB_INVALID &&
> +		    *display != hp->lsmid->slot)
> +			continue;
>  		return hp->hook.setprocattr(name, value, size);
>  	}
>  	return -EINVAL;
> @@ -2003,15 +2084,15 @@ EXPORT_SYMBOL(security_ismaclabel);
>  int security_secid_to_secctx(struct lsmblob *blob, char **secdata, u32 *seclen)
>  {
>  	struct security_hook_list *hp;
> -	int rc;
> +	int *display = current->security;
>  
>  	hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) {
>  		if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
>  			continue;
> -		rc = hp->hook.secid_to_secctx(blob->secid[hp->lsmid->slot],
> -					      secdata, seclen);
> -		if (rc != 0)
> -			return rc;
> +		if (*display == LSMBLOB_INVALID || *display == hp->lsmid->slot)
> +			return hp->hook.secid_to_secctx(
> +					blob->secid[hp->lsmid->slot],
> +					secdata, seclen);
>  	}
>  	return 0;
>  }
> @@ -2021,16 +2102,15 @@ int security_secctx_to_secid(const char *secdata, u32 seclen,
>  			     struct lsmblob *blob)
>  {
>  	struct security_hook_list *hp;
> -	int rc;
> +	int *display = current->security;
>  
>  	lsmblob_init(blob, 0);
>  	hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid, list) {
>  		if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
>  			continue;
> -		rc = hp->hook.secctx_to_secid(secdata, seclen,
> -					      &blob->secid[hp->lsmid->slot]);
> -		if (rc != 0)
> -			return rc;
> +		if (*display == LSMBLOB_INVALID || *display == hp->lsmid->slot)
> +			return hp->hook.secctx_to_secid(secdata, seclen,
> +						&blob->secid[hp->lsmid->slot]);
>  	}
>  	return 0;
>  }
> @@ -2038,7 +2118,15 @@ EXPORT_SYMBOL(security_secctx_to_secid);
>  
>  void security_release_secctx(char *secdata, u32 seclen)
>  {
> -	call_void_hook(release_secctx, secdata, seclen);
> +	struct security_hook_list *hp;
> +	int *display = current->security;
> +
> +	hlist_for_each_entry(hp, &security_hook_heads.release_secctx, list)
> +		if (*display == LSMBLOB_INVALID ||
> +		    *display == hp->lsmid->slot) {
> +			hp->hook.release_secctx(secdata, seclen);
> +			return;
> +		}
>  }
>  EXPORT_SYMBOL(security_release_secctx);
>  
> @@ -2163,8 +2251,15 @@ EXPORT_SYMBOL(security_sock_rcv_skb);
>  int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
>  				      int __user *optlen, unsigned len)
>  {
> -	return call_int_hook(socket_getpeersec_stream, -ENOPROTOOPT, sock,
> -				optval, optlen, len);
> +	int *display = current->security;
> +	struct security_hook_list *hp;
> +
> +	hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_stream,
> +			     list)
> +		if (*display == LSMBLOB_INVALID || *display == hp->lsmid->slot)
> +			return hp->hook.socket_getpeersec_stream(sock, optval,
> +								 optlen, len);
> +	return -ENOPROTOOPT;
>  }
>  
>  int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
> -- 
> 2.20.1
>
John Johansen June 27, 2019, 9:33 p.m. UTC | #2
On 6/26/19 12:22 PM, Casey Schaufler wrote:
> Create a new entry "display" in /proc/.../attr for controlling
> which LSM security information is displayed for a process.
> The name of an active LSM that supplies hooks for human readable
> data may be written to "display" to set the value. The name of
> the LSM currently in use can be read from "display".
> At this point there can only be one LSM capable of display
> active.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>

Reviewed-by: John Johansen <john.johansen@canonical.com>

> ---
>  fs/proc/base.c      |   1 +
>  security/security.c | 129 ++++++++++++++++++++++++++++++++++++++------
>  2 files changed, 113 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index ddef482f1334..7bf70e041315 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -2618,6 +2618,7 @@ static const struct pid_entry attr_dir_stuff[] = {
>  	ATTR(NULL, "fscreate",		0666),
>  	ATTR(NULL, "keycreate",		0666),
>  	ATTR(NULL, "sockcreate",	0666),
> +	ATTR(NULL, "display",		0666),
>  #ifdef CONFIG_SECURITY_SMACK
>  	DIR("smack",			0555,
>  	    proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops),
> diff --git a/security/security.c b/security/security.c
> index 3180a6f30625..82e29c477fa4 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -46,7 +46,9 @@ static struct kmem_cache *lsm_file_cache;
>  static struct kmem_cache *lsm_inode_cache;
>  
>  char *lsm_names;
> -static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init;
> +static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init = {
> +	.lbs_task = sizeof(int),	/* slot number for the "display" LSM */
> +};
>  
>  /* Boot-time LSM user choice */
>  static __initdata const char *chosen_lsm_order;
> @@ -423,8 +425,10 @@ static int lsm_append(const char *new, char **result)
>  
>  /*
>   * Current index to use while initializing the lsmblob secid list.
> + * Pointers to the LSM id structures for local use.
>   */
>  static int lsm_slot;
> +static struct lsm_id *lsm_slotlist[LSMBLOB_ENTRIES];
>  
>  /**
>   * security_add_hooks - Add a modules hooks to the hook lists.
> @@ -444,6 +448,7 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
>  	if (lsmid->slot == LSMBLOB_NEEDED) {
>  		if (lsm_slot >= LSMBLOB_ENTRIES)
>  			panic("%s Too many LSMs registered.\n", __func__);
> +		lsm_slotlist[lsm_slot] = lsmid;
>  		lsmid->slot = lsm_slot++;
>  		init_debug("%s assigned lsmblob slot %d\n", lsmid->lsm,
>  			   lsmid->slot);
> @@ -564,6 +569,8 @@ int lsm_inode_alloc(struct inode *inode)
>   */
>  static int lsm_task_alloc(struct task_struct *task)
>  {
> +	int *display;
> +
>  	if (blob_sizes.lbs_task == 0) {
>  		task->security = NULL;
>  		return 0;
> @@ -572,6 +579,15 @@ static int lsm_task_alloc(struct task_struct *task)
>  	task->security = kzalloc(blob_sizes.lbs_task, GFP_KERNEL);
>  	if (task->security == NULL)
>  		return -ENOMEM;
> +
> +	/*
> +	 * The start of the task blob contains the "display" LSM slot number.
> +	 * Start with it set to the invalid slot number, indicating that the
> +	 * default first registered LSM be displayed.
> +	 */
> +	display = task->security;
> +	*display = LSMBLOB_INVALID;
> +
>  	return 0;
>  }
>  
> @@ -1563,14 +1579,24 @@ int security_file_open(struct file *file)
>  
>  int security_task_alloc(struct task_struct *task, unsigned long clone_flags)
>  {
> +	int *odisplay = current->security;
> +	int *ndisplay;
>  	int rc = lsm_task_alloc(task);
>  
> -	if (rc)
> +	if (unlikely(rc))
>  		return rc;
> +
>  	rc = call_int_hook(task_alloc, 0, task, clone_flags);
> -	if (unlikely(rc))
> +	if (unlikely(rc)) {
>  		security_task_free(task);
> -	return rc;
> +		return rc;
> +	}
> +
> +	ndisplay = task->security;
> +	if (ndisplay && odisplay)
> +		*ndisplay = *odisplay;
> +
> +	return 0;
>  }
>  
>  void security_task_free(struct task_struct *task)
> @@ -1967,10 +1993,29 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
>  				char **value)
>  {
>  	struct security_hook_list *hp;
> +	int *display = current->security;
> +	int slot = 0;
> +
> +	if (!strcmp(name, "display")) {
> +		/*
> +		 * lsm_slot will be 0 if there are no displaying modules.
> +		 */
> +		if (lsm_slot == 0)
> +			return -EINVAL;
> +		if (*display != LSMBLOB_INVALID)
> +			slot = *display;
> +		*value = kstrdup(lsm_slotlist[slot]->lsm, GFP_KERNEL);
> +		if (*value)
> +			return strlen(*value);
> +		return -ENOMEM;
> +	}
>  
>  	hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) {
>  		if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
>  			continue;
> +		if (lsm == NULL && *display != LSMBLOB_INVALID &&
> +		    *display != hp->lsmid->slot)
> +			continue;
>  		return hp->hook.getprocattr(p, name, value);
>  	}
>  	return -EINVAL;
> @@ -1980,10 +2025,46 @@ int security_setprocattr(const char *lsm, const char *name, void *value,
>  			 size_t size)
>  {
>  	struct security_hook_list *hp;
> +	char *term;
> +	char *cp;
> +	int *display = current->security;
> +	int rc = -EINVAL;
> +	int slot = 0;
> +
> +	if (!strcmp(name, "display")) {
> +		/*
> +		 * lsm_slot will be 0 if there are no displaying modules.
> +		 */
> +		if (lsm_slot == 0 || size == 0)
> +			return -EINVAL;
> +		cp = kzalloc(size + 1, GFP_KERNEL);
> +		if (cp == NULL)
> +			return -ENOMEM;
> +		memcpy(cp, value, size);
> +
> +		term = strchr(cp, ' ');
> +		if (term == NULL)
> +			term = strchr(cp, '\n');
> +		if (term != NULL)
> +			*term = '\0';
> +
> +		for (slot = 0; slot < lsm_slot; slot++)
> +			if (!strcmp(cp, lsm_slotlist[slot]->lsm)) {
> +				*display = lsm_slotlist[slot]->slot;
> +				rc = size;
> +				break;
> +			}
> +
> +		kfree(cp);
> +		return rc;
> +	}
>  
>  	hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) {
>  		if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
>  			continue;
> +		if (lsm == NULL && *display != LSMBLOB_INVALID &&
> +		    *display != hp->lsmid->slot)
> +			continue;
>  		return hp->hook.setprocattr(name, value, size);
>  	}
>  	return -EINVAL;
> @@ -2003,15 +2084,15 @@ EXPORT_SYMBOL(security_ismaclabel);
>  int security_secid_to_secctx(struct lsmblob *blob, char **secdata, u32 *seclen)
>  {
>  	struct security_hook_list *hp;
> -	int rc;
> +	int *display = current->security;
>  
>  	hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) {
>  		if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
>  			continue;
> -		rc = hp->hook.secid_to_secctx(blob->secid[hp->lsmid->slot],
> -					      secdata, seclen);
> -		if (rc != 0)
> -			return rc;
> +		if (*display == LSMBLOB_INVALID || *display == hp->lsmid->slot)
> +			return hp->hook.secid_to_secctx(
> +					blob->secid[hp->lsmid->slot],
> +					secdata, seclen);
>  	}
>  	return 0;
>  }
> @@ -2021,16 +2102,15 @@ int security_secctx_to_secid(const char *secdata, u32 seclen,
>  			     struct lsmblob *blob)
>  {
>  	struct security_hook_list *hp;
> -	int rc;
> +	int *display = current->security;
>  
>  	lsmblob_init(blob, 0);
>  	hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid, list) {
>  		if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
>  			continue;
> -		rc = hp->hook.secctx_to_secid(secdata, seclen,
> -					      &blob->secid[hp->lsmid->slot]);
> -		if (rc != 0)
> -			return rc;
> +		if (*display == LSMBLOB_INVALID || *display == hp->lsmid->slot)
> +			return hp->hook.secctx_to_secid(secdata, seclen,
> +						&blob->secid[hp->lsmid->slot]);
>  	}
>  	return 0;
>  }
> @@ -2038,7 +2118,15 @@ EXPORT_SYMBOL(security_secctx_to_secid);
>  
>  void security_release_secctx(char *secdata, u32 seclen)
>  {
> -	call_void_hook(release_secctx, secdata, seclen);
> +	struct security_hook_list *hp;
> +	int *display = current->security;
> +
> +	hlist_for_each_entry(hp, &security_hook_heads.release_secctx, list)
> +		if (*display == LSMBLOB_INVALID ||
> +		    *display == hp->lsmid->slot) {
> +			hp->hook.release_secctx(secdata, seclen);
> +			return;
> +		}
>  }
>  EXPORT_SYMBOL(security_release_secctx);
>  
> @@ -2163,8 +2251,15 @@ EXPORT_SYMBOL(security_sock_rcv_skb);
>  int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
>  				      int __user *optlen, unsigned len)
>  {
> -	return call_int_hook(socket_getpeersec_stream, -ENOPROTOOPT, sock,
> -				optval, optlen, len);
> +	int *display = current->security;
> +	struct security_hook_list *hp;
> +
> +	hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_stream,
> +			     list)
> +		if (*display == LSMBLOB_INVALID || *display == hp->lsmid->slot)
> +			return hp->hook.socket_getpeersec_stream(sock, optval,
> +								 optlen, len);
> +	return -ENOPROTOOPT;
>  }
>  
>  int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
>
Stephen Smalley June 28, 2019, 2:45 p.m. UTC | #3
On 6/26/19 3:22 PM, Casey Schaufler wrote:
> Create a new entry "display" in /proc/.../attr for controlling
> which LSM security information is displayed for a process.
> The name of an active LSM that supplies hooks for human readable
> data may be written to "display" to set the value. The name of
> the LSM currently in use can be read from "display".
> At this point there can only be one LSM capable of display
> active.

IIUC, presently, at the end of the entire series,

1) Any process can change its display attribute to any enabled security 
module, and no security module can veto that change.

2) The display attribute is inherited across fork and exec, even execs 
that change credentials, and again no security module has control over 
the inheritance of this attribute.

3) Setting the display attribute affects more than just the contexts 
read or written by the process itself:
- Contexts reported in audit logs,
- Contexts passed across binder (generated in sender context, delivered 
to receiver),
- Contexts passed to NFS servers for new files,
- Contexts returned by NFS servers for existing files,
- Netlink-related contexts (?possibly generated in sender context rather 
than receiver context?),
- This list may not be complete.

4) A security_secid_to_secctx() in one process' context (e.g. sender) or 
with one display value followed later by a security_secctx_to_secid() 
call in a different process' context (e.g. receiver) or with a different 
display value may ask a different security module to perform the reverse 
translation of the context than the forward translation.

Is that correct?  If so, it seems problematic.
	
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
>   fs/proc/base.c      |   1 +
>   security/security.c | 129 ++++++++++++++++++++++++++++++++++++++------
>   2 files changed, 113 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index ddef482f1334..7bf70e041315 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -2618,6 +2618,7 @@ static const struct pid_entry attr_dir_stuff[] = {
>   	ATTR(NULL, "fscreate",		0666),
>   	ATTR(NULL, "keycreate",		0666),
>   	ATTR(NULL, "sockcreate",	0666),
> +	ATTR(NULL, "display",		0666),
>   #ifdef CONFIG_SECURITY_SMACK
>   	DIR("smack",			0555,
>   	    proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops),
> diff --git a/security/security.c b/security/security.c
> index 3180a6f30625..82e29c477fa4 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -46,7 +46,9 @@ static struct kmem_cache *lsm_file_cache;
>   static struct kmem_cache *lsm_inode_cache;
>   
>   char *lsm_names;
> -static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init;
> +static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init = {
> +	.lbs_task = sizeof(int),	/* slot number for the "display" LSM */
> +};
>   
>   /* Boot-time LSM user choice */
>   static __initdata const char *chosen_lsm_order;
> @@ -423,8 +425,10 @@ static int lsm_append(const char *new, char **result)
>   
>   /*
>    * Current index to use while initializing the lsmblob secid list.
> + * Pointers to the LSM id structures for local use.
>    */
>   static int lsm_slot;
> +static struct lsm_id *lsm_slotlist[LSMBLOB_ENTRIES];
>   
>   /**
>    * security_add_hooks - Add a modules hooks to the hook lists.
> @@ -444,6 +448,7 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
>   	if (lsmid->slot == LSMBLOB_NEEDED) {
>   		if (lsm_slot >= LSMBLOB_ENTRIES)
>   			panic("%s Too many LSMs registered.\n", __func__);
> +		lsm_slotlist[lsm_slot] = lsmid;
>   		lsmid->slot = lsm_slot++;
>   		init_debug("%s assigned lsmblob slot %d\n", lsmid->lsm,
>   			   lsmid->slot);
> @@ -564,6 +569,8 @@ int lsm_inode_alloc(struct inode *inode)
>    */
>   static int lsm_task_alloc(struct task_struct *task)
>   {
> +	int *display;
> +
>   	if (blob_sizes.lbs_task == 0) {
>   		task->security = NULL;
>   		return 0;
> @@ -572,6 +579,15 @@ static int lsm_task_alloc(struct task_struct *task)
>   	task->security = kzalloc(blob_sizes.lbs_task, GFP_KERNEL);
>   	if (task->security == NULL)
>   		return -ENOMEM;
> +
> +	/*
> +	 * The start of the task blob contains the "display" LSM slot number.
> +	 * Start with it set to the invalid slot number, indicating that the
> +	 * default first registered LSM be displayed.
> +	 */
> +	display = task->security;
> +	*display = LSMBLOB_INVALID;
> +
>   	return 0;
>   }
>   
> @@ -1563,14 +1579,24 @@ int security_file_open(struct file *file)
>   
>   int security_task_alloc(struct task_struct *task, unsigned long clone_flags)
>   {
> +	int *odisplay = current->security;
> +	int *ndisplay;
>   	int rc = lsm_task_alloc(task);
>   
> -	if (rc)
> +	if (unlikely(rc))
>   		return rc;
> +
>   	rc = call_int_hook(task_alloc, 0, task, clone_flags);
> -	if (unlikely(rc))
> +	if (unlikely(rc)) {
>   		security_task_free(task);
> -	return rc;
> +		return rc;
> +	}
> +
> +	ndisplay = task->security;
> +	if (ndisplay && odisplay)
> +		*ndisplay = *odisplay;
> +
> +	return 0;
>   }
>   
>   void security_task_free(struct task_struct *task)
> @@ -1967,10 +1993,29 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
>   				char **value)
>   {
>   	struct security_hook_list *hp;
> +	int *display = current->security;
> +	int slot = 0;
> +
> +	if (!strcmp(name, "display")) {
> +		/*
> +		 * lsm_slot will be 0 if there are no displaying modules.
> +		 */
> +		if (lsm_slot == 0)
> +			return -EINVAL;
> +		if (*display != LSMBLOB_INVALID)
> +			slot = *display;
> +		*value = kstrdup(lsm_slotlist[slot]->lsm, GFP_KERNEL);
> +		if (*value)
> +			return strlen(*value);
> +		return -ENOMEM;
> +	}
>   
>   	hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) {
>   		if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
>   			continue;
> +		if (lsm == NULL && *display != LSMBLOB_INVALID &&
> +		    *display != hp->lsmid->slot)
> +			continue;
>   		return hp->hook.getprocattr(p, name, value);
>   	}
>   	return -EINVAL;
> @@ -1980,10 +2025,46 @@ int security_setprocattr(const char *lsm, const char *name, void *value,
>   			 size_t size)
>   {
>   	struct security_hook_list *hp;
> +	char *term;
> +	char *cp;
> +	int *display = current->security;
> +	int rc = -EINVAL;
> +	int slot = 0;
> +
> +	if (!strcmp(name, "display")) {
> +		/*
> +		 * lsm_slot will be 0 if there are no displaying modules.
> +		 */
> +		if (lsm_slot == 0 || size == 0)
> +			return -EINVAL;
> +		cp = kzalloc(size + 1, GFP_KERNEL);
> +		if (cp == NULL)
> +			return -ENOMEM;
> +		memcpy(cp, value, size);
> +
> +		term = strchr(cp, ' ');
> +		if (term == NULL)
> +			term = strchr(cp, '\n');
> +		if (term != NULL)
> +			*term = '\0';
> +
> +		for (slot = 0; slot < lsm_slot; slot++)
> +			if (!strcmp(cp, lsm_slotlist[slot]->lsm)) {
> +				*display = lsm_slotlist[slot]->slot;
> +				rc = size;
> +				break;
> +			}
> +
> +		kfree(cp);
> +		return rc;
> +	}
>   
>   	hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) {
>   		if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
>   			continue;
> +		if (lsm == NULL && *display != LSMBLOB_INVALID &&
> +		    *display != hp->lsmid->slot)
> +			continue;
>   		return hp->hook.setprocattr(name, value, size);
>   	}
>   	return -EINVAL;
> @@ -2003,15 +2084,15 @@ EXPORT_SYMBOL(security_ismaclabel);
>   int security_secid_to_secctx(struct lsmblob *blob, char **secdata, u32 *seclen)
>   {
>   	struct security_hook_list *hp;
> -	int rc;
> +	int *display = current->security;
>   
>   	hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) {
>   		if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
>   			continue;
> -		rc = hp->hook.secid_to_secctx(blob->secid[hp->lsmid->slot],
> -					      secdata, seclen);
> -		if (rc != 0)
> -			return rc;
> +		if (*display == LSMBLOB_INVALID || *display == hp->lsmid->slot)
> +			return hp->hook.secid_to_secctx(
> +					blob->secid[hp->lsmid->slot],
> +					secdata, seclen);
>   	}
>   	return 0;
>   }
> @@ -2021,16 +2102,15 @@ int security_secctx_to_secid(const char *secdata, u32 seclen,
>   			     struct lsmblob *blob)
>   {
>   	struct security_hook_list *hp;
> -	int rc;
> +	int *display = current->security;
>   
>   	lsmblob_init(blob, 0);
>   	hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid, list) {
>   		if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
>   			continue;
> -		rc = hp->hook.secctx_to_secid(secdata, seclen,
> -					      &blob->secid[hp->lsmid->slot]);
> -		if (rc != 0)
> -			return rc;
> +		if (*display == LSMBLOB_INVALID || *display == hp->lsmid->slot)
> +			return hp->hook.secctx_to_secid(secdata, seclen,
> +						&blob->secid[hp->lsmid->slot]);
>   	}
>   	return 0;
>   }
> @@ -2038,7 +2118,15 @@ EXPORT_SYMBOL(security_secctx_to_secid);
>   
>   void security_release_secctx(char *secdata, u32 seclen)
>   {
> -	call_void_hook(release_secctx, secdata, seclen);
> +	struct security_hook_list *hp;
> +	int *display = current->security;
> +
> +	hlist_for_each_entry(hp, &security_hook_heads.release_secctx, list)
> +		if (*display == LSMBLOB_INVALID ||
> +		    *display == hp->lsmid->slot) {
> +			hp->hook.release_secctx(secdata, seclen);
> +			return;
> +		}
>   }
>   EXPORT_SYMBOL(security_release_secctx);
>   
> @@ -2163,8 +2251,15 @@ EXPORT_SYMBOL(security_sock_rcv_skb);
>   int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
>   				      int __user *optlen, unsigned len)
>   {
> -	return call_int_hook(socket_getpeersec_stream, -ENOPROTOOPT, sock,
> -				optval, optlen, len);
> +	int *display = current->security;
> +	struct security_hook_list *hp;
> +
> +	hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_stream,
> +			     list)
> +		if (*display == LSMBLOB_INVALID || *display == hp->lsmid->slot)
> +			return hp->hook.socket_getpeersec_stream(sock, optval,
> +								 optlen, len);
> +	return -ENOPROTOOPT;
>   }
>   
>   int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
>
Casey Schaufler June 28, 2019, 4:15 p.m. UTC | #4
On 6/28/2019 7:45 AM, Stephen Smalley wrote:
> On 6/26/19 3:22 PM, Casey Schaufler wrote:
>> Create a new entry "display" in /proc/.../attr for controlling
>> which LSM security information is displayed for a process.
>> The name of an active LSM that supplies hooks for human readable
>> data may be written to "display" to set the value. The name of
>> the LSM currently in use can be read from "display".
>> At this point there can only be one LSM capable of display
>> active.
>
> IIUC, presently, at the end of the entire series,
>
> 1) Any process can change its display attribute to any enabled security module, and no security module can veto that change.

That is correct. If a security module could hoard the display it
could prevent user space from functioning in a multiple module
environment.

> 2) The display attribute is inherited across fork and exec, even execs that change credentials, and again no security module has control over the inheritance of this attribute.

Also correct. Scripts don't work otherwise.

>
> 3) Setting the display attribute affects more than just the contexts read or written by the process itself:
> - Contexts reported in audit logs,
> - Contexts passed across binder (generated in sender context, delivered to receiver),
> - Contexts passed to NFS servers for new files,
> - Contexts returned by NFS servers for existing files,
> - Netlink-related contexts (?possibly generated in sender context rather than receiver context?),
> - This list may not be complete.

Any of which can be changed should a more rational behavior be proposed.
One possibility is to use lsm='value',lsm='value' encoding for internal
communications, but there's been considerable resistance to anything
like that.

> 4) A security_secid_to_secctx() in one process' context (e.g. sender) or with one display value followed later by a security_secctx_to_secid() call in a different process' context (e.g. receiver) or with a different display value may ask a different security module to perform the reverse translation of the context than the forward translation.

Do you have an example of where this might happen?
Contexts are rarely used within the kernel. The usual
behavior is to generate them, send them out to user space,
and delete them. They get cached in some networking code,
but not in cases where more than one (existing) security
module will ever use them. Binder may be an exception, but
only SELinux (currently) supports binder.


> Is that correct?  If so, it seems problematic.

Balancing backward compatibility with new behavior is hard!
What would you suggest for audit logs? Should we put all LSM
data in every record? Is NFS a concern for anyone not using
SELinux?

There is no user space that uses display, and it's going
to take some time to work out all the kinks before we even
think about teaching systemd about it.

>     
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> ---
>>   fs/proc/base.c      |   1 +
>>   security/security.c | 129 ++++++++++++++++++++++++++++++++++++++------
>>   2 files changed, 113 insertions(+), 17 deletions(-)
>>
>> diff --git a/fs/proc/base.c b/fs/proc/base.c
>> index ddef482f1334..7bf70e041315 100644
>> --- a/fs/proc/base.c
>> +++ b/fs/proc/base.c
>> @@ -2618,6 +2618,7 @@ static const struct pid_entry attr_dir_stuff[] = {
>>       ATTR(NULL, "fscreate",        0666),
>>       ATTR(NULL, "keycreate",        0666),
>>       ATTR(NULL, "sockcreate",    0666),
>> +    ATTR(NULL, "display",        0666),
>>   #ifdef CONFIG_SECURITY_SMACK
>>       DIR("smack",            0555,
>>           proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops),
>> diff --git a/security/security.c b/security/security.c
>> index 3180a6f30625..82e29c477fa4 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -46,7 +46,9 @@ static struct kmem_cache *lsm_file_cache;
>>   static struct kmem_cache *lsm_inode_cache;
>>     char *lsm_names;
>> -static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init;
>> +static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init = {
>> +    .lbs_task = sizeof(int),    /* slot number for the "display" LSM */
>> +};
>>     /* Boot-time LSM user choice */
>>   static __initdata const char *chosen_lsm_order;
>> @@ -423,8 +425,10 @@ static int lsm_append(const char *new, char **result)
>>     /*
>>    * Current index to use while initializing the lsmblob secid list.
>> + * Pointers to the LSM id structures for local use.
>>    */
>>   static int lsm_slot;
>> +static struct lsm_id *lsm_slotlist[LSMBLOB_ENTRIES];
>>     /**
>>    * security_add_hooks - Add a modules hooks to the hook lists.
>> @@ -444,6 +448,7 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
>>       if (lsmid->slot == LSMBLOB_NEEDED) {
>>           if (lsm_slot >= LSMBLOB_ENTRIES)
>>               panic("%s Too many LSMs registered.\n", __func__);
>> +        lsm_slotlist[lsm_slot] = lsmid;
>>           lsmid->slot = lsm_slot++;
>>           init_debug("%s assigned lsmblob slot %d\n", lsmid->lsm,
>>                  lsmid->slot);
>> @@ -564,6 +569,8 @@ int lsm_inode_alloc(struct inode *inode)
>>    */
>>   static int lsm_task_alloc(struct task_struct *task)
>>   {
>> +    int *display;
>> +
>>       if (blob_sizes.lbs_task == 0) {
>>           task->security = NULL;
>>           return 0;
>> @@ -572,6 +579,15 @@ static int lsm_task_alloc(struct task_struct *task)
>>       task->security = kzalloc(blob_sizes.lbs_task, GFP_KERNEL);
>>       if (task->security == NULL)
>>           return -ENOMEM;
>> +
>> +    /*
>> +     * The start of the task blob contains the "display" LSM slot number.
>> +     * Start with it set to the invalid slot number, indicating that the
>> +     * default first registered LSM be displayed.
>> +     */
>> +    display = task->security;
>> +    *display = LSMBLOB_INVALID;
>> +
>>       return 0;
>>   }
>>   @@ -1563,14 +1579,24 @@ int security_file_open(struct file *file)
>>     int security_task_alloc(struct task_struct *task, unsigned long clone_flags)
>>   {
>> +    int *odisplay = current->security;
>> +    int *ndisplay;
>>       int rc = lsm_task_alloc(task);
>>   -    if (rc)
>> +    if (unlikely(rc))
>>           return rc;
>> +
>>       rc = call_int_hook(task_alloc, 0, task, clone_flags);
>> -    if (unlikely(rc))
>> +    if (unlikely(rc)) {
>>           security_task_free(task);
>> -    return rc;
>> +        return rc;
>> +    }
>> +
>> +    ndisplay = task->security;
>> +    if (ndisplay && odisplay)
>> +        *ndisplay = *odisplay;
>> +
>> +    return 0;
>>   }
>>     void security_task_free(struct task_struct *task)
>> @@ -1967,10 +1993,29 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
>>                   char **value)
>>   {
>>       struct security_hook_list *hp;
>> +    int *display = current->security;
>> +    int slot = 0;
>> +
>> +    if (!strcmp(name, "display")) {
>> +        /*
>> +         * lsm_slot will be 0 if there are no displaying modules.
>> +         */
>> +        if (lsm_slot == 0)
>> +            return -EINVAL;
>> +        if (*display != LSMBLOB_INVALID)
>> +            slot = *display;
>> +        *value = kstrdup(lsm_slotlist[slot]->lsm, GFP_KERNEL);
>> +        if (*value)
>> +            return strlen(*value);
>> +        return -ENOMEM;
>> +    }
>>         hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) {
>>           if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
>>               continue;
>> +        if (lsm == NULL && *display != LSMBLOB_INVALID &&
>> +            *display != hp->lsmid->slot)
>> +            continue;
>>           return hp->hook.getprocattr(p, name, value);
>>       }
>>       return -EINVAL;
>> @@ -1980,10 +2025,46 @@ int security_setprocattr(const char *lsm, const char *name, void *value,
>>                size_t size)
>>   {
>>       struct security_hook_list *hp;
>> +    char *term;
>> +    char *cp;
>> +    int *display = current->security;
>> +    int rc = -EINVAL;
>> +    int slot = 0;
>> +
>> +    if (!strcmp(name, "display")) {
>> +        /*
>> +         * lsm_slot will be 0 if there are no displaying modules.
>> +         */
>> +        if (lsm_slot == 0 || size == 0)
>> +            return -EINVAL;
>> +        cp = kzalloc(size + 1, GFP_KERNEL);
>> +        if (cp == NULL)
>> +            return -ENOMEM;
>> +        memcpy(cp, value, size);
>> +
>> +        term = strchr(cp, ' ');
>> +        if (term == NULL)
>> +            term = strchr(cp, '\n');
>> +        if (term != NULL)
>> +            *term = '\0';
>> +
>> +        for (slot = 0; slot < lsm_slot; slot++)
>> +            if (!strcmp(cp, lsm_slotlist[slot]->lsm)) {
>> +                *display = lsm_slotlist[slot]->slot;
>> +                rc = size;
>> +                break;
>> +            }
>> +
>> +        kfree(cp);
>> +        return rc;
>> +    }
>>         hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) {
>>           if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
>>               continue;
>> +        if (lsm == NULL && *display != LSMBLOB_INVALID &&
>> +            *display != hp->lsmid->slot)
>> +            continue;
>>           return hp->hook.setprocattr(name, value, size);
>>       }
>>       return -EINVAL;
>> @@ -2003,15 +2084,15 @@ EXPORT_SYMBOL(security_ismaclabel);
>>   int security_secid_to_secctx(struct lsmblob *blob, char **secdata, u32 *seclen)
>>   {
>>       struct security_hook_list *hp;
>> -    int rc;
>> +    int *display = current->security;
>>         hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) {
>>           if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
>>               continue;
>> -        rc = hp->hook.secid_to_secctx(blob->secid[hp->lsmid->slot],
>> -                          secdata, seclen);
>> -        if (rc != 0)
>> -            return rc;
>> +        if (*display == LSMBLOB_INVALID || *display == hp->lsmid->slot)
>> +            return hp->hook.secid_to_secctx(
>> +                    blob->secid[hp->lsmid->slot],
>> +                    secdata, seclen);
>>       }
>>       return 0;
>>   }
>> @@ -2021,16 +2102,15 @@ int security_secctx_to_secid(const char *secdata, u32 seclen,
>>                    struct lsmblob *blob)
>>   {
>>       struct security_hook_list *hp;
>> -    int rc;
>> +    int *display = current->security;
>>         lsmblob_init(blob, 0);
>>       hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid, list) {
>>           if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
>>               continue;
>> -        rc = hp->hook.secctx_to_secid(secdata, seclen,
>> -                          &blob->secid[hp->lsmid->slot]);
>> -        if (rc != 0)
>> -            return rc;
>> +        if (*display == LSMBLOB_INVALID || *display == hp->lsmid->slot)
>> +            return hp->hook.secctx_to_secid(secdata, seclen,
>> +                        &blob->secid[hp->lsmid->slot]);
>>       }
>>       return 0;
>>   }
>> @@ -2038,7 +2118,15 @@ EXPORT_SYMBOL(security_secctx_to_secid);
>>     void security_release_secctx(char *secdata, u32 seclen)
>>   {
>> -    call_void_hook(release_secctx, secdata, seclen);
>> +    struct security_hook_list *hp;
>> +    int *display = current->security;
>> +
>> +    hlist_for_each_entry(hp, &security_hook_heads.release_secctx, list)
>> +        if (*display == LSMBLOB_INVALID ||
>> +            *display == hp->lsmid->slot) {
>> +            hp->hook.release_secctx(secdata, seclen);
>> +            return;
>> +        }
>>   }
>>   EXPORT_SYMBOL(security_release_secctx);
>>   @@ -2163,8 +2251,15 @@ EXPORT_SYMBOL(security_sock_rcv_skb);
>>   int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
>>                         int __user *optlen, unsigned len)
>>   {
>> -    return call_int_hook(socket_getpeersec_stream, -ENOPROTOOPT, sock,
>> -                optval, optlen, len);
>> +    int *display = current->security;
>> +    struct security_hook_list *hp;
>> +
>> +    hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_stream,
>> +                 list)
>> +        if (*display == LSMBLOB_INVALID || *display == hp->lsmid->slot)
>> +            return hp->hook.socket_getpeersec_stream(sock, optval,
>> +                                 optlen, len);
>> +    return -ENOPROTOOPT;
>>   }
>>     int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
>>
>
John Johansen June 28, 2019, 6:08 p.m. UTC | #5
On 6/28/19 9:15 AM, Casey Schaufler wrote:
> On 6/28/2019 7:45 AM, Stephen Smalley wrote:
>> On 6/26/19 3:22 PM, Casey Schaufler wrote:
>>> Create a new entry "display" in /proc/.../attr for controlling
>>> which LSM security information is displayed for a process.
>>> The name of an active LSM that supplies hooks for human readable
>>> data may be written to "display" to set the value. The name of
>>> the LSM currently in use can be read from "display".
>>> At this point there can only be one LSM capable of display
>>> active.
>>
>> IIUC, presently, at the end of the entire series,
>>
>> 1) Any process can change its display attribute to any enabled security module, and no security module can veto that change.
> 
> That is correct. If a security module could hoard the display it
> could prevent user space from functioning in a multiple module
> environment.
> 

It should be noted that this is also just for legacy, we agreed
last year that smack and apparmor would move to new none shared
interfaces by default, and ideally other LSMs would as well.
Smack has already added its process dir and apparmor has its
in apparmor-next

The display LSM allows for the current interfaces to be used
in a stacking situation for things like LSM in legacy container.

>> 2) The display attribute is inherited across fork and exec, even execs that change credentials, and again no security module has control over the inheritance of this attribute.
> 
> Also correct. Scripts don't work otherwise.
> 
>>
>> 3) Setting the display attribute affects more than just the contexts read or written by the process itself:
>> - Contexts reported in audit logs,
>> - Contexts passed across binder (generated in sender context, delivered to receiver),
>> - Contexts passed to NFS servers for new files,
>> - Contexts returned by NFS servers for existing files,
>> - Netlink-related contexts (?possibly generated in sender context rather than receiver context?),
>> - This list may not be complete.
> 
> Any of which can be changed should a more rational behavior be proposed.
> One possibility is to use lsm='value',lsm='value' encoding for internal
> communications, but there's been considerable resistance to anything
> like that.
> 

This is the part of the patchset that I am least happy with but
it is a hard problem, and so far using display has been the only
option that has been even sort of agreed to.


>> 4) A security_secid_to_secctx() in one process' context (e.g. sender) or with one display value followed later by a security_secctx_to_secid() call in a different process' context (e.g. receiver) or with a different display value may ask a different security module to perform the reverse translation of the context than the forward translation.
> 
> Do you have an example of where this might happen?
> Contexts are rarely used within the kernel. The usual
> behavior is to generate them, send them out to user space,
> and delete them. They get cached in some networking code,
> but not in cases where more than one (existing) security
> module will ever use them. Binder may be an exception, but
> only SELinux (currently) supports binder.
> 
> 
>> Is that correct?  If so, it seems problematic.
> 
> Balancing backward compatibility with new behavior is hard!
> What would you suggest for audit logs? Should we put all LSM
> data in every record?

If we could it would be better, but again how to do it. Every
option presented has been rejected.

> Is NFS a concern for anyone not using
> SELinux?
> 
Yes. Just because it isn't currently used doesn't mean it isn't
a concern or desired. 

> There is no user space that uses display, and it's going
> to take some time to work out all the kinks before we even
> think about teaching systemd about it.
> 

I'm not even sure we want to teach systemd et al. about it,
it would be far better to teach them about
  /proc/*/attr/{smack,apparmor,...}/*


But yes its going to take time to get the userspace updated.
Stephen Smalley June 29, 2019, 1:01 a.m. UTC | #6
On Fri, Jun 28, 2019 at 12:15 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 6/28/2019 7:45 AM, Stephen Smalley wrote:
> > On 6/26/19 3:22 PM, Casey Schaufler wrote:
> >> Create a new entry "display" in /proc/.../attr for controlling
> >> which LSM security information is displayed for a process.
> >> The name of an active LSM that supplies hooks for human readable
> >> data may be written to "display" to set the value. The name of
> >> the LSM currently in use can be read from "display".
> >> At this point there can only be one LSM capable of display
> >> active.
> >
> > IIUC, presently, at the end of the entire series,
> >
> > 1) Any process can change its display attribute to any enabled security module, and no security module can veto that change.
>
> That is correct. If a security module could hoard the display it
> could prevent user space from functioning in a multiple module
> environment.
>
> > 2) The display attribute is inherited across fork and exec, even execs that change credentials, and again no security module has control over the inheritance of this attribute.
>
> Also correct. Scripts don't work otherwise.

It's a security hole waiting to happen. Unprivileged caller sets its
display value to Smack on a mostly SELinux system that happens to
enable Smack too, then exec's a credential-changing SELinux-aware
program that uses one of the libselinux APIs to set one of the
/proc/self/attr attributes to a different SELinux context. Due to the
change in display, the SELinux-aware program instead ends up setting
one of the Smack attributes and therefore the desired SELinux context
is never applied to the process or file or socket or whatever.

>
> >
> > 3) Setting the display attribute affects more than just the contexts read or written by the process itself:
> > - Contexts reported in audit logs,
> > - Contexts passed across binder (generated in sender context, delivered to receiver),
> > - Contexts passed to NFS servers for new files,
> > - Contexts returned by NFS servers for existing files,
> > - Netlink-related contexts (?possibly generated in sender context rather than receiver context?),
> > - This list may not be complete.
>
> Any of which can be changed should a more rational behavior be proposed.
> One possibility is to use lsm='value',lsm='value' encoding for internal
> communications, but there's been considerable resistance to anything
> like that.

These are also security holes waiting to happen.  Processes can use it
to hide their SELinux contexts from the audit logs, forge different
SELinux contexts on binder IPC, forge file contexts to which they have
no SELinux permissions on new files, ... All they need is stacking to
be enabled and one other module that helpfully lets them set attribute
values that look like SELinux contexts, and then they can set those
and switch their display at the right time.

>
> > 4) A security_secid_to_secctx() in one process' context (e.g. sender) or with one display value followed later by a security_secctx_to_secid() call in a different process' context (e.g. receiver) or with a different display value may ask a different security module to perform the reverse translation of the context than the forward translation.
>
> Do you have an example of where this might happen?
> Contexts are rarely used within the kernel. The usual
> behavior is to generate them, send them out to user space,
> and delete them. They get cached in some networking code,
> but not in cases where more than one (existing) security
> module will ever use them. Binder may be an exception, but
> only SELinux (currently) supports binder.

Haven't looked but I don't like the asymmetry of the interface.
Doesn't matter that only SELinux supports binder if  you ever want any
other security module other than SELinux enabled at the same time as
SELinux.

>
>
> > Is that correct?  If so, it seems problematic.
>
> Balancing backward compatibility with new behavior is hard!
> What would you suggest for audit logs? Should we put all LSM
> data in every record? Is NFS a concern for anyone not using
> SELinux?

Yes to all on audit if stacking is going to be real.  And yes, I think
other security modules will care about NFS if they are serious.

>
> There is no user space that uses display, and it's going
> to take some time to work out all the kinks before we even
> think about teaching systemd about it.

That doesn't make it acceptable to introduce a mechanism that weakens
security now.

>
> >
> >>
> >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> >> ---
> >>   fs/proc/base.c      |   1 +
> >>   security/security.c | 129 ++++++++++++++++++++++++++++++++++++++------
> >>   2 files changed, 113 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/fs/proc/base.c b/fs/proc/base.c
> >> index ddef482f1334..7bf70e041315 100644
> >> --- a/fs/proc/base.c
> >> +++ b/fs/proc/base.c
> >> @@ -2618,6 +2618,7 @@ static const struct pid_entry attr_dir_stuff[] = {
> >>       ATTR(NULL, "fscreate",        0666),
> >>       ATTR(NULL, "keycreate",        0666),
> >>       ATTR(NULL, "sockcreate",    0666),
> >> +    ATTR(NULL, "display",        0666),
> >>   #ifdef CONFIG_SECURITY_SMACK
> >>       DIR("smack",            0555,
> >>           proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops),
> >> diff --git a/security/security.c b/security/security.c
> >> index 3180a6f30625..82e29c477fa4 100644
> >> --- a/security/security.c
> >> +++ b/security/security.c
> >> @@ -46,7 +46,9 @@ static struct kmem_cache *lsm_file_cache;
> >>   static struct kmem_cache *lsm_inode_cache;
> >>     char *lsm_names;
> >> -static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init;
> >> +static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init = {
> >> +    .lbs_task = sizeof(int),    /* slot number for the "display" LSM */
> >> +};
> >>     /* Boot-time LSM user choice */
> >>   static __initdata const char *chosen_lsm_order;
> >> @@ -423,8 +425,10 @@ static int lsm_append(const char *new, char **result)
> >>     /*
> >>    * Current index to use while initializing the lsmblob secid list.
> >> + * Pointers to the LSM id structures for local use.
> >>    */
> >>   static int lsm_slot;
> >> +static struct lsm_id *lsm_slotlist[LSMBLOB_ENTRIES];
> >>     /**
> >>    * security_add_hooks - Add a modules hooks to the hook lists.
> >> @@ -444,6 +448,7 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
> >>       if (lsmid->slot == LSMBLOB_NEEDED) {
> >>           if (lsm_slot >= LSMBLOB_ENTRIES)
> >>               panic("%s Too many LSMs registered.\n", __func__);
> >> +        lsm_slotlist[lsm_slot] = lsmid;
> >>           lsmid->slot = lsm_slot++;
> >>           init_debug("%s assigned lsmblob slot %d\n", lsmid->lsm,
> >>                  lsmid->slot);
> >> @@ -564,6 +569,8 @@ int lsm_inode_alloc(struct inode *inode)
> >>    */
> >>   static int lsm_task_alloc(struct task_struct *task)
> >>   {
> >> +    int *display;
> >> +
> >>       if (blob_sizes.lbs_task == 0) {
> >>           task->security = NULL;
> >>           return 0;
> >> @@ -572,6 +579,15 @@ static int lsm_task_alloc(struct task_struct *task)
> >>       task->security = kzalloc(blob_sizes.lbs_task, GFP_KERNEL);
> >>       if (task->security == NULL)
> >>           return -ENOMEM;
> >> +
> >> +    /*
> >> +     * The start of the task blob contains the "display" LSM slot number.
> >> +     * Start with it set to the invalid slot number, indicating that the
> >> +     * default first registered LSM be displayed.
> >> +     */
> >> +    display = task->security;
> >> +    *display = LSMBLOB_INVALID;
> >> +
> >>       return 0;
> >>   }
> >>   @@ -1563,14 +1579,24 @@ int security_file_open(struct file *file)
> >>     int security_task_alloc(struct task_struct *task, unsigned long clone_flags)
> >>   {
> >> +    int *odisplay = current->security;
> >> +    int *ndisplay;
> >>       int rc = lsm_task_alloc(task);
> >>   -    if (rc)
> >> +    if (unlikely(rc))
> >>           return rc;
> >> +
> >>       rc = call_int_hook(task_alloc, 0, task, clone_flags);
> >> -    if (unlikely(rc))
> >> +    if (unlikely(rc)) {
> >>           security_task_free(task);
> >> -    return rc;
> >> +        return rc;
> >> +    }
> >> +
> >> +    ndisplay = task->security;
> >> +    if (ndisplay && odisplay)
> >> +        *ndisplay = *odisplay;
> >> +
> >> +    return 0;
> >>   }
> >>     void security_task_free(struct task_struct *task)
> >> @@ -1967,10 +1993,29 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
> >>                   char **value)
> >>   {
> >>       struct security_hook_list *hp;
> >> +    int *display = current->security;
> >> +    int slot = 0;
> >> +
> >> +    if (!strcmp(name, "display")) {
> >> +        /*
> >> +         * lsm_slot will be 0 if there are no displaying modules.
> >> +         */
> >> +        if (lsm_slot == 0)
> >> +            return -EINVAL;
> >> +        if (*display != LSMBLOB_INVALID)
> >> +            slot = *display;
> >> +        *value = kstrdup(lsm_slotlist[slot]->lsm, GFP_KERNEL);
> >> +        if (*value)
> >> +            return strlen(*value);
> >> +        return -ENOMEM;
> >> +    }
> >>         hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) {
> >>           if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
> >>               continue;
> >> +        if (lsm == NULL && *display != LSMBLOB_INVALID &&
> >> +            *display != hp->lsmid->slot)
> >> +            continue;
> >>           return hp->hook.getprocattr(p, name, value);
> >>       }
> >>       return -EINVAL;
> >> @@ -1980,10 +2025,46 @@ int security_setprocattr(const char *lsm, const char *name, void *value,
> >>                size_t size)
> >>   {
> >>       struct security_hook_list *hp;
> >> +    char *term;
> >> +    char *cp;
> >> +    int *display = current->security;
> >> +    int rc = -EINVAL;
> >> +    int slot = 0;
> >> +
> >> +    if (!strcmp(name, "display")) {
> >> +        /*
> >> +         * lsm_slot will be 0 if there are no displaying modules.
> >> +         */
> >> +        if (lsm_slot == 0 || size == 0)
> >> +            return -EINVAL;
> >> +        cp = kzalloc(size + 1, GFP_KERNEL);
> >> +        if (cp == NULL)
> >> +            return -ENOMEM;
> >> +        memcpy(cp, value, size);
> >> +
> >> +        term = strchr(cp, ' ');
> >> +        if (term == NULL)
> >> +            term = strchr(cp, '\n');
> >> +        if (term != NULL)
> >> +            *term = '\0';
> >> +
> >> +        for (slot = 0; slot < lsm_slot; slot++)
> >> +            if (!strcmp(cp, lsm_slotlist[slot]->lsm)) {
> >> +                *display = lsm_slotlist[slot]->slot;
> >> +                rc = size;
> >> +                break;
> >> +            }
> >> +
> >> +        kfree(cp);
> >> +        return rc;
> >> +    }
> >>         hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) {
> >>           if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
> >>               continue;
> >> +        if (lsm == NULL && *display != LSMBLOB_INVALID &&
> >> +            *display != hp->lsmid->slot)
> >> +            continue;
> >>           return hp->hook.setprocattr(name, value, size);
> >>       }
> >>       return -EINVAL;
> >> @@ -2003,15 +2084,15 @@ EXPORT_SYMBOL(security_ismaclabel);
> >>   int security_secid_to_secctx(struct lsmblob *blob, char **secdata, u32 *seclen)
> >>   {
> >>       struct security_hook_list *hp;
> >> -    int rc;
> >> +    int *display = current->security;
> >>         hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) {
> >>           if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
> >>               continue;
> >> -        rc = hp->hook.secid_to_secctx(blob->secid[hp->lsmid->slot],
> >> -                          secdata, seclen);
> >> -        if (rc != 0)
> >> -            return rc;
> >> +        if (*display == LSMBLOB_INVALID || *display == hp->lsmid->slot)
> >> +            return hp->hook.secid_to_secctx(
> >> +                    blob->secid[hp->lsmid->slot],
> >> +                    secdata, seclen);
> >>       }
> >>       return 0;
> >>   }
> >> @@ -2021,16 +2102,15 @@ int security_secctx_to_secid(const char *secdata, u32 seclen,
> >>                    struct lsmblob *blob)
> >>   {
> >>       struct security_hook_list *hp;
> >> -    int rc;
> >> +    int *display = current->security;
> >>         lsmblob_init(blob, 0);
> >>       hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid, list) {
> >>           if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
> >>               continue;
> >> -        rc = hp->hook.secctx_to_secid(secdata, seclen,
> >> -                          &blob->secid[hp->lsmid->slot]);
> >> -        if (rc != 0)
> >> -            return rc;
> >> +        if (*display == LSMBLOB_INVALID || *display == hp->lsmid->slot)
> >> +            return hp->hook.secctx_to_secid(secdata, seclen,
> >> +                        &blob->secid[hp->lsmid->slot]);
> >>       }
> >>       return 0;
> >>   }
> >> @@ -2038,7 +2118,15 @@ EXPORT_SYMBOL(security_secctx_to_secid);
> >>     void security_release_secctx(char *secdata, u32 seclen)
> >>   {
> >> -    call_void_hook(release_secctx, secdata, seclen);
> >> +    struct security_hook_list *hp;
> >> +    int *display = current->security;
> >> +
> >> +    hlist_for_each_entry(hp, &security_hook_heads.release_secctx, list)
> >> +        if (*display == LSMBLOB_INVALID ||
> >> +            *display == hp->lsmid->slot) {
> >> +            hp->hook.release_secctx(secdata, seclen);
> >> +            return;
> >> +        }
> >>   }
> >>   EXPORT_SYMBOL(security_release_secctx);
> >>   @@ -2163,8 +2251,15 @@ EXPORT_SYMBOL(security_sock_rcv_skb);
> >>   int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
> >>                         int __user *optlen, unsigned len)
> >>   {
> >> -    return call_int_hook(socket_getpeersec_stream, -ENOPROTOOPT, sock,
> >> -                optval, optlen, len);
> >> +    int *display = current->security;
> >> +    struct security_hook_list *hp;
> >> +
> >> +    hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_stream,
> >> +                 list)
> >> +        if (*display == LSMBLOB_INVALID || *display == hp->lsmid->slot)
> >> +            return hp->hook.socket_getpeersec_stream(sock, optval,
> >> +                                 optlen, len);
> >> +    return -ENOPROTOOPT;
> >>   }
> >>     int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
> >>
> >
Casey Schaufler June 29, 2019, 7:45 p.m. UTC | #7
On 6/28/2019 6:01 PM, Stephen Smalley wrote:
> On Fri, Jun 28, 2019 at 12:15 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 6/28/2019 7:45 AM, Stephen Smalley wrote:
>>> On 6/26/19 3:22 PM, Casey Schaufler wrote:
>>>> Create a new entry "display" in /proc/.../attr for controlling
>>>> which LSM security information is displayed for a process.
>>>> The name of an active LSM that supplies hooks for human readable
>>>> data may be written to "display" to set the value. The name of
>>>> the LSM currently in use can be read from "display".
>>>> At this point there can only be one LSM capable of display
>>>> active.
>>> IIUC, presently, at the end of the entire series,
>>>
>>> 1) Any process can change its display attribute to any enabled security module, and no security module can veto that change.
>> That is correct. If a security module could hoard the display it
>> could prevent user space from functioning in a multiple module
>> environment.
>>
>>> 2) The display attribute is inherited across fork and exec, even execs that change credentials, and again no security module has control over the inheritance of this attribute.
>> Also correct. Scripts don't work otherwise.
> It's a security hole waiting to happen. Unprivileged caller sets its
> display value to Smack on a mostly SELinux system that happens to
> enable Smack too, then exec's a credential-changing SELinux-aware
> program that uses one of the libselinux APIs to set one of the
> /proc/self/attr attributes to a different SELinux context. Due to the
> change in display, the SELinux-aware program instead ends up setting
> one of the Smack attributes and therefore the desired SELinux context
> is never applied to the process or file or socket or whatever.

The credential-changing SELinux-aware program is getting
invoked by an unprivileged, Smack aware program? Would anyone
expect that to be a good idea? I'll admit it could happen,
but setting the Smack label of your SELinux-aware program
(which will need CAP_MAC_ADMIN, BTW) to "system_u:system_r:wheehee_t"
is unlikely to result in anything other than your SELinux-aware
program getting very frustrated. In the other direction, a
Smack-aware program that trys to set its SELinux context to "^"
is going to fail by SELinux policy. While I am willing to accept
that it is possible that there is a way to exploit this, it
would require convoluted SELinux and Smack policies. Anyone
who has reason to use a combination of Smack and SELinux on
a real system is already signing up for more configuration
headaches than I would wish on anyone.
 
I have strongly advocated addition of /proc/.../attr/
subdirectories for all LSMs, and that all user space migrate
to using them. /proc/.../attr/selinux/current would not be
affected by the display setting. We know, and have known for
years that so long as "current" is shared there will be this
sort of problem.


>
>>> 3) Setting the display attribute affects more than just the contexts read or written by the process itself:
>>> - Contexts reported in audit logs,
>>> - Contexts passed across binder (generated in sender context, delivered to receiver),
>>> - Contexts passed to NFS servers for new files,
>>> - Contexts returned by NFS servers for existing files,
>>> - Netlink-related contexts (?possibly generated in sender context rather than receiver context?),
>>> - This list may not be complete.
>> Any of which can be changed should a more rational behavior be proposed.
>> One possibility is to use lsm='value',lsm='value' encoding for internal
>> communications, but there's been considerable resistance to anything
>> like that.
> These are also security holes waiting to happen.  Processes can use it
> to hide their SELinux contexts from the audit logs, forge different
> SELinux contexts on binder IPC, forge file contexts to which they have
> no SELinux permissions on new files, ... All they need is stacking to
> be enabled and one other module that helpfully lets them set attribute
> values that look like SELinux contexts, and then they can set those
> and switch their display at the right time.

What would you propose as a more rational behavior?
Seriously, I could use some help here.


>>> 4) A security_secid_to_secctx() in one process' context (e.g. sender) or with one display value followed later by a security_secctx_to_secid() call in a different process' context (e.g. receiver) or with a different display value may ask a different security module to perform the reverse translation of the context than the forward translation.
>> Do you have an example of where this might happen?
>> Contexts are rarely used within the kernel. The usual
>> behavior is to generate them, send them out to user space,
>> and delete them. They get cached in some networking code,
>> but not in cases where more than one (existing) security
>> module will ever use them. Binder may be an exception, but
>> only SELinux (currently) supports binder.
> Haven't looked but I don't like the asymmetry of the interface.
> Doesn't matter that only SELinux supports binder if  you ever want any
> other security module other than SELinux enabled at the same time as
> SELinux.

Binder needs another look then.

>> Is that correct?  If so, it seems problematic.
>> Balancing backward compatibility with new behavior is hard!
>> What would you suggest for audit logs? Should we put all LSM
>> data in every record? Is NFS a concern for anyone not using
>> SELinux?
> Yes to all on audit if stacking is going to be real.  And yes, I think
> other security modules will care about NFS if they are serious.

I would love to get feedback from the audit maintainers about
how they would like the multiple LSM data formatted.

NFS is ... challenging. It was supposed to work with Smack
when it went in, but to the best of my understanding never
actually demonstrated.

>> There is no user space that uses display, and it's going
>> to take some time to work out all the kinks before we even
>> think about teaching systemd about it.
> That doesn't make it acceptable to introduce a mechanism that weakens
> security now.

Agreed in principle, not necessarily in detail.
James Morris July 2, 2019, 12:49 a.m. UTC | #8
On Fri, 28 Jun 2019, Stephen Smalley wrote:

> > Balancing backward compatibility with new behavior is hard!
> > What would you suggest for audit logs? Should we put all LSM
> > data in every record? Is NFS a concern for anyone not using
> > SELinux?
> 
> Yes to all on audit if stacking is going to be real.  And yes, I think
> other security modules will care about NFS if they are serious.

Agreed.

There must better way to approach this, somehow...
Casey Schaufler July 2, 2019, 1:20 a.m. UTC | #9
On 7/1/2019 5:49 PM, James Morris wrote:
> On Fri, 28 Jun 2019, Stephen Smalley wrote:
>
>>> Balancing backward compatibility with new behavior is hard!
>>> What would you suggest for audit logs? Should we put all LSM
>>> data in every record? Is NFS a concern for anyone not using
>>> SELinux?
>> Yes to all on audit if stacking is going to be real.  And yes, I think
>> other security modules will care about NFS if they are serious.
> Agreed.
>
> There must better way to approach this, somehow...

It not like I haven't proposed a number of mechanisms!
The "display" mechanism has the best backward compatibility
story, at the cost of being awkward/dangerous in the face of
sophisticated user space environments. A combined string
(smack='System",AppArmor='unconfined') sucks at compatibility,
but provides the best information.

Right now I'm looking at a way to prevent internal confusion.
I think that may be possible.

I'll point out that lib<lsm> has the option of verifying
the display before doing scary writes, but that's a lot of
work that no one is looking forward to.

Patch
diff mbox series

diff --git a/fs/proc/base.c b/fs/proc/base.c
index ddef482f1334..7bf70e041315 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2618,6 +2618,7 @@  static const struct pid_entry attr_dir_stuff[] = {
 	ATTR(NULL, "fscreate",		0666),
 	ATTR(NULL, "keycreate",		0666),
 	ATTR(NULL, "sockcreate",	0666),
+	ATTR(NULL, "display",		0666),
 #ifdef CONFIG_SECURITY_SMACK
 	DIR("smack",			0555,
 	    proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops),
diff --git a/security/security.c b/security/security.c
index 3180a6f30625..82e29c477fa4 100644
--- a/security/security.c
+++ b/security/security.c
@@ -46,7 +46,9 @@  static struct kmem_cache *lsm_file_cache;
 static struct kmem_cache *lsm_inode_cache;
 
 char *lsm_names;
-static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init;
+static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init = {
+	.lbs_task = sizeof(int),	/* slot number for the "display" LSM */
+};
 
 /* Boot-time LSM user choice */
 static __initdata const char *chosen_lsm_order;
@@ -423,8 +425,10 @@  static int lsm_append(const char *new, char **result)
 
 /*
  * Current index to use while initializing the lsmblob secid list.
+ * Pointers to the LSM id structures for local use.
  */
 static int lsm_slot;
+static struct lsm_id *lsm_slotlist[LSMBLOB_ENTRIES];
 
 /**
  * security_add_hooks - Add a modules hooks to the hook lists.
@@ -444,6 +448,7 @@  void __init security_add_hooks(struct security_hook_list *hooks, int count,
 	if (lsmid->slot == LSMBLOB_NEEDED) {
 		if (lsm_slot >= LSMBLOB_ENTRIES)
 			panic("%s Too many LSMs registered.\n", __func__);
+		lsm_slotlist[lsm_slot] = lsmid;
 		lsmid->slot = lsm_slot++;
 		init_debug("%s assigned lsmblob slot %d\n", lsmid->lsm,
 			   lsmid->slot);
@@ -564,6 +569,8 @@  int lsm_inode_alloc(struct inode *inode)
  */
 static int lsm_task_alloc(struct task_struct *task)
 {
+	int *display;
+
 	if (blob_sizes.lbs_task == 0) {
 		task->security = NULL;
 		return 0;
@@ -572,6 +579,15 @@  static int lsm_task_alloc(struct task_struct *task)
 	task->security = kzalloc(blob_sizes.lbs_task, GFP_KERNEL);
 	if (task->security == NULL)
 		return -ENOMEM;
+
+	/*
+	 * The start of the task blob contains the "display" LSM slot number.
+	 * Start with it set to the invalid slot number, indicating that the
+	 * default first registered LSM be displayed.
+	 */
+	display = task->security;
+	*display = LSMBLOB_INVALID;
+
 	return 0;
 }
 
@@ -1563,14 +1579,24 @@  int security_file_open(struct file *file)
 
 int security_task_alloc(struct task_struct *task, unsigned long clone_flags)
 {
+	int *odisplay = current->security;
+	int *ndisplay;
 	int rc = lsm_task_alloc(task);
 
-	if (rc)
+	if (unlikely(rc))
 		return rc;
+
 	rc = call_int_hook(task_alloc, 0, task, clone_flags);
-	if (unlikely(rc))
+	if (unlikely(rc)) {
 		security_task_free(task);
-	return rc;
+		return rc;
+	}
+
+	ndisplay = task->security;
+	if (ndisplay && odisplay)
+		*ndisplay = *odisplay;
+
+	return 0;
 }
 
 void security_task_free(struct task_struct *task)
@@ -1967,10 +1993,29 @@  int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
 				char **value)
 {
 	struct security_hook_list *hp;
+	int *display = current->security;
+	int slot = 0;
+
+	if (!strcmp(name, "display")) {
+		/*
+		 * lsm_slot will be 0 if there are no displaying modules.
+		 */
+		if (lsm_slot == 0)
+			return -EINVAL;
+		if (*display != LSMBLOB_INVALID)
+			slot = *display;
+		*value = kstrdup(lsm_slotlist[slot]->lsm, GFP_KERNEL);
+		if (*value)
+			return strlen(*value);
+		return -ENOMEM;
+	}
 
 	hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) {
 		if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
 			continue;
+		if (lsm == NULL && *display != LSMBLOB_INVALID &&
+		    *display != hp->lsmid->slot)
+			continue;
 		return hp->hook.getprocattr(p, name, value);
 	}
 	return -EINVAL;
@@ -1980,10 +2025,46 @@  int security_setprocattr(const char *lsm, const char *name, void *value,
 			 size_t size)
 {
 	struct security_hook_list *hp;
+	char *term;
+	char *cp;
+	int *display = current->security;
+	int rc = -EINVAL;
+	int slot = 0;
+
+	if (!strcmp(name, "display")) {
+		/*
+		 * lsm_slot will be 0 if there are no displaying modules.
+		 */
+		if (lsm_slot == 0 || size == 0)
+			return -EINVAL;
+		cp = kzalloc(size + 1, GFP_KERNEL);
+		if (cp == NULL)
+			return -ENOMEM;
+		memcpy(cp, value, size);
+
+		term = strchr(cp, ' ');
+		if (term == NULL)
+			term = strchr(cp, '\n');
+		if (term != NULL)
+			*term = '\0';
+
+		for (slot = 0; slot < lsm_slot; slot++)
+			if (!strcmp(cp, lsm_slotlist[slot]->lsm)) {
+				*display = lsm_slotlist[slot]->slot;
+				rc = size;
+				break;
+			}
+
+		kfree(cp);
+		return rc;
+	}
 
 	hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) {
 		if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
 			continue;
+		if (lsm == NULL && *display != LSMBLOB_INVALID &&
+		    *display != hp->lsmid->slot)
+			continue;
 		return hp->hook.setprocattr(name, value, size);
 	}
 	return -EINVAL;
@@ -2003,15 +2084,15 @@  EXPORT_SYMBOL(security_ismaclabel);
 int security_secid_to_secctx(struct lsmblob *blob, char **secdata, u32 *seclen)
 {
 	struct security_hook_list *hp;
-	int rc;
+	int *display = current->security;
 
 	hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) {
 		if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
 			continue;
-		rc = hp->hook.secid_to_secctx(blob->secid[hp->lsmid->slot],
-					      secdata, seclen);
-		if (rc != 0)
-			return rc;
+		if (*display == LSMBLOB_INVALID || *display == hp->lsmid->slot)
+			return hp->hook.secid_to_secctx(
+					blob->secid[hp->lsmid->slot],
+					secdata, seclen);
 	}
 	return 0;
 }
@@ -2021,16 +2102,15 @@  int security_secctx_to_secid(const char *secdata, u32 seclen,
 			     struct lsmblob *blob)
 {
 	struct security_hook_list *hp;
-	int rc;
+	int *display = current->security;
 
 	lsmblob_init(blob, 0);
 	hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid, list) {
 		if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
 			continue;
-		rc = hp->hook.secctx_to_secid(secdata, seclen,
-					      &blob->secid[hp->lsmid->slot]);
-		if (rc != 0)
-			return rc;
+		if (*display == LSMBLOB_INVALID || *display == hp->lsmid->slot)
+			return hp->hook.secctx_to_secid(secdata, seclen,
+						&blob->secid[hp->lsmid->slot]);
 	}
 	return 0;
 }
@@ -2038,7 +2118,15 @@  EXPORT_SYMBOL(security_secctx_to_secid);
 
 void security_release_secctx(char *secdata, u32 seclen)
 {
-	call_void_hook(release_secctx, secdata, seclen);
+	struct security_hook_list *hp;
+	int *display = current->security;
+
+	hlist_for_each_entry(hp, &security_hook_heads.release_secctx, list)
+		if (*display == LSMBLOB_INVALID ||
+		    *display == hp->lsmid->slot) {
+			hp->hook.release_secctx(secdata, seclen);
+			return;
+		}
 }
 EXPORT_SYMBOL(security_release_secctx);
 
@@ -2163,8 +2251,15 @@  EXPORT_SYMBOL(security_sock_rcv_skb);
 int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
 				      int __user *optlen, unsigned len)
 {
-	return call_int_hook(socket_getpeersec_stream, -ENOPROTOOPT, sock,
-				optval, optlen, len);
+	int *display = current->security;
+	struct security_hook_list *hp;
+
+	hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_stream,
+			     list)
+		if (*display == LSMBLOB_INVALID || *display == hp->lsmid->slot)
+			return hp->hook.socket_getpeersec_stream(sock, optval,
+								 optlen, len);
+	return -ENOPROTOOPT;
 }
 
 int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,