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. 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"; > > > > >
