Testing
=======
RSA
---
tpm2_createprimary --hierarchy o -G rsa2048 -c owner.txt
tpm2_evictcontrol -c owner.txt 0x81000001
tpm2_getcap handles-persistent
openssl genrsa -out private.pem 2048
tpm2_import -C 0x81000001 -G rsa -i private.pem -u key.pub -r key.priv
tpm2_encodeobject -C 0x81000001 -u key.pub -r key.priv -o key.priv.pem
openssl asn1parse -inform pem -in key.priv.pem -noout -out key.priv.der
serial=`cat key.priv.der | keyctl padd asymmetric tpm @u`
echo "abcdefg" > plaintext.txt
keyctl pkey_encrypt $serial 0 plaintext.txt enc=pkcs1 > encrypted.dat
keyctl pkey_decrypt $serial 0 encrypted.dat enc=pkcs1 > decrypted.dat
keyctl pkey_sign $serial 0 plaintext.txt enc=pkcs1 hash=sha256 > signed.dat
keyctl pkey_verify $serial 0 plaintext.txt signed.dat enc=pkcs1 hash=sha256
ECDSA
-----
tpm2_createprimary --hierarchy o -G ecc -c owner.txt
tpm2_evictcontrol -c owner.txt 0x81000001
openssl ecparam -name prime256v1 -genkey -noout -out private.pem
tpm2_import -C 0x81000001 -G ecc -i private.pem -u key.pub -r key.priv
tpm2_encodeobject -C 0x81000001 -u key.pub -r key.priv -o key.priv.pem
openssl asn1parse -inform pem -in key.priv.pem -noout -out key.priv.der
serial=`cat key.priv.der | keyctl padd asymmetric tpm @u`
echo "abcdefg" > plaintext.txt
keyctl pkey_sign $serial 0 plaintext.txt hash=sha256 > signed.dat
keyctl pkey_verify $serial 0 plaintext.txt signed.dat hash=sha256
Closed Issues
=============
* When verifying ECDSA signature, _ecdsa_verify() returns -EKEYREJECTED.
* v7: rewrote the signature encoder with a more structured layout.
References
==========
* v6: https://lore.kernel.org/linux-integrity/[email protected]/
* v5: https://lore.kernel.org/linux-integrity/[email protected]/
* v4: https://lore.kernel.org/linux-integrity/[email protected]/
* v3: https://lore.kernel.org/linux-integrity/[email protected]/
* v2: https://lore.kernel.org/linux-integrity/[email protected]/
* v1: https://lore.kernel.org/linux-integrity/[email protected]/
* Derived from https://lore.kernel.org/all/[email protected]/
Jarkko Sakkinen (5):
crypto: rsa-pkcs1pad: export rsa1_asn_lookup()
KEYS: trusted: Change -EINVAL to -E2BIG
crypto: tpm2_key: Introduce a TPM2 key type
keys: asymmetric: Add tpm2_key_rsa
keys: asymmetric: Add tpm2_key_ecdsa
crypto/Kconfig | 7 +
crypto/Makefile | 6 +
crypto/asymmetric_keys/Kconfig | 30 +
crypto/asymmetric_keys/Makefile | 2 +
crypto/asymmetric_keys/tpm2_key_ecdsa.c | 462 +++++++++++++++
crypto/asymmetric_keys/tpm2_key_rsa.c | 678 ++++++++++++++++++++++
crypto/ecdsa.c | 1 -
crypto/rsa-pkcs1pad.c | 16 +-
crypto/tpm2_key.asn1 | 11 +
crypto/tpm2_key.c | 134 +++++
drivers/char/tpm/tpm-buf.c | 2 +-
include/crypto/rsa-pkcs1pad.h | 20 +
include/crypto/tpm2_key.h | 46 ++
include/linux/tpm.h | 9 +
security/keys/trusted-keys/Kconfig | 2 +-
security/keys/trusted-keys/Makefile | 2 -
security/keys/trusted-keys/tpm2key.asn1 | 11 -
security/keys/trusted-keys/trusted_tpm2.c | 141 +----
18 files changed, 1447 insertions(+), 133 deletions(-)
create mode 100644 crypto/asymmetric_keys/tpm2_key_ecdsa.c
create mode 100644 crypto/asymmetric_keys/tpm2_key_rsa.c
create mode 100644 crypto/tpm2_key.asn1
create mode 100644 crypto/tpm2_key.c
create mode 100644 include/crypto/rsa-pkcs1pad.h
create mode 100644 include/crypto/tpm2_key.h
delete mode 100644 security/keys/trusted-keys/tpm2key.asn1
--
2.45.1
Report -E2BIG instead of -EINVAL when too large size for the key blob is
requested.
Signed-off-by: Jarkko Sakkinen <[email protected]>
---
security/keys/trusted-keys/trusted_tpm2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 8b7dd73d94c1..06c8fa7b21ae 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -122,7 +122,7 @@ static int tpm2_key_decode(struct trusted_key_payload *payload,
return ret;
if (ctx.priv_len + ctx.pub_len > MAX_BLOB_SIZE)
- return -EINVAL;
+ return -E2BIG;
blob = kmalloc(ctx.priv_len + ctx.pub_len + 4, GFP_KERNEL);
if (!blob)
--
2.45.1
TPM2 ASN.1 format is required for trusted keys and asymmetric keys. Move it
to crypto in order to make it available for both. Implement validation with
coverage of all TPMT_PUBLIC shared fields. Key type specific fields must be
covered by the different subsystems using this.
A Kconfig option CRYPTO_TPM2_KEY can be used to select the feature, which
depends only crypto subsystem itself and ASN.1 parser.
Signed-off-by: Jarkko Sakkinen <[email protected]>
---
v6:
* Relocate to crypto. Validate the shared part and provide
accessor functions. Use a fixed buffer size.
v2:
* Do not allocate blob twice. Use the one inside struct tpm2_key.
---
crypto/Kconfig | 7 ++
crypto/Makefile | 6 +
crypto/tpm2_key.asn1 | 11 ++
crypto/tpm2_key.c | 134 ++++++++++++++++++++
include/crypto/tpm2_key.h | 46 +++++++
security/keys/trusted-keys/Kconfig | 2 +-
security/keys/trusted-keys/Makefile | 2 -
security/keys/trusted-keys/tpm2key.asn1 | 11 --
security/keys/trusted-keys/trusted_tpm2.c | 141 +++++-----------------
9 files changed, 235 insertions(+), 125 deletions(-)
create mode 100644 crypto/tpm2_key.asn1
create mode 100644 crypto/tpm2_key.c
create mode 100644 include/crypto/tpm2_key.h
delete mode 100644 security/keys/trusted-keys/tpm2key.asn1
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 5688d42a59c2..c8989bc71f57 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -5,6 +5,13 @@
config XOR_BLOCKS
tristate
+config CRYPTO_TPM2_KEY
+ bool
+ depends on CRYPTO
+ select ASN1
+ select OID_REGISTRY
+ default n
+
#
# async_tx api: hardware offloaded memory transfer/transform support
#
diff --git a/crypto/Makefile b/crypto/Makefile
index edbbaa3ffef5..d932fdb72319 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -216,3 +216,9 @@ obj-$(CONFIG_CRYPTO_SIMD) += crypto_simd.o
# Key derivation function
#
obj-$(CONFIG_CRYPTO_KDF800108_CTR) += kdf_sp800108.o
+
+ifdef CONFIG_CRYPTO_TPM2_KEY
+$(obj)/tpm2_key.asn1.o: $(obj)/tpm2_key.asn1.h $(obj)/tpm2_key.asn1.c
+$(obj)/tpm2_key.o: $(obj)/tpm2_key.asn1.h
+obj-y += tpm2_key.o tpm2_key.asn1.o
+endif
diff --git a/crypto/tpm2_key.asn1 b/crypto/tpm2_key.asn1
new file mode 100644
index 000000000000..b235d02ab78e
--- /dev/null
+++ b/crypto/tpm2_key.asn1
@@ -0,0 +1,11 @@
+---
+--- ASN.1 for TPM 2.0 keys
+---
+
+TPMKey ::= SEQUENCE {
+ type OBJECT IDENTIFIER ({tpm2_key_get_type}),
+ emptyAuth [0] EXPLICIT BOOLEAN OPTIONAL,
+ parent INTEGER ({tpm2_key_get_parent}),
+ pubkey OCTET STRING ({tpm2_get_public}),
+ privkey OCTET STRING ({tpm2_get_private})
+ }
diff --git a/crypto/tpm2_key.c b/crypto/tpm2_key.c
new file mode 100644
index 000000000000..78f55478d046
--- /dev/null
+++ b/crypto/tpm2_key.c
@@ -0,0 +1,134 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <crypto/tpm2_key.h>
+#include <linux/oid_registry.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <asm/unaligned.h>
+#include "tpm2_key.asn1.h"
+
+#undef pr_fmt
+#define pr_fmt(fmt) "tpm2_key: "fmt
+
+struct tpm2_key_decoder_context {
+ u32 parent;
+ const u8 *pub;
+ u32 pub_len;
+ const u8 *priv;
+ u32 priv_len;
+ enum OID oid;
+};
+
+int tpm2_key_get_parent(void *context, size_t hdrlen,
+ unsigned char tag,
+ const void *value, size_t vlen)
+{
+ struct tpm2_key_decoder_context *decoder = context;
+ const u8 *v = value;
+ int i;
+
+ decoder->parent = 0;
+ for (i = 0; i < vlen; i++) {
+ decoder->parent <<= 8;
+ decoder->parent |= v[i];
+ }
+
+ return 0;
+}
+
+int tpm2_key_get_type(void *context, size_t hdrlen,
+ unsigned char tag,
+ const void *value, size_t vlen)
+{
+ struct tpm2_key_decoder_context *decoder = context;
+
+ decoder->oid = look_up_OID(value, vlen);
+ return 0;
+}
+
+static inline bool tpm2_key_is_valid(const void *value, size_t vlen)
+{
+ if (vlen < 2 || vlen > TPM2_KEY_BYTES_MAX)
+ return false;
+
+ if (get_unaligned_be16(value) != vlen - 2)
+ return false;
+
+ return true;
+}
+
+int tpm2_get_public(void *context, size_t hdrlen, unsigned char tag,
+ const void *value, size_t vlen)
+{
+ struct tpm2_key_decoder_context *decoder = context;
+
+ if (!tpm2_key_is_valid(value, vlen))
+ return -EBADMSG;
+
+ if (sizeof(struct tpm2_key_desc) > vlen - 2)
+ return -EBADMSG;
+
+ decoder->pub = value;
+ decoder->pub_len = vlen;
+ return 0;
+}
+
+int tpm2_get_private(void *context, size_t hdrlen, unsigned char tag,
+ const void *value, size_t vlen)
+{
+ struct tpm2_key_decoder_context *decoder = context;
+
+ if (!tpm2_key_is_valid(value, vlen))
+ return -EBADMSG;
+
+ decoder->priv = value;
+ decoder->priv_len = vlen;
+ return 0;
+}
+
+/**
+ * tpm_key_decode() - Decode TPM2 ASN.1 key
+ * @src: ASN.1 source.
+ * @src_len: ASN.1 source length.
+ *
+ * Decodes the TPM2 ASN.1 key and validates that the public key data has all
+ * the shared fields of TPMT_PUBLIC. This is full coverage of the memory that
+ * can be validated before doing any key type specific validation.
+ *
+ * Return:
+ * - TPM2 ASN.1 key on success.
+ * - -EBADMSG when decoding fails.
+ * - -ENOMEM when OOM while allocating struct tpm2_key.
+ */
+struct tpm2_key *tpm2_key_decode(const u8 *src, u32 src_len)
+{
+ struct tpm2_key_decoder_context decoder;
+ struct tpm2_key *key;
+ u8 *data;
+ int ret;
+
+ memset(&decoder, 0, sizeof(decoder));
+ ret = asn1_ber_decoder(&tpm2_key_decoder, &decoder, src, src_len);
+ if (ret < 0) {
+ if (ret != -EBADMSG)
+ pr_info("Decoder error %d\n", ret);
+
+ return ERR_PTR(-EBADMSG);
+ }
+
+ key = kzalloc(sizeof(*key), GFP_KERNEL);
+ if (!key)
+ return ERR_PTR(-ENOMEM);
+
+ data = &key->data[0];
+ memcpy(&data[0], decoder.priv, decoder.priv_len);
+ memcpy(&data[decoder.priv_len], decoder.pub, decoder.pub_len);
+
+ key->oid = decoder.oid;
+ key->priv_len = decoder.priv_len;
+ key->pub_len = decoder.pub_len;
+ key->parent = decoder.parent;
+ key->desc = (struct tpm2_key_desc *)&data[decoder.priv_len + 2];
+ return key;
+}
+EXPORT_SYMBOL_GPL(tpm2_key_decode);
diff --git a/include/crypto/tpm2_key.h b/include/crypto/tpm2_key.h
new file mode 100644
index 000000000000..74debaf707bf
--- /dev/null
+++ b/include/crypto/tpm2_key.h
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __LINUX_TPM2_KEY_H__
+#define __LINUX_TPM2_KEY_H__
+
+#include <linux/oid_registry.h>
+#include <linux/slab.h>
+
+#define TPM2_KEY_BYTES_MAX 1024
+
+/* TPM2 Structures 12.2.4: TPMT_PUBLIC */
+struct tpm2_key_desc {
+ __be16 type;
+ __be16 name_alg;
+ __be32 object_attributes;
+ __be16 policy_size;
+} __packed;
+
+/* Decoded TPM2 ASN.1 key. */
+struct tpm2_key {
+ u8 data[2 * TPM2_KEY_BYTES_MAX];
+ struct tpm2_key_desc *desc;
+ u16 priv_len;
+ u16 pub_len;
+ u32 parent;
+ enum OID oid;
+ char oid_str[64];
+};
+
+struct tpm2_key *tpm2_key_decode(const u8 *src, u32 src_len);
+
+static inline const void *tpm2_key_data(const struct tpm2_key *key)
+{
+ return &key->data[0];
+}
+
+static inline u16 tpm2_key_type(const struct tpm2_key *key)
+{
+ return be16_to_cpu(key->desc->type);
+}
+
+static inline int tpm2_key_policy_size(const struct tpm2_key *key)
+{
+ return be16_to_cpu(key->desc->policy_size);
+}
+
+#endif /* __LINUX_TPM2_KEY_H__ */
diff --git a/security/keys/trusted-keys/Kconfig b/security/keys/trusted-keys/Kconfig
index 1fb8aa001995..00d9489384ac 100644
--- a/security/keys/trusted-keys/Kconfig
+++ b/security/keys/trusted-keys/Kconfig
@@ -9,9 +9,9 @@ config TRUSTED_KEYS_TPM
select CRYPTO_HMAC
select CRYPTO_SHA1
select CRYPTO_HASH_INFO
+ select CRYPTO_TPM2_KEY
select ASN1_ENCODER
select OID_REGISTRY
- select ASN1
select HAVE_TRUSTED_KEYS
help
Enable use of the Trusted Platform Module (TPM) as trusted key
diff --git a/security/keys/trusted-keys/Makefile b/security/keys/trusted-keys/Makefile
index f0f3b27f688b..2674d5c10fc9 100644
--- a/security/keys/trusted-keys/Makefile
+++ b/security/keys/trusted-keys/Makefile
@@ -7,9 +7,7 @@ obj-$(CONFIG_TRUSTED_KEYS) += trusted.o
trusted-y += trusted_core.o
trusted-$(CONFIG_TRUSTED_KEYS_TPM) += trusted_tpm1.o
-$(obj)/trusted_tpm2.o: $(obj)/tpm2key.asn1.h
trusted-$(CONFIG_TRUSTED_KEYS_TPM) += trusted_tpm2.o
-trusted-$(CONFIG_TRUSTED_KEYS_TPM) += tpm2key.asn1.o
trusted-$(CONFIG_TRUSTED_KEYS_TEE) += trusted_tee.o
diff --git a/security/keys/trusted-keys/tpm2key.asn1 b/security/keys/trusted-keys/tpm2key.asn1
deleted file mode 100644
index f57f869ad600..000000000000
--- a/security/keys/trusted-keys/tpm2key.asn1
+++ /dev/null
@@ -1,11 +0,0 @@
----
---- ASN.1 for TPM 2.0 keys
----
-
-TPMKey ::= SEQUENCE {
- type OBJECT IDENTIFIER ({tpm2_key_type}),
- emptyAuth [0] EXPLICIT BOOLEAN OPTIONAL,
- parent INTEGER ({tpm2_key_parent}),
- pubkey OCTET STRING ({tpm2_key_pub}),
- privkey OCTET STRING ({tpm2_key_priv})
- }
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 06c8fa7b21ae..b9e505e99e8c 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -13,11 +13,10 @@
#include <keys/trusted-type.h>
#include <keys/trusted_tpm.h>
+#include <crypto/tpm2_key.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},
@@ -98,106 +97,6 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
return ret;
}
-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 -E2BIG;
-
- 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;
-}
-
/**
* tpm2_buf_append_auth() - append TPMS_AUTH_COMMAND to the buffer.
*
@@ -387,22 +286,43 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
struct trusted_key_options *options,
u32 *blob_handle)
{
- struct tpm_buf buf;
unsigned int private_len;
unsigned int public_len;
unsigned int blob_len;
- u8 *blob, *pub;
- int rc;
+ struct tpm2_key *key;
+ const u8 *blob, *pub;
+ struct tpm_buf buf;
u32 attrs;
+ int rc;
- rc = tpm2_key_decode(payload, options, &blob);
- if (rc) {
- /* old form */
+ key = tpm2_key_decode(payload->blob, payload->blob_len);
+ if (IS_ERR(key)) {
+ /* Get the error code and reset the pointer to the key: */
+ rc = PTR_ERR(key);
+ key = NULL;
+
+ if (rc == -ENOMEM)
+ return -ENOMEM;
+
+ /* A sanity check, as only -EBADMSG or -ENOMEM are expected: */
+ if (rc != -EBADMSG)
+ pr_err("tpm2_key_decode(): spurious error code %d\n", rc);
+
+ /* Fallback to the legacy format: */
blob = payload->blob;
payload->old_format = 1;
+ } else {
+ blob = tpm2_key_data(key);
+ if (key->oid != OID_TPMSealedData) {
+ kfree(key);
+ return -EBADMSG;
+ }
}
- /* new format carries keyhandle but old format doesn't */
+ /*
+ * Must be non-zero here, either extracted from the ASN.1 for the new
+ * format or specified on the command line for the old.
+ */
if (!options->keyhandle)
return -EINVAL;
@@ -464,8 +384,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
(__be32 *) &buf.data[TPM_HEADER_SIZE]);
out:
- if (blob != payload->blob)
- kfree(blob);
+ kfree(key);
tpm_buf_destroy(&buf);
if (rc > 0)
--
2.45.1
On Wed May 29, 2024 at 4:50 AM EEST, Stefan Berger wrote:
>
>
> On 5/28/24 17:08, Jarkko Sakkinen wrote:
> > Report -E2BIG instead of -EINVAL when too large size for the key blob is
> > requested.
> >
> > Signed-off-by: Jarkko Sakkinen <[email protected]>
>
> Reviewed-by: Stefan Berger <[email protected]>
Thank you.
Hmm... I'd like to add even:
Cc: [email protected] # v5.13+
Fixes: f2219745250f ("security: keys: trusted: use ASN.1 TPM2 key format for the blobs")
It turned out to be useful error message and would be useful also for
stable kernels. So if no decent counter-arguments, I'll just pick it
to my master branch.
BR, Jarkko
On Fri May 31, 2024 at 3:35 AM EEST, Stefan Berger wrote:
>
>
> On 5/28/24 17:08, Jarkko Sakkinen wrote:
> > TPM2 ASN.1 format is required for trusted keys and asymmetric keys. Move it
> > to crypto in order to make it available for both. Implement validation with
> > coverage of all TPMT_PUBLIC shared fields. Key type specific fields must be
> > covered by the different subsystems using this.
> >
> > A Kconfig option CRYPTO_TPM2_KEY can be used to select the feature, which
> > depends only crypto subsystem itself and ASN.1 parser.
> >
> > Signed-off-by: Jarkko Sakkinen <[email protected]>
> > ---
> > v6:
> > * Relocate to crypto. Validate the shared part and provide
> > accessor functions. Use a fixed buffer size.
> > v2:
> > * Do not allocate blob twice. Use the one inside struct tpm2_key.
> > ---
> > crypto/Kconfig | 7 ++
> > crypto/Makefile | 6 +
> > crypto/tpm2_key.asn1 | 11 ++
> > crypto/tpm2_key.c | 134 ++++++++++++++++++++
> > include/crypto/tpm2_key.h | 46 +++++++
> > security/keys/trusted-keys/Kconfig | 2 +-
> > security/keys/trusted-keys/Makefile | 2 -
> > security/keys/trusted-keys/tpm2key.asn1 | 11 --
> > security/keys/trusted-keys/trusted_tpm2.c | 141 +++++-----------------
> > 9 files changed, 235 insertions(+), 125 deletions(-)
> > create mode 100644 crypto/tpm2_key.asn1
> > create mode 100644 crypto/tpm2_key.c
> > create mode 100644 include/crypto/tpm2_key.h
> > delete mode 100644 security/keys/trusted-keys/tpm2key.asn1
> >
> > diff --git a/crypto/Kconfig b/crypto/Kconfig
> > index 5688d42a59c2..c8989bc71f57 100644
> > --- a/crypto/Kconfig
> > +++ b/crypto/Kconfig
> > @@ -5,6 +5,13 @@
> > config XOR_BLOCKS
> > tristate
> >
> > +config CRYPTO_TPM2_KEY
> > + bool
> > + depends on CRYPTO
> > + select ASN1
> > + select OID_REGISTRY
> > + default n
> > +
> > #
> > # async_tx api: hardware offloaded memory transfer/transform support
> > #
> > diff --git a/crypto/Makefile b/crypto/Makefile
> > index edbbaa3ffef5..d932fdb72319 100644
> > --- a/crypto/Makefile
> > +++ b/crypto/Makefile
> > @@ -216,3 +216,9 @@ obj-$(CONFIG_CRYPTO_SIMD) += crypto_simd.o
> > # Key derivation function
> > #
> > obj-$(CONFIG_CRYPTO_KDF800108_CTR) += kdf_sp800108.o
> > +
> > +ifdef CONFIG_CRYPTO_TPM2_KEY
> > +$(obj)/tpm2_key.asn1.o: $(obj)/tpm2_key.asn1.h $(obj)/tpm2_key.asn1.c
> > +$(obj)/tpm2_key.o: $(obj)/tpm2_key.asn1.h
> > +obj-y += tpm2_key.o tpm2_key.asn1.o
> > +endif
> > diff --git a/crypto/tpm2_key.asn1 b/crypto/tpm2_key.asn1
> > new file mode 100644
> > index 000000000000..b235d02ab78e
> > --- /dev/null
> > +++ b/crypto/tpm2_key.asn1
> > @@ -0,0 +1,11 @@
> > +---
> > +--- ASN.1 for TPM 2.0 keys
> > +---
> > +
> > +TPMKey ::= SEQUENCE {
> > + type OBJECT IDENTIFIER ({tpm2_key_get_type}),
> > + emptyAuth [0] EXPLICIT BOOLEAN OPTIONAL,
> > + parent INTEGER ({tpm2_key_get_parent}),
> > + pubkey OCTET STRING ({tpm2_get_public}),
> > + privkey OCTET STRING ({tpm2_get_private})
> > + }
> > diff --git a/crypto/tpm2_key.c b/crypto/tpm2_key.c
> > new file mode 100644
> > index 000000000000..78f55478d046
> > --- /dev/null
> > +++ b/crypto/tpm2_key.c
> > @@ -0,0 +1,134 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +
> > +#include <crypto/tpm2_key.h>
> > +#include <linux/oid_registry.h>
> > +#include <linux/slab.h>
> > +#include <linux/types.h>
> > +#include <asm/unaligned.h>
> > +#include "tpm2_key.asn1.h"
> > +
> > +#undef pr_fmt
> > +#define pr_fmt(fmt) "tpm2_key: "fmt
> > +
> > +struct tpm2_key_decoder_context {
> > + u32 parent;
> > + const u8 *pub;
> > + u32 pub_len;
> > + const u8 *priv;
> > + u32 priv_len;
> > + enum OID oid;
> > +};
> > +
> > +int tpm2_key_get_parent(void *context, size_t hdrlen,
> > + unsigned char tag,
> > + const void *value, size_t vlen)
> > +{
> > + struct tpm2_key_decoder_context *decoder = context;
> > + const u8 *v = value;
> > + int i;
> > +
> > + decoder->parent = 0;
> > + for (i = 0; i < vlen; i++) {
> > + decoder->parent <<= 8;
> > + decoder->parent |= v[i];
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +int tpm2_key_get_type(void *context, size_t hdrlen,
> > + unsigned char tag,
> > + const void *value, size_t vlen)
> > +{
> > + struct tpm2_key_decoder_context *decoder = context;
> > +
> > + decoder->oid = look_up_OID(value, vlen);
> > + return 0;
> > +}
> > +
> > +static inline bool tpm2_key_is_valid(const void *value, size_t vlen)
> > +{
> > + if (vlen < 2 || vlen > TPM2_KEY_BYTES_MAX)
> > + return false;
> > +
> > + if (get_unaligned_be16(value) != vlen - 2)
> > + return false;
> > +
> > + return true;
> > +}
> > +
> > +int tpm2_get_public(void *context, size_t hdrlen, unsigned char tag,
> > + const void *value, size_t vlen)
> > +{
> > + struct tpm2_key_decoder_context *decoder = context;
> > +
> > + if (!tpm2_key_is_valid(value, vlen))
> > + return -EBADMSG;
> > +
> > + if (sizeof(struct tpm2_key_desc) > vlen - 2)
> > + return -EBADMSG;
> > +
> > + decoder->pub = value;
> > + decoder->pub_len = vlen;
> > + return 0;
> > +}
> > +
> > +int tpm2_get_private(void *context, size_t hdrlen, unsigned char tag,
> > + const void *value, size_t vlen)
> > +{
> > + struct tpm2_key_decoder_context *decoder = context;
> > +
> > + if (!tpm2_key_is_valid(value, vlen))
> > + return -EBADMSG;
> > +
> > + decoder->priv = value;
> > + decoder->priv_len = vlen;
> > + return 0;
> > +}
> > +
> > +/**
> > + * tpm_key_decode() - Decode TPM2 ASN.1 key
> > + * @src: ASN.1 source.
> > + * @src_len: ASN.1 source length.
> > + *
> > + * Decodes the TPM2 ASN.1 key and validates that the public key data has all
> > + * the shared fields of TPMT_PUBLIC. This is full coverage of the memory that
> > + * can be validated before doing any key type specific validation.
>
> I am not sure what the last sentence means.
I think the whole paragraph should be rewritten.
So what it does is that it takes the private and public parts and
concanates them together so maybe just write:
"Load TPMT_PUBLIC and TPMT_PRIVATE from ASN.1 file, and concatenate the
blobs together as a single blob, as this is expected format for the TPM2
commands. In addition, validate TPMT_PUBLIC fields so that they make
sense for trusted and asymmetric keys."
What you think of this?
>
> > + *
> > + * Return:
> > + * - TPM2 ASN.1 key on success.
> > + * - -EBADMSG when decoding fails.
> > + * - -ENOMEM when OOM while allocating struct tpm2_key.
> > + */
> > +struct tpm2_key *tpm2_key_decode(const u8 *src, u32 src_len)
> > +{
> > + struct tpm2_key_decoder_context decoder;
> > + struct tpm2_key *key;
> > + u8 *data;
> > + int ret;
> > +
> > + memset(&decoder, 0, sizeof(decoder));
> > + ret = asn1_ber_decoder(&tpm2_key_decoder, &decoder, src, src_len);
> > + if (ret < 0) {
> > + if (ret != -EBADMSG)
> > + pr_info("Decoder error %d\n", ret);
> > +
> > + return ERR_PTR(-EBADMSG);
> > + }
> > +
> > + key = kzalloc(sizeof(*key), GFP_KERNEL);
> > + if (!key)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + data = &key->data[0];
> > + memcpy(&data[0], decoder.priv, decoder.priv_len);
> > + memcpy(&data[decoder.priv_len], decoder.pub, decoder.pub_len);
> > +
> > + key->oid = decoder.oid;
> > + key->priv_len = decoder.priv_len;
> > + key->pub_len = decoder.pub_len;
> > + key->parent = decoder.parent;
> > + key->desc = (struct tpm2_key_desc *)&data[decoder.priv_len + 2];
> > + return key;
> > +}
> > +EXPORT_SYMBOL_GPL(tpm2_key_decode);
> > diff --git a/include/crypto/tpm2_key.h b/include/crypto/tpm2_key.h
> > new file mode 100644
> > index 000000000000..74debaf707bf
> > --- /dev/null
> > +++ b/include/crypto/tpm2_key.h
> > @@ -0,0 +1,46 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +#ifndef __LINUX_TPM2_KEY_H__
> > +#define __LINUX_TPM2_KEY_H__
> > +
> > +#include <linux/oid_registry.h>
> > +#include <linux/slab.h>
> > +
> > +#define TPM2_KEY_BYTES_MAX 1024
> > +
> > +/* TPM2 Structures 12.2.4: TPMT_PUBLIC */
> > +struct tpm2_key_desc {
> > + __be16 type;
> > + __be16 name_alg;
> > + __be32 object_attributes;
> > + __be16 policy_size;
> > +} __packed;
> > +
> > +/* Decoded TPM2 ASN.1 key. */
> > +struct tpm2_key {
> > + u8 data[2 * TPM2_KEY_BYTES_MAX];
> > + struct tpm2_key_desc *desc;
> > + u16 priv_len;
> > + u16 pub_len;
> > + u32 parent;
> > + enum OID oid;
> > + char oid_str[64];
> > +};
> > +
> > +struct tpm2_key *tpm2_key_decode(const u8 *src, u32 src_len);
> > +
> > +static inline const void *tpm2_key_data(const struct tpm2_key *key)
> > +{
> > + return &key->data[0];
> > +}
> > +
> > +static inline u16 tpm2_key_type(const struct tpm2_key *key)
> > +{
> > + return be16_to_cpu(key->desc->type);
> > +}
> > +
> > +static inline int tpm2_key_policy_size(const struct tpm2_key *key)
> > +{
> > + return be16_to_cpu(key->desc->policy_size);
> > +}
> > +
> > +#endif /* __LINUX_TPM2_KEY_H__ */
> > diff --git a/security/keys/trusted-keys/Kconfig b/security/keys/trusted-keys/Kconfig
> > index 1fb8aa001995..00d9489384ac 100644
> > --- a/security/keys/trusted-keys/Kconfig
> > +++ b/security/keys/trusted-keys/Kconfig
> > @@ -9,9 +9,9 @@ config TRUSTED_KEYS_TPM
> > select CRYPTO_HMAC
> > select CRYPTO_SHA1
> > select CRYPTO_HASH_INFO
> > + select CRYPTO_TPM2_KEY
> > select ASN1_ENCODER
> > select OID_REGISTRY
> > - select ASN1
> > select HAVE_TRUSTED_KEYS
> > help
> > Enable use of the Trusted Platform Module (TPM) as trusted key
> > diff --git a/security/keys/trusted-keys/Makefile b/security/keys/trusted-keys/Makefile
> > index f0f3b27f688b..2674d5c10fc9 100644
> > --- a/security/keys/trusted-keys/Makefile
> > +++ b/security/keys/trusted-keys/Makefile
> > @@ -7,9 +7,7 @@ obj-$(CONFIG_TRUSTED_KEYS) += trusted.o
> > trusted-y += trusted_core.o
> > trusted-$(CONFIG_TRUSTED_KEYS_TPM) += trusted_tpm1.o
> >
> > -$(obj)/trusted_tpm2.o: $(obj)/tpm2key.asn1.h
> > trusted-$(CONFIG_TRUSTED_KEYS_TPM) += trusted_tpm2.o
> > -trusted-$(CONFIG_TRUSTED_KEYS_TPM) += tpm2key.asn1.o
> >
> > trusted-$(CONFIG_TRUSTED_KEYS_TEE) += trusted_tee.o
> >
> > diff --git a/security/keys/trusted-keys/tpm2key.asn1 b/security/keys/trusted-keys/tpm2key.asn1
> > deleted file mode 100644
> > index f57f869ad600..000000000000
> > --- a/security/keys/trusted-keys/tpm2key.asn1
> > +++ /dev/null
> > @@ -1,11 +0,0 @@
> > ----
> > ---- ASN.1 for TPM 2.0 keys
> > ----
> > -
> > -TPMKey ::= SEQUENCE {
> > - type OBJECT IDENTIFIER ({tpm2_key_type}),
> > - emptyAuth [0] EXPLICIT BOOLEAN OPTIONAL,
> > - parent INTEGER ({tpm2_key_parent}),
> > - pubkey OCTET STRING ({tpm2_key_pub}),
> > - privkey OCTET STRING ({tpm2_key_priv})
> > - }
> > diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
> > index 06c8fa7b21ae..b9e505e99e8c 100644
> > --- a/security/keys/trusted-keys/trusted_tpm2.c
> > +++ b/security/keys/trusted-keys/trusted_tpm2.c
> > @@ -13,11 +13,10 @@
> >
> > #include <keys/trusted-type.h>
> > #include <keys/trusted_tpm.h>
> > +#include <crypto/tpm2_key.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},
> > @@ -98,106 +97,6 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
> > return ret;
> > }
> >
> > -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 -E2BIG;
> > -
> > - 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;
> > -}
> > -
> > /**
> > * tpm2_buf_append_auth() - append TPMS_AUTH_COMMAND to the buffer.
> > *
> > @@ -387,22 +286,43 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
> > struct trusted_key_options *options,
> > u32 *blob_handle)
> > {
> > - struct tpm_buf buf;
> > unsigned int private_len;
> > unsigned int public_len;
> > unsigned int blob_len;
> > - u8 *blob, *pub;
> > - int rc;
> > + struct tpm2_key *key;
> > + const u8 *blob, *pub;
> > + struct tpm_buf buf;
> > u32 attrs;
> > + int rc;
> >
> > - rc = tpm2_key_decode(payload, options, &blob);
> > - if (rc) {
> > - /* old form */
> > + key = tpm2_key_decode(payload->blob, payload->blob_len);
> > + if (IS_ERR(key)) {
> > + /* Get the error code and reset the pointer to the key: */
> > + rc = PTR_ERR(key);
> > + key = NULL;
> > +
> > + if (rc == -ENOMEM)
> > + return -ENOMEM;
> > +
> > + /* A sanity check, as only -EBADMSG or -ENOMEM are expected: */
> > + if (rc != -EBADMSG)
> > + pr_err("tpm2_key_decode(): spurious error code %d\n", rc);
>
> tpm2_key_decode seems simple enough that it only returns key, -ENOMEM or
> EBADMSG.
So what is your suggestion here?
The reasoning here is that asymmetric keys use -EBADMSG not only as
error but also iterator, when probing which can load a specific key.
>
> > +
> > + /* Fallback to the legacy format: */
> > blob = payload->blob;
> > payload->old_format = 1;
> > + } else {
> > + blob = tpm2_key_data(key);
> > + if (key->oid != OID_TPMSealedData) {
> > + kfree(key);
> > + return -EBADMSG;
> > + }
> > }
> >
> > - /* new format carries keyhandle but old format doesn't */
> > + /*
> > + * Must be non-zero here, either extracted from the ASN.1 for the new
> > + * format or specified on the command line for the old.
>
> sentence seems incomplete: ... for the old one. OR ... for the old format.
Yep, I think it is a plain mistake.
>
> > + */
> > if (!options->keyhandle)
> > return -EINVAL;
> >
> > @@ -464,8 +384,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
> > (__be32 *) &buf.data[TPM_HEADER_SIZE]);
> >
> > out:
> > - if (blob != payload->blob)
> > - kfree(blob);
> > + kfree(key);
> > tpm_buf_destroy(&buf);
> >
> > if (rc > 0)
Thanks for the feedback.
BR, Jarkko
On 6/4/24 13:23, Jarkko Sakkinen wrote:
> On Fri May 31, 2024 at 3:35 AM EEST, Stefan Berger wrote:
>>
>>>
>>> - rc = tpm2_key_decode(payload, options, &blob);
>>> - if (rc) {
>>> - /* old form */
>>> + key = tpm2_key_decode(payload->blob, payload->blob_len);
>>> + if (IS_ERR(key)) {
>>> + /* Get the error code and reset the pointer to the key: */
>>> + rc = PTR_ERR(key);
>>> + key = NULL;
>>> +
>>> + if (rc == -ENOMEM)
>>> + return -ENOMEM;
>>> +
>>> + /* A sanity check, as only -EBADMSG or -ENOMEM are expected: */
>>> + if (rc != -EBADMSG)
>>> + pr_err("tpm2_key_decode(): spurious error code %d\n", rc);
>>
>> tpm2_key_decode seems simple enough that it only returns key, -ENOMEM or
>> EBADMSG.
>
> So what is your suggestion here?
You can remove the check resuling in pr_err().
>
> The reasoning here is that asymmetric keys use -EBADMSG not only as
> error but also iterator, when probing which can load a specific key.
>
On Tue Jun 4, 2024 at 9:41 PM EEST, Stefan Berger wrote:
>
>
> On 6/4/24 13:23, Jarkko Sakkinen wrote:
> > On Fri May 31, 2024 at 3:35 AM EEST, Stefan Berger wrote:
> >>
>
> >>>
> >>> - rc = tpm2_key_decode(payload, options, &blob);
> >>> - if (rc) {
> >>> - /* old form */
> >>> + key = tpm2_key_decode(payload->blob, payload->blob_len);
> >>> + if (IS_ERR(key)) {
> >>> + /* Get the error code and reset the pointer to the key: */
> >>> + rc = PTR_ERR(key);
> >>> + key = NULL;
> >>> +
> >>> + if (rc == -ENOMEM)
> >>> + return -ENOMEM;
> >>> +
> >>> + /* A sanity check, as only -EBADMSG or -ENOMEM are expected: */
> >>> + if (rc != -EBADMSG)
> >>> + pr_err("tpm2_key_decode(): spurious error code %d\n", rc);
> >>
> >> tpm2_key_decode seems simple enough that it only returns key, -ENOMEM or
> >> EBADMSG.
> >
> > So what is your suggestion here?
>
> You can remove the check resuling in pr_err().
OK, I think so too. Just had to (sanity) check.
>
> >
> > The reasoning here is that asymmetric keys use -EBADMSG not only as
> > error but also iterator, when probing which can load a specific key.
> >
BR, Jarkko