[PATCH v4 0/7] add integrity and security to TPM2 transactions

[PATCH v4 0/7] add integrity and security to TPM2 transactions

[James Bottomley]

By now, everybody knows we have a problem with the TPM2_RS_PW easy
button on TPM2 in that transactions on the TPM bus can be intercepted
and altered.  The way to fix this is to use real sessions for HMAC
capabilities to ensure integrity and to use parameter and response
encryption to ensure confidentiality of the data flowing over the TPM
bus.

This patch series is about adding a simple API which can ensure the
above properties as a layered addition to the existing TPM handling
code.  This series now includes protections for PCR extend, getting
random numbers from the TPM and data sealing and unsealing.  It
therefore eliminates all uses of TPM2_RS_PW in the kernel and adds
encryption protection to sensitive data flowing into and out of the
TPM.

In the third version I added data sealing and unsealing protection,
apart from one API based problem which means that the way trusted keys
were protected it's not currently possible to HMAC protect an authority
that comes with a policy, so the API will have to be extended to fix
that case

In this fourth version, I tidy up some of the code and add more
security features, the most notable is that we now calculate the NULL
seed name and compare our calculation to the value returned in
TPM2_ReadPublic, which means we now can't be spoofed.  This version
also gives a sysfs variable for the null seed which userspace can use
to run a key certification operation to prove that the TPM was always
secure when communicating with the kernel.

I've verified this using the test suite in the last patch on a VM
connected to a tpm2 emulator.  I also instrumented the emulator to make
sure the sensitive data was properly encrypted.

James

---


James Bottomley (7):
  tpm-buf: create new functions for handling TPM buffers
  tpm2-sessions: Add full HMAC and encrypt/decrypt session handling
  tpm2: add hmac checks to tpm2_pcr_extend()
  tpm2: add session encryption protection to tpm2_get_random()
  trusted keys: Add session encryption protection to the seal/unseal
    path
  tpm: add the null key name as a tpm2 sysfs variable
  tpm2-sessions: NOT FOR COMMITTING add sessions testing

 drivers/char/tpm/Kconfig              |    3 +
 drivers/char/tpm/Makefile             |    3 +-
 drivers/char/tpm/tpm-buf.c            |  191 ++++++
 drivers/char/tpm/tpm-chip.c           |    1 +
 drivers/char/tpm/tpm-sysfs.c          |   27 +-
 drivers/char/tpm/tpm.h                |  129 ++--
 drivers/char/tpm/tpm2-cmd.c           |  248 ++++---
 drivers/char/tpm/tpm2-sessions-test.c |  360 ++++++++++
 drivers/char/tpm/tpm2-sessions.c      | 1188 +++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm2-sessions.h      |   57 ++
 10 files changed, 2027 insertions(+), 180 deletions(-)
 create mode 100644 drivers/char/tpm/tpm-buf.c
 create mode 100644 drivers/char/tpm/tpm2-sessions-test.c
 create mode 100644 drivers/char/tpm/tpm2-sessions.c
 create mode 100644 drivers/char/tpm/tpm2-sessions.h

-- 
2.16.4

[PATCH v4 1/7] tpm-buf: create new functions for handling TPM buffers

[James Bottomley]

This separates out the old tpm_buf_... handling functions from static
inlines into tpm.h and makes them their own tpm-buf.c file.  It also
adds handling for tpm2b structures and also incremental pointer
advancing parsers.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>

---

v2: added this patch to separate out the API changes
v3: added tpm_buf_reset_cmd()
v4: renamed tpm_buf_reset_cmd() to tpm_buf_reset()
---
 drivers/char/tpm/Makefile  |   2 +-
 drivers/char/tpm/tpm-buf.c | 191 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm.h     |  96 ++++-------------------
 3 files changed, 208 insertions(+), 81 deletions(-)
 create mode 100644 drivers/char/tpm/tpm-buf.c

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 3655258bee73..65d165cce530 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -5,7 +5,7 @@
 obj-$(CONFIG_TCG_TPM) += tpm.o
 tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
 	 tpm-dev-common.o tpmrm-dev.o eventlog/common.o eventlog/tpm1.o \
-	 eventlog/tpm2.o tpm2-space.o
+	 eventlog/tpm2.o tpm2-space.o tpm-buf.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o eventlog/acpi.o
 tpm-$(CONFIG_EFI) += eventlog/efi.o
 tpm-$(CONFIG_OF) += eventlog/of.o
diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
new file mode 100644
index 000000000000..faa22bb09d99
--- /dev/null
+++ b/drivers/char/tpm/tpm-buf.c
@@ -0,0 +1,191 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Handing for tpm2b structures to facilitate the building of commands
+ */
+
+#include "tpm.h"
+
+#include <linux/module.h>
+
+#include <asm/unaligned.h>
+
+static int __tpm_buf_init(struct tpm_buf *buf)
+{
+	buf->data_page = alloc_page(GFP_HIGHUSER);
+	if (!buf->data_page)
+		return -ENOMEM;
+
+	buf->flags = 0;
+	buf->data = kmap(buf->data_page);
+
+	return 0;
+}
+
+void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal)
+{
+	struct tpm_input_header *head;
+
+	head = (struct tpm_input_header *) buf->data;
+
+	head->tag = cpu_to_be16(tag);
+	head->length = cpu_to_be32(sizeof(*head));
+	head->ordinal = cpu_to_be32(ordinal);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_reset);
+
+int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
+{
+	int rc;
+
+	rc = __tpm_buf_init(buf);
+	if (rc)
+		return rc;
+
+	tpm_buf_reset(buf, tag, ordinal);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(tpm_buf_init);
+
+int tpm_buf_init_2b(struct tpm_buf *buf)
+{
+	struct tpm_input_header *head;
+	int rc;
+
+	rc = __tpm_buf_init(buf);
+	if (rc)
+		return rc;
+
+	head = (struct tpm_input_header *) buf->data;
+
+	head->length = cpu_to_be32(sizeof(*head));
+
+	buf->flags = TPM_BUF_2B;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(tpm_buf_init_2b);
+
+void tpm_buf_destroy(struct tpm_buf *buf)
+{
+	kunmap(buf->data_page);
+	__free_page(buf->data_page);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_destroy);
+
+static void *tpm_buf_data(struct tpm_buf *buf)
+{
+	if (buf->flags & TPM_BUF_2B)
+		return buf->data + TPM_HEADER_SIZE;
+	return buf->data;
+}
+
+u32 tpm_buf_length(struct tpm_buf *buf)
+{
+	struct tpm_input_header *head = (struct tpm_input_header *)buf->data;
+	u32 len;
+
+	len = be32_to_cpu(head->length);
+	if (buf->flags & TPM_BUF_2B)
+		len -= sizeof(*head);
+	return len;
+}
+EXPORT_SYMBOL_GPL(tpm_buf_length);
+
+u16 tpm_buf_tag(struct tpm_buf *buf)
+{
+	struct tpm_input_header *head = (struct tpm_input_header *)buf->data;
+
+	return be16_to_cpu(head->tag);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_tag);
+
+void tpm_buf_append(struct tpm_buf *buf,
+		    const unsigned char *new_data,
+		    unsigned int new_len)
+{
+	struct tpm_input_header *head = (struct tpm_input_header *) buf->data;
+	u32 len = be32_to_cpu(head->length);
+
+	/* Return silently if overflow has already happened. */
+	if (buf->flags & TPM_BUF_OVERFLOW)
+		return;
+
+	if ((len + new_len) > PAGE_SIZE) {
+		WARN(1, "tpm_buf: overflow\n");
+		buf->flags |= TPM_BUF_OVERFLOW;
+		return;
+	}
+
+	memcpy(&buf->data[len], new_data, new_len);
+	head->length = cpu_to_be32(len + new_len);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_append);
+
+void tpm_buf_append_u8(struct tpm_buf *buf, const u8 value)
+{
+	tpm_buf_append(buf, &value, 1);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_append_u8);
+
+void tpm_buf_append_u16(struct tpm_buf *buf, const u16 value)
+{
+	__be16 value2 = cpu_to_be16(value);
+
+	tpm_buf_append(buf, (u8 *) &value2, 2);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_append_u16);
+
+void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
+{
+	__be32 value2 = cpu_to_be32(value);
+
+	tpm_buf_append(buf, (u8 *) &value2, 4);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_append_u32);
+
+static void tpm_buf_reset_int(struct tpm_buf *buf)
+{
+	struct tpm_input_header *head;
+
+	head = (struct tpm_input_header *)buf->data;
+	head->length = cpu_to_be32(sizeof(*head));
+}
+
+void tpm_buf_append_2b(struct tpm_buf *buf, struct tpm_buf *tpm2b)
+{
+	u16 len = tpm_buf_length(tpm2b);
+
+	tpm_buf_append_u16(buf, len);
+	tpm_buf_append(buf, tpm_buf_data(tpm2b), len);
+	/* clear the buf for reuse */
+	tpm_buf_reset_int(tpm2b);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_append_2b);
+
+/* functions for unmarshalling data and moving the cursor */
+u8 tpm_get_inc_u8(const u8 **ptr)
+{
+	return *((*ptr)++);
+}
+EXPORT_SYMBOL_GPL(tpm_get_inc_u8);
+
+u16 tpm_get_inc_u16(const u8 **ptr)
+{
+	u16 val;
+
+	val = get_unaligned_be16(*ptr);
+	*ptr += sizeof(val);
+	return val;
+}
+EXPORT_SYMBOL_GPL(tpm_get_inc_u16);
+
+u32 tpm_get_inc_u32(const u8 **ptr)
+{
+	u32 val;
+
+	val = get_unaligned_be32(*ptr);
+	*ptr += sizeof(val);
+	return val;
+}
+EXPORT_SYMBOL_GPL(tpm_get_inc_u32);
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index f3501d05264f..d73701e36eba 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -416,6 +416,7 @@ struct tpm_cmd_t {
 
 enum tpm_buf_flags {
 	TPM_BUF_OVERFLOW	= BIT(0),
+	TPM_BUF_2B		= BIT(1),
 };
 
 struct tpm_buf {
@@ -424,86 +425,21 @@ struct tpm_buf {
 	u8 *data;
 };
 
-static inline void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal)
-{
-	struct tpm_input_header *head;
-	head = (struct tpm_input_header *)buf->data;
-	head->tag = cpu_to_be16(tag);
-	head->length = cpu_to_be32(sizeof(*head));
-	head->ordinal = cpu_to_be32(ordinal);
-}
-
-static inline int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
-{
-	buf->data_page = alloc_page(GFP_HIGHUSER);
-	if (!buf->data_page)
-		return -ENOMEM;
-
-	buf->flags = 0;
-	buf->data = kmap(buf->data_page);
-	tpm_buf_reset(buf, tag, ordinal);
-	return 0;
-}
-
-static inline void tpm_buf_destroy(struct tpm_buf *buf)
-{
-	kunmap(buf->data_page);
-	__free_page(buf->data_page);
-}
-
-static inline u32 tpm_buf_length(struct tpm_buf *buf)
-{
-	struct tpm_input_header *head = (struct tpm_input_header *) buf->data;
-
-	return be32_to_cpu(head->length);
-}
-
-static inline u16 tpm_buf_tag(struct tpm_buf *buf)
-{
-	struct tpm_input_header *head = (struct tpm_input_header *) buf->data;
-
-	return be16_to_cpu(head->tag);
-}
-
-static inline void tpm_buf_append(struct tpm_buf *buf,
-				  const unsigned char *new_data,
-				  unsigned int new_len)
-{
-	struct tpm_input_header *head = (struct tpm_input_header *) buf->data;
-	u32 len = tpm_buf_length(buf);
-
-	/* Return silently if overflow has already happened. */
-	if (buf->flags & TPM_BUF_OVERFLOW)
-		return;
-
-	if ((len + new_len) > PAGE_SIZE) {
-		WARN(1, "tpm_buf: overflow\n");
-		buf->flags |= TPM_BUF_OVERFLOW;
-		return;
-	}
-
-	memcpy(&buf->data[len], new_data, new_len);
-	head->length = cpu_to_be32(len + new_len);
-}
-
-static inline void tpm_buf_append_u8(struct tpm_buf *buf, const u8 value)
-{
-	tpm_buf_append(buf, &value, 1);
-}
-
-static inline void tpm_buf_append_u16(struct tpm_buf *buf, const u16 value)
-{
-	__be16 value2 = cpu_to_be16(value);
-
-	tpm_buf_append(buf, (u8 *) &value2, 2);
-}
-
-static inline void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
-{
-	__be32 value2 = cpu_to_be32(value);
-
-	tpm_buf_append(buf, (u8 *) &value2, 4);
-}
+int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal);
+void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal);
+int tpm_buf_init_2b(struct tpm_buf *buf);
+void tpm_buf_destroy(struct tpm_buf *buf);
+u32 tpm_buf_length(struct tpm_buf *buf);
+void tpm_buf_append(struct tpm_buf *buf, const unsigned char *new_data,
+		    unsigned int new_len);
+void tpm_buf_append_u8(struct tpm_buf *buf, const u8 value);
+void tpm_buf_append_u16(struct tpm_buf *buf, const u16 value);
+void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value);
+void tpm_buf_append_2b(struct tpm_buf *buf, struct tpm_buf *tpm2b);
+
+u8 tpm_get_inc_u8(const u8 **ptr);
+u16 tpm_get_inc_u16(const u8 **ptr);
+u32 tpm_get_inc_u32(const u8 **ptr);
 
 extern struct class *tpm_class;
 extern struct class *tpmrm_class;
-- 
2.16.4

[PATCH v4 2/7] tpm2-sessions: Add full HMAC and encrypt/decrypt session handling

[James Bottomley]

This code adds true session based HMAC authentication plus parameter
decryption and response encryption using AES.

The basic design of this code is to segregate all the nasty crypto,
hash and hmac code into tpm2-sessions.c and export a usable API.

The API first of all starts off by gaining a session with

tpm2_start_auth_session()

Which initiates a session with the TPM and allocates an opaque
tpm2_auth structure to handle the session parameters.  Then the use is
simply:

* tpm_buf_append_name() in place of the tpm_buf_append_u32 for the
  handles

* tpm_buf_append_hmac_session() where tpm2_append_auth() would go

* tpm_buf_fill_hmac_session() called after the entire command buffer
  is finished but before tpm_transmit_cmd() is called which computes
  the correct HMAC and places it in the command at the correct
  location.

Finally, after tpm_transmit_cmd() is called,
tpm_buf_check_hmac_response() is called to check that the returned
HMAC matched and collect the new state for the next use of the
session, if any.

The features of the session is controlled by the session attributes
set in tpm_buf_append_hmac_session().  If TPM2_SA_CONTINUE_SESSION is
not specified, the session will be flushed and the tpm2_auth structure
freed in tpm_buf_check_hmac_response(); otherwise the session may be
used again.  Parameter encryption is specified by or'ing the flag
TPM2_SA_DECRYPT and response encryption by or'ing the flag
TPM2_SA_ENCRYPT.  the various encryptions will be taken care of by
tpm_buf_fill_hmac_session() and tpm_buf_check_hmac_response()
respectively.

To get all of this to work securely, the Kernel now needs a primary
key to encrypt the session salt to, so we derive an EC key from the
NULL seed and store it in the tpm_chip structure.  We also make sure
that this seed remains for the kernel by using a kernel space to take
it out of the TPM when userspace wants to use it.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>

---

v2: Added docbook and improved response check API
v3: Add readpublic, fix hmac length, add API for close on error
    allow for the hmac session not being first in the sessions
v4: Make NULL seed template exactly match the SRK ECC template.
    Also check the NULL primary key name is what getpublic returns
    to prevent spoofing.  Also parametrise the name size for reuse
---
 drivers/char/tpm/Kconfig         |    3 +
 drivers/char/tpm/Makefile        |    2 +-
 drivers/char/tpm/tpm.h           |   32 +
 drivers/char/tpm/tpm2-cmd.c      |   34 +-
 drivers/char/tpm/tpm2-sessions.c | 1188 ++++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm2-sessions.h |   57 ++
 6 files changed, 1300 insertions(+), 16 deletions(-)
 create mode 100644 drivers/char/tpm/tpm2-sessions.c
 create mode 100644 drivers/char/tpm/tpm2-sessions.h

diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index 0aee88df98d1..8c714d8550c4 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -8,6 +8,9 @@ menuconfig TCG_TPM
 	select SECURITYFS
 	select CRYPTO
 	select CRYPTO_HASH_INFO
+	select CRYPTO_ECDH
+	select CRYPTO_AES
+	select CRYPTO_CFB
 	---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/Makefile b/drivers/char/tpm/Makefile
index 65d165cce530..1b5f6ccbb86d 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -5,7 +5,7 @@
 obj-$(CONFIG_TCG_TPM) += tpm.o
 tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
 	 tpm-dev-common.o tpmrm-dev.o eventlog/common.o eventlog/tpm1.o \
-	 eventlog/tpm2.o tpm2-space.o tpm-buf.o
+	 eventlog/tpm2.o tpm2-space.o tpm-buf.o tpm2-sessions.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o eventlog/acpi.o
 tpm-$(CONFIG_EFI) += eventlog/efi.o
 tpm-$(CONFIG_OF) += eventlog/of.o
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index d73701e36eba..d39065d9995d 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -42,6 +42,15 @@
 #include <asm/intel-family.h>
 #endif
 
+/* fixed define for the curve we use which is NIST_P256 */
+#define EC_PT_SZ	32
+
+/*
+ * fixed define for the size of a name.  This is actually HASHALG size
+ * plus 2, so 32 for SHA256
+ */
+#define TPM2_NAME_SIZE	34
+
 enum tpm_const {
 	TPM_MINOR = 224,	/* officially assigned */
 	TPM_BUFSIZE = 4096,
@@ -103,6 +112,7 @@ enum tpm2_timeouts {
 enum tpm2_structures {
 	TPM2_ST_NO_SESSIONS	= 0x8001,
 	TPM2_ST_SESSIONS	= 0x8002,
+	TPM2_ST_CREATION	= 0x8021,
 };
 
 /* Indicates from what layer of the software stack the error comes from */
@@ -125,12 +135,20 @@ enum tpm2_return_codes {
 enum tpm2_algorithms {
 	TPM2_ALG_ERROR		= 0x0000,
 	TPM2_ALG_SHA1		= 0x0004,
+	TPM2_ALG_AES		= 0x0006,
 	TPM2_ALG_KEYEDHASH	= 0x0008,
 	TPM2_ALG_SHA256		= 0x000B,
 	TPM2_ALG_SHA384		= 0x000C,
 	TPM2_ALG_SHA512		= 0x000D,
 	TPM2_ALG_NULL		= 0x0010,
 	TPM2_ALG_SM3_256	= 0x0012,
+	TPM2_ALG_ECC		= 0x0023,
+	TPM2_ALG_CFB		= 0x0043,
+};
+
+enum tpm2_curves {
+	TPM2_ECC_NONE		= 0x0000,
+	TPM2_ECC_NIST_P256	= 0x0003,
 };
 
 enum tpm2_command_codes {
@@ -145,6 +163,8 @@ enum tpm2_command_codes {
 	TPM2_CC_CONTEXT_LOAD	= 0x0161,
 	TPM2_CC_CONTEXT_SAVE	= 0x0162,
 	TPM2_CC_FLUSH_CONTEXT	= 0x0165,
+	TPM2_CC_READ_PUBLIC	= 0x0173,
+	TPM2_CC_START_AUTH_SESS	= 0x0176,
 	TPM2_CC_GET_CAPABILITY	= 0x017A,
 	TPM2_CC_GET_RANDOM	= 0x017B,
 	TPM2_CC_PCR_READ	= 0x017E,
@@ -153,6 +173,7 @@ enum tpm2_command_codes {
 };
 
 enum tpm2_permanent_handles {
+	TPM2_RH_NULL		= 0x40000007,
 	TPM2_RS_PW		= 0x40000009,
 };
 
@@ -255,11 +276,18 @@ struct tpm_chip {
 #endif /* CONFIG_ACPI */
 
 	struct tpm_space work_space;
+	struct tpm_space kernel_space;
 	u32 nr_commands;
 	u32 *cc_attrs_tbl;
 
 	/* active locality */
 	int locality;
+
+	/* details for communication security via sessions */
+	u32 tpmkey;
+	u8 tpmkeyname[TPM2_NAME_SIZE];
+	u8 ec_point_x[EC_PT_SZ];
+	u8 ec_point_y[EC_PT_SZ];
 };
 
 #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)
@@ -441,6 +469,9 @@ u8 tpm_get_inc_u8(const u8 **ptr);
 u16 tpm_get_inc_u16(const u8 **ptr);
 u32 tpm_get_inc_u32(const u8 **ptr);
 
+/* opaque structure, holds auth session parameters like the session key */
+struct tpm2_auth;
+
 extern struct class *tpm_class;
 extern struct class *tpmrm_class;
 extern dev_t tpm_devt;
@@ -540,4 +571,5 @@ int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
 
 int tpm_bios_log_setup(struct tpm_chip *chip);
 void tpm_bios_log_teardown(struct tpm_chip *chip);
+int tpm2_sessions_init(struct tpm_chip *chip);
 #endif
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index c31b490bd41d..a17e5c573c4e 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -16,17 +16,10 @@
  */
 
 #include "tpm.h"
+#include "tpm2-sessions.h"
 #include <crypto/hash_info.h>
 #include <keys/trusted-type.h>
 
-enum tpm2_object_attributes {
-	TPM2_OA_USER_WITH_AUTH		= BIT(6),
-};
-
-enum tpm2_session_attributes {
-	TPM2_SA_CONTINUE_SESSION	= BIT(0),
-};
-
 struct tpm2_hash {
 	unsigned int crypto_id;
 	unsigned int tpm_id;
@@ -350,15 +343,19 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
 /**
  * tpm2_flush_context_cmd() - execute a TPM2_FlushContext command
  * @chip: TPM chip to use
- * @payload: the key data in clear and encrypted form
- * @options: authentication values and other options
+ * @handle: the handle to flush
+ * @flags: flags for transmit
  *
+ * Note: @flags may be TPM_TRANSMIT_UNLOCKED or 0.  If @flags is 0, we
+ * assume the flush handle is going through the kernel space.
+
  * Return: same as with tpm_transmit_cmd
  */
 void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
 			    unsigned int flags)
 {
 	struct tpm_buf buf;
+	struct tpm_space *space = NULL;
 	int rc;
 
 	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT);
@@ -367,10 +364,12 @@ void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
 			 handle);
 		return;
 	}
+	if (!flags)
+		space = &chip->kernel_space;
 
 	tpm_buf_append_u32(&buf, handle);
 
-	(void) tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, flags,
+	(void) tpm_transmit_cmd(chip, space, buf.data, PAGE_SIZE, 0, flags,
 				"flushing context");
 
 	tpm_buf_destroy(&buf);
@@ -387,10 +386,10 @@ void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
  * @hmac: the session HMAC or password, may be NULL if not used
  * @hmac_len: the session HMAC or password length, maybe 0 if not used
  */
-static void tpm2_buf_append_auth(struct tpm_buf *buf, u32 session_handle,
-				 const u8 *nonce, u16 nonce_len,
-				 u8 attributes,
-				 const u8 *hmac, u16 hmac_len)
+void tpm2_buf_append_auth(struct tpm_buf *buf, u32 session_handle,
+			  const u8 *nonce, u16 nonce_len,
+			  u8 attributes,
+			  const u8 *hmac, u16 hmac_len)
 {
 	tpm_buf_append_u32(buf, 9 + nonce_len + hmac_len);
 	tpm_buf_append_u32(buf, session_handle);
@@ -1015,6 +1014,11 @@ int tpm2_auto_startup(struct tpm_chip *chip)
 
 	rc = tpm2_get_cc_attrs_tbl(chip);
 
+	if (rc)
+		goto out;
+
+	rc = tpm2_sessions_init(chip);
+
 out:
 	if (rc > 0)
 		rc = -ENODEV;
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
new file mode 100644
index 000000000000..422c3ec64f8c
--- /dev/null
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -0,0 +1,1188 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Copyright (C) 2018 James.Bottomley@HansenPartnership.com
+ *
+ * Cryptographic helper routines for handling TPM2 sessions for
+ * authorization HMAC and request response encryption.
+ *
+ * The idea is to ensure that every TPM command is HMAC protected by a
+ * session, meaning in-flight tampering would be detected and in
+ * addition all sensitive inputs and responses should be encrypted.
+ *
+ * The basic way this works is to use a TPM feature called salted
+ * sessions where a random secret used in session construction is
+ * encrypted to the public part of a known TPM key.  The problem is we
+ * have no known keys, so initially a primary Elliptic Curve key is
+ * derived from the NULL seed (we use EC because most TPMs generate
+ * these keys much faster than RSA ones).  The curve used is NIST_P256
+ * because that's now mandated to be present in 'TCG TPM v2.0
+ * Provisioning Guidance'
+ *
+ * Threat problems: the initial TPM2_CreatePrimary is not (and cannot
+ * be) session protected, so a clever Man in the Middle could return a
+ * public key they control to this command and from there intercept
+ * and decode all subsequent session based transactions.  The kernel
+ * cannot mitigate this threat but, after boot, userspace can get
+ * proof this has not happened by asking the TPM to certify the NULL
+ * key.  This certification would chain back to the TPM Endorsement
+ * Certificate and prove the NULL seed primary had not been tampered
+ * with and thus all sessions must have been cryptographically secure.
+ * To assist with this, the initial NULL seed public key name is made
+ * available in a sysfs file.
+ *
+ * Use of these functions:
+ *
+ * The design is all the crypto, hash and hmac gunk is confined in this
+ * file and never needs to be seen even by the kernel internal user.  To
+ * the user there's an init function tpm2_sessions_init() that needs to
+ * be called once per TPM which generates the NULL seed primary key.
+ *
+ * Then there are six usage functions:
+ *
+ * tpm2_start_auth_session() which allocates the opaque auth structure
+ *	and gets a session from the TPM.  This must be called before
+ *	any of the following functions.  The session is protected by a
+ *	session_key which is derived from a random salt value
+ *	encrypted to the NULL seed.
+ * tpm2_end_auth_session() kills the session and frees the resources.
+ *	Under normal operation this function is done by
+ *	tpm_buf_check_hmac_response(), so this is only to be used on
+ *	error legs where the latter is not executed.
+ * tpm_buf_append_name() to add a handle to the buffer.  This must be
+ *	used in place of the usual tpm_buf_append_u32() for adding
+ *	handles because handles have to be processed specially when
+ *	calculating the HMAC.  In particular, for NV, volatile and
+ *	permanent objects you now need to provide the name.
+ * tpm_buf_append_hmac_session() which appends the hmac session to the
+ *	buf in the same way tpm_buf_append_auth does().
+ * tpm_buf_fill_hmac_session() This calculates the correct hash and
+ *	places it in the buffer.  It must be called after the complete
+ *	command buffer is finalized so it can fill in the correct HMAC
+ *	based on the parameters.
+ * tpm_buf_check_hmac_response() which checks the session response in
+ *	the buffer and calculates what it should be.  If there's a
+ *	mismatch it will log a warning and return an error.  If
+ *	tpm_buf_append_hmac_session() did not specify
+ *	TPM_SA_CONTINUE_SESSION then the session will be closed (if it
+ *	hasn't been consumed) and the auth structure freed.
+ */
+
+#include "tpm.h"
+#include "tpm2-sessions.h"
+
+#include <linux/random.h>
+#include <linux/scatterlist.h>
+
+#include <asm/unaligned.h>
+
+#include <crypto/aes.h>
+#include <crypto/kpp.h>
+#include <crypto/ecdh.h>
+#include <crypto/hash.h>
+#include <crypto/hmac.h>
+#include <crypto/skcipher.h>
+
+/* if you change to AES256, you only need change this */
+#define AES_KEYBYTES	AES_KEYSIZE_128
+
+#define AES_KEYBITS	(AES_KEYBYTES*8)
+#define AUTH_MAX_NAMES	3
+
+/*
+ * This is the structure that carries all the auth information (like
+ * session handle, nonces, session key and auth) from use to use it is
+ * designed to be opaque to anything outside.
+ */
+struct tpm2_auth {
+	u32 handle;
+	/*
+	 * This has two meanings: before tpm_buf_fill_hmac_session()
+	 * it marks the offset in the buffer of the start of the
+	 * sessions (i.e. after all the handles).  Once the buffer has
+	 * been filled it markes the session number of our auth
+	 * session so we can find it again in the response buffer.
+	 *
+	 * The two cases are distinguished because the first offset
+	 * must always be greater than TPM_HEADER_SIZE and the second
+	 * must be less than or equal to 5.
+	 */
+	u32 session;
+	/*
+	 * the size here is variable and set by the size of our_nonce
+	 * which must be between 16 and the name hash length. we set
+	 * the maximum sha256 size for the greatest protection
+	 */
+	u8 our_nonce[SHA256_DIGEST_SIZE];
+	u8 tpm_nonce[SHA256_DIGEST_SIZE];
+	/*
+	 * the salt is only used across the session command/response
+	 * after that it can be used as a scratch area
+	 */
+	union {
+		u8 salt[EC_PT_SZ];
+		/* scratch for key + IV */
+		u8 scratch[AES_KEYBYTES + AES_BLOCK_SIZE];
+	};
+	/*
+	 * the session key and passphrase are the same size as the
+	 * name digest (sha256 again).  The session key is constant
+	 * for the use of the session and the passphrase can change
+	 * with every invocation.
+	 *
+	 * Note: these fields must be adjacent and in this order
+	 * because several HMAC/KDF schemes use the combination of the
+	 * session_key and passphrase.
+	 */
+	u8 session_key[SHA256_DIGEST_SIZE];
+	u8 passphrase[SHA256_DIGEST_SIZE];
+	int passphraselen;
+	/* saved session attributes */
+	u8 attrs;
+	__be32 ordinal;
+	struct crypto_skcipher *aes;
+	struct tpm_chip *chip;
+	/* 3 names of handles: name_h is handle, name is name of handle */
+	u32 name_h[AUTH_MAX_NAMES];
+	u8 name[AUTH_MAX_NAMES][2 + SHA256_DIGEST_SIZE];
+};
+
+/*
+ * this is our static crypto shash.  This is possible because the hash
+ * is multi-threaded and all the state stored in the desc
+ */
+static struct crypto_shash *sha256_hash;
+
+/*
+ * It turns out the crypto hmac(sha256) is hard for us to consume
+ * because it assumes a fixed key and the TPM seems to change the key
+ * on every operation, so we weld the hmac init and final functions in
+ * here to give it the same usage characteristics as a regular hash
+ */
+static void hmac_init(struct shash_desc *desc, u8 *key, int keylen)
+{
+	u8 pad[SHA256_BLOCK_SIZE];
+	int i;
+
+	desc->tfm = sha256_hash;
+	desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
+	crypto_shash_init(desc);
+	for (i = 0; i < sizeof(pad); i++) {
+		if (i < keylen)
+			pad[i] = key[i];
+		else
+			pad[i] = 0;
+		pad[i] ^= HMAC_IPAD_VALUE;
+	}
+	crypto_shash_update(desc, pad, sizeof(pad));
+}
+
+static void hmac_final(struct shash_desc *desc, u8 *key, int keylen, u8 *out)
+{
+	u8 pad[SHA256_BLOCK_SIZE];
+	int i;
+
+	for (i = 0; i < sizeof(pad); i++) {
+		if (i < keylen)
+			pad[i] = key[i];
+		else
+			pad[i] = 0;
+		pad[i] ^= HMAC_OPAD_VALUE;
+	}
+
+	/* collect the final hash;  use out as temporary storage */
+	crypto_shash_final(desc, out);
+
+	/* reuse the desc */
+	crypto_shash_init(desc);
+	crypto_shash_update(desc, pad, sizeof(pad));
+	crypto_shash_update(desc, out, SHA256_DIGEST_SIZE);
+	crypto_shash_final(desc, out);
+}
+
+/*
+ * assume hash sha256 and nonces u, v of size SHA256_DIGEST_SIZE but
+ * otherwise standard KDFa.  Note output is in bytes not bits.
+ */
+static void KDFa(u8 *key, int keylen, const char *label, u8 *u,
+		 u8 *v, int bytes, u8 *out)
+{
+	u32 counter;
+	const __be32 bits = cpu_to_be32(bytes * 8);
+
+	for (counter = 1; bytes > 0; bytes -= SHA256_DIGEST_SIZE, counter++,
+		     out += SHA256_DIGEST_SIZE) {
+		SHASH_DESC_ON_STACK(desc, sha256_hash);
+		__be32 c = cpu_to_be32(counter);
+
+		hmac_init(desc, key, keylen);
+		crypto_shash_update(desc, (u8 *)&c, sizeof(c));
+		crypto_shash_update(desc, label, strlen(label)+1);
+		crypto_shash_update(desc, u, SHA256_DIGEST_SIZE);
+		crypto_shash_update(desc, v, SHA256_DIGEST_SIZE);
+		crypto_shash_update(desc, (u8 *)&bits, sizeof(bits));
+		hmac_final(desc, key, keylen, out);
+	}
+}
+
+/*
+ * Somewhat of a bastardization of the real KDFe.  We're assuming
+ * we're working with known point sizes for the input parameters and
+ * the hash algorithm is fixed at sha256.  Because we know that the
+ * point size is 32 bytes like the hash size, there's no need to loop
+ * in this KDF.
+ */
+static void KDFe(u8 z[EC_PT_SZ], const char *str, u8 *pt_u, u8 *pt_v,
+		 u8 *keyout)
+{
+	SHASH_DESC_ON_STACK(desc, sha256_hash);
+	/*
+	 * this should be an iterative counter, but because we know
+	 *  we're only taking 32 bytes for the point using a sha256
+	 *  hash which is also 32 bytes, there's only one loop
+	 */
+	__be32 c = cpu_to_be32(1);
+
+	desc->tfm = sha256_hash;
+	desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
+
+	crypto_shash_init(desc);
+	/* counter (BE) */
+	crypto_shash_update(desc, (u8 *)&c, sizeof(c));
+	/* secret value */
+	crypto_shash_update(desc, z, EC_PT_SZ);
+	/* string including trailing zero */
+	crypto_shash_update(desc, str, strlen(str)+1);
+	crypto_shash_update(desc, pt_u, EC_PT_SZ);
+	crypto_shash_update(desc, pt_v, EC_PT_SZ);
+	crypto_shash_final(desc, keyout);
+}
+
+static void tpm_buf_append_salt(struct tpm_buf *buf, struct tpm_chip *chip,
+				struct tpm2_auth *auth)
+{
+	struct crypto_kpp *kpp;
+	struct kpp_request *req;
+	struct scatterlist s[2], d[1];
+	struct ecdh p = {0};
+	u8 encoded_key[EC_PT_SZ], *x, *y;
+	unsigned int buf_len;
+	u8 *secret;
+
+	secret = kmalloc(EC_PT_SZ, GFP_KERNEL);
+	if (!secret)
+		return;
+
+	p.curve_id = ECC_CURVE_NIST_P256;
+
+	/* secret is two sized points */
+	tpm_buf_append_u16(buf, (EC_PT_SZ + 2)*2);
+	/*
+	 * we cheat here and append uninitialized data to form
+	 * the points.  All we care about is getting the two
+	 * co-ordinate pointers, which will be used to overwrite
+	 * the uninitialized data
+	 */
+	tpm_buf_append_u16(buf, EC_PT_SZ);
+	x = &buf->data[tpm_buf_length(buf)];
+	tpm_buf_append(buf, encoded_key, EC_PT_SZ);
+	tpm_buf_append_u16(buf, EC_PT_SZ);
+	y = &buf->data[tpm_buf_length(buf)];
+	tpm_buf_append(buf, encoded_key, EC_PT_SZ);
+	sg_init_table(s, 2);
+	sg_set_buf(&s[0], x, EC_PT_SZ);
+	sg_set_buf(&s[1], y, EC_PT_SZ);
+
+	kpp = crypto_alloc_kpp("ecdh", CRYPTO_ALG_INTERNAL, 0);
+	if (IS_ERR(kpp)) {
+		dev_err(&chip->dev, "crypto ecdh allocation failed\n");
+		return;
+	}
+
+	buf_len = crypto_ecdh_key_len(&p);
+	if (sizeof(encoded_key) < buf_len) {
+		dev_err(&chip->dev, "salt buffer too small needs %d\n",
+			buf_len);
+		goto out;
+	}
+	crypto_ecdh_encode_key(encoded_key, buf_len, &p);
+	/* this generates a random private key */
+	crypto_kpp_set_secret(kpp, encoded_key, buf_len);
+
+	/* salt is now the public point of this private key */
+	req = kpp_request_alloc(kpp, GFP_KERNEL);
+	if (!req)
+		goto out;
+	kpp_request_set_input(req, NULL, 0);
+	kpp_request_set_output(req, s, EC_PT_SZ*2);
+	crypto_kpp_generate_public_key(req);
+	/*
+	 * we're not done: now we have to compute the shared secret
+	 * which is our private key multiplied by the tpm_key public
+	 * point, we actually only take the x point and discard the y
+	 * point and feed it through KDFe to get the final secret salt
+	 */
+	sg_set_buf(&s[0], chip->ec_point_x, EC_PT_SZ);
+	sg_set_buf(&s[1], chip->ec_point_y, EC_PT_SZ);
+	kpp_request_set_input(req, s, EC_PT_SZ*2);
+	sg_init_one(d, secret, EC_PT_SZ);
+	kpp_request_set_output(req, d, EC_PT_SZ);
+	crypto_kpp_compute_shared_secret(req);
+	kpp_request_free(req);
+
+	/* pass the shared secret through KDFe for salt */
+	KDFe(secret, "SECRET", x, chip->ec_point_x, auth->salt);
+ out:
+	crypto_free_kpp(kpp);
+}
+
+/**
+ * tpm_buf_append_hmac_session() append a TPM session element
+ * @buf: The buffer to be appended
+ * @auth: the auth structure allocated by tpm2_start_auth_session()
+ * @attributes: The session attributes
+ * @passphrase: The session authority (NULL if none)
+ * @passphraselen: The length of the session authority (0 if none)
+ *
+ * This fills in a session structure in the TPM command buffer, except
+ * for the HMAC which cannot be computed until the command buffer is
+ * complete.  The type of session is controlled by the @attributes,
+ * the main ones of which are TPM2_SA_CONTINUE_SESSION which means the
+ * session won't terminate after tpm_buf_check_hmac_response(),
+ * TPM2_SA_DECRYPT which means this buffers first parameter should be
+ * encrypted with a session key and TPM2_SA_ENCRYPT, which means the
+ * response buffer's first parameter needs to be decrypted (confusing,
+ * but the defines are written from the point of view of the TPM).
+ *
+ * Any session appended by this command must be finalized by calling
+ * tpm_buf_fill_hmac_session() otherwise the HMAC will be incorrect
+ * and the TPM will reject the command.
+ *
+ * As with most tpm_buf operations, success is assumed because failure
+ * will be caused by an incorrect programming model and indicated by a
+ * kernel message.
+ */
+void tpm_buf_append_hmac_session(struct tpm_buf *buf, struct tpm2_auth *auth,
+				 u8 attributes, u8 *passphrase,
+				 int passphraselen)
+{
+	u8 nonce[SHA256_DIGEST_SIZE];
+	u32 len;
+
+	/*
+	 * The Architecture Guide requires us to strip trailing zeros
+	 * before computing the HMAC
+	 */
+	while (passphrase && passphraselen > 0
+	       && passphrase[passphraselen - 1] == '\0')
+		passphraselen--;
+
+	auth->attrs = attributes;
+	auth->passphraselen = passphraselen;
+	if (passphraselen)
+		memcpy(auth->passphrase, passphrase, passphraselen);
+
+	if (auth->session != tpm_buf_length(buf)) {
+		/* we're not the first session */
+		len = get_unaligned_be32(&buf->data[auth->session]);
+		if (4 + len + auth->session != tpm_buf_length(buf)) {
+			WARN(1, "session length mismatch, cannot append");
+			return;
+		}
+
+		/* add our new session */
+		len += 9 + 2 * SHA256_DIGEST_SIZE;
+		put_unaligned_be32(len, &buf->data[auth->session]);
+	} else {
+		tpm_buf_append_u32(buf, 9 + 2 * SHA256_DIGEST_SIZE);
+	}
+
+	/* random number for our nonce */
+	get_random_bytes(nonce, sizeof(nonce));
+	memcpy(auth->our_nonce, nonce, sizeof(nonce));
+	tpm_buf_append_u32(buf, auth->handle);
+	/* our new nonce */
+	tpm_buf_append_u16(buf, SHA256_DIGEST_SIZE);
+	tpm_buf_append(buf, nonce, SHA256_DIGEST_SIZE);
+	tpm_buf_append_u8(buf, auth->attrs);
+	/* and put a placeholder for the hmac */
+	tpm_buf_append_u16(buf, SHA256_DIGEST_SIZE);
+	tpm_buf_append(buf, nonce, SHA256_DIGEST_SIZE);
+}
+EXPORT_SYMBOL(tpm_buf_append_hmac_session);
+
+/**
+ * tpm_buf_fill_hmac_session() - finalize the session HMAC
+ * @buf: The buffer to be appended
+ * @auth: the auth structure allocated by tpm2_start_auth_session()
+ *
+ * This command must not be called until all of the parameters have
+ * been appended to @buf otherwise the computed HMAC will be
+ * incorrect.
+ *
+ * This function computes and fills in the session HMAC using the
+ * session key and, if TPM2_SA_DECRYPT was specified, computes the
+ * encryption key and encrypts the first parameter of the command
+ * buffer with it.
+ *
+ * As with most tpm_buf operations, success is assumed because failure
+ * will be caused by an incorrect programming model and indicated by a
+ * kernel message.
+ */
+void tpm_buf_fill_hmac_session(struct tpm_buf *buf, struct tpm2_auth *auth)
+{
+	u32 cc, handles, val;
+	struct tpm_chip *chip = auth->chip;
+	int i;
+	struct tpm_input_header *head = (struct tpm_input_header *)buf->data;
+	const u8 *s, *p;
+	u8 *hmac = NULL;
+	u32 attrs;
+	u8 cphash[SHA256_DIGEST_SIZE];
+	SHASH_DESC_ON_STACK(desc, sha256_hash);
+
+	/* save the command code in BE format */
+	auth->ordinal = head->ordinal;
+
+	desc->tfm = sha256_hash;
+	desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
+
+	cc = be32_to_cpu(head->ordinal);
+
+	i = tpm2_find_cc(chip, cc);
+	if (i < 0) {
+		dev_err(&chip->dev, "Command 0x%x not found in TPM\n", cc);
+		return;
+	}
+	attrs = chip->cc_attrs_tbl[i];
+
+	handles = (attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0);
+
+	s = &buf->data[TPM_HEADER_SIZE];
+	/*
+	 * just check the names, it's easy to make mistakes.  This
+	 * would happen if someone added a handle via
+	 * tpm_buf_append_u32() instead of tpm_buf_append_name()
+	 */
+	for (i = 0; i < handles; i++) {
+		u32 handle = tpm_get_inc_u32(&s);
+
+		if (auth->name_h[i] != handle) {
+			dev_err(&chip->dev, "TPM: handle %d wrong for name\n",
+				  i);
+			return;
+		}
+	}
+	/* point s to the start of the sessions */
+	val = tpm_get_inc_u32(&s);
+	/* point p to the start of the parameters */
+	p = s + val;
+	for (i = 1; s < p; i++) {
+		u32 handle = tpm_get_inc_u32(&s);
+		u16 len;
+		u8 a;
+
+		/* nonce (already in auth) */
+		len = tpm_get_inc_u16(&s);
+		s += len;
+
+		a = *s++;
+
+		len = tpm_get_inc_u16(&s);
+		if (handle == auth->handle && auth->attrs == a) {
+			hmac = (u8 *)s;
+			/*
+			 * save our session number so we know which
+			 * session in the response belongs to us
+			 */
+			auth->session = i;
+		}
+
+		s += len;
+	}
+	if (s != p) {
+		dev_err(&chip->dev, "TPM session length is incorrect\n");
+		return;
+	}
+	if (!hmac) {
+		dev_err(&chip->dev, "TPM could not find HMAC session\n");
+		return;
+	}
+
+	/* encrypt before HMAC */
+	if (auth->attrs & TPM2_SA_DECRYPT) {
+		struct scatterlist sg[1];
+		u16 len;
+		SKCIPHER_REQUEST_ON_STACK(req, auth->aes);
+
+		skcipher_request_set_tfm(req, auth->aes);
+		skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP,
+					      NULL, NULL);
+
+		/* need key and IV */
+		KDFa(auth->session_key, SHA256_DIGEST_SIZE
+		     + auth->passphraselen, "CFB", auth->our_nonce,
+		     auth->tpm_nonce, AES_KEYBYTES + AES_BLOCK_SIZE,
+		     auth->scratch);
+		crypto_skcipher_setkey(auth->aes, auth->scratch, AES_KEYBYTES);
+		len = tpm_get_inc_u16(&p);
+		sg_init_one(sg, p, len);
+		skcipher_request_set_crypt(req, sg, sg, len,
+					   auth->scratch + AES_KEYBYTES);
+		crypto_skcipher_encrypt(req);
+		/* reset p to beginning of parameters for HMAC */
+		p -= 2;
+	}
+
+	crypto_shash_init(desc);
+	/* ordinal is already BE */
+	crypto_shash_update(desc, (u8 *)&head->ordinal, sizeof(head->ordinal));
+	/* add the handle names */
+	for (i = 0; i < handles; i++) {
+		u8 mso = auth->name_h[i] >> 24;
+
+		if (mso == 0x81 || mso == 0x80 || mso == 0x01) {
+			crypto_shash_update(desc, auth->name[i],
+					    SHA256_DIGEST_SIZE + 2);
+		} else {
+			__be32 h = cpu_to_be32(auth->name_h[i]);
+
+			crypto_shash_update(desc, (u8 *)&h, 4);
+		}
+	}
+	if (buf->data - s != tpm_buf_length(buf))
+		crypto_shash_update(desc, s, buf->data
+				    + tpm_buf_length(buf) - s);
+	crypto_shash_final(desc, cphash);
+
+	/* now calculate the hmac */
+	hmac_init(desc, auth->session_key, sizeof(auth->session_key)
+		  + auth->passphraselen);
+	crypto_shash_update(desc, cphash, sizeof(cphash));
+	crypto_shash_update(desc, auth->our_nonce, sizeof(auth->our_nonce));
+	crypto_shash_update(desc, auth->tpm_nonce, sizeof(auth->tpm_nonce));
+	crypto_shash_update(desc, &auth->attrs, 1);
+	hmac_final(desc, auth->session_key, sizeof(auth->session_key)
+		   + auth->passphraselen, hmac);
+}
+EXPORT_SYMBOL(tpm_buf_fill_hmac_session);
+
+static int parse_read_public(char *name, const u8 *data)
+{
+	struct tpm_output_header *head = (struct tpm_output_header *)data;
+	u32 tot_len = be32_to_cpu(head->length);
+	u32 val;
+
+	data += TPM_HEADER_SIZE;
+	/* we're starting after the header so adjust the length */
+	tot_len -= TPM_HEADER_SIZE;
+
+	/* skip public */
+	val = tpm_get_inc_u16(&data);
+	if (val > tot_len)
+		return -EINVAL;
+	data += val;
+	/* name */
+	val = tpm_get_inc_u16(&data);
+	if (val != SHA256_DIGEST_SIZE + 2)
+		return -EINVAL;
+	memcpy(name, data, SHA256_DIGEST_SIZE + 2);
+	/* forget the rest */
+	return 0;
+}
+
+static int tpm2_readpublic(struct tpm_chip *chip, u32 handle, char *name)
+{
+	struct tpm_buf buf;
+	int rc;
+
+	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_READ_PUBLIC);
+	if (rc)
+		return rc;
+
+	tpm_buf_append_u32(&buf, handle);
+	rc = tpm_transmit_cmd(chip, &chip->kernel_space, buf.data,
+			      PAGE_SIZE, 0, 0, "read public");
+	if (rc == TPM2_RC_SUCCESS)
+		rc = parse_read_public(name, buf.data);
+
+	tpm_buf_destroy(&buf);
+
+	return rc;
+}
+
+/**
+ * tpm_buf_append_name() - add a handle area to the buffer
+ * @buf: The buffer to be appended
+ * @auth: the auth structure allocated by tpm2_start_auth_session()
+ * @handle: The handle to be appended
+ * @name: The name of the handle (may be NULL)
+ *
+ * In order to compute session HMACs, we need to know the names of the
+ * objects pointed to by the handles.  For most objects, this is simly
+ * the actual 4 byte handle or an empty buf (in these cases @name
+ * should be NULL) but for volatile objects, permanent objects and NV
+ * areas, the name is defined as the hash (according to the name
+ * algorithm which should be set to sha256) of the public area to
+ * which the two byte algorithm id has been appended.  For these
+ * objects, the @name pointer should point to this.  If a name is
+ * required but @name is NULL, then TPM2_ReadPublic() will be called
+ * on the handle to obtain the name.
+ *
+ * As with most tpm_buf operations, success is assumed because failure
+ * will be caused by an incorrect programming model and indicated by a
+ * kernel message.
+ */
+void tpm_buf_append_name(struct tpm_buf *buf, struct tpm2_auth *auth,
+			 u32 handle, u8 *name)
+{
+	int slot;
+	u8 mso = handle >> 24;
+
+	slot = (tpm_buf_length(buf) - TPM_HEADER_SIZE)/4;
+	if (slot >= AUTH_MAX_NAMES) {
+		dev_err(&auth->chip->dev, "TPM: too many handles\n");
+		return;
+	}
+	WARN(auth->session != tpm_buf_length(buf),
+	     "name added in wrong place\n");
+	tpm_buf_append_u32(buf, handle);
+	auth->session += 4;
+
+	if (mso == 0x81 || mso == 0x80 || mso == 0x01) {
+		if (!name)
+			tpm2_readpublic(auth->chip, handle, auth->name[slot]);
+	} else {
+		if (name)
+			dev_err(&auth->chip->dev, "TPM: Handle does not require name but one is specified\n");
+	}
+
+	auth->name_h[slot] = handle;
+	if (name)
+		memcpy(auth->name[slot], name, SHA256_DIGEST_SIZE + 2);
+}
+EXPORT_SYMBOL(tpm_buf_append_name);
+
+/**
+ * tpm_buf_check_hmac_response() - check the TPM return HMAC for correctness
+ * @buf: the original command buffer (which now contains the response)
+ * @auth: the auth structure allocated by tpm2_start_auth_session()
+ * @rc: the return code from tpm_transmit_cmd
+ *
+ * If @rc is non zero, @buf may not contain an actual return, so @rc
+ * is passed through as the return and the session cleaned up and
+ * de-allocated if required (this is required if
+ * TPM2_SA_CONTINUE_SESSION was not specified as a session flag).
+ *
+ * If @rc is zero, the response HMAC is computed against the returned
+ * @buf and matched to the TPM one in the session area.  If there is a
+ * mismatch, an error is logged and -EINVAL returned.
+ *
+ * The reason for this is that the command issue and HMAC check
+ * sequence should look like:
+ *
+ *	rc = tpm_transmit_cmd(...);
+ *	rc = tpm_buf_check_hmac_response(&buf, auth, rc);
+ *	if (rc)
+ *		...
+ *
+ * Which is easily layered into the current contrl flow.
+ *
+ * Returns: 0 on success or an error.
+ */
+int tpm_buf_check_hmac_response(struct tpm_buf *buf, struct tpm2_auth *auth,
+				int rc)
+{
+	struct tpm_output_header *head = (struct tpm_output_header *)buf->data;
+	struct tpm_chip *chip = auth->chip;
+	const u8 *s, *p;
+	u8 rphash[SHA256_DIGEST_SIZE];
+	u32 attrs;
+	SHASH_DESC_ON_STACK(desc, sha256_hash);
+	u16 tag = be16_to_cpu(head->tag);
+	u32 cc = be32_to_cpu(auth->ordinal);
+	int parm_len, len, i, handles;
+
+	if (auth->session >= TPM_HEADER_SIZE) {
+		WARN(1, "tpm session not filled correctly\n");
+		goto out;
+	}
+
+	if (rc != 0)
+		/* pass non success rc through and close the session */
+		goto out;
+
+	rc = -EINVAL;
+	if (tag != TPM2_ST_SESSIONS) {
+		dev_err(&chip->dev, "TPM: HMAC response check has no sessions tag\n");
+		goto out;
+	}
+
+	i = tpm2_find_cc(chip, cc);
+	if (i < 0)
+		goto out;
+	attrs = chip->cc_attrs_tbl[i];
+	handles = (attrs >> TPM2_CC_ATTR_RHANDLE) & 1;
+
+	/* point to area beyond handles */
+	s = &buf->data[TPM_HEADER_SIZE + handles * 4];
+	parm_len = tpm_get_inc_u32(&s);
+	p = s;
+	s += parm_len;
+	/* skip over any sessions before ours */
+	for (i = 0; i < auth->session - 1; i++) {
+		len = tpm_get_inc_u16(&s);
+		s += len + 1;
+		len = tpm_get_inc_u16(&s);
+		s += len;
+	}
+	/* TPM nonce */
+	len = tpm_get_inc_u16(&s);
+	if (s - buf->data + len > tpm_buf_length(buf))
+		goto out;
+	if (len != SHA256_DIGEST_SIZE)
+		goto out;
+	memcpy(auth->tpm_nonce, s, len);
+	s += len;
+	attrs = *s++;
+	len = tpm_get_inc_u16(&s);
+	if (s - buf->data + len != tpm_buf_length(buf))
+		goto out;
+	if (len != SHA256_DIGEST_SIZE)
+		goto out;
+	/*
+	 * s points to the HMAC. now calculate comparison, beginning
+	 * with rphash
+	 */
+	desc->tfm = sha256_hash;
+	desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
+	crypto_shash_init(desc);
+	/* yes, I know this is now zero, but it's what the standard says */
+	crypto_shash_update(desc, (u8 *)&head->return_code,
+			    sizeof(head->return_code));
+	/* ordinal is already BE */
+	crypto_shash_update(desc, (u8 *)&auth->ordinal, sizeof(auth->ordinal));
+	crypto_shash_update(desc, p, parm_len);
+	crypto_shash_final(desc, rphash);
+
+	/* now calculate the hmac */
+	hmac_init(desc, auth->session_key, sizeof(auth->session_key)
+		  + auth->passphraselen);
+	crypto_shash_update(desc, rphash, sizeof(rphash));
+	crypto_shash_update(desc, auth->tpm_nonce, sizeof(auth->tpm_nonce));
+	crypto_shash_update(desc, auth->our_nonce, sizeof(auth->our_nonce));
+	crypto_shash_update(desc, &auth->attrs, 1);
+	/* we're done with the rphash, so put our idea of the hmac there */
+	hmac_final(desc, auth->session_key, sizeof(auth->session_key)
+		   + auth->passphraselen, rphash);
+	if (memcmp(rphash, s, SHA256_DIGEST_SIZE) == 0) {
+		rc = 0;
+	} else {
+		dev_err(&auth->chip->dev, "TPM: HMAC check failed\n");
+		goto out;
+	}
+
+	/* now do response decryption */
+	if (auth->attrs & TPM2_SA_ENCRYPT) {
+		struct scatterlist sg[1];
+		SKCIPHER_REQUEST_ON_STACK(req, auth->aes);
+
+		skcipher_request_set_tfm(req, auth->aes);
+		skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP,
+					      NULL, NULL);
+
+		/* need key and IV */
+		KDFa(auth->session_key, SHA256_DIGEST_SIZE
+		     + auth->passphraselen, "CFB", auth->tpm_nonce,
+		     auth->our_nonce, AES_KEYBYTES + AES_BLOCK_SIZE,
+		     auth->scratch);
+		crypto_skcipher_setkey(auth->aes, auth->scratch, AES_KEYBYTES);
+		len = tpm_get_inc_u16(&p);
+		sg_init_one(sg, p, len);
+		skcipher_request_set_crypt(req, sg, sg, len,
+					   auth->scratch + AES_KEYBYTES);
+		crypto_skcipher_decrypt(req);
+	}
+
+ out:
+	if ((auth->attrs & TPM2_SA_CONTINUE_SESSION) == 0) {
+		/* manually close the session if it wasn't consumed */
+		if (rc)
+			tpm2_flush_context_cmd(chip, auth->handle, 0);
+		crypto_free_skcipher(auth->aes);
+		kfree(auth);
+	} else {
+		/* reset for next use  */
+		auth->session = TPM_HEADER_SIZE;
+	}
+
+	return rc;
+}
+EXPORT_SYMBOL(tpm_buf_check_hmac_response);
+
+/**
+ * tpm2_end_auth_session - kill the allocated auth session
+ * @auth: the auth structure allocated by tpm2_start_auth_session()
+ *
+ * ends the session started by tpm2_start_auth_session and frees all
+ * the resources.  Under normal conditions,
+ * tpm_buf_check_hmac_response() will correctly end the session if
+ * required, so this function is only for use in error legs that will
+ * bypass the normal invocation of tpm_buf_check_hmac_respons().
+ */
+void tpm2_end_auth_session(struct tpm2_auth *auth)
+{
+	tpm2_flush_context_cmd(auth->chip, auth->handle, 0);
+	crypto_free_skcipher(auth->aes);
+	kfree(auth);
+}
+EXPORT_SYMBOL(tpm2_end_auth_session);
+
+static int parse_start_auth_session(struct tpm2_auth *auth, const u8 *data)
+{
+	struct tpm_output_header *head = (struct tpm_output_header *)data;
+	u32 tot_len = be32_to_cpu(head->length);
+	u32 val;
+
+	data += TPM_HEADER_SIZE;
+	/* we're starting after the header so adjust the length */
+	tot_len -= TPM_HEADER_SIZE;
+
+	/* should have handle plus nonce */
+	if (tot_len != 4 + 2 + sizeof(auth->tpm_nonce))
+		return -EINVAL;
+
+	auth->handle = tpm_get_inc_u32(&data);
+	val = tpm_get_inc_u16(&data);
+	if (val != sizeof(auth->tpm_nonce))
+		return -EINVAL;
+	memcpy(auth->tpm_nonce, data, sizeof(auth->tpm_nonce));
+	/* now compute the session key from the nonces */
+	KDFa(auth->salt, sizeof(auth->salt), "ATH", auth->tpm_nonce,
+	     auth->our_nonce, sizeof(auth->session_key), auth->session_key);
+
+	return 0;
+}
+
+/**
+ * tpm2_start_auth_session - create a HMAC authentication session with the TPM
+ * @chip: the TPM chip structure to create the session with
+ * @authp: A pointer to an opaque tpm2_auth structure to be allocated
+ *
+ * This function contacts the TPM via the kernel space for an
+ * authentication session, allocates a tpm2_auth structure to contain
+ * all the session details necessary for performing the HMAC, encrypt
+ * and decrypt operations, fills it in and returns.
+ *
+ * Return: zero on success or actual error encountered.  If return is
+ * zero, @authp will be allocated.
+ */
+int tpm2_start_auth_session(struct tpm_chip *chip, struct tpm2_auth **authp)
+{
+	struct tpm_buf buf;
+	struct tpm2_auth *auth;
+	int rc;
+
+	auth = kmalloc(sizeof(**authp), GFP_KERNEL);
+	if (!auth)
+		return -ENOMEM;
+
+	auth->chip = chip;
+	auth->session = TPM_HEADER_SIZE;
+
+	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_START_AUTH_SESS);
+	if (rc)
+		goto out;
+
+	/* salt key handle */
+	tpm_buf_append_u32(&buf, chip->tpmkey);
+	/* bind key handle */
+	tpm_buf_append_u32(&buf, TPM2_RH_NULL);
+	/* nonce caller */
+	get_random_bytes(auth->our_nonce, sizeof(auth->our_nonce));
+	tpm_buf_append_u16(&buf, sizeof(auth->our_nonce));
+	tpm_buf_append(&buf, auth->our_nonce, sizeof(auth->our_nonce));
+
+	/* append encrypted salt and squirrel away unencrypted in auth */
+	tpm_buf_append_salt(&buf, chip, auth);
+	/* session type (HMAC, audit or policy) */
+	tpm_buf_append_u8(&buf, TPM2_SE_HMAC);
+
+	/* symmetric encryption parameters */
+	/* symmetric algorithm */
+	tpm_buf_append_u16(&buf, TPM2_ALG_AES);
+	/* bits for symmetric algorithm */
+	tpm_buf_append_u16(&buf, AES_KEYBITS);
+	/* symmetric algorithm mode (must be CFB) */
+	tpm_buf_append_u16(&buf, TPM2_ALG_CFB);
+	/* hash algorithm for session */
+	tpm_buf_append_u16(&buf, TPM2_ALG_SHA256);
+
+	rc = tpm_transmit_cmd(chip, &chip->kernel_space, buf.data, PAGE_SIZE,
+			      0, 0, "start auth session");
+	if (rc == TPM2_RC_SUCCESS)
+		rc = parse_start_auth_session(auth, buf.data);
+
+	tpm_buf_destroy(&buf);
+
+	if (rc)
+		goto out;
+
+	auth->aes = crypto_alloc_skcipher("cfb(aes)", 0, 0);
+	if (IS_ERR(auth->aes)) {
+		rc = PTR_ERR(auth->aes);
+		dev_err(&chip->dev, "TPM: error getting cfb(aes): %d\n", rc);
+	}
+ out:
+	if (rc)
+		kfree(auth);
+	else
+		*authp = auth;
+
+	return rc;
+}
+EXPORT_SYMBOL(tpm2_start_auth_session);
+
+static int parse_create_primary(struct tpm_chip *chip, u8 *data)
+{
+	struct tpm_output_header *head = (struct tpm_output_header *)data;
+	u16 len;
+	u32 tot_len = be32_to_cpu(head->length);
+	u32 handle, val, parm_len;
+	const u8 *resp, *tmp;
+	SHASH_DESC_ON_STACK(desc, sha256_hash);
+
+	data += TPM_HEADER_SIZE;
+	/* we're starting after the header so adjust the length */
+	tot_len -= TPM_HEADER_SIZE;
+
+	resp = data;
+	handle = tpm_get_inc_u32(&resp);
+	parm_len = tpm_get_inc_u32(&resp);
+	if (parm_len + 8 > tot_len)
+		return -EINVAL;
+	len = tpm_get_inc_u16(&resp);
+	tmp = resp;
+	/* now we have the public area, compute the name of the object */
+	desc->tfm = sha256_hash;
+	desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
+	put_unaligned_be16(TPM2_ALG_SHA256, chip->tpmkeyname);
+	crypto_shash_init(desc);
+	crypto_shash_update(desc, resp, len);
+	crypto_shash_final(desc, chip->tpmkeyname + 2);
+	/* validate the public key */
+	val = tpm_get_inc_u16(&tmp);
+	/* key type (must be what we asked for) */
+	if (val != TPM2_ALG_ECC)
+		return -EINVAL;
+	val = tpm_get_inc_u16(&tmp);
+	/* name algorithm */
+	if (val != TPM2_ALG_SHA256)
+		return -EINVAL;
+	val = tpm_get_inc_u32(&tmp);
+	/* object properties */
+	if (val != (TPM2_OA_NO_DA |
+		    TPM2_OA_FIXED_TPM |
+		    TPM2_OA_FIXED_PARENT |
+		    TPM2_OA_SENSITIVE_DATA_ORIGIN |
+		    TPM2_OA_USER_WITH_AUTH |
+		    TPM2_OA_DECRYPT |
+		    TPM2_OA_RESTRICTED))
+		return -EINVAL;
+	/* auth policy (empty) */
+	val = tpm_get_inc_u16(&tmp);
+	if (val != 0)
+		return -EINVAL;
+	val = tpm_get_inc_u16(&tmp);
+	/* symmetric key parameters */
+	if (val != TPM2_ALG_AES)
+		return -EINVAL;
+	val = tpm_get_inc_u16(&tmp);
+	/* symmetric key length */
+	if (val != AES_KEYBITS)
+		return -EINVAL;
+	val = tpm_get_inc_u16(&tmp);
+	/* symmetric encryption scheme */
+	if (val != TPM2_ALG_CFB)
+		return -EINVAL;
+	val = tpm_get_inc_u16(&tmp);
+	/* signing scheme */
+	if (val != TPM2_ALG_NULL)
+		return -EINVAL;
+	val = tpm_get_inc_u16(&tmp);
+	/* ECC Curve */
+	if (val != TPM2_ECC_NIST_P256)
+		return -EINVAL;
+	val = tpm_get_inc_u16(&tmp);
+	/* KDF Scheme */
+	if (val != TPM2_ALG_NULL)
+		return -EINVAL;
+	val = tpm_get_inc_u16(&tmp);
+	/* x point */
+	if (val != 32)
+		return -EINVAL;
+	memcpy(chip->ec_point_x, tmp, val);
+	tmp += val;
+	val = tpm_get_inc_u16(&tmp);
+	if (val != 32)
+		return -EINVAL;
+	memcpy(chip->ec_point_y, tmp, val);
+	tmp += val;
+	resp += len;
+	/* should have exactly consumed the tpm2b public structure */
+	if (tmp != resp)
+		return -EINVAL;
+	if (resp - data > parm_len)
+		return -EINVAL;
+	/* creation data (skip) */
+	len = tpm_get_inc_u16(&resp);
+	resp += len;
+	if (resp - data > parm_len)
+		return -EINVAL;
+	/* creation digest (must be sha256) */
+	len = tpm_get_inc_u16(&resp);
+	resp += len;
+	if (len != SHA256_DIGEST_SIZE || resp - data > parm_len)
+		return -EINVAL;
+	/* TPMT_TK_CREATION follows */
+	/* tag, must be TPM_ST_CREATION (0x8021) */
+	val = tpm_get_inc_u16(&resp);
+	if (val != TPM2_ST_CREATION || resp - data > parm_len)
+		return -EINVAL;
+	/* hierarchy (must be NULL) */
+	val = tpm_get_inc_u32(&resp);
+	if (val != TPM2_RH_NULL || resp - data > parm_len)
+		return -EINVAL;
+	/* the ticket digest HMAC (might not be sha256) */
+	len = tpm_get_inc_u16(&resp);
+	resp += len;
+	if (resp - data > parm_len)
+		return -EINVAL;
+	/*
+	 * finally we have the name, which is a sha256 digest plus a 2
+	 * byte algorithm type
+	 */
+	len = tpm_get_inc_u16(&resp);
+	if (resp + len - data != parm_len + 8)
+		return -EINVAL;
+	if (len != SHA256_DIGEST_SIZE + 2)
+		return -EINVAL;
+
+	if (memcmp(chip->tpmkeyname, resp, SHA256_DIGEST_SIZE + 2) != 0) {
+		printk("TPM NULL Seed name comparison failed\n");
+		return -EINVAL;
+	}
+
+	chip->tpmkey = handle;
+
+	return 0;
+}
+
+int tpm2_create_null_primary(struct tpm_chip *chip)
+{
+	int rc;
+	struct tpm_buf buf;
+	struct tpm_buf template;
+
+	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE_PRIMARY);
+	if (rc)
+		return rc;
+
+	rc = tpm_buf_init_2b(&template);
+	if (rc) {
+		tpm_buf_destroy(&buf);
+		return rc;
+	}
+
+	/*
+	 * create the template.  Note: in order for userspace to
+	 * verify the security of the system, it will have to create
+	 * and certify this NULL primary, meaning all the template
+	 * parameters will have to be identical, so conform exactly to
+	 * the TCG TPM v2.0 Provisioning Guidance for the SRK ECC
+	 * key
+	 */
+
+	/* key type */
+	tpm_buf_append_u16(&template, TPM2_ALG_ECC);
+	/* name algorithm */
+	tpm_buf_append_u16(&template, TPM2_ALG_SHA256);
+	/* object properties */
+	tpm_buf_append_u32(&template, TPM2_OA_NO_DA |
+			   TPM2_OA_FIXED_TPM |
+			   TPM2_OA_FIXED_PARENT |
+			   TPM2_OA_SENSITIVE_DATA_ORIGIN |
+			   TPM2_OA_USER_WITH_AUTH |
+			   TPM2_OA_DECRYPT |
+			   TPM2_OA_RESTRICTED);
+	/* sauth policy (empty) */
+	tpm_buf_append_u16(&template, 0);
+
+	/* BEGIN parameters: key specific; for ECC*/
+	/* symmetric algorithm */
+	tpm_buf_append_u16(&template, TPM2_ALG_AES);
+	/* bits for symmetric algorithm */
+	tpm_buf_append_u16(&template, 128);
+	/* algorithm mode (must be CFB) */
+	tpm_buf_append_u16(&template, TPM2_ALG_CFB);
+	/* scheme (NULL means any scheme) */
+	tpm_buf_append_u16(&template, TPM2_ALG_NULL);
+	/* ECC Curve ID */
+	tpm_buf_append_u16(&template, TPM2_ECC_NIST_P256);
+	/* KDF Scheme */
+	tpm_buf_append_u16(&template, TPM2_ALG_NULL);
+	/* unique: key specific; for ECC it is two points */
+	tpm_buf_append_u16(&template, 0);
+	tpm_buf_append_u16(&template, 0);
+	/* END parameters */
+
+	/* primary handle */
+	tpm_buf_append_u32(&buf, TPM2_RH_NULL);
+	/* simple authorization for empty auth */
+	tpm2_buf_append_auth(&buf, TPM2_RS_PW, NULL, 0, 0, NULL, 0);
+	/* sensitive create size is 4 for two empty buffers */
+	tpm_buf_append_u16(&buf, 4);
+	/* sensitive create auth data (empty) */
+	tpm_buf_append_u16(&buf, 0);
+	/* sensitive create sensitive data (empty) */
+	tpm_buf_append_u16(&buf, 0);
+	/* the public template */
+	tpm_buf_append_2b(&buf, &template);
+	tpm_buf_destroy(&template);
+	/* outside info (empty) */
+	tpm_buf_append_u16(&buf, 0);
+	/* creation PCR (none) */
+	tpm_buf_append_u32(&buf, 0);
+
+	rc = tpm_transmit_cmd(chip, &chip->kernel_space, buf.data, PAGE_SIZE,
+			      0, 0, "attempting to create NULL primary");
+
+	if (rc == TPM2_RC_SUCCESS)
+		rc = parse_create_primary(chip, buf.data);
+
+	tpm_buf_destroy(&buf);
+
+	return rc;
+}
+
+int tpm2_sessions_init(struct tpm_chip *chip)
+{
+	int rc;
+
+	rc = tpm2_init_space(&chip->kernel_space);
+	if (rc) {
+		dev_err(&chip->dev, "TPM: failed to initialize kernel space\n");
+		return -ENOMEM;
+	}
+	sha256_hash = crypto_alloc_shash("sha256", 0, 0);
+	if (!sha256_hash) {
+		dev_err(&chip->dev, "TPM: failed to allocate hash\n");
+		return -ENOMEM;
+	}
+
+	rc = tpm2_create_null_primary(chip);
+	if (rc)
+		dev_err(&chip->dev, "TPM: security failed (NULL seed derivation): %d\n", rc);
+	return rc;
+}
+EXPORT_SYMBOL(tpm2_sessions_init);
diff --git a/drivers/char/tpm/tpm2-sessions.h b/drivers/char/tpm/tpm2-sessions.h
new file mode 100644
index 000000000000..cea13f411de7
--- /dev/null
+++ b/drivers/char/tpm/tpm2-sessions.h
@@ -0,0 +1,57 @@
+/*
+ * Defines for TPM2 authentications
+ */
+
+#ifndef _TPM2_SESSIONS_H
+#define _TPM2_SESSIONS_H
+
+#include "tpm.h"
+
+enum tpm2_object_attributes {
+	TPM2_OA_FIXED_TPM		= BIT(1),
+	TPM2_OA_ST_CLEAR		= BIT(2),
+	TPM2_OA_FIXED_PARENT		= BIT(4),
+	TPM2_OA_SENSITIVE_DATA_ORIGIN	= BIT(5),
+	TPM2_OA_USER_WITH_AUTH		= BIT(6),
+	TPM2_OA_ADMIN_WITH_POLICY	= BIT(7),
+	TPM2_OA_NO_DA			= BIT(10),
+	TPM2_OA_ENCRYPTED_DUPLICATION	= BIT(11),
+	TPM2_OA_RESTRICTED		= BIT(16),
+	TPM2_OA_DECRYPT			= BIT(17),
+	TPM2_OA_SIGN			= BIT(18),
+};
+
+enum tpm2_session_attributes {
+	TPM2_SA_CONTINUE_SESSION	= BIT(0),
+	TPM2_SA_AUDIT_EXCLUSIVE		= BIT(1),
+	TPM2_SA_AUDIT_RESET		= BIT(3),
+	TPM2_SA_DECRYPT			= BIT(5),
+	TPM2_SA_ENCRYPT			= BIT(6),
+	TPM2_SA_AUDIT			= BIT(7),
+};
+
+enum tpm2_session_types {
+	TPM2_SE_HMAC	= 0x00,
+	TPM2_SE_POLICY	= 0x01,
+	TPM2_SE_TRIAL	= 0x02,
+};
+
+struct tpm2_auth;
+
+void tpm2_buf_append_auth(struct tpm_buf *buf, u32 session_handle,
+			  const u8 *nonce, u16 nonce_len,
+			  u8 attributes,
+			  const u8 *hmac, u16 hmac_len);
+
+int tpm2_start_auth_session(struct tpm_chip *chip, struct tpm2_auth **authp);
+void tpm_buf_append_name(struct tpm_buf *buf, struct tpm2_auth *auth,
+			 u32 handle, u8 *name);
+void tpm_buf_append_hmac_session(struct tpm_buf *buf, struct tpm2_auth *auth,
+				 u8 attributes, u8 *passphrase,
+				 int passphraselen);
+void tpm_buf_fill_hmac_session(struct tpm_buf *buf, struct tpm2_auth *auth);
+int tpm_buf_check_hmac_response(struct tpm_buf *buf, struct tpm2_auth *auth,
+				int rc);
+void tpm2_end_auth_session(struct tpm2_auth *auth);
+
+#endif
-- 
2.16.4

