On Tue, 2020-01-28 at 14:19 +0000, Roberto Sassu wrote: > > -----Original Message----- > > From: linux-integrity-owner@xxxxxxxxxxxxxxx [mailto:linux-integrity- > > owner@xxxxxxxxxxxxxxx] On Behalf Of Mimi Zohar > > Sent: Monday, January 27, 2020 5:02 PM > > To: linux-integrity@xxxxxxxxxxxxxxx > > Cc: Jerry Snitselaar <jsnitsel@xxxxxxxxxx>; James Bottomley > > <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>; linux- > > kernel@xxxxxxxxxxxxxxx; Mimi Zohar <zohar@xxxxxxxxxxxxx> > > Subject: [PATCH 2/2] ima: support calculating the boot_aggregate based on > > different TPM banks > > > > Calculating the boot_aggregate attempts to read the TPM SHA1 bank, > > assuming it is always enabled. With TPM 2.0 hash agility, TPM chips > > could support multiple TPM PCR banks, allowing firmware to configure and > > enable different banks. > > > > Instead of hard coding the TPM 2.0 bank hash algorithm used for calculating > > the boot-aggregate, see if the configured IMA_DEFAULT_HASH algorithm is > > an allocated TPM bank, otherwise use the first allocated TPM bank. > > > > For TPM 1.2 SHA1 is the only supported hash algorithm. > > > > Reported-by: Jerry Snitselaar <jsnitsel@xxxxxxxxxx> > > Signed-off-by: Mimi Zohar <zohar@xxxxxxxxxxxxx> > > --- > > security/integrity/ima/ima_crypto.c | 37 > > ++++++++++++++++++++++++++++++++++++- > > 1 file changed, 36 insertions(+), 1 deletion(-) > > > > diff --git a/security/integrity/ima/ima_crypto.c > > b/security/integrity/ima/ima_crypto.c > > index 7967a6904851..b1b26d61f174 100644 > > --- a/security/integrity/ima/ima_crypto.c > > +++ b/security/integrity/ima/ima_crypto.c > > @@ -656,8 +656,25 @@ static void __init ima_pcrread(u32 idx, struct > > tpm_digest *d) > > pr_err("Error Communicating to TPM chip\n"); > > } > > > > +/* tpm2_hash_map is the same as defined in tpm2-cmd.c and > > trusted_tpm2.c */ > > +static struct tpm2_hash tpm2_hash_map[] = { > > + {HASH_ALGO_SHA1, TPM_ALG_SHA1}, > > + {HASH_ALGO_SHA256, TPM_ALG_SHA256}, > > + {HASH_ALGO_SHA384, TPM_ALG_SHA384}, > > + {HASH_ALGO_SHA512, TPM_ALG_SHA512}, > > + {HASH_ALGO_SM3_256, TPM_ALG_SM3_256}, > > +}; > > + > > /* > > - * Calculate the boot aggregate hash > > + * The boot_aggregate is a cumulative hash over TPM registers 0 - 7. With > > + * TPM 2.0 hash agility, TPM chips could support multiple TPM PCR banks, > > + * allowing firmware to configure and enable different banks. > > + * > > + * Instead of hard coding the TPM bank hash algorithm used for calculating > > + * the boot-aggregate, see if the configured IMA_DEFAULT_HASH > > algorithm is > > + * an allocated TPM bank, otherwise use the first allocated TPM bank. > > + * > > + * For TPM 1.2 SHA1 is the only hash algorithm. > > */ > > static int __init ima_calc_boot_aggregate_tfm(char *digest, > > struct crypto_shash *tfm) > > @@ -673,6 +690,24 @@ static int __init ima_calc_boot_aggregate_tfm(char > > *digest, > > if (rc != 0) > > return rc; > > > > + for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) { > > + if (tpm2_hash_map[i].crypto_id == ima_hash_algo) { > > It is not necessary to define a new map. ima_tpm_chip->allocated_banks > has a crypto_id field. Ok, thanks. > > > + d.alg_id = tpm2_hash_map[i].tpm_id; > > + break; > > + } > > + } > > + > > + for (i = 0; i < ima_tpm_chip->nr_allocated_banks; i++) { > > + if (ima_tpm_chip->allocated_banks[i].alg_id == d.alg_id) > > + break; > > + } > > + > > + if (i == ima_tpm_chip->nr_allocated_banks) > > + d.alg_id = ima_tpm_chip->allocated_banks[0].alg_id; > > This code assumes that the algorithm used to calculate boot_aggregate and > the algorithm of the PCR bank can be different. I don't know if it is possible to > communicate to the verifier which bank has been selected (it depends on > the local configuration). Agreed, but defaulting to the first bank would only happen if the IMA default hash algorithm is not a configured TPM algorithm. > > In my opinion the safest approach would be to use the same algorithm for the > digest and the PCR bank. If you agree to this, then the code above must be > moved to ima_calc_boot_aggregate() so that the algorithm of the selected > PCR bank can be passed to ima_alloc_tfm(). Using the same hash algorithm, preferably the IMA hash default algorithm, for reading the TPM PCR bank and calculating the boot_aggregate makes sense. > > The selected PCR bank might be not the first, if the algorithm is unknown to > the crypto subsystem. It sounds like you're suggesting finding a common configured hash algorithm between the TPM and the kernel. > > > + pr_info("Calculating the boot-aggregregate, reading TPM PCR > > Typo. thanks Mimi
