Re: [RFC][PATCH v3] selinux: change the handling of unknown classes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thursday 14 January 2010 05:28:10 pm Stephen Smalley wrote:
> If allow_unknown==deny, SELinux treats an undefined kernel security
> class as an error condition rather than as a typical permission denial
> and thus does not allow permissions on undefined classes even when in
> permissive mode.  Change the SELinux logic so that this case is handled
> as a typical permission denial, subject to the usual permissive mode and
> permissive domain handling.
> 
> Also drop the 'requested' argument from security_compute_av() and
> helpers as it is a legacy of the original security server interface and
> is unused.
> 
> Changes:
> - Handle permissive domains consistently by moving up the test for a
> permissive domain.
> - Make security_compute_av_user() consistent with security_compute_av();
> the only difference now is that security_compute_av() performs mapping
> between the kernel-private class and permission indices and the policy
> values.  In the userspace case, this mapping is handled by libselinux.
> - Moved avd_init inside the policy lock.
> 
> Based in part on a patch by Paul Moore <paul.moore@xxxxxx>.
> 
> Reported-by: Andrew Worsley <amworsley@xxxxxxxxx>
> Signed-off-by:  Stephen D. Smalley <sds@xxxxxxxxxxxxx>

Looks good to me ... pick whichever seems most appropriate :)

Reviewed-by: Paul Moore <paul.moore@xxxxxx>
Acked-by: Paul Moore <paul.moore@xxxxxx>

