[PATCH v2 0/3] tpm_pcr_extend() code split

This patch set moves TPM 1.2 specific code to a new function called
tpm1_pcr_extend(). The purpose of splitting is to isolate TPM 2.0 code,
so that it can be more easily modified to handle multiple digests.

With TPM 2.0, a Platform Configuration Register (PCR) could have multiple
values, stored in locations called banks. Each bank stores the values
of PCRs extended with the same hash algorithm.

Currently, the TPM kernel driver does not take advantage of stronger
algorithms because PCRs are always extended with a SHA1 digest, padded
with zeros to match the length of the input for the hash algorithm
being used. Shortly after these patches, a new patch set will be provided
to allow callers of tpm_pcr_extend() to pass a digest for each algorithm
supported by the TPM.

In this patch set, TPM 1.2 specific code will prepare the command buffer
with tpm_buf_init() which, in respect to the previous method, offers
protection against buffer overflow. Moreover, CPU native to big-endian
conversion has been removed from tags and ordinals definitions, as it is
already done by tpm_buf_init().

Changelog:

v2
- restored TPM_TAG_RQU_COMMAND definition in drivers/char/tpm/tpm.h
- removed endianness conversion in TPM_TAG_RQU_COMMAND definition
- removed '#include <linux/tpm_command.h>' in tpm-interface.c
  and tpm-sysfs.c
- restored TPM_ORD_ definitions in tpm-interface.c and tpm-sysfs.c

Roberto Sassu (3):
  tpm: move endianness conversion of TPM_TAG_RQU_COMMAND to
    tpm_input_header
  tpm: move endianness conversion of ordinals to tpm_input_header
  tpm: move TPM 1.2 code of tpm_pcr_extend() to tpm1_pcr_extend()

 drivers/char/tpm/tpm-interface.c | 79 ++++++++++++++++++++++------------------
 drivers/char/tpm/tpm-sysfs.c     |  6 +--
 drivers/char/tpm/tpm.h           |  2 +-
 3 files changed, 47 insertions(+), 40 deletions(-)

-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

In the long term, TPM 1.2 functions in the driver interface will be
modified to use tpm_buf_init().

However, tag and ordinals cannot be passed directly to tpm_buf_init(),
because this function performs CPU native to big-endian conversion of these
arguments. Since TPM_TAG_RQU_COMMAND and TPM_ORD_ are already converted,
passing them to the function will undo the previous conversion.

This patch moves the conversion of TPM_TAG_RQU_COMMAND from the tpm.h
header file in the driver directory to the tpm_input_header declarations
in the driver interface and tpm-sysfs.c.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
v2
- restored TPM_TAG_RQU_COMMAND definition in drivers/char/tpm/tpm.h
- removed endianness conversion in TPM_TAG_RQU_COMMAND definition
- removed '#include <linux/tpm_command.h>' in tpm-interface.c
  and tpm-sysfs.c

 drivers/char/tpm/tpm-interface.c | 14 +++++++-------
 drivers/char/tpm/tpm-sysfs.c     |  2 +-
 drivers/char/tpm/tpm.h           |  2 +-
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index bd2128e..a6bc0ba 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c

@@ -472,7 +472,7 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void *buf,
 #define TPM_ORD_GET_RANDOM cpu_to_be32(70)
 
 static const struct tpm_input_header tpm_getcap_header = {
-	.tag = TPM_TAG_RQU_COMMAND,
+	.tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
 	.length = cpu_to_be32(22),
 	.ordinal = TPM_ORD_GET_CAP
 };

@@ -514,7 +514,7 @@ EXPORT_SYMBOL_GPL(tpm_getcap);
 #define TPM_ST_STATE cpu_to_be16(2)
 #define TPM_ST_DEACTIVATED cpu_to_be16(3)
 static const struct tpm_input_header tpm_startup_header = {
-	.tag = TPM_TAG_RQU_COMMAND,
+	.tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
 	.length = cpu_to_be32(12),
 	.ordinal = TPM_ORD_STARTUP
 };

