Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules

On Thu, 2011-03-24 at 14:03 -0400, Eric Paris wrote:

On Thu, 2011-03-24 at 10:37 -0500, Serge E. Hallyn wrote:

quoted

Quoting Eric Paris (eparis@parisplace.org):

quoted

On Tue, Mar 1, 2011 at 4:33 PM, Vasiliy Kulikov [off-list ref] wrote:

...

quoted

This patch is causing a bit of a problem in Fedora.  The problem lies

Sorry, what exactly is the problem it is causing?  I gather it's
spitting out printks?  What exactly do the printks say?  The patch
included at bottom checks for CAP_NET_ADMIN before checking for
CAP_SYS_MODULE, so these must be cases which historically always
quietly failed, and are now hitting the 'pr_err' which this patch
adds?

Not quite.  SELinux logs every time an operation is denied.  This patch
means that every time a module is requested which does not exist as
netdev-* we check CAP_SYS_MODULE.  SELinux does not allow CAP_SYS_MODULE
and thus we get SELinux complaining that tasks are trying to load
modules.

This is exactly what would have happened before 2.6.32.  Unfortunately
the incorrect behaviour introduced in 2.6.32 (CAP_NET_ADMIN allows you
to load any module installed in the usual place) is now present in
basically every current distribution, and it sounds like some of them
now assume that dev_load() no longer requires CAP_SYS_MODULE.

[...]

I think there are 3 possibilities:

Change SELinux policy so as to not complain when udev/NM/libvirt try to
check CAP_SYS_MODULE, but that's a bad idea, since if they every try to
use init_module(2) we won't get denials.

Change this callsite to a _noaudit check.  Which is better than above
but still not great since we wouldn't get a denial log if anybody had
tried to load xfs....

There are no evil bits in device or module names, so the kernel can't
tell whether the attempt should be logged.  But then, adding some sort
of policy option for whether to audit CAP_SYS_MODULE use here strikes me
as over-engineering.  Just make a decision based on what SELinux users
seem to prefer.

Figure out a way to stop the calls to "reg" "wifi0" and "virbr0" if they
don't exist.

I feel like the last one is the best way, but I don't know what a
solution could look like....

This really has to be done in userland, where these names are being
invented.  Though I suspect the usual way to check whether an interface
exists would be SIOCGIFINDEX, which calls dev_load()!  An alternate
would be to check whether /sys/class/net/<name> exists, but I seem to
recall that /sys/class is somewhat deprecated.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.