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

Re: Audit perms check on recv (Re: Two netlink patches)



On Fri, 2004-12-24 at 11:28, Serge E. Hallyn wrote:
> --- linux-2.6.8.1/security/dummy.c	2004-08-14 05:54:51.000000000 -0500
> +++ linux-2.6.8.1-audit/security/dummy.c	2004-12-22 03:01:50.000000000 -0600
> @@ -722,10 +722,12 @@ static int dummy_sem_semop (struct sem_a
>  
>  static int dummy_netlink_send (struct sock *sk, struct sk_buff *skb)
>  {
> +	NETLINK_CB (skb).eff_cap = 0;
>  	if (current->euid == 0)
> -		cap_raise (NETLINK_CB (skb).eff_cap, CAP_NET_ADMIN);
> -	else
> -		NETLINK_CB (skb).eff_cap = 0;
> +		NETLINK_CB (skb).eff_cap |= (~0 & ~CAP_TO_MASK(CAP_SETPCAP)
> +							& ~CAP_FS_MASK);
> +	if (current->fsuid == 0)
> +		NETLINK_CB (skb).eff_cap |= CAP_FS_MASK;
>  	return 0;
>  }

Is this preferable to calling dummy_capget()?  That requires fake
inheritable and permitted capability sets (or changing dummy_capget to
not set them if NULL), but avoids duplicated logic.

> +	tsec = current->security;
> +	err = avc_has_perm_noaudit(tsec->sid, tsec->sid, SECCLASS_CAPABILITY,
> +						~0, &avd);
> +	if (err)
> +		return 0;

This doesn't seem right to me (will return success without masking out
capabilities denied by SELinux if any capabilitites are denied), but on
second glance, neither does my patch (will return failure if any
capabilities are denied).  Correct options would seem to be:
1) Call security_compute_av() instead to compute the access vector,
which returns 0 unless an input argument is invalid and just sets the
access vectors, or
2) Set avd.allowed to 0 prior to the call to avc_has_perm_noaudit(),
then ignore the return value of avc_has_perm_noaudit(), just cast it to
void.

The former is cleaner, but loses the benefit of cached decisions.

-- 
Stephen Smalley <sds epoch ncsc mil>
National Security Agency


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