2024-05-28 23:16:13

by Stefan Berger

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] keys: asymmetric: Add tpm2_key_ecdsa



On 5/28/24 17:08, Jarkko Sakkinen wrote:
> * Asymmetric TPM2 ECDSA key with signing and verification.
> * Enabled with CONFIG_ASYMMETRIC_TPM2_KEY_ECDSA_SUBTYPE.
>
> Cc: Stefan Berger <[email protected]>
> Signed-off-by: Jarkko Sakkinen <[email protected]>
> ---
> v7:
> * Rewrote the signature encoder.
> * Added the missing sha256() call to the signature verifier.
> v6:
> * The very first version.
> * Stefan: any idea why the signature give -EKEYREJECTED?
> ---
> crypto/asymmetric_keys/Kconfig | 15 +
> crypto/asymmetric_keys/Makefile | 1 +
> crypto/asymmetric_keys/tpm2_key_ecdsa.c | 462 ++++++++++++++++++++++++
> crypto/ecdsa.c | 1 -
> drivers/char/tpm/tpm-buf.c | 2 +-
> include/linux/tpm.h | 7 +
> 6 files changed, 486 insertions(+), 2 deletions(-)
> create mode 100644 crypto/asymmetric_keys/tpm2_key_ecdsa.c
>
> diff --git a/crypto/asymmetric_keys/Kconfig b/crypto/asymmetric_keys/Kconfig
> index 9d88c1190621..c97f11e0340c 100644
> --- a/crypto/asymmetric_keys/Kconfig
> +++ b/crypto/asymmetric_keys/Kconfig
> @@ -24,6 +24,21 @@ config ASYMMETRIC_PUBLIC_KEY_SUBTYPE
> appropriate hash algorithms (such as SHA-1) must be available.
> ENOPKG will be reported if the requisite algorithm is unavailable.
>
> +config ASYMMETRIC_TPM2_KEY_ECDSA_SUBTYPE
> + tristate "Asymmetric TPM2 ECDSA crypto algorithm subtype"
> + depends on TCG_TPM
> + select CRYPTO_ECDSA
> + select CRYPTO_SHA256
> + select CRYPTO_HASH_INFO
> + select CRYPTO_TPM2_KEY
> + select ASN1
> + select ASN1_ENCODER
> + help
> + This option provides support for asymmetric TPM2 key type handling.
> + If signature generation and/or verification are to be used,
> + appropriate hash algorithms (such as SHA-256) must be available.
> + ENOPKG will be reported if the requisite algorithm is unavailable.
> +
> config ASYMMETRIC_TPM2_KEY_RSA_SUBTYPE
> tristate "Asymmetric TPM2 RSA crypto algorithm subtype"
> depends on TCG_TPM
> diff --git a/crypto/asymmetric_keys/Makefile b/crypto/asymmetric_keys/Makefile
> index c6da84607824..0843d2268a69 100644
> --- a/crypto/asymmetric_keys/Makefile
> +++ b/crypto/asymmetric_keys/Makefile
> @@ -11,6 +11,7 @@ asymmetric_keys-y := \
> signature.o
>
> obj-$(CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE) += public_key.o
> +obj-$(CONFIG_ASYMMETRIC_TPM2_KEY_ECDSA_SUBTYPE) += tpm2_key_ecdsa.o
> obj-$(CONFIG_ASYMMETRIC_TPM2_KEY_RSA_SUBTYPE) += tpm2_key_rsa.o
>
> #
> diff --git a/crypto/asymmetric_keys/tpm2_key_ecdsa.c b/crypto/asymmetric_keys/tpm2_key_ecdsa.c
> new file mode 100644
> index 000000000000..e2f599a0ffe0
> --- /dev/null
> +++ b/crypto/asymmetric_keys/tpm2_key_ecdsa.c
> @@ -0,0 +1,462 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/* Asymmetric TPM2 ECDSA key subtype.
> + *
> + * See Documentation/crypto/asymmetric-keys.rst
> + */
> +
> +#include <asm/unaligned.h>
> +#include <crypto/internal/ecc.h>
> +#include <crypto/akcipher.h>
> +#include <crypto/sha2.h>
> +#include <crypto/public_key.h>
> +#include <crypto/tpm2_key.h>
> +#include <keys/asymmetric-parser.h>
> +#include <keys/asymmetric-subtype.h>
> +#include <linux/asn1_encoder.h>
> +#include <linux/keyctl.h>
> +#include <linux/module.h>
> +#include <linux/scatterlist.h>
> +#include <linux/slab.h>
> +#include <linux/tpm.h>
> +
> +#undef pr_fmt
> +#define pr_fmt(fmt) "tpm2_key_ecdsa: "fmt
> +
> +struct tpm2_ecc_parms {
> + __be16 symmetric;
> + __be16 scheme;
> + __be16 ecc;
> + __be16 kdf;
> +};
> +
> +static const u8 *tpm2_key_ecdsa_ecc_x(const struct tpm2_key *key)
> +{
> + const off_t o = key->priv_len + 2 + sizeof(*key->desc);
> +
> + return &key->data[o + sizeof(struct tpm2_ecc_parms)];
> +}
> +
> +static const u8 *tpm2_key_ecdsa_ecc_y(const struct tpm2_key *key)
> +{
> + const u8 *x = tpm2_key_ecdsa_ecc_x(key);
> + u16 x_size = get_unaligned_be16(&x[0]);
> +
> + /* +2 from the size field: */
> + return &x[2 + x_size];
> +}
> +
> +static void tpm2_key_ecdsa_describe(const struct key *asymmetric_key,
> + struct seq_file *m)
> +{
> + struct tpm2_key *key = asymmetric_key->payload.data[asym_crypto];
> +
> + if (!key) {
> + pr_err("key missing");
> + return;
> + }
> +
> + seq_puts(m, "TPM2/ECDSA");
> +}
> +
> +static void tpm2_key_ecdsa_destroy(void *payload0, void *payload3)
> +{
> + struct tpm2_key *key = payload0;
> +
> + if (!key)
> + return;
> +
> + kfree(key);
> +}
> +
> +static const char *tpm2_ecc_name(u16 ecc)
> +{
> + const char *name;
> +
> + switch (ecc) {
> + case TPM2_ECC_NIST_P521:
> + name = "ecdsa-nist-p521";
> + break;
> + case TPM2_ECC_NIST_P384:
> + name = "ecdsa-nist-p384";
> + break;
> + default:
> + name = "ecdsa-nist-p256";
> + break;
> + }
> +
> + return name;
> +}
> +
> +static int tpm2_key_ecdsa_query(const struct kernel_pkey_params *params,
> + struct kernel_pkey_query *info)
> +{
> + const struct tpm2_key *key = params->key->payload.data[asym_crypto];
> + const off_t o = key->priv_len + 2 + sizeof(*key->desc);
> + const struct tpm2_ecc_parms *p =
> + (const struct tpm2_ecc_parms *)&key->data[o];
> + u16 ecc = be16_to_cpu(p->ecc);
> + const char *ecc_name = tpm2_ecc_name(ecc);
> + const u8 *x = tpm2_key_ecdsa_ecc_x(key);
> + u16 x_size = get_unaligned_be16(&x[0]);
> + struct crypto_akcipher *tfm;
> + char data[256];

Due to NIST p521 1 + 2 * 66 = 133 should be enough.

If x_size exceeeds 66 then something is wrong.

> + u8 *ptr;
> + int ret;
> +
> + memset(data, 0, sizeof(data));
> +
> + tfm = crypto_alloc_akcipher(ecc_name, 0, 0);
> + if (IS_ERR(tfm))
> + return PTR_ERR(tfm);
> +
> + /* Probe for ecdsa_set_pub_key(): */
> + ptr = &data[0];
> + *ptr++ = 0x04; /* uncompressed */
> + memcpy(&ptr[0], &x[2], x_size);
> + memcpy(&ptr[x_size], &x[2 + x_size + 2], x_size);
> + ret = crypto_akcipher_set_pub_key(tfm, data, 2 * x_size + 1);
> + crypto_free_akcipher(tfm);
> + if (ret < 0)
> + return ret;
> +
> + info->max_sig_size = 256;
> + info->key_size = 256;
> + info->max_data_size = 256;
> + info->supported_ops = KEYCTL_SUPPORTS_SIGN | KEYCTL_SUPPORTS_VERIFY;
> + return ret;
> +}
> +
> +static int tpm2_key_ecdsa_sign(struct tpm_chip *chip, struct tpm2_key *key,
> + struct kernel_pkey_params *params,
> + const void *in, void *out)
> +{
> + u8 r[SHA256_DIGEST_SIZE], s[SHA256_DIGEST_SIZE];
> + u32 in_len = params->in_len;
> + bool r_0, s_0;
> + struct tpm_header *head;
> + struct tpm_buf buf;
> + u32 key_handle;
> + u8 *ptr = out;
> + off_t offset;
> + int ret;
> +
> +
> + /* Require explicit hash algorithm: */
> + if (!params->hash_algo)
> + return -EINVAL;
> +
> + /* Currently only support SHA256: */
> + if (!!strcmp(params->hash_algo, "sha256"))
> + return -EINVAL;
> +
> + ret = tpm_try_get_ops(chip);
> + if (ret)
> + return ret;
> +
> + ret = tpm2_start_auth_session(chip);
> + if (ret)
> + goto err_ops;
> +
> + ret = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
> + if (ret < 0) {
> + tpm2_end_auth_session(chip);
> + goto err_ops;
> + }
> +
> + tpm_buf_append_name(chip, &buf, key->parent, NULL);
> + tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_CONTINUE_SESSION |
> + TPM2_SA_ENCRYPT, NULL, 0);
> + tpm_buf_append(&buf, &key->data[0], key->priv_len + key->pub_len);
> + if (buf.flags & TPM_BUF_OVERFLOW) {
> + tpm2_end_auth_session(chip);
> + ret = -E2BIG;
> + goto err_buf;
> + }
> + tpm_buf_fill_hmac_session(chip, &buf);
> + ret = tpm_transmit_cmd(chip, &buf, 4, "ECDSA loading");
> + ret = tpm_buf_check_hmac_response(chip, &buf, ret);
> + if (ret) {
> + tpm2_end_auth_session(chip);
> + ret = -EIO;
> + goto err_buf;
> + }
> +
> + key_handle = be32_to_cpup((__be32 *)&buf.data[TPM_HEADER_SIZE]);
> +
> + tpm_buf_reset(&buf, TPM2_ST_SESSIONS, TPM2_CC_SIGN);
> + tpm_buf_append_name(chip, &buf, key_handle, NULL);
> + tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_DECRYPT, NULL, 0);
> +
> + sha256(in, in_len, r);
> + tpm_buf_append_u16(&buf, SHA256_DIGEST_SIZE);
> + tpm_buf_append(&buf, r, SHA256_DIGEST_SIZE);
> + tpm_buf_append_u16(&buf, TPM_ALG_ECDSA);
> + tpm_buf_append_u16(&buf, TPM_ALG_SHA256);
> +
> + /* 10.7.2 A NULL Ticket */
> + tpm_buf_append_u16(&buf, TPM2_ST_HASHCHECK);
> + tpm_buf_append_u32(&buf, TPM2_RH_NULL);
> + tpm_buf_append_u16(&buf, 0);
> +
> + tpm_buf_fill_hmac_session(chip, &buf);
> + ret = tpm_transmit_cmd(chip, &buf, 4, "ECDSA signing");
> + ret = tpm_buf_check_hmac_response(chip, &buf, ret);
> + if (ret) {
> + tpm2_end_auth_session(chip);
> + ret = -EIO;
> + goto err_key_handle;
> + }
> +
> + /* Move to parameters: */
> + head = (struct tpm_header *)buf.data;
> + offset = sizeof(*head);
> + if (be16_to_cpu(head->tag) == TPM2_ST_SESSIONS)
> + offset += 4;
> +
> + ret = -EIO;
> +
> + /* Copy R: */
> + if (tpm_buf_read_u16(&buf, &offset) != TPM_ALG_ECDSA ||
> + tpm_buf_read_u16(&buf, &offset) != TPM_ALG_SHA256 ||
> + tpm_buf_read_u16(&buf, &offset) != SHA256_DIGEST_SIZE) {
> + pr_warn("offset=%u\n", offset);
> + goto err_key_handle;
> + }
> +
> + tpm_buf_read(&buf, &offset, SHA256_DIGEST_SIZE, r);
> + r_0 = (r[0] & 0x80) != 0;
> + pr_info("r_0=%d\n", r_0);
> +
> + /* Copy S: */
> + if (tpm_buf_read_u16(&buf, &offset) != SHA256_DIGEST_SIZE) {
> + pr_warn("offset=%u\n", offset);
> + goto err_key_handle;
> + }
> +
> + tpm_buf_read(&buf, &offset, SHA256_DIGEST_SIZE, s);
> + s_0 = (r[0] & 0x80) != 0;
> + pr_info("s_0=%d\n", r_0);
> +
> + /* Encode the ASN.1 signature: */
> +#define TPM2_KEY_ECDSA_SIG_SIZE (2 + 2 * (2 + SHA256_DIGEST_SIZE) + r_0 + s_0)
> + pr_info("sig_size=%d\n", TPM2_KEY_ECDSA_SIG_SIZE);
> + ptr[0] = 0x30; /* SEQUENCE */
> + ptr[1] = TPM2_KEY_ECDSA_SIG_SIZE - 2;
> +#define TPM2_KEY_ECDSA_SIG_R_TAG 2
> +#define TPM2_KEY_ECDSA_SIG_R_SIZE 3
> +#define TPM2_KEY_ECDSA_SIG_R_BODY 4
> + ptr[TPM2_KEY_ECDSA_SIG_R_TAG] = 0x02; /* INTEGER */
> + ptr[TPM2_KEY_ECDSA_SIG_R_SIZE] = SHA256_DIGEST_SIZE + r_0;

