Re: user ns: arbitrary module loading

[ebiederm@xmission.com (Eric W. Biederman)]

Kees Cook <keescook@google.com> writes:

> On Sun, Mar 3, 2013 at 1:58 PM, Eric W. Biederman <ebiederm@xmission.com> wrote:

> Ah-ha, thanks! Yes, that worked great. I think map_write()'s
> cap_valid/ns_capable calls confused me. :)

Yes permissions across user namespaces can be a little weird.  But
mostly if you are their creator, you are their all powerful god and
they must watch out.

>> For netdev at least I don't see it as being particularly interesting.
>> The case for tunnels is already as unprivileged as you can reasonably
>> get with "request_module("rtnl-link-%s", kind);" in
>> net/core/rtnetlink.c:rtnl_newlink().  For real physical devices there is
>> both greater chance of a buggy module and no realy need as udev will
>> load the module based on hardware auto-discovery.
>>
>> This leads to the fundamental question: Should we require privilege to
>> request the load filesystem modules?
>
> I think that the past dictated the need for privilege due to it being
> tied to mounting. With userns, this is weakened, but it seems like the
> privilege should at least attempt to segregate caps to certain classes
> of modules.

With filesystems in particular the attack surface is practically
non-existent if the filesystem is not mounted, and FS_USERNS_MOUNT
prevents most filesystems from being mounted.  So right now I do not see
any real danger in allowing filesystem modules to be auto-loaded.

>> I have looked at GRKERNSEC_MODHARDEN to see if that could give me some
>> guidance.  Unfortunately GRKERNSEC_MODHARDEN takes the position that fs
>> kernel modules are the only kernel modules that should ever auto-load
>> and only in very specific situations.  So I can't see that being the
>> normal kernel policy, especially since there are the sysctls
>
> Right -- modharden's basic goal is to block all non-root autoloading.
> It had to work around some corner-cases (mount being setuid, etc).

The mail goal appears to be disable module auto-loading and to log
a message.

>> /proc/sys/kernel/modprobe and /proc/sys/kernel/modules_disabled.
>
> Right -- this gives the granularity of "autoloading" and "loading"
> respectively, but there is no concept of "only privileged autoloading"
> in the current kernel.

As of the 3.8 patch when a module is loaded it does:

+#ifdef CONFIG_GRKERNSEC_MODHARDEN
+	{
+		char *p, *p2;
+
+		if (strstr(mod->args, "grsec_modharden_netdev")) {
+			printk(KERN_ALERT "grsec: denied auto-loading kernel module for a network device with CAP_SYS_MODULE (deprecated).  Use CAP_NET_ADMIN and alias netdev-%.64s instead.", mod->name);
+			err = -EPERM;
+			goto free_modinfo;
+		} else if ((p = strstr(mod->args, "grsec_modharden_normal"))) {
+			p += sizeof("grsec_modharden_normal") - 1;
+			p2 = strstr(p, "_");
+			if (p2) {
+				*p2 = '\0';
+				printk(KERN_ALERT "grsec: denied kernel module auto-load of %.64s by uid %.9s\n", mod->name, p);
+				*p2 = '_';
+			}
+			err = -EPERM;
+			goto free_modinfo;
+		}
+	}
+#endif

Which simplifies to
	if (strstr(mod->args, "grsec_modharden_fs") != 0) {
		err = -EPERM;
		goto  free_modinfo;
	}

So perhaps it tries but there is not really a concept of privileged
processes being able to auto-load anything except fs modules. 

In general what I see GRKERNSEC_MODHARDEN implementing is a policy of
printing a logging a message instead of auto-loading any kernel modules.

> It seems to me that unpriv users shouldn't be able to arbitrarily load
> kernel modules, but if userns continues in this direction, there will
> always be a path to doing autoloading for each different subsystem's
> modules, ultimately leading to unpriv loading. Still, I think it's
> worth creating obvious subsystem aliases so userspace can more easily
> blacklist/whitelist things.

But I don't see any problem with uprivileged users being able to cause
the kernel to request kernel modules, as long as the request is clear
enough that modprobe can implement the desired policy.

>> Overall the basic policy building blocks for controlling which modules
>> are loaded seem solid and in use.  So I don't see any particular reason
>> why the kernel's default policy should not be to allow any users actions
>> to request modules.
>>
>> So I think I am going to scrap the change sitting in my development tree
>> to require capalbe(CAP_SYS_ADMIN) to load a filesystem module and just
>> go with my request_module("fs-%.*",...); change. That is simple and
>> seems to match the rest of the kernel.
>
> Agreed.

Eric