linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] ima: use the IMA configured hash algo to calculate the boot aggregate
@ 2020-01-27 16:01 Mimi Zohar
  2020-01-27 16:01 ` [PATCH 2/2] ima: support calculating the boot_aggregate based on different TPM banks Mimi Zohar
                   ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Mimi Zohar @ 2020-01-27 16:01 UTC (permalink / raw)
  To: linux-integrity
  Cc: Jerry Snitselaar, James Bottomley, linux-kernel, Mimi Zohar

The boot aggregate is a cumulative SHA1 hash over TPM registers 0 - 7.
NIST has depreciated the usage of SHA1 in most instances.  Instead of
continuing to use SHA1 to calculate the boot_aggregate, use the
configured IMA default hash algorithm.

Although the IMA measurement list boot_aggregate template data contains
the hash algorithm followed by the digest, allowing verifiers (e.g.
attesttaion servers) to calculate and verify the boot_aggregate, the
verifiers might not have the knowledge of what constitutes a good value
based on a different hash algorithm.

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 security/integrity/ima/ima_init.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 195cb4079b2b..b1b334fe0db5 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,14 +51,14 @@ 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];
 
 	if (ima_tpm_chip) {
 		result = ima_calc_boot_aggregate(&hash.hdr);
-- 
2.7.5


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/2] ima: support calculating the boot_aggregate based on different TPM banks
  2020-01-27 16:01 [PATCH 1/2] ima: use the IMA configured hash algo to calculate the boot aggregate Mimi Zohar
@ 2020-01-27 16:01 ` Mimi Zohar
  2020-01-27 16:50   ` Lakshmi Ramasubramanian
  2020-01-28 14:19   ` Roberto Sassu
  2020-01-27 17:38 ` [PATCH 1/2] ima: use the IMA configured hash algo to calculate the boot aggregate Roberto Sassu
  2020-01-27 20:49 ` Jerry Snitselaar
  2 siblings, 2 replies; 20+ messages in thread
From: Mimi Zohar @ 2020-01-27 16:01 UTC (permalink / raw)
  To: linux-integrity
  Cc: Jerry Snitselaar, James Bottomley, linux-kernel, Mimi Zohar

Calculating the boot_aggregate attempts to read the TPM SHA1 bank,
assuming it is always enabled.  With TPM 2.0 hash agility, TPM chips
could support multiple TPM PCR banks, allowing firmware to configure and
enable different banks.

Instead of hard coding the TPM 2.0 bank hash algorithm used for calculating
the boot-aggregate, see if the configured IMA_DEFAULT_HASH algorithm is
an allocated TPM bank, otherwise use the first allocated TPM bank.

For TPM 1.2 SHA1 is the only supported hash algorithm.

Reported-by: Jerry Snitselaar <jsnitsel@redhat.com>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 security/integrity/ima/ima_crypto.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 7967a6904851..b1b26d61f174 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -656,8 +656,25 @@ static void __init ima_pcrread(u32 idx, struct tpm_digest *d)
 		pr_err("Error Communicating to TPM chip\n");
 }
 
+/* tpm2_hash_map is the same as defined in tpm2-cmd.c and trusted_tpm2.c */
+static struct tpm2_hash tpm2_hash_map[] = {
+	{HASH_ALGO_SHA1, TPM_ALG_SHA1},
+	{HASH_ALGO_SHA256, TPM_ALG_SHA256},
+	{HASH_ALGO_SHA384, TPM_ALG_SHA384},
+	{HASH_ALGO_SHA512, TPM_ALG_SHA512},
+	{HASH_ALGO_SM3_256, TPM_ALG_SM3_256},
+};
+
 /*
- * Calculate the boot aggregate hash
+ * The boot_aggregate is a cumulative hash over TPM registers 0 - 7.  With
+ * TPM 2.0 hash agility, TPM chips could support multiple TPM PCR banks,
+ * allowing firmware to configure and enable different banks.
+ *
+ * Instead of hard coding the TPM bank hash algorithm used for calculating
+ * the boot-aggregate, see if the configured IMA_DEFAULT_HASH algorithm is
+ * an allocated TPM bank, otherwise use the first allocated TPM bank.
+ *
+ * For TPM 1.2 SHA1 is the only hash algorithm.
  */
 static int __init ima_calc_boot_aggregate_tfm(char *digest,
 					      struct crypto_shash *tfm)
@@ -673,6 +690,24 @@ static int __init ima_calc_boot_aggregate_tfm(char *digest,
 	if (rc != 0)
 		return rc;
 
+	for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
+		if (tpm2_hash_map[i].crypto_id == ima_hash_algo) {
+			d.alg_id = tpm2_hash_map[i].tpm_id;
+			break;
+		}
+	}
+
+	for (i = 0; i < ima_tpm_chip->nr_allocated_banks; i++) {
+		if (ima_tpm_chip->allocated_banks[i].alg_id == d.alg_id)
+			break;
+	}
+
+	if (i == ima_tpm_chip->nr_allocated_banks)
+		d.alg_id = ima_tpm_chip->allocated_banks[0].alg_id;
+
+	pr_info("Calculating the boot-aggregregate, reading TPM PCR bank: %04x",
+		d.alg_id);
+
 	/* cumulative sha1 over tpm registers 0-7 */
 	for (i = TPM_PCR0; i < TPM_PCR8; i++) {
 		ima_pcrread(i, &d);
-- 
2.7.5


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] ima: support calculating the boot_aggregate based on different TPM banks
  2020-01-27 16:01 ` [PATCH 2/2] ima: support calculating the boot_aggregate based on different TPM banks Mimi Zohar
@ 2020-01-27 16:50   ` Lakshmi Ramasubramanian
  2020-01-27 18:01     ` Mimi Zohar
  2020-01-27 20:55     ` Ken Goldman
  2020-01-28 14:19   ` Roberto Sassu
  1 sibling, 2 replies; 20+ messages in thread
From: Lakshmi Ramasubramanian @ 2020-01-27 16:50 UTC (permalink / raw)
  To: Mimi Zohar, linux-integrity
  Cc: Jerry Snitselaar, James Bottomley, linux-kernel

On 1/27/2020 8:01 AM, Mimi Zohar wrote:

> +
> +	for (i = 0; i < ima_tpm_chip->nr_allocated_banks; i++) {
> +		if (ima_tpm_chip->allocated_banks[i].alg_id == d.alg_id)
> +			break;
> +	}
> +
> +	if (i == ima_tpm_chip->nr_allocated_banks)
> +		d.alg_id = ima_tpm_chip->allocated_banks[0].alg_id;
> +

Can the number of allocated banks (ima_tpm_chip->nr_allocated_banks) be 
zero? Should that be checked before accessing "allocated_banks"?

  -lakshmi

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH 1/2] ima: use the IMA configured hash algo to calculate the boot aggregate
  2020-01-27 16:01 [PATCH 1/2] ima: use the IMA configured hash algo to calculate the boot aggregate Mimi Zohar
  2020-01-27 16:01 ` [PATCH 2/2] ima: support calculating the boot_aggregate based on different TPM banks Mimi Zohar
@ 2020-01-27 17:38 ` Roberto Sassu
  2020-01-27 18:16   ` Mimi Zohar
  2020-01-27 20:49 ` Jerry Snitselaar
  2 siblings, 1 reply; 20+ messages in thread
From: Roberto Sassu @ 2020-01-27 17:38 UTC (permalink / raw)
  To: Mimi Zohar, linux-integrity
  Cc: Jerry Snitselaar, James Bottomley, linux-kernel

> -----Original Message-----
> From: linux-integrity-owner@vger.kernel.org [mailto:linux-integrity-
> owner@vger.kernel.org] On Behalf Of Mimi Zohar
> Sent: Monday, January 27, 2020 5:02 PM
> To: linux-integrity@vger.kernel.org
> Cc: Jerry Snitselaar <jsnitsel@redhat.com>; James Bottomley
> <James.Bottomley@HansenPartnership.com>; linux-
> kernel@vger.kernel.org; Mimi Zohar <zohar@linux.ibm.com>
> Subject: [PATCH 1/2] ima: use the IMA configured hash algo to calculate the
> boot aggregate

Hi Mimi

I did a similar change (patch 8/8) in the patch set I just sent. The patch is simpler,
as it reuses the data structures I introduced in the previous patches. Let me know
if I can keep this part in my patch set or I should remove it.

Thanks

Roberto

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] ima: support calculating the boot_aggregate based on different TPM banks
  2020-01-27 16:50   ` Lakshmi Ramasubramanian
@ 2020-01-27 18:01     ` Mimi Zohar
  2020-01-27 20:55     ` Ken Goldman
  1 sibling, 0 replies; 20+ messages in thread
From: Mimi Zohar @ 2020-01-27 18:01 UTC (permalink / raw)
  To: Lakshmi Ramasubramanian, linux-integrity
  Cc: Jerry Snitselaar, James Bottomley, linux-kernel

On Mon, 2020-01-27 at 08:50 -0800, Lakshmi Ramasubramanian wrote:
> On 1/27/2020 8:01 AM, Mimi Zohar wrote:
> 
> > +
> > +	for (i = 0; i < ima_tpm_chip->nr_allocated_banks; i++) {
> > +		if (ima_tpm_chip->allocated_banks[i].alg_id == d.alg_id)
> > +			break;
> > +	}
> > +
> > +	if (i == ima_tpm_chip->nr_allocated_banks)
> > +		d.alg_id = ima_tpm_chip->allocated_banks[0].alg_id;
> > +
> 
> Can the number of allocated banks (ima_tpm_chip->nr_allocated_banks) be 
> zero? Should that be checked before accessing "allocated_banks"?

Yes, that might be the true, but I think the solution is not fixing
the problem here, but when ima_tpm_chip is set in ima_init().
tpm_default_chip() should be modified to return a TPM with at least
one bank enabled; and ima_init() needs to go into TPM-bypass mode if
there isn't.

Can anyone look into this please?

Mimi


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] ima: use the IMA configured hash algo to calculate the boot aggregate
  2020-01-27 17:38 ` [PATCH 1/2] ima: use the IMA configured hash algo to calculate the boot aggregate Roberto Sassu
@ 2020-01-27 18:16   ` Mimi Zohar
  2020-01-27 18:35     ` Mimi Zohar
  0 siblings, 1 reply; 20+ messages in thread
From: Mimi Zohar @ 2020-01-27 18:16 UTC (permalink / raw)
  To: Roberto Sassu, linux-integrity
  Cc: Jerry Snitselaar, James Bottomley, linux-kernel

On Mon, 2020-01-27 at 17:38 +0000, Roberto Sassu wrote:
> > -----Original Message-----
> > From: linux-integrity-owner@vger.kernel.org [mailto:linux-integrity-
> > owner@vger.kernel.org] On Behalf Of Mimi Zohar
> > Sent: Monday, January 27, 2020 5:02 PM
> > To: linux-integrity@vger.kernel.org
> > Cc: Jerry Snitselaar <jsnitsel@redhat.com>; James Bottomley
> > <James.Bottomley@HansenPartnership.com>; linux-
> > kernel@vger.kernel.org; Mimi Zohar <zohar@linux.ibm.com>
> > Subject: [PATCH 1/2] ima: use the IMA configured hash algo to calculate the
> > boot aggregate
> 
> Hi Mimi
> 
> I did a similar change (patch 8/8) in the patch set I just sent. The patch is simpler,
> as it reuses the data structures I introduced in the previous patches. Let me know
> if I can keep this part in my patch set or I should remove it.

Only 2/2 "ima: support calculating the boot_aggregate based on
different TPM banks" is really needed to address Jerry's bug report.
 Let's review your patch set before making any decisions about 1/2
"ima: use the IMA configured hash algo to calculate the boot
aggregate".

thanks,

Mimi


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] ima: use the IMA configured hash algo to calculate the boot aggregate
  2020-01-27 18:16   ` Mimi Zohar
@ 2020-01-27 18:35     ` Mimi Zohar
  0 siblings, 0 replies; 20+ messages in thread
From: Mimi Zohar @ 2020-01-27 18:35 UTC (permalink / raw)
  To: Roberto Sassu, linux-integrity
  Cc: Jerry Snitselaar, James Bottomley, linux-kernel

On Mon, 2020-01-27 at 13:16 -0500, Mimi Zohar wrote:
> On Mon, 2020-01-27 at 17:38 +0000, Roberto Sassu wrote:
> > > -----Original Message-----
> > > From: linux-integrity-owner@vger.kernel.org [mailto:linux-integrity-
> > > owner@vger.kernel.org] On Behalf Of Mimi Zohar
> > > Sent: Monday, January 27, 2020 5:02 PM
> > > To: linux-integrity@vger.kernel.org
> > > Cc: Jerry Snitselaar <jsnitsel@redhat.com>; James Bottomley
> > > <James.Bottomley@HansenPartnership.com>; linux-
> > > kernel@vger.kernel.org; Mimi Zohar <zohar@linux.ibm.com>
> > > Subject: [PATCH 1/2] ima: use the IMA configured hash algo to calculate the
> > > boot aggregate
> > 
> > Hi Mimi
> > 
> > I did a similar change (patch 8/8) in the patch set I just sent. The patch is simpler,
> > as it reuses the data structures I introduced in the previous patches. Let me know
> > if I can keep this part in my patch set or I should remove it.
> 
> Only 2/2 "ima: support calculating the boot_aggregate based on
> different TPM banks" is really needed to address Jerry's bug report.
>  Let's review your patch set before making any decisions about 1/2
> "ima: use the IMA configured hash algo to calculate the boot
> aggregate".

To be more precise, we need to be able to backport the bug fix.  So
the change needs to be independent of anything you're defining now.
 Changes/improvements can be made on top of the bug fix.

thanks,

Mimi


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] ima: use the IMA configured hash algo to calculate the boot aggregate
  2020-01-27 16:01 [PATCH 1/2] ima: use the IMA configured hash algo to calculate the boot aggregate Mimi Zohar
  2020-01-27 16:01 ` [PATCH 2/2] ima: support calculating the boot_aggregate based on different TPM banks Mimi Zohar
  2020-01-27 17:38 ` [PATCH 1/2] ima: use the IMA configured hash algo to calculate the boot aggregate Roberto Sassu
@ 2020-01-27 20:49 ` Jerry Snitselaar
  2020-01-27 21:31   ` Mimi Zohar
  2 siblings, 1 reply; 20+ messages in thread
From: Jerry Snitselaar @ 2020-01-27 20:49 UTC (permalink / raw)
  To: Mimi Zohar; +Cc: linux-integrity, James Bottomley, linux-kernel

On Mon Jan 27 20, Mimi Zohar wrote:
>The boot aggregate is a cumulative SHA1 hash over TPM registers 0 - 7.
>NIST has depreciated the usage of SHA1 in most instances.  Instead of
>continuing to use SHA1 to calculate the boot_aggregate, use the
>configured IMA default hash algorithm.
>
>Although the IMA measurement list boot_aggregate template data contains
>the hash algorithm followed by the digest, allowing verifiers (e.g.
>attesttaion servers) to calculate and verify the boot_aggregate, the
>verifiers might not have the knowledge of what constitutes a good value
>based on a different hash algorithm.
>
>Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
>---
> security/integrity/ima/ima_init.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
>index 195cb4079b2b..b1b334fe0db5 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,14 +51,14 @@ 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];
>
> 	if (ima_tpm_chip) {
> 		result = ima_calc_boot_aggregate(&hash.hdr);
>-- 
>2.7.5
>

Tested the patches on the Dell and no longer spits out the error messages on boot.
/sys/kernel/security/ima/ascii_runtime_measurements shows the boot aggregate.

Is there something else I should look at to verify it is functioning properly?

Regards,
Jerry


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] ima: support calculating the boot_aggregate based on different TPM banks
  2020-01-27 16:50   ` Lakshmi Ramasubramanian
  2020-01-27 18:01     ` Mimi Zohar
@ 2020-01-27 20:55     ` Ken Goldman
  1 sibling, 0 replies; 20+ messages in thread
From: Ken Goldman @ 2020-01-27 20:55 UTC (permalink / raw)
  To: Lakshmi Ramasubramanian, Mimi Zohar, linux-integrity; +Cc: linux-kernel

On 1/27/2020 11:50 AM, Lakshmi Ramasubramanian wrote:
> Can the number of allocated banks (ima_tpm_chip->nr_allocated_banks) be 
> zero? Should that be checked before accessing "allocated_banks"?

Summary:

It's unlikely that Linux on a PC will encounter a TPM without PCR 10.

It is likely that PCR 10 will be only SHA-256, that there will be no 
SHA-1 PCR 10.

~~

In theory:

Yes, one could have a TPM with no allocated banks.

In practice:

A PC Client TPM must have at least one bank with PCR 0 and PCR 17.

Some other TPMs, like automotive or embedded, may be different.

Most platforms will be designed to meet Windows requirements, which will 
have AFAIK at least one bank of 24 PCRs.

The TPM specification permits allocation of partial banks.  In theory, 
one could encounter a TPM with e.g., PCR 0-7 but not PCR 10.

In practice, AFAIK the hardware TPMs implement only full banks. 
Platform firmware allocates full banks.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] ima: use the IMA configured hash algo to calculate the boot aggregate
  2020-01-27 20:49 ` Jerry Snitselaar
@ 2020-01-27 21:31   ` Mimi Zohar
  2020-01-29  8:30     ` Petr Vorel
  0 siblings, 1 reply; 20+ messages in thread
From: Mimi Zohar @ 2020-01-27 21:31 UTC (permalink / raw)
  To: Jerry Snitselaar; +Cc: linux-integrity, James Bottomley, linux-kernel

On Mon, 2020-01-27 at 13:49 -0700, Jerry Snitselaar wrote:
> On Mon Jan 27 20, Mimi Zohar wrote:
> >The boot aggregate is a cumulative SHA1 hash over TPM registers 0 - 7.
> >NIST has depreciated the usage of SHA1 in most instances.  Instead of
> >continuing to use SHA1 to calculate the boot_aggregate, use the
> >configured IMA default hash algorithm.
> >
> >Although the IMA measurement list boot_aggregate template data contains
> >the hash algorithm followed by the digest, allowing verifiers (e.g.
> >attesttaion servers) to calculate and verify the boot_aggregate, the
> >verifiers might not have the knowledge of what constitutes a good value
> >based on a different hash algorithm.
> >
> >Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> >---
> > security/integrity/ima/ima_init.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> >diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
> >index 195cb4079b2b..b1b334fe0db5 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,14 +51,14 @@ 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];
> >
> > 	if (ima_tpm_chip) {
> > 		result = ima_calc_boot_aggregate(&hash.hdr);
> >-- 
> >2.7.5
> >
> 
> Tested the patches on the Dell and no longer spits out the error messages on boot.
> /sys/kernel/security/ima/ascii_runtime_measurements shows the boot aggregate.
> 
> Is there something else I should look at to verify it is functioning properly?

The original LTP ima_boot_aggregate.c test needed to be updated to
support TPM 2.0 before this change.  For TPM 2.0, the PCRs are not
exported.  With this change, the kernel could be reading PCRs from a
TPM bank other than SHA1 and calculating the boot_aggregate based on a
different hash algorithm as well.  I'm not sure how a remote verifier
would know which TPM bank was read, when calculating the boot-
aggregate.

At the moment, the only test would be to make sure that the LTP test
still works for TPM 1.2 properly.

Mimi


^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH 2/2] ima: support calculating the boot_aggregate based on different TPM banks
  2020-01-27 16:01 ` [PATCH 2/2] ima: support calculating the boot_aggregate based on different TPM banks Mimi Zohar
  2020-01-27 16:50   ` Lakshmi Ramasubramanian
@ 2020-01-28 14:19   ` Roberto Sassu
  2020-01-28 15:40     ` Mimi Zohar
  1 sibling, 1 reply; 20+ messages in thread
From: Roberto Sassu @ 2020-01-28 14:19 UTC (permalink / raw)
  To: Mimi Zohar, linux-integrity
  Cc: Jerry Snitselaar, James Bottomley, linux-kernel, Silviu Vlasceanu

> -----Original Message-----
> From: linux-integrity-owner@vger.kernel.org [mailto:linux-integrity-
> owner@vger.kernel.org] On Behalf Of Mimi Zohar
> Sent: Monday, January 27, 2020 5:02 PM
> To: linux-integrity@vger.kernel.org
> Cc: Jerry Snitselaar <jsnitsel@redhat.com>; James Bottomley
> <James.Bottomley@HansenPartnership.com>; linux-
> kernel@vger.kernel.org; Mimi Zohar <zohar@linux.ibm.com>
> Subject: [PATCH 2/2] ima: support calculating the boot_aggregate based on
> different TPM banks
> 
> Calculating the boot_aggregate attempts to read the TPM SHA1 bank,
> assuming it is always enabled.  With TPM 2.0 hash agility, TPM chips
> could support multiple TPM PCR banks, allowing firmware to configure and
> enable different banks.
> 
> Instead of hard coding the TPM 2.0 bank hash algorithm used for calculating
> the boot-aggregate, see if the configured IMA_DEFAULT_HASH algorithm is
> an allocated TPM bank, otherwise use the first allocated TPM bank.
> 
> For TPM 1.2 SHA1 is the only supported hash algorithm.
> 
> Reported-by: Jerry Snitselaar <jsnitsel@redhat.com>
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
>  security/integrity/ima/ima_crypto.c | 37
> ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/security/integrity/ima/ima_crypto.c
> b/security/integrity/ima/ima_crypto.c
> index 7967a6904851..b1b26d61f174 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -656,8 +656,25 @@ static void __init ima_pcrread(u32 idx, struct
> tpm_digest *d)
>  		pr_err("Error Communicating to TPM chip\n");
>  }
> 
> +/* tpm2_hash_map is the same as defined in tpm2-cmd.c and
> trusted_tpm2.c */
> +static struct tpm2_hash tpm2_hash_map[] = {
> +	{HASH_ALGO_SHA1, TPM_ALG_SHA1},
> +	{HASH_ALGO_SHA256, TPM_ALG_SHA256},
> +	{HASH_ALGO_SHA384, TPM_ALG_SHA384},
> +	{HASH_ALGO_SHA512, TPM_ALG_SHA512},
> +	{HASH_ALGO_SM3_256, TPM_ALG_SM3_256},
> +};
> +
>  /*
> - * Calculate the boot aggregate hash
> + * The boot_aggregate is a cumulative hash over TPM registers 0 - 7.  With
> + * TPM 2.0 hash agility, TPM chips could support multiple TPM PCR banks,
> + * allowing firmware to configure and enable different banks.
> + *
> + * Instead of hard coding the TPM bank hash algorithm used for calculating
> + * the boot-aggregate, see if the configured IMA_DEFAULT_HASH
> algorithm is
> + * an allocated TPM bank, otherwise use the first allocated TPM bank.
> + *
> + * For TPM 1.2 SHA1 is the only hash algorithm.
>   */
>  static int __init ima_calc_boot_aggregate_tfm(char *digest,
>  					      struct crypto_shash *tfm)
> @@ -673,6 +690,24 @@ static int __init ima_calc_boot_aggregate_tfm(char
> *digest,
>  	if (rc != 0)
>  		return rc;
> 
> +	for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
> +		if (tpm2_hash_map[i].crypto_id == ima_hash_algo) {

It is not necessary to define a new map. ima_tpm_chip->allocated_banks
has a crypto_id field.

> +			d.alg_id = tpm2_hash_map[i].tpm_id;
> +			break;
> +		}
> +	}
> +
> +	for (i = 0; i < ima_tpm_chip->nr_allocated_banks; i++) {
> +		if (ima_tpm_chip->allocated_banks[i].alg_id == d.alg_id)
> +			break;
> +	}
> +
> +	if (i == ima_tpm_chip->nr_allocated_banks)
> +		d.alg_id = ima_tpm_chip->allocated_banks[0].alg_id;

This code assumes that the algorithm used to calculate boot_aggregate and
the algorithm of the PCR bank can be different. I don't know if it is possible to
communicate to the verifier which bank has been selected (it depends on
the local configuration).

In my opinion the safest approach would be to use the same algorithm for the
digest and the PCR bank. If you agree to this, then the code above must be
moved to ima_calc_boot_aggregate() so that the algorithm of the selected
PCR bank can be passed to ima_alloc_tfm().

The selected PCR bank might be not the first, if the algorithm is unknown to
the crypto subsystem.

> +	pr_info("Calculating the boot-aggregregate, reading TPM PCR

Typo.

Roberto

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] ima: support calculating the boot_aggregate based on different TPM banks
  2020-01-28 14:19   ` Roberto Sassu
@ 2020-01-28 15:40     ` Mimi Zohar
  2020-01-28 16:31       ` Roberto Sassu
  2020-01-29 23:20       ` Mimi Zohar
  0 siblings, 2 replies; 20+ messages in thread
From: Mimi Zohar @ 2020-01-28 15:40 UTC (permalink / raw)
  To: Roberto Sassu, linux-integrity
  Cc: Jerry Snitselaar, James Bottomley, linux-kernel, Silviu Vlasceanu

On Tue, 2020-01-28 at 14:19 +0000, Roberto Sassu wrote:
> > -----Original Message-----
> > From: linux-integrity-owner@vger.kernel.org [mailto:linux-integrity-
> > owner@vger.kernel.org] On Behalf Of Mimi Zohar
> > Sent: Monday, January 27, 2020 5:02 PM
> > To: linux-integrity@vger.kernel.org
> > Cc: Jerry Snitselaar <jsnitsel@redhat.com>; James Bottomley
> > <James.Bottomley@HansenPartnership.com>; linux-
> > kernel@vger.kernel.org; Mimi Zohar <zohar@linux.ibm.com>
> > Subject: [PATCH 2/2] ima: support calculating the boot_aggregate based on
> > different TPM banks
> > 
> > Calculating the boot_aggregate attempts to read the TPM SHA1 bank,
> > assuming it is always enabled.  With TPM 2.0 hash agility, TPM chips
> > could support multiple TPM PCR banks, allowing firmware to configure and
> > enable different banks.
> > 
> > Instead of hard coding the TPM 2.0 bank hash algorithm used for calculating
> > the boot-aggregate, see if the configured IMA_DEFAULT_HASH algorithm is
> > an allocated TPM bank, otherwise use the first allocated TPM bank.
> > 
> > For TPM 1.2 SHA1 is the only supported hash algorithm.
> > 
> > Reported-by: Jerry Snitselaar <jsnitsel@redhat.com>
> > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> > ---
> >  security/integrity/ima/ima_crypto.c | 37
> > ++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 36 insertions(+), 1 deletion(-)
> > 
> > diff --git a/security/integrity/ima/ima_crypto.c
> > b/security/integrity/ima/ima_crypto.c
> > index 7967a6904851..b1b26d61f174 100644
> > --- a/security/integrity/ima/ima_crypto.c
> > +++ b/security/integrity/ima/ima_crypto.c
> > @@ -656,8 +656,25 @@ static void __init ima_pcrread(u32 idx, struct
> > tpm_digest *d)
> >  		pr_err("Error Communicating to TPM chip\n");
> >  }
> > 
> > +/* tpm2_hash_map is the same as defined in tpm2-cmd.c and
> > trusted_tpm2.c */
> > +static struct tpm2_hash tpm2_hash_map[] = {
> > +	{HASH_ALGO_SHA1, TPM_ALG_SHA1},
> > +	{HASH_ALGO_SHA256, TPM_ALG_SHA256},
> > +	{HASH_ALGO_SHA384, TPM_ALG_SHA384},
> > +	{HASH_ALGO_SHA512, TPM_ALG_SHA512},
> > +	{HASH_ALGO_SM3_256, TPM_ALG_SM3_256},
> > +};
> > +
> >  /*
> > - * Calculate the boot aggregate hash
> > + * The boot_aggregate is a cumulative hash over TPM registers 0 - 7.  With
> > + * TPM 2.0 hash agility, TPM chips could support multiple TPM PCR banks,
> > + * allowing firmware to configure and enable different banks.
> > + *
> > + * Instead of hard coding the TPM bank hash algorithm used for calculating
> > + * the boot-aggregate, see if the configured IMA_DEFAULT_HASH
> > algorithm is
> > + * an allocated TPM bank, otherwise use the first allocated TPM bank.
> > + *
> > + * For TPM 1.2 SHA1 is the only hash algorithm.
> >   */
> >  static int __init ima_calc_boot_aggregate_tfm(char *digest,
> >  					      struct crypto_shash *tfm)
> > @@ -673,6 +690,24 @@ static int __init ima_calc_boot_aggregate_tfm(char
> > *digest,
> >  	if (rc != 0)
> >  		return rc;
> > 
> > +	for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
> > +		if (tpm2_hash_map[i].crypto_id == ima_hash_algo) {
> 
> It is not necessary to define a new map. ima_tpm_chip->allocated_banks
> has a crypto_id field.

Ok, thanks.

> 
> > +			d.alg_id = tpm2_hash_map[i].tpm_id;
> > +			break;
> > +		}
> > +	}
> > +
> > +	for (i = 0; i < ima_tpm_chip->nr_allocated_banks; i++) {
> > +		if (ima_tpm_chip->allocated_banks[i].alg_id == d.alg_id)
> > +			break;
> > +	}
> > +
> > +	if (i == ima_tpm_chip->nr_allocated_banks)
> > +		d.alg_id = ima_tpm_chip->allocated_banks[0].alg_id;
> 
> This code assumes that the algorithm used to calculate boot_aggregate and
> the algorithm of the PCR bank can be different. I don't know if it is possible to
> communicate to the verifier which bank has been selected (it depends on
> the local configuration).

Agreed, but defaulting to the first bank would only happen if the IMA
default hash algorithm is not a configured TPM algorithm.

> 
> In my opinion the safest approach would be to use the same algorithm for the
> digest and the PCR bank. If you agree to this, then the code above must be
> moved to ima_calc_boot_aggregate() so that the algorithm of the selected
> PCR bank can be passed to ima_alloc_tfm().

Using the same hash algorithm, preferably the IMA hash default
algorithm, for reading the TPM PCR bank and calculating the
boot_aggregate makes sense.

> 
> The selected PCR bank might be not the first, if the algorithm is unknown to
> the crypto subsystem.

It sounds like you're suggesting finding a common configured hash
algorithm between the TPM and the kernel. 

> 
> > +	pr_info("Calculating the boot-aggregregate, reading TPM PCR
> 
> Typo.

thanks

Mimi


^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH 2/2] ima: support calculating the boot_aggregate based on different TPM banks
  2020-01-28 15:40     ` Mimi Zohar
@ 2020-01-28 16:31       ` Roberto Sassu
  2020-01-29 23:20       ` Mimi Zohar
  1 sibling, 0 replies; 20+ messages in thread
From: Roberto Sassu @ 2020-01-28 16:31 UTC (permalink / raw)
  To: Mimi Zohar, linux-integrity
  Cc: Jerry Snitselaar, James Bottomley, linux-kernel, Silviu Vlasceanu

> -----Original Message-----
> From: Mimi Zohar [mailto:zohar@linux.ibm.com]
> Sent: Tuesday, January 28, 2020 4:41 PM
> To: Roberto Sassu <roberto.sassu@huawei.com>; linux-
> integrity@vger.kernel.org
> Cc: Jerry Snitselaar <jsnitsel@redhat.com>; James Bottomley
> <James.Bottomley@HansenPartnership.com>; linux-
> kernel@vger.kernel.org; Silviu Vlasceanu <Silviu.Vlasceanu@huawei.com>
> Subject: Re: [PATCH 2/2] ima: support calculating the boot_aggregate based
> on different TPM banks
> 
> On Tue, 2020-01-28 at 14:19 +0000, Roberto Sassu wrote:
> > > -----Original Message-----
> > > From: linux-integrity-owner@vger.kernel.org [mailto:linux-integrity-
> > > owner@vger.kernel.org] On Behalf Of Mimi Zohar
> > > Sent: Monday, January 27, 2020 5:02 PM
> > > To: linux-integrity@vger.kernel.org
> > > Cc: Jerry Snitselaar <jsnitsel@redhat.com>; James Bottomley
> > > <James.Bottomley@HansenPartnership.com>; linux-
> > > kernel@vger.kernel.org; Mimi Zohar <zohar@linux.ibm.com>
> > > Subject: [PATCH 2/2] ima: support calculating the boot_aggregate based
> on
> > > different TPM banks
> > >
> > > Calculating the boot_aggregate attempts to read the TPM SHA1 bank,
> > > assuming it is always enabled.  With TPM 2.0 hash agility, TPM chips
> > > could support multiple TPM PCR banks, allowing firmware to configure
> and
> > > enable different banks.
> > >
> > > Instead of hard coding the TPM 2.0 bank hash algorithm used for
> calculating
> > > the boot-aggregate, see if the configured IMA_DEFAULT_HASH
> algorithm is
> > > an allocated TPM bank, otherwise use the first allocated TPM bank.
> > >
> > > For TPM 1.2 SHA1 is the only supported hash algorithm.
> > >
> > > Reported-by: Jerry Snitselaar <jsnitsel@redhat.com>
> > > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> > > ---
> > >  security/integrity/ima/ima_crypto.c | 37
> > > ++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 36 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/security/integrity/ima/ima_crypto.c
> > > b/security/integrity/ima/ima_crypto.c
> > > index 7967a6904851..b1b26d61f174 100644
> > > --- a/security/integrity/ima/ima_crypto.c
> > > +++ b/security/integrity/ima/ima_crypto.c
> > > @@ -656,8 +656,25 @@ static void __init ima_pcrread(u32 idx, struct
> > > tpm_digest *d)
> > >  		pr_err("Error Communicating to TPM chip\n");
> > >  }
> > >
> > > +/* tpm2_hash_map is the same as defined in tpm2-cmd.c and
> > > trusted_tpm2.c */
> > > +static struct tpm2_hash tpm2_hash_map[] = {
> > > +	{HASH_ALGO_SHA1, TPM_ALG_SHA1},
> > > +	{HASH_ALGO_SHA256, TPM_ALG_SHA256},
> > > +	{HASH_ALGO_SHA384, TPM_ALG_SHA384},
> > > +	{HASH_ALGO_SHA512, TPM_ALG_SHA512},
> > > +	{HASH_ALGO_SM3_256, TPM_ALG_SM3_256},
> > > +};
> > > +
> > >  /*
> > > - * Calculate the boot aggregate hash
> > > + * The boot_aggregate is a cumulative hash over TPM registers 0 - 7.
> With
> > > + * TPM 2.0 hash agility, TPM chips could support multiple TPM PCR
> banks,
> > > + * allowing firmware to configure and enable different banks.
> > > + *
> > > + * Instead of hard coding the TPM bank hash algorithm used for
> calculating
> > > + * the boot-aggregate, see if the configured IMA_DEFAULT_HASH
> > > algorithm is
> > > + * an allocated TPM bank, otherwise use the first allocated TPM bank.
> > > + *
> > > + * For TPM 1.2 SHA1 is the only hash algorithm.
> > >   */
> > >  static int __init ima_calc_boot_aggregate_tfm(char *digest,
> > >  					      struct crypto_shash *tfm)
> > > @@ -673,6 +690,24 @@ static int __init
> ima_calc_boot_aggregate_tfm(char
> > > *digest,
> > >  	if (rc != 0)
> > >  		return rc;
> > >
> > > +	for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
> > > +		if (tpm2_hash_map[i].crypto_id == ima_hash_algo) {
> >
> > It is not necessary to define a new map. ima_tpm_chip->allocated_banks
> > has a crypto_id field.
> 
> Ok, thanks.

When you send the new version, please include patch 1/8 of my patch set,
that sets crypto_id to HASH_ALGO__LAST if there is no mapping between
TPM ID and crypto ID.

Thanks

Roberto

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] ima: use the IMA configured hash algo to calculate the boot aggregate
  2020-01-27 21:31   ` Mimi Zohar
@ 2020-01-29  8:30     ` Petr Vorel
  2020-01-29 22:51       ` Mimi Zohar
  0 siblings, 1 reply; 20+ messages in thread
From: Petr Vorel @ 2020-01-29  8:30 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Jerry Snitselaar, linux-integrity, James Bottomley, linux-kernel

Hi Mimi,

Reviewed-by: Petr Vorel <pvorel@suse.cz>

> The original LTP ima_boot_aggregate.c test needed to be updated to
> support TPM 2.0 before this change.  For TPM 2.0, the PCRs are not
> exported.  With this change, the kernel could be reading PCRs from a
> TPM bank other than SHA1 and calculating the boot_aggregate based on a
> different hash algorithm as well.  I'm not sure how a remote verifier
> would know which TPM bank was read, when calculating the boot-
> aggregate.
Mimi, do you plan to do update LTP test?

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] ima: use the IMA configured hash algo to calculate the boot aggregate
  2020-01-29  8:30     ` Petr Vorel
@ 2020-01-29 22:51       ` Mimi Zohar
  2020-01-30  8:41         ` Petr Vorel
  2020-01-30 15:27         ` Roberto Sassu
  0 siblings, 2 replies; 20+ messages in thread
From: Mimi Zohar @ 2020-01-29 22:51 UTC (permalink / raw)
  To: Petr Vorel
  Cc: Jerry Snitselaar, linux-integrity, James Bottomley, linux-kernel,
	Roberto Sassu

On Wed, 2020-01-29 at 09:30 +0100, Petr Vorel wrote:
> Hi Mimi,
> 
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> 
> > The original LTP ima_boot_aggregate.c test needed to be updated to
> > support TPM 2.0 before this change.  For TPM 2.0, the PCRs are not
> > exported.  With this change, the kernel could be reading PCRs from a
> > TPM bank other than SHA1 and calculating the boot_aggregate based on a
> > different hash algorithm as well.  I'm not sure how a remote verifier
> > would know which TPM bank was read, when calculating the boot-
> > aggregate.
> Mimi, do you plan to do update LTP test?

In order to test Roberto's patches that calculates and extends the
different TPM banks with the appropriate hashes, we'll need some test
to verify that it is working properly.  As to whether this will be in
LTP or ima-evm-utils, I'm not sure.

Mimi


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] ima: support calculating the boot_aggregate based on different TPM banks
  2020-01-28 15:40     ` Mimi Zohar
  2020-01-28 16:31       ` Roberto Sassu
@ 2020-01-29 23:20       ` Mimi Zohar
  2020-01-30  7:31         ` James Bottomley
  1 sibling, 1 reply; 20+ messages in thread
From: Mimi Zohar @ 2020-01-29 23:20 UTC (permalink / raw)
  To: Roberto Sassu, linux-integrity
  Cc: Jerry Snitselaar, James Bottomley, linux-kernel, Silviu Vlasceanu

On Tue, 2020-01-28 at 10:40 -0500, Mimi Zohar wrote:
> > This code assumes that the algorithm used to calculate boot_aggregate and
> > the algorithm of the PCR bank can be different. I don't know if it is possible to
> > communicate to the verifier which bank has been selected (it depends on
> > the local configuration).
> 
> Agreed, but defaulting to the first bank would only happen if the IMA
> default hash algorithm is not a configured TPM algorithm.
> 
> > 
> > In my opinion the safest approach would be to use the same algorithm for the
> > digest and the PCR bank. If you agree to this, then the code above must be
> > moved to ima_calc_boot_aggregate() so that the algorithm of the selected
> > PCR bank can be passed to ima_alloc_tfm().
> 
> Using the same hash algorithm, preferably the IMA hash default
> algorithm, for reading the TPM PCR bank and calculating the
> boot_aggregate makes sense.
> 
> > 
> > The selected PCR bank might be not the first, if the algorithm is unknown to
> > the crypto subsystem.
> 
> It sounds like you're suggesting finding a common configured hash
> algorithm between the TPM and the kernel. 

I'd like to clarify the logic for the case when a common algorithm
does not exist.  None of the TPM allocated banks are known by the
kernel.  If the hash algorithm of the boot_aggregate represents not
only that of the digest included in the measurement list, but also of
the TPM PCR bank read, what should happen?  Should the boot aggregate
be 0's, like in the case when there isn't a TPM?

thanks,

Mimi


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] ima: support calculating the boot_aggregate based on different TPM banks
  2020-01-29 23:20       ` Mimi Zohar
@ 2020-01-30  7:31         ` James Bottomley
  0 siblings, 0 replies; 20+ messages in thread
From: James Bottomley @ 2020-01-30  7:31 UTC (permalink / raw)
  To: Mimi Zohar, Roberto Sassu, linux-integrity
  Cc: Jerry Snitselaar, linux-kernel, Silviu Vlasceanu

On Wed, 2020-01-29 at 18:20 -0500, Mimi Zohar wrote:
> On Tue, 2020-01-28 at 10:40 -0500, Mimi Zohar wrote:
> > > This code assumes that the algorithm used to calculate
> > > boot_aggregate and the algorithm of the PCR bank can be
> > > different. I don't know if it is possible to communicate to the
> > > verifier which bank has been selected (it depends on the local
> > > configuration).
> > 
> > Agreed, but defaulting to the first bank would only happen if the
> > IMA default hash algorithm is not a configured TPM algorithm.
> > 
> > > 
> > > In my opinion the safest approach would be to use the same
> > > algorithm for the digest and the PCR bank. If you agree to this,
> > > then the code above must be moved to ima_calc_boot_aggregate() so
> > > that the algorithm of the selected PCR bank can be passed to
> > > ima_alloc_tfm().
> > 
> > Using the same hash algorithm, preferably the IMA hash default
> > algorithm, for reading the TPM PCR bank and calculating the
> > boot_aggregate makes sense.
> > 
> > > 
> > > The selected PCR bank might be not the first, if the algorithm is
> > > unknown to the crypto subsystem.
> > 
> > It sounds like you're suggesting finding a common configured hash
> > algorithm between the TPM and the kernel. 
> 
> I'd like to clarify the logic for the case when a common algorithm
> does not exist.

I'm not sure we need to twist ourselves in knots over this.  The TCG
client platform for 2.0 mandates sha256, so we should be able to count
on it being present.  I'd be happy to treat the failure to find sha256
on TPM 2.0 as a fatal error, and the same for the failure to find sha1
on TPM 1.2.

>   None of the TPM allocated banks are known by the kernel.  If the
> hash algorithm of the boot_aggregate represents not only that of the
> digest included in the measurement list, but also of the TPM PCR bank
> read, what should happen?  Should the boot aggregate be 0's, like in
> the case when there isn't a TPM?

For TPM 1.2 sha1 is required and for TPM2 sha256 is.  I'd just force
search for those, based on the TPM version, and fail to use the TPM if
they're not found.  There should be a boot time and config option for
weird hashes which might be the only ones on BRIC embedded devices,
like Streebog or ZM2, but the clear default should be the TCG mandates
and it should be up to anything weird to cope with their own induced
problems and not make it part of the standard setup.

James


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] ima: use the IMA configured hash algo to calculate the boot aggregate
  2020-01-29 22:51       ` Mimi Zohar
@ 2020-01-30  8:41         ` Petr Vorel
  2020-01-30 15:27         ` Roberto Sassu
  1 sibling, 0 replies; 20+ messages in thread
From: Petr Vorel @ 2020-01-30  8:41 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Jerry Snitselaar, linux-integrity, James Bottomley, linux-kernel,
	Roberto Sassu

Hi Mimi,

> > > The original LTP ima_boot_aggregate.c test needed to be updated to
> > > support TPM 2.0 before this change.  For TPM 2.0, the PCRs are not
> > > exported.  With this change, the kernel could be reading PCRs from a
> > > TPM bank other than SHA1 and calculating the boot_aggregate based on a
> > > different hash algorithm as well.  I'm not sure how a remote verifier
> > > would know which TPM bank was read, when calculating the boot-
> > > aggregate.
> > Mimi, do you plan to do update LTP test?

> In order to test Roberto's patches that calculates and extends the
> different TPM banks with the appropriate hashes, we'll need some test
> to verify that it is working properly.  As to whether this will be in
> LTP or ima-evm-utils, I'm not sure.
Sure, it's up to you where you place the test (if you plan to write it).

BTW I see evmtest [1] haven't been merged yet into ima-evm-utils.
What's blocking to merge them? (My objections to require bash shouldn't be the
reason for not being merged.)
I'd like to package them separately for developers to run them on SUT
(unless they're meant to be running only during building package).

Kind regards,
Petr

[1] https://patchwork.kernel.org/project/linux-integrity/list/?series=95303

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH 1/2] ima: use the IMA configured hash algo to calculate the boot aggregate
  2020-01-29 22:51       ` Mimi Zohar
  2020-01-30  8:41         ` Petr Vorel
@ 2020-01-30 15:27         ` Roberto Sassu
  2020-01-30 15:40           ` Roberto Sassu
  1 sibling, 1 reply; 20+ messages in thread
From: Roberto Sassu @ 2020-01-30 15:27 UTC (permalink / raw)
  To: Mimi Zohar, Petr Vorel
  Cc: Jerry Snitselaar, linux-integrity, James Bottomley, linux-kernel

> -----Original Message-----
> From: linux-integrity-owner@vger.kernel.org [mailto:linux-integrity-
> owner@vger.kernel.org] On Behalf Of Mimi Zohar
> Sent: Wednesday, January 29, 2020 11:51 PM
> To: Petr Vorel <pvorel@suse.cz>
> Cc: Jerry Snitselaar <jsnitsel@redhat.com>; linux-integrity@vger.kernel.org;
> James Bottomley <James.Bottomley@hansenpartnership.com>; linux-
> kernel@vger.kernel.org; Roberto Sassu <roberto.sassu@huawei.com>
> Subject: Re: [PATCH 1/2] ima: use the IMA configured hash algo to calculate
> the boot aggregate
> 
> On Wed, 2020-01-29 at 09:30 +0100, Petr Vorel wrote:
> > Hi Mimi,
> >
> > Reviewed-by: Petr Vorel <pvorel@suse.cz>
> >
> > > The original LTP ima_boot_aggregate.c test needed to be updated to
> > > support TPM 2.0 before this change.  For TPM 2.0, the PCRs are not
> > > exported.  With this change, the kernel could be reading PCRs from a
> > > TPM bank other than SHA1 and calculating the boot_aggregate based on
> a
> > > different hash algorithm as well.  I'm not sure how a remote verifier
> > > would know which TPM bank was read, when calculating the boot-
> > > aggregate.
> > Mimi, do you plan to do update LTP test?
> 
> In order to test Roberto's patches that calculates and extends the
> different TPM banks with the appropriate hashes, we'll need some test
> to verify that it is working properly.  As to whether this will be in
> LTP or ima-evm-utils, I'm not sure.

attest-tools (https://github.com/euleros/attest-tools, branch 0.2-devel) has the
ability to parse the BIOS and IMA event logs, and to compare boot_aggregate
with the digest of final PCR values obtained by performing in software the PCR
extend operation with digests in the BIOS event log.

To perform the test, it is necessary to have a complete BIOS event log.

Create req.json with this content:
---
{
  "reqs":{
    "dummy|verify":"",
    "ima_boot_aggregate|verify":""
  }
}
---

With the requirements above, we are telling attest-tools to verify only
boot_aggregate. Without the dummy requirement, verification would
fail (BIOS and remaining IMA measurement entries are not processed).

On server side run:
# attest_ra_server -p 10 -r req.json -s -i

-s disables TPM signature verification
-i allows IMA violations

To enable TPM signature verification it is necessary to have a valid AK
certificate. It can be obtained by following the instructions at:

https://github.com/euleros/attest-tools/blob/0.2-devel/README

On client side run:
# echo test > aik_cert.pem
# echo aik_cert.pem > list_privacy_ca
# attest_ra_client -A

The command above generates an AK.

# attest_ra_client -s <server IP> -q -p 10 -P <PCR algo> -b -i

The command above sends the TPM quote and the event logs
to the RA server and gets the response (successful/failed
verification).

-b includes the BIOS event log from securityfs
-i includes the IMA event log from securityfs

To check that boot_aggregate is calculated properly, use -P sha256
in attest_ra_client and set ima_hash=sha256 in the kernel command
line.

Roberto

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH 1/2] ima: use the IMA configured hash algo to calculate the boot aggregate
  2020-01-30 15:27         ` Roberto Sassu
@ 2020-01-30 15:40           ` Roberto Sassu
  0 siblings, 0 replies; 20+ messages in thread
From: Roberto Sassu @ 2020-01-30 15:40 UTC (permalink / raw)
  To: Roberto Sassu, Mimi Zohar, Petr Vorel
  Cc: Jerry Snitselaar, linux-integrity, James Bottomley, linux-kernel,
	Silviu Vlasceanu

> -----Original Message-----
> From: linux-integrity-owner@vger.kernel.org [mailto:linux-integrity-
> owner@vger.kernel.org] On Behalf Of Roberto Sassu
> Sent: Thursday, January 30, 2020 4:27 PM
> To: Mimi Zohar <zohar@linux.ibm.com>; Petr Vorel <pvorel@suse.cz>
> Cc: Jerry Snitselaar <jsnitsel@redhat.com>; linux-integrity@vger.kernel.org;
> James Bottomley <James.Bottomley@hansenpartnership.com>; linux-
> kernel@vger.kernel.org
> Subject: RE: [PATCH 1/2] ima: use the IMA configured hash algo to calculate
> the boot aggregate
> 
> > -----Original Message-----
> > From: linux-integrity-owner@vger.kernel.org [mailto:linux-integrity-
> > owner@vger.kernel.org] On Behalf Of Mimi Zohar
> > Sent: Wednesday, January 29, 2020 11:51 PM
> > To: Petr Vorel <pvorel@suse.cz>
> > Cc: Jerry Snitselaar <jsnitsel@redhat.com>; linux-
> integrity@vger.kernel.org;
> > James Bottomley <James.Bottomley@hansenpartnership.com>; linux-
> > kernel@vger.kernel.org; Roberto Sassu <roberto.sassu@huawei.com>
> > Subject: Re: [PATCH 1/2] ima: use the IMA configured hash algo to
> calculate
> > the boot aggregate
> >
> > On Wed, 2020-01-29 at 09:30 +0100, Petr Vorel wrote:
> > > Hi Mimi,
> > >
> > > Reviewed-by: Petr Vorel <pvorel@suse.cz>
> > >
> > > > The original LTP ima_boot_aggregate.c test needed to be updated to
> > > > support TPM 2.0 before this change.  For TPM 2.0, the PCRs are not
> > > > exported.  With this change, the kernel could be reading PCRs from a
> > > > TPM bank other than SHA1 and calculating the boot_aggregate based
> on
> > a
> > > > different hash algorithm as well.  I'm not sure how a remote verifier
> > > > would know which TPM bank was read, when calculating the boot-
> > > > aggregate.
> > > Mimi, do you plan to do update LTP test?
> >
> > In order to test Roberto's patches that calculates and extends the
> > different TPM banks with the appropriate hashes, we'll need some test
> > to verify that it is working properly.  As to whether this will be in
> > LTP or ima-evm-utils, I'm not sure.
> 
> attest-tools (https://github.com/euleros/attest-tools, branch 0.2-devel) has
> the
> ability to parse the BIOS and IMA event logs, and to compare
> boot_aggregate
> with the digest of final PCR values obtained by performing in software the
> PCR
> extend operation with digests in the BIOS event log.
> 
> To perform the test, it is necessary to have a complete BIOS event log.
> 
> Create req.json with this content:
> ---
> {
>   "reqs":{
>     "dummy|verify":"",
>     "ima_boot_aggregate|verify":""
>   }
> }
> ---
> 
> With the requirements above, we are telling attest-tools to verify only
> boot_aggregate. Without the dummy requirement, verification would
> fail (BIOS and remaining IMA measurement entries are not processed).
> 
> On server side run:
> # attest_ra_server -p 10 -r req.json -s -i
> 
> -s disables TPM signature verification
> -i allows IMA violations
> 
> To enable TPM signature verification it is necessary to have a valid AK
> certificate. It can be obtained by following the instructions at:
> 
> https://github.com/euleros/attest-tools/blob/0.2-devel/README
> 
> On client side run:
> # echo test > aik_cert.pem
> # echo aik_cert.pem > list_privacy_ca
> # attest_ra_client -A
> 
> The command above generates an AK.
> 
> # attest_ra_client -s <server IP> -q -p 10 -P <PCR algo> -b -i
> 
> The command above sends the TPM quote and the event logs
> to the RA server and gets the response (successful/failed
> verification).
> 
> -b includes the BIOS event log from securityfs
> -i includes the IMA event log from securityfs
> 
> To check that boot_aggregate is calculated properly, use -P sha256

and to check that IMA extends non-SHA1 PCR banks with an appropriate
digest,

> in attest_ra_client and set ima_hash=sha256 in the kernel command
> line.

Roberto

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2020-01-30 15:41 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-27 16:01 [PATCH 1/2] ima: use the IMA configured hash algo to calculate the boot aggregate Mimi Zohar
2020-01-27 16:01 ` [PATCH 2/2] ima: support calculating the boot_aggregate based on different TPM banks Mimi Zohar
2020-01-27 16:50   ` Lakshmi Ramasubramanian
2020-01-27 18:01     ` Mimi Zohar
2020-01-27 20:55     ` Ken Goldman
2020-01-28 14:19   ` Roberto Sassu
2020-01-28 15:40     ` Mimi Zohar
2020-01-28 16:31       ` Roberto Sassu
2020-01-29 23:20       ` Mimi Zohar
2020-01-30  7:31         ` James Bottomley
2020-01-27 17:38 ` [PATCH 1/2] ima: use the IMA configured hash algo to calculate the boot aggregate Roberto Sassu
2020-01-27 18:16   ` Mimi Zohar
2020-01-27 18:35     ` Mimi Zohar
2020-01-27 20:49 ` Jerry Snitselaar
2020-01-27 21:31   ` Mimi Zohar
2020-01-29  8:30     ` Petr Vorel
2020-01-29 22:51       ` Mimi Zohar
2020-01-30  8:41         ` Petr Vorel
2020-01-30 15:27         ` Roberto Sassu
2020-01-30 15:40           ` Roberto Sassu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).