Message-ID: <562580AA.4020907@schaufler-ca.com>
Date:	Mon, 19 Oct 2015 16:45:46 -0700
From:	Casey Schaufler <casey@...aufler-ca.com>
To:	Rafal Krypa <r.krypa@...sung.com>
Cc:	Jonathan Corbet <corbet@....net>,
	James Morris <james.l.morris@...cle.com>,
	"Serge E. Hallyn" <serge@...lyn.com>,
	linux-security-module@...r.kernel.org, linux-doc@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Zbigniew Jasinski <z.jasinski@...sung.com>
Subject: Re: [PATCH v5] Smack: limited capability for changing process label

On 10/19/2015 9:23 AM, Rafal Krypa wrote:
> From: Zbigniew Jasinski <z.jasinski@...sung.com>
>
> This feature introduces new kernel interface:
>
> - <smack_fs>/relabel-self - for setting transition labels list
>
> This list is used to control smack label transition mechanism.
> List is set by, and per process. Process can transit to new label only if
> label is on the list. Only process with CAP_MAC_ADMIN capability can add
> labels to this list. With this list, process can change it's label without
> CAP_MAC_ADMIN but only once. After label changing, list is unset.
>
> Changes in v2:
> * use list_for_each_entry instead of _rcu during label write
> * added missing description in security/Smack.txt
>
> Changes in v3:
> * squashed into one commit
>
> Changes in v4:
> * switch from global list to per-task list
> * since the per-task list is accessed only by the task itself
>   there is no need to use synchronization mechanisms on it
>
> Changes in v5:
> * change smackfs interface of relabel-self to the one used for onlycap
>   multiple labels are accepted, separated by space, which
>   replace the previous list upon write
>
> Signed-off-by: Zbigniew Jasinski <z.jasinski@...sung.com>
> Signed-off-by: Rafal Krypa <r.krypa@...sung.com>

Acked-by: Casey Schaufler <casey@...aufler-ca.com>
Applied-to: https://github.com/cschaufler/smack-next.git#smack-for-4.4

