All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/2] enhance TPM 2.0 extend function to support multiple PCR banks
@ 2017-01-18  8:44 ` Nayna Jain
  0 siblings, 0 replies; 15+ messages in thread
From: Nayna Jain @ 2017-01-18  8:44 UTC (permalink / raw)
  To: tpmdd-devel
  Cc: peterhuewe, tpmdd, jarkko.sakkinen, jgunthorpe,
	linux-security-module, linux-kernel, Nayna Jain

IMA extends its hash measurements in the TPM PCRs, based on policy.
The existing in-kernel TPM extend function extends only the SHA1
PCR bank. TPM 2.0 defines multiple PCR banks, to support different
hash algorithms. The TCG TPM 2.0 Specification[1] recommends
extending all active PCR banks to prevent malicious users from
setting unused PCR banks with fake measurements and quoting them.
This patch set adds support for extending all active PCR banks,
as recommended.

The first patch implements the TPM 2.0 capability to retrieve
the list of active PCR banks.

The second patch modifies the tpm_pcr_extend() and tpm2_pcr_extend()
interface to support extending multiple PCR banks. The existing
tpm_pcr_extend() interface expects only a SHA1 digest. Hence, to
extend all active PCR banks with differing digest sizes for TPM 2.0,
the SHA1 digest is padded with 0's as needed.

[1] TPM 2.0 Specification referred here is "TCG PC Client Specific
Platform Firmware Profile for TPM 2.0"

Changelog v4:
- Updated cover letter as per Mimi's feedback.
- Rebased to Jarkko's latest master branch (4064b6b tpm_tis: use
  default timeout value if chip reports it as zero)
- Patch "tpm: implement TPM 2.0 capability to get active PCR banks"
 - Included Jarkko's feedbacks
   - Moved call to tpm2_get_pcr_allocation to Patch 2
   - Renamed struct tpm2_tpms_pcr_selection to struct tpm2_pcr_selection 
   and moved the struct to before tpm2_get_pcr_allocation()
   - Fixed code formatting
- Patch "tpm: enchance TPM 2.0 PCR extend to support multiple banks"
 - Included Jarkkos' feedbacks
   - Updated commit msg to mention dependency on CRYPTO_HASH_INFO
   - Renamed struct tpmt_hash to struct tpm2_digest 
   - Removed struct tpml_digest_values, tpm2_pcr_extend() now accepts
   count and digests list as two separate arguments. Added check for
   count of hashes passed.
 - Cleaned up struct tpm2_pcr_extend_in as not required anymore with
 use of tpm_buf
 - Moved struct tpm2_null_auth_area just before tpm2_pcr_extend() as
 it is the only function using it for now.
 - Fixed code formatting

Changelog v3:
- Rebased to the Jarkko's latest master branch (8e25809 tpm:
  Do not print an error message when doing TPM auto startup)
- Patch "tpm: implement TPM 2.0 capability to get active PCR banks"
  - Included Jarkko's feedbacks
     - Removed getcap_in, getcap_out and used tpm_buf for getting
     capability.
     - Used ARRAY_SIZE in place of TPM_MAX_PCR_BANKS and included
     other feedbacks.
- Patch "tpm: enhance TPM 2.0 PCR extend to support multiple banks"
     - Fixed kbuild errors
       - Fixed buf.data uninitialized warning.
       - Added TCG_TPM dependency on CONFIG_CRYPTO_HASH_INFO in Kconfig.

Changelog v2:

- Patch "tpm: implement TPM 2.0 capability to get active PCR banks"
  - defined structs definition in tpm2-cmd.c.
  - no_of_active_banks field is removed. Instead, constant
  TPM2_MAX_PCR_BANKS is defined.
  - renamed tpm2_get_active_pcr_banks() to tpm2_get_pcr_allocation()
  - removed generic function tpm2_get_capability().

- Patch "tpm: enchance TPM 2.0 PCR extend to support multiple banks"
 - Removed tpm2.h, and defined structs common for extend and event log
  in tpm_eventlog.h
 - uses tpm_buf in tpm2_pcr_extend().

Nayna Jain (2):
  tpm: implement TPM 2.0 capability to get active PCR banks
  tpm: enhance TPM 2.0 PCR extend to support multiple banks

 drivers/char/tpm/Kconfig         |   1 +
 drivers/char/tpm/tpm-interface.c |  15 +++-
 drivers/char/tpm/tpm.h           |   7 +-
 drivers/char/tpm/tpm2-cmd.c      | 148 ++++++++++++++++++++++++++++-----------
 drivers/char/tpm/tpm_eventlog.h  |   7 ++
 5 files changed, 134 insertions(+), 44 deletions(-)

-- 
2.5.0

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v4 0/2] enhance TPM 2.0 extend function to support multiple PCR banks
@ 2017-01-18  8:44 ` Nayna Jain
  0 siblings, 0 replies; 15+ messages in thread
From: Nayna Jain @ 2017-01-18  8:44 UTC (permalink / raw)
  To: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA

IMA extends its hash measurements in the TPM PCRs, based on policy.
The existing in-kernel TPM extend function extends only the SHA1
PCR bank. TPM 2.0 defines multiple PCR banks, to support different
hash algorithms. The TCG TPM 2.0 Specification[1] recommends
extending all active PCR banks to prevent malicious users from
setting unused PCR banks with fake measurements and quoting them.
This patch set adds support for extending all active PCR banks,
as recommended.

The first patch implements the TPM 2.0 capability to retrieve
the list of active PCR banks.

The second patch modifies the tpm_pcr_extend() and tpm2_pcr_extend()
interface to support extending multiple PCR banks. The existing
tpm_pcr_extend() interface expects only a SHA1 digest. Hence, to
extend all active PCR banks with differing digest sizes for TPM 2.0,
the SHA1 digest is padded with 0's as needed.

[1] TPM 2.0 Specification referred here is "TCG PC Client Specific
Platform Firmware Profile for TPM 2.0"

Changelog v4:
- Updated cover letter as per Mimi's feedback.
- Rebased to Jarkko's latest master branch (4064b6b tpm_tis: use
  default timeout value if chip reports it as zero)
- Patch "tpm: implement TPM 2.0 capability to get active PCR banks"
 - Included Jarkko's feedbacks
   - Moved call to tpm2_get_pcr_allocation to Patch 2
   - Renamed struct tpm2_tpms_pcr_selection to struct tpm2_pcr_selection 
   and moved the struct to before tpm2_get_pcr_allocation()
   - Fixed code formatting
- Patch "tpm: enchance TPM 2.0 PCR extend to support multiple banks"
 - Included Jarkkos' feedbacks
   - Updated commit msg to mention dependency on CRYPTO_HASH_INFO
   - Renamed struct tpmt_hash to struct tpm2_digest 
   - Removed struct tpml_digest_values, tpm2_pcr_extend() now accepts
   count and digests list as two separate arguments. Added check for
   count of hashes passed.
 - Cleaned up struct tpm2_pcr_extend_in as not required anymore with
 use of tpm_buf
 - Moved struct tpm2_null_auth_area just before tpm2_pcr_extend() as
 it is the only function using it for now.
 - Fixed code formatting

Changelog v3:
- Rebased to the Jarkko's latest master branch (8e25809 tpm:
  Do not print an error message when doing TPM auto startup)
- Patch "tpm: implement TPM 2.0 capability to get active PCR banks"
  - Included Jarkko's feedbacks
     - Removed getcap_in, getcap_out and used tpm_buf for getting
     capability.
     - Used ARRAY_SIZE in place of TPM_MAX_PCR_BANKS and included
     other feedbacks.
- Patch "tpm: enhance TPM 2.0 PCR extend to support multiple banks"
     - Fixed kbuild errors
       - Fixed buf.data uninitialized warning.
       - Added TCG_TPM dependency on CONFIG_CRYPTO_HASH_INFO in Kconfig.

Changelog v2:

- Patch "tpm: implement TPM 2.0 capability to get active PCR banks"
  - defined structs definition in tpm2-cmd.c.
  - no_of_active_banks field is removed. Instead, constant
  TPM2_MAX_PCR_BANKS is defined.
  - renamed tpm2_get_active_pcr_banks() to tpm2_get_pcr_allocation()
  - removed generic function tpm2_get_capability().

- Patch "tpm: enchance TPM 2.0 PCR extend to support multiple banks"
 - Removed tpm2.h, and defined structs common for extend and event log
  in tpm_eventlog.h
 - uses tpm_buf in tpm2_pcr_extend().

Nayna Jain (2):
  tpm: implement TPM 2.0 capability to get active PCR banks
  tpm: enhance TPM 2.0 PCR extend to support multiple banks

 drivers/char/tpm/Kconfig         |   1 +
 drivers/char/tpm/tpm-interface.c |  15 +++-
 drivers/char/tpm/tpm.h           |   7 +-
 drivers/char/tpm/tpm2-cmd.c      | 148 ++++++++++++++++++++++++++++-----------
 drivers/char/tpm/tpm_eventlog.h  |   7 ++
 5 files changed, 134 insertions(+), 44 deletions(-)

-- 
2.5.0


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v4 1/2] tpm: implement TPM 2.0 capability to get active PCR banks
@ 2017-01-18  8:44   ` Nayna Jain
  0 siblings, 0 replies; 15+ messages in thread
From: Nayna Jain @ 2017-01-18  8:44 UTC (permalink / raw)
  To: tpmdd-devel
  Cc: peterhuewe, tpmdd, jarkko.sakkinen, jgunthorpe,
	linux-security-module, linux-kernel, Nayna Jain

This patch implements the TPM 2.0 capability TPM_CAP_PCRS to
retrieve the active PCR banks from the TPM. This is needed
to enable extending all active banks as recommended by TPM 2.0
TCG Specification.

Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
---
 drivers/char/tpm/tpm.h      |  4 ++++
 drivers/char/tpm/tpm2-cmd.c | 57 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 1ae9768..dddd573 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -127,6 +127,7 @@ enum tpm2_permanent_handles {
 };
 
 enum tpm2_capabilities {
+	TPM2_CAP_PCRS		= 5,
 	TPM2_CAP_TPM_PROPERTIES = 6,
 };
 
@@ -187,6 +188,8 @@ struct tpm_chip {
 
 	const struct attribute_group *groups[3];
 	unsigned int groups_cnt;
+
+	u16 active_banks[7];
 #ifdef CONFIG_ACPI
 	acpi_handle acpi_dev_handle;
 	char ppi_version[TPM_PPI_VERSION_LEN + 1];
@@ -545,4 +548,5 @@ int tpm2_auto_startup(struct tpm_chip *chip);
 void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type);
 unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
 int tpm2_probe(struct tpm_chip *chip);
+ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip);
 #endif
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 6eda239..75a7546 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -998,3 +998,60 @@ int tpm2_auto_startup(struct tpm_chip *chip)
 		rc = -ENODEV;
 	return rc;
 }
+
+struct tpm2_pcr_selection {
+	__be16  hash_alg;
+	u8  size_of_select;
+	u8  pcr_select[3];
+} __packed;
+
+/**
+ * tpm2_get_pcr_allocation() - get TPM active PCR banks.
+ *
+ * @chip: TPM chip to use.
+ *
+ * Return: Same as with tpm_transmit_cmd.
+ */
+ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
+{
+	struct tpm2_pcr_selection pcr_selection;
+	struct tpm_buf buf;
+	void *marker;
+	unsigned int count = 0;
+	int rc;
+	int i;
+
+	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
+	if (rc)
+		return rc;
+
+	tpm_buf_append_u32(&buf, TPM2_CAP_PCRS);
+	tpm_buf_append_u32(&buf, 0);
+	tpm_buf_append_u32(&buf, 1);
+
+	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 0,
+			      "get tpm pcr allocation");
+	if (rc < 0)
+		goto out;
+
+	count = be32_to_cpup(
+		(__be32 *)&buf.data[TPM_HEADER_SIZE + 5]);
+
+	if (count > ARRAY_SIZE(chip->active_banks))
+		return -ENODEV;
+
+	marker = &buf.data[TPM_HEADER_SIZE + 9];
+	for (i = 0; i < count; i++) {
+		memcpy(&pcr_selection, marker, sizeof(pcr_selection));
+		chip->active_banks[i] = be16_to_cpu(pcr_selection.hash_alg);
+		marker = marker + sizeof(struct tpm2_pcr_selection);
+	}
+
+out:
+	if (count < ARRAY_SIZE(chip->active_banks))
+		chip->active_banks[count] = 0;
+
+	tpm_buf_destroy(&buf);
+
+	return rc;
+}
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v4 1/2] tpm: implement TPM 2.0 capability to get active PCR banks
@ 2017-01-18  8:44   ` Nayna Jain
  0 siblings, 0 replies; 15+ messages in thread