[PATCH v4 3/7] tpm2: add hmac checks to tpm2_pcr_extend()

[James Bottomley]

We use tpm2_pcr_extend() in trusted keys to extend a PCR to prevent a
key from being re-loaded until the next reboot.  To use this
functionality securely, that extend must be protected by a session
hmac.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>

---

v3: add error handling to sessions
---
 drivers/char/tpm/tpm2-cmd.c | 33 +++++++++++++--------------------
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index a17e5c573c4e..332b34b347f1 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -209,13 +209,6 @@ int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
 	return rc;
 }
 
-struct tpm2_null_auth_area {
-	__be32  handle;
-	__be16  nonce_size;
-	u8  attributes;
-	__be16  auth_size;
-} __packed;
-
 /**
  * tpm2_pcr_extend() - extend a PCR value
  *
@@ -230,7 +223,7 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
 		    struct tpm2_digest *digests)
 {
 	struct tpm_buf buf;
-	struct tpm2_null_auth_area auth_area;
+	struct tpm2_auth *auth;
 	int rc;
 	int i;
 	int j;
@@ -238,20 +231,19 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
 	if (count > ARRAY_SIZE(chip->active_banks))
 		return -EINVAL;
 
-	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
+	rc = tpm2_start_auth_session(chip, &auth);
 	if (rc)
 		return rc;
 
-	tpm_buf_append_u32(&buf, pcr_idx);
+	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
+	if (rc) {
+		tpm2_end_auth_session(auth);
+		return rc;
+	}
 
-	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_name(&buf, auth, pcr_idx, NULL);
+	tpm_buf_append_hmac_session(&buf, auth, 0, NULL, 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++) {
@@ -264,9 +256,10 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
 			       hash_digest_size[tpm2_hash_map[j].crypto_id]);
 		}
 	}
-
-	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
-			      "attempting extend a PCR value");
+	tpm_buf_fill_hmac_session(&buf, auth);
+	rc = tpm_transmit_cmd(chip, &chip->kernel_space, buf.data, PAGE_SIZE,
+			      0, 0, "attempting extend a PCR value");
+	rc = tpm_buf_check_hmac_response(&buf, auth, rc);
 
 	tpm_buf_destroy(&buf);
 
-- 
2.16.4

[PATCH v4 4/7] tpm2: add session encryption protection to tpm2_get_random()

[James Bottomley]

If some entity is snooping the TPM bus, they can see the random
numbers we're extracting from the TPM and do prediction attacks
against their consumers.  Foil this attack by using response
encryption to prevent the attacker from seeing the random sequence.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>

---

v3: add error handling to sessions and redo to be outside loop
---
 drivers/char/tpm/tpm2-cmd.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 332b34b347f1..22f1c7bee173 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -266,7 +266,6 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
 	return rc;
 }
 
-
 struct tpm2_get_random_out {
 	__be16 size;
 	u8 buffer[TPM_MAX_RNG_DATA];
@@ -293,21 +292,32 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
 	int total = 0;
 	int retries = 5;
 	u8 *dest_ptr = dest;
+	struct tpm2_auth *auth;
 
 	if (!num_bytes || max > TPM_MAX_RNG_DATA)
 		return -EINVAL;
 
-	err = tpm_buf_init(&buf, 0, 0);
+	err = tpm2_start_auth_session(chip, &auth);
 	if (err)
 		return err;
 
+	err = tpm_buf_init(&buf, 0, 0);
+	if (err) {
+		tpm2_end_auth_session(auth);
+		return err;
+	}
+
 	do {
-		tpm_buf_reset(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_RANDOM);
+		tpm_buf_reset(&buf, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM);
+		tpm_buf_append_hmac_session(&buf, auth, TPM2_SA_ENCRYPT
+					    | TPM2_SA_CONTINUE_SESSION,
+					    NULL, 0);
 		tpm_buf_append_u16(&buf, num_bytes);
-		err = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
-				       offsetof(struct tpm2_get_random_out,
-						buffer),
+		tpm_buf_fill_hmac_session(&buf, auth);
+		err = tpm_transmit_cmd(chip, &chip->kernel_space, buf.data,
+				       PAGE_SIZE, TPM_HEADER_SIZE + 2,
 				       0, "attempting get random");
+		err = tpm_buf_check_hmac_response(&buf, auth, err);
 		if (err)
 			goto out;
 
@@ -327,6 +337,8 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
 	} while (retries-- && total < max);
 
 	tpm_buf_destroy(&buf);
+	tpm2_end_auth_session(auth);
+
 	return total ? total : -EIO;
 out:
 	tpm_buf_destroy(&buf);
-- 
2.16.4

[PATCH v4 5/7] trusted keys: Add session encryption protection to the seal/unseal path

[James Bottomley]

If some entity is snooping the TPM bus, the can see the data going in
to be sealed and the data coming out as it is unsealed.  Add parameter
and response encryption to these cases to ensure that no secrets are
leaked even if the bus is snooped.

As part of doing this conversion it was discovered that policy
sessions can't work with HMAC protected authority because of missing
pieces (the tpm Nonce).  I've added code to work the same way as
before, which will result in potential authority exposure (while still
adding security for the command and the returned blob), and a fixme to
redo the API to get rid of this security hole.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 drivers/char/tpm/tpm2-cmd.c | 155 ++++++++++++++++++++++++++++----------------
 1 file changed, 98 insertions(+), 57 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 22f1c7bee173..a8655cd535d1 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -425,7 +425,9 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 {
 	unsigned int blob_len;
 	struct tpm_buf buf;
+	struct tpm_buf t2b;
 	u32 hash;
+	struct tpm2_auth *auth;
 	int i;
 	int rc;
 
@@ -439,45 +441,56 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 	if (i == ARRAY_SIZE(tpm2_hash_map))
 		return -EINVAL;
 
-	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
+	rc = tpm2_start_auth_session(chip, &auth);
 	if (rc)
 		return rc;
 
-	tpm_buf_append_u32(&buf, options->keyhandle);
-	tpm2_buf_append_auth(&buf, TPM2_RS_PW,
-			     NULL /* nonce */, 0,
-			     0 /* session_attributes */,
-			     options->keyauth /* hmac */,
-			     TPM_DIGEST_SIZE);
+	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
+	if (rc) {
+		tpm2_end_auth_session(auth);
+		return rc;
+	}
+
+	rc = tpm_buf_init_2b(&t2b);
+	if (rc) {
+		tpm_buf_destroy(&buf);
+		tpm2_end_auth_session(auth);
+		return rc;
+	}
 
+	tpm_buf_append_name(&buf, auth, options->keyhandle, NULL);
+	tpm_buf_append_hmac_session(&buf, auth, TPM2_SA_DECRYPT,
+				    options->keyauth, TPM_DIGEST_SIZE);
 	/* sensitive */
-	tpm_buf_append_u16(&buf, 4 + TPM_DIGEST_SIZE + payload->key_len + 1);
+	tpm_buf_append_u16(&t2b, TPM_DIGEST_SIZE);
+	tpm_buf_append(&t2b, options->blobauth, TPM_DIGEST_SIZE);
+	tpm_buf_append_u16(&t2b, payload->key_len + 1);
+	tpm_buf_append(&t2b, payload->key, payload->key_len);
+	tpm_buf_append_u8(&t2b, payload->migratable);
 
-	tpm_buf_append_u16(&buf, TPM_DIGEST_SIZE);
-	tpm_buf_append(&buf, options->blobauth, TPM_DIGEST_SIZE);
-	tpm_buf_append_u16(&buf, payload->key_len + 1);
-	tpm_buf_append(&buf, payload->key, payload->key_len);
-	tpm_buf_append_u8(&buf, payload->migratable);
+	tpm_buf_append_2b(&buf, &t2b);
 
 	/* public */
-	tpm_buf_append_u16(&buf, 14 + options->policydigest_len);
-	tpm_buf_append_u16(&buf, TPM2_ALG_KEYEDHASH);
-	tpm_buf_append_u16(&buf, hash);
+	tpm_buf_append_u16(&t2b, TPM2_ALG_KEYEDHASH);
+	tpm_buf_append_u16(&t2b, hash);
 
 	/* policy */
 	if (options->policydigest_len) {
-		tpm_buf_append_u32(&buf, 0);
-		tpm_buf_append_u16(&buf, options->policydigest_len);
-		tpm_buf_append(&buf, options->policydigest,
+		tpm_buf_append_u32(&t2b, 0);
+		tpm_buf_append_u16(&t2b, options->policydigest_len);
+		tpm_buf_append(&t2b, options->policydigest,
 			       options->policydigest_len);
 	} else {
-		tpm_buf_append_u32(&buf, TPM2_OA_USER_WITH_AUTH);
-		tpm_buf_append_u16(&buf, 0);
+		tpm_buf_append_u32(&t2b, TPM2_OA_USER_WITH_AUTH);
+		tpm_buf_append_u16(&t2b, 0);
 	}
 
 	/* public parameters */
-	tpm_buf_append_u16(&buf, TPM2_ALG_NULL);
-	tpm_buf_append_u16(&buf, 0);
+	tpm_buf_append_u16(&t2b, TPM2_ALG_NULL);
+	/* unique (zero) */
+	tpm_buf_append_u16(&t2b, 0);
+
+	tpm_buf_append_2b(&buf, &t2b);
 
 	/* outside info */
 	tpm_buf_append_u16(&buf, 0);
@@ -490,8 +503,11 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 		goto out;
 	}
 
-	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 4, 0,
-			      "sealing data");
+	tpm_buf_fill_hmac_session(&buf, auth);
+
+	rc = tpm_transmit_cmd(chip, &chip->kernel_space, buf.data,
+			      PAGE_SIZE, 4, 0, "sealing data");
+	rc = tpm_buf_check_hmac_response(&buf, auth, rc);
 	if (rc)
 		goto out;
 
@@ -509,6 +525,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 	payload->blob_len = blob_len;
 
 out:
+	tpm_buf_destroy(&t2b);
 	tpm_buf_destroy(&buf);
 
 	if (rc > 0) {
@@ -528,7 +545,6 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
  * @payload: the key data in clear and encrypted form
  * @options: authentication values and other options
  * @blob_handle: returned blob handle
- * @flags: tpm transmit flags
  *
  * Return: 0 on success.
  *        -E2BIG on wrong payload size.
@@ -538,9 +554,10 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 static int tpm2_load_cmd(struct tpm_chip *chip,
 			 struct trusted_key_payload *payload,
 			 struct trusted_key_options *options,
-			 u32 *blob_handle, unsigned int flags)
+			 u32 *blob_handle)
 {
 	struct tpm_buf buf;
+	struct tpm2_auth *auth;
 	unsigned int private_len;
 	unsigned int public_len;
 	unsigned int blob_len;
@@ -555,17 +572,18 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 	if (blob_len > payload->blob_len)
 		return -E2BIG;
 
-	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
+	rc = tpm2_start_auth_session(chip, &auth);
 	if (rc)
 		return rc;
 
-	tpm_buf_append_u32(&buf, options->keyhandle);
-	tpm2_buf_append_auth(&buf, TPM2_RS_PW,
-			     NULL /* nonce */, 0,
-			     0 /* session_attributes */,
-			     options->keyauth /* hmac */,
-			     TPM_DIGEST_SIZE);
+	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
+	if (rc) {
+		tpm2_end_auth_session(auth);
+		return rc;
+	}
 
+	tpm_buf_append_name(&buf, auth, options->keyhandle, NULL);
+	tpm_buf_append_hmac_session(&buf, auth, 0, options->keyauth, TPM_DIGEST_SIZE);
 	tpm_buf_append(&buf, payload->blob, blob_len);
 
 	if (buf.flags & TPM_BUF_OVERFLOW) {
@@ -573,8 +591,10 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 		goto out;
 	}
 
-	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 4, flags,
-			      "loading blob");
+	tpm_buf_fill_hmac_session(&buf, auth);
+	rc = tpm_transmit_cmd(chip, &chip->kernel_space, buf.data, PAGE_SIZE,
+			      4, 0, "loading blob");
+	rc = tpm_buf_check_hmac_response(&buf, auth, rc);
 	if (!rc)
 		*blob_handle = be32_to_cpup(
 			(__be32 *) &buf.data[TPM_HEADER_SIZE]);
@@ -595,7 +615,6 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
  * @payload: the key data in clear and encrypted form
  * @options: authentication values and other options
  * @blob_handle: blob handle
- * @flags: tpm_transmit_cmd flags
  *
  * Return: 0 on success
  *         -EPERM on tpm error status
@@ -604,28 +623,54 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 static int tpm2_unseal_cmd(struct tpm_chip *chip,
 			   struct trusted_key_payload *payload,
 			   struct trusted_key_options *options,
-			   u32 blob_handle, unsigned int flags)
+			   u32 blob_handle)
 {
 	struct tpm_buf buf;
+	struct tpm2_auth *auth;
 	u16 data_len;
 	u8 *data;
 	int rc;
 
-	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
+	rc = tpm2_start_auth_session(chip, &auth);
 	if (rc)
 		return rc;
 
-	tpm_buf_append_u32(&buf, blob_handle);
-	tpm2_buf_append_auth(&buf,
-			     options->policyhandle ?
-			     options->policyhandle : TPM2_RS_PW,
-			     NULL /* nonce */, 0,
-			     TPM2_SA_CONTINUE_SESSION,
-			     options->blobauth /* hmac */,
-			     TPM_DIGEST_SIZE);
-
-	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 6, flags,
-			      "unsealing");
+	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
+	if (rc) {
+		tpm2_end_auth_session(auth);
+		return rc;
+	}
+
+	tpm_buf_append_name(&buf, auth, blob_handle, NULL);
+
+	if (!options->policyhandle) {
+		tpm_buf_append_hmac_session(&buf, auth, TPM2_SA_ENCRYPT,
+					    options->blobauth, TPM_DIGEST_SIZE);
+	} else {
+		/*
+		 * FIXME: the policy can't be used for HMAC protection
+		 * of the authorization because it must be generated
+		 * with the initial nonces which isn't passed in, so
+		 * append a second encryption session to at least HMAC
+		 * protect the command and encrypt the sealed blob on
+		 * return so the only thing the attacker can get is
+		 * the password.
+		 *
+		 * We also consume the policy session otherwise it
+		 * would be absorbed into the kernel space.
+		 */
+		tpm2_buf_append_auth(&buf, options->policyhandle,
+				     NULL /* nonce */, 0, 0,
+				     options->blobauth /* hmac */,
+				     TPM_DIGEST_SIZE);
+		tpm_buf_append_hmac_session(&buf, auth, TPM2_SA_ENCRYPT,
+					    NULL, 0);
+	}
+
+	tpm_buf_fill_hmac_session(&buf, auth);
+	rc = tpm_transmit_cmd(chip, &chip->kernel_space, buf.data, PAGE_SIZE,
+			      6, 0, "unsealing");
+	rc = tpm_buf_check_hmac_response(&buf, auth, rc);
 	if (rc > 0)
 		rc = -EPERM;
 
@@ -669,17 +714,13 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
 	u32 blob_handle;
 	int rc;
 
-	mutex_lock(&chip->tpm_mutex);
-	rc = tpm2_load_cmd(chip, payload, options, &blob_handle,
-			   TPM_TRANSMIT_UNLOCKED);
+	rc = tpm2_load_cmd(chip, payload, options, &blob_handle);
 	if (rc)
 		goto out;
 
-	rc = tpm2_unseal_cmd(chip, payload, options, blob_handle,
-			     TPM_TRANSMIT_UNLOCKED);
-	tpm2_flush_context_cmd(chip, blob_handle, TPM_TRANSMIT_UNLOCKED);
+	rc = tpm2_unseal_cmd(chip, payload, options, blob_handle);
+	tpm2_flush_context_cmd(chip, blob_handle, 0);
 out:
-	mutex_unlock(&chip->tpm_mutex);
 	return rc;
 }
 
-- 
2.16.4

[PATCH v4 6/7] tpm: add the null key name as a tpm2 sysfs variable

[James Bottomley]

This is the last component of encrypted tpm2 session handling that
allows us to verify from userspace that the key derived from the NULL
seed genuinely belongs to the TPM and has not been spoofed.

The procedure for doing this involves creating an attestation identity
key (which requires verification of the TPM EK certificate) and then
using that AIK to sign a certification of the Elliptic Curve key over
the NULL seed.  Userspace must create this EC Key using the parameters
prescribed in TCG TPM v2.0 Provisioning Guidance for the SRK ECC; if
this is done correctly the names will match and the TPM can then run a
TPM2_Certify operation on this derived primary key using the newly
created AIK.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 drivers/char/tpm/tpm-sysfs.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 83a77a445538..7e901d9e893e 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -284,6 +284,19 @@ static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RO(timeouts);
 
