* [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy
@ 2020-05-07 23:11 James Bottomley
2020-05-07 23:11 ` [PATCH v9 1/8] lib: add ASN.1 encoder James Bottomley
` (8 more replies)
0 siblings, 9 replies; 33+ messages in thread
From: James Bottomley @ 2020-05-07 23:11 UTC (permalink / raw)
To: linux-integrity
Cc: Mimi Zohar, Jarkko Sakkinen, David Woodhouse, keyrings, David Howells
This is a respin on v8 to make the encoder selectable and address
David's comments. The trusted key part hasn't changed except to add a
now necessary select for ASN1_ENCODER to patch 4 and the changelog of
patch 6 has been updated to correct the cut and paste error in the
keyctl statement.
General cover letter:
This patch updates the trusted key code to export keys in the ASN.1
format used by current TPM key tools (openssl_tpm2_engine and
openconnect). It also simplifies the use of policy with keys because
the ASN.1 format is designed to carry a description of how to
construct the policy, with the result that simple policies (like
authorization and PCR locking) can now be constructed and used in the
kernel, bringing the TPM 2.0 policy use into line with how TPM 1.2
works.
The key format is designed to be compatible with our two openssl
engine implementations as well as with the format used by openconnect.
I've added seal/unseal to my engine so I can use it for
interoperability testing and I'll later use this for sealed symmetric
keys via engine:
https://git.kernel.org/pub/scm/linux/kernel/git/jejb/openssl_tpm2_engine.git/
James
---
James Bottomley (8):
lib: add ASN.1 encoder
oid_registry: Add TCG defined OIDS for TPM keys
security: keys: trusted: fix TPM2 authorizations
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
.../security/keys/trusted-encrypted.rst | 64 ++-
include/keys/trusted-type.h | 7 +-
include/linux/asn1_encoder.h | 32 ++
include/linux/oid_registry.h | 5 +
include/linux/tpm.h | 8 +
lib/Kconfig | 3 +
lib/Makefile | 1 +
lib/asn1_encoder.c | 454 +++++++++++++++++
security/keys/Kconfig | 3 +
security/keys/trusted-keys/Makefile | 2 +-
security/keys/trusted-keys/tpm2-policy.c | 463 ++++++++++++++++++
security/keys/trusted-keys/tpm2-policy.h | 31 ++
security/keys/trusted-keys/tpm2key.asn1 | 23 +
security/keys/trusted-keys/trusted_tpm1.c | 56 ++-
security/keys/trusted-keys/trusted_tpm2.c | 370 +++++++++++++-
15 files changed, 1486 insertions(+), 36 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.26.1
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v9 1/8] lib: add ASN.1 encoder
2020-05-07 23:11 [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy James Bottomley
@ 2020-05-07 23:11 ` James Bottomley
2020-05-17 8:17 ` Sakkinen, Jarkko
2020-05-07 23:11 ` [PATCH v9 2/8] oid_registry: Add TCG defined OIDS for TPM keys James Bottomley
` (7 subsequent siblings)
8 siblings, 1 reply; 33+ messages in thread
From: James Bottomley @ 2020-05-07 23:11 UTC (permalink / raw)
To: linux-integrity
Cc: Mimi Zohar, Jarkko Sakkinen, David Woodhouse, keyrings, David Howells
We have a need in the TPM2 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. The specific tools are the openssl_tpm2_engine, openconnect
and the Intel tpm2-tss-engine. To do that, we have to be able to read
and write the same binary key format the tools use. The current ASN.1
decoder does fine for reading, but we need pieces of an ASN.1 encoder
to write the key blob in binary compatible form.
For backwards compatibility, the trusted key reader code will still
accept the two TPM2B quantities that it uses today, but the writer
will only output the ASN.1 form.
The current implementation only encodes the ASN.1 bits we actually need.
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
v2: updated API to use indefinite length, and made symbol exports gpl
v3: add data length error handling
v4: use end_data instead of data_len pointer
v5: mention tools and space out code
v7: reverse christmas tree variable definitions and better spacing
v9: update comments, remove indefinite length, make encoder config dependent
---
include/linux/asn1_encoder.h | 32 +++
lib/Kconfig | 3 +
lib/Makefile | 1 +
lib/asn1_encoder.c | 454 +++++++++++++++++++++++++++++++++++
4 files changed, 490 insertions(+)
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..08cd0c2ad34f
--- /dev/null
+++ b/include/linux/asn1_encoder.h
@@ -0,0 +1,32 @@
+/* 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>
+#include <linux/bug.h>
+
+#define asn1_oid_len(oid) (sizeof(oid)/sizeof(u32))
+unsigned char *
+asn1_encode_integer(unsigned char *data, const unsigned char *end_data,
+ s64 integer);
+unsigned char *
+asn1_encode_oid(unsigned char *data, const unsigned char *end_data,
+ u32 oid[], int oid_len);
+unsigned char *
+asn1_encode_tag(unsigned char *data, const unsigned char *end_data,
+ u32 tag, const unsigned char *string, int len);
+unsigned char *
+asn1_encode_octet_string(unsigned char *data,
+ const unsigned char *end_data,
+ const unsigned char *string, u32 len);
+unsigned char *
+asn1_encode_sequence(unsigned char *data, const unsigned char *end_data,
+ const unsigned char *seq, int len);
+unsigned char *
+asn1_encode_boolean(unsigned char *data, const unsigned char *end_data,
+ bool val);
+
+#endif
diff --git a/lib/Kconfig b/lib/Kconfig
index 5d53f9609c25..93be3bf6e8db 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -670,3 +670,6 @@ config GENERIC_LIB_CMPDI2
config GENERIC_LIB_UCMPDI2
bool
+
+config ASN1_ENCODER
+ tristate
diff --git a/lib/Makefile b/lib/Makefile
index 685aee60de1d..12ab0381a50a 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -245,6 +245,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_ENCODER) += 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..41e71aae3ef6
--- /dev/null
+++ b/lib/asn1_encoder.c
@@ -0,0 +1,454 @@
+// 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
+ * @end_data: end of data pointer, points one beyond last usable byte in @data
+ * @integer: integer to be encoded
+ *
+ * This is a simplified encoder: it only currently does
+ * positive integers, but it should be simple enough to add the
+ * negative case if a use comes along.
+ */
+unsigned char *
+asn1_encode_integer(unsigned char *data, const unsigned char *end_data,
+ s64 integer)
+{
+ int data_len = end_data - data;
+ unsigned char *d = &data[2];
+ bool found = false;
+ int i;
+
+ if (WARN(integer < 0,
+ "BUG: integer encode only supports positive integers"))
+ return ERR_PTR(-EINVAL);
+
+ if (IS_ERR(data))
+ return data;
+
+ /* need at least 3 bytes for tag, length and integer encoding */
+ if (data_len < 3)
+ return ERR_PTR(-EINVAL);
+
+ /* remaining length where at d (the start of the integer encoding) */
+ data_len -= 2;
+
+ 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;
+
+ /*
+ * for a positive number the first byte must have bit
+ * 7 clear in two's complement (otherwise it's a
+ * negative number) so prepend a leading zero if
+ * that's not the case
+ */
+ if (!found && (byte & 0x80)) {
+ /*
+ * no check needed here, we already know we
+ * have len >= 1
+ */
+ *d++ = 0;
+ data_len--;
+ }
+
+ found = true;
+ if (data_len == 0)
+ return ERR_PTR(-EINVAL);
+
+ *d++ = byte;
+ data_len--;
+ }
+
+ out:
+ data[1] = d - data - 2;
+
+ return d;
+}
+EXPORT_SYMBOL_GPL(asn1_encode_integer);
+
+/* calculate the base 128 digit values setting the top bit of the first octet */
+static int asn1_encode_oid_digit(unsigned char **_data, int *data_len, u32 oid)
+{
+ unsigned char *data = *_data;
+ int start = 7 + 7 + 7 + 7;
+ int ret = 0;
+
+ if (*data_len < 1)
+ return -EINVAL;
+
+ /* quick case */
+ if (oid == 0) {
+ *data++ = 0x80;
+ (*data_len)--;
+ goto out;
+ }
+
+ while (oid >> start == 0)
+ start -= 7;
+
+ while (start > 0 && *data_len > 0) {
+ u8 byte;
+
+ byte = oid >> start;
+ oid = oid - (byte << start);
+ start -= 7;
+ byte |= 0x80;
+ *data++ = byte;
+ (*data_len)--;
+ }
+
+ if (*data_len > 0) {
+ *data++ = oid;
+ (*data_len)--;
+ } else {
+ ret = -EINVAL;
+ }
+
+ out:
+ *_data = data;
+ return ret;
+}
+
+/**
+ * asn1_encode_oid() - encode an oid to ASN.1
+ * @data: position to begin encoding at
+ * @end_data: end of data pointer, points one beyond last usable byte in @data
+ * @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
+ */
+unsigned char *
+asn1_encode_oid(unsigned char *data, const unsigned char *end_data,
+ u32 oid[], int oid_len)
+{
+ int data_len = end_data - data;
+ unsigned char *d = data + 2;
+ int i, ret;
+
+ if (WARN(oid_len < 2, "OID must have at least two elements"))
+ return ERR_PTR(-EINVAL);
+
+ if (WARN(oid_len > 32, "OID is too large"))
+ return ERR_PTR(-EINVAL);
+
+ if (IS_ERR(data))
+ return data;
+
+
+ /* need at least 3 bytes for tag, length and OID encoding */
+ if (data_len < 3)
+ return ERR_PTR(-EINVAL);
+
+ data[0] = _tag(UNIV, PRIM, OID);
+ *d++ = oid[0] * 40 + oid[1];
+
+ data_len -= 3;
+
+ ret = 0;
+
+ for (i = 2; i < oid_len; i++) {
+ ret = asn1_encode_oid_digit(&d, &data_len, oid[i]);
+ if (ret < 0)
+ return ERR_PTR(ret);
+ }
+
+ data[1] = d - data - 2;
+
+ return d;
+}
+EXPORT_SYMBOL_GPL(asn1_encode_oid);
+
+/**
+ * asn1_encode_length() - encode a length to follow an ASN.1 tag
+ * @data: pointer to encode at
+ * @data_len: pointer to remaning length (adjusted by routine)
+ * @len: length to encode
+ *
+ * This routine can encode lengths up to 65535 using the ASN.1 rules.
+ * It will accept a negative length and place a zero length tag
+ * instead (to keep the ASN.1 valid). This convention allows other
+ * encoder primitives to accept negative lengths as singalling the
+ * sequence will be re-encoded when the length is known.
+ */
+static int asn1_encode_length(unsigned char **data, int *data_len, int len)
+{
+ if (*data_len < 1)
+ return -EINVAL;
+
+ if (len < 0) {
+ *((*data)++) = 0;
+ (*data_len)--;
+ return 0;
+ }
+
+ if (len <= 0x7f) {
+ *((*data)++) = len;
+ (*data_len)--;
+ return 0;
+ }
+
+ if (*data_len < 2)
+ return -EINVAL;
+
+ if (len <= 0xff) {
+ *((*data)++) = 0x81;
+ *((*data)++) = len & 0xff;
+ *data_len -= 2;
+ return 0;
+ }
+
+ if (*data_len < 3)
+ return -EINVAL;
+
+ if (len <= 0xffff) {
+ *((*data)++) = 0x82;
+ *((*data)++) = (len >> 8) & 0xff;
+ *((*data)++) = len & 0xff;
+ *data_len -= 3;
+ return 0;
+ }
+
+ if (WARN(len > 0xffffff, "ASN.1 length can't be > 0xffffff"))
+ return -EINVAL;
+
+ if (*data_len < 4)
+ return -EINVAL;
+ *((*data)++) = 0x83;
+ *((*data)++) = (len >> 16) & 0xff;
+ *((*data)++) = (len >> 8) & 0xff;
+ *((*data)++) = len & 0xff;
+ *data_len -= 4;
+
+ return 0;
+}
+
+/**
+ * asn1_encode_tag() - add a tag for optional or explicit value
+ * @data: pointer to place tag at
+ * @end_data: end of data pointer, points one beyond last usable byte in @data
+ * @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.
+ *
+ * Standard usage is to pass in a @tag, @string and @length and the
+ * @string will be ASN.1 encoded with @tag and placed into @data. If
+ * the encoding would put data past @end_data then an error is
+ * returned, otherwise a pointer to a position one beyond the encoding
+ * is returned.
+ *
+ * To encode in place pass a NULL @string and -1 for @len and the
+ * maximum allowable beginning and end of the data; all this will do
+ * is add the current maximum length and update the data pointer to
+ * the place where the tag contents should be placed is returned. The
+ * data should be copied in by the calling routine which should then
+ * repeat the prior statement but now with the known length. In order
+ * to avoid having to keep both before and after pointers, the repeat
+ * expects to be called with @data pointing to where the first encode
+ * returned it and still NULL for @string but the real length in @len.
+ */
+unsigned char *
+asn1_encode_tag(unsigned char *data, const unsigned char *end_data,
+ u32 tag, const unsigned char *string, int len)
+{
+ int data_len = end_data - data;
+ int ret;
+
+ if (WARN(tag > 30, "ASN.1 tag can't be > 30"))
+ return ERR_PTR(-EINVAL);
+
+ if (!string && WARN(len > 127,
+ "BUG: recode tag is too big (>127)"))
+ return ERR_PTR(-EINVAL);
+
+ if (IS_ERR(data))
+ return data;
+
+ if (!string && len > 0) {
+ /*
+ * we're recoding, so move back to the start of the
+ * tag and install a dummy length because the real
+ * data_len should be NULL
+ */
+ data -= 2;
+ data_len = 2;
+ }
+
+ if (data_len < 2)
+ return ERR_PTR(-EINVAL);
+
+ *(data++) = _tagn(CONT, CONS, tag);
+ data_len--;
+ ret = asn1_encode_length(&data, &data_len, len);
+ if (ret < 0)
+ return ERR_PTR(ret);
+
+ if (!string)
+ return data;
+
+ if (data_len < len)
+ return ERR_PTR(-EINVAL);
+
+ memcpy(data, string, len);
+ data += len;
+
+ return data;
+}
+EXPORT_SYMBOL_GPL(asn1_encode_tag);
+
+/**
+ * asn1_encode_octet_string() - encode an ASN.1 OCTET STRING
+ * @data: pointer to encode at
+ * @end_data: end of data pointer, points one beyond last usable byte in @data
+ * @string: string to be encoded
+ * @len: length of string
+ *
+ * Note ASN.1 octet strings may contain zeros, so the length is obligatory.
+ */
+unsigned char *
+asn1_encode_octet_string(unsigned char *data,
+ const unsigned char *end_data,
+ const unsigned char *string, u32 len)
+{
+ int data_len = end_data - data;
+ int ret;
+
+ if (IS_ERR(data))
+ return data;
+
+ /* need minimum of 2 bytes for tag and length of zero length string */
+ if (data_len < 2)
+ return ERR_PTR(-EINVAL);
+
+ *(data++) = _tag(UNIV, PRIM, OTS);
+ data_len--;
+
+ ret = asn1_encode_length(&data, &data_len, len);
+ if (ret)
+ return ERR_PTR(ret);
+
+ if (data_len < len)
+ return ERR_PTR(-EINVAL);
+
+ memcpy(data, string, len);
+ data += len;
+
+ return data;
+}
+EXPORT_SYMBOL_GPL(asn1_encode_octet_string);
+
+/**
+ * asn1_encode_sequence() - wrap a byte stream in an ASN.1 SEQUENCE
+ * @data: pointer to encode at
+ * @end_data: end of data pointer, points one beyond last usable byte in @data
+ * @seq: data to be encoded as a sequence
+ * @len: length of the data to be encoded as a sequence
+ *
+ * Fill in a sequence. To encode in place, pass NULL for @seq and -1
+ * for @len; then call again once the length is known (still with NULL
+ * for @seq). In order to avoid having to keep both before and after
+ * pointers, the repeat expects to be called with @data pointing to
+ * where the first encode placed it.
+ */
+unsigned char *
+asn1_encode_sequence(unsigned char *data, const unsigned char *end_data,
+ const unsigned char *seq, int len)
+{
+ int data_len = end_data - data;
+ int ret;
+
+ if (!seq && WARN(len > 127,
+ "BUG: recode sequence is too big (>127)"))
+ return ERR_PTR(-EINVAL);
+
+ if (IS_ERR(data))
+ return data;
+
+ if (!seq && len >= 0) {
+ /*
+ * we're recoding, so move back to the start of the
+ * sequence and install a dummy length because the
+ * real length should be NULL
+ */
+ data -= 2;
+ data_len = 2;
+ }
+
+ if (data_len < 2)
+ return ERR_PTR(-EINVAL);
+
+ *(data++) = _tag(UNIV, CONS, SEQ);
+ data_len--;
+
+ ret = asn1_encode_length(&data, &data_len, len);
+ if (ret)
+ return ERR_PTR(ret);
+
+ if (!seq)
+ return data;
+
+ if (data_len < len)
+ return ERR_PTR(-EINVAL);
+
+ memcpy(data, seq, len);
+ data += len;
+
+ return data;
+}
+EXPORT_SYMBOL_GPL(asn1_encode_sequence);
+
+/**
+ * asn1_encode_boolean() - encode a boolean value to ASN.1
+ * @data: pointer to encode at
+ * @end_data: end of data pointer, points one beyond last usable byte in @data
+ * @val: the boolean true/false value
+ */
+unsigned char *
+asn1_encode_boolean(unsigned char *data, const unsigned char *end_data,
+ bool val)
+{
+ int data_len = end_data - data;
+
+ if (IS_ERR(data))
+ return data;
+
+ /* booleans are 3 bytes: tag, length == 1 and value == 0 or 1 */
+ if (data_len < 3)
+ return ERR_PTR(-EINVAL);
+
+ *(data++) = _tag(UNIV, PRIM, BOOL);
+ data_len--;
+
+ asn1_encode_length(&data, &data_len, 1);
+
+ if (val)
+ *(data++) = 1;
+ else
+ *(data++) = 0;
+
+ return data;
+}
+EXPORT_SYMBOL_GPL(asn1_encode_boolean);
+
+MODULE_LICENSE("GPL");
--
2.26.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH v9 2/8] oid_registry: Add TCG defined OIDS for TPM keys
2020-05-07 23:11 [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy James Bottomley
2020-05-07 23:11 ` [PATCH v9 1/8] lib: add ASN.1 encoder James Bottomley
@ 2020-05-07 23:11 ` James Bottomley
2020-05-07 23:11 ` [PATCH v9 3/8] security: keys: trusted: fix TPM2 authorizations James Bottomley
` (6 subsequent siblings)
8 siblings, 0 replies; 33+ messages in thread
From: James Bottomley @ 2020-05-07 23:11 UTC (permalink / raw)
To: linux-integrity
Cc: Mimi Zohar, Jarkko Sakkinen, David Woodhouse, keyrings, David Howells
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.
The ASN.1 binary key form starts of with this OID as the first element
of a sequence, giving the binary form a unique recognizable identity
marker regardless of encoding.
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Acked-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Reviewed-by: David Howells <dhowells@redhat.com>
---
v3: correct OID_TPMImportableKey name
v7: add ack
v9: add review
---
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..f6e2276e5f30 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_TPMImportableKey, /* 2.23.133.10.1.4 */
+ OID_TPMSealedData, /* 2.23.133.10.1.5 */
+
OID__NR
};
--
2.26.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH v9 3/8] security: keys: trusted: fix TPM2 authorizations
2020-05-07 23:11 [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy James Bottomley
2020-05-07 23:11 ` [PATCH v9 1/8] lib: add ASN.1 encoder James Bottomley
2020-05-07 23:11 ` [PATCH v9 2/8] oid_registry: Add TCG defined OIDS for TPM keys James Bottomley
@ 2020-05-07 23:11 ` James Bottomley
2020-05-14 1:11 ` Jarkko Sakkinen
2020-05-07 23:11 ` [PATCH v9 4/8] security: keys: trusted: use ASN.1 TPM2 key format for the blobs James Bottomley
` (5 subsequent siblings)
8 siblings, 1 reply; 33+ messages in thread
From: James Bottomley @ 2020-05-07 23:11 UTC (permalink / raw)
To: linux-integrity
Cc: Mimi Zohar, Jarkko Sakkinen, David Woodhouse, keyrings, David Howells
In TPM 1.2 an authorization was a 20 byte number. The spec actually
recommended you to hash variable length passwords and use the sha1
hash as the authorization. Because the spec doesn't require this
hashing, the current authorization for trusted keys is a 40 digit hex
number. For TPM 2.0 the spec allows the passing in of variable length
passwords and passphrases directly, so we should allow that in trusted
keys for ease of use. Update the 'blobauth' parameter to take this
into account, so we can now use plain text passwords for the keys.
so before
keyctl add trusted kmk "new 32 blobauth=f572d396fae9206628714fb2ce00f72e94f2258f"
after we will accept both the old hex sha1 form as well as a new
directly supplied password:
keyctl add trusted kmk "new 32 blobauth=hello keyhandle=81000001"
Since a sha1 hex code must be exactly 40 bytes long and a direct
password must be 20 or less, we use the length as the discriminator
for which form is input.
Note this is both and enhancement and a potential bug fix. The TPM
2.0 spec requires us to strip leading zeros, meaning empyty
authorization is a zero length HMAC whereas we're currently passing in
20 bytes of zeros. A lot of TPMs simply accept this as OK, but the
Microsoft TPM emulator rejects it with TPM_RC_BAD_AUTH, so this patch
makes the Microsoft TPM emulator work with trusted keys.
Fixes: 0fe5480303a1 ("keys, trusted: seal/unseal with TPM 2.0 chips")
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
v6: change comment, eliminate else clauses and add fixes tag
v7: fixes before signoff
---
include/keys/trusted-type.h | 1 +
security/keys/trusted-keys/trusted_tpm1.c | 32 ++++++++++++++++++-----
security/keys/trusted-keys/trusted_tpm2.c | 10 ++++---
3 files changed, 33 insertions(+), 10 deletions(-)
diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index a94c03a61d8f..b2ed3481c6a0 100644
--- a/include/keys/trusted-type.h
+++ b/include/keys/trusted-type.h
@@ -30,6 +30,7 @@ struct trusted_key_options {
uint16_t keytype;
uint32_t keyhandle;
unsigned char keyauth[TPM_DIGEST_SIZE];
+ uint32_t blobauth_len;
unsigned char blobauth[TPM_DIGEST_SIZE];
uint32_t pcrinfo_len;
unsigned char pcrinfo[MAX_PCRINFO_SIZE];
diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
index 8001ab07e63b..3b8fa7df0d27 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -781,13 +781,33 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
return -EINVAL;
break;
case Opt_blobauth:
- if (strlen(args[0].from) != 2 * SHA1_DIGEST_SIZE)
- return -EINVAL;
- res = hex2bin(opt->blobauth, args[0].from,
- SHA1_DIGEST_SIZE);
- if (res < 0)
- return -EINVAL;
+ /*
+ * TPM 1.2 authorizations are sha1 hashes passed in as
+ * hex strings. TPM 2.0 authorizations are simple
+ * passwords (although it can take a hash as well)
+ */
+ opt->blobauth_len = strlen(args[0].from);
+
+ if (opt->blobauth_len == 2 * TPM_DIGEST_SIZE) {
+ res = hex2bin(opt->blobauth, args[0].from,
+ TPM_DIGEST_SIZE);
+ if (res < 0)
+ return -EINVAL;
+
+ opt->blobauth_len = TPM_DIGEST_SIZE;
+ return 0;
+ }
+
+ if (tpm2 && opt->blobauth_len <= sizeof(opt->blobauth)) {
+ memcpy(opt->blobauth, args[0].from,
+ opt->blobauth_len);
+ return 0;
+ }
+
+ return -EINVAL;
+
break;
+
case Opt_migratable:
if (*args[0].from == '0')
pay->migratable = 0;
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 08ec7f48f01d..b4a5058107c2 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -91,10 +91,12 @@ 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 + options->blobauth_len + payload->key_len + 1);
+
+ tpm_buf_append_u16(&buf, options->blobauth_len);
+ if (options->blobauth_len)
+ tpm_buf_append(&buf, options->blobauth, options->blobauth_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(&buf, payload->key, payload->key_len);
tpm_buf_append_u8(&buf, payload->migratable);
@@ -258,7 +260,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
NULL /* nonce */, 0,
TPM2_SA_CONTINUE_SESSION,
options->blobauth /* hmac */,
- TPM_DIGEST_SIZE);
+ options->blobauth_len);
rc = tpm_send(chip, buf.data, tpm_buf_length(&buf));
if (rc > 0)
--
2.26.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH v9 4/8] security: keys: trusted: use ASN.1 TPM2 key format for the blobs
2020-05-07 23:11 [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy James Bottomley
` (2 preceding siblings ...)
2020-05-07 23:11 ` [PATCH v9 3/8] security: keys: trusted: fix TPM2 authorizations James Bottomley
@ 2020-05-07 23:11 ` James Bottomley
2020-05-17 8:18 ` Sakkinen, Jarkko
2020-05-07 23:11 ` [PATCH v9 5/8] security: keys: trusted: Make sealed key properly interoperable James Bottomley
` (4 subsequent siblings)
8 siblings, 1 reply; 33+ messages in thread
From: James Bottomley @ 2020-05-07 23:11 UTC (permalink / raw)
To: linux-integrity
Cc: Mimi Zohar, Jarkko Sakkinen, David Woodhouse, keyrings, David Howells
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 (openssl_tpm2_engine,
openconnect and tpm2-tss-engine). 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>
---
v2: Updated encode API, added length checks
v5: correct export format after doing interoperability checks
v7: use prefix tpm2_key_ instead of tpmkey_ for functions
v8: resplit commit
v9: select ASN1_ENCODER
---
include/keys/trusted-type.h | 1 +
security/keys/Kconfig | 1 +
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 | 207 +++++++++++++++++++++-
6 files changed, 228 insertions(+), 8 deletions(-)
create mode 100644 security/keys/trusted-keys/tpm2key.asn1
diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index b2ed3481c6a0..b2d87ad21714 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/security/keys/Kconfig b/security/keys/Kconfig
index 47c041563d41..ae0dd9228fd3 100644
--- a/security/keys/Kconfig
+++ b/security/keys/Kconfig
@@ -77,6 +77,7 @@ config TRUSTED_KEYS
select CRYPTO_HMAC
select CRYPTO_SHA1
select CRYPTO_HASH_INFO
+ select ASN1_ENCODER
help
This option provides support for creating, sealing, and unsealing
keys in the kernel. Trusted keys are random number symmetric keys,
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..660b3fa917ae
--- /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 ({tpm2_key_type}),
+ emptyAuth [0] EXPLICIT BOOLEAN OPTIONAL,
+ policy [1] EXPLICIT TPMPolicySequence OPTIONAL,
+ secret [2] EXPLICIT OCTET STRING OPTIONAL,
+ parent INTEGER ({tpm2_key_parent}),
+ pubkey OCTET STRING ({tpm2_key_pub}),
+ privkey OCTET STRING ({tpm2_key_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 3b8fa7df0d27..4a09165d61cd 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -1011,7 +1011,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 b4a5058107c2..e8dcc47b3388 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,165 @@ 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)
+{
+ const int SCRATCH_SIZE = PAGE_SIZE;
+ u8 *scratch = kmalloc(SCRATCH_SIZE, GFP_KERNEL);
+ u8 *work = scratch, *work1;
+ u8 *end_work = scratch + SCRATCH_SIZE;
+ u8 *priv, *pub;
+ u16 priv_len, pub_len;
+
+ priv_len = get_unaligned_be16(src) + 2;
+ priv = src;
+
+ src += priv_len;
+
+ pub_len = get_unaligned_be16(src) + 2;
+ pub = src;
+
+ if (!scratch)
+ return -ENOMEM;
+
+ work = asn1_encode_oid(work, end_work, tpm2key_oid,
+ asn1_oid_len(tpm2key_oid));
+
+ if (options->blobauth_len == 0) {
+ unsigned char bool[3], *w = bool;
+ /* tag 0 is emptyAuth */
+ w = asn1_encode_boolean(w, w + sizeof(bool), true);
+ if (WARN(IS_ERR(w), "BUG: Boolean failed to encode"))
+ return PTR_ERR(w);
+ work = asn1_encode_tag(work, end_work, 0, bool, w - bool);
+ }
+
+ /*
+ * Assume both octet strings will encode to a 2 byte definite length
+ *
+ * Note: For a well behaved TPM, this warning should never
+ * trigger, so if it does there's something nefarious going on
+ */
+ if (WARN(work - scratch + pub_len + priv_len + 14 > SCRATCH_SIZE,
+ "BUG: scratch buffer is too small"))
+ return -EINVAL;
+
+ work = asn1_encode_integer(work, end_work, options->keyhandle);
+ work = asn1_encode_octet_string(work, end_work, pub, pub_len);
+ work = asn1_encode_octet_string(work, end_work, priv, priv_len);
+
+ work1 = payload->blob;
+ work1 = asn1_encode_sequence(work1, work1 + sizeof(payload->blob),
+ scratch, work - scratch);
+ if (WARN(IS_ERR(work1), "BUG: ASN.1 encoder failed"))
+ return PTR_ERR(work1);
+
+ return work1 - payload->blob;
+}
+
+struct tpm2_key_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 tpm2_key_context ctx;
+ u8 *blob;
+
+ memset(&ctx, 0, sizeof(ctx));
+
+ 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;
+
+ memcpy(blob, ctx.priv, ctx.priv_len);
+ blob += ctx.priv_len;
+
+ memcpy(blob, ctx.pub, ctx.pub_len);
+
+ return 0;
+}
+
+int tpm2_key_parent(void *context, size_t hdrlen,
+ unsigned char tag,
+ const void *value, size_t vlen)
+{
+ struct tpm2_key_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 tpm2_key_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 tpm2_key_pub(void *context, size_t hdrlen,
+ unsigned char tag,
+ const void *value, size_t vlen)
+{
+ struct tpm2_key_context *ctx = context;
+
+ ctx->pub = value;
+ ctx->pub_len = vlen;
+
+ return 0;
+}
+
+int tpm2_key_priv(void *context, size_t hdrlen,
+ unsigned char tag,
+ const void *value, size_t vlen)
+{
+ struct tpm2_key_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 +244,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;
@@ -146,8 +314,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);
@@ -158,6 +328,8 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
else
rc = -EPERM;
}
+ if (payload->blob_len < 0)
+ return payload->blob_len;
return rc;
}
@@ -184,13 +356,34 @@ 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]);
- if (private_len > (payload->blob_len - 2))
+ rc = tpm2_key_decode(payload, options, &blob);
+ if (rc) {
+ /* old form */
+ blob = payload->blob;
+ payload->old_format = 1;
+ }
+
+ /* new format carries keyhandle but old format doesn't */
+ if (!options->keyhandle)
+ return -EINVAL;
+
+ /* must be big enough for at least the two be16 size counts */
+ if (payload->blob_len < 4)
+ return -EINVAL;
+
+ private_len = get_unaligned_be16(blob);
+
+ /* must be big enough for following public_len */
+ if (private_len + 2 + 2 > (payload->blob_len))
+ return -E2BIG;
+
+ public_len = get_unaligned_be16(blob + 2 + private_len);
+ if (private_len + 2 + public_len + 2 > payload->blob_len)
return -E2BIG;
- public_len = be16_to_cpup((__be16 *) &payload->blob[2 + private_len]);
blob_len = private_len + public_len + 4;
if (blob_len > payload->blob_len)
return -E2BIG;
@@ -206,7 +399,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;
@@ -219,6 +412,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.26.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH v9 5/8] security: keys: trusted: Make sealed key properly interoperable
2020-05-07 23:11 [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy James Bottomley
` (3 preceding siblings ...)
2020-05-07 23:11 ` [PATCH v9 4/8] security: keys: trusted: use ASN.1 TPM2 key format for the blobs James Bottomley
@ 2020-05-07 23:11 ` James Bottomley
2020-05-07 23:11 ` [PATCH v9 6/8] security: keys: trusted: add PCR policy to TPM2 keys James Bottomley
` (3 subsequent siblings)
8 siblings, 0 replies; 33+ messages in thread
From: James Bottomley @ 2020-05-07 23:11 UTC (permalink / raw)
To: linux-integrity
Cc: Mimi Zohar, Jarkko Sakkinen, David Woodhouse, keyrings, David Howells
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>
---
v2: added length checks to untrusted payload
v8: recover patch
---
include/linux/tpm.h | 2 +
security/keys/trusted-keys/trusted_tpm2.c | 53 ++++++++++++++++-------
2 files changed, 40 insertions(+), 15 deletions(-)
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 e8dcc47b3388..905c5ca4d51c 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -231,6 +231,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;
@@ -259,31 +260,32 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
TPM_DIGEST_SIZE);
/* sensitive */
- tpm_buf_append_u16(&buf, 4 + options->blobauth_len + payload->key_len + 1);
+ tpm_buf_append_u16(&buf, 4 + options->blobauth_len + payload->key_len);
tpm_buf_append_u16(&buf, options->blobauth_len);
if (options->blobauth_len)
tpm_buf_append(&buf, options->blobauth, options->blobauth_len);
- 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);
@@ -356,8 +358,9 @@ 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) {
@@ -384,6 +387,16 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
if (private_len + 2 + public_len + 2 > payload->blob_len)
return -E2BIG;
+ 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;
@@ -464,7 +477,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;
}
@@ -475,9 +488,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.26.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH v9 6/8] security: keys: trusted: add PCR policy to TPM2 keys
2020-05-07 23:11 [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy James Bottomley
` (4 preceding siblings ...)
2020-05-07 23:11 ` [PATCH v9 5/8] security: keys: trusted: Make sealed key properly interoperable James Bottomley
@ 2020-05-07 23:11 ` James Bottomley
2020-05-07 23:11 ` [PATCH v9 7/8] security: keys: trusted: add ability to specify arbitrary policy James Bottomley
` (2 subsequent siblings)
8 siblings, 0 replies; 33+ messages in thread
From: James Bottomley @ 2020-05-07 23:11 UTC (permalink / raw)
To: linux-integrity
Cc: Mimi Zohar, Jarkko Sakkinen, David Woodhouse, keyrings, David Howells
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 add trusted kmk "new 32 keyhandle=0x81000001 hash=sha1 pcrinfo=030000016768033e216468247bd031a0a2d9876d79818f8f" @u
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
v2: fix for new ASN.1 API eliminating hack in place and check lengths
v8: recover patch and fix offsets for hash and size
v9: fix cut and paste error in commit log
---
.../security/keys/trusted-encrypted.rst | 19 +-
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 | 372 ++++++++++++++++++
security/keys/trusted-keys/tpm2-policy.h | 30 ++
security/keys/trusted-keys/tpm2key.asn1 | 4 +-
security/keys/trusted-keys/trusted_tpm1.c | 7 +-
security/keys/trusted-keys/trusted_tpm2.c | 86 +++-
10 files changed, 516 insertions(+), 16 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..053344c4df5b 100644
--- a/Documentation/security/keys/trusted-encrypted.rst
+++ b/Documentation/security/keys/trusted-encrypted.rst
@@ -60,7 +60,10 @@ 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)
@@ -151,6 +154,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 b2d87ad21714..c117bf598230 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 ae0dd9228fd3..e19617087d96 100644
--- a/security/keys/Kconfig
+++ b/security/keys/Kconfig
@@ -76,6 +76,8 @@ config TRUSTED_KEYS
select CRYPTO
select CRYPTO_HMAC
select CRYPTO_SHA1
+ select CRYPTO_SHA256
+ select CRYPTO_SHA512
select CRYPTO_HASH_INFO
select ASN1_ENCODER
help
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..080a76e879fd
--- /dev/null
+++ b/security/keys/trusted-keys/tpm2-policy.c
@@ -0,0 +1,372 @@
+// 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 tpm2_key_code(void *context, size_t hdrlen,
+ unsigned char tag,
+ const void *value, size_t vlen)
+{
+ struct tpm2_key_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 tpm2_key_policy(void *context, size_t hdrlen,
+ unsigned char tag,
+ const void *value, size_t vlen)
+{
+ struct tpm2_key_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;
+}
+
+/**
+ * tpm2_key_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 tpm2_key_policy_process(struct tpm2_key_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[4]);
+ /* and the digest appears after the attributes */
+ pols->hash_size = get_unaligned_be16(&ctx->pub[10]);
+
+ 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)
+{
+ const int SCRATCH_SIZE = PAGE_SIZE;
+ u8 *buf = kmalloc(2 * SCRATCH_SIZE, GFP_KERNEL);
+ u8 *work = buf + SCRATCH_SIZE;
+ u8 *ptr;
+ u8 *end_work = work + SCRATCH_SIZE;
+ int i, ret;
+
+ if (!buf)
+ return -ENOMEM;
+
+ for (i = 0; i < pols->count; i++) {
+ u8 *seq, *tag;
+ u32 cmd = pols->code[i];
+
+ if (WARN(work - buf + 14 + pols->len[i] > 2 * SCRATCH_SIZE,
+ "BUG: scratch buffer is too small"))
+ return -EINVAL;
+
+ work = asn1_encode_sequence(work, end_work, NULL, -1);
+ seq = work;
+
+ work = asn1_encode_tag(work, end_work, 0, NULL, -1);
+ tag = work;
+
+ work = asn1_encode_integer(work, end_work, cmd);
+ asn1_encode_tag(tag, end_work, 0, NULL, work - tag);
+
+ work = asn1_encode_tag(work, end_work, 1, NULL, -1);
+ tag = work;
+
+ work = asn1_encode_octet_string(work, end_work,
+ pols->policies[i],
+ pols->len[i]);
+
+ asn1_encode_tag(tag, end_work, 1, NULL, work - tag);
+
+ seq = asn1_encode_sequence(seq, end_work, NULL, work - seq);
+ if (IS_ERR(seq)) {
+ ret = PTR_ERR(seq);
+ goto err;
+ }
+ }
+ ptr = asn1_encode_sequence(buf, buf + SCRATCH_SIZE, buf + PAGE_SIZE,
+ work - buf - PAGE_SIZE);
+ if (IS_ERR(ptr)) {
+ ret = PTR_ERR(ptr);
+ goto err;
+ }
+
+ *data = buf;
+ *len = ptr - buf;
+
+ return 0;
+
+ err:
+ kfree(buf);
+ return ret;
+}
+
+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..46bf1f0a9325
--- /dev/null
+++ b/security/keys/trusted-keys/tpm2-policy.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+struct tpm2_key_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 tpm2_key_policy_process(struct tpm2_key_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 660b3fa917ae..93e442d713fc 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 ({tpm2_key_code}),
+ commandPolicy [1] EXPLICIT OCTET STRING ({tpm2_key_policy})
}
diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
index 4a09165d61cd..892b18275b0c 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -1065,6 +1065,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);
}
@@ -1174,7 +1175,11 @@ static long trusted_read(const struct key *key, char *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 905c5ca4d51c..98c65431ca75 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},
@@ -62,6 +63,21 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
work = asn1_encode_tag(work, end_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)
+ return ret;
+
+ work = asn1_encode_tag(work, end_work, 1, encoded_pols,
+ encoded_pol_len);
+ kfree(encoded_pols);
+ }
+
/*
* Assume both octet strings will encode to a 2 byte definite length
*
@@ -85,14 +101,6 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
return work1 - payload->blob;
}
-struct tpm2_key_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)
@@ -115,6 +123,12 @@ static int tpm2_key_decode(struct trusted_key_payload *payload,
if (!blob)
return -ENOMEM;
+ ret = tpm2_key_policy_process(&ctx, payload);
+ if (ret) {
+ kfree(blob);
+ return ret;
+ }
+
*buf = blob;
options->keyhandle = ctx.parent;
@@ -248,6 +262,42 @@ 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 (options->policydigest_len != 0 && payload->policies) {
+ /* can't specify both a digest and a policy */
+ return -EINVAL;
+ }
+
+ 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;
@@ -456,21 +506,37 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
u16 data_len;
u8 *data;
int rc;
+ u32 policyhandle;
+
+ if (payload->policies && options->policyhandle)
+ /* can't have both a passed in policy and a key resident one */
+ return -EINVAL;
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;
+ } else {
+ policyhandle = options->policyhandle;
+ }
+
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 */,
options->blobauth_len);
rc = tpm_send(chip, buf.data, tpm_buf_length(&buf));
+ if (payload->policies)
+ tpm2_flush_context(chip, policyhandle);
if (rc > 0)
rc = -EPERM;
--
2.26.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH v9 7/8] security: keys: trusted: add ability to specify arbitrary policy
2020-05-07 23:11 [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy James Bottomley
` (5 preceding siblings ...)
2020-05-07 23:11 ` [PATCH v9 6/8] security: keys: trusted: add PCR policy to TPM2 keys James Bottomley
@ 2020-05-07 23:11 ` James Bottomley
2020-05-07 23:11 ` [PATCH v9 8/8] security: keys: trusted: implement counter/timer policy James Bottomley
2020-05-14 14:31 ` [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy Jarkko Sakkinen
8 siblings, 0 replies; 33+ messages in thread
From: James Bottomley @ 2020-05-07 23:11 UTC (permalink / raw)
To: linux-integrity
Cc: Mimi Zohar, Jarkko Sakkinen, David Woodhouse, keyrings, David Howells
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>
---
.../security/keys/trusted-encrypted.rst | 16 ++++++
security/keys/trusted-keys/tpm2-policy.c | 53 +++++++++++++++++++
security/keys/trusted-keys/tpm2-policy.h | 1 +
security/keys/trusted-keys/trusted_tpm1.c | 15 ++++++
4 files changed, 85 insertions(+)
diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst
index 053344c4df5b..b68d3eb73f00 100644
--- a/Documentation/security/keys/trusted-encrypted.rst
+++ b/Documentation/security/keys/trusted-encrypted.rst
@@ -76,6 +76,9 @@ Usage::
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.
+ 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.
@@ -168,6 +171,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 080a76e879fd..87a13e607eca 100644
--- a/security/keys/trusted-keys/tpm2-policy.c
+++ b/security/keys/trusted-keys/tpm2-policy.c
@@ -370,3 +370,56 @@ 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 46bf1f0a9325..0da013116c1c 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 892b18275b0c..cd2a44fcef98 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;
@@ -709,6 +711,7 @@ enum {
Opt_hash,
Opt_policydigest,
Opt_policyhandle,
+ Opt_policy,
};
static const match_table_t key_tokens = {
@@ -724,6 +727,7 @@ static const match_table_t key_tokens = {
{Opt_hash, "hash=%s"},
{Opt_policydigest, "policydigest=%s"},
{Opt_policyhandle, "policyhandle=%s"},
+ {Opt_policy, "policy=%s"},
{Opt_err, NULL}
};
@@ -854,6 +858,17 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
return -EINVAL;
opt->policyhandle = handle;
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.26.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH v9 8/8] security: keys: trusted: implement counter/timer policy
2020-05-07 23:11 [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy James Bottomley
` (6 preceding siblings ...)
2020-05-07 23:11 ` [PATCH v9 7/8] security: keys: trusted: add ability to specify arbitrary policy James Bottomley
@ 2020-05-07 23:11 ` James Bottomley
2020-05-14 14:31 ` [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy Jarkko Sakkinen
8 siblings, 0 replies; 33+ messages in thread
From: James Bottomley @ 2020-05-07 23:11 UTC (permalink / raw)
To: linux-integrity
Cc: Mimi Zohar, Jarkko Sakkinen, David Woodhouse, keyrings, David Howells
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>
---
.../security/keys/trusted-encrypted.rst | 29 ++++++++++++++
include/linux/tpm.h | 1 +
security/keys/trusted-keys/tpm2-policy.c | 40 ++++++++++++++++++-
security/keys/trusted-keys/trusted_tpm2.c | 36 ++++++++++++++++-
4 files changed, 104 insertions(+), 2 deletions(-)
diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst
index b68d3eb73f00..53a6196c7df9 100644
--- a/Documentation/security/keys/trusted-encrypted.rst
+++ b/Documentation/security/keys/trusted-encrypted.rst
@@ -241,3 +241,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/include/linux/tpm.h b/include/linux/tpm.h
index e32e9728adce..5026a06977e1 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -233,6 +233,7 @@ enum tpm2_command_codes {
TPM2_CC_PCR_EXTEND = 0x0182,
TPM2_CC_EVENT_SEQUENCE_COMPLETE = 0x0185,
TPM2_CC_HASH_SEQUENCE_START = 0x0186,
+ TPM2_CC_POLICY_PASSWORD = 0x018c,
TPM2_CC_CREATE_LOADED = 0x0191,
TPM2_CC_LAST = 0x0193, /* Spec 1.36 */
};
diff --git a/security/keys/trusted-keys/tpm2-policy.c b/security/keys/trusted-keys/tpm2-policy.c
index 87a13e607eca..bd8eb02e1094 100644
--- a/security/keys/trusted-keys/tpm2-policy.c
+++ b/security/keys/trusted-keys/tpm2-policy.c
@@ -197,7 +197,8 @@ int tpm2_generate_policy_digest(struct tpm2_policies *pols,
len = *plen;
}
- crypto_shash_update(sdesc, policy, len);
+ if (len)
+ crypto_shash_update(sdesc, policy, len);
/* now output the intermediate to the policydigest */
crypto_shash_final(sdesc, policydigest);
@@ -332,6 +333,16 @@ int tpm2_get_policy_session(struct tpm_chip *chip, struct tpm2_policies *pols,
u32 cmd = pols->code[i];
struct tpm_buf buf;
+ if (cmd == TPM2_CC_POLICY_AUTHVALUE)
+ /*
+ * both PolicyAuthValue and PolicyPassword
+ * hash to the same thing, but one triggers
+ * HMAC authentication and the other simple
+ * authentication. Since we have no HMAC
+ * code, we're choosing the simple
+ */
+ cmd = TPM2_CC_POLICY_PASSWORD;
+
rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, cmd);
if (rc)
return rc;
@@ -352,8 +363,35 @@ 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";
+ if (pols->len[i])
+ tpm_buf_append(&buf, pols->policies[i],
+ pols->len[i]);
+
break;
}
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 98c65431ca75..3ec01ed874d9 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -248,6 +248,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
u32 flags;
int i;
int rc;
+ static const int POLICY_SIZE = 2 * PAGE_SIZE;
for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
if (options->hash == tpm2_hash_map[i].crypto_id) {
@@ -268,7 +269,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
/* 4 array len, 2 hash alg */
const int len = 4 + 2 + options->pcrinfo_len;
- pols = kmalloc(sizeof(*pols) + len, GFP_KERNEL);
+ pols = kmalloc(POLICY_SIZE, GFP_KERNEL);
if (!pols)
return -ENOMEM;
@@ -289,6 +290,39 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
return -EINVAL;
}
+ /*
+ * if we already have a policy, we have to add authorization
+ * to it. If we don't, we can simply follow the usual
+ * non-policy route.
+ */
+ if (options->blobauth_len != 0 && payload->policies) {
+ struct tpm2_policies *pols;
+ static u8 *scratch;
+ int i;
+ bool found = false;
+
+ pols = payload->policies;
+
+ /* make sure it's not already in policy */
+ for (i = 0; i < pols->count; i++) {
+ if (pols->code[i] == TPM2_CC_POLICY_AUTHVALUE) {
+ found = true;
+
+ break;
+ }
+ }
+
+ if (!found) {
+ i = pols->count++;
+ scratch = pols->policies[i - 1] + pols->len[i - 1];
+
+ /* the TPM2_PolicyPassword command has no payload */
+ pols->policies[i] = scratch;
+ pols->len[i] = 0;
+ pols->code[i] = TPM2_CC_POLICY_AUTHVALUE;
+ }
+ }
+
if (payload->policies) {
rc = tpm2_generate_policy_digest(payload->policies,
options->hash,
--
2.26.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH v9 3/8] security: keys: trusted: fix TPM2 authorizations
2020-05-07 23:11 ` [PATCH v9 3/8] security: keys: trusted: fix TPM2 authorizations James Bottomley
@ 2020-05-14 1:11 ` Jarkko Sakkinen
2020-05-14 1:12 ` Jarkko Sakkinen
0 siblings, 1 reply; 33+ messages in thread
From: Jarkko Sakkinen @ 2020-05-14 1:11 UTC (permalink / raw)
To: James Bottomley, linux-integrity
Cc: Mimi Zohar, David Woodhouse, keyrings, David Howells
On Thu, 2020-05-07 at 16:11 -0700, James Bottomley wrote:
> In TPM 1.2 an authorization was a 20 byte number. The spec actually
> recommended you to hash variable length passwords and use the sha1
> hash as the authorization. Because the spec doesn't require this
> hashing, the current authorization for trusted keys is a 40 digit hex
> number. For TPM 2.0 the spec allows the passing in of variable length
> passwords and passphrases directly, so we should allow that in trusted
> keys for ease of use. Update the 'blobauth' parameter to take this
> into account, so we can now use plain text passwords for the keys.
>
> so before
>
> keyctl add trusted kmk "new 32 blobauth=f572d396fae9206628714fb2ce00f72e94f2258f"
>
> after we will accept both the old hex sha1 form as well as a new
> directly supplied password:
>
> keyctl add trusted kmk "new 32 blobauth=hello keyhandle=81000001"
>
> Since a sha1 hex code must be exactly 40 bytes long and a direct
> password must be 20 or less, we use the length as the discriminator
> for which form is input.
>
> Note this is both and enhancement and a potential bug fix. The TPM
> 2.0 spec requires us to strip leading zeros, meaning empyty
> authorization is a zero length HMAC whereas we're currently passing in
> 20 bytes of zeros. A lot of TPMs simply accept this as OK, but the
> Microsoft TPM emulator rejects it with TPM_RC_BAD_AUTH, so this patch
> makes the Microsoft TPM emulator work with trusted keys.
>
> Fixes: 0fe5480303a1 ("keys, trusted: seal/unseal with TPM 2.0 chips")
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Have not checked yet the tail. Probably won't check before PR for v5.8
is out.
Just wondering would it hurt to merge everything up until this patch?
/Jarkko
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v9 3/8] security: keys: trusted: fix TPM2 authorizations
2020-05-14 1:11 ` Jarkko Sakkinen
@ 2020-05-14 1:12 ` Jarkko Sakkinen
2020-05-14 1:41 ` James Bottomley
0 siblings, 1 reply; 33+ messages in thread
From: Jarkko Sakkinen @ 2020-05-14 1:12 UTC (permalink / raw)
To: James Bottomley, linux-integrity
Cc: Mimi Zohar, David Woodhouse, keyrings, David Howells
On Thu, 2020-05-14 at 04:11 +0300, Jarkko Sakkinen wrote:
> On Thu, 2020-05-07 at 16:11 -0700, James Bottomley wrote:
> > In TPM 1.2 an authorization was a 20 byte number. The spec actually
> > recommended you to hash variable length passwords and use the sha1
> > hash as the authorization. Because the spec doesn't require this
> > hashing, the current authorization for trusted keys is a 40 digit hex
> > number. For TPM 2.0 the spec allows the passing in of variable length
> > passwords and passphrases directly, so we should allow that in trusted
> > keys for ease of use. Update the 'blobauth' parameter to take this
> > into account, so we can now use plain text passwords for the keys.
> >
> > so before
> >
> > keyctl add trusted kmk "new 32 blobauth=f572d396fae9206628714fb2ce00f72e94f2258f"
> >
> > after we will accept both the old hex sha1 form as well as a new
> > directly supplied password:
> >
> > keyctl add trusted kmk "new 32 blobauth=hello keyhandle=81000001"
> >
> > Since a sha1 hex code must be exactly 40 bytes long and a direct
> > password must be 20 or less, we use the length as the discriminator
> > for which form is input.
> >
> > Note this is both and enhancement and a potential bug fix. The TPM
> > 2.0 spec requires us to strip leading zeros, meaning empyty
> > authorization is a zero length HMAC whereas we're currently passing in
> > 20 bytes of zeros. A lot of TPMs simply accept this as OK, but the
> > Microsoft TPM emulator rejects it with TPM_RC_BAD_AUTH, so this patch
> > makes the Microsoft TPM emulator work with trusted keys.
> >
> > Fixes: 0fe5480303a1 ("keys, trusted: seal/unseal with TPM 2.0 chips")
> > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>
> Have not checked yet the tail. Probably won't check before PR for v5.8
> is out.
>
> Just wondering would it hurt to merge everything up until this patch?
I.e. could land it also to the release.
/Jarkko
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v9 3/8] security: keys: trusted: fix TPM2 authorizations
2020-05-14 1:12 ` Jarkko Sakkinen
@ 2020-05-14 1:41 ` James Bottomley
2020-05-14 11:19 ` Jarkko Sakkinen
0 siblings, 1 reply; 33+ messages in thread
From: James Bottomley @ 2020-05-14 1:41 UTC (permalink / raw)
To: Jarkko Sakkinen, linux-integrity
Cc: Mimi Zohar, David Woodhouse, keyrings, David Howells
On Thu, 2020-05-14 at 04:12 +0300, Jarkko Sakkinen wrote:
> On Thu, 2020-05-14 at 04:11 +0300, Jarkko Sakkinen wrote:
> > On Thu, 2020-05-07 at 16:11 -0700, James Bottomley wrote:
> > > In TPM 1.2 an authorization was a 20 byte number. The spec
> > > actually recommended you to hash variable length passwords and
> > > use the sha1 hash as the authorization. Because the spec doesn't
> > > require this hashing, the current authorization for trusted keys
> > > is a 40 digit hex number. For TPM 2.0 the spec allows the
> > > passing in of variable length passwords and passphrases directly,
> > > so we should allow that in trusted keys for ease of use. Update
> > > the 'blobauth' parameter to take this into account, so we can now
> > > use plain text passwords for the keys.
> > >
> > > so before
> > >
> > > keyctl add trusted kmk "new 32
> > > blobauth=f572d396fae9206628714fb2ce00f72e94f2258f"
> > >
> > > after we will accept both the old hex sha1 form as well as a new
> > > directly supplied password:
> > >
> > > keyctl add trusted kmk "new 32 blobauth=hello keyhandle=81000001"
> > >
> > > Since a sha1 hex code must be exactly 40 bytes long and a direct
> > > password must be 20 or less, we use the length as the
> > > discriminator for which form is input.
> > >
> > > Note this is both and enhancement and a potential bug fix. The
> > > TPM 2.0 spec requires us to strip leading zeros, meaning empyty
> > > authorization is a zero length HMAC whereas we're currently
> > > passing in 20 bytes of zeros. A lot of TPMs simply accept this
> > > as OK, but the Microsoft TPM emulator rejects it with
> > > TPM_RC_BAD_AUTH, so this patch makes the Microsoft TPM emulator
> > > work with trusted keys.
> > >
> > > Fixes: 0fe5480303a1 ("keys, trusted: seal/unseal with TPM 2.0
> > > chips")
> > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership
> > > .com>
> > > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> >
> > Have not checked yet the tail. Probably won't check before PR for
> > v5.8 is out.
> >
> > Just wondering would it hurt to merge everything up until this
> > patch?
Everything would be OK if you applied 1, 2 and 3. Except we'd have an
ASN.1 API in the tree with no consumers, which excites some people.
> I.e. could land it also to the release.
That would likely be fine and should satisfy the API with no consumers
issue.
James
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v9 3/8] security: keys: trusted: fix TPM2 authorizations
2020-05-14 1:41 ` James Bottomley
@ 2020-05-14 11:19 ` Jarkko Sakkinen
0 siblings, 0 replies; 33+ messages in thread
From: Jarkko Sakkinen @ 2020-05-14 11:19 UTC (permalink / raw)
To: James Bottomley, linux-integrity
Cc: Mimi Zohar, David Woodhouse, keyrings, David Howells
On Wed, 2020-05-13 at 18:41 -0700, James Bottomley wrote:
> On Thu, 2020-05-14 at 04:12 +0300, Jarkko Sakkinen wrote:
> > On Thu, 2020-05-14 at 04:11 +0300, Jarkko Sakkinen wrote:
> > > On Thu, 2020-05-07 at 16:11 -0700, James Bottomley wrote:
> > > > In TPM 1.2 an authorization was a 20 byte number. The spec
> > > > actually recommended you to hash variable length passwords and
> > > > use the sha1 hash as the authorization. Because the spec doesn't
> > > > require this hashing, the current authorization for trusted keys
> > > > is a 40 digit hex number. For TPM 2.0 the spec allows the
> > > > passing in of variable length passwords and passphrases directly,
> > > > so we should allow that in trusted keys for ease of use. Update
> > > > the 'blobauth' parameter to take this into account, so we can now
> > > > use plain text passwords for the keys.
> > > >
> > > > so before
> > > >
> > > > keyctl add trusted kmk "new 32
> > > > blobauth=f572d396fae9206628714fb2ce00f72e94f2258f"
> > > >
> > > > after we will accept both the old hex sha1 form as well as a new
> > > > directly supplied password:
> > > >
> > > > keyctl add trusted kmk "new 32 blobauth=hello keyhandle=81000001"
> > > >
> > > > Since a sha1 hex code must be exactly 40 bytes long and a direct
> > > > password must be 20 or less, we use the length as the
> > > > discriminator for which form is input.
> > > >
> > > > Note this is both and enhancement and a potential bug fix. The
> > > > TPM 2.0 spec requires us to strip leading zeros, meaning empyty
> > > > authorization is a zero length HMAC whereas we're currently
> > > > passing in 20 bytes of zeros. A lot of TPMs simply accept this
> > > > as OK, but the Microsoft TPM emulator rejects it with
> > > > TPM_RC_BAD_AUTH, so this patch makes the Microsoft TPM emulator
> > > > work with trusted keys.
> > > >
> > > > Fixes: 0fe5480303a1 ("keys, trusted: seal/unseal with TPM 2.0
> > > > chips")
> > > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership
> > > > .com>
> > > > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > >
> > > Have not checked yet the tail. Probably won't check before PR for
> > > v5.8 is out.
> > >
> > > Just wondering would it hurt to merge everything up until this
> > > patch?
>
> Everything would be OK if you applied 1, 2 and 3. Except we'd have an
> ASN.1 API in the tree with no consumers, which excites some people.
>
> > I.e. could land it also to the release.
>
> That would likely be fine and should satisfy the API with no consumers
> issue.
Hmm. Right, it does not sense to merge unused API.
I'd like to still pick this patch (3/8) but you need to fix these first:
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#17:
keyctl add trusted kmk "new 32 blobauth=f572d396fae9206628714fb2ce00f72e94f2258f"
WARNING: line over 80 characters
#89: FILE: security/keys/trusted-keys/trusted_tpm1.c:801:
+ if (tpm2 && opt->blobauth_len <= sizeof(opt->blobauth)) {
WARNING: line over 80 characters
#111: FILE: security/keys/trusted-keys/trusted_tpm2.c:94:
+ tpm_buf_append_u16(&buf, 4 + options->blobauth_len + payload->key_len + 1);
The best way is probably to encapsulate into helper function. It is more or
less a sign that the code is too complicated to live inside a switch-case
statement.
Can you do that and send it as a separate patch?
/Jarkko
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy
2020-05-07 23:11 [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy James Bottomley
` (7 preceding siblings ...)
2020-05-07 23:11 ` [PATCH v9 8/8] security: keys: trusted: implement counter/timer policy James Bottomley
@ 2020-05-14 14:31 ` Jarkko Sakkinen
2020-05-15 2:22 ` Jarkko Sakkinen
8 siblings, 1 reply; 33+ messages in thread
From: Jarkko Sakkinen @ 2020-05-14 14:31 UTC (permalink / raw)
To: James Bottomley, linux-integrity
Cc: Mimi Zohar, David Woodhouse, keyrings, David Howells
On Thu, 2020-05-07 at 16:11 -0700, James Bottomley wrote:
> This is a respin on v8 to make the encoder selectable and address
> David's comments. The trusted key part hasn't changed except to add a
> now necessary select for ASN1_ENCODER to patch 4 and the changelog of
> patch 6 has been updated to correct the cut and paste error in the
> keyctl statement.
>
> General cover letter:
>
> This patch updates the trusted key code to export keys in the ASN.1
> format used by current TPM key tools (openssl_tpm2_engine and
> openconnect). It also simplifies the use of policy with keys because
> the ASN.1 format is designed to carry a description of how to
> construct the policy, with the result that simple policies (like
> authorization and PCR locking) can now be constructed and used in the
> kernel, bringing the TPM 2.0 policy use into line with how TPM 1.2
> works.
>
> The key format is designed to be compatible with our two openssl
> engine implementations as well as with the format used by openconnect.
> I've added seal/unseal to my engine so I can use it for
> interoperability testing and I'll later use this for sealed symmetric
> keys via engine:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/jejb/openssl_tpm2_engine.git/
>
> James
I'm compiling now kernel with all series included.
Kind of checking if I could just take the whole series. Let see.
In all cases I want the style errors in 3/8 to be fixes with a helper
but maybe better to hold before sending anything. Possibly that is all
needed I'll just carve that patch myself.
Please don't do anything for the moment.
/Jarkko
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy
2020-05-14 14:31 ` [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy Jarkko Sakkinen
@ 2020-05-15 2:22 ` Jarkko Sakkinen
2020-05-15 3:44 ` James Bottomley
0 siblings, 1 reply; 33+ messages in thread
From: Jarkko Sakkinen @ 2020-05-15 2:22 UTC (permalink / raw)
To: James Bottomley, linux-integrity
Cc: Mimi Zohar, David Woodhouse, keyrings, David Howells
On Thu, 2020-05-14 at 17:31 +0300, Jarkko Sakkinen wrote:
> I'm compiling now kernel with all series included.
>
> Kind of checking if I could just take the whole series. Let see.
>
> In all cases I want the style errors in 3/8 to be fixes with a helper
> but maybe better to hold before sending anything. Possibly that is all
> needed I'll just carve that patch myself.
>
> Please don't do anything for the moment.
This is what I tried first (with the full series applied):
#!/bin/sh
die()
{
keyctl clear @u
./tpm2-flush --all-transient
exit $1
}
KEYHANDLE=$(./tpm2-root-key || die 1)
KEYID=$(keyctl add trusted kmk "new 32 keyhandle=$KEYHANDLE hash=sha256" @u || die 1)
echo "$KEYID ($KEYHANDLE)"
keyctl pipe $KEYID > blob.hex || die 1
keyctl clear @u || die 1
echo "Import key from blob"
keyctl add trusted kmk "load `cat blob.hex` keyhandle=$KEYHANDLE" @u || die 1
die 0
Result:
sudo ./keyctl-smoke.sh
566201053 (0x80000000)
keyctl_read_alloc: Permission denied
Any ideas what I might have done wrong? Have not tried auth value yet
but afaik the above should fully test import and export.
Uses tpm2-scripts:
https://github.com/jsakkine-intel/tpm2-scripts
I'll probably move these to git.infradead.org because I don't like
really like at all Github and my kernel tree is there anyway.
/Jarkko
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy
2020-05-15 2:22 ` Jarkko Sakkinen
@ 2020-05-15 3:44 ` James Bottomley
2020-05-15 8:47 ` Jarkko Sakkinen
0 siblings, 1 reply; 33+ messages in thread
From: James Bottomley @ 2020-05-15 3:44 UTC (permalink / raw)
To: Jarkko Sakkinen, linux-integrity
Cc: Mimi Zohar, David Woodhouse, keyrings, David Howells
On Fri, 2020-05-15 at 05:22 +0300, Jarkko Sakkinen wrote:
> On Thu, 2020-05-14 at 17:31 +0300, Jarkko Sakkinen wrote:
> > I'm compiling now kernel with all series included.
> >
> > Kind of checking if I could just take the whole series. Let see.
> >
> > In all cases I want the style errors in 3/8 to be fixes with a
> > helper
> > but maybe better to hold before sending anything. Possibly that is
> > all
> > needed I'll just carve that patch myself.
> >
> > Please don't do anything for the moment.
>
> This is what I tried first (with the full series applied):
>
> #!/bin/sh
>
> die()
> {
> keyctl clear @u
> ./tpm2-flush --all-transient
> exit $1
> }
>
> KEYHANDLE=$(./tpm2-root-key || die 1)
> KEYID=$(keyctl add trusted kmk "new 32 keyhandle=$KEYHANDLE
> hash=sha256" @u || die 1)
>
> echo "$KEYID ($KEYHANDLE)"
>
> keyctl pipe $KEYID > blob.hex || die 1
> keyctl clear @u || die 1
>
> echo "Import key from blob"
>
> keyctl add trusted kmk "load `cat blob.hex` keyhandle=$KEYHANDLE" @u
> || die 1
>
> die 0
>
> Result:
>
> sudo ./keyctl-smoke.sh
> 566201053 (0x80000000)
> keyctl_read_alloc: Permission denied
Well, it's clearly failing in keyctl pipe
I do confess to never having tested a volatile primary, but I just did
so and it works for me. I will also add the keyhandle in the load
isn't necessary, because it should be in the blob, but there should
also be no harm (just tested).
However, I don't have keyctl_read_alloc in my tree, so it may be an
incompatibility with another patch set. What's your base and what
other patches do you have applied?
James
> Any ideas what I might have done wrong? Have not tried auth value yet
> but afaik the above should fully test import and export.
>
> Uses tpm2-scripts:
>
> https://github.com/jsakkine-intel/tpm2-scripts
>
> I'll probably move these to git.infradead.org because I don't like
> really like at all Github and my kernel tree is there anyway.
>
> /Jarkko
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy
2020-05-15 3:44 ` James Bottomley
@ 2020-05-15 8:47 ` Jarkko Sakkinen
2020-05-15 9:30 ` Jerry Snitselaar
2020-05-15 19:17 ` Jerry Snitselaar
0 siblings, 2 replies; 33+ messages in thread
From: Jarkko Sakkinen @ 2020-05-15 8:47 UTC (permalink / raw)
To: James Bottomley
Cc: linux-integrity, Mimi Zohar, David Woodhouse, keyrings, David Howells
On Thu, May 14, 2020 at 08:44:23PM -0700, James Bottomley wrote:
> On Fri, 2020-05-15 at 05:22 +0300, Jarkko Sakkinen wrote:
> > On Thu, 2020-05-14 at 17:31 +0300, Jarkko Sakkinen wrote:
> > > I'm compiling now kernel with all series included.
> > >
> > > Kind of checking if I could just take the whole series. Let see.
> > >
> > > In all cases I want the style errors in 3/8 to be fixes with a
> > > helper
> > > but maybe better to hold before sending anything. Possibly that is
> > > all
> > > needed I'll just carve that patch myself.
> > >
> > > Please don't do anything for the moment.
> >
> > This is what I tried first (with the full series applied):
> >
> > #!/bin/sh
> >
> > die()
> > {
> > keyctl clear @u
> > ./tpm2-flush --all-transient
> > exit $1
> > }
> >
> > KEYHANDLE=$(./tpm2-root-key || die 1)
> > KEYID=$(keyctl add trusted kmk "new 32 keyhandle=$KEYHANDLE
> > hash=sha256" @u || die 1)
> >
> > echo "$KEYID ($KEYHANDLE)"
> >
> > keyctl pipe $KEYID > blob.hex || die 1
> > keyctl clear @u || die 1
> >
> > echo "Import key from blob"
> >
> > keyctl add trusted kmk "load `cat blob.hex` keyhandle=$KEYHANDLE" @u
> > || die 1
> >
> > die 0
> >
> > Result:
> >
> > sudo ./keyctl-smoke.sh
> > 566201053 (0x80000000)
> > keyctl_read_alloc: Permission denied
>
> Well, it's clearly failing in keyctl pipe
>
> I do confess to never having tested a volatile primary, but I just did
> so and it works for me. I will also add the keyhandle in the load
> isn't necessary, because it should be in the blob, but there should
> also be no harm (just tested).
>
> However, I don't have keyctl_read_alloc in my tree, so it may be an
> incompatibility with another patch set. What's your base and what
> other patches do you have applied?
http://git.infradead.org/users/jjs/linux-tpmdd.git
Or exactly:
git://git.infradead.org/users/jjs/linux-tpmdd.git (master)
/Jarkko
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy
2020-05-15 8:47 ` Jarkko Sakkinen
@ 2020-05-15 9:30 ` Jerry Snitselaar
2020-05-15 18:48 ` James Bottomley
2020-05-16 9:59 ` Jarkko Sakkinen
2020-05-15 19:17 ` Jerry Snitselaar
1 sibling, 2 replies; 33+ messages in thread
From: Jerry Snitselaar @ 2020-05-15 9:30 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: James Bottomley, linux-integrity, Mimi Zohar, David Woodhouse,
keyrings, David Howells
On Fri May 15 20, Jarkko Sakkinen wrote:
>On Thu, May 14, 2020 at 08:44:23PM -0700, James Bottomley wrote:
>> On Fri, 2020-05-15 at 05:22 +0300, Jarkko Sakkinen wrote:
>> > On Thu, 2020-05-14 at 17:31 +0300, Jarkko Sakkinen wrote:
>> > > I'm compiling now kernel with all series included.
>> > >
>> > > Kind of checking if I could just take the whole series. Let see.
>> > >
>> > > In all cases I want the style errors in 3/8 to be fixes with a
>> > > helper
>> > > but maybe better to hold before sending anything. Possibly that is
>> > > all
>> > > needed I'll just carve that patch myself.
>> > >
>> > > Please don't do anything for the moment.
>> >
>> > This is what I tried first (with the full series applied):
>> >
>> > #!/bin/sh
>> >
>> > die()
>> > {
>> > keyctl clear @u
>> > ./tpm2-flush --all-transient
>> > exit $1
>> > }
>> >
>> > KEYHANDLE=$(./tpm2-root-key || die 1)
>> > KEYID=$(keyctl add trusted kmk "new 32 keyhandle=$KEYHANDLE
>> > hash=sha256" @u || die 1)
>> >
>> > echo "$KEYID ($KEYHANDLE)"
>> >
>> > keyctl pipe $KEYID > blob.hex || die 1
>> > keyctl clear @u || die 1
>> >
>> > echo "Import key from blob"
>> >
>> > keyctl add trusted kmk "load `cat blob.hex` keyhandle=$KEYHANDLE" @u
>> > || die 1
>> >
>> > die 0
>> >
>> > Result:
>> >
>> > sudo ./keyctl-smoke.sh
>> > 566201053 (0x80000000)
>> > keyctl_read_alloc: Permission denied
>>
>> Well, it's clearly failing in keyctl pipe
>>
>> I do confess to never having tested a volatile primary, but I just did
>> so and it works for me. I will also add the keyhandle in the load
>> isn't necessary, because it should be in the blob, but there should
>> also be no harm (just tested).
>>
>> However, I don't have keyctl_read_alloc in my tree, so it may be an
>> incompatibility with another patch set. What's your base and what
>> other patches do you have applied?
>
>http://git.infradead.org/users/jjs/linux-tpmdd.git
>
>Or exactly:
>
>git://git.infradead.org/users/jjs/linux-tpmdd.git (master)
>
>/Jarkko
>
keyctl_read_alloc is in the userspace keyctl program, right?
git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/keyutils.git
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy
2020-05-15 9:30 ` Jerry Snitselaar
@ 2020-05-15 18:48 ` James Bottomley
2020-05-16 12:24 ` Jarkko Sakkinen
2020-05-16 9:59 ` Jarkko Sakkinen
1 sibling, 1 reply; 33+ messages in thread
From: James Bottomley @ 2020-05-15 18:48 UTC (permalink / raw)
To: Jerry Snitselaar, Jarkko Sakkinen
Cc: linux-integrity, Mimi Zohar, David Woodhouse, keyrings, David Howells
On Fri, 2020-05-15 at 02:30 -0700, Jerry Snitselaar wrote:
> On Fri May 15 20, Jarkko Sakkinen wrote:
> > On Thu, May 14, 2020 at 08:44:23PM -0700, James Bottomley wrote:
[...]
> > > However, I don't have keyctl_read_alloc in my tree, so it may be
> > > an incompatibility with another patch set. What's your base and
> > > what other patches do you have applied?
> >
> > http://git.infradead.org/users/jjs/linux-tpmdd.git
> >
> > Or exactly:
> >
> > git://git.infradead.org/users/jjs/linux-tpmdd.git (master)
> >
> > /Jarkko
> >
>
> keyctl_read_alloc is in the userspace keyctl program, right?
>
> git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/keyutils.git
Hm, right thanks! I just confirmed that linux-tpmdd.git with these
patches applied still works for me. I'm using the keyctl in debian
testing, which identifies itself as version 1.6-6
I'll try building from git.
James
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy
2020-05-15 8:47 ` Jarkko Sakkinen
2020-05-15 9:30 ` Jerry Snitselaar
@ 2020-05-15 19:17 ` Jerry Snitselaar
2020-05-15 19:34 ` James Bottomley
2020-05-16 12:33 ` Jarkko Sakkinen
1 sibling, 2 replies; 33+ messages in thread
From: Jerry Snitselaar @ 2020-05-15 19:17 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: James Bottomley, linux-integrity, Mimi Zohar, David Woodhouse,
keyrings, David Howells
On Fri May 15 20, Jarkko Sakkinen wrote:
>On Thu, May 14, 2020 at 08:44:23PM -0700, James Bottomley wrote:
>> On Fri, 2020-05-15 at 05:22 +0300, Jarkko Sakkinen wrote:
>> > On Thu, 2020-05-14 at 17:31 +0300, Jarkko Sakkinen wrote:
>> > > I'm compiling now kernel with all series included.
>> > >
>> > > Kind of checking if I could just take the whole series. Let see.
>> > >
>> > > In all cases I want the style errors in 3/8 to be fixes with a
>> > > helper
>> > > but maybe better to hold before sending anything. Possibly that is
>> > > all
>> > > needed I'll just carve that patch myself.
>> > >
>> > > Please don't do anything for the moment.
>> >
>> > This is what I tried first (with the full series applied):
>> >
>> > #!/bin/sh
>> >
>> > die()
>> > {
>> > keyctl clear @u
>> > ./tpm2-flush --all-transient
>> > exit $1
>> > }
>> >
>> > KEYHANDLE=$(./tpm2-root-key || die 1)
>> > KEYID=$(keyctl add trusted kmk "new 32 keyhandle=$KEYHANDLE
>> > hash=sha256" @u || die 1)
>> >
>> > echo "$KEYID ($KEYHANDLE)"
>> >
>> > keyctl pipe $KEYID > blob.hex || die 1
>> > keyctl clear @u || die 1
>> >
>> > echo "Import key from blob"
>> >
>> > keyctl add trusted kmk "load `cat blob.hex` keyhandle=$KEYHANDLE" @u
>> > || die 1
>> >
>> > die 0
>> >
>> > Result:
>> >
>> > sudo ./keyctl-smoke.sh
>> > 566201053 (0x80000000)
>> > keyctl_read_alloc: Permission denied
I get keyctl_read_alloc -EPERM when I 'sudo su' and try to play with keyctl print.
If I 'sudo su -' and then try it works as expected. Also works for normal user.
>>
>> Well, it's clearly failing in keyctl pipe
>>
>> I do confess to never having tested a volatile primary, but I just did
>> so and it works for me. I will also add the keyhandle in the load
>> isn't necessary, because it should be in the blob, but there should
>> also be no harm (just tested).
>>
>> However, I don't have keyctl_read_alloc in my tree, so it may be an
>> incompatibility with another patch set. What's your base and what
>> other patches do you have applied?
>
>http://git.infradead.org/users/jjs/linux-tpmdd.git
>
>Or exactly:
>
>git://git.infradead.org/users/jjs/linux-tpmdd.git (master)
>
>/Jarkko
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy
2020-05-15 19:17 ` Jerry Snitselaar
@ 2020-05-15 19:34 ` James Bottomley
2020-05-15 19:50 ` Mimi Zohar
2020-05-15 20:10 ` James Bottomley
2020-05-16 12:33 ` Jarkko Sakkinen
1 sibling, 2 replies; 33+ messages in thread
From: James Bottomley @ 2020-05-15 19:34 UTC (permalink / raw)
To: Jerry Snitselaar, Jarkko Sakkinen
Cc: linux-integrity, Mimi Zohar, David Woodhouse, keyrings, David Howells
On Fri, 2020-05-15 at 12:17 -0700, Jerry Snitselaar wrote:
> On Fri May 15 20, Jarkko Sakkinen wrote:
> > On Thu, May 14, 2020 at 08:44:23PM -0700, James Bottomley wrote:
> > > On Fri, 2020-05-15 at 05:22 +0300, Jarkko Sakkinen wrote:
[...]
> > > > sudo ./keyctl-smoke.sh
> > > > 566201053 (0x80000000)
> > > > keyctl_read_alloc: Permission denied
>
> I get keyctl_read_alloc -EPERM when I 'sudo su' and try to play with
> keyctl print.
> If I 'sudo su -' and then try it works as expected. Also works for
> normal user.
OK, I confirm on debian as well. If I create a key as real root and
then try to sudo su keyctl pipe it as an ordinary user, I get EPERM.
It smells like a cockup in real vs effective permissions somewhere in
the keyctl handler.
James
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy
2020-05-15 19:34 ` James Bottomley
@ 2020-05-15 19:50 ` Mimi Zohar
2020-05-15 20:10 ` James Bottomley
1 sibling, 0 replies; 33+ messages in thread
From: Mimi Zohar @ 2020-05-15 19:50 UTC (permalink / raw)
To: James Bottomley, Jerry Snitselaar, Jarkko Sakkinen
Cc: linux-integrity, David Woodhouse, keyrings, David Howells
On Fri, 2020-05-15 at 12:34 -0700, James Bottomley wrote:
> On Fri, 2020-05-15 at 12:17 -0700, Jerry Snitselaar wrote:
> > On Fri May 15 20, Jarkko Sakkinen wrote:
> > > On Thu, May 14, 2020 at 08:44:23PM -0700, James Bottomley wrote:
> > > > On Fri, 2020-05-15 at 05:22 +0300, Jarkko Sakkinen wrote:
> [...]
> > > > > sudo ./keyctl-smoke.sh
> > > > > 566201053 (0x80000000)
> > > > > keyctl_read_alloc: Permission denied
> >
> > I get keyctl_read_alloc -EPERM when I 'sudo su' and try to play with
> > keyctl print.
> > If I 'sudo su -' and then try it works as expected. Also works for
> > normal user.
>
> OK, I confirm on debian as well. If I create a key as real root and
> then try to sudo su keyctl pipe it as an ordinary user, I get EPERM.
>
> It smells like a cockup in real vs effective permissions somewhere in
> the keyctl handler.
Doing "sudo su -" has always been required. "su -" must set some
environment variables. This isn't a problem for dracut as it is
running as root.
Mimi
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy
2020-05-15 19:34 ` James Bottomley
2020-05-15 19:50 ` Mimi Zohar
@ 2020-05-15 20:10 ` James Bottomley
2020-05-15 21:03 ` Kayaalp, Mehmet
2020-05-16 13:01 ` Sakkinen, Jarkko
1 sibling, 2 replies; 33+ messages in thread
From: James Bottomley @ 2020-05-15 20:10 UTC (permalink / raw)
To: Jerry Snitselaar, Jarkko Sakkinen
Cc: linux-integrity, Mimi Zohar, David Woodhouse, keyrings, David Howells
On Fri, 2020-05-15 at 12:34 -0700, James Bottomley wrote:
> On Fri, 2020-05-15 at 12:17 -0700, Jerry Snitselaar wrote:
> > On Fri May 15 20, Jarkko Sakkinen wrote:
> > > On Thu, May 14, 2020 at 08:44:23PM -0700, James Bottomley wrote:
> > > > On Fri, 2020-05-15 at 05:22 +0300, Jarkko Sakkinen wrote:
>
> [...]
> > > > > sudo ./keyctl-smoke.sh
> > > > > 566201053 (0x80000000)
> > > > > keyctl_read_alloc: Permission denied
> >
> > I get keyctl_read_alloc -EPERM when I 'sudo su' and try to play
> > with
> > keyctl print.
> > If I 'sudo su -' and then try it works as expected. Also works for
> > normal user.
>
> OK, I confirm on debian as well. If I create a key as real root and
> then try to sudo su keyctl pipe it as an ordinary user, I get EPERM.
>
> It smells like a cockup in real vs effective permissions somewhere in
> the keyctl handler.
OK, so the problem is
sudo keyctl list @s
Still shows the session keys of the previous user
that causes sudo keyctl show on a root owned key to fail the
is_key_possessed() check, returning -EACCESS which gets translated to
EPERM
if you do
sudo su -
Then keyctl list @s shows the root session keyring and everything works
I think that means the solution is not to run the smoke test under sudo
but to do sudo -s and then run it.
James
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy
2020-05-15 20:10 ` James Bottomley
@ 2020-05-15 21:03 ` Kayaalp, Mehmet
2020-05-15 22:19 ` James Bottomley
2020-05-16 13:01 ` Sakkinen, Jarkko
1 sibling, 1 reply; 33+ messages in thread
From: Kayaalp, Mehmet @ 2020-05-15 21:03 UTC (permalink / raw)
To: James Bottomley
Cc: Jerry Snitselaar, Jarkko Sakkinen, linux-integrity, Mimi Zohar,
David Woodhouse, keyrings, David Howells
> On May 15, 2020, at 4:10 PM, James Bottomley <James.Bottomley@hansenpartnership.com> wrote:
>
> I think that means the solution is not to run the smoke test under sudo
> but to do sudo -s and then run it.
>
> James
How about "sudo -i":
https://askubuntu.com/questions/376199/sudo-su-vs-sudo-i-vs-sudo-bin-bash-when-does-it-matter-which-is-used
Mehmet
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy
2020-05-15 21:03 ` Kayaalp, Mehmet
@ 2020-05-15 22:19 ` James Bottomley
2020-05-15 23:23 ` James Bottomley
0 siblings, 1 reply; 33+ messages in thread
From: James Bottomley @ 2020-05-15 22:19 UTC (permalink / raw)
To: Kayaalp, Mehmet
Cc: Jerry Snitselaar, Jarkko Sakkinen, linux-integrity, Mimi Zohar,
David Woodhouse, keyrings, David Howells
On Fri, 2020-05-15 at 21:03 +0000, Kayaalp, Mehmet wrote:
> > On May 15, 2020, at 4:10 PM, James Bottomley <James.Bottomley@hanse
> > npartnership.com> wrote:
> >
> > I think that means the solution is not to run the smoke test under
> > sudo
> > but to do sudo -s and then run it.
> >
> > James
>
> How about "sudo -i":
>
> https://askubuntu.com/questions/376199/sudo-su-vs-sudo-i-vs-sudo-bin-
> bash-when-does-it-matter-which-is-used
Actually, no that doesn't work either:
jejb@testdeb> sudo -i keyctl list @s
1 key in keyring:
1041514063: ---lswrv 1000 65534 keyring: _uid.1000
I suspect this might be a very subtle bug to do with when you get a new
session keyring.
James
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy
2020-05-15 22:19 ` James Bottomley
@ 2020-05-15 23:23 ` James Bottomley
2020-05-16 21:44 ` Jarkko Sakkinen
0 siblings, 1 reply; 33+ messages in thread
From: James Bottomley @ 2020-05-15 23:23 UTC (permalink / raw)
To: Kayaalp, Mehmet
Cc: Jerry Snitselaar, Jarkko Sakkinen, linux-integrity, Mimi Zohar,
David Woodhouse, keyrings, David Howells
On Fri, 2020-05-15 at 15:19 -0700, James Bottomley wrote:
> On Fri, 2020-05-15 at 21:03 +0000, Kayaalp, Mehmet wrote:
> > > On May 15, 2020, at 4:10 PM, James Bottomley <James.Bottomley@han
> > > se
> > > npartnership.com> wrote:
> > >
> > > I think that means the solution is not to run the smoke test
> > > under sudo but to do sudo -s and then run it.
> > >
> > > James
> >
> > How about "sudo -i":
> >
> > https://askubuntu.com/questions/376199/sudo-su-vs-sudo-i-vs-sudo-
> > bin-bash-when-does-it-matter-which-is-used
>
> Actually, no that doesn't work either:
>
> jejb@testdeb> sudo -i keyctl list @s
> 1 key in keyring:
> 1041514063: ---lswrv 1000 65534 keyring: _uid.1000
>
> I suspect this might be a very subtle bug to do with when you get a
> new session keyring.
It turns out to be incredibly subtle, but I'm not sure if it's a bug or
not. the util-linux sudo like commands have special pam profiles
/etc/pam.d/su-l
/etc/pam.d/runuser-l
These special profiles call pam_keyinit.so with flags to install a new
session keyring. sudo doesn't have this, so it never, on its own, even
when called with -i or -s, installs a new session keyring. This does
strike me as a bizarre oversight which leads directly to this weird
keyctl pipe behaviour.
I'm also not sure the keyctl pipe behaviour is correct: why should
keyctl pipe deny access to root to read a key just because it's in a
different session keyring? It defintely seems intentional, because if
I create a key as a non root user and try to print it by id as root I
get EPERM.
The weirdest behaviour, though seems to be keyctl: keyctl add ... @u
will add to the session keyrings of the actual uid regardless of what
session keyring the creator actually has, which is why keyctl add ...
@u works under sudo but you get EPERM back trying to pipe it by id.
James
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy
2020-05-15 9:30 ` Jerry Snitselaar
2020-05-15 18:48 ` James Bottomley
@ 2020-05-16 9:59 ` Jarkko Sakkinen
1 sibling, 0 replies; 33+ messages in thread
From: Jarkko Sakkinen @ 2020-05-16 9:59 UTC (permalink / raw)
To: Jerry Snitselaar
Cc: James Bottomley, linux-integrity, Mimi Zohar, David Woodhouse,
keyrings, David Howells
On Fri, 2020-05-15 at 02:30 -0700, Jerry Snitselaar wrote:
> On Fri May 15 20, Jarkko Sakkinen wrote:
> > On Thu, May 14, 2020 at 08:44:23PM -0700, James Bottomley wrote:
> > > On Fri, 2020-05-15 at 05:22 +0300, Jarkko Sakkinen wrote:
> > > > On Thu, 2020-05-14 at 17:31 +0300, Jarkko Sakkinen wrote:
> > > > > I'm compiling now kernel with all series included.
> > > > >
> > > > > Kind of checking if I could just take the whole series. Let see.
> > > > >
> > > > > In all cases I want the style errors in 3/8 to be fixes with a
> > > > > helper
> > > > > but maybe better to hold before sending anything. Possibly that is
> > > > > all
> > > > > needed I'll just carve that patch myself.
> > > > >
> > > > > Please don't do anything for the moment.
> > > >
> > > > This is what I tried first (with the full series applied):
> > > >
> > > > #!/bin/sh
> > > >
> > > > die()
> > > > {
> > > > keyctl clear @u
> > > > ./tpm2-flush --all-transient
> > > > exit $1
> > > > }
> > > >
> > > > KEYHANDLE=$(./tpm2-root-key || die 1)
> > > > KEYID=$(keyctl add trusted kmk "new 32 keyhandle=$KEYHANDLE
> > > > hash=sha256" @u || die 1)
> > > >
> > > > echo "$KEYID ($KEYHANDLE)"
> > > >
> > > > keyctl pipe $KEYID > blob.hex || die 1
> > > > keyctl clear @u || die 1
> > > >
> > > > echo "Import key from blob"
> > > >
> > > > keyctl add trusted kmk "load `cat blob.hex` keyhandle=$KEYHANDLE" @u
> > > > > > die 1
> > > >
> > > > die 0
> > > >
> > > > Result:
> > > >
> > > > sudo ./keyctl-smoke.sh
> > > > 566201053 (0x80000000)
> > > > keyctl_read_alloc: Permission denied
> > >
> > > Well, it's clearly failing in keyctl pipe
> > >
> > > I do confess to never having tested a volatile primary, but I just did
> > > so and it works for me. I will also add the keyhandle in the load
> > > isn't necessary, because it should be in the blob, but there should
> > > also be no harm (just tested).
> > >
> > > However, I don't have keyctl_read_alloc in my tree, so it may be an
> > > incompatibility with another patch set. What's your base and what
> > > other patches do you have applied?
> >
> > http://git.infradead.org/users/jjs/linux-tpmdd.git
> >
> > Or exactly:
> >
> > git://git.infradead.org/users/jjs/linux-tpmdd.git (master)
> >
> > /Jarkko
> >
>
> keyctl_read_alloc is in the userspace keyctl program, right?
>
> git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/keyutils.git
Yes.
/Jarkko
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy
2020-05-15 18:48 ` James Bottomley
@ 2020-05-16 12:24 ` Jarkko Sakkinen
0 siblings, 0 replies; 33+ messages in thread
From: Jarkko Sakkinen @ 2020-05-16 12:24 UTC (permalink / raw)
To: James Bottomley, Jerry Snitselaar
Cc: linux-integrity, Mimi Zohar, David Woodhouse, keyrings, David Howells
On Fri, 2020-05-15 at 11:48 -0700, James Bottomley wrote:
> On Fri, 2020-05-15 at 02:30 -0700, Jerry Snitselaar wrote:
> > On Fri May 15 20, Jarkko Sakkinen wrote:
> > > On Thu, May 14, 2020 at 08:44:23PM -0700, James Bottomley wrote:
> [...]
> > > > However, I don't have keyctl_read_alloc in my tree, so it may be
> > > > an incompatibility with another patch set. What's your base and
> > > > what other patches do you have applied?
> > >
> > > http://git.infradead.org/users/jjs/linux-tpmdd.git
> > >
> > > Or exactly:
> > >
> > > git://git.infradead.org/users/jjs/linux-tpmdd.git (master)
> > >
> > > /Jarkko
> > >
> >
> > keyctl_read_alloc is in the userspace keyctl program, right?
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/keyutils.git
>
> Hm, right thanks! I just confirmed that linux-tpmdd.git with these
> patches applied still works for me. I'm using the keyctl in debian
> testing, which identifies itself as version 1.6-6
>
> I'll try building from git.
> James
Can you run the same exact command sequence that I posted?
/Jarkko
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy
2020-05-15 19:17 ` Jerry Snitselaar
2020-05-15 19:34 ` James Bottomley
@ 2020-05-16 12:33 ` Jarkko Sakkinen
1 sibling, 0 replies; 33+ messages in thread
From: Jarkko Sakkinen @ 2020-05-16 12:33 UTC (permalink / raw)
To: Jerry Snitselaar
Cc: James Bottomley, linux-integrity, Mimi Zohar, David Woodhouse,
keyrings, David Howells
On Fri, 2020-05-15 at 12:17 -0700, Jerry Snitselaar wrote:
> > > > sudo ./keyctl-smoke.sh
> > > > 566201053 (0x80000000)
> > > > keyctl_read_alloc: Permission denied
>
> I get keyctl_read_alloc -EPERM when I 'sudo su' and try to play with keyctl print.
> If I 'sudo su -' and then try it works as expected. Also works for normal user.
Can confirm these results with NUC7PJYH (Gemini Lake).
/Jarkko
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy
2020-05-15 20:10 ` James Bottomley
2020-05-15 21:03 ` Kayaalp, Mehmet
@ 2020-05-16 13:01 ` Sakkinen, Jarkko
1 sibling, 0 replies; 33+ messages in thread
From: Sakkinen, Jarkko @ 2020-05-16 13:01 UTC (permalink / raw)
To: jsnitsel, James.Bottomley
Cc: dhowells, keyrings, linux-integrity, zohar, dwmw2
On Fri, 2020-05-15 at 13:10 -0700, James Bottomley wrote:
> On Fri, 2020-05-15 at 12:34 -0700, James Bottomley wrote:
> > On Fri, 2020-05-15 at 12:17 -0700, Jerry Snitselaar wrote:
> > > On Fri May 15 20, Jarkko Sakkinen wrote:
> > > > On Thu, May 14, 2020 at 08:44:23PM -0700, James Bottomley wrote:
> > > > > On Fri, 2020-05-15 at 05:22 +0300, Jarkko Sakkinen wrote:
> >
> > [...]
> > > > > > sudo ./keyctl-smoke.sh
> > > > > > 566201053 (0x80000000)
> > > > > > keyctl_read_alloc: Permission denied
> > >
> > > I get keyctl_read_alloc -EPERM when I 'sudo su' and try to play
> > > with
> > > keyctl print.
> > > If I 'sudo su -' and then try it works as expected. Also works for
> > > normal user.
> >
> > OK, I confirm on debian as well. If I create a key as real root and
> > then try to sudo su keyctl pipe it as an ordinary user, I get EPERM.
> >
> > It smells like a cockup in real vs effective permissions somewhere in
> > the keyctl handler.
>
> OK, so the problem is
>
> sudo keyctl list @s
>
> Still shows the session keys of the previous user
>
> that causes sudo keyctl show on a root owned key to fail the
> is_key_possessed() check, returning -EACCESS which gets translated to
> EPERM
>
> if you do
>
> sudo su -
>
> Then keyctl list @s shows the root session keyring and everything works
>
> I think that means the solution is not to run the smoke test under sudo
> but to do sudo -s and then run it.
Right, makes sense and I can also confirm this in my environment.
Thanks!
/Jarkko
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy
2020-05-15 23:23 ` James Bottomley
@ 2020-05-16 21:44 ` Jarkko Sakkinen
0 siblings, 0 replies; 33+ messages in thread
From: Jarkko Sakkinen @ 2020-05-16 21:44 UTC (permalink / raw)
To: James Bottomley, Kayaalp, Mehmet
Cc: Jerry Snitselaar, linux-integrity, Mimi Zohar, David Woodhouse,
keyrings, David Howells
On Fri, 2020-05-15 at 16:23 -0700, James Bottomley wrote:
> On Fri, 2020-05-15 at 15:19 -0700, James Bottomley wrote:
> > On Fri, 2020-05-15 at 21:03 +0000, Kayaalp, Mehmet wrote:
> > > > On May 15, 2020, at 4:10 PM, James Bottomley <James.Bottomley@han
> > > > se
> > > > npartnership.com> wrote:
> > > >
> > > > I think that means the solution is not to run the smoke test
> > > > under sudo but to do sudo -s and then run it.
> > > >
> > > > James
> > >
> > > How about "sudo -i":
> > >
> > > https://askubuntu.com/questions/376199/sudo-su-vs-sudo-i-vs-sudo-
> > > bin-bash-when-does-it-matter-which-is-used
> >
> > Actually, no that doesn't work either:
> >
> > jejb@testdeb> sudo -i keyctl list @s
> > 1 key in keyring:
> > 1041514063: ---lswrv 1000 65534 keyring: _uid.1000
> >
> > I suspect this might be a very subtle bug to do with when you get a
> > new session keyring.
>
> It turns out to be incredibly subtle, but I'm not sure if it's a bug or
> not. the util-linux sudo like commands have special pam profiles
>
> /etc/pam.d/su-l
> /etc/pam.d/runuser-l
>
> These special profiles call pam_keyinit.so with flags to install a new
> session keyring. sudo doesn't have this, so it never, on its own, even
> when called with -i or -s, installs a new session keyring. This does
> strike me as a bizarre oversight which leads directly to this weird
> keyctl pipe behaviour.
>
> I'm also not sure the keyctl pipe behaviour is correct: why should
> keyctl pipe deny access to root to read a key just because it's in a
> different session keyring? It defintely seems intentional, because if
> I create a key as a non root user and try to print it by id as root I
> get EPERM.
>
> The weirdest behaviour, though seems to be keyctl: keyctl add ... @u
> will add to the session keyrings of the actual uid regardless of what
> session keyring the creator actually has, which is why keyctl add ...
> @u works under sudo but you get EPERM back trying to pipe it by id.
>
> James
I think I construct a low priority bug to kernel bugzilla and link these
from l.k.o. Thanks for digging this all up.
/Jarkko
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v9 1/8] lib: add ASN.1 encoder
2020-05-07 23:11 ` [PATCH v9 1/8] lib: add ASN.1 encoder James Bottomley
@ 2020-05-17 8:17 ` Sakkinen, Jarkko
0 siblings, 0 replies; 33+ messages in thread
From: Sakkinen, Jarkko @ 2020-05-17 8:17 UTC (permalink / raw)
To: James.Bottomley, linux-integrity
Cc: jarkko.sakkinen, dhowells, keyrings, zohar, dwmw2
On Thu, 2020-05-07 at 16:11 -0700, James Bottomley wrote:
> We have a need in the TPM2 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. The specific tools are the openssl_tpm2_engine, openconnect
> and the Intel tpm2-tss-engine. To do that, we have to be able to read
> and write the same binary key format the tools use. The current ASN.1
> decoder does fine for reading, but we need pieces of an ASN.1 encoder
> to write the key blob in binary compatible form.
>
> For backwards compatibility, the trusted key reader code will still
> accept the two TPM2B quantities that it uses today, but the writer
> will only output the ASN.1 form.
>
> The current implementation only encodes the ASN.1 bits we actually need.
>
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
/Jarkko
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v9 4/8] security: keys: trusted: use ASN.1 TPM2 key format for the blobs
2020-05-07 23:11 ` [PATCH v9 4/8] security: keys: trusted: use ASN.1 TPM2 key format for the blobs James Bottomley
@ 2020-05-17 8:18 ` Sakkinen, Jarkko
0 siblings, 0 replies; 33+ messages in thread
From: Sakkinen, Jarkko @ 2020-05-17 8:18 UTC (permalink / raw)
To: James.Bottomley, linux-integrity
Cc: jarkko.sakkinen, dhowells, keyrings, zohar, dwmw2
On Thu, 2020-05-07 at 16:11 -0700, 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.
>
> The benefit of the ASN.1 format is that it's a standard and thus the
> exported key can be used by userspace tools (openssl_tpm2_engine,
> openconnect and tpm2-tss-engine). 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>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
/Jarkko
^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2020-05-17 8:18 UTC | newest]
Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-07 23:11 [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy James Bottomley
2020-05-07 23:11 ` [PATCH v9 1/8] lib: add ASN.1 encoder James Bottomley
2020-05-17 8:17 ` Sakkinen, Jarkko
2020-05-07 23:11 ` [PATCH v9 2/8] oid_registry: Add TCG defined OIDS for TPM keys James Bottomley
2020-05-07 23:11 ` [PATCH v9 3/8] security: keys: trusted: fix TPM2 authorizations James Bottomley
2020-05-14 1:11 ` Jarkko Sakkinen
2020-05-14 1:12 ` Jarkko Sakkinen
2020-05-14 1:41 ` James Bottomley
2020-05-14 11:19 ` Jarkko Sakkinen
2020-05-07 23:11 ` [PATCH v9 4/8] security: keys: trusted: use ASN.1 TPM2 key format for the blobs James Bottomley
2020-05-17 8:18 ` Sakkinen, Jarkko
2020-05-07 23:11 ` [PATCH v9 5/8] security: keys: trusted: Make sealed key properly interoperable James Bottomley
2020-05-07 23:11 ` [PATCH v9 6/8] security: keys: trusted: add PCR policy to TPM2 keys James Bottomley
2020-05-07 23:11 ` [PATCH v9 7/8] security: keys: trusted: add ability to specify arbitrary policy James Bottomley
2020-05-07 23:11 ` [PATCH v9 8/8] security: keys: trusted: implement counter/timer policy James Bottomley
2020-05-14 14:31 ` [PATCH v9 0/8] TPM 2.0 trusted keys with attached policy Jarkko Sakkinen
2020-05-15 2:22 ` Jarkko Sakkinen
2020-05-15 3:44 ` James Bottomley
2020-05-15 8:47 ` Jarkko Sakkinen
2020-05-15 9:30 ` Jerry Snitselaar
2020-05-15 18:48 ` James Bottomley
2020-05-16 12:24 ` Jarkko Sakkinen
2020-05-16 9:59 ` Jarkko Sakkinen
2020-05-15 19:17 ` Jerry Snitselaar
2020-05-15 19:34 ` James Bottomley
2020-05-15 19:50 ` Mimi Zohar
2020-05-15 20:10 ` James Bottomley
2020-05-15 21:03 ` Kayaalp, Mehmet
2020-05-15 22:19 ` James Bottomley
2020-05-15 23:23 ` James Bottomley
2020-05-16 21:44 ` Jarkko Sakkinen
2020-05-16 13:01 ` Sakkinen, Jarkko
2020-05-16 12:33 ` Jarkko Sakkinen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).