| Message ID | 20240214143525.2205481-4-roberto.sassu@huaweicloud.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | ima: Integrate with digest_cache LSM | expand |
On Wed, 2024-02-14 at 15:35 +0100, Roberto Sassu wrote: > From: Roberto Sassu <roberto.sassu@huawei.com> > > 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@huawei.com> > --- > 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;
On Thu, 2024-03-07 at 14:43 -0500, Mimi Zohar wrote: > On Wed, 2024-02-14 at 15:35 +0100, Roberto Sassu wrote: > > From: Roberto Sassu <roberto.sassu@huawei.com> > > > > 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'. I wanted to express the fact that digest caches, if available, can only be used to appraise file data, if there is no metadata (similarly to module-style appended signatures). That would prevent for example the scenario where appraisal of file data is successful without having verified current metadata, and EVM attaches to the file a valid HMAC on file close, based on the current xattr value (trust at first use). An IMA rule with 'digest_cache=metadata' would take a different code path. It would make IMA send to evm_verifyxattr() the calculated file digest (since there is no security.ima), and let EVM calculate and search the digest of file metadata in the digest cache. I didn't go that far yet, but this is more or less what I would like to do, also based on my old implementation of the IMA Digest Lists extension (which supports file metadata lookup). > Without having to pass around "digest_cache_mask" would simplify this patch. We need to pass it anyway, to let process_measurement() know that it can use digest caches. Or we can make 'flags' in ima_iint_cache a u64, and introduce new flags. Roberto > 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@huawei.com> > > --- > > 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; >
On Fri, 2024-03-08 at 10:05 +0100, Roberto Sassu wrote: > On Thu, 2024-03-07 at 14:43 -0500, Mimi Zohar wrote: > > On Wed, 2024-02-14 at 15:35 +0100, Roberto Sassu wrote: > > > From: Roberto Sassu <roberto.sassu@huawei.com> > > > > > > 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'. > > I wanted to express the fact that digest caches, if available, can only > be used to appraise file data, if there is no metadata (similarly to > module-style appended signatures). > > That would prevent for example the scenario where appraisal of file > data is successful without having verified current metadata, and EVM > attaches to the file a valid HMAC on file close, based on the current > xattr value (trust at first use). Correct. There's no requirement for 'security.ima' to exist to calculate the EVM HMAC. Before using EVM HMAC, the filesystem needs to be properly labeled by walking the filesystem. The HMAC is calculated based on the existing file metadata. The first use is during a setup stage and subsequently for new files. > > An IMA rule with 'digest_cache=metadata' would take a different code > path. It would make IMA send to evm_verifyxattr() the calculated file > digest (since there is no security.ima), and let EVM calculate and > search the digest of file metadata in the digest cache. Ok. So no 'security.evm' either, but a metadata digest cache. I understand the need to provide EVM with the file hash in this case, but as much as possible, IMA and EVM should be independent of each other. > > I didn't go that far yet, but this is more or less what I would like to > do, also based on my old implementation of the IMA Digest Lists > extension (which supports file metadata lookup). > > > Without having to pass around "digest_cache_mask" would simplify this patch. > > We need to pass it anyway, to let process_measurement() know that it > can use digest caches. Or we can make 'flags' in ima_iint_cache a u64, > and introduce new flags. It's bad enough that the function parameters change when there's actual data. Yes, please increase the size of 'flags' and introduce new flags. thanks, 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@huawei.com> > > >
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 * * 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;