2011-03-09 19:33:41

by Eric Paris

[permalink] [raw]
Subject: [PATCH -v2] capabilites: allow the application of capability limits to usermode helpers

There is no way to limit the capabilities of usermodehelpers. This problem
reared its head recently when someone complained that any user with
cap_net_admin was able to load arbitrary kernel modules, even though the user
didn't have cap_sys_module. The reason is because the actual load is done by
a usermode helper and those always have the full cap set. This patch addes new
sysctls which allow us to bound the permissions of usermode helpers.

/proc/sys/kernel/usermodehelper/bset
/proc/sys/kernel/usermodehelper/inheritable

You must have CAP_SYS_MODULE to change these (changes are &= ONLY).
When the kernel launches a usermodehelper it will do so with these as
the bset and pI.

-v2: make globals static
create spinlock to protect globals

Signed-off-by: Eric Paris <[email protected]>
No-objection-from: Serge E. Hallyn <[email protected]>
---

include/linux/kmod.h | 3 ++
kernel/kmod.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++
kernel/sysctl.c | 6 +++
3 files changed, 108 insertions(+), 0 deletions(-)

diff --git a/include/linux/kmod.h b/include/linux/kmod.h
index 6efd7a7..79bb98d 100644
--- a/include/linux/kmod.h
+++ b/include/linux/kmod.h
@@ -24,6 +24,7 @@
#include <linux/errno.h>
#include <linux/compiler.h>
#include <linux/workqueue.h>
+#include <linux/sysctl.h>

#define KMOD_PATH_LEN 256

@@ -109,6 +110,8 @@ call_usermodehelper(char *path, char **argv, char **envp, enum umh_wait wait)
NULL, NULL, NULL);
}

+extern struct ctl_table usermodehelper_table[];
+
extern void usermodehelper_init(void);

extern int usermodehelper_disable(void);
diff --git a/kernel/kmod.c b/kernel/kmod.c
index 9cd0591..c1faee7 100644
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -25,6 +25,7 @@
#include <linux/kmod.h>
#include <linux/slab.h>
#include <linux/completion.h>
+#include <linux/cred.h>
#include <linux/file.h>
#include <linux/fdtable.h>
#include <linux/workqueue.h>
@@ -43,6 +44,13 @@ extern int max_threads;

static struct workqueue_struct *khelper_wq;

+#define CAP_BSET (void *)1
+#define CAP_PI (void *)2
+
+static kernel_cap_t usermodehelper_bset = CAP_FULL_SET;
+static kernel_cap_t usermodehelper_inheritable = CAP_FULL_SET;
+static DEFINE_SPINLOCK(umh_sysctl_lock);
+
#ifdef CONFIG_MODULES