@@ -664,7 +664,7 @@ EXPORT_SYMBOL_GPL(tpm_get_timeouts);
 #define CONTINUE_SELFTEST_RESULT_SIZE 10
 
 static const struct tpm_input_header continue_selftest_header = {
-	.tag = TPM_TAG_RQU_COMMAND,
+	.tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
 	.length = cpu_to_be32(10),
 	.ordinal = cpu_to_be32(TPM_ORD_CONTINUE_SELFTEST),
 };

@@ -691,7 +691,7 @@ static int tpm_continue_selftest(struct tpm_chip *chip)
 #define READ_PCR_RESULT_SIZE 30
 #define READ_PCR_RESULT_BODY_SIZE 20
 static const struct tpm_input_header pcrread_header = {
-	.tag = TPM_TAG_RQU_COMMAND,
+	.tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
 	.length = cpu_to_be32(14),
 	.ordinal = TPM_ORDINAL_PCRREAD
 };

@@ -769,7 +769,7 @@ EXPORT_SYMBOL_GPL(tpm_pcr_read);
 #define EXTEND_PCR_RESULT_SIZE 34
 #define EXTEND_PCR_RESULT_BODY_SIZE 20
 static const struct tpm_input_header pcrextend_header = {
-	.tag = TPM_TAG_RQU_COMMAND,
+	.tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
 	.length = cpu_to_be32(34),
 	.ordinal = TPM_ORD_PCR_EXTEND
 };

@@ -991,7 +991,7 @@ EXPORT_SYMBOL_GPL(wait_for_tpm_stat);
 #define SAVESTATE_RESULT_SIZE 10
 
 static const struct tpm_input_header savestate_header = {
-	.tag = TPM_TAG_RQU_COMMAND,
+	.tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
 	.length = cpu_to_be32(10),
 	.ordinal = TPM_ORD_SAVESTATE
 };

@@ -1076,7 +1076,7 @@ EXPORT_SYMBOL_GPL(tpm_pm_resume);
 
 #define TPM_GETRANDOM_RESULT_SIZE	18
 static const struct tpm_input_header tpm_getrandom_header = {
-	.tag = TPM_TAG_RQU_COMMAND,
+	.tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
 	.length = cpu_to_be32(14),
 	.ordinal = TPM_ORD_GET_RANDOM
 };

diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 2f596d7..a078b4d 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c

@@ -24,7 +24,7 @@
 #define READ_PUBEK_RESULT_MIN_BODY_SIZE (28 + 256)
 #define TPM_ORD_READPUBEK cpu_to_be32(124)
 static const struct tpm_input_header tpm_readpubek_header = {
-	.tag = TPM_TAG_RQU_COMMAND,
+	.tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
 	.length = cpu_to_be32(30),
 	.ordinal = TPM_ORD_READPUBEK
 };

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 4937b56..7d6a606 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h

@@ -215,7 +215,7 @@ struct tpm_output_header {
 	__be32	return_code;
 } __packed;
 
