RE: [PATCH] extend IMA boot_aggregate with kernel measurements

[Roberto Sassu]

> From: linux-integrity-owner@xxxxxxxxxxxxxxx [mailto:linux-integrity-
> owner@xxxxxxxxxxxxxxx] On Behalf Of Maurizio Drocco
> Sent: Friday, June 12, 2020 4:38 PM
> IMA is not considering TPM registers 8-9 when calculating the boot
> aggregate. When registers 8-9 are used to store measurements of the
> kernel and its command line (e.g., grub2 bootloader with tpm module
> enabled), IMA should include them in the boot aggregate.
> 
> Signed-off-by: Maurizio Drocco <maurizio.drocco@xxxxxxx>
> ---
>  security/integrity/ima/ima.h        |  2 +-
>  security/integrity/ima/ima_crypto.c | 15 ++++++++++++++-
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index df93ac258e01..9d94080bdad8 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -30,7 +30,7 @@
> 
>  enum ima_show_type { IMA_SHOW_BINARY,
> IMA_SHOW_BINARY_NO_FIELD_LEN,
>  		     IMA_SHOW_BINARY_OLD_STRING_FMT,
> IMA_SHOW_ASCII };
> -enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8 };
> +enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
> 
>  /* digest size for IMA, fits SHA1 or MD5 */
>  #define IMA_DIGEST_SIZE		SHA1_DIGEST_SIZE
> diff --git a/security/integrity/ima/ima_crypto.c
> b/security/integrity/ima/ima_crypto.c
> index 220b14920c37..64f5e3151e18 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -809,7 +809,7 @@ static void ima_pcrread(u32 idx, struct tpm_digest *d)
>  static int ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,
>  				       struct crypto_shash *tfm)
>  {
> -	struct tpm_digest d = { .alg_id = alg_id, .digest = {0} };
> +	struct tpm_digest d = { .alg_id = alg_id, .digest = {0} }, d0 = d;
>  	int rc;
>  	u32 i;
>  	SHASH_DESC_ON_STACK(shash, tfm);
> @@ -830,6 +830,19 @@ static int ima_calc_boot_aggregate_tfm(char
> *digest, u16 alg_id,
>  		rc = crypto_shash_update(shash, d.digest,
>  					 crypto_shash_digestsize(tfm));
>  	}
> +	/*
> +	 * extend cumulative sha1 over tpm registers 8-9, which contain

Hi Maurizio

with recent patches, boot_aggregate can be calculated from non-SHA1
PCR banks. I would replace with:

Extend cumulative digest over ...

Given that with this patch boot_aggregate is calculated differently,
shouldn't we call it boot_aggregate_v2 and enable it with a new
option?

Thanks

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli

> +	 * measurement for the kernel command line (reg. 8) and image
> (reg. 9)
> +	 * in a typical PCR allocation.
> +	 */
> +	for (i = TPM_PCR8; i < TPM_PCR10; i++) {
> +		ima_pcrread(i, &d);
> +		/* if not zero, accumulate with current aggregate */
> +		if (memcmp(d.digest, d0.digest,
> +			   crypto_shash_digestsize(tfm)) != 0)
> +			rc = crypto_shash_update(shash, d.digest,
> +
> crypto_shash_digestsize(tfm));
> +	}
>  	if (!rc)
>  		crypto_shash_final(shash, digest);
>  	return rc;
> --
> 2.17.1

> --- Begin Message ---
> Total: 0 warnings, 0 errors, 3 items checked
>
> All 3 test items SUCCESS.
>
> Link: http://patchwork.huawei.com/patch/52890/
>
> ---
> Hulk Robot
>
> --- End Message ---