Sorry it took me some time before I could write a patch (in addition it's my first kernel contribution so I'm still learning how things should be done, like fighting Outlook on the web, giving up on it and moving to Mozilla Thunderbird). This patch is a follow up on the discussion at https://lore.kernel.org/linux-integrity/10dde047d76b447f32ca91356599be679b8a76e5.camel@xxxxxxxxxxxxx/t/#m0f8127c6982ef94aa42f5cc13ea83b9f9000917e. This patch was built on top of linux-5.14.0-rc1, and I successfully tested the patch in the following configurations: - disabled: no visible impact (as expected) - with ima_allowed_hashes=sha256,sha512: - alone: blocks both execution and xattr writes (tested with `emvctl ima_hash -a md5 <my binary>`) - with ima_appraise=log: audit logs but still permits access - with ima_appraise=fix: audit logs but still permits access, however it doesn't fix the hash (so maybe I should do something about it, because right now it's basically the same as ima_appraise=log w.r.t. hash algorithms ?) - with ima_accept_any_hash: permits access, no warnings whatsoever Do you want ideas of other configurations that I could test? I would also like to point out that I'm more than open to suggestions for changing the names of the parameters (`ima_allowed_hashes` and `ima_accept_any_hash`) and the "cause" in the audit log (currently forbidden-hash-algorithm"), because as you know, "naming things is hard". IMA: add an option to restrict the accepted hash algorithms Adds two command-line parameters to limit the hash algorithms allowed for the security.ima xattr. This gives users the ability to ensure their systems will not accept weak hashes, and potentially increase users trust in their IMA configuration, because they can ensure only strong collision-resistant hashes are employed and files generated otherwise will not be accepted. The main point is to safeguard users from mislabelling their files when using userland utilities to update their files, e.g. evmctl (`evmctl ima_hash` defaults to sha1). Another possible target would be people that have deployed IMA years ago, possibly using algorithms that was then deemed sufficiently collision-resistant, but that proved to be weak with the passage of time (note that this could also happen in the future with algorithms considered safe today). This patch also provides a migration path for users. The first parameter supplies a list of allowed algorithms to the kernel, restricting appraisal to files hashed with "strong" hash algorithms (by default IMA will keep accepting any hash algorithm, as enabling such a feature by default would both be backward-incompatible and very probably break real systems). The second parameter is an escape hatch that adds a weaker form of backward-compatibility: when activated, IMA apparaisal will keep working on files hashed with an algorithm not present in the list, but updates to these files or new writes to the security.ima xattr will enforce the selected hash algorithms. This may be useful to perform an online migration from one algorithm to another. Signed-off-by: Simon Thoby <simon.thoby@xxxxxxxxxx> --- .../admin-guide/kernel-parameters.txt | 42 +++++++++ security/integrity/ima/ima.h | 6 +- security/integrity/ima/ima_appraise.c | 22 +++++ security/integrity/ima/ima_main.c | 88 ++++++++++++++++++- 4 files changed, 156 insertions(+), 2 deletions(-) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index bdb22006f713..af7567b3a44e 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -1817,6 +1817,48 @@ The list of supported hash algorithms is defined in crypto/hash_info.h. + ima_allowed_hashes= [IMA] + Format: comma-separated list of { md5 | sha1 | rmd160 + | sha256 | sha384 | sha512 | ... } + + When using IMA in hashing mode, this attribute specifies the + list of hash algorithms that are allowed in the security.ima + xattr. Files hashed with an algorithm not present in that + list will not pass appraisal, and writes of invalid xattrs + are disallowed too. + + Use this option to ensure only strong and collision-resistant + hashes are used and accepted by the kernel, instead of the + default that will happily appraise files hashed with any of + the numerous hash algorithms the kenrel supports. + + See also "ima_accept_any_hash" to only restrict writes. + The list of supported hash algorithms is defined + in crypto/hash_info.h. + + ima_accept_any_hash [IMA] + + Accept for appraisal files with an IMA hash not allowed + in "ima_allowed_hashes", to allow iterative migrations + of your files to stronger algorithms. + + When using IMA in hashing mode with "ima_allowed_hashes" + selected, the kernel restricts the hash algorithms + considered valid, and prevent both writes of files with + invalid xattrs and execution/read/... (depending on + the IMA policy) of such files. + + This option changes that behavior to be backward compatible: + the kernel accept for appraisal files hashed with algorithms + not present in the list, but still enforces the algorithms + on write. + This may be useful for migrating to a new hash algorithm: + first changing ima_allowed_hashed and adding the ima_accept_any_hash + parameter to keep accepting the old files, rebooting and + relabelling all the files on the system, and finally + rebooting again without the ima_accept_any_hash parameter + to enforce the new policy. + ima_policy= [IMA] The builtin policies to load during IMA setup. Format: "tcb | appraise_tcb | secure_boot | diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index f0e448ed1f9f..ef173fcb7112 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -21,6 +21,7 @@ #include <linux/tpm.h> #include <linux/audit.h> #include <crypto/hash_info.h> +#include <stdbool.h> #include "../integrity.h" @@ -47,7 +48,9 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 }; extern int ima_policy_flag; /* set during initialization */ -extern int ima_hash_algo; +extern int ima_hash_algo __ro_after_init; +extern unsigned int ima_allowed_hashes __ro_after_init; +extern bool ima_accept_any_hash __ro_after_init; extern int ima_sha1_idx __ro_after_init; extern int ima_hash_algo_idx __ro_after_init; extern int ima_extra_slots __ro_after_init; @@ -164,6 +167,7 @@ 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); +int ima_validate_hash_algorithm(enum hash_algo hash_alg, bool xattr_update); /* * used to protect h_table and sha_table diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index ef9dcfce45d4..ecbfd644dca8 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -592,6 +592,28 @@ int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name, digsig = (xvalue->type == EVM_XATTR_PORTABLE_DIGSIG); } if (result == 1 || evm_revalidate_status(xattr_name)) { + enum hash_algo hash_alg = + ima_get_hash_algo((struct evm_ima_xattr_data *)xattr_value, + xattr_value_len); + + /* Ensure the user-supplied xattr value uses a whitelisted hash algorithm */ + int rc = ima_validate_hash_algorithm(hash_alg, true); + if (rc != 0) { + char *path = NULL; + char *pathbuf = kmalloc(PATH_MAX, GFP_KERNEL); + /* no memory available ? no file path for you */ + if (pathbuf) + path = dentry_path(dentry, pathbuf, PATH_MAX); + + integrity_audit_msg(AUDIT_INTEGRITY_DATA, d_inode(dentry), + path, "collect_data", "forbidden-hash-algorithm", rc, 0); + + if (pathbuf) + kfree(pathbuf); + + return rc; + } + ima_reset_appraise_flags(d_backing_inode(dentry), digsig); if (result == 1) result = 0; diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 287b90509006..4206f2459bc1 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -38,6 +38,17 @@ int ima_appraise; int ima_hash_algo = HASH_ALGO_SHA1; static int hash_setup_done; +unsigned int ima_allowed_hashes; +static_assert( + 8 * sizeof(ima_allowed_hashes) >= HASH_ALGO__LAST, + "The bitfield ima_allowed_hashes is too small to contain all the supported hash algorithms"); +static char *ima_allowed_hashes_cmdline __initdata; +core_param(ima_allowed_hashes, ima_allowed_hashes_cmdline, charp, 0); + +bool ima_accept_any_hash; +core_param(ima_accept_any_hash, ima_accept_any_hash, bool, 0); + + static struct notifier_block ima_lsm_policy_notifier = { .notifier_call = ima_lsm_policy_change, }; @@ -76,6 +87,46 @@ static int __init hash_setup(char *str) } __setup("ima_hash=", hash_setup); +static void __init hash_restrictions_setup(void) +{ + char *str_copy, *iter; + + if (!ima_allowed_hashes_cmdline) + return; + + pr_info("restricting allowed hash algorithms to %s", ima_allowed_hashes_cmdline); + + /* copy the string to perform strsep() on it */ + str_copy = kmalloc(strlen(ima_allowed_hashes_cmdline) + 1, GFP_KERNEL); + if (!str_copy) { + pr_err("couldn't allocate memory for parsing the list of allowed hashes"); + return; + } + + memcpy(str_copy, ima_allowed_hashes_cmdline, strlen(ima_allowed_hashes_cmdline) + 1); + + iter = str_copy; + do { + int idx; + char *token = strsep(&iter, ","); + + if (token == NULL) + break; + + idx = match_string(hash_algo_name, HASH_ALGO__LAST, token); + if (idx < 0) { + pr_err("invalid hash algorithm \"%s\", ignoring", + token); + continue; + } + + /* Add the hash algorithm to the 'allowed' bitfield */ + ima_allowed_hashes |= (1U << idx); + } while (iter != NULL); + + kfree(str_copy); +} + /* 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, @@ -172,6 +223,29 @@ static void ima_check_last_writer(struct integrity_iint_cache *iint, mutex_unlock(&iint->mutex); } +/** + * ima_validate_hash_algorithm + * @hash_alg: the requested hash algorithm + * @attr_update: true if the user is trying to write the security.ima + * xattr with 'hash_alg' format. False means we are performing appraisal + * against a file that possesses a xattr hashed with 'hash_alg'. + * + * Ensure that the digest was generated using an allowed algorithm + */ +int ima_validate_hash_algorithm(enum hash_algo hash_alg, bool xattr_update) +{ + /* when reading/executing a file in "permissive" mode, accept any hash */ + if (ima_accept_any_hash && !xattr_update) + return 0; + + if (ima_allowed_hashes != 0 && + unlikely(!(ima_allowed_hashes & (1U << hash_alg)))) { + return -EACCES; + } + + return 0; +} + /** * ima_file_free - called on __fput() * @file: pointer to file structure being freed @@ -327,11 +401,22 @@ static int process_measurement(struct file *file, const struct cred *cred, hash_algo = ima_get_hash_algo(xattr_value, xattr_len); + rc = ima_validate_hash_algorithm(hash_algo, false); + if (rc != 0) { + if (!pathbuf) /* ima_rdwr_violation possibly pre-fetched */ + pathname = ima_d_path(&file->f_path, &pathbuf, filename); + + integrity_audit_msg(AUDIT_INTEGRITY_DATA, file_inode(file), + pathname, "collect_data", "forbidden-hash-algorithm", rc, 0); + + goto out_locked; + } + rc = ima_collect_measurement(iint, file, buf, size, hash_algo, modsig); if (rc != 0 && rc != -EBADF && rc != -EINVAL) goto out_locked; - if (!pathbuf) /* ima_rdwr_violation possibly pre-fetched */ + if (!pathbuf) /* ima_rdwr_violation or hash_algs checks above possibly pre-fetched */ pathname = ima_d_path(&file->f_path, &pathbuf, filename); if (action & IMA_MEASURE) @@ -993,6 +1078,7 @@ static int __init init_ima(void) ima_appraise_parse_cmdline(); ima_init_template_list(); hash_setup(CONFIG_IMA_DEFAULT_HASH); + hash_restrictions_setup(); error = ima_init(); if (error && strcmp(hash_algo_name[ima_hash_algo], -- 2.31.1
