Re: [PATCH v1 4/5] ima: support fs-verity file digest based signatures

[Mimi Zohar]

Hi Eric,

On Thu, 2021-12-02 at 14:07 -0800, Eric Biggers wrote:
> On Thu, Dec 02, 2021 at 04:55:06PM -0500, Mimi Zohar wrote:
> >  	case IMA_VERITY_DIGSIG:
> > -		fallthrough;
> > +		set_bit(IMA_DIGSIG, &iint->atomic_flags);
> > +
> > +		/*
> > +		 * The IMA signature is based on a hash of IMA_VERITY_DIGSIG
> > +		 * and the fs-verity file digest, not directly on the
> > +		 * fs-verity file digest.  Both digests should probably be
> > +		 * included in the IMA measurement list, but for now this
> > +		 * digest is only used for verifying the IMA signature.
> > +		 */
> > +		verity_digest[0] = IMA_VERITY_DIGSIG;
> > +		memcpy(verity_digest + 1, iint->ima_hash->digest,
> > +		       iint->ima_hash->length);
> > +
> > +		hash.hdr.algo = iint->ima_hash->algo;
> > +		hash.hdr.length = iint->ima_hash->length;
> 
> This is still wrong because the bytes being signed don't include the hash
> algorithm.  Unless you mean for it to be implicitly always SHA-256?  fs-verity
> supports SHA-512 too, and it may support other hash algorithms in the future.

IMA assumes that the file hash algorithm and the signature algorithm
are the same.   If they're not the same, for whatever reason, the
signature verification would simply fail.

Based on the v2 signature header 'type' field, IMA can differentiate
between regular IMA file hash based signatures and fs-verity file
digest based signatures.  The digest field (d-ng) in the IMA
meausrement list prefixes the digest with the hash algorithm. I'm
missing the reason for needing to hash fs-verity's file digest with
other metadata, and sign that hash rather than fs-verity's file digest
directly.

thanks,

Mimi