From patchwork Mon Nov 27 17:18:34 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Djalal Harouni X-Patchwork-Id: 10077639 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 7CEF3602BC for ; Mon, 27 Nov 2017 17:19:35 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5D85C28DC1 for ; Mon, 27 Nov 2017 17:19:35 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 51A5028E4E; Mon, 27 Nov 2017 17:19:35 +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 999D828DC1 for ; Mon, 27 Nov 2017 17:19:33 +0000 (UTC) Received: (qmail 30237 invoked by uid 550); 27 Nov 2017 17:19:22 -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 30032 invoked from network); 27 Nov 2017 17:19:20 -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=3DXqO672+Sc/HCHGS3geajug2NhcZt1tvhE0nQjKIEo=; b=AvhxGHtsN2H8cJI8Lk/sLSPGre5xRnsv3a+r1bkfSXMKSltXIyoAn6iuY7veSVNymk Rcyo5t9wvYV26LVBn+k+ZHhPLAJ72G914if5izC0bEHryt8ApqR/hajLy2T3GviDDnMK 2OXvfYX6wjvWBaaoQqQFKoytZA08gQty+fp5pEZBCx51Xy1z0vO3LlbFrxCJ4q7IPufC LUyDjirkCNwM/rtJ+FTbmFEABSKPavA1IhG6LdDzbzz5tsscaQ4wWRvpnTmLEsFVVCjh h1jiB409sVCzscaCNNHD526nFJK00jo8m4XwgEULXfdc3p01Xt5DlBqo0lFMONTJyzw/ B0qA== 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=3DXqO672+Sc/HCHGS3geajug2NhcZt1tvhE0nQjKIEo=; b=WbNbqmb7Z2j0GBN1Wfq8C/y09SNvVzRwiufffImLy2rd4za/XD6nYx2E3XEw0Q+6W+ pD0yuX+b7OTwRlE4hDFfhS7c8w3IrFjlnJoLJksGy8ypqAbna9dhKMuc1FhBtIfwgagt 1c79rtAC2U9BvM4CBmT5GEm/j5DCVUT7Lq9aX+uSt1mlbhRdfH65BjP8W6iekkJaEuyU nxucHTJYQv1IQDBRwEdkuhtHUMO10te3k1+rUlMbIg56a7hsAeiLxC4rLub/8NDO+zIv enUKnVOhj+YCFGwiYjXtaZ7qzpfFxFjQMAfjkZgkX9yx6ZhAbAlcINB2jSwGu3U8vlKZ Y8sw== X-Gm-Message-State: AJaThX60JmhSlLQtPV8SGEd/6CGvW3CUUz0M9jhyWvrA3NeM6B+EYsdd sVTSvxJyxUKC6xa6fheAFwY= X-Google-Smtp-Source: AGs4zMaaGMLu1n+rSKFlljSqopvrDbpcTHrp9vzid64j59HYq7/AYgCTNHkGVgH735ATmgeCU9vuTg== X-Received: by 10.80.170.87 with SMTP id p23mr55669371edc.289.1511803149156; Mon, 27 Nov 2017 09:19:09 -0800 (PST) From: Djalal Harouni To: Kees Cook , Andy Lutomirski , Andrew Morton , "Luis R. Rodriguez" , James Morris , Ben Hutchings , Solar Designer , Serge Hallyn , Jessica Yu , Rusty Russell , linux-kernel@vger.kernel.org, linux-security-module@vger.kernel.org, kernel-hardening@lists.openwall.com Cc: Jonathan Corbet , Ingo Molnar , "David S. Miller" , netdev@vger.kernel.org, Peter Zijlstra , Linus Torvalds , Djalal Harouni Date: Mon, 27 Nov 2017 18:18:34 +0100 Message-Id: <1511803118-2552-2-git-send-email-tixxdz@gmail.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1511803118-2552-1-git-send-email-tixxdz@gmail.com> References: <1511803118-2552-1-git-send-email-tixxdz@gmail.com> Subject: [kernel-hardening] [PATCH v5 next 1/5] modules:capabilities: add request_module_cap() X-Virus-Scanned: ClamAV using ClamSMTP This is a preparation patch to improve the module auto-load infrastructure. We need this patch to have more control on module auto-load operations. The operation by default is allowed unless enduser or the calling code requests that we need to perform futher permission checks. With this change subsystems will be able to decide if module auto-load feature first will have to do a capability check and load the module if the permission check succeeds or deny the operation. As an example "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 load "netdev-%s" modules with CAP_NET_ADMIN, we have added: request_module_cap(required_cap, prefix, fmt...) This new function will take: '@required_cap': Required capability to load the module '@prefix': The module prefix if any, otherwise NULL '@fmt': printf style format string for the name of the module with its arguments if any ex: request_module_cap(CAP_NET_ADMIN, "netdev", "%s", mod); 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, and after review from Kees Cook [2] and experimenting with the code, the patch now does the following: * Adds the request_module_cap() function. * Updates the __request_module() to take the "required_cap" argument with the "prefix". This patch also updates SELinux which is currently the only user of security_kernel_module_request(), the security hook now accepts 'required_cap' and 'prefix' as arguments. Based on patch by Rusty Russell and discussion with Kees Cook: [1] https://lkml.org/lkml/2017/4/26/735 [2] https://lkml.org/lkml/2017/5/23/775 Cc: Serge Hallyn Cc: Andy Lutomirski Suggested-by: Rusty Russell Suggested-by: Kees Cook Signed-off-by: Djalal Harouni --- include/linux/kmod.h | 65 ++++++++++++++++++++++++++++++++++++++++++----- include/linux/lsm_hooks.h | 6 ++++- include/linux/security.h | 7 +++-- kernel/kmod.c | 29 ++++++++++++++++----- security/security.c | 6 +++-- security/selinux/hooks.c | 3 ++- 6 files changed, 97 insertions(+), 19 deletions(-) diff --git a/include/linux/kmod.h b/include/linux/kmod.h index 40c89ad..ccd6a1c 100644 --- a/include/linux/kmod.h +++ b/include/linux/kmod.h @@ -33,16 +33,67 @@ 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(4, 5) +int __request_module(bool wait, int required_cap, + const char *prefix, const char *name, ...); #define try_then_request_module(x, mod...) \ - ((x) ?: (__request_module(true, mod), (x))) + ((x) ?: (__request_module(true, -1, NULL, 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(4, 5) +int __request_module(bool wait, int required_cap, + const char *prefix, const char *name, ...) +{ return -ENOSYS; } #define try_then_request_module(x, mod...) (x) #endif +/** + * request_module Try to load a kernel module + * + * Automatically loads the request module. + * + * @mod...: The module name + */ +#define request_module(mod...) __request_module(true, -1, NULL, mod) + +#define request_module_nowait(mod...) __request_module(false, -1, NULL, mod) + +/** + * request_module_cap Load kernel module only if the required capability is set + * + * Automatically load a module if the required capability is set and it + * corresponds to the appropriate subsystem that is indicated by prefix. + * + * This allows to load aliased modules like 'netdev-%s' with CAP_NET_ADMIN. + * + * ex: + * request_module_cap(CAP_NET_ADMIN, "netdev", "%s", mod); + * + * @required_cap: Required capability to load the module + * @prefix: The module prefix if any, otherwise NULL + * @fmt: printf style format string for the name of the module with its + * arguments if any + * + * If '@required_cap' is positive, the security subsystem will check if + * '@prefix' is set and if caller has the required capability then the + * operation is allowed. + * The security subsystem can not make assumption about the boundaries + * of other subsystems, it is their responsability to make a call with + * the right capability and module alias. + * + * If '@required_cap' is positive and '@prefix' is NULL then we assume + * that the '@required_cap' is CAP_SYS_MODULE. + * + * If '@required_cap' is negative then there are no permission checks, this + * is the equivalent to request_module() function. + * + * This function trust callers to pass the right capability with the + * appropriate prefix. + * + * Note: the permission checks may still fail, even if the required + * capability is negative, this is due to module loading restrictions + * that are controlled by the enduser. + */ +#define request_module_cap(required_cap, prefix, fmt...) \ + __request_module(true, required_cap, prefix, fmt) + #endif /* __LINUX_KMOD_H__ */ diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 7161d8e..d898bd0 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -571,6 +571,9 @@ * 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 + * @required_cap If positive, the required capability to automatically load + * the correspondig kernel module. + * @prefix The prefix of the module if any. Can be NULL. * Return 0 if successful. * @kernel_read_file: * Read a file specified by userspace. @@ -1543,7 +1546,8 @@ 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 required_cap, + const char *prefix); 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 73f1ef6..41e700a 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -326,7 +326,8 @@ 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 required_cap, + const char *prefix); 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); @@ -919,7 +920,9 @@ 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 required_cap, + const char *prefix) { return 0; } diff --git a/kernel/kmod.c b/kernel/kmod.c index bc6addd..679d401 100644 --- a/kernel/kmod.c +++ b/kernel/kmod.c @@ -109,6 +109,8 @@ 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 + * @required_cap: required capability to load the module + * @prefix: module prefix if any otherwise NULL * @fmt: printf style format string for the name of the module * @...: arguments as specified in the format string * @@ -119,14 +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 module auto-loading support is disabled then this function - * becomes a no-operation. + * If "required_cap" is positive, The security subsystem will trust the caller + * that if it has "required_cap" then it may allow to load some modules that + * have a specific alias. + * + * If module auto-loading support is disabled by clearing the modprobe path + * then this function becomes a no-operation. */ -int __request_module(bool wait, const char *fmt, ...) +int __request_module(bool wait, int required_cap, + const char *prefix, const char *fmt, ...) { va_list args; char module_name[MODULE_NAME_LEN]; int ret; + int len = 0; /* * We don't allow synchronous module loading from async. Module @@ -139,13 +147,22 @@ int __request_module(bool wait, const char *fmt, ...) if (!modprobe_path[0]) return 0; + /* + * Lets attach the prefix to the module name + */ + if (prefix != NULL && *prefix != '\0') { + len += snprintf(module_name, MODULE_NAME_LEN, "%s-", prefix); + if (len >= MODULE_NAME_LEN) + return -ENAMETOOLONG; + } + va_start(args, fmt); - ret = vsnprintf(module_name, MODULE_NAME_LEN, fmt, args); + ret = vsnprintf(module_name + len, MODULE_NAME_LEN - len, fmt, args); va_end(args); - if (ret >= MODULE_NAME_LEN) + if (ret >= MODULE_NAME_LEN - len) return -ENAMETOOLONG; - ret = security_kernel_module_request(module_name); + ret = security_kernel_module_request(module_name, required_cap, prefix); if (ret) return ret; diff --git a/security/security.c b/security/security.c index 1cd8526..91ecebd 100644 --- a/security/security.c +++ b/security/security.c @@ -1015,9 +1015,11 @@ 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 required_cap, + const char *prefix) { - return call_int_hook(kernel_module_request, 0, kmod_name); + return call_int_hook(kernel_module_request, 0, kmod_name, + required_cap, prefix); } 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 d07299d..69f25da 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3889,7 +3889,8 @@ 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 required_cap, + const char *prefix) { struct common_audit_data ad;