/*
@@ -132,6 +140,7 @@ EXPORT_SYMBOL(__request_module);
static int ____call_usermodehelper(void *data)
{
struct subprocess_info *sub_info = data;
+ struct cred *new;
int retval;

spin_lock_irq(&current->sighand->siglock);
@@ -153,6 +162,19 @@ static int ____call_usermodehelper(void *data)
goto fail;
}

+ retval = -ENOMEM;
+ new = prepare_kernel_cred(current);
+ if (!new)
+ goto fail;
+
+ spin_lock(&umh_sysctl_lock);
+ new->cap_bset = cap_intersect(usermodehelper_bset, new->cap_bset);
+ new->cap_inheritable = cap_intersect(usermodehelper_inheritable,
+ new->cap_inheritable);
+ spin_unlock(&umh_sysctl_lock);
+
+ commit_creds(new);
+
retval = kernel_execve(sub_info->path,
(const char *const *)sub_info->argv,
(const char *const *)sub_info->envp);
@@ -418,6 +440,83 @@ unlock:
}
EXPORT_SYMBOL(call_usermodehelper_exec);

+static int proc_cap_handler(struct ctl_table *table, int write,
+ void __user *buffer, size_t *lenp, loff_t *ppos)
+{
+ struct ctl_table t;
+ unsigned long cap_array[_KERNEL_CAPABILITY_U32S];
+ kernel_cap_t new_cap;
+ int err, i;
+
+ if (write && !capable(CAP_SYS_MODULE))
+ return -EPERM;
+
+ /*
+ * convert from the global kernel_cap_t to the ulong array to print to
+ * userspace if this is a read.
+ */
+ spin_lock(&umh_sysctl_lock);
+ for (i = 0; i < _KERNEL_CAPABILITY_U32S; i++) {
+ if (table->data == CAP_BSET)
+ cap_array[i] = usermodehelper_bset.cap[i];
+ else if (table->data == CAP_PI)
+ cap_array[i] = usermodehelper_inheritable.cap[i];
+ else
+ BUG();
+ }
+ spin_unlock(&umh_sysctl_lock);
+
+ t = *table;
+ t.data = &cap_array;
+
+ /*
+ * actually read or write and array of ulongs from userspace. Remember
+ * these are least significant 32 bits first
+ */
+ err = proc_doulongvec_minmax(&t, write, buffer, lenp, ppos);
+ if (err < 0)
+ return err;
+
+ /*
+ * convert from the sysctl array of ulongs to the kernel_cap_t
+ * internal representation
+ */
+ for (i = 0; i < _KERNEL_CAPABILITY_U32S; i++)
+ new_cap.cap[i] = cap_array[i];
+
+ /*
+ * Drop everything not in the new_cap (but don't add things)
+ */
+ spin_lock(&umh_sysctl_lock);
+ if (write) {
+ if (table->data == CAP_BSET)
+ usermodehelper_bset = cap_intersect(usermodehelper_bset, new_cap);
+ if (table->data == CAP_PI)
+ usermodehelper_inheritable = cap_intersect(usermodehelper_inheritable, new_cap);
+ }
+ spin_unlock(&umh_sysctl_lock);
+
+ return 0;
+}
+
+struct ctl_table usermodehelper_table[] = {
+ {
+ .procname = "bset",
+ .data = CAP_BSET,
+ .maxlen = _KERNEL_CAPABILITY_U32S * sizeof(unsigned long),
+ .mode = 0600,
+ .proc_handler = proc_cap_handler,
+ },
+ {
+ .procname = "inheritable",
+ .data = CAP_PI,
+ .maxlen = _KERNEL_CAPABILITY_U32S * sizeof(unsigned long),
+ .mode = 0600,
+ .proc_handler = proc_cap_handler,
+ },
+ { }
+};
+
void __init usermodehelper_init(void)
{
khelper_wq = create_singlethread_workqueue("khelper");
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 79268ee..f8ecc96 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -56,6 +56,7 @@
#include <linux/kprobes.h>
#include <linux/pipe_fs_i.h>
#include <linux/oom.h>
+#include <linux/kmod.h>

#include <asm/uaccess.h>
#include <asm/processor.h>
@@ -610,6 +611,11 @@ static struct ctl_table kern_table[] = {
.child = random_table,
},
{
+ .procname = "usermodehelper",
+ .mode = 0555,
+ .child = usermodehelper_table,
+ },
+ {
.procname = "overflowuid",
.data = &overflowuid,
.maxlen = sizeof(int),


2011-03-09 19:46:32

by Vasily Kulikov

[permalink] [raw]
Subject: Re: [PATCH -v2] capabilites: allow the application of capability limits to usermode helpers

Eric,

On Wed, Mar 09, 2011 at 14:33 -0500, Eric Paris wrote:
> someone complained that any user with
> cap_net_admin was able to load arbitrary kernel modules, even though the user
> didn't have cap_sys_module. The reason is because the actual load is done by
> a usermode helper and those always have the full cap set.

AFAIU, your patch sets system-wide caps for _all_ usermode helpers,
right? Then it does nothing with cap_net_admin's problem as it should
restrict caps of specific helpers spawned from specific networking code,
but not touching anything related to another helpers.

> sysctls which allow us to bound the permissions of usermode helpers.
>
> /proc/sys/kernel/usermodehelper/bset
> /proc/sys/kernel/usermodehelper/inheritable
>
> You must have CAP_SYS_MODULE to change these (changes are &= ONLY).
> When the kernel launches a usermodehelper it will do so with these as
> the bset and pI.

Thanks,

--
Vasiliy

2011-03-09 20:00:57

by Eric Paris

[permalink] [raw]
Subject: Re: [PATCH -v2] capabilites: allow the application of capability limits to usermode helpers

On Wed, 2011-03-09 at 22:45 +0300, Vasiliy Kulikov wrote:
> Eric,
>
> On Wed, Mar 09, 2011 at 14:33 -0500, Eric Paris wrote:
> > someone complained that any user with
> > cap_net_admin was able to load arbitrary kernel modules, even though the user
> > didn't have cap_sys_module. The reason is because the actual load is done by
> > a usermode helper and those always have the full cap set.
>
> AFAIU, your patch sets system-wide caps for _all_ usermode helpers,
> right? Then it does nothing with cap_net_admin's problem as it should
> restrict caps of specific helpers spawned from specific networking code,
> but not touching anything related to another helpers.

I'm actually solving 2 problems at once and it just so happens the the
CAP_NET_ADMIN brew-ha-ha came up more recently. The original problem,
and reason I wrote this patch, is because it's impossible on a modern
Linux system to permanently and irrevocably drop capabilities system
wide. You can get very close by dropping capabilities from the bset
before init is launched which means everything launched by userspace
can't have the dropped capabilities. But, khelper is still going to
have the full set and thus usermodehelpers launched by the kernel will
have the full set. This patch allows one to control usermode helpers.
How one chooses to use that, is up to them.

-Eric

2011-03-09 20:56:07

by David Howells

[permalink] [raw]
Subject: Re: [PATCH -v2] capabilites: allow the application of capability limits to usermode helpers

Eric Paris <[email protected]> wrote:

> There is no way to limit the capabilities of usermodehelpers. This problem
> reared its head recently when someone complained that any user with
> cap_net_admin was able to load arbitrary kernel modules, even though the user
> didn't have cap_sys_module. The reason is because the actual load is done by
> a usermode helper and those always have the full cap set. This patch addes new
> sysctls which allow us to bound the permissions of usermode helpers.
>
> /proc/sys/kernel/usermodehelper/bset
> /proc/sys/kernel/usermodehelper/inheritable
>
> You must have CAP_SYS_MODULE to change these (changes are &= ONLY).
> When the kernel launches a usermodehelper it will do so with these as
> the bset and pI.
>
> -v2: make globals static
> create spinlock to protect globals
>
> Signed-off-by: Eric Paris <[email protected]>
> No-objection-from: Serge E. Hallyn <[email protected]>

Acked-by: David Howells <[email protected]>

2011-03-09 21:38:20

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH -v2] capabilites: allow the application of capability limits to usermode helpers

On Wed, Mar 09, 2011 at 02:33:31PM -0500, Eric Paris wrote:
> There is no way to limit the capabilities of usermodehelpers. This problem
> reared its head recently when someone complained that any user with
> cap_net_admin was able to load arbitrary kernel modules, even though the user
> didn't have cap_sys_module. The reason is because the actual load is done by
> a usermode helper and those always have the full cap set. This patch addes new
> sysctls which allow us to bound the permissions of usermode helpers.
>
> /proc/sys/kernel/usermodehelper/bset
> /proc/sys/kernel/usermodehelper/inheritable

Shouldn't these be documented somewhere? Documentation/ABI?

> You must have CAP_SYS_MODULE to change these (changes are &= ONLY).

Why that permission? Just because 'modprobe' is usually run from this
callback? Or some other reason?

> When the kernel launches a usermodehelper it will do so with these as
> the bset and pI.

Shouldn't the caller of these functions be the ones dictating the
capabilities it should be run with?

thanks,

greg k-h

2011-03-09 22:11:27

by Eric Paris

[permalink] [raw]
Subject: Re: [PATCH -v2] capabilites: allow the application of capability limits to usermode helpers

On Wed, 2011-03-09 at 13:38 -0800, Greg KH wrote:
> On Wed, Mar 09, 2011 at 02:33:31PM -0500, Eric Paris wrote:
> > There is no way to limit the capabilities of usermodehelpers. This problem
> > reared its head recently when someone complained that any user with
> > cap_net_admin was able to load arbitrary kernel modules, even though the user
> > didn't have cap_sys_module. The reason is because the actual load is done by
> > a usermode helper and those always have the full cap set. This patch addes new
> > sysctls which allow us to bound the permissions of usermode helpers.
> >
> > /proc/sys/kernel/usermodehelper/bset
> > /proc/sys/kernel/usermodehelper/inheritable
>
> Shouldn't these be documented somewhere? Documentation/ABI?

Yes, they should. Will do.

> > You must have CAP_SYS_MODULE to change these (changes are &= ONLY).
>
> Why that permission? Just because 'modprobe' is usually run from this
> callback? Or some other reason?

I don't have a good answer for this. I was originally looking at this
thinking about modprobe. Since it's a purely restrictive interface I
probably don't need something quite so strong. I'm trying to decide
what the implications are of usemode helpers being forced to run with
reduced permissions. Andrew, do you have thoughts?

> > When the kernel launches a usermodehelper it will do so with these as
> > the bset and pI.
>
> Shouldn't the caller of these functions be the ones dictating the
> capabilities it should be run with?

Yes. And no. It depends what you mean. The caps of the 'caller' task
are irrelevant. If I run ifconfig ipv6 I need CAP_NET_ADMIN but the
upcall needs CAP_SYS_MODULE. If I plug in a USB drive there is no
'caller' task which makes sense.

Now if by 'caller' you mean 'call site' then yes, we could probably
launch usermodehelpers with reduced privileged sets. We know in the
code when we are asking to launch modprobe that we are going to need
CAP_SYS_MODULE and don't need caps like CAP_SYS_RAWIO and CAP_MAC_ADMIN.
We know when we upcall to hotplug we don't really need any priv, since
it's another task that is going to do the real work. So yeah, there
might be some value in another patch to address this....

But neither solves the problem of being able to eliminate capabilities
from a machine globally. In olden times we had a global cap-bound but
it was dropped in favor of an inheritance from init type mechanism.
Since kthreads don't inherit from init we still end up with this patch.

2011-03-09 22:26:52

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH -v2] capabilites: allow the application of capability limits to usermode helpers