From: Nayna Jain @ 2017-01-18  8:44 UTC (permalink / raw)
  To: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA

This patch implements the TPM 2.0 capability TPM_CAP_PCRS to
retrieve the active PCR banks from the TPM. This is needed
to enable extending all active banks as recommended by TPM 2.0
TCG Specification.

Signed-off-by: Nayna Jain <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 drivers/char/tpm/tpm.h      |  4 ++++
 drivers/char/tpm/tpm2-cmd.c | 57 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 1ae9768..dddd573 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -127,6 +127,7 @@ enum tpm2_permanent_handles {
 };
 
 enum tpm2_capabilities {
+	TPM2_CAP_PCRS		= 5,
 	TPM2_CAP_TPM_PROPERTIES = 6,
 };
 
@@ -187,6 +188,8 @@ struct tpm_chip {
 
 	const struct attribute_group *groups[3];
 	unsigned int groups_cnt;
+
+	u16 active_banks[7];
 #ifdef CONFIG_ACPI
 	acpi_handle acpi_dev_handle;
 	char ppi_version[TPM_PPI_VERSION_LEN + 1];
@@ -545,4 +548,5 @@ int tpm2_auto_startup(struct tpm_chip *chip);
 void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type);
 unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
 int tpm2_probe(struct tpm_chip *chip);
+ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip);
 #endif
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 6eda239..75a7546 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -998,3 +998,60 @@ int tpm2_auto_startup(struct tpm_chip *chip)
 		rc = -ENODEV;
 	return rc;
 }
+
+struct tpm2_pcr_selection {
+	__be16  hash_alg;
+	u8  size_of_select;
+	u8  pcr_select[3];
+} __packed;
+
+/**
+ * tpm2_get_pcr_allocation() - get TPM active PCR banks.
+ *
+ * @chip: TPM chip to use.
+ *
+ * Return: Same as with tpm_transmit_cmd.
+ */
+ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
+{
+	struct tpm2_pcr_selection pcr_selection;
+	struct tpm_buf buf;
+	void *marker;
+	unsigned int count = 0;
+	int rc;
+	int i;
+
+	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
+	if (rc)
+		return rc;
+
+	tpm_buf_append_u32(&buf, TPM2_CAP_PCRS);
+	tpm_buf_append_u32(&buf, 0);
+	tpm_buf_append_u32(&buf, 1);
+
+	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 0,
+			      "get tpm pcr allocation");
+	if (rc < 0)
+		goto out;
+
+	count = be32_to_cpup(
+		(__be32 *)&buf.data[TPM_HEADER_SIZE + 5]);
+
+	if (count > ARRAY_SIZE(chip->active_banks))
+		return -ENODEV;
+
+	marker = &buf.data[TPM_HEADER_SIZE + 9];
+	for (i = 0; i < count; i++) {
+		memcpy(&pcr_selection, marker, sizeof(pcr_selection));
+		chip->active_banks[i] = be16_to_cpu(pcr_selection.hash_alg);
+		marker = marker + sizeof(struct tpm2_pcr_selection);
+	}
+
+out:
+	if (count < ARRAY_SIZE(chip->active_banks))
+		chip->active_banks[count] = 0;
+
+	tpm_buf_destroy(&buf);
+
+	return rc;
+}
-- 
2.5.0


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v4 2/2] tpm: enhance TPM 2.0 PCR extend to support multiple banks
@ 2017-01-18  8:44   ` Nayna Jain
  0 siblings, 0 replies; 15+ messages in thread
From: Nayna Jain @ 2017-01-18  8:44 UTC (permalink / raw)
  To: tpmdd-devel
  Cc: peterhuewe, tpmdd, jarkko.sakkinen, jgunthorpe,
	linux-security-module, linux-kernel, Nayna Jain

The current TPM 2.0 device driver extends only the SHA1 PCR bank
but the TCG Specification[1] recommends extending all active PCR
banks, to prevent malicious users from setting unused PCR banks with
fake measurements and quoting them.

The existing in-kernel interface(tpm_pcr_extend()) expects only a
SHA1 digest.  To extend all active PCR banks with differing
digest sizes, the SHA1 digest is padded with trailing 0's as needed.

This patch reuses the defined digest sizes from the crypto subsystem,
adding a dependency on CRYPTO_HASH_INFO module.

[1] TPM 2.0 Specification referred here is "TCG PC Client Specific
Platform Firmware Profile for TPM 2.0"

Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
---
 drivers/char/tpm/Kconfig         |  1 +
 drivers/char/tpm/tpm-interface.c | 15 ++++++-
 drivers/char/tpm/tpm.h           |  3 +-
 drivers/char/tpm/tpm2-cmd.c      | 91 +++++++++++++++++++++-------------------
 drivers/char/tpm/tpm_eventlog.h  |  7 ++++
 5 files changed, 73 insertions(+), 44 deletions(-)

diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index 277186d..af985cc 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -6,6 +6,7 @@ menuconfig TCG_TPM
 	tristate "TPM Hardware Support"
 	depends on HAS_IOMEM
 	select SECURITYFS
+	select CRYPTO_HASH_INFO
 	---help---
 	  If you have a TPM security chip in your system, which
 	  implements the Trusted Computing Group's specification,
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index a3461cb..00612dd 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -770,6 +770,7 @@ static const struct tpm_input_header pcrextend_header = {
 int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
 {
 	struct tpm_cmd_t cmd;
+	int i;
 	int rc;
 	struct tpm_chip *chip;
 
@@ -778,7 +779,19 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
 		return -ENODEV;
 
 	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
-		rc = tpm2_pcr_extend(chip, pcr_idx, hash);
+		struct tpm2_digest digest_list[ARRAY_SIZE(chip->active_banks)];
+		u32 count = 0;
+
+		memset(digest_list, 0, sizeof(digest_list));
+
+		for (i = 0; (chip->active_banks[i] != 0) &&
+		     (i < ARRAY_SIZE(chip->active_banks)); i++) {
+			digest_list[i].alg_id = chip->active_banks[i];
+			memcpy(digest_list[i].digest, hash, TPM_DIGEST_SIZE);
+			count++;
+		}
+
+		rc = tpm2_pcr_extend(chip, pcr_idx, count, digest_list);
 		tpm_put_ops(chip);
 		return rc;
 	}
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index dddd573..f578822 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -533,7 +533,8 @@ static inline void tpm_add_ppi(struct tpm_chip *chip)
 #endif
 
 int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
-int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash);
+int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
+		    struct tpm2_digest *digests);
 int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max);
 int tpm2_seal_trusted(struct tpm_chip *chip,
 		      struct trusted_key_payload *payload,
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 75a7546..7ceebbd 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -53,22 +53,6 @@ struct tpm2_pcr_read_out {
 	u8	digest[TPM_DIGEST_SIZE];
 } __packed;
 
-struct tpm2_null_auth_area {
-	__be32			handle;
-	__be16			nonce_size;
-	u8			attributes;
-	__be16			auth_size;
-} __packed;
-
-struct tpm2_pcr_extend_in {
-	__be32				pcr_idx;
-	__be32				auth_area_size;
-	struct tpm2_null_auth_area	auth_area;
-	__be32				digest_cnt;
-	__be16				hash_alg;
-	u8				digest[TPM_DIGEST_SIZE];
-} __packed;
-
 struct tpm2_get_tpm_pt_in {
 	__be32	cap_id;
 	__be32	property_id;
@@ -97,7 +81,6 @@ union tpm2_cmd_params {
 	struct	tpm2_self_test_in	selftest_in;
 	struct	tpm2_pcr_read_in	pcrread_in;
 	struct	tpm2_pcr_read_out	pcrread_out;
-	struct	tpm2_pcr_extend_in	pcrextend_in;
 	struct	tpm2_get_tpm_pt_in	get_tpm_pt_in;
 	struct	tpm2_get_tpm_pt_out	get_tpm_pt_out;
 	struct	tpm2_get_random_in	getrandom_in;
@@ -290,46 +273,68 @@ int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
 	return rc;
 }
 
-#define TPM2_GET_PCREXTEND_IN_SIZE \
-	(sizeof(struct tpm_input_header) + \
-	 sizeof(struct tpm2_pcr_extend_in))
-
-static const struct tpm_input_header tpm2_pcrextend_header = {
-	.tag = cpu_to_be16(TPM2_ST_SESSIONS),
-	.length = cpu_to_be32(TPM2_GET_PCREXTEND_IN_SIZE),
-	.ordinal = cpu_to_be32(TPM2_CC_PCR_EXTEND)
-};
+struct tpm2_null_auth_area {
+	__be32  handle;
+	__be16  nonce_size;
+	u8  attributes;
+	__be16  auth_size;
+} __packed;
 
 /**
  * tpm2_pcr_extend() - extend a PCR value
  *
  * @chip:	TPM chip to use.
  * @pcr_idx:	index of the PCR.
- * @hash:	hash value to use for the extend operation.
+ * @count:	number of digests passed.
+ * @digests:	list of pcr banks and corresponding digest values to extend.
  *
  * Return: Same as with tpm_transmit_cmd.
  */
-int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash)
+int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
+		    struct tpm2_digest *digests)
 {
-	struct tpm2_cmd cmd;
+	struct tpm_buf buf;
+	struct tpm2_null_auth_area auth_area;
 	int rc;
+	int i;
+	int j;
 
-	cmd.header.in = tpm2_pcrextend_header;
-	cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(pcr_idx);
-	cmd.params.pcrextend_in.auth_area_size =
-		cpu_to_be32(sizeof(struct tpm2_null_auth_area));
-	cmd.params.pcrextend_in.auth_area.handle =
-		cpu_to_be32(TPM2_RS_PW);
-	cmd.params.pcrextend_in.auth_area.nonce_size = 0;
-	cmd.params.pcrextend_in.auth_area.attributes = 0;
-	cmd.params.pcrextend_in.auth_area.auth_size = 0;
-	cmd.params.pcrextend_in.digest_cnt = cpu_to_be32(1);
-	cmd.params.pcrextend_in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
-	memcpy(cmd.params.pcrextend_in.digest, hash, TPM_DIGEST_SIZE);
+	if (count > ARRAY_SIZE(chip->active_banks))
+		return -EINVAL;
 
-	rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), 0,
+	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
+	if (rc)
+		return rc;
+
+	tpm_buf_append_u32(&buf, pcr_idx);
+
+	auth_area.handle = cpu_to_be32(TPM2_RS_PW);
+	auth_area.nonce_size = 0;
+	auth_area.attributes = 0;
+	auth_area.auth_size = 0;
+
+	tpm_buf_append_u32(&buf, sizeof(struct tpm2_null_auth_area));
+	tpm_buf_append(&buf, (const unsigned char *)&auth_area,
+		       sizeof(auth_area));
+	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]);
+		}
+	}
+
+	rc = tpm_transmit_cmd(chip, buf.data, tpm_buf_length(&buf), 0,
 			      "attempting extend a PCR value");
 
+	tpm_buf_destroy(&buf);
+
 	return rc;
 }
 
@@ -993,6 +998,8 @@ int tpm2_auto_startup(struct tpm_chip *chip)
 		}
 	}
 
+	rc = tpm2_get_pcr_allocation(chip);
+
 out:
 	if (rc > 0)
 		rc = -ENODEV;
diff --git a/drivers/char/tpm/tpm_eventlog.h b/drivers/char/tpm/tpm_eventlog.h
index 1660d74..b5ae372 100644
--- a/drivers/char/tpm/tpm_eventlog.h
+++ b/drivers/char/tpm/tpm_eventlog.h
@@ -2,6 +2,8 @@
 #ifndef __TPM_EVENTLOG_H__
 #define __TPM_EVENTLOG_H__
 
+#include <crypto/hash_info.h>
+
 #define TCG_EVENT_NAME_LEN_MAX	255
 #define MAX_TEXT_EVENT		1000	/* Max event string length */
 #define ACPI_TCPA_SIG		"TCPA"	/* 0x41504354 /'TCPA' */
@@ -73,6 +75,11 @@ enum tcpa_pc_event_ids {
 	HOST_TABLE_OF_DEVICES,
 };
 
+struct tpm2_digest {
+	u16 alg_id;
+	u8 digest[SHA512_DIGEST_SIZE];
+} __packed;
+
 #if defined(CONFIG_ACPI)
 int tpm_read_log_acpi(struct tpm_chip *chip);
 #else
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v4 2/2] tpm: enhance TPM 2.0 PCR extend to support multiple banks
@ 2017-01-18  8:44   ` Nayna Jain
  0 siblings, 0 replies; 15+ messages in thread
From: Nayna Jain @ 2017-01-18  8:44 UTC (permalink / raw)
  To: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA

The current TPM 2.0 device driver extends only the SHA1 PCR bank
but the TCG Specification[1] recommends extending all active PCR
banks, to prevent malicious users from setting unused PCR banks with
fake measurements and quoting them.

The existing in-kernel interface(tpm_pcr_extend()) expects only a
SHA1 digest.  To extend all active PCR banks with differing
digest sizes, the SHA1 digest is padded with trailing 0's as needed.

This patch reuses the defined digest sizes from the crypto subsystem,
adding a dependency on CRYPTO_HASH_INFO module.

[1] TPM 2.0 Specification referred here is "TCG PC Client Specific
Platform Firmware Profile for TPM 2.0"

Signed-off-by: Nayna Jain <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 drivers/char/tpm/Kconfig         |  1 +
 drivers/char/tpm/tpm-interface.c | 15 ++++++-
 drivers/char/tpm/tpm.h           |  3 +-
 drivers/char/tpm/tpm2-cmd.c      | 91 +++++++++++++++++++++-------------------
 drivers/char/tpm/tpm_eventlog.h  |  7 ++++
 5 files changed, 73 insertions(+), 44 deletions(-)

diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index 277186d..af985cc 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -6,6 +6,7 @@ menuconfig TCG_TPM
 	tristate "TPM Hardware Support"
 	depends on HAS_IOMEM
 	select SECURITYFS
+	select CRYPTO_HASH_INFO
 	---help---
 	  If you have a TPM security chip in your system, which
 	  implements the Trusted Computing Group's specification,
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index a3461cb..00612dd 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -770,6 +770,7 @@ static const struct tpm_input_header pcrextend_header = {
 int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
 {
 	struct tpm_cmd_t cmd;
+	int i;
 	int rc;
 	struct tpm_chip *chip;
 
@@ -778,7 +779,19 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
 		return -ENODEV;
 
 	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
-		rc = tpm2_pcr_extend(chip, pcr_idx, hash);
+		struct tpm2_digest digest_list[ARRAY_SIZE(chip->active_banks)];
+		u32 count = 0;
+
+		memset(digest_list, 0, sizeof(digest_list));
+
+		for (i = 0; (chip->active_banks[i] != 0) &&
+		     (i < ARRAY_SIZE(chip->active_banks)); i++) {
+			digest_list[i].alg_id = chip->active_banks[i];
+			memcpy(digest_list[i].digest, hash, TPM_DIGEST_SIZE);
+			count++;
+		}
+
+		rc = tpm2_pcr_extend(chip, pcr_idx, count, digest_list);
 		tpm_put_ops(chip);
 		return rc;
 	}
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index dddd573..f578822 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -533,7 +533,8 @@ static inline void tpm_add_ppi(struct tpm_chip *chip)
 #endif
 
 int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
-int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash);
+int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
+		    struct tpm2_digest *digests);
 int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max);
 int tpm2_seal_trusted(struct tpm_chip *chip,
 		      struct trusted_key_payload *payload,
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 75a7546..7ceebbd 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -53,22 +53,6 @@ struct tpm2_pcr_read_out {
 	u8	digest[TPM_DIGEST_SIZE];
 } __packed;
 
-struct tpm2_null_auth_area {
-	__be32			handle;
-	__be16			nonce_size;
-	u8			attributes;
-	__be16			auth_size;
-} __packed;
-
-struct tpm2_pcr_extend_in {
-	__be32				pcr_idx;
-	__be32				auth_area_size;
-	struct tpm2_null_auth_area	auth_area;
-	__be32				digest_cnt;
-	__be16				hash_alg;
-	u8				digest[TPM_DIGEST_SIZE];
-} __packed;
-
 struct tpm2_get_tpm_pt_in {
 	__be32	cap_id;
 	__be32	property_id;
@@ -97,7 +81,6 @@ union tpm2_cmd_params {
 	struct	tpm2_self_test_in	selftest_in;
 	struct	tpm2_pcr_read_in	pcrread_in;
 	struct	tpm2_pcr_read_out	pcrread_out;
-	struct	tpm2_pcr_extend_in	pcrextend_in;
 	struct	tpm2_get_tpm_pt_in	get_tpm_pt_in;
 	struct	tpm2_get_tpm_pt_out	get_tpm_pt_out;
 	struct	tpm2_get_random_in	getrandom_in;
@@ -290,46 +273,68 @@ int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
 	return rc;
 }
 
-#define TPM2_GET_PCREXTEND_IN_SIZE \
-	(sizeof(struct tpm_input_header) + \
-	 sizeof(struct tpm2_pcr_extend_in))
-
-static const struct tpm_input_header tpm2_pcrextend_header = {
-	.tag = cpu_to_be16(TPM2_ST_SESSIONS),
-	.length = cpu_to_be32(TPM2_GET_PCREXTEND_IN_SIZE),
-	.ordinal = cpu_to_be32(TPM2_CC_PCR_EXTEND)
-};
+struct tpm2_null_auth_area {
+	__be32  handle;
+	__be16  nonce_size;
+	u8  attributes;
+	__be16  auth_size;
+} __packed;
 
 /**
  * tpm2_pcr_extend() - extend a PCR value
  *
  * @chip:	TPM chip to use.
  * @pcr_idx:	index of the PCR.
- * @hash:	hash value to use for the extend operation.
+ * @count:	number of digests passed.
+ * @digests:	list of pcr banks and corresponding digest values to extend.
  *
  * Return: Same as with tpm_transmit_cmd.
  */
-int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash)
+int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
+		    struct tpm2_digest *digests)
 {
-	struct tpm2_cmd cmd;
+	struct tpm_buf buf;
+	struct tpm2_null_auth_area auth_area;
 	int rc;
+	int i;
+	int j;
 
-	cmd.header.in = tpm2_pcrextend_header;
-	cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(pcr_idx);
-	cmd.params.pcrextend_in.auth_area_size =
-		cpu_to_be32(sizeof(struct tpm2_null_auth_area));
-	cmd.params.pcrextend_in.auth_area.handle =
-		cpu_to_be32(TPM2_RS_PW);
-	cmd.params.pcrextend_in.auth_area.nonce_size = 0;
-	cmd.params.pcrextend_in.auth_area.attributes = 0;
-	cmd.params.pcrextend_in.auth_area.auth_size = 0;
-	cmd.params.pcrextend_in.digest_cnt = cpu_to_be32(1);
-	cmd.params.pcrextend_in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
-	memcpy(cmd.params.pcrextend_in.digest, hash, TPM_DIGEST_SIZE);
+	if (count > ARRAY_SIZE(chip->active_banks))
+		return -EINVAL;
 
-	rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), 0,
+	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
+	if (rc)
+		return rc;
+
+	tpm_buf_append_u32(&buf, pcr_idx);
+
+	auth_area.handle = cpu_to_be32(TPM2_RS_PW);
+	auth_area.nonce_size = 0;
+	auth_area.attributes = 0;
+	auth_area.auth_size = 0;
+
+	tpm_buf_append_u32(&buf, sizeof(struct tpm2_null_auth_area));
+	tpm_buf_append(&buf, (const unsigned char *)&auth_area,
+		       sizeof(auth_area));
+	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]);
+		}
+	}
+
+	rc = tpm_transmit_cmd(chip, buf.data, tpm_buf_length(&buf), 0,
 			      "attempting extend a PCR value");
 
+	tpm_buf_destroy(&buf);
+
 	return rc;
 }
 
@@ -993,6 +998,8 @@ int tpm2_auto_startup(struct tpm_chip *chip)
 		}
 	}
 
+	rc = tpm2_get_pcr_allocation(chip);
+
 out:
 	if (rc > 0)
 		rc = -ENODEV;
diff --git a/drivers/char/tpm/tpm_eventlog.h b/drivers/char/tpm/tpm_eventlog.h
index 1660d74..b5ae372 100644
--- a/drivers/char/tpm/tpm_eventlog.h
+++ b/drivers/char/tpm/tpm_eventlog.h
@@ -2,6 +2,8 @@
 #ifndef __TPM_EVENTLOG_H__
 #define __TPM_EVENTLOG_H__
 
+#include <crypto/hash_info.h>
+
 #define TCG_EVENT_NAME_LEN_MAX	255
 #define MAX_TEXT_EVENT		1000	/* Max event string length */
 #define ACPI_TCPA_SIG		"TCPA"	/* 0x41504354 /'TCPA' */
@@ -73,6 +75,11 @@ enum tcpa_pc_event_ids {
 	HOST_TABLE_OF_DEVICES,
 };
 
+struct tpm2_digest {
+	u16 alg_id;
+	u8 digest[SHA512_DIGEST_SIZE];
+} __packed;
+
 #if defined(CONFIG_ACPI)
 int tpm_read_log_acpi(struct tpm_chip *chip);
 #else