The size of the signature has nothing to do with the size of the hash.
SHA256_DIGEST_SIZE (32) happens to match the number of bytes of a
coordinate of prime256v1 / NIST p256 but should fail when you use
secp521r1 / NIST p521 since then r or s may then be 66 or 67 bytes (if
most sign. bit is set) long.

> + ptr[TPM2_KEY_ECDSA_SIG_R_BODY] = 0x00; /* maybe dummy write */
> + memcpy(&ptr[TPM2_KEY_ECDSA_SIG_R_BODY + r_0], r, SHA256_DIGEST_SIZE);
> +#define TPM2_KEY_ECDSA_SIG_S_TAG (4 + r_0 + SHA256_DIGEST_SIZE)
> +#define TPM2_KEY_ECDSA_SIG_S_SIZE (5 + r_0 + SHA256_DIGEST_SIZE)
> +#define TPM2_KEY_ECDSA_SIG_S_BODY (6 + r_0 + SHA256_DIGEST_SIZE)
> + ptr[TPM2_KEY_ECDSA_SIG_S_TAG] = 0x02; /* INTEGER */
> + ptr[TPM2_KEY_ECDSA_SIG_S_SIZE] = SHA256_DIGEST_SIZE + s_0;
> + ptr[TPM2_KEY_ECDSA_SIG_S_BODY] = 0x00; /* maybe dummy write */
> + memcpy(&ptr[TPM2_KEY_ECDSA_SIG_S_BODY + s_0], s, SHA256_DIGEST_SIZE);
> + ret = TPM2_KEY_ECDSA_SIG_SIZE;
> +
> +err_key_handle:
> + tpm2_flush_context(chip, key_handle);
> +
> +err_buf:
> + tpm_buf_destroy(&buf);
> +
> +err_ops:
> + tpm_put_ops(chip);
> + return ret;
> +}
> +