-#define TPM_TAG_RQU_COMMAND cpu_to_be16(193)
+#define TPM_TAG_RQU_COMMAND 193
 
 struct	stclear_flags_t {
 	__be16	tag;

-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Wed, May 03, 2017 at 06:19:09PM +0200, Roberto Sassu wrote:

In the long term, TPM 1.2 functions in the driver interface will be
modified to use tpm_buf_init().

However, tag and ordinals cannot be passed directly to tpm_buf_init(),
because this function performs CPU native to big-endian conversion of these
arguments. Since TPM_TAG_RQU_COMMAND and TPM_ORD_ are already converted,
passing them to the function will undo the previous conversion.

This patch moves the conversion of TPM_TAG_RQU_COMMAND from the tpm.h
header file in the driver directory to the tpm_input_header declarations
in the driver interface and tpm-sysfs.c.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

Reviwed-by: Jarkko Sakkinen [off-list ref]
Tested-by: Jarkko Sakkinen <redacted>

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Move CPU native value to big-endian conversion of ordinals to the
tpm_input_header declarations.

With the previous and this patch it will now be possible to modify TPM 1.2
functions to use tpm_buf_init(), which expects CPU native value for the
tag and ordinal arguments.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
v2
- restored TPM_ORD_ definitions in tpm-interface.c and tpm-sysfs.c

 drivers/char/tpm/tpm-interface.c | 24 ++++++++++++------------
 drivers/char/tpm/tpm-sysfs.c     |  4 ++--
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index a6bc0ba..a8351dd 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c

@@ -468,13 +468,13 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void *buf,
 #define TPM_DIGEST_SIZE 20
 #define TPM_RET_CODE_IDX 6
 #define TPM_INTERNAL_RESULT_SIZE 200
-#define TPM_ORD_GET_CAP cpu_to_be32(101)
-#define TPM_ORD_GET_RANDOM cpu_to_be32(70)
+#define TPM_ORD_GET_CAP 101
+#define TPM_ORD_GET_RANDOM 70
 
 static const struct tpm_input_header tpm_getcap_header = {
 	.tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
 	.length = cpu_to_be32(22),
-	.ordinal = TPM_ORD_GET_CAP
+	.ordinal = cpu_to_be32(TPM_ORD_GET_CAP)
 };
 
 ssize_t tpm_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,

@@ -509,14 +509,14 @@ ssize_t tpm_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
 }
 EXPORT_SYMBOL_GPL(tpm_getcap);
 
-#define TPM_ORD_STARTUP cpu_to_be32(153)
+#define TPM_ORD_STARTUP 153
 #define TPM_ST_CLEAR cpu_to_be16(1)
 #define TPM_ST_STATE cpu_to_be16(2)
 #define TPM_ST_DEACTIVATED cpu_to_be16(3)
 static const struct tpm_input_header tpm_startup_header = {
 	.tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
 	.length = cpu_to_be32(12),
-	.ordinal = TPM_ORD_STARTUP
+	.ordinal = cpu_to_be32(TPM_ORD_STARTUP)
 };
 
 static int tpm_startup(struct tpm_chip *chip, __be16 startup_type)

@@ -687,13 +687,13 @@ static int tpm_continue_selftest(struct tpm_chip *chip)
 	return rc;
 }
 
-#define TPM_ORDINAL_PCRREAD cpu_to_be32(21)
+#define TPM_ORDINAL_PCRREAD 21
 #define READ_PCR_RESULT_SIZE 30
 #define READ_PCR_RESULT_BODY_SIZE 20
 static const struct tpm_input_header pcrread_header = {
 	.tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
 	.length = cpu_to_be32(14),
-	.ordinal = TPM_ORDINAL_PCRREAD
+	.ordinal = cpu_to_be32(TPM_ORDINAL_PCRREAD)
 };
 
 int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)

@@ -765,13 +765,13 @@ int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf)
 }
 EXPORT_SYMBOL_GPL(tpm_pcr_read);
 
