module loading permissions and request_module permission inconsistencies

module loading permissions and request_module permission inconsistencies

[Eric Paris]

I'd like to hear thoughts on how we currently do permissions handling on
request_module() and if it really makes sense?  request_module() is the
function which will do an upcall to try to get modprobe to load a
specified module into the kernel.  It is called in a lot of places
around the kernel (~128).  Of those places only three check to see if
the REQUESTING process has some sort of module loading permissions
(CAP_SYS_RAWIO.)  Those three are in net/core/dev.c::dev_load() and in
the IPv4 tcp congestion code in tcp_set_default_congestion_control() and
tcp_set_congestion_control().  All 125 other calls to request_module()
appear to be done without any permissions check against the triggering
process.  The actual loading of a module is done in another thread which
always has permissions, so that side of things appears to not be an
issue.

First question, why does networking do it's own CAP_SYS_MODULE checks?
(this is VERY old code, pre-git days)  And, does it make sense?  In the
past this has come up in [1] when /sbin/ip triggered the loading of a
module to get IPv6 tunnel support.  It's perfectly reasonable
for /sbin/ip to do this.  But is it reasonable for /sbin/ip to need
CAP_SYS_MODULE?  CAP_SYS_MODULE says that /sbin/ip has permissions to
load any arbitrary binary it feels like as a kernel module directly.  Is
this really what we want?  Should SELinux have to give a hacked /sbin/ip
permissions to load any arbitrary module?  Recently in [2] we find that
now bluetoothd needs to be granted permissions to directly load any
kernel module it pleases, just so it can request the upcall loads bnep.
The same holds basically true for congestion control hooks.  Note that
I'm saying we are giving permission for these to load kernel modules
directly, not just through the upcall.

Nowhere else in the kernel do we do CAP_SYS_MODULE checks on the
triggering side of request_module() and, as I assume at least one of
those allows the user to control the module name, it seems to me that
the current checks are actually lowering the security bar.  We are
granting wide dangerous permissions to binaries when a more directed
permission would be much more sensible.  And we are doing it for no
security gain since I assume we have 10's if not more than 100 other
ways around it.

The second problem is this lack of control over the rest of the users of
request_module().  If we looks at [3] we see that this oversight created
an interesting and useful situation for a xen framebuffer exploit.  They
created an invalid binary and tried to execute it.  This caused the
kernel to enter search_binary_handler() which in turn called
request_module() which triggered the modprobe upcall and the were able
to load a module which they controlled.  As soon as they got their
module into the kernel it was game over for everything.  So we really
should be trying to prevent modules from getting into the kernel.  There
is no security hook on the triggering side and the security hook on the
other side always passes (and needs to always pass)

I recommend we make 2 changes to better our situation:

1) remove CAP_SYS_MODULE from the networking code and instead check
CAP_NET_ADMIN.  Maybe CAP_NET_ADMIN is already being checked and I'll
just remove the capable call altogether but at least I can more
intelligently limit the powers of these processes and they will still be
root limited according to DAC permissions like they are today.

2) Add a new security hook inside request_module().  On a non-selinux
system this would be a noop hook and all 128 callers of request_module()
would perform exactly as they do today.  On SELinux systems I would add
a new permission to see if a process was allowed to trigger a module
load.  This permission would need to be added for things like /sbin/ip
which are supposed to be allowed to trigger module loading, but would
allow us to prevent [3] from taking place.

Please, comments?  Thoughts?  What did I miss?

-Eric

[1] SELinux blocks IPv6 Tunnel
https://bugzilla.redhat.com/show_bug.cgi?id=241401

[2] SELinux blocks bluetooth
https://bugzilla.redhat.com/show_bug.cgi?id=481618

[3] xenfb exploit
http://invisiblethingslab.com/pub/xenfb-adventures-10.pdf
  page 6 section 3.4

Re: module loading permissions and request_module permission inconsistencies

[Neil Horman]

On Mon, Aug 10, 2009 at 03:45:13PM -0400, Eric Paris wrote:
> I'd like to hear thoughts on how we currently do permissions handling on
> request_module() and if it really makes sense?  request_module() is the
> function which will do an upcall to try to get modprobe to load a
> specified module into the kernel.  It is called in a lot of places
> around the kernel (~128).  Of those places only three check to see if
> the REQUESTING process has some sort of module loading permissions
> (CAP_SYS_RAWIO.)  Those three are in net/core/dev.c::dev_load() and in
> the IPv4 tcp congestion code in tcp_set_default_congestion_control() and
> tcp_set_congestion_control().  All 125 other calls to request_module()
> appear to be done without any permissions check against the triggering
> process.  The actual loading of a module is done in another thread which
> always has permissions, so that side of things appears to not be an
> issue.
> 
> First question, why does networking do it's own CAP_SYS_MODULE checks?
> (this is VERY old code, pre-git days)  And, does it make sense?  In the
> past this has come up in [1] when /sbin/ip triggered the loading of a
> module to get IPv6 tunnel support.  It's perfectly reasonable
> for /sbin/ip to do this.  But is it reasonable for /sbin/ip to need
> CAP_SYS_MODULE?  CAP_SYS_MODULE says that /sbin/ip has permissions to
> load any arbitrary binary it feels like as a kernel module directly.  Is
> this really what we want?  Should SELinux have to give a hacked /sbin/ip
> permissions to load any arbitrary module?  Recently in [2] we find that
> now bluetoothd needs to be granted permissions to directly load any
> kernel module it pleases, just so it can request the upcall loads bnep.
> The same holds basically true for congestion control hooks.  Note that
> I'm saying we are giving permission for these to load kernel modules
> directly, not just through the upcall.
> 
> Nowhere else in the kernel do we do CAP_SYS_MODULE checks on the
> triggering side of request_module() and, as I assume at least one of
> those allows the user to control the module name, it seems to me that
> the current checks are actually lowering the security bar.  We are
> granting wide dangerous permissions to binaries when a more directed
> permission would be much more sensible.  And we are doing it for no
> security gain since I assume we have 10's if not more than 100 other
> ways around it.
> 
> The second problem is this lack of control over the rest of the users of
> request_module().  If we looks at [3] we see that this oversight created
> an interesting and useful situation for a xen framebuffer exploit.  They
> created an invalid binary and tried to execute it.  This caused the
> kernel to enter search_binary_handler() which in turn called
> request_module() which triggered the modprobe upcall and the were able
> to load a module which they controlled.  As soon as they got their
> module into the kernel it was game over for everything.  So we really
> should be trying to prevent modules from getting into the kernel.  There
> is no security hook on the triggering side and the security hook on the
> other side always passes (and needs to always pass)
> 
> I recommend we make 2 changes to better our situation:
> 
> 1) remove CAP_SYS_MODULE from the networking code and instead check
> CAP_NET_ADMIN.  Maybe CAP_NET_ADMIN is already being checked and I'll
> just remove the capable call altogether but at least I can more
> intelligently limit the powers of these processes and they will still be
> root limited according to DAC permissions like they are today.
> 
Would this have any adverse effect on how user space sees this working.
Intuitively I would think that if you wanted to load a module (directly or
indirectly, via an iptables command or whatnot), you would need CAP_SYS_MODULE
capabilities on the calling process, not just CAP_NET_ADMIN.  I honestly don't
know the answer here, I'm just raising the question.

> 2) Add a new security hook inside request_module().  On a non-selinux
> system this would be a noop hook and all 128 callers of request_module()
> would perform exactly as they do today.  On SELinux systems I would add
> a new permission to see if a process was allowed to trigger a module
> load.  This permission would need to be added for things like /sbin/ip
> which are supposed to be allowed to trigger module loading, but would
> allow us to prevent [3] from taking place.
> 
This seems perfectly reasonable to me.

Neil

> Please, comments?  Thoughts?  What did I miss?
> 
> -Eric
> 
> [1] SELinux blocks IPv6 Tunnel
> https://bugzilla.redhat.com/show_bug.cgi?id=241401
> 
> [2] SELinux blocks bluetooth
> https://bugzilla.redhat.com/show_bug.cgi?id=481618
> 
> [3] xenfb exploit
> http://invisiblethingslab.com/pub/xenfb-adventures-10.pdf
>   page 6 section 3.4
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: module loading permissions and request_module permission inconsistencies

[Eric Paris]

On Mon, 2009-08-10 at 16:23 -0400, Neil Horman wrote:
> On Mon, Aug 10, 2009 at 03:45:13PM -0400, Eric Paris wrote:

> > 1) remove CAP_SYS_MODULE from the networking code and instead check
> > CAP_NET_ADMIN.  Maybe CAP_NET_ADMIN is already being checked and I'll
> > just remove the capable call altogether but at least I can more
> > intelligently limit the powers of these processes and they will still be
> > root limited according to DAC permissions like they are today.
> > 
> Would this have any adverse effect on how user space sees this working.
> Intuitively I would think that if you wanted to load a module (directly or
> indirectly, via an iptables command or whatnot), you would need CAP_SYS_MODULE
> capabilities on the calling process, not just CAP_NET_ADMIN.  I honestly don't
> know the answer here, I'm just raising the question.