2024-05-29 01:15:08

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v7 5/5] keys: asymmetric: Add tpm2_key_ecdsa

On Wed May 29, 2024 at 2:15 AM EEST, Stefan Berger wrote:
> > + ptr[TPM2_KEY_ECDSA_SIG_R_TAG] = 0x02; /* INTEGER */
> > + ptr[TPM2_KEY_ECDSA_SIG_R_SIZE] = SHA256_DIGEST_SIZE + r_0;
>
> The size of the signature has nothing to do with the size of the hash.
> SHA256_DIGEST_SIZE (32) happens to match the number of bytes of a
> coordinate of prime256v1 / NIST p256 but should fail when you use
> secp521r1 / NIST p521 since then r or s may then be 66 or 67 bytes (if
> most sign. bit is set) long.

First remark did not go unnoticed, so thanks for both. There was not
just much to comment on it :-)

I could just replace the constant with a (range checked) variable
read from the response and overall structure woud be the same.

This will also mean that in the case of P521 also prefix byte (0x81) is
required but just for the sequence I think, not for the integers.

Finally, I need to implement p521 smoke test for testing this patch set.

One big letdown that I only now have consciously realized, is that TCG
does not have p256k1 in their algorithm repository. It is the basis for
quite a few blockchain technologies. I wonder why...

BR, Jarkko