-#define TPM_ORD_PCR_EXTEND cpu_to_be32(20)
+#define TPM_ORD_PCR_EXTEND 20
 #define EXTEND_PCR_RESULT_SIZE 34
 #define EXTEND_PCR_RESULT_BODY_SIZE 20
 static const struct tpm_input_header pcrextend_header = {
 	.tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
 	.length = cpu_to_be32(34),
-	.ordinal = TPM_ORD_PCR_EXTEND
+	.ordinal = cpu_to_be32(TPM_ORD_PCR_EXTEND)
 };
 
 /**

@@ -987,13 +987,13 @@ int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
 }
 EXPORT_SYMBOL_GPL(wait_for_tpm_stat);
 
-#define TPM_ORD_SAVESTATE cpu_to_be32(152)
+#define TPM_ORD_SAVESTATE 152
 #define SAVESTATE_RESULT_SIZE 10
 
 static const struct tpm_input_header savestate_header = {
 	.tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
 	.length = cpu_to_be32(10),
-	.ordinal = TPM_ORD_SAVESTATE
+	.ordinal = cpu_to_be32(TPM_ORD_SAVESTATE)
 };
 
 /*

@@ -1078,7 +1078,7 @@ EXPORT_SYMBOL_GPL(tpm_pm_resume);
 static const struct tpm_input_header tpm_getrandom_header = {
 	.tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
 	.length = cpu_to_be32(14),
-	.ordinal = TPM_ORD_GET_RANDOM
+	.ordinal = cpu_to_be32(TPM_ORD_GET_RANDOM)
 };
 
 /**

diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index a078b4d..c8cc0d5 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c

@@ -22,11 +22,11 @@
 
 #define READ_PUBEK_RESULT_SIZE 314
 #define READ_PUBEK_RESULT_MIN_BODY_SIZE (28 + 256)
-#define TPM_ORD_READPUBEK cpu_to_be32(124)
+#define TPM_ORD_READPUBEK 124
 static const struct tpm_input_header tpm_readpubek_header = {
 	.tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
 	.length = cpu_to_be32(30),
-	.ordinal = TPM_ORD_READPUBEK
+	.ordinal = cpu_to_be32(TPM_ORD_READPUBEK)
 };
 static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
 			  char *buf)

-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Wed, May 03, 2017 at 06:19:10PM +0200, Roberto Sassu wrote:

Move CPU native value to big-endian conversion of ordinals to the
tpm_input_header declarations.

With the previous and this patch it will now be possible to modify TPM 1.2
functions to use tpm_buf_init(), which expects CPU native value for the
tag and ordinal arguments.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

Reviwed-by: Jarkko Sakkinen [off-list ref]
Tested-by: Jarkko Sakkinen <redacted>

/Jarkko

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

In preparation of the modifications to tpm_pcr_extend(), which will
allow callers to supply a digest for each PCR bank of a TPM 2.0,
the TPM 1.2 specific code has been moved to tpm1_pcr_extend().

tpm1_pcr_extend() uses tpm_buf_init() to prepare the command buffer,
which offers protection against buffer overflow. It is called by
tpm_pcr_extend() and tpm_pm_suspend().

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 drivers/char/tpm/tpm-interface.c | 41 +++++++++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index a8351dd..0277b99 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c

@@ -774,6 +774,25 @@ static const struct tpm_input_header pcrextend_header = {
 	.ordinal = cpu_to_be32(TPM_ORD_PCR_EXTEND)
 };
 
+static int tpm1_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash,
+			   char *log_msg)
+{
+	struct tpm_buf buf;
+	int rc;
+
+	rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_PCR_EXTEND);
+	if (rc)
+		return rc;
+
+	tpm_buf_append_u32(&buf, pcr_idx);
+	tpm_buf_append(&buf, hash, TPM_DIGEST_SIZE);
+
+	rc = tpm_transmit_cmd(chip, buf.data, EXTEND_PCR_RESULT_SIZE,
+			      EXTEND_PCR_RESULT_BODY_SIZE, 0, log_msg);
+	tpm_buf_destroy(&buf);
+	return rc;
+}
+
 /**
  * tpm_pcr_extend - extend pcr value with hash
  * @chip_num:	tpm idx # or AN&

@@ -786,7 +805,6 @@ 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 rc;
 	struct tpm_chip *chip;
 	struct tpm2_digest digest_list[ARRAY_SIZE(chip->active_banks)];

@@ -812,13 +830,8 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
 		return rc;
 	}
 
-	cmd.header.in = pcrextend_header;
-	cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(pcr_idx);
-	memcpy(cmd.params.pcrextend_in.hash, hash, TPM_DIGEST_SIZE);
-	rc = tpm_transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE,
-			      EXTEND_PCR_RESULT_BODY_SIZE, 0,
-			      "attempting extend a PCR value");
-
+	rc = tpm1_pcr_extend(chip, pcr_idx, hash,
+			     "attempting extend a PCR value");
 	tpm_put_ops(chip);
 	return rc;
 }

@@ -1017,15 +1030,9 @@ int tpm_pm_suspend(struct device *dev)
 	}
 
 	/* for buggy tpm, flush pcrs with extend to selected dummy */