+static ssize_t null_name_show(struct device *dev, struct device_attribute *attr,
+			      char *buf)
+{
+	struct tpm_chip *chip = to_tpm_chip(dev);
+	int size = TPM2_NAME_SIZE;
+
+	bin2hex(buf, chip->tpmkeyname, size);
+	size *= 2;
+	buf[size++] = '\n';
+	return size;
+}
+static DEVICE_ATTR_RO(null_name);
+
 static struct attribute *tpm_dev_attrs[] = {
 	&dev_attr_pubek.attr,
 	&dev_attr_pcrs.attr,
@@ -298,17 +311,29 @@ static struct attribute *tpm_dev_attrs[] = {
 	NULL,
 };
 
+static struct attribute *tpm2_dev_attrs[] = {
+	&dev_attr_null_name.attr,
+	NULL,
+};
+
 static const struct attribute_group tpm_dev_group = {
 	.attrs = tpm_dev_attrs,
 };
 
+static const struct attribute_group tpm2_dev_group = {
+	.attrs = tpm2_dev_attrs,
+};
+
 void tpm_sysfs_add_device(struct tpm_chip *chip)
 {
 	/* XXX: If you wish to remove this restriction, you must first update
 	 * tpm_sysfs to explicitly lock chip->ops.
 	 */
-	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+		WARN_ON(chip->groups_cnt != 0);
+		chip->groups[chip->groups_cnt++] = &tpm2_dev_group;
 		return;
+	}
 
 	/* The sysfs routines rely on an implicit tpm_try_get_ops, device_del
 	 * is called before ops is null'd and the sysfs core synchronizes this
-- 
2.16.4

[PATCH v4 7/7] tpm2-sessions: NOT FOR COMMITTING add sessions testing

[James Bottomley]

This runs through a preset sequence using sessions to demonstrate that
the session handling code functions.  It does both HMAC, encryption
and decryption by testing an encrypted sealing operation with
authority and proving that the same sealed data comes back again via
an HMAC and response encryption.  It also does policy unsealing which
mimics the more complex of the trusted key scenarios.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>

---
v3: add policy unseal testing with two sessions
---
 drivers/char/tpm/Makefile             |   1 +
 drivers/char/tpm/tpm-chip.c           |   1 +
 drivers/char/tpm/tpm.h                |   1 +
 drivers/char/tpm/tpm2-cmd.c           |   2 +
 drivers/char/tpm/tpm2-sessions-test.c | 360 ++++++++++++++++++++++++++++++++++
 5 files changed, 365 insertions(+)
 create mode 100644 drivers/char/tpm/tpm2-sessions-test.c

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 1b5f6ccbb86d..b3f5e95679f9 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_TCG_TPM) += tpm.o
 tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
 	 tpm-dev-common.o tpmrm-dev.o eventlog/common.o eventlog/tpm1.o \
 	 eventlog/tpm2.o tpm2-space.o tpm-buf.o tpm2-sessions.o
+obj-m +=  tpm2-sessions-test.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o eventlog/acpi.o
 tpm-$(CONFIG_EFI) += eventlog/efi.o
 tpm-$(CONFIG_OF) += eventlog/of.o
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 46caadca916a..ef7ff32bf827 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -142,6 +142,7 @@ struct tpm_chip *tpm_find_get_ops(struct tpm_chip *chip)
 		return NULL;
 	return chip;
 }
+EXPORT_SYMBOL(tpm_find_get_ops);
 
 /**
  * tpm_dev_release() - free chip memory and the device number
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index d39065d9995d..8cb8b32a8418 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -163,6 +163,7 @@ enum tpm2_command_codes {
 	TPM2_CC_CONTEXT_LOAD	= 0x0161,
 	TPM2_CC_CONTEXT_SAVE	= 0x0162,
 	TPM2_CC_FLUSH_CONTEXT	= 0x0165,
+	TPM2_CC_POLICY_COMMAND_CODE = 0x16c,
 	TPM2_CC_READ_PUBLIC	= 0x0173,
 	TPM2_CC_START_AUTH_SESS	= 0x0176,
 	TPM2_CC_GET_CAPABILITY	= 0x017A,
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index a8655cd535d1..7afe0f50dcce 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -379,6 +379,7 @@ void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
 
 	tpm_buf_destroy(&buf);
 }
+EXPORT_SYMBOL_GPL(tpm2_flush_context_cmd);
 
 /**
  * tpm_buf_append_auth() - append TPMS_AUTH_COMMAND to the buffer.
@@ -409,6 +410,7 @@ void tpm2_buf_append_auth(struct tpm_buf *buf, u32 session_handle,
 	if (hmac && hmac_len)
 		tpm_buf_append(buf, hmac, hmac_len);
 }
+EXPORT_SYMBOL_GPL(tpm2_buf_append_auth);
 
 /**
  * tpm2_seal_trusted() - seal the payload of a trusted key
diff --git a/drivers/char/tpm/tpm2-sessions-test.c b/drivers/char/tpm/tpm2-sessions-test.c
new file mode 100644
index 000000000000..7b4a01ad3745
--- /dev/null
+++ b/drivers/char/tpm/tpm2-sessions-test.c
@@ -0,0 +1,360 @@
+/* run a set of tests of the sessions code */
+#include "tpm.h"
+#include "tpm2-sessions.h"
+
+#include <linux/random.h>
+
+#include <asm/unaligned.h>
+
+/* simple policy: command code must be TPM2_CC_UNSEAL */
+static u8 policy[] = {
+	0xe6, 0x13, 0x13, 0x70, 0x76, 0x52, 0x4b, 0xde,
+	0x48, 0x75, 0x33, 0x86, 0x58, 0x84, 0xe9, 0x73,
+	0x2e, 0xbe, 0xe3, 0xaa, 0xcb, 0x09, 0x5d, 0x94,
+	0xa6, 0xde, 0x49, 0x2e, 0xc0, 0x6c, 0x46, 0xfa,
+};
+
+static u32 get_policy(struct tpm_chip *chip)
+{
+	struct tpm_buf buf;
+	u8 nonce[SHA256_DIGEST_SIZE];
+	u32 h;
+	int rc;
+
+	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_START_AUTH_SESS);
+	if (rc)
+		return 0;
+
+	/* salt key */
+	tpm_buf_append_u32(&buf, TPM2_RH_NULL);
+	/* bind key */
+	tpm_buf_append_u32(&buf, TPM2_RH_NULL);
+	/* zero nonce */
+	memset(nonce, 0, sizeof(nonce));
+	tpm_buf_append_u16(&buf, sizeof(nonce));
+	tpm_buf_append(&buf, nonce, sizeof(nonce));
+	/* encrypted salt (empty) */
+	tpm_buf_append_u16(&buf, 0);
+	/* session type (HMAC, audit or policy) */
+	tpm_buf_append_u8(&buf, TPM2_SE_POLICY);
+	/* symmetric encryption parameters */
+	/* symmetric algorithm */
+	tpm_buf_append_u16(&buf, TPM2_ALG_NULL);
+
+	/* hash algorithm for session */
+	tpm_buf_append_u16(&buf, TPM2_ALG_SHA256);
+
+	rc = tpm_transmit_cmd(chip, &chip->kernel_space, buf.data, PAGE_SIZE,
+			      0, 0, "start policy session");
+
+	h = get_unaligned_be32(&buf.data[TPM_HEADER_SIZE]);
+
+	tpm_buf_reset(&buf, TPM2_ST_NO_SESSIONS,
+		      TPM2_CC_POLICY_COMMAND_CODE);
+	tpm_buf_append_u32(&buf, h);
+	tpm_buf_append_u32(&buf, TPM2_CC_UNSEAL);
+
+	rc = tpm_transmit_cmd(chip, &chip->kernel_space, buf.data, PAGE_SIZE,
+			      0, 0, "start policy session");
+
+	return h;
+}
+
+int tpm2_sessions_test(void)
+{
+	struct tpm2_auth *auth;
+	struct tpm_buf buf, b1;
+	struct tpm_buf t2b;
+	struct tpm_chip *chip;
+	int rc;
+	char payload[29];
+	char *password = "Passw0Rd";
+	const u8 *p;
+	u32 h, ps;
+	u8 name[34];
+	u16 len;
+	int ret = -EINVAL;
+
+	chip = tpm_find_get_ops(NULL);
+	if (!chip)
+		return -ENODEV;
+
+	if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
+		return -ENODEV;
+
+	get_random_bytes(payload, sizeof(payload));
+
+	/* precursor: get a session */
+	rc = tpm2_start_auth_session(chip, &auth);
+	dev_info(&chip->dev, "TPM: start auth session returned %d\n", rc);
+	if (rc)
+		goto out;
+
+	/* first test: get random bytes from TPM */
+	tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM);
+	tpm_buf_append_hmac_session(&buf, auth, TPM2_SA_ENCRYPT
+				    | TPM2_SA_CONTINUE_SESSION, NULL, 0);
+	tpm_buf_append_u16(&buf, 29);
+	tpm_buf_fill_hmac_session(&buf, auth);
+	rc = tpm_transmit_cmd(chip, &chip->kernel_space, buf.data, PAGE_SIZE,
+			      0, 0, "get random");
+	rc = tpm_buf_check_hmac_response(&buf, auth, rc);
+	dev_info(&chip->dev, "TPM: check hmac response returned %d\n", rc);
+	tpm_buf_destroy(&buf);
+
+	/*
+	 * second test, seal random data protecting sensitive by
+	 * encryption and also doing response encryption (not
+	 * necessary) The encrypted payload has two components: an
+	 * authorization password which must be presented on useal and
+	 * the actual data (the random payload)
+	 */
+	tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
+	tpm_buf_append_name(&buf, auth, chip->tpmkey, chip->tpmkeyname);
+	tpm_buf_append_hmac_session(&buf, auth, TPM2_SA_DECRYPT
+				    | TPM2_SA_ENCRYPT
+				    | TPM2_SA_CONTINUE_SESSION, NULL, 0);
+	/* sensitive */
+	tpm_buf_init_2b(&t2b);
+	/* the authorization */
+	tpm_buf_append_u16(&t2b, strlen(password));
+	tpm_buf_append(&t2b, password, strlen(password));
+	/* the payload */
+	tpm_buf_append_u16(&t2b, sizeof(payload));
+	tpm_buf_append(&t2b, payload, sizeof(payload));
+	tpm_buf_append_2b(&buf, &t2b);
+	/* the public */
+	/* type */
+	tpm_buf_append_u16(&t2b, TPM2_ALG_KEYEDHASH);
+	/* name hash */
+	tpm_buf_append_u16(&t2b, TPM2_ALG_SHA256);
+	/* object properties */
+	tpm_buf_append_u32(&t2b, TPM2_OA_USER_WITH_AUTH | TPM2_OA_NO_DA);
+	/* auth policy (empty) */
+	tpm_buf_append_u16(&t2b, 0);
+	/* keyed hash parameters (we're null for a non-HMAC data blob) */
+	tpm_buf_append_u16(&t2b, TPM2_ALG_NULL);
+	/* unique */
+	tpm_buf_append_u16(&t2b, 0);
+	tpm_buf_append_2b(&buf, &t2b);
+	/* outside info (also empty) */
+	tpm_buf_append_u16(&buf, 0);
+	/* creation PCR (empty) */
+	tpm_buf_append_u32(&buf, 0);
+	tpm_buf_fill_hmac_session(&buf, auth);
+	rc = tpm_transmit_cmd(chip, &chip->kernel_space, buf.data, PAGE_SIZE,
+			      4, 0, "sealing data");
+	rc = tpm_buf_check_hmac_response(&buf, auth, rc);
+	dev_info(&chip->dev, "TPM: sealing response returned %d\n", rc);
+	if (rc)
+		goto out;
+
+	/*
+	 * now load the sealed object (we need the pub and priv parts
+	 * returned from prior command
+	 */
+	tpm_buf_init(&b1, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
+	/* parent */
+	tpm_buf_append_name(&b1, auth, chip->tpmkey, chip->tpmkeyname);
+	tpm_buf_append_hmac_session(&b1, auth, TPM2_SA_CONTINUE_SESSION,
+				    NULL, 0);
+	p = &buf.data[TPM_HEADER_SIZE+4];
+	/* private */
+	len = tpm_get_inc_u16(&p);
+	tpm_buf_append_u16(&b1, len);
+	tpm_buf_append(&b1, p, len);
+	p += len;
+	/* public */
+	len = tpm_get_inc_u16(&p);
+	tpm_buf_append_u16(&b1, len);
+	tpm_buf_append(&b1, p, len);
+	tpm_buf_fill_hmac_session(&b1, auth);
+	rc = tpm_transmit_cmd(chip, &chip->kernel_space, b1.data, PAGE_SIZE,
+			      4, 0, "loading seal");
+	rc = tpm_buf_check_hmac_response(&b1, auth, rc);
+	dev_info(&chip->dev, "TPM: load response returned %d\n", rc);
+	if (rc)
+		goto out;
+	p = &b1.data[TPM_HEADER_SIZE];
+	h = tpm_get_inc_u32(&p);
+	dev_info(&chip->dev, "sealed data loaded at %08x\n", h);
+	/* skip over parameter size */
+	p += 4;
+	len = tpm_get_inc_u16(&p);
+	if (len != sizeof(name)) {
+		dev_err(&chip->dev, "Wrong name size %d\n", len);
+		goto out;
+	}
+	memcpy(name, p, len);
+	tpm_buf_destroy(&b1);
+	tpm_buf_destroy(&buf);
+
+	/*
+	 * now unseal the data using the authority in a HMAC and
+	 * protecting the returned unseal by encryption
+	 */
+	tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
+	tpm_buf_append_name(&buf, auth, h, name);
+	tpm_buf_append_hmac_session(&buf, auth, TPM2_SA_ENCRYPT,
+				    password, strlen(password));
+	tpm_buf_fill_hmac_session(&buf, auth);
+	rc = tpm_transmit_cmd(chip, &chip->kernel_space, buf.data, PAGE_SIZE,
+			      4, 0, "unseal");
+	rc = tpm_buf_check_hmac_response(&buf, auth, rc);
+	tpm2_flush_context_cmd(chip, h, 0);
+	dev_info(&chip->dev, "unseal returns %d\n", rc);
+	if (rc)
+		goto out;
+	p = &buf.data[TPM_HEADER_SIZE + 4];
+	len = tpm_get_inc_u16(&p);
+	if (len != sizeof(payload)) {
+		dev_err(&chip->dev, "wrong unseal payload size %d != %ld",
+			len, sizeof(payload));
+		goto out;
+	}
+	if (memcmp(payload, p, len) != 0) {
+		dev_err(&chip->dev, "Payload DID NOT compare correctly\n");
+		goto out;
+	}
+
+	/*
+	 * third test, seal random data protecting sensitive by
+	 * encryption and also doing response encryption (not
+	 * necessary) The encrypted payload has two components: an
+	 * authorization password which must be presented on useal and
+	 * the actual data (the random payload)
+	 *
+	 * For this test use a policy to release the data testing that
+	 * we can have two sessions
+	 */
+	get_random_bytes(payload, sizeof(payload));
+	/* auth session should be closed, so get a new one */
+	rc = tpm2_start_auth_session(chip, &auth);
+	if (rc) {
+		dev_err(&chip->dev, "new auth session failed: %d\n", rc);
+		goto out;
+	}
+
+	tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
+	tpm_buf_append_name(&buf, auth, chip->tpmkey, chip->tpmkeyname);
+	tpm_buf_append_hmac_session(&buf, auth, TPM2_SA_DECRYPT
+				    | TPM2_SA_ENCRYPT
+				    | TPM2_SA_CONTINUE_SESSION, NULL, 0);
+	/* sensitive */
+	tpm_buf_init_2b(&t2b);
+	/* the authorization */
+	tpm_buf_append_u16(&t2b, 0);
+	/* the payload */
+	tpm_buf_append_u16(&t2b, sizeof(payload));
+	tpm_buf_append(&t2b, payload, sizeof(payload));
+	tpm_buf_append_2b(&buf, &t2b);
+	/* the public */
+	/* type */
+	tpm_buf_append_u16(&t2b, TPM2_ALG_KEYEDHASH);
+	/* name hash */
+	tpm_buf_append_u16(&t2b, TPM2_ALG_SHA256);
+	/* object properties */
+	tpm_buf_append_u32(&t2b, TPM2_OA_USER_WITH_AUTH | TPM2_OA_NO_DA);
+	/* auth policy (specific to command code) */
+	tpm_buf_append_u16(&t2b, sizeof(policy));
+	tpm_buf_append(&t2b, policy, sizeof(policy));
+	/* keyed hash parameters (we're null for a non-HMAC data blob) */
+	tpm_buf_append_u16(&t2b, TPM2_ALG_NULL);
+	/* unique */
+	tpm_buf_append_u16(&t2b, 0);
+	tpm_buf_append_2b(&buf, &t2b);
+	/* outside info (also empty) */
+	tpm_buf_append_u16(&buf, 0);
+	/* creation PCR (empty) */
+	tpm_buf_append_u32(&buf, 0);
+	tpm_buf_fill_hmac_session(&buf, auth);
+	rc = tpm_transmit_cmd(chip, &chip->kernel_space, buf.data, PAGE_SIZE,
+			      4, 0, "sealing data");
+	rc = tpm_buf_check_hmac_response(&buf, auth, rc);
+	dev_info(&chip->dev, "policy sealing response returned %d\n", rc);
+	if (rc)
+		goto out;
+
+	/*
+	 * now load the sealed object (we need the pub and priv parts
+	 * returned from prior command
+	 */
+	tpm_buf_init(&b1, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
+	/* parent */
+	tpm_buf_append_name(&b1, auth, chip->tpmkey, chip->tpmkeyname);
+	tpm_buf_append_hmac_session(&b1, auth, TPM2_SA_CONTINUE_SESSION,
+				    NULL, 0);
+	p = &buf.data[TPM_HEADER_SIZE+4];
+	/* private */
+	len = tpm_get_inc_u16(&p);
+	tpm_buf_append_u16(&b1, len);
+	tpm_buf_append(&b1, p, len);
+	p += len;
+	/* public */
+	len = tpm_get_inc_u16(&p);
+	tpm_buf_append_u16(&b1, len);
+	tpm_buf_append(&b1, p, len);
+	tpm_buf_fill_hmac_session(&b1, auth);
+	rc = tpm_transmit_cmd(chip, &chip->kernel_space, b1.data, PAGE_SIZE,
+			      4, 0, "loading seal");
+	rc = tpm_buf_check_hmac_response(&b1, auth, rc);
+	dev_info(&chip->dev, "TPM: load response returned %d\n", rc);
+	if (rc)
+		goto out;
+	p = &b1.data[TPM_HEADER_SIZE];
+	h = tpm_get_inc_u32(&p);
+	dev_info(&chip->dev, "sealed data loaded at %08x\n", h);
+	/* skip over parameter size */
+	p += 4;
+	len = tpm_get_inc_u16(&p);
+	if (len != sizeof(name)) {
+		dev_err(&chip->dev, "Wrong name size %d\n", len);
+		goto out;
+	}
+	memcpy(name, p, len);
+	tpm_buf_destroy(&b1);
+	tpm_buf_destroy(&buf);
+
+	/* now get a policy session */
+	ps = get_policy(chip);
+	/*
+	 * now unseal the data using the authority in a HMAC and
+	 * protecting the returned unseal by encryption
+	 */
+	tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
+	tpm_buf_append_name(&buf, auth, h, name);
+	tpm2_buf_append_auth(&buf, ps, NULL, 0, 0, NULL, 0);
+	tpm_buf_append_hmac_session(&buf, auth, TPM2_SA_ENCRYPT,
+				    NULL, 0);
+	tpm_buf_fill_hmac_session(&buf, auth);
+	rc = tpm_transmit_cmd(chip, &chip->kernel_space, buf.data, PAGE_SIZE,
+			      4, 0, "unseal");
+	rc = tpm_buf_check_hmac_response(&buf, auth, rc);
+	tpm2_flush_context_cmd(chip, h, 0);
+	dev_info(&chip->dev, "unseal returns %d\n", rc);
+	if (rc)
+		goto out;
+	p = &buf.data[TPM_HEADER_SIZE + 4];
+	len = tpm_get_inc_u16(&p);
+	if (len != sizeof(payload)) {
+		dev_err(&chip->dev, "wrong unseal payload size %d != %ld",
+			len, sizeof(payload));
+		goto out;
+	}
+	if (memcmp(payload, p, len) != 0) {
+		dev_err(&chip->dev, "Payload DID NOT compare correctly\n");
+		goto out;
+	}
+
+	dev_info(&chip->dev, "All tests passed\n");
+	ret = 0;
+
+ out:
+	tpm_put_ops(chip);
+
+	return ret;
+}
+
+module_init(tpm2_sessions_test);
+
+MODULE_LICENSE("GPL");
-- 
2.16.4

Re: [PATCH v4 0/7] add integrity and security to TPM2 transactions

[Ken Goldman]

On 10/22/2018 3:33 AM, James Bottomley wrote:
> By now, everybody knows we have a problem with the TPM2_RS_PW easy
> button on TPM2 in that transactions on the TPM bus can be intercepted
> and altered.  The way to fix this is to use real sessions for HMAC
> capabilities to ensure integrity and to use parameter and response
> encryption to ensure confidentiality of the data flowing over the TPM
> bus.

Does this design assume that there was at time zero no monitoring?
This would permit some shared secret to be established.

Or does it assume that the interception may have been present from
the first boot?  If so, how is the first shared secret established.
Salting using the EK is the usual method, but this requires walking the 
EK certificate chain and embedding the TPM vendor CA certificates in the 
kernel.


> This patch series is about adding a simple API which can ensure the
> above properties as a layered addition to the existing TPM handling
> code.  This series now includes protections for PCR extend, getting
> random numbers from the TPM and data sealing and unsealing.  It
> therefore eliminates all uses of TPM2_RS_PW in the kernel and adds
> encryption protection to sensitive data flowing into and out of the
> TPM.
> 
> In the third version I added data sealing and unsealing protection,
> apart from one API based problem which means that the way trusted keys
> were protected it's not currently possible to HMAC protect an authority
> that comes with a policy, so the API will have to be extended to fix
> that case

TPM 2.0 observations (questioning 'not possible'):

1 - Any policy that requires a password (policypassword) can substitute
an HMAC (policyauthvalue) at the callers discretion.  They result
in the same policy digest.

2 - Any command can be HMAC protected, even one like pcrread that does 
not require authorization.  Start an HMAC session and specify audit.
Of course, a shared secret has to be used, either a bind or salted
session.

3 - If the command is already using a policy session, but does not 
require a password, I believe that the same technique #2 can be
used - specify audit with that policy session.

I've tested #3 with bind but not salt.  I can test if there's
interest.

> In this fourth version, I tidy up some of the code and add more
> security features, the most notable is that we now calculate the NULL
> seed name and compare our calculation to the value returned in
> TPM2_ReadPublic, which means we now can't be spoofed.  This version
> also gives a sysfs variable for the null seed which userspace can use
> to run a key certification operation to prove that the TPM was always
> secure when communicating with the kernel.
> 
> I've verified this using the test suite in the last patch on a VM
> connected to a tpm2 emulator.  I also instrumented the emulator to make
> sure the sensitive data was properly encrypted.

Re: [PATCH v4 0/7] add integrity and security to TPM2 transactions

[James Bottomley]

On Mon, 2018-10-22 at 09:53 -0400, Ken Goldman wrote:
> On 10/22/2018 3:33 AM, James Bottomley wrote:
> > By now, everybody knows we have a problem with the TPM2_RS_PW easy
> > button on TPM2 in that transactions on the TPM bus can be
> > intercepted and altered.  The way to fix this is to use real
> > sessions for HMAC capabilities to ensure integrity and to use
> > parameter and response encryption to ensure confidentiality of the
> > data flowing over the TPM bus.
> 
> Does this design assume that there was at time zero no monitoring?
> This would permit some shared secret to be established.
> 
> Or does it assume that the interception may have been present from
> the first boot?  If so, how is the first shared secret established.
> Salting using the EK is the usual method, but this requires walking
> the  EK certificate chain and embedding the TPM vendor CA
> certificates in the  kernel.

The design establishes the shared secret at start of day using an EC
derived key from the null seed.  This can obviously be spoofed by a TPM
Genie running before the system was rebooted.  However, the computed
key name is exposed to user space and TPM2_Certify will fail when
userspace checks the null seed so you will know after the fact whether
the communication channel established on boot was secure or not.

It is possible to use either the EPS or SPS if we pass in the public
points as kernel parameters but this is getting rather complicated for
casual users.

> > This patch series is about adding a simple API which can ensure the
> > above properties as a layered addition to the existing TPM handling
> > code.  This series now includes protections for PCR extend, getting
> > random numbers from the TPM and data sealing and unsealing.  It
> > therefore eliminates all uses of TPM2_RS_PW in the kernel and adds
> > encryption protection to sensitive data flowing into and out of the
> > TPM.
> > 
> > In the third version I added data sealing and unsealing protection,
> > apart from one API based problem which means that the way trusted
> > keys were protected it's not currently possible to HMAC protect an
> > authority that comes with a policy, so the API will have to be
> > extended to fix that case
> 
> TPM 2.0 observations (questioning 'not possible'):

Actually, let me correct that: not possible with the way policy based
trusted keys API works because it expects you to pass in a policy
session handle.  The way to fix this is to fix the API to pass in the
actual policy sequence instead of the commands, but that's long term.

> 1 - Any policy that requires a password (policypassword) can
> substitute
> an HMAC (policyauthvalue) at the callers discretion.  They result
> in the same policy digest.

Right but this requires the co-operation of the policy handle creator,
so it's an API change.

> 2 - Any command can be HMAC protected, even one like pcrread that
> does not require authorization.  Start an HMAC session and specify
> audit Of course, a shared secret has to be used, either a bind or
> salted session.

Right, so if we create the session, we can then set it up to use a
salted session, but we don't create it under the current API.

> 3 - If the command is already using a policy session, but does not 
> require a password, I believe that the same technique #2 can be
> used - specify audit with that policy session.
> 
> I've tested #3 with bind but not salt.  I can test if there's
> interest.

I think the API is going to have to change, so if we're looking at a
new API, we should set it up to do the right thing with policy anyway. 
It turns out to be quite easy as I found out when Huawei patched the
openssl_tpm2_engine to add policy to keys.

James

> > In this fourth version, I tidy up some of the code and add more
> > security features, the most notable is that we now calculate the
> > NULL seed name and compare our calculation to the value returned in
> > TPM2_ReadPublic, which means we now can't be spoofed.  This version
> > also gives a sysfs variable for the null seed which userspace can
> > use to run a key certification operation to prove that the TPM was
> > always secure when communicating with the kernel.
> > 
> > I've verified this using the test suite in the last patch on a VM
> > connected to a tpm2 emulator.  I also instrumented the emulator to
> > make sure the sensitive data was properly encrypted.
> 
> 

Re: [PATCH v4 0/7] add integrity and security to TPM2 transactions

[Ken Goldman]

On 10/22/2018 10:18 AM, James Bottomley wrote:
>> 1 - Any policy that requires a password (policypassword) can
>> substitute
>> an HMAC (policyauthvalue) at the callers discretion.  They result
>> in the same policy digest.
> Right but this requires the co-operation of the policy handle creator,
> so it's an API change.
> 

In case it wasn't clear:  The choice is not made at the time the
policy is calculated, nor is it made at startauthsession (when
the policy is created).

The choice is made when the policy is being satisfied.

policypassword tells the TPM to expect a plaintext password, while
policyauthvalue tells the TPM to expect an HMAC.

It's subtle that either policy command results in the same policy
digest.

Re: [PATCH v4 0/7] add integrity and security to TPM2 transactions

[James Bottomley]

On Mon, 2018-10-22 at 11:50 -0400, Ken Goldman wrote:
> On 10/22/2018 10:18 AM, James Bottomley wrote:
> > > 1 - Any policy that requires a password (policypassword) can
> > > substitute
> > > an HMAC (policyauthvalue) at the callers discretion.  They result
> > > in the same policy digest.
> > 
> > Right but this requires the co-operation of the policy handle
> > creator, so it's an API change.
> > 
> 
> In case it wasn't clear:  The choice is not made at the time the
> policy is calculated, nor is it made at startauthsession (when
> the policy is created).
> 
> The choice is made when the policy is being satisfied.
> 
> policypassword tells the TPM to expect a plaintext password, while
> policyauthvalue tells the TPM to expect an HMAC.
> 
> It's subtle that either policy command results in the same policy
> digest.

I understand, but the API just has a policy handle from a fully
constructed policy passed in.  There's no way to deconstruct this even
to know if it has auth requirements and what type.  To get that info we
 have to see the policy commands that go into building up the policy
session ... this means the API needs to change.

James

Re: [PATCH v4 2/7] tpm2-sessions: Add full HMAC and encrypt/decrypt session handling

[Ard Biesheuvel]

Hi James,

Some comments below on how you are using the crypto API.

On 22 October 2018 at 04:36, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> This code adds true session based HMAC authentication plus parameter
> decryption and response encryption using AES.
>
> The basic design of this code is to segregate all the nasty crypto,
> hash and hmac code into tpm2-sessions.c and export a usable API.
>
> The API first of all starts off by gaining a session with
>
> tpm2_start_auth_session()
>
> Which initiates a session with the TPM and allocates an opaque
> tpm2_auth structure to handle the session parameters.  Then the use is
> simply:
>
> * tpm_buf_append_name() in place of the tpm_buf_append_u32 for the
>   handles
>
> * tpm_buf_append_hmac_session() where tpm2_append_auth() would go
>
> * tpm_buf_fill_hmac_session() called after the entire command buffer
>   is finished but before tpm_transmit_cmd() is called which computes
>   the correct HMAC and places it in the command at the correct
>   location.
>
> Finally, after tpm_transmit_cmd() is called,
> tpm_buf_check_hmac_response() is called to check that the returned
> HMAC matched and collect the new state for the next use of the
> session, if any.
>
> The features of the session is controlled by the session attributes
> set in tpm_buf_append_hmac_session().  If TPM2_SA_CONTINUE_SESSION is
> not specified, the session will be flushed and the tpm2_auth structure
> freed in tpm_buf_check_hmac_response(); otherwise the session may be
> used again.  Parameter encryption is specified by or'ing the flag
> TPM2_SA_DECRYPT and response encryption by or'ing the flag
> TPM2_SA_ENCRYPT.  the various encryptions will be taken care of by
> tpm_buf_fill_hmac_session() and tpm_buf_check_hmac_response()
> respectively.
>
> To get all of this to work securely, the Kernel now needs a primary
> key to encrypt the session salt to, so we derive an EC key from the
> NULL seed and store it in the tpm_chip structure.  We also make sure
> that this seed remains for the kernel by using a kernel space to take
> it out of the TPM when userspace wants to use it.
>
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
>
> ---
>
> v2: Added docbook and improved response check API
> v3: Add readpublic, fix hmac length, add API for close on error
>     allow for the hmac session not being first in the sessions
> v4: Make NULL seed template exactly match the SRK ECC template.
>     Also check the NULL primary key name is what getpublic returns
>     to prevent spoofing.  Also parametrise the name size for reuse
> ---
>  drivers/char/tpm/Kconfig         |    3 +
>  drivers/char/tpm/Makefile        |    2 +-
>  drivers/char/tpm/tpm.h           |   32 +
>  drivers/char/tpm/tpm2-cmd.c      |   34 +-
>  drivers/char/tpm/tpm2-sessions.c | 1188 ++++++++++++++++++++++++++++++++++++++
>  drivers/char/tpm/tpm2-sessions.h |   57 ++
>  6 files changed, 1300 insertions(+), 16 deletions(-)
>  create mode 100644 drivers/char/tpm/tpm2-sessions.c
>  create mode 100644 drivers/char/tpm/tpm2-sessions.h
>
...
> diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
> new file mode 100644
> index 000000000000..422c3ec64f8c
> --- /dev/null
> +++ b/drivers/char/tpm/tpm2-sessions.c
> @@ -0,0 +1,1188 @@
...
> +/*
> + * this is our static crypto shash.  This is possible because the hash
> + * is multi-threaded and all the state stored in the desc
> + */
> +static struct crypto_shash *sha256_hash;
> +
> +/*
> + * It turns out the crypto hmac(sha256) is hard for us to consume
> + * because it assumes a fixed key and the TPM seems to change the key
> + * on every operation, so we weld the hmac init and final functions in
> + * here to give it the same usage characteristics as a regular hash
> + */
> +static void hmac_init(struct shash_desc *desc, u8 *key, int keylen)
> +{
> +       u8 pad[SHA256_BLOCK_SIZE];
> +       int i;
> +
> +       desc->tfm = sha256_hash;
> +       desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;

I don't think this actually does anything in the shash API
implementation, so you can drop this. However, I take it this means
that hmac_init() is never called in contexts where sleeping is not
allowed? For the relevance of that, please see below.

> +       crypto_shash_init(desc);
> +       for (i = 0; i < sizeof(pad); i++) {
> +               if (i < keylen)
> +                       pad[i] = key[i];
> +               else
> +                       pad[i] = 0;
> +               pad[i] ^= HMAC_IPAD_VALUE;
> +       }
> +       crypto_shash_update(desc, pad, sizeof(pad));
> +}
> +
> +static void hmac_final(struct shash_desc *desc, u8 *key, int keylen, u8 *out)
> +{
> +       u8 pad[SHA256_BLOCK_SIZE];
> +       int i;
> +
> +       for (i = 0; i < sizeof(pad); i++) {
> +               if (i < keylen)
> +                       pad[i] = key[i];
> +               else
> +                       pad[i] = 0;
> +               pad[i] ^= HMAC_OPAD_VALUE;
> +       }
> +
> +       /* collect the final hash;  use out as temporary storage */
> +       crypto_shash_final(desc, out);
> +
> +       /* reuse the desc */
> +       crypto_shash_init(desc);
> +       crypto_shash_update(desc, pad, sizeof(pad));
> +       crypto_shash_update(desc, out, SHA256_DIGEST_SIZE);
> +       crypto_shash_final(desc, out);
> +}
> +
...
> +/**
> + * tpm_buf_fill_hmac_session() - finalize the session HMAC
> + * @buf: The buffer to be appended
> + * @auth: the auth structure allocated by tpm2_start_auth_session()
> + *
> + * This command must not be called until all of the parameters have
> + * been appended to @buf otherwise the computed HMAC will be
> + * incorrect.
> + *
> + * This function computes and fills in the session HMAC using the
> + * session key and, if TPM2_SA_DECRYPT was specified, computes the
> + * encryption key and encrypts the first parameter of the command
> + * buffer with it.
> + *
> + * As with most tpm_buf operations, success is assumed because failure
> + * will be caused by an incorrect programming model and indicated by a
> + * kernel message.
> + */
> +void tpm_buf_fill_hmac_session(struct tpm_buf *buf, struct tpm2_auth *auth)
> +{
> +       u32 cc, handles, val;
> +       struct tpm_chip *chip = auth->chip;
> +       int i;
> +       struct tpm_input_header *head = (struct tpm_input_header *)buf->data;
> +       const u8 *s, *p;
> +       u8 *hmac = NULL;
> +       u32 attrs;
> +       u8 cphash[SHA256_DIGEST_SIZE];
> +       SHASH_DESC_ON_STACK(desc, sha256_hash);
> +
> +       /* save the command code in BE format */
> +       auth->ordinal = head->ordinal;
> +
> +       desc->tfm = sha256_hash;
> +       desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
> +

Same here.

> +       cc = be32_to_cpu(head->ordinal);
> +
> +       i = tpm2_find_cc(chip, cc);
> +       if (i < 0) {
> +               dev_err(&chip->dev, "Command 0x%x not found in TPM\n", cc);
> +               return;
> +       }
> +       attrs = chip->cc_attrs_tbl[i];
> +
> +       handles = (attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0);
> +
> +       s = &buf->data[TPM_HEADER_SIZE];
> +       /*
> +        * just check the names, it's easy to make mistakes.  This
> +        * would happen if someone added a handle via
> +        * tpm_buf_append_u32() instead of tpm_buf_append_name()
> +        */
> +       for (i = 0; i < handles; i++) {
> +               u32 handle = tpm_get_inc_u32(&s);
> +
> +               if (auth->name_h[i] != handle) {
> +                       dev_err(&chip->dev, "TPM: handle %d wrong for name\n",
> +                                 i);
> +                       return;
> +               }
> +       }
> +       /* point s to the start of the sessions */
> +       val = tpm_get_inc_u32(&s);
> +       /* point p to the start of the parameters */
> +       p = s + val;
> +       for (i = 1; s < p; i++) {
> +               u32 handle = tpm_get_inc_u32(&s);
> +               u16 len;
> +               u8 a;
> +
> +               /* nonce (already in auth) */
> +               len = tpm_get_inc_u16(&s);
> +               s += len;
> +
> +               a = *s++;
> +
> +               len = tpm_get_inc_u16(&s);
> +               if (handle == auth->handle && auth->attrs == a) {
> +                       hmac = (u8 *)s;
> +                       /*
> +                        * save our session number so we know which
> +                        * session in the response belongs to us
> +                        */
> +                       auth->session = i;
> +               }
> +
> +               s += len;
> +       }
> +       if (s != p) {
> +               dev_err(&chip->dev, "TPM session length is incorrect\n");
> +               return;
> +       }
> +       if (!hmac) {
> +               dev_err(&chip->dev, "TPM could not find HMAC session\n");
> +               return;
> +       }
> +
> +       /* encrypt before HMAC */
> +       if (auth->attrs & TPM2_SA_DECRYPT) {
> +               struct scatterlist sg[1];
> +               u16 len;
> +               SKCIPHER_REQUEST_ON_STACK(req, auth->aes);
> +
> +               skcipher_request_set_tfm(req, auth->aes);
> +               skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP,
> +                                             NULL, NULL);
> +

Your crypto_alloc_skcipher() call [further down] does not mask out
CRYPTO_ALG_ASYNC, and so it permits asynchronous implementations to be
selected. Your generic cfb template only permits synchronous
implementations, since it wraps the cipher directly (which is always
synchronous). However, we have support in the tree for some
accelerators that implement cfb(aes), and those will return
-EINPROGRESS when invoking crypto_skcipher_en/decrypt(req), which you
are not set up to handle.

So the simple solution is to call 'crypto_alloc_skcipher("cfb(aes)",
0, CRYPTO_ALG_ASYNC)' below instead.

However, I would prefer it if we permit asynchronous skciphers here.
The reason is that, on a given platform, the accelerator may be the
only truly time invariant AES implementation that is available, and
since we are dealing with the TPM, a bit of paranoia does not hurt. It
also makes it easier to implement it as a [time invariant] SIMD based
asynchronous skcipher, which are simpler than synchronous ones since
they don't require a non-SIMD fallback path for calls from contexts
where the SIMD unit may not be used.

I have already implemented cfb(aes) for arm64/NEON. I will post the
patch after the merge window closes.

> +               /* need key and IV */
> +               KDFa(auth->session_key, SHA256_DIGEST_SIZE
> +                    + auth->passphraselen, "CFB", auth->our_nonce,
> +                    auth->tpm_nonce, AES_KEYBYTES + AES_BLOCK_SIZE,
> +                    auth->scratch);
> +               crypto_skcipher_setkey(auth->aes, auth->scratch, AES_KEYBYTES);
> +               len = tpm_get_inc_u16(&p);
> +               sg_init_one(sg, p, len);
> +               skcipher_request_set_crypt(req, sg, sg, len,
> +                                          auth->scratch + AES_KEYBYTES);
> +               crypto_skcipher_encrypt(req);

So please consider replacing this with something like.

DECLARE_CRYPTO_WAIT(wait); [further up]
skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP,
                              crypto_req_done, &wait);
crypto_wait_req(crypto_skcipher_encrypt(req), &wait);


> +               /* reset p to beginning of parameters for HMAC */
> +               p -= 2;
> +       }
> +
> +       crypto_shash_init(desc);
> +       /* ordinal is already BE */
> +       crypto_shash_update(desc, (u8 *)&head->ordinal, sizeof(head->ordinal));
> +       /* add the handle names */
> +       for (i = 0; i < handles; i++) {
> +               u8 mso = auth->name_h[i] >> 24;
> +
> +               if (mso == 0x81 || mso == 0x80 || mso == 0x01) {
> +                       crypto_shash_update(desc, auth->name[i],
> +                                           SHA256_DIGEST_SIZE + 2);
> +               } else {
> +                       __be32 h = cpu_to_be32(auth->name_h[i]);
> +
> +                       crypto_shash_update(desc, (u8 *)&h, 4);
> +               }
> +       }
> +       if (buf->data - s != tpm_buf_length(buf))
> +               crypto_shash_update(desc, s, buf->data
> +                                   + tpm_buf_length(buf) - s);
> +       crypto_shash_final(desc, cphash);
> +
> +       /* now calculate the hmac */
> +       hmac_init(desc, auth->session_key, sizeof(auth->session_key)
> +                 + auth->passphraselen);
> +       crypto_shash_update(desc, cphash, sizeof(cphash));
> +       crypto_shash_update(desc, auth->our_nonce, sizeof(auth->our_nonce));
> +       crypto_shash_update(desc, auth->tpm_nonce, sizeof(auth->tpm_nonce));
> +       crypto_shash_update(desc, &auth->attrs, 1);
> +       hmac_final(desc, auth->session_key, sizeof(auth->session_key)
> +                  + auth->passphraselen, hmac);
> +}
> +EXPORT_SYMBOL(tpm_buf_fill_hmac_session);
> +
...
> +/**
> + * tpm_buf_check_hmac_response() - check the TPM return HMAC for correctness
> + * @buf: the original command buffer (which now contains the response)
> + * @auth: the auth structure allocated by tpm2_start_auth_session()
> + * @rc: the return code from tpm_transmit_cmd
> + *
> + * If @rc is non zero, @buf may not contain an actual return, so @rc
> + * is passed through as the return and the session cleaned up and
> + * de-allocated if required (this is required if
> + * TPM2_SA_CONTINUE_SESSION was not specified as a session flag).
> + *
> + * If @rc is zero, the response HMAC is computed against the returned
> + * @buf and matched to the TPM one in the session area.  If there is a
> + * mismatch, an error is logged and -EINVAL returned.
> + *
> + * The reason for this is that the command issue and HMAC check
> + * sequence should look like:
> + *
> + *     rc = tpm_transmit_cmd(...);
> + *     rc = tpm_buf_check_hmac_response(&buf, auth, rc);
> + *     if (rc)
> + *             ...
> + *
> + * Which is easily layered into the current contrl flow.
> + *
> + * Returns: 0 on success or an error.
> + */
> +int tpm_buf_check_hmac_response(struct tpm_buf *buf, struct tpm2_auth *auth,
> +                               int rc)
> +{
> +       struct tpm_output_header *head = (struct tpm_output_header *)buf->data;
> +       struct tpm_chip *chip = auth->chip;
> +       const u8 *s, *p;
> +       u8 rphash[SHA256_DIGEST_SIZE];
> +       u32 attrs;
> +       SHASH_DESC_ON_STACK(desc, sha256_hash);
> +       u16 tag = be16_to_cpu(head->tag);
> +       u32 cc = be32_to_cpu(auth->ordinal);
> +       int parm_len, len, i, handles;
> +
> +       if (auth->session >= TPM_HEADER_SIZE) {
> +               WARN(1, "tpm session not filled correctly\n");
> +               goto out;
> +       }
> +
> +       if (rc != 0)
> +               /* pass non success rc through and close the session */
> +               goto out;
> +
> +       rc = -EINVAL;
> +       if (tag != TPM2_ST_SESSIONS) {
> +               dev_err(&chip->dev, "TPM: HMAC response check has no sessions tag\n");
> +               goto out;
> +       }
> +
> +       i = tpm2_find_cc(chip, cc);
> +       if (i < 0)
> +               goto out;
> +       attrs = chip->cc_attrs_tbl[i];
> +       handles = (attrs >> TPM2_CC_ATTR_RHANDLE) & 1;
> +
> +       /* point to area beyond handles */
> +       s = &buf->data[TPM_HEADER_SIZE + handles * 4];
> +       parm_len = tpm_get_inc_u32(&s);
> +       p = s;
> +       s += parm_len;
> +       /* skip over any sessions before ours */
> +       for (i = 0; i < auth->session - 1; i++) {
> +               len = tpm_get_inc_u16(&s);
> +               s += len + 1;
> +               len = tpm_get_inc_u16(&s);
> +               s += len;
> +       }
> +       /* TPM nonce */
> +       len = tpm_get_inc_u16(&s);
> +       if (s - buf->data + len > tpm_buf_length(buf))
> +               goto out;
> +       if (len != SHA256_DIGEST_SIZE)
> +               goto out;
> +       memcpy(auth->tpm_nonce, s, len);
> +       s += len;
> +       attrs = *s++;
> +       len = tpm_get_inc_u16(&s);
> +       if (s - buf->data + len != tpm_buf_length(buf))
> +               goto out;
> +       if (len != SHA256_DIGEST_SIZE)
> +               goto out;
> +       /*
> +        * s points to the HMAC. now calculate comparison, beginning
> +        * with rphash
> +        */
> +       desc->tfm = sha256_hash;
> +       desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
> +       crypto_shash_init(desc);
> +       /* yes, I know this is now zero, but it's what the standard says */
> +       crypto_shash_update(desc, (u8 *)&head->return_code,
> +                           sizeof(head->return_code));
> +       /* ordinal is already BE */
> +       crypto_shash_update(desc, (u8 *)&auth->ordinal, sizeof(auth->ordinal));
> +       crypto_shash_update(desc, p, parm_len);
> +       crypto_shash_final(desc, rphash);
> +
> +       /* now calculate the hmac */
> +       hmac_init(desc, auth->session_key, sizeof(auth->session_key)
> +                 + auth->passphraselen);
> +       crypto_shash_update(desc, rphash, sizeof(rphash));
> +       crypto_shash_update(desc, auth->tpm_nonce, sizeof(auth->tpm_nonce));
> +       crypto_shash_update(desc, auth->our_nonce, sizeof(auth->our_nonce));
> +       crypto_shash_update(desc, &auth->attrs, 1);
> +       /* we're done with the rphash, so put our idea of the hmac there */
> +       hmac_final(desc, auth->session_key, sizeof(auth->session_key)
> +                  + auth->passphraselen, rphash);
> +       if (memcmp(rphash, s, SHA256_DIGEST_SIZE) == 0) {
> +               rc = 0;
> +       } else {
> +               dev_err(&auth->chip->dev, "TPM: HMAC check failed\n");
> +               goto out;
> +       }
> +
> +       /* now do response decryption */
> +       if (auth->attrs & TPM2_SA_ENCRYPT) {
> +               struct scatterlist sg[1];
> +               SKCIPHER_REQUEST_ON_STACK(req, auth->aes);
> +
> +               skcipher_request_set_tfm(req, auth->aes);
> +               skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP,
> +                                             NULL, NULL);
> +
> +               /* need key and IV */
> +               KDFa(auth->session_key, SHA256_DIGEST_SIZE
> +                    + auth->passphraselen, "CFB", auth->tpm_nonce,
> +                    auth->our_nonce, AES_KEYBYTES + AES_BLOCK_SIZE,
> +                    auth->scratch);
> +               crypto_skcipher_setkey(auth->aes, auth->scratch, AES_KEYBYTES);
> +               len = tpm_get_inc_u16(&p);
> +               sg_init_one(sg, p, len);
> +               skcipher_request_set_crypt(req, sg, sg, len,
> +                                          auth->scratch + AES_KEYBYTES);
> +               crypto_skcipher_decrypt(req);
> +       }
> +
> + out:
> +       if ((auth->attrs & TPM2_SA_CONTINUE_SESSION) == 0) {
> +               /* manually close the session if it wasn't consumed */
> +               if (rc)
> +                       tpm2_flush_context_cmd(chip, auth->handle, 0);
> +               crypto_free_skcipher(auth->aes);
> +               kfree(auth);
> +       } else {
> +               /* reset for next use  */
> +               auth->session = TPM_HEADER_SIZE;
> +       }
> +
> +       return rc;
> +}
> +EXPORT_SYMBOL(tpm_buf_check_hmac_response);
> +
...
> +/**
> + * tpm2_start_auth_session - create a HMAC authentication session with the TPM
> + * @chip: the TPM chip structure to create the session with
> + * @authp: A pointer to an opaque tpm2_auth structure to be allocated
> + *
> + * This function contacts the TPM via the kernel space for an
> + * authentication session, allocates a tpm2_auth structure to contain
> + * all the session details necessary for performing the HMAC, encrypt
> + * and decrypt operations, fills it in and returns.
> + *
> + * Return: zero on success or actual error encountered.  If return is
> + * zero, @authp will be allocated.
> + */
> +int tpm2_start_auth_session(struct tpm_chip *chip, struct tpm2_auth **authp)
> +{
> +       struct tpm_buf buf;
> +       struct tpm2_auth *auth;
> +       int rc;
> +
> +       auth = kmalloc(sizeof(**authp), GFP_KERNEL);
> +       if (!auth)
> +               return -ENOMEM;
> +
> +       auth->chip = chip;
> +       auth->session = TPM_HEADER_SIZE;
> +
> +       rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_START_AUTH_SESS);
> +       if (rc)
> +               goto out;
> +
> +       /* salt key handle */
> +       tpm_buf_append_u32(&buf, chip->tpmkey);
> +       /* bind key handle */
> +       tpm_buf_append_u32(&buf, TPM2_RH_NULL);
> +       /* nonce caller */
> +       get_random_bytes(auth->our_nonce, sizeof(auth->our_nonce));
> +       tpm_buf_append_u16(&buf, sizeof(auth->our_nonce));
> +       tpm_buf_append(&buf, auth->our_nonce, sizeof(auth->our_nonce));
> +
> +       /* append encrypted salt and squirrel away unencrypted in auth */
> +       tpm_buf_append_salt(&buf, chip, auth);
> +       /* session type (HMAC, audit or policy) */
> +       tpm_buf_append_u8(&buf, TPM2_SE_HMAC);
> +
> +       /* symmetric encryption parameters */
> +       /* symmetric algorithm */
> +       tpm_buf_append_u16(&buf, TPM2_ALG_AES);
> +       /* bits for symmetric algorithm */
> +       tpm_buf_append_u16(&buf, AES_KEYBITS);
> +       /* symmetric algorithm mode (must be CFB) */
> +       tpm_buf_append_u16(&buf, TPM2_ALG_CFB);
> +       /* hash algorithm for session */
> +       tpm_buf_append_u16(&buf, TPM2_ALG_SHA256);
> +
> +       rc = tpm_transmit_cmd(chip, &chip->kernel_space, buf.data, PAGE_SIZE,
> +                             0, 0, "start auth session");
> +       if (rc == TPM2_RC_SUCCESS)
> +               rc = parse_start_auth_session(auth, buf.data);
> +
> +       tpm_buf_destroy(&buf);
> +
> +       if (rc)
> +               goto out;
> +
> +       auth->aes = crypto_alloc_skcipher("cfb(aes)", 0, 0);

^^^

> +       if (IS_ERR(auth->aes)) {
> +               rc = PTR_ERR(auth->aes);
> +               dev_err(&chip->dev, "TPM: error getting cfb(aes): %d\n", rc);
> +       }
> + out:
> +       if (rc)
> +               kfree(auth);
> +       else
> +               *authp = auth;
> +
> +       return rc;
> +}
> +EXPORT_SYMBOL(tpm2_start_auth_session);
> +

Re: [PATCH v4 2/7] tpm2-sessions: Add full HMAC and encrypt/decrypt session handling

[James Bottomley]

On Mon, 2018-10-22 at 19:19 -0300, Ard Biesheuvel wrote:
[...]
> > +static void hmac_init(struct shash_desc *desc, u8 *key, int
> > keylen)
> > +{
> > +       u8 pad[SHA256_BLOCK_SIZE];
> > +       int i;
> > +
> > +       desc->tfm = sha256_hash;
> > +       desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
> 
> I don't think this actually does anything in the shash API
> implementation, so you can drop this.

OK, I find crypto somewhat hard to follow.  There were bits I had to
understand, like when I wrote the CFB implementation or when I fixed
the ECDH scatterlist handling, but I've got to confess, in time
honoured tradition I simply copied this from EVM crypto without
actually digging into the code to understand why.

>  However, I take it this means that hmac_init() is never called in
> contexts where sleeping is not allowed? For the relevance of that,
> please see below.

Right, these routines are always called as an adjunct to a TPM
operation and TPM operations can sleep, so we must at least have kernel
thread context.

[...]
> > +       /* encrypt before HMAC */
> > +       if (auth->attrs & TPM2_SA_DECRYPT) {
> > +               struct scatterlist sg[1];
> > +               u16 len;
> > +               SKCIPHER_REQUEST_ON_STACK(req, auth->aes);
> > +
> > +               skcipher_request_set_tfm(req, auth->aes);
> > +               skcipher_request_set_callback(req,
> > CRYPTO_TFM_REQ_MAY_SLEEP,
> > +                                             NULL, NULL);
> > +
> 
> Your crypto_alloc_skcipher() call [further down] does not mask out
> CRYPTO_ALG_ASYNC, and so it permits asynchronous implementations to
> be selected. Your generic cfb template only permits synchronous
> implementations, since it wraps the cipher directly (which is always
> synchronous). However, we have support in the tree for some
> accelerators that implement cfb(aes), and those will return
> -EINPROGRESS when invoking crypto_skcipher_en/decrypt(req), which you
> are not set up to handle.
> 
> So the simple solution is to call 'crypto_alloc_skcipher("cfb(aes)",
> 0, CRYPTO_ALG_ASYNC)' below instead.
> 
> However, I would prefer it if we permit asynchronous skciphers here.
> The reason is that, on a given platform, the accelerator may be the
> only truly time invariant AES implementation that is available, and
> since we are dealing with the TPM, a bit of paranoia does not hurt.
> It also makes it easier to implement it as a [time invariant] SIMD
> based asynchronous skcipher, which are simpler than synchronous ones
> since they don't require a non-SIMD fallback path for calls from
> contexts where the SIMD unit may not be used.
> 
> I have already implemented cfb(aes) for arm64/NEON. I will post the
> patch after the merge window closes.
> 
> > +               /* need key and IV */
> > +               KDFa(auth->session_key, SHA256_DIGEST_SIZE
> > +                    + auth->passphraselen, "CFB", auth->our_nonce,
> > +                    auth->tpm_nonce, AES_KEYBYTES +
> > AES_BLOCK_SIZE,
> > +                    auth->scratch);
> > +               crypto_skcipher_setkey(auth->aes, auth->scratch,
> > AES_KEYBYTES);
> > +               len = tpm_get_inc_u16(&p);
> > +               sg_init_one(sg, p, len);
> > +               skcipher_request_set_crypt(req, sg, sg, len,
> > +                                          auth->scratch +
> > AES_KEYBYTES);
> > +               crypto_skcipher_encrypt(req);
> 
> So please consider replacing this with something like.
> 
> DECLARE_CRYPTO_WAIT(wait); [further up]
> skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP,
>                               crypto_req_done, &wait);
> crypto_wait_req(crypto_skcipher_encrypt(req), &wait);

Sure, I can do this ... the crypto skcipher handling was also cut and
paste, but I forget where from this time.  So what I think you're
asking for is below as the incremental diff?  I've tested this out and
it all works fine in my session testing environment (and on my real
laptop) ... although since I'm fully sync, I won't really have tested
the -EINPROGRESS do the wait case.

James

---

diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index 422c3ec64f8c..bbd3e8580954 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -165,7 +165,6 @@ static void hmac_init(struct shash_desc *desc, u8 *key, int keylen)
 	int i;
 
 	desc->tfm = sha256_hash;
-	desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
 	crypto_shash_init(desc);
 	for (i = 0; i < sizeof(pad); i++) {
 		if (i < keylen)
@@ -244,7 +243,6 @@ static void KDFe(u8 z[EC_PT_SZ], const char *str, u8 *pt_u, u8 *pt_v,
 	__be32 c = cpu_to_be32(1);
 
 	desc->tfm = sha256_hash;
-	desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
 
 	crypto_shash_init(desc);
 	/* counter (BE) */
@@ -445,7 +443,6 @@ void tpm_buf_fill_hmac_session(struct tpm_buf *buf, struct tpm2_auth *auth)
 	auth->ordinal = head->ordinal;
 
 	desc->tfm = sha256_hash;
-	desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
 
 	cc = be32_to_cpu(head->ordinal);
 
@@ -514,10 +511,11 @@ void tpm_buf_fill_hmac_session(struct tpm_buf *buf, struct tpm2_auth *auth)
 		struct scatterlist sg[1];
 		u16 len;
 		SKCIPHER_REQUEST_ON_STACK(req, auth->aes);
+		DECLARE_CRYPTO_WAIT(wait);
 
 		skcipher_request_set_tfm(req, auth->aes);
 		skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP,
-					      NULL, NULL);
+					      crypto_req_done, &wait);
 
 		/* need key and IV */
 		KDFa(auth->session_key, SHA256_DIGEST_SIZE
@@ -529,7 +527,7 @@ void tpm_buf_fill_hmac_session(struct tpm_buf *buf, struct tpm2_auth *auth)
 		sg_init_one(sg, p, len);
 		skcipher_request_set_crypt(req, sg, sg, len,
 					   auth->scratch + AES_KEYBYTES);
-		crypto_skcipher_encrypt(req);
+		crypto_wait_req(crypto_skcipher_encrypt(req), &wait);
 		/* reset p to beginning of parameters for HMAC */
 		p -= 2;
 	}
@@ -755,7 +753,6 @@ int tpm_buf_check_hmac_response(struct tpm_buf *buf, struct tpm2_auth *auth,
 	 * with rphash
 	 */
 	desc->tfm = sha256_hash;
-	desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
 	crypto_shash_init(desc);
 	/* yes, I know this is now zero, but it's what the standard says */
 	crypto_shash_update(desc, (u8 *)&head->return_code,
@@ -786,10 +783,11 @@ int tpm_buf_check_hmac_response(struct tpm_buf *buf, struct tpm2_auth *auth,
 	if (auth->attrs & TPM2_SA_ENCRYPT) {
 		struct scatterlist sg[1];
 		SKCIPHER_REQUEST_ON_STACK(req, auth->aes);
+		DECLARE_CRYPTO_WAIT(wait);
 
 		skcipher_request_set_tfm(req, auth->aes);
 		skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP,
-					      NULL, NULL);
+					      crypto_req_done, &wait);
 
 		/* need key and IV */
 		KDFa(auth->session_key, SHA256_DIGEST_SIZE
@@ -801,7 +799,7 @@ int tpm_buf_check_hmac_response(struct tpm_buf *buf, struct tpm2_auth *auth,
 		sg_init_one(sg, p, len);
 		skcipher_request_set_crypt(req, sg, sg, len,
 					   auth->scratch + AES_KEYBYTES);
-		crypto_skcipher_decrypt(req);
+		crypto_wait_req(crypto_skcipher_decrypt(req), &wait);
 	}
 
  out:
@@ -965,7 +963,6 @@ static int parse_create_primary(struct tpm_chip *chip, u8 *data)
 	tmp = resp;
 	/* now we have the public area, compute the name of the object */
 	desc->tfm = sha256_hash;
-	desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
 	put_unaligned_be16(TPM2_ALG_SHA256, chip->tpmkeyname);
 	crypto_shash_init(desc);
 	crypto_shash_update(desc, resp, len);

Re: [PATCH v4 2/7] tpm2-sessions: Add full HMAC and encrypt/decrypt session handling

[Ard Biesheuvel]

On 23 October 2018 at 04:01, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Mon, 2018-10-22 at 19:19 -0300, Ard Biesheuvel wrote:
> [...]
>> > +static void hmac_init(struct shash_desc *desc, u8 *key, int
>> > keylen)
>> > +{
>> > +       u8 pad[SHA256_BLOCK_SIZE];
>> > +       int i;
>> > +
>> > +       desc->tfm = sha256_hash;
>> > +       desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
>>
>> I don't think this actually does anything in the shash API
>> implementation, so you can drop this.
>
> OK, I find crypto somewhat hard to follow.  There were bits I had to
> understand, like when I wrote the CFB implementation or when I fixed
> the ECDH scatterlist handling, but I've got to confess, in time
> honoured tradition I simply copied this from EVM crypto without
> actually digging into the code to understand why.
>

Yeah, it is notoriously hard to use, and we should try to improve that.

>>  However, I take it this means that hmac_init() is never called in
>> contexts where sleeping is not allowed? For the relevance of that,
>> please see below.
>
> Right, these routines are always called as an adjunct to a TPM
> operation and TPM operations can sleep, so we must at least have kernel
> thread context.
>
> [...]
>> > +       /* encrypt before HMAC */
>> > +       if (auth->attrs & TPM2_SA_DECRYPT) {
>> > +               struct scatterlist sg[1];
>> > +               u16 len;
>> > +               SKCIPHER_REQUEST_ON_STACK(req, auth->aes);
>> > +
>> > +               skcipher_request_set_tfm(req, auth->aes);
>> > +               skcipher_request_set_callback(req,
>> > CRYPTO_TFM_REQ_MAY_SLEEP,
>> > +                                             NULL, NULL);
>> > +
>>
>> Your crypto_alloc_skcipher() call [further down] does not mask out
>> CRYPTO_ALG_ASYNC, and so it permits asynchronous implementations to
>> be selected. Your generic cfb template only permits synchronous
>> implementations, since it wraps the cipher directly (which is always
>> synchronous). However, we have support in the tree for some
>> accelerators that implement cfb(aes), and those will return
>> -EINPROGRESS when invoking crypto_skcipher_en/decrypt(req), which you
>> are not set up to handle.
>>
>> So the simple solution is to call 'crypto_alloc_skcipher("cfb(aes)",
>> 0, CRYPTO_ALG_ASYNC)' below instead.
>>
>> However, I would prefer it if we permit asynchronous skciphers here.
>> The reason is that, on a given platform, the accelerator may be the
>> only truly time invariant AES implementation that is available, and
>> since we are dealing with the TPM, a bit of paranoia does not hurt.
>> It also makes it easier to implement it as a [time invariant] SIMD
>> based asynchronous skcipher, which are simpler than synchronous ones
>> since they don't require a non-SIMD fallback path for calls from
>> contexts where the SIMD unit may not be used.
>>
>> I have already implemented cfb(aes) for arm64/NEON. I will post the
>> patch after the merge window closes.
>>
>> > +               /* need key and IV */
>> > +               KDFa(auth->session_key, SHA256_DIGEST_SIZE
>> > +                    + auth->passphraselen, "CFB", auth->our_nonce,
>> > +                    auth->tpm_nonce, AES_KEYBYTES +
>> > AES_BLOCK_SIZE,
>> > +                    auth->scratch);
>> > +               crypto_skcipher_setkey(auth->aes, auth->scratch,
>> > AES_KEYBYTES);
>> > +               len = tpm_get_inc_u16(&p);
>> > +               sg_init_one(sg, p, len);
>> > +               skcipher_request_set_crypt(req, sg, sg, len,
>> > +                                          auth->scratch +
>> > AES_KEYBYTES);
>> > +               crypto_skcipher_encrypt(req);
>>
>> So please consider replacing this with something like.
>>
>> DECLARE_CRYPTO_WAIT(wait); [further up]
>> skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP,
>>                               crypto_req_done, &wait);
>> crypto_wait_req(crypto_skcipher_encrypt(req), &wait);
>
> Sure, I can do this ... the crypto skcipher handling was also cut and
> paste, but I forget where from this time.  So what I think you're
> asking for is below as the incremental diff?  I've tested this out and
> it all works fine in my session testing environment (and on my real
> laptop) ... although since I'm fully sync, I won't really have tested
> the -EINPROGRESS do the wait case.
>

Yes that looks fine. I will try to test this on one of my arm64
systems, but it may take me some time to get around to it.

In any case,

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> # crypto API parts

>
> diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
> index 422c3ec64f8c..bbd3e8580954 100644
> --- a/drivers/char/tpm/tpm2-sessions.c
> +++ b/drivers/char/tpm/tpm2-sessions.c
> @@ -165,7 +165,6 @@ static void hmac_init(struct shash_desc *desc, u8 *key, int keylen)
>         int i;
>
>         desc->tfm = sha256_hash;
> -       desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
>         crypto_shash_init(desc);
>         for (i = 0; i < sizeof(pad); i++) {
>                 if (i < keylen)
> @@ -244,7 +243,6 @@ static void KDFe(u8 z[EC_PT_SZ], const char *str, u8 *pt_u, u8 *pt_v,
>         __be32 c = cpu_to_be32(1);
>
>         desc->tfm = sha256_hash;
> -       desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
>
>         crypto_shash_init(desc);
>         /* counter (BE) */
> @@ -445,7 +443,6 @@ void tpm_buf_fill_hmac_session(struct tpm_buf *buf, struct tpm2_auth *auth)
>         auth->ordinal = head->ordinal;
>
>         desc->tfm = sha256_hash;
> -       desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
>
>         cc = be32_to_cpu(head->ordinal);
>
> @@ -514,10 +511,11 @@ void tpm_buf_fill_hmac_session(struct tpm_buf *buf, struct tpm2_auth *auth)
>                 struct scatterlist sg[1];
>                 u16 len;
>                 SKCIPHER_REQUEST_ON_STACK(req, auth->aes);
> +               DECLARE_CRYPTO_WAIT(wait);
>
>                 skcipher_request_set_tfm(req, auth->aes);
>                 skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP,
> -                                             NULL, NULL);
> +                                             crypto_req_done, &wait);
>
>                 /* need key and IV */
>                 KDFa(auth->session_key, SHA256_DIGEST_SIZE
> @@ -529,7 +527,7 @@ void tpm_buf_fill_hmac_session(struct tpm_buf *buf, struct tpm2_auth *auth)
>                 sg_init_one(sg, p, len);
>                 skcipher_request_set_crypt(req, sg, sg, len,
>                                            auth->scratch + AES_KEYBYTES);
> -               crypto_skcipher_encrypt(req);
> +               crypto_wait_req(crypto_skcipher_encrypt(req), &wait);
>                 /* reset p to beginning of parameters for HMAC */
>                 p -= 2;
>         }
> @@ -755,7 +753,6 @@ int tpm_buf_check_hmac_response(struct tpm_buf *buf, struct tpm2_auth *auth,
>          * with rphash
>          */
>         desc->tfm = sha256_hash;
> -       desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
>         crypto_shash_init(desc);
>         /* yes, I know this is now zero, but it's what the standard says */
>         crypto_shash_update(desc, (u8 *)&head->return_code,
> @@ -786,10 +783,11 @@ int tpm_buf_check_hmac_response(struct tpm_buf *buf, struct tpm2_auth *auth,
>         if (auth->attrs & TPM2_SA_ENCRYPT) {
>                 struct scatterlist sg[1];
>                 SKCIPHER_REQUEST_ON_STACK(req, auth->aes);
> +               DECLARE_CRYPTO_WAIT(wait);
>
>                 skcipher_request_set_tfm(req, auth->aes);
>                 skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP,
> -                                             NULL, NULL);
> +                                             crypto_req_done, &wait);
>
>                 /* need key and IV */
>                 KDFa(auth->session_key, SHA256_DIGEST_SIZE
> @@ -801,7 +799,7 @@ int tpm_buf_check_hmac_response(struct tpm_buf *buf, struct tpm2_auth *auth,
>                 sg_init_one(sg, p, len);
>                 skcipher_request_set_crypt(req, sg, sg, len,
>                                            auth->scratch + AES_KEYBYTES);
> -               crypto_skcipher_decrypt(req);
> +               crypto_wait_req(crypto_skcipher_decrypt(req), &wait);
>         }
>
>   out:
> @@ -965,7 +963,6 @@ static int parse_create_primary(struct tpm_chip *chip, u8 *data)
>         tmp = resp;
>         /* now we have the public area, compute the name of the object */
>         desc->tfm = sha256_hash;
> -       desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
>         put_unaligned_be16(TPM2_ALG_SHA256, chip->tpmkeyname);
>         crypto_shash_init(desc);
>         crypto_shash_update(desc, resp, len);

Re: [PATCH v4 1/7] tpm-buf: create new functions for handling TPM buffers

[Jarkko Sakkinen]

On Mon, 22 Oct 2018, James Bottomley wrote:
> This separates out the old tpm_buf_... handling functions from static
> inlines into tpm.h and makes them their own tpm-buf.c file.  It also
> adds handling for tpm2b structures and also incremental pointer
> advancing parsers.

Nitpicking: when my SGX patch set has been reviewed one of the comments
has been that commit messages should be in imperative form e.g.

"Separate out the old tpm_buf handling functions from static inlines
into tpm.h and make them their..."

> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
>
> ---
>
> v2: added this patch to separate out the API changes
> v3: added tpm_buf_reset_cmd()
> v4: renamed tpm_buf_reset_cmd() to tpm_buf_reset()
> ---
> drivers/char/tpm/Makefile  |   2 +-
> drivers/char/tpm/tpm-buf.c | 191 +++++++++++++++++++++++++++++++++++++++++++++
> drivers/char/tpm/tpm.h     |  96 ++++-------------------
> 3 files changed, 208 insertions(+), 81 deletions(-)
> create mode 100644 drivers/char/tpm/tpm-buf.c
>
> diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
> index 3655258bee73..65d165cce530 100644
> --- a/drivers/char/tpm/Makefile
> +++ b/drivers/char/tpm/Makefile
> @@ -5,7 +5,7 @@
> obj-$(CONFIG_TCG_TPM) += tpm.o
> tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
> 	 tpm-dev-common.o tpmrm-dev.o eventlog/common.o eventlog/tpm1.o \
> -	 eventlog/tpm2.o tpm2-space.o
> +	 eventlog/tpm2.o tpm2-space.o tpm-buf.o
> tpm-$(CONFIG_ACPI) += tpm_ppi.o eventlog/acpi.o
> tpm-$(CONFIG_EFI) += eventlog/efi.o
> tpm-$(CONFIG_OF) += eventlog/of.o
> diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
> new file mode 100644
> index 000000000000..faa22bb09d99
> --- /dev/null
> +++ b/drivers/char/tpm/tpm-buf.c
> @@ -0,0 +1,191 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Handing for tpm2b structures to facilitate the building of commands
> + */
> +
> +#include "tpm.h"
> +
> +#include <linux/module.h>
> +
> +#include <asm/unaligned.h>
> +
> +static int __tpm_buf_init(struct tpm_buf *buf)
> +{
> +	buf->data_page = alloc_page(GFP_HIGHUSER);
> +	if (!buf->data_page)
> +		return -ENOMEM;
> +
> +	buf->flags = 0;
> +	buf->data = kmap(buf->data_page);
> +
> +	return 0;
> +}
> +
> +void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal)
> +{
> +	struct tpm_input_header *head;
> +
> +	head = (struct tpm_input_header *) buf->data;
> +
> +	head->tag = cpu_to_be16(tag);
> +	head->length = cpu_to_be32(sizeof(*head));
> +	head->ordinal = cpu_to_be32(ordinal);
> +}
> +EXPORT_SYMBOL_GPL(tpm_buf_reset);
> +
> +int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
> +{
> +	int rc;
> +
> +	rc = __tpm_buf_init(buf);
> +	if (rc)
> +		return rc;
> +
> +	tpm_buf_reset(buf, tag, ordinal);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(tpm_buf_init);
> +
> +int tpm_buf_init_2b(struct tpm_buf *buf)
> +{
> +	struct tpm_input_header *head;
> +	int rc;
> +
> +	rc = __tpm_buf_init(buf);
> +	if (rc)
> +		return rc;
> +
> +	head = (struct tpm_input_header *) buf->data;
> +
> +	head->length = cpu_to_be32(sizeof(*head));
> +
> +	buf->flags = TPM_BUF_2B;
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(tpm_buf_init_2b);
> +
> +void tpm_buf_destroy(struct tpm_buf *buf)
> +{
> +	kunmap(buf->data_page);
> +	__free_page(buf->data_page);
> +}
> +EXPORT_SYMBOL_GPL(tpm_buf_destroy);
> +
> +static void *tpm_buf_data(struct tpm_buf *buf)
> +{
> +	if (buf->flags & TPM_BUF_2B)
> +		return buf->data + TPM_HEADER_SIZE;
> +	return buf->data;
> +}
> +
> +u32 tpm_buf_length(struct tpm_buf *buf)
> +{
> +	struct tpm_input_header *head = (struct tpm_input_header *)buf->data;
> +	u32 len;
> +
> +	len = be32_to_cpu(head->length);
> +	if (buf->flags & TPM_BUF_2B)
> +		len -= sizeof(*head);
> +	return len;
> +}
> +EXPORT_SYMBOL_GPL(tpm_buf_length);
> +
> +u16 tpm_buf_tag(struct tpm_buf *buf)
> +{
> +	struct tpm_input_header *head = (struct tpm_input_header *)buf->data;
> +
> +	return be16_to_cpu(head->tag);
> +}
> +EXPORT_SYMBOL_GPL(tpm_buf_tag);
> +
> +void tpm_buf_append(struct tpm_buf *buf,
> +		    const unsigned char *new_data,
> +		    unsigned int new_len)
> +{
> +	struct tpm_input_header *head = (struct tpm_input_header *) buf->data;
> +	u32 len = be32_to_cpu(head->length);
> +
> +	/* Return silently if overflow has already happened. */
> +	if (buf->flags & TPM_BUF_OVERFLOW)
> +		return;
> +
> +	if ((len + new_len) > PAGE_SIZE) {
> +		WARN(1, "tpm_buf: overflow\n");
> +		buf->flags |= TPM_BUF_OVERFLOW;
> +		return;
> +	}
> +
> +	memcpy(&buf->data[len], new_data, new_len);
> +	head->length = cpu_to_be32(len + new_len);
> +}
> +EXPORT_SYMBOL_GPL(tpm_buf_append);
> +
> +void tpm_buf_append_u8(struct tpm_buf *buf, const u8 value)
> +{
> +	tpm_buf_append(buf, &value, 1);
> +}
> +EXPORT_SYMBOL_GPL(tpm_buf_append_u8);
> +
> +void tpm_buf_append_u16(struct tpm_buf *buf, const u16 value)
> +{
> +	__be16 value2 = cpu_to_be16(value);
> +
> +	tpm_buf_append(buf, (u8 *) &value2, 2);
> +}
> +EXPORT_SYMBOL_GPL(tpm_buf_append_u16);
> +
> +void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
> +{
> +	__be32 value2 = cpu_to_be32(value);
> +
> +	tpm_buf_append(buf, (u8 *) &value2, 4);
> +}
> +EXPORT_SYMBOL_GPL(tpm_buf_append_u32);
> +
> +static void tpm_buf_reset_int(struct tpm_buf *buf)
> +{
> +	struct tpm_input_header *head;
> +
> +	head = (struct tpm_input_header *)buf->data;
> +	head->length = cpu_to_be32(sizeof(*head));
> +}
> +
> +void tpm_buf_append_2b(struct tpm_buf *buf, struct tpm_buf *tpm2b)
> +{
> +	u16 len = tpm_buf_length(tpm2b);
> +
> +	tpm_buf_append_u16(buf, len);
> +	tpm_buf_append(buf, tpm_buf_data(tpm2b), len);
> +	/* clear the buf for reuse */
> +	tpm_buf_reset_int(tpm2b);
> +}
> +EXPORT_SYMBOL_GPL(tpm_buf_append_2b);

I think it would be more clean to rename tpm_buf_reset_int() as
tpm_buf_reset_2b() and call it outside tpm_buf_append_2b(). Easier
to understand later on in the call site what is going on.

> +
> +/* functions for unmarshalling data and moving the cursor */
> +u8 tpm_get_inc_u8(const u8 **ptr)
> +{
> +	return *((*ptr)++);
> +}
> +EXPORT_SYMBOL_GPL(tpm_get_inc_u8);
> +
> +u16 tpm_get_inc_u16(const u8 **ptr)
> +{
> +	u16 val;
> +
> +	val = get_unaligned_be16(*ptr);
> +	*ptr += sizeof(val);
> +	return val;
> +}
> +EXPORT_SYMBOL_GPL(tpm_get_inc_u16);
> +
> +u32 tpm_get_inc_u32(const u8 **ptr)
> +{
> +	u32 val;
> +
> +	val = get_unaligned_be32(*ptr);
> +	*ptr += sizeof(val);
> +	return val;
> +}
> +EXPORT_SYMBOL_GPL(tpm_get_inc_u32);

These tpm_get_ functions should be first of all documented and I would
use tpm_buf_ prefix also in these simply because they are used in
conjuction with that stuff. Maybe we could call the parameter simply as
'iter' as that is what it is? These definitely should be in their own
commit.

> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index f3501d05264f..d73701e36eba 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -416,6 +416,7 @@ struct tpm_cmd_t {
>
> enum tpm_buf_flags {
> 	TPM_BUF_OVERFLOW	= BIT(0),
> +	TPM_BUF_2B		= BIT(1),
> };
>
> struct tpm_buf {
> @@ -424,86 +425,21 @@ struct tpm_buf {
> 	u8 *data;
> };
>
> -static inline void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal)
> -{
> -	struct tpm_input_header *head;
> -	head = (struct tpm_input_header *)buf->data;
> -	head->tag = cpu_to_be16(tag);
> -	head->length = cpu_to_be32(sizeof(*head));
> -	head->ordinal = cpu_to_be32(ordinal);
> -}
> -
> -static inline int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
> -{
> -	buf->data_page = alloc_page(GFP_HIGHUSER);
> -	if (!buf->data_page)
> -		return -ENOMEM;
> -
> -	buf->flags = 0;
> -	buf->data = kmap(buf->data_page);
> -	tpm_buf_reset(buf, tag, ordinal);
> -	return 0;
> -}
> -
> -static inline void tpm_buf_destroy(struct tpm_buf *buf)
> -{
> -	kunmap(buf->data_page);
> -	__free_page(buf->data_page);
> -}
> -
> -static inline u32 tpm_buf_length(struct tpm_buf *buf)
> -{
> -	struct tpm_input_header *head = (struct tpm_input_header *) buf->data;
> -
> -	return be32_to_cpu(head->length);
> -}
> -
> -static inline u16 tpm_buf_tag(struct tpm_buf *buf)
> -{
> -	struct tpm_input_header *head = (struct tpm_input_header *) buf->data;
> -
> -	return be16_to_cpu(head->tag);
> -}
> -
> -static inline void tpm_buf_append(struct tpm_buf *buf,
> -				  const unsigned char *new_data,
> -				  unsigned int new_len)
> -{
> -	struct tpm_input_header *head = (struct tpm_input_header *) buf->data;
> -	u32 len = tpm_buf_length(buf);
> -
> -	/* Return silently if overflow has already happened. */
> -	if (buf->flags & TPM_BUF_OVERFLOW)
> -		return;
> -
> -	if ((len + new_len) > PAGE_SIZE) {
> -		WARN(1, "tpm_buf: overflow\n");
> -		buf->flags |= TPM_BUF_OVERFLOW;
> -		return;
> -	}
> -
> -	memcpy(&buf->data[len], new_data, new_len);
> -	head->length = cpu_to_be32(len + new_len);
> -}
> -
> -static inline void tpm_buf_append_u8(struct tpm_buf *buf, const u8 value)
> -{
> -	tpm_buf_append(buf, &value, 1);
> -}
> -
> -static inline void tpm_buf_append_u16(struct tpm_buf *buf, const u16 value)
> -{
> -	__be16 value2 = cpu_to_be16(value);
> -
> -	tpm_buf_append(buf, (u8 *) &value2, 2);
> -}
> -
> -static inline void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
> -{
> -	__be32 value2 = cpu_to_be32(value);
> -
> -	tpm_buf_append(buf, (u8 *) &value2, 4);
> -}
> +int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal);
> +void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal);
> +int tpm_buf_init_2b(struct tpm_buf *buf);
> +void tpm_buf_destroy(struct tpm_buf *buf);
> +u32 tpm_buf_length(struct tpm_buf *buf);
> +void tpm_buf_append(struct tpm_buf *buf, const unsigned char *new_data,
> +		    unsigned int new_len);
> +void tpm_buf_append_u8(struct tpm_buf *buf, const u8 value);
> +void tpm_buf_append_u16(struct tpm_buf *buf, const u16 value);
> +void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value);
> +void tpm_buf_append_2b(struct tpm_buf *buf, struct tpm_buf *tpm2b);
> +
> +u8 tpm_get_inc_u8(const u8 **ptr);
> +u16 tpm_get_inc_u16(const u8 **ptr);
> +u32 tpm_get_inc_u32(const u8 **ptr);
>
> extern struct class *tpm_class;
> extern struct class *tpmrm_class;
> -- 
> 2.16.4
>
>

Please, split this into three commits:

1. Move tpm-buf functions to tpm-buf.c.
2. Extensions to the buffer construction (i.e. TPM2B).
3. The parsing code.

Then we can see clearly what is going on and properly evaluate these
changes.

/Jarkko

Re: [PATCH v4 1/7] tpm-buf: create new functions for handling TPM buffers

[Jarkko Sakkinen]

On Mon, 22 Oct 2018, James Bottomley wrote:
> This separates out the old tpm_buf_... handling functions from static
> inlines into tpm.h and makes them their own tpm-buf.c file.  It also
> adds handling for tpm2b structures and also incremental pointer
> advancing parsers.
>
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>

Would also prefer simply "tpm:" tag e.g. (in all of the commits actually
in this series but I remark it just here).

"tpm: create new tpm_buf_functions for parsing and TPM2B"

/Jarkko

Re: [PATCH v4 2/7] tpm2-sessions: Add full HMAC and encrypt/decrypt session handling

[Jarkko Sakkinen]

On Mon, 22 Oct 2018, James Bottomley wrote:
> This code adds true session based HMAC authentication plus parameter
> decryption and response encryption using AES.

In order to reduce complexity it would make sense to split into two
commits: authentication and parameter encryption.

>
> The basic design of this code is to segregate all the nasty crypto,
> hash and hmac code into tpm2-sessions.c and export a usable API.
>
> The API first of all starts off by gaining a session with
>
> tpm2_start_auth_session()
>
> Which initiates a session with the TPM and allocates an opaque
> tpm2_auth structure to handle the session parameters.  Then the use is
> simply:
>
> * tpm_buf_append_name() in place of the tpm_buf_append_u32 for the
>  handles

Do not understand the description of this function.

>
> * tpm_buf_append_hmac_session() where tpm2_append_auth() would go

Another fuzzy description.

>
> * tpm_buf_fill_hmac_session() called after the entire command buffer
>  is finished but before tpm_transmit_cmd() is called which computes
>  the correct HMAC and places it in the command at the correct
>  location.

I would call this simply tpm_buf_insert_hmac(). It is clear and precise
name what it does.

These all sould be combined with the existing session stuff inside
tpm2-cmd.c and not have duplicate infrastructures. The file name should
be tpm2-session.c (we neither have tpm2-cmds.c).

>
> Finally, after tpm_transmit_cmd() is called,
> tpm_buf_check_hmac_response() is called to check that the returned
> HMAC matched and collect the new state for the next use of the
> session, if any.
>
> The features of the session is controlled by the session attributes
> set in tpm_buf_append_hmac_session().  If TPM2_SA_CONTINUE_SESSION is
> not specified, the session will be flushed and the tpm2_auth structure
> freed in tpm_buf_check_hmac_response(); otherwise the session may be
> used again.  Parameter encryption is specified by or'ing the flag
> TPM2_SA_DECRYPT and response encryption by or'ing the flag
> TPM2_SA_ENCRYPT.  the various encryptions will be taken care of by
> tpm_buf_fill_hmac_session() and tpm_buf_check_hmac_response()
> respectively.
>
> To get all of this to work securely, the Kernel now needs a primary
> key to encrypt the session salt to, so we derive an EC key from the
> NULL seed and store it in the tpm_chip structure.  We also make sure
> that this seed remains for the kernel by using a kernel space to take
> it out of the TPM when userspace wants to use it.
>
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
>
> ---
>
> v2: Added docbook and improved response check API
> v3: Add readpublic, fix hmac length, add API for close on error
>    allow for the hmac session not being first in the sessions
> v4: Make NULL seed template exactly match the SRK ECC template.
>    Also check the NULL primary key name is what getpublic returns
>    to prevent spoofing.  Also parametrise the name size for reuse
> ---
> drivers/char/tpm/Kconfig         |    3 +
> drivers/char/tpm/Makefile        |    2 +-
> drivers/char/tpm/tpm.h           |   32 +
> drivers/char/tpm/tpm2-cmd.c      |   34 +-
> drivers/char/tpm/tpm2-sessions.c | 1188 ++++++++++++++++++++++++++++++++++++++
> drivers/char/tpm/tpm2-sessions.h |   57 ++
> 6 files changed, 1300 insertions(+), 16 deletions(-)
> create mode 100644 drivers/char/tpm/tpm2-sessions.c
> create mode 100644 drivers/char/tpm/tpm2-sessions.h
>

tpm2-cmd.c changes should be in their own commit e.g.:

"tpm: add own space for in-kernel TPM communication"

> diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
> index 0aee88df98d1..8c714d8550c4 100644
> --- a/drivers/char/tpm/Kconfig
> +++ b/drivers/char/tpm/Kconfig
> @@ -8,6 +8,9 @@ menuconfig TCG_TPM
> 	select SECURITYFS
> 	select CRYPTO
> 	select CRYPTO_HASH_INFO
> +	select CRYPTO_ECDH
> +	select CRYPTO_AES
> +	select CRYPTO_CFB
> 	---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/Makefile b/drivers/char/tpm/Makefile
> index 65d165cce530..1b5f6ccbb86d 100644
> --- a/drivers/char/tpm/Makefile
> +++ b/drivers/char/tpm/Makefile
> @@ -5,7 +5,7 @@
> obj-$(CONFIG_TCG_TPM) += tpm.o
> tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
> 	 tpm-dev-common.o tpmrm-dev.o eventlog/common.o eventlog/tpm1.o \
> -	 eventlog/tpm2.o tpm2-space.o tpm-buf.o
> +	 eventlog/tpm2.o tpm2-space.o tpm-buf.o tpm2-sessions.o
> tpm-$(CONFIG_ACPI) += tpm_ppi.o eventlog/acpi.o
> tpm-$(CONFIG_EFI) += eventlog/efi.o
> tpm-$(CONFIG_OF) += eventlog/of.o
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index d73701e36eba..d39065d9995d 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -42,6 +42,15 @@
> #include <asm/intel-family.h>
> #endif
>
> +/* fixed define for the curve we use which is NIST_P256 */
> +#define EC_PT_SZ	32
> +
> +/*
> + * fixed define for the size of a name.  This is actually HASHALG size
> + * plus 2, so 32 for SHA256
> + */
> +#define TPM2_NAME_SIZE	34

Please, remove this definition completely. It only causes confusion.

> +
> enum tpm_const {
> 	TPM_MINOR = 224,	/* officially assigned */
> 	TPM_BUFSIZE = 4096,
> @@ -103,6 +112,7 @@ enum tpm2_timeouts {
> enum tpm2_structures {
> 	TPM2_ST_NO_SESSIONS	= 0x8001,
> 	TPM2_ST_SESSIONS	= 0x8002,
> +	TPM2_ST_CREATION	= 0x8021,
> };
>
> /* Indicates from what layer of the software stack the error comes from */
> @@ -125,12 +135,20 @@ enum tpm2_return_codes {
> enum tpm2_algorithms {
> 	TPM2_ALG_ERROR		= 0x0000,
> 	TPM2_ALG_SHA1		= 0x0004,
> +	TPM2_ALG_AES		= 0x0006,
> 	TPM2_ALG_KEYEDHASH	= 0x0008,
> 	TPM2_ALG_SHA256		= 0x000B,
> 	TPM2_ALG_SHA384		= 0x000C,
> 	TPM2_ALG_SHA512		= 0x000D,
> 	TPM2_ALG_NULL		= 0x0010,
> 	TPM2_ALG_SM3_256	= 0x0012,
> +	TPM2_ALG_ECC		= 0x0023,
> +	TPM2_ALG_CFB		= 0x0043,
> +};
> +
> +enum tpm2_curves {
> +	TPM2_ECC_NONE		= 0x0000,
> +	TPM2_ECC_NIST_P256	= 0x0003,
> };
>
> enum tpm2_command_codes {
> @@ -145,6 +163,8 @@ enum tpm2_command_codes {
> 	TPM2_CC_CONTEXT_LOAD	= 0x0161,
> 	TPM2_CC_CONTEXT_SAVE	= 0x0162,
> 	TPM2_CC_FLUSH_CONTEXT	= 0x0165,
> +	TPM2_CC_READ_PUBLIC	= 0x0173,
> +	TPM2_CC_START_AUTH_SESS	= 0x0176,
> 	TPM2_CC_GET_CAPABILITY	= 0x017A,
> 	TPM2_CC_GET_RANDOM	= 0x017B,
> 	TPM2_CC_PCR_READ	= 0x017E,
> @@ -153,6 +173,7 @@ enum tpm2_command_codes {
> };
>
> enum tpm2_permanent_handles {
> +	TPM2_RH_NULL		= 0x40000007,
> 	TPM2_RS_PW		= 0x40000009,
> };
>
> @@ -255,11 +276,18 @@ struct tpm_chip {
> #endif /* CONFIG_ACPI */
>
> 	struct tpm_space work_space;
> +	struct tpm_space kernel_space;
> 	u32 nr_commands;
> 	u32 *cc_attrs_tbl;
>
> 	/* active locality */
> 	int locality;
> +
> +	/* details for communication security via sessions */
> +	u32 tpmkey;
> +	u8 tpmkeyname[TPM2_NAME_SIZE];

u32 tpm_key_handle;
u8 tpm_key_name[SHA256_DIGEST_SIZE + 2];

The comment above these variables is complete bogus (tells nothing
about nothing).

> +	u8 ec_point_x[EC_PT_SZ];
> +	u8 ec_point_y[EC_PT_SZ];

A comment above these would be good with a description what these are
and what they are used for.

> };
>
> #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)
> @@ -441,6 +469,9 @@ u8 tpm_get_inc_u8(const u8 **ptr);
> u16 tpm_get_inc_u16(const u8 **ptr);
> u32 tpm_get_inc_u32(const u8 **ptr);
>
> +/* opaque structure, holds auth session parameters like the session key */
> +struct tpm2_auth;
> +
> extern struct class *tpm_class;
> extern struct class *tpmrm_class;
> extern dev_t tpm_devt;
> @@ -540,4 +571,5 @@ int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
>
> int tpm_bios_log_setup(struct tpm_chip *chip);
> void tpm_bios_log_teardown(struct tpm_chip *chip);
> +int tpm2_sessions_init(struct tpm_chip *chip);
> #endif
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index c31b490bd41d..a17e5c573c4e 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -16,17 +16,10 @@
>  */
>
> #include "tpm.h"
> +#include "tpm2-sessions.h"
> #include <crypto/hash_info.h>
> #include <keys/trusted-type.h>
>
> -enum tpm2_object_attributes {
> -	TPM2_OA_USER_WITH_AUTH		= BIT(6),
> -};
> -
> -enum tpm2_session_attributes {
> -	TPM2_SA_CONTINUE_SESSION	= BIT(0),
> -};
> -
> struct tpm2_hash {
> 	unsigned int crypto_id;
> 	unsigned int tpm_id;
> @@ -350,15 +343,19 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
> /**
>  * tpm2_flush_context_cmd() - execute a TPM2_FlushContext command
>  * @chip: TPM chip to use
> - * @payload: the key data in clear and encrypted form
> - * @options: authentication values and other options
> + * @handle: the handle to flush
> + * @flags: flags for transmit
>  *
> + * Note: @flags may be TPM_TRANSMIT_UNLOCKED or 0.  If @flags is 0, we
> + * assume the flush handle is going through the kernel space.
> +
>  * Return: same as with tpm_transmit_cmd
>  */
> void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
> 			    unsigned int flags)
> {
> 	struct tpm_buf buf;
> +	struct tpm_space *space = NULL;
> 	int rc;
>
> 	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT);
> @@ -367,10 +364,12 @@ void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
> 			 handle);
> 		return;
> 	}
> +	if (!flags)
> +		space = &chip->kernel_space;
>
> 	tpm_buf_append_u32(&buf, handle);
>
> -	(void) tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, flags,
> +	(void) tpm_transmit_cmd(chip, space, buf.data, PAGE_SIZE, 0, flags,
> 				"flushing context");
>
> 	tpm_buf_destroy(&buf);
> @@ -387,10 +386,10 @@ void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
>  * @hmac: the session HMAC or password, may be NULL if not used
>  * @hmac_len: the session HMAC or password length, maybe 0 if not used
>  */
> -static void tpm2_buf_append_auth(struct tpm_buf *buf, u32 session_handle,
> -				 const u8 *nonce, u16 nonce_len,
> -				 u8 attributes,
> -				 const u8 *hmac, u16 hmac_len)
> +void tpm2_buf_append_auth(struct tpm_buf *buf, u32 session_handle,
> +			  const u8 *nonce, u16 nonce_len,
> +			  u8 attributes,
> +			  const u8 *hmac, u16 hmac_len)
> {
> 	tpm_buf_append_u32(buf, 9 + nonce_len + hmac_len);
> 	tpm_buf_append_u32(buf, session_handle);
> @@ -1015,6 +1014,11 @@ int tpm2_auto_startup(struct tpm_chip *chip)
>
> 	rc = tpm2_get_cc_attrs_tbl(chip);
>
> +	if (rc)

Remove the newline in-between.

> +		goto out;
> +
> +	rc = tpm2_sessions_init(chip);
> +
> out:
> 	if (rc > 0)
> 		rc = -ENODEV;
> diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
> new file mode 100644
> index 000000000000..422c3ec64f8c
> --- /dev/null
> +++ b/drivers/char/tpm/tpm2-sessions.c
> @@ -0,0 +1,1188 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Copyright (C) 2018 James.Bottomley@HansenPartnership.com
> + *
> + * Cryptographic helper routines for handling TPM2 sessions for
> + * authorization HMAC and request response encryption.
> + *
> + * The idea is to ensure that every TPM command is HMAC protected by a
> + * session, meaning in-flight tampering would be detected and in
> + * addition all sensitive inputs and responses should be encrypted.
> + *
> + * The basic way this works is to use a TPM feature called salted
> + * sessions where a random secret used in session construction is
> + * encrypted to the public part of a known TPM key.  The problem is we
> + * have no known keys, so initially a primary Elliptic Curve key is
> + * derived from the NULL seed (we use EC because most TPMs generate
> + * these keys much faster than RSA ones).  The curve used is NIST_P256
> + * because that's now mandated to be present in 'TCG TPM v2.0
> + * Provisioning Guidance'
> + *
> + * Threat problems: the initial TPM2_CreatePrimary is not (and cannot
> + * be) session protected, so a clever Man in the Middle could return a
> + * public key they control to this command and from there intercept
> + * and decode all subsequent session based transactions.  The kernel
> + * cannot mitigate this threat but, after boot, userspace can get
> + * proof this has not happened by asking the TPM to certify the NULL
> + * key.  This certification would chain back to the TPM Endorsement
> + * Certificate and prove the NULL seed primary had not been tampered
> + * with and thus all sessions must have been cryptographically secure.
> + * To assist with this, the initial NULL seed public key name is made
> + * available in a sysfs file.
> + *
> + * Use of these functions:
> + *
> + * The design is all the crypto, hash and hmac gunk is confined in this
> + * file and never needs to be seen even by the kernel internal user.  To
> + * the user there's an init function tpm2_sessions_init() that needs to
> + * be called once per TPM which generates the NULL seed primary key.
> + *
> + * Then there are six usage functions:
> + *
> + * tpm2_start_auth_session() which allocates the opaque auth structure
> + *	and gets a session from the TPM.  This must be called before
> + *	any of the following functions.  The session is protected by a
> + *	session_key which is derived from a random salt value
> + *	encrypted to the NULL seed.
> + * tpm2_end_auth_session() kills the session and frees the resources.
> + *	Under normal operation this function is done by
> + *	tpm_buf_check_hmac_response(), so this is only to be used on
> + *	error legs where the latter is not executed.
> + * tpm_buf_append_name() to add a handle to the buffer.  This must be
> + *	used in place of the usual tpm_buf_append_u32() for adding
> + *	handles because handles have to be processed specially when
> + *	calculating the HMAC.  In particular, for NV, volatile and
> + *	permanent objects you now need to provide the name.
> + * tpm_buf_append_hmac_session() which appends the hmac session to the
> + *	buf in the same way tpm_buf_append_auth does().
> + * tpm_buf_fill_hmac_session() This calculates the correct hash and
> + *	places it in the buffer.  It must be called after the complete
> + *	command buffer is finalized so it can fill in the correct HMAC
> + *	based on the parameters.
> + * tpm_buf_check_hmac_response() which checks the session response in
> + *	the buffer and calculates what it should be.  If there's a
> + *	mismatch it will log a warning and return an error.  If
> + *	tpm_buf_append_hmac_session() did not specify
> + *	TPM_SA_CONTINUE_SESSION then the session will be closed (if it
> + *	hasn't been consumed) and the auth structure freed.

Please, move these to the correct places in the file and use kdoc.

> + */
> +
> +#include "tpm.h"
> +#include "tpm2-sessions.h"
> +
> +#include <linux/random.h>
> +#include <linux/scatterlist.h>
> +
> +#include <asm/unaligned.h>
> +
> +#include <crypto/aes.h>
> +#include <crypto/kpp.h>
> +#include <crypto/ecdh.h>
> +#include <crypto/hash.h>
> +#include <crypto/hmac.h>
> +#include <crypto/skcipher.h>
> +
> +/* if you change to AES256, you only need change this */
> +#define AES_KEYBYTES	AES_KEYSIZE_128
> +
> +#define AES_KEYBITS	(AES_KEYBYTES*8)
> +#define AUTH_MAX_NAMES	3
> +
> +/*
> + * This is the structure that carries all the auth information (like
> + * session handle, nonces, session key and auth) from use to use it is
> + * designed to be opaque to anything outside.
> + */
> +struct tpm2_auth {
> +	u32 handle;
> +	/*
> +	 * This has two meanings: before tpm_buf_fill_hmac_session()
> +	 * it marks the offset in the buffer of the start of the
> +	 * sessions (i.e. after all the handles).  Once the buffer has
> +	 * been filled it markes the session number of our auth
> +	 * session so we can find it again in the response buffer.
> +	 *
> +	 * The two cases are distinguished because the first offset
> +	 * must always be greater than TPM_HEADER_SIZE and the second
> +	 * must be less than or equal to 5.
> +	 */
> +	u32 session;
> +	/*
> +	 * the size here is variable and set by the size of our_nonce
> +	 * which must be between 16 and the name hash length. we set
> +	 * the maximum sha256 size for the greatest protection
> +	 */
> +	u8 our_nonce[SHA256_DIGEST_SIZE];
> +	u8 tpm_nonce[SHA256_DIGEST_SIZE];
> +	/*
> +	 * the salt is only used across the session command/response
> +	 * after that it can be used as a scratch area
> +	 */
> +	union {
> +		u8 salt[EC_PT_SZ];
> +		/* scratch for key + IV */
> +		u8 scratch[AES_KEYBYTES + AES_BLOCK_SIZE];
> +	};
> +	/*
> +	 * the session key and passphrase are the same size as the
> +	 * name digest (sha256 again).  The session key is constant
> +	 * for the use of the session and the passphrase can change
> +	 * with every invocation.
> +	 *
> +	 * Note: these fields must be adjacent and in this order
> +	 * because several HMAC/KDF schemes use the combination of the
> +	 * session_key and passphrase.
> +	 */
> +	u8 session_key[SHA256_DIGEST_SIZE];
> +	u8 passphrase[SHA256_DIGEST_SIZE];
> +	int passphraselen;
> +	/* saved session attributes */
> +	u8 attrs;
> +	__be32 ordinal;
> +	struct crypto_skcipher *aes;
> +	struct tpm_chip *chip;
> +	/* 3 names of handles: name_h is handle, name is name of handle */
> +	u32 name_h[AUTH_MAX_NAMES];
> +	u8 name[AUTH_MAX_NAMES][2 + SHA256_DIGEST_SIZE];
> +};
> +
> +/*
> + * this is our static crypto shash.  This is possible because the hash
> + * is multi-threaded and all the state stored in the desc
> + */
> +static struct crypto_shash *sha256_hash;
> +
> +/*
> + * It turns out the crypto hmac(sha256) is hard for us to consume
> + * because it assumes a fixed key and the TPM seems to change the key
> + * on every operation, so we weld the hmac init and final functions in
> + * here to give it the same usage characteristics as a regular hash
> + */
> +static void hmac_init(struct shash_desc *desc, u8 *key, int keylen)
> +{
> +	u8 pad[SHA256_BLOCK_SIZE];
> +	int i;
> +
> +	desc->tfm = sha256_hash;
> +	desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
> +	crypto_shash_init(desc);
> +	for (i = 0; i < sizeof(pad); i++) {
> +		if (i < keylen)
> +			pad[i] = key[i];
> +		else
> +			pad[i] = 0;
> +		pad[i] ^= HMAC_IPAD_VALUE;
> +	}
> +	crypto_shash_update(desc, pad, sizeof(pad));
> +}
> +
> +static void hmac_final(struct shash_desc *desc, u8 *key, int keylen, u8 *out)
> +{
> +	u8 pad[SHA256_BLOCK_SIZE];
> +	int i;
> +
> +	for (i = 0; i < sizeof(pad); i++) {
> +		if (i < keylen)
> +			pad[i] = key[i];
> +		else
> +			pad[i] = 0;
> +		pad[i] ^= HMAC_OPAD_VALUE;
> +	}
> +
> +	/* collect the final hash;  use out as temporary storage */
> +	crypto_shash_final(desc, out);
> +
> +	/* reuse the desc */
> +	crypto_shash_init(desc);
> +	crypto_shash_update(desc, pad, sizeof(pad));
> +	crypto_shash_update(desc, out, SHA256_DIGEST_SIZE);
> +	crypto_shash_final(desc, out);
> +}

Please, add 'tpm_' prefix to these functions as they are tpm internal and
it makes tracing TPM internal stuff easier.

> +
> +/*
> + * assume hash sha256 and nonces u, v of size SHA256_DIGEST_SIZE but
> + * otherwise standard KDFa.  Note output is in bytes not bits.
> + */
> +static void KDFa(u8 *key, int keylen, const char *label, u8 *u,
> +		 u8 *v, int bytes, u8 *out)

Should this be in lower case? I would rename it as tpm_kdfa().

> +{
> +	u32 counter;
> +	const __be32 bits = cpu_to_be32(bytes * 8);
> +
> +	for (counter = 1; bytes > 0; bytes -= SHA256_DIGEST_SIZE, counter++,
> +		     out += SHA256_DIGEST_SIZE) {

Only one counter is actually used for anything so this is overly
complicated and IMHO it is ok to call the counter just 'i'. Maybe just:

for (i = 1; (bytes - (i - 1) * SHA256_DIGEST_SIZE) > 0; i++) {

> +		SHASH_DESC_ON_STACK(desc, sha256_hash);
> +		__be32 c = cpu_to_be32(counter);
> +
> +		hmac_init(desc, key, keylen);
> +		crypto_shash_update(desc, (u8 *)&c, sizeof(c));
> +		crypto_shash_update(desc, label, strlen(label)+1);
> +		crypto_shash_update(desc, u, SHA256_DIGEST_SIZE);
> +		crypto_shash_update(desc, v, SHA256_DIGEST_SIZE);
> +		crypto_shash_update(desc, (u8 *)&bits, sizeof(bits));
> +		hmac_final(desc, key, keylen, out);
> +	}
> +}
> +
> +/*
> + * Somewhat of a bastardization of the real KDFe.  We're assuming
> + * we're working with known point sizes for the input parameters and
> + * the hash algorithm is fixed at sha256.  Because we know that the
> + * point size is 32 bytes like the hash size, there's no need to loop
> + * in this KDF.
> + */
> +static void KDFe(u8 z[EC_PT_SZ], const char *str, u8 *pt_u, u8 *pt_v,
> +		 u8 *keyout)
> +{
> +	SHASH_DESC_ON_STACK(desc, sha256_hash);
> +	/*
> +	 * this should be an iterative counter, but because we know
> +	 *  we're only taking 32 bytes for the point using a sha256
> +	 *  hash which is also 32 bytes, there's only one loop
> +	 */
> +	__be32 c = cpu_to_be32(1);
> +
> +	desc->tfm = sha256_hash;
> +	desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
> +
> +	crypto_shash_init(desc);
> +	/* counter (BE) */
> +	crypto_shash_update(desc, (u8 *)&c, sizeof(c));
> +	/* secret value */
> +	crypto_shash_update(desc, z, EC_PT_SZ);
> +	/* string including trailing zero */
> +	crypto_shash_update(desc, str, strlen(str)+1);
> +	crypto_shash_update(desc, pt_u, EC_PT_SZ);
> +	crypto_shash_update(desc, pt_v, EC_PT_SZ);
> +	crypto_shash_final(desc, keyout);
> +}
> +
> +static void tpm_buf_append_salt(struct tpm_buf *buf, struct tpm_chip *chip,
> +				struct tpm2_auth *auth)

Given the complexity of this function and some not that obvious choices
in the implementation (coordinates), it would make sense to document
this function.

> +{
> +	struct crypto_kpp *kpp;
> +	struct kpp_request *req;
> +	struct scatterlist s[2], d[1];
> +	struct ecdh p = {0};
> +	u8 encoded_key[EC_PT_SZ], *x, *y;

Why you use one character variable name 'p' and longer name
'encoded_key'?

> +	unsigned int buf_len;
> +	u8 *secret;
> +
> +	secret = kmalloc(EC_PT_SZ, GFP_KERNEL);
> +	if (!secret)
> +		return;
> +
> +	p.curve_id = ECC_CURVE_NIST_P256;

Could this be set already in the initialization?

> +
> +	/* secret is two sized points */
> +	tpm_buf_append_u16(buf, (EC_PT_SZ + 2)*2);

White space missing. Should be "(EC_PT_SZ + 2) * 2". The comment is a
bit obscure (maybe, do not have any specific suggestion how to make it
less obscure).

> +	/*
> +	 * we cheat here and append uninitialized data to form
> +	 * the points.  All we care about is getting the two
> +	 * co-ordinate pointers, which will be used to overwrite
> +	 * the uninitialized data
> +	 */

"unitialized data" != "random data"

Why doesn't it matter here?

> +	tpm_buf_append_u16(buf, EC_PT_SZ);
> +	x = &buf->data[tpm_buf_length(buf)];
> +	tpm_buf_append(buf, encoded_key, EC_PT_SZ);
> +	tpm_buf_append_u16(buf, EC_PT_SZ);
> +	y = &buf->data[tpm_buf_length(buf)];
> +	tpm_buf_append(buf, encoded_key, EC_PT_SZ);

The points have matching coordinates. Is that ok?

> +	sg_init_table(s, 2);
> +	sg_set_buf(&s[0], x, EC_PT_SZ);
> +	sg_set_buf(&s[1], y, EC_PT_SZ);
> +
> +	kpp = crypto_alloc_kpp("ecdh", CRYPTO_ALG_INTERNAL, 0);
> +	if (IS_ERR(kpp)) {
> +		dev_err(&chip->dev, "crypto ecdh allocation failed\n");
> +		return;
> +	}
> +
> +	buf_len = crypto_ecdh_key_len(&p);
> +	if (sizeof(encoded_key) < buf_len) {
> +		dev_err(&chip->dev, "salt buffer too small needs %d\n",
> +			buf_len);
> +		goto out;
> +	}

In what situation this can happen? Can sizeof(encoded_key) >= buf_len?

> +	crypto_ecdh_encode_key(encoded_key, buf_len, &p);
> +	/* this generates a random private key */
> +	crypto_kpp_set_secret(kpp, encoded_key, buf_len);
> +
> +	/* salt is now the public point of this private key */
> +	req = kpp_request_alloc(kpp, GFP_KERNEL);
> +	if (!req)
> +		goto out;
> +	kpp_request_set_input(req, NULL, 0);
> +	kpp_request_set_output(req, s, EC_PT_SZ*2);
> +	crypto_kpp_generate_public_key(req);
> +	/*
> +	 * we're not done: now we have to compute the shared secret
> +	 * which is our private key multiplied by the tpm_key public
> +	 * point, we actually only take the x point and discard the y
> +	 * point and feed it through KDFe to get the final secret salt
> +	 */
> +	sg_set_buf(&s[0], chip->ec_point_x, EC_PT_SZ);
> +	sg_set_buf(&s[1], chip->ec_point_y, EC_PT_SZ);
> +	kpp_request_set_input(req, s, EC_PT_SZ*2);
> +	sg_init_one(d, secret, EC_PT_SZ);
> +	kpp_request_set_output(req, d, EC_PT_SZ);
> +	crypto_kpp_compute_shared_secret(req);
> +	kpp_request_free(req);
> +
> +	/* pass the shared secret through KDFe for salt */
> +	KDFe(secret, "SECRET", x, chip->ec_point_x, auth->salt);

How is the label chosen?

> + out:
> +	crypto_free_kpp(kpp);
> +}

In general this function should have a clear explanation what it does
and maybe less these one character variables but instead variables
with more documenting names. Explain in high level what algorithms
are used and how the salt is calculated.

> +
> +/**
> + * tpm_buf_append_hmac_session() append a TPM session element
> + * @buf: The buffer to be appended
> + * @auth: the auth structure allocated by tpm2_start_auth_session()
> + * @attributes: The session attributes
> + * @passphrase: The session authority (NULL if none)
> + * @passphraselen: The length of the session authority (0 if none)

The alignment.

> + *
> + * This fills in a session structure in the TPM command buffer, except
> + * for the HMAC which cannot be computed until the command buffer is
> + * complete.  The type of session is controlled by the @attributes,
> + * the main ones of which are TPM2_SA_CONTINUE_SESSION which means the
> + * session won't terminate after tpm_buf_check_hmac_response(),
> + * TPM2_SA_DECRYPT which means this buffers first parameter should be
> + * encrypted with a session key and TPM2_SA_ENCRYPT, which means the
> + * response buffer's first parameter needs to be decrypted (confusing,
> + * but the defines are written from the point of view of the TPM).

In the commit message it would be probably relevant to acknowledge that
only the first parameter can be encrypted/decrypted.

> + *
> + * Any session appended by this command must be finalized by calling
> + * tpm_buf_fill_hmac_session() otherwise the HMAC will be incorrect
> + * and the TPM will reject the command.
> + *
> + * As with most tpm_buf operations, success is assumed because failure
> + * will be caused by an incorrect programming model and indicated by a
> + * kernel message.
> + */
> +void tpm_buf_append_hmac_session(struct tpm_buf *buf, struct tpm2_auth *auth,
> +				 u8 attributes, u8 *passphrase,
> +				 int passphraselen)

Would prefer underscore between the words.

> +{
> +	u8 nonce[SHA256_DIGEST_SIZE];
> +	u32 len;
> +
> +	/*
> +	 * The Architecture Guide requires us to strip trailing zeros
> +	 * before computing the HMAC
> +	 */
> +	while (passphrase && passphraselen > 0
> +	       && passphrase[passphraselen - 1] == '\0')
> +		passphraselen--;

Why there would be trailing zeros?

> +
> +	auth->attrs = attributes;
> +	auth->passphraselen = passphraselen;
> +	if (passphraselen)
> +		memcpy(auth->passphrase, passphrase, passphraselen);
> +
> +	if (auth->session != tpm_buf_length(buf)) {
> +		/* we're not the first session */
> +		len = get_unaligned_be32(&buf->data[auth->session]);
> +		if (4 + len + auth->session != tpm_buf_length(buf)) {
> +			WARN(1, "session length mismatch, cannot append");
> +			return;
> +		}
> +
> +		/* add our new session */
> +		len += 9 + 2 * SHA256_DIGEST_SIZE;
> +		put_unaligned_be32(len, &buf->data[auth->session]);
> +	} else {
> +		tpm_buf_append_u32(buf, 9 + 2 * SHA256_DIGEST_SIZE);
> +	}
> +
> +	/* random number for our nonce */
> +	get_random_bytes(nonce, sizeof(nonce));
> +	memcpy(auth->our_nonce, nonce, sizeof(nonce));
> +	tpm_buf_append_u32(buf, auth->handle);
> +	/* our new nonce */
> +	tpm_buf_append_u16(buf, SHA256_DIGEST_SIZE);
> +	tpm_buf_append(buf, nonce, SHA256_DIGEST_SIZE);
> +	tpm_buf_append_u8(buf, auth->attrs);
> +	/* and put a placeholder for the hmac */
> +	tpm_buf_append_u16(buf, SHA256_DIGEST_SIZE);
> +	tpm_buf_append(buf, nonce, SHA256_DIGEST_SIZE);
> +}
> +EXPORT_SYMBOL(tpm_buf_append_hmac_session);
> +
> +/**
> + * tpm_buf_fill_hmac_session() - finalize the session HMAC
> + * @buf: The buffer to be appended
> + * @auth: the auth structure allocated by tpm2_start_auth_session()
> + *
> + * This command must not be called until all of the parameters have
> + * been appended to @buf otherwise the computed HMAC will be
> + * incorrect.
> + *
> + * This function computes and fills in the session HMAC using the
> + * session key and, if TPM2_SA_DECRYPT was specified, computes the
> + * encryption key and encrypts the first parameter of the command
> + * buffer with it.
> + *
> + * As with most tpm_buf operations, success is assumed because failure
> + * will be caused by an incorrect programming model and indicated by a
> + * kernel message.
> + */
> +void tpm_buf_fill_hmac_session(struct tpm_buf *buf, struct tpm2_auth *auth)
> +{
> +	u32 cc, handles, val;
> +	struct tpm_chip *chip = auth->chip;
> +	int i;
> +	struct tpm_input_header *head = (struct tpm_input_header *)buf->data;
> +	const u8 *s, *p;
> +	u8 *hmac = NULL;
> +	u32 attrs;
> +	u8 cphash[SHA256_DIGEST_SIZE];
> +	SHASH_DESC_ON_STACK(desc, sha256_hash);
> +
> +	/* save the command code in BE format */
> +	auth->ordinal = head->ordinal;
> +
> +	desc->tfm = sha256_hash;
> +	desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
> +
> +	cc = be32_to_cpu(head->ordinal);
> +
> +	i = tpm2_find_cc(chip, cc);
> +	if (i < 0) {
> +		dev_err(&chip->dev, "Command 0x%x not found in TPM\n", cc);
> +		return;
> +	}
> +	attrs = chip->cc_attrs_tbl[i];
> +
> +	handles = (attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0);
> +
> +	s = &buf->data[TPM_HEADER_SIZE];
> +	/*
> +	 * just check the names, it's easy to make mistakes.  This
> +	 * would happen if someone added a handle via
> +	 * tpm_buf_append_u32() instead of tpm_buf_append_name()
> +	 */
> +	for (i = 0; i < handles; i++) {
> +		u32 handle = tpm_get_inc_u32(&s);
> +
> +		if (auth->name_h[i] != handle) {
> +			dev_err(&chip->dev, "TPM: handle %d wrong for name\n",
> +				  i);
> +			return;
> +		}
> +	}
> +	/* point s to the start of the sessions */
> +	val = tpm_get_inc_u32(&s);
> +	/* point p to the start of the parameters */
> +	p = s + val;
> +	for (i = 1; s < p; i++) {
> +		u32 handle = tpm_get_inc_u32(&s);
> +		u16 len;
> +		u8 a;
> +
> +		/* nonce (already in auth) */
> +		len = tpm_get_inc_u16(&s);
> +		s += len;
> +
> +		a = *s++;
> +
> +		len = tpm_get_inc_u16(&s);
> +		if (handle == auth->handle && auth->attrs == a) {
> +			hmac = (u8 *)s;
> +			/*
> +			 * save our session number so we know which
> +			 * session in the response belongs to us
> +			 */
> +			auth->session = i;
> +		}
> +
> +		s += len;
> +	}
> +	if (s != p) {
> +		dev_err(&chip->dev, "TPM session length is incorrect\n");
> +		return;
> +	}
> +	if (!hmac) {
> +		dev_err(&chip->dev, "TPM could not find HMAC session\n");
> +		return;
> +	}
> +
> +	/* encrypt before HMAC */
> +	if (auth->attrs & TPM2_SA_DECRYPT) {
> +		struct scatterlist sg[1];
> +		u16 len;
> +		SKCIPHER_REQUEST_ON_STACK(req, auth->aes);
> +
> +		skcipher_request_set_tfm(req, auth->aes);
> +		skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP,
> +					      NULL, NULL);
> +
> +		/* need key and IV */
> +		KDFa(auth->session_key, SHA256_DIGEST_SIZE
> +		     + auth->passphraselen, "CFB", auth->our_nonce,
> +		     auth->tpm_nonce, AES_KEYBYTES + AES_BLOCK_SIZE,
> +		     auth->scratch);
> +		crypto_skcipher_setkey(auth->aes, auth->scratch, AES_KEYBYTES);
> +		len = tpm_get_inc_u16(&p);
> +		sg_init_one(sg, p, len);
> +		skcipher_request_set_crypt(req, sg, sg, len,
> +					   auth->scratch + AES_KEYBYTES);
> +		crypto_skcipher_encrypt(req);
> +		/* reset p to beginning of parameters for HMAC */
> +		p -= 2;
> +	}
> +
> +	crypto_shash_init(desc);
> +	/* ordinal is already BE */
> +	crypto_shash_update(desc, (u8 *)&head->ordinal, sizeof(head->ordinal));
> +	/* add the handle names */
> +	for (i = 0; i < handles; i++) {
> +		u8 mso = auth->name_h[i] >> 24;
> +
> +		if (mso == 0x81 || mso == 0x80 || mso == 0x01) {
> +			crypto_shash_update(desc, auth->name[i],
> +					    SHA256_DIGEST_SIZE + 2);
> +		} else {
> +			__be32 h = cpu_to_be32(auth->name_h[i]);
> +
> +			crypto_shash_update(desc, (u8 *)&h, 4);
> +		}
> +	}
> +	if (buf->data - s != tpm_buf_length(buf))
> +		crypto_shash_update(desc, s, buf->data
> +				    + tpm_buf_length(buf) - s);
> +	crypto_shash_final(desc, cphash);
> +
> +	/* now calculate the hmac */
> +	hmac_init(desc, auth->session_key, sizeof(auth->session_key)
> +		  + auth->passphraselen);
> +	crypto_shash_update(desc, cphash, sizeof(cphash));
> +	crypto_shash_update(desc, auth->our_nonce, sizeof(auth->our_nonce));
> +	crypto_shash_update(desc, auth->tpm_nonce, sizeof(auth->tpm_nonce));
> +	crypto_shash_update(desc, &auth->attrs, 1);
> +	hmac_final(desc, auth->session_key, sizeof(auth->session_key)
> +		   + auth->passphraselen, hmac);
> +}
> +EXPORT_SYMBOL(tpm_buf_fill_hmac_session);
> +
> +static int parse_read_public(char *name, const u8 *data)

Should have 'tpm_' prefix in order to make it easier to trace.

> +{
> +	struct tpm_output_header *head = (struct tpm_output_header *)data;
> +	u32 tot_len = be32_to_cpu(head->length);
> +	u32 val;
> +
> +	data += TPM_HEADER_SIZE;
> +	/* we're starting after the header so adjust the length */
> +	tot_len -= TPM_HEADER_SIZE;
> +
> +	/* skip public */
> +	val = tpm_get_inc_u16(&data);
> +	if (val > tot_len)
> +		return -EINVAL;
> +	data += val;
> +	/* name */
> +	val = tpm_get_inc_u16(&data);
> +	if (val != SHA256_DIGEST_SIZE + 2)
> +		return -EINVAL;
> +	memcpy(name, data, SHA256_DIGEST_SIZE + 2);
> +	/* forget the rest */
> +	return 0;
> +}
> +
> +static int tpm2_readpublic(struct tpm_chip *chip, u32 handle, char *name)

Underscore missing.

> +{
> +	struct tpm_buf buf;
> +	int rc;
> +
> +	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_READ_PUBLIC);
> +	if (rc)
> +		return rc;
> +
> +	tpm_buf_append_u32(&buf, handle);
> +	rc = tpm_transmit_cmd(chip, &chip->kernel_space, buf.data,
> +			      PAGE_SIZE, 0, 0, "read public");
> +	if (rc == TPM2_RC_SUCCESS)
> +		rc = parse_read_public(name, buf.data);

Here, on the other hand, you have the underscore so the name is very
incoherent ATM.

> +
> +	tpm_buf_destroy(&buf);
> +
> +	return rc;
> +}
> +
> +/**
> + * tpm_buf_append_name() - add a handle area to the buffer
> + * @buf: The buffer to be appended
> + * @auth: the auth structure allocated by tpm2_start_auth_session()
> + * @handle: The handle to be appended
> + * @name: The name of the handle (may be NULL)
> + *
> + * In order to compute session HMACs, we need to know the names of the
> + * objects pointed to by the handles.  For most objects, this is simly
> + * the actual 4 byte handle or an empty buf (in these cases @name
> + * should be NULL) but for volatile objects, permanent objects and NV
> + * areas, the name is defined as the hash (according to the name
> + * algorithm which should be set to sha256) of the public area to
> + * which the two byte algorithm id has been appended.  For these
> + * objects, the @name pointer should point to this.  If a name is
> + * required but @name is NULL, then TPM2_ReadPublic() will be called
> + * on the handle to obtain the name.
> + *
> + * As with most tpm_buf operations, success is assumed because failure
> + * will be caused by an incorrect programming model and indicated by a
> + * kernel message.
> + */
> +void tpm_buf_append_name(struct tpm_buf *buf, struct tpm2_auth *auth,
> +			 u32 handle, u8 *name)
> +{
> +	int slot;
> +	u8 mso = handle >> 24;
> +
> +	slot = (tpm_buf_length(buf) - TPM_HEADER_SIZE)/4;
> +	if (slot >= AUTH_MAX_NAMES) {
> +		dev_err(&auth->chip->dev, "TPM: too many handles\n");
> +		return;
> +	}
> +	WARN(auth->session != tpm_buf_length(buf),
> +	     "name added in wrong place\n");
> +	tpm_buf_append_u32(buf, handle);
> +	auth->session += 4;
> +
> +	if (mso == 0x81 || mso == 0x80 || mso == 0x01) {
> +		if (!name)
> +			tpm2_readpublic(auth->chip, handle, auth->name[slot]);
> +	} else {
> +		if (name)
> +			dev_err(&auth->chip->dev, "TPM: Handle does not require name but one is specified\n");

Elsewhere the prefix is in lower case.
`
> +	}
> +
> +	auth->name_h[slot] = handle;
> +	if (name)
> +		memcpy(auth->name[slot], name, SHA256_DIGEST_SIZE + 2);
> +}
> +EXPORT_SYMBOL(tpm_buf_append_name);
> +
> +/**
> + * tpm_buf_check_hmac_response() - check the TPM return HMAC for correctness
> + * @buf: the original command buffer (which now contains the response)
> + * @auth: the auth structure allocated by tpm2_start_auth_session()
> + * @rc: the return code from tpm_transmit_cmd
> + *
> + * If @rc is non zero, @buf may not contain an actual return, so @rc
> + * is passed through as the return and the session cleaned up and
> + * de-allocated if required (this is required if
> + * TPM2_SA_CONTINUE_SESSION was not specified as a session flag).
> + *
> + * If @rc is zero, the response HMAC is computed against the returned
> + * @buf and matched to the TPM one in the session area.  If there is a
> + * mismatch, an error is logged and -EINVAL returned.
> + *
> + * The reason for this is that the command issue and HMAC check
> + * sequence should look like:
> + *
> + *	rc = tpm_transmit_cmd(...);
> + *	rc = tpm_buf_check_hmac_response(&buf, auth, rc);
> + *	if (rc)
> + *		...
> + *
> + * Which is easily layered into the current contrl flow.
> + *
> + * Returns: 0 on success or an error.
> + */
> +int tpm_buf_check_hmac_response(struct tpm_buf *buf, struct tpm2_auth *auth,
> +				int rc)
> +{
> +	struct tpm_output_header *head = (struct tpm_output_header *)buf->data;
> +	struct tpm_chip *chip = auth->chip;
> +	const u8 *s, *p;
> +	u8 rphash[SHA256_DIGEST_SIZE];
> +	u32 attrs;
> +	SHASH_DESC_ON_STACK(desc, sha256_hash);
> +	u16 tag = be16_to_cpu(head->tag);
> +	u32 cc = be32_to_cpu(auth->ordinal);
> +	int parm_len, len, i, handles;
> +
> +	if (auth->session >= TPM_HEADER_SIZE) {
> +		WARN(1, "tpm session not filled correctly\n");
> +		goto out;
> +	}
> +
> +	if (rc != 0)
> +		/* pass non success rc through and close the session */
> +		goto out;
> +
> +	rc = -EINVAL;
> +	if (tag != TPM2_ST_SESSIONS) {
> +		dev_err(&chip->dev, "TPM: HMAC response check has no sessions tag\n");
> +		goto out;
> +	}
> +
> +	i = tpm2_find_cc(chip, cc);
> +	if (i < 0)
> +		goto out;
> +	attrs = chip->cc_attrs_tbl[i];
> +	handles = (attrs >> TPM2_CC_ATTR_RHANDLE) & 1;
> +
> +	/* point to area beyond handles */
> +	s = &buf->data[TPM_HEADER_SIZE + handles * 4];
> +	parm_len = tpm_get_inc_u32(&s);
> +	p = s;
> +	s += parm_len;
> +	/* skip over any sessions before ours */
> +	for (i = 0; i < auth->session - 1; i++) {
> +		len = tpm_get_inc_u16(&s);
> +		s += len + 1;
> +		len = tpm_get_inc_u16(&s);
> +		s += len;
> +	}
> +	/* TPM nonce */
> +	len = tpm_get_inc_u16(&s);
> +	if (s - buf->data + len > tpm_buf_length(buf))
> +		goto out;
> +	if (len != SHA256_DIGEST_SIZE)
> +		goto out;
> +	memcpy(auth->tpm_nonce, s, len);
> +	s += len;
> +	attrs = *s++;
> +	len = tpm_get_inc_u16(&s);
> +	if (s - buf->data + len != tpm_buf_length(buf))
> +		goto out;
> +	if (len != SHA256_DIGEST_SIZE)
> +		goto out;
> +	/*
> +	 * s points to the HMAC. now calculate comparison, beginning
> +	 * with rphash
> +	 */
> +	desc->tfm = sha256_hash;
> +	desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
> +	crypto_shash_init(desc);
> +	/* yes, I know this is now zero, but it's what the standard says */
> +	crypto_shash_update(desc, (u8 *)&head->return_code,
> +			    sizeof(head->return_code));
> +	/* ordinal is already BE */
> +	crypto_shash_update(desc, (u8 *)&auth->ordinal, sizeof(auth->ordinal));
> +	crypto_shash_update(desc, p, parm_len);
> +	crypto_shash_final(desc, rphash);
> +
> +	/* now calculate the hmac */
> +	hmac_init(desc, auth->session_key, sizeof(auth->session_key)
> +		  + auth->passphraselen);
> +	crypto_shash_update(desc, rphash, sizeof(rphash));
> +	crypto_shash_update(desc, auth->tpm_nonce, sizeof(auth->tpm_nonce));
> +	crypto_shash_update(desc, auth->our_nonce, sizeof(auth->our_nonce));
> +	crypto_shash_update(desc, &auth->attrs, 1);
> +	/* we're done with the rphash, so put our idea of the hmac there */
> +	hmac_final(desc, auth->session_key, sizeof(auth->session_key)
> +		   + auth->passphraselen, rphash);
> +	if (memcmp(rphash, s, SHA256_DIGEST_SIZE) == 0) {
> +		rc = 0;
> +	} else {
> +		dev_err(&auth->chip->dev, "TPM: HMAC check failed\n");
> +		goto out;
> +	}
> +
> +	/* now do response decryption */
> +	if (auth->attrs & TPM2_SA_ENCRYPT) {
> +		struct scatterlist sg[1];
> +		SKCIPHER_REQUEST_ON_STACK(req, auth->aes);
> +
> +		skcipher_request_set_tfm(req, auth->aes);
> +		skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP,
> +					      NULL, NULL);
> +
> +		/* need key and IV */
> +		KDFa(auth->session_key, SHA256_DIGEST_SIZE
> +		     + auth->passphraselen, "CFB", auth->tpm_nonce,
> +		     auth->our_nonce, AES_KEYBYTES + AES_BLOCK_SIZE,
> +		     auth->scratch);
> +		crypto_skcipher_setkey(auth->aes, auth->scratch, AES_KEYBYTES);
> +		len = tpm_get_inc_u16(&p);
> +		sg_init_one(sg, p, len);
> +		skcipher_request_set_crypt(req, sg, sg, len,
> +					   auth->scratch + AES_KEYBYTES);
> +		crypto_skcipher_decrypt(req);
> +	}
> +
> + out:
> +	if ((auth->attrs & TPM2_SA_CONTINUE_SESSION) == 0) {
> +		/* manually close the session if it wasn't consumed */
> +		if (rc)
> +			tpm2_flush_context_cmd(chip, auth->handle, 0);
> +		crypto_free_skcipher(auth->aes);
> +		kfree(auth);
> +	} else {
> +		/* reset for next use  */
> +		auth->session = TPM_HEADER_SIZE;
> +	}
> +
> +	return rc;
> +}
> +EXPORT_SYMBOL(tpm_buf_check_hmac_response);
> +
> +/**
> + * tpm2_end_auth_session - kill the allocated auth session
> + * @auth: the auth structure allocated by tpm2_start_auth_session()
> + *
> + * ends the session started by tpm2_start_auth_session and frees all
> + * the resources.  Under normal conditions,
> + * tpm_buf_check_hmac_response() will correctly end the session if
> + * required, so this function is only for use in error legs that will
> + * bypass the normal invocation of tpm_buf_check_hmac_respons().
> + */
> +void tpm2_end_auth_session(struct tpm2_auth *auth)
> +{
> +	tpm2_flush_context_cmd(auth->chip, auth->handle, 0);
> +	crypto_free_skcipher(auth->aes);
> +	kfree(auth);
> +}
> +EXPORT_SYMBOL(tpm2_end_auth_session);
> +
> +static int parse_start_auth_session(struct tpm2_auth *auth, const u8 *data)

The prefix...

> +{
> +	struct tpm_output_header *head = (struct tpm_output_header *)data;
> +	u32 tot_len = be32_to_cpu(head->length);
> +	u32 val;
> +
> +	data += TPM_HEADER_SIZE;
> +	/* we're starting after the header so adjust the length */
> +	tot_len -= TPM_HEADER_SIZE;
> +
> +	/* should have handle plus nonce */
> +	if (tot_len != 4 + 2 + sizeof(auth->tpm_nonce))
> +		return -EINVAL;
> +
> +	auth->handle = tpm_get_inc_u32(&data);
> +	val = tpm_get_inc_u16(&data);
> +	if (val != sizeof(auth->tpm_nonce))
> +		return -EINVAL;
> +	memcpy(auth->tpm_nonce, data, sizeof(auth->tpm_nonce));
> +	/* now compute the session key from the nonces */
> +	KDFa(auth->salt, sizeof(auth->salt), "ATH", auth->tpm_nonce,
> +	     auth->our_nonce, sizeof(auth->session_key), auth->session_key);
> +
> +	return 0;
> +}
> +
> +/**
> + * tpm2_start_auth_session - create a HMAC authentication session with the TPM
> + * @chip: the TPM chip structure to create the session with
> + * @authp: A pointer to an opaque tpm2_auth structure to be allocated
> + *
> + * This function contacts the TPM via the kernel space for an
> + * authentication session, allocates a tpm2_auth structure to contain
> + * all the session details necessary for performing the HMAC, encrypt
> + * and decrypt operations, fills it in and returns.
> + *
> + * Return: zero on success or actual error encountered.  If return is
> + * zero, @authp will be allocated.
> + */
> +int tpm2_start_auth_session(struct tpm_chip *chip, struct tpm2_auth **authp)

Not good that we have two disjoint things going on in tpm2-cmd.c and
here.

> +{
> +	struct tpm_buf buf;
> +	struct tpm2_auth *auth;
> +	int rc;
> +
> +	auth = kmalloc(sizeof(**authp), GFP_KERNEL);
> +	if (!auth)
> +		return -ENOMEM;
> +
> +	auth->chip = chip;
> +	auth->session = TPM_HEADER_SIZE;
> +
> +	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_START_AUTH_SESS);
> +	if (rc)
> +		goto out;
> +
> +	/* salt key handle */
> +	tpm_buf_append_u32(&buf, chip->tpmkey);
> +	/* bind key handle */
> +	tpm_buf_append_u32(&buf, TPM2_RH_NULL);
> +	/* nonce caller */
> +	get_random_bytes(auth->our_nonce, sizeof(auth->our_nonce));
> +	tpm_buf_append_u16(&buf, sizeof(auth->our_nonce));
> +	tpm_buf_append(&buf, auth->our_nonce, sizeof(auth->our_nonce));
> +
> +	/* append encrypted salt and squirrel away unencrypted in auth */
> +	tpm_buf_append_salt(&buf, chip, auth);
> +	/* session type (HMAC, audit or policy) */
> +	tpm_buf_append_u8(&buf, TPM2_SE_HMAC);
> +
> +	/* symmetric encryption parameters */
> +	/* symmetric algorithm */
> +	tpm_buf_append_u16(&buf, TPM2_ALG_AES);
> +	/* bits for symmetric algorithm */
> +	tpm_buf_append_u16(&buf, AES_KEYBITS);
> +	/* symmetric algorithm mode (must be CFB) */
> +	tpm_buf_append_u16(&buf, TPM2_ALG_CFB);
> +	/* hash algorithm for session */
> +	tpm_buf_append_u16(&buf, TPM2_ALG_SHA256);
> +
> +	rc = tpm_transmit_cmd(chip, &chip->kernel_space, buf.data, PAGE_SIZE,
> +			      0, 0, "start auth session");
> +	if (rc == TPM2_RC_SUCCESS)
> +		rc = parse_start_auth_session(auth, buf.data);
> +
> +	tpm_buf_destroy(&buf);
> +
> +	if (rc)
> +		goto out;
> +
> +	auth->aes = crypto_alloc_skcipher("cfb(aes)", 0, 0);
> +	if (IS_ERR(auth->aes)) {
> +		rc = PTR_ERR(auth->aes);
> +		dev_err(&chip->dev, "TPM: error getting cfb(aes): %d\n", rc);
> +	}
> + out:
> +	if (rc)
> +		kfree(auth);
> +	else
> +		*authp = auth;
> +
> +	return rc;
> +}
> +EXPORT_SYMBOL(tpm2_start_auth_session);
> +
> +static int parse_create_primary(struct tpm_chip *chip, u8 *data)

Ugh.

> +{
> +	struct tpm_output_header *head = (struct tpm_output_header *)data;
> +	u16 len;
> +	u32 tot_len = be32_to_cpu(head->length);
> +	u32 handle, val, parm_len;
> +	const u8 *resp, *tmp;
> +	SHASH_DESC_ON_STACK(desc, sha256_hash);
> +
> +	data += TPM_HEADER_SIZE;
> +	/* we're starting after the header so adjust the length */
> +	tot_len -= TPM_HEADER_SIZE;
> +
> +	resp = data;
> +	handle = tpm_get_inc_u32(&resp);
> +	parm_len = tpm_get_inc_u32(&resp);
> +	if (parm_len + 8 > tot_len)
> +		return -EINVAL;
> +	len = tpm_get_inc_u16(&resp);
> +	tmp = resp;
> +	/* now we have the public area, compute the name of the object */
> +	desc->tfm = sha256_hash;
> +	desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
> +	put_unaligned_be16(TPM2_ALG_SHA256, chip->tpmkeyname);
> +	crypto_shash_init(desc);
> +	crypto_shash_update(desc, resp, len);
> +	crypto_shash_final(desc, chip->tpmkeyname + 2);
> +	/* validate the public key */
> +	val = tpm_get_inc_u16(&tmp);
> +	/* key type (must be what we asked for) */
> +	if (val != TPM2_ALG_ECC)
> +		return -EINVAL;
> +	val = tpm_get_inc_u16(&tmp);
> +	/* name algorithm */
> +	if (val != TPM2_ALG_SHA256)
> +		return -EINVAL;
> +	val = tpm_get_inc_u32(&tmp);
> +	/* object properties */
> +	if (val != (TPM2_OA_NO_DA |
> +		    TPM2_OA_FIXED_TPM |
> +		    TPM2_OA_FIXED_PARENT |
> +		    TPM2_OA_SENSITIVE_DATA_ORIGIN |
> +		    TPM2_OA_USER_WITH_AUTH |
> +		    TPM2_OA_DECRYPT |
> +		    TPM2_OA_RESTRICTED))
> +		return -EINVAL;
> +	/* auth policy (empty) */
> +	val = tpm_get_inc_u16(&tmp);
> +	if (val != 0)
> +		return -EINVAL;
> +	val = tpm_get_inc_u16(&tmp);
> +	/* symmetric key parameters */
> +	if (val != TPM2_ALG_AES)
> +		return -EINVAL;
> +	val = tpm_get_inc_u16(&tmp);
> +	/* symmetric key length */
> +	if (val != AES_KEYBITS)
> +		return -EINVAL;
> +	val = tpm_get_inc_u16(&tmp);
> +	/* symmetric encryption scheme */
> +	if (val != TPM2_ALG_CFB)
> +		return -EINVAL;
> +	val = tpm_get_inc_u16(&tmp);
> +	/* signing scheme */
> +	if (val != TPM2_ALG_NULL)
> +		return -EINVAL;
> +	val = tpm_get_inc_u16(&tmp);
> +	/* ECC Curve */
> +	if (val != TPM2_ECC_NIST_P256)
> +		return -EINVAL;
> +	val = tpm_get_inc_u16(&tmp);
> +	/* KDF Scheme */
> +	if (val != TPM2_ALG_NULL)
> +		return -EINVAL;
> +	val = tpm_get_inc_u16(&tmp);
> +	/* x point */
> +	if (val != 32)
> +		return -EINVAL;
> +	memcpy(chip->ec_point_x, tmp, val);
> +	tmp += val;
> +	val = tpm_get_inc_u16(&tmp);
> +	if (val != 32)
> +		return -EINVAL;
> +	memcpy(chip->ec_point_y, tmp, val);
> +	tmp += val;
> +	resp += len;
> +	/* should have exactly consumed the tpm2b public structure */
> +	if (tmp != resp)
> +		return -EINVAL;
> +	if (resp - data > parm_len)
> +		return -EINVAL;
> +	/* creation data (skip) */
> +	len = tpm_get_inc_u16(&resp);
> +	resp += len;
> +	if (resp - data > parm_len)
> +		return -EINVAL;
> +	/* creation digest (must be sha256) */
> +	len = tpm_get_inc_u16(&resp);
> +	resp += len;
> +	if (len != SHA256_DIGEST_SIZE || resp - data > parm_len)
> +		return -EINVAL;
> +	/* TPMT_TK_CREATION follows */
> +	/* tag, must be TPM_ST_CREATION (0x8021) */
> +	val = tpm_get_inc_u16(&resp);
> +	if (val != TPM2_ST_CREATION || resp - data > parm_len)
> +		return -EINVAL;
> +	/* hierarchy (must be NULL) */
> +	val = tpm_get_inc_u32(&resp);
> +	if (val != TPM2_RH_NULL || resp - data > parm_len)
> +		return -EINVAL;
> +	/* the ticket digest HMAC (might not be sha256) */
> +	len = tpm_get_inc_u16(&resp);
> +	resp += len;
> +	if (resp - data > parm_len)
> +		return -EINVAL;
> +	/*
> +	 * finally we have the name, which is a sha256 digest plus a 2
> +	 * byte algorithm type
> +	 */
> +	len = tpm_get_inc_u16(&resp);
> +	if (resp + len - data != parm_len + 8)
> +		return -EINVAL;
> +	if (len != SHA256_DIGEST_SIZE + 2)
> +		return -EINVAL;
> +
> +	if (memcmp(chip->tpmkeyname, resp, SHA256_DIGEST_SIZE + 2) != 0) {
> +		printk("TPM NULL Seed name comparison failed\n");
> +		return -EINVAL;
> +	}
> +
> +	chip->tpmkey = handle;
> +
> +	return 0;
> +}
> +
> +int tpm2_create_null_primary(struct tpm_chip *chip)
> +{
> +	int rc;
> +	struct tpm_buf buf;
> +	struct tpm_buf template;
> +
> +	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE_PRIMARY);
> +	if (rc)
> +		return rc;
> +
> +	rc = tpm_buf_init_2b(&template);
> +	if (rc) {
> +		tpm_buf_destroy(&buf);
> +		return rc;
> +	}
> +
> +	/*
> +	 * create the template.  Note: in order for userspace to
> +	 * verify the security of the system, it will have to create
> +	 * and certify this NULL primary, meaning all the template
> +	 * parameters will have to be identical, so conform exactly to
> +	 * the TCG TPM v2.0 Provisioning Guidance for the SRK ECC
> +	 * key
> +	 */
> +
> +	/* key type */
> +	tpm_buf_append_u16(&template, TPM2_ALG_ECC);
> +	/* name algorithm */
> +	tpm_buf_append_u16(&template, TPM2_ALG_SHA256);
> +	/* object properties */
> +	tpm_buf_append_u32(&template, TPM2_OA_NO_DA |
> +			   TPM2_OA_FIXED_TPM |
> +			   TPM2_OA_FIXED_PARENT |
> +			   TPM2_OA_SENSITIVE_DATA_ORIGIN |
> +			   TPM2_OA_USER_WITH_AUTH |
> +			   TPM2_OA_DECRYPT |
> +			   TPM2_OA_RESTRICTED);
> +	/* sauth policy (empty) */
> +	tpm_buf_append_u16(&template, 0);
> +
> +	/* BEGIN parameters: key specific; for ECC*/
> +	/* symmetric algorithm */
> +	tpm_buf_append_u16(&template, TPM2_ALG_AES);
> +	/* bits for symmetric algorithm */
> +	tpm_buf_append_u16(&template, 128);
> +	/* algorithm mode (must be CFB) */
> +	tpm_buf_append_u16(&template, TPM2_ALG_CFB);
> +	/* scheme (NULL means any scheme) */
> +	tpm_buf_append_u16(&template, TPM2_ALG_NULL);
> +	/* ECC Curve ID */
> +	tpm_buf_append_u16(&template, TPM2_ECC_NIST_P256);
> +	/* KDF Scheme */
> +	tpm_buf_append_u16(&template, TPM2_ALG_NULL);
> +	/* unique: key specific; for ECC it is two points */
> +	tpm_buf_append_u16(&template, 0);
> +	tpm_buf_append_u16(&template, 0);
> +	/* END parameters */
> +
> +	/* primary handle */
> +	tpm_buf_append_u32(&buf, TPM2_RH_NULL);
> +	/* simple authorization for empty auth */
> +	tpm2_buf_append_auth(&buf, TPM2_RS_PW, NULL, 0, 0, NULL, 0);
> +	/* sensitive create size is 4 for two empty buffers */
> +	tpm_buf_append_u16(&buf, 4);
> +	/* sensitive create auth data (empty) */
> +	tpm_buf_append_u16(&buf, 0);
> +	/* sensitive create sensitive data (empty) */
> +	tpm_buf_append_u16(&buf, 0);
> +	/* the public template */
> +	tpm_buf_append_2b(&buf, &template);
> +	tpm_buf_destroy(&template);
> +	/* outside info (empty) */
> +	tpm_buf_append_u16(&buf, 0);
> +	/* creation PCR (none) */
> +	tpm_buf_append_u32(&buf, 0);
> +
> +	rc = tpm_transmit_cmd(chip, &chip->kernel_space, buf.data, PAGE_SIZE,
> +			      0, 0, "attempting to create NULL primary");
> +
> +	if (rc == TPM2_RC_SUCCESS)
> +		rc = parse_create_primary(chip, buf.data);
> +
> +	tpm_buf_destroy(&buf);
> +
> +	return rc;
> +}
> +
> +int tpm2_sessions_init(struct tpm_chip *chip)
> +{
> +	int rc;
> +
> +	rc = tpm2_init_space(&chip->kernel_space);
> +	if (rc) {
> +		dev_err(&chip->dev, "TPM: failed to initialize kernel space\n");
> +		return -ENOMEM;
> +	}
> +	sha256_hash = crypto_alloc_shash("sha256", 0, 0);
> +	if (!sha256_hash) {
> +		dev_err(&chip->dev, "TPM: failed to allocate hash\n");
> +		return -ENOMEM;
> +	}
> +
> +	rc = tpm2_create_null_primary(chip);
> +	if (rc)
> +		dev_err(&chip->dev, "TPM: security failed (NULL seed derivation): %d\n", rc);
> +	return rc;
> +}
> +EXPORT_SYMBOL(tpm2_sessions_init);
> diff --git a/drivers/char/tpm/tpm2-sessions.h b/drivers/char/tpm/tpm2-sessions.h
> new file mode 100644
> index 000000000000..cea13f411de7
> --- /dev/null
> +++ b/drivers/char/tpm/tpm2-sessions.h
> @@ -0,0 +1,57 @@
> +/*
> + * Defines for TPM2 authentications
> + */
> +
> +#ifndef _TPM2_SESSIONS_H
> +#define _TPM2_SESSIONS_H
> +
> +#include "tpm.h"
> +
> +enum tpm2_object_attributes {
> +	TPM2_OA_FIXED_TPM		= BIT(1),
> +	TPM2_OA_ST_CLEAR		= BIT(2),
> +	TPM2_OA_FIXED_PARENT		= BIT(4),
> +	TPM2_OA_SENSITIVE_DATA_ORIGIN	= BIT(5),
> +	TPM2_OA_USER_WITH_AUTH		= BIT(6),
> +	TPM2_OA_ADMIN_WITH_POLICY	= BIT(7),
> +	TPM2_OA_NO_DA			= BIT(10),
> +	TPM2_OA_ENCRYPTED_DUPLICATION	= BIT(11),
> +	TPM2_OA_RESTRICTED		= BIT(16),
> +	TPM2_OA_DECRYPT			= BIT(17),
> +	TPM2_OA_SIGN			= BIT(18),
> +};
> +
> +enum tpm2_session_attributes {
> +	TPM2_SA_CONTINUE_SESSION	= BIT(0),
> +	TPM2_SA_AUDIT_EXCLUSIVE		= BIT(1),
> +	TPM2_SA_AUDIT_RESET		= BIT(3),
> +	TPM2_SA_DECRYPT			= BIT(5),
> +	TPM2_SA_ENCRYPT			= BIT(6),
> +	TPM2_SA_AUDIT			= BIT(7),
> +};
> +
> +enum tpm2_session_types {
> +	TPM2_SE_HMAC	= 0x00,
> +	TPM2_SE_POLICY	= 0x01,
> +	TPM2_SE_TRIAL	= 0x02,
> +};
> +
> +struct tpm2_auth;
> +
> +void tpm2_buf_append_auth(struct tpm_buf *buf, u32 session_handle,
> +			  const u8 *nonce, u16 nonce_len,
> +			  u8 attributes,
> +			  const u8 *hmac, u16 hmac_len);
> +
> +int tpm2_start_auth_session(struct tpm_chip *chip, struct tpm2_auth **authp);
> +void tpm_buf_append_name(struct tpm_buf *buf, struct tpm2_auth *auth,
> +			 u32 handle, u8 *name);
> +void tpm_buf_append_hmac_session(struct tpm_buf *buf, struct tpm2_auth *auth,
> +				 u8 attributes, u8 *passphrase,
> +				 int passphraselen);
> +void tpm_buf_fill_hmac_session(struct tpm_buf *buf, struct tpm2_auth *auth);
> +int tpm_buf_check_hmac_response(struct tpm_buf *buf, struct tpm2_auth *auth,
> +				int rc);
> +void tpm2_end_auth_session(struct tpm2_auth *auth);
> +
> +#endif
> -- 
> 2.16.4
>
>

Last 1/3-1/2 I did not check that good care anymore because at this
point it does not make sense and I already had spent a lot of time on
this. This commit should be split into smaller commits that are easier
to evaluate as a whole. The overal design is something that I can cope
with but this needs a lot of polishing before considering to merge.

/Jarkko

Re: [PATCH v4 0/7] add integrity and security to TPM2 transactions

[Jarkko Sakkinen]

I would consider sending first a patch set that would iterate the existing
session stuff to be ready for this i.e. merge in two iterations 
(emphasis on the word "consider"). We can probably merge the groundwork
quite fast.

/Jarkko

On Mon, 22 Oct 2018, James Bottomley wrote:
> By now, everybody knows we have a problem with the TPM2_RS_PW easy
> button on TPM2 in that transactions on the TPM bus can be intercepted
> and altered.  The way to fix this is to use real sessions for HMAC
> capabilities to ensure integrity and to use parameter and response
> encryption to ensure confidentiality of the data flowing over the TPM
> bus.
>
> This patch series is about adding a simple API which can ensure the
> above properties as a layered addition to the existing TPM handling
> code.  This series now includes protections for PCR extend, getting
> random numbers from the TPM and data sealing and unsealing.  It
> therefore eliminates all uses of TPM2_RS_PW in the kernel and adds
> encryption protection to sensitive data flowing into and out of the
> TPM.
>
> In the third version I added data sealing and unsealing protection,
> apart from one API based problem which means that the way trusted keys
> were protected it's not currently possible to HMAC protect an authority
> that comes with a policy, so the API will have to be extended to fix
> that case
>
> In this fourth version, I tidy up some of the code and add more
> security features, the most notable is that we now calculate the NULL
> seed name and compare our calculation to the value returned in
> TPM2_ReadPublic, which means we now can't be spoofed.  This version
> also gives a sysfs variable for the null seed which userspace can use
> to run a key certification operation to prove that the TPM was always
> secure when communicating with the kernel.
>
> I've verified this using the test suite in the last patch on a VM
> connected to a tpm2 emulator.  I also instrumented the emulator to make
> sure the sensitive data was properly encrypted.
>
> James
>
> ---
>
>
> James Bottomley (7):
>  tpm-buf: create new functions for handling TPM buffers
>  tpm2-sessions: Add full HMAC and encrypt/decrypt session handling
>  tpm2: add hmac checks to tpm2_pcr_extend()
>  tpm2: add session encryption protection to tpm2_get_random()
>  trusted keys: Add session encryption protection to the seal/unseal
>    path
>  tpm: add the null key name as a tpm2 sysfs variable
>  tpm2-sessions: NOT FOR COMMITTING add sessions testing
>
> drivers/char/tpm/Kconfig              |    3 +
> drivers/char/tpm/Makefile             |    3 +-
> drivers/char/tpm/tpm-buf.c            |  191 ++++++
> drivers/char/tpm/tpm-chip.c           |    1 +
> drivers/char/tpm/tpm-sysfs.c          |   27 +-
> drivers/char/tpm/tpm.h                |  129 ++--
> drivers/char/tpm/tpm2-cmd.c           |  248 ++++---
> drivers/char/tpm/tpm2-sessions-test.c |  360 ++++++++++
> drivers/char/tpm/tpm2-sessions.c      | 1188 +++++++++++++++++++++++++++++++++
> drivers/char/tpm/tpm2-sessions.h      |   57 ++
> 10 files changed, 2027 insertions(+), 180 deletions(-)
> create mode 100644 drivers/char/tpm/tpm-buf.c
> create mode 100644 drivers/char/tpm/tpm2-sessions-test.c
> create mode 100644 drivers/char/tpm/tpm2-sessions.c
> create mode 100644 drivers/char/tpm/tpm2-sessions.h
>
> -- 
> 2.16.4
>
>

Re: [PATCH v4 5/7] trusted keys: Add session encryption protection to the seal/unseal path

[Jarkko Sakkinen]

[-- Attachment #1: Type: text/plain, Size: 10361 bytes --]

The tag in the short description does not look at all. Should be either
"tpm:" or "keys, trusted:".

On Mon, 22 Oct 2018, James Bottomley wrote:
> If some entity is snooping the TPM bus, the can see the data going in
> to be sealed and the data coming out as it is unsealed.  Add parameter
> and response encryption to these cases to ensure that no secrets are
> leaked even if the bus is snooped.
>
> As part of doing this conversion it was discovered that policy
> sessions can't work with HMAC protected authority because of missing
> pieces (the tpm Nonce).  I've added code to work the same way as
> before, which will result in potential authority exposure (while still
> adding security for the command and the returned blob), and a fixme to
> redo the API to get rid of this security hole.
>
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> ---
> drivers/char/tpm/tpm2-cmd.c | 155 ++++++++++++++++++++++++++++----------------
> 1 file changed, 98 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 22f1c7bee173..a8655cd535d1 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -425,7 +425,9 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> {
> 	unsigned int blob_len;
> 	struct tpm_buf buf;
> +	struct tpm_buf t2b;
> 	u32 hash;
> +	struct tpm2_auth *auth;
> 	int i;
> 	int rc;
>
> @@ -439,45 +441,56 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> 	if (i == ARRAY_SIZE(tpm2_hash_map))
> 		return -EINVAL;
>
> -	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
> +	rc = tpm2_start_auth_session(chip, &auth);
> 	if (rc)
> 		return rc;
>
> -	tpm_buf_append_u32(&buf, options->keyhandle);
> -	tpm2_buf_append_auth(&buf, TPM2_RS_PW,
> -			     NULL /* nonce */, 0,
> -			     0 /* session_attributes */,
> -			     options->keyauth /* hmac */,
> -			     TPM_DIGEST_SIZE);
> +	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
> +	if (rc) {
> +		tpm2_end_auth_session(auth);
> +		return rc;
> +	}
> +
> +	rc = tpm_buf_init_2b(&t2b);
> +	if (rc) {
> +		tpm_buf_destroy(&buf);
> +		tpm2_end_auth_session(auth);
> +		return rc;
> +	}
>
> +	tpm_buf_append_name(&buf, auth, options->keyhandle, NULL);
> +	tpm_buf_append_hmac_session(&buf, auth, TPM2_SA_DECRYPT,
> +				    options->keyauth, TPM_DIGEST_SIZE);
> 	/* sensitive */
> -	tpm_buf_append_u16(&buf, 4 + TPM_DIGEST_SIZE + payload->key_len + 1);
> +	tpm_buf_append_u16(&t2b, TPM_DIGEST_SIZE);
> +	tpm_buf_append(&t2b, options->blobauth, TPM_DIGEST_SIZE);
> +	tpm_buf_append_u16(&t2b, payload->key_len + 1);
> +	tpm_buf_append(&t2b, payload->key, payload->key_len);
> +	tpm_buf_append_u8(&t2b, payload->migratable);
>
> -	tpm_buf_append_u16(&buf, TPM_DIGEST_SIZE);
> -	tpm_buf_append(&buf, options->blobauth, TPM_DIGEST_SIZE);
> -	tpm_buf_append_u16(&buf, payload->key_len + 1);
> -	tpm_buf_append(&buf, payload->key, payload->key_len);
> -	tpm_buf_append_u8(&buf, payload->migratable);
> +	tpm_buf_append_2b(&buf, &t2b);
>
> 	/* public */
> -	tpm_buf_append_u16(&buf, 14 + options->policydigest_len);
> -	tpm_buf_append_u16(&buf, TPM2_ALG_KEYEDHASH);
> -	tpm_buf_append_u16(&buf, hash);
> +	tpm_buf_append_u16(&t2b, TPM2_ALG_KEYEDHASH);
> +	tpm_buf_append_u16(&t2b, hash);
>
> 	/* policy */
> 	if (options->policydigest_len) {
> -		tpm_buf_append_u32(&buf, 0);
> -		tpm_buf_append_u16(&buf, options->policydigest_len);
> -		tpm_buf_append(&buf, options->policydigest,
> +		tpm_buf_append_u32(&t2b, 0);
> +		tpm_buf_append_u16(&t2b, options->policydigest_len);
> +		tpm_buf_append(&t2b, options->policydigest,
> 			       options->policydigest_len);
> 	} else {
> -		tpm_buf_append_u32(&buf, TPM2_OA_USER_WITH_AUTH);
> -		tpm_buf_append_u16(&buf, 0);
> +		tpm_buf_append_u32(&t2b, TPM2_OA_USER_WITH_AUTH);
> +		tpm_buf_append_u16(&t2b, 0);
> 	}
>
> 	/* public parameters */
> -	tpm_buf_append_u16(&buf, TPM2_ALG_NULL);
> -	tpm_buf_append_u16(&buf, 0);
> +	tpm_buf_append_u16(&t2b, TPM2_ALG_NULL);
> +	/* unique (zero) */
> +	tpm_buf_append_u16(&t2b, 0);
> +
> +	tpm_buf_append_2b(&buf, &t2b);
>
> 	/* outside info */
> 	tpm_buf_append_u16(&buf, 0);
> @@ -490,8 +503,11 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> 		goto out;
> 	}
>
> -	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 4, 0,
> -			      "sealing data");
> +	tpm_buf_fill_hmac_session(&buf, auth);
> +
> +	rc = tpm_transmit_cmd(chip, &chip->kernel_space, buf.data,
> +			      PAGE_SIZE, 4, 0, "sealing data");
> +	rc = tpm_buf_check_hmac_response(&buf, auth, rc);
> 	if (rc)
> 		goto out;
>
> @@ -509,6 +525,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> 	payload->blob_len = blob_len;
>
> out:
> +	tpm_buf_destroy(&t2b);
> 	tpm_buf_destroy(&buf);
>
> 	if (rc > 0) {
> @@ -528,7 +545,6 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>  * @payload: the key data in clear and encrypted form
>  * @options: authentication values and other options
>  * @blob_handle: returned blob handle
> - * @flags: tpm transmit flags
>  *
>  * Return: 0 on success.
>  *        -E2BIG on wrong payload size.
> @@ -538,9 +554,10 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> static int tpm2_load_cmd(struct tpm_chip *chip,
> 			 struct trusted_key_payload *payload,
> 			 struct trusted_key_options *options,
> -			 u32 *blob_handle, unsigned int flags)
> +			 u32 *blob_handle)
> {
> 	struct tpm_buf buf;
> +	struct tpm2_auth *auth;
> 	unsigned int private_len;
> 	unsigned int public_len;
> 	unsigned int blob_len;
> @@ -555,17 +572,18 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
> 	if (blob_len > payload->blob_len)
> 		return -E2BIG;
>
> -	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
> +	rc = tpm2_start_auth_session(chip, &auth);
> 	if (rc)
> 		return rc;
>
> -	tpm_buf_append_u32(&buf, options->keyhandle);
> -	tpm2_buf_append_auth(&buf, TPM2_RS_PW,
> -			     NULL /* nonce */, 0,
> -			     0 /* session_attributes */,
> -			     options->keyauth /* hmac */,
> -			     TPM_DIGEST_SIZE);
> +	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
> +	if (rc) {
> +		tpm2_end_auth_session(auth);
> +		return rc;
> +	}
>
> +	tpm_buf_append_name(&buf, auth, options->keyhandle, NULL);
> +	tpm_buf_append_hmac_session(&buf, auth, 0, options->keyauth, TPM_DIGEST_SIZE);
> 	tpm_buf_append(&buf, payload->blob, blob_len);
>
> 	if (buf.flags & TPM_BUF_OVERFLOW) {
> @@ -573,8 +591,10 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
> 		goto out;
> 	}
>
> -	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 4, flags,
> -			      "loading blob");
> +	tpm_buf_fill_hmac_session(&buf, auth);
> +	rc = tpm_transmit_cmd(chip, &chip->kernel_space, buf.data, PAGE_SIZE,
> +			      4, 0, "loading blob");
> +	rc = tpm_buf_check_hmac_response(&buf, auth, rc);
> 	if (!rc)
> 		*blob_handle = be32_to_cpup(
> 			(__be32 *) &buf.data[TPM_HEADER_SIZE]);
> @@ -595,7 +615,6 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
>  * @payload: the key data in clear and encrypted form
>  * @options: authentication values and other options
>  * @blob_handle: blob handle
> - * @flags: tpm_transmit_cmd flags
>  *
>  * Return: 0 on success
>  *         -EPERM on tpm error status
> @@ -604,28 +623,54 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
> static int tpm2_unseal_cmd(struct tpm_chip *chip,
> 			   struct trusted_key_payload *payload,
> 			   struct trusted_key_options *options,
> -			   u32 blob_handle, unsigned int flags)
> +			   u32 blob_handle)
> {
> 	struct tpm_buf buf;
> +	struct tpm2_auth *auth;
> 	u16 data_len;
> 	u8 *data;
> 	int rc;
>
> -	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
> +	rc = tpm2_start_auth_session(chip, &auth);
> 	if (rc)
> 		return rc;
>
> -	tpm_buf_append_u32(&buf, blob_handle);
> -	tpm2_buf_append_auth(&buf,
> -			     options->policyhandle ?
> -			     options->policyhandle : TPM2_RS_PW,
> -			     NULL /* nonce */, 0,
> -			     TPM2_SA_CONTINUE_SESSION,
> -			     options->blobauth /* hmac */,
> -			     TPM_DIGEST_SIZE);
> -
> -	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 6, flags,
> -			      "unsealing");
> +	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
> +	if (rc) {
> +		tpm2_end_auth_session(auth);
> +		return rc;
> +	}
> +
> +	tpm_buf_append_name(&buf, auth, blob_handle, NULL);
> +
> +	if (!options->policyhandle) {
> +		tpm_buf_append_hmac_session(&buf, auth, TPM2_SA_ENCRYPT,
> +					    options->blobauth, TPM_DIGEST_SIZE);
> +	} else {
> +		/*
> +		 * FIXME: the policy can't be used for HMAC protection
> +		 * of the authorization because it must be generated
> +		 * with the initial nonces which isn't passed in, so
> +		 * append a second encryption session to at least HMAC
> +		 * protect the command and encrypt the sealed blob on
> +		 * return so the only thing the attacker can get is
> +		 * the password.
> +		 *
> +		 * We also consume the policy session otherwise it
> +		 * would be absorbed into the kernel space.
> +		 */
> +		tpm2_buf_append_auth(&buf, options->policyhandle,
> +				     NULL /* nonce */, 0, 0,
> +				     options->blobauth /* hmac */,
> +				     TPM_DIGEST_SIZE);
> +		tpm_buf_append_hmac_session(&buf, auth, TPM2_SA_ENCRYPT,
> +					    NULL, 0);
> +	}
> +
> +	tpm_buf_fill_hmac_session(&buf, auth);
> +	rc = tpm_transmit_cmd(chip, &chip->kernel_space, buf.data, PAGE_SIZE,
> +			      6, 0, "unsealing");
> +	rc = tpm_buf_check_hmac_response(&buf, auth, rc);
> 	if (rc > 0)
> 		rc = -EPERM;
>
> @@ -669,17 +714,13 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
> 	u32 blob_handle;
> 	int rc;
>
> -	mutex_lock(&chip->tpm_mutex);
> -	rc = tpm2_load_cmd(chip, payload, options, &blob_handle,
> -			   TPM_TRANSMIT_UNLOCKED);
> +	rc = tpm2_load_cmd(chip, payload, options, &blob_handle);
> 	if (rc)
> 		goto out;
>
> -	rc = tpm2_unseal_cmd(chip, payload, options, blob_handle,
> -			     TPM_TRANSMIT_UNLOCKED);
> -	tpm2_flush_context_cmd(chip, blob_handle, TPM_TRANSMIT_UNLOCKED);
> +	rc = tpm2_unseal_cmd(chip, payload, options, blob_handle);
> +	tpm2_flush_context_cmd(chip, blob_handle, 0);
> out:
> -	mutex_unlock(&chip->tpm_mutex);
> 	return rc;
> }
>
> -- 
> 2.16.4
>
>

This commit makes me almost demand a preliminary patch set that just
does the groundwork and takes new stuff into use (tpm2b).

BTW, the whole patch set should speak integrity and encryption.
"security" essentially means absolutely nothing.

/Jarkko

Re: [PATCH v4 0/7] add integrity and security to TPM2 transactions

[Jarkko Sakkinen]

On Mon, 22 Oct 2018, Ken Goldman wrote:
> Does this design assume that there was at time zero no monitoring?
> This would permit some shared secret to be established.
>
> Or does it assume that the interception may have been present from
> the first boot?  If so, how is the first shared secret established.
> Salting using the EK is the usual method, but this requires walking the EK
> certificate chain and embedding the TPM vendor CA certificates in the kernel.

Kernel gets the public portion EK and uses its own key pair in its own
end so everything should be good, right?

/Jarkko

Re: [PATCH v4 0/7] add integrity and security to TPM2 transactions

[Jarkko Sakkinen]

On Mon, 22 Oct 2018, James Bottomley wrote:
> On Mon, 2018-10-22 at 09:53 -0400, Ken Goldman wrote:
>> On 10/22/2018 3:33 AM, James Bottomley wrote:
>>> By now, everybody knows we have a problem with the TPM2_RS_PW easy
>>> button on TPM2 in that transactions on the TPM bus can be
>>> intercepted and altered.  The way to fix this is to use real
>>> sessions for HMAC capabilities to ensure integrity and to use
>>> parameter and response encryption to ensure confidentiality of the
>>> data flowing over the TPM bus.
>>
>> Does this design assume that there was at time zero no monitoring?
>> This would permit some shared secret to be established.
>>
>> Or does it assume that the interception may have been present from
>> the first boot?  If so, how is the first shared secret established.
>> Salting using the EK is the usual method, but this requires walking
>> the  EK certificate chain and embedding the TPM vendor CA
>> certificates in the  kernel.
>
> The design establishes the shared secret at start of day using an EC
> derived key from the null seed.  This can obviously be spoofed by a TPM
> Genie running before the system was rebooted.  However, the computed
> key name is exposed to user space and TPM2_Certify will fail when
> userspace checks the null seed so you will know after the fact whether
> the communication channel established on boot was secure or not.
>
> It is possible to use either the EPS or SPS if we pass in the public
> points as kernel parameters but this is getting rather complicated for
> casual users.

Where was the code that exposes it to the user space?

/Jarkko

Re: [PATCH v4 0/7] add integrity and security to TPM2 transactions

[James Bottomley]

On Wed, 2018-10-24 at 03:06 +0300, Jarkko Sakkinen wrote:
> On Mon, 22 Oct 2018, Ken Goldman wrote:
> > Does this design assume that there was at time zero no monitoring?
> > This would permit some shared secret to be established.
> > 
> > Or does it assume that the interception may have been present from
> > the first boot?  If so, how is the first shared secret established.
> > Salting using the EK is the usual method, but this requires walking
> > the EK certificate chain and embedding the TPM vendor CA
> > certificates in the kernel.
> 
> Kernel gets the public portion EK and uses its own key pair in its
> own end so everything should be good, right?

Only if we pass it in.  The kernel can't run an X509 proof on the EK
certificate, which is how you verify the EK:  Usually the X509 cert is
RSA, and for speed and efficiency we're salting with Elliptic Curve
keys, so we'd have to derive the RSA EK primary (which can take up to
60 seconds), verify the same public key as the cert and then chain up
to the manufacturer. Then we'd have to use the RSA keys (so a lot more
code) because we can't trust the TPM not to lie about the RSA public
key but then substitute it's own EC primary. We really don't want to be
trying to do all that on boot.

James

Re: [PATCH v4 0/7] add integrity and security to TPM2 transactions

[James Bottomley]

On Wed, 2018-10-24 at 03:13 +0300, Jarkko Sakkinen wrote:
> On Mon, 22 Oct 2018, James Bottomley wrote:
> > On Mon, 2018-10-22 at 09:53 -0400, Ken Goldman wrote:
> > > On 10/22/2018 3:33 AM, James Bottomley wrote:
> > > > By now, everybody knows we have a problem with the TPM2_RS_PW
> > > > easy button on TPM2 in that transactions on the TPM bus can be
> > > > intercepted and altered.  The way to fix this is to use real
> > > > sessions for HMAC capabilities to ensure integrity and to use
> > > > parameter and response encryption to ensure confidentiality of
> > > > the data flowing over the TPM bus.
> > > 
> > > Does this design assume that there was at time zero no
> > > monitoring? This would permit some shared secret to be
> > > established.
> > > 
> > > Or does it assume that the interception may have been present
> > > from the first boot?  If so, how is the first shared secret
> > > established. Salting using the EK is the usual method, but this
> > > requires walking the  EK certificate chain and embedding the TPM
> > > vendor CA certificates in the  kernel.
> > 
> > The design establishes the shared secret at start of day using an
> > EC derived key from the null seed.  This can obviously be spoofed
> > by a TPM Genie running before the system was rebooted.  However,
> > the computed key name is exposed to user space and TPM2_Certify
> > will fail when userspace checks the null seed so you will know
> > after the fact whether the communication channel established on
> > boot was secure or not.
> > 
> > It is possible to use either the EPS or SPS if we pass in the
> > public points as kernel parameters but this is getting rather
> > complicated for casual users.
> 
> Where was the code that exposes it to the user space?

It's patch 6/7.  It exposes the null ec primary name in sysfs:

jejb@jarvis~> cat /sys/class/tpm0/tpm/null_name
000ba4fa35ecfbf7c85e5407d07edc27f6a522c8b1a011bcb68c60b27baf21f9d9ec

The key certification gives you back a signed copy of the name which
you can verify against this.

James

Re: [PATCH v4 0/7] add integrity and security to TPM2 transactions

[James Bottomley]

On Wed, 2018-10-24 at 02:51 +0300, Jarkko Sakkinen wrote:
> I would consider sending first a patch set that would iterate the
> existing session stuff to be ready for this i.e. merge in two
> iterations (emphasis on the word "consider"). We can probably merge
> the groundwork quite fast.

I realise we're going to have merge conflicts on the later ones, so why
don't we do this: I'll still send as one series, but you apply the ones
you think are precursors and I'll rebase and resend the rest?

James

Re: [PATCH v4 2/7] tpm2-sessions: Add full HMAC and encrypt/decrypt session handling

[Jarkko Sakkinen]

On Tue, 23 Oct 2018, Ard Biesheuvel wrote:
> On 23 October 2018 at 04:01, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
>> On Mon, 2018-10-22 at 19:19 -0300, Ard Biesheuvel wrote:
>> [...]
>>>> +static void hmac_init(struct shash_desc *desc, u8 *key, int
>>>> keylen)
>>>> +{
>>>> +       u8 pad[SHA256_BLOCK_SIZE];
>>>> +       int i;
>>>> +
>>>> +       desc->tfm = sha256_hash;
>>>> +       desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
>>>
>>> I don't think this actually does anything in the shash API
>>> implementation, so you can drop this.
>>
>> OK, I find crypto somewhat hard to follow.  There were bits I had to
>> understand, like when I wrote the CFB implementation or when I fixed
>> the ECDH scatterlist handling, but I've got to confess, in time
>> honoured tradition I simply copied this from EVM crypto without
>> actually digging into the code to understand why.
>>
>
> Yeah, it is notoriously hard to use, and we should try to improve that.

James,

I would hope (already said in my review) to use longer than one
character variable names for most of the stuff. I did not quite
understand why you decided to use 'counter' for obvious counter
variable and one character names for non-obvious stuff :-)

I'm not sure where the 'encoded' exactly comes in the variable
name 'encoded_key' especially in the context of these cryptic
names.

/Jarkko

Re: [PATCH v4 2/7] tpm2-sessions: Add full HMAC and encrypt/decrypt session handling

[James Bottomley]

On Wed, 2018-10-24 at 02:48 +0300, Jarkko Sakkinen wrote:
> On Mon, 22 Oct 2018, James Bottomley wrote:
> > [...]

I'll tidy up the descriptions.

> These all sould be combined with the existing session stuff inside
> tpm2-cmd.c and not have duplicate infrastructures. The file name
> should be tpm2-session.c (we neither have tpm2-cmds.c).

You mean move tpm2_buf_append_auth() into the new sessions file as well
... sure, I can do that.

[...]
> > +
> > +/*
> > + * assume hash sha256 and nonces u, v of size SHA256_DIGEST_SIZE
> > but
> > + * otherwise standard KDFa.  Note output is in bytes not bits.
> > + */
> > +static void KDFa(u8 *key, int keylen, const char *label, u8 *u,
> > +		 u8 *v, int bytes, u8 *out)
> 
> Should this be in lower case? I would rename it as tpm_kdfa().

This one is defined as KDFa() in the standards and it's not TPM
specific (although some standards refer to it as KDFA).  I'm not averse
to making them tpm_kdfe() and tpm_kdfa() but I was hoping that one day
the crypto subsystem would need them and we could move them in there
because KDFs are the new shiny in crypto primitives (TLS 1.2 started
using them, for instance).

> > +{
> > +	u32 counter;
> > +	const __be32 bits = cpu_to_be32(bytes * 8);
> > +
> > +	for (counter = 1; bytes > 0; bytes -= SHA256_DIGEST_SIZE,
> > counter++,
> > +		     out += SHA256_DIGEST_SIZE) {
> 
> Only one counter is actually used for anything so this is overly
> complicated and IMHO it is ok to call the counter just 'i'. Maybe
> just:
> 
> for (i = 1; (bytes - (i - 1) * SHA256_DIGEST_SIZE) > 0; i++) {
> 
> > +		SHASH_DESC_ON_STACK(desc, sha256_hash);
> > +		__be32 c = cpu_to_be32(counter);
> > +
> > +		hmac_init(desc, key, keylen);
> > +		crypto_shash_update(desc, (u8 *)&c, sizeof(c));
> > +		crypto_shash_update(desc, label, strlen(label)+1);
> > +		crypto_shash_update(desc, u, SHA256_DIGEST_SIZE);
> > +		crypto_shash_update(desc, v, SHA256_DIGEST_SIZE);
> > +		crypto_shash_update(desc, (u8 *)&bits,
> > sizeof(bits));
> > +		hmac_final(desc, key, keylen, out);
> > +	}
> > +}
> > +
> > +/*
> > + * Somewhat of a bastardization of the real KDFe.  We're assuming
> > + * we're working with known point sizes for the input parameters
> > and
> > + * the hash algorithm is fixed at sha256.  Because we know that
> > the
> > + * point size is 32 bytes like the hash size, there's no need to
> > loop
> > + * in this KDF.
> > + */
> > +static void KDFe(u8 z[EC_PT_SZ], const char *str, u8 *pt_u, u8
> > *pt_v,
> > +		 u8 *keyout)
> > +{
> > +	SHASH_DESC_ON_STACK(desc, sha256_hash);
> > +	/*
> > +	 * this should be an iterative counter, but because we
> > know
> > +	 *  we're only taking 32 bytes for the point using a
> > sha256
> > +	 *  hash which is also 32 bytes, there's only one loop
> > +	 */
> > +	__be32 c = cpu_to_be32(1);
> > +
> > +	desc->tfm = sha256_hash;
> > +	desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
> > +
> > +	crypto_shash_init(desc);
> > +	/* counter (BE) */
> > +	crypto_shash_update(desc, (u8 *)&c, sizeof(c));
> > +	/* secret value */
> > +	crypto_shash_update(desc, z, EC_PT_SZ);
> > +	/* string including trailing zero */
> > +	crypto_shash_update(desc, str, strlen(str)+1);
> > +	crypto_shash_update(desc, pt_u, EC_PT_SZ);
> > +	crypto_shash_update(desc, pt_v, EC_PT_SZ);
> > +	crypto_shash_final(desc, keyout);
> > +}
> > +
> > +static void tpm_buf_append_salt(struct tpm_buf *buf, struct
> > tpm_chip *chip,
> > +				struct tpm2_auth *auth)
> 
> Given the complexity of this function and some not that obvious
> choices in the implementation (coordinates), it would make sense to
> document this function.

I'll try to beef up the salting description

> > +{
> > +	struct crypto_kpp *kpp;
> > +	struct kpp_request *req;
> > +	struct scatterlist s[2], d[1];
> > +	struct ecdh p = {0};
> > +	u8 encoded_key[EC_PT_SZ], *x, *y;
> 
> Why you use one character variable name 'p' and longer name
> 'encoded_key'?
> 
> > +	unsigned int buf_len;
> > +	u8 *secret;
> > +
> > +	secret = kmalloc(EC_PT_SZ, GFP_KERNEL);
> > +	if (!secret)
> > +		return;
> > +
> > +	p.curve_id = ECC_CURVE_NIST_P256;
> 
> Could this be set already in the initialization?

I'm never sure about designated initializers, but I think, after
looking them up again, it will zero fill unmentioned elements.

> > +
> > +	/* secret is two sized points */
> > +	tpm_buf_append_u16(buf, (EC_PT_SZ + 2)*2);
> 
> White space missing. Should be "(EC_PT_SZ + 2) * 2". The comment is a
> bit obscure (maybe, do not have any specific suggestion how to make
> it less obscure).
> 
> > +	/*
> > +	 * we cheat here and append uninitialized data to form
> > +	 * the points.  All we care about is getting the two
> > +	 * co-ordinate pointers, which will be used to overwrite
> > +	 * the uninitialized data
> > +	 */
> 
> "unitialized data" != "random data"
> 
> Why doesn't it matter here?

Because, as the comment says, it eventually gets overwritten by running
ecdh to derive the two co-ordinates.  (pointers to these two
uninitialized areas are passed into the ecdh destination sg list).

> > +	tpm_buf_append_u16(buf, EC_PT_SZ);
> > +	x = &buf->data[tpm_buf_length(buf)];
> > +	tpm_buf_append(buf, encoded_key, EC_PT_SZ);
> > +	tpm_buf_append_u16(buf, EC_PT_SZ);
> > +	y = &buf->data[tpm_buf_length(buf)];
> > +	tpm_buf_append(buf, encoded_key, EC_PT_SZ);
> 
> The points have matching coordinates. Is that ok?

What points?  At the moment all we have are two pointers (x and y) to
uninitialized data areas.

> > +	sg_init_table(s, 2);
> > +	sg_set_buf(&s[0], x, EC_PT_SZ);
> > +	sg_set_buf(&s[1], y, EC_PT_SZ);

Here is the code where we set the output SG list of the crypto ecdh to
the areas x and y point to.

> > +
> > +	kpp = crypto_alloc_kpp("ecdh", CRYPTO_ALG_INTERNAL, 0);
> > +	if (IS_ERR(kpp)) {
> > +		dev_err(&chip->dev, "crypto ecdh allocation
> > failed\n");
> > +		return;
> > +	}
> > +
> > +	buf_len = crypto_ecdh_key_len(&p);
> > +	if (sizeof(encoded_key) < buf_len) {
> > +		dev_err(&chip->dev, "salt buffer too small needs
> > %d\n",
> > +			buf_len);
> > +		goto out;
> > +	}
> 
> In what situation this can happen? Can sizeof(encoded_key) >=
> buf_len?

Yes, but only if someone is trying to crack your ecdh.  One of the
security issues in ecdh is if someone makes a very specific point
choice (usually in the cofactor space) that has a very short period,
the attacker can guess the input to KDFe.  In this case if TPM genie
provided a specially crafted attack EC point, we'd detect it here
because the resulting buffer would be too short.

> > +	crypto_ecdh_encode_key(encoded_key, buf_len, &p);
> > +	/* this generates a random private key */
> > +	crypto_kpp_set_secret(kpp, encoded_key, buf_len);
> > +
> > +	/* salt is now the public point of this private key */
> > +	req = kpp_request_alloc(kpp, GFP_KERNEL);
> > +	if (!req)
> > +		goto out;
> > +	kpp_request_set_input(req, NULL, 0);
> > +	kpp_request_set_output(req, s, EC_PT_SZ*2);
> > +	crypto_kpp_generate_public_key(req);
> > +	/*
> > +	 * we're not done: now we have to compute the shared
> > secret
> > +	 * which is our private key multiplied by the tpm_key
> > public
> > +	 * point, we actually only take the x point and discard
> > the y
> > +	 * point and feed it through KDFe to get the final secret
> > salt
> > +	 */
> > +	sg_set_buf(&s[0], chip->ec_point_x, EC_PT_SZ);
> > +	sg_set_buf(&s[1], chip->ec_point_y, EC_PT_SZ);
> > +	kpp_request_set_input(req, s, EC_PT_SZ*2);
> > +	sg_init_one(d, secret, EC_PT_SZ);
> > +	kpp_request_set_output(req, d, EC_PT_SZ);
> > +	crypto_kpp_compute_shared_secret(req);
> > +	kpp_request_free(req);
> > +
> > +	/* pass the shared secret through KDFe for salt */
> > +	KDFe(secret, "SECRET", x, chip->ec_point_x, auth->salt);
> 
> How is the label chosen?

It's mandated in the TPM standards.

> > + out:
> > +	crypto_free_kpp(kpp);
> > +}
> 
> In general this function should have a clear explanation what it does
> and maybe less these one character variables but instead variables
> with more documenting names. Explain in high level what algorithms
> are used and how the salt is calculated.

I'll try, but this is a rather complex function.

> > +
> > +/**
> > + * tpm_buf_append_hmac_session() append a TPM session element
> > + * @buf: The buffer to be appended
> > + * @auth: the auth structure allocated by
> > tpm2_start_auth_session()
> > + * @attributes: The session attributes
> > + * @passphrase: The session authority (NULL if none)
> > + * @passphraselen: The length of the session authority (0 if none)
> 
> The alignment.

the alignment of what?

> > + *
> > + * This fills in a session structure in the TPM command buffer,
> > except
> > + * for the HMAC which cannot be computed until the command buffer
> > is
> > + * complete.  The type of session is controlled by the
> > @attributes,
> > + * the main ones of which are TPM2_SA_CONTINUE_SESSION which means
> > the
> > + * session won't terminate after tpm_buf_check_hmac_response(),
> > + * TPM2_SA_DECRYPT which means this buffers first parameter should
> > be
> > + * encrypted with a session key and TPM2_SA_ENCRYPT, which means
> > the
> > + * response buffer's first parameter needs to be decrypted
> > (confusing,
> > + * but the defines are written from the point of view of the TPM).
> 
> In the commit message it would be probably relevant to acknowledge
> that only the first parameter can be encrypted/decrypted.

OK.

> > + *
> > + * Any session appended by this command must be finalized by
> > calling
> > + * tpm_buf_fill_hmac_session() otherwise the HMAC will be
> > incorrect
> > + * and the TPM will reject the command.
> > + *
> > + * As with most tpm_buf operations, success is assumed because
> > failure
> > + * will be caused by an incorrect programming model and indicated
> > by a
> > + * kernel message.
> > + */
> > +void tpm_buf_append_hmac_session(struct tpm_buf *buf, struct
> > tpm2_auth *auth,
> > +				 u8 attributes, u8 *passphrase,
> > +				 int passphraselen)
> 
> Would prefer underscore between the words.

You mean passphrase_len?  OK.

> > +{
> > +	u8 nonce[SHA256_DIGEST_SIZE];
> > +	u32 len;
> > +
> > +	/*
> > +	 * The Architecture Guide requires us to strip trailing
> > zeros
> > +	 * before computing the HMAC
> > +	 */
> > +	while (passphrase && passphraselen > 0
> > +	       && passphrase[passphraselen - 1] == '\0')
> > +		passphraselen--;
> 
> Why there would be trailing zeros?

Because TPM 1.2 mandated zero padded fixed size passphrases so the TPM
2.0 standard specifies a way of converting these to variable size
strings by eliminating the zero padding.

James

Re: [PATCH v4 0/7] add integrity and security to TPM2 transactions

[Jarkko Sakkinen]

On Wed, 24 Oct 2018, James Bottomley wrote:
> On Wed, 2018-10-24 at 03:13 +0300, Jarkko Sakkinen wrote:
>> On Mon, 22 Oct 2018, James Bottomley wrote:
>>> On Mon, 2018-10-22 at 09:53 -0400, Ken Goldman wrote:
>>>> On 10/22/2018 3:33 AM, James Bottomley wrote:
>>>>> By now, everybody knows we have a problem with the TPM2_RS_PW
>>>>> easy button on TPM2 in that transactions on the TPM bus can be
>>>>> intercepted and altered.  The way to fix this is to use real
>>>>> sessions for HMAC capabilities to ensure integrity and to use
>>>>> parameter and response encryption to ensure confidentiality of
>>>>> the data flowing over the TPM bus.
>>>>
>>>> Does this design assume that there was at time zero no
>>>> monitoring? This would permit some shared secret to be
>>>> established.
>>>>
>>>> Or does it assume that the interception may have been present
>>>> from the first boot?  If so, how is the first shared secret
>>>> established. Salting using the EK is the usual method, but this
>>>> requires walking the  EK certificate chain and embedding the TPM
>>>> vendor CA certificates in the  kernel.
>>>
>>> The design establishes the shared secret at start of day using an
>>> EC derived key from the null seed.  This can obviously be spoofed
>>> by a TPM Genie running before the system was rebooted.  However,
>>> the computed key name is exposed to user space and TPM2_Certify
>>> will fail when userspace checks the null seed so you will know
>>> after the fact whether the communication channel established on
>>> boot was secure or not.
>>>
>>> It is possible to use either the EPS or SPS if we pass in the
>>> public points as kernel parameters but this is getting rather
>>> complicated for casual users.
>>
>> Where was the code that exposes it to the user space?
>
> It's patch 6/7.  It exposes the null ec primary name in sysfs:
>
> jejb@jarvis~> cat /sys/class/tpm0/tpm/null_name
> 000ba4fa35ecfbf7c85e5407d07edc27f6a522c8b1a011bcb68c60b27baf21f9d9ec
>
> The key certification gives you back a signed copy of the name which
> you can verify against this.
>
> James

I missed 6/7, sorry.

Do you have an example user space program for doing the verification
by using this file?

/Jarkko

Re: [PATCH v4 0/7] add integrity and security to TPM2 transactions

[Jarkko Sakkinen]

On Wed, 24 Oct 2018, James Bottomley wrote:
> On Wed, 2018-10-24 at 02:51 +0300, Jarkko Sakkinen wrote:
>> I would consider sending first a patch set that would iterate the
>> existing session stuff to be ready for this i.e. merge in two
>> iterations (emphasis on the word "consider"). We can probably merge
>> the groundwork quite fast.
>
> I realise we're going to have merge conflicts on the later ones, so why
> don't we do this: I'll still send as one series, but you apply the ones
> you think are precursors and I'll rebase and resend the rest?
>
> James

Works for me and now I think after yesterdays dicussions etc. that this
should be merged as one series.

/Jarkko

Re: [PATCH v4 2/7] tpm2-sessions: Add full HMAC and encrypt/decrypt session handling

[Jarkko Sakkinen]

On Wed, 24 Oct 2018, James Bottomley wrote:
>>> +static void KDFa(u8 *key, int keylen, const char *label, u8 *u,
>>> +		 u8 *v, int bytes, u8 *out)
>>
>> Should this be in lower case? I would rename it as tpm_kdfa().
>
> This one is defined as KDFa() in the standards and it's not TPM
> specific (although some standards refer to it as KDFA).  I'm not averse
> to making them tpm_kdfe() and tpm_kdfa() but I was hoping that one day
> the crypto subsystem would need them and we could move them in there
> because KDFs are the new shiny in crypto primitives (TLS 1.2 started
> using them, for instance).

I care more about tracing and debugging than naming and having 'tpm_' in
front of every TPM function makes tracing a lean process. AFAIK using
upper case letters is against kernel coding conventions. I'm not sure
why this would make an exception on that.

>> Why doesn't it matter here?
>
> Because, as the comment says, it eventually gets overwritten by running
> ecdh to derive the two co-ordinates.  (pointers to these two
> uninitialized areas are passed into the ecdh destination sg list).

Oh, I just misunderstood the comment. Wouldn't it be easier to say that
the data is initialized later?

>>> +	buf_len = crypto_ecdh_key_len(&p);
>>> +	if (sizeof(encoded_key) < buf_len) {
>>> +		dev_err(&chip->dev, "salt buffer too small needs
>>> %d\n",
>>> +			buf_len);
>>> +		goto out;
>>> +	}
>>
>> In what situation this can happen? Can sizeof(encoded_key) >=
>> buf_len?
>
> Yes, but only if someone is trying to crack your ecdh.  One of the
> security issues in ecdh is if someone makes a very specific point
> choice (usually in the cofactor space) that has a very short period,
> the attacker can guess the input to KDFe.  In this case if TPM genie
> provided a specially crafted attack EC point, we'd detect it here
> because the resulting buffer would be too short.

Right. Thank you for the explanation. Here some kind of comment might
not be a bad idea...

>> In general this function should have a clear explanation what it does
>> and maybe less these one character variables but instead variables
>> with more documenting names. Explain in high level what algorithms
>> are used and how the salt is calculated.
>
> I'll try, but this is a rather complex function.

Understood. I do not expect perfection here and we can improve
documetation later on.

For anyone wanting to review James' patches and w/o much experience on
EC, I recommend reading this article:

https://arstechnica.com/information-technology/2013/10/a-relatively-easy-to-understand-primer-on-elliptic-curve-cryptography/

I read it few years ago and refreshed my memory few days ago by
re-reading it.

>
>>> +
>>> +/**
>>> + * tpm_buf_append_hmac_session() append a TPM session element
>>> + * @buf: The buffer to be appended
>>> + * @auth: the auth structure allocated by
>>> tpm2_start_auth_session()
>>> + * @attributes: The session attributes
>>> + * @passphrase: The session authority (NULL if none)
>>> + * @passphraselen: The length of the session authority (0 if none)
>>
>> The alignment.
>
> the alignment of what?

We generally have parameter descriptions tab-aligned.

>> Why there would be trailing zeros?
>
> Because TPM 1.2 mandated zero padded fixed size passphrases so the TPM
> 2.0 standard specifies a way of converting these to variable size
> strings by eliminating the zero padding.

Ok.

> James

I'm also looking forward for the CONTEXT_GAP patch based on the
yesterdays discussion. We do want it and I was stupid not to take it
couple years ago :-) Thanks.

/Jarkko

Re: [PATCH v4 0/7] add integrity and security to TPM2 transactions

[Ken Goldman]

On 10/24/2018 3:34 AM, James Bottomley wrote:
> so we'd have to derive the RSA EK primary (which can take up to
> 60 seconds)

Just FYI:

The TPM vendors seem to be caching the two standard EKs during
manufacturing.  When the TPM detects the EK template during
createprimary, it quickly returns the cached value.

This optimization is not required, but it is common.