From: Djalal Harouni <tixxdz@gmail.com>
To: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-security-module@vger.kernel.org,
kernel-hardening@lists.openwall.com,
Andy Lutomirski <luto@kernel.org>,
Kees Cook <keescook@chromium.org>,
Andrew Morton <akpm@linux-foundation.org>,
Rusty Russell <rusty@rustcorp.com.au>,
"Serge E. Hallyn" <serge@hallyn.com>,
Jessica Yu <jeyu@redhat.com>
Cc: "David S. Miller" <davem@davemloft.net>,
James Morris <james.l.morris@oracle.com>,
Paul Moore <paul@paul-moore.com>,
Stephen Smalley <sds@tycho.nsa.gov>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
Ingo Molnar <mingo@kernel.org>,
Linux API <linux-api@vger.kernel.org>,
Dongsu Park <dpark@posteo.net>,
Casey Schaufler <casey@schaufler-ca.com>,
Jonathan Corbet <corbet@lwn.net>,
Arnaldo Carvalho de Melo <acme@redhat.com>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
Zendyani <zendyani@gmail.com>,
linux-doc@vger.kernel.org, Al Viro <viro@zeniv.linux.org.uk>,
Ben Hutchings <ben.hutchings@codethink.co.uk>,
Djalal Harouni <tixxdz@gmail.com>
Subject: [PATCH v4 next 1/3] modules:capabilities: allow __request_module() to take a capability argument
Date: Mon, 22 May 2017 13:57:04 +0200 [thread overview]
Message-ID: <1495454226-10027-2-git-send-email-tixxdz@gmail.com> (raw)
In-Reply-To: <1495454226-10027-1-git-send-email-tixxdz@gmail.com>
This is a preparation patch for the module auto-load restriction feature.
In order to restrict module auto-load operations we need to check if the
caller has CAP_SYS_MODULE capability. This allows to align security
checks of automatic module loading with the checks of the explicit operations.
However for "netdev-%s" modules, they are allowed to be loaded if
CAP_NET_ADMIN is set. Therefore, in order to not break this assumption,
and allow userspace to only load "netdev-%s" modules with CAP_NET_ADMIN
capability which is considered a privileged operation, we have two
choices: 1) parse "netdev-%s" alias and check the capability or 2) hand
the capability form request_module() to security_kernel_module_request()
hook and let the capability subsystem decide.
After a discussion with Rusty Russell [1], the suggestion was to pass
the capability from request_module() to security_kernel_module_request()
for 'netdev-%s' modules that need CAP_NET_ADMIN.
The patch does not update request_module(), it updates the internal
__request_module() that will take an extra "allow_cap" argument. If
positive, then automatic module load operation can be allowed.
__request_module() will be only called by networking code which is the
exception to this, so we do not break userspace and CAP_NET_ADMIN can
continue to load 'netdev-%s' modules. Other kernel code should continue
to use request_module() which calls security_kernel_module_request() and
will check for CAP_SYS_MODULE capability in next patch. Allowing more
control on who can trigger automatic module loading.
This patch updates security_kernel_module_request() to take the
'allow_cap' argument and SELinux which is currently the only user of
security_kernel_module_request() hook.
Based on patch by Rusty Russell:
https://lkml.org/lkml/2017/4/26/735
Cc: Serge Hallyn <serge@hallyn.com>
Cc: Andy Lutomirski <luto@kernel.org>
Suggested-by: Rusty Russell <rusty@rustcorp.com.au>
Suggested-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Djalal Harouni <tixxdz@gmail.com>
[1] https://lkml.org/lkml/2017/4/24/7
---
include/linux/kmod.h | 15 ++++++++-------
include/linux/lsm_hooks.h | 4 +++-
include/linux/security.h | 4 ++--
kernel/kmod.c | 15 +++++++++++++--
net/core/dev_ioctl.c | 10 +++++++++-
security/security.c | 4 ++--
security/selinux/hooks.c | 2 +-
7 files changed, 38 insertions(+), 16 deletions(-)
diff --git a/include/linux/kmod.h b/include/linux/kmod.h
index c4e441e..a314432 100644
--- a/include/linux/kmod.h
+++ b/include/linux/kmod.h
@@ -32,18 +32,19 @@
extern char modprobe_path[]; /* for sysctl */
/* modprobe exit status on success, -ve on error. Return value
* usually useless though. */
-extern __printf(2, 3)
-int __request_module(bool wait, const char *name, ...);
-#define request_module(mod...) __request_module(true, mod)
-#define request_module_nowait(mod...) __request_module(false, mod)
+extern __printf(3, 4)
+int __request_module(bool wait, int allow_cap, const char *name, ...);
#define try_then_request_module(x, mod...) \
- ((x) ?: (__request_module(true, mod), (x)))
+ ((x) ?: (__request_module(true, -1, mod), (x)))
#else
-static inline int request_module(const char *name, ...) { return -ENOSYS; }
-static inline int request_module_nowait(const char *name, ...) { return -ENOSYS; }
+static inline __printf(3, 4)
+int __request_module(bool wait, int allow_cap, const char *name, ...)
+{ return -ENOSYS; }
#define try_then_request_module(x, mod...) (x)
#endif
+#define request_module(mod...) __request_module(true, -1, mod)
+#define request_module_nowait(mod...) __request_module(false, -1, mod)
struct cred;
struct file;
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index f7914d9..7688f79 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -578,6 +578,8 @@
* Ability to trigger the kernel to automatically upcall to userspace for
* userspace to load a kernel module with the given name.
* @kmod_name name of the module requested by the kernel
+ * @allow_cap capability that allows to automatically load a kernel
+ * module.
* Return 0 if successful.
* @kernel_read_file:
* Read a file specified by userspace.
@@ -1516,7 +1518,7 @@ union security_list_options {
void (*cred_transfer)(struct cred *new, const struct cred *old);
int (*kernel_act_as)(struct cred *new, u32 secid);
int (*kernel_create_files_as)(struct cred *new, struct inode *inode);
- int (*kernel_module_request)(char *kmod_name);
+ int (*kernel_module_request)(char *kmod_name, int allow_cap);
int (*kernel_read_file)(struct file *file, enum kernel_read_file_id id);
int (*kernel_post_read_file)(struct file *file, char *buf, loff_t size,
enum kernel_read_file_id id);
diff --git a/include/linux/security.h b/include/linux/security.h
index 549cb82..2f4c9d3 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -325,7 +325,7 @@ int security_prepare_creds(struct cred *new, const struct cred *old, gfp_t gfp);
void security_transfer_creds(struct cred *new, const struct cred *old);
int security_kernel_act_as(struct cred *new, u32 secid);
int security_kernel_create_files_as(struct cred *new, struct inode *inode);
-int security_kernel_module_request(char *kmod_name);
+int security_kernel_module_request(char *kmod_name, int allow_cap);
int security_kernel_read_file(struct file *file, enum kernel_read_file_id id);
int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
enum kernel_read_file_id id);
@@ -926,7 +926,7 @@ static inline int security_kernel_create_files_as(struct cred *cred,
return 0;
}
-static inline int security_kernel_module_request(char *kmod_name)
+static inline int security_kernel_module_request(char *kmod_name, int allow_cap)
{
return 0;
}
diff --git a/kernel/kmod.c b/kernel/kmod.c
index 563f97e..15c96e8 100644
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -110,6 +110,7 @@ static int call_modprobe(char *module_name, int wait)
/**
* __request_module - try to load a kernel module
* @wait: wait (or not) for the operation to complete
+ * @allow_cap: if positive, may allow modprobe if this capability is set.
* @fmt: printf style format string for the name of the module
* @...: arguments as specified in the format string
*
@@ -120,10 +121,20 @@ static int call_modprobe(char *module_name, int wait)
* must check that the service they requested is now available not blindly
* invoke it.
*
+ * If "allow_cap" is positive, The security subsystem will trust the caller
+ * that "allow_cap" may allow to load some modules with a specific alias,
+ * the security subsystem will make some exceptions based on that. This is
+ * primally useful for backward compatibility. A permission check should not
+ * be that strict and userspace should be able to continue to trigger module
+ * auto-loading if needed.
+ *
* If module auto-loading support is disabled then this function
* becomes a no-operation.
+ *
+ * This function should not be directly used by other subsystems, for that
+ * please call request_module().
*/
-int __request_module(bool wait, const char *fmt, ...)
+int __request_module(bool wait, int allow_cap, const char *fmt, ...)
{
va_list args;
char module_name[MODULE_NAME_LEN];
@@ -150,7 +161,7 @@ int __request_module(bool wait, const char *fmt, ...)
if (ret >= MODULE_NAME_LEN)
return -ENAMETOOLONG;
- ret = security_kernel_module_request(module_name);
+ ret = security_kernel_module_request(module_name, allow_cap);
if (ret)
return ret;
diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index b94b1d2..c494351 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -366,8 +366,16 @@ void dev_load(struct net *net, const char *name)
rcu_read_unlock();
no_module = !dev;
+ /*
+ * First do the CAP_NET_ADMIN check, then let the security
+ * subsystem checks know that this can be allowed since this is
+ * a "netdev-%s" module and CAP_NET_ADMIN is set.
+ *
+ * For this exception call __request_module().
+ */
if (no_module && capable(CAP_NET_ADMIN))
- no_module = request_module("netdev-%s", name);
+ no_module = __request_module(true, CAP_NET_ADMIN,
+ "netdev-%s", name);
if (no_module && capable(CAP_SYS_MODULE))
request_module("%s", name);
}
diff --git a/security/security.c b/security/security.c
index 714433e..cedb790 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1021,9 +1021,9 @@ int security_kernel_create_files_as(struct cred *new, struct inode *inode)
return call_int_hook(kernel_create_files_as, 0, new, inode);
}
-int security_kernel_module_request(char *kmod_name)
+int security_kernel_module_request(char *kmod_name, int allow_cap)
{
- return call_int_hook(kernel_module_request, 0, kmod_name);
+ return call_int_hook(kernel_module_request, 0, kmod_name, allow_cap);
}
int security_kernel_read_file(struct file *file, enum kernel_read_file_id id)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 158f6a0..85eeff6 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3842,7 +3842,7 @@ static int selinux_kernel_create_files_as(struct cred *new, struct inode *inode)
return ret;
}
-static int selinux_kernel_module_request(char *kmod_name)
+static int selinux_kernel_module_request(char *kmod_name, int allow_cap)
{
struct common_audit_data ad;
--
2.10.2
next prev parent reply other threads:[~2017-05-22 11:57 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-22 11:57 [PATCH v4 next 0/3] modules: automatic module loading restrictions Djalal Harouni
2017-05-22 11:57 ` Djalal Harouni [this message]
2017-05-22 22:20 ` [PATCH v4 next 1/3] modules:capabilities: allow __request_module() to take a capability argument Kees Cook
2017-05-23 10:29 ` Djalal Harouni
2017-05-23 19:19 ` Kees Cook
2017-05-24 14:16 ` Djalal Harouni
2017-05-30 17:59 ` Kees Cook
2017-06-01 14:56 ` Djalal Harouni
2017-06-01 19:10 ` Kees Cook
2017-09-02 6:31 ` Djalal Harouni
2017-05-22 11:57 ` [PATCH v4 next 2/3] modules:capabilities: automatic module loading restriction Djalal Harouni
2017-05-22 22:28 ` Kees Cook
2017-05-22 11:57 ` [PATCH v4 next 3/3] modules:capabilities: add a per-task modules auto-load mode Djalal Harouni
2017-05-23 14:18 ` kbuild test robot
2017-05-22 12:08 ` [PATCH v4 next 0/3] modules: automatic module loading restrictions Solar Designer
[not found] ` <20170522120848.GA3003-cxoSlKxDwOJWk0Htik3J/w@public.gmane.org>
2017-05-22 13:49 ` [kernel-hardening] " Djalal Harouni
[not found] ` <CAEiveUdqfMk4+vLg6TaEJNSGwoQHxYq0P4aqZoL4i9GgR3Vdtw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-05-22 16:43 ` Solar Designer
[not found] ` <20170522164323.GA2048-cxoSlKxDwOJWk0Htik3J/w@public.gmane.org>
2017-05-22 19:55 ` Djalal Harouni
2017-05-22 23:07 ` Kees Cook
2017-05-22 23:38 ` Andy Lutomirski
2017-05-22 23:52 ` Kees Cook
2017-05-23 13:02 ` Djalal Harouni
2017-05-23 7:48 ` [kernel-hardening] " Solar Designer
[not found] ` <20170523074808.GA4562-cxoSlKxDwOJWk0Htik3J/w@public.gmane.org>
2017-05-23 18:36 ` Kees Cook
2017-05-23 19:50 ` Andy Lutomirski
2017-05-24 18:06 ` Djalal Harouni
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1495454226-10027-2-git-send-email-tixxdz@gmail.com \
--to=tixxdz@gmail.com \
--cc=acme@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=ben.hutchings@codethink.co.uk \
--cc=casey@schaufler-ca.com \
--cc=corbet@lwn.net \
--cc=davem@davemloft.net \
--cc=dpark@posteo.net \
--cc=gregkh@linuxfoundation.org \
--cc=james.l.morris@oracle.com \
--cc=jeyu@redhat.com \
--cc=keescook@chromium.org \
--cc=kernel-hardening@lists.openwall.com \
--cc=linux-api@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=luto@kernel.org \
--cc=mchehab@kernel.org \
--cc=mingo@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=paul@paul-moore.com \
--cc=penguin-kernel@I-love.SAKURA.ne.jp \
--cc=peterz@infradead.org \
--cc=rusty@rustcorp.com.au \
--cc=sds@tycho.nsa.gov \
--cc=serge@hallyn.com \
--cc=viro@zeniv.linux.org.uk \
--cc=zendyani@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).