> ---
>  Documentation/security/Smack.txt |  10 ++
>  security/smack/smack.h           |   4 +-
>  security/smack/smack_access.c    |   6 +-
>  security/smack/smack_lsm.c       |  57 ++++++++++-
>  security/smack/smackfs.c         | 202 ++++++++++++++++++++++++++++++++-------
>  5 files changed, 238 insertions(+), 41 deletions(-)
>
> diff --git a/Documentation/security/Smack.txt b/Documentation/security/Smack.txt
> index 5e6d07f..945cc63 100644
> --- a/Documentation/security/Smack.txt
> +++ b/Documentation/security/Smack.txt
> @@ -255,6 +255,16 @@ unconfined
>  	the access permitted if it wouldn't be otherwise. Note that this
>  	is dangerous and can ruin the proper labeling of your system.
>  	It should never be used in production.
> +relabel-self
> +	This interface contains a list of labels to which the process can
> +	transition to, by writing to /proc/self/attr/current.
> +	Normally a process can change its own label to any legal value, but only
> +	if it has CAP_MAC_ADMIN. This interface allows a process without
> +	CAP_MAC_ADMIN to relabel itself to one of labels from predefined list.
> +	A process without CAP_MAC_ADMIN can change its label only once. When it
> +	does, this list will be cleared.
> +	The values are set by writing the desired labels, separated
> +	by spaces, to the file or cleared by writing "-" to the file.
>  
>  If you are using the smackload utility
>  you can add access rules in /etc/smack/accesses. They take the form:
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index fff0c61..6c91156 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -115,6 +115,7 @@ struct task_smack {
>  	struct smack_known	*smk_forked;	/* label when forked */
>  	struct list_head	smk_rules;	/* per task access rules */
>  	struct mutex		smk_rules_lock;	/* lock for the rules */
> +	struct list_head	smk_relabel;	/* transit allowed labels */
>  };
>  
>  #define	SMK_INODE_INSTANT	0x01	/* inode is instantiated */
> @@ -169,7 +170,7 @@ struct smk_port_label {
>  };
>  #endif /* SMACK_IPV6_PORT_LABELING */
>  
> -struct smack_onlycap {
> +struct smack_known_list_elem {
>  	struct list_head	list;
>  	struct smack_known	*smk_label;
>  };
> @@ -301,6 +302,7 @@ struct smack_known *smk_import_entry(const char *, int);
>  void smk_insert_entry(struct smack_known *skp);
>  struct smack_known *smk_find_entry(const char *);
>  int smack_privileged(int cap);
> +void smk_destroy_label_list(struct list_head *list);
>  
>  /*
>   * Shared data.
> diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c
> index bc1053f..a283f9e 100644
> --- a/security/smack/smack_access.c
> +++ b/security/smack/smack_access.c
> @@ -637,7 +637,7 @@ DEFINE_MUTEX(smack_onlycap_lock);
>  int smack_privileged(int cap)
>  {
>  	struct smack_known *skp = smk_of_current();
> -	struct smack_onlycap *sop;
> +	struct smack_known_list_elem *sklep;
>  
>  	/*
>  	 * All kernel tasks are privileged
> @@ -654,8 +654,8 @@ int smack_privileged(int cap)
>  		return 1;
>  	}
>  
> -	list_for_each_entry_rcu(sop, &smack_onlycap_list, list) {
> -		if (sop->smk_label == skp) {
> +	list_for_each_entry_rcu(sklep, &smack_onlycap_list, list) {
> +		if (sklep->smk_label == skp) {
>  			rcu_read_unlock();
>  			return 1;
>  		}
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 996c889..036d8b2 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -326,6 +326,7 @@ static struct task_smack *new_task_smack(struct smack_known *task,
>  	tsp->smk_task = task;
>  	tsp->smk_forked = forked;
>  	INIT_LIST_HEAD(&tsp->smk_rules);
> +	INIT_LIST_HEAD(&tsp->smk_relabel);
>  	mutex_init(&tsp->smk_rules_lock);
>  
>  	return tsp;
> @@ -361,6 +362,35 @@ static int smk_copy_rules(struct list_head *nhead, struct list_head *ohead,
>  }
>  
>  /**
> + * smk_copy_relabel - copy smk_relabel labels list
> + * @nhead: new rules header pointer
> + * @ohead: old rules header pointer
> + * @gfp: type of the memory for the allocation
> + *
> + * Returns 0 on success, -ENOMEM on error
> + */
> +static int smk_copy_relabel(struct list_head *nhead, struct list_head *ohead,
> +				gfp_t gfp)
> +{
> +	struct smack_known_list_elem *nklep;
> +	struct smack_known_list_elem *oklep;
> +
> +	INIT_LIST_HEAD(nhead);
> +
> +	list_for_each_entry(oklep, ohead, list) {
> +		nklep = kzalloc(sizeof(struct smack_known_list_elem), gfp);
> +		if (nklep == NULL) {
> +			smk_destroy_label_list(nhead);
> +			return -ENOMEM;
> +		}
> +		nklep->smk_label = oklep->smk_label;
> +		list_add(&nklep->list, nhead);
> +	}
> +
> +	return 0;
> +}
> +
> +/**
>   * smk_ptrace_mode - helper function for converting PTRACE_MODE_* into MAY_*
>   * @mode - input mode in form of PTRACE_MODE_*
>   *
> @@ -1922,6 +1952,8 @@ static void smack_cred_free(struct cred *cred)
>  		return;
>  	cred->security = NULL;
>  
> +	smk_destroy_label_list(&tsp->smk_relabel);
> +
>  	list_for_each_safe(l, n, &tsp->smk_rules) {
>  		rp = list_entry(l, struct smack_rule, list);
>  		list_del(&rp->list);
> @@ -1953,6 +1985,10 @@ static int smack_cred_prepare(struct cred *new, const struct cred *old,
>  	if (rc != 0)
>  		return rc;
>  
> +	rc = smk_copy_relabel(&new_tsp->smk_relabel, &old_tsp->smk_relabel, gfp);
> +	if (rc != 0)
> +		return rc;
> +
>  	new->security = new_tsp;
>  	return 0;
>  }
> @@ -3549,9 +3585,11 @@ static int smack_getprocattr(struct task_struct *p, char *name, char **value)
>  static int smack_setprocattr(struct task_struct *p, char *name,
>  			     void *value, size_t size)
>  {
> -	struct task_smack *tsp;
> +	struct task_smack *tsp = current_security();
>  	struct cred *new;
>  	struct smack_known *skp;
> +	struct smack_known_list_elem *sklep;
> +	int rc;
>  
>  	/*
>  	 * Changing another process' Smack value is too dangerous
> @@ -3560,7 +3598,7 @@ static int smack_setprocattr(struct task_struct *p, char *name,
>  	if (p != current)
>  		return -EPERM;
>  
> -	if (!smack_privileged(CAP_MAC_ADMIN))
> +	if (!smack_privileged(CAP_MAC_ADMIN) && list_empty(&tsp->smk_relabel))
>  		return -EPERM;
>  
>  	if (value == NULL || size == 0 || size >= SMK_LONGLABEL)
> @@ -3579,12 +3617,27 @@ static int smack_setprocattr(struct task_struct *p, char *name,
>  	if (skp == &smack_known_web)
>  		return -EPERM;
>  
> +	if (!smack_privileged(CAP_MAC_ADMIN)) {
> +		rc = -EPERM;
> +		list_for_each_entry(sklep, &tsp->smk_relabel, list)
> +			if (sklep->smk_label == skp) {
> +				rc = 0;
> +				break;
> +			}
> +		if (rc)
> +			return rc;
> +	}
> +
>  	new = prepare_creds();
>  	if (new == NULL)
>  		return -ENOMEM;
>  
>  	tsp = new->security;
>  	tsp->smk_task = skp;
> +	/*
> +	 * process can change its label only once
> +	 */
> +	smk_destroy_label_list(&tsp->smk_relabel);
>  
>  	commit_creds(new);
>  	return size;
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index c20b154..35366ba 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
> @@ -61,6 +61,7 @@ enum smk_inos {
>  #if IS_ENABLED(CONFIG_IPV6)
>  	SMK_NET6ADDR	= 23,	/* single label IPv6 hosts */
>  #endif /* CONFIG_IPV6 */
> +	SMK_RELABEL_SELF = 24, /* relabel possible without CAP_MAC_ADMIN */
>  };
>  
>  /*
> @@ -1914,10 +1915,10 @@ static void *onlycap_seq_next(struct seq_file *s, void *v, loff_t *pos)
>  static int onlycap_seq_show(struct seq_file *s, void *v)
>  {
>  	struct list_head *list = v;
> -	struct smack_onlycap *sop =
> -		list_entry_rcu(list, struct smack_onlycap, list);
> +	struct smack_known_list_elem *sklep =
> +		list_entry_rcu(list, struct smack_known_list_elem, list);
>  
> -	seq_puts(s, sop->smk_label->smk_known);
> +	seq_puts(s, sklep->smk_label->smk_known);
>  	seq_putc(s, ' ');
>  
>  	return 0;
> @@ -1974,6 +1975,54 @@ static void smk_list_swap_rcu(struct list_head *public,
>  }
>  
>  /**
> + * smk_parse_label_list - parse list of Smack labels, separated by spaces
> + *
> + * @data: the string to parse
> + * @private: destination list
> + *
> + * Returns zero on success or error code, as appropriate
> + */
> +static int smk_parse_label_list(char *data, struct list_head *list)
> +{
> +	char *tok;
> +	struct smack_known *skp;
> +	struct smack_known_list_elem *sklep;
> +
> +	while ((tok = strsep(&data, " ")) != NULL) {
> +		if (!*tok)
> +			continue;
> +
> +		skp = smk_import_entry(tok, 0);
> +		if (IS_ERR(skp))
> +			return PTR_ERR(skp);
> +
> +		sklep = kzalloc(sizeof(*sklep), GFP_KERNEL);
> +		if (sklep == NULL)
> +			return -ENOMEM;
> +
> +		sklep->smk_label = skp;
> +		list_add(&sklep->list, list);
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * smk_destroy_label_list - destroy a list of smack_known_list_elem
> + * @head: header pointer of the list to destroy
> + */
> +void smk_destroy_label_list(struct list_head *list)
> +{
> +	struct smack_known_list_elem *sklep;
> +	struct smack_known_list_elem *sklep2;
> +
> +	list_for_each_entry_safe(sklep, sklep2, list, list)
> +		kfree(sklep);
> +
> +	INIT_LIST_HEAD(list);
> +}
> +
> +/**
>   * smk_write_onlycap - write() for smackfs/onlycap
>   * @file: file pointer, not actually used
>   * @buf: where to get the data from
> @@ -1986,13 +2035,8 @@ static ssize_t smk_write_onlycap(struct file *file, const char __user *buf,
>  				 size_t count, loff_t *ppos)
>  {
>  	char *data;
> -	char *data_parse;
> -	char *tok;
> -	struct smack_known *skp;
> -	struct smack_onlycap *sop;
> -	struct smack_onlycap *sop2;
>  	LIST_HEAD(list_tmp);
> -	int rc = count;
> +	int rc;
>  
>  	if (!smack_privileged(CAP_MAC_ADMIN))
>  		return -EPERM;
> @@ -2006,26 +2050,7 @@ static ssize_t smk_write_onlycap(struct file *file, const char __user *buf,
>  		return -EFAULT;
>  	}
>  
> -	data_parse = data;
> -	while ((tok = strsep(&data_parse, " ")) != NULL) {
> -		if (!*tok)
> -			continue;
> -
> -		skp = smk_import_entry(tok, 0);
> -		if (IS_ERR(skp)) {
> -			rc = PTR_ERR(skp);
> -			break;
> -		}
> -
> -		sop = kzalloc(sizeof(*sop), GFP_KERNEL);
> -		if (sop == NULL) {
> -			rc = -ENOMEM;
> -			break;
> -		}
> -
> -		sop->smk_label = skp;
> -		list_add_rcu(&sop->list, &list_tmp);
> -	}
> +	rc = smk_parse_label_list(data, &list_tmp);
>  	kfree(data);
>  
>  	/*
> @@ -2038,17 +2063,14 @@ static ssize_t smk_write_onlycap(struct file *file, const char __user *buf,
>  	 * But do so only on invalid label, not on system errors.
>  	 * The invalid label must be first to count as clearing attempt.
>  	 */
> -	if (rc == -EINVAL && list_empty(&list_tmp))
> -		rc = count;
> -
> -	if (rc >= 0) {
> +	if (!rc || (rc == -EINVAL && list_empty(&list_tmp))) {
>  		mutex_lock(&smack_onlycap_lock);
>  		smk_list_swap_rcu(&smack_onlycap_list, &list_tmp);
>  		mutex_unlock(&smack_onlycap_lock);
> +		rc = count;
>  	}
>  
> -	list_for_each_entry_safe(sop, sop2, &list_tmp, list)
> -		kfree(sop);
> +	smk_destroy_label_list(&list_tmp);
>  
>  	return rc;
>  }
> @@ -2698,6 +2720,113 @@ static const struct file_operations smk_syslog_ops = {
>  	.llseek		= default_llseek,
>  };
>  
> +/*
> + * Seq_file read operations for /smack/relabel-self
> + */
> +
> +static void *relabel_self_seq_start(struct seq_file *s, loff_t *pos)
> +{
> +	struct task_smack *tsp = current_security();
> +
> +	return smk_seq_start(s, pos, &tsp->smk_relabel);
> +}
> +
> +static void *relabel_self_seq_next(struct seq_file *s, void *v, loff_t *pos)
> +{
> +	struct task_smack *tsp = current_security();
> +
> +	return smk_seq_next(s, v, pos, &tsp->smk_relabel);
> +}
> +
> +static int relabel_self_seq_show(struct seq_file *s, void *v)
> +{
> +	struct list_head *list = v;
> +	struct smack_known_list_elem *sklep =
> +		list_entry(list, struct smack_known_list_elem, list);
> +
> +	seq_puts(s, sklep->smk_label->smk_known);
> +	seq_putc(s, ' ');
> +
> +	return 0;
> +}
> +
> +static const struct seq_operations relabel_self_seq_ops = {
> +	.start = relabel_self_seq_start,
> +	.next  = relabel_self_seq_next,
> +	.show  = relabel_self_seq_show,
> +	.stop  = smk_seq_stop,
> +};
> +
> +/**
> + * smk_open_relabel_self - open() for /smack/relabel-self
> + * @inode: inode structure representing file
> + * @file: "relabel-self" file pointer
> + *
> + * Connect our relabel_self_seq_* operations with /smack/relabel-self
> + * file_operations
> + */
> +static int smk_open_relabel_self(struct inode *inode, struct file *file)
> +{
> +	return seq_open(file, &relabel_self_seq_ops);
> +}
> +
> +/**
> + * smk_write_relabel_self - write() for /smack/relabel-self
> + * @file: file pointer, not actually used
> + * @buf: where to get the data from
> + * @count: bytes sent
> + * @ppos: where to start - must be 0
> + *
> + */
> +static ssize_t smk_write_relabel_self(struct file *file, const char __user *buf,
> +				size_t count, loff_t *ppos)
> +{
> +	struct task_smack *tsp = current_security();
> +	char *data;
> +	int rc;
> +	LIST_HEAD(list_tmp);
> +
> +	/*
> +	 * Must have privilege.
> +	 */
> +	if (!smack_privileged(CAP_MAC_ADMIN))
> +		return -EPERM;
> +
> +	/*
> +	 * Enough data must be present.
> +	 */
> +	if (*ppos != 0)
> +		return -EINVAL;
> +
> +	data = kzalloc(count + 1, GFP_KERNEL);
> +	if (data == NULL)
> +		return -ENOMEM;
> +
> +	if (copy_from_user(data, buf, count) != 0) {
> +		kfree(data);
> +		return -EFAULT;
> +	}
> +
> +	rc = smk_parse_label_list(data, &list_tmp);
> +	kfree(data);
> +
> +	if (!rc || (rc == -EINVAL && list_empty(&list_tmp))) {
> +		smk_destroy_label_list(&tsp->smk_relabel);
> +		list_splice(&list_tmp, &tsp->smk_relabel);
> +		return count;
> +	}
> +
> +	smk_destroy_label_list(&list_tmp);
> +	return rc;
> +}
> +
> +static const struct file_operations smk_relabel_self_ops = {
> +	.open		= smk_open_relabel_self,
> +	.read		= seq_read,
> +	.llseek		= seq_lseek,
> +	.write		= smk_write_relabel_self,
> +	.release	= seq_release,
> +};
>  
>  /**
>   * smk_read_ptrace - read() for /smack/ptrace
> @@ -2824,6 +2953,9 @@ static int smk_fill_super(struct super_block *sb, void *data, int silent)
>  		[SMK_NET6ADDR] = {
>  			"ipv6host", &smk_net6addr_ops, S_IRUGO|S_IWUSR},
>  #endif /* CONFIG_IPV6 */
> +		[SMK_RELABEL_SELF] = {
> +			"relabel-self", &smk_relabel_self_ops,
> +				S_IRUGO|S_IWUGO},
>  		/* last one */
>  			{""}
>  	};

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists