[1/2] ima: use the IMA configured hash algo to calculate the boot aggregate

Commit Message

Mimi Zohar Jan. 27, 2020, 4:01 p.m. UTC

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 <[e-mail]>
---
 security/integrity/ima/ima_init.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Roberto Sassu Jan. 27, 2020, 5:38 p.m. UTC | #1

> -----Original Message-----
> From: [e-mail] [mailto:linux-integrity-
> [e-mail]] On Behalf Of Mimi Zohar
> Sent: Monday, January 27, 2020 5:02 PM
> To: [e-mail]
> Cc: Jerry Snitselaar <[e-mail]>; James Bottomley
> <[e-mail]>; linux-
> [e-mail]; Mimi Zohar <[e-mail]>
> 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

Mimi Zohar Jan. 27, 2020, 6:16 p.m. UTC | #2

On Mon, 2020-01-27 at 17:38 +0000, Roberto Sassu wrote:
> > -----Original Message-----
> > From: [e-mail] [mailto:linux-integrity-
> > [e-mail]] On Behalf Of Mimi Zohar
> > Sent: Monday, January 27, 2020 5:02 PM
> > To: [e-mail]
> > Cc: Jerry Snitselaar <[e-mail]>; James Bottomley
> > <[e-mail]>; linux-
> > [e-mail]; Mimi Zohar <[e-mail]>
> > 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

Mimi Zohar Jan. 27, 2020, 6:35 p.m. UTC | #3

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: [e-mail] [mailto:linux-integrity-
> > > [e-mail]] On Behalf Of Mimi Zohar
> > > Sent: Monday, January 27, 2020 5:02 PM
> > > To: [e-mail]
> > > Cc: Jerry Snitselaar <[e-mail]>; James Bottomley
> > > <[e-mail]>; linux-
> > > [e-mail]; Mimi Zohar <[e-mail]>
> > > 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

Jerry Snitselaar Jan. 27, 2020, 8:49 p.m. UTC | #4

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 <[e-mail]>
>---
> 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

Mimi Zohar Jan. 27, 2020, 9:31 p.m. UTC | #5

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 <[e-mail]>
> >---
> > 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

Petr Vorel Jan. 29, 2020, 8:30 a.m. UTC | #6

Hi Mimi,

Reviewed-by: Petr Vorel <[e-mail]>

> 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

Mimi Zohar Jan. 29, 2020, 10:51 p.m. UTC | #7

On Wed, 2020-01-29 at 09:30 +0100, Petr Vorel wrote:
> Hi Mimi,
> 
> Reviewed-by: Petr Vorel <[e-mail]>
> 
> > 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

Petr Vorel Jan. 30, 2020, 8:41 a.m. UTC | #8

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

Roberto Sassu Jan. 30, 2020, 3:27 p.m. UTC | #9

> -----Original Message-----
> From: [e-mail] [mailto:linux-integrity-
> [e-mail]] On Behalf Of Mimi Zohar
> Sent: Wednesday, January 29, 2020 11:51 PM
> To: Petr Vorel <[e-mail]>
> Cc: Jerry Snitselaar <[e-mail]>; [e-mail];
> James Bottomley <[e-mail]>; linux-
> [e-mail]; Roberto Sassu <[e-mail]>
> 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 <[e-mail]>
> >
> > > 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

Roberto Sassu Jan. 30, 2020, 3:40 p.m. UTC | #10

> -----Original Message-----
> From: [e-mail] [mailto:linux-integrity-
> [e-mail]] On Behalf Of Roberto Sassu
> Sent: Thursday, January 30, 2020 4:27 PM
> To: Mimi Zohar <[e-mail]>; Petr Vorel <[e-mail]>
> Cc: Jerry Snitselaar <[e-mail]>; [e-mail];
> James Bottomley <[e-mail]>; linux-
> [e-mail]
> Subject: RE: [PATCH 1/2] ima: use the IMA configured hash algo to calculate
> the boot aggregate
> 
> > -----Original Message-----
> > From: [e-mail] [mailto:linux-integrity-
> > [e-mail]] On Behalf Of Mimi Zohar
> > Sent: Wednesday, January 29, 2020 11:51 PM
> > To: Petr Vorel <[e-mail]>
> > Cc: Jerry Snitselaar <[e-mail]>; linux-
> [e-mail];
> > James Bottomley <[e-mail]>; linux-
> > [e-mail]; Roberto Sassu <[e-mail]>
> > 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 <[e-mail]>
> > >
> > > > 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

Patch

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