Subject: Re: [PATCH] netfilter: xt_condition: add security capability support

From: Luciano Coelho <[email-masked]>
Date: Mon, 23 Aug 2010 21:45:01 +0300

On Mon, 2010-08-23 at 16:34 +0200, ext Jan Engelhardt wrote:
> On Monday 2010-08-23 15:42, Luciano Coelho wrote:
> >> >  /* Defaults, these can be overridden on the module command-line. */
> >> >  static unsigned int condition_list_perms = S_IRUGO | S_IWUSR;
> >> >  static unsigned int condition_uid_perms = 0;
> >> >  static unsigned int condition_gid_perms = 0;
> >> > +static unsigned int condition_capabilities = CAP_NET_ADMIN;
> >> >
> >> It is strange that we set security policy in this way. Maybe the
> >> permission of the proc file is enough in this case.
> >
> >Yes, that is another way to do it.  But in our device we use security
> >capabilities more extensively than normal file permissions.  That's why
> >we need this.
> >
> >If this is too restrictive (ie. having CAP_NET_ADMIN) for most users, we
> >could change the default value to no capabilities needed.  Then we can
> >set CAP_NET_ADMIN when loading the module.
> 
> But it looks as strange as the Yama code attempt.

What is so strange about it? Is it because it's possible to set the
capability requirement from modprobe arguments? The capability check
already exists in at least in nfnetlink, where it checks for received
messages for the CAP_NET_ADMIN capability.  Is it strange because we're
checking for the capability when someone tries to write to a file?

>  This is the one time 
> where I would personally be looking into SELinux, or perhaps SMACK if 
> the former is too complex, to whether _t'ing off procfs is possible.

Yeah, but it's not up to me to decide this.  We have one entire team
dedicated to figuring out how to ensure "security" in our device.  It
was that team who advised us to protect this file by checking for
CAP_NET_ADMIN.

-- 
Cheers,
Luca.