Re: Questions on SHA1 in ima_init

[James Bottomley <James.Bottomley@HansenPartnership.com>]

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@lists.01.org/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@huawei.com/

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@huawei.com/

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