[PATCH 0/8] Fix TPM 2.0 trusted keys

[PATCH 0/8] Fix TPM 2.0 trusted keys

[James Bottomley]

This fixes a wide array of problems with the current TPM 2.0
implementation of trusted keys.  Since policy based trusted keys never
worked in the current implementation, I've rewritten the policy
implementation to make it easier to use and so the trusted key handler
can understand what elements of a policy are failing and why.

Apart from fixing bugs like volatile object leakage, I've changed the
output format to use the standardised ASN.1 coding for TPM2 keys,
meaning they should interoperate with userspace TPM2 key
implementations.  Apart from interoperability, another advantage of the
existing key format is that it carries all parameters like parent and
hash with it and it is capable of carrying policy directives in a way
that mean they're tied permanently to the key (no having to try to
remember what the policy was and reconstruct it from userspace).  This
actually allows us to support the TPM 1.2 commands like pcrinfo easily
in 2.0.

The big problem with this patch is still that we can't yet combine
policy with authorization because that requires proper session
handling, but at least with this rewrite it becomes possible (whereas
it was never possible with the old external policy session code). 
Thus, when we have the TPM 2.0 security patch upstream, we'll be able
to use the session logic from that patch to imlement authorizations.

James

---

James Bottomley (8):
  security: keys: trusted: flush the key handle after use
  lib: add asn.1 encoder
  oid_registry: Add TCG defined OIDS for TPM keys
  security: keys: trusted: use ASN.1 tpm2 key format for the blobs
  security: keys: trusted: Make sealed key properly interoperable
  security: keys: trusted: add PCR policy to TPM2 keys
  security: keys: trusted: add ability to specify arbitrary policy
  security: keys: trusted: implement counter/timer policy

 Documentation/security/keys/trusted-encrypted.rst |  70 +++-
 drivers/char/tpm/tpm.h                            |   1 -
 drivers/char/tpm/tpm2-cmd.c                       |   1 +
 include/keys/trusted-type.h                       |   6 +-
 include/linux/asn1_encoder.h                      |  21 ++
 include/linux/oid_registry.h                      |   5 +
 include/linux/tpm.h                               |   8 +
 lib/Makefile                                      |   2 +-
 lib/asn1_encoder.c                                | 201 +++++++++++
 security/keys/Kconfig                             |   2 +
 security/keys/trusted-keys/Makefile               |   2 +-
 security/keys/trusted-keys/tpm2-policy.c          | 409 ++++++++++++++++++++++
 security/keys/trusted-keys/tpm2-policy.h          |  31 ++
 security/keys/trusted-keys/tpm2key.asn1           |  23 ++
 security/keys/trusted-keys/trusted_tpm1.c         |  40 +--
 security/keys/trusted-keys/trusted_tpm2.c         | 285 +++++++++++++--
 16 files changed, 1050 insertions(+), 57 deletions(-)
 create mode 100644 include/linux/asn1_encoder.h
 create mode 100644 lib/asn1_encoder.c
 create mode 100644 security/keys/trusted-keys/tpm2-policy.c
 create mode 100644 security/keys/trusted-keys/tpm2-policy.h
 create mode 100644 security/keys/trusted-keys/tpm2key.asn1

-- 
2.16.4

[PATCH 1/8] security: keys: trusted: flush the key handle after use

[James Bottomley]

The trusted keys code currently loads a blob into the TPM and unseals
on the handle.  However, it never flushes the handle meaning that
volatile contexts build up until the TPM becomes unusable.  Fix this
by flushing the handle after the unseal.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 drivers/char/tpm/tpm.h                    | 1 -
 drivers/char/tpm/tpm2-cmd.c               | 1 +
 include/linux/tpm.h                       | 1 +
 security/keys/trusted-keys/trusted_tpm2.c | 1 +
 4 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index b9e1547be6b5..5620747da0cf 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -218,7 +218,6 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
 int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
 		    struct tpm_digest *digests);
 int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
-void tpm2_flush_context(struct tpm_chip *chip, u32 handle);
 ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
 			u32 *value, const char *desc);
 
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index fdb457704aa7..b87592f4a6c7 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -362,6 +362,7 @@ void tpm2_flush_context(struct tpm_chip *chip, u32 handle)
 	tpm_transmit_cmd(chip, &buf, 0, "flushing context");
 	tpm_buf_destroy(&buf);
 }
+EXPORT_SYMBOL(tpm2_flush_context);
 
 struct tpm2_get_cap_out {
 	u8 more_data;
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 0d6e949ba315..03e9b184411b 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -403,6 +403,7 @@ extern int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
 extern int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen);
 extern int tpm_get_random(struct tpm_chip *chip, u8 *data, size_t max);
 extern struct tpm_chip *tpm_default_chip(void);
+void tpm2_flush_context(struct tpm_chip *chip, u32 handle);
 #else
 static inline int tpm_is_tpm2(struct tpm_chip *chip)
 {
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index a9810ac2776f..08ec7f48f01d 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -309,6 +309,7 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
 		return rc;
 
 	rc = tpm2_unseal_cmd(chip, payload, options, blob_handle);
+	tpm2_flush_context(chip, blob_handle);
 
 	return rc;
 }
-- 
2.16.4

Re: [PATCH 1/8] security: keys: trusted: flush the key handle after use

[David Woodhouse]

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

On Sat, 2019-12-07 at 21:07 -0800, James Bottomley wrote:
> The trusted keys code currently loads a blob into the TPM and unseals
> on the handle.  However, it never flushes the handle meaning that
> volatile contexts build up until the TPM becomes unusable.  Fix this
> by flushing the handle after the unseal.
> 
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> ---
>  drivers/char/tpm/tpm.h                    | 1 -
>  drivers/char/tpm/tpm2-cmd.c               | 1 +
>  include/linux/tpm.h                       | 1 +
>  security/keys/trusted-keys/trusted_tpm2.c | 1 +
>  4 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index b9e1547be6b5..5620747da0cf 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -218,7 +218,6 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
>  int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
>  		    struct tpm_digest *digests);
>  int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
> -void tpm2_flush_context(struct tpm_chip *chip, u32 handle);
>  ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
>  			u32 *value, const char *desc);
>  
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index fdb457704aa7..b87592f4a6c7 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -362,6 +362,7 @@ void tpm2_flush_context(struct tpm_chip *chip, u32 handle)
>  	tpm_transmit_cmd(chip, &buf, 0, "flushing context");
>  	tpm_buf_destroy(&buf);
>  }
> +EXPORT_SYMBOL(tpm2_flush_context);


Everything else is EXPORT_SYMBOL_GPL(). Why EXPORT_SYMBOL() here?



[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5174 bytes --]

Re: [PATCH 1/8] security: keys: trusted: flush the key handle after use

[James Bottomley]

On Mon, 2019-12-09 at 08:31 +0000, David Woodhouse wrote:
> On Sat, 2019-12-07 at 21:07 -0800, James Bottomley wrote:
> > The trusted keys code currently loads a blob into the TPM and
> > unseals
> > on the handle.  However, it never flushes the handle meaning that
> > volatile contexts build up until the TPM becomes unusable.  Fix
> > this
> > by flushing the handle after the unseal.
> > 
> > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.c
> > om>
> > ---
> >  drivers/char/tpm/tpm.h                    | 1 -
> >  drivers/char/tpm/tpm2-cmd.c               | 1 +
> >  include/linux/tpm.h                       | 1 +
> >  security/keys/trusted-keys/trusted_tpm2.c | 1 +
> >  4 files changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > index b9e1547be6b5..5620747da0cf 100644
> > --- a/drivers/char/tpm/tpm.h
> > +++ b/drivers/char/tpm/tpm.h
> > @@ -218,7 +218,6 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32
> > pcr_idx,
> >  int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
> >  		    struct tpm_digest *digests);
> >  int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
> > -void tpm2_flush_context(struct tpm_chip *chip, u32 handle);
> >  ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
> >  			u32 *value, const char *desc);
> >  
> > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-
> > cmd.c
> > index fdb457704aa7..b87592f4a6c7 100644
> > --- a/drivers/char/tpm/tpm2-cmd.c
> > +++ b/drivers/char/tpm/tpm2-cmd.c
> > @@ -362,6 +362,7 @@ void tpm2_flush_context(struct tpm_chip *chip,
> > u32 handle)
> >  	tpm_transmit_cmd(chip, &buf, 0, "flushing context");
> >  	tpm_buf_destroy(&buf);
> >  }
> > +EXPORT_SYMBOL(tpm2_flush_context);
> 
> 
> Everything else is EXPORT_SYMBOL_GPL(). Why EXPORT_SYMBOL() here?

No reason ... well, except I'm not sure the difference has any utility,
but since I don't really care either way, I'll change all the exports.

James

[PATCH 2/8] lib: add asn.1 encoder

[James Bottomley]

We have a need in the TPM trusted keys to return the ASN.1 form of the
TPM key blob so it can be operated on by tools outside of the kernel.
To do that, we have to be able to read and write the key format.  The
current ASN.1 decoder does fine for reading, but we need pieces of an
ASN.1 encoder to return the key blob.

The current implementation only encodes the ASN.1 bits we actually need.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 include/linux/asn1_encoder.h |  21 +++++
 lib/Makefile                 |   2 +-
 lib/asn1_encoder.c           | 201 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 223 insertions(+), 1 deletion(-)
 create mode 100644 include/linux/asn1_encoder.h
 create mode 100644 lib/asn1_encoder.c

diff --git a/include/linux/asn1_encoder.h b/include/linux/asn1_encoder.h
new file mode 100644
index 000000000000..0e908d748c8e
--- /dev/null
+++ b/include/linux/asn1_encoder.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef _LINUX_ASN1_ENCODER_H
+#define _LINUX_ASN1_ENCODER_H
+
+#include <linux/types.h>
+#include <linux/asn1.h>
+#include <linux/asn1_ber_bytecode.h>
+
+#define asn1_oid_len(oid) (sizeof(oid)/sizeof(u32))
+void asn1_encode_integer(unsigned char **_data, u64 integer);
+void asn1_encode_oid(unsigned char **_data, u32 oid[], int oid_len);
+void asn1_encode_tag(unsigned char **data, u32 tag,
+		     const unsigned char *string, u32 len);
+void asn1_encode_octet_string(unsigned char **data, const unsigned char *string,
+			      u32 len);
+void asn1_encode_sequence(unsigned char **data, const unsigned char *seq,
+			  u32 len);
+void asn1_encode_boolean(unsigned char **data, bool val);
+
+#endif
diff --git a/lib/Makefile b/lib/Makefile
index c2f0e2a4e4e8..515b35f92c3c 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -233,7 +233,7 @@ obj-$(CONFIG_INTERVAL_TREE_TEST) += interval_tree_test.o
 
 obj-$(CONFIG_PERCPU_TEST) += percpu_test.o
 
-obj-$(CONFIG_ASN1) += asn1_decoder.o
+obj-$(CONFIG_ASN1) += asn1_decoder.o asn1_encoder.o
 
 obj-$(CONFIG_FONT_SUPPORT) += fonts/
 
diff --git a/lib/asn1_encoder.c b/lib/asn1_encoder.c
new file mode 100644
index 000000000000..2a3b96761344
--- /dev/null
+++ b/lib/asn1_encoder.c
@@ -0,0 +1,201 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Simple encoder primitives for ASN.1 BER/DER/CER
+ *
+ * Copyright (C) 2019 James.Bottomley@HansenPartnership.com
+ */
+
+#include <linux/asn1_encoder.h>
+#include <linux/bug.h>
+#include <linux/string.h>
+#include <linux/module.h>
+
+/**
+ * asn1_encode_integer - encode positive integer to ASN.1
+ * @_data: pointer to the pointer to the data
+ * @integer: integer to be encoded
+ *
+ * This is a simplified encoder: since we know the integer is
+ * positive we don't have to bother with twos complement and since we
+ * know the largest integer is a u64, we know the max length is 8.
+ */
+void asn1_encode_integer(unsigned char **_data, u64 integer)
+{
+	unsigned char *data = *_data, *d = &data[2];
+	int i;
+	bool found = false;
+
+	data[0] = _tag(UNIV, PRIM, INT);
+	if (integer == 0) {
+		*d++ = 0;
+		goto out;
+	}
+	for (i = sizeof(integer); i > 0 ; i--) {
+		int byte = integer >> (8*(i-1));
+
+		if (!found && byte == 0)
+			continue;
+		found = true;
+		if (byte & 0x80)
+			*d++ = 0;
+		*d++ = byte;
+	}
+ out:
+	data[1] = d - data - 2;
+	*_data = d;
+}
+EXPORT_SYMBOL(asn1_encode_integer);
+
+/* calculate the base 128 digit values setting the top bit of the first octet */
+static void asn1_encode_oid_digit(unsigned char **_data, u32 oid)
+{
+	int start = 7 + 7 + 7 + 7;
+	unsigned char *data = *_data;
+
+	/* quick case */
+	if (oid == 0) {
+		*data++ = 0x80;
+		goto out;
+	}
+
+	while (oid >> start == 0)
+		start -= 7;
+
+	while (start > 0) {
+		u8 byte;
+
+		byte = oid >> start;
+		oid = oid - (byte << start);
+		start -= 7;
+		byte |= 0x80;
+		*data++ = byte;
+	}
+	*data++ = oid;
+
+ out:
+	*_data = data;
+}
+
+/**
+ * asn1_encode_oid - encode an oid to ASN.1
+ * @_data: position to begin encoding at
+ * @oid: array of oids
+ * @oid_len: length of oid array
+ *
+ * this encodes an OID up to ASN.1 when presented as an array of OID values
+ */
+void asn1_encode_oid(unsigned char **_data, u32 oid[], int oid_len)
+{
+	unsigned char *data = *_data;
+	unsigned char *d = data + 2;
+	int i;
+
+	if (WARN(oid_len < 2, "OID must have at least two elements"))
+		return;
+	if (WARN(oid_len > 32, "OID is too large"))
+		return;
+
+	data[0] = _tag(UNIV, PRIM, OID);
+	*d++ = oid[0] * 40 + oid[1];
+	for (i = 2; i < oid_len; i++)
+		asn1_encode_oid_digit(&d, oid[i]);
+	data[1] = d - data - 2;
+	*_data = d;
+}
+EXPORT_SYMBOL(asn1_encode_oid);
+
+static void asn1_encode_definite_length(unsigned char **data, u32 len)
+{
+	if (len <= 0x7f) {
+		*((*data)++) = len;
+		return;
+	}
+	if (len <= 0xff) {
+		*((*data)++) = 0x81;
+		*((*data)++) = len & 0xff;
+		return;
+	}
+	if (len <= 0xffff) {
+		*((*data)++) = 0x82;
+		*((*data)++) = (len >> 8) & 0xff;
+		*((*data)++) = len & 0xff;
+		return;
+	}
+
+	if (WARN(len > 0xffffff, "ASN.1 length can't be > 0xffffff"))
+		return;
+
+	*((*data)++) = 0x83;
+	*((*data)++) = (len >> 16) & 0xff;
+	*((*data)++) = (len >> 8) & 0xff;
+	*((*data)++) = len & 0xff;
+}
+
+/**
+ * asn1_encode_tag - add a tag for optional or explicit value
+ * @data: pointer to place tag at
+ * @tag: tag to be placed
+ * @string: the data to be tagged
+ * @len: the length of the data to be tagged
+ *
+ * Note this currently only handles short form tags < 31
+ */
+void asn1_encode_tag(unsigned char **data, u32 tag,
+		     const unsigned char *string, u32 len)
+{
+	if (WARN(tag > 30, "ASN.1 tag can't be > 30"))
+		return;
+
+	*((*data)++) = _tagn(CONT, CONS, tag);
+	asn1_encode_definite_length(data, len);
+	memcpy(*data, string, len);
+	*data += len;
+}
+EXPORT_SYMBOL(asn1_encode_tag);
+
+/**
+ * asn1_encode_octet_string - encode an ASN.1 OCTET STRING
+ * @data: pointer to encode at
+ * @string: string to be encoded
+ * @len: length of string
+ *
+ * Note ASN.1 octet strings may contain zeros, so the length is obligatory.
+ */
+void asn1_encode_octet_string(unsigned char **data, const unsigned char *string,
+			      u32 len)
+{
+	*((*data)++) = _tag(UNIV, PRIM, OTS);
+	asn1_encode_definite_length(data, len);
+	memcpy(*data, string, len);
+	*data += len;
+}
+EXPORT_SYMBOL(asn1_encode_octet_string);
+
+/**
+ * asn1_encode_sequence - wrap a byte stream in an ASN.1 SEQUENCE
+ * @data: pointer to encode at
+ * @seq: data to be encoded as a sequence
+ * @len: length of the data to be encoded as a sequence
+ */
+void asn1_encode_sequence(unsigned char **data, const unsigned char *seq,
+			  u32 len)
+{
+	*((*data)++) = _tag(UNIV, CONS, SEQ);
+	asn1_encode_definite_length(data, len);
+	memcpy(*data, seq, len);
+	*data += len;
+}
+EXPORT_SYMBOL(asn1_encode_sequence);
+
+/**
+ * asn1_encode_boolean - encode a boolean value to ASN.1
+ * @data: pointer to encode at
+ * @val: the boolean true/false value
+ */
+void asn1_encode_boolean(unsigned char **data, bool val)
+{
+	*((*data)++) = _tag(UNIV, PRIM, BOOL);
+	asn1_encode_definite_length(data, 1);
+	*((*data)++) = val ? 1 : 0;
+}
+EXPORT_SYMBOL(asn1_encode_boolean);
-- 
2.16.4

Re: [PATCH 2/8] lib: add asn.1 encoder

[David Woodhouse]

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

On Sat, 2019-12-07 at 21:08 -0800, James Bottomley wrote:
> +/**
> + * asn1_encode_integer - encode positive integer to ASN.1
> + * @_data: pointer to the pointer to the data
> + * @integer: integer to be encoded
> + *
> + * This is a simplified encoder: since we know the integer is
> + * positive we don't have to bother with twos complement and since we
> + * know the largest integer is a u64, we know the max length is 8.
> + */
> +void asn1_encode_integer(unsigned char **_data, u64 integer)
> +{
> +	unsigned char *data = *_data, *d = &data[2];
> +	int i;
> +	bool found = false;
> +
> +	data[0] = _tag(UNIV, PRIM, INT);
> +	if (integer == 0) {
> +		*d++ = 0;
> +		goto out;
> +	}
> +	for (i = sizeof(integer); i > 0 ; i--) {
> +		int byte = integer >> (8*(i-1));
> +
> +		if (!found && byte == 0)
> +			continue;
> +		found = true;
> +		if (byte & 0x80)
> +			*d++ = 0;
> +		*d++ = byte;
> +	}
> + out:
> +	data[1] = d - data - 2;
> +	*_data = d;
> +}

I'd be a lot happier to see a 'buffer length' argument here. This API is just one accidental u64 underflow away from a caller which "knows" its <128 integer is only three bytes long, actually taking eleven and overflowing its buffer. Especially since  you are actively encouraging people to create fragments on the stack and then assemble them into SEQUENCES later (qv¹).

Also: is documenting it as taking a 'positive integer' enough? Making
that explicit in the function name might be more likely to prevent
future users from assuming it actually encodes an arbitrary INTEGER.

> +static void asn1_encode_definite_length(unsigned char **data, u32 len)
> +{
> +	if (len <= 0x7f) {
> +		*((*data)++) = len;
> +		return;
> +	}
> +	if (len <= 0xff) {
> +		*((*data)++) = 0x81;
> +		*((*data)++) = len & 0xff;
> +		return;
> +	}
> +	if (len <= 0xffff) {
> +		*((*data)++) = 0x82;
> +		*((*data)++) = (len >> 8) & 0xff;
> +		*((*data)++) = len & 0xff;
> +		return;
> +	}
> +
> +	if (WARN(len > 0xffffff, "ASN.1 length can't be > 0xffffff"))
> +		return;
> +
> +	*((*data)++) = 0x83;
> +	*((*data)++) = (len >> 16) & 0xff;
> +	*((*data)++) = (len >> 8) & 0xff;
> +	*((*data)++) = len & 0xff;
> +}

(¹)

That's nice when you know the length in advance. Less so when you
don't, because you have to either calculate it first or actually create
the whole of the content in a separate buffer and copy it around.

It would be useful to permit sequences with indeterminate length. You
could even return a pointer which allows them to be changed to definite
length if they are <128 bytes at the end.

I note that later in this series in tpm2_encode_policy() you are
eschewing your own API for this, and doing just what I said above —
going back and filling in the length later.

> +/**
> + * asn1_encode_tag - add a tag for optional or explicit value
> + * @data: pointer to place tag at
> + * @tag: tag to be placed
> + * @string: the data to be tagged
> + * @len: the length of the data to be tagged
> + *
> + * Note this currently only handles short form tags < 31
> + */
> +void asn1_encode_tag(unsigned char **data, u32 tag,
> +		     const unsigned char *string, u32 len)
> +{
> +	if (WARN(tag > 30, "ASN.1 tag can't be > 30"))
> +		return;
> +
> +	*((*data)++) = _tagn(CONT, CONS, tag);
> +	asn1_encode_definite_length(data, len);
> +	memcpy(*data, string, len);
> +	*data += len;
> +}
> +EXPORT_SYMBOL(asn1_encode_tag);

EXPORT_SYMBOL() again when everything else here uses
EXPORT_SYMBOL_GPL().



[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5174 bytes --]

Re: [PATCH 2/8] lib: add asn.1 encoder

[James Bottomley]

On Mon, 2019-12-09 at 08:50 +0000, David Woodhouse wrote:
> On Sat, 2019-12-07 at 21:08 -0800, James Bottomley wrote:
> > +/**
> > + * asn1_encode_integer - encode positive integer to ASN.1
> > + * @_data: pointer to the pointer to the data
> > + * @integer: integer to be encoded
> > + *
> > + * This is a simplified encoder: since we know the integer is
> > + * positive we don't have to bother with twos complement and since
> > we
> > + * know the largest integer is a u64, we know the max length is 8.
> > + */
> > +void asn1_encode_integer(unsigned char **_data, u64 integer)
> > +{
> > +	unsigned char *data = *_data, *d = &data[2];
> > +	int i;
> > +	bool found = false;
> > +
> > +	data[0] = _tag(UNIV, PRIM, INT);
> > +	if (integer == 0) {
> > +		*d++ = 0;
> > +		goto out;
> > +	}
> > +	for (i = sizeof(integer); i > 0 ; i--) {
> > +		int byte = integer >> (8*(i-1));
> > +
> > +		if (!found && byte == 0)
> > +			continue;
> > +		found = true;
> > +		if (byte & 0x80)
> > +			*d++ = 0;
> > +		*d++ = byte;
> > +	}
> > + out:
> > +	data[1] = d - data - 2;
> > +	*_data = d;
> > +}
> 
> I'd be a lot happier to see a 'buffer length' argument here. This API
> is just one accidental u64 underflow away from a caller which "knows"
> its <128 integer is only three bytes long, actually taking eleven and
> overflowing its buffer. Especially since  you are actively
> encouraging people to create fragments on the stack and then assemble
> them into SEQUENCES later (qv¹).

OK, I'll add a length argument.

> Also: is documenting it as taking a 'positive integer' enough? Making
> that explicit in the function name might be more likely to prevent
> future users from assuming i actually encodes an arbitrary INTEGER.

Well, I have no use case for a signed integer at the moment, but it
shouldn't be too hard to add, so if that use case came along someone
could fill in the code ... it just involves stripping off leading
0xff's.

How about I make the input an s64 and return EINVAL on negative?  That
way the API is ready for the negative case.

> > +static void asn1_encode_definite_length(unsigned char **data, u32
> > len)
> > +{
> > +	if (len <= 0x7f) {
> > +		*((*data)++) = len;
> > +		return;
> > +	}
> > +	if (len <= 0xff) {
> > +		*((*data)++) = 0x81;
> > +		*((*data)++) = len & 0xff;
> > +		return;
> > +	}
> > +	if (len <= 0xffff) {
> > +		*((*data)++) = 0x82;
> > +		*((*data)++) = (len >> 8) & 0xff;
> > +		*((*data)++) = len & 0xff;
> > +		return;
> > +	}
> > +
> > +	if (WARN(len > 0xffffff, "ASN.1 length can't be >
> > 0xffffff"))
> > +		return;
> > +
> > +	*((*data)++) = 0x83;
> > +	*((*data)++) = (len >> 16) & 0xff;
> > +	*((*data)++) = (len >> 8) & 0xff;
> > +	*((*data)++) = len & 0xff;
> > +}
> 
> (¹)
> 
> That's nice when you know the length in advance. Less so when you
> don't, because you have to either calculate it first or actually
> create
> the whole of the content in a separate buffer and copy it around.
> 
> It would be useful to permit sequences with indeterminate length. You
> could even return a pointer which allows them to be changed to
> definite
> length if they are <128 bytes at the end.
> 
> I note that later in this series in tpm2_encode_policy() you are
> eschewing your own API for this, and doing just what I said above —
> going back and filling in the length later.

Yes, that bit bothers me.  I'll fix it to do indefinite followed by
updateable sequence lengths and the same for the tag code.

James


> > +/**
> > + * asn1_encode_tag - add a tag for optional or explicit value
> > + * @data: pointer to place tag at
> > + * @tag: tag to be placed
> > + * @string: the data to be tagged
> > + * @len: the length of the data to be tagged
> > + *
> > + * Note this currently only handles short form tags < 31
> > + */
> > +void asn1_encode_tag(unsigned char **data, u32 tag,
> > +		     const unsigned char *string, u32 len)
> > +{
> > +	if (WARN(tag > 30, "ASN.1 tag can't be > 30"))
> > +		return;
> > +
> > +	*((*data)++) = _tagn(CONT, CONS, tag);
> > +	asn1_encode_definite_length(data, len);
> > +	memcpy(*data, string, len);
> > +	*data += len;
> > +}
> > +EXPORT_SYMBOL(asn1_encode_tag);
> 
> EXPORT_SYMBOL() again when everything else here uses
> EXPORT_SYMBOL_GPL().
> 
> 

Re: [PATCH 2/8] lib: add asn.1 encoder

[Matthew Garrett]

On Sat, Dec 7, 2019 at 9:08 PM James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
>
> We have a need in the TPM trusted keys to return the ASN.1 form of the
> TPM key blob so it can be operated on by tools outside of the kernel.
> To do that, we have to be able to read and write the key format.  The
> current ASN.1 decoder does fine for reading, but we need pieces of an
> ASN.1 encoder to return the key blob.

Is there a reason the kernel needs to do this encoding, rather than
having something in userland do the translation?

Re: [PATCH 2/8] lib: add asn.1 encoder

[James Bottomley]

On Mon, 2019-12-09 at 14:05 -0800, Matthew Garrett wrote:
> On Sat, Dec 7, 2019 at 9:08 PM James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > 
> > We have a need in the TPM trusted keys to return the ASN.1 form of
> > the TPM key blob so it can be operated on by tools outside of the
> > kernel. To do that, we have to be able to read and write the key
> > format.  The current ASN.1 decoder does fine for reading, but we
> > need pieces of an ASN.1 encoder to return the key blob.
> 
> Is there a reason the kernel needs to do this encoding, rather than
> having something in userland do the translation?

Well, yes, we'd have to define a format to pass up first and then you'd
always need an encoder programme to do it.  Given it's fairly simple to
encode the key format, doing it directly in ASN.1 ... especially as we
already read ASN.1 keys, seems to be the best for the user.

James

[PATCH 3/8] oid_registry: Add TCG defined OIDS for TPM keys

[James Bottomley]

The TCG has defined an OID prefix "2.23.133.10.1" for the various TPM
key uses.  We've defined three of the available numbers:

2.23.133.10.1.3 TPM Loadable key.  This is an asymmetric key (Usually
		RSA2048 or Elliptic Curve) which can be imported by a
		TPM2_Load() operation.

2.23.133.10.1.4 TPM Importable Key.  This is an asymmetric key (Usually
		RSA2048 or Elliptic Curve) which can be imported by a
		TPM2_Import() operation.

Both loadable and importable keys are specific to a given TPM, the
difference is that a loadable key is wrapped with the symmetric
secret, so must have been created by the TPM itself.  An importable
key is wrapped with a DH shared secret, and may be created without
access to the TPM provided you know the public part of the parent key.

2.23.133.10.1.5 TPM Sealed Data.  This is a set of data (up to 128
		bytes) which is sealed by the TPM.  It usually
		represents a symmetric key and must be unsealed before
		use.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 include/linux/oid_registry.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h
index 657d6bf2c064..a4cee888f9b0 100644
--- a/include/linux/oid_registry.h
+++ b/include/linux/oid_registry.h
@@ -107,6 +107,11 @@ enum OID {
 	OID_gostTC26Sign512B,		/* 1.2.643.7.1.2.1.2.2 */
 	OID_gostTC26Sign512C,		/* 1.2.643.7.1.2.1.2.3 */
 
+	/* TCG defined OIDS for TPM based keys */
+	OID_TPMLoadableKey,		/* 2.23.133.10.1.3 */
+	OID_TPMImporableKey,		/* 2.23.133.10.1.4 */
+	OID_TPMSealedData,		/* 2.23.133.10.1.5 */
+
 	OID__NR
 };
 
-- 
2.16.4

Re: [PATCH 3/8] oid_registry: Add TCG defined OIDS for TPM keys

[David Woodhouse]

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

On Sat, 2019-12-07 at 21:09 -0800, James Bottomley wrote:
> The TCG has defined an OID prefix "2.23.133.10.1" for the various TPM
> key uses.  We've defined three of the available numbers:
> 
> 2.23.133.10.1.3 TPM Loadable key.  This is an asymmetric key (Usually
> 		RSA2048 or Elliptic Curve) which can be imported by a
> 		TPM2_Load() operation.
> 
> 2.23.133.10.1.4 TPM Importable Key.  This is an asymmetric key (Usually
> 		RSA2048 or Elliptic Curve) which can be imported by a
> 		TPM2_Import() operation.
> 
> Both loadable and importable keys are specific to a given TPM, the
> difference is that a loadable key is wrapped with the symmetric
> secret, so must have been created by the TPM itself.  An importable
> key is wrapped with a DH shared secret, and may be created without
> access to the TPM provided you know the public part of the parent key.
> 
> 2.23.133.10.1.5 TPM Sealed Data.  This is a set of data (up to 128
> 		bytes) which is sealed by the TPM.  It usually
> 		represents a symmetric key and must be unsealed before
> 		use.

Do we still not have an official reference for these that you can
provide in the commit or the file itself?

It would be very nice to have something more than a verbal assurance
that they're in Monty's spreadsheet.


> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> ---
>  include/linux/oid_registry.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h
> index 657d6bf2c064..a4cee888f9b0 100644
> --- a/include/linux/oid_registry.h
> +++ b/include/linux/oid_registry.h
> @@ -107,6 +107,11 @@ enum OID {
>  	OID_gostTC26Sign512B,		/* 1.2.643.7.1.2.1.2.2 */
>  	OID_gostTC26Sign512C,		/* 1.2.643.7.1.2.1.2.3 */
>  
> +	/* TCG defined OIDS for TPM based keys */
> +	OID_TPMLoadableKey,		/* 2.23.133.10.1.3 */
> +	OID_TPMImporableKey,		/* 2.23.133.10.1.4 */
> +	OID_TPMSealedData,		/* 2.23.133.10.1.5 */
> +
>  	OID__NR
>  };
>  


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5174 bytes --]

Re: [PATCH 3/8] oid_registry: Add TCG defined OIDS for TPM keys

[James Bottomley]

On Mon, 2019-12-09 at 08:55 +0000, David Woodhouse wrote:
> On Sat, 2019-12-07 at 21:09 -0800, James Bottomley wrote:
> > The TCG has defined an OID prefix "2.23.133.10.1" for the various
> > TPM
> > key uses.  We've defined three of the available numbers:
> > 
> > 2.23.133.10.1.3 TPM Loadable key.  This is an asymmetric key
> > (Usually
> > 		RSA2048 or Elliptic Curve) which can be imported by a
> > 		TPM2_Load() operation.
> > 
> > 2.23.133.10.1.4 TPM Importable Key.  This is an asymmetric key
> > (Usually
> > 		RSA2048 or Elliptic Curve) which can be imported by a
> > 		TPM2_Import() operation.
> > 
> > Both loadable and importable keys are specific to a given TPM, the
> > difference is that a loadable key is wrapped with the symmetric
> > secret, so must have been created by the TPM itself.  An importable
> > key is wrapped with a DH shared secret, and may be created without
> > access to the TPM provided you know the public part of the parent
> > key.
> > 
> > 2.23.133.10.1.5 TPM Sealed Data.  This is a set of data (up to 128
> > 		bytes) which is sealed by the TPM.  It usually
> > 		represents a symmetric key and must be unsealed before
> > 		use.
> 
> Do we still not have an official reference for these that you can
> provide in the commit or the file itself?
> 
> It would be very nice to have something more than a verbal assurance
> that they're in Monty's spreadsheet.

Well, I've asked Monty several times ... he seems to think it's enough
that it's in his spreadsheet.  I assume at some point the TCG will get
around to publishing it when they identify a document to do it with but
until then we have to take Monty's word.

James

[PATCH 3/8] oid_registry: Add TCG defined OIDS for TPM keys

[Wiseman, Monty (GE Research, US)]

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

James,

> -----Original Message-----
> From: David Woodhouse <dwmw2@infradead.org>
> Sent: December 9, 2019 03:56 AM
> To: James Bottomley <James.Bottomley@HansenPartnership.com>; linux-
> integrity@vger.kernel.org; Wiseman, Monty (GE Global Research, US)
> <monty.wiseman@ge.com>
> Cc: Mimi Zohar <zohar@linux.ibm.com>; Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com>
> Subject: EXT: Re: [PATCH 3/8] oid_registry: Add TCG defined OIDS for TPM
> keys
>
> On Sat, 2019-12-07 at 21:09 -0800, James Bottomley wrote:
> > The TCG has defined an OID prefix "2.23.133.10.1" for the various TPM
> > key uses.  We've defined three of the available numbers:
> >
> > 2.23.133.10.1.3 TPM Loadable key.  This is an asymmetric key (Usually
> > 		RSA2048 or Elliptic Curve) which can be imported by a
> > 		TPM2_Load() operation.
> >
> > 2.23.133.10.1.4 TPM Importable Key.  This is an asymmetric key (Usually
> > 		RSA2048 or Elliptic Curve) which can be imported by a
> > 		TPM2_Import() operation.
> >
> > Both loadable and importable keys are specific to a given TPM, the
> > difference is that a loadable key is wrapped with the symmetric
> > secret, so must have been created by the TPM itself.  An importable
> > key is wrapped with a DH shared secret, and may be created without
> > access to the TPM provided you know the public part of the parent key.
> >
> > 2.23.133.10.1.5 TPM Sealed Data.  This is a set of data (up to 128
> > 		bytes) which is sealed by the TPM.  It usually
> > 		represents a symmetric key and must be unsealed before
> > 		use.
>
> Do we still not have an official reference for these that you can
> provide in the commit or the file itself?
>
> It would be very nice to have something more than a verbal assurance
> that they're in Monty's spreadsheet.
>
>
> > Signed-off-by: James Bottomley
> <James.Bottomley@HansenPartnership.com>
> > ---
> >  include/linux/oid_registry.h | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h
> > index 657d6bf2c064..a4cee888f9b0 100644
> > --- a/include/linux/oid_registry.h
> > +++ b/include/linux/oid_registry.h
> > @@ -107,6 +107,11 @@ enum OID {
> >  	OID_gostTC26Sign512B,		/* 1.2.643.7.1.2.1.2.2 */
> >  	OID_gostTC26Sign512C,		/* 1.2.643.7.1.2.1.2.3 */
> >
> > +	/* TCG defined OIDS for TPM based keys */
> > +	OID_TPMLoadableKey,		/* 2.23.133.10.1.3 */
> > +	OID_TPMImporableKey,		/* 2.23.133.10.1.4 */
> > +	OID_TPMSealedData,		/* 2.23.133.10.1.5 */
> > +
> >  	OID__NR
> >  };
> >
Bring back an old thread.  We are finally getting the TCG OID registry ready
to publish and wanted to verifier the OIDs you requested and we assigned
above.

I can find 2.23.133.10.1.3 TPM Loadable key in the tpm2-tss-engine project.

I do not see this one, nor the others list above in the kernel source. Did 
these ever
get used? If so, where and can you provide a use case for a relying party?

Also, I have in my local spreadsheet the following which I believe were just
drafts and never assigned. Please confirm.
2.23.133.10.1.1.2
Secondary Identifier: tcg-wellKnownAuthValue

This in intended to be bitmap of well-known authValues. This is not intended
to contain an actual authValue. For example. Bit 1 means and authValue of
hashsize all zeros, Bit 2 means an authValue of hashsize all NULLs, etc.
[Note: Bit 1 is lsb in this notation]

2.23.133.10.1.1.3
No secondary identifier or description

2.23.133.10.1.1.4
No secondary identifier or description


Monty Wiseman
Principal Engineer, Security Architecture
Controls & Optimization

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 6681 bytes --]

Re: [PATCH 3/8] oid_registry: Add TCG defined OIDS for TPM keys

[Jerry Snitselaar]

On Fri Jun 19 20, Wiseman, Monty (GE Research, US) wrote:
>James,
>
>> -----Original Message-----
>> From: David Woodhouse <dwmw2@infradead.org>
>> Sent: December 9, 2019 03:56 AM
>> To: James Bottomley <James.Bottomley@HansenPartnership.com>; linux-
>> integrity@vger.kernel.org; Wiseman, Monty (GE Global Research, US)
>> <monty.wiseman@ge.com>
>> Cc: Mimi Zohar <zohar@linux.ibm.com>; Jarkko Sakkinen
>> <jarkko.sakkinen@linux.intel.com>
>> Subject: EXT: Re: [PATCH 3/8] oid_registry: Add TCG defined OIDS for TPM
>> keys
>>
>> On Sat, 2019-12-07 at 21:09 -0800, James Bottomley wrote:
>> > The TCG has defined an OID prefix "2.23.133.10.1" for the various TPM
>> > key uses.  We've defined three of the available numbers:
>> >
>> > 2.23.133.10.1.3 TPM Loadable key.  This is an asymmetric key (Usually
>> > 		RSA2048 or Elliptic Curve) which can be imported by a
>> > 		TPM2_Load() operation.
>> >
>> > 2.23.133.10.1.4 TPM Importable Key.  This is an asymmetric key (Usually
>> > 		RSA2048 or Elliptic Curve) which can be imported by a
>> > 		TPM2_Import() operation.
>> >
>> > Both loadable and importable keys are specific to a given TPM, the
>> > difference is that a loadable key is wrapped with the symmetric
>> > secret, so must have been created by the TPM itself.  An importable
>> > key is wrapped with a DH shared secret, and may be created without
>> > access to the TPM provided you know the public part of the parent key.
>> >
>> > 2.23.133.10.1.5 TPM Sealed Data.  This is a set of data (up to 128
>> > 		bytes) which is sealed by the TPM.  It usually
>> > 		represents a symmetric key and must be unsealed before
>> > 		use.
>>
>> Do we still not have an official reference for these that you can
>> provide in the commit or the file itself?
>>
>> It would be very nice to have something more than a verbal assurance
>> that they're in Monty's spreadsheet.
>>
>>
>> > Signed-off-by: James Bottomley
>> <James.Bottomley@HansenPartnership.com>
>> > ---
>> >  include/linux/oid_registry.h | 5 +++++
>> >  1 file changed, 5 insertions(+)
>> >
>> > diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h
>> > index 657d6bf2c064..a4cee888f9b0 100644
>> > --- a/include/linux/oid_registry.h
>> > +++ b/include/linux/oid_registry.h
>> > @@ -107,6 +107,11 @@ enum OID {
>> >  	OID_gostTC26Sign512B,		/* 1.2.643.7.1.2.1.2.2 */
>> >  	OID_gostTC26Sign512C,		/* 1.2.643.7.1.2.1.2.3 */
>> >
>> > +	/* TCG defined OIDS for TPM based keys */
>> > +	OID_TPMLoadableKey,		/* 2.23.133.10.1.3 */
>> > +	OID_TPMImporableKey,		/* 2.23.133.10.1.4 */
>> > +	OID_TPMSealedData,		/* 2.23.133.10.1.5 */
>> > +
>> >  	OID__NR
>> >  };
>> >
>Bring back an old thread.  We are finally getting the TCG OID registry ready
>to publish and wanted to verifier the OIDs you requested and we assigned
>above.
>
>I can find 2.23.133.10.1.3 TPM Loadable key in the tpm2-tss-engine project.
>
>I do not see this one, nor the others list above in the kernel source. Did
>these ever
>get used? If so, where and can you provide a use case for a relying party?
>
>Also, I have in my local spreadsheet the following which I believe were just
>drafts and never assigned. Please confirm.
>2.23.133.10.1.1.2
>Secondary Identifier: tcg-wellKnownAuthValue
>
>This in intended to be bitmap of well-known authValues. This is not intended
>to contain an actual authValue. For example. Bit 1 means and authValue of
>hashsize all zeros, Bit 2 means an authValue of hashsize all NULLs, etc.
>[Note: Bit 1 is lsb in this notation]
>
>2.23.133.10.1.1.3
>No secondary identifier or description
>
>2.23.133.10.1.1.4
>No secondary identifier or description
>
>
>Monty Wiseman
>Principal Engineer, Security Architecture
>Controls & Optimization

Hi Monty,

The patchset is still being reviewed and discussed:

https://lore.kernel.org/linux-integrity/20200616160229.8018-3-James.Bottomley@HansenPartnership.com/

Re: [PATCH 3/8] oid_registry: Add TCG defined OIDS for TPM keys

[James Bottomley]

On Fri, 2020-06-19 at 20:45 +0000, Wiseman, Monty (GE Research, US)
wrote:
> James,
> 
> > -----Original Message-----
> > From: David Woodhouse <dwmw2@infradead.org>
> > Sent: December 9, 2019 03:56 AM
> > To: James Bottomley <James.Bottomley@HansenPartnership.com>; linux-
> > integrity@vger.kernel.org; Wiseman, Monty (GE Global Research, US)
> > <monty.wiseman@ge.com>
> > Cc: Mimi Zohar <zohar@linux.ibm.com>; Jarkko Sakkinen
> > <jarkko.sakkinen@linux.intel.com>
> > Subject: EXT: Re: [PATCH 3/8] oid_registry: Add TCG defined OIDS
> > for TPM
> > keys
> > 
> > On Sat, 2019-12-07 at 21:09 -0800, James Bottomley wrote:
> > > The TCG has defined an OID prefix "2.23.133.10.1" for the various
> > > TPM
> > > key uses.  We've defined three of the available numbers:
> > > 
> > > 2.23.133.10.1.3 TPM Loadable key.  This is an asymmetric key
> > > (Usually
> > > 		RSA2048 or Elliptic Curve) which can be imported by a
> > > 		TPM2_Load() operation.
> > > 
> > > 2.23.133.10.1.4 TPM Importable Key.  This is an asymmetric key
> > > (Usually
> > > 		RSA2048 or Elliptic Curve) which can be imported by a
> > > 		TPM2_Import() operation.
> > > 
> > > Both loadable and importable keys are specific to a given TPM,
> > > the
> > > difference is that a loadable key is wrapped with the symmetric
> > > secret, so must have been created by the TPM itself.  An
> > > importable
> > > key is wrapped with a DH shared secret, and may be created
> > > without
> > > access to the TPM provided you know the public part of the parent
> > > key.
> > > 
> > > 2.23.133.10.1.5 TPM Sealed Data.  This is a set of data (up to
> > > 128
> > > 		bytes) which is sealed by the TPM.  It usually
> > > 		represents a symmetric key and must be unsealed before
> > > 		use.
> > 
> > Do we still not have an official reference for these that you can
> > provide in the commit or the file itself?
> > 
> > It would be very nice to have something more than a verbal
> > assurance
> > that they're in Monty's spreadsheet.
> > 
> > 
> > > Signed-off-by: James Bottomley
> > 
> > <James.Bottomley@HansenPartnership.com>
> > > ---
> > >  include/linux/oid_registry.h | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/include/linux/oid_registry.h
> > > b/include/linux/oid_registry.h
> > > index 657d6bf2c064..a4cee888f9b0 100644
> > > --- a/include/linux/oid_registry.h
> > > +++ b/include/linux/oid_registry.h
> > > @@ -107,6 +107,11 @@ enum OID {
> > >  	OID_gostTC26Sign512B,		/*
> > > 1.2.643.7.1.2.1.2.2 */
> > >  	OID_gostTC26Sign512C,		/*
> > > 1.2.643.7.1.2.1.2.3 */
> > > 
> > > +	/* TCG defined OIDS for TPM based keys */
> > > +	OID_TPMLoadableKey,		/* 2.23.133.10.1.3 */
> > > +	OID_TPMImporableKey,		/* 2.23.133.10.1.4
> > > */
> > > +	OID_TPMSealedData,		/* 2.23.133.10.1.5 */
> > > +
> > >  	OID__NR
> > >  };
> > > 
> 
> Bring back an old thread.  We are finally getting the TCG OID
> registry ready to publish and wanted to verifier the OIDs you
> requested and we assigned
> above.
> 
> I can find 2.23.133.10.1.3 TPM Loadable key in the tpm2-tss-engine
> project.
> 
> I do not see this one, nor the others list above in the kernel
> source. Did these ever get used? If so, where and can you provide a
> use case for a relying party?

Yes, the openssl_tpm2_engine project.  It's a more sophisticated
version of the above:

https://git.kernel.org/pub/scm/linux/kernel/git/jejb/openssl_tpm2_engine.git/

The use case is that we can use the same ASN.1 format for data entities
that respond to different TPM2 commands, so:

2.23.133.10.1.3: Public and Private parts used as input to TPM2_Load
which produce a keyhandle for public key operations

2.23.133.10.1.4: Public and Private parts used as input to TPM2_Import
which produce a key handle for public key operations

2.23.133.10.1.5: Public and Private parts used as input to TPM2_Load
which produce a keyandle for TPM2_Unseal

I believe we discussed the fact that we'd need several OIDs for the
various key formats which are the same data structure but are used
differently.  However, the only other use I can think of for the ASN.1
structure would be importable unsealed data.  I don't think anyone's
yet implemented that, but I can see there might be a use case and it
would be convenient to have a published OID for it, say
2.23.133.10.1.6?

> Also, I have in my local spreadsheet the following which I believe
> were just drafts and never assigned. Please confirm.
> 2.23.133.10.1.1.2
> Secondary Identifier: tcg-wellKnownAuthValue
> 
> This in intended to be bitmap of well-known authValues. This is not
> intended to contain an actual authValue. For example. Bit 1 means and
> authValue of hashsize all zeros, Bit 2 means an authValue of hashsize
> all NULLs,
> etc.
> [Note: Bit 1 is lsb in this notation]
> 
> 2.23.133.10.1.1.3
> No secondary identifier or description
> 
> 2.23.133.10.1.1.4
> No secondary identifier or description

I don't think they were ever anything to do with the TPM engine
projects.  They were just something you already had in the spreadsheet
at the time, which is why you told me to start at ...10.1.3

James

Re: [PATCH 3/8] oid_registry: Add TCG defined OIDS for TPM keys

[Jarkko Sakkinen]

On Fri, Jun 19, 2020 at 08:45:24PM +0000, Wiseman, Monty (GE Research, US) wrote:
> James,
> 
> > -----Original Message-----
> > From: David Woodhouse <dwmw2@infradead.org>
> > Sent: December 9, 2019 03:56 AM
> > To: James Bottomley <James.Bottomley@HansenPartnership.com>; linux-
> > integrity@vger.kernel.org; Wiseman, Monty (GE Global Research, US)
> > <monty.wiseman@ge.com>
> > Cc: Mimi Zohar <zohar@linux.ibm.com>; Jarkko Sakkinen
> > <jarkko.sakkinen@linux.intel.com>
> > Subject: EXT: Re: [PATCH 3/8] oid_registry: Add TCG defined OIDS for TPM
> > keys
> >
> > On Sat, 2019-12-07 at 21:09 -0800, James Bottomley wrote:
> > > The TCG has defined an OID prefix "2.23.133.10.1" for the various TPM
> > > key uses.  We've defined three of the available numbers:
> > >
> > > 2.23.133.10.1.3 TPM Loadable key.  This is an asymmetric key (Usually
> > > 		RSA2048 or Elliptic Curve) which can be imported by a
> > > 		TPM2_Load() operation.
> > >
> > > 2.23.133.10.1.4 TPM Importable Key.  This is an asymmetric key (Usually
> > > 		RSA2048 or Elliptic Curve) which can be imported by a
> > > 		TPM2_Import() operation.
> > >
> > > Both loadable and importable keys are specific to a given TPM, the
> > > difference is that a loadable key is wrapped with the symmetric
> > > secret, so must have been created by the TPM itself.  An importable
> > > key is wrapped with a DH shared secret, and may be created without
> > > access to the TPM provided you know the public part of the parent key.
> > >
> > > 2.23.133.10.1.5 TPM Sealed Data.  This is a set of data (up to 128
> > > 		bytes) which is sealed by the TPM.  It usually
> > > 		represents a symmetric key and must be unsealed before
> > > 		use.
> >
> > Do we still not have an official reference for these that you can
> > provide in the commit or the file itself?
> >
> > It would be very nice to have something more than a verbal assurance
> > that they're in Monty's spreadsheet.
> >
> >
> > > Signed-off-by: James Bottomley
> > <James.Bottomley@HansenPartnership.com>
> > > ---
> > >  include/linux/oid_registry.h | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > >
> > > diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h
> > > index 657d6bf2c064..a4cee888f9b0 100644
> > > --- a/include/linux/oid_registry.h
> > > +++ b/include/linux/oid_registry.h
> > > @@ -107,6 +107,11 @@ enum OID {
> > >  	OID_gostTC26Sign512B,		/* 1.2.643.7.1.2.1.2.2 */
> > >  	OID_gostTC26Sign512C,		/* 1.2.643.7.1.2.1.2.3 */
> > >
> > > +	/* TCG defined OIDS for TPM based keys */

Would be nice to have a link to the TCG OID specification instead of
this text.

> > > +	OID_TPMLoadableKey,		/* 2.23.133.10.1.3 */
> > > +	OID_TPMImporableKey,		/* 2.23.133.10.1.4 */
> > > +	OID_TPMSealedData,		/* 2.23.133.10.1.5 */
> > > +
> > >  	OID__NR
> > >  };
> > >
> Bring back an old thread.  We are finally getting the TCG OID registry ready
> to publish and wanted to verifier the OIDs you requested and we assigned
> above.
> 
> I can find 2.23.133.10.1.3 TPM Loadable key in the tpm2-tss-engine project.
> 
> I do not see this one, nor the others list above in the kernel source. Did 
> these ever
> get used? If so, where and can you provide a use case for a relying party?
> 
> Also, I have in my local spreadsheet the following which I believe were just
> drafts and never assigned. Please confirm.
> 2.23.133.10.1.1.2
> Secondary Identifier: tcg-wellKnownAuthValue
> 
> This in intended to be bitmap of well-known authValues. This is not intended
> to contain an actual authValue. For example. Bit 1 means and authValue of
> hashsize all zeros, Bit 2 means an authValue of hashsize all NULLs, etc.
> [Note: Bit 1 is lsb in this notation]
> 
> 2.23.133.10.1.1.3
> No secondary identifier or description
> 
> 2.23.133.10.1.1.4
> No secondary identifier or description
> 
> 
> Monty Wiseman
> Principal Engineer, Security Architecture
> Controls & Optimization

/Jarkko

[PATCH 4/8] security: keys: trusted: use ASN.1 tpm2 key format for the blobs

[James Bottomley]

Modify the tpm2 key format blob output to export and import in the
ASN.1 form for tpm2 sealed object keys.  For compatibility with prior
trusted keys, the importer will also accept two tpm2b quantities
representing the public and private parts of the key.  However, the
export via keyctl pipe will only output the ASN.1 format.

The benefit of the ASN.1 format is that it's a standard and thus the
exported key can be used by userspace tools.  The format includes
policy specifications, thus it gets us out of having to construct
policy handles in userspace and the format includes the parent meaning
you don't have to keep passing it in each time.

This patch only implements basic handling for the ASN.1 format, so
keys with passwords but no policy.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 security/keys/trusted-keys/Makefile       |   2 +-
 security/keys/trusted-keys/tpm2key.asn1   |  23 ++++
 security/keys/trusted-keys/trusted_tpm1.c |   2 +-
 security/keys/trusted-keys/trusted_tpm2.c | 170 +++++++++++++++++++++++++++++-
 4 files changed, 190 insertions(+), 7 deletions(-)
 create mode 100644 security/keys/trusted-keys/tpm2key.asn1

diff --git a/security/keys/trusted-keys/Makefile b/security/keys/trusted-keys/Makefile
index 7b73cebbb378..e0198641eff2 100644
--- a/security/keys/trusted-keys/Makefile
+++ b/security/keys/trusted-keys/Makefile
@@ -5,4 +5,4 @@
 
 obj-$(CONFIG_TRUSTED_KEYS) += trusted.o
 trusted-y += trusted_tpm1.o
-trusted-y += trusted_tpm2.o
+trusted-y += trusted_tpm2.o tpm2key.asn1.o
diff --git a/security/keys/trusted-keys/tpm2key.asn1 b/security/keys/trusted-keys/tpm2key.asn1
new file mode 100644
index 000000000000..1851b7c80f08
--- /dev/null
+++ b/security/keys/trusted-keys/tpm2key.asn1
@@ -0,0 +1,23 @@
+---
+--- Note: This isn't quite the definition in the standard
+---       However, the Linux asn.1 parser doesn't understand
+---       [2] EXPLICIT SEQUENCE OF OPTIONAL
+---       So there's an extra intermediate TPMPolicySequence
+---       definition to work around this
+
+TPMKey ::= SEQUENCE {
+	type		OBJECT IDENTIFIER ({tpmkey_type}),
+	emptyAuth	[0] EXPLICIT BOOLEAN OPTIONAL,
+	policy		[1] EXPLICIT TPMPolicySequence OPTIONAL,
+	secret		[2] EXPLICIT OCTET STRING OPTIONAL,
+	parent		INTEGER ({tpmkey_parent}),
+	pubkey		OCTET STRING ({tpmkey_pub}),
+	privkey		OCTET STRING ({tpmkey_priv})
+	}
+
+TPMPolicySequence ::= SEQUENCE OF TPMPolicy
+
+TPMPolicy ::= SEQUENCE {
+	commandCode		[0] EXPLICIT INTEGER,
+	commandPolicy		[1] EXPLICIT OCTET STRING
+	}
diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
index d2c5ec1e040b..d744a0d1cb89 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -991,7 +991,7 @@ static int trusted_instantiate(struct key *key,
 		goto out;
 	}
 
