Re: [PATCH v7 3/5] crypto: tpm2_key: Introduce a TPM2 key type

Stefan Berger
2024-05-31T00:36:01Z

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 <jarkko@kernel.org>
> ---
> 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.

> + *
> + * 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.

> +
> + /* 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.

> + */
> 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)