[PATCH] ima_evm_utils: extended calc_bootaggr to PCRs 8 - 9

[PATCH] ima_evm_utils: extended calc_bootaggr to PCRs 8 - 9

[Maurizio Drocco]

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 | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/evmctl.c b/src/evmctl.c
index 1d065ce..701d643 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -1930,6 +1930,16 @@ static void calc_bootaggr(struct tpm_bank_info *bank)
 		}
 	}
 
+	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 +1983,8 @@ 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), only PCRs 0 - 7 are considered.
  */
 static int cmd_ima_bootaggr(struct command *cmd)
 {
-- 
2.17.1

[PATCH] ima_evm_utils: tests: boot_aggregate.test spans PCRs 0-9

[Maurizio Drocco]

display_pcrs() should include PCRS 8 - 9 as they are non-zeros on some
systems. boot_aggregate may span PCRs 0 - 9 so check()'s info message
should be fixed accordingly.

Signed-off-by: Maurizio Drocco <maurizio.drocco@ibm.com>
---
 tests/boot_aggregate.test | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/boot_aggregate.test b/tests/boot_aggregate.test
index 39bd058..8a5581f 100755
--- a/tests/boot_aggregate.test
+++ b/tests/boot_aggregate.test
@@ -81,12 +81,12 @@ swtpm_init() {
 
 # In VERBOSE mode, display the calculated TPM PCRs for the different banks.
 display_pcrs() {
-	local PCRMAX=7
+	local PCRMAX=9
 	local banks=("sha1" "sha256")
 	local i;
 
 	for bank in "${banks[@]}"; do
-		echo "INFO: Displaying ${bank} TPM bank (PCRs 0 - 7)"
+		echo "INFO: Displaying ${bank} TPM bank (PCRs 0 - 9)"
 		for i in $(seq 0 $PCRMAX); do
 			rc=0
 			pcr=$("${TSSDIR}/tsspcrread" -halg "${bank}" -ha "${i}" -ns)
@@ -107,7 +107,7 @@ display_pcrs() {
 # Verify that the last "boot_aggregate" record in the IMA measurement
 # list matches.
 check() {
-	echo "INFO: Calculating the boot_aggregate (PCRs 0 - 7) for multiple banks"
+	echo "INFO: Calculating the boot_aggregate (PCRs 0 - 9) for multiple banks"
 	bootaggr=$(evmctl ima_boot_aggregate)
 	if [ $? -ne 0 ]; then
 		echo "${CYAN}SKIP: evmctl ima_boot_aggregate: $bootaggr${NORM}"
-- 
2.17.1

Re: [PATCH] ima_evm_utils: extended calc_bootaggr to PCRs 8 - 9

[Mimi Zohar]

Hi Maurizio,

On Tue, 2020-06-16 at 08:02 -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>

Thank you, this patch seems to be working properly.  As a separate
patch, could you fix the tests/boot_aggregate.test comments "PCRs 0
-7" and other things, like displaying just PCRs 0 - 7?

thanks,

Mimi

[PATCH] ima_evm_utils: extended calc_bootaggr to PCRs 8 - 9

[Maurizio Drocco]

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

Re: [PATCH] ima_evm_utils: extended calc_bootaggr to PCRs 8 - 9

[Mimi Zohar]

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

[PATCH] ima_evm_utils: extended calc_bootaggr to PCRs 8 - 9

[Maurizio Drocco]

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