Hi Roberto,

> diff --git a/security/integrity/ima/ima_main.c
> b/security/integrity/ima/ima_main.c
> index 3fc48214850a..48a09747ae7a 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -222,7 +222,9 @@ static int process_measurement(struct file *file, const
> struct cred *cred,
>       bool violation_check;
>       enum hash_algo hash_algo;
>       unsigned int allowed_algos = 0;
> -     u64 verif_mask = 0;
> +     u64 verif_mask = 0, *verif_mask_ptr, policy_mask = 0, allow_mask = 0;
> +     struct digest_cache *digest_cache = NULL, *found_cache;
> +     digest_cache_found_t found;
>  
>       if (!ima_policy_flag || !S_ISREG(inode->i_mode))
>               return 0;
> @@ -233,7 +235,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, NULL);
> +                             &allowed_algos, &policy_mask);
>       violation_check = ((func == FILE_CHECK || func == MMAP_CHECK ||
>                           func == MMAP_CHECK_REQPROT) &&
>                          (ima_policy_flag & IMA_MEASURE));
> @@ -364,10 +366,34 @@ static int process_measurement(struct file *file, const
> struct cred *cred,
>       if (!pathbuf)   /* ima_rdwr_violation possibly pre-fetched */
>               pathname = ima_d_path(&file->f_path, &pathbuf, filename);
>  
> +     /*
> +      * For now we don't support nested verification with digest caches.

I haven't reviewed the digest_cache LSM patch set yet.  What does 'nested' mean
in this context?  Why mention it here?

> +      * Since we allow IMA policy rules without func=, we have to enforce
> +      * this restriction here.
> +      */
> +     if (rc == 0 && policy_mask && func != DIGEST_LIST_CHECK)
> +             digest_cache = digest_cache_get(file_dentry(file));

So whether or not a DIGEST_LIST_CHECK policy rule even exists,
digest_cache_get() will be called.  Similarly, even if a digest_cache list
hasn't been measured or appraised, digest_cache_get() will be called.

Basically every file in policy will check the digest_cache.

> +
> +     if (digest_cache) {
> +             found = digest_cache_lookup(file_dentry(file), digest_cache,
> +                                         iint->ima_hash->digest,
> +                                         iint->ima_hash->algo);
> +             /* AND what is allowed by the policy, and what IMA verified. */
> +             if (found) {
> +                     found_cache = digest_cache_from_found_t(found);
> +                     verif_mask_ptr = digest_cache_verif_get(found_cache,
> +                                                             "ima");

Instead of using "verif_{set,get}' consider using '{set,get}_usage', where usage
here means measure or appraise.

> +                     if (verif_mask_ptr)
> +                             allow_mask = policy_mask & *verif_mask_ptr;
> +             }
> +
> +             digest_cache_put(digest_cache);
> +     }
> +

I'm wondering if it makes sense to create IMA wrappers for each of the
digest_cache functions - checking the digest_cache for the hash, setting the
digest_cache permitted usage, etc - and put all of them in a separate
ima_digest_cache.c file.  The file would only be included in the Makefile if
digest_cache is configured.

In this file you could define a static local global variable to detect whether
the digest_cache is ready to be used.  Only after successfully measuring and
appraising a digest_cache list, based on policy, set the variable.

>       if (action & IMA_MEASURE)
>               ima_store_measurement(iint, file, pathname,
>                                     xattr_value, xattr_len, modsig, pcr,
> -                                   template_desc);
> +                                   template_desc, allow_mask);

'allowed_usage'?

>       if (rc == 0 && (action & IMA_APPRAISE_SUBMASK)) {
>               rc = ima_check_blacklist(iint, modsig, pcr);
>               if (rc != -EPERM) {

thanks,

Mimi


Reply via email to