[PATCH v2 1/8] tpm: Initialize crypto_id of allocated_banks to HASH_ALGO__LAST

[PATCH v2 1/8] tpm: Initialize crypto_id of allocated_banks to HASH_ALGO__LAST

[Roberto Sassu]

chip->allocated_banks, an array of tpm_bank_info structures, contains the
list of TPM algorithm IDs of allocated PCR banks. It also contains the
corresponding ID of the crypto subsystem, so that users of the TPM driver
can calculate a digest for a PCR extend operation.

However, if there is no mapping between TPM algorithm ID and crypto ID, the
crypto_id field of tpm_bank_info remains set to zero (the array is
allocated and initialized with kcalloc() in tpm2_get_pcr_allocation()).
Zero should not be used as value for unknown mappings, as it is a valid
crypto ID (HASH_ALGO_MD4).

Thus, initialize crypto_id to HASH_ALGO__LAST.

Fixes: 879b589210a9 ("tpm: retrieve digest size of unknown algorithms with PCR read")
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Cc: stable@vger.kernel.org
---
 drivers/char/tpm/tpm2-cmd.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 13696deceae8..760329598b99 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -525,6 +525,8 @@ static int tpm2_init_bank_info(struct tpm_chip *chip, u32 bank_index)
 		return 0;
 	}
 
+	bank->crypto_id = HASH_ALGO__LAST;
+
 	return tpm2_pcr_read(chip, 0, &digest, &bank->digest_size);
 }
 
-- 
2.17.1

Re: [PATCH v2 1/8] tpm: Initialize crypto_id of allocated_banks to HASH_ALGO__LAST

[Jarkko Sakkinen]

On Wed, Feb 05, 2020 at 11:33:10AM +0100, Roberto Sassu wrote:
> chip->allocated_banks, an array of tpm_bank_info structures, contains the
> list of TPM algorithm IDs of allocated PCR banks. It also contains the
> corresponding ID of the crypto subsystem, so that users of the TPM driver
> can calculate a digest for a PCR extend operation.
> 
> However, if there is no mapping between TPM algorithm ID and crypto ID, the
> crypto_id field of tpm_bank_info remains set to zero (the array is
> allocated and initialized with kcalloc() in tpm2_get_pcr_allocation()).
> Zero should not be used as value for unknown mappings, as it is a valid
> crypto ID (HASH_ALGO_MD4).
> 
> Thus, initialize crypto_id to HASH_ALGO__LAST.
> 
> Fixes: 879b589210a9 ("tpm: retrieve digest size of unknown algorithms with PCR read")
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> Cc: stable@vger.kernel.org

Cc should be first.

/Jarkko

[PATCH v2 2/8] ima: Switch to ima_hash_algo for boot aggregate

[Roberto Sassu]

boot_aggregate is the first entry of IMA measurement list. Its purpose is
to link pre-boot measurements to IMA measurements. As IMA was designed to
work with a TPM 1.2, the SHA1 PCR bank was always selected.

Currently, even if a TPM 2.0 is used, the SHA1 PCR bank is selected.
However, the assumption that the SHA1 PCR bank is always available is not
correct, as PCR banks can be selected with the PCR_Allocate() TPM command.

This patch tries to use ima_hash_algo as hash algorithm for boot_aggregate.
If no PCR bank uses that algorithm, the patch tries to find the SHA256 PCR
bank (which is mandatory in the TCG PC Client specification). If also this
bank is not found, the patch selects the first one. If the TPM algorithm
of that bank is not mapped to a crypto ID, boot_aggregate is set to zero.

Changelog

v1:
- add Mimi's comments
- if there is no PCR bank for the IMA default algorithm use SHA256 or the
  first bank (suggested by James Bottomley)

Reported-by: Jerry Snitselaar <jsnitsel@redhat.com>
Suggested-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Cc: stable@vger.kernel.org
---
 security/integrity/ima/ima_crypto.c | 45 +++++++++++++++++++++++++----
 security/integrity/ima/ima_init.c   | 22 ++++++++++----
 2 files changed, 56 insertions(+), 11 deletions(-)

diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 73044fc6a952..f2f41a2bc3d4 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -655,18 +655,29 @@ static void __init ima_pcrread(u32 idx, struct tpm_digest *d)
 }
 
 /*
- * Calculate the boot aggregate hash
+ * The boot_aggregate is a cumulative hash over TPM registers 0 - 7.  With
+ * TPM 1.2 the boot_aggregate was based on reading the SHA1 PCRs, but with
+ * TPM 2.0 hash agility, TPM chips could support multiple TPM PCR banks,
+ * allowing firmware to configure and enable different banks.
+ *
+ * Knowing which TPM bank is read to calculate the boot_aggregate digest
+ * needs to be conveyed to a verifier.  For this reason, use the same
+ * hash algorithm for reading the TPM PCRs as for calculating the boot
+ * aggregate digest as stored in the measurement list.
  */
-static int __init ima_calc_boot_aggregate_tfm(char *digest,
+static int __init ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,
 					      struct crypto_shash *tfm)
 {
-	struct tpm_digest d = { .alg_id = TPM_ALG_SHA1, .digest = {0} };
+	struct tpm_digest d = { .alg_id = alg_id, .digest = {0} };
 	int rc;
 	u32 i;
 	SHASH_DESC_ON_STACK(shash, tfm);
 
 	shash->tfm = tfm;
 
+	pr_devel("calculating the boot-aggregate based on TPM bank: %04x\n",
+		 d.alg_id);
+
 	rc = crypto_shash_init(shash);
 	if (rc != 0)
 		return rc;
@@ -675,7 +686,8 @@ static int __init ima_calc_boot_aggregate_tfm(char *digest,
 	for (i = TPM_PCR0; i < TPM_PCR8; i++) {
 		ima_pcrread(i, &d);
 		/* now accumulate with current aggregate */
-		rc = crypto_shash_update(shash, d.digest, TPM_DIGEST_SIZE);
+		rc = crypto_shash_update(shash, d.digest,
+					 crypto_shash_digestsize(tfm));
 	}
 	if (!rc)
 		crypto_shash_final(shash, digest);
@@ -685,14 +697,35 @@ static int __init ima_calc_boot_aggregate_tfm(char *digest,
 int __init ima_calc_boot_aggregate(struct ima_digest_data *hash)
 {
 	struct crypto_shash *tfm;
-	int rc;
+	u16 crypto_id, alg_id;
+	int rc, i, bank_idx = 0;
+
+	for (i = 0; i < ima_tpm_chip->nr_allocated_banks; i++) {
+		crypto_id = ima_tpm_chip->allocated_banks[i].crypto_id;
+		if (crypto_id == hash->algo) {
+			bank_idx = i;
+			break;
+		}
+
+		if (crypto_id == HASH_ALGO_SHA256)
+			bank_idx = i;
+	}
+
+	if (bank_idx == 0 &&
+	    ima_tpm_chip->allocated_banks[0].crypto_id == HASH_ALGO__LAST) {
+		pr_err("No suitable TPM algorithm for boot aggregate\n");
+		return 0;
+	}
+
+	hash->algo = ima_tpm_chip->allocated_banks[bank_idx].crypto_id;
 
 	tfm = ima_alloc_tfm(hash->algo);
 	if (IS_ERR(tfm))
 		return PTR_ERR(tfm);
 
 	hash->length = crypto_shash_digestsize(tfm);
-	rc = ima_calc_boot_aggregate_tfm(hash->digest, tfm);
+	alg_id = ima_tpm_chip->allocated_banks[bank_idx].alg_id;
+	rc = ima_calc_boot_aggregate_tfm(hash->digest, alg_id, tfm);
 
 	ima_free_tfm(tfm);
 
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 5d55ade5f3b9..fbd7a8e28a6b 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -27,7 +27,7 @@ struct tpm_chip *ima_tpm_chip;
 /* Add the boot aggregate to the IMA measurement list and extend
  * the PCR register.
  *
- * Calculate the boot aggregate, a SHA1 over tpm registers 0-7,
+ * Calculate the boot aggregate, a hash over tpm registers 0-7,
  * assuming a TPM chip exists, and zeroes if the TPM chip does not
  * exist.  Add the boot aggregate measurement to the measurement
  * list and extend the PCR register.
@@ -51,15 +51,27 @@ static int __init ima_add_boot_aggregate(void)
 	int violation = 0;
 	struct {
 		struct ima_digest_data hdr;
-		char digest[TPM_DIGEST_SIZE];
+		char digest[TPM_MAX_DIGEST_SIZE];
 	} hash;
 
 	memset(iint, 0, sizeof(*iint));
 	memset(&hash, 0, sizeof(hash));
 	iint->ima_hash = &hash.hdr;
-	iint->ima_hash->algo = HASH_ALGO_SHA1;
-	iint->ima_hash->length = SHA1_DIGEST_SIZE;
-
+	iint->ima_hash->algo = ima_hash_algo;
+	iint->ima_hash->length = hash_digest_size[ima_hash_algo];
+
+	/*
+	 * With TPM 2.0 hash agility, TPM chips could support multiple TPM
+	 * PCR banks, allowing firmware to configure and enable different
+	 * banks.  The SHA1 bank is not necessarily enabled.
+	 *
+	 * Use the same hash algorithm for reading the TPM PCRs as for
+	 * calculating the boot aggregate digest.  Preference is given to
+	 * the configured IMA default hash algorithm.  Otherwise, use the
+	 * TPM required banks - SHA256 for TPM 2.0, SHA1 for TPM 1.2.  If
+	 * SHA256 is not available, use the first PCR bank and if that is
+	 * not mapped to a crypto ID, set boot_aggregate to zero.
+	 */
 	if (ima_tpm_chip) {
 		result = ima_calc_boot_aggregate(&hash.hdr);
 		if (result < 0) {
-- 
2.17.1

RE: [PATCH v2 2/8] ima: Switch to ima_hash_algo for boot aggregate

[Roberto Sassu]

> -----Original Message-----
> From: Sasha Levin [mailto:sashal@kernel.org]
> Sent: Wednesday, February 5, 2020 3:45 PM
> To: Sasha Levin <sashal@kernel.org>; Roberto Sassu
> <roberto.sassu@huawei.com>; zohar@linux.ibm.com;
> James.Bottomley@HansenPartnership.com
> Cc: linux-integrity@vger.kernel.org; stable@vger.kernel.org;
> stable@vger.kernel.org
> Subject: Re: [PATCH v2 2/8] ima: Switch to ima_hash_algo for boot
> aggregate
> 
> Hi,
> 
> [This is an automated email]
> 
> This commit has been processed because it contains a -stable tag.
> The stable tag indicates that it's relevant for the following trees: all
> 
> The bot has tested the following trees: v5.5.1, v5.4.17, v4.19.101, v4.14.169,
> v4.9.212, v4.4.212.
> 
> v5.5.1: Build OK!
> v5.4.17: Build OK!
> v4.19.101: Failed to apply! Possible dependencies:
>     100b16a6f290 ("tpm: sort objects in the Makefile")
>     1ad6640cd614 ("tpm: move tpm1_pcr_extend to tpm1-cmd.c")
>     70a3199a7101 ("tpm: factor out tpm_get_timeouts()")
>     879b589210a9 ("tpm: retrieve digest size of unknown algorithms with PCR
> read")

Hi Sasha

this patch is necessary. However, backporting it won't be that easy
as it was part of a patch set. Before this patch, users of the TPM driver
could only read the SHA1 PCR bank. The IMA patch needs to read also
other PCR banks.

> NOTE: The patch will not be queued to stable trees until it is upstream.
> 
> How should we proceed with this patch?

This question should be for Jarkko (added in CC), as some patches for the
TPM driver must be backported to apply the IMA patch.

Roberto

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

Re: [PATCH v2 2/8] ima: Switch to ima_hash_algo for boot aggregate

[Mimi Zohar]

On Wed, 2020-02-05 at 11:33 +0100, Roberto Sassu wrote:
> boot_aggregate is the first entry of IMA measurement list. Its purpose is
> to link pre-boot measurements to IMA measurements. As IMA was designed to
> work with a TPM 1.2, the SHA1 PCR bank was always selected.
> 
> Currently, even if a TPM 2.0 is used, the SHA1 PCR bank is selected.
> However, the assumption that the SHA1 PCR bank is always available is not
> correct, as PCR banks can be selected with the PCR_Allocate() TPM command.
> 
> This patch tries to use ima_hash_algo as hash algorithm for boot_aggregate.
> If no PCR bank uses that algorithm, the patch tries to find the SHA256 PCR
> bank (which is mandatory in the TCG PC Client specification). If also this
> bank is not found, the patch selects the first one. If the TPM algorithm
> of that bank is not mapped to a crypto ID, boot_aggregate is set to zero.
> 
> Changelog
> 
> v1:
> - add Mimi's comments
> - if there is no PCR bank for the IMA default algorithm use SHA256 or the
>   first bank (suggested by James Bottomley)

If the IMA default hash algorithm is not enabled, James' comment was
to use SHA256 for TPM 2.0 and SHA1 for TPM 1.2.  I don't remember him
saying anything about using the first bank, that was in v1 of my
patch.  Please refer to v2 of my patch, based on James' comments.

> 
> Reported-by: Jerry Snitselaar <jsnitsel@redhat.com>
> Suggested-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> Cc: stable@vger.kernel.org
> ---
>  security/integrity/ima/ima_crypto.c | 45 +++++++++++++++++++++++++----
>  security/integrity/ima/ima_init.c   | 22 ++++++++++----
>  2 files changed, 56 insertions(+), 11 deletions(-)
> 
> diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
> index 73044fc6a952..f2f41a2bc3d4 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -655,18 +655,29 @@ static void __init ima_pcrread(u32 idx, struct tpm_digest *d)
>  }
>  
>  /*
> - * Calculate the boot aggregate hash
> + * The boot_aggregate is a cumulative hash over TPM registers 0 - 7.  With
> + * TPM 1.2 the boot_aggregate was based on reading the SHA1 PCRs, but with
> + * TPM 2.0 hash agility, TPM chips could support multiple TPM PCR banks,
> + * allowing firmware to configure and enable different banks.
> + *
> + * Knowing which TPM bank is read to calculate the boot_aggregate digest
> + * needs to be conveyed to a verifier.  For this reason, use the same
> + * hash algorithm for reading the TPM PCRs as for calculating the boot
> + * aggregate digest as stored in the measurement list.
>   */
> -static int __init ima_calc_boot_aggregate_tfm(char *digest,
> +static int __init ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,
>  					      struct crypto_shash *tfm)
>  {
> -	struct tpm_digest d = { .alg_id = TPM_ALG_SHA1, .digest = {0} };
> +	struct tpm_digest d = { .alg_id = alg_id, .digest = {0} };
>  	int rc;
>  	u32 i;
>  	SHASH_DESC_ON_STACK(shash, tfm);
>  
>  	shash->tfm = tfm;
>  
> +	pr_devel("calculating the boot-aggregate based on TPM bank: %04x\n",
> +		 d.alg_id);
> +

Good

>  	rc = crypto_shash_init(shash);
>  	if (rc != 0)
>  		return rc;
> @@ -675,7 +686,8 @@ static int __init ima_calc_boot_aggregate_tfm(char *digest,
>  	for (i = TPM_PCR0; i < TPM_PCR8; i++) {
>  		ima_pcrread(i, &d);
>  		/* now accumulate with current aggregate */
> -		rc = crypto_shash_update(shash, d.digest, TPM_DIGEST_SIZE);
> +		rc = crypto_shash_update(shash, d.digest,
> +					 crypto_shash_digestsize(tfm));
>  	}
>  	if (!rc)
>  		crypto_shash_final(shash, digest);
> @@ -685,14 +697,35 @@ static int __init ima_calc_boot_aggregate_tfm(char *digest,
>  int __init ima_calc_boot_aggregate(struct ima_digest_data *hash)
>  {
> 
< snip >
>  
>  	hash->length = crypto_shash_digestsize(tfm);
> -	rc = ima_calc_boot_aggregate_tfm(hash->digest, tfm);
> +	alg_id = ima_tpm_chip->allocated_banks[bank_idx].alg_id;
> +	rc = ima_calc_boot_aggregate_tfm(hash->digest, alg_id, tfm);

Sure, backporting this change to ima_calc_boot_aggregate_tfm() should
be fine.

Mimi

>  	ima_free_tfm(tfm);
>  

RE: [PATCH v2 2/8] ima: Switch to ima_hash_algo for boot aggregate

[Roberto Sassu]

> -----Original Message-----
> From: Mimi Zohar [mailto:zohar@linux.ibm.com]
> Sent: Wednesday, February 5, 2020 9:41 PM
> To: Roberto Sassu <roberto.sassu@huawei.com>;
> James.Bottomley@HansenPartnership.com;
> jarkko.sakkinen@linux.intel.com
> Cc: linux-integrity@vger.kernel.org; linux-security-module@vger.kernel.org;
> linux-kernel@vger.kernel.org; Silviu Vlasceanu
> <Silviu.Vlasceanu@huawei.com>; stable@vger.kernel.org
> Subject: Re: [PATCH v2 2/8] ima: Switch to ima_hash_algo for boot
> aggregate
> 
> On Wed, 2020-02-05 at 11:33 +0100, Roberto Sassu wrote:
> > boot_aggregate is the first entry of IMA measurement list. Its purpose is
> > to link pre-boot measurements to IMA measurements. As IMA was
> designed to
> > work with a TPM 1.2, the SHA1 PCR bank was always selected.
> >
> > Currently, even if a TPM 2.0 is used, the SHA1 PCR bank is selected.
> > However, the assumption that the SHA1 PCR bank is always available is not
> > correct, as PCR banks can be selected with the PCR_Allocate() TPM
> command.
> >
> > This patch tries to use ima_hash_algo as hash algorithm for
> boot_aggregate.
> > If no PCR bank uses that algorithm, the patch tries to find the SHA256 PCR
> > bank (which is mandatory in the TCG PC Client specification). If also this
> > bank is not found, the patch selects the first one. If the TPM algorithm
> > of that bank is not mapped to a crypto ID, boot_aggregate is set to zero.
> >
> > Changelog
> >
> > v1:
> > - add Mimi's comments
> > - if there is no PCR bank for the IMA default algorithm use SHA256 or the
> >   first bank (suggested by James Bottomley)
> 
> If the IMA default hash algorithm is not enabled, James' comment was
> to use SHA256 for TPM 2.0 and SHA1 for TPM 1.2.  I don't remember him
> saying anything about using the first bank, that was in v1 of my
> patch.  Please refer to v2 of my patch, based on James' comments.

Right. Will fix in the next version.

Thanks

Roberto

 > Reported-by: Jerry Snitselaar <jsnitsel@redhat.com>
> > Suggested-by: James Bottomley
> <James.Bottomley@HansenPartnership.com>
> > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > Cc: stable@vger.kernel.org
> > ---
> >  security/integrity/ima/ima_crypto.c | 45
> +++++++++++++++++++++++++----
> >  security/integrity/ima/ima_init.c   | 22 ++++++++++----
> >  2 files changed, 56 insertions(+), 11 deletions(-)
> >
> > diff --git a/security/integrity/ima/ima_crypto.c
> b/security/integrity/ima/ima_crypto.c
> > index 73044fc6a952..f2f41a2bc3d4 100644
> > --- a/security/integrity/ima/ima_crypto.c
> > +++ b/security/integrity/ima/ima_crypto.c
> > @@ -655,18 +655,29 @@ static void __init ima_pcrread(u32 idx, struct
> tpm_digest *d)
> >  }
> >
> >  /*
> > - * Calculate the boot aggregate hash
> > + * The boot_aggregate is a cumulative hash over TPM registers 0 - 7.
> With
> > + * TPM 1.2 the boot_aggregate was based on reading the SHA1 PCRs, but
> with
> > + * TPM 2.0 hash agility, TPM chips could support multiple TPM PCR banks,
> > + * allowing firmware to configure and enable different banks.
> > + *
> > + * Knowing which TPM bank is read to calculate the boot_aggregate
> digest
> > + * needs to be conveyed to a verifier.  For this reason, use the same
> > + * hash algorithm for reading the TPM PCRs as for calculating the boot
> > + * aggregate digest as stored in the measurement list.
> >   */
> > -static int __init ima_calc_boot_aggregate_tfm(char *digest,
> > +static int __init ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,
> >  					      struct crypto_shash *tfm)
> >  {
> > -	struct tpm_digest d = { .alg_id = TPM_ALG_SHA1, .digest = {0} };
> > +	struct tpm_digest d = { .alg_id = alg_id, .digest = {0} };
> >  	int rc;
> >  	u32 i;
> >  	SHASH_DESC_ON_STACK(shash, tfm);
> >
> >  	shash->tfm = tfm;
> >
> > +	pr_devel("calculating the boot-aggregate based on TPM
> bank: %04x\n",
> > +		 d.alg_id);
> > +
> 
> Good
> 
> >  	rc = crypto_shash_init(shash);
> >  	if (rc != 0)
> >  		return rc;
> > @@ -675,7 +686,8 @@ static int __init ima_calc_boot_aggregate_tfm(char
> *digest,
> >  	for (i = TPM_PCR0; i < TPM_PCR8; i++) {
> >  		ima_pcrread(i, &d);
> >  		/* now accumulate with current aggregate */
> > -		rc = crypto_shash_update(shash, d.digest,
> TPM_DIGEST_SIZE);
> > +		rc = crypto_shash_update(shash, d.digest,
> > +					 crypto_shash_digestsize(tfm));
> >  	}
> >  	if (!rc)
> >  		crypto_shash_final(shash, digest);
> > @@ -685,14 +697,35 @@ static int __init
> ima_calc_boot_aggregate_tfm(char *digest,
> >  int __init ima_calc_boot_aggregate(struct ima_digest_data *hash)
> >  {
> >
> < snip >
> >
> >  	hash->length = crypto_shash_digestsize(tfm);
> > -	rc = ima_calc_boot_aggregate_tfm(hash->digest, tfm);
> > +	alg_id = ima_tpm_chip->allocated_banks[bank_idx].alg_id;
> > +	rc = ima_calc_boot_aggregate_tfm(hash->digest, alg_id, tfm);
> 
> Sure, backporting this change to ima_calc_boot_aggregate_tfm() should
> be fine.
> 
> Mimi
> 
> >  	ima_free_tfm(tfm);
> >

Re: [PATCH v2 2/8] ima: Switch to ima_hash_algo for boot aggregate

[Mimi Zohar]

Hi Roberto,

On Wed, 2020-02-05 at 11:33 +0100, Roberto Sassu wrote:

<snip>

> Reported-by: Jerry Snitselaar <jsnitsel@redhat.com>
> Suggested-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> Cc: stable@vger.kernel.org

Cc'ing stable resulted in Sasha's automated message.  If you're going
to Cc stable, then please include the stable kernel release (e.g. Cc: 
stable@vger.kernel.org # v5.3).  Also please include a "Fixes" tag.
 Normally only bug fixes are backported.

Mimi

RE: [PATCH v2 2/8] ima: Switch to ima_hash_algo for boot aggregate

[Roberto Sassu]

> -----Original Message-----
> From: owner-linux-security-module@vger.kernel.org [mailto:owner-linux-
> security-module@vger.kernel.org] On Behalf Of Mimi Zohar
> Sent: Wednesday, February 5, 2020 10:01 PM
> To: Roberto Sassu <roberto.sassu@huawei.com>;
> James.Bottomley@HansenPartnership.com;
> jarkko.sakkinen@linux.intel.com
> Cc: linux-integrity@vger.kernel.org; linux-security-module@vger.kernel.org;
> linux-kernel@vger.kernel.org; Silviu Vlasceanu
> <Silviu.Vlasceanu@huawei.com>; stable@vger.kernel.org
> Subject: Re: [PATCH v2 2/8] ima: Switch to ima_hash_algo for boot
> aggregate
> 
> Hi Roberto,
> 
> On Wed, 2020-02-05 at 11:33 +0100, Roberto Sassu wrote:
> 
> <snip>
> 
> > Reported-by: Jerry Snitselaar <jsnitsel@redhat.com>
> > Suggested-by: James Bottomley
> <James.Bottomley@HansenPartnership.com>
> > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > Cc: stable@vger.kernel.org
> 
> Cc'ing stable resulted in Sasha's automated message.  If you're going
> to Cc stable, then please include the stable kernel release (e.g. Cc:
> stable@vger.kernel.org # v5.3).  Also please include a "Fixes" tag.
>  Normally only bug fixes are backported.

Ok, will add the kernel version. I also thought which commit I should
mention in the Fixes tag. IMA always read the SHA1 bank from the
beginning. I could mention the patch that introduces the new API
to read other banks, but I'm not sure. What do you think?

Thanks

Roberto

Re: [PATCH v2 2/8] ima: Switch to ima_hash_algo for boot aggregate

[Mimi Zohar]

On Thu, 2020-02-06 at 09:36 +0000, Roberto Sassu wrote:
> > Hi Roberto,
> > 
> > On Wed, 2020-02-05 at 11:33 +0100, Roberto Sassu wrote:
> > 
> > <snip>
> > 
> > > Reported-by: Jerry Snitselaar <jsnitsel@redhat.com>
> > > Suggested-by: James Bottomley
> > <James.Bottomley@HansenPartnership.com>
> > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > > Cc: stable@vger.kernel.org
> > 
> > Cc'ing stable resulted in Sasha's automated message.  If you're going
> > to Cc stable, then please include the stable kernel release (e.g. Cc:
> > stable@vger.kernel.org # v5.3).  Also please include a "Fixes" tag.
> >  Normally only bug fixes are backported.
> 
> Ok, will add the kernel version. I also thought which commit I should
> mention in the Fixes tag. IMA always read the SHA1 bank from the
> beginning. I could mention the patch that introduces the new API
> to read other banks, but I'm not sure. What do you think?

This patch is dependent on nr_allocated_banks.  Please try applying
this patch to the earliest stable kernel with the commit that
introduces nr_allocated_banks and test to make sure it works properly.

thanks,

Mimi

RE: [PATCH v2 2/8] ima: Switch to ima_hash_algo for boot aggregate

[Roberto Sassu]

> -----Original Message-----
> From: Mimi Zohar [mailto:zohar@linux.ibm.com]
> Sent: Thursday, February 6, 2020 1:17 PM
> To: Roberto Sassu <roberto.sassu@huawei.com>;
> James.Bottomley@HansenPartnership.com;
> jarkko.sakkinen@linux.intel.com
> Cc: linux-integrity@vger.kernel.org; linux-security-module@vger.kernel.org;
> linux-kernel@vger.kernel.org; Silviu Vlasceanu
> <Silviu.Vlasceanu@huawei.com>; stable@vger.kernel.org
> Subject: Re: [PATCH v2 2/8] ima: Switch to ima_hash_algo for boot
> aggregate
> 
> On Thu, 2020-02-06 at 09:36 +0000, Roberto Sassu wrote:
> > > Hi Roberto,
> > >
> > > On Wed, 2020-02-05 at 11:33 +0100, Roberto Sassu wrote:
> > >
> > > <snip>
> > >
> > > > Reported-by: Jerry Snitselaar <jsnitsel@redhat.com>
> > > > Suggested-by: James Bottomley
> > > <James.Bottomley@HansenPartnership.com>
> > > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > > > Cc: stable@vger.kernel.org
> > >
> > > Cc'ing stable resulted in Sasha's automated message.  If you're going
> > > to Cc stable, then please include the stable kernel release (e.g. Cc:
> > > stable@vger.kernel.org # v5.3).  Also please include a "Fixes" tag.
> > >  Normally only bug fixes are backported.
> >
> > Ok, will add the kernel version. I also thought which commit I should
> > mention in the Fixes tag. IMA always read the SHA1 bank from the
> > beginning. I could mention the patch that introduces the new API
> > to read other banks, but I'm not sure. What do you think?
> 
> This patch is dependent on nr_allocated_banks.  Please try applying
> this patch to the earliest stable kernel with the commit that
> introduces nr_allocated_banks and test to make sure it works properly.

It also depends on 879b589210a9 ("tpm: retrieve digest size of unknown"
algorithms with PCR read") which exported the mapping between TPM
algorithm ID and crypto ID, and changed the definition of tpm_pcr_read()
to read non-SHA1 PCR banks. It requires many patches, so backporting it
is not a trivial task. I think the earliest kernel this patch can be backported to
is 5.1.

Roberto

Re: [PATCH v2 2/8] ima: Switch to ima_hash_algo for boot aggregate

[Mimi Zohar]

On Thu, 2020-02-06 at 12:28 +0000, Roberto Sassu wrote:
> > -----Original Message-----
> > From: Mimi Zohar [mailto:zohar@linux.ibm.com]
> > Sent: Thursday, February 6, 2020 1:17 PM
> > To: Roberto Sassu <roberto.sassu@huawei.com>;
> > James.Bottomley@HansenPartnership.com;
> > jarkko.sakkinen@linux.intel.com
> > Cc: linux-integrity@vger.kernel.org; linux-security-module@vger.kernel.org;
> > linux-kernel@vger.kernel.org; Silviu Vlasceanu
> > <Silviu.Vlasceanu@huawei.com>; stable@vger.kernel.org
> > Subject: Re: [PATCH v2 2/8] ima: Switch to ima_hash_algo for boot
> > aggregate
> > 
> > On Thu, 2020-02-06 at 09:36 +0000, Roberto Sassu wrote:
> > > > Hi Roberto,
> > > >
> > > > On Wed, 2020-02-05 at 11:33 +0100, Roberto Sassu wrote:
> > > >
> > > > <snip>
> > > >
> > > > > Reported-by: Jerry Snitselaar <jsnitsel@redhat.com>
> > > > > Suggested-by: James Bottomley
> > > > <James.Bottomley@HansenPartnership.com>
> > > > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > > > > Cc: stable@vger.kernel.org
> > > >
> > > > Cc'ing stable resulted in Sasha's automated message.  If you're going
> > > > to Cc stable, then please include the stable kernel release (e.g. Cc:
> > > > stable@vger.kernel.org # v5.3).  Also please include a "Fixes" tag.
> > > >  Normally only bug fixes are backported.
> > >
> > > Ok, will add the kernel version. I also thought which commit I should
> > > mention in the Fixes tag. IMA always read the SHA1 bank from the
> > > beginning. I could mention the patch that introduces the new API
> > > to read other banks, but I'm not sure. What do you think?
> > 
> > This patch is dependent on nr_allocated_banks.  Please try applying
> > this patch to the earliest stable kernel with the commit that
> > introduces nr_allocated_banks and test to make sure it works properly.
> 
> It also depends on 879b589210a9 ("tpm: retrieve digest size of unknown"
> algorithms with PCR read") which exported the mapping between TPM
> algorithm ID and crypto ID, and changed the definition of tpm_pcr_read()
> to read non-SHA1 PCR banks. It requires many patches, so backporting it
> is not a trivial task. I think the earliest kernel this patch can be backported to
> is 5.1.

Agreed.  Thank you for checking.

Mimi

RE: [PATCH v2 3/8] ima: Evaluate error in init_ima()

[Roberto Sassu]

> -----Original Message-----
> From: Roberto Sassu
> Sent: Wednesday, February 5, 2020 11:33 AM
> To: zohar@linux.ibm.com; James.Bottomley@HansenPartnership.com;
> jarkko.sakkinen@linux.intel.com
> Cc: linux-integrity@vger.kernel.org; linux-security-module@vger.kernel.org;
> linux-kernel@vger.kernel.org; Silviu Vlasceanu
> <Silviu.Vlasceanu@huawei.com>; Roberto Sassu
> <roberto.sassu@huawei.com>
> Subject: [PATCH v2 3/8] ima: Evaluate error in init_ima()
> 
> Evaluate error in init_ima() before register_blocking_lsm_notifier() and
> return if not zero.
> 
> Fixes: b16942455193 ("ima: use the lsm policy update notifier")
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

Add in CC stable@vger.kernel.org.

Roberto

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

> ---
>  security/integrity/ima/ima_main.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/security/integrity/ima/ima_main.c
> b/security/integrity/ima/ima_main.c
> index d7e987baf127..a16c148ed90d 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -738,6 +738,9 @@ static int __init init_ima(void)
>  		error = ima_init();
>  	}
> 
> +	if (error)
> +		return error;
> +
>  	error = register_blocking_lsm_notifier(&ima_lsm_policy_notifier);
>  	if (error)
>  		pr_warn("Couldn't register LSM notifier, error %d\n", error);
> --
> 2.17.1