> ---
> 
>  security/selinux/avc.c              |    5
>  security/selinux/include/security.h |   10 -
>  security/selinux/selinuxfs.c        |    7 -
>  security/selinux/ss/services.c      |  186
>  +++++++++++++++--------------------- 4 files changed, 88 insertions(+),
>  120 deletions(-)
> 
> diff --git a/security/selinux/avc.c b/security/selinux/avc.c
> index f2dde26..3ee9b6a 100644
> --- a/security/selinux/avc.c
> +++ b/security/selinux/avc.c
> @@ -746,9 +746,7 @@ int avc_has_perm_noaudit(u32 ssid, u32 tsid,
>  		else
>  			avd = &avd_entry;
> 
> -		rc = security_compute_av(ssid, tsid, tclass, requested, avd);
> -		if (rc)
> -			goto out;
> +		security_compute_av(ssid, tsid, tclass, avd);
>  		rcu_read_lock();
>  		node = avc_insert(ssid, tsid, tclass, avd);
>  	} else {
> @@ -770,7 +768,6 @@ int avc_has_perm_noaudit(u32 ssid, u32 tsid,
>  	}
> 
>  	rcu_read_unlock();
> -out:
>  	return rc;
>  }
> 
> diff --git a/security/selinux/include/security.h
>  b/security/selinux/include/security.h index 2553266..022cf06 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -96,13 +96,11 @@ struct av_decision {
>  /* definitions of av_decision.flags */
>  #define AVD_FLAGS_PERMISSIVE	0x0001
> 
> -int security_compute_av(u32 ssid, u32 tsid,
> -			u16 tclass, u32 requested,
> -			struct av_decision *avd);
> +void security_compute_av(u32 ssid, u32 tsid,
> +			 u16 tclass, struct av_decision *avd);
> 
> -int security_compute_av_user(u32 ssid, u32 tsid,
> -			     u16 tclass, u32 requested,
> -			     struct av_decision *avd);
> +void security_compute_av_user(u32 ssid, u32 tsid,
> +			     u16 tclass, struct av_decision *avd);
> 
>  int security_transition_sid(u32 ssid, u32 tsid,
>  			    u16 tclass, u32 *out_sid);
> diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
> index fab36fd..b7bb0f5 100644
> --- a/security/selinux/selinuxfs.c
> +++ b/security/selinux/selinuxfs.c
> @@ -494,7 +494,6 @@ static ssize_t sel_write_access(struct file *file, 
char
>  *buf, size_t size) char *scon, *tcon;
>  	u32 ssid, tsid;
>  	u16 tclass;
> -	u32 req;
>  	struct av_decision avd;
>  	ssize_t length;
> 
> @@ -512,7 +511,7 @@ static ssize_t sel_write_access(struct file *file, 
char
>  *buf, size_t size) goto out;
> 
>  	length = -EINVAL;
> -	if (sscanf(buf, "%s %s %hu %x", scon, tcon, &tclass, &req) != 4)
> +	if (sscanf(buf, "%s %s %hu", scon, tcon, &tclass) != 3)
>  		goto out2;
> 
>  	length = security_context_to_sid(scon, strlen(scon)+1, &ssid);
> @@ -522,9 +521,7 @@ static ssize_t sel_write_access(struct file *file, 
char
>  *buf, size_t size) if (length < 0)
>  		goto out2;
> 
> -	length = security_compute_av_user(ssid, tsid, tclass, req, &avd);
> -	if (length < 0)
> -		goto out2;
> +	security_compute_av_user(ssid, tsid, tclass, &avd);
> 
>  	length = scnprintf(buf, SIMPLE_TRANSACTION_LIMIT,
>  			  "%x %x %x %x %u %x",
> diff --git a/security/selinux/ss/services.c
>  b/security/selinux/ss/services.c index b3efae2..99e6e9d 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -87,11 +87,10 @@ static u32 latest_granting;
>  static int context_struct_to_string(struct context *context, char
>  **scontext, u32 *scontext_len);
> 
> -static int context_struct_compute_av(struct context *scontext,
> -				     struct context *tcontext,
> -				     u16 tclass,
> -				     u32 requested,
> -				     struct av_decision *avd);
> +static void context_struct_compute_av(struct context *scontext,
> +				      struct context *tcontext,
> +				      u16 tclass,
> +				      struct av_decision *avd);
> 
>  struct selinux_mapping {
>  	u16 value; /* policy value */
> @@ -196,23 +195,6 @@ static u16 unmap_class(u16 tclass)
>  	return tclass;
>  }
> 
> -static u32 unmap_perm(u16 tclass, u32 tperm)
> -{
> -	if (tclass < current_mapping_size) {
> -		unsigned i;
> -		u32 kperm = 0;
> -
> -		for (i = 0; i < current_mapping[tclass].num_perms; i++)
> -			if (tperm & (1<<i)) {
> -				kperm |= current_mapping[tclass].perms[i];
> -				tperm &= ~(1<<i);
> -			}
> -		return kperm;
> -	}
> -
> -	return tperm;
> -}
> -
>  static void map_decision(u16 tclass, struct av_decision *avd,
>  			 int allow_unknown)
>  {
> @@ -532,7 +514,6 @@ out:
>  static void type_attribute_bounds_av(struct context *scontext,
>  				     struct context *tcontext,
>  				     u16 tclass,
> -				     u32 requested,
>  				     struct av_decision *avd)
>  {
>  	struct context lo_scontext;
> @@ -553,7 +534,6 @@ static void type_attribute_bounds_av(struct context
>  *scontext, context_struct_compute_av(&lo_scontext,
>  					  tcontext,
>  					  tclass,
> -					  requested,
>  					  &lo_avd);
>  		if ((lo_avd.allowed & avd->allowed) == avd->allowed)
>  			return;		/* no masked permission */
> @@ -569,7 +549,6 @@ static void type_attribute_bounds_av(struct context
>  *scontext, context_struct_compute_av(scontext,
>  					  &lo_tcontext,
>  					  tclass,
> -					  requested,
>  					  &lo_avd);
>  		if ((lo_avd.allowed & avd->allowed) == avd->allowed)
>  			return;		/* no masked permission */
> @@ -586,7 +565,6 @@ static void type_attribute_bounds_av(struct context
>  *scontext, context_struct_compute_av(&lo_scontext,
>  					  &lo_tcontext,
>  					  tclass,
> -					  requested,
>  					  &lo_avd);
>  		if ((lo_avd.allowed & avd->allowed) == avd->allowed)
>  			return;		/* no masked permission */
> @@ -607,11 +585,10 @@ static void type_attribute_bounds_av(struct context
>  *scontext, * Compute access vectors based on a context structure pair for
>   * the permissions in a particular class.
>   */
> -static int context_struct_compute_av(struct context *scontext,
> -				     struct context *tcontext,
> -				     u16 tclass,
> -				     u32 requested,
> -				     struct av_decision *avd)
> +static void context_struct_compute_av(struct context *scontext,
> +				      struct context *tcontext,
> +				      u16 tclass,
> +				      struct av_decision *avd)
>  {
>  	struct constraint_node *constraint;
>  	struct role_allow *ra;
> @@ -622,19 +599,14 @@ static int context_struct_compute_av(struct context
>  *scontext, struct ebitmap_node *snode, *tnode;
>  	unsigned int i, j;
> 
> -	/*
> -	 * Initialize the access vectors to the default values.
> -	 */
>  	avd->allowed = 0;
>  	avd->auditallow = 0;
>  	avd->auditdeny = 0xffffffff;
> -	avd->seqno = latest_granting;
> -	avd->flags = 0;
> 
>  	if (unlikely(!tclass || tclass > policydb.p_classes.nprim)) {
>  		if (printk_ratelimit())
>  			printk(KERN_WARNING "SELinux:  Invalid class %hu\n", tclass);
> -		return -EINVAL;
> +		return;
>  	}
> 
>  	tclass_datum = policydb.class_val_to_struct[tclass - 1];
> @@ -705,9 +677,7 @@ static int context_struct_compute_av(struct context
>  *scontext, * permission and notice it to userspace via audit.
>  	 */
>  	type_attribute_bounds_av(scontext, tcontext,
> -				 tclass, requested, avd);
> -
> -	return 0;
> +				 tclass, avd);
>  }
> 
>  static int security_validtrans_handle_fail(struct context *ocontext,
> @@ -886,110 +856,116 @@ out:
>  	return rc;
>  }
> 
> -
> -static int security_compute_av_core(u32 ssid,
> -				    u32 tsid,
> -				    u16 tclass,
> -				    u32 requested,
> -				    struct av_decision *avd)
> +static void avd_init(struct av_decision *avd)
>  {
> -	struct context *scontext = NULL, *tcontext = NULL;
> -	int rc = 0;
> -
> -	scontext = sidtab_search(&sidtab, ssid);
> -	if (!scontext) {
> -		printk(KERN_ERR "SELinux: %s:  unrecognized SID %d\n",
> -		       __func__, ssid);
> -		return -EINVAL;
> -	}
> -	tcontext = sidtab_search(&sidtab, tsid);
> -	if (!tcontext) {
> -		printk(KERN_ERR "SELinux: %s:  unrecognized SID %d\n",
> -		       __func__, tsid);
> -		return -EINVAL;
> -	}
> -
> -	rc = context_struct_compute_av(scontext, tcontext, tclass,
> -				       requested, avd);
> -
> -	/* permissive domain? */
> -	if (ebitmap_get_bit(&policydb.permissive_map, scontext->type))
> -		avd->flags |= AVD_FLAGS_PERMISSIVE;
> -
> -	return rc;
> +	avd->allowed = 0;
> +	avd->auditallow = 0;
> +	avd->auditdeny = 0xffffffff;
> +	avd->seqno = latest_granting;
> +	avd->flags = 0;
>  }
> 
> +
>  /**
>   * security_compute_av - Compute access vector decisions.
>   * @ssid: source security identifier
>   * @tsid: target security identifier
>   * @tclass: target security class
> - * @requested: requested permissions
>   * @avd: access vector decisions
>   *
>   * Compute a set of access vector decisions based on the
>   * SID pair (@ssid, @tsid) for the permissions in @tclass.
> - * Return -%EINVAL if any of the parameters are invalid or %0
> - * if the access vector decisions were computed successfully.
>   */
> -int security_compute_av(u32 ssid,
> -			u32 tsid,
> -			u16 orig_tclass,
> -			u32 orig_requested,
> -			struct av_decision *avd)
> +void security_compute_av(u32 ssid,
> +			 u32 tsid,
> +			 u16 orig_tclass,
> +			 struct av_decision *avd)
>  {
>  	u16 tclass;
> -	u32 requested;
> -	int rc;
> +	struct context *scontext = NULL, *tcontext = NULL;
> 
>  	read_lock(&policy_rwlock);
> -
> +	avd_init(avd);
>  	if (!ss_initialized)
>  		goto allow;
> 
> -	requested = unmap_perm(orig_tclass, orig_requested);
> +	scontext = sidtab_search(&sidtab, ssid);
> +	if (!scontext) {
> +		printk(KERN_ERR "SELinux: %s:  unrecognized SID %d\n",
> +		       __func__, ssid);
> +		goto out;
> +	}
> +
> +	/* permissive domain? */
> +	if (ebitmap_get_bit(&policydb.permissive_map, scontext->type))
> +		avd->flags |= AVD_FLAGS_PERMISSIVE;
> +
> +	tcontext = sidtab_search(&sidtab, tsid);
> +	if (!tcontext) {
> +		printk(KERN_ERR "SELinux: %s:  unrecognized SID %d\n",
> +		       __func__, tsid);
> +		goto out;
> +	}
> +
>  	tclass = unmap_class(orig_tclass);
>  	if (unlikely(orig_tclass && !tclass)) {
>  		if (policydb.allow_unknown)
>  			goto allow;
> -		rc = -EINVAL;
>  		goto out;
>  	}
> -	rc = security_compute_av_core(ssid, tsid, tclass, requested, avd);
> +	context_struct_compute_av(scontext, tcontext, tclass, avd);
>  	map_decision(orig_tclass, avd, policydb.allow_unknown);
>  out:
>  	read_unlock(&policy_rwlock);
> -	return rc;
> +	return;
>  allow:
>  	avd->allowed = 0xffffffff;
> -	avd->auditallow = 0;
> -	avd->auditdeny = 0xffffffff;
> -	avd->seqno = latest_granting;
> -	avd->flags = 0;
> -	rc = 0;
>  	goto out;
>  }
> 
> -int security_compute_av_user(u32 ssid,
> -			     u32 tsid,
> -			     u16 tclass,
> -			     u32 requested,
> -			     struct av_decision *avd)
> +void security_compute_av_user(u32 ssid,
> +			      u32 tsid,
> +			      u16 tclass,
> +			      struct av_decision *avd)
>  {
> -	int rc;
> +	struct context *scontext = NULL, *tcontext = NULL;
> 
> -	if (!ss_initialized) {
> -		avd->allowed = 0xffffffff;
> -		avd->auditallow = 0;
> -		avd->auditdeny = 0xffffffff;
> -		avd->seqno = latest_granting;
> -		return 0;
> +	read_lock(&policy_rwlock);
> +	avd_init(avd);
> +	if (!ss_initialized)
> +		goto allow;
> +
> +	scontext = sidtab_search(&sidtab, ssid);
> +	if (!scontext) {
> +		printk(KERN_ERR "SELinux: %s:  unrecognized SID %d\n",
> +		       __func__, ssid);
> +		goto out;
>  	}
> 
> -	read_lock(&policy_rwlock);
> -	rc = security_compute_av_core(ssid, tsid, tclass, requested, avd);
> +	/* permissive domain? */
> +	if (ebitmap_get_bit(&policydb.permissive_map, scontext->type))
> +		avd->flags |= AVD_FLAGS_PERMISSIVE;
> +
> +	tcontext = sidtab_search(&sidtab, tsid);
> +	if (!tcontext) {
> +		printk(KERN_ERR "SELinux: %s:  unrecognized SID %d\n",
> +		       __func__, tsid);
> +		goto out;
> +	}
> +
> +	if (unlikely(!tclass)) {
> +		if (policydb.allow_unknown)
> +			goto allow;
> +		goto out;
> +	}
> +
> +	context_struct_compute_av(scontext, tcontext, tclass, avd);
> + out:
>  	read_unlock(&policy_rwlock);
> -	return rc;
> +	return;
> +allow:
> +	avd->allowed = 0xffffffff;
> +	goto out;
>  }
> 
>  /*
> 

-- 
paul moore
linux @ hp

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@xxxxxxxxxxxxx with
the words "unsubscribe selinux" without quotes as the message.

[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux