* module loading permissions and request_module permission inconsistencies @ 2009-08-10 19:45 Eric Paris 2009-08-10 20:23 ` Neil Horman 2009-08-12 23:48 ` Serge E. Hallyn 0 siblings, 2 replies; 6+ messages in thread From: Eric Paris @ 2009-08-10 19:45 UTC (permalink / raw) To: linux-kernel, netdev, linux-security-module Cc: sds, davem, shemminger, kees, morgan, serue, casey, dwlash 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: module loading permissions and request_module permission inconsistencies 2009-08-10 19:45 module loading permissions and request_module permission inconsistencies Eric Paris @ 2009-08-10 20:23 ` Neil Horman 2009-08-10 20:48 ` Eric Paris 2009-08-12 23:48 ` Serge E. Hallyn 1 sibling, 1 reply; 6+ messages in thread From: Neil Horman @ 2009-08-10 20:23 UTC (permalink / raw) To: Eric Paris Cc: linux-kernel, netdev, linux-security-module, sds, davem, shemminger, kees, morgan, serue, casey, dwlash 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 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: module loading permissions and request_module permission inconsistencies 2009-08-10 20:23 ` Neil Horman @ 2009-08-10 20:48 ` Eric Paris 2009-08-10 23:25 ` Neil Horman 0 siblings, 1 reply; 6+ messages in thread From: Eric Paris @ 2009-08-10 20:48 UTC (permalink / raw) To: Neil Horman Cc: linux-kernel, netdev, linux-security-module, sds, davem, shemminger, kees, morgan, serue, casey, dwlash 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: module loading permissions and request_module permission inconsistencies 2009-08-10 20:48 ` Eric Paris @ 2009-08-10 23:25 ` Neil Horman 2009-08-11 1:29 ` Eric Paris 0 siblings, 1 reply; 6+ messages in thread From: Neil Horman @ 2009-08-10 23:25 UTC (permalink / raw) To: Eric Paris Cc: linux-kernel, netdev, linux-security-module, sds, davem, shemminger, kees, morgan, serue, casey, dwlash 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 > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: module loading permissions and request_module permission inconsistencies 2009-08-10 23:25 ` Neil Horman @ 2009-08-11 1:29 ` Eric Paris 0 siblings, 0 replies; 6+ messages in thread From: Eric Paris @ 2009-08-11 1:29 UTC (permalink / raw) To: Neil Horman Cc: linux-kernel, netdev, linux-security-module, sds, davem, shemminger, kees, morgan, serue, casey 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: module loading permissions and request_module permission inconsistencies 2009-08-10 19:45 module loading permissions and request_module permission inconsistencies Eric Paris 2009-08-10 20:23 ` Neil Horman @ 2009-08-12 23:48 ` Serge E. Hallyn 1 sibling, 0 replies; 6+ messages in thread From: Serge E. Hallyn @ 2009-08-12 23:48 UTC (permalink / raw) To: Eric Paris Cc: linux-kernel, netdev, linux-security-module, sds, davem, shemminger, kees, morgan, casey, dwlash 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-08-12 23:48 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-08-10 19:45 module loading permissions and request_module permission inconsistencies Eric Paris 2009-08-10 20:23 ` Neil Horman 2009-08-10 20:48 ` Eric Paris 2009-08-10 23:25 ` Neil Horman 2009-08-11 1:29 ` Eric Paris 2009-08-12 23:48 ` Serge E. Hallyn
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox