Linux-Integrity Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] extend IMA boot_aggregate with kernel measurements
@ 2020-06-11 19:54 Maurizio Drocco
  2020-06-12  0:29 ` Mimi Zohar
  0 siblings, 1 reply; 22+ messages in thread
From: Maurizio Drocco @ 2020-06-11 19:54 UTC (permalink / raw)
  To: linux-integrity
  Cc: jejb, Maurizio Drocco, Mimi Zohar, Dmitry Kasatkin, James Morris,
	Serge E. Hallyn, open list:SECURITY SUBSYSTEM, open list

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@ibm.com>
---
 security/integrity/ima/ima.h        |  2 +-
 security/integrity/ima/ima_crypto.c | 11 ++++++++++-
 2 files changed, 11 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..6f0137bdaf61 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,15 @@ 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 */
+	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


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

* Re: [PATCH] extend IMA boot_aggregate with kernel measurements
  2020-06-11 19:54 [PATCH] extend IMA boot_aggregate with kernel measurements Maurizio Drocco
@ 2020-06-12  0:29 ` Mimi Zohar
  2020-06-12 14:38   ` Maurizio Drocco
  0 siblings, 1 reply; 22+ messages in thread
From: Mimi Zohar @ 2020-06-12  0:29 UTC (permalink / raw)
  To: Maurizio Drocco, linux-integrity
  Cc: jejb, Dmitry Kasatkin, James Morris, Serge E. Hallyn,
	open list:SECURITY SUBSYSTEM, open list

Hi Maurizo,

On Thu, 2020-06-11 at 15:54 -0400, Maurizio Drocco wrote:
> 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@ibm.com>

Looks good.  Just a minor comment below.  Could you be a bit more
specific as to what is being measured into which PCR.  Perhaps include
a reference to some doc or spec.

In order to test, ima-evm-utils needs to be updated as well.  Could
you post the corresponding evmctl change?  Please post the patch
against the ima-evm-utils next-testing branch.

> ---
>  security/integrity/ima/ima.h        |  2 +-
>  security/integrity/ima/ima_crypto.c | 11 ++++++++++-
>  2 files changed, 11 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..6f0137bdaf61 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,15 @@ 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 */
> +	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))

The formatting here is a bit off.

thanks,

Mimi

> +			rc = crypto_shash_update(shash, d.digest,
> +					crypto_shash_digestsize(tfm));
> +	}
>  	if (!rc)
>  		crypto_shash_final(shash, digest);
>  	return rc;


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

* [PATCH] extend IMA boot_aggregate with kernel measurements
  2020-06-12  0:29 ` Mimi Zohar
@ 2020-06-12 14:38   ` Maurizio Drocco
  2020-06-12 15:11     ` Roberto Sassu
  0 siblings, 1 reply; 22+ messages in thread
From: Maurizio Drocco @ 2020-06-12 14:38 UTC (permalink / raw)
  To: zohar
  Cc: dmitry.kasatkin, jejb, jmorris, linux-integrity, linux-kernel,
	linux-security-module, maurizio.drocco, serge

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@ibm.com>
---
 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
+	 * 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


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

* RE: [PATCH] extend IMA boot_aggregate with kernel measurements
  2020-06-12 14:38   ` Maurizio Drocco
@ 2020-06-12 15:11     ` Roberto Sassu
  2020-06-12 17:14       ` James Bottomley
  0 siblings, 1 reply; 22+ messages in thread
From: Roberto Sassu @ 2020-06-12 15:11 UTC (permalink / raw)
  To: Maurizio Drocco, zohar
  Cc: dmitry.kasatkin, jejb, jmorris, linux-integrity, linux-kernel,
	linux-security-module, serge, Silviu Vlasceanu


[-- Attachment #1: Type: text/plain, Size: 3082 bytes --]

> From: linux-integrity-owner@vger.kernel.org [mailto:linux-integrity-
> owner@vger.kernel.org] 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@ibm.com>
> ---
>  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


[-- Attachment #2: Type: message/rfc822, Size: 2632 bytes --]

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

* Re: [PATCH] extend IMA boot_aggregate with kernel measurements
  2020-06-12 15:11     ` Roberto Sassu
@ 2020-06-12 17:14       ` James Bottomley
  2020-06-16 17:29         ` Roberto Sassu
  0 siblings, 1 reply; 22+ messages in thread
From: James Bottomley @ 2020-06-12 17:14 UTC (permalink / raw)
  To: Roberto Sassu, Maurizio Drocco, zohar
  Cc: dmitry.kasatkin, jmorris, linux-integrity, linux-kernel,
	linux-security-module, serge, Silviu Vlasceanu

On Fri, 2020-06-12 at 15:11 +0000, Roberto Sassu wrote:
> 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?

So here's the problem: if your current grub doesn't do any TPM
extensions (as most don't), then the two boot aggregates are the same
because PCRs 8 and 9 are zero and there's a test that doesn't add them
to the aggregate if they are zero.  For these people its a nop so we
shouldn't force them to choose a different version of the same thing.

If, however, you're on a distribution where grub is automatically
measuring the kernel and command line into PCRs 8 and 9 (I think Fedora
32 does this), your boot aggregate will change.  It strikes me in that
case we can call this a bug fix, since the boot aggregate isn't
properly binding to the previous measurements without PCRs 8 and 9.  In
this case, do we want to allow people to select an option which doesn't
properly bind the IMA log to the boot measurements?  That sounds like a
security hole to me.

However, since it causes a user visible difference in the grub already
measures case, do you have a current use case that would be affected? 
As in are lots of people already running a distro with the TPM grub
updates and relying on the old boot aggregate?

James


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

* RE: [PATCH] extend IMA boot_aggregate with kernel measurements
  2020-06-12 17:14       ` James Bottomley
@ 2020-06-16 17:29         ` Roberto Sassu
  2020-06-16 18:11           ` Mimi Zohar
  0 siblings, 1 reply; 22+ messages in thread
From: Roberto Sassu @ 2020-06-16 17:29 UTC (permalink / raw)
  To: jejb, Maurizio Drocco, zohar
  Cc: dmitry.kasatkin, jmorris, linux-integrity, linux-kernel,
	linux-security-module, serge, Silviu Vlasceanu

> From: James Bottomley [mailto:jejb@linux.ibm.com]
> Sent: Friday, June 12, 2020 7:14 PM
> On Fri, 2020-06-12 at 15:11 +0000, Roberto Sassu wrote:
> > 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?
> 
> So here's the problem: if your current grub doesn't do any TPM
> extensions (as most don't), then the two boot aggregates are the same
> because PCRs 8 and 9 are zero and there's a test that doesn't add them
> to the aggregate if they are zero.  For these people its a nop so we
> shouldn't force them to choose a different version of the same thing.
> 
> If, however, you're on a distribution where grub is automatically
> measuring the kernel and command line into PCRs 8 and 9 (I think Fedora
> 32 does this), your boot aggregate will change.  It strikes me in that
> case we can call this a bug fix, since the boot aggregate isn't
> properly binding to the previous measurements without PCRs 8 and 9.  In
> this case, do we want to allow people to select an option which doesn't
> properly bind the IMA log to the boot measurements?  That sounds like a
> security hole to me.
> 
> However, since it causes a user visible difference in the grub already
> measures case, do you have a current use case that would be affected?
> As in are lots of people already running a distro with the TPM grub
> updates and relying on the old boot aggregate?

I don't know how many people would be affected. However, if an
attestation tool processes both measurement lists from unpatched kernels
and patched kernels, keeping the same name would be a problem as it
cannot be determined from the measurement list how boot_aggregate
was calculated.

Anyway, I agree this should be fixed. At least, I suggest to add a Fixes tag,
to ensure that this patch is applied to all stable kernels.

Roberto

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

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

* Re: [PATCH] extend IMA boot_aggregate with kernel measurements
  2020-06-16 17:29         ` Roberto Sassu
@ 2020-06-16 18:11           ` Mimi Zohar
  2020-06-18 12:38             ` Roberto Sassu
  0 siblings, 1 reply; 22+ messages in thread
From: Mimi Zohar @ 2020-06-16 18:11 UTC (permalink / raw)
  To: Roberto Sassu, jejb, Maurizio Drocco
  Cc: dmitry.kasatkin, jmorris, linux-integrity, linux-kernel,
	linux-security-module, serge, Silviu Vlasceanu

On Tue, 2020-06-16 at 17:29 +0000, Roberto Sassu wrote:
> > From: James Bottomley [mailto:jejb@linux.ibm.com]
> > Sent: Friday, June 12, 2020 7:14 PM
> > On Fri, 2020-06-12 at 15:11 +0000, Roberto Sassu wrote:
> > > 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?
> > 
> > So here's the problem: if your current grub doesn't do any TPM
> > extensions (as most don't), then the two boot aggregates are the same
> > because PCRs 8 and 9 are zero and there's a test that doesn't add them
> > to the aggregate if they are zero.  For these people its a nop so we
> > shouldn't force them to choose a different version of the same thing.
> > 
> > If, however, you're on a distribution where grub is automatically
> > measuring the kernel and command line into PCRs 8 and 9 (I think Fedora
> > 32 does this), your boot aggregate will change.  It strikes me in that
> > case we can call this a bug fix, since the boot aggregate isn't
> > properly binding to the previous measurements without PCRs 8 and 9.  In
> > this case, do we want to allow people to select an option which doesn't
> > properly bind the IMA log to the boot measurements?  That sounds like a
> > security hole to me.
> > 
> > However, since it causes a user visible difference in the grub already
> > measures case, do you have a current use case that would be affected?
> > As in are lots of people already running a distro with the TPM grub
> > updates and relying on the old boot aggregate?
> 
> I don't know how many people would be affected. However, if an
> attestation tool processes both measurement lists from unpatched kernels
> and patched kernels, keeping the same name would be a problem as it
> cannot be determined from the measurement list how boot_aggregate
> was calculated.
> 
> Anyway, I agree this should be fixed. At least, I suggest to add a Fixes tag,
> to ensure that this patch is applied to all stable kernels.

The boot aggregate on existing systems would be sha1.  Does it make
sense to limit this change to larger digests?  Anyone backporting
support for larger digests would also need to backport this change as
well.

Mimi

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

* RE: [PATCH] extend IMA boot_aggregate with kernel measurements
  2020-06-16 18:11           ` Mimi Zohar
@ 2020-06-18 12:38             ` Roberto Sassu
  2020-06-18 20:11               ` Maurizio Drocco
  0 siblings, 1 reply; 22+ messages in thread
From: Roberto Sassu @ 2020-06-18 12:38 UTC (permalink / raw)
  To: Mimi Zohar, jejb, Maurizio Drocco
  Cc: dmitry.kasatkin, jmorris, linux-integrity, linux-kernel,
	linux-security-module, serge, Silviu Vlasceanu

> From: Mimi Zohar [mailto:zohar@linux.ibm.com]
> Sent: Tuesday, June 16, 2020 8:11 PM
> On Tue, 2020-06-16 at 17:29 +0000, Roberto Sassu wrote:
> > > From: James Bottomley [mailto:jejb@linux.ibm.com]
> > > Sent: Friday, June 12, 2020 7:14 PM
> > > On Fri, 2020-06-12 at 15:11 +0000, Roberto Sassu wrote:
> > > > 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?
> > >
> > > So here's the problem: if your current grub doesn't do any TPM
> > > extensions (as most don't), then the two boot aggregates are the same
> > > because PCRs 8 and 9 are zero and there's a test that doesn't add them
> > > to the aggregate if they are zero.  For these people its a nop so we
> > > shouldn't force them to choose a different version of the same thing.
> > >
> > > If, however, you're on a distribution where grub is automatically
> > > measuring the kernel and command line into PCRs 8 and 9 (I think
> Fedora
> > > 32 does this), your boot aggregate will change.  It strikes me in that
> > > case we can call this a bug fix, since the boot aggregate isn't
> > > properly binding to the previous measurements without PCRs 8 and 9.
> In
> > > this case, do we want to allow people to select an option which doesn't
> > > properly bind the IMA log to the boot measurements?  That sounds like
> a
> > > security hole to me.
> > >
> > > However, since it causes a user visible difference in the grub already
> > > measures case, do you have a current use case that would be affected?
> > > As in are lots of people already running a distro with the TPM grub
> > > updates and relying on the old boot aggregate?
> >
> > I don't know how many people would be affected. However, if an
> > attestation tool processes both measurement lists from unpatched
> kernels
> > and patched kernels, keeping the same name would be a problem as it
> > cannot be determined from the measurement list how boot_aggregate
> > was calculated.
> >
> > Anyway, I agree this should be fixed. At least, I suggest to add a Fixes tag,
> > to ensure that this patch is applied to all stable kernels.
> 
> The boot aggregate on existing systems would be sha1.  Does it make
> sense to limit this change to larger digests?  Anyone backporting
> support for larger digests would also need to backport this change as
> well.

Yes, it would be a safe choice.

Roberto

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

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

* [PATCH] extend IMA boot_aggregate with kernel measurements
  2020-06-18 12:38             ` Roberto Sassu
@ 2020-06-18 20:11               ` Maurizio Drocco
  2020-06-18 20:11                 ` [PATCH] ima_evm_utils: extended calc_bootaggr to PCRs 8 - 9 Maurizio Drocco
  0 siblings, 1 reply; 22+ messages in thread
From: Maurizio Drocco @ 2020-06-18 20:11 UTC (permalink / raw)
  To: roberto.sassu
  Cc: Silviu.Vlasceanu, dmitry.kasatkin, jejb, jmorris,
	linux-integrity, linux-kernel, linux-security-module,
	maurizio.drocco, serge, zohar, mdrocco

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@ibm.com>
---
 security/integrity/ima/ima.h        |  2 +-
 security/integrity/ima/ima_crypto.c | 20 ++++++++++++++++++--
 2 files changed, 19 insertions(+), 3 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..299b23dad8d9 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);
@@ -823,13 +823,29 @@ static int ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,
 	if (rc != 0)
 		return rc;
 
-	/* cumulative sha1 over tpm registers 0-7 */
+	/* cumulative digest over tpm registers 0-7 */
 	for (i = TPM_PCR0; i < TPM_PCR8; i++) {
 		ima_pcrread(i, &d);
 		/* now accumulate with current aggregate */
 		rc = crypto_shash_update(shash, d.digest,
 					 crypto_shash_digestsize(tfm));
 	}
+	/*
+	 * extend cumulative digest over tpm registers 8-9, which contain
+	 * measurement for the kernel command line (reg. 8) and image (reg. 9)
+	 * in a typical PCR allocation. Registers 8-9 are only included in
+	 * non-SHA1 boot_aggregate digests to avoid ambiguity.
+	 */
+	if (alg_id != TPM_ALG_SHA1) {
+		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


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

* [PATCH] ima_evm_utils: extended calc_bootaggr to PCRs 8 - 9
  2020-06-18 20:11               ` Maurizio Drocco
@ 2020-06-18 20:11                 ` Maurizio Drocco
  2020-06-22 20:14                   ` Mimi Zohar
  0 siblings, 1 reply; 22+ messages in thread
From: Maurizio Drocco @ 2020-06-18 20:11 UTC (permalink / raw)
  To: roberto.sassu
  Cc: Silviu.Vlasceanu, dmitry.kasatkin, jejb, jmorris,
	linux-integrity, linux-kernel, linux-security-module,
	maurizio.drocco, serge, zohar, mdrocco

From: Maurizio <maurizio.drocco@ibm.com>

If PCRs 8 - 9 are set (i.e. not all-zeros), cal_bootaggr should include
them into the digest.

Signed-off-by: Maurizio Drocco <maurizio.drocco@ibm.com>
---
 src/evmctl.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/src/evmctl.c b/src/evmctl.c
index 1d065ce..554571e 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -1930,6 +1930,18 @@ static void calc_bootaggr(struct tpm_bank_info *bank)
 		}
 	}
 
+	if (strcmp(bank->algo_name, "sha1") != 0) {
+		for (i = 8; i < 10; i++) {
+			if (memcmp(bank->pcr[i], zero, bank->digest_size) != 0) {
+				err = EVP_DigestUpdate(pctx, bank->pcr[i], bank->digest_size);
+				if (!err) {
+					log_err("EVP_DigestUpdate() failed\n");
+					return;
+				}
+			}
+		}
+	}
+
 	err = EVP_DigestFinal(pctx, bank->digest, &mdlen);
 	if (!err) {
 		log_err("EVP_DigestFinal() failed\n");
@@ -1973,7 +1985,9 @@ static int append_bootaggr(char *bootaggr, struct tpm_bank_info *tpm_banks)
  * The IMA measurement list boot_aggregate is the link between the preboot
  * event log and the IMA measurement list.  Read and calculate all the
  * possible per TPM bank boot_aggregate digests based on the existing
- * PCRs 0 - 7 to validate against the IMA boot_aggregate record.
+ * PCRs 0 - 9 to validate against the IMA boot_aggregate record. If PCRs
+ * 8 - 9 are not set (i.e. all-zeros) or the digest algorithm is SHA1, only
+ * PCRs 0 - 7 are considered.
  */
 static int cmd_ima_bootaggr(struct command *cmd)
 {
-- 
2.17.1


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

* [PATCH] ima: extend boot_aggregate with kernel measurements
  2020-06-22 20:14                   ` Mimi Zohar
@ 2020-06-22  4:50                     ` Maurizio Drocco
  2020-06-23 14:03                       ` Mimi Zohar
  2020-06-23 18:01                     ` [PATCH v2] ima_evm_utils: extended calc_bootaggr to PCRs 8 - 9 Maurizio Drocco
  1 sibling, 1 reply; 22+ messages in thread
From: Maurizio Drocco @ 2020-06-22  4:50 UTC (permalink / raw)
  To: zohar
  Cc: Silviu.Vlasceanu, dmitry.kasatkin, jejb, jmorris,
	linux-integrity, linux-kernel, linux-security-module,
	maurizio.drocco, mdrocco, roberto.sassu, serge

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. Registers
8-9 are only included in non-SHA1 boot_aggregate digests to avoid
ambiguity.

Signed-off-by: Maurizio Drocco <maurizio.drocco@ibm.com>
---
 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..d02917d85033 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -823,13 +823,26 @@ static int ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,
 	if (rc != 0)
 		return rc;
 
-	/* cumulative sha1 over tpm registers 0-7 */
+	/* cumulative digest over tpm registers 0-7 */
 	for (i = TPM_PCR0; i < TPM_PCR8; i++) {
 		ima_pcrread(i, &d);
 		/* now accumulate with current aggregate */
 		rc = crypto_shash_update(shash, d.digest,
 					 crypto_shash_digestsize(tfm));
 	}
+	/*
+	 * extend cumulative digest over tpm registers 8-9, which contain
+	 * measurement for the kernel command line (reg. 8) and image (reg. 9)
+	 * in a typical PCR allocation. Registers 8-9 are only included in
+	 * non-SHA1 boot_aggregate digests to avoid ambiguity.
+	 */
+	if (alg_id != TPM_ALG_SHA1) {
+		for (i = TPM_PCR8; i < TPM_PCR10; i++) {
+			ima_pcrread(i, &d);
+			rc = crypto_shash_update(shash, d.digest,
+						crypto_shash_digestsize(tfm));
+		}
+	}
 	if (!rc)
 		crypto_shash_final(shash, digest);
 	return rc;
-- 
2.17.1


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

* Re: [PATCH] ima_evm_utils: extended calc_bootaggr to PCRs 8 - 9
  2020-06-18 20:11                 ` [PATCH] ima_evm_utils: extended calc_bootaggr to PCRs 8 - 9 Maurizio Drocco
@ 2020-06-22 20:14                   ` Mimi Zohar
  2020-06-22  4:50                     ` [PATCH] ima: extend boot_aggregate with kernel measurements Maurizio Drocco
  2020-06-23 18:01                     ` [PATCH v2] ima_evm_utils: extended calc_bootaggr to PCRs 8 - 9 Maurizio Drocco
  0 siblings, 2 replies; 22+ messages in thread
From: Mimi Zohar @ 2020-06-22 20:14 UTC (permalink / raw)
  To: Maurizio Drocco, roberto.sassu
  Cc: Silviu.Vlasceanu, dmitry.kasatkin, jejb, jmorris,
	linux-integrity, linux-kernel, linux-security-module, serge,
	mdrocco

On Thu, 2020-06-18 at 16:11 -0400, Maurizio Drocco wrote:
> From: Maurizio <maurizio.drocco@ibm.com>
> 
> If PCRs 8 - 9 are set (i.e. not all-zeros), cal_bootaggr should include
> them into the digest.
> 
> Signed-off-by: Maurizio Drocco <maurizio.drocco@ibm.com>
> ---
>  src/evmctl.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/src/evmctl.c b/src/evmctl.c
> index 1d065ce..554571e 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -1930,6 +1930,18 @@ static void calc_bootaggr(struct tpm_bank_info *bank)
>  		}
>  	}
>  
> +	if (strcmp(bank->algo_name, "sha1") != 0) {
> +		for (i = 8; i < 10; i++) {
> +			if (memcmp(bank->pcr[i], zero, bank->digest_size) != 0) {
> +				err = EVP_DigestUpdate(pctx, bank->pcr[i], bank->digest_size);
> +				if (!err) {
> +					log_err("EVP_DigestUpdate() failed\n");
> +					return;
> +				}
> +			}
> +		}
> +	}

Roberto, now that we're only including the PCRs 8 & 9 in the non-sha1
"boot_aggregate", they can always be included.

Please reflect this change in the patch description and, here, in the
code.

thanks,

Mimi

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

* Re: [PATCH] ima: extend boot_aggregate with kernel measurements
  2020-06-22  4:50                     ` [PATCH] ima: extend boot_aggregate with kernel measurements Maurizio Drocco
@ 2020-06-23 14:03                       ` Mimi Zohar
  2020-06-23 15:57                         ` [PATCH v4] " Maurizio Drocco
  0 siblings, 1 reply; 22+ messages in thread
From: Mimi Zohar @ 2020-06-23 14:03 UTC (permalink / raw)
  To: Maurizio Drocco
  Cc: Silviu.Vlasceanu, dmitry.kasatkin, jejb, jmorris,
	linux-integrity, linux-kernel, linux-security-module, mdrocco,
	roberto.sassu, serge

Hi Maurizio,

When re-posting patches, please include the version number (e.g.
[PATCH v4] ima: ... ).

On Mon, 2020-06-22 at 00:50 -0400, Maurizio Drocco wrote:
> IMA is not considering TPM registers 8-9 when calculating the boot
> aggregate.

This line is unnecessary with the following change.

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

The "When" clause makes this sound like PCRs 8 & 9 are not always
included.  I would split this into two sentences.

>  Registers
> 8-9 are only included in non-SHA1 boot_aggregate digests to avoid
> ambiguity.
> 
> Signed-off-by: Maurizio Drocco <maurizio.drocco@ibm.com>
> ---

Missing "Changelog:".

Changelog:
v2: 
- Limit including PCRs 8 & 9 to non-sha1 hashes
v1:
- Include non zero PCRs 8 & 9 in the boot_aggregate

>  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..d02917d85033 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -823,13 +823,26 @@ static int ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,
>  	if (rc != 0)
>  		return rc;
>  
> -	/* cumulative sha1 over tpm registers 0-7 */
> +	/* cumulative digest over tpm registers 0-7 */

Please uppercase "tpm" here and below.

>  	for (i = TPM_PCR0; i < TPM_PCR8; i++) {
>  		ima_pcrread(i, &d);
>  		/* now accumulate with current aggregate */
>  		rc = crypto_shash_update(shash, d.digest,
>  					 crypto_shash_digestsize(tfm));
>  	}
> +	/*
> +	 * extend cumulative digest over tpm registers 8-9, which contain
> +	 * measurement for the kernel command line (reg. 8) and image (reg. 9)
> +	 * in a typical PCR allocation. Registers 8-9 are only included in
> +	 * non-SHA1 boot_aggregate digests to avoid ambiguity.
> +	 */

Comments that are full sentences should start with an uppercase letter
and end with a period (e.g. Extend).

thanks,

Mimi

> +	if (alg_id != TPM_ALG_SHA1) {
> +		for (i = TPM_PCR8; i < TPM_PCR10; i++) {
> +			ima_pcrread(i, &d);
> +			rc = crypto_shash_update(shash, d.digest,
> +						crypto_shash_digestsize(tfm));
> +		}
> +	}
>  	if (!rc)
>  		crypto_shash_final(shash, digest);
>  	return rc;


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

* [PATCH v4] ima: extend boot_aggregate with kernel measurements
  2020-06-23 14:03                       ` Mimi Zohar
@ 2020-06-23 15:57                         ` Maurizio Drocco
  2020-06-23 18:53                           ` Bruno Meneguele
  0 siblings, 1 reply; 22+ messages in thread
From: Maurizio Drocco @ 2020-06-23 15:57 UTC (permalink / raw)
  To: zohar
  Cc: Silviu.Vlasceanu, dmitry.kasatkin, jejb, jmorris,
	linux-integrity, linux-kernel, linux-security-module,
	maurizio.drocco, mdrocco, roberto.sassu, serge

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. Registers 8-9 should be
only included in non-SHA1 digests to avoid ambiguity.

Signed-off-by: Maurizio Drocco <maurizio.drocco@ibm.com>
---
Changelog:
v4:
- Reworded comments: PCRs 8 & 9 are always included in non-sha1 digests
v3:
- Limit including PCRs 8 & 9 to non-sha1 hashes
v2:
- Minor comment improvements
v1:
- Include non zero PCRs 8 & 9 in the boot_aggregate

 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..011c3c76af86 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -823,13 +823,26 @@ static int ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,
 	if (rc != 0)
 		return rc;
 
-	/* cumulative sha1 over tpm registers 0-7 */
+	/* cumulative digest over TPM registers 0-7 */
 	for (i = TPM_PCR0; i < TPM_PCR8; i++) {
 		ima_pcrread(i, &d);
 		/* now accumulate with current aggregate */
 		rc = crypto_shash_update(shash, d.digest,
 					 crypto_shash_digestsize(tfm));
 	}
+	/*
+	 * Extend cumulative digest over TPM registers 8-9, which contain
+	 * measurement for the kernel command line (reg. 8) and image (reg. 9)
+	 * in a typical PCR allocation. Registers 8-9 are only included in
+	 * non-SHA1 boot_aggregate digests to avoid ambiguity.
+	 */
+	if (alg_id != TPM_ALG_SHA1) {
+		for (i = TPM_PCR8; i < TPM_PCR10; i++) {
+			ima_pcrread(i, &d);
+			rc = crypto_shash_update(shash, d.digest,
+						crypto_shash_digestsize(tfm));
+		}
+	}
 	if (!rc)
 		crypto_shash_final(shash, digest);
 	return rc;
-- 
2.17.1


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

* [PATCH v2] ima_evm_utils: extended calc_bootaggr to PCRs 8 - 9
  2020-06-22 20:14                   ` Mimi Zohar
  2020-06-22  4:50                     ` [PATCH] ima: extend boot_aggregate with kernel measurements Maurizio Drocco
@ 2020-06-23 18:01                     ` Maurizio Drocco
  2020-06-23 18:13                       ` Bruno Meneguele
  1 sibling, 1 reply; 22+ messages in thread
From: Maurizio Drocco @ 2020-06-23 18:01 UTC (permalink / raw)
  To: zohar
  Cc: Silviu.Vlasceanu, dmitry.kasatkin, jejb, jmorris,
	linux-integrity, linux-kernel, linux-security-module,
	maurizio.drocco, mdrocco, roberto.sassu, serge

From: Maurizio <maurizio.drocco@ibm.com>

If PCRs 8 - 9 are set (i.e. not all-zeros), cal_bootaggr should include
them into the digest.

Signed-off-by: Maurizio Drocco <maurizio.drocco@ibm.com>
---
Changelog:
v2:
- Always include PCRs 8 & 9 to non-sha1 hashes
v1:
- Include non-zero PCRs 8 & 9 to boot aggregates 

 src/evmctl.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/evmctl.c b/src/evmctl.c
index 1d065ce..46b7092 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -1930,6 +1930,16 @@ static void calc_bootaggr(struct tpm_bank_info *bank)
 		}
 	}
 
+	if (strcmp(bank->algo_name, "sha1") != 0) {
+		for (i = 8; i < 10; i++) {
+			err = EVP_DigestUpdate(pctx, bank->pcr[i], bank->digest_size);
+			if (!err) {
+				log_err("EVP_DigestUpdate() failed\n");
+				return;
+			}
+		}
+	}
+
 	err = EVP_DigestFinal(pctx, bank->digest, &mdlen);
 	if (!err) {
 		log_err("EVP_DigestFinal() failed\n");
@@ -1972,8 +1982,9 @@ static int append_bootaggr(char *bootaggr, struct tpm_bank_info *tpm_banks)
 /*
  * The IMA measurement list boot_aggregate is the link between the preboot
  * event log and the IMA measurement list.  Read and calculate all the
- * possible per TPM bank boot_aggregate digests based on the existing
- * PCRs 0 - 7 to validate against the IMA boot_aggregate record.
+ * possible per TPM bank boot_aggregate digests based on the existing PCRs
+ * 0 - 9 to validate against the IMA boot_aggregate record. If the digest
+ * algorithm is SHA1, only PCRs 0 - 7 are considered to avoid ambiguity.
  */
 static int cmd_ima_bootaggr(struct command *cmd)
 {
-- 
2.17.1


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

* Re: [PATCH v2] ima_evm_utils: extended calc_bootaggr to PCRs 8 - 9
  2020-06-23 18:01                     ` [PATCH v2] ima_evm_utils: extended calc_bootaggr to PCRs 8 - 9 Maurizio Drocco
@ 2020-06-23 18:13                       ` Bruno Meneguele
  2020-06-24 21:17                         ` Stefan Berger
  0 siblings, 1 reply; 22+ messages in thread
From: Bruno Meneguele @ 2020-06-23 18:13 UTC (permalink / raw)
  To: Maurizio Drocco
  Cc: zohar, Silviu.Vlasceanu, dmitry.kasatkin, jejb, jmorris,
	linux-integrity, linux-kernel, linux-security-module, mdrocco,
	roberto.sassu, serge


[-- Attachment #1: Type: text/plain, Size: 1952 bytes --]

On Tue, Jun 23, 2020 at 02:01:22PM -0400, Maurizio Drocco wrote:
> From: Maurizio <maurizio.drocco@ibm.com>
> 
> If PCRs 8 - 9 are set (i.e. not all-zeros), cal_bootaggr should include
> them into the digest.
> 
> Signed-off-by: Maurizio Drocco <maurizio.drocco@ibm.com>
> ---
> Changelog:
> v2:
> - Always include PCRs 8 & 9 to non-sha1 hashes
> v1:
> - Include non-zero PCRs 8 & 9 to boot aggregates 
> 
>  src/evmctl.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/src/evmctl.c b/src/evmctl.c
> index 1d065ce..46b7092 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -1930,6 +1930,16 @@ static void calc_bootaggr(struct tpm_bank_info *bank)
>  		}
>  	}
>  
> +	if (strcmp(bank->algo_name, "sha1") != 0) {
> +		for (i = 8; i < 10; i++) {
> +			err = EVP_DigestUpdate(pctx, bank->pcr[i], bank->digest_size);
> +			if (!err) {
> +				log_err("EVP_DigestUpdate() failed\n");
> +				return;
> +			}
> +		}
> +	}
> +
>  	err = EVP_DigestFinal(pctx, bank->digest, &mdlen);
>  	if (!err) {
>  		log_err("EVP_DigestFinal() failed\n");
> @@ -1972,8 +1982,9 @@ static int append_bootaggr(char *bootaggr, struct tpm_bank_info *tpm_banks)
>  /*
>   * The IMA measurement list boot_aggregate is the link between the preboot
>   * event log and the IMA measurement list.  Read and calculate all the
> - * possible per TPM bank boot_aggregate digests based on the existing
> - * PCRs 0 - 7 to validate against the IMA boot_aggregate record.
> + * possible per TPM bank boot_aggregate digests based on the existing PCRs
> + * 0 - 9 to validate against the IMA boot_aggregate record. If the digest
> + * algorithm is SHA1, only PCRs 0 - 7 are considered to avoid ambiguity.
>   */
>  static int cmd_ima_bootaggr(struct command *cmd)
>  {
> -- 
> 2.17.1
> 

Reviewed-by: Bruno Meneguele <bmeneg@redhat.com>

-- 
bmeneg 
PGP Key: http://bmeneg.com/pubkey.txt

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH v4] ima: extend boot_aggregate with kernel measurements
  2020-06-23 15:57                         ` [PATCH v4] " Maurizio Drocco
@ 2020-06-23 18:53                           ` Bruno Meneguele
  0 siblings, 0 replies; 22+ messages in thread
From: Bruno Meneguele @ 2020-06-23 18:53 UTC (permalink / raw)
  To: Maurizio Drocco
  Cc: zohar, Silviu.Vlasceanu, dmitry.kasatkin, jejb, jmorris,
	linux-integrity, linux-kernel, linux-security-module, mdrocco,
	roberto.sassu, serge


[-- Attachment #1: Type: text/plain, Size: 2902 bytes --]

On Tue, Jun 23, 2020 at 11:57:32AM -0400, Maurizio Drocco wrote:
> 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. Registers 8-9 should be
> only included in non-SHA1 digests to avoid ambiguity.
> 
> Signed-off-by: Maurizio Drocco <maurizio.drocco@ibm.com>
> ---
> Changelog:
> v4:
> - Reworded comments: PCRs 8 & 9 are always included in non-sha1 digests
> v3:
> - Limit including PCRs 8 & 9 to non-sha1 hashes
> v2:
> - Minor comment improvements
> v1:
> - Include non zero PCRs 8 & 9 in the boot_aggregate
> 
>  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..011c3c76af86 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -823,13 +823,26 @@ static int ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,
>  	if (rc != 0)
>  		return rc;
>  
> -	/* cumulative sha1 over tpm registers 0-7 */
> +	/* cumulative digest over TPM registers 0-7 */
>  	for (i = TPM_PCR0; i < TPM_PCR8; i++) {
>  		ima_pcrread(i, &d);
>  		/* now accumulate with current aggregate */
>  		rc = crypto_shash_update(shash, d.digest,
>  					 crypto_shash_digestsize(tfm));
>  	}
> +	/*
> +	 * Extend cumulative digest over TPM registers 8-9, which contain
> +	 * measurement for the kernel command line (reg. 8) and image (reg. 9)
> +	 * in a typical PCR allocation. Registers 8-9 are only included in
> +	 * non-SHA1 boot_aggregate digests to avoid ambiguity.
> +	 */
> +	if (alg_id != TPM_ALG_SHA1) {
> +		for (i = TPM_PCR8; i < TPM_PCR10; i++) {
> +			ima_pcrread(i, &d);
> +			rc = crypto_shash_update(shash, d.digest,
> +						crypto_shash_digestsize(tfm));
> +		}
> +	}
>  	if (!rc)
>  		crypto_shash_final(shash, digest);
>  	return rc;
> -- 
> 2.17.1
> 

Reviewed-by: Bruno Meneguele <bmeneg@redhat.com>

I've tested this patch with both TPM 1.2 and TPM 2.0 + ima-evm-utils
support patch. Everything seems fine.

Thanks.

-- 
bmeneg 
PGP Key: http://bmeneg.com/pubkey.txt

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH v2] ima_evm_utils: extended calc_bootaggr to PCRs 8 - 9
  2020-06-23 18:13                       ` Bruno Meneguele
@ 2020-06-24 21:17                         ` Stefan Berger
  2020-06-24 21:33                           ` [PATCH] " Maurizio Drocco
  2020-06-24 21:33                           ` [PATCH v2] " Bruno Meneguele
  0 siblings, 2 replies; 22+ messages in thread
From: Stefan Berger @ 2020-06-24 21:17 UTC (permalink / raw)
  To: Bruno Meneguele, Maurizio Drocco
  Cc: zohar, Silviu.Vlasceanu, dmitry.kasatkin, jejb, jmorris,
	linux-integrity, linux-kernel, linux-security-module, mdrocco,
	roberto.sassu, serge

On 6/23/20 2:13 PM, Bruno Meneguele wrote:
> On Tue, Jun 23, 2020 at 02:01:22PM -0400, Maurizio Drocco wrote:
>> From: Maurizio <maurizio.drocco@ibm.com>
>>
>> If PCRs 8 - 9 are set (i.e. not all-zeros), cal_bootaggr should include
>> them into the digest.


Wouldn't you have to check for not all-zeros in your code?


    Stefan


>>
>> Signed-off-by: Maurizio Drocco <maurizio.drocco@ibm.com>
>> ---
>> Changelog:
>> v2:
>> - Always include PCRs 8 & 9 to non-sha1 hashes
>> v1:
>> - Include non-zero PCRs 8 & 9 to boot aggregates
>>
>>   src/evmctl.c | 15 +++++++++++++--
>>   1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/evmctl.c b/src/evmctl.c
>> index 1d065ce..46b7092 100644
>> --- a/src/evmctl.c
>> +++ b/src/evmctl.c
>> @@ -1930,6 +1930,16 @@ static void calc_bootaggr(struct tpm_bank_info *bank)
>>   		}
>>   	}
>>   
>> +	if (strcmp(bank->algo_name, "sha1") != 0) {
>> +		for (i = 8; i < 10; i++) {
>> +			err = EVP_DigestUpdate(pctx, bank->pcr[i], bank->digest_size);
>> +			if (!err) {
>> +				log_err("EVP_DigestUpdate() failed\n");
>> +				return;
>> +			}
>> +		}
>> +	}
>> +
>>   	err = EVP_DigestFinal(pctx, bank->digest, &mdlen);
>>   	if (!err) {
>>   		log_err("EVP_DigestFinal() failed\n");
>> @@ -1972,8 +1982,9 @@ static int append_bootaggr(char *bootaggr, struct tpm_bank_info *tpm_banks)
>>   /*
>>    * The IMA measurement list boot_aggregate is the link between the preboot
>>    * event log and the IMA measurement list.  Read and calculate all the
>> - * possible per TPM bank boot_aggregate digests based on the existing
>> - * PCRs 0 - 7 to validate against the IMA boot_aggregate record.
>> + * possible per TPM bank boot_aggregate digests based on the existing PCRs
>> + * 0 - 9 to validate against the IMA boot_aggregate record. If the digest
>> + * algorithm is SHA1, only PCRs 0 - 7 are considered to avoid ambiguity.
>>    */
>>   static int cmd_ima_bootaggr(struct command *cmd)
>>   {
>> -- 
>> 2.17.1
>>
> Reviewed-by: Bruno Meneguele <bmeneg@redhat.com>
>


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

* [PATCH] ima_evm_utils: extended calc_bootaggr to PCRs 8 - 9
  2020-06-24 21:17                         ` Stefan Berger
@ 2020-06-24 21:33                           ` Maurizio Drocco
  2020-06-24 21:33                           ` [PATCH v2] " Bruno Meneguele
  1 sibling, 0 replies; 22+ messages in thread
From: Maurizio Drocco @ 2020-06-24 21:33 UTC (permalink / raw)
  To: stefanb
  Cc: Silviu.Vlasceanu, bmeneg, dmitry.kasatkin, jejb, jmorris,
	linux-integrity, linux-kernel, linux-security-module,
	maurizio.drocco, mdrocco, roberto.sassu, serge, zohar

From: Maurizio <maurizio.drocco@ibm.com>

cal_bootaggr should include PCRs 8-9 in non-SHA1 digests.

Signed-off-by: Maurizio Drocco <maurizio.drocco@ibm.com>
---
Changelog:
v3:
- Fixed patch description
v2:
- Always include PCRs 8 & 9 to non-sha1 hashes
v1:
- Include non-zero PCRs 8 & 9 to boot aggregates

 src/evmctl.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/evmctl.c b/src/evmctl.c
index 1d065ce..46b7092 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -1930,6 +1930,16 @@ static void calc_bootaggr(struct tpm_bank_info *bank)
 		}
 	}
 
+	if (strcmp(bank->algo_name, "sha1") != 0) {
+		for (i = 8; i < 10; i++) {
+			err = EVP_DigestUpdate(pctx, bank->pcr[i], bank->digest_size);
+			if (!err) {
+				log_err("EVP_DigestUpdate() failed\n");
+				return;
+			}
+		}
+	}
+
 	err = EVP_DigestFinal(pctx, bank->digest, &mdlen);
 	if (!err) {
 		log_err("EVP_DigestFinal() failed\n");
@@ -1972,8 +1982,9 @@ static int append_bootaggr(char *bootaggr, struct tpm_bank_info *tpm_banks)
 /*
  * The IMA measurement list boot_aggregate is the link between the preboot
  * event log and the IMA measurement list.  Read and calculate all the
- * possible per TPM bank boot_aggregate digests based on the existing
- * PCRs 0 - 7 to validate against the IMA boot_aggregate record.
+ * possible per TPM bank boot_aggregate digests based on the existing PCRs
+ * 0 - 9 to validate against the IMA boot_aggregate record. If the digest
+ * algorithm is SHA1, only PCRs 0 - 7 are considered to avoid ambiguity.
  */
 static int cmd_ima_bootaggr(struct command *cmd)
 {
-- 
2.17.1


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

* Re: [PATCH v2] ima_evm_utils: extended calc_bootaggr to PCRs 8 - 9
  2020-06-24 21:17                         ` Stefan Berger
  2020-06-24 21:33                           ` [PATCH] " Maurizio Drocco
@ 2020-06-24 21:33                           ` Bruno Meneguele
  2020-06-24 21:35                             ` [PATCH v3] " Maurizio Drocco
  1 sibling, 1 reply; 22+ messages in thread
From: Bruno Meneguele @ 2020-06-24 21:33 UTC (permalink / raw)
  To: Stefan Berger
  Cc: Maurizio Drocco, zohar, Silviu.Vlasceanu, dmitry.kasatkin, jejb,
	jmorris, linux-integrity, linux-kernel, linux-security-module,
	mdrocco, roberto.sassu, serge


[-- Attachment #1: Type: text/plain, Size: 2663 bytes --]

On Wed, Jun 24, 2020 at 05:17:52PM -0400, Stefan Berger wrote:
> On 6/23/20 2:13 PM, Bruno Meneguele wrote:
> > On Tue, Jun 23, 2020 at 02:01:22PM -0400, Maurizio Drocco wrote:
> > > From: Maurizio <maurizio.drocco@ibm.com>
> > > 
> > > If PCRs 8 - 9 are set (i.e. not all-zeros), cal_bootaggr should include
> > > them into the digest.
> 
> 
> Wouldn't you have to check for not all-zeros in your code?
> 

boot_aggregate in kernel, after the following patch be applied:

https://lkml.org/lkml/2020/6/23/833

is calculated regardless PCR 8 & 9 being zero or not.
Thus evmctl is only reflecting the same behavior.

I think it would be worth changing the commit log here.

> 
>    Stefan
> 
> 
> > > 
> > > Signed-off-by: Maurizio Drocco <maurizio.drocco@ibm.com>
> > > ---
> > > Changelog:
> > > v2:
> > > - Always include PCRs 8 & 9 to non-sha1 hashes
> > > v1:
> > > - Include non-zero PCRs 8 & 9 to boot aggregates
> > > 
> > >   src/evmctl.c | 15 +++++++++++++--
> > >   1 file changed, 13 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/src/evmctl.c b/src/evmctl.c
> > > index 1d065ce..46b7092 100644
> > > --- a/src/evmctl.c
> > > +++ b/src/evmctl.c
> > > @@ -1930,6 +1930,16 @@ static void calc_bootaggr(struct tpm_bank_info *bank)
> > >   		}
> > >   	}
> > > +	if (strcmp(bank->algo_name, "sha1") != 0) {
> > > +		for (i = 8; i < 10; i++) {
> > > +			err = EVP_DigestUpdate(pctx, bank->pcr[i], bank->digest_size);
> > > +			if (!err) {
> > > +				log_err("EVP_DigestUpdate() failed\n");
> > > +				return;
> > > +			}
> > > +		}
> > > +	}
> > > +
> > >   	err = EVP_DigestFinal(pctx, bank->digest, &mdlen);
> > >   	if (!err) {
> > >   		log_err("EVP_DigestFinal() failed\n");
> > > @@ -1972,8 +1982,9 @@ static int append_bootaggr(char *bootaggr, struct tpm_bank_info *tpm_banks)
> > >   /*
> > >    * The IMA measurement list boot_aggregate is the link between the preboot
> > >    * event log and the IMA measurement list.  Read and calculate all the
> > > - * possible per TPM bank boot_aggregate digests based on the existing
> > > - * PCRs 0 - 7 to validate against the IMA boot_aggregate record.
> > > + * possible per TPM bank boot_aggregate digests based on the existing PCRs
> > > + * 0 - 9 to validate against the IMA boot_aggregate record. If the digest
> > > + * algorithm is SHA1, only PCRs 0 - 7 are considered to avoid ambiguity.
> > >    */
> > >   static int cmd_ima_bootaggr(struct command *cmd)
> > >   {
> > > -- 
> > > 2.17.1
> > > 
> > Reviewed-by: Bruno Meneguele <bmeneg@redhat.com>
> > 
> 

-- 
bmeneg 
PGP Key: http://bmeneg.com/pubkey.txt

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* [PATCH v3] ima_evm_utils: extended calc_bootaggr to PCRs 8 - 9
  2020-06-24 21:33                           ` [PATCH v2] " Bruno Meneguele
@ 2020-06-24 21:35                             ` Maurizio Drocco
  2020-06-24 21:50                               ` Bruno Meneguele
  0 siblings, 1 reply; 22+ messages in thread
From: Maurizio Drocco @ 2020-06-24 21:35 UTC (permalink / raw)
  To: bmeneg
  Cc: Silviu.Vlasceanu, dmitry.kasatkin, jejb, jmorris,
	linux-integrity, linux-kernel, linux-security-module,
	maurizio.drocco, mdrocco, roberto.sassu, serge, stefanb, zohar

From: Maurizio <maurizio.drocco@ibm.com>

cal_bootaggr should include PCRs 8-9 in non-SHA1 digests.

Signed-off-by: Maurizio Drocco <maurizio.drocco@ibm.com>
---
Changelog:
v3:
- Fixed patch description
v2:
- Always include PCRs 8 & 9 to non-sha1 hashes
v1:
- Include non-zero PCRs 8 & 9 to boot aggregates

 src/evmctl.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/evmctl.c b/src/evmctl.c
index 1d065ce..46b7092 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -1930,6 +1930,16 @@ static void calc_bootaggr(struct tpm_bank_info *bank)
 		}
 	}
 