-	if (!options->keyhandle) {
+	if (!options->keyhandle && !tpm2) {
 		ret = -EINVAL;
 		goto out;
 	}
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 08ec7f48f01d..4efc7b64d1cd 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -4,6 +4,8 @@
  * Copyright (C) 2014 Intel Corporation
  */
 
+#include <linux/asn1_encoder.h>
+#include <linux/oid_registry.h>
 #include <linux/string.h>
 #include <linux/err.h>
 #include <linux/tpm.h>
@@ -12,6 +14,10 @@
 #include <keys/trusted-type.h>
 #include <keys/trusted_tpm.h>
 
+#include <asm/unaligned.h>
+
+#include "tpm2key.asn1.h"
+
 static struct tpm2_hash tpm2_hash_map[] = {
 	{HASH_ALGO_SHA1, TPM_ALG_SHA1},
 	{HASH_ALGO_SHA256, TPM_ALG_SHA256},
@@ -20,6 +26,141 @@ static struct tpm2_hash tpm2_hash_map[] = {
 	{HASH_ALGO_SM3_256, TPM_ALG_SM3_256},
 };
 
+static u32 tpm2key_oid[] = { 2,23,133,10,1,5 };
+
+static int tpm2_key_encode(struct trusted_key_payload *payload,
+			   struct trusted_key_options *options,
+			   u8 *src, u32 len)
+{
+	u8 *scratch = kmalloc(PAGE_SIZE, GFP_KERNEL);
+	u8 *work = scratch, *work1;
+	u8 *priv, *pub;
+	u16 priv_len, pub_len;
+
+	priv_len = get_unaligned_be16(src);
+	src += 2;
+	priv = src;
+	src += priv_len;
+	pub_len = get_unaligned_be16(src);
+	src += 2;
+	pub = src;
+
+	if (!scratch)
+		return -ENOMEM;
+
+	asn1_encode_oid(&work, tpm2key_oid, asn1_oid_len(tpm2key_oid));
+	if (options->blobauth[0] == 0) {
+		unsigned char bool[3], *w = bool;
+		/* tag 0 is emptyAuth */
+		asn1_encode_boolean(&w, true);
+		asn1_encode_tag(&work, 0, bool, w - bool);
+	}
+	asn1_encode_integer(&work, options->keyhandle);
+	asn1_encode_octet_string(&work, pub, pub_len);
+	asn1_encode_octet_string(&work, priv, priv_len);
+
+	work1 = payload->blob;
+	asn1_encode_sequence(&work1, scratch, work - scratch);
+
+	return work1 - payload->blob;
+}
+
+struct tpm2key_context {
+	u32 parent;
+	const u8 *pub;
+	u32 pub_len;
+	const u8 *priv;
+	u32 priv_len;
+};
+
+static int tpm2_key_decode(struct trusted_key_payload *payload,
+			   struct trusted_key_options *options,
+			   u8 **buf)
+{
+	int ret;
+	struct tpm2key_context ctx;
+	u8 *blob;
+
+	ret = asn1_ber_decoder(&tpm2key_decoder, &ctx, payload->blob,
+			       payload->blob_len);
+	if (ret < 0)
+		return ret;
+
+	if (ctx.priv_len + ctx.pub_len > MAX_BLOB_SIZE)
+		return -EINVAL;
+
+	blob = kmalloc(ctx.priv_len + ctx.pub_len + 4, GFP_KERNEL);
+	if (!blob)
+		return -ENOMEM;
+
+	*buf = blob;
+	options->keyhandle = ctx.parent;
+	put_unaligned_be16(ctx.priv_len, blob);
+	blob += 2;
+	memcpy(blob, ctx.priv, ctx.priv_len);
+	blob += ctx.priv_len;
+	put_unaligned_be16(ctx.pub_len, blob);
+	blob += 2;
+	memcpy(blob, ctx.pub, ctx.pub_len);
+
+	return 0;
+}
+
+int tpmkey_parent(void *context, size_t hdrlen,
+		  unsigned char tag,
+		  const void *value, size_t vlen)
+{
+	struct tpm2key_context *ctx = context;
+	const u8 *v = value;
+	int i;
+
+	ctx->parent = 0;
+	for (i = 0; i < vlen; i++) {
+		ctx->parent <<= 8;
+		ctx->parent |= v[i];
+	}
+	return 0;
+}
+
+int tpmkey_type(void *context, size_t hdrlen,
+		unsigned char tag,
+		const void *value, size_t vlen)
+{
+	enum OID oid = look_up_OID(value, vlen);
+
+	if (oid != OID_TPMSealedData) {
+		char buffer[50];
+
+		sprint_oid(value, vlen, buffer, sizeof(buffer));
+		pr_debug("OID is \"%s\" which is not TPMSealedData\n",
+			 buffer);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+int tpmkey_pub(void *context, size_t hdrlen,
+	       unsigned char tag,
+	       const void *value, size_t vlen)
+{
+	struct tpm2key_context *ctx = context;
+
+	ctx->pub = value;
+	ctx->pub_len = vlen;
+	return 0;
+}
+
+int tpmkey_priv(void *context, size_t hdrlen,
+		unsigned char tag,
+		const void *value, size_t vlen)
+{
+	struct tpm2key_context *ctx = context;
+
+	ctx->priv = value;
+	ctx->priv_len = vlen;
+	return 0;
+}
+
 /**
  * tpm_buf_append_auth() - append TPMS_AUTH_COMMAND to the buffer.
  *
@@ -79,6 +220,9 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 	if (i == ARRAY_SIZE(tpm2_hash_map))
 		return -EINVAL;
 
+	if (!options->keyhandle)
+		return -EINVAL;
+
 	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
 	if (rc)
 		return rc;
@@ -144,8 +288,10 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 		goto out;
 	}
 
-	memcpy(payload->blob, &buf.data[TPM_HEADER_SIZE + 4], blob_len);
-	payload->blob_len = blob_len;
+	payload->blob_len =
+		tpm2_key_encode(payload, options,
+				&buf.data[TPM_HEADER_SIZE + 4],
+				blob_len);
 
 out:
 	tpm_buf_destroy(&buf);
@@ -156,6 +302,8 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 		else
 			rc = -EPERM;
 	}
+	if (payload->blob_len < 0)
+		return payload->blob_len;
 
 	return rc;
 }
@@ -182,13 +330,23 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 	unsigned int private_len;
 	unsigned int public_len;
 	unsigned int blob_len;
+	u8 *blob;
 	int rc;
 
-	private_len = be16_to_cpup((__be16 *) &payload->blob[0]);
+	rc = tpm2_key_decode(payload, options, &blob);
+	if (rc)
+		/* old form */
+		blob = payload->blob;
+
+	/* new format carries keyhandle but old format doesn't */
+	if (!options->keyhandle)
+		return -EINVAL;
+
+	private_len = be16_to_cpup((__be16 *) &blob[0]);
 	if (private_len > (payload->blob_len - 2))
 		return -E2BIG;
 
-	public_len = be16_to_cpup((__be16 *) &payload->blob[2 + private_len]);
+	public_len = be16_to_cpup((__be16 *) &blob[2 + private_len]);
 	blob_len = private_len + public_len + 4;
 	if (blob_len > payload->blob_len)
 		return -E2BIG;
@@ -204,7 +362,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 			     options->keyauth /* hmac */,
 			     TPM_DIGEST_SIZE);
 
-	tpm_buf_append(&buf, payload->blob, blob_len);
+	tpm_buf_append(&buf, blob, blob_len);
 
 	if (buf.flags & TPM_BUF_OVERFLOW) {
 		rc = -E2BIG;
@@ -217,6 +375,8 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 			(__be32 *) &buf.data[TPM_HEADER_SIZE]);
 
 out:
+	if (blob != payload->blob)
+		kfree(blob);
 	tpm_buf_destroy(&buf);
 
 	if (rc > 0)
-- 
2.16.4

Re: [PATCH 4/8] security: keys: trusted: use ASN.1 tpm2 key format for the blobs

[David Woodhouse]

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

On Sat, 2019-12-07 at 21:10 -0800, James Bottomley wrote:
> Modify the tpm2 key format blob output to export and import in the
> ASN.1 form for tpm2 sealed object keys.  For compatibility with prior
> trusted keys, the importer will also accept two tpm2b quantities
> representing the public and private parts of the key.  However, the
> export via keyctl pipe will only output the ASN.1 format.

You still have a tpm2_key_encode() function which spits out the raw
private/public blobs each prefixed with a length word. What's that
still used for?

> The benefit of the ASN.1 format is that it's a standard

We should probably make that true. Did we even get as far as writing up
an RFC-style description of the ASN.1? 

>  and thus the
> exported key can be used by userspace tools.  The format includes
> policy specifications, thus it gets us out of having to construct
> policy handles in userspace and the format includes the parent meaning
> you don't have to keep passing it in each time.
> 
> This patch only implements basic handling for the ASN.1 format, so
> keys with passwords but no policy.

... but doesn't bail out with an error when it sees something it
doesn't yet understand? Including the 'secret' field which is only
relevant for importable keys, etc.

> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> ---
>  security/keys/trusted-keys/Makefile       |   2 +-
>  security/keys/trusted-keys/tpm2key.asn1   |  23 ++++
>  security/keys/trusted-keys/trusted_tpm1.c |   2 +-
>  security/keys/trusted-keys/trusted_tpm2.c | 170 +++++++++++++++++++++++++++++-
>  4 files changed, 190 insertions(+), 7 deletions(-)
>  create mode 100644 security/keys/trusted-keys/tpm2key.asn1
> 
> diff --git a/security/keys/trusted-keys/Makefile b/security/keys/trusted-keys/Makefile
> index 7b73cebbb378..e0198641eff2 100644
> --- a/security/keys/trusted-keys/Makefile
> +++ b/security/keys/trusted-keys/Makefile
> @@ -5,4 +5,4 @@
>  
>  obj-$(CONFIG_TRUSTED_KEYS) += trusted.o
>  trusted-y += trusted_tpm1.o
> -trusted-y += trusted_tpm2.o
> +trusted-y += trusted_tpm2.o tpm2key.asn1.o
> diff --git a/security/keys/trusted-keys/tpm2key.asn1 b/security/keys/trusted-keys/tpm2key.asn1
> new file mode 100644
> index 000000000000..1851b7c80f08
> --- /dev/null
> +++ b/security/keys/trusted-keys/tpm2key.asn1
> @@ -0,0 +1,23 @@
> +---
> +--- Note: This isn't quite the definition in the standard
> +---       However, the Linux asn.1 parser doesn't understand
> +---       [2] EXPLICIT SEQUENCE OF OPTIONAL
> +---       So there's an extra intermediate TPMPolicySequence
> +---       definition to work around this

At the very least we should prod David with a pointy stick on that
topic, rather than quietly working around it.


> +
> +TPMKey ::= SEQUENCE {
> +	type		OBJECT IDENTIFIER ({tpmkey_type}),
> +	emptyAuth	[0] EXPLICIT BOOLEAN OPTIONAL,
> +	policy		[1] EXPLICIT TPMPolicySequence OPTIONAL,
> +	secret		[2] EXPLICIT OCTET STRING OPTIONAL,
> +	parent		INTEGER ({tpmkey_parent}),
> +	pubkey		OCTET STRING ({tpmkey_pub}),
> +	privkey		OCTET STRING ({tpmkey_priv})
> +	}
> +
> +TPMPolicySequence ::= SEQUENCE OF TPMPolicy
> +
> +TPMPolicy ::= SEQUENCE {
> +	commandCode		[0] EXPLICIT INTEGER,
> +	commandPolicy		[1] EXPLICIT OCTET STRING
> +	}
> diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> index d2c5ec1e040b..d744a0d1cb89 100644
> --- a/security/keys/trusted-keys/trusted_tpm1.c
> +++ b/security/keys/trusted-keys/trusted_tpm1.c
> @@ -991,7 +991,7 @@ static int trusted_instantiate(struct key *key,
>  		goto out;
>  	}
>  
> -	if (!options->keyhandle) {
> +	if (!options->keyhandle && !tpm2) {
>  		ret = -EINVAL;
>  		goto out;
>  	}
> diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
> index 08ec7f48f01d..4efc7b64d1cd 100644
> --- a/security/keys/trusted-keys/trusted_tpm2.c
> +++ b/security/keys/trusted-keys/trusted_tpm2.c
> @@ -4,6 +4,8 @@
>   * Copyright (C) 2014 Intel Corporation
>   */
>  
> +#include <linux/asn1_encoder.h>
> +#include <linux/oid_registry.h>
>  #include <linux/string.h>
>  #include <linux/err.h>
>  #include <linux/tpm.h>
> @@ -12,6 +14,10 @@
>  #include <keys/trusted-type.h>
>  #include <keys/trusted_tpm.h>
>  
> +#include <asm/unaligned.h>
> +
> +#include "tpm2key.asn1.h"
> +
>  static struct tpm2_hash tpm2_hash_map[] = {
>  	{HASH_ALGO_SHA1, TPM_ALG_SHA1},
>  	{HASH_ALGO_SHA256, TPM_ALG_SHA256},
> @@ -20,6 +26,141 @@ static struct tpm2_hash tpm2_hash_map[] = {
>  	{HASH_ALGO_SM3_256, TPM_ALG_SM3_256},
>  };
>  
> +static u32 tpm2key_oid[] = { 2,23,133,10,1,5 };
> +
> +static int tpm2_key_encode(struct trusted_key_payload *payload,
> +			   struct trusted_key_options *options,
> +			   u8 *src, u32 len)
> +{
> +	u8 *scratch = kmalloc(PAGE_SIZE, GFP_KERNEL);
> +	u8 *work = scratch, *work1;
> +	u8 *priv, *pub;
> +	u16 priv_len, pub_len;
> +
> +	priv_len = get_unaligned_be16(src);
> +	src += 2;
> +	priv = src;
> +	src += priv_len;
> +	pub_len = get_unaligned_be16(src);
> +	src += 2;
> +	pub = src;
> +
> +	if (!scratch)
> +		return -ENOMEM;
> +
> +	asn1_encode_oid(&work, tpm2key_oid, asn1_oid_len(tpm2key_oid));
> +	if (options->blobauth[0] == 0) {
> +		unsigned char bool[3], *w = bool;
> +		/* tag 0 is emptyAuth */
> +		asn1_encode_boolean(&w, true);
> +		asn1_encode_tag(&work, 0, bool, w - bool);
> +	}
> +	asn1_encode_integer(&work, options->keyhandle);
> +	asn1_encode_octet_string(&work, pub, pub_len);
> +	asn1_encode_octet_string(&work, priv, priv_len);
> +
> +	work1 = payload->blob;
> +	asn1_encode_sequence(&work1, scratch, work - scratch);
> +
> +	return work1 - payload->blob;
> +}

I still don't like the lack of overflow protection here, one layer up
from the underlying encoding APIs I already commented on.


> +struct tpm2key_context {
> +	u32 parent;
> +	const u8 *pub;
> +	u32 pub_len;
> +	const u8 *priv;
> +	u32 priv_len;
> +};
> +
> +static int tpm2_key_decode(struct trusted_key_payload *payload,
> +			   struct trusted_key_options *options,
> +			   u8 **buf)
> +{
> +	int ret;
> +	struct tpm2key_context ctx;
> +	u8 *blob;
> +
> +	ret = asn1_ber_decoder(&tpm2key_decoder, &ctx, payload->blob,
> +			       payload->blob_len);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (ctx.priv_len + ctx.pub_len > MAX_BLOB_SIZE)
> +		return -EINVAL;
> +
> +	blob = kmalloc(ctx.priv_len + ctx.pub_len + 4, GFP_KERNEL);
> +	if (!blob)
> +		return -ENOMEM;
> +
> +	*buf = blob;
> +	options->keyhandle = ctx.parent;
> +	put_unaligned_be16(ctx.priv_len, blob);
> +	blob += 2;
> +	memcpy(blob, ctx.priv, ctx.priv_len);
> +	blob += ctx.priv_len;
> +	put_unaligned_be16(ctx.pub_len, blob);
> +	blob += 2;
> +	memcpy(blob, ctx.pub, ctx.pub_len);
> 

Hm, do we really have to create this legacy form here and pass it
around? Can't we change whatever consumes this?

> +	return 0;
> +}
> +
> +int tpmkey_parent(void *context, size_t hdrlen,
> +		  unsigned char tag,
> +		  const void *value, size_t vlen)
> +{
> +	struct tpm2key_context *ctx = context;
> +	const u8 *v = value;
> +	int i;
> +
> +	ctx->parent = 0;
> +	for (i = 0; i < vlen; i++) {
> +		ctx->parent <<= 8;
> +		ctx->parent |= v[i];
> +	}
> +	return 0;
> +}
> +
> +int tpmkey_type(void *context, size_t hdrlen,
> +		unsigned char tag,
> +		const void *value, size_t vlen)
> +{
> +	enum OID oid = look_up_OID(value, vlen);
> +
> +	if (oid != OID_TPMSealedData) {
> +		char buffer[50];
> +
> +		sprint_oid(value, vlen, buffer, sizeof(buffer));
> +		pr_debug("OID is \"%s\" which is not TPMSealedData\n",
> +			 buffer);
> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
> +int tpmkey_pub(void *context, size_t hdrlen,
> +	       unsigned char tag,
> +	       const void *value, size_t vlen)
> +{
> +	struct tpm2key_context *ctx = context;
> +
> +	ctx->pub = value;
> +	ctx->pub_len = vlen;
> +	return 0;
> +}
> +
> +int tpmkey_priv(void *context, size_t hdrlen,
> +		unsigned char tag,
> +		const void *value, size_t vlen)
> +{
> +	struct tpm2key_context *ctx = context;
> +
> +	ctx->priv = value;
> +	ctx->priv_len = vlen;
> +	return 0;
> +}
> +
>  /**
>   * tpm_buf_append_auth() - append TPMS_AUTH_COMMAND to the buffer.
>   *
> @@ -79,6 +220,9 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>  	if (i == ARRAY_SIZE(tpm2_hash_map))
>  		return -EINVAL;
>  
> +	if (!options->keyhandle)
> +		return -EINVAL;
> +
>  	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
>  	if (rc)
>  		return rc;
> @@ -144,8 +288,10 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>  		goto out;
>  	}
>  
> -	memcpy(payload->blob, &buf.data[TPM_HEADER_SIZE + 4], blob_len);
> -	payload->blob_len = blob_len;
> +	payload->blob_len =
> +		tpm2_key_encode(payload, options,
> +				&buf.data[TPM_HEADER_SIZE + 4],
> +				blob_len);
>  
>  out:
>  	tpm_buf_destroy(&buf);
> @@ -156,6 +302,8 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>  		else
>  			rc = -EPERM;
>  	}
> +	if (payload->blob_len < 0)
> +		return payload->blob_len;
>  
>  	return rc;
>  }
> @@ -182,13 +330,23 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
>  	unsigned int private_len;
>  	unsigned int public_len;
>  	unsigned int blob_len;
> +	u8 *blob;
>  	int rc;
>  
> -	private_len = be16_to_cpup((__be16 *) &payload->blob[0]);
> +	rc = tpm2_key_decode(payload, options, &blob);
> +	if (rc)
> +		/* old form */
> +		blob = payload->blob;
> +
> +	/* new format carries keyhandle but old format doesn't */
> +	if (!options->keyhandle)
> +		return -EINVAL;
> +
> +	private_len = be16_to_cpup((__be16 *) &blob[0]);
>  	if (private_len > (payload->blob_len - 2))
>  		return -E2BIG;
>  
> -	public_len = be16_to_cpup((__be16 *) &payload->blob[2 + private_len]);
> +	public_len = be16_to_cpup((__be16 *) &blob[2 + private_len]);
>  	blob_len = private_len + public_len + 4;
>  	if (blob_len > payload->blob_len)
>  		return -E2BIG;
> @@ -204,7 +362,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
>  			     options->keyauth /* hmac */,
>  			     TPM_DIGEST_SIZE);
>  
> -	tpm_buf_append(&buf, payload->blob, blob_len);
> +	tpm_buf_append(&buf, blob, blob_len);
>  
>  	if (buf.flags & TPM_BUF_OVERFLOW) {
>  		rc = -E2BIG;
> @@ -217,6 +375,8 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
>  			(__be32 *) &buf.data[TPM_HEADER_SIZE]);
>  
>  out:
> +	if (blob != payload->blob)
> +		kfree(blob);
>  	tpm_buf_destroy(&buf);
>  
>  	if (rc > 0)


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5174 bytes --]

Re: [PATCH 4/8] security: keys: trusted: use ASN.1 tpm2 key format for the blobs

[James Bottomley]

On Mon, 2019-12-09 at 10:04 +0000, David Woodhouse wrote:
> On Sat, 2019-12-07 at 21:10 -0800, James Bottomley wrote:
> > Modify the tpm2 key format blob output to export and import in the
> > ASN.1 form for tpm2 sealed object keys.  For compatibility with
> > prior trusted keys, the importer will also accept two tpm2b
> > quantities representing the public and private parts of the
> > key.  However, the export via keyctl pipe will only output the
> > ASN.1 format.
> 
> You still have a tpm2_key_encode() function which spits out the raw
> private/public blobs each prefixed with a length word. What's that
> still used for?

It's the TPM2B format for input of the keys to the TPM.

> > The benefit of the ASN.1 format is that it's a standard
> 
> We should probably make that true. Did we even get as far as writing
> up an RFC-style description of the ASN.1? 

No ... I was going to ask you if you'd made a start.  I take it linux-
integrity is as good a list as any to begin this on.  I'll cc the
openssl_tpm2_engine list ... are there any others?

> >  and thus the
> > exported key can be used by userspace tools.  The format includes
> > policy specifications, thus it gets us out of having to construct
> > policy handles in userspace and the format includes the parent
> > meaning you don't have to keep passing it in each time.
> > 
> > This patch only implements basic handling for the ASN.1 format, so
> > keys with passwords but no policy.
> 
> ... but doesn't bail out with an error when it sees something it
> doesn't yet understand? Including the 'secret' field which is only
> relevant for importable keys, etc.

I hadn't actually got around to defining importable blobs ... but I
think keeping the same OID and adding the shared import secret in [2]
would be good enough?  In which case the TPM will error out for us
because it won't be able to unwrap the private blob.  If keyctl were
capable of returning information about the source of the problem it
might be worth checking, but while it can't, I think getting -EINVAL
from the TPM is as good as putting a check returning -EINVAL.

> > Signed-off-by: James Bottomley
> > <James.Bottomley@HansenPartnership.com>
> > ---
> >  security/keys/trusted-keys/Makefile       |   2 +-
> >  security/keys/trusted-keys/tpm2key.asn1   |  23 ++++
> >  security/keys/trusted-keys/trusted_tpm1.c |   2 +-
> >  security/keys/trusted-keys/trusted_tpm2.c | 170
> > +++++++++++++++++++++++++++++-
> >  4 files changed, 190 insertions(+), 7 deletions(-)
> >  create mode 100644 security/keys/trusted-keys/tpm2key.asn1
> > 
> > diff --git a/security/keys/trusted-keys/Makefile
> > b/security/keys/trusted-keys/Makefile
> > index 7b73cebbb378..e0198641eff2 100644
> > --- a/security/keys/trusted-keys/Makefile
> > +++ b/security/keys/trusted-keys/Makefile
> > @@ -5,4 +5,4 @@
> >  
> >  obj-$(CONFIG_TRUSTED_KEYS) += trusted.o
> >  trusted-y += trusted_tpm1.o
> > -trusted-y += trusted_tpm2.o
> > +trusted-y += trusted_tpm2.o tpm2key.asn1.o
> > diff --git a/security/keys/trusted-keys/tpm2key.asn1
> > b/security/keys/trusted-keys/tpm2key.asn1
> > new file mode 100644
> > index 000000000000..1851b7c80f08
> > --- /dev/null
> > +++ b/security/keys/trusted-keys/tpm2key.asn1
> > @@ -0,0 +1,23 @@
> > +---
> > +--- Note: This isn't quite the definition in the standard
> > +---       However, the Linux asn.1 parser doesn't understand
> > +---       [2] EXPLICIT SEQUENCE OF OPTIONAL
> > +---       So there's an extra intermediate TPMPolicySequence
> > +---       definition to work around this
> 
> At the very least we should prod David with a pointy stick on that
> topic, rather than quietly working around it.

OK, I'll add him to the cc list on this one.

[...]
> > +static int tpm2_key_encode(struct trusted_key_payload *payload,
> > +			   struct trusted_key_options *options,
> > +			   u8 *src, u32 len)
> > +{
> > +	u8 *scratch = kmalloc(PAGE_SIZE, GFP_KERNEL);
> > +	u8 *work = scratch, *work1;
> > +	u8 *priv, *pub;
> > +	u16 priv_len, pub_len;
> > +
> > +	priv_len = get_unaligned_be16(src);
> > +	src += 2;
> > +	priv = src;
> > +	src += priv_len;
> > +	pub_len = get_unaligned_be16(src);
> > +	src += 2;
> > +	pub = src;
> > +
> > +	if (!scratch)
> > +		return -ENOMEM;
> > +
> > +	asn1_encode_oid(&work, tpm2key_oid,
> > asn1_oid_len(tpm2key_oid));
> > +	if (options->blobauth[0] == 0) {
> > +		unsigned char bool[3], *w = bool;
> > +		/* tag 0 is emptyAuth */
> > +		asn1_encode_boolean(&w, true);
> > +		asn1_encode_tag(&work, 0, bool, w - bool);
> > +	}
> > +	asn1_encode_integer(&work, options->keyhandle);
> > +	asn1_encode_octet_string(&work, pub, pub_len);
> > +	asn1_encode_octet_string(&work, priv, priv_len);
> > +
> > +	work1 = payload->blob;
> > +	asn1_encode_sequence(&work1, scratch, work - scratch);
> > +
> > +	return work1 - payload->blob;
> > +}
> 
> I still don't like the lack of overflow protection here, one layer up
> from the underlying encoding APIs I already commented on.

Fixed.

> > +struct tpm2key_context {
> > +	u32 parent;
> > +	const u8 *pub;
> > +	u32 pub_len;
> > +	const u8 *priv;
> > +	u32 priv_len;
> > +};
> > +
> > +static int tpm2_key_decode(struct trusted_key_payload *payload,
> > +			   struct trusted_key_options *options,
> > +			   u8 **buf)
> > +{
> > +	int ret;
> > +	struct tpm2key_context ctx;
> > +	u8 *blob;
> > +
> > +	ret = asn1_ber_decoder(&tpm2key_decoder, &ctx, payload-
> > >blob,
> > +			       payload->blob_len);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	if (ctx.priv_len + ctx.pub_len > MAX_BLOB_SIZE)
> > +		return -EINVAL;
> > +
> > +	blob = kmalloc(ctx.priv_len + ctx.pub_len + 4,
> > GFP_KERNEL);
> > +	if (!blob)
> > +		return -ENOMEM;
> > +
> > +	*buf = blob;
> > +	options->keyhandle = ctx.parent;
> > +	put_unaligned_be16(ctx.priv_len, blob);
> > +	blob += 2;
> > +	memcpy(blob, ctx.priv, ctx.priv_len);
> > +	blob += ctx.priv_len;
> > +	put_unaligned_be16(ctx.pub_len, blob);
> > +	blob += 2;
> > +	memcpy(blob, ctx.pub, ctx.pub_len);
> > 
> 
> Hm, do we really have to create this legacy form here and pass it
> around? Can't we change whatever consumes this?

It's not a legacy form ... it's the intrinsic TPM2B form the TPM 2.0
uses so it's the natural destination form for the conversion.

James

[PATCH 5/8] security: keys: trusted: Make sealed key properly interoperable

[James Bottomley]

The current implementation appends a migratable flag to the end of a
key, meaning the format isn't exactly interoperable because the using
party needs to know to strip this extra byte.  However, all other
consumers of TPM sealed blobs expect the unseal to return exactly the
key.  Since TPM2 keys have a key property flag that corresponds to
migratable, use that flag instead and make the actual key the only
sealed quantity.  This is secure because the key properties are bound
to a hash in the private part, so if they're altered the key won't
load.

Backwards compatibility is implemented by detecting whether we're
loading a new format key or not and correctly setting migratable from
the last byte of old format keys.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 include/keys/trusted-type.h               |  1 +
 include/linux/tpm.h                       |  2 ++
 security/keys/trusted-keys/trusted_tpm2.c | 57 ++++++++++++++++++++++---------
 3 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index a94c03a61d8f..4728e13aada8 100644
--- a/include/keys/trusted-type.h
+++ b/include/keys/trusted-type.h
@@ -22,6 +22,7 @@ struct trusted_key_payload {
 	unsigned int key_len;
 	unsigned int blob_len;
 	unsigned char migratable;
+	unsigned char old_format;
 	unsigned char key[MAX_KEY_SIZE + 1];
 	unsigned char blob[MAX_BLOB_SIZE];
 };
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 03e9b184411b..cd46ab27baa5 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -297,6 +297,8 @@ struct tpm_buf {
 };
 
 enum tpm2_object_attributes {
+	TPM2_OA_FIXED_TPM		= BIT(1),
+	TPM2_OA_FIXED_PARENT		= BIT(4),
 	TPM2_OA_USER_WITH_AUTH		= BIT(6),
 };
 
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 4efc7b64d1cd..a34ab6f90f76 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -207,6 +207,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 	unsigned int blob_len;
 	struct tpm_buf buf;
 	u32 hash;
+	u32 flags;
 	int i;
 	int rc;
 
@@ -235,29 +236,30 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 			     TPM_DIGEST_SIZE);
 
 	/* sensitive */
-	tpm_buf_append_u16(&buf, 4 + TPM_DIGEST_SIZE + payload->key_len + 1);
+	tpm_buf_append_u16(&buf, 4 + TPM_DIGEST_SIZE + payload->key_len);
 
 	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_u16(&buf, payload->key_len);
 	tpm_buf_append(&buf, payload->key, payload->key_len);
-	tpm_buf_append_u8(&buf, payload->migratable);
 
 	/* public */
 	tpm_buf_append_u16(&buf, 14 + options->policydigest_len);
 	tpm_buf_append_u16(&buf, TPM_ALG_KEYEDHASH);
 	tpm_buf_append_u16(&buf, hash);
 
+	/* key properties */
+	flags = 0;
+	flags |= options->policydigest_len ? 0 : TPM2_OA_USER_WITH_AUTH;
+	flags |= payload->migratable ? (TPM2_OA_FIXED_TPM |
+					TPM2_OA_FIXED_PARENT) : 0;
+	tpm_buf_append_u32(&buf, flags);
+
 	/* policy */
-	if (options->policydigest_len) {
-		tpm_buf_append_u32(&buf, 0);
-		tpm_buf_append_u16(&buf, options->policydigest_len);
+	tpm_buf_append_u16(&buf, options->policydigest_len);
+	if (options->policydigest_len)
 		tpm_buf_append(&buf, options->policydigest,
 			       options->policydigest_len);
-	} else {
-		tpm_buf_append_u32(&buf, TPM2_OA_USER_WITH_AUTH);
-		tpm_buf_append_u16(&buf, 0);
-	}
 
 	/* public parameters */
 	tpm_buf_append_u16(&buf, TPM_ALG_NULL);
@@ -330,13 +332,16 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 	unsigned int private_len;
 	unsigned int public_len;
 	unsigned int blob_len;
-	u8 *blob;
+	u8 *blob, *pub;
 	int rc;
+	u32 attrs;
 
 	rc = tpm2_key_decode(payload, options, &blob);
-	if (rc)
+	if (rc) {
 		/* old form */
 		blob = payload->blob;
+		payload->old_format = 1;
+	}
 
 	/* new format carries keyhandle but old format doesn't */
 	if (!options->keyhandle)
@@ -347,6 +352,16 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 		return -E2BIG;
 
 	public_len = be16_to_cpup((__be16 *) &blob[2 + private_len]);
+
+	pub = blob + 2 + private_len + 2;
+	/* key attributes are always at offset 4 */
+	attrs = get_unaligned_be32(pub + 4);
+	if ((attrs & (TPM2_OA_FIXED_TPM | TPM2_OA_FIXED_PARENT)) ==
+	    (TPM2_OA_FIXED_TPM | TPM2_OA_FIXED_PARENT))
+		payload->migratable = 0;
+	else
+		payload->migratable = 1;
+
 	blob_len = private_len + public_len + 4;
 	if (blob_len > payload->blob_len)
 		return -E2BIG;
@@ -427,7 +442,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 	if (!rc) {
 		data_len = be16_to_cpup(
 			(__be16 *) &buf.data[TPM_HEADER_SIZE + 4]);
-		if (data_len < MIN_KEY_SIZE ||  data_len > MAX_KEY_SIZE + 1) {
+		if (data_len < MIN_KEY_SIZE ||  data_len > MAX_KEY_SIZE) {
 			rc = -EFAULT;
 			goto out;
 		}
@@ -438,9 +453,19 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 		}
 		data = &buf.data[TPM_HEADER_SIZE + 6];
 
-		memcpy(payload->key, data, data_len - 1);
-		payload->key_len = data_len - 1;
-		payload->migratable = data[data_len - 1];
+		if (payload->old_format) {
+			/* migratable flag is at the end of the key */
+			memcpy(payload->key, data, data_len - 1);
+			payload->key_len = data_len - 1;
+			payload->migratable = data[data_len - 1];
+		} else {
+			/*
+			 * migratable flag already collected from key
+			 * attributes
+			 */
+			memcpy(payload->key, data, data_len);
+			payload->key_len = data_len;
+		}
 	}
 
 out:
-- 
2.16.4

Re: [PATCH 5/8] security: keys: trusted: Make sealed key properly interoperable

[David Woodhouse]

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

On Sat, 2019-12-07 at 21:11 -0800, James Bottomley wrote:
> The current implementation appends a migratable flag to the end of a
> key, meaning the format isn't exactly interoperable because the using
> party needs to know to strip this extra byte.  However, all other
> consumers of TPM sealed blobs expect the unseal to return exactly the
> key.  Since TPM2 keys have a key property flag that corresponds to
> migratable, use that flag instead and make the actual key the only
> sealed quantity.  This is secure because the key properties are bound
> to a hash in the private part, so if they're altered the key won't
> load.
> 
> Backwards compatibility is implemented by detecting whether we're
> loading a new format key or not and correctly setting migratable from
> the last byte of old format keys.
> 
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> ---
>  include/keys/trusted-type.h               |  1 +
>  include/linux/tpm.h                       |  2 ++
>  security/keys/trusted-keys/trusted_tpm2.c | 57 ++++++++++++++++++++++---------
>  3 files changed, 44 insertions(+), 16 deletions(-)
> 
> diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
> index a94c03a61d8f..4728e13aada8 100644
> --- a/include/keys/trusted-type.h
> +++ b/include/keys/trusted-type.h
> @@ -22,6 +22,7 @@ struct trusted_key_payload {
>  	unsigned int key_len;
>  	unsigned int blob_len;
>  	unsigned char migratable;
> +	unsigned char old_format;
>  	unsigned char key[MAX_KEY_SIZE + 1];
>  	unsigned char blob[MAX_BLOB_SIZE];
>  };
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index 03e9b184411b..cd46ab27baa5 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -297,6 +297,8 @@ struct tpm_buf {
>  };
>  
>  enum tpm2_object_attributes {
> +	TPM2_OA_FIXED_TPM		= BIT(1),
> +	TPM2_OA_FIXED_PARENT		= BIT(4),
>  	TPM2_OA_USER_WITH_AUTH		= BIT(6),
>  };
>  
> diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
> index 4efc7b64d1cd..a34ab6f90f76 100644
> --- a/security/keys/trusted-keys/trusted_tpm2.c
> +++ b/security/keys/trusted-keys/trusted_tpm2.c
> @@ -207,6 +207,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>  	unsigned int blob_len;
>  	struct tpm_buf buf;
>  	u32 hash;
> +	u32 flags;
>  	int i;
>  	int rc;
>  
> @@ -235,29 +236,30 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>  			     TPM_DIGEST_SIZE);
>  
>  	/* sensitive */
> -	tpm_buf_append_u16(&buf, 4 + TPM_DIGEST_SIZE + payload->key_len + 1);
> +	tpm_buf_append_u16(&buf, 4 + TPM_DIGEST_SIZE + payload->key_len);
>  
>  	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_u16(&buf, payload->key_len);
>  	tpm_buf_append(&buf, payload->key, payload->key_len);
> -	tpm_buf_append_u8(&buf, payload->migratable);
>  
>  	/* public */
>  	tpm_buf_append_u16(&buf, 14 + options->policydigest_len);
>  	tpm_buf_append_u16(&buf, TPM_ALG_KEYEDHASH);
>  	tpm_buf_append_u16(&buf, hash);
>  
> +	/* key properties */
> +	flags = 0;
> +	flags |= options->policydigest_len ? 0 : TPM2_OA_USER_WITH_AUTH;
> +	flags |= payload->migratable ? (TPM2_OA_FIXED_TPM |
> +					TPM2_OA_FIXED_PARENT) : 0;
> +	tpm_buf_append_u32(&buf, flags);
> +
>  	/* policy */
> -	if (options->policydigest_len) {
> -		tpm_buf_append_u32(&buf, 0);
> -		tpm_buf_append_u16(&buf, options->policydigest_len);
> +	tpm_buf_append_u16(&buf, options->policydigest_len);
> +	if (options->policydigest_len)
>  		tpm_buf_append(&buf, options->policydigest,
>  			       options->policydigest_len);
> -	} else {
> -		tpm_buf_append_u32(&buf, TPM2_OA_USER_WITH_AUTH);
> -		tpm_buf_append_u16(&buf, 0);
> -	}
>  
>  	/* public parameters */
>  	tpm_buf_append_u16(&buf, TPM_ALG_NULL);
> @@ -330,13 +332,16 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
>  	unsigned int private_len;
>  	unsigned int public_len;
>  	unsigned int blob_len;
> -	u8 *blob;
> +	u8 *blob, *pub;
>  	int rc;
> +	u32 attrs;
>  
>  	rc = tpm2_key_decode(payload, options, &blob);
> -	if (rc)
> +	if (rc) {
>  		/* old form */
>  		blob = payload->blob;
> +		payload->old_format = 1;
> +	}
>  
>  	/* new format carries keyhandle but old format doesn't */
>  	if (!options->keyhandle)
> @@ -347,6 +352,16 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
>  		return -E2BIG;
>  
>  	public_len = be16_to_cpup((__be16 *) &blob[2 + private_len]);
> +
> +	pub = blob + 2 + private_len + 2;
> +	/* key attributes are always at offset 4 */
> +	attrs = get_unaligned_be32(pub + 4);


At this point I don't believe you've checked yet that payload->blob_len 
is sufficient to know that these bytes exist.

I think you're reading 'private_len' from non-existent bytes too, if
payload->blob_len is zero or one? Which I think was there before you
started, but you touched it last...


> +	if ((attrs & (TPM2_OA_FIXED_TPM | TPM2_OA_FIXED_PARENT)) ==
> +	    (TPM2_OA_FIXED_TPM | TPM2_OA_FIXED_PARENT))
> +		payload->migratable = 0;
> +	else
> +		payload->migratable = 1;
> +
>  	blob_len = private_len + public_len + 4;
>  	if (blob_len > payload->blob_len)
>  		return -E2BIG;
> @@ -427,7 +442,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
>  	if (!rc) {
>  		data_len = be16_to_cpup(
>  			(__be16 *) &buf.data[TPM_HEADER_SIZE + 4]);
> -		if (data_len < MIN_KEY_SIZE ||  data_len > MAX_KEY_SIZE + 1) {
> +		if (data_len < MIN_KEY_SIZE ||  data_len > MAX_KEY_SIZE) {
>  			rc = -EFAULT;
>  			goto out;
>  		}
> @@ -438,9 +453,19 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
>  		}
>  		data = &buf.data[TPM_HEADER_SIZE + 6];
>  
> -		memcpy(payload->key, data, data_len - 1);
> -		payload->key_len = data_len - 1;
> -		payload->migratable = data[data_len - 1];
> +		if (payload->old_format) {
> +			/* migratable flag is at the end of the key */
> +			memcpy(payload->key, data, data_len - 1);
> +			payload->key_len = data_len - 1;
> +			payload->migratable = data[data_len - 1];
> +		} else {
> +			/*
> +			 * migratable flag already collected from key
> +			 * attributes
> +			 */
> +			memcpy(payload->key, data, data_len);
> +			payload->key_len = data_len;
> +		}
>  	}
>  
>  out:


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5174 bytes --]

Re: [PATCH 5/8] security: keys: trusted: Make sealed key properly interoperable

[James Bottomley]

On Mon, 2019-12-09 at 10:09 +0000, David Woodhouse wrote:
> On Sat, 2019-12-07 at 21:11 -0800, James Bottomley wrote:
[...]
> > @@ -330,13 +332,16 @@ static int tpm2_load_cmd(struct tpm_chip
> > *chip,
> >  	unsigned int private_len;
> >  	unsigned int public_len;
> >  	unsigned int blob_len;
> > -	u8 *blob;
> > +	u8 *blob, *pub;
> >  	int rc;
> > +	u32 attrs;
> >  
> >  	rc = tpm2_key_decode(payload, options, &blob);
> > -	if (rc)
> > +	if (rc) {
> >  		/* old form */
> >  		blob = payload->blob;
> > +		payload->old_format = 1;
> > +	}
> >  
> >  	/* new format carries keyhandle but old format doesn't */
> >  	if (!options->keyhandle)
> > @@ -347,6 +352,16 @@ static int tpm2_load_cmd(struct tpm_chip
> > *chip,
> >  		return -E2BIG;
> >  
> >  	public_len = be16_to_cpup((__be16 *) &blob[2 +
> > private_len]);
> > +
> > +	pub = blob + 2 + private_len + 2;
> > +	/* key attributes are always at offset 4 */
> > +	attrs = get_unaligned_be32(pub + 4);
> 
> 
> At this point I don't believe you've checked yet that payload-
> >blob_len  is sufficient to know that these bytes exist.

Check added.

> I think you're reading 'private_len' from non-existent bytes too, if
> payload->blob_len is zero or one? Which I think was there before you
> started, but you touched it last...

Well, I started this because of bugs in the current code, so this is
just one more bug I have to fix.

James

[PATCH 6/8] security: keys: trusted: add PCR policy to TPM2 keys

[James Bottomley]

This commit adds the ability to specify a PCR lock policy to TPM2
keys.  There is a complexity in that the creator of the key must chose
either to use a PCR lock policy or to use authentication.  At the
current time they can't use both due to a complexity with the way
authentication works when policy registers are in use.  The way to
construct a pcrinfo statement for a key is simply to use the
TPMS_PCR_SELECT structure to specify the PCRs and follow this by a
hash of all their values in order of ascending PCR number.

For simplicity, we require the policy name hash and the hash used for
the PCRs to be the same.  Thus to construct a policy around the value
of the resettable PCR 16 using the sha1 bank, first reset the pcr to
zero giving a hash of all zeros as:

6768033e216468247bd031a0a2d9876d79818f8f

Then the TPMS_PCR_SELECT value for PCR 16 is

03000001

So create a new 32 byte key with a policy policy locking the key to
this value of PCR 16 with a parent key of 81000001 would be:

keyctl new 32 keyhandle=0x81000001 hash=sha1 pcrinfo=030000016768033e216468247bd031a0a2d9876d79818f8f" @u

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 Documentation/security/keys/trusted-encrypted.rst |  25 +-
 include/keys/trusted-type.h                       |   5 +-
 include/linux/tpm.h                               |   5 +
 security/keys/Kconfig                             |   2 +
 security/keys/trusted-keys/Makefile               |   2 +-
 security/keys/trusted-keys/tpm2-policy.c          | 344 ++++++++++++++++++++++
 security/keys/trusted-keys/tpm2-policy.h          |  30 ++
 security/keys/trusted-keys/tpm2key.asn1           |   4 +-
 security/keys/trusted-keys/trusted_tpm1.c         |  32 +-
 security/keys/trusted-keys/trusted_tpm2.c         |  77 ++++-
 10 files changed, 478 insertions(+), 48 deletions(-)
 create mode 100644 security/keys/trusted-keys/tpm2-policy.c
 create mode 100644 security/keys/trusted-keys/tpm2-policy.h

diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst
index 50ac8bcd6970..1a3ca84ad3cd 100644
--- a/Documentation/security/keys/trusted-encrypted.rst
+++ b/Documentation/security/keys/trusted-encrypted.rst
@@ -60,19 +60,16 @@ Usage::
                      (40 ascii zeros)
        blobauth=     ascii hex auth for sealed data default 0x00...
                      (40 ascii zeros)
-       pcrinfo=	     ascii hex of PCR_INFO or PCR_INFO_LONG (no default)
+       pcrinfo=      ascii hex of PCR_INFO or PCR_INFO_LONG (no
+                     default) on TPM 1.2 and a TPMS_PCR_SELECTION
+                     coupled with a hash of all the selected PCRs on
+                     TPM 2.0 using the selected hash.
        pcrlock=	     pcr number to be extended to "lock" blob
        migratable=   0|1 indicating permission to reseal to new PCR values,
                      default 1 (resealing allowed)
        hash=         hash algorithm name as a string. For TPM 1.x the only
                      allowed value is sha1. For TPM 2.x the allowed values
                      are sha1, sha256, sha384, sha512 and sm3-256.
-       policydigest= digest for the authorization policy. must be calculated
-                     with the same hash algorithm as specified by the 'hash='
-                     option.
-       policyhandle= handle to an authorization policy session that defines the
-                     same policy and with the same hash algorithm as was used to
-                     seal the key.
 
 "keyctl print" returns an ascii hex copy of the sealed key, which is in standard
 TPM_STORED_DATA format.  The key length for new keys are always in bytes.
@@ -151,6 +148,20 @@ Load a trusted key from the saved blob::
     f1f8fff03ad0acb083725535636addb08d73dedb9832da198081e5deae84bfaf0409c22b
     e4a8aea2b607ec96931e6f4d4fe563ba
 
+Create a trusted key on TPM 2.0 using an all zero value of PCR16 and
+using the NV storage root 81000001 as the parent::
+
+    $ keyctl add trusted kmk "new 32 keyhandle=0x81000001 hash=sha1 pcrinfo=030000016768033e216468247bd031a0a2d9876d79818f8f" @u
+
+Note the TPMS_PCR_SELECT value for PCR 16 is 03000001 because all
+current TPMs have 24 PCRs, so the initial 03 says there are three
+following bytes of selection and then because the bytes are big
+endian, 16 is bit zero of byte 2. the hash is the sha1 sum of all
+zeros (the value of PCR 16)::
+
+    $ dd if=/dev/zero bs=1 count=20 2>/dev/null|sha1sum
+    6768033e216468247bd031a0a2d9876d79818f8f
+
 Reseal a trusted key under new pcr values::
 
     $ keyctl update 268728824 "update pcrinfo=`cat pcr.blob`"
diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index 4728e13aada8..fc9c13802c06 100644
--- a/include/keys/trusted-type.h
+++ b/include/keys/trusted-type.h
@@ -14,9 +14,11 @@
 #define MIN_KEY_SIZE			32
 #define MAX_KEY_SIZE			128
 #define MAX_BLOB_SIZE			512
-#define MAX_PCRINFO_SIZE		64
+#define MAX_PCRINFO_SIZE		128
 #define MAX_DIGEST_SIZE			64
 
+#define TPM2_MAX_POLICIES		16
+
 struct trusted_key_payload {
 	struct rcu_head rcu;
 	unsigned int key_len;
@@ -25,6 +27,7 @@ struct trusted_key_payload {
 	unsigned char old_format;
 	unsigned char key[MAX_KEY_SIZE + 1];
 	unsigned char blob[MAX_BLOB_SIZE];
+	struct tpm2_policies *policies;
 };
 
 struct trusted_key_options {
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index cd46ab27baa5..e32e9728adce 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -222,10 +222,14 @@ enum tpm2_command_codes {
 	TPM2_CC_CONTEXT_LOAD	        = 0x0161,
 	TPM2_CC_CONTEXT_SAVE	        = 0x0162,
 	TPM2_CC_FLUSH_CONTEXT	        = 0x0165,
+	TPM2_CC_POLICY_AUTHVALUE	= 0x016B,
+	TPM2_CC_POLICY_COUNTER_TIMER	= 0x016D,
+	TPM2_CC_START_AUTH_SESS		= 0x0176,
 	TPM2_CC_VERIFY_SIGNATURE        = 0x0177,
 	TPM2_CC_GET_CAPABILITY	        = 0x017A,
 	TPM2_CC_GET_RANDOM	        = 0x017B,
 	TPM2_CC_PCR_READ	        = 0x017E,
+	TPM2_CC_POLICY_PCR		= 0x017F,
 	TPM2_CC_PCR_EXTEND	        = 0x0182,
 	TPM2_CC_EVENT_SEQUENCE_COMPLETE = 0x0185,
 	TPM2_CC_HASH_SEQUENCE_START     = 0x0186,
@@ -234,6 +238,7 @@ enum tpm2_command_codes {
 };
 
 enum tpm2_permanent_handles {
+	TPM2_RH_NULL		= 0x40000007,
 	TPM2_RS_PW		= 0x40000009,
 };
 
diff --git a/security/keys/Kconfig b/security/keys/Kconfig
index dd313438fecf..6c2f2c22b284 100644
--- a/security/keys/Kconfig
+++ b/security/keys/Kconfig
@@ -80,6 +80,8 @@ config TRUSTED_KEYS
 	select CRYPTO
 	select CRYPTO_HMAC
 	select CRYPTO_SHA1
+	select CRYPTO_SHA256
+	select CRYPTO_SHA512
 	select CRYPTO_HASH_INFO
 	help
 	  This option provides support for creating, sealing, and unsealing
diff --git a/security/keys/trusted-keys/Makefile b/security/keys/trusted-keys/Makefile
index e0198641eff2..194febacf362 100644
--- a/security/keys/trusted-keys/Makefile
+++ b/security/keys/trusted-keys/Makefile
@@ -5,4 +5,4 @@
 
 obj-$(CONFIG_TRUSTED_KEYS) += trusted.o
 trusted-y += trusted_tpm1.o
-trusted-y += trusted_tpm2.o tpm2key.asn1.o
+trusted-y += trusted_tpm2.o tpm2key.asn1.o tpm2-policy.o
diff --git a/security/keys/trusted-keys/tpm2-policy.c b/security/keys/trusted-keys/tpm2-policy.c
new file mode 100644
index 000000000000..ae83636ece37
--- /dev/null
+++ b/security/keys/trusted-keys/tpm2-policy.c
@@ -0,0 +1,344 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2019 James.Bottomley@HansenPartnership.com
+ */
+
+#include <linux/asn1_encoder.h>
+#include <linux/err.h>
+#include <linux/types.h>
+#include <linux/printk.h>
+#include <linux/string.h>
+#include <linux/tpm.h>
+
+#include <asm/unaligned.h>
+
+#include <crypto/hash.h>
+
+#include <keys/trusted-type.h>
+#include <keys/trusted_tpm.h>
+
+#include "tpm2key.asn1.h"
+#include "tpm2-policy.h"
+
+int tpmkey_code(void *context, size_t hdrlen,
+		unsigned char tag,
+		const void *value, size_t vlen)
+{
+	struct tpm2key_context *ctx = context;
+	u32 code = 0;
+	const u8 *v = value;
+	int i;
+
+	for (i = 0; i < vlen; i++) {
+		code <<= 8;
+		code |= v[i];
+	}
+
+	ctx->policy_code[ctx->policy_count] = code;
+
+	return 0;
+}
+
+int tpmkey_policy(void *context, size_t hdrlen,
+		  unsigned char tag,
+		  const void *value, size_t vlen)
+{
+	struct tpm2key_context *ctx = context;
+
+	ctx->policies[ctx->policy_count] = value;
+	ctx->policy_len[ctx->policy_count++] = vlen;
+
+	return 0;
+}
+
+/* we only support a limited number of policy statement so
+ * make sure we don't have anything we can't support
+ */
+static int tpm2_validate_policy(struct tpm2_policies *pols)
+{
+	int i;
+
+	if (pols->count == 0)
+		return 0;
+
+	for (i = 0; i < pols->count; i++) {
+		switch (pols->code[i]) {
+		case TPM2_CC_POLICY_COUNTER_TIMER:
+		case TPM2_CC_POLICY_PCR:
+		case TPM2_CC_POLICY_AUTHVALUE:
+			break;
+		default:
+			printk(KERN_INFO "tpm2 policy 0x%x is unsupported",
+			       pols->code[i]);
+			return -EINVAL;
+		}
+	}
+	return 0;
+}
+
+/**
+ * tpmkey_process_policy - collect the policty from the context
+ * @ctx: the context to collect from
+ * @payload: the payload structure to place it in
+ *
+ * THis function sizes the policy statements and allocates space
+ * within the payload to receive them before copying them over.  It
+ * should be used after the ber decoder has completed successfully
+ */
+int tpmkey_policy_process(struct tpm2key_context *ctx,
+			  struct trusted_key_payload *payload)
+{
+	int tot_len = 0;
+	u8 *buf;
+	int i, ret, len = 0;
+	struct tpm2_policies *pols;
+
+	if (ctx->policy_count == 0)
+		return 0;
+
+	for (i = 0; i < ctx->policy_count; i++)
+		tot_len += ctx->policy_len[i];
+	tot_len += sizeof(*pols);
+
+	pols = kmalloc(tot_len, GFP_KERNEL);
+	if (!pols)
+		return -ENOMEM;
+
+	payload->policies = pols;
+	buf = (u8 *)(pols + 1);
+
+	for (i = 0; i < ctx->policy_count; i++) {
+		pols->policies[i] = &buf[len];
+		pols->len[i] = ctx->policy_len[i];
+		pols->code[i] = ctx->policy_code[i];
+		if (pols->len[i])
+			memcpy(pols->policies[i], ctx->policies[i],
+			       ctx->policy_len[i]);
+		len += ctx->policy_len[i];
+	}
+	pols->count = ctx->policy_count;
+
+	ret = tpm2_validate_policy(pols);
+	if (ret) {
+		kfree(pols);
+		payload->policies = NULL;
+	}
+
+	/* capture the hash and size */
+
+	/* the hash is the second algorithm */
+	pols->hash = get_unaligned_be16(&ctx->pub[2]);
+	/* and the digest appears after the attributes */
+	pols->hash_size = get_unaligned_be16(&ctx->pub[8]);
+
+	return ret;
+}
+
+int tpm2_generate_policy_digest(struct tpm2_policies *pols,
+				u32 hash, u8 *policydigest, u32 *plen)
+{
+	int i;
+	struct crypto_shash *tfm;
+	int rc;
+
+	if (pols->count == 0)
+		return 0;
+
+	tfm = crypto_alloc_shash(hash_algo_name[hash], 0, 0);
+	if (IS_ERR(tfm))
+		return PTR_ERR(tfm);
+
+	rc = crypto_shash_digestsize(tfm);
+	if (WARN(rc > MAX_DIGEST_SIZE,
+		 "BUG: trusted key code has alg %s with digest too large (%d)",
+		 hash_algo_name[hash], rc)) {
+		rc = -EINVAL;
+		goto err;
+	}
+	pols->hash = hash;
+	pols->hash_size = rc;
+	*plen = rc;
+
+	/* policy digests always start out all zeros */
+	memset(policydigest, 0, rc);
+
+	for (i = 0; i < pols->count; i++) {
+		u8 *policy = pols->policies[i];
+		int len = pols->len[i];
+		u32 cmd = pols->code[i];
+		u8 digest[MAX_DIGEST_SIZE];
+		u8 code[4];
+		SHASH_DESC_ON_STACK(sdesc, tfm);
+
+		sdesc->tfm = tfm;
+		rc = crypto_shash_init(sdesc);
+		if (rc)
+			goto err;
+
+		/* first hash the previous digest */
+		crypto_shash_update(sdesc, policydigest, *plen);
+		/* then hash the command code */
+		put_unaligned_be32(cmd, code);
+		crypto_shash_update(sdesc, code, 4);
+
+		/* commands that need special handling */
+		if (cmd == TPM2_CC_POLICY_COUNTER_TIMER) {
+			SHASH_DESC_ON_STACK(sdesc1, tfm);
+
+			sdesc1->tfm = tfm;
+
+			/* counter timer policies are double hashed */
+			crypto_shash_digest(sdesc1, policy, len,
+					    digest);
+			policy = digest;
+			len = *plen;
+		}
+		crypto_shash_update(sdesc, policy, len);
+		/* now output the intermediate to the policydigest */
+		crypto_shash_final(sdesc, policydigest);
+
+	}
+	rc = 0;
+
+ err:
+	crypto_free_shash(tfm);
+	return rc;
+}
+
+int tpm2_encode_policy(struct tpm2_policies *pols, u8 **data, u32 *len)
+{
+	u8 *buf = kmalloc(2 * PAGE_SIZE, GFP_KERNEL);
+	u8 *work = buf + PAGE_SIZE, *ptr;
+	int i;
+
+	if (!buf)
+		return -ENOMEM;
+
+	for (i = 0; i < pols->count; i++) {
+		u8 *seq_len, *tag_len;
+		u32 cmd = pols->code[i];
+		int l;
+
+		/*
+		 * cheat a bit here: we know a policy is < 128 bytes,
+		 * so the sequence and cons tags will only be two
+		 * bytes long
+		 */
+		*work++ = _tag(UNIV, CONS, SEQ);
+		seq_len = work++;
+		*work++ = _tagn(CONT, CONS, 0);
+		tag_len = work++;
+		asn1_encode_integer(&work, cmd);
+		*tag_len = work - tag_len - 1;
+		*work++ = _tagn(CONT, CONS, 1);
+		tag_len = work++;
+		asn1_encode_octet_string(&work, pols->policies[i],
+					 pols->len[i]);
+		*tag_len = work - tag_len - 1;
+		l = work - seq_len - 1;
+		/* our assumption about policy length failed */
+		if (WARN(l > 127,
+			 "policy is too long: %d but must be < 128", l)) {
+			kfree(buf);
+			return -EINVAL;
+		}
+		*seq_len = l;
+	}
+	ptr = buf;
+	asn1_encode_sequence(&ptr, buf + PAGE_SIZE, work - buf - PAGE_SIZE);
+	*data = buf;
+	*len = ptr - buf;
+
+	return 0;
+}
+
+int tpm2_start_policy_session(struct tpm_chip *chip, u16 hash, u32 *handle)
+{
+	struct tpm_buf buf;
+	int rc;
+	int i;
+
+	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_START_AUTH_SESS);
+	if (rc)
+		return rc;
+
+	/* NULL salt key handle */
+	tpm_buf_append_u32(&buf, TPM2_RH_NULL);
+	/* NULL bind key handle */
+	tpm_buf_append_u32(&buf, TPM2_RH_NULL);
+	/* empty nonce caller */
+	tpm_buf_append_u16(&buf, 20);
+	for (i = 0; i < 20; i++)
+		tpm_buf_append_u8(&buf, 0);
+	/* empty auth */
+	tpm_buf_append_u16(&buf, 0);
+	/* session type policy */
+	tpm_buf_append_u8(&buf, 0x01);
+
+	/* symmetric encryption parameters */
+	/* symmetric algorithm  */
+	tpm_buf_append_u16(&buf, TPM_ALG_NULL);
+	/* hash algorithm for session */
+	tpm_buf_append_u16(&buf, hash);
+
+	rc = tpm_send(chip, buf.data, tpm_buf_length(&buf));
+	if (rc)
+		goto out;
+
+	*handle = get_unaligned_be32(buf.data + TPM_HEADER_SIZE);
+ out:
+	tpm_buf_destroy(&buf);
+
+	return rc <= 0 ? rc : -EPERM;
+}
+
+int tpm2_get_policy_session(struct tpm_chip *chip, struct tpm2_policies *pols,
+			    u32 *handle)
+{
+	int i, rc;
+	const char *failure;
+
+	rc = tpm2_start_policy_session(chip, pols->hash, handle);
+	if (rc)
+		return rc;
+
+	for (i = 0; i < pols->count; i++) {
+		u32 cmd = pols->code[i];
+		struct tpm_buf buf;
+
+		rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, cmd);
+		if (rc)
+			return rc;
+
+		tpm_buf_append_u32(&buf, *handle);
+
+		switch (cmd) {
+		case TPM2_CC_POLICY_PCR:
+			failure = "PCR";
+			/*
+			 * for reasons best known to the TCG we have
+			 * to reverse the two arguments to send to the
+			 * policy command
+			 */
+			tpm_buf_append_u16(&buf, pols->hash_size);
+			tpm_buf_append(&buf, pols->policies[i] + pols->len[i] -
+				       pols->hash_size, pols->hash_size);
+			tpm_buf_append(&buf, pols->policies[i],
+				       pols->len[i] - pols->hash_size);
+			break;
+		default:
+			failure = "unknown policy";
+			break;
+		}
+		rc = tpm_send(chip, buf.data, tpm_buf_length(&buf));
+		tpm_buf_destroy(&buf);
+		if (rc) {
+			printk(KERN_NOTICE "TPM policy %s failed, rc=%d\n",
+			       failure, rc);
+			tpm2_flush_context(chip, *handle);
+			*handle = 0;
+			return -EPERM;
+		}
+	}
+	return 0;
+}
diff --git a/security/keys/trusted-keys/tpm2-policy.h b/security/keys/trusted-keys/tpm2-policy.h
new file mode 100644
index 000000000000..152c948743f3
--- /dev/null
+++ b/security/keys/trusted-keys/tpm2-policy.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+struct tpm2key_context {
+	u32 parent;
+	const u8 *pub;
+	u32 pub_len;
+	const u8 *priv;
+	u32 priv_len;
+	const u8 *policies[TPM2_MAX_POLICIES];
+	u32 policy_code[TPM2_MAX_POLICIES];
+	u16 policy_len[TPM2_MAX_POLICIES];
+	u8 policy_count;
+};
+
+struct tpm2_policies {
+	u32 code[TPM2_MAX_POLICIES];
+	u8 *policies[TPM2_MAX_POLICIES];
+	u16 len[TPM2_MAX_POLICIES];
+	u8 count;
+	u16 hash;
+	u16 hash_size;
+};
+
+int tpmkey_policy_process(struct tpm2key_context *ctx,
+			  struct trusted_key_payload *payload);
+int tpm2_generate_policy_digest(struct tpm2_policies *pols, u32 hash,
+				u8 *policydigest, u32 *plen);
+int tpm2_encode_policy(struct tpm2_policies *pols, u8 **data, u32 *len);
+int tpm2_get_policy_session(struct tpm_chip *chip, struct tpm2_policies *pols,
+			    u32 *handle);
diff --git a/security/keys/trusted-keys/tpm2key.asn1 b/security/keys/trusted-keys/tpm2key.asn1
index 1851b7c80f08..f930fd812db3 100644
--- a/security/keys/trusted-keys/tpm2key.asn1
+++ b/security/keys/trusted-keys/tpm2key.asn1
@@ -18,6 +18,6 @@ TPMKey ::= SEQUENCE {
 TPMPolicySequence ::= SEQUENCE OF TPMPolicy
 
 TPMPolicy ::= SEQUENCE {
-	commandCode		[0] EXPLICIT INTEGER,
-	commandPolicy		[1] EXPLICIT OCTET STRING
+	commandCode		[0] EXPLICIT INTEGER ({tpmkey_code}),
+	commandPolicy		[1] EXPLICIT OCTET STRING ({tpmkey_policy})
 	}
diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
index d744a0d1cb89..6290e611b632 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -707,8 +707,6 @@ enum {
 	Opt_keyhandle, Opt_keyauth, Opt_blobauth,
 	Opt_pcrinfo, Opt_pcrlock, Opt_migratable,
 	Opt_hash,
-	Opt_policydigest,
-	Opt_policyhandle,
 };
 
 static const match_table_t key_tokens = {
@@ -722,8 +720,6 @@ static const match_table_t key_tokens = {
 	{Opt_pcrlock, "pcrlock=%s"},
 	{Opt_migratable, "migratable=%s"},
 	{Opt_hash, "hash=%s"},
-	{Opt_policydigest, "policydigest=%s"},
-	{Opt_policyhandle, "policyhandle=%s"},
 	{Opt_err, NULL}
 };
 
@@ -738,7 +734,6 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 	unsigned long handle;
 	unsigned long lock;
 	unsigned long token_mask = 0;
-	unsigned int digest_len;
 	int i;
 	int tpm2;
 
@@ -801,8 +796,6 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 			opt->pcrlock = lock;
 			break;
 		case Opt_hash:
-			if (test_bit(Opt_policydigest, &token_mask))
-				return -EINVAL;
 			for (i = 0; i < HASH_ALGO__LAST; i++) {
 				if (!strcmp(args[0].from, hash_algo_name[i])) {
 					opt->hash = i;
@@ -816,24 +809,6 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 				return -EINVAL;
 			}
 			break;
-		case Opt_policydigest:
-			digest_len = hash_digest_size[opt->hash];
-			if (!tpm2 || strlen(args[0].from) != (2 * digest_len))
-				return -EINVAL;
-			res = hex2bin(opt->policydigest, args[0].from,
-				      digest_len);
-			if (res < 0)
-				return -EINVAL;
-			opt->policydigest_len = digest_len;
-			break;
-		case Opt_policyhandle:
-			if (!tpm2)
-				return -EINVAL;
-			res = kstrtoul(args[0].from, 16, &handle);
-			if (res < 0)
-				return -EINVAL;
-			opt->policyhandle = handle;
-			break;
 		default:
 			return -EINVAL;
 		}
@@ -1045,6 +1020,7 @@ static void trusted_rcu_free(struct rcu_head *rcu)
 	struct trusted_key_payload *p;
 
 	p = container_of(rcu, struct trusted_key_payload, rcu);
+	kzfree(p->policies);
 	kzfree(p);
 }
 
@@ -1164,7 +1140,11 @@ static long trusted_read(const struct key *key, char __user *buffer,
  */
 static void trusted_destroy(struct key *key)
 {
-	kzfree(key->payload.data[0]);
+	struct trusted_key_payload *p;
+
+	p = key->payload.data[0];
+	kzfree(p->policies);
+	kzfree(p);
 }
 
 struct key_type key_type_trusted = {
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index a34ab6f90f76..6d0d427bc5c5 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -17,6 +17,7 @@
 #include <asm/unaligned.h>
 
 #include "tpm2key.asn1.h"
+#include "tpm2-policy.h"
 
 static struct tpm2_hash tpm2_hash_map[] = {
 	{HASH_ALGO_SHA1, TPM_ALG_SHA1},
@@ -55,6 +56,20 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
 		asn1_encode_boolean(&w, true);
 		asn1_encode_tag(&work, 0, bool, w - bool);
 	}
+	if (payload->policies) {
+		u8 *encoded_pols;
+		u32 encoded_pol_len;
+		int ret;
+
+		ret = tpm2_encode_policy(payload->policies, &encoded_pols,
+					 &encoded_pol_len);
+
+		if (!ret) {
+			asn1_encode_tag(&work, 1, encoded_pols,
+					encoded_pol_len);
+			kfree(encoded_pols);
+		}
+	}
 	asn1_encode_integer(&work, options->keyhandle);
 	asn1_encode_octet_string(&work, pub, pub_len);
 	asn1_encode_octet_string(&work, priv, priv_len);
@@ -65,14 +80,6 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
 	return work1 - payload->blob;
 }
 
-struct tpm2key_context {
-	u32 parent;
-	const u8 *pub;
-	u32 pub_len;
-	const u8 *priv;
-	u32 priv_len;
-};
-
 static int tpm2_key_decode(struct trusted_key_payload *payload,
 			   struct trusted_key_options *options,
 			   u8 **buf)
@@ -81,6 +88,8 @@ static int tpm2_key_decode(struct trusted_key_payload *payload,
 	struct tpm2key_context ctx;
 	u8 *blob;
 
+	memset(&ctx, 0, sizeof(ctx));
+
 	ret = asn1_ber_decoder(&tpm2key_decoder, &ctx, payload->blob,
 			       payload->blob_len);
 	if (ret < 0)
@@ -93,6 +102,12 @@ static int tpm2_key_decode(struct trusted_key_payload *payload,
 	if (!blob)
 		return -ENOMEM;
 
+	ret = tpmkey_policy_process(&ctx, payload);
+	if (ret) {
+		kfree(blob);
+		return ret;
+	}
+
 	*buf = blob;
 	options->keyhandle = ctx.parent;
 	put_unaligned_be16(ctx.priv_len, blob);
@@ -224,6 +239,37 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 	if (!options->keyhandle)
 		return -EINVAL;
 
+	if (options->pcrinfo_len != 0) {
+		struct tpm2_policies *pols;
+		static u8 *scratch;
+		/* 4 array len, 2 hash alg */
+		const int len = 4 + 2 + options->pcrinfo_len;
+
+		pols = kmalloc(sizeof(*pols) + len, GFP_KERNEL);
+		if (!pols)
+			return -ENOMEM;
+
+		pols->count = 1;
+		pols->len[0] = len;
+		scratch = (u8 *)(pols + 1);
+		pols->policies[0] = scratch;
+		pols->code[0] = TPM2_CC_POLICY_PCR;
+
+		put_unaligned_be32(1, &scratch[0]);
+		put_unaligned_be16(hash, &scratch[4]);
+		memcpy(&scratch[6], options->pcrinfo, options->pcrinfo_len);
+		payload->policies = pols;
+	}
+
+	if (payload->policies) {
+		rc = tpm2_generate_policy_digest(payload->policies,
+						 options->hash,
+						 options->policydigest,
+						 &options->policydigest_len);
+		if (rc)
+			return rc;
+	}
+
 	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
 	if (rc)
 		return rc;
@@ -421,21 +467,30 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 	u16 data_len;
 	u8 *data;
 	int rc;
+	u32 policyhandle = 0;
 
 	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
 	if (rc)
 		return rc;
 
+	if (payload->policies) {
+		rc = tpm2_get_policy_session(chip, payload->policies,
+					     &policyhandle);
+		if (rc)
+			return rc;
+	}
+
 	tpm_buf_append_u32(&buf, blob_handle);
 	tpm2_buf_append_auth(&buf,
-			     options->policyhandle ?
-			     options->policyhandle : TPM2_RS_PW,
+			     policyhandle ?
+			     policyhandle : TPM2_RS_PW,
 			     NULL /* nonce */, 0,
 			     TPM2_SA_CONTINUE_SESSION,
 			     options->blobauth /* hmac */,
-			     TPM_DIGEST_SIZE);
+			     policyhandle ? 0 : TPM_DIGEST_SIZE);
 
 	rc = tpm_send(chip, buf.data, tpm_buf_length(&buf));
+	tpm2_flush_context(chip, policyhandle);
 	if (rc > 0)
 		rc = -EPERM;
 
-- 
2.16.4

Re: [PATCH 6/8] security: keys: trusted: add PCR policy to TPM2 keys

[David Woodhouse]

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

On Sat, 2019-12-07 at 21:12 -0800, James Bottomley wrote:
> This commit adds the ability to specify a PCR lock policy to TPM2
> keys.  There is a complexity in that the creator of the key must chose
> either to use a PCR lock policy or to use authentication.  At the
> current time they can't use both due to a complexity with the way
> authentication works when policy registers are in use.  The way to
> construct a pcrinfo statement for a key is simply to use the
> TPMS_PCR_SELECT structure to specify the PCRs and follow this by a
> hash of all their values in order of ascending PCR number.
> 
> For simplicity, we require the policy name hash and the hash used for
> the PCRs to be the same.  Thus to construct a policy around the value
> of the resettable PCR 16 using the sha1 bank, first reset the pcr to
> zero giving a hash of all zeros as:
> 
> 6768033e216468247bd031a0a2d9876d79818f8f
> 
> Then the TPMS_PCR_SELECT value for PCR 16 is
> 
> 03000001
> 
> So create a new 32 byte key with a policy policy locking the key to
> this value of PCR 16 with a parent key of 81000001 would be:
> 
> keyctl new 32 keyhandle=0x81000001 hash=sha1 pcrinfo=030000016768033e216468247bd031a0a2d9876d79818f8f" @u

OK... but I've love to see a more formal definition of this binary
format, as part of the "standard" we allegedly have for the overall
ASN.1 representation.

> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> ---
>  Documentation/security/keys/trusted-encrypted.rst |  25 +-
>  include/keys/trusted-type.h                       |   5 +-
>  include/linux/tpm.h                               |   5 +
>  security/keys/Kconfig                             |   2 +
>  security/keys/trusted-keys/Makefile               |   2 +-
>  security/keys/trusted-keys/tpm2-policy.c          | 344 ++++++++++++++++++++++
>  security/keys/trusted-keys/tpm2-policy.h          |  30 ++
>  security/keys/trusted-keys/tpm2key.asn1           |   4 +-
>  security/keys/trusted-keys/trusted_tpm1.c         |  32 +-
>  security/keys/trusted-keys/trusted_tpm2.c         |  77 ++++-
>  10 files changed, 478 insertions(+), 48 deletions(-)
>  create mode 100644 security/keys/trusted-keys/tpm2-policy.c
>  create mode 100644 security/keys/trusted-keys/tpm2-policy.h
> 
> diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst
> index 50ac8bcd6970..1a3ca84ad3cd 100644
> --- a/Documentation/security/keys/trusted-encrypted.rst
> +++ b/Documentation/security/keys/trusted-encrypted.rst
> @@ -60,19 +60,16 @@ Usage::
>                       (40 ascii zeros)
>         blobauth=     ascii hex auth for sealed data default 0x00...
>                       (40 ascii zeros)
> -       pcrinfo=	     ascii hex of PCR_INFO or PCR_INFO_LONG (no default)
> +       pcrinfo=      ascii hex of PCR_INFO or PCR_INFO_LONG (no
> +                     default) on TPM 1.2 and a TPMS_PCR_SELECTION
> +                     coupled with a hash of all the selected PCRs on
> +                     TPM 2.0 using the selected hash.
>         pcrlock=	     pcr number to be extended to "lock" blob
>         migratable=   0|1 indicating permission to reseal to new PCR values,
>                       default 1 (resealing allowed)
>         hash=         hash algorithm name as a string. For TPM 1.x the only
>                       allowed value is sha1. For TPM 2.x the allowed values
>                       are sha1, sha256, sha384, sha512 and sm3-256.
> -       policydigest= digest for the authorization policy. must be calculated
> -                     with the same hash algorithm as specified by the 'hash='
> -                     option.
> -       policyhandle= handle to an authorization policy session that defines the
> -                     same policy and with the same hash algorithm as was used to
> -                     seal the key.
>  
>  "keyctl print" returns an ascii hex copy of the sealed key, which is in standard
>  TPM_STORED_DATA format.  The key length for new keys are always in bytes.
> @@ -151,6 +148,20 @@ Load a trusted key from the saved blob::
>      f1f8fff03ad0acb083725535636addb08d73dedb9832da198081e5deae84bfaf0409c22b
>      e4a8aea2b607ec96931e6f4d4fe563ba
>  
> +Create a trusted key on TPM 2.0 using an all zero value of PCR16 and
> +using the NV storage root 81000001 as the parent::
> +
> +    $ keyctl add trusted kmk "new 32 keyhandle=0x81000001 hash=sha1 pcrinfo=030000016768033e216468247bd031a0a2d9876d79818f8f" @u
> +
> +Note the TPMS_PCR_SELECT value for PCR 16 is 03000001 because all
> +current TPMs have 24 PCRs, so the initial 03 says there are three
> +following bytes of selection and then because the bytes are big
> +endian, 16 is bit zero of byte 2. the hash is the sha1 sum of all
> +zeros (the value of PCR 16)::
> +
> +    $ dd if=/dev/zero bs=1 count=20 2>/dev/null|sha1sum
> +    6768033e216468247bd031a0a2d9876d79818f8f
> +
>  Reseal a trusted key under new pcr values::
>  
>      $ keyctl update 268728824 "update pcrinfo=`cat pcr.blob`"
> diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
> index 4728e13aada8..fc9c13802c06 100644
> --- a/include/keys/trusted-type.h
> +++ b/include/keys/trusted-type.h
> @@ -14,9 +14,11 @@
>  #define MIN_KEY_SIZE			32
>  #define MAX_KEY_SIZE			128
>  #define MAX_BLOB_SIZE			512
> -#define MAX_PCRINFO_SIZE		64
> +#define MAX_PCRINFO_SIZE		128
>  #define MAX_DIGEST_SIZE			64
>  
> +#define TPM2_MAX_POLICIES		16
> +
>  struct trusted_key_payload {
>  	struct rcu_head rcu;
>  	unsigned int key_len;
> @@ -25,6 +27,7 @@ struct trusted_key_payload {
>  	unsigned char old_format;
>  	unsigned char key[MAX_KEY_SIZE + 1];
>  	unsigned char blob[MAX_BLOB_SIZE];
> +	struct tpm2_policies *policies;
>  };
>  
>  struct trusted_key_options {
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index cd46ab27baa5..e32e9728adce 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -222,10 +222,14 @@ enum tpm2_command_codes {
>  	TPM2_CC_CONTEXT_LOAD	        = 0x0161,
>  	TPM2_CC_CONTEXT_SAVE	        = 0x0162,
>  	TPM2_CC_FLUSH_CONTEXT	        = 0x0165,
> +	TPM2_CC_POLICY_AUTHVALUE	= 0x016B,
> +	TPM2_CC_POLICY_COUNTER_TIMER	= 0x016D,
> +	TPM2_CC_START_AUTH_SESS		= 0x0176,
>  	TPM2_CC_VERIFY_SIGNATURE        = 0x0177,
>  	TPM2_CC_GET_CAPABILITY	        = 0x017A,
>  	TPM2_CC_GET_RANDOM	        = 0x017B,
>  	TPM2_CC_PCR_READ	        = 0x017E,
> +	TPM2_CC_POLICY_PCR		= 0x017F,
>  	TPM2_CC_PCR_EXTEND	        = 0x0182,
>  	TPM2_CC_EVENT_SEQUENCE_COMPLETE = 0x0185,
>  	TPM2_CC_HASH_SEQUENCE_START     = 0x0186,
> @@ -234,6 +238,7 @@ enum tpm2_command_codes {
>  };
>  
>  enum tpm2_permanent_handles {
> +	TPM2_RH_NULL		= 0x40000007,
>  	TPM2_RS_PW		= 0x40000009,
>  };
>  
> diff --git a/security/keys/Kconfig b/security/keys/Kconfig
> index dd313438fecf..6c2f2c22b284 100644
> --- a/security/keys/Kconfig
> +++ b/security/keys/Kconfig
> @@ -80,6 +80,8 @@ config TRUSTED_KEYS
>  	select CRYPTO
>  	select CRYPTO_HMAC
>  	select CRYPTO_SHA1
> +	select CRYPTO_SHA256
> +	select CRYPTO_SHA512
>  	select CRYPTO_HASH_INFO
>  	help
>  	  This option provides support for creating, sealing, and unsealing
> diff --git a/security/keys/trusted-keys/Makefile b/security/keys/trusted-keys/Makefile
> index e0198641eff2..194febacf362 100644
> --- a/security/keys/trusted-keys/Makefile
> +++ b/security/keys/trusted-keys/Makefile
> @@ -5,4 +5,4 @@
>  
>  obj-$(CONFIG_TRUSTED_KEYS) += trusted.o
>  trusted-y += trusted_tpm1.o
> -trusted-y += trusted_tpm2.o tpm2key.asn1.o
> +trusted-y += trusted_tpm2.o tpm2key.asn1.o tpm2-policy.o
> diff --git a/security/keys/trusted-keys/tpm2-policy.c b/security/keys/trusted-keys/tpm2-policy.c
> new file mode 100644
> index 000000000000..ae83636ece37
> --- /dev/null
> +++ b/security/keys/trusted-keys/tpm2-policy.c
> @@ -0,0 +1,344 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2019 James.Bottomley@HansenPartnership.com
> + */
> +
> +#include <linux/asn1_encoder.h>
> +#include <linux/err.h>
> +#include <linux/types.h>
> +#include <linux/printk.h>
> +#include <linux/string.h>
> +#include <linux/tpm.h>
> +
> +#include <asm/unaligned.h>
> +
> +#include <crypto/hash.h>
> +
> +#include <keys/trusted-type.h>
> +#include <keys/trusted_tpm.h>
> +
> +#include "tpm2key.asn1.h"
> +#include "tpm2-policy.h"
> +
> +int tpmkey_code(void *context, size_t hdrlen,
> +		unsigned char tag,
> +		const void *value, size_t vlen)
> +{
> +	struct tpm2key_context *ctx = context;
> +	u32 code = 0;
> +	const u8 *v = value;
> +	int i;
> +
> +	for (i = 0; i < vlen; i++) {
> +		code <<= 8;
> +		code |= v[i];
> +	}
> +
> +	ctx->policy_code[ctx->policy_count] = code;
> +
> +	return 0;
> +}
> +
> +int tpmkey_policy(void *context, size_t hdrlen,
> +		  unsigned char tag,
> +		  const void *value, size_t vlen)
> +{
> +	struct tpm2key_context *ctx = context;
> +
> +	ctx->policies[ctx->policy_count] = value;
> +	ctx->policy_len[ctx->policy_count++] = vlen;
> +
> +	return 0;
> +}
> +
> +/* we only support a limited number of policy statement so
> + * make sure we don't have anything we can't support
> + */
> +static int tpm2_validate_policy(struct tpm2_policies *pols)
> +{
> +	int i;
> +
> +	if (pols->count == 0)
> +		return 0;
> +
> +	for (i = 0; i < pols->count; i++) {
> +		switch (pols->code[i]) {
> +		case TPM2_CC_POLICY_COUNTER_TIMER:
> +		case TPM2_CC_POLICY_PCR:
> +		case TPM2_CC_POLICY_AUTHVALUE:
> +			break;
> +		default:
> +			printk(KERN_INFO "tpm2 policy 0x%x is unsupported",
> +			       pols->code[i]);
> +			return -EINVAL;
> +		}
> +	}
> +	return 0;
> +}
> +
> +/**
> + * tpmkey_process_policy - collect the policty from the context
> + * @ctx: the context to collect from
> + * @payload: the payload structure to place it in
> + *
> + * THis function sizes the policy statements and allocates space
> + * within the payload to receive them before copying them over.  It
> + * should be used after the ber decoder has completed successfully
> + */
> +int tpmkey_policy_process(struct tpm2key_context *ctx,
> +			  struct trusted_key_payload *payload)
> +{
> +	int tot_len = 0;
> +	u8 *buf;
> +	int i, ret, len = 0;
> +	struct tpm2_policies *pols;
> +
> +	if (ctx->policy_count == 0)
> +		return 0;
> +
> +	for (i = 0; i < ctx->policy_count; i++)
> +		tot_len += ctx->policy_len[i];
> +	tot_len += sizeof(*pols);
> +
> +	pols = kmalloc(tot_len, GFP_KERNEL);
> +	if (!pols)
> +		return -ENOMEM;
> +
> +	payload->policies = pols;
> +	buf = (u8 *)(pols + 1);
> +
> +	for (i = 0; i < ctx->policy_count; i++) {
> +		pols->policies[i] = &buf[len];
> +		pols->len[i] = ctx->policy_len[i];
> +		pols->code[i] = ctx->policy_code[i];
> +		if (pols->len[i])
> +			memcpy(pols->policies[i], ctx->policies[i],
> +			       ctx->policy_len[i]);
> +		len += ctx->policy_len[i];
> +	}
> +	pols->count = ctx->policy_count;
> +
> +	ret = tpm2_validate_policy(pols);
> +	if (ret) {
> +		kfree(pols);
> +		payload->policies = NULL;
> +	}
> +
> +	/* capture the hash and size */
> +
> +	/* the hash is the second algorithm */
> +	pols->hash = get_unaligned_be16(&ctx->pub[2]);
> +	/* and the digest appears after the attributes */
> +	pols->hash_size = get_unaligned_be16(&ctx->pub[8]);
> +
> +	return ret;
> +}
> +
> +int tpm2_generate_policy_digest(struct tpm2_policies *pols,
> +				u32 hash, u8 *policydigest, u32 *plen)
> +{
> +	int i;
> +	struct crypto_shash *tfm;
> +	int rc;
> +
> +	if (pols->count == 0)
> +		return 0;
> +
> +	tfm = crypto_alloc_shash(hash_algo_name[hash], 0, 0);
> +	if (IS_ERR(tfm))
> +		return PTR_ERR(tfm);
> +
> +	rc = crypto_shash_digestsize(tfm);
> +	if (WARN(rc > MAX_DIGEST_SIZE,
> +		 "BUG: trusted key code has alg %s with digest too large (%d)",
> +		 hash_algo_name[hash], rc)) {
> +		rc = -EINVAL;
> +		goto err;
> +	}
> +	pols->hash = hash;
> +	pols->hash_size = rc;
> +	*plen = rc;
> +
> +	/* policy digests always start out all zeros */
> +	memset(policydigest, 0, rc);
> +
> +	for (i = 0; i < pols->count; i++) {
> +		u8 *policy = pols->policies[i];
> +		int len = pols->len[i];
> +		u32 cmd = pols->code[i];
> +		u8 digest[MAX_DIGEST_SIZE];
> +		u8 code[4];
> +		SHASH_DESC_ON_STACK(sdesc, tfm);
> +
> +		sdesc->tfm = tfm;
> +		rc = crypto_shash_init(sdesc);
> +		if (rc)
> +			goto err;
> +
> +		/* first hash the previous digest */
> +		crypto_shash_update(sdesc, policydigest, *plen);
> +		/* then hash the command code */
> +		put_unaligned_be32(cmd, code);
> +		crypto_shash_update(sdesc, code, 4);
> +
> +		/* commands that need special handling */
> +		if (cmd == TPM2_CC_POLICY_COUNTER_TIMER) {
> +			SHASH_DESC_ON_STACK(sdesc1, tfm);
> +
> +			sdesc1->tfm = tfm;
> +
> +			/* counter timer policies are double hashed */
> +			crypto_shash_digest(sdesc1, policy, len,
> +					    digest);
> +			policy = digest;
> +			len = *plen;
> +		}
> +		crypto_shash_update(sdesc, policy, len);
> +		/* now output the intermediate to the policydigest */
> +		crypto_shash_final(sdesc, policydigest);
> +
> +	}
> +	rc = 0;
> +
> + err:
> +	crypto_free_shash(tfm);
> +	return rc;
> +}
> +
> +int tpm2_encode_policy(struct tpm2_policies *pols, u8 **data, u32 *len)
> +{
> +	u8 *buf = kmalloc(2 * PAGE_SIZE, GFP_KERNEL);
> +	u8 *work = buf + PAGE_SIZE, *ptr;
> +	int i;
> +
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < pols->count; i++) {
> +		u8 *seq_len, *tag_len;
> +		u32 cmd = pols->code[i];
> +		int l;
> +
> +		/*
> +		 * cheat a bit here: we know a policy is < 128 bytes,
> +		 * so the sequence and cons tags will only be two
> +		 * bytes long
> +		 */
> +		*work++ = _tag(UNIV, CONS, SEQ);
> +		seq_len = work++;
> +		*work++ = _tagn(CONT, CONS, 0);
> +		tag_len = work++;
> +		asn1_encode_integer(&work, cmd);
> +		*tag_len = work - tag_len - 1;
> +		*work++ = _tagn(CONT, CONS, 1);
> +		tag_len = work++;
> +		asn1_encode_octet_string(&work, pols->policies[i],
> +					 pols->len[i]);
> +		*tag_len = work - tag_len - 1;
> +		l = work - seq_len - 1;
> +		/* our assumption about policy length failed */
> +		if (WARN(l > 127,
> +			 "policy is too long: %d but must be < 128", l)) {
> +			kfree(buf);
> +			return -EINVAL;
> +		}
> +		*seq_len = l;



You're not even using your own sequence encoding here, because it only
works when you know the length in advance. How about setting *seq_len
to 0x80 to start with, for an indeterminate length.

Then in the happy case where it is <128, just go back and fill it in as
you currently do. Otherwise append 0x00 0x00 as the end marker.

None of this has to be DER, does it?

<Insert more whining about PAGE_SIZE assumptions and buffer overflows>

> +	}
> +	ptr = buf;
> +	asn1_encode_sequence(&ptr, buf + PAGE_SIZE, work - buf - PAGE_SIZE);
> +	*data = buf;
> +	*len = ptr - buf;
> +
> +	return 0;
> +}
> +
> +int tpm2_start_policy_session(struct tpm_chip *chip, u16 hash, u32 *handle)
> +{
> +	struct tpm_buf buf;
> +	int rc;
> +	int i;
> +
> +	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_START_AUTH_SESS);
> +	if (rc)
> +		return rc;
> +
> +	/* NULL salt key handle */
> +	tpm_buf_append_u32(&buf, TPM2_RH_NULL);
> +	/* NULL bind key handle */
> +	tpm_buf_append_u32(&buf, TPM2_RH_NULL);
> +	/* empty nonce caller */
> +	tpm_buf_append_u16(&buf, 20);
> +	for (i = 0; i < 20; i++)
> +		tpm_buf_append_u8(&buf, 0);
> +	/* empty auth */
> +	tpm_buf_append_u16(&buf, 0);
> +	/* session type policy */
> +	tpm_buf_append_u8(&buf, 0x01);
> +
> +	/* symmetric encryption parameters */
> +	/* symmetric algorithm  */
> +	tpm_buf_append_u16(&buf, TPM_ALG_NULL);
> +	/* hash algorithm for session */
> +	tpm_buf_append_u16(&buf, hash);
> +
> +	rc = tpm_send(chip, buf.data, tpm_buf_length(&buf));
> +	if (rc)
> +		goto out;
> +
> +	*handle = get_unaligned_be32(buf.data + TPM_HEADER_SIZE);
> + out:
> +	tpm_buf_destroy(&buf);
> +
> +	return rc <= 0 ? rc : -EPERM;
> +}
> +
> +int tpm2_get_policy_session(struct tpm_chip *chip, struct tpm2_policies *pols,
> +			    u32 *handle)
> +{
> +	int i, rc;
> +	const char *failure;
> +
> +	rc = tpm2_start_policy_session(chip, pols->hash, handle);
> +	if (rc)
> +		return rc;
> +
> +	for (i = 0; i < pols->count; i++) {
> +		u32 cmd = pols->code[i];
> +		struct tpm_buf buf;
> +
> +		rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, cmd);
> +		if (rc)
> +			return rc;
> +
> +		tpm_buf_append_u32(&buf, *handle);
> +
> +		switch (cmd) {
> +		case TPM2_CC_POLICY_PCR:
> +			failure = "PCR";
> +			/*
> +			 * for reasons best known to the TCG we have
> +			 * to reverse the two arguments to send to the
> +			 * policy command
> +			 */
> +			tpm_buf_append_u16(&buf, pols->hash_size);
> +			tpm_buf_append(&buf, pols->policies[i] + pols->len[i] -
> +				       pols->hash_size, pols->hash_size);
> +			tpm_buf_append(&buf, pols->policies[i],
> +				       pols->len[i] - pols->hash_size);
> +			break;
> +		default:
> +			failure = "unknown policy";
> +			break;
> +		}
> +		rc = tpm_send(chip, buf.data, tpm_buf_length(&buf));
> +		tpm_buf_destroy(&buf);
> +		if (rc) {
> +			printk(KERN_NOTICE "TPM policy %s failed, rc=%d\n",
> +			       failure, rc);
> +			tpm2_flush_context(chip, *handle);
> +			*handle = 0;
> +			return -EPERM;
> +		}
> +	}
> +	return 0;
> +}
> diff --git a/security/keys/trusted-keys/tpm2-policy.h b/security/keys/trusted-keys/tpm2-policy.h
> new file mode 100644
> index 000000000000..152c948743f3
> --- /dev/null
> +++ b/security/keys/trusted-keys/tpm2-policy.h
> @@ -0,0 +1,30 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +struct tpm2key_context {
> +	u32 parent;
> +	const u8 *pub;
> +	u32 pub_len;
> +	const u8 *priv;
> +	u32 priv_len;
> +	const u8 *policies[TPM2_MAX_POLICIES];
> +	u32 policy_code[TPM2_MAX_POLICIES];
> +	u16 policy_len[TPM2_MAX_POLICIES];
> +	u8 policy_count;
> +};
> +
> +struct tpm2_policies {
> +	u32 code[TPM2_MAX_POLICIES];
> +	u8 *policies[TPM2_MAX_POLICIES];
> +	u16 len[TPM2_MAX_POLICIES];
> +	u8 count;
> +	u16 hash;
> +	u16 hash_size;
> +};
> +
> +int tpmkey_policy_process(struct tpm2key_context *ctx,
> +			  struct trusted_key_payload *payload);
> +int tpm2_generate_policy_digest(struct tpm2_policies *pols, u32 hash,
> +				u8 *policydigest, u32 *plen);
> +int tpm2_encode_policy(struct tpm2_policies *pols, u8 **data, u32 *len);
> +int tpm2_get_policy_session(struct tpm_chip *chip, struct tpm2_policies *pols,
> +			    u32 *handle);
> diff --git a/security/keys/trusted-keys/tpm2key.asn1 b/security/keys/trusted-keys/tpm2key.asn1
> index 1851b7c80f08..f930fd812db3 100644
> --- a/security/keys/trusted-keys/tpm2key.asn1
> +++ b/security/keys/trusted-keys/tpm2key.asn1
> @@ -18,6 +18,6 @@ TPMKey ::= SEQUENCE {
>  TPMPolicySequence ::= SEQUENCE OF TPMPolicy
>  
>  TPMPolicy ::= SEQUENCE {
> -	commandCode		[0] EXPLICIT INTEGER,
> -	commandPolicy		[1] EXPLICIT OCTET STRING
> +	commandCode		[0] EXPLICIT INTEGER ({tpmkey_code}),
> +	commandPolicy		[1] EXPLICIT OCTET STRING ({tpmkey_policy})
>  	}
> diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> index d744a0d1cb89..6290e611b632 100644
> --- a/security/keys/trusted-keys/trusted_tpm1.c
> +++ b/security/keys/trusted-keys/trusted_tpm1.c
> @@ -707,8 +707,6 @@ enum {
>  	Opt_keyhandle, Opt_keyauth, Opt_blobauth,
>  	Opt_pcrinfo, Opt_pcrlock, Opt_migratable,
>  	Opt_hash,
> -	Opt_policydigest,
> -	Opt_policyhandle,
>  };
>  
>  static const match_table_t key_tokens = {
> @@ -722,8 +720,6 @@ static const match_table_t key_tokens = {
>  	{Opt_pcrlock, "pcrlock=%s"},
>  	{Opt_migratable, "migratable=%s"},
>  	{Opt_hash, "hash=%s"},
> -	{Opt_policydigest, "policydigest=%s"},
> -	{Opt_policyhandle, "policyhandle=%s"},
>  	{Opt_err, NULL}
>  };
>  
> @@ -738,7 +734,6 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>  	unsigned long handle;
>  	unsigned long lock;
>  	unsigned long token_mask = 0;
> -	unsigned int digest_len;
>  	int i;
>  	int tpm2;
>  
> @@ -801,8 +796,6 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>  			opt->pcrlock = lock;
>  			break;
>  		case Opt_hash:
> -			if (test_bit(Opt_policydigest, &token_mask))
> -				return -EINVAL;
>  			for (i = 0; i < HASH_ALGO__LAST; i++) {
>  				if (!strcmp(args[0].from, hash_algo_name[i])) {
>  					opt->hash = i;
> @@ -816,24 +809,6 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>  				return -EINVAL;
>  			}
>  			break;
> -		case Opt_policydigest:
> -			digest_len = hash_digest_size[opt->hash];
> -			if (!tpm2 || strlen(args[0].from) != (2 * digest_len))
> -				return -EINVAL;
> -			res = hex2bin(opt->policydigest, args[0].from,
> -				      digest_len);
> -			if (res < 0)
> -				return -EINVAL;
> -			opt->policydigest_len = digest_len;
> -			break;
> -		case Opt_policyhandle:
> -			if (!tpm2)
> -				return -EINVAL;
> -			res = kstrtoul(args[0].from, 16, &handle);
> -			if (res < 0)
> -				return -EINVAL;
> -			opt->policyhandle = handle;
> -			break;
>  		default:
>  			return -EINVAL;
>  		}
> @@ -1045,6 +1020,7 @@ static void trusted_rcu_free(struct rcu_head *rcu)
>  	struct trusted_key_payload *p;
>  
>  	p = container_of(rcu, struct trusted_key_payload, rcu);
> +	kzfree(p->policies);
>  	kzfree(p);
>  }
>  
> @@ -1164,7 +1140,11 @@ static long trusted_read(const struct key *key, char __user *buffer,
>   */
>  static void trusted_destroy(struct key *key)
>  {
> -	kzfree(key->payload.data[0]);
> +	struct trusted_key_payload *p;
> +
> +	p = key->payload.data[0];
> +	kzfree(p->policies);
> +	kzfree(p);
>  }
>  
>  struct key_type key_type_trusted = {
> diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
> index a34ab6f90f76..6d0d427bc5c5 100644
> --- a/security/keys/trusted-keys/trusted_tpm2.c
> +++ b/security/keys/trusted-keys/trusted_tpm2.c
> @@ -17,6 +17,7 @@
>  #include <asm/unaligned.h>
>  
>  #include "tpm2key.asn1.h"
> +#include "tpm2-policy.h"
>  
>  static struct tpm2_hash tpm2_hash_map[] = {
>  	{HASH_ALGO_SHA1, TPM_ALG_SHA1},
> @@ -55,6 +56,20 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
>  		asn1_encode_boolean(&w, true);
>  		asn1_encode_tag(&work, 0, bool, w - bool);
>  	}
> +	if (payload->policies) {
> +		u8 *encoded_pols;
> +		u32 encoded_pol_len;
> +		int ret;
> +
> +		ret = tpm2_encode_policy(payload->policies, &encoded_pols,
> +					 &encoded_pol_len);
> +
> +		if (!ret) {
> +			asn1_encode_tag(&work, 1, encoded_pols,
> +					encoded_pol_len);
> +			kfree(encoded_pols);
> +		}
> +	}
>  	asn1_encode_integer(&work, options->keyhandle);
>  	asn1_encode_octet_string(&work, pub, pub_len);
>  	asn1_encode_octet_string(&work, priv, priv_len);
> @@ -65,14 +80,6 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
>  	return work1 - payload->blob;
>  }
>  
> -struct tpm2key_context {
> -	u32 parent;
> -	const u8 *pub;
> -	u32 pub_len;
> -	const u8 *priv;
> -	u32 priv_len;
> -};
> -
>  static int tpm2_key_decode(struct trusted_key_payload *payload,
>  			   struct trusted_key_options *options,
>  			   u8 **buf)
> @@ -81,6 +88,8 @@ static int tpm2_key_decode(struct trusted_key_payload *payload,
>  	struct tpm2key_context ctx;
>  	u8 *blob;
>  
> +	memset(&ctx, 0, sizeof(ctx));
> +
>  	ret = asn1_ber_decoder(&tpm2key_decoder, &ctx, payload->blob,
>  			       payload->blob_len);
>  	if (ret < 0)
> @@ -93,6 +102,12 @@ static int tpm2_key_decode(struct trusted_key_payload *payload,
>  	if (!blob)
>  		return -ENOMEM;
>  
> +	ret = tpmkey_policy_process(&ctx, payload);
> +	if (ret) {
> +		kfree(blob);
> +		return ret;
> +	}
> +
>  	*buf = blob;
>  	options->keyhandle = ctx.parent;
>  	put_unaligned_be16(ctx.priv_len, blob);
> @@ -224,6 +239,37 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>  	if (!options->keyhandle)
>  		return -EINVAL;
>  
> +	if (options->pcrinfo_len != 0) {
> +		struct tpm2_policies *pols;
> +		static u8 *scratch;
> +		/* 4 array len, 2 hash alg */
> +		const int len = 4 + 2 + options->pcrinfo_len;
> +
> +		pols = kmalloc(sizeof(*pols) + len, GFP_KERNEL);
> +		if (!pols)
> +			return -ENOMEM;
> +
> +		pols->count = 1;
> +		pols->len[0] = len;
> +		scratch = (u8 *)(pols + 1);
> +		pols->policies[0] = scratch;
> +		pols->code[0] = TPM2_CC_POLICY_PCR;
> +
> +		put_unaligned_be32(1, &scratch[0]);
> +		put_unaligned_be16(hash, &scratch[4]);
> +		memcpy(&scratch[6], options->pcrinfo, options->pcrinfo_len);
> +		payload->policies = pols;
> +	}
> +
> +	if (payload->policies) {
> +		rc = tpm2_generate_policy_digest(payload->policies,
> +						 options->hash,
> +						 options->policydigest,
> +						 &options->policydigest_len);
> +		if (rc)
> +			return rc;
> +	}
> +
>  	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
>  	if (rc)
>  		return rc;
> @@ -421,21 +467,30 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
>  	u16 data_len;
>  	u8 *data;
>  	int rc;
> +	u32 policyhandle = 0;
>  
>  	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
>  	if (rc)
>  		return rc;
>  
> +	if (payload->policies) {
> +		rc = tpm2_get_policy_session(chip, payload->policies,
> +					     &policyhandle);
> +		if (rc)
> +			return rc;
> +	}
> +
>  	tpm_buf_append_u32(&buf, blob_handle);
>  	tpm2_buf_append_auth(&buf,
> -			     options->policyhandle ?
> -			     options->policyhandle : TPM2_RS_PW,
> +			     policyhandle ?
> +			     policyhandle : TPM2_RS_PW,
>  			     NULL /* nonce */, 0,
>  			     TPM2_SA_CONTINUE_SESSION,
>  			     options->blobauth /* hmac */,
> -			     TPM_DIGEST_SIZE);
> +			     policyhandle ? 0 : TPM_DIGEST_SIZE);
>  
>  	rc = tpm_send(chip, buf.data, tpm_buf_length(&buf));
> +	tpm2_flush_context(chip, policyhandle);
>  	if (rc > 0)
>  		rc = -EPERM;
>  


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5174 bytes --]

Re: [PATCH 6/8] security: keys: trusted: add PCR policy to TPM2 keys

[James Bottomley]

On Mon, 2019-12-09 at 10:18 +0000, David Woodhouse wrote:
> On Sat, 2019-12-07 at 21:12 -0800, James Bottomley wrote:
> > This commit adds the ability to specify a PCR lock policy to TPM2
> > keys.  There is a complexity in that the creator of the key must
> > chose either to use a PCR lock policy or to use authentication.  At
> > the current time they can't use both due to a complexity with the
> > way authentication works when policy registers are in use.  The way
> > to construct a pcrinfo statement for a key is simply to use the
> > TPMS_PCR_SELECT structure to specify the PCRs and follow this by a
> > hash of all their values in order of ascending PCR number.
> > 
> > For simplicity, we require the policy name hash and the hash used
> > for the PCRs to be the same.  Thus to construct a policy around the
> > value of the resettable PCR 16 using the sha1 bank, first reset the
> > pcr to zero giving a hash of all zeros as:
> > 
> > 6768033e216468247bd031a0a2d9876d79818f8f
> > 
> > Then the TPMS_PCR_SELECT value for PCR 16 is
> > 
> > 03000001
> > 
> > So create a new 32 byte key with a policy policy locking the key to
> > this value of PCR 16 with a parent key of 81000001 would be:
> > 
> > keyctl new 32 keyhandle=0x81000001 hash=sha1
> > pcrinfo=030000016768033e216468247bd031a0a2d9876d79818f8f" @u
> 
> OK... but I've love to see a more formal definition of this binary
> format, as part of the "standard" we allegedly have for the overall
> ASN.1 representation.

It's actually defined in the TPM2 command manual ... it's basically the
policy commands you send to the TPM ordered so they can be directly
hashed.

However, I agree a standards definition would be good.  This format
doesn't support TPM2_PolicyOr directly (and the command manual is
silent on how it should be supported), so that's going to have to be
defined in the standard anyway.

[...]
> > +int tpm2_encode_policy(struct tpm2_policies *pols, u8 **data, u32
> > *len)
> > +{
> > +	u8 *buf = kmalloc(2 * PAGE_SIZE, GFP_KERNEL);
> > +	u8 *work = buf + PAGE_SIZE, *ptr;
> > +	int i;
> > +
> > +	if (!buf)
> > +		return -ENOMEM;
> > +
> > +	for (i = 0; i < pols->count; i++) {
> > +		u8 *seq_len, *tag_len;
> > +		u32 cmd = pols->code[i];
> > +		int l;
> > +
> > +		/*
> > +		 * cheat a bit here: we know a policy is < 128
> > bytes,
> > +		 * so the sequence and cons tags will only be two
> > +		 * bytes long
> > +		 */
> > +		*work++ = _tag(UNIV, CONS, SEQ);
> > +		seq_len = work++;
> > +		*work++ = _tagn(CONT, CONS, 0);
> > +		tag_len = work++;
> > +		asn1_encode_integer(&work, cmd);
> > +		*tag_len = work - tag_len - 1;
> > +		*work++ = _tagn(CONT, CONS, 1);
> > +		tag_len = work++;
> > +		asn1_encode_octet_string(&work, pols->policies[i],
> > +					 pols->len[i]);
> > +		*tag_len = work - tag_len - 1;
> > +		l = work - seq_len - 1;
> > +		/* our assumption about policy length failed */
> > +		if (WARN(l > 127,
> > +			 "policy is too long: %d but must be <
> > 128", l)) {
> > +			kfree(buf);
> > +			return -EINVAL;
> > +		}
> > +		*seq_len = l;
> 
> 
> 
> You're not even using your own sequence encoding here, because it
> only works when you know the length in advance. How about setting
> *seq_len to 0x80 to start with, for an indeterminate length.

I already did that in the asn.1 patch, so I've updated this one to use
it.

> Then in the happy case where it is <128, just go back and fill it in
> as you currently do. Otherwise append 0x00 0x00 as the end marker.

That doesn't work ... the format of these octet strings is likely to
have two zeros together, so they *have* to be definite length encoded.

> None of this has to be DER, does it?

None of what?  The policy?  the DER format is already in use so we
can't change it.

> <Insert more whining about PAGE_SIZE assumptions and buffer
> overflows>

OK, OK, I fixed that too.

James

Re: [PATCH 6/8] security: keys: trusted: add PCR policy to TPM2 keys

[David Woodhouse]

On 9 December 2019 18:03:11 GMT, James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
>On Mon, 2019-12-09 at 10:18 +0000, David Woodhouse wrote:
>> On Sat, 2019-12-07 at 21:12 -0800, James Bottomley wrote:
>> > This commit adds the ability to specify a PCR lock policy to TPM2
>> > keys.  There is a complexity in that the creator of the key must
>> > chose either to use a PCR lock policy or to use authentication.  At
>> > the current time they can't use both due to a complexity with the
>> > way authentication works when policy registers are in use.  The way
>> > to construct a pcrinfo statement for a key is simply to use the
>> > TPMS_PCR_SELECT structure to specify the PCRs and follow this by a
>> > hash of all their values in order of ascending PCR number.
>> > 
>> > For simplicity, we require the policy name hash and the hash used
>> > for the PCRs to be the same.  Thus to construct a policy around the
>> > value of the resettable PCR 16 using the sha1 bank, first reset the
>> > pcr to zero giving a hash of all zeros as:
>> > 
>> > 6768033e216468247bd031a0a2d9876d79818f8f
>> > 
>> > Then the TPMS_PCR_SELECT value for PCR 16 is
>> > 
>> > 03000001
>> > 
>> > So create a new 32 byte key with a policy policy locking the key to
>> > this value of PCR 16 with a parent key of 81000001 would be:
>> > 
>> > keyctl new 32 keyhandle=0x81000001 hash=sha1
>> > pcrinfo=030000016768033e216468247bd031a0a2d9876d79818f8f" @u
>> 
>> OK... but I've love to see a more formal definition of this binary
>> format, as part of the "standard" we allegedly have for the overall
>> ASN.1 representation.
>
>It's actually defined in the TPM2 command manual ... it's basically the
>policy commands you send to the TPM ordered so they can be directly
>hashed.
>
>However, I agree a standards definition would be good.  This format
>doesn't support TPM2_PolicyOr directly (and the command manual is
>silent on how it should be supported), so that's going to have to be
>defined in the standard anyway.
>
>[...]
>> > +int tpm2_encode_policy(struct tpm2_policies *pols, u8 **data, u32
>> > *len)
>> > +{
>> > +	u8 *buf = kmalloc(2 * PAGE_SIZE, GFP_KERNEL);
>> > +	u8 *work = buf + PAGE_SIZE, *ptr;
>> > +	int i;
>> > +
>> > +	if (!buf)
>> > +		return -ENOMEM;
>> > +
>> > +	for (i = 0; i < pols->count; i++) {
>> > +		u8 *seq_len, *tag_len;
>> > +		u32 cmd = pols->code[i];
>> > +		int l;
>> > +
>> > +		/*
>> > +		 * cheat a bit here: we know a policy is < 128
>> > bytes,
>> > +		 * so the sequence and cons tags will only be two
>> > +		 * bytes long
>> > +		 */
>> > +		*work++ = _tag(UNIV, CONS, SEQ);
>> > +		seq_len = work++;
>> > +		*work++ = _tagn(CONT, CONS, 0);
>> > +		tag_len = work++;
>> > +		asn1_encode_integer(&work, cmd);
>> > +		*tag_len = work - tag_len - 1;
>> > +		*work++ = _tagn(CONT, CONS, 1);
>> > +		tag_len = work++;
>> > +		asn1_encode_octet_string(&work, pols->policies[i],
>> > +					 pols->len[i]);
>> > +		*tag_len = work - tag_len - 1;
>> > +		l = work - seq_len - 1;
>> > +		/* our assumption about policy length failed */
>> > +		if (WARN(l > 127,
>> > +			 "policy is too long: %d but must be <
>> > 128", l)) {
>> > +			kfree(buf);
>> > +			return -EINVAL;
>> > +		}
>> > +		*seq_len = l;
>> 
>> 
>> 
>> You're not even using your own sequence encoding here, because it
>> only works when you know the length in advance. How about setting
>> *seq_len to 0x80 to start with, for an indeterminate length.
>
>I already did that in the asn.1 patch, so I've updated this one to use
>it.
>
>> Then in the happy case where it is <128, just go back and fill it in
>> as you currently do. Otherwise append 0x00 0x00 as the end marker.
>
>That doesn't work ... the format of these octet strings is likely to
>have two zeros together, so they *have* to be definite length encoded.

The octet-strings sure, but we know the length of those. It was the sequence you have that <127 check and bail out for... wasn't it?

>> None of this has to be DER, does it?
>
>None of what?  The policy?  the DER format is already in use so we
>can't change it.

What we *output* doesn't need to be DER (mandatory definite length) and can be BER though, right?

>> <Insert more whining about PAGE_SIZE assumptions and buffer
>> overflows>
>
>OK, OK, I fixed that too.
>
>James

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCH 6/8] security: keys: trusted: add PCR policy to TPM2 keys

[James Bottomley]

On Mon, 2019-12-09 at 18:44 +0000, David Woodhouse wrote:
> On 9 December 2019 18:03:11 GMT, James Bottomley
> <James.Bottomley@HansenPartnership> wrote:
[...]
> > > Then in the happy case where it is <128, just go back and fill it
> > > in as you currently do. Otherwise append 0x00 0x00 as the end
> > > marker.
> > 
> > That doesn't work ... the format of these octet strings is likely
> > to have two zeros together, so they *have* to be definite length
> > encoded.
> 
> The octet-strings sure, but we know the length of those. It was the
> sequence you have that <127 check and bail out for... wasn't it?

The interior sequence encloses the octet streams.  In theory the
standard requires any parser to skip over the interior structures
rather than simply chunk down the sequence looking for the two byte
zero, but doing definite length allows us not to have to worry about
buggy parsers in that case.

> > > None of this has to be DER, does it?
> > 
> > None of what?  The policy?  the DER format is already in use so we
> > can't change it.
> 
> What we *output* doesn't need to be DER (mandatory definite length)
> and can be BER though, right?

I'm not sure.  I think the openssl routines that save and load the
structure in userspace do BER but I'd rather stick to DER to be on the
safe side.

Plus, I know of no policy statement that's anywhere near 127 bytes
long, so there's no problem with doing the single byte fixed length
that DER requires.

James

Re: [PATCH 6/8] security: keys: trusted: add PCR policy to TPM2 keys

[Ken Goldman]

On 12/9/2019 2:11 PM, James Bottomley wrote:
> Plus, I know of no policy statement that's anywhere near 127 bytes
> long, so there's no problem with doing the single byte fixed length
> that DER requires.

Is "a policy statement" the TPM command?

PolicyOr takes a list of hashes.  A typical policy may only have 3
sha256 hashes, but it could potentially be 8 sha384 hashes.

PolicySigned has a policy with a 256 byte public key and a TPM
command with a 256 byte signature.

In general, since the TPM input command buffer is 1 - 1.5k,
that's a reasonable value for input parameters.

[PATCH 7/8] security: keys: trusted: add ability to specify arbitrary policy

[James Bottomley]

This patch adds a policy= argument to key creation.  The policy is the
standard tss policymaker format and each separate policy line must
have a newline after it.

Thus to construct a policy requiring authorized value and pcr 16
locking using a sha256 hash, the policy (policy.txt) file would be two
lines:

0000017F00000001000B03000001303095B49BE85E381E5B20E557E46363EF55B0F43B132C2D8E3DE9AC436656F2
0000016b

This can be inserted into the key with

keyctl add trusted kmk "new 32 policy=`cat policy.txt` keyhandle=0x81000001 hash=sha256" @u

Note that although a few policies work like this, most require special
handling which must be added to the kernel policy construction
routine.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 Documentation/security/keys/trusted-encrypted.rst | 16 ++++++++
 security/keys/trusted-keys/tpm2-policy.c          | 46 +++++++++++++++++++++++
 security/keys/trusted-keys/tpm2-policy.h          |  1 +
 security/keys/trusted-keys/trusted_tpm1.c         | 14 ++++++-
 4 files changed, 76 insertions(+), 1 deletion(-)

diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst
index 1a3ca84ad3cd..ade1a9dc8367 100644
--- a/Documentation/security/keys/trusted-encrypted.rst
+++ b/Documentation/security/keys/trusted-encrypted.rst
@@ -70,6 +70,9 @@ Usage::
        hash=         hash algorithm name as a string. For TPM 1.x the only
                      allowed value is sha1. For TPM 2.x the allowed values
                      are sha1, sha256, sha384, sha512 and sm3-256.
+       policy=       specify an arbitrary set of policies.  These must
+                     be in policymaker format with each separate
+                     policy line newline terminated.
 
 "keyctl print" returns an ascii hex copy of the sealed key, which is in standard
 TPM_STORED_DATA format.  The key length for new keys are always in bytes.
@@ -162,6 +165,19 @@ zeros (the value of PCR 16)::
     $ dd if=/dev/zero bs=1 count=20 2>/dev/null|sha1sum
     6768033e216468247bd031a0a2d9876d79818f8f
 
+You can also specify arbitrary policy in policymaker format, so a two
+value policy (the pcr example above and authvalue) would look like
+this in policymaker format::
+
+    0000017F000000010004030000016768033e216468247bd031a0a2d9876d79818f8f
+    0000016b
+
+This can be placed in a file (say policy.txt) and then added to the key as::
+
+    $ keyctl add trusted kmk "new 32 keyhandle=0x81000001 hash=sha1 policy=`cat policy.txt`" @u
+
+The newlines in the file policy.txt will be automatically processed.
+
 Reseal a trusted key under new pcr values::
 
     $ keyctl update 268728824 "update pcrinfo=`cat pcr.blob`"
diff --git a/security/keys/trusted-keys/tpm2-policy.c b/security/keys/trusted-keys/tpm2-policy.c
index ae83636ece37..4f8549219055 100644
--- a/security/keys/trusted-keys/tpm2-policy.c
+++ b/security/keys/trusted-keys/tpm2-policy.c
@@ -342,3 +342,49 @@ int tpm2_get_policy_session(struct tpm_chip *chip, struct tpm2_policies *pols,
 	}
 	return 0;
 }
+
+int tpm2_parse_policies(struct tpm2_policies **ppols, char *str)
+{
+	struct tpm2_policies *pols;
+	char *p;
+	u8 *ptr;
+	int i = 0, left = PAGE_SIZE, res;
+
+	pols = kmalloc(left, GFP_KERNEL);
+	if (!pols)
+		return -ENOMEM;
+
+	ptr = (u8 *)(pols + 1);
+	left -= ptr - (u8 *)pols;
+
+	while ((p = strsep(&str, "\n"))) {
+		if (*p == '\0' || *p == '\n')
+			continue;
+		pols->len[i] = strlen(p)/2;
+		if (pols->len[i] > left) {
+			res = -E2BIG;
+			goto err;
+		}
+		res = hex2bin(ptr, p, pols->len[i]);
+		if (res)
+			goto err;
+		/* get command code and skip past */
+		pols->code[i] = get_unaligned_be32(ptr);
+		pols->policies[i] = ptr + 4;
+		ptr += pols->len[i];
+		left -= pols->len[i];
+		pols->len[i] -= 4;
+		/*
+		 * FIXME: this does leave the code embedded in dead
+		 * regions of the memory, but it's easier than
+		 * hexdumping to a temporary or copying over
+		 */
+		i++;
+	}
+	pols->count = i;
+	*ppols = pols;
+	return 0;
+ err:
+	kfree(pols);
+	return res;
+}
diff --git a/security/keys/trusted-keys/tpm2-policy.h b/security/keys/trusted-keys/tpm2-policy.h
index 152c948743f3..cb804a544ced 100644
--- a/security/keys/trusted-keys/tpm2-policy.h
+++ b/security/keys/trusted-keys/tpm2-policy.h
@@ -28,3 +28,4 @@ int tpm2_generate_policy_digest(struct tpm2_policies *pols, u32 hash,
 int tpm2_encode_policy(struct tpm2_policies *pols, u8 **data, u32 *len);
 int tpm2_get_policy_session(struct tpm_chip *chip, struct tpm2_policies *pols,
 			    u32 *handle);
+int tpm2_parse_policies(struct tpm2_policies **ppols, char *str);
diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
index 6290e611b632..ba05c75c3170 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -29,6 +29,8 @@
 
 #include <keys/trusted_tpm.h>
 
+#include "tpm2-policy.h"
+
 static const char hmac_alg[] = "hmac(sha1)";
 static const char hash_alg[] = "sha1";
 static struct tpm_chip *chip;
@@ -706,7 +708,7 @@ enum {
 	Opt_new, Opt_load, Opt_update,
 	Opt_keyhandle, Opt_keyauth, Opt_blobauth,
 	Opt_pcrinfo, Opt_pcrlock, Opt_migratable,
-	Opt_hash,
+	Opt_hash, Opt_policy,
 };
 
 static const match_table_t key_tokens = {
@@ -720,6 +722,7 @@ static const match_table_t key_tokens = {
 	{Opt_pcrlock, "pcrlock=%s"},
 	{Opt_migratable, "migratable=%s"},
 	{Opt_hash, "hash=%s"},
+	{Opt_policy, "policy=%s"},
 	{Opt_err, NULL}
 };
 
@@ -809,6 +812,15 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 				return -EINVAL;
 			}
 			break;
+		case Opt_policy:
+			if (pay->policies)
+				return -EINVAL;
+			if (!tpm2)
+				return -EINVAL;
+			res = tpm2_parse_policies(&pay->policies, args[0].from);
+			if (res)
+				return res;
+			break;
 		default:
 			return -EINVAL;
 		}
-- 
2.16.4

[PATCH 8/8] security: keys: trusted: implement counter/timer policy

[James Bottomley]

This is actually a generic policy allowing a range of comparisons
against any value set in the TPM Clock, which includes things like the
reset count, a monotonic millisecond count and the restart count.  The
most useful comparison is against the millisecond count for expiring
keys.  However, you have to remember that currently Linux doesn't try
to sync the epoch timer with the TPM, so the expiration is actually
measured in how long the TPM itself has been powered on ... the TPM
timer doesn't count while the system is powered down.  The millisecond
counter is a u64 quantity found at offset 8 in the timer structure,
and the <= comparision operand is 9, so a policy set to expire after the
TPM has been up for 100 seconds would look like

0000016d00000000000f424000080009

Where 0x16d is the counter timer policy code and 0xf4240 is 100 000 in
hex.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 Documentation/security/keys/trusted-encrypted.rst | 29 +++++++++++++++++++++++
 security/keys/trusted-keys/tpm2-policy.c          | 19 +++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst
index ade1a9dc8367..52d8bd8bef65 100644
--- a/Documentation/security/keys/trusted-encrypted.rst
+++ b/Documentation/security/keys/trusted-encrypted.rst
@@ -235,3 +235,32 @@ about the usage can be found in the file
 Another new format 'enc32' has been defined in order to support encrypted keys
 with payload size of 32 bytes. This will initially be used for nvdimm security
 but may expand to other usages that require 32 bytes payload.
+
+Appendix
+--------
+
+TPM 2.0 Policies
+----------------
+
+The current TPM supports PCR lock policies as documented above and
+CounterTimer policies which can be used to create expiring keys.  One
+caveat with expiring keys is that the TPM millisecond counter does not
+update while a system is powered off and Linux does not sync the TPM
+millisecond count with its internal clock, so the best you can expire
+in is in terms of how long any given TPM has been powered on.  (FIXME:
+Linux should simply update the millisecond clock to the current number
+of seconds past the epoch on boot).
+
+A CounterTimer policy is expressed in terms of length and offset
+against the TPM clock structure (TPMS_TIME_INFO), which looks like the
+packed structure::
+
+    struct tpms_time_info {
+            u64 uptime;       /* time in ms since last start or reset */
+	    u64 clock;        /* cumulative uptime in ms */
+	    u32 resetcount;   /* numer of times the TPM has been reset */
+	    u32 restartcount; /* number of times the TPM has been restarted */
+	    u8  safe          /* time was safely loaded from NVRam */
+    };
+
+The usual comparison for expiring keys is against clock, at offset 8.
diff --git a/security/keys/trusted-keys/tpm2-policy.c b/security/keys/trusted-keys/tpm2-policy.c
index 4f8549219055..4baa445e7859 100644
--- a/security/keys/trusted-keys/tpm2-policy.c
+++ b/security/keys/trusted-keys/tpm2-policy.c
@@ -326,6 +326,25 @@ int tpm2_get_policy_session(struct tpm_chip *chip, struct tpm2_policies *pols,
 			tpm_buf_append(&buf, pols->policies[i],
 				       pols->len[i] - pols->hash_size);
 			break;
+		case TPM2_CC_POLICY_COUNTER_TIMER: {
+			/*
+			 * the format of this is the last two u16
+			 * quantities are the offset and operation
+			 * respectively.  The rest is operandB which
+			 * must be zero padded in a hash digest
+			 */
+			u16 opb_len = pols->len[i] - 4;
+
+			if (opb_len > pols->hash_size)
+				return -EINVAL;
+
+			tpm_buf_append_u16(&buf, opb_len);
+			tpm_buf_append(&buf, pols->policies[i], opb_len);
+			/* offset and operand*/
+			tpm_buf_append(&buf, pols->policies[i] + opb_len, 4);
+			failure = "Counter Timer";
+			break;
+		}
 		default:
 			failure = "unknown policy";
 			break;
-- 
2.16.4

Re: [PATCH 0/8] Fix TPM 2.0 trusted keys

[Jarkko Sakkinen]

On Sat, Dec 07, 2019 at 09:06:40PM -0800, James Bottomley wrote:
> The big problem with this patch is still that we can't yet combine
> policy with authorization because that requires proper session
> handling, but at least with this rewrite it becomes possible (whereas
> it was never possible with the old external policy session code). 
> Thus, when we have the TPM 2.0 security patch upstream, we'll be able
> to use the session logic from that patch to imlement authorizations.

This essentially means that this is an RFC, not something that can be
merged at this point before whatever you mean by proper has been landed.

/Jarkko

Re: [PATCH 0/8] Fix TPM 2.0 trusted keys

[James Bottomley]

On Mon, 2019-12-09 at 22:20 +0200, Jarkko Sakkinen wrote:
> On Sat, Dec 07, 2019 at 09:06:40PM -0800, James Bottomley wrote:
> > The big problem with this patch is still that we can't yet combine
> > policy with authorization because that requires proper session
> > handling, but at least with this rewrite it becomes possible
> > (whereas it was never possible with the old external policy session
> > code). Thus, when we have the TPM 2.0 security patch upstream,
> > we'll be able to use the session logic from that patch to imlement
> > authorizations.
> 
> This essentially means that this is an RFC, not something that can be
> merged at this point before whatever you mean by proper has been
> landed.

No it doesn't.  It just means we have a limitation in the keys that
needs to be removed at a later time when we have the authentication
mechanisms.  Since there will simply be a feature added with no
backward compat problems, it's not a merge blocker.

James