-	if (tpm_suspend_pcr) {
-		cmd.header.in = pcrextend_header;
-		cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(tpm_suspend_pcr);
-		memcpy(cmd.params.pcrextend_in.hash, dummy_hash,
-		       TPM_DIGEST_SIZE);
-		rc = tpm_transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE,
-				     EXTEND_PCR_RESULT_BODY_SIZE, 0,
-				      "extending dummy pcr before suspend");
-	}
+	if (tpm_suspend_pcr)
+		rc = tpm1_pcr_extend(chip, tpm_suspend_pcr, dummy_hash,
+				     "extending dummy pcr before suspend");
 
 	/* now do the actual savestate */
 	for (try = 0; try < TPM_RETRY; try++) {

-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info@ http://vger.kernel.org/majordomo-info.html

On Wed, May 03, 2017 at 06:19:11PM +0200, Roberto Sassu wrote:

In preparation of the modifications to tpm_pcr_extend(), which will
allow callers to supply a digest for each PCR bank of a TPM 2.0,
the TPM 1.2 specific code has been moved to tpm1_pcr_extend().

tpm1_pcr_extend() uses tpm_buf_init() to prepare the command buffer,
which offers protection against buffer overflow. It is called by
tpm_pcr_extend() and tpm_pm_suspend().

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

Please rebase this to the latest tree. It does not apply cleanly.
No need to send two other patches.

Use --subject-prefix="PATCH v2, RESEND" as tag.

/Jarkko

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Wed, May 03, 2017 at 06:19:08PM +0200, Roberto Sassu wrote:

This patch set moves TPM 1.2 specific code to a new function called
tpm1_pcr_extend(). The purpose of splitting is to isolate TPM 2.0 code,
so that it can be more easily modified to handle multiple digests.

With TPM 2.0, a Platform Configuration Register (PCR) could have multiple
values, stored in locations called banks. Each bank stores the values
of PCRs extended with the same hash algorithm.

Currently, the TPM kernel driver does not take advantage of stronger
algorithms because PCRs are always extended with a SHA1 digest, padded
with zeros to match the length of the input for the hash algorithm
being used. Shortly after these patches, a new patch set will be provided
to allow callers of tpm_pcr_extend() to pass a digest for each algorithm
supported by the TPM.

In this patch set, TPM 1.2 specific code will prepare the command buffer
with tpm_buf_init() which, in respect to the previous method, offers
protection against buffer overflow. Moreover, CPU native to big-endian
conversion has been removed from tags and ordinals definitions, as it is
already done by tpm_buf_init().

Changelog:

v2
- restored TPM_TAG_RQU_COMMAND definition in drivers/char/tpm/tpm.h
- removed endianness conversion in TPM_TAG_RQU_COMMAND definition
- removed '#include <linux/tpm_command.h>' in tpm-interface.c
  and tpm-sysfs.c
- restored TPM_ORD_ definitions in tpm-interface.c and tpm-sysfs.c

Roberto Sassu (3):
  tpm: move endianness conversion of TPM_TAG_RQU_COMMAND to
    tpm_input_header
  tpm: move endianness conversion of ordinals to tpm_input_header
  tpm: move TPM 1.2 code of tpm_pcr_extend() to tpm1_pcr_extend()

 drivers/char/tpm/tpm-interface.c | 79 ++++++++++++++++++++++------------------
 drivers/char/tpm/tpm-sysfs.c     |  6 +--
 drivers/char/tpm/tpm.h           |  2 +-
 3 files changed, 47 insertions(+), 40 deletions(-)

Thanks for good quality patches! I'll test these but with quick skim
no complains whatsoever :-)

/Jarkko 

-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 5/4/2017 11:24 AM, Jarkko Sakkinen wrote:

On Wed, May 03, 2017 at 06:19:08PM +0200, Roberto Sassu wrote:

quoted

This patch set moves TPM 1.2 specific code to a new function called
tpm1_pcr_extend(). The purpose of splitting is to isolate TPM 2.0 code,
so that it can be more easily modified to handle multiple digests.

With TPM 2.0, a Platform Configuration Register (PCR) could have multiple
values, stored in locations called banks. Each bank stores the values
of PCRs extended with the same hash algorithm.

