While users can restrict the accepted hash algorithms for the security.ima xattr file signature when appraising said file, users cannot restrict the algorithms that can be set on that attribute: any algorithm built in the kernel is accepted on a write. Define a new value for the ima policy option 'func' that restricts globally the hash algorithms accepted when writing the security.ima xattr. When a policy contains a rule of the form appraise func=SETXATTR_CHECK appraise_hash=sha256,sha384,sha512 only values corresponding to one of these three digest algorithms will be accepted for writing the security.ima xattr. Attempting to write the attribute using another algorithm (or "free-form" data) will be denied with an audit log message. In the absence of such a policy rule, the default is still to only accept hash algorithms built in the kernel (with all the limitations that entails). Signed-off-by: Simon Thoby <simon.thoby@xxxxxxxxxx> --- Documentation/ABI/testing/ima_policy | 9 ++++- security/integrity/ima/ima.h | 4 ++ security/integrity/ima/ima_appraise.c | 31 ++++++++++++--- security/integrity/ima/ima_policy.c | 57 +++++++++++++++++++++++++++ 4 files changed, 94 insertions(+), 7 deletions(-) diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy index aeb622698047..537be0e1720e 100644 --- a/Documentation/ABI/testing/ima_policy +++ b/Documentation/ABI/testing/ima_policy @@ -30,9 +30,10 @@ Description: [appraise_flag=] [appraise_hash=] [keyrings=] base: func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK] - [FIRMWARE_CHECK] + [FIRMWARE_CHECK] [KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK] [KEXEC_CMDLINE] [KEY_CHECK] [CRITICAL_DATA] + [SETXATTR_CHECK] mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND] [[^]MAY_EXEC] fsmagic:= hex value @@ -138,3 +139,9 @@ Description: keys added to .builtin_trusted_keys or .ima keyring: measure func=KEY_CHECK keyrings=.builtin_trusted_keys|.ima + + Example of the special SETXATTR_CHECK appraise rule, that + restricts the hash algorithms allowed when writing to the + security.ima xattr of a file: + + appraise func=SETXATTR_CHECK appraise_hash=sha256,sha384,sha512 diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 7ef1b214d358..aeb3bf30c0f9 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -46,6 +46,9 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 }; /* current content of the policy */ extern int ima_policy_flag; +/* bitset of digests algorithms allowed in the setxattr hook */ +extern atomic_t ima_setxattr_allowed_hash_algorithms; + /* set during initialization */ extern int ima_hash_algo __ro_after_init; extern int ima_sha1_idx __ro_after_init; @@ -198,6 +201,7 @@ static inline unsigned int ima_hash_key(u8 *digest) hook(KEXEC_CMDLINE, kexec_cmdline) \ hook(KEY_CHECK, key) \ hook(CRITICAL_DATA, critical_data) \ + hook(SETXATTR_CHECK, setxattr_check) \ hook(MAX_CHECK, none) #define __ima_hook_enumify(ENUM, str) ENUM, diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 12d406b5ab35..c4a43c84da5f 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -596,13 +596,33 @@ static int validate_hash_algo(struct dentry *dentry, { char *path = NULL, *pathbuf = NULL; enum hash_algo xattr_hash_algo; + const char *errmsg = "unavailable-hash-algorithm"; + unsigned int allowed_hashes; xattr_hash_algo = ima_get_hash_algo((struct evm_ima_xattr_data *)xattr_value, xattr_value_len); - if (likely(xattr_hash_algo == ima_hash_algo || - crypto_has_alg(hash_algo_name[xattr_hash_algo], 0, 0))) - return 0; + allowed_hashes = atomic_read(&ima_setxattr_allowed_hash_algorithms); + + if (allowed_hashes) { + /* success if the algorithm is whitelisted in the ima policy */ + if (allowed_hashes & (1U << xattr_hash_algo)) + return 0; + + /* + * We use a different audit message when the hash algorithm + * is denied by a policy rule, instead of not being built + * in the kernel image + */ + errmsg = "denied-hash-algorithm"; + } else { + if (likely(xattr_hash_algo == ima_hash_algo)) + return 0; + + /* allow any xattr using an algorithm built in the kernel */ + if (crypto_has_alg(hash_algo_name[xattr_hash_algo], 0, 0)) + return 0; + } pathbuf = kmalloc(PATH_MAX, GFP_KERNEL); if (pathbuf) { @@ -610,11 +630,10 @@ static int validate_hash_algo(struct dentry *dentry, /* * disallow xattr writes with algorithms disabled in the - * kernel configuration + * kernel configuration or denied by policy */ integrity_audit_msg(AUDIT_INTEGRITY_DATA, d_inode(dentry), - path, "collect_data", - "unavailable-hash-algorithm", + path, "collect_data", errmsg, -EACCES, 0); kfree(pathbuf); diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index f944c3e7f340..4e2410b826e2 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -53,6 +53,8 @@ int ima_policy_flag; static int temp_ima_appraise; static int build_ima_appraise __ro_after_init; +atomic_t ima_setxattr_allowed_hash_algorithms; + #define MAX_LSM_RULES 6 enum lsm_rule_types { LSM_OBJ_USER, LSM_OBJ_ROLE, LSM_OBJ_TYPE, LSM_SUBJ_USER, LSM_SUBJ_ROLE, LSM_SUBJ_TYPE @@ -903,6 +905,8 @@ void __init ima_init_policy(void) ARRAY_SIZE(critical_data_rules), IMA_DEFAULT_POLICY); + atomic_xchg(&ima_setxattr_allowed_hash_algorithms, 0); + ima_update_policy_flag(); } @@ -914,6 +918,42 @@ int ima_check_policy(void) return 0; } +/** update_allowed_hash_algorithms - update the hash algorithms allowed + * for setxattr writes + * + * Update the atomic variable holding the set of allowed hash algorithms + * that can be used to update the security.ima xattr of a file. + * + * Context: called when updating the IMA policy. + * + * SETXATTR_CHECK rules do not implement a full policy check because of + * the performance impact performing rules checking on setxattr() would + * have. The consequence is that only one SETXATTR_CHECK can be active at + * a time. + */ +static void update_allowed_hash_algorithms(void) +{ + struct ima_rule_entry *entry; + + /* + * We scan in reverse order because only the last entry with the + * 'func=SETXATTR_CHECK' apply: this allows runtime upgrades of the + * digest algorithm policy, unlike the other IMA rules that are + * usually append-only. Old rules will still be present in the + * ruleset, but inactive. + */ + rcu_read_lock(); + list_for_each_entry_reverse(entry, ima_rules, list) { + if (entry->func != SETXATTR_CHECK) + continue; + + atomic_xchg(&ima_setxattr_allowed_hash_algorithms, + entry->allowed_hashes); + break; + } + rcu_read_unlock(); +} + /** * ima_update_policy - update default_rules with new measure rules * @@ -931,6 +971,7 @@ void ima_update_policy(void) list_splice_tail_init_rcu(&ima_temp_rules, policy, synchronize_rcu); + if (ima_rules != policy) { ima_policy_flag = 0; ima_rules = policy; @@ -944,6 +985,7 @@ void ima_update_policy(void) kfree(arch_policy_entry); } ima_update_policy_flag(); + update_allowed_hash_algorithms(); /* Custom IMA policy has been loaded */ ima_process_queued_keys(); @@ -1176,6 +1218,19 @@ static bool ima_validate_rule(struct ima_rule_entry *entry) if (ima_rule_contains_lsm_cond(entry)) return false; + break; + case SETXATTR_CHECK: + /* any action other than APPRAISE is unsupported */ + if (entry->action != APPRAISE) + return false; + + /* + * full policies are not supported, they would have too + * much of a performance impact + */ + if (entry->flags & ~(IMA_FUNC | IMA_VALIDATE_HASH)) + return false; + break; default: return false; @@ -1332,6 +1387,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) entry->func = KEY_CHECK; else if (strcmp(args[0].from, "CRITICAL_DATA") == 0) entry->func = CRITICAL_DATA; + else if (strcmp(args[0].from, "SETXATTR_CHECK") == 0) + entry->func = SETXATTR_CHECK; else result = -EINVAL; if (!result) -- 2.31.1