From 377179cd28cd417dcfb4396edb824533431e607e Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Fri, 13 Jul 2018 14:05:56 -0400 Subject: 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 --- security/security.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'security') 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) { -- cgit v1.2.3 From 16c267aac86b463b1fcccd43c89f4c8e5c5c86fa Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Fri, 13 Jul 2018 14:05:58 -0400 Subject: 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 --- security/integrity/ima/ima.h | 1 + security/integrity/ima/ima_main.c | 27 +++++++++++++++++++++++++++ security/integrity/ima/ima_policy.c | 2 ++ security/security.c | 7 ++++++- 4 files changed, 36 insertions(+), 1 deletion(-) (limited to 'security') 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, -- cgit v1.2.3 From fed2512a7ccc8fc4b8e1de22925d127e4caac300 Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Fri, 13 Jul 2018 14:06:00 -0400 Subject: 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(-) (limited to 'security') 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; } -- cgit v1.2.3 From ef96837b0de4af47732e2a8ebf5c18e8a885ded6 Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Fri, 13 Jul 2018 14:06:01 -0400 Subject: 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(-) (limited to 'security') 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); -- cgit v1.2.3 From c77b8cdf745d91eca138e7bfa430dc6640b604a0 Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Fri, 13 Jul 2018 14:06:02 -0400 Subject: 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 --- security/integrity/ima/ima_main.c | 23 ++++++++++------------- security/loadpin/loadpin.c | 6 ++++++ security/selinux/hooks.c | 15 +++++++++++++++ 3 files changed, 31 insertions(+), 13 deletions(-) (limited to 'security') 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), -- cgit v1.2.3 From 4f0496d8ffa3ed5956a2ddee5d84f0246977799d Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Fri, 13 Jul 2018 14:06:03 -0400 Subject: 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(+) (limited to 'security') 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; } -- cgit v1.2.3 From 83a68a06795fa47e77ea758f293a5946e9e02e84 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Tue, 17 Jul 2018 22:23:37 +0200 Subject: 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(+) (limited to 'security') 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) -- cgit v1.2.3 From 87ea58433208d17295e200d56be5e2a4fe4ce7d6 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 17 Jul 2018 10:36:04 -0700 Subject: 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(+) (limited to 'security') 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)) -- cgit v1.2.3