On Wed, Mar 09, 2011 at 05:11:12PM -0500, Eric Paris wrote:
> On Wed, 2011-03-09 at 13:38 -0800, Greg KH wrote:
> > > When the kernel launches a usermodehelper it will do so with these as
> > > the bset and pI.
> >
> > Shouldn't the caller of these functions be the ones dictating the
> > capabilities it should be run with?
>
> Yes. And no. It depends what you mean. The caps of the 'caller' task
> are irrelevant. If I run ifconfig ipv6 I need CAP_NET_ADMIN but the
> upcall needs CAP_SYS_MODULE. If I plug in a USB drive there is no
> 'caller' task which makes sense.
>
> Now if by 'caller' you mean 'call site' then yes, we could probably
> launch usermodehelpers with reduced privileged sets. We know in the
> code when we are asking to launch modprobe that we are going to need
> CAP_SYS_MODULE and don't need caps like CAP_SYS_RAWIO and CAP_MAC_ADMIN.
> We know when we upcall to hotplug we don't really need any priv, since
> it's another task that is going to do the real work. So yeah, there
> might be some value in another patch to address this....

Yes, that is what I was referring to.

> But neither solves the problem of being able to eliminate capabilities
> from a machine globally. In olden times we had a global cap-bound but
> it was dropped in favor of an inheritance from init type mechanism.
> Since kthreads don't inherit from init we still end up with this patch.

