Re: Two netlink patches
* Stephen Smalley (sds(a)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