From: Maurizio <[email protected]>
If PCRs 8 - 9 are set (i.e. not all-zeros), cal_bootaggr should include
them into the digest.
Signed-off-by: Maurizio Drocco <[email protected]>
---
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
On Thu, 2020-06-18 at 16:11 -0400, Maurizio Drocco wrote:
> From: Maurizio <[email protected]>
>
> If PCRs 8 - 9 are set (i.e. not all-zeros), cal_bootaggr should include
> them into the digest.
>
> Signed-off-by: Maurizio Drocco <[email protected]>
> ---
> 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
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 <[email protected]>
---
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
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 <[email protected]>
> ---
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;
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 <[email protected]>
---
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
From: Maurizio <[email protected]>
If PCRs 8 - 9 are set (i.e. not all-zeros), cal_bootaggr should include
them into the digest.
Signed-off-by: Maurizio Drocco <[email protected]>
---
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
On Tue, Jun 23, 2020 at 02:01:22PM -0400, Maurizio Drocco wrote:
> From: Maurizio <[email protected]>
>
> If PCRs 8 - 9 are set (i.e. not all-zeros), cal_bootaggr should include
> them into the digest.
>
> Signed-off-by: Maurizio Drocco <[email protected]>
> ---
> 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 <[email protected]>
--
bmeneg
PGP Key: http://bmeneg.com/pubkey.txt
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 <[email protected]>
> ---
> 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 <[email protected]>
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
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 <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>> 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 <[email protected]>
>
From: Maurizio <[email protected]>
cal_bootaggr should include PCRs 8-9 in non-SHA1 digests.
Signed-off-by: Maurizio Drocco <[email protected]>
---
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
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 <[email protected]>
> > >
> > > 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 <[email protected]>
> > > ---
> > > 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 <[email protected]>
> >
>
--
bmeneg
PGP Key: http://bmeneg.com/pubkey.txt
From: Maurizio <[email protected]>
cal_bootaggr should include PCRs 8-9 in non-SHA1 digests.
Signed-off-by: Maurizio Drocco <[email protected]>
---
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
On Wed, Jun 24, 2020 at 05:35:58PM -0400, Maurizio Drocco wrote:
> From: Maurizio <[email protected]>
>
> cal_bootaggr should include PCRs 8-9 in non-SHA1 digests.
>
> Signed-off-by: Maurizio Drocco <[email protected]>
> ---
> 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 <[email protected]>
Thanks.
--
bmeneg
PGP Key: http://bmeneg.com/pubkey.txt