While that might make intuitive sense, it's actually proving to be a bad
idea to use the same capability for direct and indirect module loading
(especially considering we have 125 other places in the kernel where you
can do indirect module loading without any security check)  And believe
me, if someone suggests I move a CAP_SYS_MODULE check down into
__request_module I'll scream about what a horrible idea that is (and
then laugh at them behind their back).

While I think there should be some check in __request_module I don't
think it should be CAP_SYS_MODULE.

CAP_NET_ADMIN at least limits us to root and in all reality to the same
situation everyone is in today.  I just checked every single selinux
domain that grants CAP_SYS_MODULE already grants CAP_NET_ADMIN, so we
can somewhat safely say that nothing (on a fedora system at least) would
break with this change.

-Eric

Re: module loading permissions and request_module permission inconsistencies

[Neil Horman]

On Mon, Aug 10, 2009 at 04:48:59PM -0400, Eric Paris wrote:
> On Mon, 2009-08-10 at 16:23 -0400, Neil Horman wrote:
> > On Mon, Aug 10, 2009 at 03:45:13PM -0400, Eric Paris wrote:
> 
> > > 1) remove CAP_SYS_MODULE from the networking code and instead check
> > > CAP_NET_ADMIN.  Maybe CAP_NET_ADMIN is already being checked and I'll
> > > just remove the capable call altogether but at least I can more
> > > intelligently limit the powers of these processes and they will still be
> > > root limited according to DAC permissions like they are today.
> > > 
> > Would this have any adverse effect on how user space sees this working.
> > Intuitively I would think that if you wanted to load a module (directly or
> > indirectly, via an iptables command or whatnot), you would need CAP_SYS_MODULE
> > capabilities on the calling process, not just CAP_NET_ADMIN.  I honestly don't
> > know the answer here, I'm just raising the question.
> 
> While that might make intuitive sense, it's actually proving to be a bad
> idea to use the same capability for direct and indirect module loading
> (especially considering we have 125 other places in the kernel where you
> can do indirect module loading without any security check)  And believe
> me, if someone suggests I move a CAP_SYS_MODULE check down into
> __request_module I'll scream about what a horrible idea that is (and
> then laugh at them behind their back).
> 
> While I think there should be some check in __request_module I don't
> think it should be CAP_SYS_MODULE.
> 
Forgive my excessive density, but what exactly would be wrong with putting a
CAP_SYS_MODULE check into __request_module?  Is there some sort of implicit
behavioral change that will mess with user space in doing so?

> CAP_NET_ADMIN at least limits us to root and in all reality to the same
> situation everyone is in today.  I just checked every single selinux
> domain that grants CAP_SYS_MODULE already grants CAP_NET_ADMIN, so we
> can somewhat safely say that nothing (on a fedora system at least) would
> break with this change.
> 
Ok, well its definately good that CAP_NET_ADMIN is a superset of CAP_SYS_MODULE
at the moment, but can we guarantee that in the future?  


Neil

> -Eric
> 
> 

Re: module loading permissions and request_module permission inconsistencies

[Eric Paris]

On Mon, 2009-08-10 at 19:25 -0400, Neil Horman wrote:
> On Mon, Aug 10, 2009 at 04:48:59PM -0400, Eric Paris wrote:
> > On Mon, 2009-08-10 at 16:23 -0400, Neil Horman wrote:
> > > On Mon, Aug 10, 2009 at 03:45:13PM -0400, Eric Paris wrote:
> > 
> > > > 1) remove CAP_SYS_MODULE from the networking code and instead check
> > > > CAP_NET_ADMIN.  Maybe CAP_NET_ADMIN is already being checked and I'll
> > > > just remove the capable call altogether but at least I can more
> > > > intelligently limit the powers of these processes and they will still be
> > > > root limited according to DAC permissions like they are today.
> > > > 
> > > Would this have any adverse effect on how user space sees this working.
> > > Intuitively I would think that if you wanted to load a module (directly or
> > > indirectly, via an iptables command or whatnot), you would need CAP_SYS_MODULE
> > > capabilities on the calling process, not just CAP_NET_ADMIN.  I honestly don't
> > > know the answer here, I'm just raising the question.
> > 
> > While that might make intuitive sense, it's actually proving to be a bad
> > idea to use the same capability for direct and indirect module loading
> > (especially considering we have 125 other places in the kernel where you
> > can do indirect module loading without any security check)  And believe
> > me, if someone suggests I move a CAP_SYS_MODULE check down into
> > __request_module I'll scream about what a horrible idea that is (and
> > then laugh at them behind their back).
> > 
> > While I think there should be some check in __request_module I don't
> > think it should be CAP_SYS_MODULE.
> > 
> Forgive my excessive density, but what exactly would be wrong with putting a
> CAP_SYS_MODULE check into __request_module?  Is there some sort of implicit
> behavioral change that will mess with user space in doing so?