-- 
2.5.0


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 1/2] tpm: implement TPM 2.0 capability to get active PCR banks
@ 2017-01-18 13:45     ` Jarkko Sakkinen
  0 siblings, 0 replies; 15+ messages in thread
From: Jarkko Sakkinen @ 2017-01-18 13:45 UTC (permalink / raw)
  To: Nayna Jain
  Cc: tpmdd-devel, peterhuewe, tpmdd, jgunthorpe,
	linux-security-module, linux-kernel

On Wed, Jan 18, 2017 at 03:44:49AM -0500, Nayna Jain wrote:
> This patch implements the TPM 2.0 capability TPM_CAP_PCRS to
> retrieve the active PCR banks from the TPM. This is needed
> to enable extending all active banks as recommended by TPM 2.0
> TCG Specification.
> 
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> ---
>  drivers/char/tpm/tpm.h      |  4 ++++
>  drivers/char/tpm/tpm2-cmd.c | 57 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 61 insertions(+)
> 
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 1ae9768..dddd573 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -127,6 +127,7 @@ enum tpm2_permanent_handles {
>  };
>  
>  enum tpm2_capabilities {
> +	TPM2_CAP_PCRS		= 5,
>  	TPM2_CAP_TPM_PROPERTIES = 6,
>  };
>  
> @@ -187,6 +188,8 @@ struct tpm_chip {
>  
>  	const struct attribute_group *groups[3];
>  	unsigned int groups_cnt;
> +
> +	u16 active_banks[7];
>  #ifdef CONFIG_ACPI
>  	acpi_handle acpi_dev_handle;
>  	char ppi_version[TPM_PPI_VERSION_LEN + 1];
> @@ -545,4 +548,5 @@ int tpm2_auto_startup(struct tpm_chip *chip);
>  void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type);
>  unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
>  int tpm2_probe(struct tpm_chip *chip);
> +ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip);
>  #endif
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 6eda239..75a7546 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -998,3 +998,60 @@ int tpm2_auto_startup(struct tpm_chip *chip)
>  		rc = -ENODEV;
>  	return rc;
>  }
> +
> +struct tpm2_pcr_selection {
> +	__be16  hash_alg;
> +	u8  size_of_select;
> +	u8  pcr_select[3];
> +} __packed;
> +
> +/**
> + * tpm2_get_pcr_allocation() - get TPM active PCR banks.
> + *
> + * @chip: TPM chip to use.
> + *
> + * Return: Same as with tpm_transmit_cmd.
> + */
> +ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
> +{
> +	struct tpm2_pcr_selection pcr_selection;
> +	struct tpm_buf buf;
> +	void *marker;
> +	unsigned int count = 0;

You shouldn't initialize 'count'.

> +	int rc;
> +	int i;
> +
> +	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
> +	if (rc)
> +		return rc;
> +
> +	tpm_buf_append_u32(&buf, TPM2_CAP_PCRS);
> +	tpm_buf_append_u32(&buf, 0);
> +	tpm_buf_append_u32(&buf, 1);
> +
> +	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 0,
> +			      "get tpm pcr allocation");
> +	if (rc < 0)
> +		goto out;
> +
> +	count = be32_to_cpup(
> +		(__be32 *)&buf.data[TPM_HEADER_SIZE + 5]);
> +
> +	if (count > ARRAY_SIZE(chip->active_banks))
> +		return -ENODEV;
> +
> +	marker = &buf.data[TPM_HEADER_SIZE + 9];
> +	for (i = 0; i < count; i++) {
> +		memcpy(&pcr_selection, marker, sizeof(pcr_selection));
> +		chip->active_banks[i] = be16_to_cpu(pcr_selection.hash_alg);
> +		marker = marker + sizeof(struct tpm2_pcr_selection);
> +	}
> +
> +out:
> +	if (count < ARRAY_SIZE(chip->active_banks))
> +		chip->active_banks[count] = 0;
> +
> +	tpm_buf_destroy(&buf);
> +
> +	return rc;
> +}
> -- 
> 2.5.0

Otherwise,

Reviewed-by: Jarkko Sakkinen <jarkko.sakkien@linux.intel.com>

/Jarkko

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 1/2] tpm: implement TPM 2.0 capability to get active PCR banks
@ 2017-01-18 13:45     ` Jarkko Sakkinen
  0 siblings, 0 replies; 15+ messages in thread
From: Jarkko Sakkinen @ 2017-01-18 13:45 UTC (permalink / raw)
  To: Nayna Jain
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

On Wed, Jan 18, 2017 at 03:44:49AM -0500, Nayna Jain wrote:
> This patch implements the TPM 2.0 capability TPM_CAP_PCRS to
> retrieve the active PCR banks from the TPM. This is needed
> to enable extending all active banks as recommended by TPM 2.0
> TCG Specification.
> 
> Signed-off-by: Nayna Jain <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> ---
>  drivers/char/tpm/tpm.h      |  4 ++++
>  drivers/char/tpm/tpm2-cmd.c | 57 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 61 insertions(+)
> 
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 1ae9768..dddd573 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -127,6 +127,7 @@ enum tpm2_permanent_handles {
>  };
>  
>  enum tpm2_capabilities {
> +	TPM2_CAP_PCRS		= 5,
>  	TPM2_CAP_TPM_PROPERTIES = 6,
>  };
>  
> @@ -187,6 +188,8 @@ struct tpm_chip {
>  
>  	const struct attribute_group *groups[3];
>  	unsigned int groups_cnt;
> +
> +	u16 active_banks[7];
>  #ifdef CONFIG_ACPI
>  	acpi_handle acpi_dev_handle;
>  	char ppi_version[TPM_PPI_VERSION_LEN + 1];
> @@ -545,4 +548,5 @@ int tpm2_auto_startup(struct tpm_chip *chip);
>  void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type);
>  unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
>  int tpm2_probe(struct tpm_chip *chip);
> +ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip);
>  #endif
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 6eda239..75a7546 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -998,3 +998,60 @@ int tpm2_auto_startup(struct tpm_chip *chip)
>  		rc = -ENODEV;
>  	return rc;
>  }
> +
> +struct tpm2_pcr_selection {
> +	__be16  hash_alg;
> +	u8  size_of_select;
> +	u8  pcr_select[3];
> +} __packed;
> +
> +/**
> + * tpm2_get_pcr_allocation() - get TPM active PCR banks.
> + *
> + * @chip: TPM chip to use.
> + *
> + * Return: Same as with tpm_transmit_cmd.
> + */
> +ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
> +{
> +	struct tpm2_pcr_selection pcr_selection;
> +	struct tpm_buf buf;
> +	void *marker;
> +	unsigned int count = 0;

You shouldn't initialize 'count'.

> +	int rc;
> +	int i;
> +
> +	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
> +	if (rc)
> +		return rc;
> +
> +	tpm_buf_append_u32(&buf, TPM2_CAP_PCRS);
> +	tpm_buf_append_u32(&buf, 0);
> +	tpm_buf_append_u32(&buf, 1);
> +
> +	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 0,
> +			      "get tpm pcr allocation");
> +	if (rc < 0)
> +		goto out;
> +
> +	count = be32_to_cpup(
> +		(__be32 *)&buf.data[TPM_HEADER_SIZE + 5]);
> +
> +	if (count > ARRAY_SIZE(chip->active_banks))
> +		return -ENODEV;
> +
> +	marker = &buf.data[TPM_HEADER_SIZE + 9];
> +	for (i = 0; i < count; i++) {
> +		memcpy(&pcr_selection, marker, sizeof(pcr_selection));
> +		chip->active_banks[i] = be16_to_cpu(pcr_selection.hash_alg);
> +		marker = marker + sizeof(struct tpm2_pcr_selection);
> +	}
> +
> +out:
> +	if (count < ARRAY_SIZE(chip->active_banks))
> +		chip->active_banks[count] = 0;
> +
> +	tpm_buf_destroy(&buf);
> +
> +	return rc;
> +}
> -- 
> 2.5.0

Otherwise,

Reviewed-by: Jarkko Sakkinen <jarkko.sakkien-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

/Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 1/2] tpm: implement TPM 2.0 capability to get active PCR banks
@ 2017-01-18 13:49       ` Nayna
  0 siblings, 0 replies; 15+ messages in thread
From: Nayna @ 2017-01-18 13:49 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: tpmdd-devel, peterhuewe, tpmdd, jgunthorpe,
	linux-security-module, linux-kernel



On 01/18/2017 07:15 PM, Jarkko Sakkinen wrote:
> On Wed, Jan 18, 2017 at 03:44:49AM -0500, Nayna Jain wrote:
>> This patch implements the TPM 2.0 capability TPM_CAP_PCRS to
>> retrieve the active PCR banks from the TPM. This is needed
>> to enable extending all active banks as recommended by TPM 2.0
>> TCG Specification.
>>
>> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
>> ---
>>   drivers/char/tpm/tpm.h      |  4 ++++
>>   drivers/char/tpm/tpm2-cmd.c | 57 +++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 61 insertions(+)
>>
>> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
>> index 1ae9768..dddd573 100644
>> --- a/drivers/char/tpm/tpm.h
>> +++ b/drivers/char/tpm/tpm.h
>> @@ -127,6 +127,7 @@ enum tpm2_permanent_handles {
>>   };
>>
>>   enum tpm2_capabilities {
>> +	TPM2_CAP_PCRS		= 5,
>>   	TPM2_CAP_TPM_PROPERTIES = 6,
>>   };
>>
>> @@ -187,6 +188,8 @@ struct tpm_chip {
>>
>>   	const struct attribute_group *groups[3];
>>   	unsigned int groups_cnt;
>> +
>> +	u16 active_banks[7];
>>   #ifdef CONFIG_ACPI
>>   	acpi_handle acpi_dev_handle;
>>   	char ppi_version[TPM_PPI_VERSION_LEN + 1];
>> @@ -545,4 +548,5 @@ int tpm2_auto_startup(struct tpm_chip *chip);
>>   void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type);
>>   unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
>>   int tpm2_probe(struct tpm_chip *chip);
>> +ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip);
>>   #endif
>> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
>> index 6eda239..75a7546 100644
>> --- a/drivers/char/tpm/tpm2-cmd.c
>> +++ b/drivers/char/tpm/tpm2-cmd.c
>> @@ -998,3 +998,60 @@ int tpm2_auto_startup(struct tpm_chip *chip)
>>   		rc = -ENODEV;
>>   	return rc;
>>   }
>> +
>> +struct tpm2_pcr_selection {
>> +	__be16  hash_alg;
>> +	u8  size_of_select;
>> +	u8  pcr_select[3];
>> +} __packed;
>> +
>> +/**
>> + * tpm2_get_pcr_allocation() - get TPM active PCR banks.
>> + *
>> + * @chip: TPM chip to use.
>> + *
>> + * Return: Same as with tpm_transmit_cmd.
>> + */
>> +ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
>> +{
>> +	struct tpm2_pcr_selection pcr_selection;
>> +	struct tpm_buf buf;
>> +	void *marker;
>> +	unsigned int count = 0;
>
> You shouldn't initialize 'count'.

It is initialized for reason below:

>
>> +	int rc;
>> +	int i;
>> +
>> +	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
>> +	if (rc)
>> +		return rc;
>> +
>> +	tpm_buf_append_u32(&buf, TPM2_CAP_PCRS);
>> +	tpm_buf_append_u32(&buf, 0);
>> +	tpm_buf_append_u32(&buf, 1);
>> +
>> +	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 0,
>> +			      "get tpm pcr allocation");
>> +	if (rc < 0)
>> +		goto out;

if  tpm_transmit_cmd()fails, it jumps to out:
where count is used in if(..).

Thanks & Regards,
    - Nayna

>> +
>> +	count = be32_to_cpup(
>> +		(__be32 *)&buf.data[TPM_HEADER_SIZE + 5]);
>> +
>> +	if (count > ARRAY_SIZE(chip->active_banks))
>> +		return -ENODEV;
>> +
>> +	marker = &buf.data[TPM_HEADER_SIZE + 9];
>> +	for (i = 0; i < count; i++) {
>> +		memcpy(&pcr_selection, marker, sizeof(pcr_selection));
>> +		chip->active_banks[i] = be16_to_cpu(pcr_selection.hash_alg);
>> +		marker = marker + sizeof(struct tpm2_pcr_selection);
>> +	}
>> +
>> +out:
>> +	if (count < ARRAY_SIZE(chip->active_banks))
>> +		chip->active_banks[count] = 0;
>> +
>> +	tpm_buf_destroy(&buf);
>> +
>> +	return rc;
>> +}
>> --
>> 2.5.0
>
> Otherwise,
>
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkien@linux.intel.com>
>
> /Jarkko
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 1/2] tpm: implement TPM 2.0 capability to get active PCR banks
@ 2017-01-18 13:49       ` Nayna
  0 siblings, 0 replies; 15+ messages in thread
From: Nayna @ 2017-01-18 13:49 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f



On 01/18/2017 07:15 PM, Jarkko Sakkinen wrote:
> On Wed, Jan 18, 2017 at 03:44:49AM -0500, Nayna Jain wrote:
>> This patch implements the TPM 2.0 capability TPM_CAP_PCRS to
>> retrieve the active PCR banks from the TPM. This is needed
>> to enable extending all active banks as recommended by TPM 2.0
>> TCG Specification.
>>
>> Signed-off-by: Nayna Jain <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>> ---
>>   drivers/char/tpm/tpm.h      |  4 ++++
>>   drivers/char/tpm/tpm2-cmd.c | 57 +++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 61 insertions(+)
>>
>> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
>> index 1ae9768..dddd573 100644
>> --- a/drivers/char/tpm/tpm.h
>> +++ b/drivers/char/tpm/tpm.h
>> @@ -127,6 +127,7 @@ enum tpm2_permanent_handles {
>>   };
>>
>>   enum tpm2_capabilities {
>> +	TPM2_CAP_PCRS		= 5,
>>   	TPM2_CAP_TPM_PROPERTIES = 6,
>>   };
>>
>> @@ -187,6 +188,8 @@ struct tpm_chip {
>>
>>   	const struct attribute_group *groups[3];
>>   	unsigned int groups_cnt;
>> +
>> +	u16 active_banks[7];
>>   #ifdef CONFIG_ACPI
>>   	acpi_handle acpi_dev_handle;
>>   	char ppi_version[TPM_PPI_VERSION_LEN + 1];
>> @@ -545,4 +548,5 @@ int tpm2_auto_startup(struct tpm_chip *chip);
>>   void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type);
>>   unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
>>   int tpm2_probe(struct tpm_chip *chip);
>> +ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip);
>>   #endif
>> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
>> index 6eda239..75a7546 100644
>> --- a/drivers/char/tpm/tpm2-cmd.c
>> +++ b/drivers/char/tpm/tpm2-cmd.c
>> @@ -998,3 +998,60 @@ int tpm2_auto_startup(struct tpm_chip *chip)
>>   		rc = -ENODEV;
>>   	return rc;
>>   }
>> +
>> +struct tpm2_pcr_selection {
>> +	__be16  hash_alg;
>> +	u8  size_of_select;
>> +	u8  pcr_select[3];
>> +} __packed;
>> +
>> +/**
>> + * tpm2_get_pcr_allocation() - get TPM active PCR banks.
>> + *
>> + * @chip: TPM chip to use.
>> + *
>> + * Return: Same as with tpm_transmit_cmd.
>> + */
>> +ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
>> +{
>> +	struct tpm2_pcr_selection pcr_selection;
>> +	struct tpm_buf buf;
>> +	void *marker;
>> +	unsigned int count = 0;
>
> You shouldn't initialize 'count'.

It is initialized for reason below:

>
>> +	int rc;
>> +	int i;
>> +
>> +	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
>> +	if (rc)
>> +		return rc;
>> +
>> +	tpm_buf_append_u32(&buf, TPM2_CAP_PCRS);
>> +	tpm_buf_append_u32(&buf, 0);
>> +	tpm_buf_append_u32(&buf, 1);
>> +
>> +	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 0,
>> +			      "get tpm pcr allocation");
>> +	if (rc < 0)
>> +		goto out;

if  tpm_transmit_cmd()fails, it jumps to out:
where count is used in if(..).

Thanks & Regards,
    - Nayna

>> +
>> +	count = be32_to_cpup(
>> +		(__be32 *)&buf.data[TPM_HEADER_SIZE + 5]);
>> +
>> +	if (count > ARRAY_SIZE(chip->active_banks))
>> +		return -ENODEV;
>> +
>> +	marker = &buf.data[TPM_HEADER_SIZE + 9];
>> +	for (i = 0; i < count; i++) {
>> +		memcpy(&pcr_selection, marker, sizeof(pcr_selection));
>> +		chip->active_banks[i] = be16_to_cpu(pcr_selection.hash_alg);
>> +		marker = marker + sizeof(struct tpm2_pcr_selection);
>> +	}
>> +
>> +out:
>> +	if (count < ARRAY_SIZE(chip->active_banks))
>> +		chip->active_banks[count] = 0;
>> +
>> +	tpm_buf_destroy(&buf);
>> +
>> +	return rc;
>> +}
>> --
>> 2.5.0
>
> Otherwise,
>
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkien-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>
> /Jarkko
>


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 2/2] tpm: enhance TPM 2.0 PCR extend to support multiple banks
  2017-01-18  8:44   ` Nayna Jain
  (?)
@ 2017-01-18 13:58   ` Jarkko Sakkinen
  2017-01-19  9:18     ` Nayna
  -1 siblings, 1 reply; 15+ messages in thread
From: Jarkko Sakkinen @ 2017-01-18 13:58 UTC (permalink / raw)
  To: Nayna Jain
  Cc: tpmdd-devel, peterhuewe, tpmdd, jgunthorpe,
	linux-security-module, linux-kernel

On Wed, Jan 18, 2017 at 03:44:50AM -0500, Nayna Jain wrote:
> The current TPM 2.0 device driver extends only the SHA1 PCR bank
> but the TCG Specification[1] recommends extending all active PCR
> banks, to prevent malicious users from setting unused PCR banks with
> fake measurements and quoting them.
> 
> The existing in-kernel interface(tpm_pcr_extend()) expects only a
> SHA1 digest.  To extend all active PCR banks with differing
> digest sizes, the SHA1 digest is padded with trailing 0's as needed.
> 
> This patch reuses the defined digest sizes from the crypto subsystem,
> adding a dependency on CRYPTO_HASH_INFO module.
> 
> [1] TPM 2.0 Specification referred here is "TCG PC Client Specific
> Platform Firmware Profile for TPM 2.0"
> 
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> ---
>  drivers/char/tpm/Kconfig         |  1 +
>  drivers/char/tpm/tpm-interface.c | 15 ++++++-
>  drivers/char/tpm/tpm.h           |  3 +-
>  drivers/char/tpm/tpm2-cmd.c      | 91 +++++++++++++++++++++-------------------
>  drivers/char/tpm/tpm_eventlog.h  |  7 ++++
>  5 files changed, 73 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
> index 277186d..af985cc 100644
> --- a/drivers/char/tpm/Kconfig
> +++ b/drivers/char/tpm/Kconfig
> @@ -6,6 +6,7 @@ menuconfig TCG_TPM
>  	tristate "TPM Hardware Support"
>  	depends on HAS_IOMEM
>  	select SECURITYFS
> +	select CRYPTO_HASH_INFO
>  	---help---
>  	  If you have a TPM security chip in your system, which
>  	  implements the Trusted Computing Group's specification,
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index a3461cb..00612dd 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -770,6 +770,7 @@ static const struct tpm_input_header pcrextend_header = {
>  int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
>  {
>  	struct tpm_cmd_t cmd;
> +	int i;
>  	int rc;
>  	struct tpm_chip *chip;
>  
> @@ -778,7 +779,19 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
>  		return -ENODEV;
>  
>  	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> -		rc = tpm2_pcr_extend(chip, pcr_idx, hash);
> +		struct tpm2_digest digest_list[ARRAY_SIZE(chip->active_banks)];
> +		u32 count = 0;

Declare variables in the beginning.

> +
> +		memset(digest_list, 0, sizeof(digest_list));
> +
> +		for (i = 0; (chip->active_banks[i] != 0) &&
> +		     (i < ARRAY_SIZE(chip->active_banks)); i++) {
> +			digest_list[i].alg_id = chip->active_banks[i];
> +			memcpy(digest_list[i].digest, hash, TPM_DIGEST_SIZE);
> +			count++;
> +		}
> +
> +		rc = tpm2_pcr_extend(chip, pcr_idx, count, digest_list);
>  		tpm_put_ops(chip);
>  		return rc;
>  	}
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index dddd573..f578822 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -533,7 +533,8 @@ static inline void tpm_add_ppi(struct tpm_chip *chip)
>  #endif
>  
>  int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
> -int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash);
> +int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
> +		    struct tpm2_digest *digests);
>  int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max);
>  int tpm2_seal_trusted(struct tpm_chip *chip,
>  		      struct trusted_key_payload *payload,
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 75a7546..7ceebbd 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -53,22 +53,6 @@ struct tpm2_pcr_read_out {
>  	u8	digest[TPM_DIGEST_SIZE];
>  } __packed;
>  
> -struct tpm2_null_auth_area {
> -	__be32			handle;
> -	__be16			nonce_size;
> -	u8			attributes;
> -	__be16			auth_size;
> -} __packed;
> -
> -struct tpm2_pcr_extend_in {
> -	__be32				pcr_idx;
> -	__be32				auth_area_size;
> -	struct tpm2_null_auth_area	auth_area;
> -	__be32				digest_cnt;
> -	__be16				hash_alg;
> -	u8				digest[TPM_DIGEST_SIZE];
> -} __packed;
> -
>  struct tpm2_get_tpm_pt_in {
>  	__be32	cap_id;
>  	__be32	property_id;
> @@ -97,7 +81,6 @@ union tpm2_cmd_params {
>  	struct	tpm2_self_test_in	selftest_in;
>  	struct	tpm2_pcr_read_in	pcrread_in;
>  	struct	tpm2_pcr_read_out	pcrread_out;
> -	struct	tpm2_pcr_extend_in	pcrextend_in;
>  	struct	tpm2_get_tpm_pt_in	get_tpm_pt_in;
>  	struct	tpm2_get_tpm_pt_out	get_tpm_pt_out;
>  	struct	tpm2_get_random_in	getrandom_in;
> @@ -290,46 +273,68 @@ int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
>  	return rc;
>  }
>  
> -#define TPM2_GET_PCREXTEND_IN_SIZE \
> -	(sizeof(struct tpm_input_header) + \
> -	 sizeof(struct tpm2_pcr_extend_in))
> -
> -static const struct tpm_input_header tpm2_pcrextend_header = {
> -	.tag = cpu_to_be16(TPM2_ST_SESSIONS),
> -	.length = cpu_to_be32(TPM2_GET_PCREXTEND_IN_SIZE),
> -	.ordinal = cpu_to_be32(TPM2_CC_PCR_EXTEND)
> -};
> +struct tpm2_null_auth_area {
> +	__be32  handle;
> +	__be16  nonce_size;
> +	u8  attributes;
> +	__be16  auth_size;
> +} __packed;
>  
>  /**
>   * tpm2_pcr_extend() - extend a PCR value
>   *
>   * @chip:	TPM chip to use.
>   * @pcr_idx:	index of the PCR.
> - * @hash:	hash value to use for the extend operation.
> + * @count:	number of digests passed.
> + * @digests:	list of pcr banks and corresponding digest values to extend.
>   *
>   * Return: Same as with tpm_transmit_cmd.
>   *
> -int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash)
> +int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
> +		    struct tpm2_digest *digests)

Yes, this much much better. Now you can understand what is happening
even if you never have read the TPM 2.0 spec.

>  {
> -	struct tpm2_cmd cmd;
> +	struct tpm_buf buf;
> +	struct tpm2_null_auth_area auth_area;
>  	int rc;
> +	int i;
> +	int j;
>  
> -	cmd.header.in = tpm2_pcrextend_header;
> -	cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(pcr_idx);
> -	cmd.params.pcrextend_in.auth_area_size =
> -		cpu_to_be32(sizeof(struct tpm2_null_auth_area));
> -	cmd.params.pcrextend_in.auth_area.handle =
> -		cpu_to_be32(TPM2_RS_PW);
> -	cmd.params.pcrextend_in.auth_area.nonce_size = 0;
> -	cmd.params.pcrextend_in.auth_area.attributes = 0;
> -	cmd.params.pcrextend_in.auth_area.auth_size = 0;
> -	cmd.params.pcrextend_in.digest_cnt = cpu_to_be32(1);
> -	cmd.params.pcrextend_in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
> -	memcpy(cmd.params.pcrextend_in.digest, hash, TPM_DIGEST_SIZE);
> +	if (count > ARRAY_SIZE(chip->active_banks))
> +		return -EINVAL;
>  
> -	rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), 0,
> +	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
> +	if (rc)
> +		return rc;
> +
> +	tpm_buf_append_u32(&buf, pcr_idx);
> +
> +	auth_area.handle = cpu_to_be32(TPM2_RS_PW);
> +	auth_area.nonce_size = 0;
> +	auth_area.attributes = 0;
> +	auth_area.auth_size = 0;
> +
> +	tpm_buf_append_u32(&buf, sizeof(struct tpm2_null_auth_area));
> +	tpm_buf_append(&buf, (const unsigned char *)&auth_area,
> +		       sizeof(auth_area));
> +	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]);
> +		}
> +	}
> +
> +	rc = tpm_transmit_cmd(chip, buf.data, tpm_buf_length(&buf), 0,
>  			      "attempting extend a PCR value");
>  
> +	tpm_buf_destroy(&buf);
> +
>  	return rc;
>  }
>  
> @@ -993,6 +998,8 @@ int tpm2_auto_startup(struct tpm_chip *chip)
>  		}
>  	}
>  
> +	rc = tpm2_get_pcr_allocation(chip);
> +
>  out:
>  	if (rc > 0)
>  		rc = -ENODEV;
> diff --git a/drivers/char/tpm/tpm_eventlog.h b/drivers/char/tpm/tpm_eventlog.h
> index 1660d74..b5ae372 100644
> --- a/drivers/char/tpm/tpm_eventlog.h
> +++ b/drivers/char/tpm/tpm_eventlog.h
> @@ -2,6 +2,8 @@
>  #ifndef __TPM_EVENTLOG_H__
>  #define __TPM_EVENTLOG_H__
>  
> +#include <crypto/hash_info.h>
> +
>  #define TCG_EVENT_NAME_LEN_MAX	255
>  #define MAX_TEXT_EVENT		1000	/* Max event string length */
>  #define ACPI_TCPA_SIG		"TCPA"	/* 0x41504354 /'TCPA' */
> @@ -73,6 +75,11 @@ enum tcpa_pc_event_ids {
>  	HOST_TABLE_OF_DEVICES,
>  };
>  
> +struct tpm2_digest {
> +	u16 alg_id;
> +	u8 digest[SHA512_DIGEST_SIZE];
> +} __packed;
> +
>  #if defined(CONFIG_ACPI)
>  int tpm_read_log_acpi(struct tpm_chip *chip);
>  #else
> -- 
> 2.5.0

Otherwise,

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

/Jarkko

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 2/2] tpm: enhance TPM 2.0 PCR extend to support multiple banks
  2017-01-18 13:58   ` Jarkko Sakkinen
