On Wed, 2024-02-14 at 15:35 +0100, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@xxxxxxxxxx>
>
> Add the 'digest_cache=' policy keyword, to enable the usage of digest
> caches for specific IMA actions and purposes.
>
> At the moment, it accepts only 'content' as value, as digest caches can be
> only used only for measurement and appraisal of file content. In the
> future, it might be possible to use them for file metadata too.
At least from this patch, it is unclear why 'digest_cache' requires an option.
The usage - measure, appraise - is based on 'action'. From an IMA perspective,
does the file content make a difference? And if it did, then file 'data' would
parallel file 'metadata'.
Without having to pass around "digest_cache_mask" would simplify this patch.
Mimi
> The 'digest_cache=' keyword can be specified for the subset of IMA hooks
> listed in ima_digest_cache_func_allowed().
>
> POLICY_CHECK has been excluded for measurement, because policy changes must
> be visible in the IMA measurement list. For appraisal, instead, it might be
> useful to load custom policies in the initial ram disk (no security.ima
> xattr).
>
> Add the digest_cache_mask member to the ima_rule_entry structure, and set
> the flag IMA_DIGEST_CACHE_MEASURE_CONTENT if 'digest_cache=content' was
> specified for a measure rule, IMA_DIGEST_CACHE_APPRAISE_CONTENT for an
> appraise rule.
>
> Propagate the mask down to ima_match_policy() and ima_get_action(), so that
> process_measurement() can make the final decision on whether or not digest
> caches should be used to measure/appraise the file being evaluated.
>
> Since using digest caches changes the meaning of the IMA measurement list,
> which will include only digest lists and unknown files, enforce specifying
> 'pcr=' with a non-standard value, when 'digest_cache=content' is specified
> in a measure rule.
>
> This removes the ambiguity on the meaning of the IMA measurement list.
>
> Signed-off-by: Roberto Sassu <roberto.sassu@xxxxxxxxxx>
> ---
> Documentation/ABI/testing/ima_policy | 5 +-
> security/integrity/ima/ima.h | 10 +++-
> security/integrity/ima/ima_api.c | 6 ++-
> security/integrity/ima/ima_appraise.c | 2 +-
> security/integrity/ima/ima_main.c | 8 +--
> security/integrity/ima/ima_policy.c | 70 ++++++++++++++++++++++++++-
> 6 files changed, 89 insertions(+), 12 deletions(-)
>
> diff --git a/Documentation/ABI/testing/ima_policy
> b/Documentation/ABI/testing/ima_policy
> index 22237fec5532..be045fb60530 100644
> --- a/Documentation/ABI/testing/ima_policy
> +++ b/Documentation/ABI/testing/ima_policy
> @@ -29,7 +29,7 @@ Description:
> [obj_user=] [obj_role=] [obj_type=]]
> option: [digest_type=] [template=] [permit_directio]
> [appraise_type=] [appraise_flag=]
> - [appraise_algos=] [keyrings=]
> + [appraise_algos=] [keyrings=] [digest_cache=]
> base:
> func:=
> [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
> [FIRMWARE_CHECK]
> @@ -77,6 +77,9 @@ Description:
> For example, "sha256,sha512" to only accept to appraise
> files where the security.ima xattr was hashed with one
> of these two algorithms.
> + digest_cache:= [content]
> + "content" means that the digest cache is used only
> + for file content measurement and/or appraisal.
>
> default policy:
> # PROC_SUPER_MAGIC
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index c9140a57b591..deee56d99d6f 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -43,6 +43,10 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10
> };
>
> #define NR_BANKS(chip) ((chip != NULL) ? chip->nr_allocated_banks : 0)
>
> +/* Digest cache usage flags. */
> +#define IMA_DIGEST_CACHE_MEASURE_CONTENT 0x0000000000000001
> +#define IMA_DIGEST_CACHE_APPRAISE_CONTENT 0x0000000000000002
> +
> /* current content of the policy */
> extern int ima_policy_flag;
>
> @@ -367,7 +371,8 @@ int ima_get_action(struct mnt_idmap *idmap, struct inode
> *inode,
> const struct cred *cred, u32 secid, int mask,
> enum ima_hooks func, int *pcr,
> struct ima_template_desc **template_desc,
> - const char *func_data, unsigned int *allowed_algos);
> + const char *func_data, unsigned int *allowed_algos,
> + u64 *digest_cache_mask);
> int ima_must_measure(struct inode *inode, int mask, enum ima_hooks func);
> int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> void *buf, loff_t size, enum hash_algo algo,
> @@ -398,7 +403,8 @@ int ima_match_policy(struct mnt_idmap *idmap, struct inode
> *inode,
> const struct cred *cred, u32 secid, enum ima_hooks func,
> int mask, int flags, int *pcr,
> struct ima_template_desc **template_desc,
> - const char *func_data, unsigned int *allowed_algos);
> + const char *func_data, unsigned int *allowed_algos,
> + u64 *digest_cache_mask);
> void ima_init_policy(void);
> void ima_update_policy(void);
> void ima_update_policy_flags(void);
> diff --git a/security/integrity/ima/ima_api.c
> b/security/integrity/ima/ima_api.c
> index b37d043d5748..87e286ace43c 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -173,6 +173,7 @@ void ima_add_violation(struct file *file, const unsigned
> char *filename,
> * @template_desc: pointer filled in if matched measure policy sets template=
> * @func_data: func specific data, may be NULL
> * @allowed_algos: allowlist of hash algorithms for the IMA xattr
> + * @digest_cache_mask: Actions and purposes for which digest cache is allowed
> *
> * The policy is defined in terms of keypairs:
> * subj=, obj=, type=, func=, mask=, fsmagic=
> @@ -190,7 +191,8 @@ int ima_get_action(struct mnt_idmap *idmap, struct inode
> *inode,
> const struct cred *cred, u32 secid, int mask,
> enum ima_hooks func, int *pcr,
> struct ima_template_desc **template_desc,
> - const char *func_data, unsigned int *allowed_algos)
> + const char *func_data, unsigned int *allowed_algos,
> + u64 *digest_cache_mask)
> {
> int flags = IMA_MEASURE | IMA_AUDIT | IMA_APPRAISE | IMA_HASH;
>
> @@ -198,7 +200,7 @@ int ima_get_action(struct mnt_idmap *idmap, struct inode
> *inode,
>
> return ima_match_policy(idmap, inode, cred, secid, func, mask,
> flags, pcr, template_desc, func_data,
> - allowed_algos);
> + allowed_algos, digest_cache_mask);
> }
>
> static bool ima_get_verity_digest(struct ima_iint_cache *iint,
> diff --git a/security/integrity/ima/ima_appraise.c
> b/security/integrity/ima/ima_appraise.c
> index 3497741caea9..27ccc9a2c09f 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -81,7 +81,7 @@ int ima_must_appraise(struct mnt_idmap *idmap, struct inode
> *inode,
> security_current_getsecid_subj(&secid);
> return ima_match_policy(idmap, inode, current_cred(), secid,
> func, mask, IMA_APPRAISE | IMA_HASH, NULL,
> - NULL, NULL, NULL);
> + NULL, NULL, NULL, NULL);
> }
>
> static int ima_fix_xattr(struct dentry *dentry, struct ima_iint_cache *iint)
> diff --git a/security/integrity/ima/ima_main.c
> b/security/integrity/ima/ima_main.c
> index 18285fc8ac07..e3ca80098c4c 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -232,7 +232,7 @@ static int process_measurement(struct file *file, const
> struct cred *cred,
> */
> action = ima_get_action(file_mnt_idmap(file), inode, cred, secid,
> mask, func, &pcr, &template_desc, NULL,
> - &allowed_algos);
> + &allowed_algos, NULL);
> violation_check = ((func == FILE_CHECK || func == MMAP_CHECK ||
> func == MMAP_CHECK_REQPROT) &&
> (ima_policy_flag & IMA_MEASURE));
> @@ -489,11 +489,11 @@ static int ima_file_mprotect(struct vm_area_struct *vma,
> unsigned long reqprot,
> inode = file_inode(vma->vm_file);
> action = ima_get_action(file_mnt_idmap(vma->vm_file), inode,
> current_cred(), secid, MAY_EXEC, MMAP_CHECK,
> - &pcr, &template, NULL, NULL);
> + &pcr, &template, NULL, NULL, NULL);
> action |= ima_get_action(file_mnt_idmap(vma->vm_file), inode,
> current_cred(), secid, MAY_EXEC,
> MMAP_CHECK_REQPROT, &pcr, &template, NULL,
> - NULL);
> + NULL, NULL);
>
> /* Is the mmap'ed file in policy? */
> if (!(action & (IMA_MEASURE | IMA_APPRAISE_SUBMASK)))
> @@ -972,7 +972,7 @@ int process_buffer_measurement(struct mnt_idmap *idmap,
> security_current_getsecid_subj(&secid);
> action = ima_get_action(idmap, inode, current_cred(),
> secid, 0, func, &pcr, &template,
> - func_data, NULL);
> + func_data, NULL, NULL);
> if (!(action & IMA_MEASURE) && !digest)
> return -ENOENT;
> }
> diff --git a/security/integrity/ima/ima_policy.c
> b/security/integrity/ima/ima_policy.c
> index 7cfd1860791f..4ac83df8d255 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -122,6 +122,7 @@ struct ima_rule_entry {
> struct ima_rule_opt_list *keyrings; /* Measure keys added to these
> keyrings */
> struct ima_rule_opt_list *label; /* Measure data grouped under this
> label */
> struct ima_template_desc *template;
> + u64 digest_cache_mask; /* Actions and purposes for which digest cache
> is allowed */
> };
>
> /*
> @@ -726,6 +727,7 @@ static int get_subaction(struct ima_rule_entry *rule, enum
> ima_hooks func)
> * @template_desc: the template that should be used for this rule
> * @func_data: func specific data, may be NULL
> * @allowed_algos: allowlist of hash algorithms for the IMA xattr
> + * @digest_cache_mask: Actions and purposes for which digest cache is allowed
Purpose
> *
> * Measure decision based on func/mask/fsmagic and LSM(subj/obj/type)
> * conditions.
> @@ -738,7 +740,8 @@ int ima_match_policy(struct mnt_idmap *idmap, struct inode
> *inode,
> const struct cred *cred, u32 secid, enum ima_hooks func,
> int mask, int flags, int *pcr,
> struct ima_template_desc **template_desc,
> - const char *func_data, unsigned int *allowed_algos)
> + const char *func_data, unsigned int *allowed_algos,
> + u64 *digest_cache_mask)
> {
> struct ima_rule_entry *entry;
> int action = 0, actmask = flags | (flags << 1);
> @@ -783,6 +786,9 @@ int ima_match_policy(struct mnt_idmap *idmap, struct inode
> *inode,
> if (template_desc && entry->template)
> *template_desc = entry->template;
>
> + if (digest_cache_mask)
> + *digest_cache_mask |= entry->digest_cache_mask;
> +
> if (!actmask)
> break;
> }
> @@ -859,6 +865,30 @@ static int ima_appraise_flag(enum ima_hooks func)
> return 0;
> }
>
> +static bool ima_digest_cache_func_allowed(struct ima_rule_entry *entry)
> +{
> + switch (entry->func) {
> + case NONE:
> + case FILE_CHECK:
> + case MMAP_CHECK:
> + case MMAP_CHECK_REQPROT:
> + case BPRM_CHECK:
> + case CREDS_CHECK:
> + case FIRMWARE_CHECK:
> + case POLICY_CHECK:
> + case MODULE_CHECK:
> + case KEXEC_KERNEL_CHECK:
> + case KEXEC_INITRAMFS_CHECK:
> + /* Exception: always add policy updates to measurement list! */
> + if (entry->action == MEASURE && entry->func == POLICY_CHECK)
> + return false;
> +
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> static void add_rules(struct ima_rule_entry *entries, int count,
> enum policy_rule_list policy_rule)
> {
> @@ -1073,7 +1103,7 @@ enum policy_opt {
> Opt_digest_type,
> Opt_appraise_type, Opt_appraise_flag, Opt_appraise_algos,
> Opt_permit_directio, Opt_pcr, Opt_template, Opt_keyrings,
> - Opt_label, Opt_err
> + Opt_label, Opt_digest_cache, Opt_err
> };
>
> static const match_table_t policy_tokens = {
> @@ -1122,6 +1152,7 @@ static const match_table_t policy_tokens = {
> {Opt_template, "template=%s"},
> {Opt_keyrings, "keyrings=%s"},
> {Opt_label, "label=%s"},
> + {Opt_digest_cache, "digest_cache=%s"},
> {Opt_err, NULL}
> };
>
> @@ -1245,6 +1276,18 @@ static bool ima_validate_rule(struct ima_rule_entry
> *entry)
> if (entry->action != MEASURE && entry->flags & IMA_PCR)
> return false;
>
> + /* New-style measurements with digest cache cannot be on default PCR. */
> + if (entry->action == MEASURE &&
> + (entry->digest_cache_mask & IMA_DIGEST_CACHE_MEASURE_CONTENT)) {
> + if (!(entry->flags & IMA_PCR) ||
> + entry->pcr == CONFIG_IMA_MEASURE_PCR_IDX)
> + return false;
> + }
> +
> + /* Digest caches can be used only for a subset of the IMA hooks. */
> + if (entry->digest_cache_mask && !ima_digest_cache_func_allowed(entry))
> + return false;
> +
> if (entry->action != APPRAISE &&
> entry->flags & (IMA_DIGSIG_REQUIRED | IMA_MODSIG_ALLOWED |
> IMA_CHECK_BLACKLIST | IMA_VALIDATE_ALGOS))
> @@ -1881,6 +1924,26 @@ static int ima_parse_rule(char *rule, struct
> ima_rule_entry *entry)
> &(template_desc->num_fields));
> entry->template = template_desc;
> break;
> + case Opt_digest_cache:
> + ima_log_string(ab, "digest_cache", args[0].from);
> +
> + result = -EINVAL;
> +
> + if (!strcmp(args[0].from, "content")) {
> + switch (entry->action) {
> + case MEASURE:
> + entry->digest_cache_mask |=
> IMA_DIGEST_CACHE_MEASURE_CONTENT;
> + result = 0;
> + break;
> + case APPRAISE:
> + entry->digest_cache_mask |=
> IMA_DIGEST_CACHE_APPRAISE_CONTENT;
> + result = 0;
> + break;
> + default:
> + break;
> + }
> + }
> + break;
> case Opt_err:
> ima_log_string(ab, "UNKNOWN", p);
> result = -EINVAL;
> @@ -2271,6 +2334,9 @@ int ima_policy_show(struct seq_file *m, void *v)
> seq_puts(m, "digest_type=verity ");
> if (entry->flags & IMA_PERMIT_DIRECTIO)
> seq_puts(m, "permit_directio ");
> + if ((entry->digest_cache_mask & IMA_DIGEST_CACHE_MEASURE_CONTENT) ||
> + (entry->digest_cache_mask & IMA_DIGEST_CACHE_APPRAISE_CONTENT))
> + seq_puts(m, "digest_cache=content ");
> rcu_read_unlock();
> seq_puts(m, "\n");
> return 0;
