Two netlink patches

[Chris Wright]

* Stephen Smalley (sds at epoch.ncsc.mil) wrote:
> On Wed, 2004-12-15 at 17:05, Serge Hallyn wrote:
> <snip>
> +static int cap_netlink_audit_check (struct sk_buff *skb)
> +{
> +	int msgtype = netlink_get_msgtype(skb);
> +
> +	switch(msgtype) {
> +		case 0:  /* not an audit msg */
> +
> +		case AUDIT_GET:
> +		case AUDIT_LIST:
> +			return 0;
> +
> +		case AUDIT_SET:
> +		case AUDIT_USER:
> +		case AUDIT_LOGIN:
> +
> +		case AUDIT_ADD:
> +		case AUDIT_DEL:
> +			if (!capable(CAP_SYS_ADMIN))
> +				return -EPERM;
> +			return 0;
> +
> +		default:  /* permission denied: bad msg */
> +			return msgtype;
> +	}
> <snip>
> 
> Shouldn't this function return -EPERM in the default case, not the
> msgtype?

Should be -EINVAL according to original code.

> Also, do we truly need separate dummy and commoncap implementations, or
> can capability re-use the dummy function (as long as it internally calls
> the top-level capable function)?  Or do you plan on changing that to not
> use the top-level capable function?

I really dislike duplicating code.  I agree it should be put in a
central location.  Does it really need to be broken out into the
security framework?  Why not place it in audit itself?

Just a simple helper:

int audit_netlink_ok(struct nlmsghdr *nlh)
{
	int err = -EINVAL;

	if (audit_bad_header(nlh))
		goto out;

	err = 0;
	switch() {
		ok:
			break;
		capable:
			if (!capable())
				err = -EPERM;
			break;
		default:
			err = -EINVAL;
			break;
	}
out:
	return err;
}

audit_recieve_msg(skb, nlh)
{
	...
	err = audit_netlink_ok(nlh);
	if (err)
		return err;
	...
}

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net