> -----Original Message----- > From: linux-integrity-owner@xxxxxxxxxxxxxxx [mailto:linux-integrity- > owner@xxxxxxxxxxxxxxx] On Behalf Of James Bottomley > Sent: Monday, May 11, 2020 2:14 PM > To: Roberts, William C <william.c.roberts@xxxxxxxxx>; linux- > integrity@xxxxxxxxxxxxxxx > Subject: Re: Questions on SHA1 in ima_init > > On Mon, 2020-05-11 at 17:49 +0000, Roberts, William C wrote: > > Hello, > > > > I'm part of the tpm2 users pace tooling and libraries, and I am trying > > to track down an issue in where boot aggregate is only extended in the > > SHA1 bank of PCR10. > > > > You can read the details on the link below, but ill summarize here > > - https://lists.01.org/hyperkitty/list/tpm2@xxxxxxxxxxxx/thread/FUB > > D3MY5U5YICNUYSF3NE2STO3YAW7Y4/ > > > > It looks like ima_add_boot_aggregate() is hardcoded to SHA1, our guess > > is, that it's so it works between TPM 1.X and TPM2.0 chips. Is that > > correct? > > > > I was wondering if that synopsis is correct and if there would be > > traction to add something like querying the tpm chip and getting the > > version And picking SHA256 if its tpm2.0, as a sample to guide the > > discussion I included the patch below (totally untested/not- > > compiled). The main downside would be leaking TPM versions into IMA to > > make a decisions, so it may be better to have a helper in the tpm code > > to pick the best default algorithm where it could pick SHA1 for TPM1.X > > and SHA256 for TPM2.0. Thoughts? > > I think you're not tracking the list. The current patch set doing this among other > things is here: > > https://lore.kernel.org/linux-integrity/20200325104712.25694-1- > roberto.sassu@xxxxxxxxxx/ > > The patch below is too simplistic. If you follow the threads on the list, you'll see > we found a Dell with a TPM2 that won't enable the > sha256 bank if it's set in bios to sha1 mode, which is why the actual patch here: > > https://lore.kernel.org/linux-integrity/20200325104712.25694-1- > roberto.sassu@xxxxxxxxxx/ > > Checks the supported banks and uses sha256 if it's listed. > No I am not tracking this list and my grep foo was failing in the archives. This is exactly what I was looking for , thanks! > James > > > diff --git a/security/integrity/ima/ima_init.c > > b/security/integrity/ima/ima_init.c > > index 567468188a61..d0513bafeebf 100644 > > --- a/security/integrity/ima/ima_init.c > > +++ b/security/integrity/ima/ima_init.c > > @@ -15,6 +15,7 @@ > > #include <linux/scatterlist.h> > > #include <linux/slab.h> > > #include <linux/err.h> > > +#include <linux/printk.h> > > > > #include "ima.h" > > > > @@ -59,6 +60,16 @@ static int __init ima_add_boot_aggregate(void) > > iint->ima_hash->length = SHA1_DIGEST_SIZE; > > > > if (ima_tpm_chip) { > > + result = tpm_is_tpm2(ima_tpm_chip); > > + if (result > 0) { > > + /* yes it's a TPM2 chipset use sha256 */ > > + iint->ima_hash->algo = HASH_ALGO_SHA256; > > + iint->ima_hash->length = SHA256_DIGEST_SIZE; > > + } else if (result < 0) { > > + /* ignore errors here, as we can just move on > > with SHA1 */ > > + pr_warn("Could not query TPM chip version, > > got: %d\n", result); > > + } > > + > > result = ima_calc_boot_aggregate(&hash.hdr); > > if (result < 0) { > > audit_cause = "hashing_error"; > > > > > > > > > >