I'm not objecting to the patch, or the idea, just want to make sure it
is correct.

thanks,

greg k-h

2011-03-12 17:01:38

by Andrew G. Morgan

[permalink] [raw]
Subject: Re: [PATCH -v2] capabilites: allow the application of capability limits to usermode helpers

I'd prefer it if the invoker had both CAP_SETPCAP and CAP_SYS_MODULE
in its pE set.

Also, have you considered an interface for setting the default values
that is something like a "clone mine"? That is, cache the invoker's
cap_inheritable, and cap_bound? (But not cap_permitted and
cap_effective!) This would take care of atomicity and also allow for
potentially including more state when folk find its logically
necessary as the kernel evolves.

Cheers

Andrew

On Wed, Mar 9, 2011 at 2:26 PM, Greg KH <[email protected]> wrote:
> On Wed, Mar 09, 2011 at 05:11:12PM -0500, Eric Paris wrote:
>> On Wed, 2011-03-09 at 13:38 -0800, Greg KH wrote:
>> > > When the kernel launches a usermodehelper it will do so with these as
>> > > the bset and pI.
>> >
>> > Shouldn't the caller of these functions be the ones dictating the
>> > capabilities it should be run with?
>>
>> Yes. And no. ?It depends what you mean. ?The caps of the 'caller' task
>> are irrelevant. ?If I run ifconfig ipv6 I need CAP_NET_ADMIN but the
>> upcall needs CAP_SYS_MODULE. ?If I plug in a USB drive there is no
>> 'caller' task which makes sense.
>>
>> Now if by 'caller' you mean 'call site' then yes, we could probably
>> launch usermodehelpers with reduced privileged sets. ?We know in the
>> code when we are asking to launch modprobe that we are going to need
>> CAP_SYS_MODULE and don't need caps like CAP_SYS_RAWIO and CAP_MAC_ADMIN.
>> We know when we upcall to hotplug we don't really need any priv, since
>> it's another task that is going to do the real work. ?So yeah, there
>> might be some value in another patch to address this....
>
> Yes, that is what I was referring to.
>
>> But neither solves the problem of being able to eliminate capabilities
>> from a machine globally. ?In olden times we had a global cap-bound but
>> it was dropped in favor of an inheritance from init type mechanism.
>> Since kthreads don't inherit from init we still end up with this patch.
>
> I'm not objecting to the patch, or the idea, just want to make sure it
> is correct.
>
> thanks,
>
> greg k-h
>