From 980ef4d22a95a3cd84a9b8ffaa7b81b391d173c6 Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Wed, 24 Apr 2019 13:05:46 -0400 Subject: x86/ima: check EFI SetupMode too Checking "SecureBoot" mode is not sufficient, also check "SetupMode". Fixes: 399574c64eaf ("x86/ima: retry detecting secure boot mode") Reported-by: Matthew Garrett Signed-off-by: Mimi Zohar --- arch/x86/kernel/ima_arch.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/ima_arch.c b/arch/x86/kernel/ima_arch.c index 64b973f0e985..4c407833faca 100644 --- a/arch/x86/kernel/ima_arch.c +++ b/arch/x86/kernel/ima_arch.c @@ -11,10 +11,11 @@ extern struct boot_params boot_params; static enum efi_secureboot_mode get_sb_mode(void) { efi_char16_t efi_SecureBoot_name[] = L"SecureBoot"; + efi_char16_t efi_SetupMode_name[] = L"SecureBoot"; efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID; efi_status_t status; unsigned long size; - u8 secboot; + u8 secboot, setupmode; size = sizeof(secboot); @@ -36,7 +37,14 @@ static enum efi_secureboot_mode get_sb_mode(void) return efi_secureboot_mode_unknown; } - if (secboot == 0) { + size = sizeof(setupmode); + status = efi.get_variable(efi_SetupMode_name, &efi_variable_guid, + NULL, &size, &setupmode); + + if (status != EFI_SUCCESS) /* ignore unknown SetupMode */ + setupmode = 0; + + if (secboot == 0 || setupmode == 1) { pr_info("ima: secureboot mode disabled\n"); return efi_secureboot_mode_disabled; } -- cgit v1.2.3 From 2cd4737bc850225de426a8dbee7a7a312a4f5304 Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Tue, 30 Apr 2019 08:34:44 -0400 Subject: ima: prevent a file already mmap'ed write to be mmap'ed execute The kernel calls deny_write_access() to prevent a file already opened for write from being executed and also prevents files being executed from being opened for write. For some reason this does not extend to files being mmap'ed execute. From an IMA perspective, measuring/appraising the integrity of a file being mmap'ed shared execute, without first making sure the file cannot be modified, makes no sense. This patch prevents files, in policy, already mmap'ed shared write, from being mmap'ed execute. Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_main.c | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 357edd140c09..f16353b5097e 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -72,6 +72,27 @@ out: } __setup("ima_hash=", hash_setup); +/* Prevent mmap'ing a file execute that is already mmap'ed write */ +static int mmap_violation_check(enum ima_hooks func, struct file *file, + char **pathbuf, const char **pathname, + char *filename) +{ + struct inode *inode; + int rc = 0; + + if ((func == MMAP_CHECK) && mapping_writably_mapped(file->f_mapping)) { + rc = -ETXTBSY; + inode = file_inode(file); + + if (!*pathbuf) /* ima_rdwr_violation possibly pre-fetched */ + *pathname = ima_d_path(&file->f_path, pathbuf, + filename); + integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, *pathname, + "mmap_file", "mmapped_writers", rc, 0); + } + return rc; +} + /* * ima_rdwr_violation_check * @@ -270,8 +291,12 @@ static int process_measurement(struct file *file, const struct cred *cred, /* Nothing to do, just return existing appraised status */ if (!action) { - if (must_appraise) - rc = ima_get_cache_status(iint, func); + if (must_appraise) { + rc = mmap_violation_check(func, file, &pathbuf, + &pathname, filename); + if (!rc) + rc = ima_get_cache_status(iint, func); + } goto out_locked; } @@ -298,6 +323,9 @@ static int process_measurement(struct file *file, const struct cred *cred, rc = ima_appraise_measurement(func, iint, file, pathname, xattr_value, xattr_len); inode_unlock(inode); + if (!rc) + rc = mmap_violation_check(func, file, &pathbuf, + &pathname, filename); } if (action & IMA_AUDIT) ima_audit_measurement(iint, pathname); -- cgit v1.2.3 From 68f2529078f97dd4dc7f01bc4d495cf5f5814363 Mon Sep 17 00:00:00 2001 From: YueHaibing Date: Tue, 11 Jun 2019 21:40:32 +0800 Subject: ima: Make arch_policy_entry static Fix sparse warning: security/integrity/ima/ima_policy.c:202:23: warning: symbol 'arch_policy_entry' was not declared. Should it be static? Fixes: 6191706246de ("ima: add support for arch specific policies") Reported-by: Hulk Robot Signed-off-by: YueHaibing Cc: stable@vger.kernel.org (linux-5.0) Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_policy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 1cc822a59054..cd1b7281244e 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -199,7 +199,7 @@ static struct ima_rule_entry secure_boot_rules[] __ro_after_init = { }; /* An array of architecture specific rules */ -struct ima_rule_entry *arch_policy_entry __ro_after_init; +static struct ima_rule_entry *arch_policy_entry __ro_after_init; static LIST_HEAD(ima_default_rules); static LIST_HEAD(ima_policy_rules); -- cgit v1.2.3 From 9e1e5d4372d641d3cd6d6cb8dc0b488a7850d222 Mon Sep 17 00:00:00 2001 From: Nayna Jain Date: Fri, 7 Jun 2019 15:12:06 -0400 Subject: x86/ima: fix the Kconfig dependency for IMA_ARCH_POLICY If enabled, ima arch specific policies always adds the measurements rules, this makes it dependent on CONFIG_IMA. CONFIG_IMA_APPRAISE implicitly takes care of this, however it is needed explicitly for CONFIG_KEXEC_VERIFY_SIG. This patch adds the CONFIG_IMA dependency in combination with CONFIG_KEXEC_VERIFY_SIG for CONFIG_IMA_ARCH_POLICY Fixes: d958083a8f640 (x86/ima: define arch_get_ima_policy() for x86) Signed-off-by: Nayna Jain Cc: Eric Biederman Cc: Dave Young Signed-off-by: Mimi Zohar --- security/integrity/ima/Kconfig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig index a18f8c6d13b5..df65d2d41905 100644 --- a/security/integrity/ima/Kconfig +++ b/security/integrity/ima/Kconfig @@ -159,7 +159,8 @@ config IMA_APPRAISE config IMA_ARCH_POLICY bool "Enable loading an IMA architecture specific policy" - depends on KEXEC_VERIFY_SIG || IMA_APPRAISE && INTEGRITY_ASYMMETRIC_KEYS + depends on (KEXEC_VERIFY_SIG && IMA) || IMA_APPRAISE \ + && INTEGRITY_ASYMMETRIC_KEYS default n help This option enables loading an IMA architecture specific policy -- cgit v1.2.3 From 42df744c4166af6959eda2df1ee5cde744d4a1c3 Mon Sep 17 00:00:00 2001 From: Janne Karhunen Date: Fri, 14 Jun 2019 15:20:14 +0300 Subject: LSM: switch to blocking policy update notifiers Atomic policy updaters are not very useful as they cannot usually perform the policy updates on their own. Since it seems that there is no strict need for the atomicity, switch to the blocking variant. While doing so, rename the functions accordingly. Signed-off-by: Janne Karhunen Acked-by: Paul Moore Acked-by: James Morris Signed-off-by: Mimi Zohar --- drivers/infiniband/core/device.c | 6 +++--- include/linux/security.h | 12 ++++++------ security/security.c | 23 +++++++++++++---------- security/selinux/hooks.c | 2 +- security/selinux/selinuxfs.c | 2 +- 5 files changed, 24 insertions(+), 21 deletions(-) diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c index 78dc07c6ac4b..61c0c93a2e73 100644 --- a/drivers/infiniband/core/device.c +++ b/drivers/infiniband/core/device.c @@ -2499,7 +2499,7 @@ static int __init ib_core_init(void) goto err_mad; } - ret = register_lsm_notifier(&ibdev_lsm_nb); + ret = register_blocking_lsm_notifier(&ibdev_lsm_nb); if (ret) { pr_warn("Couldn't register LSM notifier. ret %d\n", ret); goto err_sa; @@ -2518,7 +2518,7 @@ static int __init ib_core_init(void) return 0; err_compat: - unregister_lsm_notifier(&ibdev_lsm_nb); + unregister_blocking_lsm_notifier(&ibdev_lsm_nb); err_sa: ib_sa_cleanup(); err_mad: @@ -2544,7 +2544,7 @@ static void __exit ib_core_cleanup(void) nldev_exit(); rdma_nl_unregister(RDMA_NL_LS); unregister_pernet_device(&rdma_dev_net_ops); - unregister_lsm_notifier(&ibdev_lsm_nb); + unregister_blocking_lsm_notifier(&ibdev_lsm_nb); ib_sa_cleanup(); ib_mad_cleanup(); addr_cleanup(); diff --git a/include/linux/security.h b/include/linux/security.h index 659071c2e57c..5f7441abbf42 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -189,9 +189,9 @@ static inline const char *kernel_load_data_id_str(enum kernel_load_data_id id) #ifdef CONFIG_SECURITY -int call_lsm_notifier(enum lsm_event event, void *data); -int register_lsm_notifier(struct notifier_block *nb); -int unregister_lsm_notifier(struct notifier_block *nb); +int call_blocking_lsm_notifier(enum lsm_event event, void *data); +int register_blocking_lsm_notifier(struct notifier_block *nb); +int unregister_blocking_lsm_notifier(struct notifier_block *nb); /* prototypes */ extern int security_init(void); @@ -394,17 +394,17 @@ int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen); int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen); #else /* CONFIG_SECURITY */ -static inline int call_lsm_notifier(enum lsm_event event, void *data) +static inline int call_blocking_lsm_notifier(enum lsm_event event, void *data) { return 0; } -static inline int register_lsm_notifier(struct notifier_block *nb) +static inline int register_blocking_lsm_notifier(struct notifier_block *nb) { return 0; } -static inline int unregister_lsm_notifier(struct notifier_block *nb) +static inline int unregister_blocking_lsm_notifier(struct notifier_block *nb) { return 0; } diff --git a/security/security.c b/security/security.c index 613a5c00e602..47e5849d7557 100644 --- a/security/security.c +++ b/security/security.c @@ -39,7 +39,7 @@ #define LSM_COUNT (__end_lsm_info - __start_lsm_info) struct security_hook_heads security_hook_heads __lsm_ro_after_init; -static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain); +static BLOCKING_NOTIFIER_HEAD(blocking_lsm_notifier_chain); static struct kmem_cache *lsm_file_cache; static struct kmem_cache *lsm_inode_cache; @@ -430,23 +430,26 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count, panic("%s - Cannot get early memory.\n", __func__); } -int call_lsm_notifier(enum lsm_event event, void *data) +int call_blocking_lsm_notifier(enum lsm_event event, void *data) { - return atomic_notifier_call_chain(&lsm_notifier_chain, event, data); + return blocking_notifier_call_chain(&blocking_lsm_notifier_chain, + event, data); } -EXPORT_SYMBOL(call_lsm_notifier); +EXPORT_SYMBOL(call_blocking_lsm_notifier); -int register_lsm_notifier(struct notifier_block *nb) +int register_blocking_lsm_notifier(struct notifier_block *nb) { - return atomic_notifier_chain_register(&lsm_notifier_chain, nb); + return blocking_notifier_chain_register(&blocking_lsm_notifier_chain, + nb); } -EXPORT_SYMBOL(register_lsm_notifier); +EXPORT_SYMBOL(register_blocking_lsm_notifier); -int unregister_lsm_notifier(struct notifier_block *nb) +int unregister_blocking_lsm_notifier(struct notifier_block *nb) { - return atomic_notifier_chain_unregister(&lsm_notifier_chain, nb); + return blocking_notifier_chain_unregister(&blocking_lsm_notifier_chain, + nb); } -EXPORT_SYMBOL(unregister_lsm_notifier); +EXPORT_SYMBOL(unregister_blocking_lsm_notifier); /** * lsm_cred_alloc - allocate a composite cred blob diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index c61787b15f27..c1e37018c8eb 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -197,7 +197,7 @@ static int selinux_lsm_notifier_avc_callback(u32 event) { if (event == AVC_CALLBACK_RESET) { sel_ib_pkey_flush(); - call_lsm_notifier(LSM_POLICY_CHANGE, NULL); + call_blocking_lsm_notifier(LSM_POLICY_CHANGE, NULL); } return 0; diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index 145ee62f205a..1e2e3e4b5fdb 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -180,7 +180,7 @@ static ssize_t sel_write_enforce(struct file *file, const char __user *buf, selnl_notify_setenforce(new_value); selinux_status_update_setenforce(state, new_value); if (!new_value) - call_lsm_notifier(LSM_POLICY_CHANGE, NULL); + call_blocking_lsm_notifier(LSM_POLICY_CHANGE, NULL); } length = count; out: -- cgit v1.2.3 From b169424551930a9325f700f502802f4d515194e5 Mon Sep 17 00:00:00 2001 From: Janne Karhunen Date: Fri, 14 Jun 2019 15:20:15 +0300 Subject: ima: use the lsm policy update notifier Don't do lazy policy updates while running the rule matching, run the updates as they happen. Depends on commit f242064c5df3 ("LSM: switch to blocking policy update notifiers") Signed-off-by: Janne Karhunen Signed-off-by: Mimi Zohar --- security/integrity/ima/ima.h | 2 + security/integrity/ima/ima_main.c | 8 +++ security/integrity/ima/ima_policy.c | 116 +++++++++++++++++++++++++++++------- 3 files changed, 106 insertions(+), 20 deletions(-) diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index d213e835c498..2203451862d4 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -154,6 +154,8 @@ unsigned long ima_get_binary_runtime_size(void); int ima_init_template(void); void ima_init_template_list(void); int __init ima_init_digests(void); +int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event, + void *lsm_data); /* * used to protect h_table and sha_table diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index f16353b5097e..9e3ea8a3f2db 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -43,6 +43,10 @@ int ima_appraise; int ima_hash_algo = HASH_ALGO_SHA1; static int hash_setup_done; +static struct notifier_block ima_lsm_policy_notifier = { + .notifier_call = ima_lsm_policy_change, +}; + static int __init hash_setup(char *str) { struct ima_template_desc *template_desc = ima_template_desc_current(); @@ -621,6 +625,10 @@ static int __init init_ima(void) error = ima_init(); } + error = register_blocking_lsm_notifier(&ima_lsm_policy_notifier); + if (error) + pr_warn("Couldn't register LSM notifier, error %d\n", error); + if (!error) ima_update_policy_flag(); diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index cd1b7281244e..ec4efa8ea366 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -249,31 +249,113 @@ static int __init default_appraise_policy_setup(char *str) } __setup("ima_appraise_tcb", default_appraise_policy_setup); +static void ima_lsm_free_rule(struct ima_rule_entry *entry) +{ + int i; + + for (i = 0; i < MAX_LSM_RULES; i++) { + kfree(entry->lsm[i].rule); + kfree(entry->lsm[i].args_p); + } + kfree(entry); +} + +static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry) +{ + struct ima_rule_entry *nentry; + int i, result; + + nentry = kmalloc(sizeof(*nentry), GFP_KERNEL); + if (!nentry) + return NULL; + + /* + * Immutable elements are copied over as pointers and data; only + * lsm rules can change + */ + memcpy(nentry, entry, sizeof(*nentry)); + memset(nentry->lsm, 0, FIELD_SIZEOF(struct ima_rule_entry, lsm)); + + for (i = 0; i < MAX_LSM_RULES; i++) { + if (!entry->lsm[i].rule) + continue; + + nentry->lsm[i].type = entry->lsm[i].type; + nentry->lsm[i].args_p = kstrdup(entry->lsm[i].args_p, + GFP_KERNEL); + if (!nentry->lsm[i].args_p) + goto out_err; + + result = security_filter_rule_init(nentry->lsm[i].type, + Audit_equal, + nentry->lsm[i].args_p, + &nentry->lsm[i].rule); + if (result == -EINVAL) + pr_warn("ima: rule for LSM \'%d\' is undefined\n", + entry->lsm[i].type); + } + return nentry; + +out_err: + ima_lsm_free_rule(nentry); + return NULL; +} + +static int ima_lsm_update_rule(struct ima_rule_entry *entry) +{ + struct ima_rule_entry *nentry; + + nentry = ima_lsm_copy_rule(entry); + if (!nentry) + return -ENOMEM; + + list_replace_rcu(&entry->list, &nentry->list); + synchronize_rcu(); + ima_lsm_free_rule(entry); + + return 0; +} + /* * The LSM policy can be reloaded, leaving the IMA LSM based rules referring * to the old, stale LSM policy. Update the IMA LSM based rules to reflect - * the reloaded LSM policy. We assume the rules still exist; and BUG_ON() if - * they don't. + * the reloaded LSM policy. */ static void ima_lsm_update_rules(void) { - struct ima_rule_entry *entry; - int result; - int i; + struct ima_rule_entry *entry, *e; + int i, result, needs_update; - list_for_each_entry(entry, &ima_policy_rules, list) { + list_for_each_entry_safe(entry, e, &ima_policy_rules, list) { + needs_update = 0; for (i = 0; i < MAX_LSM_RULES; i++) { - if (!entry->lsm[i].rule) - continue; - result = security_filter_rule_init(entry->lsm[i].type, - Audit_equal, - entry->lsm[i].args_p, - &entry->lsm[i].rule); - BUG_ON(!entry->lsm[i].rule); + if (entry->lsm[i].rule) { + needs_update = 1; + break; + } + } + if (!needs_update) + continue; + + result = ima_lsm_update_rule(entry); + if (result) { + pr_err("ima: lsm rule update error %d\n", + result); + return; } } } +int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event, + void *lsm_data) +{ + if (event != LSM_POLICY_CHANGE) + return NOTIFY_DONE; + + ima_lsm_update_rules(); + return NOTIFY_OK; +} + /** * ima_match_rules - determine whether an inode matches the measure rule. * @rule: a pointer to a rule @@ -327,11 +409,10 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, for (i = 0; i < MAX_LSM_RULES; i++) { int rc = 0; u32 osid; - int retried = 0; if (!rule->lsm[i].rule) continue; -retry: + switch (i) { case LSM_OBJ_USER: case LSM_OBJ_ROLE: @@ -352,11 +433,6 @@ retry: default: break; } - if ((rc < 0) && (!retried)) { - retried = 1; - ima_lsm_update_rules(); - goto retry; - } if (!rc) return false; } -- cgit v1.2.3 From e038f5f691e003af25cda0f1593d2cc0a4d1c866 Mon Sep 17 00:00:00 2001 From: Thiago Jung Bauermann Date: Tue, 11 Jun 2019 03:28:10 -0300 Subject: ima: Use designated initializers for struct ima_event_data Designated initializers allow specifying only the members of the struct that need initialization. Non-mentioned members are initialized to zero. This makes the code a bit clearer (particularly in ima_add_boot_aggregate) and also allows adding a new member to the struct without having to update all struct initializations. Signed-off-by: Thiago Jung Bauermann Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_api.c | 13 +++++++++---- security/integrity/ima/ima_init.c | 4 ++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index c7505fb122d4..4aec6b035fe9 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -133,8 +133,10 @@ void ima_add_violation(struct file *file, const unsigned char *filename, { struct ima_template_entry *entry; struct inode *inode = file_inode(file); - struct ima_event_data event_data = {iint, file, filename, NULL, 0, - cause}; + struct ima_event_data event_data = { .iint = iint, + .file = file, + .filename = filename, + .violation = cause }; int violation = 1; int result; @@ -284,8 +286,11 @@ void ima_store_measurement(struct integrity_iint_cache *iint, int result = -ENOMEM; struct inode *inode = file_inode(file); struct ima_template_entry *entry; - struct ima_event_data event_data = {iint, file, filename, xattr_value, - xattr_len, NULL}; + struct ima_event_data event_data = { .iint = iint, + .file = file, + .filename = filename, + .xattr_value = xattr_value, + .xattr_len = xattr_len }; int violation = 0; if (iint->measured_pcrs & (0x1 << pcr)) diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c index 6c9295449751..ef6c3a26296e 100644 --- a/security/integrity/ima/ima_init.c +++ b/security/integrity/ima/ima_init.c @@ -49,8 +49,8 @@ static int __init ima_add_boot_aggregate(void) const char *audit_cause = "ENOMEM"; struct ima_template_entry *entry; struct integrity_iint_cache tmp_iint, *iint = &tmp_iint; - struct ima_event_data event_data = {iint, NULL, boot_aggregate_name, - NULL, 0, NULL}; + struct ima_event_data event_data = { .iint = iint, + .filename = boot_aggregate_name }; int result = -ENOMEM; int violation = 0; struct { -- cgit v1.2.3 From 8c655784e2cf59cb6140759b8b546d98261d1ad9 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Mon, 17 Jun 2019 09:44:52 +0200 Subject: integrity: Fix __integrity_init_keyring() section mismatch With gcc-4.6.3: WARNING: vmlinux.o(.text.unlikely+0x24c64): Section mismatch in reference from the function __integrity_init_keyring() to the function .init.text:set_platform_trusted_keys() The function __integrity_init_keyring() references the function __init set_platform_trusted_keys(). This is often because __integrity_init_keyring lacks a __init annotation or the annotation of set_platform_trusted_keys is wrong. Indeed, if the compiler decides not to inline __integrity_init_keyring(), a warning is issued. Fix this by adding the missing __init annotation. Fixes: 9dc92c45177ab70e ("integrity: Define a trusted platform keyring") Signed-off-by: Geert Uytterhoeven Reviewed-by: Nayna Jain Reviewed-by: James Morris Signed-off-by: Mimi Zohar --- security/integrity/digsig.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c index e19c2eb72c51..37869214c243 100644 --- a/security/integrity/digsig.c +++ b/security/integrity/digsig.c @@ -73,8 +73,9 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen, return -EOPNOTSUPP; } -static int __integrity_init_keyring(const unsigned int id, key_perm_t perm, - struct key_restriction *restriction) +static int __init __integrity_init_keyring(const unsigned int id, + key_perm_t perm, + struct key_restriction *restriction) { const struct cred *cred = current_cred(); int err = 0; -- cgit v1.2.3 From 19453ce0bcfbdf7332a104eebf5d835977af7284 Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Wed, 19 Jun 2019 15:46:11 -0700 Subject: IMA: support for per policy rule template formats Admins may wish to log different measurements using different IMA templates. Add support for overriding the default template on a per-rule basis. Inspired-by: Roberto Sassu Signed-off-by: Matthew Garrett Signed-off-by: Mimi Zohar --- Documentation/ABI/testing/ima_policy | 5 +++-- security/integrity/ima/ima.h | 16 +++++++++++---- security/integrity/ima/ima_api.c | 24 +++++++++++++++------- security/integrity/ima/ima_appraise.c | 2 +- security/integrity/ima/ima_init.c | 2 +- security/integrity/ima/ima_main.c | 9 +++++---- security/integrity/ima/ima_policy.c | 38 +++++++++++++++++++++++++++++++++-- security/integrity/ima/ima_template.c | 12 ++++------- 8 files changed, 79 insertions(+), 29 deletions(-) diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy index 74c6702de74e..b383c1763610 100644 --- a/Documentation/ABI/testing/ima_policy +++ b/Documentation/ABI/testing/ima_policy @@ -24,8 +24,7 @@ Description: [euid=] [fowner=] [fsname=]] lsm: [[subj_user=] [subj_role=] [subj_type=] [obj_user=] [obj_role=] [obj_type=]] - option: [[appraise_type=]] [permit_directio] - + option: [[appraise_type=]] [template=] [permit_directio] base: func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK] [FIRMWARE_CHECK] [KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK] @@ -38,6 +37,8 @@ Description: fowner:= decimal value lsm: are LSM specific option: appraise_type:= [imasig] + template:= name of a defined IMA template type + (eg, ima-ng). Only valid when action is "measure". pcr:= decimal value default policy: diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 2203451862d4..e7b9ea7732d9 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -146,7 +146,11 @@ void ima_add_violation(struct file *file, const unsigned char *filename, int ima_init_crypto(void); void ima_putc(struct seq_file *m, void *data, int datalen); void ima_print_digest(struct seq_file *m, u8 *digest, u32 size); +int template_desc_init_fields(const char *template_fmt, + const struct ima_template_field ***fields, + int *num_fields); struct ima_template_desc *ima_template_desc_current(void); +struct ima_template_desc *lookup_template_desc(const char *name); int ima_restore_measurement_entry(struct ima_template_entry *entry); int ima_restore_measurement_list(loff_t bufsize, void *buf); int ima_measurements_show(struct seq_file *m, void *v); @@ -195,7 +199,8 @@ enum ima_hooks { /* LIM API function definitions */ int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid, - int mask, enum ima_hooks func, int *pcr); + int mask, enum ima_hooks func, int *pcr, + struct ima_template_desc **template_desc); int ima_must_measure(struct inode *inode, int mask, enum ima_hooks func); int ima_collect_measurement(struct integrity_iint_cache *iint, struct file *file, void *buf, loff_t size, @@ -203,11 +208,13 @@ int ima_collect_measurement(struct integrity_iint_cache *iint, void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file, const unsigned char *filename, struct evm_ima_xattr_data *xattr_value, - int xattr_len, int pcr); + int xattr_len, int pcr, + struct ima_template_desc *template_desc); void ima_audit_measurement(struct integrity_iint_cache *iint, const unsigned char *filename); int ima_alloc_init_template(struct ima_event_data *event_data, - struct ima_template_entry **entry); + struct ima_template_entry **entry, + struct ima_template_desc *template_desc); int ima_store_template(struct ima_template_entry *entry, int violation, struct inode *inode, const unsigned char *filename, int pcr); @@ -216,7 +223,8 @@ const char *ima_d_path(const struct path *path, char **pathbuf, char *filename); /* IMA policy related functions */ int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid, - enum ima_hooks func, int mask, int flags, int *pcr); + enum ima_hooks func, int mask, int flags, int *pcr, + struct ima_template_desc **template_desc); void ima_init_policy(void); void ima_update_policy(void); void ima_update_policy_flag(void); diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index 4aec6b035fe9..c0cf4bcfc82f 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -38,11 +38,17 @@ void ima_free_template_entry(struct ima_template_entry *entry) * ima_alloc_init_template - create and initialize a new template entry */ int ima_alloc_init_template(struct ima_event_data *event_data, - struct ima_template_entry **entry) + struct ima_template_entry **entry, + struct ima_template_desc *desc) { - struct ima_template_desc *template_desc = ima_template_desc_current(); + struct ima_template_desc *template_desc; int i, result = 0; + if (desc) + template_desc = desc; + else + template_desc = ima_template_desc_current(); + *entry = kzalloc(sizeof(**entry) + template_desc->num_fields * sizeof(struct ima_field_data), GFP_NOFS); if (!*entry) @@ -143,7 +149,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename, /* can overflow, only indicator */ atomic_long_inc(&ima_htable.violations); - result = ima_alloc_init_template(&event_data, &entry); + result = ima_alloc_init_template(&event_data, &entry, NULL); if (result < 0) { result = -ENOMEM; goto err_out; @@ -166,6 +172,7 @@ err_out: * MAY_APPEND) * @func: caller identifier * @pcr: pointer filled in if matched measure policy sets pcr= + * @template_desc: pointer filled in if matched measure policy sets template= * * The policy is defined in terms of keypairs: * subj=, obj=, type=, func=, mask=, fsmagic= @@ -178,13 +185,15 @@ err_out: * */ int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid, - int mask, enum ima_hooks func, int *pcr) + int mask, enum ima_hooks func, int *pcr, + struct ima_template_desc **template_desc) { int flags = IMA_MEASURE | IMA_AUDIT | IMA_APPRAISE | IMA_HASH; flags &= ima_policy_flag; - return ima_match_policy(inode, cred, secid, func, mask, flags, pcr); + return ima_match_policy(inode, cred, secid, func, mask, flags, pcr, + template_desc); } /* @@ -279,7 +288,8 @@ out: void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file, const unsigned char *filename, struct evm_ima_xattr_data *xattr_value, - int xattr_len, int pcr) + int xattr_len, int pcr, + struct ima_template_desc *template_desc) { static const char op[] = "add_template_measure"; static const char audit_cause[] = "ENOMEM"; @@ -296,7 +306,7 @@ void ima_store_measurement(struct integrity_iint_cache *iint, if (iint->measured_pcrs & (0x1 << pcr)) return; - result = ima_alloc_init_template(&event_data, &entry); + result = ima_alloc_init_template(&event_data, &entry, template_desc); if (result < 0) { integrity_audit_msg(AUDIT_INTEGRITY_PCR, inode, filename, op, audit_cause, result, 0); diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 5fb7127bbe68..2f6536ab69e8 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -57,7 +57,7 @@ int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func) security_task_getsecid(current, &secid); return ima_match_policy(inode, current_cred(), secid, func, mask, - IMA_APPRAISE | IMA_HASH, NULL); + IMA_APPRAISE | IMA_HASH, NULL, NULL); } static int ima_fix_xattr(struct dentry *dentry, diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c index ef6c3a26296e..368ef658a1cd 100644 --- a/security/integrity/ima/ima_init.c +++ b/security/integrity/ima/ima_init.c @@ -72,7 +72,7 @@ static int __init ima_add_boot_aggregate(void) } } - result = ima_alloc_init_template(&event_data, &entry); + result = ima_alloc_init_template(&event_data, &entry, NULL); if (result < 0) { audit_cause = "alloc_entry"; goto err_out; diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 9e3ea8a3f2db..a7e7e2d7224c 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -199,7 +199,7 @@ static int process_measurement(struct file *file, const struct cred *cred, { struct inode *inode = file_inode(file); struct integrity_iint_cache *iint = NULL; - struct ima_template_desc *template_desc; + struct ima_template_desc *template_desc = NULL; char *pathbuf = NULL; char filename[NAME_MAX]; const char *pathname = NULL; @@ -217,7 +217,8 @@ static int process_measurement(struct file *file, const struct cred *cred, * bitmask based on the appraise/audit/measurement policy. * Included is the appraise submask. */ - action = ima_get_action(inode, cred, secid, mask, func, &pcr); + action = ima_get_action(inode, cred, secid, mask, func, &pcr, + &template_desc); violation_check = ((func == FILE_CHECK || func == MMAP_CHECK) && (ima_policy_flag & IMA_MEASURE)); if (!action && !violation_check) @@ -304,7 +305,6 @@ static int process_measurement(struct file *file, const struct cred *cred, goto out_locked; } - template_desc = ima_template_desc_current(); if ((action & IMA_APPRAISE_SUBMASK) || strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) != 0) /* read 'security.ima' */ @@ -321,7 +321,8 @@ static int process_measurement(struct file *file, const struct cred *cred, if (action & IMA_MEASURE) ima_store_measurement(iint, file, pathname, - xattr_value, xattr_len, pcr); + xattr_value, xattr_len, pcr, + template_desc); if (rc == 0 && (action & IMA_APPRAISE_SUBMASK)) { inode_lock(inode); rc = ima_appraise_measurement(func, iint, file, pathname, diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index ec4efa8ea366..98c289559079 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -80,6 +80,7 @@ struct ima_rule_entry { int type; /* audit type */ } lsm[MAX_LSM_RULES]; char *fsname; + struct ima_template_desc *template; }; /* @@ -473,6 +474,7 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func) * @func: IMA hook identifier * @mask: requested action (MAY_READ | MAY_WRITE | MAY_APPEND | MAY_EXEC) * @pcr: set the pcr to extend + * @template_desc: the template that should be used for this rule * * Measure decision based on func/mask/fsmagic and LSM(subj/obj/type) * conditions. @@ -482,7 +484,8 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func) * than writes so ima_match_policy() is classical RCU candidate. */ int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid, - enum ima_hooks func, int mask, int flags, int *pcr) + enum ima_hooks func, int mask, int flags, int *pcr, + struct ima_template_desc **template_desc) { struct ima_rule_entry *entry; int action = 0, actmask = flags | (flags << 1); @@ -514,6 +517,11 @@ int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid, if ((pcr) && (entry->flags & IMA_PCR)) *pcr = entry->pcr; + if (template_desc && entry->template) + *template_desc = entry->template; + else if (template_desc) + *template_desc = ima_template_desc_current(); + if (!actmask) break; } @@ -752,7 +760,7 @@ enum { Opt_uid_gt, Opt_euid_gt, Opt_fowner_gt, Opt_uid_lt, Opt_euid_lt, Opt_fowner_lt, Opt_appraise_type, Opt_permit_directio, - Opt_pcr, Opt_err + Opt_pcr, Opt_template, Opt_err }; static const match_table_t policy_tokens = { @@ -786,6 +794,7 @@ static const match_table_t policy_tokens = { {Opt_appraise_type, "appraise_type=%s"}, {Opt_permit_directio, "permit_directio"}, {Opt_pcr, "pcr=%s"}, + {Opt_template, "template=%s"}, {Opt_err, NULL} }; @@ -839,6 +848,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) char *from; char *p; bool uid_token; + struct ima_template_desc *template_desc; int result = 0; ab = integrity_audit_log_start(audit_context(), GFP_KERNEL, @@ -1134,6 +1144,28 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) else entry->flags |= IMA_PCR; + break; + case Opt_template: + ima_log_string(ab, "template", args[0].from); + if (entry->action != MEASURE) { + result = -EINVAL; + break; + } + template_desc = lookup_template_desc(args[0].from); + if (!template_desc || entry->template) { + result = -EINVAL; + break; + } + + /* + * template_desc_init_fields() does nothing if + * the template is already initialised, so + * it's safe to do this unconditionally + */ + template_desc_init_fields(template_desc->fmt, + &(template_desc->fields), + &(template_desc->num_fields)); + entry->template = template_desc; break; case Opt_err: ima_log_string(ab, "UNKNOWN", p); @@ -1410,6 +1442,8 @@ int ima_policy_show(struct seq_file *m, void *v) } } } + if (entry->template) + seq_printf(m, "template=%s ", entry->template->name); if (entry->flags & IMA_DIGSIG_REQUIRED) seq_puts(m, "appraise_type=imasig "); if (entry->flags & IMA_PERMIT_DIRECTIO) diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c index b631b8bc7624..00dd5a434689 100644 --- a/security/integrity/ima/ima_template.c +++ b/security/integrity/ima/ima_template.c @@ -47,10 +47,6 @@ static const struct ima_template_field supported_fields[] = { #define MAX_TEMPLATE_NAME_LEN 15 static struct ima_template_desc *ima_template; -static struct ima_template_desc *lookup_template_desc(const char *name); -static int template_desc_init_fields(const char *template_fmt, - const struct ima_template_field ***fields, - int *num_fields); static int __init ima_template_setup(char *str) { @@ -108,7 +104,7 @@ static int __init ima_template_fmt_setup(char *str) } __setup("ima_template_fmt=", ima_template_fmt_setup); -static struct ima_template_desc *lookup_template_desc(const char *name) +struct ima_template_desc *lookup_template_desc(const char *name) { struct ima_template_desc *template_desc; int found = 0; @@ -153,9 +149,9 @@ static int template_fmt_size(const char *template_fmt) return j + 1; } -static int template_desc_init_fields(const char *template_fmt, - const struct ima_template_field ***fields, - int *num_fields) +int template_desc_init_fields(const char *template_fmt, + const struct ima_template_field ***fields, + int *num_fields) { const char *template_fmt_ptr; const struct ima_template_field *found_fields[IMA_TEMPLATE_NUM_FIELDS_MAX]; -- cgit v1.2.3 From b0935123a18360d19f1dcc779ea33841cdc304cc Mon Sep 17 00:00:00 2001 From: Prakhar Srivastava Date: Sun, 23 Jun 2019 23:23:29 -0700 Subject: IMA: Define a new hook to measure the kexec boot command line arguments Currently during soft reboot(kexec_file_load) boot command line arguments are not measured. Define hooks needed to measure kexec command line arguments during soft reboot(kexec_file_load). - A new ima hook ima_kexec_cmdline is defined to be called by the kexec code. - A new function process_buffer_measurement is defined to measure the buffer hash into the IMA measurement list. - A new func policy KEXEC_CMDLINE is defined to control the measurement. Signed-off-by: Prakhar Srivastava Signed-off-by: Mimi Zohar --- Documentation/ABI/testing/ima_policy | 1 + include/linux/ima.h | 2 + security/integrity/ima/ima.h | 1 + security/integrity/ima/ima_api.c | 1 + security/integrity/ima/ima_main.c | 72 ++++++++++++++++++++++++++++++++++++ security/integrity/ima/ima_policy.c | 7 ++++ 6 files changed, 84 insertions(+) diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy index b383c1763610..fc376a323908 100644 --- a/Documentation/ABI/testing/ima_policy +++ b/Documentation/ABI/testing/ima_policy @@ -28,6 +28,7 @@ Description: base: func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK] [FIRMWARE_CHECK] [KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK] + [KEXEC_CMDLINE] mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND] [[^]MAY_EXEC] fsmagic:= hex value diff --git a/include/linux/ima.h b/include/linux/ima.h index fd9f7cf4cdf5..b42f5a006042 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -26,6 +26,7 @@ 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); extern void ima_post_path_mknod(struct dentry *dentry); +extern void ima_kexec_cmdline(const void *buf, int size); #ifdef CONFIG_IMA_KEXEC extern void ima_add_kexec_buffer(struct kimage *image); @@ -92,6 +93,7 @@ static inline void ima_post_path_mknod(struct dentry *dentry) return; } +static inline void ima_kexec_cmdline(const void *buf, int size) {} #endif /* CONFIG_IMA */ #ifndef CONFIG_IMA_KEXEC diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index e7b9ea7732d9..bdca641f9e51 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -190,6 +190,7 @@ static inline unsigned long ima_hash_key(u8 *digest) hook(KEXEC_KERNEL_CHECK) \ hook(KEXEC_INITRAMFS_CHECK) \ hook(POLICY_CHECK) \ + hook(KEXEC_CMDLINE) \ hook(MAX_CHECK) #define __ima_hook_enumify(ENUM) ENUM, diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index c0cf4bcfc82f..d426d4d1fe04 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -178,6 +178,7 @@ err_out: * subj=, obj=, type=, func=, mask=, fsmagic= * subj,obj, and type: are LSM specific. * func: FILE_CHECK | BPRM_CHECK | CREDS_CHECK | MMAP_CHECK | MODULE_CHECK + * | KEXEC_CMDLINE * mask: contains the permission mask * fsmagic: hex value * diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index a7e7e2d7224c..2507bee1b762 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -609,6 +609,78 @@ int ima_load_data(enum kernel_load_data_id id) return 0; } +/* + * process_buffer_measurement - Measure the buffer to ima log. + * @buf: pointer to the buffer that needs to be added to the log. + * @size: size of buffer(in bytes). + * @eventname: event name to be used for the buffer entry. + * @cred: a pointer to a credentials structure for user validation. + * @secid: the secid of the task to be validated. + * + * Based on policy, the buffer is measured into the ima log. + */ +static void process_buffer_measurement(const void *buf, int size, + const char *eventname, + const struct cred *cred, u32 secid) +{ + int ret = 0; + struct ima_template_entry *entry = NULL; + struct integrity_iint_cache iint = {}; + struct ima_event_data event_data = {.iint = &iint, + .filename = eventname}; + struct ima_template_desc *template_desc = NULL; + struct { + struct ima_digest_data hdr; + char digest[IMA_MAX_DIGEST_SIZE]; + } hash = {}; + int violation = 0; + int pcr = CONFIG_IMA_MEASURE_PCR_IDX; + int action = 0; + + action = ima_get_action(NULL, cred, secid, 0, KEXEC_CMDLINE, &pcr, + &template_desc); + if (!(action & IMA_MEASURE)) + return; + + iint.ima_hash = &hash.hdr; + iint.ima_hash->algo = ima_hash_algo; + iint.ima_hash->length = hash_digest_size[ima_hash_algo]; + + ret = ima_calc_buffer_hash(buf, size, iint.ima_hash); + if (ret < 0) + goto out; + + ret = ima_alloc_init_template(&event_data, &entry, template_desc); + if (ret < 0) + goto out; + + ret = ima_store_template(entry, violation, NULL, buf, pcr); + + if (ret < 0) + ima_free_template_entry(entry); + +out: + return; +} + +/** + * ima_kexec_cmdline - measure kexec cmdline boot args + * @buf: pointer to buffer + * @size: size of buffer + * + * Buffers can only be measured, not appraised. + */ +void ima_kexec_cmdline(const void *buf, int size) +{ + u32 secid; + + if (buf && size != 0) { + security_task_getsecid(current, &secid); + process_buffer_measurement(buf, size, "kexec-cmdline", + current_cred(), secid); + } +} + 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 98c289559079..a3058b03a955 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -374,6 +374,11 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, { int i; + if (func == KEXEC_CMDLINE) { + if ((rule->flags & IMA_FUNC) && (rule->func == func)) + return true; + return false; + } if ((rule->flags & IMA_FUNC) && (rule->func != func && func != POST_SETATTR)) return false; @@ -956,6 +961,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) entry->func = KEXEC_INITRAMFS_CHECK; else if (strcmp(args[0].from, "POLICY_CHECK") == 0) entry->func = POLICY_CHECK; + else if (strcmp(args[0].from, "KEXEC_CMDLINE") == 0) + entry->func = KEXEC_CMDLINE; else result = -EINVAL; if (!result) -- cgit v1.2.3 From 86b4da8c0e7fcb6c217c604efcd9438ad55dd055 Mon Sep 17 00:00:00 2001 From: Prakhar Srivastava Date: Sun, 23 Jun 2019 23:23:30 -0700 Subject: IMA: Define a new template field buf A buffer(kexec boot command line arguments) measured into IMA measuremnt list cannot be appraised, without already being aware of the buffer contents. Since hashes are non-reversible, raw buffer is needed for validation or regenerating hash for appraisal/attestation. Add support to store/read the buffer contents in HEX. The kexec cmdline hash is stored in the "d-ng" field of the template data. It can be verified using sudo cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements | grep kexec-cmdline | cut -d' ' -f 6 | xxd -r -p | sha256sum - Add two new fields to ima_event_data to hold the buf and buf_len - Add a new template field 'buf' to be used to store/read the buffer data. - Updated process_buffer_meaurement to add the buffer to ima_event_data. process_buffer_measurement added in "Define a new IMA hook to measure the boot command line arguments" - Add a new template policy name ima-buf to represent 'd-ng|n-ng|buf' Signed-off-by: Prakhar Srivastava Reviewed-by: Roberto Sassu Reviewed-by: James Morris Signed-off-by: Mimi Zohar --- Documentation/security/IMA-templates.rst | 7 ++++--- security/integrity/ima/ima.h | 2 ++ security/integrity/ima/ima_main.c | 4 +++- security/integrity/ima/ima_template.c | 3 +++ security/integrity/ima/ima_template_lib.c | 21 +++++++++++++++++++++ security/integrity/ima/ima_template_lib.h | 4 ++++ 6 files changed, 37 insertions(+), 4 deletions(-) diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst index 2cd0e273cc9a..3d1cca287aa4 100644 --- a/Documentation/security/IMA-templates.rst +++ b/Documentation/security/IMA-templates.rst @@ -69,15 +69,16 @@ descriptors by adding their identifier to the format string algorithm (field format: [:]digest, where the digest prefix is shown only if the hash algorithm is not SHA1 or MD5); - 'n-ng': the name of the event, without size limitations; - - 'sig': the file signature. + - 'sig': the file signature; + - 'buf': the buffer data that was used to generate the hash without size limitations; Below, there is the list of defined template descriptors: - "ima": its format is ``d|n``; - "ima-ng" (default): its format is ``d-ng|n-ng``; - - "ima-sig": its format is ``d-ng|n-ng|sig``. - + - "ima-sig": its format is ``d-ng|n-ng|sig``; + - "ima-buf": its format is ``d-ng|n-ng|buf``; Use diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index bdca641f9e51..6aa28ab53d27 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -65,6 +65,8 @@ struct ima_event_data { struct evm_ima_xattr_data *xattr_value; int xattr_len; const char *violation; + const void *buf; + int buf_len; }; /* IMA template field data definition */ diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 2507bee1b762..317c4b6f2c18 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -627,7 +627,9 @@ static void process_buffer_measurement(const void *buf, int size, struct ima_template_entry *entry = NULL; struct integrity_iint_cache iint = {}; struct ima_event_data event_data = {.iint = &iint, - .filename = eventname}; + .filename = eventname, + .buf = buf, + .buf_len = size}; struct ima_template_desc *template_desc = NULL; struct { struct ima_digest_data hdr; diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c index 00dd5a434689..a01a17e5c581 100644 --- a/security/integrity/ima/ima_template.c +++ b/security/integrity/ima/ima_template.c @@ -26,6 +26,7 @@ static struct ima_template_desc builtin_templates[] = { {.name = IMA_TEMPLATE_IMA_NAME, .fmt = IMA_TEMPLATE_IMA_FMT}, {.name = "ima-ng", .fmt = "d-ng|n-ng"}, {.name = "ima-sig", .fmt = "d-ng|n-ng|sig"}, + {.name = "ima-buf", .fmt = "d-ng|n-ng|buf"}, {.name = "", .fmt = ""}, /* placeholder for a custom format */ }; @@ -43,6 +44,8 @@ static const struct ima_template_field supported_fields[] = { .field_show = ima_show_template_string}, {.field_id = "sig", .field_init = ima_eventsig_init, .field_show = ima_show_template_sig}, + {.field_id = "buf", .field_init = ima_eventbuf_init, + .field_show = ima_show_template_buf}, }; #define MAX_TEMPLATE_NAME_LEN 15 diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c index 513b457ae900..baf4de45c5aa 100644 --- a/security/integrity/ima/ima_template_lib.c +++ b/security/integrity/ima/ima_template_lib.c @@ -162,6 +162,12 @@ void ima_show_template_sig(struct seq_file *m, enum ima_show_type show, ima_show_template_field_data(m, show, DATA_FMT_HEX, field_data); } +void ima_show_template_buf(struct seq_file *m, enum ima_show_type show, + struct ima_field_data *field_data) +{ + ima_show_template_field_data(m, show, DATA_FMT_HEX, field_data); +} + /** * ima_parse_buf() - Parses lengths and data from an input buffer * @bufstartp: Buffer start address. @@ -389,3 +395,18 @@ int ima_eventsig_init(struct ima_event_data *event_data, return ima_write_template_field_data(xattr_value, event_data->xattr_len, DATA_FMT_HEX, field_data); } + +/* + * ima_eventbuf_init - include the buffer(kexec-cmldine) as part of the + * template data. + */ +int ima_eventbuf_init(struct ima_event_data *event_data, + struct ima_field_data *field_data) +{ + if ((!event_data->buf) || (event_data->buf_len == 0)) + return 0; + + return ima_write_template_field_data(event_data->buf, + event_data->buf_len, DATA_FMT_HEX, + field_data); +} diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h index 6a3d8b831deb..12f1a8578b31 100644 --- a/security/integrity/ima/ima_template_lib.h +++ b/security/integrity/ima/ima_template_lib.h @@ -29,6 +29,8 @@ void ima_show_template_string(struct seq_file *m, enum ima_show_type show, struct ima_field_data *field_data); void ima_show_template_sig(struct seq_file *m, enum ima_show_type show, struct ima_field_data *field_data); +void ima_show_template_buf(struct seq_file *m, enum ima_show_type show, + struct ima_field_data *field_data); int ima_parse_buf(void *bufstartp, void *bufendp, void **bufcurp, int maxfields, struct ima_field_data *fields, int *curfields, unsigned long *len_mask, int enforce_mask, char *bufname); @@ -42,4 +44,6 @@ int ima_eventname_ng_init(struct ima_event_data *event_data, struct ima_field_data *field_data); int ima_eventsig_init(struct ima_event_data *event_data, struct ima_field_data *field_data); +int ima_eventbuf_init(struct ima_event_data *event_data, + struct ima_field_data *field_data); #endif /* __LINUX_IMA_TEMPLATE_LIB_H */ -- cgit v1.2.3 From 6a31fcd4cff84490bf5ac11dbeedfdca5b38b39a Mon Sep 17 00:00:00 2001 From: Prakhar Srivastava Date: Sun, 23 Jun 2019 23:23:31 -0700 Subject: KEXEC: Call ima_kexec_cmdline to measure the boot command line args During soft reboot(kexec_file_load) boot command line arguments are not measured. Call ima hook ima_kexec_cmdline to measure the boot command line arguments into IMA measurement list. - call ima_kexec_cmdline from kexec_file_load. - move the call ima_add_kexec_buffer after the cmdline args have been measured. Signed-off-by: Prakhar Srivastava Reviewed-by: James Morris Acked-by: Dave Young Signed-off-by: Mimi Zohar --- kernel/kexec_file.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c index 072b6ee55e3f..b0c724e5d86c 100644 --- a/kernel/kexec_file.c +++ b/kernel/kexec_file.c @@ -198,9 +198,6 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd, return ret; image->kernel_buf_len = size; - /* IMA needs to pass the measurement list to the next kernel. */ - ima_add_kexec_buffer(image); - /* Call arch image probe handlers */ ret = arch_kexec_kernel_image_probe(image, image->kernel_buf, image->kernel_buf_len); @@ -241,8 +238,14 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd, ret = -EINVAL; goto out; } + + ima_kexec_cmdline(image->cmdline_buf, + image->cmdline_buf_len - 1); } + /* IMA needs to pass the measurement list to the next kernel. */ + ima_add_kexec_buffer(image); + /* Call arch image load handlers */ ldata = arch_kexec_kernel_image_load(image); -- cgit v1.2.3 From 337619eb44ef663d329c5c19efd18488ce7de492 Mon Sep 17 00:00:00 2001 From: Thiago Jung Bauermann Date: Thu, 27 Jun 2019 20:25:46 -0300 Subject: ima: Update MAX_TEMPLATE_NAME_LEN to fit largest reasonable definition MAX_TEMPLATE_NAME_LEN is used when restoring measurements carried over from a kexec. It should be set to the length of a template containing all fields except for 'd' and 'n', which don't need to be accounted for since they shouldn't be defined in the same template description as 'd-ng' and 'n-ng'. That length is greater than the current 15, so update using a sizeof() to show where the number comes from and also can be visually shown to be correct. The sizeof() is calculated at compile time. Signed-off-by: Thiago Jung Bauermann Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_template.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c index a01a17e5c581..7343e8e0ae2f 100644 --- a/security/integrity/ima/ima_template.c +++ b/security/integrity/ima/ima_template.c @@ -47,7 +47,13 @@ static const struct ima_template_field supported_fields[] = { {.field_id = "buf", .field_init = ima_eventbuf_init, .field_show = ima_show_template_buf}, }; -#define MAX_TEMPLATE_NAME_LEN 15 + +/* + * Used when restoring measurements carried over from a kexec. 'd' and 'n' don't + * need to be accounted for since they shouldn't be defined in the same template + * description as 'd-ng' and 'n-ng' respectively. + */ +#define MAX_TEMPLATE_NAME_LEN sizeof("d-ng|n-ng|sig|buf") static struct ima_template_desc *ima_template; -- cgit v1.2.3 From 650b29dbdf2caf7db27cdc8bfa8fc009b28a6ce3 Mon Sep 17 00:00:00 2001 From: Thiago Jung Bauermann Date: Tue, 11 Jun 2019 03:28:08 -0300 Subject: integrity: Introduce struct evm_xattr Even though struct evm_ima_xattr_data includes a fixed-size array to hold a SHA1 digest, most of the code ignores the array and uses the struct to mean "type indicator followed by data of unspecified size" and tracks the real size of what the struct represents in a separate length variable. The only exception to that is the EVM code, which correctly uses the definition of struct evm_ima_xattr_data. So make this explicit in the code by removing the length specification from the array in struct evm_ima_xattr_data. Also, change the name of the element from digest to data since in most places the array doesn't hold a digest. A separate struct evm_xattr is introduced, with the original definition of evm_ima_xattr_data to be used in the places that actually expect that definition, specifically the EVM HMAC code. Signed-off-by: Thiago Jung Bauermann Signed-off-by: Mimi Zohar --- security/integrity/evm/evm_main.c | 8 ++++---- security/integrity/ima/ima_appraise.c | 7 ++++--- security/integrity/integrity.h | 6 ++++++ 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index b6d9f14bc234..588f22f1b5bd 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -169,7 +169,7 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, /* check value type */ switch (xattr_data->type) { case EVM_XATTR_HMAC: - if (xattr_len != sizeof(struct evm_ima_xattr_data)) { + if (xattr_len != sizeof(struct evm_xattr)) { evm_status = INTEGRITY_FAIL; goto out; } @@ -179,7 +179,7 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, xattr_value_len, &digest); if (rc) break; - rc = crypto_memneq(xattr_data->digest, digest.digest, + rc = crypto_memneq(xattr_data->data, digest.digest, SHA1_DIGEST_SIZE); if (rc) rc = -EINVAL; @@ -523,7 +523,7 @@ int evm_inode_init_security(struct inode *inode, const struct xattr *lsm_xattr, struct xattr *evm_xattr) { - struct evm_ima_xattr_data *xattr_data; + struct evm_xattr *xattr_data; int rc; if (!evm_key_loaded() || !evm_protected_xattr(lsm_xattr->name)) @@ -533,7 +533,7 @@ int evm_inode_init_security(struct inode *inode, if (!xattr_data) return -ENOMEM; - xattr_data->type = EVM_XATTR_HMAC; + xattr_data->data.type = EVM_XATTR_HMAC; rc = evm_init_hmac(inode, lsm_xattr, xattr_data->digest); if (rc < 0) goto out; diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 2f6536ab69e8..18bbe753421a 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -168,7 +168,8 @@ enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, return sig->hash_algo; break; case IMA_XATTR_DIGEST_NG: - ret = xattr_value->digest[0]; + /* first byte contains algorithm id */ + ret = xattr_value->data[0]; if (ret < HASH_ALGO__LAST) return ret; break; @@ -176,7 +177,7 @@ enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, /* this is for backward compatibility */ if (xattr_len == 21) { unsigned int zero = 0; - if (!memcmp(&xattr_value->digest[16], &zero, 4)) + if (!memcmp(&xattr_value->data[16], &zero, 4)) return HASH_ALGO_MD5; else return HASH_ALGO_SHA1; @@ -275,7 +276,7 @@ int ima_appraise_measurement(enum ima_hooks func, /* xattr length may be longer. md5 hash in previous version occupied 20 bytes in xattr, instead of 16 */ - rc = memcmp(&xattr_value->digest[hash_start], + rc = memcmp(&xattr_value->data[hash_start], iint->ima_hash->digest, iint->ima_hash->length); else diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index 7de59f44cba3..88a29f72a74f 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -79,6 +79,12 @@ enum evm_ima_xattr_type { struct evm_ima_xattr_data { u8 type; + u8 data[]; +} __packed; + +/* Only used in the EVM HMAC code. */ +struct evm_xattr { + struct evm_ima_xattr_data data; u8 digest[SHA1_DIGEST_SIZE]; } __packed; -- cgit v1.2.3