diff mbox series

[v6,5/7] tpm: retrieve digest size of unknown algorithms with PCR read

Message ID 20181204082138.24600-6-roberto.sassu@huawei.com (mailing list archive)
State New, archived
Headers show
Series tpm: retrieve digest size of unknown algorithms from TPM | expand

Commit Message

Roberto Sassu Dec. 4, 2018, 8:21 a.m. UTC 
Currently, the TPM driver retrieves the digest size from a table mapping
TPM algorithms identifiers to identifiers defined by the crypto subsystem.
If the algorithm is not defined by the latter, the digest size can be
retrieved from the output of the PCR read command.

The patch retrieves at TPM startup the digest sizes for each PCR bank and
stores them in the new structure tpm_bank_info, member of tpm_chip, so that
the information can be passed to other kernel subsystems.

tpm_bank_info contains: the TPM algorithm identifier, necessary to generate
the event log as defined by Trusted Computing Group (TCG); the digest size,
to pad/truncate a digest calculated with a different algorithm; the crypto
subsystem identifier, to calculate the digest of event data.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Acked-by: Mimi Zohar <zohar@linux.ibm.com>
---
 drivers/char/tpm/tpm-interface.c |  9 +++----
 drivers/char/tpm/tpm.h           |  4 +--
 drivers/char/tpm/tpm1-cmd.c      |  4 ++-
 drivers/char/tpm/tpm2-cmd.c      | 45 ++++++++++++++++++++++++--------
 include/linux/tpm.h              |  6 +++++
 5 files changed, 49 insertions(+), 19 deletions(-)

Comments

Jarkko Sakkinen Dec. 4, 2018, 11:53 p.m. UTC | #1 
On Tue, Dec 04, 2018 at 09:21:36AM +0100, Roberto Sassu wrote:
> +	for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
> +		enum hash_algo crypto_algo = tpm2_hash_map[i].crypto_id;
> +
> +		if (bank->alg_id != tpm2_hash_map[i].tpm_id)
> +			continue;
> +
> +		bank->digest_size = hash_digest_size[crypto_algo];
> +		bank->crypto_id = crypto_algo;
> +		return 0;
> +	}
> +
> +	return tpm2_pcr_read(chip, 0, &digest, &bank->digest_size);
> +}

This is a part that I don't get. Coud you just always call
tpm2_pcr_read() instead of this complexity

/Jarkko
Roberto Sassu Dec. 6, 2018, 6 p.m. UTC | #2 
On 12/5/2018 12:53 AM, Jarkko Sakkinen wrote:
> On Tue, Dec 04, 2018 at 09:21:36AM +0100, Roberto Sassu wrote:
>> +	for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
>> +		enum hash_algo crypto_algo = tpm2_hash_map[i].crypto_id;
>> +
>> +		if (bank->alg_id != tpm2_hash_map[i].tpm_id)
>> +			continue;
>> +
>> +		bank->digest_size = hash_digest_size[crypto_algo];
>> +		bank->crypto_id = crypto_algo;
>> +		return 0;
>> +	}
>> +
>> +	return tpm2_pcr_read(chip, 0, &digest, &bank->digest_size);
>> +}
> 
> This is a part that I don't get. Coud you just always call
> tpm2_pcr_read() instead of this complexity

First, we avoid operations that may increase the boot time. Second, the
loop is necessary to obtain the crypto subsystem identifier from a TPM
algorithm identifier.

Roberto


> /Jarkko
>
Jarkko Sakkinen Dec. 12, 2018, 6:16 p.m. UTC | #3 
On Thu, Dec 06, 2018 at 07:00:13PM +0100, Roberto Sassu wrote:
> On 12/5/2018 12:53 AM, Jarkko Sakkinen wrote:
> > On Tue, Dec 04, 2018 at 09:21:36AM +0100, Roberto Sassu wrote:
> > > +	for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
> > > +		enum hash_algo crypto_algo = tpm2_hash_map[i].crypto_id;
> > > +
> > > +		if (bank->alg_id != tpm2_hash_map[i].tpm_id)
> > > +			continue;
> > > +
> > > +		bank->digest_size = hash_digest_size[crypto_algo];
> > > +		bank->crypto_id = crypto_algo;
> > > +		return 0;
> > > +	}
> > > +
> > > +	return tpm2_pcr_read(chip, 0, &digest, &bank->digest_size);
> > > +}
> > 
> > This is a part that I don't get. Coud you just always call
> > tpm2_pcr_read() instead of this complexity
> 
> First, we avoid operations that may increase the boot time. Second, the
> loop is necessary to obtain the crypto subsystem identifier from a TPM
> algorithm identifier.

I think here would be a comment in place and it would be fine.

/Jarkko
diff mbox series

Patch

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index cea4c8d75ad0..eb7c79ca8a94 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -465,7 +465,7 @@  int tpm_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
 		return -ENODEV;
 
 	if (chip->flags & TPM_CHIP_FLAG_TPM2)
-		rc = tpm2_pcr_read(chip, pcr_idx, digest);
+		rc = tpm2_pcr_read(chip, pcr_idx, digest, NULL);
 	else
 		rc = tpm1_pcr_read(chip, pcr_idx, digest->digest);
 
@@ -480,9 +480,8 @@  EXPORT_SYMBOL_GPL(tpm_pcr_read);
  * @pcr_idx:	the PCR to be retrieved
  * @hash:	the hash value used to extend the PCR value
  *
- * Note: with TPM 2.0 extends also those banks with a known digest size to the
- * cryto subsystem in order to prevent malicious use of those PCR banks. In the
- * future we should dynamically determine digest sizes.
+ * Note: with TPM 2.0 extends also those banks for which no digest was
+ * specified in order to prevent malicious use of those PCR banks.
  *
  * Return: same as with tpm_transmit_cmd()
  */
@@ -503,7 +502,7 @@  int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash)
 			return -ENOMEM;
 
 		for (i = 0; i < chip->nr_allocated_banks; i++) {
-			digest_list[i].alg_id = chip->allocated_banks[i];
+			digest_list[i].alg_id = chip->allocated_banks[i].alg_id;
 			memcpy(digest_list[i].digest, hash, TPM_DIGEST_SIZE);
 		}
 
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 343aa58424f7..64d93d26087f 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -247,7 +247,7 @@  struct tpm_chip {
 	unsigned int groups_cnt;
 
 	u32 nr_allocated_banks;
-	u16 *allocated_banks;
+	struct tpm_bank_info *allocated_banks;
 #ifdef CONFIG_ACPI
 	acpi_handle acpi_dev_handle;
 	char ppi_version[TPM_PPI_VERSION_LEN + 1];
@@ -549,7 +549,7 @@  static inline u32 tpm2_rc_value(u32 rc)
 
 int tpm2_get_timeouts(struct tpm_chip *chip);
 int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
-		  struct tpm_digest *digest);
+		  struct tpm_digest *digest, u16 *digest_size_ptr);
 int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
 		    struct tpm_digest *digests);
 int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
index ec4f197bae0a..8b70a7f884a7 100644
--- a/drivers/char/tpm/tpm1-cmd.c
+++ b/drivers/char/tpm/tpm1-cmd.c
@@ -716,7 +716,9 @@  int tpm1_auto_startup(struct tpm_chip *chip)
 		goto out;
 	}
 
