Re: [PATCH v3 7/8] ima: Calculate and extend PCR with digests in ima_template_entry
From: Mimi Zohar
Date: Thu Mar 19 2020 - 05:46:07 EST
On Thu, 2020-03-19 at 08:31 +0000, Roberto Sassu wrote:
> > -----Original Message-----
> > From: linux-integrity-owner@xxxxxxxxxxxxxxx [mailto:linux-integrity-
> > owner@xxxxxxxxxxxxxxx] On Behalf Of Mimi Zohar
> > Sent: Wednesday, March 18, 2020 10:55 PM
> > To: Roberto Sassu <roberto.sassu@xxxxxxxxxx>;
> > James.Bottomley@xxxxxxxxxxxxxxxxxxxxx;
> > jarkko.sakkinen@xxxxxxxxxxxxxxx
> > Cc: linux-integrity@xxxxxxxxxxxxxxx; linux-security-module@xxxxxxxxxxxxxxx;
> > linux-kernel@xxxxxxxxxxxxxxx; Silviu Vlasceanu
> > <Silviu.Vlasceanu@xxxxxxxxxx>
> > Subject: Re: [PATCH v3 7/8] ima: Calculate and extend PCR with digests in
> > ima_template_entry
> >
> > On Wed, 2020-03-18 at 12:42 +0000, Roberto Sassu wrote:
> > > > -----Original Message-----
> > > > From: owner-linux-security-module@xxxxxxxxxxxxxxx [mailto:owner-
> > linux-
> > > > security-module@xxxxxxxxxxxxxxx] On Behalf Of Mimi Zohar
> > > > Sent: Tuesday, March 3, 2020 5:04 AM
> > > > To: Roberto Sassu <roberto.sassu@xxxxxxxxxx>;
> > > > James.Bottomley@xxxxxxxxxxxxxxxxxxxxx;
> > > > jarkko.sakkinen@xxxxxxxxxxxxxxx
> > > > Cc: linux-integrity@xxxxxxxxxxxxxxx; linux-security-
> > module@xxxxxxxxxxxxxxx;
> > > > linux-kernel@xxxxxxxxxxxxxxx; Silviu Vlasceanu
> > > > <Silviu.Vlasceanu@xxxxxxxxxx>
> > > > Subject: Re: [PATCH v3 7/8] ima: Calculate and extend PCR with digests
> > in
> > > > ima_template_entry
> > > >
> > > > On Mon, 2020-02-10 at 11:04 +0100, Roberto Sassu wrote:
> > > >
> > > > > @@ -219,6 +214,8 @@ int ima_restore_measurement_entry(struct
> > > > ima_template_entry *entry)
> > > > >
> > > > > int __init ima_init_digests(void)
> > > > > {
> > > > > + u16 digest_size;
> > > > > + u16 crypto_id;
> > > > > int i;
> > > > >
> > > > > if (!ima_tpm_chip)
> > > > > @@ -229,8 +226,17 @@ int __init ima_init_digests(void)
> > > > > if (!digests)
> > > > > return -ENOMEM;
> > > > >
> > > > > - for (i = 0; i < ima_tpm_chip->nr_allocated_banks; i++)
> > > > > + for (i = 0; i < ima_tpm_chip->nr_allocated_banks; i++) {
> > > > > digests[i].alg_id = ima_tpm_chip->allocated_banks[i].alg_id;
> > > > > + digest_size = ima_tpm_chip->allocated_banks[i].digest_size;
> > > > > + crypto_id = ima_tpm_chip->allocated_banks[i].crypto_id;
> > > > > +
> > > > > + /* for unmapped TPM algorithms digest is still a padded
> > > > SHA1 */
> > > > > + if (crypto_id == HASH_ALGO__LAST)
> > > > > + digest_size = SHA1_DIGEST_SIZE;
> > > > > +
> > > > > + memset(digests[i].digest, 0xff, digest_size);
> > > >
> > > > Shouldn't the memset here be of the actual digest size even for
> > > > unmapped TPM algorithms.
> > >
> > > This is consistent with ima_calc_field_array_hash(), so that a verifier
> > > will always pad the SHA1 digest with zeros to obtain the final PCR value.
> > >
> > > I can set all bytes if you prefer.
> >
> > My concern is with violations. ÂThe measurement list will be padded
> > with 0's, but the value being extended into the TPM will only
> > partially be 0xFF's. ÂWhen verifying the measurement list, replacing
> > all 0x00's with all 0xFF's is simpler.
>
> If the TPM algorithm is unknown, the starting point is the SHA1 digest.
> If there is a violation, this should be the one to be modified. Then, after
> that, padding is done for all entries in the same way, regardless of
> whether the entry is a violation or not.
Ok. ÂIn the case that the verifier supports the hash algorithm and
calculates the template hash, walking the measurement list will fail
anyway. ÂIn the case that the verifier does not support the hash
algorithm, then it will pad/truncate the SHA1 hash consistently. ÂThat
works for now with the SHA1 based measurement list and should work
with a hash agile measurement list.
thanks,
Mimi