@ 2017-01-19  9:18     ` Nayna
  0 siblings, 0 replies; 15+ messages in thread
From: Nayna @ 2017-01-19  9:18 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: tpmdd-devel, peterhuewe, tpmdd, jgunthorpe,
	linux-security-module, linux-kernel



On 01/18/2017 07:28 PM, Jarkko Sakkinen wrote:
> On Wed, Jan 18, 2017 at 03:44:50AM -0500, Nayna Jain wrote:
>> The current TPM 2.0 device driver extends only the SHA1 PCR bank
>> but the TCG Specification[1] recommends extending all active PCR
>> banks, to prevent malicious users from setting unused PCR banks with
>> fake measurements and quoting them.
>>
>> The existing in-kernel interface(tpm_pcr_extend()) expects only a
>> SHA1 digest.  To extend all active PCR banks with differing
>> digest sizes, the SHA1 digest is padded with trailing 0's as needed.
>>
>> This patch reuses the defined digest sizes from the crypto subsystem,
>> adding a dependency on CRYPTO_HASH_INFO module.
>>
>> [1] TPM 2.0 Specification referred here is "TCG PC Client Specific
>> Platform Firmware Profile for TPM 2.0"
>>
>> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
>> ---
>>   drivers/char/tpm/Kconfig         |  1 +
>>   drivers/char/tpm/tpm-interface.c | 15 ++++++-
>>   drivers/char/tpm/tpm.h           |  3 +-
>>   drivers/char/tpm/tpm2-cmd.c      | 91 +++++++++++++++++++++-------------------
>>   drivers/char/tpm/tpm_eventlog.h  |  7 ++++
>>   5 files changed, 73 insertions(+), 44 deletions(-)
>>
>> diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
>> index 277186d..af985cc 100644
>> --- a/drivers/char/tpm/Kconfig
>> +++ b/drivers/char/tpm/Kconfig
>> @@ -6,6 +6,7 @@ menuconfig TCG_TPM
>>   	tristate "TPM Hardware Support"
>>   	depends on HAS_IOMEM
>>   	select SECURITYFS
>> +	select CRYPTO_HASH_INFO
>>   	---help---
>>   	  If you have a TPM security chip in your system, which
>>   	  implements the Trusted Computing Group's specification,
>> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
>> index a3461cb..00612dd 100644
>> --- a/drivers/char/tpm/tpm-interface.c
>> +++ b/drivers/char/tpm/tpm-interface.c
>> @@ -770,6 +770,7 @@ static const struct tpm_input_header pcrextend_header = {
>>   int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
>>   {
>>   	struct tpm_cmd_t cmd;
>> +	int i;
>>   	int rc;
>>   	struct tpm_chip *chip;
>>
>> @@ -778,7 +779,19 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
>>   		return -ENODEV;
>>
>>   	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
>> -		rc = tpm2_pcr_extend(chip, pcr_idx, hash);
>> +		struct tpm2_digest digest_list[ARRAY_SIZE(chip->active_banks)];
>> +		u32 count = 0;
>
> Declare variables in the beginning.

Done, posted new version with this change.

>
>> +
>> +		memset(digest_list, 0, sizeof(digest_list));
>> +
>> +		for (i = 0; (chip->active_banks[i] != 0) &&
>> +		     (i < ARRAY_SIZE(chip->active_banks)); i++) {
>> +			digest_list[i].alg_id = chip->active_banks[i];
>> +			memcpy(digest_list[i].digest, hash, TPM_DIGEST_SIZE);
>> +			count++;
>> +		}
>> +
>> +		rc = tpm2_pcr_extend(chip, pcr_idx, count, digest_list);
>>   		tpm_put_ops(chip);
>>   		return rc;
>>   	}
>> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
>> index dddd573..f578822 100644
>> --- a/drivers/char/tpm/tpm.h
>> +++ b/drivers/char/tpm/tpm.h
>> @@ -533,7 +533,8 @@ static inline void tpm_add_ppi(struct tpm_chip *chip)
>>   #endif
>>
>>   int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
>> -int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash);
>> +int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
>> +		    struct tpm2_digest *digests);
>>   int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max);
>>   int tpm2_seal_trusted(struct tpm_chip *chip,
>>   		      struct trusted_key_payload *payload,
>> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
>> index 75a7546..7ceebbd 100644
>> --- a/drivers/char/tpm/tpm2-cmd.c
>> +++ b/drivers/char/tpm/tpm2-cmd.c
>> @@ -53,22 +53,6 @@ struct tpm2_pcr_read_out {
>>   	u8	digest[TPM_DIGEST_SIZE];
>>   } __packed;
>>
>> -struct tpm2_null_auth_area {
>> -	__be32			handle;
>> -	__be16			nonce_size;
>> -	u8			attributes;
>> -	__be16			auth_size;
>> -} __packed;
>> -
>> -struct tpm2_pcr_extend_in {
>> -	__be32				pcr_idx;
>> -	__be32				auth_area_size;
>> -	struct tpm2_null_auth_area	auth_area;
>> -	__be32				digest_cnt;
>> -	__be16				hash_alg;
>> -	u8				digest[TPM_DIGEST_SIZE];
>> -} __packed;
>> -
>>   struct tpm2_get_tpm_pt_in {
>>   	__be32	cap_id;
>>   	__be32	property_id;
>> @@ -97,7 +81,6 @@ union tpm2_cmd_params {
>>   	struct	tpm2_self_test_in	selftest_in;
>>   	struct	tpm2_pcr_read_in	pcrread_in;
>>   	struct	tpm2_pcr_read_out	pcrread_out;
>> -	struct	tpm2_pcr_extend_in	pcrextend_in;
>>   	struct	tpm2_get_tpm_pt_in	get_tpm_pt_in;
>>   	struct	tpm2_get_tpm_pt_out	get_tpm_pt_out;
>>   	struct	tpm2_get_random_in	getrandom_in;
>> @@ -290,46 +273,68 @@ int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
>>   	return rc;
>>   }
>>
>> -#define TPM2_GET_PCREXTEND_IN_SIZE \
>> -	(sizeof(struct tpm_input_header) + \
>> -	 sizeof(struct tpm2_pcr_extend_in))
>> -
>> -static const struct tpm_input_header tpm2_pcrextend_header = {
>> -	.tag = cpu_to_be16(TPM2_ST_SESSIONS),
>> -	.length = cpu_to_be32(TPM2_GET_PCREXTEND_IN_SIZE),
>> -	.ordinal = cpu_to_be32(TPM2_CC_PCR_EXTEND)
>> -};
>> +struct tpm2_null_auth_area {
>> +	__be32  handle;
>> +	__be16  nonce_size;
>> +	u8  attributes;
>> +	__be16  auth_size;
>> +} __packed;
>>
>>   /**
>>    * tpm2_pcr_extend() - extend a PCR value
>>    *
>>    * @chip:	TPM chip to use.
>>    * @pcr_idx:	index of the PCR.
>> - * @hash:	hash value to use for the extend operation.
>> + * @count:	number of digests passed.
>> + * @digests:	list of pcr banks and corresponding digest values to extend.
>>    *
>>    * Return: Same as with tpm_transmit_cmd.
>>    *
>> -int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash)
>> +int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
>> +		    struct tpm2_digest *digests)
>
> Yes, this much much better. Now you can understand what is happening
> even if you never have read the TPM 2.0 spec.

Yeah.. I agree..

Thanks & Regards,
    - Nayna

>
>>   {
>> -	struct tpm2_cmd cmd;
>> +	struct tpm_buf buf;
>> +	struct tpm2_null_auth_area auth_area;
>>   	int rc;
>> +	int i;
>> +	int j;
>>
>> -	cmd.header.in = tpm2_pcrextend_header;
>> -	cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(pcr_idx);
>> -	cmd.params.pcrextend_in.auth_area_size =
>> -		cpu_to_be32(sizeof(struct tpm2_null_auth_area));
>> -	cmd.params.pcrextend_in.auth_area.handle =
>> -		cpu_to_be32(TPM2_RS_PW);
>> -	cmd.params.pcrextend_in.auth_area.nonce_size = 0;
>> -	cmd.params.pcrextend_in.auth_area.attributes = 0;
>> -	cmd.params.pcrextend_in.auth_area.auth_size = 0;
>> -	cmd.params.pcrextend_in.digest_cnt = cpu_to_be32(1);
>> -	cmd.params.pcrextend_in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
>> -	memcpy(cmd.params.pcrextend_in.digest, hash, TPM_DIGEST_SIZE);
>> +	if (count > ARRAY_SIZE(chip->active_banks))
>> +		return -EINVAL;
>>
>> -	rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), 0,
>> +	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
>> +	if (rc)
>> +		return rc;
>> +
>> +	tpm_buf_append_u32(&buf, pcr_idx);
>> +
>> +	auth_area.handle = cpu_to_be32(TPM2_RS_PW);
>> +	auth_area.nonce_size = 0;
>> +	auth_area.attributes = 0;
>> +	auth_area.auth_size = 0;
>> +
>> +	tpm_buf_append_u32(&buf, sizeof(struct tpm2_null_auth_area));
>> +	tpm_buf_append(&buf, (const unsigned char *)&auth_area,
>> +		       sizeof(auth_area));
>> +	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]);
>> +		}
>> +	}
>> +
>> +	rc = tpm_transmit_cmd(chip, buf.data, tpm_buf_length(&buf), 0,
>>   			      "attempting extend a PCR value");
>>
>> +	tpm_buf_destroy(&buf);
>> +
>>   	return rc;
>>   }
>>
>> @@ -993,6 +998,8 @@ int tpm2_auto_startup(struct tpm_chip *chip)
>>   		}
>>   	}
>>
>> +	rc = tpm2_get_pcr_allocation(chip);
>> +
>>   out:
>>   	if (rc > 0)
>>   		rc = -ENODEV;
>> diff --git a/drivers/char/tpm/tpm_eventlog.h b/drivers/char/tpm/tpm_eventlog.h
>> index 1660d74..b5ae372 100644
>> --- a/drivers/char/tpm/tpm_eventlog.h
>> +++ b/drivers/char/tpm/tpm_eventlog.h
>> @@ -2,6 +2,8 @@
>>   #ifndef __TPM_EVENTLOG_H__
>>   #define __TPM_EVENTLOG_H__
>>
>> +#include <crypto/hash_info.h>
>> +
>>   #define TCG_EVENT_NAME_LEN_MAX	255
>>   #define MAX_TEXT_EVENT		1000	/* Max event string length */
>>   #define ACPI_TCPA_SIG		"TCPA"	/* 0x41504354 /'TCPA' */
>> @@ -73,6 +75,11 @@ enum tcpa_pc_event_ids {
>>   	HOST_TABLE_OF_DEVICES,
>>   };
>>
>> +struct tpm2_digest {
>> +	u16 alg_id;
>> +	u8 digest[SHA512_DIGEST_SIZE];
>> +} __packed;
>> +
>>   #if defined(CONFIG_ACPI)
>>   int tpm_read_log_acpi(struct tpm_chip *chip);
>>   #else
>> --
>> 2.5.0
>
> Otherwise,
>
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>
> /Jarkko
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 1/2] tpm: implement TPM 2.0 capability to get active PCR banks
  2017-01-18 13:49       ` Nayna
  (?)
@ 2017-01-19 10:47       ` Jarkko Sakkinen
  2017-01-19 13:34           ` Nayna
  -1 siblings, 1 reply; 15+ messages in thread
From: Jarkko Sakkinen @ 2017-01-19 10:47 UTC (permalink / raw)
  To: Nayna
  Cc: tpmdd-devel, peterhuewe, tpmdd, jgunthorpe,
	linux-security-module, linux-kernel

On Wed, Jan 18, 2017 at 07:19:06PM +0530, Nayna wrote:
> 
> 
> On 01/18/2017 07:15 PM, Jarkko Sakkinen wrote:
> > On Wed, Jan 18, 2017 at 03:44:49AM -0500, Nayna Jain wrote:
> > > This patch implements the TPM 2.0 capability TPM_CAP_PCRS to
> > > retrieve the active PCR banks from the TPM. This is needed
> > > to enable extending all active banks as recommended by TPM 2.0
> > > TCG Specification.
> > > 
> > > Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> > > ---
> > >   drivers/char/tpm/tpm.h      |  4 ++++
> > >   drivers/char/tpm/tpm2-cmd.c | 57 +++++++++++++++++++++++++++++++++++++++++++++
> > >   2 files changed, 61 insertions(+)
> > > 
> > > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > > index 1ae9768..dddd573 100644
> > > --- a/drivers/char/tpm/tpm.h
> > > +++ b/drivers/char/tpm/tpm.h
> > > @@ -127,6 +127,7 @@ enum tpm2_permanent_handles {
> > >   };
> > > 
> > >   enum tpm2_capabilities {
> > > +	TPM2_CAP_PCRS		= 5,
> > >   	TPM2_CAP_TPM_PROPERTIES = 6,
> > >   };
> > > 
> > > @@ -187,6 +188,8 @@ struct tpm_chip {
> > > 
> > >   	const struct attribute_group *groups[3];
> > >   	unsigned int groups_cnt;
> > > +
> > > +	u16 active_banks[7];
> > >   #ifdef CONFIG_ACPI
> > >   	acpi_handle acpi_dev_handle;
> > >   	char ppi_version[TPM_PPI_VERSION_LEN + 1];
> > > @@ -545,4 +548,5 @@ int tpm2_auto_startup(struct tpm_chip *chip);
> > >   void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type);
> > >   unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
> > >   int tpm2_probe(struct tpm_chip *chip);
> > > +ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip);
> > >   #endif
> > > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> > > index 6eda239..75a7546 100644
> > > --- a/drivers/char/tpm/tpm2-cmd.c
> > > +++ b/drivers/char/tpm/tpm2-cmd.c
> > > @@ -998,3 +998,60 @@ int tpm2_auto_startup(struct tpm_chip *chip)
> > >   		rc = -ENODEV;
> > >   	return rc;
> > >   }
> > > +
> > > +struct tpm2_pcr_selection {
> > > +	__be16  hash_alg;
> > > +	u8  size_of_select;
> > > +	u8  pcr_select[3];
> > > +} __packed;
> > > +
> > > +/**
> > > + * tpm2_get_pcr_allocation() - get TPM active PCR banks.
> > > + *
> > > + * @chip: TPM chip to use.
> > > + *
> > > + * Return: Same as with tpm_transmit_cmd.
> > > + */
> > > +ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
> > > +{
> > > +	struct tpm2_pcr_selection pcr_selection;
> > > +	struct tpm_buf buf;
> > > +	void *marker;
> > > +	unsigned int count = 0;
> > 
> > You shouldn't initialize 'count'.
> 
> It is initialized for reason below:
> 
> > 
> > > +	int rc;
> > > +	int i;
> > > +
> > > +	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
> > > +	if (rc)
> > > +		return rc;
> > > +
> > > +	tpm_buf_append_u32(&buf, TPM2_CAP_PCRS);
> > > +	tpm_buf_append_u32(&buf, 0);
> > > +	tpm_buf_append_u32(&buf, 1);
> > > +
> > > +	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 0,
> > > +			      "get tpm pcr allocation");
> > > +	if (rc < 0)
> > > +		goto out;
> 
> if  tpm_transmit_cmd()fails, it jumps to out:
> where count is used in if(..).

Why the out label is placed there and not after?

> 
> Thanks & Regards,
>    - Nayna
> 
> > > +
> > > +	count = be32_to_cpup(
> > > +		(__be32 *)&buf.data[TPM_HEADER_SIZE + 5]);
> > > +
> > > +	if (count > ARRAY_SIZE(chip->active_banks))
> > > +		return -ENODEV;

Hey, here's a regression (didn't notice it last time). You
nee to call tpm_buf_destroy().


> > > +
> > > +	marker = &buf.data[TPM_HEADER_SIZE + 9];
> > > +	for (i = 0; i < count; i++) {
> > > +		memcpy(&pcr_selection, marker, sizeof(pcr_selection));
> > > +		chip->active_banks[i] = be16_to_cpu(pcr_selection.hash_alg);
> > > +		marker = marker + sizeof(struct tpm2_pcr_selection);
> > > +	}
> > > +
> > > +out:
> > > +	if (count < ARRAY_SIZE(chip->active_banks))
> > > +		chip->active_banks[count] = 0;
> > > +
> > > +	tpm_buf_destroy(&buf);
> > > +
> > > +	return rc;
> > > +}
> > > --
> > > 2.5.0
> > 
> > Otherwise,
> > 
> > Reviewed-by: Jarkko Sakkinen <jarkko.sakkien@linux.intel.com>
> > 
> > /Jarkko
> > 
> 

