From 65793f3cd93abf4ca1109f78e07c1b7193abdfec Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Wed, 13 Jun 2018 16:26:46 -0400 Subject: [PATCH 01/11] MAINTAINERS: remove the outdated "LINUX SECURITY MODULE (LSM) FRAMEWORK" entry This has been replaced with the "SECURITY SUBSYSTEM" entry. Signed-off-by: Paul Moore Signed-off-by: James Morris --- MAINTAINERS | 5 ----- 1 file changed, 5 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 79bb02ff812f..6d2340f572e2 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -8229,11 +8229,6 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git F: tools/memory-model/ F: Documentation/memory-barriers.txt -LINUX SECURITY MODULE (LSM) FRAMEWORK -M: Chris Wright -L: linux-security-module@vger.kernel.org -S: Supported - LIS3LV02D ACCELEROMETER DRIVER M: Eric Piel S: Maintained From 377179cd28cd417dcfb4396edb824533431e607e Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Fri, 13 Jul 2018 14:05:56 -0400 Subject: [PATCH 02/11] security: define new LSM hook named security_kernel_load_data Differentiate between the kernel reading a file specified by userspace from the kernel loading a buffer containing data provided by userspace. This patch defines a new LSM hook named security_kernel_load_data(). Signed-off-by: Mimi Zohar Cc: Eric Biederman Cc: Luis R. Rodriguez Cc: Kees Cook Cc: Casey Schaufler Acked-by: Serge Hallyn Acked-by: Kees Cook Signed-off-by: James Morris --- include/linux/lsm_hooks.h | 6 ++++++ include/linux/security.h | 27 +++++++++++++++++++++++++++ security/security.c | 5 +++++ 3 files changed, 38 insertions(+) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 8f1131c8dd54..a08bc2587b96 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -576,6 +576,10 @@ * userspace to load a kernel module with the given name. * @kmod_name name of the module requested by the kernel * Return 0 if successful. + * @kernel_load_data: + * Load data provided by userspace. + * @id kernel load data identifier + * Return 0 if permission is granted. * @kernel_read_file: * Read a file specified by userspace. * @file contains the file structure pointing to the file being read @@ -1582,6 +1586,7 @@ union security_list_options { 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_load_data)(enum kernel_load_data_id id); 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); @@ -1872,6 +1877,7 @@ struct security_hook_heads { struct hlist_head cred_getsecid; struct hlist_head kernel_act_as; struct hlist_head kernel_create_files_as; + struct hlist_head kernel_load_data; struct hlist_head kernel_read_file; struct hlist_head kernel_post_read_file; struct hlist_head kernel_module_request; diff --git a/include/linux/security.h b/include/linux/security.h index 63030c85ee19..3410acfe139c 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -159,6 +159,27 @@ extern int mmap_min_addr_handler(struct ctl_table *table, int write, typedef int (*initxattrs) (struct inode *inode, const struct xattr *xattr_array, void *fs_data); + +/* Keep the kernel_load_data_id enum in sync with kernel_read_file_id */ +#define __data_id_enumify(ENUM, dummy) LOADING_ ## ENUM, +#define __data_id_stringify(dummy, str) #str, + +enum kernel_load_data_id { + __kernel_read_file_id(__data_id_enumify) +}; + +static const char * const kernel_load_data_str[] = { + __kernel_read_file_id(__data_id_stringify) +}; + +static inline const char *kernel_load_data_id_str(enum kernel_load_data_id id) +{ + if ((unsigned)id >= LOADING_MAX_ID) + return kernel_load_data_str[LOADING_UNKNOWN]; + + return kernel_load_data_str[id]; +} + #ifdef CONFIG_SECURITY struct security_mnt_opts { @@ -320,6 +341,7 @@ void security_cred_getsecid(const struct cred *c, u32 *secid); 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_load_data(enum kernel_load_data_id id); 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); @@ -909,6 +931,11 @@ static inline int security_kernel_module_request(char *kmod_name) return 0; } +static inline int security_kernel_load_data(enum kernel_load_data_id id) +{ + return 0; +} + static inline int security_kernel_read_file(struct file *file, enum kernel_read_file_id id) { diff --git a/security/security.c b/security/security.c index 68f46d849abe..c2de2f134854 100644 --- a/security/security.c +++ b/security/security.c @@ -1056,6 +1056,11 @@ int security_kernel_post_read_file(struct file *file, char *buf, loff_t size, } EXPORT_SYMBOL_GPL(security_kernel_post_read_file); +int security_kernel_load_data(enum kernel_load_data_id id) +{ + return call_int_hook(kernel_load_data, 0, id); +} + int security_task_fix_setuid(struct cred *new, const struct cred *old, int flags) { From a210fd32a46bae6d05b43860fe3b47732501d63b Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Fri, 13 Jul 2018 14:05:57 -0400 Subject: [PATCH 03/11] kexec: add call to LSM hook in original kexec_load syscall In order for LSMs and IMA-appraisal to differentiate between kexec_load and kexec_file_load syscalls, both the original and new syscalls must call an LSM hook. This patch adds a call to security_kernel_load_data() in the original kexec_load syscall. Signed-off-by: Mimi Zohar Cc: Eric Biederman Cc: Kees Cook Acked-by: Serge Hallyn Acked-by: Kees Cook Signed-off-by: James Morris --- kernel/kexec.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/kernel/kexec.c b/kernel/kexec.c index aed8fb2564b3..68559808fdfa 100644 --- a/kernel/kexec.c +++ b/kernel/kexec.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -195,10 +196,17 @@ out: static inline int kexec_load_check(unsigned long nr_segments, unsigned long flags) { + int result; + /* We only trust the superuser with rebooting the system. */ if (!capable(CAP_SYS_BOOT) || kexec_load_disabled) return -EPERM; + /* Permit LSMs and IMA to fail the kexec */ + result = security_kernel_load_data(LOADING_KEXEC_IMAGE); + if (result < 0) + return result; + /* * Verify we have a legal set of flags * This leaves us room for future extensions. From 16c267aac86b463b1fcccd43c89f4c8e5c5c86fa Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Fri, 13 Jul 2018 14:05:58 -0400 Subject: [PATCH 04/11] ima: based on policy require signed kexec kernel images The original kexec_load syscall can not verify file signatures, nor can the kexec image be measured. Based on policy, deny the kexec_load syscall. Signed-off-by: Mimi Zohar Cc: Eric Biederman Cc: Kees Cook Reviewed-by: Kees Cook Signed-off-by: James Morris --- include/linux/ima.h | 7 +++++++ security/integrity/ima/ima.h | 1 + security/integrity/ima/ima_main.c | 27 +++++++++++++++++++++++++++ security/integrity/ima/ima_policy.c | 2 ++ security/security.c | 7 ++++++- 5 files changed, 43 insertions(+), 1 deletion(-) diff --git a/include/linux/ima.h b/include/linux/ima.h index 0e4647e0eb60..84806b54b50a 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -11,6 +11,7 @@ #define _LINUX_IMA_H #include +#include #include struct linux_binprm; @@ -19,6 +20,7 @@ extern int ima_bprm_check(struct linux_binprm *bprm); extern int ima_file_check(struct file *file, int mask, int opened); extern void ima_file_free(struct file *file); extern int ima_file_mmap(struct file *file, unsigned long prot); +extern int ima_load_data(enum kernel_load_data_id id); extern int ima_read_file(struct file *file, enum kernel_read_file_id id); extern int ima_post_read_file(struct file *file, void *buf, loff_t size, enum kernel_read_file_id id); @@ -49,6 +51,11 @@ static inline int ima_file_mmap(struct file *file, unsigned long prot) return 0; } +static inline int ima_load_data(enum kernel_load_data_id id) +{ + return 0; +} + static inline int ima_read_file(struct file *file, enum kernel_read_file_id id) { return 0; diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 354bb5716ce3..78c15264b17b 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -232,6 +232,7 @@ int ima_policy_show(struct seq_file *m, void *v); #define IMA_APPRAISE_MODULES 0x08 #define IMA_APPRAISE_FIRMWARE 0x10 #define IMA_APPRAISE_POLICY 0x20 +#define IMA_APPRAISE_KEXEC 0x40 #ifdef CONFIG_IMA_APPRAISE int ima_appraise_measurement(enum ima_hooks func, diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index dca44cf7838e..71fecfef0939 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -496,6 +496,33 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size, MAY_READ, func, 0); } +/** + * ima_load_data - appraise decision based on policy + * @id: kernel load data caller identifier + * + * Callers of this LSM hook can not measure, appraise, or audit the + * data provided by userspace. Enforce policy rules requring a file + * signature (eg. kexec'ed kernel image). + * + * For permission return 0, otherwise return -EACCES. + */ +int ima_load_data(enum kernel_load_data_id id) +{ + if ((ima_appraise & IMA_APPRAISE_ENFORCE) != IMA_APPRAISE_ENFORCE) + return 0; + + switch (id) { + case LOADING_KEXEC_IMAGE: + if (ima_appraise & IMA_APPRAISE_KEXEC) { + pr_err("impossible to appraise a kernel image without a file descriptor; try using kexec_file_load syscall.\n"); + return -EACCES; /* INTEGRITY_UNKNOWN */ + } + default: + break; + } + return 0; +} + static int __init init_ima(void) { int error; diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index cdcc9a7b4e24..d5b4958decc5 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -448,6 +448,8 @@ static int ima_appraise_flag(enum ima_hooks func) return IMA_APPRAISE_FIRMWARE; else if (func == POLICY_CHECK) return IMA_APPRAISE_POLICY; + else if (func == KEXEC_KERNEL_CHECK) + return IMA_APPRAISE_KEXEC; return 0; } diff --git a/security/security.c b/security/security.c index c2de2f134854..4927e7cc7d96 100644 --- a/security/security.c +++ b/security/security.c @@ -1058,7 +1058,12 @@ EXPORT_SYMBOL_GPL(security_kernel_post_read_file); int security_kernel_load_data(enum kernel_load_data_id id) { - return call_int_hook(kernel_load_data, 0, id); + int ret; + + ret = call_int_hook(kernel_load_data, 0, id); + if (ret) + return ret; + return ima_load_data(id); } int security_task_fix_setuid(struct cred *new, const struct cred *old, From 6e852651f28eee851069c7d40989281fae4bf9d2 Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Fri, 13 Jul 2018 14:05:59 -0400 Subject: [PATCH 05/11] firmware: add call to LSM hook before firmware sysfs fallback Add an LSM hook prior to allowing firmware sysfs fallback loading. Signed-off-by: Mimi Zohar Acked-by: Luis R. Rodriguez Reviewed-by: Kees Cook Signed-off-by: James Morris --- drivers/base/firmware_loader/fallback.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c index 7f732744f0d3..202324291542 100644 --- a/drivers/base/firmware_loader/fallback.c +++ b/drivers/base/firmware_loader/fallback.c @@ -651,6 +651,8 @@ static bool fw_force_sysfs_fallback(enum fw_opt opt_flags) static bool fw_run_sysfs_fallback(enum fw_opt opt_flags) { + int ret; + if (fw_fallback_config.ignore_sysfs_fallback) { pr_info_once("Ignoring firmware sysfs fallback due to sysctl knob\n"); return false; @@ -659,6 +661,11 @@ static bool fw_run_sysfs_fallback(enum fw_opt opt_flags) if ((opt_flags & FW_OPT_NOFALLBACK)) return false; + /* Also permit LSMs and IMA to fail firmware sysfs fallback */ + ret = security_kernel_load_data(LOADING_FIRMWARE); + if (ret < 0) + return ret; + return fw_force_sysfs_fallback(opt_flags); } From fed2512a7ccc8fc4b8e1de22925d127e4caac300 Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Fri, 13 Jul 2018 14:06:00 -0400 Subject: [PATCH 06/11] ima: based on policy require signed firmware (sysfs fallback) With an IMA policy requiring signed firmware, this patch prevents the sysfs fallback method of loading firmware. Signed-off-by: Mimi Zohar Reviewed-by: Kees Cook Cc: Luis R. Rodriguez Cc: Matthew Garrett Signed-off-by: James Morris --- security/integrity/ima/ima_main.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 71fecfef0939..e467664965e7 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -472,8 +472,10 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size, if (!file && read_id == READING_FIRMWARE) { if ((ima_appraise & IMA_APPRAISE_FIRMWARE) && - (ima_appraise & IMA_APPRAISE_ENFORCE)) + (ima_appraise & IMA_APPRAISE_ENFORCE)) { + pr_err("Prevent firmware loading_store.\n"); return -EACCES; /* INTEGRITY_UNKNOWN */ + } return 0; } @@ -517,6 +519,12 @@ int ima_load_data(enum kernel_load_data_id id) pr_err("impossible to appraise a kernel image without a file descriptor; try using kexec_file_load syscall.\n"); return -EACCES; /* INTEGRITY_UNKNOWN */ } + break; + case LOADING_FIRMWARE: + if (ima_appraise & IMA_APPRAISE_FIRMWARE) { + pr_err("Prevent firmware sysfs fallback loading.\n"); + return -EACCES; /* INTEGRITY_UNKNOWN */ + } default: break; } From ef96837b0de4af47732e2a8ebf5c18e8a885ded6 Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Fri, 13 Jul 2018 14:06:01 -0400 Subject: [PATCH 07/11] ima: add build time policy IMA by default does not measure, appraise or audit files, but can be enabled at runtime by specifying a builtin policy on the boot command line or by loading a custom policy. This patch defines a build time policy, which verifies kernel modules, firmware, kexec image, and/or the IMA policy signatures. This build time policy is automatically enabled at runtime and persists after loading a custom policy. Signed-off-by: Mimi Zohar Reviewed-by: Kees Cook Signed-off-by: James Morris --- security/integrity/ima/Kconfig | 58 +++++++++++++++++++++++++++++ security/integrity/ima/ima_policy.c | 46 +++++++++++++++++++++-- 2 files changed, 101 insertions(+), 3 deletions(-) diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig index 6a8f67714c83..004919d9bf09 100644 --- a/security/integrity/ima/Kconfig +++ b/security/integrity/ima/Kconfig @@ -156,6 +156,64 @@ config IMA_APPRAISE If unsure, say N. +config IMA_APPRAISE_BUILD_POLICY + bool "IMA build time configured policy rules" + depends on IMA_APPRAISE && INTEGRITY_ASYMMETRIC_KEYS + default n + help + This option defines an IMA appraisal policy at build time, which + is enforced at run time without having to specify a builtin + policy name on the boot command line. The build time appraisal + policy rules persist after loading a custom policy. + + Depending on the rules configured, this policy may require kernel + modules, firmware, the kexec kernel image, and/or the IMA policy + to be signed. Unsigned files might prevent the system from + booting or applications from working properly. + +config IMA_APPRAISE_REQUIRE_FIRMWARE_SIGS + bool "Appraise firmware signatures" + depends on IMA_APPRAISE_BUILD_POLICY + default n + help + This option defines a policy requiring all firmware to be signed, + including the regulatory.db. If both this option and + CFG80211_REQUIRE_SIGNED_REGDB are enabled, then both signature + verification methods are necessary. + +config IMA_APPRAISE_REQUIRE_KEXEC_SIGS + bool "Appraise kexec kernel image signatures" + depends on IMA_APPRAISE_BUILD_POLICY + default n + help + Enabling this rule will require all kexec'ed kernel images to + be signed and verified by a public key on the trusted IMA + keyring. + + Kernel image signatures can not be verified by the original + kexec_load syscall. Enabling this rule will prevent its + usage. + +config IMA_APPRAISE_REQUIRE_MODULE_SIGS + bool "Appraise kernel modules signatures" + depends on IMA_APPRAISE_BUILD_POLICY + default n + help + Enabling this rule will require all kernel modules to be signed + and verified by a public key on the trusted IMA keyring. + + Kernel module signatures can only be verified by IMA-appraisal, + via the finit_module syscall. Enabling this rule will prevent + the usage of the init_module syscall. + +config IMA_APPRAISE_REQUIRE_POLICY_SIGS + bool "Appraise IMA policy signature" + depends on IMA_APPRAISE_BUILD_POLICY + default n + help + Enabling this rule will require the IMA policy to be signed and + and verified by a key on the trusted IMA keyring. + config IMA_APPRAISE_BOOTPARAM bool "ima_appraise boot parameter" depends on IMA_APPRAISE diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index d5b4958decc5..1659abb344f9 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -49,6 +49,7 @@ int ima_policy_flag; static int temp_ima_appraise; +static int build_ima_appraise __ro_after_init; #define MAX_LSM_RULES 6 enum lsm_rule_types { LSM_OBJ_USER, LSM_OBJ_ROLE, LSM_OBJ_TYPE, @@ -162,6 +163,25 @@ static struct ima_rule_entry default_appraise_rules[] __ro_after_init = { #endif }; +static struct ima_rule_entry build_appraise_rules[] __ro_after_init = { +#ifdef CONFIG_IMA_APPRAISE_REQUIRE_MODULE_SIGS + {.action = APPRAISE, .func = MODULE_CHECK, + .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED}, +#endif +#ifdef CONFIG_IMA_APPRAISE_REQUIRE_FIRMWARE_SIGS + {.action = APPRAISE, .func = FIRMWARE_CHECK, + .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED}, +#endif +#ifdef CONFIG_IMA_APPRAISE_REQUIRE_KEXEC_SIGS + {.action = APPRAISE, .func = KEXEC_KERNEL_CHECK, + .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED}, +#endif +#ifdef CONFIG_IMA_APPRAISE_REQUIRE_POLICY_SIGS + {.action = APPRAISE, .func = POLICY_CHECK, + .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED}, +#endif +}; + static struct ima_rule_entry secure_boot_rules[] __ro_after_init = { {.action = APPRAISE, .func = MODULE_CHECK, .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED}, @@ -435,7 +455,7 @@ void ima_update_policy_flag(void) ima_policy_flag |= entry->action; } - ima_appraise |= temp_ima_appraise; + ima_appraise |= (build_ima_appraise | temp_ima_appraise); if (!ima_appraise) ima_policy_flag &= ~IMA_APPRAISE; } @@ -488,8 +508,8 @@ void __init ima_init_policy(void) } /* - * Insert the appraise rules requiring file signatures, prior to - * any other appraise rules. + * Insert the builtin "secure_boot" policy rules requiring file + * signatures, prior to any other appraise rules. */ for (i = 0; i < secure_boot_entries; i++) { list_add_tail(&secure_boot_rules[i].list, &ima_default_rules); @@ -497,6 +517,26 @@ void __init ima_init_policy(void) ima_appraise_flag(secure_boot_rules[i].func); } + /* + * Insert the build time appraise rules requiring file signatures + * for both the initial and custom policies, prior to other appraise + * rules. + */ + for (i = 0; i < ARRAY_SIZE(build_appraise_rules); i++) { + struct ima_rule_entry *entry; + + if (!secure_boot_entries) + list_add_tail(&build_appraise_rules[i].list, + &ima_default_rules); + + entry = kmemdup(&build_appraise_rules[i], sizeof(*entry), + GFP_KERNEL); + if (entry) + list_add_tail(&entry->list, &ima_policy_rules); + build_ima_appraise |= + ima_appraise_flag(build_appraise_rules[i].func); + } + for (i = 0; i < appraise_entries; i++) { list_add_tail(&default_appraise_rules[i].list, &ima_default_rules); From c77b8cdf745d91eca138e7bfa430dc6640b604a0 Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Fri, 13 Jul 2018 14:06:02 -0400 Subject: [PATCH 08/11] module: replace the existing LSM hook in init_module Both the init_module and finit_module syscalls call either directly or indirectly the security_kernel_read_file LSM hook. This patch replaces the direct call in init_module with a call to the new security_kernel_load_data hook and makes the corresponding changes in SELinux, LoadPin, and IMA. Signed-off-by: Mimi Zohar Cc: Jeff Vander Stoep Cc: Casey Schaufler Cc: Kees Cook Acked-by: Jessica Yu Acked-by: Paul Moore Acked-by: Kees Cook Signed-off-by: James Morris --- kernel/module.c | 2 +- security/integrity/ima/ima_main.c | 23 ++++++++++------------- security/loadpin/loadpin.c | 6 ++++++ security/selinux/hooks.c | 15 +++++++++++++++ 4 files changed, 32 insertions(+), 14 deletions(-) diff --git a/kernel/module.c b/kernel/module.c index f475f30eed8c..a7615d661910 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -2876,7 +2876,7 @@ static int copy_module_from_user(const void __user *umod, unsigned long len, if (info->len < sizeof(*(info->hdr))) return -ENOEXEC; - err = security_kernel_read_file(NULL, READING_MODULE); + err = security_kernel_load_data(LOADING_MODULE); if (err) return err; diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index e467664965e7..ef349a761609 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -429,16 +429,6 @@ void ima_post_path_mknod(struct dentry *dentry) */ int ima_read_file(struct file *file, enum kernel_read_file_id read_id) { - bool sig_enforce = is_module_sig_enforced(); - - if (!file && read_id == READING_MODULE) { - if (!sig_enforce && (ima_appraise & IMA_APPRAISE_MODULES) && - (ima_appraise & IMA_APPRAISE_ENFORCE)) { - pr_err("impossible to appraise a module without a file descriptor. sig_enforce kernel parameter might help\n"); - return -EACCES; /* INTEGRITY_UNKNOWN */ - } - return 0; /* We rely on module signature checking */ - } return 0; } @@ -479,9 +469,6 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size, return 0; } - if (!file && read_id == READING_MODULE) /* MODULE_SIG_FORCE enabled */ - return 0; - /* permit signed certs */ if (!file && read_id == READING_X509_CERTIFICATE) return 0; @@ -510,6 +497,8 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size, */ int ima_load_data(enum kernel_load_data_id id) { + bool sig_enforce; + if ((ima_appraise & IMA_APPRAISE_ENFORCE) != IMA_APPRAISE_ENFORCE) return 0; @@ -525,6 +514,14 @@ int ima_load_data(enum kernel_load_data_id id) pr_err("Prevent firmware sysfs fallback loading.\n"); return -EACCES; /* INTEGRITY_UNKNOWN */ } + break; + case LOADING_MODULE: + sig_enforce = is_module_sig_enforced(); + + if (!sig_enforce && (ima_appraise & IMA_APPRAISE_MODULES)) { + pr_err("impossible to appraise a module without a file descriptor. sig_enforce kernel parameter might help\n"); + return -EACCES; /* INTEGRITY_UNKNOWN */ + } default: break; } diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c index 5fa191252c8f..0716af28808a 100644 --- a/security/loadpin/loadpin.c +++ b/security/loadpin/loadpin.c @@ -173,9 +173,15 @@ static int loadpin_read_file(struct file *file, enum kernel_read_file_id id) return 0; } +static int loadpin_load_data(enum kernel_load_data_id id) +{ + return loadpin_read_file(NULL, (enum kernel_read_file_id) id); +} + static struct security_hook_list loadpin_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(sb_free_security, loadpin_sb_free_security), LSM_HOOK_INIT(kernel_read_file, loadpin_read_file), + LSM_HOOK_INIT(kernel_load_data, loadpin_load_data), }; void __init loadpin_add_hooks(void) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 2b5ee5fbd652..a8bf324130f5 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -4073,6 +4073,20 @@ static int selinux_kernel_read_file(struct file *file, return rc; } +static int selinux_kernel_load_data(enum kernel_load_data_id id) +{ + int rc = 0; + + switch (id) { + case LOADING_MODULE: + rc = selinux_kernel_module_from_file(NULL); + default: + break; + } + + return rc; +} + static int selinux_task_setpgid(struct task_struct *p, pid_t pgid) { return avc_has_perm(&selinux_state, @@ -6972,6 +6986,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(kernel_act_as, selinux_kernel_act_as), LSM_HOOK_INIT(kernel_create_files_as, selinux_kernel_create_files_as), LSM_HOOK_INIT(kernel_module_request, selinux_kernel_module_request), + LSM_HOOK_INIT(kernel_load_data, selinux_kernel_load_data), LSM_HOOK_INIT(kernel_read_file, selinux_kernel_read_file), LSM_HOOK_INIT(task_setpgid, selinux_task_setpgid), LSM_HOOK_INIT(task_getpgid, selinux_task_getpgid), From 4f0496d8ffa3ed5956a2ddee5d84f0246977799d Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Fri, 13 Jul 2018 14:06:03 -0400 Subject: [PATCH 09/11] ima: based on policy warn about loading firmware (pre-allocated buffer) Some systems are memory constrained but they need to load very large firmwares. The firmware subsystem allows drivers to request this firmware be loaded from the filesystem, but this requires that the entire firmware be loaded into kernel memory first before it's provided to the driver. This can lead to a situation where we map the firmware twice, once to load the firmware into kernel memory and once to copy the firmware into the final resting place. To resolve this problem, commit a098ecd2fa7d ("firmware: support loading into a pre-allocated buffer") introduced request_firmware_into_buf() API that allows drivers to request firmware be loaded directly into a pre-allocated buffer. Do devices using pre-allocated memory run the risk of the firmware being accessible to the device prior to the completion of IMA's signature verification any more than when using two buffers? (Refer to mailing list discussion[1]). Only on systems with an IOMMU can the access be prevented. As long as the signature verification completes prior to the DMA map is performed, the device can not access the buffer. This implies that the same buffer can not be re-used. Can we ensure the buffer has not been DMA mapped before using the pre-allocated buffer? [1] https://lkml.org/lkml/2018/7/10/56 Signed-off-by: Mimi Zohar Cc: Luis R. Rodriguez Cc: Stephen Boyd Cc: Bjorn Andersson Cc: Ard Biesheuvel Reviewed-by: Kees Cook Signed-off-by: James Morris --- security/integrity/ima/ima_main.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index ef349a761609..dce0a8a217bb 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -429,6 +429,14 @@ void ima_post_path_mknod(struct dentry *dentry) */ int ima_read_file(struct file *file, enum kernel_read_file_id read_id) { + /* + * READING_FIRMWARE_PREALLOC_BUFFER + * + * Do devices using pre-allocated memory run the risk of the + * firmware being accessible to the device prior to the completion + * of IMA's signature verification any more than when using two + * buffers? + */ return 0; } From 83a68a06795fa47e77ea758f293a5946e9e02e84 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Tue, 17 Jul 2018 22:23:37 +0200 Subject: [PATCH 10/11] security: export security_kernel_load_data function The firmware_loader can be built as a loadable module, which now fails when CONFIG_SECURITY is enabled, because a call to the security_kernel_load_data() function got added, and this is not exported to modules: ERROR: "security_kernel_load_data" [drivers/base/firmware_loader/firmware_class.ko] undefined! Add an EXPORT_SYMBOL_GPL() to make it available here. Fixes: 6e852651f28e ("firmware: add call to LSM hook before firmware sysfs fallback") Signed-off-by: Arnd Bergmann Signed-off-by: James Morris --- security/security.c | 1 + 1 file changed, 1 insertion(+) diff --git a/security/security.c b/security/security.c index 4927e7cc7d96..6e149d0ffe33 100644 --- a/security/security.c +++ b/security/security.c @@ -1065,6 +1065,7 @@ int security_kernel_load_data(enum kernel_load_data_id id) return ret; return ima_load_data(id); } +EXPORT_SYMBOL_GPL(security_kernel_load_data); int security_task_fix_setuid(struct cred *new, const struct cred *old, int flags) From 87ea58433208d17295e200d56be5e2a4fe4ce7d6 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 17 Jul 2018 10:36:04 -0700 Subject: [PATCH 11/11] security: check for kstrdup() failure in lsm_append() lsm_append() should return -ENOMEM if memory allocation failed. Fixes: d69dece5f5b6 ("LSM: Add /sys/kernel/security/lsm") Signed-off-by: Eric Biggers Signed-off-by: James Morris --- security/security.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/security/security.c b/security/security.c index 6e149d0ffe33..b49ee810371b 100644 --- a/security/security.c +++ b/security/security.c @@ -118,6 +118,8 @@ static int lsm_append(char *new, char **result) if (*result == NULL) { *result = kstrdup(new, GFP_KERNEL); + if (*result == NULL) + return -ENOMEM; } else { /* Check if it is the last registered name */ if (match_last_lsm(*result, new))