* module loading with CAP_NET_ADMIN @ 2011-02-24 15:12 Vasiliy Kulikov 2011-02-24 16:34 ` Ben Hutchings 0 siblings, 1 reply; 54+ messages in thread From: Vasiliy Kulikov @ 2011-02-24 15:12 UTC (permalink / raw) To: netdev Cc: linux-kernel, Kees Cook, Eugene Teo, Dan Rosenberg, David S. Miller Hi netdev folks, I'd like to discuss the ability to load any modules from /lib/modules/ by a process with CAP_NET_ADMIN. Since Linux 2.6.32 [1] there is such possibility: root@albatros:~# grep Cap /proc/$$/status CapInh: 0000000000000000 CapPrm: fffffffc00001000 CapEff: fffffffc00001000 CapBnd: fffffffc00001000 root@albatros:~# lsmod | grep xfs root@albatros:~# ifconfig xfs xfs: error fetching interface information: Device not found root@albatros:~# lsmod | grep xfs xfs 767011 0 exportfs 4226 2 xfs,nfsd Ability of CAP_NET_ADMIN to load the driver to work with a particular network device is rational; however, one may load any module not even related to network this way. Hopefully, this is not equal to CAP_SYS_MODULE since the module set is restricted to /lib/modules (additionally may be disabled with /proc/sys/kernel/modules_disabled), but the idea of non-netdev module loading is weird. My proposal is changing request_module("%s", name) to something like request_module("netdev-%s", name) inside of dev_load() and adding aliases to related drivers. This would allow to load only netdev modules via these ioctls. I'm not sure what modules should be patches - at least real physical netdevices have names different from drivers' names, so they don't need patching. I suppose the list is not big. Any comments are welcome. [1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565c Thanks, -- Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: module loading with CAP_NET_ADMIN 2011-02-24 15:12 module loading with CAP_NET_ADMIN Vasiliy Kulikov @ 2011-02-24 16:34 ` Ben Hutchings 2011-02-25 12:30 ` Vasiliy Kulikov 0 siblings, 1 reply; 54+ messages in thread From: Ben Hutchings @ 2011-02-24 16:34 UTC (permalink / raw) To: Vasiliy Kulikov Cc: netdev, linux-kernel, Kees Cook, Eugene Teo, Dan Rosenberg, David S. Miller On Thu, 2011-02-24 at 18:12 +0300, Vasiliy Kulikov wrote: > Hi netdev folks, > > I'd like to discuss the ability to load any modules from /lib/modules/ > by a process with CAP_NET_ADMIN. Since Linux 2.6.32 [1] there is such > possibility: > > root@albatros:~# grep Cap /proc/$$/status > CapInh: 0000000000000000 > CapPrm: fffffffc00001000 > CapEff: fffffffc00001000 > CapBnd: fffffffc00001000 > root@albatros:~# lsmod | grep xfs > root@albatros:~# ifconfig xfs > xfs: error fetching interface information: Device not found > root@albatros:~# lsmod | grep xfs > xfs 767011 0 > exportfs 4226 2 xfs,nfsd Eek! > Ability of CAP_NET_ADMIN to load the driver to work with a particular > network device is rational; however, one may load any module not even > related to network this way. Hopefully, this is not equal to > CAP_SYS_MODULE since the module set is restricted to /lib/modules > (additionally may be disabled with /proc/sys/kernel/modules_disabled), > but the idea of non-netdev module loading is weird. > > My proposal is changing request_module("%s", name) to something like > request_module("netdev-%s", name) inside of dev_load() and adding > aliases to related drivers. AFAIK these interface-name aliases are usually defined by distribution configuration files rather than within the modules themselves. And that behaviour is pretty much obsolete now that we have hotplug and udev. > This would allow to load only netdev > modules via these ioctls. I'm not sure what modules should be patches - > at least real physical netdevices have names different from drivers' > names, so they don't need patching. I suppose the list is not big. The only modules I can see that declare aliases like this are: net/ipv4/ip_gre.c:MODULE_ALIAS("gre0"); net/ipv4/ipip.c:MODULE_ALIAS("tunl0"); net/ipv6/sit.c:MODULE_ALIAS("sit0"); Ben. -- Ben Hutchings, Senior Software Engineer, Solarflare Communications Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked. ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: module loading with CAP_NET_ADMIN 2011-02-24 16:34 ` Ben Hutchings @ 2011-02-25 12:30 ` Vasiliy Kulikov 2011-02-25 15:14 ` [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules Vasiliy Kulikov 2011-02-25 15:29 ` module loading with CAP_NET_ADMIN Michael Tokarev 0 siblings, 2 replies; 54+ messages in thread From: Vasiliy Kulikov @ 2011-02-25 12:30 UTC (permalink / raw) To: Ben Hutchings Cc: netdev, linux-kernel, Kees Cook, Eugene Teo, Dan Rosenberg, David S. Miller On Thu, Feb 24, 2011 at 16:34 +0000, Ben Hutchings wrote: > On Thu, 2011-02-24 at 18:12 +0300, Vasiliy Kulikov wrote: > > My proposal is changing request_module("%s", name) to something like > > request_module("netdev-%s", name) inside of dev_load() and adding > > aliases to related drivers. > > AFAIK these interface-name aliases are usually defined by distribution > configuration files rather than within the modules themselves. And that > behaviour is pretty much obsolete now that we have hotplug and udev. > > > This would allow to load only netdev > > modules via these ioctls. I'm not sure what modules should be patches - > > at least real physical netdevices have names different from drivers' > > names, so they don't need patching. I suppose the list is not big. > > The only modules I can see that declare aliases like this are: > > net/ipv4/ip_gre.c:MODULE_ALIAS("gre0"); > net/ipv4/ipip.c:MODULE_ALIAS("tunl0"); > net/ipv6/sit.c:MODULE_ALIAS("sit0"); Thank you! These are not handled via udev or anything, it is usefull to load it via "ifconfig sit0 up", so they do require patching. I didn't find any modules requiring autoloading except these three. Good candidate should be "isatap0" as it is seen in ip help, but there is no such alias. $ ip tunnel add help 2>&1 | grep mode [ mode { ipip | gre | sit | isatap } ] [ remote ADDR ] [ local ADDR ] I'll send the patch for request_module() and three drivers. Thanks, -- Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments ^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-02-25 12:30 ` Vasiliy Kulikov @ 2011-02-25 15:14 ` Vasiliy Kulikov 2011-02-25 17:25 ` Valdis.Kletnieks 2011-02-25 18:47 ` David Miller 2011-02-25 15:29 ` module loading with CAP_NET_ADMIN Michael Tokarev 1 sibling, 2 replies; 54+ messages in thread From: Vasiliy Kulikov @ 2011-02-25 15:14 UTC (permalink / raw) To: David S. Miller Cc: netdev, linux-kernel, Alexey Kuznetsov, Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI, Patrick McHardy, Eric Dumazet, Tom Herbert, Changli Gao, Jesse Gross Since a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565c any process with CAP_NET_ADMIN may load any module from /lib/modules/. This doesn't mean that CAP_NET_ADMIN is a superset of CAP_SYS_MODULE as modules are limited to /lib/modules/**. However, CAP_NET_ADMIN capability shouldn't allow anybody load any module not related to networking. This patch restricts an ability of autoloading modules to netdev modules with explicit aliases. Currently there are only three users of the feature: ipip, ip_gre and sit. Before the patch: root@albatros:~# grep Cap /proc/$$/status CapInh: 0000000000000000 CapPrm: fffffffc00001000 CapEff: fffffffc00001000 CapBnd: fffffffc00001000 root@albatros:~# lsmod | grep xfs root@albatros:~# ifconfig xfs xfs: error fetching interface information: Device not found root@albatros:~# lsmod | grep xfs xfs 767011 0 exportfs 4226 2 xfs,nfsd After: root@albatros:~# grep Cap /proc/$$/status CapInh: 0000000000000000 CapPrm: ffffffffffffffff CapEff: ffffffffffffffff CapBnd: ffffffffffffffff root@albatros:~# lsmod | grep scsi_wait_scan root@albatros:~# ifconfig scsi_wait_scan scsi_wait_scan: error fetching interface information: Device not found root@albatros:~# lsmod | grep scsi_wait_scan root@albatros:~# modprobe scsi_wait_scan root@albatros:~# lsmod | grep scsi_wait_scan scsi_wait_scan 829 0 Reference: http://www.openwall.com/lists/oss-security/2011/02/24/17 Signed-off-by: Vasiliy Kulikov <segoon@openwall.com> --- include/linux/netdevice.h | 3 +++ net/core/dev.c | 2 +- net/ipv4/ip_gre.c | 2 +- net/ipv4/ipip.c | 2 +- net/ipv6/sit.c | 2 +- 5 files changed, 7 insertions(+), 4 deletions(-) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index d971346..71caf7a 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -2392,6 +2392,9 @@ extern int netdev_notice(const struct net_device *dev, const char *format, ...) extern int netdev_info(const struct net_device *dev, const char *format, ...) __attribute__ ((format (printf, 2, 3))); +#define MODULE_ALIAS_NETDEV(device) \ + MODULE_ALIAS("netdev-" device) + #if defined(DEBUG) #define netdev_dbg(__dev, format, args...) \ netdev_printk(KERN_DEBUG, __dev, format, ##args) diff --git a/net/core/dev.c b/net/core/dev.c index 8ae6631..79b33d9 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -1120,7 +1120,7 @@ void dev_load(struct net *net, const char *name) rcu_read_unlock(); if (!dev && capable(CAP_NET_ADMIN)) - request_module("%s", name); + request_module("netdev-%s", name); } EXPORT_SYMBOL(dev_load); diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c index 6613edf..d1d0e2c 100644 --- a/net/ipv4/ip_gre.c +++ b/net/ipv4/ip_gre.c @@ -1765,4 +1765,4 @@ module_exit(ipgre_fini); MODULE_LICENSE("GPL"); MODULE_ALIAS_RTNL_LINK("gre"); MODULE_ALIAS_RTNL_LINK("gretap"); -MODULE_ALIAS("gre0"); +MODULE_ALIAS_NETDEV("gre0"); diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c index 988f52f..a5f58e7 100644 --- a/net/ipv4/ipip.c +++ b/net/ipv4/ipip.c @@ -913,4 +913,4 @@ static void __exit ipip_fini(void) module_init(ipip_init); module_exit(ipip_fini); MODULE_LICENSE("GPL"); -MODULE_ALIAS("tunl0"); +MODULE_ALIAS_NETDEV("tunl0"); diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c index 8ce38f1..d2c16e1 100644 --- a/net/ipv6/sit.c +++ b/net/ipv6/sit.c @@ -1290,4 +1290,4 @@ static int __init sit_init(void) module_init(sit_init); module_exit(sit_cleanup); MODULE_LICENSE("GPL"); -MODULE_ALIAS("sit0"); +MODULE_ALIAS_NETDEV("sit0"); -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 54+ messages in thread
* Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-02-25 15:14 ` [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules Vasiliy Kulikov @ 2011-02-25 17:25 ` Valdis.Kletnieks 2011-02-25 17:47 ` Vasiliy Kulikov 2011-02-25 17:48 ` Ben Hutchings 2011-02-25 18:47 ` David Miller 1 sibling, 2 replies; 54+ messages in thread From: Valdis.Kletnieks @ 2011-02-25 17:25 UTC (permalink / raw) To: Vasiliy Kulikov Cc: David S. Miller, netdev, linux-kernel, Alexey Kuznetsov, Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI, Patrick McHardy, Eric Dumazet, Tom Herbert, Changli Gao, Jesse Gross [-- Attachment #1: Type: text/plain, Size: 1145 bytes --] On Fri, 25 Feb 2011 18:14:14 +0300, Vasiliy Kulikov said: > Since a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565c any process with > CAP_NET_ADMIN may load any module from /lib/modules/. This doesn't mean > that CAP_NET_ADMIN is a superset of CAP_SYS_MODULE as modules are limited > to /lib/modules/**. However, CAP_NET_ADMIN capability shouldn't allow > anybody load any module not related to networking. > > This patch restricts an ability of autoloading modules to netdev modules > with explicit aliases. Currently there are only three users of the > feature: ipip, ip_gre and sit. And you stop an attacker from simply recompiling the module with a suitable MODULE_ALIAS line added, how, exactly? This patch may make sense down the road, but not while it's still trivial for a malicious root user to drop stuff into /lib/modules. And if you're going the route "but SELinux/SMACK/Tomoyo will prevent a malicious root user from doing that", then the obvious reply is "this should be part of those subsystems rather than something done one-off like this (especially as it has a chance of breaking legitimate setups that use the current scheme). [-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --] ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-02-25 17:25 ` Valdis.Kletnieks @ 2011-02-25 17:47 ` Vasiliy Kulikov 2011-02-25 17:48 ` Ben Hutchings 1 sibling, 0 replies; 54+ messages in thread From: Vasiliy Kulikov @ 2011-02-25 17:47 UTC (permalink / raw) To: Valdis.Kletnieks Cc: David S. Miller, netdev, linux-kernel, Alexey Kuznetsov, Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI, Patrick McHardy, Eric Dumazet, Tom Herbert, Changli Gao, Jesse Gross On Fri, Feb 25, 2011 at 12:25 -0500, Valdis.Kletnieks@vt.edu wrote: > And you stop an attacker from simply recompiling the module with a suitable > MODULE_ALIAS line added, how, exactly? This patch may make sense down the > road, but not while it's still trivial for a malicious root user to drop stuff > into /lib/modules. The threat is not a malicious root, but non-root with CAP_NET_ADMIN. It's hardly possible to load arbitrary module into the kernel having CAP_NET_ADMIN without other capabilities. > And if you're going the route "but SELinux/SMACK/Tomoyo will prevent a malicious > root user from doing that", then the obvious reply is "this should be part of those > subsystems rather than something done one-off like this (especially as it has a chance > of breaking legitimate setups that use the current scheme). No, I don't want to add anything about LSMs at all. Thanks, -- Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-02-25 17:25 ` Valdis.Kletnieks 2011-02-25 17:47 ` Vasiliy Kulikov @ 2011-02-25 17:48 ` Ben Hutchings 1 sibling, 0 replies; 54+ messages in thread From: Ben Hutchings @ 2011-02-25 17:48 UTC (permalink / raw) To: Valdis.Kletnieks Cc: Vasiliy Kulikov, David S. Miller, netdev, linux-kernel, Alexey Kuznetsov, Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI, Patrick McHardy, Eric Dumazet, Tom Herbert, Changli Gao, Jesse Gross On Fri, 2011-02-25 at 12:25 -0500, Valdis.Kletnieks@vt.edu wrote: > On Fri, 25 Feb 2011 18:14:14 +0300, Vasiliy Kulikov said: > > Since a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565c any process with > > CAP_NET_ADMIN may load any module from /lib/modules/. This doesn't mean > > that CAP_NET_ADMIN is a superset of CAP_SYS_MODULE as modules are limited > > to /lib/modules/**. However, CAP_NET_ADMIN capability shouldn't allow > > anybody load any module not related to networking. > > > > This patch restricts an ability of autoloading modules to netdev modules > > with explicit aliases. Currently there are only three users of the > > feature: ipip, ip_gre and sit. > > And you stop an attacker from simply recompiling the module with a suitable > MODULE_ALIAS line added, how, exactly? This patch may make sense down the > road, but not while it's still trivial for a malicious root user to drop stuff > into /lib/modules. A process running as root normally has CAP_NET_ADMIN, but not every process with CAP_NET_ADMIN will be running as root. > And if you're going the route "but SELinux/SMACK/Tomoyo will prevent a malicious > root user from doing that", then the obvious reply is "this should be part of those > subsystems rather than something done one-off like this (especially as it has a chance > of breaking legitimate setups that use the current scheme). The notional attacker has CAP_NET_ADMIN, perhaps through a vulnerable service or a vulnerable set-capability executable. They do not yet have full root access and so cannot install a module, even in the absence of an LSM. So long as the attacker is able to load arbitrary modules, however, they could exploit a vulnerability in any installed (not loaded) module. Again, LSMs are irrelevant to this as they do not protect against kernel bugs. Ben. -- Ben Hutchings, Senior Software Engineer, Solarflare Communications Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked. ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-02-25 15:14 ` [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules Vasiliy Kulikov 2011-02-25 17:25 ` Valdis.Kletnieks @ 2011-02-25 18:47 ` David Miller 2011-02-25 19:02 ` Vasiliy Kulikov 1 sibling, 1 reply; 54+ messages in thread From: David Miller @ 2011-02-25 18:47 UTC (permalink / raw) To: segoon Cc: netdev, linux-kernel, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse From: Vasiliy Kulikov <segoon@openwall.com> Date: Fri, 25 Feb 2011 18:14:14 +0300 > Since a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565c any process with > CAP_NET_ADMIN may load any module from /lib/modules/. This doesn't mean > that CAP_NET_ADMIN is a superset of CAP_SYS_MODULE as modules are limited > to /lib/modules/**. However, CAP_NET_ADMIN capability shouldn't allow > anybody load any module not related to networking. Why go through this naming change, which does break things, instead of simply adding a capability mask tag or similar to modules somehow. You could stick it into a special elf section or similar. Doesn't that make tons more sense than this? ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-02-25 18:47 ` David Miller @ 2011-02-25 19:02 ` Vasiliy Kulikov 2011-02-25 19:05 ` David Miller 0 siblings, 1 reply; 54+ messages in thread From: Vasiliy Kulikov @ 2011-02-25 19:02 UTC (permalink / raw) To: David Miller Cc: netdev, linux-kernel, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, Kees Cook, Eugene Teo, Dan Rosenberg, Andrew Morton On Fri, Feb 25, 2011 at 10:47 -0800, David Miller wrote: > From: Vasiliy Kulikov <segoon@openwall.com> > Date: Fri, 25 Feb 2011 18:14:14 +0300 > > > Since a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565c any process with > > CAP_NET_ADMIN may load any module from /lib/modules/. This doesn't mean > > that CAP_NET_ADMIN is a superset of CAP_SYS_MODULE as modules are limited > > to /lib/modules/**. However, CAP_NET_ADMIN capability shouldn't allow > > anybody load any module not related to networking. > > Why go through this naming change, which does break things, instead of > simply adding a capability mask tag or similar to modules somehow. You > could stick it into a special elf section or similar. > > Doesn't that make tons more sense than this? This is not "simply", adding special section for a single workaround seems like an overkill for me - this touches the core (modules' internals), which is not related to the initial CAP_* problem at all. I'd be happy with not breaking anything, but I don't see any acceptable solution. Thanks, -- Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-02-25 19:02 ` Vasiliy Kulikov @ 2011-02-25 19:05 ` David Miller 2011-02-25 19:07 ` Ben Hutchings 0 siblings, 1 reply; 54+ messages in thread From: David Miller @ 2011-02-25 19:05 UTC (permalink / raw) To: segoon Cc: netdev, linux-kernel, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm From: Vasiliy Kulikov <segoon@openwall.com> Date: Fri, 25 Feb 2011 22:02:05 +0300 > On Fri, Feb 25, 2011 at 10:47 -0800, David Miller wrote: >> From: Vasiliy Kulikov <segoon@openwall.com> >> Date: Fri, 25 Feb 2011 18:14:14 +0300 >> >> > Since a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565c any process with >> > CAP_NET_ADMIN may load any module from /lib/modules/. This doesn't mean >> > that CAP_NET_ADMIN is a superset of CAP_SYS_MODULE as modules are limited >> > to /lib/modules/**. However, CAP_NET_ADMIN capability shouldn't allow >> > anybody load any module not related to networking. >> >> Why go through this naming change, which does break things, instead of >> simply adding a capability mask tag or similar to modules somehow. You >> could stick it into a special elf section or similar. >> >> Doesn't that make tons more sense than this? > > This is not "simply", adding special section for a single workaround > seems like an overkill for me - this touches the core (modules' > internals), which is not related to the initial CAP_* problem at all. > > I'd be happy with not breaking anything, but I don't see any acceptable > solution. I think it's warranted given that it allows us to avoid breaking things. I don't understand there is resistence in response to the first idea I've seen proprosed that actually allows to fix the problem and not break anything at the same time. That seems silly. ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-02-25 19:05 ` David Miller @ 2011-02-25 19:07 ` Ben Hutchings 2011-02-25 19:16 ` David Miller 0 siblings, 1 reply; 54+ messages in thread From: Ben Hutchings @ 2011-02-25 19:07 UTC (permalink / raw) To: David Miller Cc: segoon, netdev, linux-kernel, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm On Fri, 2011-02-25 at 11:05 -0800, David Miller wrote: > From: Vasiliy Kulikov <segoon@openwall.com> > Date: Fri, 25 Feb 2011 22:02:05 +0300 > > > On Fri, Feb 25, 2011 at 10:47 -0800, David Miller wrote: > >> From: Vasiliy Kulikov <segoon@openwall.com> > >> Date: Fri, 25 Feb 2011 18:14:14 +0300 > >> > >> > Since a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565c any process with > >> > CAP_NET_ADMIN may load any module from /lib/modules/. This doesn't mean > >> > that CAP_NET_ADMIN is a superset of CAP_SYS_MODULE as modules are limited > >> > to /lib/modules/**. However, CAP_NET_ADMIN capability shouldn't allow > >> > anybody load any module not related to networking. > >> > >> Why go through this naming change, which does break things, instead of > >> simply adding a capability mask tag or similar to modules somehow. You > >> could stick it into a special elf section or similar. > >> > >> Doesn't that make tons more sense than this? > > > > This is not "simply", adding special section for a single workaround > > seems like an overkill for me - this touches the core (modules' > > internals), which is not related to the initial CAP_* problem at all. > > > > I'd be happy with not breaking anything, but I don't see any acceptable > > solution. > > I think it's warranted given that it allows us to avoid breaking things. > > I don't understand there is resistence in response to the first idea > I've seen proprosed that actually allows to fix the problem and not > break anything at the same time. > > That seems silly. You realise that module loading doesn't actually run in the context of request_module(), right? Ben. -- Ben Hutchings, Senior Software Engineer, Solarflare Communications Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked. ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-02-25 19:07 ` Ben Hutchings @ 2011-02-25 19:16 ` David Miller 2011-02-25 19:30 ` Ben Hutchings 2011-02-27 11:44 ` [PATCH] " Vasiliy Kulikov 0 siblings, 2 replies; 54+ messages in thread From: David Miller @ 2011-02-25 19:16 UTC (permalink / raw) To: bhutchings Cc: segoon, netdev, linux-kernel, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm From: Ben Hutchings <bhutchings@solarflare.com> Date: Fri, 25 Feb 2011 19:07:59 +0000 > You realise that module loading doesn't actually run in the context of > request_module(), right? Why is that a barrier? We could simply pass a capability mask into request_module if necessary. It's an implementation detail, and not a deterrant to my suggested scheme. ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-02-25 19:16 ` David Miller @ 2011-02-25 19:30 ` Ben Hutchings 2011-02-25 19:43 ` David Miller 2011-02-27 11:44 ` [PATCH] " Vasiliy Kulikov 1 sibling, 1 reply; 54+ messages in thread From: Ben Hutchings @ 2011-02-25 19:30 UTC (permalink / raw) To: David Miller Cc: segoon, netdev, linux-kernel, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm On Fri, 2011-02-25 at 11:16 -0800, David Miller wrote: > From: Ben Hutchings <bhutchings@solarflare.com> > Date: Fri, 25 Feb 2011 19:07:59 +0000 > > > You realise that module loading doesn't actually run in the context of > > request_module(), right? > > Why is that a barrier? We could simply pass a capability mask into > request_module if necessary. > > It's an implementation detail, and not a deterrant to my suggested > scheme. It's not an implementation detail. modprobe currently runs with full capabilities; your proposal requires its capabilities to be limited to those of the capabilities of the process that triggered the request_module() (plus, presumably, CAP_SYS_MODULE). Now modprobe doesn't have CAP_DAC_OVERRIDE and can't read modprobe configuration files that belong to users other than root. It doesn't have CAP_SYS_MKNOD so it can't run hooks that call mknod. etc. Ben. -- Ben Hutchings, Senior Software Engineer, Solarflare Communications Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked. ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-02-25 19:30 ` Ben Hutchings @ 2011-02-25 19:43 ` David Miller 2011-02-25 19:53 ` Ben Hutchings 0 siblings, 1 reply; 54+ messages in thread From: David Miller @ 2011-02-25 19:43 UTC (permalink / raw) To: bhutchings Cc: segoon, netdev, linux-kernel, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm From: Ben Hutchings <bhutchings@solarflare.com> Date: Fri, 25 Feb 2011 19:30:16 +0000 > On Fri, 2011-02-25 at 11:16 -0800, David Miller wrote: >> From: Ben Hutchings <bhutchings@solarflare.com> >> Date: Fri, 25 Feb 2011 19:07:59 +0000 >> >> > You realise that module loading doesn't actually run in the context of >> > request_module(), right? >> >> Why is that a barrier? We could simply pass a capability mask into >> request_module if necessary. >> >> It's an implementation detail, and not a deterrant to my suggested >> scheme. > > It's not an implementation detail. modprobe currently runs with full > capabilities; your proposal requires its capabilities to be limited to > those of the capabilities of the process that triggered the > request_module() (plus, presumably, CAP_SYS_MODULE). The idea was that the kernel will be the entity that will inspect the elf sections and validate the capability bits, not the userspace module loader. Surely we if we can pass an arbitrary string out to the loading process as part of the module loading context, we can pass along capability bits as well. ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-02-25 19:43 ` David Miller @ 2011-02-25 19:53 ` Ben Hutchings 2011-02-25 20:37 ` David Miller 2011-02-25 20:38 ` Ben Hutchings 0 siblings, 2 replies; 54+ messages in thread From: Ben Hutchings @ 2011-02-25 19:53 UTC (permalink / raw) To: David Miller Cc: segoon, netdev, linux-kernel, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm On Fri, 2011-02-25 at 11:43 -0800, David Miller wrote: > From: Ben Hutchings <bhutchings@solarflare.com> > Date: Fri, 25 Feb 2011 19:30:16 +0000 > > > On Fri, 2011-02-25 at 11:16 -0800, David Miller wrote: > >> From: Ben Hutchings <bhutchings@solarflare.com> > >> Date: Fri, 25 Feb 2011 19:07:59 +0000 > >> > >> > You realise that module loading doesn't actually run in the context of > >> > request_module(), right? > >> > >> Why is that a barrier? We could simply pass a capability mask into > >> request_module if necessary. > >> > >> It's an implementation detail, and not a deterrant to my suggested > >> scheme. > > > > It's not an implementation detail. modprobe currently runs with full > > capabilities; your proposal requires its capabilities to be limited to > > those of the capabilities of the process that triggered the > > request_module() (plus, presumably, CAP_SYS_MODULE). > > The idea was that the kernel will be the entity that will inspect the > elf sections and validate the capability bits, not the userspace > module loader. Yes, I understand that. > Surely we if we can pass an arbitrary string out to the loading > process as part of the module loading context, we can pass along > capability bits as well. If you want insert_module() to be able to deny loading some modules based on the capabilities of the process calling request_module() then you either have to *reduce* the capabilities given to modprobe or create some extra process state, separate from the usual capability state, specifically for this purpose. Ben. -- Ben Hutchings, Senior Software Engineer, Solarflare Communications Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked. ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-02-25 19:53 ` Ben Hutchings @ 2011-02-25 20:37 ` David Miller 2011-02-25 20:38 ` Ben Hutchings 1 sibling, 0 replies; 54+ messages in thread From: David Miller @ 2011-02-25 20:37 UTC (permalink / raw) To: bhutchings Cc: segoon, netdev, linux-kernel, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm From: Ben Hutchings <bhutchings@solarflare.com> Date: Fri, 25 Feb 2011 19:53:05 +0000 > On Fri, 2011-02-25 at 11:43 -0800, David Miller wrote: >> Surely we if we can pass an arbitrary string out to the loading >> process as part of the module loading context, we can pass along >> capability bits as well. > > If you want insert_module() to be able to deny loading some modules > based on the capabilities of the process calling request_module() then > you either have to *reduce* the capabilities given to modprobe or create > some extra process state, separate from the usual capability state, > specifically for this purpose. How is this any different from the patch posted which ties capabilities to the prefix of name of the module to be loaded? There is simply no difference, except that in my proposal existing things do not break since the module name will not change. I don't see where the complexity is, if the only place we can pass the capability bits is in the execv args, then in the worst case we could take a peek at those in the module load system call. ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-02-25 19:53 ` Ben Hutchings 2011-02-25 20:37 ` David Miller @ 2011-02-25 20:38 ` Ben Hutchings 2011-02-25 20:59 ` Michał Mirosław 1 sibling, 1 reply; 54+ messages in thread From: Ben Hutchings @ 2011-02-25 20:38 UTC (permalink / raw) To: David Miller Cc: segoon, netdev, linux-kernel, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm On Fri, 2011-02-25 at 19:53 +0000, Ben Hutchings wrote: > On Fri, 2011-02-25 at 11:43 -0800, David Miller wrote: > > From: Ben Hutchings <bhutchings@solarflare.com> > > Date: Fri, 25 Feb 2011 19:30:16 +0000 > > > > > On Fri, 2011-02-25 at 11:16 -0800, David Miller wrote: > > >> From: Ben Hutchings <bhutchings@solarflare.com> > > >> Date: Fri, 25 Feb 2011 19:07:59 +0000 > > >> > > >> > You realise that module loading doesn't actually run in the context of > > >> > request_module(), right? > > >> > > >> Why is that a barrier? We could simply pass a capability mask into > > >> request_module if necessary. > > >> > > >> It's an implementation detail, and not a deterrant to my suggested > > >> scheme. > > > > > > It's not an implementation detail. modprobe currently runs with full > > > capabilities; your proposal requires its capabilities to be limited to > > > those of the capabilities of the process that triggered the > > > request_module() (plus, presumably, CAP_SYS_MODULE). > > > > The idea was that the kernel will be the entity that will inspect the > > elf sections and validate the capability bits, not the userspace > > module loader. > > Yes, I understand that. > > > Surely we if we can pass an arbitrary string out to the loading > > process as part of the module loading context, we can pass along > > capability bits as well. > > If you want insert_module() to be able to deny loading some modules > based on the capabilities of the process calling request_module() then > you either have to *reduce* the capabilities given to modprobe or create > some extra process state, separate from the usual capability state, > specifically for this purpose. I bet something like this (plus Vasiliy's changes to static module aliases) would cover 99.9% of legitimate uses of this feature: diff --git a/net/core/dev.c b/net/core/dev.c index 54aaca6..0d09baa 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -1120,8 +1120,20 @@ void dev_load(struct net *net, const char *name) dev = dev_get_by_name_rcu(net, name); rcu_read_unlock(); - if (!dev && capable(CAP_NET_ADMIN)) - request_module("%s", name); + if (!dev && capable(CAP_NET_ADMIN)) { + /* Check whether the name looks like one that a net + * driver will generate initially. If not, require a + * module alias with a suitable prefix, so that this + * can't be used to load arbitrary modules. + */ + if ((strncmp(name, "eth", 3) == 0 && + isdigit((unsigned char)name[3])) || + (strncmp(name, "wlan", 4) == 0 && + isdigit((unsigned char)name[4]))) + request_module("%s", name); + else + request_module("netdev-%s", name); + } } EXPORT_SYMBOL(dev_load); --- Note that we don't have to care about interfaces that get renamed from eth%d or wlan%d, since renaming is triggered asynchronously and therefore can't be used in conjunction with the auto-loading feature. Ben. -- Ben Hutchings, Senior Software Engineer, Solarflare Communications Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked. ^ permalink raw reply related [flat|nested] 54+ messages in thread
* Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-02-25 20:38 ` Ben Hutchings @ 2011-02-25 20:59 ` Michał Mirosław 2011-02-27 20:22 ` Arnd Bergmann 0 siblings, 1 reply; 54+ messages in thread From: Michał Mirosław @ 2011-02-25 20:59 UTC (permalink / raw) To: Ben Hutchings Cc: David Miller, segoon, netdev, linux-kernel, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm 2011/2/25 Ben Hutchings <bhutchings@solarflare.com>: > I bet something like this (plus Vasiliy's changes to static module > aliases) would cover 99.9% of legitimate uses of this feature: > > diff --git a/net/core/dev.c b/net/core/dev.c > index 54aaca6..0d09baa 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -1120,8 +1120,20 @@ void dev_load(struct net *net, const char *name) > dev = dev_get_by_name_rcu(net, name); > rcu_read_unlock(); > > - if (!dev && capable(CAP_NET_ADMIN)) > - request_module("%s", name); > + if (!dev && capable(CAP_NET_ADMIN)) { > + /* Check whether the name looks like one that a net > + * driver will generate initially. If not, require a > + * module alias with a suitable prefix, so that this > + * can't be used to load arbitrary modules. > + */ > + if ((strncmp(name, "eth", 3) == 0 && > + isdigit((unsigned char)name[3])) || > + (strncmp(name, "wlan", 4) == 0 && > + isdigit((unsigned char)name[4]))) > + request_module("%s", name); > + else > + request_module("netdev-%s", name); > + } > } > EXPORT_SYMBOL(dev_load); > This might be better as: if (request_module("netdev-%s", name)) ... fallback Then after some years the fallback could be removed if announced properly. Best Regards, Michał Mirosław ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-02-25 20:59 ` Michał Mirosław @ 2011-02-27 20:22 ` Arnd Bergmann 2011-02-28 9:29 ` Michael Tokarev 0 siblings, 1 reply; 54+ messages in thread From: Arnd Bergmann @ 2011-02-27 20:22 UTC (permalink / raw) To: Michał Mirosław Cc: Ben Hutchings, David Miller, segoon, netdev, linux-kernel, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm On Friday 25 February 2011, Michał Mirosław wrote: > > diff --git a/net/core/dev.c b/net/core/dev.c > > index 54aaca6..0d09baa 100644 > > --- a/net/core/dev.c > > +++ b/net/core/dev.c > > @@ -1120,8 +1120,20 @@ void dev_load(struct net *net, const char *name) > > dev = dev_get_by_name_rcu(net, name); > > rcu_read_unlock(); > > > > - if (!dev && capable(CAP_NET_ADMIN)) > > - request_module("%s", name); > > + if (!dev && capable(CAP_NET_ADMIN)) { > > + /* Check whether the name looks like one that a net > > + * driver will generate initially. If not, require a > > + * module alias with a suitable prefix, so that this > > + * can't be used to load arbitrary modules. > > + */ > > + if ((strncmp(name, "eth", 3) == 0 && > > + isdigit((unsigned char)name[3])) || > > + (strncmp(name, "wlan", 4) == 0 && > > + isdigit((unsigned char)name[4]))) > > + request_module("%s", name); > > + else > > + request_module("netdev-%s", name); > > + } > > } > > EXPORT_SYMBOL(dev_load); > > > > This might be better as: > > if (request_module("netdev-%s", name)) > ... fallback > > Then after some years the fallback could be removed if announced properly. The backwards compatibility should mostly be for systems that today don't use split capabilities, right? The fallback could therefore rely on CAP_SYS_MODULE as well: if (request_module("netdev-%s", name)) { if (capable(CAP_SYS_MODULE)) request_module("%s", name); } Not 100% solution, but should solve the capability escalation nicely without causing much pain. Arnd ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-02-27 20:22 ` Arnd Bergmann @ 2011-02-28 9:29 ` Michael Tokarev 2011-02-28 9:51 ` Vasiliy Kulikov 0 siblings, 1 reply; 54+ messages in thread From: Michael Tokarev @ 2011-02-28 9:29 UTC (permalink / raw) To: Arnd Bergmann Cc: Michał Mirosław, Ben Hutchings, David Miller, segoon, netdev, linux-kernel, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm 27.02.2011 23:22, Arnd Bergmann wrote: > On Friday 25 February 2011, Michał Mirosław wrote: >>> diff --git a/net/core/dev.c b/net/core/dev.c >>> index 54aaca6..0d09baa 100644 >>> --- a/net/core/dev.c >>> +++ b/net/core/dev.c >>> @@ -1120,8 +1120,20 @@ void dev_load(struct net *net, const char *name) >>> dev = dev_get_by_name_rcu(net, name); >>> rcu_read_unlock(); >>> >>> - if (!dev && capable(CAP_NET_ADMIN)) >>> - request_module("%s", name); >>> + if (!dev && capable(CAP_NET_ADMIN)) { >>> + /* Check whether the name looks like one that a net >>> + * driver will generate initially. If not, require a >>> + * module alias with a suitable prefix, so that this >>> + * can't be used to load arbitrary modules. >>> + */ >>> + if ((strncmp(name, "eth", 3) == 0 && >>> + isdigit((unsigned char)name[3])) || >>> + (strncmp(name, "wlan", 4) == 0 && >>> + isdigit((unsigned char)name[4]))) >>> + request_module("%s", name); >>> + else >>> + request_module("netdev-%s", name); >>> + } >>> } >>> EXPORT_SYMBOL(dev_load); >>> >> >> This might be better as: >> >> if (request_module("netdev-%s", name)) >> ... fallback >> >> Then after some years the fallback could be removed if announced properly. > > The backwards compatibility should mostly be for systems that today don't > use split capabilities, right? > > The fallback could therefore rely on CAP_SYS_MODULE as well: > > if (request_module("netdev-%s", name)) { > if (capable(CAP_SYS_MODULE)) > request_module("%s", name); > } > > Not 100% solution, but should solve the capability escalation nicely without > causing much pain. To me this looks like the best solution so far - trivial and compatible. Thanks! /mjt ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-02-28 9:29 ` Michael Tokarev @ 2011-02-28 9:51 ` Vasiliy Kulikov 2011-02-28 19:23 ` David Miller 0 siblings, 1 reply; 54+ messages in thread From: Vasiliy Kulikov @ 2011-02-28 9:51 UTC (permalink / raw) To: Michael Tokarev Cc: Arnd Bergmann, Michał Mirosław, Ben Hutchings, David Miller, netdev, linux-kernel, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm On Mon, Feb 28, 2011 at 12:29 +0300, Michael Tokarev wrote: > 27.02.2011 23:22, Arnd Bergmann wrote: > > The backwards compatibility should mostly be for systems that today don't > > use split capabilities, right? > > > > The fallback could therefore rely on CAP_SYS_MODULE as well: > > > > if (request_module("netdev-%s", name)) { > > if (capable(CAP_SYS_MODULE)) > > request_module("%s", name); > > } > > > > Not 100% solution, but should solve the capability escalation nicely without > > causing much pain. > > To me this looks like the best solution so far - trivial and > compatible. Agreed, it's looks good. But before the request_module() there is a check for capabile(CAP_NET_ADMIN), IMO it's better to request either CAP_NET_ADMIN or CAP_SYS_MODULE, not both of them. if (!dev) { if (capable(CAP_NET_ADMIN)) request_module("netdev-%s", name)) if (capable(CAP_SYS_MODULE) { if (!request_module("%s", name)) WARN_ONE(1, "Loading kernel module for a network device" " with CAP_SYS_MODULE (deprecated). Use CAP_NET_ADMIN and alias" " netdev-%s instead\n", name); } } The only drawback is distributions/setups that already use CAP_SYS_MODULE'less network scripts. David, are you OK with this way? Thanks, -- Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-02-28 9:51 ` Vasiliy Kulikov @ 2011-02-28 19:23 ` David Miller 2011-03-01 19:48 ` [PATCH] net: " Vasiliy Kulikov 0 siblings, 1 reply; 54+ messages in thread From: David Miller @ 2011-02-28 19:23 UTC (permalink / raw) To: segoon Cc: mjt, arnd, mirqus, bhutchings, netdev, linux-kernel, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm From: Vasiliy Kulikov <segoon@openwall.com> Date: Mon, 28 Feb 2011 12:51:33 +0300 > David, are you OK with this way? Sure. ^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-02-28 19:23 ` David Miller @ 2011-03-01 19:48 ` Vasiliy Kulikov 2011-03-01 20:13 ` Ben Hutchings 0 siblings, 1 reply; 54+ messages in thread From: Vasiliy Kulikov @ 2011-03-01 19:48 UTC (permalink / raw) To: linux-kernel Cc: mjt, arnd, mirqus, bhutchings, netdev, David Miller, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm Since a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565c any process with CAP_NET_ADMIN may load any module from /lib/modules/. This doesn't mean that CAP_NET_ADMIN is a superset of CAP_SYS_MODULE as modules are limited to /lib/modules/**. However, CAP_NET_ADMIN capability shouldn't allow anybody load any module not related to networking. This patch restricts an ability of autoloading modules to netdev modules with explicit aliases. This fixes CVE-2011-1019. Arnd Bergmann suggested to leave untouched the old pre-v2.6.32 behavior of loading netdev modules by name (without any prefix) for processes with CAP_SYS_MODULE to maintain the compatibility with network scripts that use autoloading netdev modules by aliases like "eth0", "wlan0". Currently there are only three users of the feature in the upstream kernel: ipip, ip_gre and sit. root@albatros:~# capsh --drop=$(seq -s, 0 11),$(seq -s, 13 34) -- root@albatros:~# grep Cap /proc/$$/status CapInh: 0000000000000000 CapPrm: fffffff800001000 CapEff: fffffff800001000 CapBnd: fffffff800001000 root@albatros:~# modprobe xfs FATAL: Error inserting xfs (/lib/modules/2.6.38-rc6-00001-g2bf4ca3/kernel/fs/xfs/xfs.ko): Operation not permitted root@albatros:~# lsmod | grep xfs root@albatros:~# ifconfig xfs xfs: error fetching interface information: Device not found root@albatros:~# lsmod | grep xfs root@albatros:~# lsmod | grep sit root@albatros:~# ifconfig sit sit: error fetching interface information: Device not found root@albatros:~# lsmod | grep sit root@albatros:~# ifconfig sit0 sit0 Link encap:IPv6-in-IPv4 NOARP MTU:1480 Metric:1 root@albatros:~# lsmod | grep sit sit 10457 0 tunnel4 2957 1 sit For CAP_SYS_MODULE module loading is still relaxed: root@albatros:~# grep Cap /proc/$$/status CapInh: 0000000000000000 CapPrm: ffffffffffffffff CapEff: ffffffffffffffff CapBnd: ffffffffffffffff root@albatros:~# ifconfig xfs xfs: error fetching interface information: Device not found root@albatros:~# lsmod | grep xfs xfs 745319 0 Reference: https://lkml.org/lkml/2011/2/24/203 Signed-off-by: Vasiliy Kulikov <segoon@openwall.com> --- include/linux/netdevice.h | 3 +++ net/core/dev.c | 12 ++++++++++-- net/ipv4/ip_gre.c | 2 +- net/ipv4/ipip.c | 2 +- net/ipv6/sit.c | 2 +- 5 files changed, 16 insertions(+), 5 deletions(-) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index d971346..71caf7a 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -2392,6 +2392,9 @@ extern int netdev_notice(const struct net_device *dev, const char *format, ...) extern int netdev_info(const struct net_device *dev, const char *format, ...) __attribute__ ((format (printf, 2, 3))); +#define MODULE_ALIAS_NETDEV(device) \ + MODULE_ALIAS("netdev-" device) + #if defined(DEBUG) #define netdev_dbg(__dev, format, args...) \ netdev_printk(KERN_DEBUG, __dev, format, ##args) diff --git a/net/core/dev.c b/net/core/dev.c index 8ae6631..fc6f037 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -1119,8 +1119,16 @@ void dev_load(struct net *net, const char *name) dev = dev_get_by_name_rcu(net, name); rcu_read_unlock(); - if (!dev && capable(CAP_NET_ADMIN)) - request_module("%s", name); + if (!dev) { + if (capable(CAP_NET_ADMIN)) + request_module("netdev-%s", name); + if (capable(CAP_SYS_MODULE)) { + if (!request_module("%s", name)) + WARN_ONCE(1, "Loading kernel module for a " +"network device with CAP_SYS_MODULE (deprecated). Use CAP_NET_ADMIN and alias " +"netdev-%s instead\n", name); + } + } } EXPORT_SYMBOL(dev_load); diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c index 6613edf..d1d0e2c 100644 --- a/net/ipv4/ip_gre.c +++ b/net/ipv4/ip_gre.c @@ -1765,4 +1765,4 @@ module_exit(ipgre_fini); MODULE_LICENSE("GPL"); MODULE_ALIAS_RTNL_LINK("gre"); MODULE_ALIAS_RTNL_LINK("gretap"); -MODULE_ALIAS("gre0"); +MODULE_ALIAS_NETDEV("gre0"); diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c index 988f52f..a5f58e7 100644 --- a/net/ipv4/ipip.c +++ b/net/ipv4/ipip.c @@ -913,4 +913,4 @@ static void __exit ipip_fini(void) module_init(ipip_init); module_exit(ipip_fini); MODULE_LICENSE("GPL"); -MODULE_ALIAS("tunl0"); +MODULE_ALIAS_NETDEV("tunl0"); diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c index 8ce38f1..d2c16e1 100644 --- a/net/ipv6/sit.c +++ b/net/ipv6/sit.c @@ -1290,4 +1290,4 @@ static int __init sit_init(void) module_init(sit_init); module_exit(sit_cleanup); MODULE_LICENSE("GPL"); -MODULE_ALIAS("sit0"); +MODULE_ALIAS_NETDEV("sit0"); -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 54+ messages in thread
* Re: [PATCH] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-03-01 19:48 ` [PATCH] net: " Vasiliy Kulikov @ 2011-03-01 20:13 ` Ben Hutchings 2011-03-01 21:33 ` [PATCH v2] " Vasiliy Kulikov 0 siblings, 1 reply; 54+ messages in thread From: Ben Hutchings @ 2011-03-01 20:13 UTC (permalink / raw) To: Vasiliy Kulikov Cc: linux-kernel, mjt, arnd, mirqus, netdev, David Miller, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm On Tue, 2011-03-01 at 22:48 +0300, Vasiliy Kulikov wrote: > Since a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565c any process with > CAP_NET_ADMIN may load any module from /lib/modules/. This doesn't mean > that CAP_NET_ADMIN is a superset of CAP_SYS_MODULE as modules are > limited to /lib/modules/**. However, CAP_NET_ADMIN capability shouldn't > allow anybody load any module not related to networking. > > This patch restricts an ability of autoloading modules to netdev modules > with explicit aliases. This fixes CVE-2011-1019. > > Arnd Bergmann suggested to leave untouched the old pre-v2.6.32 behavior > of loading netdev modules by name (without any prefix) for processes > with CAP_SYS_MODULE to maintain the compatibility with network scripts > that use autoloading netdev modules by aliases like "eth0", "wlan0". [...] > diff --git a/net/core/dev.c b/net/core/dev.c > index 8ae6631..fc6f037 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -1119,8 +1119,16 @@ void dev_load(struct net *net, const char *name) > dev = dev_get_by_name_rcu(net, name); > rcu_read_unlock(); > > - if (!dev && capable(CAP_NET_ADMIN)) > - request_module("%s", name); > + if (!dev) { > + if (capable(CAP_NET_ADMIN)) > + request_module("netdev-%s", name); If this succeeds then the second request_module() should be skipped. > + if (capable(CAP_SYS_MODULE)) { > + if (!request_module("%s", name)) > + WARN_ONCE(1, "Loading kernel module for a " > +"network device with CAP_SYS_MODULE (deprecated). Use CAP_NET_ADMIN and alias " > +"netdev-%s instead\n", name); [...] If this feature is to be deprecated, there should be an error message for each interface that depends on it. However, use of the feature is not a bug so WARN is not appropriate. I think pr_err() would be fine. Ben. -- Ben Hutchings, Senior Software Engineer, Solarflare Communications Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked. ^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-03-01 20:13 ` Ben Hutchings @ 2011-03-01 21:33 ` Vasiliy Kulikov 2011-03-02 7:15 ` Michael Tokarev ` (3 more replies) 0 siblings, 4 replies; 54+ messages in thread From: Vasiliy Kulikov @ 2011-03-01 21:33 UTC (permalink / raw) To: linux-kernel Cc: mjt, arnd, mirqus, netdev, Ben Hutchings, David Miller, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm Since a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565c any process with CAP_NET_ADMIN may load any module from /lib/modules/. This doesn't mean that CAP_NET_ADMIN is a superset of CAP_SYS_MODULE as modules are limited to /lib/modules/**. However, CAP_NET_ADMIN capability shouldn't allow anybody load any module not related to networking. This patch restricts an ability of autoloading modules to netdev modules with explicit aliases. This fixes CVE-2011-1019. Arnd Bergmann suggested to leave untouched the old pre-v2.6.32 behavior of loading netdev modules by name (without any prefix) for processes with CAP_SYS_MODULE to maintain the compatibility with network scripts that use autoloading netdev modules by aliases like "eth0", "wlan0". Currently there are only three users of the feature in the upstream kernel: ipip, ip_gre and sit. root@albatros:~# capsh --drop=$(seq -s, 0 11),$(seq -s, 13 34) -- root@albatros:~# grep Cap /proc/$$/status CapInh: 0000000000000000 CapPrm: fffffff800001000 CapEff: fffffff800001000 CapBnd: fffffff800001000 root@albatros:~# modprobe xfs FATAL: Error inserting xfs (/lib/modules/2.6.38-rc6-00001-g2bf4ca3/kernel/fs/xfs/xfs.ko): Operation not permitted root@albatros:~# lsmod | grep xfs root@albatros:~# ifconfig xfs xfs: error fetching interface information: Device not found root@albatros:~# lsmod | grep xfs root@albatros:~# lsmod | grep sit root@albatros:~# ifconfig sit sit: error fetching interface information: Device not found root@albatros:~# lsmod | grep sit root@albatros:~# ifconfig sit0 sit0 Link encap:IPv6-in-IPv4 NOARP MTU:1480 Metric:1 root@albatros:~# lsmod | grep sit sit 10457 0 tunnel4 2957 1 sit For CAP_SYS_MODULE module loading is still relaxed: root@albatros:~# grep Cap /proc/$$/status CapInh: 0000000000000000 CapPrm: ffffffffffffffff CapEff: ffffffffffffffff CapBnd: ffffffffffffffff root@albatros:~# ifconfig xfs xfs: error fetching interface information: Device not found root@albatros:~# lsmod | grep xfs xfs 745319 0 Reference: https://lkml.org/lkml/2011/2/24/203 Signed-off-by: Vasiliy Kulikov <segoon@openwall.com> --- v2 - use pr_err() - don't try to load $name if netdev-$name is loaded include/linux/netdevice.h | 3 +++ net/core/dev.c | 12 ++++++++++-- net/ipv4/ip_gre.c | 2 +- net/ipv4/ipip.c | 2 +- net/ipv6/sit.c | 2 +- 5 files changed, 16 insertions(+), 5 deletions(-) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index d971346..71caf7a 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -2392,6 +2392,9 @@ extern int netdev_notice(const struct net_device *dev, const char *format, ...) extern int netdev_info(const struct net_device *dev, const char *format, ...) __attribute__ ((format (printf, 2, 3))); +#define MODULE_ALIAS_NETDEV(device) \ + MODULE_ALIAS("netdev-" device) + #if defined(DEBUG) #define netdev_dbg(__dev, format, args...) \ netdev_printk(KERN_DEBUG, __dev, format, ##args) diff --git a/net/core/dev.c b/net/core/dev.c index 8ae6631..6561021 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -1114,13 +1114,21 @@ EXPORT_SYMBOL(netdev_bonding_change); void dev_load(struct net *net, const char *name) { struct net_device *dev; + int no_module; rcu_read_lock(); dev = dev_get_by_name_rcu(net, name); rcu_read_unlock(); - if (!dev && capable(CAP_NET_ADMIN)) - request_module("%s", name); + no_module = !dev; + if (no_module && capable(CAP_NET_ADMIN)) + no_module = request_module("netdev-%s", name); + if (no_module && capable(CAP_SYS_MODULE)) { + if (!request_module("%s", name)) + pr_err("Loading kernel module for a network device " +"with CAP_SYS_MODULE (deprecated). Use CAP_NET_ADMIN and alias netdev-%s " +"instead\n", name); + } } EXPORT_SYMBOL(dev_load); diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c index 6613edf..d1d0e2c 100644 --- a/net/ipv4/ip_gre.c +++ b/net/ipv4/ip_gre.c @@ -1765,4 +1765,4 @@ module_exit(ipgre_fini); MODULE_LICENSE("GPL"); MODULE_ALIAS_RTNL_LINK("gre"); MODULE_ALIAS_RTNL_LINK("gretap"); -MODULE_ALIAS("gre0"); +MODULE_ALIAS_NETDEV("gre0"); diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c index 988f52f..a5f58e7 100644 --- a/net/ipv4/ipip.c +++ b/net/ipv4/ipip.c @@ -913,4 +913,4 @@ static void __exit ipip_fini(void) module_init(ipip_init); module_exit(ipip_fini); MODULE_LICENSE("GPL"); -MODULE_ALIAS("tunl0"); +MODULE_ALIAS_NETDEV("tunl0"); diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c index 8ce38f1..d2c16e1 100644 --- a/net/ipv6/sit.c +++ b/net/ipv6/sit.c @@ -1290,4 +1290,4 @@ static int __init sit_init(void) module_init(sit_init); module_exit(sit_cleanup); MODULE_LICENSE("GPL"); -MODULE_ALIAS("sit0"); +MODULE_ALIAS_NETDEV("sit0"); -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 54+ messages in thread
* Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-03-01 21:33 ` [PATCH v2] " Vasiliy Kulikov @ 2011-03-02 7:15 ` Michael Tokarev 2011-03-09 22:06 ` Vasiliy Kulikov 2011-03-02 16:01 ` Kees Cook ` (2 subsequent siblings) 3 siblings, 1 reply; 54+ messages in thread From: Michael Tokarev @ 2011-03-02 7:15 UTC (permalink / raw) To: Vasiliy Kulikov Cc: linux-kernel, arnd, mirqus, netdev, Ben Hutchings, David Miller, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm 02.03.2011 00:33, Vasiliy Kulikov wrote: > Since a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565c any process with > CAP_NET_ADMIN may load any module from /lib/modules/. This doesn't mean > that CAP_NET_ADMIN is a superset of CAP_SYS_MODULE as modules are > limited to /lib/modules/**. However, CAP_NET_ADMIN capability shouldn't > allow anybody load any module not related to networking. > > This patch restricts an ability of autoloading modules to netdev modules > with explicit aliases. This fixes CVE-2011-1019. [] > Reference: https://lkml.org/lkml/2011/2/24/203 > > Signed-off-by: Vasiliy Kulikov <segoon@openwall.com> This looks much saner :) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> /mjt ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-03-02 7:15 ` Michael Tokarev @ 2011-03-09 22:06 ` Vasiliy Kulikov 2011-03-09 22:09 ` David Miller 0 siblings, 1 reply; 54+ messages in thread From: Vasiliy Kulikov @ 2011-03-09 22:06 UTC (permalink / raw) To: Michael Tokarev Cc: linux-kernel, arnd, mirqus, netdev, Ben Hutchings, David Miller, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg On Wed, Mar 02, 2011 at 10:15 +0300, Michael Tokarev wrote: > 02.03.2011 00:33, Vasiliy Kulikov wrote: > > Since a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565c any process with > > CAP_NET_ADMIN may load any module from /lib/modules/. This doesn't mean > > that CAP_NET_ADMIN is a superset of CAP_SYS_MODULE as modules are > > limited to /lib/modules/**. However, CAP_NET_ADMIN capability shouldn't > > allow anybody load any module not related to networking. > > > > This patch restricts an ability of autoloading modules to netdev modules > > with explicit aliases. This fixes CVE-2011-1019. > [] > > Reference: https://lkml.org/lkml/2011/2/24/203 > > > > Signed-off-by: Vasiliy Kulikov <segoon@openwall.com> > > This looks much saner :) > > Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> Is there any reason why it is not yet merged? I've answered to Jake Edge that it is not a regression. Thanks, -- Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-03-09 22:06 ` Vasiliy Kulikov @ 2011-03-09 22:09 ` David Miller 2011-03-09 22:53 ` James Morris 0 siblings, 1 reply; 54+ messages in thread From: David Miller @ 2011-03-09 22:09 UTC (permalink / raw) To: segoon Cc: mjt, linux-kernel, arnd, mirqus, netdev, bhutchings, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg From: Vasiliy Kulikov <segoon@openwall.com> Date: Thu, 10 Mar 2011 01:06:34 +0300 > On Wed, Mar 02, 2011 at 10:15 +0300, Michael Tokarev wrote: >> 02.03.2011 00:33, Vasiliy Kulikov wrote: >> > Since a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565c any process with >> > CAP_NET_ADMIN may load any module from /lib/modules/. This doesn't mean >> > that CAP_NET_ADMIN is a superset of CAP_SYS_MODULE as modules are >> > limited to /lib/modules/**. However, CAP_NET_ADMIN capability shouldn't >> > allow anybody load any module not related to networking. >> > >> > This patch restricts an ability of autoloading modules to netdev modules >> > with explicit aliases. This fixes CVE-2011-1019. >> [] >> > Reference: https://lkml.org/lkml/2011/2/24/203 >> > >> > Signed-off-by: Vasiliy Kulikov <segoon@openwall.com> >> >> This looks much saner :) >> >> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> > > Is there any reason why it is not yet merged? I've answered to Jake > Edge that it is not a regression. I was hoping someone other than me would take this upstream, feel free to submit it directly to Linus with my ack: Acked-by: David S. Miller <davem@davemloft.net> ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-03-09 22:09 ` David Miller @ 2011-03-09 22:53 ` James Morris 2011-03-10 9:49 ` Vasiliy Kulikov 0 siblings, 1 reply; 54+ messages in thread From: James Morris @ 2011-03-09 22:53 UTC (permalink / raw) To: David Miller Cc: segoon, mjt, linux-kernel, arnd, mirqus, netdev, bhutchings, kuznet, pekkas, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg On Wed, 9 Mar 2011, David Miller wrote: > I was hoping someone other than me would take this upstream, feel free > to submit it directly to Linus with my ack: > > Acked-by: David S. Miller <davem@davemloft.net> I can submit it via my tree. -- James Morris <jmorris@namei.org> ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-03-09 22:53 ` James Morris @ 2011-03-10 9:49 ` Vasiliy Kulikov 0 siblings, 0 replies; 54+ messages in thread From: Vasiliy Kulikov @ 2011-03-10 9:49 UTC (permalink / raw) To: James Morris Cc: David Miller, mjt, linux-kernel, arnd, mirqus, netdev, bhutchings, kuznet, pekkas, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg James, On Thu, Mar 10, 2011 at 09:53 +1100, James Morris wrote: > I can submit it via my tree. Thank you! -- Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-03-01 21:33 ` [PATCH v2] " Vasiliy Kulikov 2011-03-02 7:15 ` Michael Tokarev @ 2011-03-02 16:01 ` Kees Cook 2011-03-02 19:39 ` Jake Edge 2011-03-22 20:47 ` Eric Paris 3 siblings, 0 replies; 54+ messages in thread From: Kees Cook @ 2011-03-02 16:01 UTC (permalink / raw) To: Vasiliy Kulikov Cc: linux-kernel, mjt, arnd, mirqus, netdev, Ben Hutchings, David Miller, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, eugene, dan.j.rosenberg, akpm On Wed, Mar 02, 2011 at 12:33:13AM +0300, Vasiliy Kulikov wrote: > This patch restricts an ability of autoloading modules to netdev modules > with explicit aliases. This fixes CVE-2011-1019. > ... > Signed-off-by: Vasiliy Kulikov <segoon@openwall.com> Looks good; thanks for sorting this out. Acked-by: Kees Cook <kees.cook@canonical.com> -Kees -- Kees Cook Ubuntu Security Team ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-03-01 21:33 ` [PATCH v2] " Vasiliy Kulikov 2011-03-02 7:15 ` Michael Tokarev 2011-03-02 16:01 ` Kees Cook @ 2011-03-02 19:39 ` Jake Edge 2011-03-02 19:43 ` Vasiliy Kulikov 2011-03-22 20:47 ` Eric Paris 3 siblings, 1 reply; 54+ messages in thread From: Jake Edge @ 2011-03-02 19:39 UTC (permalink / raw) To: Vasiliy Kulikov Cc: linux-kernel, mjt, arnd, mirqus, netdev, Ben Hutchings, David Miller, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm I am probably missing something, but shouldn't the existing MODULE_ALIASes stay? Vasiliy Kulikov <segoon@openwall.com> writes: > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c > index 6613edf..d1d0e2c 100644 > --- a/net/ipv4/ip_gre.c > +++ b/net/ipv4/ip_gre.c > @@ -1765,4 +1765,4 @@ module_exit(ipgre_fini); > MODULE_LICENSE("GPL"); > MODULE_ALIAS_RTNL_LINK("gre"); > MODULE_ALIAS_RTNL_LINK("gretap"); > -MODULE_ALIAS("gre0"); > +MODULE_ALIAS_NETDEV("gre0"); that is, instead of replacing MODULE_ALIAS("gre0") with the NETDEV version, don't you want both for backward compatibility? (if so, same goes for the other two) jake -- Jake Edge - jake@lwn.net - http://lwn.net ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-03-02 19:39 ` Jake Edge @ 2011-03-02 19:43 ` Vasiliy Kulikov 2011-03-02 19:49 ` Jake Edge 0 siblings, 1 reply; 54+ messages in thread From: Vasiliy Kulikov @ 2011-03-02 19:43 UTC (permalink / raw) To: Jake Edge Cc: linux-kernel, mjt, arnd, mirqus, netdev, Ben Hutchings, David Miller, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm On Wed, Mar 02, 2011 at 12:39 -0700, Jake Edge wrote: > > @@ -1765,4 +1765,4 @@ module_exit(ipgre_fini); > > MODULE_LICENSE("GPL"); > > MODULE_ALIAS_RTNL_LINK("gre"); > > MODULE_ALIAS_RTNL_LINK("gretap"); > > -MODULE_ALIAS("gre0"); > > +MODULE_ALIAS_NETDEV("gre0"); > > that is, instead of replacing MODULE_ALIAS("gre0") with the NETDEV > version, don't you want both for backward compatibility? The networking script will run with CAP_NET_ADMIN, this would request netdev-gre0 and load ip_gre. -- Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-03-02 19:43 ` Vasiliy Kulikov @ 2011-03-02 19:49 ` Jake Edge 2011-03-02 20:18 ` Vasiliy Kulikov 0 siblings, 1 reply; 54+ messages in thread From: Jake Edge @ 2011-03-02 19:49 UTC (permalink / raw) To: Vasiliy Kulikov Cc: linux-kernel, mjt, arnd, mirqus, netdev, Ben Hutchings, David Miller, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm On Wed, 2 Mar 2011 22:43:54 +0300 Vasiliy Kulikov wrote: > On Wed, Mar 02, 2011 at 12:39 -0700, Jake Edge wrote: > > > @@ -1765,4 +1765,4 @@ module_exit(ipgre_fini); > > > MODULE_LICENSE("GPL"); > > > MODULE_ALIAS_RTNL_LINK("gre"); > > > MODULE_ALIAS_RTNL_LINK("gretap"); > > > -MODULE_ALIAS("gre0"); > > > +MODULE_ALIAS_NETDEV("gre0"); > > > > that is, instead of replacing MODULE_ALIAS("gre0") with the NETDEV > > version, don't you want both for backward compatibility? > > The networking script will run with CAP_NET_ADMIN, this would request > netdev-gre0 and load ip_gre. and on systems that today use CAP_SYS_MODULE (or really the full set of capabilities cuz they are running as root)? won't those try to load gre0 and fail because that alias doesn't exist? jake -- Jake Edge - LWN - jake@lwn.net - http://lwn.net ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-03-02 19:49 ` Jake Edge @ 2011-03-02 20:18 ` Vasiliy Kulikov 2011-03-02 20:38 ` Jake Edge 2011-03-02 20:40 ` Jake Edge 0 siblings, 2 replies; 54+ messages in thread From: Vasiliy Kulikov @ 2011-03-02 20:18 UTC (permalink / raw) To: Jake Edge Cc: linux-kernel, mjt, arnd, mirqus, netdev, Ben Hutchings, David Miller, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm On Wed, Mar 02, 2011 at 12:49 -0700, Jake Edge wrote: > On Wed, 2 Mar 2011 22:43:54 +0300 Vasiliy Kulikov wrote: > > On Wed, Mar 02, 2011 at 12:39 -0700, Jake Edge wrote: > > > > @@ -1765,4 +1765,4 @@ module_exit(ipgre_fini); > > > > MODULE_LICENSE("GPL"); > > > > MODULE_ALIAS_RTNL_LINK("gre"); > > > > MODULE_ALIAS_RTNL_LINK("gretap"); > > > > -MODULE_ALIAS("gre0"); > > > > +MODULE_ALIAS_NETDEV("gre0"); > > > > > > that is, instead of replacing MODULE_ALIAS("gre0") with the NETDEV > > > version, don't you want both for backward compatibility? > > > > The networking script will run with CAP_NET_ADMIN, this would request > > netdev-gre0 and load ip_gre. > > and on systems that today use CAP_SYS_MODULE Since Linux 2.6.32 CAP_SYS_MODULE may not load modules via "ifconfig gre0". It was changed to CAP_NET_ADMIN. So nothing is broken here. > (or really the full set of > capabilities cuz they are running as root)? As root has CAP_NET_ADMIN, the alias netdev-gre0 is tried and it succeeds. -- Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-03-02 20:18 ` Vasiliy Kulikov @ 2011-03-02 20:38 ` Jake Edge 2011-03-02 20:40 ` Jake Edge 1 sibling, 0 replies; 54+ messages in thread From: Jake Edge @ 2011-03-02 20:38 UTC (permalink / raw) To: Vasiliy Kulikov Cc: linux-kernel, mjt, arnd, mirqus, netdev, Ben Hutchings, David Miller, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm On Wed, 2 Mar 2011 23:18:07 +0300 Vasiliy Kulikov wrote: > > and on systems that today use CAP_SYS_MODULE > > Since Linux 2.6.32 CAP_SYS_MODULE may not load modules via "ifconfig > gre0". It was changed to CAP_NET_ADMIN. So nothing is broken here. > > > (or really the full set of > > capabilities cuz they are running as root)? > > As root has CAP_NET_ADMIN, the alias netdev-gre0 is tried and it > succeeds. (I feel like I'm beating a dead horse here, sorry if so ...) If I have a setuid-root program today that loads ip_gre by using the alias "gre0", and I run that program on a kernel with this change, won't it fail because the "gre0" alias is missing? That program doesn't know to try "netdev-gre0". i.e. won't backward compatibility be affected by this change? jake -- Jake Edge - LWN - jake@lwn.net - http://lwn.net ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-03-02 20:18 ` Vasiliy Kulikov 2011-03-02 20:38 ` Jake Edge @ 2011-03-02 20:40 ` Jake Edge 1 sibling, 0 replies; 54+ messages in thread From: Jake Edge @ 2011-03-02 20:40 UTC (permalink / raw) To: Vasiliy Kulikov Cc: linux-kernel, mjt, arnd, mirqus, netdev, Ben Hutchings, David Miller, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm On Wed, 2 Mar 2011 23:18:07 +0300 Vasiliy Kulikov wrote: > As root has CAP_NET_ADMIN, the alias netdev-gre0 is tried and it > succeeds. Never mind, my apologies for the noise ... one always reads replies most carefully just *after* hitting send ... jake -- Jake Edge - LWN - jake@lwn.net - http://lwn.net ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-03-01 21:33 ` [PATCH v2] " Vasiliy Kulikov @ 2011-03-22 20:47 ` Eric Paris 2011-03-02 16:01 ` Kees Cook ` (2 subsequent siblings) 3 siblings, 0 replies; 54+ messages in thread From: Eric Paris @ 2011-03-22 20:47 UTC (permalink / raw) To: Vasiliy Kulikov Cc: linux-kernel, mjt, arnd, mirqus, netdev, Ben Hutchings, David Miller, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm, Greg KH, Stephen Smalley, LSM List, Daniel J Walsh, David Howells On Tue, Mar 1, 2011 at 4:33 PM, Vasiliy Kulikov <segoon@openwall.com> wrote: > Since a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565c any process with > CAP_NET_ADMIN may load any module from /lib/modules/. This doesn't mean > that CAP_NET_ADMIN is a superset of CAP_SYS_MODULE as modules are > limited to /lib/modules/**. However, CAP_NET_ADMIN capability shouldn't > allow anybody load any module not related to networking. > > This patch restricts an ability of autoloading modules to netdev modules > with explicit aliases. This fixes CVE-2011-1019. > > Arnd Bergmann suggested to leave untouched the old pre-v2.6.32 behavior > of loading netdev modules by name (without any prefix) for processes > with CAP_SYS_MODULE to maintain the compatibility with network scripts > that use autoloading netdev modules by aliases like "eth0", "wlan0". > > Currently there are only three users of the feature in the upstream > kernel: ipip, ip_gre and sit. This patch is causing a bit of a problem in Fedora. The problem lies mostly in the fact that we, by means of using SELinux, grant very very few domains CAP_SYS_MODULE (and we record when domains attempt to use this permission). Unlike most other distros in which uid==0 is for all intensive purposes == CAP_FULL_SET and there is no logging of failures to use capabilities. What happened is that as soon as we instituted this change we started getting SELinux denials because lots of domains (libvirt, udev, iw, NetworkManager) all the sudden started trying to use CAP_SYS_MODULE. It took me a minute to make sure this patch was the problem because I wasn't seeing any printk messages. I had to make some changes to the patch to print every time a task got into the CAP_SYS_MODULE case in order to get an idea what was causing the problem. I found on my machine I hit the problem 3 times trying to load "reg", "wifi0", and "virbr0" None of these are actual modules in userspace so the upcall failed. I'm trying to figure out how I should be dealing with this. My first idea is changing the capable(CAP_SYS_MODULE) into a has_capability_noaudit(). Which will not audit the access attempt. I'm not sure I like that since it uses the read cred, it doesn't set PF_SUPERPRIV, and it means we could likely miss recording a problem if a task is doing this intentionally... The next idea is I guess figuring out what's causing these and fix them there, but I'm not certain a good way to debug it. I know from our audit logs that wpa_supplicant is calling SIOCGIFINDEX which is causing one of these, libvirt is calling SIOCGIFFLAGS. I'm not sure what udev->iw is doing to trigger it's problem..... If the name in question is not coming from direct userspace request I guess I need help figuring out what is causing them.... So while this patch might not necessarily be breaking things it certainly is not regression free and could potentially be breaking systems with fine grained capabilities controls.... -Eric > root@albatros:~# capsh --drop=$(seq -s, 0 11),$(seq -s, 13 34) -- > root@albatros:~# grep Cap /proc/$$/status > CapInh: 0000000000000000 > CapPrm: fffffff800001000 > CapEff: fffffff800001000 > CapBnd: fffffff800001000 > root@albatros:~# modprobe xfs > FATAL: Error inserting xfs > (/lib/modules/2.6.38-rc6-00001-g2bf4ca3/kernel/fs/xfs/xfs.ko): Operation not permitted > root@albatros:~# lsmod | grep xfs > root@albatros:~# ifconfig xfs > xfs: error fetching interface information: Device not found > root@albatros:~# lsmod | grep xfs > root@albatros:~# lsmod | grep sit > root@albatros:~# ifconfig sit > sit: error fetching interface information: Device not found > root@albatros:~# lsmod | grep sit > root@albatros:~# ifconfig sit0 > sit0 Link encap:IPv6-in-IPv4 > NOARP MTU:1480 Metric:1 > > root@albatros:~# lsmod | grep sit > sit 10457 0 > tunnel4 2957 1 sit > > For CAP_SYS_MODULE module loading is still relaxed: > > root@albatros:~# grep Cap /proc/$$/status > CapInh: 0000000000000000 > CapPrm: ffffffffffffffff > CapEff: ffffffffffffffff > CapBnd: ffffffffffffffff > root@albatros:~# ifconfig xfs > xfs: error fetching interface information: Device not found > root@albatros:~# lsmod | grep xfs > xfs 745319 0 > > Reference: https://lkml.org/lkml/2011/2/24/203 > > Signed-off-by: Vasiliy Kulikov <segoon@openwall.com> > --- > v2 - use pr_err() > - don't try to load $name if netdev-$name is loaded > > include/linux/netdevice.h | 3 +++ > net/core/dev.c | 12 ++++++++++-- > net/ipv4/ip_gre.c | 2 +- > net/ipv4/ipip.c | 2 +- > net/ipv6/sit.c | 2 +- > 5 files changed, 16 insertions(+), 5 deletions(-) > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > index d971346..71caf7a 100644 > --- a/include/linux/netdevice.h > +++ b/include/linux/netdevice.h > @@ -2392,6 +2392,9 @@ extern int netdev_notice(const struct net_device *dev, const char *format, ...) > extern int netdev_info(const struct net_device *dev, const char *format, ...) > __attribute__ ((format (printf, 2, 3))); > > +#define MODULE_ALIAS_NETDEV(device) \ > + MODULE_ALIAS("netdev-" device) > + > #if defined(DEBUG) > #define netdev_dbg(__dev, format, args...) \ > netdev_printk(KERN_DEBUG, __dev, format, ##args) > diff --git a/net/core/dev.c b/net/core/dev.c > index 8ae6631..6561021 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -1114,13 +1114,21 @@ EXPORT_SYMBOL(netdev_bonding_change); > void dev_load(struct net *net, const char *name) > { > struct net_device *dev; > + int no_module; > > rcu_read_lock(); > dev = dev_get_by_name_rcu(net, name); > rcu_read_unlock(); > > - if (!dev && capable(CAP_NET_ADMIN)) > - request_module("%s", name); > + no_module = !dev; > + if (no_module && capable(CAP_NET_ADMIN)) > + no_module = request_module("netdev-%s", name); > + if (no_module && capable(CAP_SYS_MODULE)) { > + if (!request_module("%s", name)) > + pr_err("Loading kernel module for a network device " > +"with CAP_SYS_MODULE (deprecated). Use CAP_NET_ADMIN and alias netdev-%s " > +"instead\n", name); > + } > } > EXPORT_SYMBOL(dev_load); > > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c > index 6613edf..d1d0e2c 100644 > --- a/net/ipv4/ip_gre.c > +++ b/net/ipv4/ip_gre.c > @@ -1765,4 +1765,4 @@ module_exit(ipgre_fini); > MODULE_LICENSE("GPL"); > MODULE_ALIAS_RTNL_LINK("gre"); > MODULE_ALIAS_RTNL_LINK("gretap"); > -MODULE_ALIAS("gre0"); > +MODULE_ALIAS_NETDEV("gre0"); > diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c > index 988f52f..a5f58e7 100644 > --- a/net/ipv4/ipip.c > +++ b/net/ipv4/ipip.c > @@ -913,4 +913,4 @@ static void __exit ipip_fini(void) > module_init(ipip_init); > module_exit(ipip_fini); > MODULE_LICENSE("GPL"); > -MODULE_ALIAS("tunl0"); > +MODULE_ALIAS_NETDEV("tunl0"); > diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c > index 8ce38f1..d2c16e1 100644 > --- a/net/ipv6/sit.c > +++ b/net/ipv6/sit.c > @@ -1290,4 +1290,4 @@ static int __init sit_init(void) > module_init(sit_init); > module_exit(sit_cleanup); > MODULE_LICENSE("GPL"); > -MODULE_ALIAS("sit0"); > +MODULE_ALIAS_NETDEV("sit0"); > -- > 1.7.0.4 > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules @ 2011-03-22 20:47 ` Eric Paris 0 siblings, 0 replies; 54+ messages in thread From: Eric Paris @ 2011-03-22 20:47 UTC (permalink / raw) To: Vasiliy Kulikov Cc: linux-kernel, mjt, arnd, mirqus, netdev, Ben Hutchings, David Miller, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm, Greg KH, Stephen Smalley, LSM List, Daniel J Walsh, David Howells On Tue, Mar 1, 2011 at 4:33 PM, Vasiliy Kulikov <segoon@openwall.com> wrote: > Since a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565c any process with > CAP_NET_ADMIN may load any module from /lib/modules/. This doesn't mean > that CAP_NET_ADMIN is a superset of CAP_SYS_MODULE as modules are > limited to /lib/modules/**. However, CAP_NET_ADMIN capability shouldn't > allow anybody load any module not related to networking. > > This patch restricts an ability of autoloading modules to netdev modules > with explicit aliases. This fixes CVE-2011-1019. > > Arnd Bergmann suggested to leave untouched the old pre-v2.6.32 behavior > of loading netdev modules by name (without any prefix) for processes > with CAP_SYS_MODULE to maintain the compatibility with network scripts > that use autoloading netdev modules by aliases like "eth0", "wlan0". > > Currently there are only three users of the feature in the upstream > kernel: ipip, ip_gre and sit. This patch is causing a bit of a problem in Fedora. The problem lies mostly in the fact that we, by means of using SELinux, grant very very few domains CAP_SYS_MODULE (and we record when domains attempt to use this permission). Unlike most other distros in which uid==0 is for all intensive purposes == CAP_FULL_SET and there is no logging of failures to use capabilities. What happened is that as soon as we instituted this change we started getting SELinux denials because lots of domains (libvirt, udev, iw, NetworkManager) all the sudden started trying to use CAP_SYS_MODULE. It took me a minute to make sure this patch was the problem because I wasn't seeing any printk messages. I had to make some changes to the patch to print every time a task got into the CAP_SYS_MODULE case in order to get an idea what was causing the problem. I found on my machine I hit the problem 3 times trying to load "reg", "wifi0", and "virbr0" None of these are actual modules in userspace so the upcall failed. I'm trying to figure out how I should be dealing with this. My first idea is changing the capable(CAP_SYS_MODULE) into a has_capability_noaudit(). Which will not audit the access attempt. I'm not sure I like that since it uses the read cred, it doesn't set PF_SUPERPRIV, and it means we could likely miss recording a problem if a task is doing this intentionally... The next idea is I guess figuring out what's causing these and fix them there, but I'm not certain a good way to debug it. I know from our audit logs that wpa_supplicant is calling SIOCGIFINDEX which is causing one of these, libvirt is calling SIOCGIFFLAGS. I'm not sure what udev->iw is doing to trigger it's problem..... If the name in question is not coming from direct userspace request I guess I need help figuring out what is causing them.... So while this patch might not necessarily be breaking things it certainly is not regression free and could potentially be breaking systems with fine grained capabilities controls.... -Eric > root@albatros:~# capsh --drop=$(seq -s, 0 11),$(seq -s, 13 34) -- > root@albatros:~# grep Cap /proc/$$/status > CapInh: 0000000000000000 > CapPrm: fffffff800001000 > CapEff: fffffff800001000 > CapBnd: fffffff800001000 > root@albatros:~# modprobe xfs > FATAL: Error inserting xfs > (/lib/modules/2.6.38-rc6-00001-g2bf4ca3/kernel/fs/xfs/xfs.ko): Operation not permitted > root@albatros:~# lsmod | grep xfs > root@albatros:~# ifconfig xfs > xfs: error fetching interface information: Device not found > root@albatros:~# lsmod | grep xfs > root@albatros:~# lsmod | grep sit > root@albatros:~# ifconfig sit > sit: error fetching interface information: Device not found > root@albatros:~# lsmod | grep sit > root@albatros:~# ifconfig sit0 > sit0 Link encap:IPv6-in-IPv4 > NOARP MTU:1480 Metric:1 > > root@albatros:~# lsmod | grep sit > sit 10457 0 > tunnel4 2957 1 sit > > For CAP_SYS_MODULE module loading is still relaxed: > > root@albatros:~# grep Cap /proc/$$/status > CapInh: 0000000000000000 > CapPrm: ffffffffffffffff > CapEff: ffffffffffffffff > CapBnd: ffffffffffffffff > root@albatros:~# ifconfig xfs > xfs: error fetching interface information: Device not found > root@albatros:~# lsmod | grep xfs > xfs 745319 0 > > Reference: https://lkml.org/lkml/2011/2/24/203 > > Signed-off-by: Vasiliy Kulikov <segoon@openwall.com> > --- > v2 - use pr_err() > - don't try to load $name if netdev-$name is loaded > > include/linux/netdevice.h | 3 +++ > net/core/dev.c | 12 ++++++++++-- > net/ipv4/ip_gre.c | 2 +- > net/ipv4/ipip.c | 2 +- > net/ipv6/sit.c | 2 +- > 5 files changed, 16 insertions(+), 5 deletions(-) > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > index d971346..71caf7a 100644 > --- a/include/linux/netdevice.h > +++ b/include/linux/netdevice.h > @@ -2392,6 +2392,9 @@ extern int netdev_notice(const struct net_device *dev, const char *format, ...) > extern int netdev_info(const struct net_device *dev, const char *format, ...) > __attribute__ ((format (printf, 2, 3))); > > +#define MODULE_ALIAS_NETDEV(device) \ > + MODULE_ALIAS("netdev-" device) > + > #if defined(DEBUG) > #define netdev_dbg(__dev, format, args...) \ > netdev_printk(KERN_DEBUG, __dev, format, ##args) > diff --git a/net/core/dev.c b/net/core/dev.c > index 8ae6631..6561021 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -1114,13 +1114,21 @@ EXPORT_SYMBOL(netdev_bonding_change); > void dev_load(struct net *net, const char *name) > { > struct net_device *dev; > + int no_module; > > rcu_read_lock(); > dev = dev_get_by_name_rcu(net, name); > rcu_read_unlock(); > > - if (!dev && capable(CAP_NET_ADMIN)) > - request_module("%s", name); > + no_module = !dev; > + if (no_module && capable(CAP_NET_ADMIN)) > + no_module = request_module("netdev-%s", name); > + if (no_module && capable(CAP_SYS_MODULE)) { > + if (!request_module("%s", name)) > + pr_err("Loading kernel module for a network device " > +"with CAP_SYS_MODULE (deprecated). Use CAP_NET_ADMIN and alias netdev-%s " > +"instead\n", name); > + } > } > EXPORT_SYMBOL(dev_load); > > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c > index 6613edf..d1d0e2c 100644 > --- a/net/ipv4/ip_gre.c > +++ b/net/ipv4/ip_gre.c > @@ -1765,4 +1765,4 @@ module_exit(ipgre_fini); > MODULE_LICENSE("GPL"); > MODULE_ALIAS_RTNL_LINK("gre"); > MODULE_ALIAS_RTNL_LINK("gretap"); > -MODULE_ALIAS("gre0"); > +MODULE_ALIAS_NETDEV("gre0"); > diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c > index 988f52f..a5f58e7 100644 > --- a/net/ipv4/ipip.c > +++ b/net/ipv4/ipip.c > @@ -913,4 +913,4 @@ static void __exit ipip_fini(void) > module_init(ipip_init); > module_exit(ipip_fini); > MODULE_LICENSE("GPL"); > -MODULE_ALIAS("tunl0"); > +MODULE_ALIAS_NETDEV("tunl0"); > diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c > index 8ce38f1..d2c16e1 100644 > --- a/net/ipv6/sit.c > +++ b/net/ipv6/sit.c > @@ -1290,4 +1290,4 @@ static int __init sit_init(void) > module_init(sit_init); > module_exit(sit_cleanup); > MODULE_LICENSE("GPL"); > -MODULE_ALIAS("sit0"); > +MODULE_ALIAS_NETDEV("sit0"); > -- > 1.7.0.4 > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" 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] 54+ messages in thread
* Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-03-22 20:47 ` Eric Paris (?) @ 2011-03-24 15:37 ` Serge E. Hallyn 2011-03-24 18:03 ` Eric Paris -1 siblings, 1 reply; 54+ messages in thread From: Serge E. Hallyn @ 2011-03-24 15:37 UTC (permalink / raw) To: Eric Paris Cc: Vasiliy Kulikov, linux-kernel, mjt, arnd, mirqus, netdev, Ben Hutchings, David Miller, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm, Greg KH, Stephen Smalley, LSM List, Daniel J Walsh, David Howells [-- Attachment #1: Type: text/plain, Size: 8450 bytes --] Quoting Eric Paris (eparis@parisplace.org): > On Tue, Mar 1, 2011 at 4:33 PM, Vasiliy Kulikov <segoon@openwall.com> wrote: ... > 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? If it really is the capable(CAP_SYS_ADMIN) call itself, then there must be a bug in the no_module logic in the patch. Every case where it's currently checking for capable(CAP_SYS_ADMIN) (and potentially auditing a failure) should be one where in the past it would have (and currently it still would) spit out an audit msg for the capable(CAP_NET_ADMIN) failure anyway. If it's just the pr_err that's causing problems, then perhaps we can turn it into a warn_once? > mostly in the fact that we, by means of using SELinux, grant very very > few domains CAP_SYS_MODULE (and we record when domains attempt to use > this permission). Unlike most other distros in which uid==0 is for > all intensive purposes == CAP_FULL_SET and there is no logging of > failures to use capabilities. What happened is that as soon as we > instituted this change we started getting SELinux denials because lots > of domains (libvirt, udev, iw, NetworkManager) all the sudden started > trying to use CAP_SYS_MODULE. It took me a minute to make sure this > patch was the problem because I wasn't seeing any printk messages. I > had to make some changes to the patch to print every time a task got > into the CAP_SYS_MODULE case in order to get an idea what was causing > the problem. I found on my machine I hit the problem 3 times trying > to load "reg", "wifi0", and "virbr0" None of these are actual > modules in userspace so the upcall failed. > > I'm trying to figure out how I should be dealing with this. > > My first idea is changing the capable(CAP_SYS_MODULE) into a > has_capability_noaudit(). Which will not audit the access attempt. > I'm not sure I like that since it uses the read cred, it doesn't set > PF_SUPERPRIV, and it means we could likely miss recording a problem if > a task is doing this intentionally... > > The next idea is I guess figuring out what's causing these and fix > them there, but I'm not certain a good way to debug it. I know from > our audit logs that wpa_supplicant is calling SIOCGIFINDEX which is > causing one of these, libvirt is calling SIOCGIFFLAGS. I'm not sure > what udev->iw is doing to trigger it's problem..... > > If the name in question is not coming from direct userspace request I > guess I need help figuring out what is causing them.... > > So while this patch might not necessarily be breaking things it > certainly is not regression free and could potentially be breaking > systems with fine grained capabilities controls.... > > -Eric > > > root@albatros:~# capsh --drop=$(seq -s, 0 11),$(seq -s, 13 34) -- > > root@albatros:~# grep Cap /proc/$$/status > > CapInh: 0000000000000000 > > CapPrm: fffffff800001000 > > CapEff: fffffff800001000 > > CapBnd: fffffff800001000 > > root@albatros:~# modprobe xfs > > FATAL: Error inserting xfs > > (/lib/modules/2.6.38-rc6-00001-g2bf4ca3/kernel/fs/xfs/xfs.ko): Operation not permitted > > root@albatros:~# lsmod | grep xfs > > root@albatros:~# ifconfig xfs > > xfs: error fetching interface information: Device not found > > root@albatros:~# lsmod | grep xfs > > root@albatros:~# lsmod | grep sit > > root@albatros:~# ifconfig sit > > sit: error fetching interface information: Device not found > > root@albatros:~# lsmod | grep sit > > root@albatros:~# ifconfig sit0 > > sit0 Link encap:IPv6-in-IPv4 > > NOARP MTU:1480 Metric:1 > > > > root@albatros:~# lsmod | grep sit > > sit 10457 0 > > tunnel4 2957 1 sit > > > > For CAP_SYS_MODULE module loading is still relaxed: > > > > root@albatros:~# grep Cap /proc/$$/status > > CapInh: 0000000000000000 > > CapPrm: ffffffffffffffff > > CapEff: ffffffffffffffff > > CapBnd: ffffffffffffffff > > root@albatros:~# ifconfig xfs > > xfs: error fetching interface information: Device not found > > root@albatros:~# lsmod | grep xfs > > xfs 745319 0 > > > > Reference: https://lkml.org/lkml/2011/2/24/203 > > > > Signed-off-by: Vasiliy Kulikov <segoon@openwall.com> > > --- > > v2 - use pr_err() > > - don't try to load $name if netdev-$name is loaded > > > > include/linux/netdevice.h | 3 +++ > > net/core/dev.c | 12 ++++++++++-- > > net/ipv4/ip_gre.c | 2 +- > > net/ipv4/ipip.c | 2 +- > > net/ipv6/sit.c | 2 +- > > 5 files changed, 16 insertions(+), 5 deletions(-) > > > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > > index d971346..71caf7a 100644 > > --- a/include/linux/netdevice.h > > +++ b/include/linux/netdevice.h > > @@ -2392,6 +2392,9 @@ extern int netdev_notice(const struct net_device *dev, const char *format, ...) > > extern int netdev_info(const struct net_device *dev, const char *format, ...) > > __attribute__ ((format (printf, 2, 3))); > > > > +#define MODULE_ALIAS_NETDEV(device) \ > > + MODULE_ALIAS("netdev-" device) > > + > > #if defined(DEBUG) > > #define netdev_dbg(__dev, format, args...) \ > > netdev_printk(KERN_DEBUG, __dev, format, ##args) > > diff --git a/net/core/dev.c b/net/core/dev.c > > index 8ae6631..6561021 100644 > > --- a/net/core/dev.c > > +++ b/net/core/dev.c > > @@ -1114,13 +1114,21 @@ EXPORT_SYMBOL(netdev_bonding_change); > > void dev_load(struct net *net, const char *name) > > { > > struct net_device *dev; > > + int no_module; > > > > rcu_read_lock(); > > dev = dev_get_by_name_rcu(net, name); > > rcu_read_unlock(); > > > > - if (!dev && capable(CAP_NET_ADMIN)) > > - request_module("%s", name); > > + no_module = !dev; > > + if (no_module && capable(CAP_NET_ADMIN)) > > + no_module = request_module("netdev-%s", name); > > + if (no_module && capable(CAP_SYS_MODULE)) { > > + if (!request_module("%s", name)) > > + pr_err("Loading kernel module for a network device " > > +"with CAP_SYS_MODULE (deprecated). Use CAP_NET_ADMIN and alias netdev-%s " > > +"instead\n", name); > > + } > > } > > EXPORT_SYMBOL(dev_load); > > > > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c > > index 6613edf..d1d0e2c 100644 > > --- a/net/ipv4/ip_gre.c > > +++ b/net/ipv4/ip_gre.c > > @@ -1765,4 +1765,4 @@ module_exit(ipgre_fini); > > MODULE_LICENSE("GPL"); > > MODULE_ALIAS_RTNL_LINK("gre"); > > MODULE_ALIAS_RTNL_LINK("gretap"); > > -MODULE_ALIAS("gre0"); > > +MODULE_ALIAS_NETDEV("gre0"); > > diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c > > index 988f52f..a5f58e7 100644 > > --- a/net/ipv4/ipip.c > > +++ b/net/ipv4/ipip.c > > @@ -913,4 +913,4 @@ static void __exit ipip_fini(void) > > module_init(ipip_init); > > module_exit(ipip_fini); > > MODULE_LICENSE("GPL"); > > -MODULE_ALIAS("tunl0"); > > +MODULE_ALIAS_NETDEV("tunl0"); > > diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c > > index 8ce38f1..d2c16e1 100644 > > --- a/net/ipv6/sit.c > > +++ b/net/ipv6/sit.c > > @@ -1290,4 +1290,4 @@ static int __init sit_init(void) > > module_init(sit_init); > > module_exit(sit_cleanup); > > MODULE_LICENSE("GPL"); > > -MODULE_ALIAS("sit0"); > > +MODULE_ALIAS_NETDEV("sit0"); > > -- > > 1.7.0.4 > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > Please read the FAQ at http://www.tux.org/lkml/ > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --] ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-03-24 15:37 ` Serge E. Hallyn @ 2011-03-24 18:03 ` Eric Paris 2011-03-24 18:33 ` Ben Hutchings 0 siblings, 1 reply; 54+ messages in thread From: Eric Paris @ 2011-03-24 18:03 UTC (permalink / raw) To: Serge E. Hallyn Cc: Eric Paris, Vasiliy Kulikov, linux-kernel, mjt, arnd, mirqus, netdev, Ben Hutchings, David Miller, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm, Greg KH, Stephen Smalley, LSM List, Daniel J Walsh, David Howells On Thu, 2011-03-24 at 10:37 -0500, Serge E. Hallyn wrote: > Quoting Eric Paris (eparis@parisplace.org): > > On Tue, Mar 1, 2011 at 4:33 PM, Vasiliy Kulikov <segoon@openwall.com> wrote: > ... > > 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. I do have one report from a user who claims this is breaking his system, but I'm not sure I believe him as I have yet to see any dmesg printk from the pr_err. On my local system reproduce the SELinux denials on every boot as something tries to autoload "reg", "wifi0", and "virbr0". I have no modules which match these. Thus the first try for CAP_NET_ADMIN +netdev-wifi0 fails. We then hit the CAP_SYS_MODULE check which SELinux rejects and puts up a huge warning that someone is trying to load code into the kernel. Big red flags. Even in permissive, where the capable(CAP_SYS_MODULE) passes, we won't hit the pr_err() since there is not module for "wifi" 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.... 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.... ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-03-24 18:03 ` Eric Paris @ 2011-03-24 18:33 ` Ben Hutchings 2011-03-24 20:26 ` Serge E. Hallyn 0 siblings, 1 reply; 54+ messages in thread From: Ben Hutchings @ 2011-03-24 18:33 UTC (permalink / raw) To: Eric Paris Cc: Serge E. Hallyn, Eric Paris, Vasiliy Kulikov, linux-kernel, mjt, arnd, mirqus, netdev, David Miller, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm, Greg KH, Stephen Smalley, LSM List, Daniel J Walsh, David Howells 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: > > Quoting Eric Paris (eparis@parisplace.org): > > > On Tue, Mar 1, 2011 at 4:33 PM, Vasiliy Kulikov <segoon@openwall.com> wrote: > > ... > > > 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. ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-03-24 18:33 ` Ben Hutchings @ 2011-03-24 20:26 ` Serge E. Hallyn 2011-03-24 21:39 ` Stephen Hemminger 0 siblings, 1 reply; 54+ messages in thread From: Serge E. Hallyn @ 2011-03-24 20:26 UTC (permalink / raw) To: Ben Hutchings Cc: Eric Paris, Serge E. Hallyn, Eric Paris, Vasiliy Kulikov, linux-kernel, mjt, arnd, mirqus, netdev, David Miller, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm, Greg KH, Stephen Smalley, LSM List, Daniel J Walsh, David Howells [-- Attachment #1: Type: text/plain, Size: 1089 bytes --] Quoting Ben Hutchings (bhutchings@solarflare.com): > On Thu, 2011-03-24 at 14:03 -0400, Eric Paris wrote: > > 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 Ah. I see. ... > > I think there are 3 possibilities: ...(3) > > 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. Of course this will mean pain for users until distributions have updated to userspace which is doing this, but yeah, this clearly seems like the best way. (And the only sane one.) thanks, -serge [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --] ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-03-24 20:26 ` Serge E. Hallyn @ 2011-03-24 21:39 ` Stephen Hemminger 2011-03-24 21:46 ` David Miller 0 siblings, 1 reply; 54+ messages in thread From: Stephen Hemminger @ 2011-03-24 21:39 UTC (permalink / raw) To: Serge E. Hallyn Cc: Ben Hutchings, Eric Paris, Eric Paris, Vasiliy Kulikov, linux-kernel, mjt, arnd, mirqus, netdev, David Miller, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm, Greg KH, Stephen Smalley, LSM List, Daniel J Walsh, David Howells On Thu, 24 Mar 2011 15:26:34 -0500 "Serge E. Hallyn" <serge.hallyn@ubuntu.com> wrote: > Quoting Ben Hutchings (bhutchings@solarflare.com): > > On Thu, 2011-03-24 at 14:03 -0400, Eric Paris wrote: > > > 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 > > Ah. I see. > > ... > > > > I think there are 3 possibilities: > > ...(3) > > > > 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. > > Of course this will mean pain for users until distributions have > updated to userspace which is doing this, but yeah, this clearly seems > like the best way. (And the only sane one.) > This breaks for many of the tunneling protocols, that rely on autoload for names like "sit0" ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-03-24 21:39 ` Stephen Hemminger @ 2011-03-24 21:46 ` David Miller 2011-03-24 21:57 ` Serge E. Hallyn ` (2 more replies) 0 siblings, 3 replies; 54+ messages in thread From: David Miller @ 2011-03-24 21:46 UTC (permalink / raw) To: shemminger Cc: serge.hallyn, bhutchings, eparis, eparis, segoon, linux-kernel, mjt, arnd, mirqus, netdev, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm, greg, sds, linux-security-module, dwalsh, dhowells From: Stephen Hemminger <shemminger@vyatta.com> Date: Thu, 24 Mar 2011 14:39:44 -0700 > This breaks for many of the tunneling protocols, that rely on > autoload for names like "sit0" Frankly I'm very disappointed in the fallout this has been causing. Everyone supporting this change, get real, and admit it doing in fact cause a serious regression. If you can't get past that simple fact, you cannot discuss this issue intelligently. You can't say "userland will fix things up" Because we're never supposed to break userland in the first place. There is simply no excuse for this and I want this change reverted both in Linus's tree and in -stable. ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-03-24 21:46 ` David Miller @ 2011-03-24 21:57 ` Serge E. Hallyn 2011-03-24 22:15 ` Eric Paris 2011-03-24 21:57 ` Greg KH 2011-03-26 10:35 ` Vasiliy Kulikov 2 siblings, 1 reply; 54+ messages in thread From: Serge E. Hallyn @ 2011-03-24 21:57 UTC (permalink / raw) To: David Miller Cc: shemminger, serge.hallyn, bhutchings, eparis, eparis, segoon, linux-kernel, mjt, arnd, mirqus, netdev, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm, greg, sds, linux-security-module, dwalsh, dhowells Quoting David Miller (davem@davemloft.net): > From: Stephen Hemminger <shemminger@vyatta.com> > Date: Thu, 24 Mar 2011 14:39:44 -0700 > > > This breaks for many of the tunneling protocols, that rely on > > autoload for names like "sit0" > > Frankly I'm very disappointed in the fallout this has been causing. > > Everyone supporting this change, get real, and admit it doing in fact > cause a serious regression. Sorry, I thought this was causing some extra audit messages but no actual breakage? > If you can't get past that simple fact, you cannot discuss this issue > intelligently. > > You can't say "userland will fix things up" > > Because we're never supposed to break userland in the first place. > > There is simply no excuse for this and I want this change reverted > both in Linus's tree and in -stable. Eric, in this particular case, since we've already done a 'capable(CAP_NET_ADMIN)', I woudl argue that doing the check for CAP_SYS_ADMIN without auditing failure (even if it requires a new helper in capability.c) isn't horrible. Thoughts? -serge ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-03-24 21:57 ` Serge E. Hallyn @ 2011-03-24 22:15 ` Eric Paris 0 siblings, 0 replies; 54+ messages in thread From: Eric Paris @ 2011-03-24 22:15 UTC (permalink / raw) To: Serge E. Hallyn Cc: David Miller, shemminger, bhutchings, eparis, segoon, linux-kernel, mjt, arnd, mirqus, netdev, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm, greg, sds, linux-security-module, dwalsh, dhowells On Thu, 2011-03-24 at 16:57 -0500, Serge E. Hallyn wrote: > Quoting David Miller (davem@davemloft.net): > > From: Stephen Hemminger <shemminger@vyatta.com> > > Date: Thu, 24 Mar 2011 14:39:44 -0700 > > > > > This breaks for many of the tunneling protocols, that rely on > > > autoload for names like "sit0" > > > > Frankly I'm very disappointed in the fallout this has been causing. > > > > Everyone supporting this change, get real, and admit it doing in fact > > cause a serious regression. > > Sorry, I thought this was causing some extra audit messages but no > actual breakage? I've got one report of someone claiming their system broke, but I'm not convinced I believe it since his dmesg didn't show the magic pr_err() when it should have. It's certainly possible this can break someone in a system which uses fine grained capabilities controls, but I agree it's pretty unlikely. My biggest personal concern is that I have a whole darn bunch of new scary messages which are popping out of people's computers since they don't have CAP_SYS_MODULE. While I can silence them, it's going to hide use of init_module() directly as well, which I really don't want to hide from the scary logs.... > > If you can't get past that simple fact, you cannot discuss this issue > > intelligently. > > > > You can't say "userland will fix things up" > > > > Because we're never supposed to break userland in the first place. > > > > There is simply no excuse for this and I want this change reverted > > both in Linus's tree and in -stable. > > Eric, in this particular case, since we've already done a > 'capable(CAP_NET_ADMIN)', I woudl argue that doing the check > for CAP_SYS_ADMIN without auditing failure (even if it requires > a new helper in capability.c) isn't horrible. Thoughts? s/CAP_SYS_ADMIN/CAP_SYS_MODULE/ I can do that. It was actually my #2 suggestion. But, I'm certainly willing to put some of the burden on userspace. SELinux policy is a userspace construct and we often force other userspace applications to fix things they do poorly (even if it gets us a rep for being 'difficult') Non-SELinux systems aren't going to see this problem, because basically noone else I know of tries to enforce any kind of capabilities sets other than all or none, so you'll never see CAP_NET_ADMIN without CAP_SYS_MODULE. I guess what it comes down to is that I'm happy to break Fedora user's with SELinux if in the end it gets us a better system. I'd be happy to just rip the whole CAP_SYS_MODULE portion out and blame it on SELinux, but I know that's not what upstream does. So given what we have today I personally would push for a no_audit() interface rather than a complete revert. (or maybe a compile option so I can turn off the fallback altogether and force people to come into compliance) -Eric ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-03-24 21:46 ` David Miller 2011-03-24 21:57 ` Serge E. Hallyn @ 2011-03-24 21:57 ` Greg KH 2011-03-26 10:35 ` Vasiliy Kulikov 2 siblings, 0 replies; 54+ messages in thread From: Greg KH @ 2011-03-24 21:57 UTC (permalink / raw) To: David Miller Cc: shemminger, serge.hallyn, bhutchings, eparis, eparis, segoon, linux-kernel, mjt, arnd, mirqus, netdev, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm, sds, linux-security-module, dwalsh, dhowells On Thu, Mar 24, 2011 at 02:46:28PM -0700, David Miller wrote: > From: Stephen Hemminger <shemminger@vyatta.com> > Date: Thu, 24 Mar 2011 14:39:44 -0700 > > > This breaks for many of the tunneling protocols, that rely on > > autoload for names like "sit0" > > Frankly I'm very disappointed in the fallout this has been causing. > > Everyone supporting this change, get real, and admit it doing in fact > cause a serious regression. > > If you can't get past that simple fact, you cannot discuss this issue > intelligently. > > You can't say "userland will fix things up" > > Because we're never supposed to break userland in the first place. > > There is simply no excuse for this and I want this change reverted > both in Linus's tree and in -stable. Ok, as I totally got lost in the thread here, what is the git commit id of the patch to revert? thanks, greg k-h ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-03-24 21:46 ` David Miller 2011-03-24 21:57 ` Serge E. Hallyn 2011-03-24 21:57 ` Greg KH @ 2011-03-26 10:35 ` Vasiliy Kulikov 2 siblings, 0 replies; 54+ messages in thread From: Vasiliy Kulikov @ 2011-03-26 10:35 UTC (permalink / raw) To: David Miller Cc: shemminger, serge.hallyn, bhutchings, eparis, eparis, linux-kernel, mjt, arnd, mirqus, netdev, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm, greg, sds, linux-security-module, dwalsh, dhowells On Thu, Mar 24, 2011 at 14:46 -0700, David Miller wrote: > You can't say "userland will fix things up" > > Because we're never supposed to break userland in the first place. I admit that the patch breaks things. But the thing is that kernel changes _are_ breaking userspace here and there, not only by such obvious policy changes, but by indirect changes. Note that the patch that changed CAP_SYS_MODULE to CAP_NET_ADMIN has broken userspace behavior too - one could load modules with CAP_SYS_MODULE without CAP_NET_ADMIN via "ifconfig wifi0" and after the patch it could not. Look at this patch: http://patchwork.ozlabs.org/patch/42148/ It breaks userspace tools too - one might run LSM in learning mode to create a profile for netfilter configuring, saw it didn't need any CAP_* and totally denied them in the profile. After many years (the bug was fixed after 5+ years!) of good work it was broken by the patch. The same with plenty of patches that introduce different checks in places where there were no permission checks at all or these checks were broken. -- Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-02-25 19:16 ` David Miller 2011-02-25 19:30 ` Ben Hutchings @ 2011-02-27 11:44 ` Vasiliy Kulikov 2011-02-27 23:18 ` David Miller 2011-02-27 23:19 ` David Miller 1 sibling, 2 replies; 54+ messages in thread From: Vasiliy Kulikov @ 2011-02-27 11:44 UTC (permalink / raw) To: David Miller Cc: bhutchings, netdev, linux-kernel, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm David, On Fri, Feb 25, 2011 at 11:16 -0800, David Miller wrote: > From: Ben Hutchings <bhutchings@solarflare.com> > Date: Fri, 25 Feb 2011 19:07:59 +0000 > > > You realise that module loading doesn't actually run in the context of > > request_module(), right? > > Why is that a barrier? We could simply pass a capability mask into > request_module if necessary. > > It's an implementation detail, and not a deterrant to my suggested > scheme. Let's discuss your scheme. AFAIU, you suggest to change: 1. a) request_module("%s", devname) => request_module_with_caps(CAP_NET_ADMIN, "%s", devname) b) call_usermodehelper() => call_usermodehelper_with_caps() c) add some bits/sections into kernel module image indicating that this module is safe to be loaded via CAP_NET_ADMIN d) run modprobe with CAP_NET_ADMIN only e) in load_module() check whether (the process has CAP_SYS_MODULE) or (the process has CAP_NET_ADMIN and bit SAFE_NET_MODULE is raised in the module image) This obviously doesn't work - the kernel is not able to verify whether the bit/section is not malformed by user with CAP_NET_ADMIN. -OR- 1. a) request_module("%s", devname) => request_module_with_argument("--netdev", "%s", devname) b) patch modprobe to add "--netmodule-only" argument (or bitmask, whatever), this would indicate that only net/** modules may be loaded. Then the things are still broken - a user has to update modprobe together with the kernel, otherwise the updated kernel would call "modprobe" with unsupported argument and even "sit0" wouldn't work. Additionally this touches module loading process, which is not buggy. Or you propose something else besides these 2 ways? Please clarify. Thanks, -- Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-02-27 11:44 ` [PATCH] " Vasiliy Kulikov @ 2011-02-27 23:18 ` David Miller 2011-02-27 23:19 ` David Miller 1 sibling, 0 replies; 54+ messages in thread From: David Miller @ 2011-02-27 23:18 UTC (permalink / raw) To: segoon Cc: bhutchings, netdev, linux-kernel, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm From: Vasiliy Kulikov <segoon@openwall.com> Date: Sun, 27 Feb 2011 14:44:38 +0300 > d) run modprobe with CAP_NET_ADMIN only This is not part of my scheme. The module loading will run with existing module loading privileges, the "allowed capability" bits will be passed along back into the kernel at module load time (via modprobe arguments or similar) and validated by the kernel as it walks the ELF sections anyways to perform relocations and whatnot. ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules 2011-02-27 11:44 ` [PATCH] " Vasiliy Kulikov 2011-02-27 23:18 ` David Miller @ 2011-02-27 23:19 ` David Miller 1 sibling, 0 replies; 54+ messages in thread From: David Miller @ 2011-02-27 23:19 UTC (permalink / raw) To: segoon Cc: bhutchings, netdev, linux-kernel, kuznet, pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo, jesse, kees.cook, eugene, dan.j.rosenberg, akpm From: Vasiliy Kulikov <segoon@openwall.com> Date: Sun, 27 Feb 2011 14:44:38 +0300 > Then the things are still broken - a user has to update modprobe > together with the kernel, otherwise the updated kernel would call > "modprobe" with unsupported argument and even "sit0" wouldn't work. The capability bits get passed on the modprobe command line. The module loading system call in the kernel inspects the command line looking for the argument, and uses it to validate the module load by comparing the mask with the special ELF section. ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: module loading with CAP_NET_ADMIN 2011-02-25 12:30 ` Vasiliy Kulikov 2011-02-25 15:14 ` [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules Vasiliy Kulikov @ 2011-02-25 15:29 ` Michael Tokarev 2011-02-25 15:57 ` Vasiliy Kulikov 1 sibling, 1 reply; 54+ messages in thread From: Michael Tokarev @ 2011-02-25 15:29 UTC (permalink / raw) To: Vasiliy Kulikov Cc: Ben Hutchings, netdev, linux-kernel, Kees Cook, Eugene Teo, Dan Rosenberg, David S. Miller 25.02.2011 15:30, Vasiliy Kulikov wrote: > On Thu, Feb 24, 2011 at 16:34 +0000, Ben Hutchings wrote: >> On Thu, 2011-02-24 at 18:12 +0300, Vasiliy Kulikov wrote: >>> My proposal is changing request_module("%s", name) to something like >>> request_module("netdev-%s", name) inside of dev_load() and adding >>> aliases to related drivers. It is not the kernel patching which we should worry about, kernel part is trivial. What is not trivial is to patch all the systems out there who autoloads network drivers based on /etc/modprobe.d/network-aliases.conf (some local file), ie, numerous working setups which already uses this mechanism since stone age. And patching these is not trivial at all, unfortunately. Somewhat weird setups (one can load the modules explicitly, and nowadays this all is handled by udev anyway), but this change will break some working systems. Maybe the cost (some pain for some users) isn't large enough but the outcome is good, and I think it _is_ good, but it needs some wider discussion first, imho. I can't think of a way to handle this without breaking stuff. Thanks! /mjt ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: module loading with CAP_NET_ADMIN 2011-02-25 15:29 ` module loading with CAP_NET_ADMIN Michael Tokarev @ 2011-02-25 15:57 ` Vasiliy Kulikov 0 siblings, 0 replies; 54+ messages in thread From: Vasiliy Kulikov @ 2011-02-25 15:57 UTC (permalink / raw) To: Michael Tokarev Cc: Ben Hutchings, netdev, linux-kernel, Kees Cook, Eugene Teo, Dan Rosenberg, David S. Miller On Fri, Feb 25, 2011 at 18:29 +0300, Michael Tokarev wrote: > 25.02.2011 15:30, Vasiliy Kulikov wrote: > > On Thu, Feb 24, 2011 at 16:34 +0000, Ben Hutchings wrote: > >> On Thu, 2011-02-24 at 18:12 +0300, Vasiliy Kulikov wrote: > >>> My proposal is changing request_module("%s", name) to something like > >>> request_module("netdev-%s", name) inside of dev_load() and adding > >>> aliases to related drivers. > > It is not the kernel patching which we should worry about, kernel > part is trivial. > > What is not trivial is to patch all the systems out there who > autoloads network drivers based on /etc/modprobe.d/network-aliases.conf > (some local file), ie, numerous working setups which already > uses this mechanism since stone age. And patching these is > not trivial at all, unfortunately. > > Somewhat weird setups (one can load the modules explicitly, and > nowadays this all is handled by udev anyway), but this change > will break some working systems. > > Maybe the cost (some pain for some users) isn't large enough > but the outcome is good, and I think it _is_ good, but it needs > some wider discussion first, imho. > > I can't think of a way to handle this without breaking stuff. Currently Linux slowly moves in the direction of rootless systems. This definitely need proper restrictions of CAP_* power. Network admin does nothing with general modules. It _has_ to break something one day because old assumptions about permission stuff don't conform CAP_* things: old assumptions are very closely connected with just everything. I'm not sure how this particular CAP_NET_ADMIN misuse should be fixed, maybe distributions should supply script to upgrade modprobe configs. Also note that change s/CAP_SYS_MODULE/CAP_NET_ADMIN/ was made in 2.6.32, so there is a possibility that the set of affected distributions (that doesn't use udev stuff) is very small. Thanks for your input, -- Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments ^ permalink raw reply [flat|nested] 54+ messages in thread
end of thread, other threads:[~2011-03-26 10:35 UTC | newest] Thread overview: 54+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-02-24 15:12 module loading with CAP_NET_ADMIN Vasiliy Kulikov 2011-02-24 16:34 ` Ben Hutchings 2011-02-25 12:30 ` Vasiliy Kulikov 2011-02-25 15:14 ` [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules Vasiliy Kulikov 2011-02-25 17:25 ` Valdis.Kletnieks 2011-02-25 17:47 ` Vasiliy Kulikov 2011-02-25 17:48 ` Ben Hutchings 2011-02-25 18:47 ` David Miller 2011-02-25 19:02 ` Vasiliy Kulikov 2011-02-25 19:05 ` David Miller 2011-02-25 19:07 ` Ben Hutchings 2011-02-25 19:16 ` David Miller 2011-02-25 19:30 ` Ben Hutchings 2011-02-25 19:43 ` David Miller 2011-02-25 19:53 ` Ben Hutchings 2011-02-25 20:37 ` David Miller 2011-02-25 20:38 ` Ben Hutchings 2011-02-25 20:59 ` Michał Mirosław 2011-02-27 20:22 ` Arnd Bergmann 2011-02-28 9:29 ` Michael Tokarev 2011-02-28 9:51 ` Vasiliy Kulikov 2011-02-28 19:23 ` David Miller 2011-03-01 19:48 ` [PATCH] net: " Vasiliy Kulikov 2011-03-01 20:13 ` Ben Hutchings 2011-03-01 21:33 ` [PATCH v2] " Vasiliy Kulikov 2011-03-02 7:15 ` Michael Tokarev 2011-03-09 22:06 ` Vasiliy Kulikov 2011-03-09 22:09 ` David Miller 2011-03-09 22:53 ` James Morris 2011-03-10 9:49 ` Vasiliy Kulikov 2011-03-02 16:01 ` Kees Cook 2011-03-02 19:39 ` Jake Edge 2011-03-02 19:43 ` Vasiliy Kulikov 2011-03-02 19:49 ` Jake Edge 2011-03-02 20:18 ` Vasiliy Kulikov 2011-03-02 20:38 ` Jake Edge 2011-03-02 20:40 ` Jake Edge 2011-03-22 20:47 ` Eric Paris 2011-03-22 20:47 ` Eric Paris 2011-03-24 15:37 ` Serge E. Hallyn 2011-03-24 18:03 ` Eric Paris 2011-03-24 18:33 ` Ben Hutchings 2011-03-24 20:26 ` Serge E. Hallyn 2011-03-24 21:39 ` Stephen Hemminger 2011-03-24 21:46 ` David Miller 2011-03-24 21:57 ` Serge E. Hallyn 2011-03-24 22:15 ` Eric Paris 2011-03-24 21:57 ` Greg KH 2011-03-26 10:35 ` Vasiliy Kulikov 2011-02-27 11:44 ` [PATCH] " Vasiliy Kulikov 2011-02-27 23:18 ` David Miller 2011-02-27 23:19 ` David Miller 2011-02-25 15:29 ` module loading with CAP_NET_ADMIN Michael Tokarev 2011-02-25 15:57 ` Vasiliy Kulikov
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.