-	chip->allocated_banks[0] = TPM_ALG_SHA1;
+	chip->allocated_banks[0].alg_id = TPM_ALG_SHA1;
+	chip->allocated_banks[0].digest_size = hash_digest_size[HASH_ALGO_SHA1];
+	chip->allocated_banks[0].crypto_id = HASH_ALGO_SHA1;
 	chip->nr_allocated_banks = 1;
 
 	return rc;
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 81740d55f19c..a804f0b7126a 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -172,11 +172,12 @@  struct tpm2_pcr_read_out {
  * @chip:	TPM chip to use.
  * @pcr_idx:	index of the PCR to read.
  * @digest:	PCR bank and buffer current PCR value is written to.
+ * @digest_size_ptr:	pointer to variable that stores the digest size.
  *
  * Return: Same as with tpm_transmit_cmd.
  */
 int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
-		  struct tpm_digest *digest)
+		  struct tpm_digest *digest, u16 *digest_size_ptr)
 {
 	int rc;
 	struct tpm_buf buf;
@@ -211,6 +212,9 @@  int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
 		goto out;
 	}
 
+	if (digest_size_ptr)
+		*digest_size_ptr = digest_size;
+
 	memcpy(digest->digest, out->digest, digest_size);
 out:
 	tpm_buf_destroy(&buf);
@@ -241,7 +245,6 @@  int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
 	struct tpm2_null_auth_area auth_area;
 	int rc;
 	int i;
-	int j;
 
 	if (count > chip->nr_allocated_banks)
 		return -EINVAL;
@@ -263,14 +266,9 @@  int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
 	tpm_buf_append_u32(&buf, count);
 
 	for (i = 0; i < count; i++) {
-		for (j = 0; j < ARRAY_SIZE(tpm2_hash_map); j++) {
-			if (digests[i].alg_id != tpm2_hash_map[j].tpm_id)
-				continue;
-			tpm_buf_append_u16(&buf, digests[i].alg_id);
-			tpm_buf_append(&buf, (const unsigned char
-					      *)&digests[i].digest,
-			       hash_digest_size[tpm2_hash_map[j].crypto_id]);
-		}
+		tpm_buf_append_u16(&buf, digests[i].alg_id);
+		tpm_buf_append(&buf, (const unsigned char *)&digests[i].digest,
+			       chip->allocated_banks[i].digest_size);
 	}
 
 	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
@@ -821,6 +819,26 @@  int tpm2_probe(struct tpm_chip *chip)
 }
 EXPORT_SYMBOL_GPL(tpm2_probe);
 
+static int tpm2_init_bank_info(struct tpm_chip *chip, u32 bank_index)
+{
+	struct tpm_bank_info *bank = chip->allocated_banks + bank_index;
+	struct tpm_digest digest = { .alg_id = bank->alg_id };
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
+		enum hash_algo crypto_algo = tpm2_hash_map[i].crypto_id;
+
+		if (bank->alg_id != tpm2_hash_map[i].tpm_id)
+			continue;
+
+		bank->digest_size = hash_digest_size[crypto_algo];
+		bank->crypto_id = crypto_algo;
+		return 0;
+	}
+
+	return tpm2_pcr_read(chip, 0, &digest, &bank->digest_size);
+}
+
 struct tpm2_pcr_selection {
 	__be16  hash_alg;
 	u8  size_of_select;
@@ -888,7 +906,12 @@  static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
 				break;
 
 		if (j < pcr_selection.size_of_select) {
-			chip->allocated_banks[nr_alloc_banks] = hash_alg;
+			chip->allocated_banks[nr_alloc_banks].alg_id = hash_alg;
+
+			rc = tpm2_init_bank_info(chip, nr_alloc_banks);
+			if (rc < 0)
+				break;
+
 			nr_alloc_banks++;
 		}
 
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 95e11688826e..72df8d4252ef 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -46,6 +46,12 @@  struct tpm_digest {
 	u8 digest[SHA512_DIGEST_SIZE];
 } __packed;
 
+struct tpm_bank_info {
+	u16 alg_id;
+	u16 digest_size;
+	u16 crypto_id;
+};
+
 enum TPM_OPS_FLAGS {
 	TPM_OPS_AUTO_STARTUP = BIT(0),
 };