From patchwork Mon May 22 11:57:04 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Djalal Harouni X-Patchwork-Id: 9739985 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 6C3166034C for ; Mon, 22 May 2017 11:58:15 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5281B286D1 for ; Mon, 22 May 2017 11:58:15 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 46F4C286DE; Mon, 22 May 2017 11:58:15 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_MED, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from mother.openwall.net (mother.openwall.net [195.42.179.200]) by mail.wl.linuxfoundation.org (Postfix) with SMTP id 913D0286DC for ; Mon, 22 May 2017 11:58:12 +0000 (UTC) Received: (qmail 24434 invoked by uid 550); 22 May 2017 11:58:06 -0000 Mailing-List: contact kernel-hardening-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Delivered-To: mailing list kernel-hardening@lists.openwall.com Received: (qmail 24308 invoked from network); 22 May 2017 11:58:05 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=8Xa1ciLc61jV95Xu0u3grOfmPMQnk1fdCZ9fFOqrcTQ=; b=OO83skoaYefAK/dbrcCd+y3UqGAM1zq9Lbfv8Z31QvJLuOnOz+rVw96HKknYVAjTVp TWrc3ntUW4dKTt8pHNYbXeeSJ5KMIpT+B9a9CoovCqPnVsUHLbkUWdxftwcyBoTWtvf5 dlKbLb+6afRn1yqIsLzjbBOgRvNDkgsanSUVZZaR6IZMTV7vI5YsDU6UfbRGDUu04Jcr /i4rJL1rg2bft8DrnFbgh/7VvUeypqt97TTSfpPKtx9JwQIYMS2XRQZI5J+LiAK5vgYE 6IcNJytec6BLzveZoqeNdDuIk1qCAmaaLhNIDN6xK2CzOEg0x1tc8ULHz4tco3Mvyc3W fDlA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=8Xa1ciLc61jV95Xu0u3grOfmPMQnk1fdCZ9fFOqrcTQ=; b=LUa3RcwiGEx5dhU9rgDwEERFLMyBFs4yA1mLOf5hr4g4gP/72nFOeWSTXLbTIvq93t 2mZFySZ/CEJJW7AHinRmfY6fGrJKJ2GExD3FaIA9yjJegKFzezy7fASbfV80FMrpDMqL RvKpRLfeTn5D//gVO7g3LWnPXB1Md+IFke2lJ6dfjqHXOhj2a+tkdA/6YeEXxauYvvs+ eF36/ytB/bfMt6gh6agX10FrglMeay9ACaF9+d2JRJHuhdYkia+Y6nOOzIC5pZuP2oSf qTHAwKcv8m4gTRN//+1BIZcpv186P23WHfyjnUjgO2VwhuG1FP4lr2EQpDWy3dFWZS89 jLQQ== X-Gm-Message-State: AODbwcDHUO5DTBjb7QEKX009K7CDY9ncuN4p6Py/vAfNyFhPD8BTW/De VrlXqvIK7SZY8g== X-Received: by 10.80.168.102 with SMTP id j93mr17711926edc.32.1495454274396; Mon, 22 May 2017 04:57:54 -0700 (PDT) From: Djalal Harouni To: linux-kernel@vger.kernel.org, netdev@vger.kernel.org, linux-security-module@vger.kernel.org, kernel-hardening@lists.openwall.com, Andy Lutomirski , Kees Cook , Andrew Morton , Rusty Russell , "Serge E. Hallyn" , Jessica Yu Cc: "David S. Miller" , James Morris , Paul Moore , Stephen Smalley , Greg Kroah-Hartman , Tetsuo Handa , Ingo Molnar , Linux API , Dongsu Park , Casey Schaufler , Jonathan Corbet , Arnaldo Carvalho de Melo , Mauro Carvalho Chehab , Peter Zijlstra , Zendyani , linux-doc@vger.kernel.org, Al Viro , Ben Hutchings , Djalal Harouni Date: Mon, 22 May 2017 13:57:04 +0200 Message-Id: <1495454226-10027-2-git-send-email-tixxdz@gmail.com> X-Mailer: git-send-email 2.5.5 In-Reply-To: <1495454226-10027-1-git-send-email-tixxdz@gmail.com> References: <1495454226-10027-1-git-send-email-tixxdz@gmail.com> Subject: [kernel-hardening] [PATCH v4 next 1/3] modules:capabilities: allow __request_module() to take a capability argument X-Virus-Scanned: ClamAV using ClamSMTP 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 Cc: Andy Lutomirski Suggested-by: Rusty Russell Suggested-by: Kees Cook Signed-off-by: Djalal Harouni [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;