/Jarkko

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 1/2] tpm: implement TPM 2.0 capability to get active PCR banks
@ 2017-01-19 13:34           ` Nayna
  0 siblings, 0 replies; 15+ messages in thread
From: Nayna @ 2017-01-19 13:34 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: tpmdd-devel, peterhuewe, tpmdd, jgunthorpe,
	linux-security-module, linux-kernel



On 01/19/2017 04:17 PM, Jarkko Sakkinen wrote:
> On Wed, Jan 18, 2017 at 07:19:06PM +0530, Nayna wrote:
>>
>>
>> On 01/18/2017 07:15 PM, Jarkko Sakkinen wrote:
>>> On Wed, Jan 18, 2017 at 03:44:49AM -0500, Nayna Jain wrote:
>>>> This patch implements the TPM 2.0 capability TPM_CAP_PCRS to
>>>> retrieve the active PCR banks from the TPM. This is needed
>>>> to enable extending all active banks as recommended by TPM 2.0
>>>> TCG Specification.
>>>>
>>>> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
>>>> ---
>>>>    drivers/char/tpm/tpm.h      |  4 ++++
>>>>    drivers/char/tpm/tpm2-cmd.c | 57 +++++++++++++++++++++++++++++++++++++++++++++
>>>>    2 files changed, 61 insertions(+)
>>>>
>>>> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
>>>> index 1ae9768..dddd573 100644
>>>> --- a/drivers/char/tpm/tpm.h
>>>> +++ b/drivers/char/tpm/tpm.h
>>>> @@ -127,6 +127,7 @@ enum tpm2_permanent_handles {
>>>>    };
>>>>
>>>>    enum tpm2_capabilities {
>>>> +	TPM2_CAP_PCRS		= 5,
>>>>    	TPM2_CAP_TPM_PROPERTIES = 6,
>>>>    };
>>>>
>>>> @@ -187,6 +188,8 @@ struct tpm_chip {
>>>>
>>>>    	const struct attribute_group *groups[3];
>>>>    	unsigned int groups_cnt;
>>>> +
>>>> +	u16 active_banks[7];
>>>>    #ifdef CONFIG_ACPI
>>>>    	acpi_handle acpi_dev_handle;
>>>>    	char ppi_version[TPM_PPI_VERSION_LEN + 1];
>>>> @@ -545,4 +548,5 @@ int tpm2_auto_startup(struct tpm_chip *chip);
>>>>    void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type);
>>>>    unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
>>>>    int tpm2_probe(struct tpm_chip *chip);
>>>> +ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip);
>>>>    #endif
>>>> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
>>>> index 6eda239..75a7546 100644
>>>> --- a/drivers/char/tpm/tpm2-cmd.c
>>>> +++ b/drivers/char/tpm/tpm2-cmd.c
>>>> @@ -998,3 +998,60 @@ int tpm2_auto_startup(struct tpm_chip *chip)
>>>>    		rc = -ENODEV;
>>>>    	return rc;
>>>>    }
>>>> +
>>>> +struct tpm2_pcr_selection {
>>>> +	__be16  hash_alg;
>>>> +	u8  size_of_select;
>>>> +	u8  pcr_select[3];
>>>> +} __packed;
>>>> +
>>>> +/**
>>>> + * tpm2_get_pcr_allocation() - get TPM active PCR banks.
>>>> + *
>>>> + * @chip: TPM chip to use.
>>>> + *
>>>> + * Return: Same as with tpm_transmit_cmd.
>>>> + */
>>>> +ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
>>>> +{
>>>> +	struct tpm2_pcr_selection pcr_selection;
>>>> +	struct tpm_buf buf;
>>>> +	void *marker;
>>>> +	unsigned int count = 0;
>>>
>>> You shouldn't initialize 'count'.
>>
>> It is initialized for reason below:
>>
>>>
>>>> +	int rc;
>>>> +	int i;
>>>> +
>>>> +	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
>>>> +	if (rc)
>>>> +		return rc;
>>>> +
>>>> +	tpm_buf_append_u32(&buf, TPM2_CAP_PCRS);
>>>> +	tpm_buf_append_u32(&buf, 0);
>>>> +	tpm_buf_append_u32(&buf, 1);
>>>> +
>>>> +	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 0,
>>>> +			      "get tpm pcr allocation");
>>>> +	if (rc < 0)
>>>> +		goto out;
>>
>> if  tpm_transmit_cmd()fails, it jumps to out:
>> where count is used in if(..).
>
> Why the out label is placed there and not after?

Since tpm2_get_pcr_allocation() returns from here as failure,
chip->active_banks doesn't get anytime initialized.

By placing it there, for failure, chip->active_bank[0] is initialized to 0.
Then its later use like in tpm_pcr_extend(), it loops on this array till 
there is a value "0"(considered as invalid) or reaches max size of array.

>
>>
>> Thanks & Regards,
>>     - Nayna
>>
>>>> +
>>>> +	count = be32_to_cpup(
>>>> +		(__be32 *)&buf.data[TPM_HEADER_SIZE + 5]);
>>>> +
>>>> +	if (count > ARRAY_SIZE(chip->active_banks))
>>>> +		return -ENODEV;
>
> Hey, here's a regression (didn't notice it last time). You
> nee to call tpm_buf_destroy().

Oh!! Yes, Thanks !!. I will fix this with:
rc = -ENODEV;
goto out;

Thanks & Regards
    - Nayna

>
>
>>>> +
>>>> +	marker = &buf.data[TPM_HEADER_SIZE + 9];
>>>> +	for (i = 0; i < count; i++) {
>>>> +		memcpy(&pcr_selection, marker, sizeof(pcr_selection));
>>>> +		chip->active_banks[i] = be16_to_cpu(pcr_selection.hash_alg);
>>>> +		marker = marker + sizeof(struct tpm2_pcr_selection);
>>>> +	}
>>>> +
>>>> +out:
>>>> +	if (count < ARRAY_SIZE(chip->active_banks))
>>>> +		chip->active_banks[count] = 0;
>>>> +
>>>> +	tpm_buf_destroy(&buf);
>>>> +
>>>> +	return rc;
>>>> +}
>>>> --
>>>> 2.5.0
>>>
>>> Otherwise,
>>>
>>> Reviewed-by: Jarkko Sakkinen <jarkko.sakkien@linux.intel.com>
>>>
>>> /Jarkko
>>>
>>
>
> /Jarkko
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 1/2] tpm: implement TPM 2.0 capability to get active PCR banks
@ 2017-01-19 13:34           ` Nayna
  0 siblings, 0 replies; 15+ messages in thread
From: Nayna @ 2017-01-19 13:34 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f



On 01/19/2017 04:17 PM, Jarkko Sakkinen wrote:
> On Wed, Jan 18, 2017 at 07:19:06PM +0530, Nayna wrote:
>>
>>
>> On 01/18/2017 07:15 PM, Jarkko Sakkinen wrote:
>>> On Wed, Jan 18, 2017 at 03:44:49AM -0500, Nayna Jain wrote:
>>>> This patch implements the TPM 2.0 capability TPM_CAP_PCRS to
>>>> retrieve the active PCR banks from the TPM. This is needed
>>>> to enable extending all active banks as recommended by TPM 2.0
>>>> TCG Specification.
>>>>
>>>> Signed-off-by: Nayna Jain <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>>>> ---
>>>>    drivers/char/tpm/tpm.h      |  4 ++++
>>>>    drivers/char/tpm/tpm2-cmd.c | 57 +++++++++++++++++++++++++++++++++++++++++++++
>>>>    2 files changed, 61 insertions(+)
>>>>
>>>> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
>>>> index 1ae9768..dddd573 100644
>>>> --- a/drivers/char/tpm/tpm.h
>>>> +++ b/drivers/char/tpm/tpm.h
>>>> @@ -127,6 +127,7 @@ enum tpm2_permanent_handles {
>>>>    };
>>>>
>>>>    enum tpm2_capabilities {
>>>> +	TPM2_CAP_PCRS		= 5,
>>>>    	TPM2_CAP_TPM_PROPERTIES = 6,
>>>>    };
>>>>
>>>> @@ -187,6 +188,8 @@ struct tpm_chip {
>>>>
>>>>    	const struct attribute_group *groups[3];
>>>>    	unsigned int groups_cnt;
>>>> +
>>>> +	u16 active_banks[7];
>>>>    #ifdef CONFIG_ACPI
>>>>    	acpi_handle acpi_dev_handle;
>>>>    	char ppi_version[TPM_PPI_VERSION_LEN + 1];
>>>> @@ -545,4 +548,5 @@ int tpm2_auto_startup(struct tpm_chip *chip);
>>>>    void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type);
>>>>    unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
>>>>    int tpm2_probe(struct tpm_chip *chip);
>>>> +ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip);
>>>>    #endif
>>>> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
>>>> index 6eda239..75a7546 100644
>>>> --- a/drivers/char/tpm/tpm2-cmd.c
>>>> +++ b/drivers/char/tpm/tpm2-cmd.c
>>>> @@ -998,3 +998,60 @@ int tpm2_auto_startup(struct tpm_chip *chip)
>>>>    		rc = -ENODEV;
>>>>    	return rc;
>>>>    }
>>>> +
>>>> +struct tpm2_pcr_selection {
>>>> +	__be16  hash_alg;
>>>> +	u8  size_of_select;
>>>> +	u8  pcr_select[3];
>>>> +} __packed;
>>>> +
>>>> +/**
>>>> + * tpm2_get_pcr_allocation() - get TPM active PCR banks.
>>>> + *
>>>> + * @chip: TPM chip to use.
>>>> + *
>>>> + * Return: Same as with tpm_transmit_cmd.
>>>> + */
>>>> +ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
>>>> +{
>>>> +	struct tpm2_pcr_selection pcr_selection;
>>>> +	struct tpm_buf buf;
>>>> +	void *marker;
>>>> +	unsigned int count = 0;
>>>
>>> You shouldn't initialize 'count'.
>>
>> It is initialized for reason below:
>>
>>>
>>>> +	int rc;
>>>> +	int i;
>>>> +
>>>> +	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
>>>> +	if (rc)
>>>> +		return rc;
>>>> +
>>>> +	tpm_buf_append_u32(&buf, TPM2_CAP_PCRS);
>>>> +	tpm_buf_append_u32(&buf, 0);
>>>> +	tpm_buf_append_u32(&buf, 1);
>>>> +
>>>> +	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 0,
>>>> +			      "get tpm pcr allocation");
>>>> +	if (rc < 0)
>>>> +		goto out;
>>
>> if  tpm_transmit_cmd()fails, it jumps to out:
>> where count is used in if(..).
>
> Why the out label is placed there and not after?

Since tpm2_get_pcr_allocation() returns from here as failure,
chip->active_banks doesn't get anytime initialized.

By placing it there, for failure, chip->active_bank[0] is initialized to 0.
Then its later use like in tpm_pcr_extend(), it loops on this array till 
there is a value "0"(considered as invalid) or reaches max size of array.

>
>>
>> Thanks & Regards,
>>     - Nayna
>>
>>>> +
>>>> +	count = be32_to_cpup(
>>>> +		(__be32 *)&buf.data[TPM_HEADER_SIZE + 5]);
>>>> +
>>>> +	if (count > ARRAY_SIZE(chip->active_banks))
>>>> +		return -ENODEV;
>
> Hey, here's a regression (didn't notice it last time). You
> nee to call tpm_buf_destroy().

Oh!! Yes, Thanks !!. I will fix this with:
rc = -ENODEV;
goto out;

Thanks & Regards
    - Nayna

>
>
>>>> +
>>>> +	marker = &buf.data[TPM_HEADER_SIZE + 9];
>>>> +	for (i = 0; i < count; i++) {
>>>> +		memcpy(&pcr_selection, marker, sizeof(pcr_selection));
>>>> +		chip->active_banks[i] = be16_to_cpu(pcr_selection.hash_alg);
>>>> +		marker = marker + sizeof(struct tpm2_pcr_selection);
>>>> +	}
>>>> +
>>>> +out:
>>>> +	if (count < ARRAY_SIZE(chip->active_banks))
>>>> +		chip->active_banks[count] = 0;
>>>> +
>>>> +	tpm_buf_destroy(&buf);
>>>> +
>>>> +	return rc;
>>>> +}
>>>> --
>>>> 2.5.0
>>>
>>> Otherwise,
>>>
>>> Reviewed-by: Jarkko Sakkinen <jarkko.sakkien-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>>>
>>> /Jarkko
>>>
>>
>
> /Jarkko
>


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2017-01-19 13:40 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-18  8:44 [PATCH v4 0/2] enhance TPM 2.0 extend function to support multiple PCR banks Nayna Jain
2017-01-18  8:44 ` Nayna Jain
2017-01-18  8:44 ` [PATCH v4 1/2] tpm: implement TPM 2.0 capability to get active " Nayna Jain
2017-01-18  8:44   ` Nayna Jain
2017-01-18 13:45   ` Jarkko Sakkinen
2017-01-18 13:45     ` Jarkko Sakkinen
2017-01-18 13:49     ` Nayna
2017-01-18 13:49       ` Nayna
2017-01-19 10:47       ` Jarkko Sakkinen
2017-01-19 13:34         ` Nayna
2017-01-19 13:34           ` Nayna
2017-01-18  8:44 ` [PATCH v4 2/2] tpm: enhance TPM 2.0 PCR extend to support multiple banks Nayna Jain
2017-01-18  8:44   ` Nayna Jain
2017-01-18 13:58   ` Jarkko Sakkinen
2017-01-19  9:18     ` Nayna

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.