Currently, the TPM kernel driver does not take advantage of stronger
algorithms because PCRs are always extended with a SHA1 digest, padded
with zeros to match the length of the input for the hash algorithm
being used. Shortly after these patches, a new patch set will be provided
to allow callers of tpm_pcr_extend() to pass a digest for each algorithm
supported by the TPM.

In this patch set, TPM 1.2 specific code will prepare the command buffer
with tpm_buf_init() which, in respect to the previous method, offers
protection against buffer overflow. Moreover, CPU native to big-endian
conversion has been removed from tags and ordinals definitions, as it is
already done by tpm_buf_init().

Changelog:

v2
- restored TPM_TAG_RQU_COMMAND definition in drivers/char/tpm/tpm.h
- removed endianness conversion in TPM_TAG_RQU_COMMAND definition
- removed '#include <linux/tpm_command.h>' in tpm-interface.c
  and tpm-sysfs.c
- restored TPM_ORD_ definitions in tpm-interface.c and tpm-sysfs.c

Roberto Sassu (3):
  tpm: move endianness conversion of TPM_TAG_RQU_COMMAND to
    tpm_input_header
  tpm: move endianness conversion of ordinals to tpm_input_header
  tpm: move TPM 1.2 code of tpm_pcr_extend() to tpm1_pcr_extend()

 drivers/char/tpm/tpm-interface.c | 79 ++++++++++++++++++++++------------------
 drivers/char/tpm/tpm-sysfs.c     |  6 +--
 drivers/char/tpm/tpm.h           |  2 +-
 3 files changed, 47 insertions(+), 40 deletions(-)

Thanks for good quality patches! I'll test these but with quick skim
no complains whatsoever :-)

Very good! We will see if it will be the same for the next patch set!

Roberto
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Thu, May 04, 2017 at 12:24:47PM +0300, Jarkko Sakkinen wrote:

On Wed, May 03, 2017 at 06:19:08PM +0200, Roberto Sassu wrote:

quoted

This patch set moves TPM 1.2 specific code to a new function called
tpm1_pcr_extend(). The purpose of splitting is to isolate TPM 2.0 code,
so that it can be more easily modified to handle multiple digests.

With TPM 2.0, a Platform Configuration Register (PCR) could have multiple
values, stored in locations called banks. Each bank stores the values
of PCRs extended with the same hash algorithm.

Currently, the TPM kernel driver does not take advantage of stronger
algorithms because PCRs are always extended with a SHA1 digest, padded
with zeros to match the length of the input for the hash algorithm
being used. Shortly after these patches, a new patch set will be provided
to allow callers of tpm_pcr_extend() to pass a digest for each algorithm
supported by the TPM.

In this patch set, TPM 1.2 specific code will prepare the command buffer
with tpm_buf_init() which, in respect to the previous method, offers
protection against buffer overflow. Moreover, CPU native to big-endian
conversion has been removed from tags and ordinals definitions, as it is
already done by tpm_buf_init().

Changelog:

v2
- restored TPM_TAG_RQU_COMMAND definition in drivers/char/tpm/tpm.h
- removed endianness conversion in TPM_TAG_RQU_COMMAND definition
- removed '#include <linux/tpm_command.h>' in tpm-interface.c
  and tpm-sysfs.c
- restored TPM_ORD_ definitions in tpm-interface.c and tpm-sysfs.c

Roberto Sassu (3):
  tpm: move endianness conversion of TPM_TAG_RQU_COMMAND to
    tpm_input_header
  tpm: move endianness conversion of ordinals to tpm_input_header
  tpm: move TPM 1.2 code of tpm_pcr_extend() to tpm1_pcr_extend()

 drivers/char/tpm/tpm-interface.c | 79 ++++++++++++++++++++++------------------
 drivers/char/tpm/tpm-sysfs.c     |  6 +--
 drivers/char/tpm/tpm.h           |  2 +-
 3 files changed, 47 insertions(+), 40 deletions(-)

Thanks for good quality patches! I'll test these but with quick skim
no complains whatsoever :-)

1/3 and 2/3 are now applied to master and next (which will be pulled to
linux-next).

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

