Re: [PATCH v6 3/5] IMA: add support to restrict the hash algorithms used for file appraisal

Mimi Zohar
2021-08-09T17:44:31Z

On Wed, 2021-08-04 at 09:20 +0000, THOBY Simon wrote:
> The kernel accepts any hash algorithm as a value for the security.ima
> xattr. Users may wish to restrict the accepted algorithms to only
> support strong cryptographic ones.
>
> Provide the plumbing to restrict the permitted set of hash algorithms
> used for verifying file hashes and digest algorithms stored in
> security.ima xattr.

simplify by saying "file hashes and signatures stored ..."

>
> This do not apply only to IMA in hash mode, it also works with digital
> signatures, where the hash from which the signature is derived (by
> signing it with the trusted private key) must obey the same
> restrictions.

The patch is limited to appraisal. Is the above paragraph needed?

>
> Signed-off-by: Simon Thoby <simon.thoby@viveris.fr>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>

This patch restricts the "hash algorithms". Looking this over again
after some time, does truncating variable names here, and in the other
patches, to just "_hash|_hashes" make sense? Perhaps the emphasis
should not be on "hash", but on "algo".

> @@ -684,8 +695,11 @@ int ima_match_policy(struct user_namespace *mnt_userns, struct inode *inode,
> action &= ~IMA_HASH;
> if (ima_fail_unverifiable_sigs)
> action |= IMA_FAIL_UNVERIFIABLE_SIGS;
> - }
>
> + if (allowed_hashes &&
> + entry->flags & IMA_VALIDATE_HASH)
> + *allowed_hashes = entry->allowed_hashes;
> + }
>
> if (entry->action & IMA_DO_MASK)
> actmask &= ~(entry->action | entry->action << 1);

"allowed_hashes" sounds like a set of digests. Instead of
"allowed_hashes" and "IMA_VALIDATE_HASH", should it be "allowed_algo"
and "IMA_ALLOWED_ALGO"?

thanks,

Mimi