Yes I believe it would almost certainly break all of the programs
outside of the 3 networking spots which trigger the kernel to try to
load a module.  Take a look at everything that call request_module().  I
believe this because currently there are no security checks on those 125
call sites and security policy does not grant CAP_SYS_MODULE to many
domains (for good reason, it's a @#$% dangerous permission to give
things.)

If the kernel does go that route it will force the security policy to be
loosened such that all of those programs will have permission to
directly load arbitrary data they can create.  As it stands today,
subverting any of those calling programs 'merely' allows the attacker to
make the kernel load a module using modprobe whose name is sometimes
attacker controlled and sometimes not.  I contend that triggering the
kernel to do an upcall is not the same security behavior as loading my
own data directly into the kernel.

That's the whole problem.  There are separate security concerns between
triggering the loading of a known module and directly loading arbitrary
data.  Pushing a CAP_SYS_MODULE check down into __request_module will
result in a less secure system policy in which the attack space on a
system grows since there would be a significantly increased number of
programs with permission to load data directly into the kernel.

If the community feels that CAP_SYS_MODULE in __request_module is the
right way to go that's fine (but I'm not going to send that patch which
i believe will break systems).  I'll just create new security hooks in
init_module, delete_module, and __request_module, which have no DAC
meaning but which LSMs can implement policy with.  Given those 2/3 new
hooks an LSMs can freely grant CAP_SYS_MODULE like candy, since in my
opinion, it becomes a lot less meaningful and we can still meet
meaningful security goals.

-Eric

Re: module loading permissions and request_module permission inconsistencies

[Serge E. Hallyn]

Quoting Eric Paris (eparis@redhat.com):
> I'd like to hear thoughts on how we currently do permissions handling on
> request_module() and if it really makes sense?  request_module() is the
> function which will do an upcall to try to get modprobe to load a
> specified module into the kernel.  It is called in a lot of places
> around the kernel (~128).  Of those places only three check to see if
> the REQUESTING process has some sort of module loading permissions
> (CAP_SYS_RAWIO.)  Those three are in net/core/dev.c::dev_load() and in
> the IPv4 tcp congestion code in tcp_set_default_congestion_control() and
> tcp_set_congestion_control().  All 125 other calls to request_module()
> appear to be done without any permissions check against the triggering
> process.  The actual loading of a module is done in another thread which
> always has permissions, so that side of things appears to not be an
> issue.
> 
> First question, why does networking do it's own CAP_SYS_MODULE checks?
> (this is VERY old code, pre-git days)  And, does it make sense?  In the
> past this has come up in [1] when /sbin/ip triggered the loading of a
> module to get IPv6 tunnel support.  It's perfectly reasonable
> for /sbin/ip to do this.  But is it reasonable for /sbin/ip to need
> CAP_SYS_MODULE?  CAP_SYS_MODULE says that /sbin/ip has permissions to
> load any arbitrary binary it feels like as a kernel module directly.  Is
> this really what we want?  Should SELinux have to give a hacked /sbin/ip
> permissions to load any arbitrary module?  Recently in [2] we find that
> now bluetoothd needs to be granted permissions to directly load any
> kernel module it pleases, just so it can request the upcall loads bnep.
> The same holds basically true for congestion control hooks.  Note that
> I'm saying we are giving permission for these to load kernel modules
> directly, not just through the upcall.

Right, so taking a more extreme example, the request_module() in
search_binary_handler...  requiring CAP_SYS_MODULE there would mean
you'd have to be privileged to be the first to execute say a
binfmt_misc.

The actual modules are to be protected by protecting /lib/modules
and /sbin/modprobe themselves.  So long as those are properly
protected, the ability to cause a call to __request_module() at most
takes up more memory.

So what you say seems to make sense.

-serge