+	if (strcmp(bank->algo_name, "sha1") != 0) {
+		for (i = 8; i < 10; i++) {
+			err = EVP_DigestUpdate(pctx, bank->pcr[i], bank->digest_size);
+			if (!err) {
+				log_err("EVP_DigestUpdate() failed\n");
+				return;
+			}
+		}
+	}
+
 	err = EVP_DigestFinal(pctx, bank->digest, &mdlen);
 	if (!err) {
 		log_err("EVP_DigestFinal() failed\n");
@@ -1972,8 +1982,9 @@ static int append_bootaggr(char *bootaggr, struct tpm_bank_info *tpm_banks)
 /*
  * The IMA measurement list boot_aggregate is the link between the preboot
  * event log and the IMA measurement list.  Read and calculate all the
- * possible per TPM bank boot_aggregate digests based on the existing
- * PCRs 0 - 7 to validate against the IMA boot_aggregate record.
+ * possible per TPM bank boot_aggregate digests based on the existing PCRs
+ * 0 - 9 to validate against the IMA boot_aggregate record. If the digest
+ * algorithm is SHA1, only PCRs 0 - 7 are considered to avoid ambiguity.
  */
 static int cmd_ima_bootaggr(struct command *cmd)
 {
-- 
2.17.1


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

* Re: [PATCH v3] ima_evm_utils: extended calc_bootaggr to PCRs 8 - 9
  2020-06-24 21:35                             ` [PATCH v3] " Maurizio Drocco
@ 2020-06-24 21:50                               ` Bruno Meneguele
  0 siblings, 0 replies; 22+ messages in thread
From: Bruno Meneguele @ 2020-06-24 21:50 UTC (permalink / raw)
  To: Maurizio Drocco
  Cc: Silviu.Vlasceanu, dmitry.kasatkin, jejb, jmorris,
	linux-integrity, linux-kernel, linux-security-module, mdrocco,
	roberto.sassu, serge, stefanb, zohar


[-- Attachment #1: Type: text/plain, Size: 1959 bytes --]

On Wed, Jun 24, 2020 at 05:35:58PM -0400, Maurizio Drocco wrote:
> From: Maurizio <maurizio.drocco@ibm.com>
> 
> cal_bootaggr should include PCRs 8-9 in non-SHA1 digests.
> 
> Signed-off-by: Maurizio Drocco <maurizio.drocco@ibm.com>
> ---
> Changelog:
> v3:
> - Fixed patch description
> v2:
> - Always include PCRs 8 & 9 to non-sha1 hashes
> v1:
> - Include non-zero PCRs 8 & 9 to boot aggregates
> 
>  src/evmctl.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/src/evmctl.c b/src/evmctl.c
> index 1d065ce..46b7092 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -1930,6 +1930,16 @@ static void calc_bootaggr(struct tpm_bank_info *bank)
>  		}
>  	}
>  
> +	if (strcmp(bank->algo_name, "sha1") != 0) {
> +		for (i = 8; i < 10; i++) {
> +			err = EVP_DigestUpdate(pctx, bank->pcr[i], bank->digest_size);
> +			if (!err) {
> +				log_err("EVP_DigestUpdate() failed\n");
> +				return;
> +			}
> +		}
> +	}
> +
>  	err = EVP_DigestFinal(pctx, bank->digest, &mdlen);
>  	if (!err) {
>  		log_err("EVP_DigestFinal() failed\n");
> @@ -1972,8 +1982,9 @@ static int append_bootaggr(char *bootaggr, struct tpm_bank_info *tpm_banks)
>  /*
>   * The IMA measurement list boot_aggregate is the link between the preboot
>   * event log and the IMA measurement list.  Read and calculate all the
> - * possible per TPM bank boot_aggregate digests based on the existing
> - * PCRs 0 - 7 to validate against the IMA boot_aggregate record.
> + * possible per TPM bank boot_aggregate digests based on the existing PCRs
> + * 0 - 9 to validate against the IMA boot_aggregate record. If the digest
> + * algorithm is SHA1, only PCRs 0 - 7 are considered to avoid ambiguity.
>   */
>  static int cmd_ima_bootaggr(struct command *cmd)
>  {
> -- 
> 2.17.1
> 

Reviewed-by: Bruno Meneguele <bmeneg@redhat.com>

Thanks.

-- 
bmeneg 
PGP Key: http://bmeneg.com/pubkey.txt

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

end of thread, back to index

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-11 19:54 [PATCH] extend IMA boot_aggregate with kernel measurements Maurizio Drocco
2020-06-12  0:29 ` Mimi Zohar
2020-06-12 14:38   ` Maurizio Drocco
2020-06-12 15:11     ` Roberto Sassu
2020-06-12 17:14       ` James Bottomley
2020-06-16 17:29         ` Roberto Sassu
2020-06-16 18:11           ` Mimi Zohar
2020-06-18 12:38             ` Roberto Sassu
2020-06-18 20:11               ` Maurizio Drocco
2020-06-18 20:11                 ` [PATCH] ima_evm_utils: extended calc_bootaggr to PCRs 8 - 9 Maurizio Drocco
2020-06-22 20:14                   ` Mimi Zohar
2020-06-22  4:50                     ` [PATCH] ima: extend boot_aggregate with kernel measurements Maurizio Drocco
2020-06-23 14:03                       ` Mimi Zohar
2020-06-23 15:57                         ` [PATCH v4] " Maurizio Drocco
2020-06-23 18:53                           ` Bruno Meneguele
2020-06-23 18:01                     ` [PATCH v2] ima_evm_utils: extended calc_bootaggr to PCRs 8 - 9 Maurizio Drocco
2020-06-23 18:13                       ` Bruno Meneguele
2020-06-24 21:17                         ` Stefan Berger
2020-06-24 21:33                           ` [PATCH] " Maurizio Drocco
2020-06-24 21:33                           ` [PATCH v2] " Bruno Meneguele
2020-06-24 21:35                             ` [PATCH v3] " Maurizio Drocco
2020-06-24 21:50                               ` Bruno Meneguele

Linux-Integrity Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-integrity/0 linux-integrity/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-integrity linux-integrity/ https://lore.kernel.org/linux-integrity \
		linux-integrity@vger.kernel.org
	public-inbox-index linux-integrity

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-integrity


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git