From b36f281f4a314de4be0a51d6511b794691f8a244 Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Fri, 19 Jul 2019 07:16:57 -0400 Subject: ima: initialize the "template" field with the default template IMA policy rules are walked sequentially. Depending on the ordering of the policy rules, the "template" field might be defined in one rule, but will be replaced by subsequent, applicable rules, even if the rule does not explicitly define the "template" field. This patch initializes the "template" once and only replaces the "template", when explicitly defined. Fixes: 19453ce0bcfb ("IMA: support for per policy rule template formats") Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_policy.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 6df7f641ff66..36a0727f1d7a 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -491,6 +491,9 @@ int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid, struct ima_rule_entry *entry; int action = 0, actmask = flags | (flags << 1); + if (template_desc) + *template_desc = ima_template_desc_current(); + rcu_read_lock(); list_for_each_entry_rcu(entry, ima_rules, list) { @@ -510,6 +513,7 @@ int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid, action |= IMA_FAIL_UNVERIFIABLE_SIGS; } + if (entry->action & IMA_DO_MASK) actmask &= ~(entry->action | entry->action << 1); else @@ -520,8 +524,6 @@ int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid, if (template_desc && entry->template) *template_desc = entry->template; - else if (template_desc) - *template_desc = ima_template_desc_current(); if (!actmask) break; -- cgit From cf38fed1e183dd2410f62d49ae635fe593082f0c Mon Sep 17 00:00:00 2001 From: Thiago Jung Bauermann Date: Thu, 27 Jun 2019 23:19:27 -0300 Subject: integrity: Select CONFIG_KEYS instead of depending on it This avoids a dependency cycle in soon-to-be-introduced CONFIG_IMA_APPRAISE_MODSIG: it will select CONFIG_MODULE_SIG_FORMAT which in turn selects CONFIG_KEYS. Kconfig then complains that CONFIG_INTEGRITY_SIGNATURE depends on CONFIG_KEYS. Signed-off-by: Thiago Jung Bauermann Signed-off-by: Mimi Zohar --- security/integrity/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig index c352532b8f84..0bae6adb63a9 100644 --- a/security/integrity/Kconfig +++ b/security/integrity/Kconfig @@ -18,8 +18,8 @@ if INTEGRITY config INTEGRITY_SIGNATURE bool "Digital signature verification using multiple keyrings" - depends on KEYS default n + select KEYS select SIGNATURE help This option enables digital signature verification support -- cgit From 9044d627fd18f9fca49b62d4619ee14914b91464 Mon Sep 17 00:00:00 2001 From: Thiago Jung Bauermann Date: Thu, 27 Jun 2019 23:19:28 -0300 Subject: ima: Add modsig appraise_type option for module-style appended signatures Introduce the modsig keyword to the IMA policy syntax to specify that a given hook should expect the file to have the IMA signature appended to it. Here is how it can be used in a rule: appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig With this rule, IMA will accept either a signature stored in the extended attribute or an appended signature. For now, the rule above will behave exactly the same as if appraise_type=imasig was specified. The actual modsig implementation will be introduced separately. Suggested-by: Mimi Zohar Signed-off-by: Thiago Jung Bauermann Signed-off-by: Mimi Zohar --- security/integrity/ima/Kconfig | 10 ++++++++++ security/integrity/ima/Makefile | 1 + security/integrity/ima/ima.h | 9 +++++++++ security/integrity/ima/ima_modsig.c | 31 +++++++++++++++++++++++++++++++ security/integrity/ima/ima_policy.c | 12 ++++++++++-- security/integrity/integrity.h | 1 + 6 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 security/integrity/ima/ima_modsig.c (limited to 'security') diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig index 2ced99dde694..8bf46646b185 100644 --- a/security/integrity/ima/Kconfig +++ b/security/integrity/ima/Kconfig @@ -233,6 +233,16 @@ config IMA_APPRAISE_BOOTPARAM This option enables the different "ima_appraise=" modes (eg. fix, log) from the boot command line. +config IMA_APPRAISE_MODSIG + bool "Support module-style signatures for appraisal" + depends on IMA_APPRAISE + default n + help + Adds support for signatures appended to files. The format of the + appended signature is the same used for signed kernel modules. + The modsig keyword can be used in the IMA policy to allow a hook + to accept such signatures. + config IMA_TRUSTED_KEYRING bool "Require all keys on the .ima keyring be signed (deprecated)" depends on IMA_APPRAISE && SYSTEM_TRUSTED_KEYRING diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile index d921dc4f9eb0..31d57cdf2421 100644 --- a/security/integrity/ima/Makefile +++ b/security/integrity/ima/Makefile @@ -9,5 +9,6 @@ obj-$(CONFIG_IMA) += ima.o ima-y := ima_fs.o ima_queue.o ima_init.o ima_main.o ima_crypto.o ima_api.o \ ima_policy.o ima_template.o ima_template_lib.o ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o +ima-$(CONFIG_IMA_APPRAISE_MODSIG) += ima_modsig.o ima-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o obj-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 011b91c79351..e21b06942858 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -302,6 +302,15 @@ static inline int ima_read_xattr(struct dentry *dentry, #endif /* CONFIG_IMA_APPRAISE */ +#ifdef CONFIG_IMA_APPRAISE_MODSIG +bool ima_hook_supports_modsig(enum ima_hooks func); +#else +static inline bool ima_hook_supports_modsig(enum ima_hooks func) +{ + return false; +} +#endif /* CONFIG_IMA_APPRAISE_MODSIG */ + /* LSM based policy rules require audit */ #ifdef CONFIG_IMA_LSM_RULES diff --git a/security/integrity/ima/ima_modsig.c b/security/integrity/ima/ima_modsig.c new file mode 100644 index 000000000000..87503bfe8c8b --- /dev/null +++ b/security/integrity/ima/ima_modsig.c @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * IMA support for appraising module-style appended signatures. + * + * Copyright (C) 2019 IBM Corporation + * + * Author: + * Thiago Jung Bauermann + */ + +#include "ima.h" + +/** + * ima_hook_supports_modsig - can the policy allow modsig for this hook? + * + * modsig is only supported by hooks using ima_post_read_file(), because only + * they preload the contents of the file in a buffer. FILE_CHECK does that in + * some cases, but not when reached from vfs_open(). POLICY_CHECK can support + * it, but it's not useful in practice because it's a text file so deny. + */ +bool ima_hook_supports_modsig(enum ima_hooks func) +{ + switch (func) { + case KEXEC_KERNEL_CHECK: + case KEXEC_INITRAMFS_CHECK: + case MODULE_CHECK: + return true; + default: + return false; + } +} diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 36a0727f1d7a..5b6061d6bce0 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -1130,6 +1130,10 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) ima_log_string(ab, "appraise_type", args[0].from); if ((strcmp(args[0].from, "imasig")) == 0) entry->flags |= IMA_DIGSIG_REQUIRED; + else if (ima_hook_supports_modsig(entry->func) && + strcmp(args[0].from, "imasig|modsig") == 0) + entry->flags |= IMA_DIGSIG_REQUIRED | + IMA_MODSIG_ALLOWED; else result = -EINVAL; break; @@ -1449,8 +1453,12 @@ 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_DIGSIG_REQUIRED) { + if (entry->flags & IMA_MODSIG_ALLOWED) + seq_puts(m, "appraise_type=imasig|modsig "); + else + seq_puts(m, "appraise_type=imasig "); + } if (entry->flags & IMA_PERMIT_DIRECTIO) seq_puts(m, "permit_directio "); rcu_read_unlock(); diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index ed12d8e13d04..8c5736b68156 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -31,6 +31,7 @@ #define IMA_NEW_FILE 0x04000000 #define EVM_IMMUTABLE_DIGSIG 0x08000000 #define IMA_FAIL_UNVERIFIABLE_SIGS 0x10000000 +#define IMA_MODSIG_ALLOWED 0x20000000 #define IMA_DO_MASK (IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \ IMA_HASH | IMA_APPRAISE_SUBMASK) -- cgit From a5fbeb615ca42f913ace3291d636e96feabcc545 Mon Sep 17 00:00:00 2001 From: Thiago Jung Bauermann Date: Thu, 27 Jun 2019 23:19:29 -0300 Subject: ima: Factor xattr_verify() out of ima_appraise_measurement() Verify xattr signature in a separate function so that the logic in ima_appraise_measurement() remains clear when it gains the ability to also verify an appended module signature. The code in the switch statement is unchanged except for having to dereference the status and cause variables (since they're now pointers), and fixing the style of a block comment to appease checkpatch. Suggested-by: Mimi Zohar Signed-off-by: Thiago Jung Bauermann Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_appraise.c | 141 +++++++++++++++++++--------------- 1 file changed, 81 insertions(+), 60 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 89b83194d1dc..d3298045737f 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -199,6 +199,83 @@ int ima_read_xattr(struct dentry *dentry, return ret; } +/* + * xattr_verify - verify xattr digest or signature + * + * Verify whether the hash or signature matches the file contents. + * + * Return 0 on success, error code otherwise. + */ +static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint, + struct evm_ima_xattr_data *xattr_value, int xattr_len, + enum integrity_status *status, const char **cause) +{ + int rc = -EINVAL, hash_start = 0; + + switch (xattr_value->type) { + case IMA_XATTR_DIGEST_NG: + /* first byte contains algorithm id */ + hash_start = 1; + /* fall through */ + case IMA_XATTR_DIGEST: + if (iint->flags & IMA_DIGSIG_REQUIRED) { + *cause = "IMA-signature-required"; + *status = INTEGRITY_FAIL; + break; + } + clear_bit(IMA_DIGSIG, &iint->atomic_flags); + if (xattr_len - sizeof(xattr_value->type) - hash_start >= + iint->ima_hash->length) + /* + * xattr length may be longer. md5 hash in previous + * version occupied 20 bytes in xattr, instead of 16 + */ + rc = memcmp(&xattr_value->data[hash_start], + iint->ima_hash->digest, + iint->ima_hash->length); + else + rc = -EINVAL; + if (rc) { + *cause = "invalid-hash"; + *status = INTEGRITY_FAIL; + break; + } + *status = INTEGRITY_PASS; + break; + case EVM_IMA_XATTR_DIGSIG: + set_bit(IMA_DIGSIG, &iint->atomic_flags); + rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA, + (const char *)xattr_value, + xattr_len, + iint->ima_hash->digest, + iint->ima_hash->length); + if (rc == -EOPNOTSUPP) { + *status = INTEGRITY_UNKNOWN; + break; + } + if (IS_ENABLED(CONFIG_INTEGRITY_PLATFORM_KEYRING) && rc && + func == KEXEC_KERNEL_CHECK) + rc = integrity_digsig_verify(INTEGRITY_KEYRING_PLATFORM, + (const char *)xattr_value, + xattr_len, + iint->ima_hash->digest, + iint->ima_hash->length); + if (rc) { + *cause = "invalid-signature"; + *status = INTEGRITY_FAIL; + } else { + *status = INTEGRITY_PASS; + } + break; + default: + *status = INTEGRITY_UNKNOWN; + *cause = "unknown-ima-data"; + break; + } + + return rc; +} + /* * ima_appraise_measurement - appraise file measurement * @@ -218,7 +295,7 @@ int ima_appraise_measurement(enum ima_hooks func, struct dentry *dentry = file_dentry(file); struct inode *inode = d_backing_inode(dentry); enum integrity_status status = INTEGRITY_UNKNOWN; - int rc = xattr_len, hash_start = 0; + int rc = xattr_len; if (!(inode->i_opflags & IOP_XATTR)) return INTEGRITY_UNKNOWN; @@ -256,65 +333,9 @@ int ima_appraise_measurement(enum ima_hooks func, WARN_ONCE(true, "Unexpected integrity status %d\n", status); } - switch (xattr_value->type) { - case IMA_XATTR_DIGEST_NG: - /* first byte contains algorithm id */ - hash_start = 1; - /* fall through */ - case IMA_XATTR_DIGEST: - if (iint->flags & IMA_DIGSIG_REQUIRED) { - cause = "IMA-signature-required"; - status = INTEGRITY_FAIL; - break; - } - clear_bit(IMA_DIGSIG, &iint->atomic_flags); - if (xattr_len - sizeof(xattr_value->type) - hash_start >= - iint->ima_hash->length) - /* xattr length may be longer. md5 hash in previous - version occupied 20 bytes in xattr, instead of 16 - */ - rc = memcmp(&xattr_value->data[hash_start], - iint->ima_hash->digest, - iint->ima_hash->length); - else - rc = -EINVAL; - if (rc) { - cause = "invalid-hash"; - status = INTEGRITY_FAIL; - break; - } - status = INTEGRITY_PASS; - break; - case EVM_IMA_XATTR_DIGSIG: - set_bit(IMA_DIGSIG, &iint->atomic_flags); - rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA, - (const char *)xattr_value, - xattr_len, - iint->ima_hash->digest, - iint->ima_hash->length); - if (rc == -EOPNOTSUPP) { - status = INTEGRITY_UNKNOWN; - break; - } - if (IS_ENABLED(CONFIG_INTEGRITY_PLATFORM_KEYRING) && rc && - func == KEXEC_KERNEL_CHECK) - rc = integrity_digsig_verify(INTEGRITY_KEYRING_PLATFORM, - (const char *)xattr_value, - xattr_len, - iint->ima_hash->digest, - iint->ima_hash->length); - if (rc) { - cause = "invalid-signature"; - status = INTEGRITY_FAIL; - } else { - status = INTEGRITY_PASS; - } - break; - default: - status = INTEGRITY_UNKNOWN; - cause = "unknown-ima-data"; - break; - } + if (xattr_value) + rc = xattr_verify(func, iint, xattr_value, xattr_len, &status, + &cause); out: /* -- cgit From 39b07096364a42c516415d5f841069e885234e61 Mon Sep 17 00:00:00 2001 From: Thiago Jung Bauermann Date: Thu, 27 Jun 2019 23:19:30 -0300 Subject: ima: Implement support for module-style appended signatures Implement the appraise_type=imasig|modsig option, allowing IMA to read and verify modsig signatures. In case a file has both an xattr signature and an appended modsig, IMA will only use the appended signature if the key used by the xattr signature isn't present in the IMA or platform keyring. Because modsig verification needs to convert from an integrity keyring id to the keyring itself, add an integrity_keyring_from_id() function in digsig.c so that integrity_modsig_verify() can use it. Signed-off-by: Thiago Jung Bauermann Signed-off-by: Mimi Zohar --- security/integrity/digsig.c | 43 ++++++++++++++++----- security/integrity/ima/Kconfig | 3 ++ security/integrity/ima/ima.h | 22 ++++++++++- security/integrity/ima/ima_appraise.c | 51 +++++++++++++++++++++++-- security/integrity/ima/ima_main.c | 11 +++++- security/integrity/ima/ima_modsig.c | 71 +++++++++++++++++++++++++++++++++++ security/integrity/ima/ima_policy.c | 12 +++--- security/integrity/integrity.h | 19 ++++++++++ 8 files changed, 209 insertions(+), 23 deletions(-) (limited to 'security') diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c index 868ade3e8970..ea1aae3d07b3 100644 --- a/security/integrity/digsig.c +++ b/security/integrity/digsig.c @@ -39,11 +39,10 @@ static const char * const keyring_name[INTEGRITY_KEYRING_MAX] = { #define restrict_link_to_ima restrict_link_by_builtin_trusted #endif -int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen, - const char *digest, int digestlen) +static struct key *integrity_keyring_from_id(const unsigned int id) { - if (id >= INTEGRITY_KEYRING_MAX || siglen < 2) - return -EINVAL; + if (id >= INTEGRITY_KEYRING_MAX) + return ERR_PTR(-EINVAL); if (!keyring[id]) { keyring[id] = @@ -52,23 +51,49 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen, int err = PTR_ERR(keyring[id]); pr_err("no %s keyring: %d\n", keyring_name[id], err); keyring[id] = NULL; - return err; + return ERR_PTR(err); } } + return keyring[id]; +} + +int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen, + const char *digest, int digestlen) +{ + struct key *keyring; + + if (siglen < 2) + return -EINVAL; + + keyring = integrity_keyring_from_id(id); + if (IS_ERR(keyring)) + return PTR_ERR(keyring); + switch (sig[1]) { case 1: /* v1 API expect signature without xattr type */ - return digsig_verify(keyring[id], sig + 1, siglen - 1, - digest, digestlen); + return digsig_verify(keyring, sig + 1, siglen - 1, digest, + digestlen); case 2: - return asymmetric_verify(keyring[id], sig, siglen, - digest, digestlen); + return asymmetric_verify(keyring, sig, siglen, digest, + digestlen); } return -EOPNOTSUPP; } +int integrity_modsig_verify(const unsigned int id, const struct modsig *modsig) +{ + struct key *keyring; + + keyring = integrity_keyring_from_id(id); + if (IS_ERR(keyring)) + return PTR_ERR(keyring); + + return ima_modsig_verify(keyring, modsig); +} + static int __init __integrity_init_keyring(const unsigned int id, key_perm_t perm, struct key_restriction *restriction) diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig index 8bf46646b185..897bafc59a33 100644 --- a/security/integrity/ima/Kconfig +++ b/security/integrity/ima/Kconfig @@ -236,6 +236,9 @@ config IMA_APPRAISE_BOOTPARAM config IMA_APPRAISE_MODSIG bool "Support module-style signatures for appraisal" depends on IMA_APPRAISE + depends on INTEGRITY_ASYMMETRIC_KEYS + select PKCS7_MESSAGE_PARSER + select MODULE_SIG_FORMAT default n help Adds support for signatures appended to files. The format of the diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index e21b06942858..ed8e19d38805 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -196,6 +196,10 @@ enum ima_hooks { __ima_hooks(__ima_hook_enumify) }; +extern const char *const func_tokens[]; + +struct modsig; + /* 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, @@ -249,7 +253,7 @@ int ima_appraise_measurement(enum ima_hooks func, struct integrity_iint_cache *iint, struct file *file, const unsigned char *filename, struct evm_ima_xattr_data *xattr_value, - int xattr_len); + int xattr_len, const struct modsig *modsig); int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func); void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file); enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint, @@ -265,7 +269,8 @@ static inline int ima_appraise_measurement(enum ima_hooks func, struct file *file, const unsigned char *filename, struct evm_ima_xattr_data *xattr_value, - int xattr_len) + int xattr_len, + const struct modsig *modsig) { return INTEGRITY_UNKNOWN; } @@ -304,11 +309,24 @@ static inline int ima_read_xattr(struct dentry *dentry, #ifdef CONFIG_IMA_APPRAISE_MODSIG bool ima_hook_supports_modsig(enum ima_hooks func); +int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len, + struct modsig **modsig); +void ima_free_modsig(struct modsig *modsig); #else static inline bool ima_hook_supports_modsig(enum ima_hooks func) { return false; } + +static inline int ima_read_modsig(enum ima_hooks func, const void *buf, + loff_t buf_len, struct modsig **modsig) +{ + return -EOPNOTSUPP; +} + +static inline void ima_free_modsig(struct modsig *modsig) +{ +} #endif /* CONFIG_IMA_APPRAISE_MODSIG */ /* LSM based policy rules require audit */ diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index d3298045737f..d21e40eaba42 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -276,6 +276,33 @@ static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint, return rc; } +/* + * modsig_verify - verify modsig signature + * + * Verify whether the signature matches the file contents. + * + * Return 0 on success, error code otherwise. + */ +static int modsig_verify(enum ima_hooks func, const struct modsig *modsig, + enum integrity_status *status, const char **cause) +{ + int rc; + + rc = integrity_modsig_verify(INTEGRITY_KEYRING_IMA, modsig); + if (IS_ENABLED(CONFIG_INTEGRITY_PLATFORM_KEYRING) && rc && + func == KEXEC_KERNEL_CHECK) + rc = integrity_modsig_verify(INTEGRITY_KEYRING_PLATFORM, + modsig); + if (rc) { + *cause = "invalid-signature"; + *status = INTEGRITY_FAIL; + } else { + *status = INTEGRITY_PASS; + } + + return rc; +} + /* * ima_appraise_measurement - appraise file measurement * @@ -288,7 +315,7 @@ int ima_appraise_measurement(enum ima_hooks func, struct integrity_iint_cache *iint, struct file *file, const unsigned char *filename, struct evm_ima_xattr_data *xattr_value, - int xattr_len) + int xattr_len, const struct modsig *modsig) { static const char op[] = "appraise_data"; const char *cause = "unknown"; @@ -296,11 +323,14 @@ int ima_appraise_measurement(enum ima_hooks func, struct inode *inode = d_backing_inode(dentry); enum integrity_status status = INTEGRITY_UNKNOWN; int rc = xattr_len; + bool try_modsig = iint->flags & IMA_MODSIG_ALLOWED && modsig; - if (!(inode->i_opflags & IOP_XATTR)) + /* If not appraising a modsig, we need an xattr. */ + if (!(inode->i_opflags & IOP_XATTR) && !try_modsig) return INTEGRITY_UNKNOWN; - if (rc <= 0) { + /* If reading the xattr failed and there's no modsig, error out. */ + if (rc <= 0 && !try_modsig) { if (rc && rc != -ENODATA) goto out; @@ -323,6 +353,10 @@ int ima_appraise_measurement(enum ima_hooks func, case INTEGRITY_UNKNOWN: break; case INTEGRITY_NOXATTRS: /* No EVM protected xattrs. */ + /* It's fine not to have xattrs when using a modsig. */ + if (try_modsig) + break; + /* fall through */ case INTEGRITY_NOLABEL: /* No security.evm xattr. */ cause = "missing-HMAC"; goto out; @@ -337,6 +371,15 @@ int ima_appraise_measurement(enum ima_hooks func, rc = xattr_verify(func, iint, xattr_value, xattr_len, &status, &cause); + /* + * If we have a modsig and either no imasig or the imasig's key isn't + * known, then try verifying the modsig. + */ + if (try_modsig && + (!xattr_value || xattr_value->type == IMA_XATTR_DIGEST_NG || + rc == -ENOKEY)) + rc = modsig_verify(func, modsig, &status, &cause); + out: /* * File signatures on some filesystems can not be properly verified. @@ -353,7 +396,7 @@ out: op, cause, rc, 0); } else if (status != INTEGRITY_PASS) { /* Fix mode, but don't replace file signatures. */ - if ((ima_appraise & IMA_APPRAISE_FIX) && + if ((ima_appraise & IMA_APPRAISE_FIX) && !try_modsig && (!xattr_value || xattr_value->type != EVM_IMA_XATTR_DIGSIG)) { if (!ima_fix_xattr(dentry, iint)) diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 584019728660..d8672e850615 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -202,6 +202,7 @@ static int process_measurement(struct file *file, const struct cred *cred, int rc = 0, action, must_appraise = 0; int pcr = CONFIG_IMA_MEASURE_PCR_IDX; struct evm_ima_xattr_data *xattr_value = NULL; + struct modsig *modsig = NULL; int xattr_len = 0; bool violation_check; enum hash_algo hash_algo; @@ -302,10 +303,15 @@ static int process_measurement(struct file *file, const struct cred *cred, } if ((action & IMA_APPRAISE_SUBMASK) || - strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) != 0) + strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) != 0) { /* read 'security.ima' */ xattr_len = ima_read_xattr(file_dentry(file), &xattr_value); + /* Read the appended modsig if allowed by the policy. */ + if (iint->flags & IMA_MODSIG_ALLOWED) + ima_read_modsig(func, buf, size, &modsig); + } + hash_algo = ima_get_hash_algo(xattr_value, xattr_len); rc = ima_collect_measurement(iint, file, buf, size, hash_algo); @@ -322,7 +328,7 @@ static int process_measurement(struct file *file, const struct cred *cred, if (rc == 0 && (action & IMA_APPRAISE_SUBMASK)) { inode_lock(inode); rc = ima_appraise_measurement(func, iint, file, pathname, - xattr_value, xattr_len); + xattr_value, xattr_len, modsig); inode_unlock(inode); if (!rc) rc = mmap_violation_check(func, file, &pathbuf, @@ -339,6 +345,7 @@ out_locked: rc = -EACCES; mutex_unlock(&iint->mutex); kfree(xattr_value); + ima_free_modsig(modsig); out: if (pathbuf) __putname(pathbuf); diff --git a/security/integrity/ima/ima_modsig.c b/security/integrity/ima/ima_modsig.c index 87503bfe8c8b..f41ebe370fa0 100644 --- a/security/integrity/ima/ima_modsig.c +++ b/security/integrity/ima/ima_modsig.c @@ -8,8 +8,17 @@ * Thiago Jung Bauermann */ +#include +#include +#include +#include + #include "ima.h" +struct modsig { + struct pkcs7_message *pkcs7_msg; +}; + /** * ima_hook_supports_modsig - can the policy allow modsig for this hook? * @@ -29,3 +38,65 @@ bool ima_hook_supports_modsig(enum ima_hooks func) return false; } } + +/* + * ima_read_modsig - Read modsig from buf. + * + * Return: 0 on success, error code otherwise. + */ +int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len, + struct modsig **modsig) +{ + const size_t marker_len = strlen(MODULE_SIG_STRING); + const struct module_signature *sig; + struct modsig *hdr; + size_t sig_len; + const void *p; + int rc; + + if (buf_len <= marker_len + sizeof(*sig)) + return -ENOENT; + + p = buf + buf_len - marker_len; + if (memcmp(p, MODULE_SIG_STRING, marker_len)) + return -ENOENT; + + buf_len -= marker_len; + sig = (const struct module_signature *)(p - sizeof(*sig)); + + rc = mod_check_sig(sig, buf_len, func_tokens[func]); + if (rc) + return rc; + + sig_len = be32_to_cpu(sig->sig_len); + buf_len -= sig_len + sizeof(*sig); + + hdr = kmalloc(sizeof(*hdr), GFP_KERNEL); + if (!hdr) + return -ENOMEM; + + hdr->pkcs7_msg = pkcs7_parse_message(buf + buf_len, sig_len); + if (IS_ERR(hdr->pkcs7_msg)) { + kfree(hdr); + return PTR_ERR(hdr->pkcs7_msg); + } + + *modsig = hdr; + + return 0; +} + +int ima_modsig_verify(struct key *keyring, const struct modsig *modsig) +{ + return verify_pkcs7_message_sig(NULL, 0, modsig->pkcs7_msg, keyring, + VERIFYING_MODULE_SIGNATURE, NULL, NULL); +} + +void ima_free_modsig(struct modsig *modsig) +{ + if (!modsig) + return; + + pkcs7_free_message(modsig->pkcs7_msg); + kfree(modsig); +} diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 5b6061d6bce0..873dd7edaa78 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -1258,6 +1258,12 @@ void ima_delete_rules(void) } } +#define __ima_hook_stringify(str) (#str), + +const char *const func_tokens[] = { + __ima_hooks(__ima_hook_stringify) +}; + #ifdef CONFIG_IMA_READ_POLICY enum { mask_exec = 0, mask_write, mask_read, mask_append @@ -1270,12 +1276,6 @@ static const char *const mask_tokens[] = { "^MAY_APPEND" }; -#define __ima_hook_stringify(str) (#str), - -static const char *const func_tokens[] = { - __ima_hooks(__ima_hook_stringify) -}; - void *ima_policy_start(struct seq_file *m, loff_t *pos) { loff_t l = *pos; diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index 8c5736b68156..d9323d31a3a8 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -148,10 +148,13 @@ int integrity_kernel_read(struct file *file, loff_t offset, extern struct dentry *integrity_dir; +struct modsig; + #ifdef CONFIG_INTEGRITY_SIGNATURE int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen, const char *digest, int digestlen); +int integrity_modsig_verify(unsigned int id, const struct modsig *modsig); int __init integrity_init_keyring(const unsigned int id); int __init integrity_load_x509(const unsigned int id, const char *path); @@ -166,6 +169,12 @@ static inline int integrity_digsig_verify(const unsigned int id, return -EOPNOTSUPP; } +static inline int integrity_modsig_verify(unsigned int id, + const struct modsig *modsig) +{ + return -EOPNOTSUPP; +} + static inline int integrity_init_keyring(const unsigned int id) { return 0; @@ -191,6 +200,16 @@ static inline int asymmetric_verify(struct key *keyring, const char *sig, } #endif +#ifdef CONFIG_IMA_APPRAISE_MODSIG +int ima_modsig_verify(struct key *keyring, const struct modsig *modsig); +#else +static inline int ima_modsig_verify(struct key *keyring, + const struct modsig *modsig) +{ + return -EOPNOTSUPP; +} +#endif + #ifdef CONFIG_IMA_LOAD_X509 void __init ima_load_x509(void); #else -- cgit From 15588227e086ec662d59df144e48af82e3e592f1 Mon Sep 17 00:00:00 2001 From: Thiago Jung Bauermann Date: Thu, 27 Jun 2019 23:19:31 -0300 Subject: ima: Collect modsig Obtain the modsig and calculate its corresponding hash in ima_collect_measurement(). Signed-off-by: Thiago Jung Bauermann Signed-off-by: Mimi Zohar --- security/integrity/ima/ima.h | 8 +++++- security/integrity/ima/ima_api.c | 5 +++- security/integrity/ima/ima_appraise.c | 2 +- security/integrity/ima/ima_main.c | 2 +- security/integrity/ima/ima_modsig.c | 48 ++++++++++++++++++++++++++++++++++- 5 files changed, 60 insertions(+), 5 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index ed8e19d38805..0bc764c80327 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -207,7 +207,7 @@ int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid, 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, - enum hash_algo algo); + enum hash_algo algo, struct modsig *modsig); void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file, const unsigned char *filename, struct evm_ima_xattr_data *xattr_value, @@ -311,6 +311,7 @@ static inline int ima_read_xattr(struct dentry *dentry, bool ima_hook_supports_modsig(enum ima_hooks func); int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len, struct modsig **modsig); +void ima_collect_modsig(struct modsig *modsig, const void *buf, loff_t size); void ima_free_modsig(struct modsig *modsig); #else static inline bool ima_hook_supports_modsig(enum ima_hooks func) @@ -324,6 +325,11 @@ static inline int ima_read_modsig(enum ima_hooks func, const void *buf, return -EOPNOTSUPP; } +static inline void ima_collect_modsig(struct modsig *modsig, const void *buf, + loff_t size) +{ +} + static inline void ima_free_modsig(struct modsig *modsig) { } diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index f614e22bf39f..ff8b7fb03ea0 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -205,7 +205,7 @@ int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid, */ int ima_collect_measurement(struct integrity_iint_cache *iint, struct file *file, void *buf, loff_t size, - enum hash_algo algo) + enum hash_algo algo, struct modsig *modsig) { const char *audit_cause = "failed"; struct inode *inode = file_inode(file); @@ -252,6 +252,9 @@ int ima_collect_measurement(struct integrity_iint_cache *iint, memcpy(iint->ima_hash, &hash, length); iint->version = i_version; + if (modsig) + ima_collect_modsig(modsig, buf, size); + /* Possibly temporary failure due to type of read (eg. O_DIRECT) */ if (!result) iint->flags |= IMA_COLLECTED; diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index d21e40eaba42..136ae4e0ee92 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -435,7 +435,7 @@ void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file) !(iint->flags & IMA_HASH)) return; - rc = ima_collect_measurement(iint, file, NULL, 0, ima_hash_algo); + rc = ima_collect_measurement(iint, file, NULL, 0, ima_hash_algo, NULL); if (rc < 0) return; diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index d8672e850615..7c8d92cf2755 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -314,7 +314,7 @@ static int process_measurement(struct file *file, const struct cred *cred, hash_algo = ima_get_hash_algo(xattr_value, xattr_len); - rc = ima_collect_measurement(iint, file, buf, size, hash_algo); + rc = ima_collect_measurement(iint, file, buf, size, hash_algo, modsig); if (rc != 0 && rc != -EBADF && rc != -EINVAL) goto out_locked; diff --git a/security/integrity/ima/ima_modsig.c b/security/integrity/ima/ima_modsig.c index f41ebe370fa0..257387ce3b70 100644 --- a/security/integrity/ima/ima_modsig.c +++ b/security/integrity/ima/ima_modsig.c @@ -17,6 +17,19 @@ struct modsig { struct pkcs7_message *pkcs7_msg; + + enum hash_algo hash_algo; + + /* This digest will go in the 'd-modsig' field of the IMA template. */ + const u8 *digest; + u32 digest_size; + + /* + * This is what will go to the measurement list if the template requires + * storing the signature. + */ + int raw_pkcs7_len; + u8 raw_pkcs7[]; }; /** @@ -71,7 +84,8 @@ int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len, sig_len = be32_to_cpu(sig->sig_len); buf_len -= sig_len + sizeof(*sig); - hdr = kmalloc(sizeof(*hdr), GFP_KERNEL); + /* Allocate sig_len additional bytes to hold the raw PKCS#7 data. */ + hdr = kzalloc(sizeof(*hdr) + sig_len, GFP_KERNEL); if (!hdr) return -ENOMEM; @@ -81,11 +95,43 @@ int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len, return PTR_ERR(hdr->pkcs7_msg); } + memcpy(hdr->raw_pkcs7, buf + buf_len, sig_len); + hdr->raw_pkcs7_len = sig_len; + + /* We don't know the hash algorithm yet. */ + hdr->hash_algo = HASH_ALGO__LAST; + *modsig = hdr; return 0; } +/** + * ima_collect_modsig - Calculate the file hash without the appended signature. + * + * Since the modsig is part of the file contents, the hash used in its signature + * isn't the same one ordinarily calculated by IMA. Therefore PKCS7 code + * calculates a separate one for signature verification. + */ +void ima_collect_modsig(struct modsig *modsig, const void *buf, loff_t size) +{ + int rc; + + /* + * Provide the file contents (minus the appended sig) so that the PKCS7 + * code can calculate the file hash. + */ + size -= modsig->raw_pkcs7_len + strlen(MODULE_SIG_STRING) + + sizeof(struct module_signature); + rc = pkcs7_supply_detached_data(modsig->pkcs7_msg, buf, size); + if (rc) + return; + + /* Ask the PKCS7 code to calculate the file hash. */ + rc = pkcs7_get_digest(modsig->pkcs7_msg, &modsig->digest, + &modsig->digest_size, &modsig->hash_algo); +} + int ima_modsig_verify(struct key *keyring, const struct modsig *modsig) { return verify_pkcs7_message_sig(NULL, 0, modsig->pkcs7_msg, keyring, -- cgit From 3878d505aa718bcc7b1eb4089ab9b9fb27dee957 Mon Sep 17 00:00:00 2001 From: Thiago Jung Bauermann Date: Thu, 27 Jun 2019 23:19:32 -0300 Subject: ima: Define ima-modsig template Define new "d-modsig" template field which holds the digest that is expected to match the one contained in the modsig, and also new "modsig" template field which holds the appended file signature. Add a new "ima-modsig" defined template descriptor with the new fields as well as the ones from the "ima-sig" descriptor. Change ima_store_measurement() to accept a struct modsig * argument so that it can be passed along to the templates via struct ima_event_data. Suggested-by: Mimi Zohar Signed-off-by: Thiago Jung Bauermann Signed-off-by: Mimi Zohar --- security/integrity/ima/ima.h | 20 +++++++++- security/integrity/ima/ima_api.c | 5 ++- security/integrity/ima/ima_main.c | 2 +- security/integrity/ima/ima_modsig.c | 19 +++++++++ security/integrity/ima/ima_policy.c | 41 ++++++++++++++++++++ security/integrity/ima/ima_template.c | 7 +++- security/integrity/ima/ima_template_lib.c | 64 ++++++++++++++++++++++++++++++- security/integrity/ima/ima_template_lib.h | 4 ++ 8 files changed, 156 insertions(+), 6 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 0bc764c80327..cfc8b0887776 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -60,6 +60,7 @@ struct ima_event_data { const unsigned char *filename; struct evm_ima_xattr_data *xattr_value; int xattr_len; + const struct modsig *modsig; const char *violation; const void *buf; int buf_len; @@ -211,7 +212,7 @@ 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, const struct modsig *modsig, int pcr, struct ima_template_desc *template_desc); void ima_audit_measurement(struct integrity_iint_cache *iint, const unsigned char *filename); @@ -312,6 +313,10 @@ bool ima_hook_supports_modsig(enum ima_hooks func); int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len, struct modsig **modsig); void ima_collect_modsig(struct modsig *modsig, const void *buf, loff_t size); +int ima_get_modsig_digest(const struct modsig *modsig, enum hash_algo *algo, + const u8 **digest, u32 *digest_size); +int ima_get_raw_modsig(const struct modsig *modsig, const void **data, + u32 *data_len); void ima_free_modsig(struct modsig *modsig); #else static inline bool ima_hook_supports_modsig(enum ima_hooks func) @@ -330,6 +335,19 @@ static inline void ima_collect_modsig(struct modsig *modsig, const void *buf, { } +static inline int ima_get_modsig_digest(const struct modsig *modsig, + enum hash_algo *algo, const u8 **digest, + u32 *digest_size) +{ + return -EOPNOTSUPP; +} + +static inline int ima_get_raw_modsig(const struct modsig *modsig, + const void **data, u32 *data_len) +{ + return -EOPNOTSUPP; +} + static inline void ima_free_modsig(struct modsig *modsig) { } diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index ff8b7fb03ea0..ca930e2ebc2c 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -288,7 +288,7 @@ 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, const struct modsig *modsig, int pcr, struct ima_template_desc *template_desc) { static const char op[] = "add_template_measure"; @@ -300,7 +300,8 @@ void ima_store_measurement(struct integrity_iint_cache *iint, .file = file, .filename = filename, .xattr_value = xattr_value, - .xattr_len = xattr_len }; + .xattr_len = xattr_len, + .modsig = modsig }; int violation = 0; if (iint->measured_pcrs & (0x1 << pcr)) diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 7c8d92cf2755..c87645c2c4c0 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -323,7 +323,7 @@ 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, modsig, pcr, template_desc); if (rc == 0 && (action & IMA_APPRAISE_SUBMASK)) { inode_lock(inode); diff --git a/security/integrity/ima/ima_modsig.c b/security/integrity/ima/ima_modsig.c index 257387ce3b70..c412e31d1714 100644 --- a/security/integrity/ima/ima_modsig.c +++ b/security/integrity/ima/ima_modsig.c @@ -138,6 +138,25 @@ int ima_modsig_verify(struct key *keyring, const struct modsig *modsig) VERIFYING_MODULE_SIGNATURE, NULL, NULL); } +int ima_get_modsig_digest(const struct modsig *modsig, enum hash_algo *algo, + const u8 **digest, u32 *digest_size) +{ + *algo = modsig->hash_algo; + *digest = modsig->digest; + *digest_size = modsig->digest_size; + + return 0; +} + +int ima_get_raw_modsig(const struct modsig *modsig, const void **data, + u32 *data_len) +{ + *data = &modsig->raw_pkcs7; + *data_len = modsig->raw_pkcs7_len; + + return 0; +} + void ima_free_modsig(struct modsig *modsig) { if (!modsig) diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 873dd7edaa78..4badc4fcda98 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -6,6 +6,9 @@ * ima_policy.c * - initialize default measure policy rules */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include #include #include @@ -845,6 +848,38 @@ static void ima_log_string(struct audit_buffer *ab, char *key, char *value) ima_log_string_op(ab, key, value, NULL); } +/* + * Validating the appended signature included in the measurement list requires + * the file hash calculated without the appended signature (i.e., the 'd-modsig' + * field). Therefore, notify the user if they have the 'modsig' field but not + * the 'd-modsig' field in the template. + */ +static void check_template_modsig(const struct ima_template_desc *template) +{ +#define MSG "template with 'modsig' field also needs 'd-modsig' field\n" + bool has_modsig, has_dmodsig; + static bool checked; + int i; + + /* We only need to notify the user once. */ + if (checked) + return; + + has_modsig = has_dmodsig = false; + for (i = 0; i < template->num_fields; i++) { + if (!strcmp(template->fields[i]->field_id, "modsig")) + has_modsig = true; + else if (!strcmp(template->fields[i]->field_id, "d-modsig")) + has_dmodsig = true; + } + + if (has_modsig && !has_dmodsig) + pr_notice(MSG); + + checked = true; +#undef MSG +} + static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) { struct audit_buffer *ab; @@ -1187,6 +1222,12 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) else if (entry->action == APPRAISE) temp_ima_appraise |= ima_appraise_flag(entry->func); + if (!result && entry->flags & IMA_MODSIG_ALLOWED) { + template_desc = entry->template ? entry->template : + ima_template_desc_current(); + check_template_modsig(template_desc); + } + audit_log_format(ab, "res=%d", !result); audit_log_end(ab); return result; diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c index cb349d7b2601..88d494ca6248 100644 --- a/security/integrity/ima/ima_template.c +++ b/security/integrity/ima/ima_template.c @@ -23,6 +23,7 @@ static struct ima_template_desc builtin_templates[] = { {.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 = "ima-modsig", .fmt = "d-ng|n-ng|sig|d-modsig|modsig"}, {.name = "", .fmt = ""}, /* placeholder for a custom format */ }; @@ -42,6 +43,10 @@ static const struct ima_template_field supported_fields[] = { .field_show = ima_show_template_sig}, {.field_id = "buf", .field_init = ima_eventbuf_init, .field_show = ima_show_template_buf}, + {.field_id = "d-modsig", .field_init = ima_eventdigest_modsig_init, + .field_show = ima_show_template_digest_ng}, + {.field_id = "modsig", .field_init = ima_eventmodsig_init, + .field_show = ima_show_template_sig}, }; /* @@ -49,7 +54,7 @@ static const struct ima_template_field supported_fields[] = { * 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") +#define MAX_TEMPLATE_NAME_LEN sizeof("d-ng|n-ng|sig|buf|d-modisg|modsig") static struct ima_template_desc *ima_template; diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c index 2fb9a10bc6b7..32ae05d88257 100644 --- a/security/integrity/ima/ima_template_lib.c +++ b/security/integrity/ima/ima_template_lib.c @@ -225,7 +225,8 @@ int ima_parse_buf(void *bufstartp, void *bufendp, void **bufcurp, return 0; } -static int ima_eventdigest_init_common(u8 *digest, u32 digestsize, u8 hash_algo, +static int ima_eventdigest_init_common(const u8 *digest, u32 digestsize, + u8 hash_algo, struct ima_field_data *field_data) { /* @@ -328,6 +329,41 @@ out: hash_algo, field_data); } +/* + * This function writes the digest of the file which is expected to match the + * digest contained in the file's appended signature. + */ +int ima_eventdigest_modsig_init(struct ima_event_data *event_data, + struct ima_field_data *field_data) +{ + enum hash_algo hash_algo; + const u8 *cur_digest; + u32 cur_digestsize; + + if (!event_data->modsig) + return 0; + + if (event_data->violation) { + /* Recording a violation. */ + hash_algo = HASH_ALGO_SHA1; + cur_digest = NULL; + cur_digestsize = 0; + } else { + int rc; + + rc = ima_get_modsig_digest(event_data->modsig, &hash_algo, + &cur_digest, &cur_digestsize); + if (rc) + return rc; + else if (hash_algo == HASH_ALGO__LAST || cur_digestsize == 0) + /* There was some error collecting the digest. */ + return -EINVAL; + } + + return ima_eventdigest_init_common(cur_digest, cur_digestsize, + hash_algo, field_data); +} + static int ima_eventname_init_common(struct ima_event_data *event_data, struct ima_field_data *field_data, bool size_limit) @@ -406,3 +442,29 @@ int ima_eventbuf_init(struct ima_event_data *event_data, event_data->buf_len, DATA_FMT_HEX, field_data); } + +/* + * ima_eventmodsig_init - include the appended file signature as part of the + * template data + */ +int ima_eventmodsig_init(struct ima_event_data *event_data, + struct ima_field_data *field_data) +{ + const void *data; + u32 data_len; + int rc; + + if (!event_data->modsig) + return 0; + + /* + * modsig is a runtime structure containing pointers. Get its raw data + * instead. + */ + rc = ima_get_raw_modsig(event_data->modsig, &data, &data_len); + if (rc) + return rc; + + return ima_write_template_field_data(data, data_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 652aa5de81ef..9a88c79a7a61 100644 --- a/security/integrity/ima/ima_template_lib.h +++ b/security/integrity/ima/ima_template_lib.h @@ -36,10 +36,14 @@ int ima_eventname_init(struct ima_event_data *event_data, struct ima_field_data *field_data); int ima_eventdigest_ng_init(struct ima_event_data *event_data, struct ima_field_data *field_data); +int ima_eventdigest_modsig_init(struct ima_event_data *event_data, + struct ima_field_data *field_data); 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); +int ima_eventmodsig_init(struct ima_event_data *event_data, + struct ima_field_data *field_data); #endif /* __LINUX_IMA_TEMPLATE_LIB_H */ -- cgit From e5092255bb3967bcc473dc86492dbbd5f7714023 Mon Sep 17 00:00:00 2001 From: Thiago Jung Bauermann Date: Thu, 27 Jun 2019 23:19:33 -0300 Subject: ima: Store the measurement again when appraising a modsig If the IMA template contains the "modsig" or "d-modsig" field, then the modsig should be added to the measurement list when the file is appraised. And that is what normally happens, but if a measurement rule caused a file containing a modsig to be measured before a different rule causes it to be appraised, the resulting measurement entry will not contain the modsig because it is only fetched during appraisal. When the appraisal rule triggers, it won't store a new measurement containing the modsig because the file was already measured. We need to detect that situation and store an additional measurement with the modsig. This is done by adding an IMA_MEASURE action flag if we read a modsig and the IMA template contains a modsig field. Suggested-by: Mimi Zohar Signed-off-by: Thiago Jung Bauermann Signed-off-by: Mimi Zohar --- security/integrity/ima/ima.h | 1 + security/integrity/ima/ima_api.c | 19 +++++++++++++++---- security/integrity/ima/ima_main.c | 15 ++++++++++++--- security/integrity/ima/ima_template.c | 19 +++++++++++++++++++ 4 files changed, 47 insertions(+), 7 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index cfc8b0887776..19769bf5f6ab 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -150,6 +150,7 @@ int template_desc_init_fields(const char *template_fmt, int *num_fields); struct ima_template_desc *ima_template_desc_current(void); struct ima_template_desc *lookup_template_desc(const char *name); +bool ima_template_has_modsig(const struct ima_template_desc *ima_template); 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); diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index ca930e2ebc2c..65224474675b 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -219,6 +219,14 @@ int ima_collect_measurement(struct integrity_iint_cache *iint, char digest[IMA_MAX_DIGEST_SIZE]; } hash; + /* + * Always collect the modsig, because IMA might have already collected + * the file digest without collecting the modsig in a previous + * measurement rule. + */ + if (modsig) + ima_collect_modsig(modsig, buf, size); + if (iint->flags & IMA_COLLECTED) goto out; @@ -252,9 +260,6 @@ int ima_collect_measurement(struct integrity_iint_cache *iint, memcpy(iint->ima_hash, &hash, length); iint->version = i_version; - if (modsig) - ima_collect_modsig(modsig, buf, size); - /* Possibly temporary failure due to type of read (eg. O_DIRECT) */ if (!result) iint->flags |= IMA_COLLECTED; @@ -304,7 +309,13 @@ void ima_store_measurement(struct integrity_iint_cache *iint, .modsig = modsig }; int violation = 0; - if (iint->measured_pcrs & (0x1 << pcr)) + /* + * We still need to store the measurement in the case of MODSIG because + * we only have its contents to put in the list at the time of + * appraisal, but a file measurement from earlier might already exist in + * the measurement list. + */ + if (iint->measured_pcrs & (0x1 << pcr) && !modsig) return; result = ima_alloc_init_template(&event_data, &entry, template_desc); diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index c87645c2c4c0..79c01516211b 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -307,9 +307,18 @@ static int process_measurement(struct file *file, const struct cred *cred, /* read 'security.ima' */ xattr_len = ima_read_xattr(file_dentry(file), &xattr_value); - /* Read the appended modsig if allowed by the policy. */ - if (iint->flags & IMA_MODSIG_ALLOWED) - ima_read_modsig(func, buf, size, &modsig); + /* + * Read the appended modsig if allowed by the policy, and allow + * an additional measurement list entry, if needed, based on the + * template format and whether the file was already measured. + */ + if (iint->flags & IMA_MODSIG_ALLOWED) { + rc = ima_read_modsig(func, buf, size, &modsig); + + if (!rc && ima_template_has_modsig(template_desc) && + iint->flags & IMA_MEASURED) + action |= IMA_MEASURE; + } } hash_algo = ima_get_hash_algo(xattr_value, xattr_len); diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c index 88d494ca6248..f5b950e0a955 100644 --- a/security/integrity/ima/ima_template.c +++ b/security/integrity/ima/ima_template.c @@ -58,6 +58,25 @@ static const struct ima_template_field supported_fields[] = { static struct ima_template_desc *ima_template; +/** + * ima_template_has_modsig - Check whether template has modsig-related fields. + * @ima_template: IMA template to check. + * + * Tells whether the given template has fields referencing a file's appended + * signature. + */ +bool ima_template_has_modsig(const struct ima_template_desc *ima_template) +{ + int i; + + for (i = 0; i < ima_template->num_fields; i++) + if (!strcmp(ima_template->fields[i]->field_id, "modsig") || + !strcmp(ima_template->fields[i]->field_id, "d-modsig")) + return true; + + return false; +} + static int __init ima_template_setup(char *str) { struct ima_template_desc *template_desc; -- cgit From f5e1040196dbfe14c77ce3dfe3b7b08d2d961e88 Mon Sep 17 00:00:00 2001 From: Sascha Hauer Date: Tue, 2 Jul 2019 10:00:40 +0200 Subject: ima: always return negative code for error integrity_kernel_read() returns the number of bytes read. If this is a short read then this positive value is returned from ima_calc_file_hash_atfm(). Currently this is only indirectly called from ima_calc_file_hash() and this function only tests for the return value being zero or nonzero and also doesn't forward the return value. Nevertheless there's no point in returning a positive value as an error, so translate a short read into -EINVAL. Signed-off-by: Sascha Hauer Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_crypto.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'security') diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index d4c7b8e1b083..7532b062be59 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -268,8 +268,11 @@ static int ima_calc_file_hash_atfm(struct file *file, rbuf_len = min_t(loff_t, i_size - offset, rbuf_size[active]); rc = integrity_kernel_read(file, offset, rbuf[active], rbuf_len); - if (rc != rbuf_len) + if (rc != rbuf_len) { + if (rc >= 0) + rc = -EINVAL; goto out3; + } if (rbuf[1] && offset) { /* Using two buffers, and it is not the first -- cgit From 4ece3125f21b1d42b84896c5646dbf0e878464e1 Mon Sep 17 00:00:00 2001 From: Sascha Hauer Date: Tue, 2 Jul 2019 10:00:41 +0200 Subject: ima: fix freeing ongoing ahash_request integrity_kernel_read() can fail in which case we forward to call ahash_request_free() on a currently running request. We have to wait for its completion before we can free the request. This was observed by interrupting a "find / -type f -xdev -print0 | xargs -0 cat 1>/dev/null" with ctrl-c on an IMA enabled filesystem. Signed-off-by: Sascha Hauer Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_crypto.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'security') diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index 7532b062be59..73044fc6a952 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -271,6 +271,11 @@ static int ima_calc_file_hash_atfm(struct file *file, if (rc != rbuf_len) { if (rc >= 0) rc = -EINVAL; + /* + * Forward current rc, do not overwrite with return value + * from ahash_wait() + */ + ahash_wait(ahash_rc, &wait); goto out3; } -- cgit From 556d971bdae643de4cd7e2976e14f70ca2a3061d Mon Sep 17 00:00:00 2001 From: Thiago Jung Bauermann Date: Wed, 7 Aug 2019 21:43:18 -0300 Subject: ima: Fix use after free in ima_read_modsig() If we can't parse the PKCS7 in the appended modsig, we will free the modsig structure and then access one of its members to determine the error value. Fixes: 39b07096364a ("ima: Implement support for module-style appended signatures") Reported-by: kbuild test robot Reported-by: Julia Lawall Reported-by: Dan Carpenter Signed-off-by: Thiago Jung Bauermann Reviewed-by: Gustavo A. R. Silva Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_modsig.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'security') diff --git a/security/integrity/ima/ima_modsig.c b/security/integrity/ima/ima_modsig.c index c412e31d1714..d106885cc495 100644 --- a/security/integrity/ima/ima_modsig.c +++ b/security/integrity/ima/ima_modsig.c @@ -91,8 +91,9 @@ int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len, hdr->pkcs7_msg = pkcs7_parse_message(buf + buf_len, sig_len); if (IS_ERR(hdr->pkcs7_msg)) { + rc = PTR_ERR(hdr->pkcs7_msg); kfree(hdr); - return PTR_ERR(hdr->pkcs7_msg); + return rc; } memcpy(hdr->raw_pkcs7, buf + buf_len, sig_len); -- cgit From fa5b57175364431245b006c2afcbf94dc2b15400 Mon Sep 17 00:00:00 2001 From: "Gustavo A. R. Silva" Date: Wed, 29 May 2019 11:53:43 -0500 Subject: ima: use struct_size() in kzalloc() One of the more common cases of allocation size calculations is finding the size of a structure that has a zero-sized array at the end, along with memory for some number of elements for that array. For example: struct foo { int stuff; struct boo entry[]; }; instance = kzalloc(sizeof(struct foo) + count * sizeof(struct boo), GFP_KERNEL); Instead of leaving these open-coded and prone to type mistakes, we can now use the new struct_size() helper: instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL); This code was detected with the help of Coccinelle. Signed-off-by: Gustavo A. R. Silva Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_template.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c index f5b950e0a955..6aa6408603e3 100644 --- a/security/integrity/ima/ima_template.c +++ b/security/integrity/ima/ima_template.c @@ -306,9 +306,8 @@ static int ima_restore_template_data(struct ima_template_desc *template_desc, int ret = 0; int i; - *entry = kzalloc(sizeof(**entry) + - template_desc->num_fields * sizeof(struct ima_field_data), - GFP_NOFS); + *entry = kzalloc(struct_size(*entry, template_data, + template_desc->num_fields), GFP_NOFS); if (!*entry) return -ENOMEM; -- cgit From 2a7f0e53daf29ca6dc9fbe2a27158f13474ec1b5 Mon Sep 17 00:00:00 2001 From: "Gustavo A. R. Silva" Date: Thu, 29 Aug 2019 12:29:16 -0500 Subject: ima: ima_api: Use struct_size() in kzalloc() One of the more common cases of allocation size calculations is finding the size of a structure that has a zero-sized array at the end, along with memory for some number of elements for that array. For example: struct ima_template_entry { ... struct ima_field_data template_data[0]; /* template related data */ }; instance = kzalloc(sizeof(struct ima_template_entry) + count * sizeof(struct ima_field_data), GFP_NOFS); Instead of leaving these open-coded and prone to type mistakes, we can now use the new struct_size() helper: instance = kzalloc(struct_size(instance, entry, count), GFP_NOFS); This code was detected with the help of Coccinelle. Signed-off-by: Gustavo A. R. Silva Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_api.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index 65224474675b..610759fe63b8 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -45,8 +45,8 @@ int ima_alloc_init_template(struct ima_event_data *event_data, else template_desc = ima_template_desc_current(); - *entry = kzalloc(sizeof(**entry) + template_desc->num_fields * - sizeof(struct ima_field_data), GFP_NOFS); + *entry = kzalloc(struct_size(*entry, template_data, + template_desc->num_fields), GFP_NOFS); if (!*entry) return -ENOMEM; -- cgit