linux-kernel - Re: [PATCH] ima: fix calculating the boot

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 26 Jan 2020 09:45:59 -0800
From:   James Bottomley <James.Bottomley@...senPartnership.com>
To:     Mimi Zohar <zohar@...ux.ibm.com>, linux-integrity@...r.kernel.org
Cc:     Jerry Snitselaar <jsnitsel@...hat.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ima: fix calculating the boot_aggregate

On Sun, 2020-01-26 at 08:13 -0500, Mimi Zohar wrote:
> Calculating the boot_aggregate assumes that the TPM SHA1 bank is
> enabled.  Before trying to read the TPM SHA1 bank, ensure it is
> enabled. If it isn't enabled, calculate the boot_aggregate using the
> first bank enabled.

Isn't it about time we shifted IMA away from SHA1 as a NIST deprecated
algorithm especially as in this case if someone can manufacture a sha1
hash collision, they can fake the TCB?  I think we should always try
use SHA256 if we have a TPM2, then fall back to whatever bank0 is if
SHA256 can't be found (that will cope with DELLs that violate the TPM2
spec by disabling the sha256 bank if the bios setting is sha1).  This
should also cope with other ODMs who violate the spec in other ways,
like not updating the sha1 bank but still leaving it allocated.

Mechanically, also, you don't need the found variable, you can see if i
reaches the max value.

James

---

diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 73044fc6a952..f5f7a3aec826 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -665,12 +665,29 @@ static int __init ima_calc_boot_aggregate_tfm(char *digest,
 	u32 i;
 	SHASH_DESC_ON_STACK(shash, tfm);
 
+	if (ima_tpm_chip->flags & TPM_CHIP_FLAG_TPM2)
+		/* TPM2 default should be sha256 */
+		d.alg_id = TPM_ALG_SHA256;
+
 	shash->tfm = tfm;
 
 	rc = crypto_shash_init(shash);
 	if (rc != 0)
 		return rc;
 
+	/*
+	 * Check the TPM default bank is allocated otherwise use the first one
+	 */
+	for (i = 0; i < ima_tpm_chip->nr_allocated_banks; i++)
+		if (ima_tpm_chip->allocated_banks[i].alg_id == d.alg_id)
+			break;
+
+	if (i == ima_tpm_chip->nr_allocated_banks) {
+		d.alg_id = ima_tpm_chip->allocated_banks[0].alg_id;
+		pr_info("Calculating the boot-aggregregate (TPM algorithm: %d)",
+			d.alg_id);
+	}
+
 	/* cumulative sha1 over tpm registers 0-7 */
 	for (i = TPM_PCR0; i < TPM_PCR8; i++) {
 		ima_pcrread(i, &d);