In preparation of the modifications to tpm_pcr_extend(), which will
allow callers to supply a digest for each PCR bank of a TPM 2.0,
the TPM 1.2 specific code has been moved to tpm1_pcr_extend().

tpm1_pcr_extend() uses tpm_buf_init() to prepare the command buffer,
which offers protection against buffer overflow. It is called by
tpm_pcr_extend() and tpm_pm_suspend().

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 drivers/char/tpm/tpm-interface.c | 41 +++++++++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 7966d8d..4ed08ab 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c

@@ -847,6 +847,25 @@ static const struct tpm_input_header pcrextend_header = {
 	.ordinal = cpu_to_be32(TPM_ORD_PCR_EXTEND)
 };
 
+static int tpm1_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash,
+			   char *log_msg)
+{
+	struct tpm_buf buf;
+	int rc;
+
+	rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_PCR_EXTEND);
+	if (rc)
+		return rc;
+
+	tpm_buf_append_u32(&buf, pcr_idx);
+	tpm_buf_append(&buf, hash, TPM_DIGEST_SIZE);
+
+	rc = tpm_transmit_cmd(chip, NULL, buf.data, EXTEND_PCR_RESULT_SIZE,
+			      EXTEND_PCR_RESULT_BODY_SIZE, 0, log_msg);
+	tpm_buf_destroy(&buf);
+	return rc;
+}
+
 /**
  * tpm_pcr_extend - extend pcr value with hash
  * @chip_num:	tpm idx # or AN&

@@ -859,7 +878,6 @@ 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 rc;
 	struct tpm_chip *chip;
 	struct tpm2_digest digest_list[ARRAY_SIZE(chip->active_banks)];

@@ -885,13 +903,8 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
 		return rc;
 	}
 
-	cmd.header.in = pcrextend_header;
-	cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(pcr_idx);
-	memcpy(cmd.params.pcrextend_in.hash, hash, TPM_DIGEST_SIZE);
-	rc = tpm_transmit_cmd(chip, NULL, &cmd, EXTEND_PCR_RESULT_SIZE,
-			      EXTEND_PCR_RESULT_BODY_SIZE, 0,
-			      "attempting extend a PCR value");
-
+	rc = tpm1_pcr_extend(chip, pcr_idx, hash,
+			     "attempting extend a PCR value");
 	tpm_put_ops(chip);
 	return rc;
 }

@@ -1090,15 +1103,9 @@ int tpm_pm_suspend(struct device *dev)
 	}
 
 	/* for buggy tpm, flush pcrs with extend to selected dummy */
-	if (tpm_suspend_pcr) {
-		cmd.header.in = pcrextend_header;
-		cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(tpm_suspend_pcr);
-		memcpy(cmd.params.pcrextend_in.hash, dummy_hash,
-		       TPM_DIGEST_SIZE);
-		rc = tpm_transmit_cmd(chip, NULL, &cmd, EXTEND_PCR_RESULT_SIZE,
-				      EXTEND_PCR_RESULT_BODY_SIZE, 0,
-				      "extending dummy pcr before suspend");
-	}
+	if (tpm_suspend_pcr)
+		rc = tpm1_pcr_extend(chip, tpm_suspend_pcr, dummy_hash,
+				     "extending dummy pcr before suspend");
 
 	/* now do the actual savestate */
 	for (try = 0; try < TPM_RETRY; try++) {

-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info@ http://vger.kernel.org/majordomo-info.html

On Thu, May 04, 2017 at 01:16:47PM +0200, Roberto Sassu wrote:

In preparation of the modifications to tpm_pcr_extend(), which will
allow callers to supply a digest for each PCR bank of a TPM 2.0,
the TPM 1.2 specific code has been moved to tpm1_pcr_extend().

tpm1_pcr_extend() uses tpm_buf_init() to prepare the command buffer,
which offers protection against buffer overflow. It is called by
tpm_pcr_extend() and tpm_pm_suspend().

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

Reviewed-by: Jarkko Sakkinen <redacted>
Tested-by: Jarkko Sakkinen <redacted>

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html