module loading with CAP_NET_ADMIN
Subsystems (3)
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.
Thanks,
Signature · 2 lines
Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments
Quoted 6 lines · Vasiliy Kulikov
Hi netdev folks,
I'd like to discuss the ability to load any modules from
/lib/modules/by a process withCAP_NET_ADMIN. Since Linux 2.6.32 [1] there is such possibility:
Quoted 11 lines
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!
Quoted 10 lines
Ability of
CAP_NET_ADMINto 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 toCAP_SYS_MODULEsince 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 likerequest_module("netdev-%s", name)inside ofdev_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.
Quoted 4 lines
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.
Signature · 3 lines
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.
Quoted 14 lines · Ben Hutchings
Quoted 3 lines · Vasiliy Kulikov
My proposal is changing
request_module("%s", name)to something likerequest_module("netdev-%s", name)inside ofdev_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.
Quoted 4 lines
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.
Quoted 5 lines
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,
Signature · 2 lines
Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments
PATCH 1/1 don't allow CAP_NET_ADMIN to load non-netdev kernel modules
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,nfsdAfter:
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 0Reference: http://www.openwall.com/lists/oss-security/2011/02/24/17
--- 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(-)
include/linux/netdevice.h +3 -0
@@ -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)
net/core/dev.c +1 -1
@@ -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);
net/ipv4/ip_gre.c +1 -1
@@ -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" );
net/ipv4/ipip.c +1 -1
@@ -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" );
net/ipv6/sit.c +1 -1
@@ -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" );
Signature · 1 line
1.7.0.4
Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules
Quoted 9 lines · Vasiliy Kulikov
Since
a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565cany process withCAP_NET_ADMINmay load any module from/lib/modules/. This doesn't mean thatCAP_NET_ADMINis a superset ofCAP_SYS_MODULEas modules are limited to/lib/modules/**. However,CAP_NET_ADMINcapability 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_greand 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).
Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules
Quoted 4 lines · On Fri, Feb 25, 2011 at 12:25 -0500, [email protected] wrote:
And you stop an attacker from simply recompiling the module with a suitable
MODULE_ALIASline 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.
Quoted 4 lines
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,
Signature · 2 lines
Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments
Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules
In reply to [email protected]Quoted 7 lines · On Fri, 2011-02-25 at 12:25 -0500, [email protected] wrote:
Quoted 6 lines · Vasiliy Kulikov
Since
a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565cany process withCAP_NET_ADMINmay load any module from/lib/modules/. This doesn't mean thatCAP_NET_ADMINis a superset ofCAP_SYS_MODULEas modules are limited to/lib/modules/**. However,CAP_NET_ADMINcapability shouldn't allow anybody load any module not related to networking.
Quoted 8 lines
Quoted 3 lines
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_greand sit.And you stop an attacker from simply recompiling the module with a suitable
MODULE_ALIASline 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.
Quoted 4 lines
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.
Signature · 3 lines
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.
David Miller
maintainerRe: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules
In reply to Vasiliy KulikovFrom: Vasiliy Kulikov <[email protected]>
Date: Fri, 25 Feb 2011 18:14:14 +0300
Quoted 5 lines
Since
a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565cany process withCAP_NET_ADMINmay load any module from/lib/modules/. This doesn't mean thatCAP_NET_ADMINis a superset ofCAP_SYS_MODULEas modules are limited to/lib/modules/**. However,CAP_NET_ADMINcapability 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?
Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules
Quoted 9 lines · David Miller
From: Vasiliy Kulikov <[email protected]>
Date: Fri, 25 Feb 2011 18:14:14 +0300Quoted 5 lines
Since
a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565cany process withCAP_NET_ADMINmay load any module from/lib/modules/. This doesn't mean thatCAP_NET_ADMINis a superset ofCAP_SYS_MODULEas modules are limited to/lib/modules/**. However,CAP_NET_ADMINcapability shouldn't allow anybody load any module not related to networking.
Quoted 5 lines
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,
Signature · 2 lines
Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments
David Miller
maintainerRe: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules
From: Vasiliy Kulikov <[email protected]>
Date: Fri, 25 Feb 2011 22:02:05 +0300
Quoted 14 lines · David Miller
Quoted 13 lines
From: Vasiliy Kulikov <[email protected]>
Date: Fri, 25 Feb 2011 18:14:14 +0300Quoted 5 lines
Since
a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565cany process withCAP_NET_ADMINmay load any module from/lib/modules/. This doesn't mean thatCAP_NET_ADMINis a superset ofCAP_SYS_MODULEas modules are limited to/lib/modules/**. However,CAP_NET_ADMINcapability 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.
Quoted 8 lines
Quoted 1 line
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.
Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules
Quoted 26 lines · David Miller
From: Vasiliy Kulikov <[email protected]>
Date: Fri, 25 Feb 2011 22:02:05 +0300Quoted 14 lines · David Miller
Quoted 13 lines
From: Vasiliy Kulikov <[email protected]>
Date: Fri, 25 Feb 2011 18:14:14 +0300Quoted 5 lines
Since
a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565cany process withCAP_NET_ADMINmay load any module from/lib/modules/. This doesn't mean thatCAP_NET_ADMINis a superset ofCAP_SYS_MODULEas modules are limited to/lib/modules/**. However,CAP_NET_ADMINcapability 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.
Quoted 8 lines
Quoted 1 line
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.
Quoted 7 lines
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.
Signature · 3 lines
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.
David Miller
maintainerRe: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules
From: Ben Hutchings <[email protected]>
Date: Fri, 25 Feb 2011 19:07:59 +0000
Quoted 2 lines
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.
Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules
Quoted 11 lines · David Miller
From: Ben Hutchings <[email protected]>
Date: Fri, 25 Feb 2011 19:07:59 +0000Quoted 2 lines
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_moduleif 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.
Signature · 3 lines
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.
David Miller
maintainerRe: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules
From: Ben Hutchings <[email protected]>
Date: Fri, 25 Feb 2011 19:30:16 +0000
Quoted 10 lines · David Miller
Quoted 9 lines
From: Ben Hutchings <[email protected]>
Date: Fri, 25 Feb 2011 19:07:59 +0000Quoted 2 lines
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_moduleif necessary.
Quoted 7 lines
Quoted 2 lines
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.
Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules
Quoted 16 lines · David Miller
From: Ben Hutchings <[email protected]>
Date: Fri, 25 Feb 2011 19:30:16 +0000Quoted 13 lines · David Miller
Quoted 11 lines
From: Ben Hutchings <[email protected]>
Date: Fri, 25 Feb 2011 19:07:59 +0000Quoted 2 lines
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_moduleif necessary.It's an implementation detail, and not a deterrant to my suggested scheme.
Quoted 8 lines
Quoted 4 lines
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.
Quoted 3 lines
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.
Signature · 3 lines
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.
David Miller
maintainerRe: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules
From: Ben Hutchings <[email protected]>
Date: Fri, 25 Feb 2011 19:53:05 +0000
Quoted 10 lines · David Miller
Quoted 3 lines
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 callingrequest_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.
Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules
In reply to Ben HutchingsQuoted 32 lines · Ben Hutchings
Quoted 16 lines · David Miller
From: Ben Hutchings <[email protected]>
Date: Fri, 25 Feb 2011 19:30:16 +0000Quoted 13 lines · David Miller
Quoted 11 lines
From: Ben Hutchings <[email protected]>
Date: Fri, 25 Feb 2011 19:07:59 +0000Quoted 2 lines
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_moduleif necessary.It's an implementation detail, and not a deterrant to my suggested scheme.
Quoted 8 lines
Quoted 4 lines
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.
Quoted 3 lines
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.
Quoted 5 lines
If you want
insert_module()to be able to deny loading some modules based on the capabilities of the process callingrequest_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:
net/core/dev.c +14 -2
@@ -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.
Signature · 3 lines
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.
Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules
2011/2/25 Ben Hutchings <[email protected]>:
Quoted 20 lines
I bet something like this (plus Vasiliy's changes to static module aliases) would cover 99.9% of legitimate uses of this feature:
net/core/dev.c +7
-2@@ -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 &&
net/core/dev.c void dev_load(struct net *net, const char *name) · line 1139
Quoted 10 lines
net/core/dev.c +7
-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
Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules
Quoted 30 lines · Michał Mirosław
Quoted 17 lines
net/core/dev.c +7
-2@@ -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 &&net/core/dev.c void dev_load(struct net *net, const char *name) · line 1139
Quoted 10 lines
net/core/dev.c +7
-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:
Quoted 4 lines
if (
request_module("netdev-%s", name))
... fallbackThen 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
Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules
Quoted 41 lines · Arnd Bergmann
Quoted 30 lines · Michał Mirosław
Quoted 17 lines
net/core/dev.c +7
-2@@ -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 &&net/core/dev.c void dev_load(struct net *net, const char *name) · line 1139
Quoted 10 lines
net/core/dev.c +7
-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:
Quoted 4 lines
if (
request_module("netdev-%s", name))
... fallbackThen 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_MODULEas well:
Quoted 7 lines
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
Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules
Quoted 11 lines · Michael Tokarev
Quoted 10 lines · Arnd Bergmann
The backwards compatibility should mostly be for systems that today don't use split capabilities, right?
The fallback could therefore rely on
CAP_SYS_MODULEas well:if (request_module("netdev-%s", name)) { if (capable(CAP_SYS_MODULE)) request_module("%s", name); }
Quoted 5 lines
Quoted 2 lines
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,
Signature · 2 lines
Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments
David Miller
maintainerRe: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules
From: Vasiliy Kulikov <[email protected]>
Date: Mon, 28 Feb 2011 12:51:33 +0300
Quoted 1 line
David, are you OK with this way?
Sure.
PATCH 1/1 net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
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 sitFor 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
--- 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(-)
include/linux/netdevice.h +3 -0
@@ -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)
net/core/dev.c +10 -2
@@ -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);
net/ipv4/ip_gre.c +1 -1
@@ -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" );
net/ipv4/ipip.c +1 -1
@@ -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" );
net/ipv6/sit.c +1 -1
@@ -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" );
Signature · 1 line
1.7.0.4
Re: [PATCH] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
Quoted 13 lines · Vasiliy Kulikov
Since
a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565cany process withCAP_NET_ADMINmay load any module from/lib/modules/. This doesn't mean thatCAP_NET_ADMINis a superset ofCAP_SYS_MODULEas modules are limited to/lib/modules/**. However,CAP_NET_ADMINcapability 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_MODULEto maintain the compatibility with network scripts that use autoloading netdev modules by aliases like "eth0", "wlan0".
[...]
net/core/dev.c void dev_load(struct net *net, const char *name) · line 1124
Quoted 13 lines
net/core/dev.c +3
-2@@ -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.
net/core/dev.c void dev_load(struct net *net, const char *name) · line 1129
Quoted 5 lines
net/core/dev.c +5
-0+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.
Signature · 3 lines
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.
PATCH v2 1/1 net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
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 sitFor 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 0Reference: https://lkml.org/lkml/2011/2/24/203
---
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(-)
include/linux/netdevice.h +3 -0
@@ -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)
net/core/dev.c +10 -2
@@ -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);
net/ipv4/ip_gre.c +1 -1
@@ -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" );
net/ipv4/ipip.c +1 -1
@@ -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" );
net/ipv6/sit.c +1 -1
@@ -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" );
Signature · 1 line
1.7.0.4
Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
Quoted 8 lines · Vasiliy Kulikov
Since
a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565cany process withCAP_NET_ADMINmay load any module from/lib/modules/. This doesn't mean thatCAP_NET_ADMINis a superset ofCAP_SYS_MODULEas modules are limited to/lib/modules/**. However,CAP_NET_ADMINcapability 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.
[]
Quoted 3 lines
Reference: https://lkml.org/lkml/2011/2/24/203
Signed-off-by Vasiliy Kulikov <[email protected]>
This looks much saner :)
/mjt
Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
Quoted 9 lines · Michael Tokarev
Quoted 8 lines · Vasiliy Kulikov
Since
a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565cany process withCAP_NET_ADMINmay load any module from/lib/modules/. This doesn't mean thatCAP_NET_ADMINis a superset ofCAP_SYS_MODULEas modules are limited to/lib/modules/**. However,CAP_NET_ADMINcapability 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.
Quoted 8 lines
[]
Quoted 3 lines
Reference: https://lkml.org/lkml/2011/2/24/203
Signed-off-by Vasiliy Kulikov <[email protected]>This looks much saner :)
Signed-off-by Michael Tokarev <[email protected]>
Is there any reason why it is not yet merged? I've answered to Jake Edge that it is not a regression.
Thanks,
Signature · 2 lines
Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments
Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
From: Vasiliy Kulikov <[email protected]>
Date: Thu, 10 Mar 2011 01:06:34 +0300
Quoted 13 lines · Michael Tokarev
Quoted 12 lines
Quoted 8 lines · Vasiliy Kulikov
Since
a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565cany process withCAP_NET_ADMINmay load any module from/lib/modules/. This doesn't mean thatCAP_NET_ADMINis a superset ofCAP_SYS_MODULEas modules are limited to/lib/modules/**. However,CAP_NET_ADMINcapability 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.
[]
Quoted 2 lines
Reference: https://lkml.org/lkml/2011/2/24/203
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
Signed-off-by Vasiliy Kulikov <[email protected]>This looks much saner :)
Signed-off-by Michael Tokarev <[email protected]>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:
Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
Quoted 4 lines · David Miller
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 <[email protected]>
I can submit it via my tree.
Signature · 2 lines
James Morris <[email protected]>
Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
James,
Quoted 1 line · James Morris
I can submit it via my tree.
Thank you!
Signature · 2 lines
Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments
Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
In reply to Vasiliy KulikovQuoted 4 lines · Vasiliy Kulikov
This patch restricts an ability of autoloading modules to netdev modules with explicit aliases. This fixes CVE-2011-1019.
...Signed-off-by Vasiliy Kulikov <[email protected]>
Looks good; thanks for sorting this out.
-Kees
Signature · 2 lines
Kees Cook Ubuntu Security Team
Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
In reply to Vasiliy KulikovI am probably missing something, but shouldn't the existing MODULE_ALIASes stay?
net/ipv4/ip_gre.c module_exit(ipgre_fini); · line 1768
Quoted 10 lines · Vasiliy Kulikov
net/ipv4/ip_gre.c +1
-1@@ -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
Signature · 1 line
Jake Edge - [email protected] - http://lwn.net
Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
Quoted 9 lines · Jake Edge
net/ipv4/ip_gre.c module_exit(ipgre_fini); · line 1768
Quoted 6 lines
net/ipv4/ip_gre.c +1
-1@@ -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.
Signature · 2 lines
Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments
Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
Quoted 11 lines · On Wed, 2 Mar 2011 22:43:54 +0300 Vasiliy Kulikov wrote:
Quoted 9 lines · Jake Edge
net/ipv4/ip_gre.c module_exit(ipgre_fini); · line 1768
Quoted 6 lines
net/ipv4/ip_gre.c +1
-1@@ -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?
Quoted 2 lines
The networking script will run with
CAP_NET_ADMIN, this would request netdev-gre0 and loadip_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
Signature · 1 line
Jake Edge - LWN - [email protected] - http://lwn.net
Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
Quoted 12 lines · Jake Edge
Quoted 11 lines · On Wed, 2 Mar 2011 22:43:54 +0300 Vasiliy Kulikov wrote:
Quoted 9 lines · Jake Edge
net/ipv4/ip_gre.c module_exit(ipgre_fini); · line 1768
Quoted 6 lines
net/ipv4/ip_gre.c +1
-1@@ -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?
Quoted 4 lines
Quoted 2 lines
The networking script will run with
CAP_NET_ADMIN, this would request netdev-gre0 and loadip_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.
Quoted 2 lines
(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.
Signature · 2 lines
Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments
Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
Quoted 10 lines · On Wed, 2 Mar 2011 23:18:07 +0300 Vasiliy Kulikov wrote:
Quoted 1 line
and on systems that today use
CAP_SYS_MODULESince Linux 2.6.32
CAP_SYS_MODULEmay not load modules via "ifconfig gre0". It was changed toCAP_NET_ADMIN. So nothing is broken here.Quoted 2 lines
(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
Signature · 1 line
Jake Edge - LWN - [email protected] - http://lwn.net
Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
In reply to Vasiliy KulikovQuoted 2 lines · 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
Signature · 1 line
Jake Edge - LWN - [email protected] - http://lwn.net
Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
In reply to Vasiliy KulikovQuoted 9 lines · Vasiliy Kulikov
Since
a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565cany process withCAP_NET_ADMINmay load any module from/lib/modules/. This doesn't mean thatCAP_NET_ADMINis a superset ofCAP_SYS_MODULEas modules are limited to/lib/modules/**. However,CAP_NET_ADMINcapability 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.
Quoted 7 lines
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_MODULEto 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_greand 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
Quoted 124 lines
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:1root@albatros:~# lsmod | grep sit sit 10457 0 tunnel4 2957 1 sitFor
CAP_SYS_MODULEmodule 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 0Reference: https://lkml.org/lkml/2011/2/24/203
Signed-off-by Vasiliy Kulikov <[email protected]>---
v2 - usepr_err()
- don't try to load $name if netdev-$name is loadedinclude/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(-)include/linux/netdevice.h +3
-0@@ -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)net/core/dev.c +10
-2@@ -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);net/ipv4/ip_gre.c +1
-1@@ -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" );net/ipv4/ipip.c +1
-1@@ -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" );net/ipv6/sit.c +1
-2@@ -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" ); --
Quoted 7 lines
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
Quoted 1 line · Eric Paris
On Tue, Mar 1, 2011 at 4:33 PM, Vasiliy Kulikov <[email protected]> wrote:
...
Quoted 1 line
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?
Quoted 173 lines
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_SETand 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 useCAP_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 theCAP_SYS_MODULEcase 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 ahas_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 setPF_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_supplicantis calling SIOCGIFINDEX which is causing one of these, libvirt is calling SIOCGIFFLAGS. I'm not sure whatudev->iwis 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
Quoted 124 lines
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:1root@albatros:~# lsmod | grep sit sit 10457 0 tunnel4 2957 1 sitFor
CAP_SYS_MODULEmodule 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 0Reference: https://lkml.org/lkml/2011/2/24/203
Signed-off-by Vasiliy Kulikov <[email protected]>---
v2 - usepr_err()
- don't try to load $name if netdev-$name is loadedinclude/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(-)include/linux/netdevice.h +3
-0@@ -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)net/core/dev.c +10
-2@@ -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);net/ipv4/ip_gre.c +1
-1@@ -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" );net/ipv4/ipip.c +1
-1@@ -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" );net/ipv6/sit.c +1
-2@@ -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" ); --Quoted 7 lines
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
Quoted 11 lines · Serge E. Hallyn
Quoted 1 line · Eric Paris
On Tue, Mar 1, 2011 at 4:33 PM, Vasiliy Kulikov <[email protected]> wrote:
...
Quoted 1 line
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_ADMINbefore checking forCAP_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....
Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
Quoted 13 lines · Eric Paris
Quoted 11 lines · Serge E. Hallyn
Quoted 1 line · Eric Paris
On Tue, Mar 1, 2011 at 4:33 PM, Vasiliy Kulikov <[email protected]> wrote:
...
Quoted 1 line
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_ADMINbefore checking forCAP_SYS_MODULE, so these must be cases which historically always quietly failed, and are now hitting the 'pr_err' which this patch adds?
Quoted 5 lines
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 allowCAP_SYS_MODULEand 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.
[...]
Quoted 9 lines
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 useinit_module(2)we won't get denials.Change this callsite to a
_noauditcheck. 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.
Quoted 5 lines
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.
Signature · 3 lines
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.
Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
Quoted 3 lines · Ben Hutchings
Quoted 2 lines · Eric Paris
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.
...
Quoted 1 line
Quoted 1 line
I think there are 3 possibilities:
...(3)
Quoted 11 lines
Quoted 5 lines
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/classis 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
Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
Quoted 25 lines · On Thu, 24 Mar 2011 15:26:34 -0500 "Serge E. Hallyn" <[email protected]>…
Quoted 3 lines · Ben Hutchings
Quoted 2 lines · Eric Paris
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.
...
Quoted 1 line
Quoted 1 line
I think there are 3 possibilities:
...(3)
Quoted 11 lines
Quoted 5 lines
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/classis somewhat deprecated.
Quoted 4 lines
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"
David Miller
maintainerRe: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
From: Stephen Hemminger <[email protected]>
Date: Thu, 24 Mar 2011 14:39:44 -0700
Quoted 2 lines
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.
Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
Quoted 10 lines · David Miller
From: Stephen Hemminger <[email protected]>
Date: Thu, 24 Mar 2011 14:39:44 -0700Quoted 2 lines
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?
Quoted 9 lines
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
Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
Quoted 7 lines · Serge E. Hallyn
Quoted 6 lines · David Miller
From: Stephen Hemminger <[email protected]>
Date: Thu, 24 Mar 2011 14:39:44 -0700Quoted 2 lines
This breaks for many of the tunneling protocols, that rely on autoload for names like "sit0"
Quoted 7 lines
Quoted 4 lines
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....
Quoted 7 lines
Quoted 7 lines
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.
Quoted 7 lines
Quoted 2 lines
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 forCAP_SYS_ADMINwithout auditing failure (even if it requires a new helper incapability.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
Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
In reply to David MillerQuoted 14 lines · David Miller
From: Stephen Hemminger <[email protected]>
Date: Thu, 24 Mar 2011 14:39:44 -0700Quoted 2 lines
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.
Quoted 6 lines
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
Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
In reply to David MillerQuoted 3 lines · David Miller
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.
Signature · 2 lines
Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments
Re: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules
In reply to David MillerDavid,
Quoted 11 lines · David Miller
From: Ben Hutchings <[email protected]>
Date: Fri, 25 Feb 2011 19:07:59 +0000Quoted 2 lines
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_moduleif 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,
Signature · 2 lines
Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments
David Miller
maintainerRe: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules
From: Vasiliy Kulikov <[email protected]>
Date: Sun, 27 Feb 2011 14:44:38 +0300
Quoted 1 line
d) run modprobe with
CAP_NET_ADMINonly
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.
David Miller
maintainerRe: [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules
In reply to Vasiliy KulikovFrom: Vasiliy Kulikov <[email protected]>
Date: Sun, 27 Feb 2011 14:44:38 +0300
Quoted 3 lines
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.
Quoted 5 lines · Vasiliy Kulikov
Quoted 4 lines · Ben Hutchings
Quoted 3 lines · Vasiliy Kulikov
My proposal is changing
request_module("%s", name)to something likerequest_module("netdev-%s", name)inside ofdev_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
Quoted 20 lines · Michael Tokarev
Quoted 5 lines · Vasiliy Kulikov
Quoted 4 lines · Ben Hutchings
Quoted 3 lines · Vasiliy Kulikov
My proposal is changing
request_module("%s", name)to something likerequest_module("netdev-%s", name)inside ofdev_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.
Quoted 5 lines
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,
Signature · 2 lines
Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments