[PATCH v2 0/6] KEYS: asymmetric: tpm2_key_rsa

[PATCH v2 0/6] KEYS: asymmetric: tpm2_key_rsa

[Jarkko Sakkinen]

## Overview

Introduce tpm2_key_rsa module, which implements asymmetric TPM2 RSA key.
The feature can be enabled with the CONFIG_ASYMMETRIC_TPM2_KEY_RSA_SUBTYPE 
kconfig option.

The idea in the design is to over time to have submodule per key type
For instance, tpm2_key_ecdsa could be one potential future addition in
the future. Perhaps, it might sense to consider at that point also a
top-level tpm2_key module. The gist is that the naming convention is
free from potential future bottlencks.

## Change Log

v2
Cleaned up all the low-hanging fruit for the sake of saving everyones
time. After this I move into reactive mode (I promise) ;-)

## Testing

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

## References

- v1: https://lore.kernel.org/linux-integrity/20240520184727.22038-1-jarkko@kernel.org/
- Derived from https://lore.kernel.org/all/20200518172704.29608-1-prestwoj@gmail.com/

James Prestwood (1):
  keys: asymmetric: ASYMMETRIC_TPM2_KEY_RSA_SUBTYPE

Jarkko Sakkinen (5):
  crypto: rsa-pkcs1pad: export rsa1_asn_lookup()
  lib: Expand asn1_encode_integer() to variable size integers
  tpm: Export tpm2_load_context()
  KEYS: trusted: Move tpm2_key_decode() to the TPM driver
  tpm: tpm2_key: Extend parser to TPM_LoadableKey

 crypto/asymmetric_keys/Kconfig                |  16 +
 crypto/asymmetric_keys/Makefile               |   1 +
 crypto/asymmetric_keys/tpm2_key_rsa.c         | 698 ++++++++++++++++++
 crypto/rsa-pkcs1pad.c                         |  16 +-
 drivers/char/tpm/Kconfig                      |   1 +
 drivers/char/tpm/Makefile                     |   5 +
 drivers/char/tpm/tpm.h                        |   2 -
 drivers/char/tpm/tpm2-cmd.c                   |  77 ++
 drivers/char/tpm/tpm2-space.c                 |  61 --
 drivers/char/tpm/tpm2_key.c                   | 119 +++
 .../char/tpm}/tpm2key.asn1                    |   0
 include/crypto/rsa-pkcs1pad.h                 |  20 +
 include/crypto/tpm2_key.h                     |  35 +
 include/linux/asn1_encoder.h                  |   3 +-
 include/linux/tpm.h                           |   4 +
 lib/asn1_encoder.c                            | 185 ++---
 security/keys/trusted-keys/Makefile           |   2 -
 security/keys/trusted-keys/trusted_tpm2.c     | 135 +---
 18 files changed, 1110 insertions(+), 270 deletions(-)
 create mode 100644 crypto/asymmetric_keys/tpm2_key_rsa.c
 create mode 100644 drivers/char/tpm/tpm2_key.c
 rename {security/keys/trusted-keys => drivers/char/tpm}/tpm2key.asn1 (100%)
 create mode 100644 include/crypto/rsa-pkcs1pad.h
 create mode 100644 include/crypto/tpm2_key.h

-- 
2.45.1

[PATCH v2 1/6] crypto: rsa-pkcs1pad: export rsa1_asn_lookup()

[Jarkko Sakkinen]

ASN.1 template is required for TPM2 asymmetric keys, as it needs to be
piggy-packed with the input data before applying TPM2_RSA_Decrypt. This
patch prepares crypto subsystem for the addition of those keys.

Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 crypto/rsa-pkcs1pad.c         | 16 ++++++++++------
 include/crypto/rsa-pkcs1pad.h | 20 ++++++++++++++++++++
 2 files changed, 30 insertions(+), 6 deletions(-)
 create mode 100644 include/crypto/rsa-pkcs1pad.h

diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index cd501195f34a..00b6c14f861c 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -7,6 +7,7 @@
 
 #include <crypto/algapi.h>
 #include <crypto/akcipher.h>
+#include <crypto/rsa-pkcs1pad.h>
 #include <crypto/internal/akcipher.h>
 #include <crypto/internal/rsa.h>
 #include <linux/err.h>
@@ -79,11 +80,7 @@ static const u8 rsa_digest_info_sha3_512[] = {
 	0x05, 0x00, 0x04, 0x40
 };
 
-static const struct rsa_asn1_template {
-	const char	*name;
-	const u8	*data;
-	size_t		size;
-} rsa_asn1_templates[] = {
+static const struct rsa_asn1_template rsa_asn1_templates[] = {
 #define _(X) { #X, rsa_digest_info_##X, sizeof(rsa_digest_info_##X) }
 	_(md5),
 	_(sha1),
@@ -101,7 +98,13 @@ static const struct rsa_asn1_template {
 	{ NULL }
 };
 
-static const struct rsa_asn1_template *rsa_lookup_asn1(const char *name)
+/**
+ * rsa_lookup_asn1() - Lookup the ASN.1 digest info given the hash
+ * name:	hash algorithm name
+ *
+ * Returns the ASN.1 digest info on success, and NULL on failure.
+ */
+const struct rsa_asn1_template *rsa_lookup_asn1(const char *name)
 {
 	const struct rsa_asn1_template *p;
 
@@ -110,6 +113,7 @@ static const struct rsa_asn1_template *rsa_lookup_asn1(const char *name)
 			return p;
 	return NULL;
 }
+EXPORT_SYMBOL_GPL(rsa_lookup_asn1);
 
 struct pkcs1pad_ctx {
 	struct crypto_akcipher *child;
diff --git a/include/crypto/rsa-pkcs1pad.h b/include/crypto/rsa-pkcs1pad.h
new file mode 100644
index 000000000000..32c7453ff644
--- /dev/null
+++ b/include/crypto/rsa-pkcs1pad.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * RSA padding templates.
+ */
+
+#ifndef _CRYPTO_RSA_PKCS1PAD_H
+#define _CRYPTO_RSA_PKCS1PAD_H
+
+/*
+ * Hash algorithm name to ASN.1 template mapping.
+ */
+struct rsa_asn1_template {
+	const char *name;
+	const u8 *data;
+	size_t size;
+};
+
+const struct rsa_asn1_template *rsa_lookup_asn1(const char *name);
+
+#endif /* _CRYPTO_RSA_PKCS1PAD_H */
-- 
2.45.1

[PATCH v2 2/6] lib: Expand asn1_encode_integer() to variable size integers

[Jarkko Sakkinen]

Expand asn1_encode_integer() to variable size integers, meaning that it
will get a blob in big-endian format as integer and length of the blob as
parameters. This is required in order to encode RSA public key modulus.

Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 include/linux/asn1_encoder.h              |   3 +-
 lib/asn1_encoder.c                        | 185 ++++++++++++----------
 security/keys/trusted-keys/trusted_tpm2.c |   4 +-
 3 files changed, 103 insertions(+), 89 deletions(-)

diff --git a/include/linux/asn1_encoder.h b/include/linux/asn1_encoder.h
index 08cd0c2ad34f..ad5fb18db9e2 100644
--- a/include/linux/asn1_encoder.h
+++ b/include/linux/asn1_encoder.h
@@ -9,9 +9,10 @@
 #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);
+		    const u8 *integer, int integer_len);
 unsigned char *
 asn1_encode_oid(unsigned char *data, const unsigned char *end_data,
 		u32 oid[], int oid_len);
diff --git a/lib/asn1_encoder.c b/lib/asn1_encoder.c
index 0fd3c454a468..51a2d7010a67 100644
--- a/lib/asn1_encoder.c
+++ b/lib/asn1_encoder.c
@@ -9,12 +9,78 @@
 #include <linux/bug.h>
 #include <linux/string.h>
 #include <linux/module.h>
+#include <linux/slab.h>
+
+/**
+ * asn1_encode_length() - encode a length to follow an ASN.1 tag
+ * @data: pointer to encode at
+ * @data_len: pointer to remaining 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_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
+ * @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
+ * @integer_len:	length in bytes of the integer blob
  *
  * This is a simplified encoder: it only currently does
  * positive integers, but it should be simple enough to add the
@@ -22,15 +88,17 @@
  */
 unsigned char *
 asn1_encode_integer(unsigned char *data, const unsigned char *end_data,
-		    s64 integer)
+		    const u8 *integer, int integer_len)
 {
 	int data_len = end_data - data;
-	unsigned char *d = &data[2];
 	bool found = false;
+	unsigned char *d;
+	int encoded_len;
+	u8 *encoded;
+	int ret;
 	int i;
 
-	if (WARN(integer < 0,
-		 "BUG: integer encode only supports positive integers"))
+	if (WARN(!integer, "BUG: integer is null"))
 		return ERR_PTR(-EINVAL);
 
 	if (IS_ERR(data))
@@ -40,17 +108,22 @@ asn1_encode_integer(unsigned char *data, const unsigned char *end_data,
 	if (data_len < 3)
 		return ERR_PTR(-EINVAL);
 
-	/* remaining length where at d (the start of the integer encoding) */
-	data_len -= 2;
+	(*data++) = _tag(UNIV, PRIM, INT);
+	data_len--;
 
-	data[0] = _tag(UNIV, PRIM, INT);
-	if (integer == 0) {
-		*d++ = 0;
-		goto out;
+	if (!memchr_inv(integer, 0, integer_len)) {
+		data[1] = 1;
+		data[2] = 0;
+		return &data[2];
 	}
 
-	for (i = sizeof(integer); i > 0 ; i--) {
-		int byte = integer >> (8 * (i - 1));
+	encoded = kzalloc(integer_len, GFP_KERNEL);
+	if (!encoded)
+		return ERR_PTR(-ENOMEM);
+	d = encoded;
+
+	for (i = 0; i < integer_len; i++) {
+		int byte = integer[i];
 
 		if (!found && byte == 0)
 			continue;
@@ -67,21 +140,23 @@ asn1_encode_integer(unsigned char *data, const unsigned char *end_data,
 			 * 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;
+	encoded_len = d - encoded;
 
-	return d;
+	ret = asn1_encode_length(&data, &data_len, encoded_len);
+	if (ret)  {
+		kfree(encoded);
+		return ERR_PTR(ret);
+	}
+
+	memcpy(data, encoded, encoded_len);
+	kfree(encoded);
+	return data + encoded_len;
 }
 EXPORT_SYMBOL_GPL(asn1_encode_integer);
 
@@ -176,70 +251,6 @@ asn1_encode_oid(unsigned char *data, const unsigned char *end_data,
 }
 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 remaining 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
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 8b7dd73d94c1..ec59f9389a2d 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -38,6 +38,7 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
 	u8 *end_work = scratch + SCRATCH_SIZE;
 	u8 *priv, *pub;
 	u16 priv_len, pub_len;
+	u32 key_handle;
 	int ret;
 
 	priv_len = get_unaligned_be16(src) + 2;
@@ -77,7 +78,8 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
 		goto err;
 	}
 
-	work = asn1_encode_integer(work, end_work, options->keyhandle);
+	key_handle = cpu_to_be32(options->keyhandle);
+	work = asn1_encode_integer(work, end_work, (u8 *)&key_handle, 4);
 	work = asn1_encode_octet_string(work, end_work, pub, pub_len);
 	work = asn1_encode_octet_string(work, end_work, priv, priv_len);
 
-- 
2.45.1

RE: [EXTERNAL] [PATCH v2 2/6] lib: Expand asn1_encode_integer() to variable size integers

[Bharat Bhushan]

> -----Original Message-----
> From: Jarkko Sakkinen <jarkko@kernel.org>
> Sent: Tuesday, May 21, 2024 8:46 AM
> To: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: linux-integrity@vger.kernel.org; keyrings@vger.kernel.org;
> Andreas.Fuchs@infineon.com; James Prestwood <prestwoj@gmail.com>;
> David Woodhouse <dwmw2@infradead.org>; Eric Biggers
> <ebiggers@kernel.org>; James Bottomley
> <James.Bottomley@hansenpartnership.com>; Jarkko Sakkinen
> <jarkko@kernel.org>; David S. Miller <davem@davemloft.net>; open
> list:CRYPTO API <linux-crypto@vger.kernel.org>; open list <linux-
> kernel@vger.kernel.org>; Andrew Morton <akpm@linux-foundation.org>;
> James Bottomley <James.Bottomley@HansenPartnership.com>; Mimi Zohar
> <zohar@linux.ibm.com>; David Howells <dhowells@redhat.com>; Paul Moore
> <paul@paul-moore.com>; James Morris <jmorris@namei.org>; Serge E. Hallyn
> <serge@hallyn.com>; open list:SECURITY SUBSYSTEM <linux-security-
> module@vger.kernel.org>
> Subject: [EXTERNAL] [PATCH v2 2/6] lib: Expand asn1_encode_integer() to
> variable size integers
> 
> ----------------------------------------------------------------------
> Expand asn1_encode_integer() to variable size integers, meaning that it
> will get a blob in big-endian format as integer and length of the blob as
> parameters. This is required in order to encode RSA public key modulus.
> 
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
>  include/linux/asn1_encoder.h              |   3 +-
>  lib/asn1_encoder.c                        | 185 ++++++++++++----------
>  security/keys/trusted-keys/trusted_tpm2.c |   4 +-
>  3 files changed, 103 insertions(+), 89 deletions(-)
> 
> diff --git a/include/linux/asn1_encoder.h b/include/linux/asn1_encoder.h
> index 08cd0c2ad34f..ad5fb18db9e2 100644
> --- a/include/linux/asn1_encoder.h
> +++ b/include/linux/asn1_encoder.h
> @@ -9,9 +9,10 @@
>  #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);
> +		    const u8 *integer, int integer_len);
>  unsigned char *
>  asn1_encode_oid(unsigned char *data, const unsigned char *end_data,
>  		u32 oid[], int oid_len);
> diff --git a/lib/asn1_encoder.c b/lib/asn1_encoder.c
> index 0fd3c454a468..51a2d7010a67 100644
> --- a/lib/asn1_encoder.c
> +++ b/lib/asn1_encoder.c
> @@ -9,12 +9,78 @@
>  #include <linux/bug.h>
>  #include <linux/string.h>
>  #include <linux/module.h>
> +#include <linux/slab.h>
> +
> +/**
> + * asn1_encode_length() - encode a length to follow an ASN.1 tag
> + * @data: pointer to encode at
> + * @data_len: pointer to remaining 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_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
> + * @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
> + * @integer_len:	length in bytes of the integer blob
>   *
>   * This is a simplified encoder: it only currently does
>   * positive integers, but it should be simple enough to add the
> @@ -22,15 +88,17 @@
>   */
>  unsigned char *
>  asn1_encode_integer(unsigned char *data, const unsigned char *end_data,
> -		    s64 integer)
> +		    const u8 *integer, int integer_len)
>  {
>  	int data_len = end_data - data;
> -	unsigned char *d = &data[2];
>  	bool found = false;
> +	unsigned char *d;
> +	int encoded_len;
> +	u8 *encoded;
> +	int ret;
>  	int i;
> 
> -	if (WARN(integer < 0,
> -		 "BUG: integer encode only supports positive integers"))
> +	if (WARN(!integer, "BUG: integer is null"))
>  		return ERR_PTR(-EINVAL);
> 
>  	if (IS_ERR(data))
> @@ -40,17 +108,22 @@ asn1_encode_integer(unsigned char *data, const
> unsigned char *end_data,
>  	if (data_len < 3)
>  		return ERR_PTR(-EINVAL);
> 
> -	/* remaining length where at d (the start of the integer encoding) */
> -	data_len -= 2;
> +	(*data++) = _tag(UNIV, PRIM, INT);

Just for my clarification: 
	First index of "data" is updated here with tag and data pointer incremented.
	Next comment for continuation

> +	data_len--;
> 
> -	data[0] = _tag(UNIV, PRIM, INT);
> -	if (integer == 0) {
> -		*d++ = 0;
> -		goto out;
> +	if (!memchr_inv(integer, 0, integer_len)) {
> +		data[1] = 1;
> +		data[2] = 0;
> +		return &data[2];

Here we are effectively setting second and third index of original "data" pointer as "data" pointer was incremented earlier.
So second index of original "data" pointer is not touched. Also returning 3rd index pointer of original data pointer

Is that intentional?

Thanks
-Bharat

>  	}
> 
> -	for (i = sizeof(integer); i > 0 ; i--) {
> -		int byte = integer >> (8 * (i - 1));
> +	encoded = kzalloc(integer_len, GFP_KERNEL);
> +	if (!encoded)
> +		return ERR_PTR(-ENOMEM);
> +	d = encoded;
> +
> +	for (i = 0; i < integer_len; i++) {
> +		int byte = integer[i];
> 
>  		if (!found && byte == 0)
>  			continue;
> @@ -67,21 +140,23 @@ asn1_encode_integer(unsigned char *data, const
> unsigned char *end_data,
>  			 * 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;
> +	encoded_len = d - encoded;
> 
> -	return d;
> +	ret = asn1_encode_length(&data, &data_len, encoded_len);
> +	if (ret)  {
> +		kfree(encoded);
> +		return ERR_PTR(ret);
> +	}
> +
> +	memcpy(data, encoded, encoded_len);
> +	kfree(encoded);
> +	return data + encoded_len;
>  }
>  EXPORT_SYMBOL_GPL(asn1_encode_integer);
> 
> @@ -176,70 +251,6 @@ asn1_encode_oid(unsigned char *data, const
> unsigned char *end_data,
>  }
>  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 remaining 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
> diff --git a/security/keys/trusted-keys/trusted_tpm2.c
> b/security/keys/trusted-keys/trusted_tpm2.c
> index 8b7dd73d94c1..ec59f9389a2d 100644
> --- a/security/keys/trusted-keys/trusted_tpm2.c
> +++ b/security/keys/trusted-keys/trusted_tpm2.c
> @@ -38,6 +38,7 @@ static int tpm2_key_encode(struct trusted_key_payload
> *payload,
>  	u8 *end_work = scratch + SCRATCH_SIZE;
>  	u8 *priv, *pub;
>  	u16 priv_len, pub_len;
> +	u32 key_handle;
>  	int ret;
> 
>  	priv_len = get_unaligned_be16(src) + 2;
> @@ -77,7 +78,8 @@ static int tpm2_key_encode(struct trusted_key_payload
> *payload,
>  		goto err;
>  	}
> 
> -	work = asn1_encode_integer(work, end_work, options->keyhandle);
> +	key_handle = cpu_to_be32(options->keyhandle);
> +	work = asn1_encode_integer(work, end_work, (u8 *)&key_handle, 4);
>  	work = asn1_encode_octet_string(work, end_work, pub, pub_len);
>  	work = asn1_encode_octet_string(work, end_work, priv, priv_len);
> 
> --
> 2.45.1
> 

Re: [EXTERNAL] [PATCH v2 2/6] lib: Expand asn1_encode_integer() to variable size integers

[Jarkko Sakkinen]

On Tue May 21, 2024 at 8:36 AM EEST, Bharat Bhushan wrote:
> > -	data_len -= 2;
> > +	(*data++) = _tag(UNIV, PRIM, INT);
>
> Just for my clarification: 
> 	First index of "data" is updated here with tag and data pointer incremented.
> 	Next comment for continuation
>
> > +	data_len--;
> > 
> > -	data[0] = _tag(UNIV, PRIM, INT);
> > -	if (integer == 0) {
> > -		*d++ = 0;
> > -		goto out;
> > +	if (!memchr_inv(integer, 0, integer_len)) {
> > +		data[1] = 1;
> > +		data[2] = 0;
> > +		return &data[2];
>
> Here we are effectively setting second and third index of original
> "data" pointer as "data" pointer was incremented earlier.
> So second index of original "data" pointer is not touched. Also
> returning 3rd index pointer of original data pointer
>
> Is that intentional?

No! I read the diff few times, and I think you have a point.

Indices should be 0 (length) and 1 (value). I.e. it forms an encoded
version of zero. The last index what it should be, i.e. return address
of the next byte after the encoded integer.

Thanks for pointing this out.

> Thanks
> -Bharat

BR, Jarkko

[PATCH v2 3/6] tpm: Export tpm2_load_context()

[Jarkko Sakkinen]

Export tpm2_load_context() so that the null key can be loaded as the
parent of a asymmetric TPM2 key.

Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 drivers/char/tpm/tpm.h        |  2 -
 drivers/char/tpm/tpm2-cmd.c   | 77 +++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm2-space.c | 61 ---------------------------
 include/linux/tpm.h           |  2 +
 4 files changed, 79 insertions(+), 63 deletions(-)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 6b8b9956ba69..c9c67fe84f33 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -314,8 +314,6 @@ int tpm_devs_add(struct tpm_chip *chip);
 void tpm_devs_remove(struct tpm_chip *chip);
 int tpm2_save_context(struct tpm_chip *chip, u32 handle, u8 *buf,
 		      unsigned int buf_size, unsigned int *offset);
-int tpm2_load_context(struct tpm_chip *chip, u8 *buf,
-		      unsigned int *offset, u32 *handle);
 
 void tpm_bios_log_setup(struct tpm_chip *chip);
 void tpm_bios_log_teardown(struct tpm_chip *chip);
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 0cdf892ec2a7..eb07a109e2ba 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -370,6 +370,83 @@ void tpm2_flush_context(struct tpm_chip *chip, u32 handle)
 }
 EXPORT_SYMBOL_GPL(tpm2_flush_context);
 
+struct tpm2_context {
+	__be64 sequence;
+	__be32 saved_handle;
+	__be32 hierarchy;
+	__be16 blob_size;
+} __packed;
+
+/**
+ * tpm2_load_context() - Load TPM2 object to the TPM memory
+ * @chip:	TPM chip to use
+ * @buf:	Blob containing TPM2 object.
+ * @offset:	Output variable for the offset in @buf reached.
+ * @handle:	Output variable for the handle of the object in TPM memory.
+ *
+ * Load a blob encrypted with TPM from the memory to the TPM chip.
+ *
+ * Return:
+ * - 0 when the blob is successfully loaded to the TPM.
+ * - -EFAULT if the TPM chip itself fails.
+ * - -ENOENT if the TPM object is replayed.
+ * - -EINVAL if the TPM object is corrupted.
+ */
+int tpm2_load_context(struct tpm_chip *chip, const u8 *buf,
+		      unsigned int *offset, u32 *handle)
+{
+	struct tpm_buf tbuf;
+	struct tpm2_context *ctx;
+	unsigned int body_size;
+	int rc;
+
+	rc = tpm_buf_init(&tbuf, TPM2_ST_NO_SESSIONS, TPM2_CC_CONTEXT_LOAD);
+	if (rc)
+		return rc;
+
+	ctx = (struct tpm2_context *)&buf[*offset];
+	body_size = sizeof(*ctx) + be16_to_cpu(ctx->blob_size);
+	tpm_buf_append(&tbuf, &buf[*offset], body_size);
+
+	rc = tpm_transmit_cmd(chip, &tbuf, 4, NULL);
+	if (rc < 0) {
+		dev_warn(&chip->dev, "%s: failed with a system error %d\n",
+			 __func__, rc);
+		tpm_buf_destroy(&tbuf);
+		return -EFAULT;
+	} else if (tpm2_rc_value(rc) == TPM2_RC_HANDLE ||
+		   rc == TPM2_RC_REFERENCE_H0) {
+		/*
+		 * TPM_RC_HANDLE means that the session context can't
+		 * be loaded because of an internal counter mismatch
+		 * that makes the TPM think there might have been a
+		 * replay.  This might happen if the context was saved
+		 * and loaded outside the space.
+		 *
+		 * TPM_RC_REFERENCE_H0 means the session has been
+		 * flushed outside the space
+		 */
+		*handle = 0;
+		tpm_buf_destroy(&tbuf);
+		return -ENOENT;
+	} else if (tpm2_rc_value(rc) == TPM2_RC_INTEGRITY) {
+		tpm_buf_destroy(&tbuf);
+		return -EINVAL;
+	} else if (rc > 0) {
+		dev_warn(&chip->dev, "%s: failed with a TPM error 0x%04X\n",
+			 __func__, rc);
+		tpm_buf_destroy(&tbuf);
+		return -EFAULT;
+	}
+
+	*handle = be32_to_cpup((__be32 *)&tbuf.data[TPM_HEADER_SIZE]);
+	*offset += body_size;
+
+	tpm_buf_destroy(&tbuf);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(tpm2_load_context);
+
 struct tpm2_get_cap_out {
 	u8 more_data;
 	__be32 subcap_id;
diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
index 4892d491da8d..708c6e4d64cd 100644
--- a/drivers/char/tpm/tpm2-space.c
+++ b/drivers/char/tpm/tpm2-space.c
@@ -21,13 +21,6 @@ enum tpm2_handle_types {
 	TPM2_HT_TRANSIENT	= 0x80000000,
 };
 
-struct tpm2_context {
-	__be64 sequence;
-	__be32 saved_handle;
-	__be32 hierarchy;
-	__be16 blob_size;
-} __packed;
-
 static void tpm2_flush_sessions(struct tpm_chip *chip, struct tpm_space *space)
 {
 	int i;
@@ -68,60 +61,6 @@ void tpm2_del_space(struct tpm_chip *chip, struct tpm_space *space)
 	kfree(space->session_buf);
 }
 
-int tpm2_load_context(struct tpm_chip *chip, u8 *buf,
-		      unsigned int *offset, u32 *handle)
-{
-	struct tpm_buf tbuf;
-	struct tpm2_context *ctx;
-	unsigned int body_size;
-	int rc;
-
-	rc = tpm_buf_init(&tbuf, TPM2_ST_NO_SESSIONS, TPM2_CC_CONTEXT_LOAD);
-	if (rc)
-		return rc;
-
-	ctx = (struct tpm2_context *)&buf[*offset];
-	body_size = sizeof(*ctx) + be16_to_cpu(ctx->blob_size);
-	tpm_buf_append(&tbuf, &buf[*offset], body_size);
-
-	rc = tpm_transmit_cmd(chip, &tbuf, 4, NULL);
-	if (rc < 0) {
-		dev_warn(&chip->dev, "%s: failed with a system error %d\n",
-			 __func__, rc);
-		tpm_buf_destroy(&tbuf);
-		return -EFAULT;
-	} else if (tpm2_rc_value(rc) == TPM2_RC_HANDLE ||
-		   rc == TPM2_RC_REFERENCE_H0) {
-		/*
-		 * TPM_RC_HANDLE means that the session context can't
-		 * be loaded because of an internal counter mismatch
-		 * that makes the TPM think there might have been a
-		 * replay.  This might happen if the context was saved
-		 * and loaded outside the space.
-		 *
-		 * TPM_RC_REFERENCE_H0 means the session has been
-		 * flushed outside the space
-		 */
-		*handle = 0;
-		tpm_buf_destroy(&tbuf);
-		return -ENOENT;
-	} else if (tpm2_rc_value(rc) == TPM2_RC_INTEGRITY) {
-		tpm_buf_destroy(&tbuf);
-		return -EINVAL;
-	} else if (rc > 0) {
-		dev_warn(&chip->dev, "%s: failed with a TPM error 0x%04X\n",
-			 __func__, rc);
-		tpm_buf_destroy(&tbuf);
-		return -EFAULT;
-	}
-
-	*handle = be32_to_cpup((__be32 *)&tbuf.data[TPM_HEADER_SIZE]);
-	*offset += body_size;
-
-	tpm_buf_destroy(&tbuf);
-	return 0;
-}
-
 int tpm2_save_context(struct tpm_chip *chip, u32 handle, u8 *buf,
 		      unsigned int buf_size, unsigned int *offset)
 {
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index c17e4efbb2e5..2f25ca07127b 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -466,6 +466,8 @@ extern int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
 extern int tpm_get_random(struct tpm_chip *chip, u8 *data, size_t max);
 extern struct tpm_chip *tpm_default_chip(void);
 void tpm2_flush_context(struct tpm_chip *chip, u32 handle);
+int tpm2_load_context(struct tpm_chip *chip, const u8 *buf,
+		      unsigned int *offset, u32 *handle);
 
 static inline void tpm_buf_append_empty_auth(struct tpm_buf *buf, u32 handle)
 {
-- 
2.45.1

[PATCH v2 4/6] KEYS: trusted: Move tpm2_key_decode() to the TPM driver

[Jarkko Sakkinen]

Move tpm2_key_decode() to the TPM driver and export the symbols to make
them callable from trusted keys. It can re-used for asymmetric keys.

Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v2:
Do not allocate blob twice. Use the one inside struct tpm2_key.
---
 drivers/char/tpm/Kconfig                      |   1 +
 drivers/char/tpm/Makefile                     |   5 +
 drivers/char/tpm/tpm2_key.c                   | 111 +++++++++++++++
 .../char/tpm}/tpm2key.asn1                    |   0
 include/crypto/tpm2_key.h                     |  33 +++++
 security/keys/trusted-keys/Makefile           |   2 -
 security/keys/trusted-keys/trusted_tpm2.c     | 127 +++---------------
 7 files changed, 167 insertions(+), 112 deletions(-)
 create mode 100644 drivers/char/tpm/tpm2_key.c
 rename {security/keys/trusted-keys => drivers/char/tpm}/tpm2key.asn1 (100%)
 create mode 100644 include/crypto/tpm2_key.h

diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index e63a6a17793c..de2f4093c939 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -7,6 +7,7 @@ menuconfig TCG_TPM
 	tristate "TPM Hardware Support"
 	depends on HAS_IOMEM
 	imply SECURITYFS
+	select ASN1
 	select CRYPTO
 	select CRYPTO_HASH_INFO
 	help
diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 4c695b0388f3..071437058ef6 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -17,6 +17,11 @@ tpm-y += eventlog/tpm1.o
 tpm-y += eventlog/tpm2.o
 tpm-y += tpm-buf.o
 
+# TPM2 Asymmetric Key
+$(obj)/trusted_tpm2.o: $(obj)/tpm2key.asn1.h
+tpm-y += tpm2key.asn1.o
+tpm-y += tpm2_key.o
+
 tpm-$(CONFIG_TCG_TPM2_HMAC) += tpm2-sessions.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o eventlog/acpi.o
 tpm-$(CONFIG_EFI) += eventlog/efi.o
diff --git a/drivers/char/tpm/tpm2_key.c b/drivers/char/tpm/tpm2_key.c
new file mode 100644
index 000000000000..0112362e432e
--- /dev/null
+++ b/drivers/char/tpm/tpm2_key.c
@@ -0,0 +1,111 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/oid_registry.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <crypto/tpm2_key.h>
+#include <asm/unaligned.h>
+#include "tpm2key.asn1.h"
+
+#undef pr_fmt
+#define pr_fmt(fmt) "tpm2_key: "fmt
+
+int tpm2_key_parent(void *context, size_t hdrlen,
+		    unsigned char tag,
+		    const void *value, size_t vlen)
+{
+	struct tpm2_key *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 *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 *ctx = context;
+
+	ctx->priv = value;
+	ctx->priv_len = vlen;
+
+	return 0;
+}
+
+/**
+ * tpm_key_decode() - Decode TPM2 ASN.1 key.
+ * @src:		ASN.1 source.
+ * @src_len:		ASN.1 source length.
+ * @key:		TPM2 asymmetric key.
+ * @max_key_len:	Maximum length of the TPM2 asymmetric key.
+ *
+ * Decodes TPM2 ASN.1 key on success. Returns POSIX error code on failure.
+ */
+int tpm2_key_decode(const u8 *src, u32 src_len, struct tpm2_key *key,
+		    u32 max_key_len)
+{
+	struct tpm2_key ctx;
+	u32 blob_len;
+	int ret;
+
+	memset(&ctx, 0, sizeof(ctx));
+
+	ret = asn1_ber_decoder(&tpm2key_decoder, &ctx, src, src_len);
+	if (ret < 0)
+		return ret;
+
+	blob_len = ctx.priv_len + ctx.pub_len;
+	if (blob_len > max_key_len)
+		return -E2BIG;
+
+	ctx.blob_len = blob_len;
+	ctx.blob = kmalloc(blob_len, GFP_KERNEL);
+	if (!ctx.blob)
+		return -ENOMEM;
+
+	memcpy((void *)ctx.blob, ctx.priv, ctx.priv_len);
+	memcpy((void *)ctx.blob + ctx.priv_len, ctx.pub, ctx.pub_len);
+	ctx.priv = ctx.blob;
+	ctx.pub = ctx.blob + ctx.priv_len;
+
+	memcpy(key, &ctx, sizeof(ctx));
+	return 0;
+}
+EXPORT_SYMBOL_GPL(tpm2_key_decode);
diff --git a/security/keys/trusted-keys/tpm2key.asn1 b/drivers/char/tpm/tpm2key.asn1
similarity index 100%
rename from security/keys/trusted-keys/tpm2key.asn1
rename to drivers/char/tpm/tpm2key.asn1
diff --git a/include/crypto/tpm2_key.h b/include/crypto/tpm2_key.h
new file mode 100644
index 000000000000..acf41b2e0c92
--- /dev/null
+++ b/include/crypto/tpm2_key.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __LINUX_TPM2_KEY_H__
+#define __LINUX_TPM2_KEY_H__
+
+#include <linux/slab.h>
+
+/*
+ * TPM2 ASN.1 key
+ */
+struct tpm2_key {
+	u32 parent;
+	const u8 *blob;
+	u32 blob_len;
+	const u8 *pub;
+	u32 pub_len;
+	const u8 *priv;
+	u32 priv_len;
+};
+
+int tpm2_key_decode(const u8 *src, u32 src_len, struct tpm2_key *key,
+		    u32 max_key_len);
+
+/**
+ * tpm2_key_free() - Release TPM2 asymmetric key resources and reset values
+ * @key:	TPM2 asymmetric key.
+ */
+static inline void tpm2_key_destroy(struct tpm2_key *key)
+{
+	kfree(key->blob);
+	memset(key, 0, sizeof(*key));
+}
+
+#endif /* __LINUX_TPM2_KEY_H__ */
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/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index ec59f9389a2d..f255388d32b8 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},
@@ -28,9 +27,9 @@ static struct tpm2_hash tpm2_hash_map[] = {
 
 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)
+static int tpm2_trusted_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);
@@ -100,106 +99,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 -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;
-}
-
 /**
  * tpm2_buf_append_auth() - append TPMS_AUTH_COMMAND to the buffer.
  *
@@ -349,7 +248,8 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 		goto out;
 	}
 
-	blob_len = tpm2_key_encode(payload, options, &buf.data[offset], blob_len);
+	blob_len = tpm2_trusted_key_encode(payload, options, &buf.data[offset],
+					   blob_len);
 
 out:
 	tpm_buf_destroy(&sized);
@@ -389,21 +289,27 @@ 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;
+	struct tpm2_key key;
+	struct tpm_buf buf;
+	const u8 *blob, *pub;
 	int rc;
 	u32 attrs;
 
-	rc = tpm2_key_decode(payload, options, &blob);
+	rc = tpm2_key_decode(payload->blob, payload->blob_len, &key, PAGE_SIZE);
 	if (rc) {
 		/* old form */
 		blob = payload->blob;
 		payload->old_format = 1;
+	} else {
+		blob = key.blob;
 	}
 
+	if (!blob)
+		return -ENOMEM;
+
 	/* new format carries keyhandle but old format doesn't */
 	if (!options->keyhandle)
 		return -EINVAL;
@@ -467,7 +373,8 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 
 out:
 	if (blob != payload->blob)
-		kfree(blob);
+		tpm2_key_destroy(&key);
+
 	tpm_buf_destroy(&buf);
 
 	if (rc > 0)
-- 
2.45.1

Re: [PATCH v2 4/6] KEYS: trusted: Move tpm2_key_decode() to the TPM driver

[James Bottomley]

On Tue, 2024-05-21 at 06:16 +0300, Jarkko Sakkinen wrote:
[...]
> diff --git a/include/crypto/tpm2_key.h b/include/crypto/tpm2_key.h
> new file mode 100644
> index 000000000000..acf41b2e0c92
> --- /dev/null
> +++ b/include/crypto/tpm2_key.h
> @@ -0,0 +1,33 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef __LINUX_TPM2_KEY_H__
> +#define __LINUX_TPM2_KEY_H__
> +
> +#include <linux/slab.h>
> +
> +/*
> + * TPM2 ASN.1 key
> + */
> +struct tpm2_key {
> +       u32 parent;
> +       const u8 *blob;
> +       u32 blob_len;
> +       const u8 *pub;
> +       u32 pub_len;
> +       const u8 *priv;
> +       u32 priv_len;
> +};
> +
> +int tpm2_key_decode(const u8 *src, u32 src_len, struct tpm2_key
> *key,
> +                   u32 max_key_len);

I don't think this is a good idea.  Trusted keys already have a pre-
defined max payload size (MAX_BLOB_SIZE in include/keys/trusted-type.h)
and I've already had to increase this several times because once you
get policy attached to a key, it can get pretty big (over a page). 
Exactly the same thing will happen to asymmetric keys as well, so it
does make sense that they share the same maximum (probably in a more
generic header, though).

Since the code already right sizes the allocation and all we check with
this is whether it's over a pre-defined maximum, it's way easier if
that maximum is defined in a header rather than passed in in several
places making increasing the maximum really hard because you have to
chase all the threading.

James

Re: [PATCH v2 4/6] KEYS: trusted: Move tpm2_key_decode() to the TPM driver

[Jarkko Sakkinen]

On Tue May 21, 2024 at 9:18 PM EEST, James Bottomley wrote:
> On Tue, 2024-05-21 at 06:16 +0300, Jarkko Sakkinen wrote:
> [...]
> > diff --git a/include/crypto/tpm2_key.h b/include/crypto/tpm2_key.h
> > new file mode 100644
> > index 000000000000..acf41b2e0c92
> > --- /dev/null
> > +++ b/include/crypto/tpm2_key.h
> > @@ -0,0 +1,33 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +#ifndef __LINUX_TPM2_KEY_H__
> > +#define __LINUX_TPM2_KEY_H__
> > +
> > +#include <linux/slab.h>
> > +
> > +/*
> > + * TPM2 ASN.1 key
> > + */
> > +struct tpm2_key {
> > +       u32 parent;
> > +       const u8 *blob;
> > +       u32 blob_len;
> > +       const u8 *pub;
> > +       u32 pub_len;
> > +       const u8 *priv;
> > +       u32 priv_len;
> > +};
> > +
> > +int tpm2_key_decode(const u8 *src, u32 src_len, struct tpm2_key
> > *key,
> > +                   u32 max_key_len);
>
> I don't think this is a good idea.  Trusted keys already have a pre-
> defined max payload size (MAX_BLOB_SIZE in include/keys/trusted-type.h)
> and I've already had to increase this several times because once you
> get policy attached to a key, it can get pretty big (over a page). 
> Exactly the same thing will happen to asymmetric keys as well, so it
> does make sense that they share the same maximum (probably in a more
> generic header, though).

ECDSA and RSA have different space requirements. With that solution you
actually max out space requirements given same cap for everything.

Even tpm2_key_ecdsa should use a different value than tpm2_key_rsa to
save memory.

> Since the code already right sizes the allocation and all we check with
> this is whether it's over a pre-defined maximum, it's way easier if
> that maximum is defined in a header rather than passed in in several
> places making increasing the maximum really hard because you have to
> chase all the threading.

You don't save a single byte of memory with any constant that dictates
the size requirements for multiple modules in two disjoint subsystems.

You are maximizing the use of memory.

> James

BR, Jarkko

Re: [PATCH v2 4/6] KEYS: trusted: Move tpm2_key_decode() to the TPM driver

[David Howells]

Jarkko Sakkinen <jarkko@kernel.org> wrote:

> On Tue May 21, 2024 at 9:18 PM EEST, James Bottomley wrote:
> ...
> You don't save a single byte of memory with any constant that dictates
> the size requirements for multiple modules in two disjoint subsystems.

I think James is just suggesting you replace your limit argument with a
constant not that you always allocate that amount of memory.  What the limit
should be, OTOH, is up for discussion, but PAGE_SIZE seems not unreasonable.

David

Re: [PATCH v2 4/6] KEYS: trusted: Move tpm2_key_decode() to the TPM driver

[James Bottomley]

On Tue, 2024-05-21 at 22:44 +0100, David Howells wrote:
> Jarkko Sakkinen <jarkko@kernel.org> wrote:
> 
> > On Tue May 21, 2024 at 9:18 PM EEST, James Bottomley wrote:
> > ...
> > You don't save a single byte of memory with any constant that
> > dictates the size requirements for multiple modules in two disjoint
> > subsystems.
> 
> I think James is just suggesting you replace your limit argument with
> a constant not that you always allocate that amount of memory.

Exactly.  All we use it for is the -E2BIG check to ensure user space
isn't allowed to run away with loads of kernel memory.

> What the limit should be, OTOH, is up for discussion, but PAGE_SIZE
> seems not unreasonable.

A page is fine currently (MAX_BLOB_SIZE is 512).  However, it may be
too small for some of the complex policies when they're introduced. 
I'm not bothered about what it currently is, I just want it to be able
to be increased easily when the time comes.

James

Re: [PATCH v2 4/6] KEYS: trusted: Move tpm2_key_decode() to the TPM driver

[Jarkko Sakkinen]

On Wed May 22, 2024 at 12:59 AM EEST, James Bottomley wrote:
> On Tue, 2024-05-21 at 22:44 +0100, David Howells wrote:
> > Jarkko Sakkinen <jarkko@kernel.org> wrote:
> > 
> > > On Tue May 21, 2024 at 9:18 PM EEST, James Bottomley wrote:
> > > ...
> > > You don't save a single byte of memory with any constant that
> > > dictates the size requirements for multiple modules in two disjoint
> > > subsystems.
> > 
> > I think James is just suggesting you replace your limit argument with
> > a constant not that you always allocate that amount of memory.
>
> Exactly.  All we use it for is the -E2BIG check to ensure user space
> isn't allowed to run away with loads of kernel memory.

Not true.

It did return -EINVAL. This patch changes it to -E2BIG.

>
> > What the limit should be, OTOH, is up for discussion, but PAGE_SIZE
> > seems not unreasonable.
>
> A page is fine currently (MAX_BLOB_SIZE is 512).  However, it may be
> too small for some of the complex policies when they're introduced. 
> I'm not bothered about what it currently is, I just want it to be able
> to be increased easily when the time comes.

MAX_BLOB_SIZE would be used to cap key blob, not the policy.

And you are ignoring it yourself too in the driver.


> James


BR, Jarkko

Re: [PATCH v2 4/6] KEYS: trusted: Move tpm2_key_decode() to the TPM driver

[Jarkko Sakkinen]

On Wed May 22, 2024 at 1:45 AM EEST, Jarkko Sakkinen wrote:
> On Wed May 22, 2024 at 12:59 AM EEST, James Bottomley wrote:
> > On Tue, 2024-05-21 at 22:44 +0100, David Howells wrote:
> > > Jarkko Sakkinen <jarkko@kernel.org> wrote:
> > > 
> > > > On Tue May 21, 2024 at 9:18 PM EEST, James Bottomley wrote:
> > > > ...
> > > > You don't save a single byte of memory with any constant that
> > > > dictates the size requirements for multiple modules in two disjoint
> > > > subsystems.
> > > 
> > > I think James is just suggesting you replace your limit argument with
> > > a constant not that you always allocate that amount of memory.
> >
> > Exactly.  All we use it for is the -E2BIG check to ensure user space
> > isn't allowed to run away with loads of kernel memory.
>
> Not true.
>
> It did return -EINVAL. This patch changes it to -E2BIG.
>
> >
> > > What the limit should be, OTOH, is up for discussion, but PAGE_SIZE
> > > seems not unreasonable.
> >
> > A page is fine currently (MAX_BLOB_SIZE is 512).  However, it may be
> > too small for some of the complex policies when they're introduced. 
> > I'm not bothered about what it currently is, I just want it to be able
> > to be increased easily when the time comes.
>
> MAX_BLOB_SIZE would be used to cap key blob, not the policy.
>
> And you are ignoring it yourself too in the driver.

Obviously policy is part of the key blob i.e. expected value for that.

... but that does not reduce space requirements to rsa asymmetric keys.
It increases them but I think at this point 8192 is good starting point.
And it cap can be scaled later.

Being a parameter also allows to have even kernel-command line or sysfs
parameter and stuff like that. It is robust not a bad choice.

BR, Jarkko

Re: [PATCH v2 4/6] KEYS: trusted: Move tpm2_key_decode() to the TPM driver

[Jarkko Sakkinen]

On Wed May 22, 2024 at 12:44 AM EEST, David Howells wrote:
> Jarkko Sakkinen <jarkko@kernel.org> wrote:
>
> > On Tue May 21, 2024 at 9:18 PM EEST, James Bottomley wrote:
> > ...
> > You don't save a single byte of memory with any constant that dictates
> > the size requirements for multiple modules in two disjoint subsystems.
>
> I think James is just suggesting you replace your limit argument with a
> constant not that you always allocate that amount of memory.  What the limit
> should be, OTOH, is up for discussion, but PAGE_SIZE seems not unreasonable.

When the decoder for ASN.1 was part of trusted keys, the check used to
be:

	if (ctx.priv_len + ctx.pub_len > MAX_BLOB_SIZE)
		return -EINVAL;

And MAX_BLOB_SIZE is only 512 bytes, which does not fit event 2048 bit
RSA key but that 512 bytes cap seems to be just fine for trusted keys.

So the new check is:

	if (blob_len > max_key_len)
		return -E2BIG;

1. Too big value is not invalid value, thus -E2BIG. It is has also
   shown to be practically useful while testing this key type.
2. tpm2_key_rsa needs up to 8192 bytes for a blob to fit 4096-bit
   RSA key. 

Just saying but there is also primary null key allocated by the driver.
And neither driver uses MAX_BLOB_SiZE. It uses value 8x MAX_BLOB_SIZE
i.e. 4096 bytes so not really following the idea suggested.

Finaly, there is three completely separate algorithms:

- KEYEDHASH (trusted_keys)
- RSA (tpm2_key_rsa)
- ECDSA (driver)§
	
With all this put together it is just common sense to have parametrized
cap value, and it would have no logic at all to treat them unified way.

For tpm2_key_rsa I will define for clarity:

#define TPM2_KEY_RSA_MAX_SIZE 8192

For tpm2_key_ecdsa you would define

#define TPM2_KEY_ECDSA_MAX_SIZE 4096

So yeah, this is how I will proceed because it is really the only
senseful way to proceed.

>
> David

BR, Jarkko

[PATCH v2 5/6] tpm: tpm2_key: Extend parser to TPM_LoadableKey

[Jarkko Sakkinen]

Extend parser to TPM_LoadableKey. Add field for oid to struct tpm2_key
so that callers can differentiate different key types.

Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 drivers/char/tpm/tpm2_key.c               | 14 +++++++++++---
 include/crypto/tpm2_key.h                 |  2 ++
 security/keys/trusted-keys/trusted_tpm2.c |  4 ++++
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/char/tpm/tpm2_key.c b/drivers/char/tpm/tpm2_key.c
index 0112362e432e..59797dc232f1 100644
--- a/drivers/char/tpm/tpm2_key.c
+++ b/drivers/char/tpm/tpm2_key.c
@@ -32,16 +32,24 @@ int tpm2_key_type(void *context, size_t hdrlen,
 		  const void *value, size_t vlen)
 {
 	enum OID oid = look_up_OID(value, vlen);
-
-	if (oid != OID_TPMSealedData) {
+	struct tpm2_key *key = context;
+
+	switch (oid) {
+	case OID_TPMSealedData:
+		pr_info("TPMSealedData\n");
+		break;
+	case OID_TPMLoadableKey:
+		pr_info("TPMLodableKey\n");
+		break;
+	default:
 		char buffer[50];
-
 		sprint_oid(value, vlen, buffer, sizeof(buffer));
 		pr_debug("OID is \"%s\" which is not TPMSealedData\n",
 			 buffer);
 		return -EINVAL;
 	}
 
+	key->oid = oid;
 	return 0;
 }
 
diff --git a/include/crypto/tpm2_key.h b/include/crypto/tpm2_key.h
index acf41b2e0c92..2d2434233000 100644
--- a/include/crypto/tpm2_key.h
+++ b/include/crypto/tpm2_key.h
@@ -2,12 +2,14 @@
 #ifndef __LINUX_TPM2_KEY_H__
 #define __LINUX_TPM2_KEY_H__
 
+#include <linux/oid_registry.h>
 #include <linux/slab.h>
 
 /*
  * TPM2 ASN.1 key
  */
 struct tpm2_key {
+	enum OID oid;
 	u32 parent;
 	const u8 *blob;
 	u32 blob_len;
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index f255388d32b8..ce4c667c3ee3 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -305,6 +305,10 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 		payload->old_format = 1;
 	} else {
 		blob = key.blob;
+		if (key.oid != OID_TPMSealedData) {
+			tpm2_key_destroy(&key);
+			return -EINVAL;
+		}
 	}
 
 	if (!blob)
-- 
2.45.1

RE: [EXTERNAL] [PATCH v2 5/6] tpm: tpm2_key: Extend parser to TPM_LoadableKey

[Bharat Bhushan]

> -----Original Message-----
> From: Jarkko Sakkinen <jarkko@kernel.org>
> Sent: Tuesday, May 21, 2024 8:47 AM
> To: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: linux-integrity@vger.kernel.org; keyrings@vger.kernel.org;
> Andreas.Fuchs@infineon.com; James Prestwood <prestwoj@gmail.com>;
> David Woodhouse <dwmw2@infradead.org>; Eric Biggers
> <ebiggers@kernel.org>; James Bottomley
> <James.Bottomley@hansenpartnership.com>; Jarkko Sakkinen
> <jarkko@kernel.org>; David S. Miller <davem@davemloft.net>; open
> list:CRYPTO API <linux-crypto@vger.kernel.org>; open list <linux-
> kernel@vger.kernel.org>; Peter Huewe <peterhuewe@gmx.de>; Jason
> Gunthorpe <jgg@ziepe.ca>; James Bottomley
> <James.Bottomley@HansenPartnership.com>; Mimi Zohar
> <zohar@linux.ibm.com>; David Howells <dhowells@redhat.com>; Paul Moore
> <paul@paul-moore.com>; James Morris <jmorris@namei.org>; Serge E. Hallyn
> <serge@hallyn.com>; open list:SECURITY SUBSYSTEM <linux-security-
> module@vger.kernel.org>
> Subject: [EXTERNAL] [PATCH v2 5/6] tpm: tpm2_key: Extend parser to
> TPM_LoadableKey
> 
> ----------------------------------------------------------------------
> Extend parser to TPM_LoadableKey. Add field for oid to struct tpm2_key
> so that callers can differentiate different key types.
> 
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
>  drivers/char/tpm/tpm2_key.c               | 14 +++++++++++---
>  include/crypto/tpm2_key.h                 |  2 ++
>  security/keys/trusted-keys/trusted_tpm2.c |  4 ++++
>  3 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm2_key.c b/drivers/char/tpm/tpm2_key.c
> index 0112362e432e..59797dc232f1 100644
> --- a/drivers/char/tpm/tpm2_key.c
> +++ b/drivers/char/tpm/tpm2_key.c
> @@ -32,16 +32,24 @@ int tpm2_key_type(void *context, size_t hdrlen,
>  		  const void *value, size_t vlen)
>  {
>  	enum OID oid = look_up_OID(value, vlen);
> -
> -	if (oid != OID_TPMSealedData) {
> +	struct tpm2_key *key = context;
> +
> +	switch (oid) {
> +	case OID_TPMSealedData:
> +		pr_info("TPMSealedData\n");
> +		break;
> +	case OID_TPMLoadableKey:
> +		pr_info("TPMLodableKey\n");
> +		break;
> +	default:
>  		char buffer[50];
> -
>  		sprint_oid(value, vlen, buffer, sizeof(buffer));
>  		pr_debug("OID is \"%s\" which is not TPMSealedData\n",
>  			 buffer);

Maybe extend this print to say "neither TPMSealedData nor TPMLodableKey"

Thanks
-Bharat

>  		return -EINVAL;
>  	}
> 
> +	key->oid = oid;
>  	return 0;
>  }
> 
> diff --git a/include/crypto/tpm2_key.h b/include/crypto/tpm2_key.h
> index acf41b2e0c92..2d2434233000 100644
> --- a/include/crypto/tpm2_key.h
> +++ b/include/crypto/tpm2_key.h
> @@ -2,12 +2,14 @@
>  #ifndef __LINUX_TPM2_KEY_H__
>  #define __LINUX_TPM2_KEY_H__
> 
> +#include <linux/oid_registry.h>
>  #include <linux/slab.h>
> 
>  /*
>   * TPM2 ASN.1 key
>   */
>  struct tpm2_key {
> +	enum OID oid;
>  	u32 parent;
>  	const u8 *blob;
>  	u32 blob_len;
> diff --git a/security/keys/trusted-keys/trusted_tpm2.c
> b/security/keys/trusted-keys/trusted_tpm2.c
> index f255388d32b8..ce4c667c3ee3 100644
> --- a/security/keys/trusted-keys/trusted_tpm2.c
> +++ b/security/keys/trusted-keys/trusted_tpm2.c
> @@ -305,6 +305,10 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
>  		payload->old_format = 1;
>  	} else {
>  		blob = key.blob;
> +		if (key.oid != OID_TPMSealedData) {
> +			tpm2_key_destroy(&key);
> +			return -EINVAL;
> +		}
>  	}
> 
>  	if (!blob)
> --
> 2.45.1
> 

Re: [EXTERNAL] [PATCH v2 5/6] tpm: tpm2_key: Extend parser to TPM_LoadableKey

[Jarkko Sakkinen]

On Tue May 21, 2024 at 8:47 AM EEST, Bharat Bhushan wrote:
>
>
> > -----Original Message-----
> > From: Jarkko Sakkinen <jarkko@kernel.org>
> > Sent: Tuesday, May 21, 2024 8:47 AM
> > To: Herbert Xu <herbert@gondor.apana.org.au>
> > Cc: linux-integrity@vger.kernel.org; keyrings@vger.kernel.org;
> > Andreas.Fuchs@infineon.com; James Prestwood <prestwoj@gmail.com>;
> > David Woodhouse <dwmw2@infradead.org>; Eric Biggers
> > <ebiggers@kernel.org>; James Bottomley
> > <James.Bottomley@hansenpartnership.com>; Jarkko Sakkinen
> > <jarkko@kernel.org>; David S. Miller <davem@davemloft.net>; open
> > list:CRYPTO API <linux-crypto@vger.kernel.org>; open list <linux-
> > kernel@vger.kernel.org>; Peter Huewe <peterhuewe@gmx.de>; Jason
> > Gunthorpe <jgg@ziepe.ca>; James Bottomley
> > <James.Bottomley@HansenPartnership.com>; Mimi Zohar
> > <zohar@linux.ibm.com>; David Howells <dhowells@redhat.com>; Paul Moore
> > <paul@paul-moore.com>; James Morris <jmorris@namei.org>; Serge E. Hallyn
> > <serge@hallyn.com>; open list:SECURITY SUBSYSTEM <linux-security-
> > module@vger.kernel.org>
> > Subject: [EXTERNAL] [PATCH v2 5/6] tpm: tpm2_key: Extend parser to
> > TPM_LoadableKey
> > 
> > ----------------------------------------------------------------------
> > Extend parser to TPM_LoadableKey. Add field for oid to struct tpm2_key
> > so that callers can differentiate different key types.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > ---
> >  drivers/char/tpm/tpm2_key.c               | 14 +++++++++++---
> >  include/crypto/tpm2_key.h                 |  2 ++
> >  security/keys/trusted-keys/trusted_tpm2.c |  4 ++++
> >  3 files changed, 17 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/char/tpm/tpm2_key.c b/drivers/char/tpm/tpm2_key.c
> > index 0112362e432e..59797dc232f1 100644
> > --- a/drivers/char/tpm/tpm2_key.c
> > +++ b/drivers/char/tpm/tpm2_key.c
> > @@ -32,16 +32,24 @@ int tpm2_key_type(void *context, size_t hdrlen,
> >  		  const void *value, size_t vlen)
> >  {
> >  	enum OID oid = look_up_OID(value, vlen);
> > -
> > -	if (oid != OID_TPMSealedData) {
> > +	struct tpm2_key *key = context;
> > +
> > +	switch (oid) {
> > +	case OID_TPMSealedData:
> > +		pr_info("TPMSealedData\n");
> > +		break;
> > +	case OID_TPMLoadableKey:
> > +		pr_info("TPMLodableKey\n");

These should be pr_debug() (forgot to change).

> > +		break;
> > +	default:
> >  		char buffer[50];
> > -
> >  		sprint_oid(value, vlen, buffer, sizeof(buffer));
> >  		pr_debug("OID is \"%s\" which is not TPMSealedData\n",
> >  			 buffer);
>
> Maybe extend this print to say "neither TPMSealedData nor TPMLodableKey"

Right, I tried to apply minimal delta to patches where existing code
needs to be carved to a new form :-)

I think it could be just "OID \"%s\" is unknown"?

BR, Jarkko

[PATCH v2 6/6] keys: asymmetric: ASYMMETRIC_TPM2_KEY_RSA_SUBTYPE

[Jarkko Sakkinen]

From: James Prestwood <prestwoj@gmail.com>

Based on earlier work by James Prestwood.

Add ASN.1 compatible asymmetric TPM2 RSA key subtype:

1. Signing and decryption (with the private key) is handled by
   TPM2_RSA_Decrypt.
2. Encryption (with the public key) is handled by the kernel RSA
   implementation.

Link: https://lore.kernel.org/all/20200518172704.29608-1-prestwoj@gmail.com/
Signed-off-by: James Prestwood <prestwoj@gmail.com>
Co-developed-by: Jarkko Sakkinen <jarkko@kernel.org>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v2:
* Remove two spurios pr_info() messsages that I forgot to remove.
* Clean up padding functions and add additional checks for length
  also in tpm2_unpad_pcks1().
* Add the missing success check kzalloc() in tpm2_key_rsa_decrypt().
* Check that params->out_len for capacity before copying the result.
---
 crypto/asymmetric_keys/Kconfig        |  16 +
 crypto/asymmetric_keys/Makefile       |   1 +
 crypto/asymmetric_keys/tpm2_key_rsa.c | 698 ++++++++++++++++++++++++++
 include/linux/tpm.h                   |   2 +
 4 files changed, 717 insertions(+)
 create mode 100644 crypto/asymmetric_keys/tpm2_key_rsa.c

diff --git a/crypto/asymmetric_keys/Kconfig b/crypto/asymmetric_keys/Kconfig
index e1345b8f39f1..4d14bb0c346e 100644
--- a/crypto/asymmetric_keys/Kconfig
+++ b/crypto/asymmetric_keys/Kconfig
@@ -15,6 +15,7 @@ config ASYMMETRIC_PUBLIC_KEY_SUBTYPE
 	select MPILIB
 	select CRYPTO_HASH_INFO
 	select CRYPTO_AKCIPHER
+	select CRYPTO_RSA
 	select CRYPTO_SIG
 	select CRYPTO_HASH
 	help
@@ -23,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_RSA_SUBTYPE
+	tristate "Asymmetric TPM2 RSA crypto algorithm subtype"
+	depends on TCG_TPM
+	select TCG_TPM2_HMAC
+	select CRYPTO_RSA
+	select CRYPTO_SHA256
+	select CRYPTO_HASH_INFO
+	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 X509_CERTIFICATE_PARSER
 	tristate "X.509 certificate parser"
 	depends on ASYMMETRIC_PUBLIC_KEY_SUBTYPE
diff --git a/crypto/asymmetric_keys/Makefile b/crypto/asymmetric_keys/Makefile
index bc65d3b98dcb..c6da84607824 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_RSA_SUBTYPE) += tpm2_key_rsa.o
 
 #
 # X.509 Certificate handling
diff --git a/crypto/asymmetric_keys/tpm2_key_rsa.c b/crypto/asymmetric_keys/tpm2_key_rsa.c
new file mode 100644
index 000000000000..32250ff38268
--- /dev/null
+++ b/crypto/asymmetric_keys/tpm2_key_rsa.c
@@ -0,0 +1,698 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/* TPM2 asymmetric public-key crypto subtype
+ *
+ * See Documentation/crypto/asymmetric-keys.rst
+ *
+ * Copyright (c) 2020 Intel Corporation
+ */
+
+#include <asm/unaligned.h>
+#include <crypto/akcipher.h>
+#include <crypto/public_key.h>
+#include <crypto/rsa-pkcs1pad.h>
+#include <crypto/tpm2_key.h>
+#include <keys/asymmetric-parser.h>
+#include <keys/asymmetric-subtype.h>
+#include <keys/trusted-type.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_rsa: "fmt
+
+#define PUB_KEY_BUF_SIZE 512
+
+struct tpm2_key_rsa {
+	struct tpm2_key key;
+	const u8 *pub;
+	int pub_len;
+};
+
+static const int PKCS1_PAD_MIN_SIZE = 11;
+
+/*
+ * Fill the data with PKCS#1 v1.5 padding.
+ */
+static int tpm2_pad_pkcs1(const u8 *in, int in_len, u8 *out, int out_len)
+{
+	unsigned int prefix_len = out_len - in_len - 3;
+
+	if (in_len > out_len - PKCS1_PAD_MIN_SIZE)
+		return -EBADMSG;
+
+	/* prefix */
+	out[0] = 0;
+	out[1] = 1;
+	memset(&out[2], 0xff, prefix_len);
+	out[2 + prefix_len] = 0;
+	/* payload */
+	memcpy(&out[2 + prefix_len + 1], in, in_len);
+
+	return 0;
+}
+
+/*
+ * RFC 3447 - Section 7.2.2
+ * Size of the input data should be checked against public key size by
+ * the caller.
+ */
+static const u8 *tpm2_unpad_pkcs1(const u8 *in, int in_len, int *out_len)
+{
+	int i;
+
+	if (in[0] != 0 || in[1] != 2)
+		return NULL;
+
+	i = 2;
+	while (in[i] != 0 && i < in_len)
+		i++;
+
+	if (i == in_len || i < (PKCS1_PAD_MIN_SIZE - 1))
+		return NULL;
+
+	*out_len = in_len - i - 1;
+	return in + i + 1;
+}
+
+/*
+ * Outputs the cipher algorithm name on success, and retuns -ENOPKG
+ * on failure.
+ */
+static int tpm2_key_get_akcipher(const char *encoding, const char *hash_algo,
+				 char *cipher)
+{
+	ssize_t ret;
+
+	if (strcmp(encoding, "pkcs1") == 0) {
+		if (!hash_algo) {
+			strcpy(cipher, "pkcs1pad(rsa)");
+			return 0;
+		}
+
+		ret = snprintf(cipher, CRYPTO_MAX_ALG_NAME,
+			       "pkcs1pad(rsa,%s)",
+			       hash_algo);
+		if (ret >= CRYPTO_MAX_ALG_NAME)
+			return -ENOPKG;
+
+		return 0;
+	}
+
+	if (strcmp(encoding, "raw") == 0) {
+		strcpy(cipher, "rsa");
+		return 0;
+	}
+
+	return -ENOPKG;
+}
+
+static int tpm2_key_rsa_extract_pub(struct tpm2_key_rsa *key_rsa)
+{
+	struct tpm2_key *key = &key_rsa->key;
+	struct tpm_buf buf;
+	off_t offset = 2;
+	u16 policy_len;
+	u32 attr;
+	u16 bits;
+	u16 type;
+	u16 len;
+	u16 alg;
+	u32 exp;
+
+	buf.flags = TPM_BUF_TPM2B;
+	buf.length = key->pub_len;
+	buf.data = (void *)key->pub;
+
+	if (get_unaligned_be16(key->pub) != buf.length - 2)
+		return -EINVAL;
+
+	type = tpm_buf_read_u16(&buf, &offset);
+	pr_debug("pub type: 0x%04x\n", type);
+	if (type != TPM_ALG_RSA)
+		return -EINVAL;
+
+	alg = tpm_buf_read_u16(&buf, &offset);
+	pr_debug("pub name alg: 0x%04x\n", alg);
+	attr = tpm_buf_read_u32(&buf, &offset);
+	pr_debug("pub attributes: 0x%08x\n", attr);
+	policy_len = tpm_buf_read_u16(&buf, &offset);
+	pr_debug("pub policy length: %u bytes\n", policy_len);
+	offset += policy_len;
+
+	alg = tpm_buf_read_u16(&buf, &offset);
+	pr_debug("pub symmetric: 0x%04x\n", alg);
+	if (alg != TPM_ALG_NULL)
+		return -EINVAL;
+
+	alg = tpm_buf_read_u16(&buf, &offset);
+	pr_debug("pub symmetric scheme: 0x%04x\n", alg);
+	if (alg != TPM_ALG_NULL)
+		return -EINVAL;
+
+	bits = tpm_buf_read_u16(&buf, &offset);
+	pr_debug("pub bits: %u\n", bits);
+
+	exp = tpm_buf_read_u32(&buf, &offset);
+	pr_debug("pub exponent: 0x%08x\n", exp);
+	if (exp != 0x00000000 && exp !=  0x00010001)
+		return -EINVAL;
+
+	len = tpm_buf_read_u16(&buf, &offset);
+	pr_debug("pub modulus: %u bytes\n", len);
+	key_rsa->pub = key->pub + offset;
+	key_rsa->pub_len = len;
+
+	return buf.flags & TPM_BUF_BOUNDARY_ERROR ? -EIO : 0;
+}
+
+static int tpm2_key_rsa_encode(const struct tpm2_key_rsa *key, u8 *buf)
+{
+	const int SCRATCH_SIZE = PAGE_SIZE;
+	const u8 exp[3] = {1, 0, 1};
+	u8 *scratch, *work, *work1, *end_work;
+	int pub_len = key->pub_len;
+	const u8 *pub = key->pub;
+	int ret;
+
+	scratch = kmalloc(SCRATCH_SIZE, GFP_KERNEL);
+	if (!scratch)
+		return -ENOMEM;
+
+	work = scratch;
+	end_work = &scratch[SCRATCH_SIZE];
+
+	work = asn1_encode_integer(work, end_work, pub, pub_len);
+	if (IS_ERR(work)) {
+		ret = PTR_ERR(work);
+		goto err;
+	}
+
+	work = asn1_encode_integer(work, end_work, exp, 3);
+	if (IS_ERR(work)) {
+		ret = PTR_ERR(work);
+		goto err;
+	}
+
+	work1 = buf;
+	work1 = asn1_encode_sequence(work1, &work1[PUB_KEY_BUF_SIZE],
+				     scratch, work - scratch);
+	if (IS_ERR(work1)) {
+		ret = PTR_ERR(work1);
+		goto err;
+	}
+
+	memset(work1, 0, 8);
+
+	kfree(scratch);
+	return work1 - buf;
+
+err:
+	kfree(scratch);
+	return ret;
+}
+
+/*
+ * Encryption operation is performed with the public key.  Hence it is done
+ * in software
+ */
+static int tpm2_key_rsa_encrypt(struct tpm2_key_rsa *key,
+				struct kernel_pkey_params *params,
+				const void *in, void *out)
+{
+	char cipher[CRYPTO_MAX_ALG_NAME];
+	struct scatterlist in_sg, out_sg;
+	u8 enc_pub_key[PUB_KEY_BUF_SIZE];
+	struct akcipher_request *req;
+	struct crypto_akcipher *tfm;
+	struct crypto_wait cwait;
+	int rc;
+
+	rc = tpm2_key_get_akcipher(params->encoding, params->hash_algo, cipher);
+	if (rc < 0)
+		return rc;
+
+	tfm = crypto_alloc_akcipher(cipher, 0, 0);
+	if (IS_ERR(tfm))
+		return PTR_ERR(tfm);
+
+	rc = tpm2_key_rsa_encode(key, enc_pub_key);
+	if (rc < 0)
+		goto err_tfm;
+
+	rc = crypto_akcipher_set_pub_key(tfm, enc_pub_key, rc);
+	if (rc < 0)
+		goto err_tfm;
+
+	req = akcipher_request_alloc(tfm, GFP_KERNEL);
+	if (!req) {
+		rc = -ENOMEM;
+		goto err_tfm;
+	}
+
+	sg_init_one(&in_sg, in, params->in_len);
+	sg_init_one(&out_sg, out, params->out_len);
+	akcipher_request_set_crypt(req, &in_sg, &out_sg, params->in_len,
+				   params->out_len);
+
+	crypto_init_wait(&cwait);
+	akcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
+				      CRYPTO_TFM_REQ_MAY_SLEEP,
+				      crypto_req_done, &cwait);
+
+	rc = crypto_akcipher_encrypt(req);
+	rc = crypto_wait_req(rc, &cwait);
+
+	if (!rc)
+		rc = req->dst_len;
+
+	akcipher_request_free(req);
+
+err_tfm:
+	crypto_free_akcipher(tfm);
+
+	return rc;
+}
+
+static int __tpm2_key_rsa_decrypt(struct tpm_chip *chip,
+				  struct tpm2_key_rsa *key,
+				  struct kernel_pkey_params *params,
+				  const void *in, int in_len, void *out)
+{
+	unsigned int offset = 0;
+	u32 key_handle = 0;
+	struct tpm_buf buf;
+	u16 decrypted_len;
+	u32 parent;
+	u8 *pos;
+	int ret;
+
+	ret = tpm_try_get_ops(chip);
+	if (ret)
+		return ret;
+
+	ret = tpm2_start_auth_session(chip);
+	if (ret)
+		goto err_ops;
+
+	if (key->key.parent == TPM2_RH_NULL) {
+		ret = tpm2_load_context(chip, chip->null_key_context, &offset,
+					&parent);
+		if (ret) {
+			ret = -EIO;
+			goto err_auth;
+		}
+	} else {
+		parent = key->key.parent;
+	}
+
+	ret = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
+	if (ret < 0)
+		goto err_parent;
+
+	tpm_buf_append_name(chip, &buf, parent, NULL);
+	tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_CONTINUE_SESSION |
+				    TPM2_SA_ENCRYPT, NULL, 0);
+	tpm_buf_append(&buf, key->key.blob, key->key.blob_len);
+	if (buf.flags & TPM_BUF_OVERFLOW) {
+		ret = -E2BIG;
+		goto err_buf;
+	}
+	tpm_buf_fill_hmac_session(chip, &buf);
+	ret = tpm_transmit_cmd(chip, &buf, 4, "RSA key loading");
+	ret = tpm_buf_check_hmac_response(chip, &buf, ret);
+	if (ret) {
+		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_RSA_DECRYPT);
+	tpm_buf_append_name(chip, &buf, key_handle, NULL);
+	tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_DECRYPT, NULL, 0);
+	tpm_buf_append_u16(&buf, in_len);
+	tpm_buf_append(&buf, in, in_len);
+	tpm_buf_append_u16(&buf, TPM_ALG_NULL);
+	tpm_buf_append_u16(&buf, 0);
+	tpm_buf_fill_hmac_session(chip, &buf);
+	ret = tpm_transmit_cmd(chip, &buf, 4, "RSA key decrypting");
+	ret = tpm_buf_check_hmac_response(chip, &buf, ret);
+	if (ret) {
+		ret = -EIO;
+		goto err_blob;
+	}
+
+	pos = buf.data + TPM_HEADER_SIZE + 4;
+	decrypted_len = be16_to_cpup((__be16 *)pos);
+	pos += 2;
+
+	if (params->out_len < decrypted_len) {
+		ret = -EMSGSIZE;
+		goto err_blob;
+	}
+
+	memcpy(out, pos, decrypted_len);
+	ret = decrypted_len;
+
+err_blob:
+	tpm2_flush_context(chip, key_handle);
+
+err_buf:
+	tpm_buf_destroy(&buf);
+
+err_parent:
+	if (key->key.parent == TPM2_RH_NULL)
+		tpm2_flush_context(chip, parent);
+
+err_auth:
+	if (ret < 0)
+		tpm2_end_auth_session(chip);
+
+err_ops:
+	tpm_put_ops(chip);
+	return ret;
+}
+
+static int tpm2_key_rsa_decrypt(struct tpm_chip *chip, struct tpm2_key_rsa *key,
+				struct kernel_pkey_params *params,
+				const void *in, void *out)
+{
+	const u8 *ptr;
+	int out_len;
+	u8 *work;
+	int ret;
+
+	work = kzalloc(params->out_len, GFP_KERNEL);
+	if (!work)
+		return -ENOMEM;
+
+	ret = __tpm2_key_rsa_decrypt(chip, key, params, in, params->in_len,
+				     work);
+	if (ret < 0)
+		goto err;
+
+	ptr = tpm2_unpad_pkcs1(work, ret, &out_len);
+	if (!ptr) {
+		ret = -EINVAL;
+		goto err;
+	}
+
+	if (out_len > params->out_len) {
+		ret = -EMSGSIZE;
+		goto err;
+	}
+
+	memcpy(out, ptr, out_len);
+	kfree(work);
+	return out_len;
+
+err:
+	kfree(work);
+	return ret;
+}
+
+/*
+ * Sign operation is an encryption using the TPM's private key. With RSA the
+ * only difference between encryption and decryption is where the padding goes.
+ * Since own padding can be used, TPM2_RSA_Decrypt can be repurposed to do
+ * encryption.
+ */
+static int tpm2_key_rsa_sign(struct tpm_chip *chip, struct tpm2_key_rsa *key,
+			     struct kernel_pkey_params *params,
+			     const void *in, void *out)
+{
+	const struct rsa_asn1_template *asn1;
+	u32 in_len = params->in_len;
+	void *asn1_wrapped = NULL;
+	int pub_len = key->pub_len;
+	u8 *padded;
+	int ret;
+
+	if (strcmp(params->encoding, "pkcs1") != 0) {
+		ret = -ENOPKG;
+		goto err;
+	}
+
+	if (params->hash_algo) {
+		asn1 = rsa_lookup_asn1(params->hash_algo);
+		if (!asn1) {
+			ret = -ENOPKG;
+			goto err;
+		}
+
+		/* Request enough space for the ASN.1 template + input hash */
+		asn1_wrapped = kzalloc(in_len + asn1->size, GFP_KERNEL);
+		if (!asn1_wrapped) {
+			ret = -ENOMEM;
+			goto err;
+		}
+
+		/* Copy ASN.1 template, then the input */
+		memcpy(asn1_wrapped, asn1->data, asn1->size);
+		memcpy(asn1_wrapped + asn1->size, in, in_len);
+
+		in = asn1_wrapped;
+		in_len += asn1->size;
+	}
+
+	/* with padding: */
+	padded = kmalloc(pub_len, GFP_KERNEL);
+	tpm2_pad_pkcs1(in, in_len, padded, pub_len);
+	ret = __tpm2_key_rsa_decrypt(chip, key, params, padded, pub_len, out);
+	kfree(padded);
+
+err:
+	kfree(asn1_wrapped);
+	return ret;
+}
+
+static void tpm2_key_rsa_describe(const struct key *asymmetric_key,
+				  struct seq_file *m)
+{
+	struct tpm2_key_rsa *key = asymmetric_key->payload.data[asym_crypto];
+
+	if (!key) {
+		pr_err("key blob missing");
+		return;
+	}
+
+	seq_puts(m, "TPM2/RSA");
+}
+
+static void tpm2_key_rsa_destroy(void *payload0, void *payload3)
+{
+	struct tpm2_key *key = payload0;
+
+	if (!key)
+		return;
+
+	tpm2_key_destroy(key);
+	kfree(key);
+}
+
+static int tpm2_key_rsa_eds_op(struct kernel_pkey_params *params,
+			       const void *in, void *out)
+{
+	struct tpm2_key_rsa *key = params->key->payload.data[asym_crypto];
+	struct tpm_chip *chip = tpm_default_chip();
+
+	if (!chip)
+		return -ENODEV;
+
+	switch (params->op) {
+	case kernel_pkey_encrypt:
+		return tpm2_key_rsa_encrypt(key, params, in, out);
+	case kernel_pkey_decrypt:
+		return tpm2_key_rsa_decrypt(chip, key, params, in, out);
+	case kernel_pkey_sign:
+		return tpm2_key_rsa_sign(chip, key, params, in, out);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int tpm2_key_rsa_verify(const struct key *key,
+			       const struct public_key_signature *sig)
+{
+	const struct tpm2_key_rsa *tpm2_key = key->payload.data[asym_crypto];
+	char alg_name[CRYPTO_MAX_ALG_NAME];
+	u8 enc_pub_key[PUB_KEY_BUF_SIZE];
+	struct akcipher_request *req;
+	struct scatterlist src_sg[2];
+	struct crypto_akcipher *tfm;
+	struct crypto_wait cwait;
+	int rc;
+
+	if (!sig->digest)
+		return -ENOPKG;
+
+	rc = tpm2_key_get_akcipher(sig->encoding, sig->hash_algo, alg_name);
+	if (rc < 0)
+		return rc;
+
+	tfm = crypto_alloc_akcipher(alg_name, 0, 0);
+	if (IS_ERR(tfm))
+		return PTR_ERR(tfm);
+
+	rc = tpm2_key_rsa_encode(tpm2_key, enc_pub_key);
+	if (rc < 0)
+		goto err_tfm;
+
+	rc = crypto_akcipher_set_pub_key(tfm, enc_pub_key, rc);
+	if (rc < 0)
+		goto err_tfm;
+
+	rc = -ENOMEM;
+	req = akcipher_request_alloc(tfm, GFP_KERNEL);
+	if (!req)
+		goto err_tfm;
+
+	sg_init_table(src_sg, 2);
+	sg_set_buf(&src_sg[0], sig->s, sig->s_size);
+	sg_set_buf(&src_sg[1], sig->digest, sig->digest_size);
+	akcipher_request_set_crypt(req, src_sg, NULL, sig->s_size,
+				   sig->digest_size);
+	crypto_init_wait(&cwait);
+	akcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
+				      CRYPTO_TFM_REQ_MAY_SLEEP,
+				      crypto_req_done, &cwait);
+	rc = crypto_wait_req(crypto_akcipher_verify(req), &cwait);
+
+	akcipher_request_free(req);
+
+err_tfm:
+	crypto_free_akcipher(tfm);
+	return rc;
+}
+
+static int tpm2_key_rsa_query(const struct kernel_pkey_params *params,
+			      struct kernel_pkey_query *info)
+{
+	struct tpm2_key_rsa *tk = params->key->payload.data[asym_crypto];
+	char alg_name[CRYPTO_MAX_ALG_NAME];
+	u8 enc_pub_key[PUB_KEY_BUF_SIZE];
+	struct crypto_akcipher *tfm;
+	unsigned int len;
+	int ret;
+
+	ret = tpm2_key_get_akcipher(params->encoding, params->hash_algo, alg_name);
+	if (ret < 0)
+		return ret;
+
+	tfm = crypto_alloc_akcipher(alg_name, 0, 0);
+	if (IS_ERR(tfm))
+		return PTR_ERR(tfm);
+
+	ret = tpm2_key_rsa_encode(tk, enc_pub_key);
+	if (ret < 0)
+		goto err_tfm;
+
+	ret = crypto_akcipher_set_pub_key(tfm, enc_pub_key, ret);
+	if (ret < 0)
+		goto err_tfm;
+
+	len = crypto_akcipher_maxsize(tfm);
+
+	info->key_size = tk->pub_len * 8;
+	info->max_data_size = tk->pub_len;
+	info->max_sig_size = len;
+	info->max_enc_size = len;
+	info->max_dec_size = tk->pub_len;
+
+	info->supported_ops = KEYCTL_SUPPORTS_ENCRYPT |
+			      KEYCTL_SUPPORTS_DECRYPT |
+			      KEYCTL_SUPPORTS_VERIFY |
+			      KEYCTL_SUPPORTS_SIGN;
+
+err_tfm:
+	crypto_free_akcipher(tfm);
+	return ret;
+}
+
+/*
+ * Asymmetric TPM2 RSA key. Signs and decrypts with TPM.
+ */
+struct asymmetric_key_subtype tpm2_key_rsa_subtype = {
+	.owner			= THIS_MODULE,
+	.name			= "tpm2_key_rsa",
+	.name_len		= sizeof("tpm2_key_rsa") - 1,
+	.describe		= tpm2_key_rsa_describe,
+	.destroy		= tpm2_key_rsa_destroy,
+	.query			= tpm2_key_rsa_query,
+	.eds_op			= tpm2_key_rsa_eds_op,
+	.verify_signature	= tpm2_key_rsa_verify,
+};
+EXPORT_SYMBOL_GPL(tpm2_key_rsa_subtype);
+
+/*
+ * Attempt to parse a data blob for a key as a TPM private key blob.
+ */
+static int tpm2_key_preparse(struct key_preparsed_payload *prep)
+{
+	struct tpm2_key_rsa *key;
+	int ret;
+
+	key = kzalloc(sizeof(*key), GFP_KERNEL);
+	if (!key)
+		return -ENOMEM;
+
+	/*
+	 * TPM 2.0 RSA keys are recommended to be 2048 bits long. Assume the
+	 * blob is no more than 4x that.
+	 */
+	if (prep->datalen > 256 * 4) {
+		kfree(key);
+		return -EMSGSIZE;
+	}
+
+	ret = tpm2_key_decode(prep->data, prep->datalen, &key->key, PAGE_SIZE);
+	if (ret) {
+		kfree(key);
+		return ret;
+	}
+
+	if (key->key.oid != OID_TPMLoadableKey) {
+		tpm2_key_destroy(&key->key);
+		kfree(key);
+		return -EINVAL;
+	}
+
+	ret = tpm2_key_rsa_extract_pub(key);
+	if (ret < 0) {
+		tpm2_key_destroy(&key->key);
+		kfree(key);
+		return ret;
+	}
+
+	prep->payload.data[asym_subtype] = &tpm2_key_rsa_subtype;
+	prep->payload.data[asym_key_ids] = NULL;
+	prep->payload.data[asym_crypto] = key;
+	prep->payload.data[asym_auth] = NULL;
+	prep->quotalen = 100;
+	return 0;
+}
+
+static struct asymmetric_key_parser tpm2_key_rsa_parser = {
+	.owner	= THIS_MODULE,
+	.name	= "tpm2_key_rsa_parser",
+	.parse	= tpm2_key_preparse,
+};
+
+static int __init tpm2_key_rsa_init(void)
+{
+	return register_asymmetric_key_parser(&tpm2_key_rsa_parser);
+}
+
+static void __exit tpm2_key_rsa_exit(void)
+{
+	unregister_asymmetric_key_parser(&tpm2_key_rsa_parser);
+}
+
+module_init(tpm2_key_rsa_init);
+module_exit(tpm2_key_rsa_exit);
+
+MODULE_DESCRIPTION("Asymmetric TPM2 RSA key");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 2f25ca07127b..8161758da19a 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -43,6 +43,7 @@ enum tpm2_session_types {
 /* if you add a new hash to this, increment TPM_MAX_HASHES below */
 enum tpm_algorithms {
 	TPM_ALG_ERROR		= 0x0000,
+	TPM_ALG_RSA		= 0x0001,
 	TPM_ALG_SHA1		= 0x0004,
 	TPM_ALG_AES		= 0x0006,
 	TPM_ALG_KEYEDHASH	= 0x0008,
@@ -271,6 +272,7 @@ enum tpm2_command_codes {
 	TPM2_CC_NV_READ                 = 0x014E,
 	TPM2_CC_CREATE		        = 0x0153,
 	TPM2_CC_LOAD		        = 0x0157,
+	TPM2_CC_RSA_DECRYPT	        = 0x0159,
 	TPM2_CC_SEQUENCE_UPDATE         = 0x015C,
 	TPM2_CC_UNSEAL		        = 0x015E,
 	TPM2_CC_CONTEXT_LOAD	        = 0x0161,
-- 
2.45.1

RE: [EXTERNAL] [PATCH v2 6/6] keys: asymmetric: ASYMMETRIC_TPM2_KEY_RSA_SUBTYPE

[Bharat Bhushan]

> -----Original Message-----
> From: Jarkko Sakkinen <jarkko@kernel.org>
> Sent: Tuesday, May 21, 2024 8:47 AM
> To: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: linux-integrity@vger.kernel.org; keyrings@vger.kernel.org;
> Andreas.Fuchs@infineon.com; James Prestwood <prestwoj@gmail.com>;
> David Woodhouse <dwmw2@infradead.org>; Eric Biggers
> <ebiggers@kernel.org>; James Bottomley
> <James.Bottomley@hansenpartnership.com>; Jarkko Sakkinen
> <jarkko@kernel.org>; David S. Miller <davem@davemloft.net>; open
> list:CRYPTO API <linux-crypto@vger.kernel.org>; open list <linux-
> kernel@vger.kernel.org>; David Howells <dhowells@redhat.com>; James
> Bottomley <James.Bottomley@HansenPartnership.com>; Stefan Berger
> <stefanb@linux.ibm.com>; Ard Biesheuvel <ardb@kernel.org>; Mario
> Limonciello <mario.limonciello@amd.com>
> Subject: [EXTERNAL] [PATCH v2 6/6] keys: asymmetric:
> ASYMMETRIC_TPM2_KEY_RSA_SUBTYPE
> 
> ----------------------------------------------------------------------
> From: James Prestwood <prestwoj@gmail.com>
> 
> Based on earlier work by James Prestwood.
> 
> Add ASN.1 compatible asymmetric TPM2 RSA key subtype:
> 
> 1. Signing and decryption (with the private key) is handled by
>    TPM2_RSA_Decrypt.
> 2. Encryption (with the public key) is handled by the kernel RSA
>    implementation.
> 
> Link: https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__lore.kernel.org_all_20200518172704.29608-2D1-2Dprestwoj-
> 40gmail.com_&d=DwIDAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=PAAlWswPe7d8
> gHlGbCLmy2YezyK7O3Hv_t2heGnouBw&m=OMixrhGWcekpXpja15IeSeghOU
> 4mBNCZOSB2Vgtzbn7xcodoWU_Hnnpzp_eZh-
> XR&s=pKVMCPyvi19wJur3Bzq2xo3MtPHsEicDLBGr--NWRjs&e=
> Signed-off-by: James Prestwood <prestwoj@gmail.com>
> Co-developed-by: Jarkko Sakkinen <jarkko@kernel.org>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> v2:
> * Remove two spurios pr_info() messsages that I forgot to remove.
> * Clean up padding functions and add additional checks for length
>   also in tpm2_unpad_pcks1().
> * Add the missing success check kzalloc() in tpm2_key_rsa_decrypt().
> * Check that params->out_len for capacity before copying the result.
> ---
>  crypto/asymmetric_keys/Kconfig        |  16 +
>  crypto/asymmetric_keys/Makefile       |   1 +
>  crypto/asymmetric_keys/tpm2_key_rsa.c | 698
> ++++++++++++++++++++++++++
>  include/linux/tpm.h                   |   2 +
>  4 files changed, 717 insertions(+)
>  create mode 100644 crypto/asymmetric_keys/tpm2_key_rsa.c
> 
> diff --git a/crypto/asymmetric_keys/Kconfig
> b/crypto/asymmetric_keys/Kconfig
> index e1345b8f39f1..4d14bb0c346e 100644
> --- a/crypto/asymmetric_keys/Kconfig
> +++ b/crypto/asymmetric_keys/Kconfig
> @@ -15,6 +15,7 @@ config ASYMMETRIC_PUBLIC_KEY_SUBTYPE
>  	select MPILIB
>  	select CRYPTO_HASH_INFO
>  	select CRYPTO_AKCIPHER
> +	select CRYPTO_RSA
>  	select CRYPTO_SIG
>  	select CRYPTO_HASH
>  	help
> @@ -23,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_RSA_SUBTYPE
> +	tristate "Asymmetric TPM2 RSA crypto algorithm subtype"
> +	depends on TCG_TPM
> +	select TCG_TPM2_HMAC
> +	select CRYPTO_RSA
> +	select CRYPTO_SHA256
> +	select CRYPTO_HASH_INFO
> +	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 X509_CERTIFICATE_PARSER
>  	tristate "X.509 certificate parser"
>  	depends on ASYMMETRIC_PUBLIC_KEY_SUBTYPE
> diff --git a/crypto/asymmetric_keys/Makefile
> b/crypto/asymmetric_keys/Makefile
> index bc65d3b98dcb..c6da84607824 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_RSA_SUBTYPE) += tpm2_key_rsa.o
> 
>  #
>  # X.509 Certificate handling
> diff --git a/crypto/asymmetric_keys/tpm2_key_rsa.c
> b/crypto/asymmetric_keys/tpm2_key_rsa.c
> new file mode 100644
> index 000000000000..32250ff38268
> --- /dev/null
> +++ b/crypto/asymmetric_keys/tpm2_key_rsa.c
> @@ -0,0 +1,698 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/* TPM2 asymmetric public-key crypto subtype
> + *
> + * See Documentation/crypto/asymmetric-keys.rst
> + *
> + * Copyright (c) 2020 Intel Corporation
> + */
> +
> +#include <asm/unaligned.h>
> +#include <crypto/akcipher.h>
> +#include <crypto/public_key.h>
> +#include <crypto/rsa-pkcs1pad.h>
> +#include <crypto/tpm2_key.h>
> +#include <keys/asymmetric-parser.h>
> +#include <keys/asymmetric-subtype.h>
> +#include <keys/trusted-type.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_rsa: "fmt
> +
> +#define PUB_KEY_BUF_SIZE 512
> +
> +struct tpm2_key_rsa {
> +	struct tpm2_key key;
> +	const u8 *pub;
> +	int pub_len;
> +};
> +
> +static const int PKCS1_PAD_MIN_SIZE = 11;
> +
> +/*
> + * Fill the data with PKCS#1 v1.5 padding.
> + */
> +static int tpm2_pad_pkcs1(const u8 *in, int in_len, u8 *out, int out_len)
> +{
> +	unsigned int prefix_len = out_len - in_len - 3;
> +
> +	if (in_len > out_len - PKCS1_PAD_MIN_SIZE)
> +		return -EBADMSG;
> +
> +	/* prefix */
> +	out[0] = 0;
> +	out[1] = 1;
> +	memset(&out[2], 0xff, prefix_len);
> +	out[2 + prefix_len] = 0;
> +	/* payload */
> +	memcpy(&out[2 + prefix_len + 1], in, in_len);
> +
> +	return 0;
> +}
> +
> +/*
> + * RFC 3447 - Section 7.2.2
> + * Size of the input data should be checked against public key size by
> + * the caller.
> + */
> +static const u8 *tpm2_unpad_pkcs1(const u8 *in, int in_len, int *out_len)
> +{
> +	int i;
> +
> +	if (in[0] != 0 || in[1] != 2)
> +		return NULL;
> +
> +	i = 2;
> +	while (in[i] != 0 && i < in_len)
> +		i++;
> +
> +	if (i == in_len || i < (PKCS1_PAD_MIN_SIZE - 1))
> +		return NULL;
> +
> +	*out_len = in_len - i - 1;
> +	return in + i + 1;
> +}
> +
> +/*
> + * Outputs the cipher algorithm name on success, and retuns -ENOPKG
> + * on failure.
> + */
> +static int tpm2_key_get_akcipher(const char *encoding, const char
> *hash_algo,
> +				 char *cipher)
> +{
> +	ssize_t ret;
> +
> +	if (strcmp(encoding, "pkcs1") == 0) {
> +		if (!hash_algo) {
> +			strcpy(cipher, "pkcs1pad(rsa)");
> +			return 0;
> +		}
> +
> +		ret = snprintf(cipher, CRYPTO_MAX_ALG_NAME,
> +			       "pkcs1pad(rsa,%s)",
> +			       hash_algo);
> +		if (ret >= CRYPTO_MAX_ALG_NAME)
> +			return -ENOPKG;
> +
> +		return 0;
> +	}
> +
> +	if (strcmp(encoding, "raw") == 0) {
> +		strcpy(cipher, "rsa");
> +		return 0;
> +	}
> +
> +	return -ENOPKG;
> +}
> +
> +static int tpm2_key_rsa_extract_pub(struct tpm2_key_rsa *key_rsa)
> +{
> +	struct tpm2_key *key = &key_rsa->key;
> +	struct tpm_buf buf;
> +	off_t offset = 2;
> +	u16 policy_len;
> +	u32 attr;
> +	u16 bits;
> +	u16 type;
> +	u16 len;
> +	u16 alg;
> +	u32 exp;
> +
> +	buf.flags = TPM_BUF_TPM2B;
> +	buf.length = key->pub_len;
> +	buf.data = (void *)key->pub;
> +
> +	if (get_unaligned_be16(key->pub) != buf.length - 2)
> +		return -EINVAL;
> +
> +	type = tpm_buf_read_u16(&buf, &offset);
> +	pr_debug("pub type: 0x%04x\n", type);
> +	if (type != TPM_ALG_RSA)
> +		return -EINVAL;
> +
> +	alg = tpm_buf_read_u16(&buf, &offset);
> +	pr_debug("pub name alg: 0x%04x\n", alg);
> +	attr = tpm_buf_read_u32(&buf, &offset);
> +	pr_debug("pub attributes: 0x%08x\n", attr);
> +	policy_len = tpm_buf_read_u16(&buf, &offset);
> +	pr_debug("pub policy length: %u bytes\n", policy_len);
> +	offset += policy_len;
> +
> +	alg = tpm_buf_read_u16(&buf, &offset);
> +	pr_debug("pub symmetric: 0x%04x\n", alg);
> +	if (alg != TPM_ALG_NULL)
> +		return -EINVAL;
> +
> +	alg = tpm_buf_read_u16(&buf, &offset);
> +	pr_debug("pub symmetric scheme: 0x%04x\n", alg);
> +	if (alg != TPM_ALG_NULL)
> +		return -EINVAL;
> +
> +	bits = tpm_buf_read_u16(&buf, &offset);
> +	pr_debug("pub bits: %u\n", bits);
> +
> +	exp = tpm_buf_read_u32(&buf, &offset);
> +	pr_debug("pub exponent: 0x%08x\n", exp);
> +	if (exp != 0x00000000 && exp !=  0x00010001)
> +		return -EINVAL;
> +
> +	len = tpm_buf_read_u16(&buf, &offset);
> +	pr_debug("pub modulus: %u bytes\n", len);
> +	key_rsa->pub = key->pub + offset;
> +	key_rsa->pub_len = len;
> +
> +	return buf.flags & TPM_BUF_BOUNDARY_ERROR ? -EIO : 0;
> +}
> +
> +static int tpm2_key_rsa_encode(const struct tpm2_key_rsa *key, u8 *buf)
> +{
> +	const int SCRATCH_SIZE = PAGE_SIZE;
> +	const u8 exp[3] = {1, 0, 1};
> +	u8 *scratch, *work, *work1, *end_work;
> +	int pub_len = key->pub_len;
> +	const u8 *pub = key->pub;
> +	int ret;
> +
> +	scratch = kmalloc(SCRATCH_SIZE, GFP_KERNEL);
> +	if (!scratch)
> +		return -ENOMEM;
> +
> +	work = scratch;
> +	end_work = &scratch[SCRATCH_SIZE];
> +
> +	work = asn1_encode_integer(work, end_work, pub, pub_len);
> +	if (IS_ERR(work)) {
> +		ret = PTR_ERR(work);
> +		goto err;
> +	}
> +
> +	work = asn1_encode_integer(work, end_work, exp, 3);
> +	if (IS_ERR(work)) {
> +		ret = PTR_ERR(work);
> +		goto err;
> +	}
> +
> +	work1 = buf;
> +	work1 = asn1_encode_sequence(work1,
> &work1[PUB_KEY_BUF_SIZE],
> +				     scratch, work - scratch);
> +	if (IS_ERR(work1)) {
> +		ret = PTR_ERR(work1);
> +		goto err;
> +	}
> +
> +	memset(work1, 0, 8);
> +
> +	kfree(scratch);
> +	return work1 - buf;
> +
> +err:
> +	kfree(scratch);
> +	return ret;
> +}
> +
> +/*
> + * Encryption operation is performed with the public key.  Hence it is done
> + * in software
> + */
> +static int tpm2_key_rsa_encrypt(struct tpm2_key_rsa *key,
> +				struct kernel_pkey_params *params,
> +				const void *in, void *out)
> +{
> +	char cipher[CRYPTO_MAX_ALG_NAME];
> +	struct scatterlist in_sg, out_sg;
> +	u8 enc_pub_key[PUB_KEY_BUF_SIZE];
> +	struct akcipher_request *req;
> +	struct crypto_akcipher *tfm;
> +	struct crypto_wait cwait;
> +	int rc;
> +
> +	rc = tpm2_key_get_akcipher(params->encoding, params->hash_algo,
> cipher);
> +	if (rc < 0)
> +		return rc;
> +
> +	tfm = crypto_alloc_akcipher(cipher, 0, 0);
> +	if (IS_ERR(tfm))
> +		return PTR_ERR(tfm);
> +
> +	rc = tpm2_key_rsa_encode(key, enc_pub_key);
> +	if (rc < 0)
> +		goto err_tfm;
> +
> +	rc = crypto_akcipher_set_pub_key(tfm, enc_pub_key, rc);
> +	if (rc < 0)
> +		goto err_tfm;
> +
> +	req = akcipher_request_alloc(tfm, GFP_KERNEL);
> +	if (!req) {
> +		rc = -ENOMEM;
> +		goto err_tfm;
> +	}
> +
> +	sg_init_one(&in_sg, in, params->in_len);
> +	sg_init_one(&out_sg, out, params->out_len);
> +	akcipher_request_set_crypt(req, &in_sg, &out_sg, params->in_len,
> +				   params->out_len);
> +
> +	crypto_init_wait(&cwait);
> +	akcipher_request_set_callback(req,
> CRYPTO_TFM_REQ_MAY_BACKLOG |
> +				      CRYPTO_TFM_REQ_MAY_SLEEP,
> +				      crypto_req_done, &cwait);
> +
> +	rc = crypto_akcipher_encrypt(req);
> +	rc = crypto_wait_req(rc, &cwait);
> +

Few Minor comments, 
Extra line here 

> +	if (!rc)
> +		rc = req->dst_len;
> +
> +	akcipher_request_free(req);
> +
> +err_tfm:
> +	crypto_free_akcipher(tfm);
> +
> +	return rc;
> +}
> +
> +static int __tpm2_key_rsa_decrypt(struct tpm_chip *chip,
> +				  struct tpm2_key_rsa *key,
> +				  struct kernel_pkey_params *params,
> +				  const void *in, int in_len, void *out)
> +{
> +	unsigned int offset = 0;
> +	u32 key_handle = 0;
> +	struct tpm_buf buf;
> +	u16 decrypted_len;
> +	u32 parent;
> +	u8 *pos;
> +	int ret;
> +
> +	ret = tpm_try_get_ops(chip);
> +	if (ret)
> +		return ret;
> +
> +	ret = tpm2_start_auth_session(chip);
> +	if (ret)
> +		goto err_ops;
> +
> +	if (key->key.parent == TPM2_RH_NULL) {
> +		ret = tpm2_load_context(chip, chip->null_key_context,
> &offset,
> +					&parent);
> +		if (ret) {
> +			ret = -EIO;
> +			goto err_auth;
> +		}
> +	} else {
> +		parent = key->key.parent;
> +	}

Do we need braces here?

> +
> +	ret = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
> +	if (ret < 0)
> +		goto err_parent;
> +
> +	tpm_buf_append_name(chip, &buf, parent, NULL);
> +	tpm_buf_append_hmac_session(chip, &buf,
> TPM2_SA_CONTINUE_SESSION |
> +				    TPM2_SA_ENCRYPT, NULL, 0);
> +	tpm_buf_append(&buf, key->key.blob, key->key.blob_len);
> +	if (buf.flags & TPM_BUF_OVERFLOW) {
> +		ret = -E2BIG;
> +		goto err_buf;
> +	}
> +	tpm_buf_fill_hmac_session(chip, &buf);
> +	ret = tpm_transmit_cmd(chip, &buf, 4, "RSA key loading");
> +	ret = tpm_buf_check_hmac_response(chip, &buf, ret);
> +	if (ret) {
> +		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_RSA_DECRYPT);
> +	tpm_buf_append_name(chip, &buf, key_handle, NULL);
> +	tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_DECRYPT,
> NULL, 0);
> +	tpm_buf_append_u16(&buf, in_len);
> +	tpm_buf_append(&buf, in, in_len);
> +	tpm_buf_append_u16(&buf, TPM_ALG_NULL);
> +	tpm_buf_append_u16(&buf, 0);
> +	tpm_buf_fill_hmac_session(chip, &buf);
> +	ret = tpm_transmit_cmd(chip, &buf, 4, "RSA key decrypting");
> +	ret = tpm_buf_check_hmac_response(chip, &buf, ret);
> +	if (ret) {
> +		ret = -EIO;
> +		goto err_blob;
> +	}
> +
> +	pos = buf.data + TPM_HEADER_SIZE + 4;
> +	decrypted_len = be16_to_cpup((__be16 *)pos);
> +	pos += 2;
> +
> +	if (params->out_len < decrypted_len) {
> +		ret = -EMSGSIZE;
> +		goto err_blob;
> +	}
> +
> +	memcpy(out, pos, decrypted_len);
> +	ret = decrypted_len;
> +
> +err_blob:
> +	tpm2_flush_context(chip, key_handle);
> +
> +err_buf:
> +	tpm_buf_destroy(&buf);
> +
> +err_parent:
> +	if (key->key.parent == TPM2_RH_NULL)
> +		tpm2_flush_context(chip, parent);
> +
> +err_auth:
> +	if (ret < 0)
> +		tpm2_end_auth_session(chip);
> +
> +err_ops:
> +	tpm_put_ops(chip);
> +	return ret;
> +}
> +
> +static int tpm2_key_rsa_decrypt(struct tpm_chip *chip, struct tpm2_key_rsa
> *key,
> +				struct kernel_pkey_params *params,
> +				const void *in, void *out)
> +{
> +	const u8 *ptr;
> +	int out_len;
> +	u8 *work;
> +	int ret;
> +
> +	work = kzalloc(params->out_len, GFP_KERNEL);
> +	if (!work)
> +		return -ENOMEM;

Maybe use ERR_PTR() here and couple of more places

> +
> +	ret = __tpm2_key_rsa_decrypt(chip, key, params, in, params->in_len,
> +				     work);
> +	if (ret < 0)
> +		goto err;
> +
> +	ptr = tpm2_unpad_pkcs1(work, ret, &out_len);
> +	if (!ptr) {
> +		ret = -EINVAL;
> +		goto err;
> +	}
> +
> +	if (out_len > params->out_len) {
> +		ret = -EMSGSIZE;
> +		goto err;
> +	}
> +
> +	memcpy(out, ptr, out_len);
> +	kfree(work);
> +	return out_len;
> +
> +err:
> +	kfree(work);
> +	return ret;
> +}
> +
> +/*
> + * Sign operation is an encryption using the TPM's private key. With RSA the
> + * only difference between encryption and decryption is where the padding
> goes.
> + * Since own padding can be used, TPM2_RSA_Decrypt can be repurposed to
> do
> + * encryption.
> + */
> +static int tpm2_key_rsa_sign(struct tpm_chip *chip, struct tpm2_key_rsa
> *key,
> +			     struct kernel_pkey_params *params,
> +			     const void *in, void *out)
> +{
> +	const struct rsa_asn1_template *asn1;
> +	u32 in_len = params->in_len;
> +	void *asn1_wrapped = NULL;
> +	int pub_len = key->pub_len;
> +	u8 *padded;
> +	int ret;
> +
> +	if (strcmp(params->encoding, "pkcs1") != 0) {
> +		ret = -ENOPKG;
> +		goto err;
> +	}
> +
> +	if (params->hash_algo) {
> +		asn1 = rsa_lookup_asn1(params->hash_algo);
> +		if (!asn1) {
> +			ret = -ENOPKG;
> +			goto err;
> +		}
> +
> +		/* Request enough space for the ASN.1 template + input hash
> */
> +		asn1_wrapped = kzalloc(in_len + asn1->size, GFP_KERNEL);
> +		if (!asn1_wrapped) {
> +			ret = -ENOMEM;
> +			goto err;
> +		}
> +
> +		/* Copy ASN.1 template, then the input */
> +		memcpy(asn1_wrapped, asn1->data, asn1->size);
> +		memcpy(asn1_wrapped + asn1->size, in, in_len);
> +
> +		in = asn1_wrapped;
> +		in_len += asn1->size;
> +	}
> +
> +	/* with padding: */
> +	padded = kmalloc(pub_len, GFP_KERNEL);
> +	tpm2_pad_pkcs1(in, in_len, padded, pub_len);
> +	ret = __tpm2_key_rsa_decrypt(chip, key, params, padded, pub_len,
> out);
> +	kfree(padded);
> +
> +err:
> +	kfree(asn1_wrapped);
> +	return ret;
> +}
> +
> +static void tpm2_key_rsa_describe(const struct key *asymmetric_key,
> +				  struct seq_file *m)
> +{
> +	struct tpm2_key_rsa *key = asymmetric_key-
> >payload.data[asym_crypto];
> +
> +	if (!key) {
> +		pr_err("key blob missing");
> +		return;
> +	}
> +
> +	seq_puts(m, "TPM2/RSA");
> +}
> +
> +static void tpm2_key_rsa_destroy(void *payload0, void *payload3)
> +{
> +	struct tpm2_key *key = payload0;
> +
> +	if (!key)
> +		return;
> +
> +	tpm2_key_destroy(key);
> +	kfree(key);
> +}
> +
> +static int tpm2_key_rsa_eds_op(struct kernel_pkey_params *params,
> +			       const void *in, void *out)
> +{
> +	struct tpm2_key_rsa *key = params->key-
> >payload.data[asym_crypto];
> +	struct tpm_chip *chip = tpm_default_chip();
> +
> +	if (!chip)
> +		return -ENODEV;
> +
> +	switch (params->op) {
> +	case kernel_pkey_encrypt:
> +		return tpm2_key_rsa_encrypt(key, params, in, out);
> +	case kernel_pkey_decrypt:
> +		return tpm2_key_rsa_decrypt(chip, key, params, in, out);
> +	case kernel_pkey_sign:
> +		return tpm2_key_rsa_sign(chip, key, params, in, out);
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static int tpm2_key_rsa_verify(const struct key *key,
> +			       const struct public_key_signature *sig)
> +{
> +	const struct tpm2_key_rsa *tpm2_key = key-
> >payload.data[asym_crypto];
> +	char alg_name[CRYPTO_MAX_ALG_NAME];
> +	u8 enc_pub_key[PUB_KEY_BUF_SIZE];
> +	struct akcipher_request *req;
> +	struct scatterlist src_sg[2];
> +	struct crypto_akcipher *tfm;
> +	struct crypto_wait cwait;
> +	int rc;
> +
> +	if (!sig->digest)
> +		return -ENOPKG;
> +
> +	rc = tpm2_key_get_akcipher(sig->encoding, sig->hash_algo,
> alg_name);
> +	if (rc < 0)
> +		return rc;
> +
> +	tfm = crypto_alloc_akcipher(alg_name, 0, 0);
> +	if (IS_ERR(tfm))
> +		return PTR_ERR(tfm);
> +
> +	rc = tpm2_key_rsa_encode(tpm2_key, enc_pub_key);
> +	if (rc < 0)
> +		goto err_tfm;
> +
> +	rc = crypto_akcipher_set_pub_key(tfm, enc_pub_key, rc);
> +	if (rc < 0)
> +		goto err_tfm;
> +
> +	rc = -ENOMEM;
> +	req = akcipher_request_alloc(tfm, GFP_KERNEL);
> +	if (!req)
> +		goto err_tfm;
> +
> +	sg_init_table(src_sg, 2);
> +	sg_set_buf(&src_sg[0], sig->s, sig->s_size);
> +	sg_set_buf(&src_sg[1], sig->digest, sig->digest_size);
> +	akcipher_request_set_crypt(req, src_sg, NULL, sig->s_size,
> +				   sig->digest_size);
> +	crypto_init_wait(&cwait);
> +	akcipher_request_set_callback(req,
> CRYPTO_TFM_REQ_MAY_BACKLOG |
> +				      CRYPTO_TFM_REQ_MAY_SLEEP,
> +				      crypto_req_done, &cwait);
> +	rc = crypto_wait_req(crypto_akcipher_verify(req), &cwait);
> +
> +	akcipher_request_free(req);
> +
> +err_tfm:
> +	crypto_free_akcipher(tfm);
> +	return rc;
> +}
> +
> +static int tpm2_key_rsa_query(const struct kernel_pkey_params *params,
> +			      struct kernel_pkey_query *info)
> +{
> +	struct tpm2_key_rsa *tk = params->key->payload.data[asym_crypto];
> +	char alg_name[CRYPTO_MAX_ALG_NAME];
> +	u8 enc_pub_key[PUB_KEY_BUF_SIZE];
> +	struct crypto_akcipher *tfm;
> +	unsigned int len;
> +	int ret;
> +
> +	ret = tpm2_key_get_akcipher(params->encoding, params->hash_algo,
> alg_name);
> +	if (ret < 0)
> +		return ret;
> +
> +	tfm = crypto_alloc_akcipher(alg_name, 0, 0);
> +	if (IS_ERR(tfm))
> +		return PTR_ERR(tfm);
> +
> +	ret = tpm2_key_rsa_encode(tk, enc_pub_key);
> +	if (ret < 0)
> +		goto err_tfm;
> +
> +	ret = crypto_akcipher_set_pub_key(tfm, enc_pub_key, ret);
> +	if (ret < 0)
> +		goto err_tfm;
> +
> +	len = crypto_akcipher_maxsize(tfm);
> +
> +	info->key_size = tk->pub_len * 8;
> +	info->max_data_size = tk->pub_len;
> +	info->max_sig_size = len;
> +	info->max_enc_size = len;
> +	info->max_dec_size = tk->pub_len;
> +
> +	info->supported_ops = KEYCTL_SUPPORTS_ENCRYPT |
> +			      KEYCTL_SUPPORTS_DECRYPT |
> +			      KEYCTL_SUPPORTS_VERIFY |
> +			      KEYCTL_SUPPORTS_SIGN;
> +
> +err_tfm:
> +	crypto_free_akcipher(tfm);
> +	return ret;
> +}
> +
> +/*
> + * Asymmetric TPM2 RSA key. Signs and decrypts with TPM.
> + */
> +struct asymmetric_key_subtype tpm2_key_rsa_subtype = {
> +	.owner			= THIS_MODULE,
> +	.name			= "tpm2_key_rsa",
> +	.name_len		= sizeof("tpm2_key_rsa") - 1,
> +	.describe		= tpm2_key_rsa_describe,
> +	.destroy		= tpm2_key_rsa_destroy,
> +	.query			= tpm2_key_rsa_query,
> +	.eds_op			= tpm2_key_rsa_eds_op,
> +	.verify_signature	= tpm2_key_rsa_verify,
> +};
> +EXPORT_SYMBOL_GPL(tpm2_key_rsa_subtype);
> +
> +/*
> + * Attempt to parse a data blob for a key as a TPM private key blob.
> + */
> +static int tpm2_key_preparse(struct key_preparsed_payload *prep)
> +{
> +	struct tpm2_key_rsa *key;
> +	int ret;
> +
> +	key = kzalloc(sizeof(*key), GFP_KERNEL);
> +	if (!key)
> +		return -ENOMEM;
> +
> +	/*
> +	 * TPM 2.0 RSA keys are recommended to be 2048 bits long. Assume
> the
> +	 * blob is no more than 4x that.
> +	 */
> +	if (prep->datalen > 256 * 4) {
> +		kfree(key);
> +		return -EMSGSIZE;
> +	}
> +
> +	ret = tpm2_key_decode(prep->data, prep->datalen, &key->key,
> PAGE_SIZE);
> +	if (ret) {
> +		kfree(key);
> +		return ret;
> +	}
> +
> +	if (key->key.oid != OID_TPMLoadableKey) {
> +		tpm2_key_destroy(&key->key);
> +		kfree(key);
> +		return -EINVAL;
> +	}
> +
> +	ret = tpm2_key_rsa_extract_pub(key);
> +	if (ret < 0) {
> +		tpm2_key_destroy(&key->key);
> +		kfree(key);
> +		return ret;
> +	}
> +
> +	prep->payload.data[asym_subtype] = &tpm2_key_rsa_subtype;
> +	prep->payload.data[asym_key_ids] = NULL;
> +	prep->payload.data[asym_crypto] = key;
> +	prep->payload.data[asym_auth] = NULL;
> +	prep->quotalen = 100;
> +	return 0;
> +}
> +
> +static struct asymmetric_key_parser tpm2_key_rsa_parser = {
> +	.owner	= THIS_MODULE,
> +	.name	= "tpm2_key_rsa_parser",
> +	.parse	= tpm2_key_preparse,
> +};
> +
> +static int __init tpm2_key_rsa_init(void)
> +{
> +	return register_asymmetric_key_parser(&tpm2_key_rsa_parser);
> +}
> +
> +static void __exit tpm2_key_rsa_exit(void)
> +{
> +	unregister_asymmetric_key_parser(&tpm2_key_rsa_parser);
> +}
> +
> +module_init(tpm2_key_rsa_init);
> +module_exit(tpm2_key_rsa_exit);
> +
> +MODULE_DESCRIPTION("Asymmetric TPM2 RSA key");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index 2f25ca07127b..8161758da19a 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -43,6 +43,7 @@ enum tpm2_session_types {
>  /* if you add a new hash to this, increment TPM_MAX_HASHES below */
>  enum tpm_algorithms {
>  	TPM_ALG_ERROR		= 0x0000,
> +	TPM_ALG_RSA		= 0x0001,
>  	TPM_ALG_SHA1		= 0x0004,
>  	TPM_ALG_AES		= 0x0006,
>  	TPM_ALG_KEYEDHASH	= 0x0008,
> @@ -271,6 +272,7 @@ enum tpm2_command_codes {
>  	TPM2_CC_NV_READ                 = 0x014E,
>  	TPM2_CC_CREATE		        = 0x0153,
>  	TPM2_CC_LOAD		        = 0x0157,
> +	TPM2_CC_RSA_DECRYPT	        = 0x0159,
>  	TPM2_CC_SEQUENCE_UPDATE         = 0x015C,
>  	TPM2_CC_UNSEAL		        = 0x015E,
>  	TPM2_CC_CONTEXT_LOAD	        = 0x0161,
> --
> 2.45.1
> 

Re: [EXTERNAL] [PATCH v2 6/6] keys: asymmetric: ASYMMETRIC_TPM2_KEY_RSA_SUBTYPE

[Jarkko Sakkinen]

On Tue May 21, 2024 at 10:25 AM EEST, Bharat Bhushan wrote:
> > +	rc = crypto_akcipher_encrypt(req);
> > +	rc = crypto_wait_req(rc, &cwait);
> > +
>
> Few Minor comments, 
> Extra line here 

Yeah, makes sense.


> > +	if (!rc)
> > +		rc = req->dst_len;
> > +
> > +	akcipher_request_free(req);
> > +
> > +err_tfm:
> > +	crypto_free_akcipher(tfm);
> > +
> > +	return rc;
> > +}
> > +
> > +static int __tpm2_key_rsa_decrypt(struct tpm_chip *chip,
> > +				  struct tpm2_key_rsa *key,
> > +				  struct kernel_pkey_params *params,
> > +				  const void *in, int in_len, void *out)
> > +{
> > +	unsigned int offset = 0;
> > +	u32 key_handle = 0;
> > +	struct tpm_buf buf;
> > +	u16 decrypted_len;
> > +	u32 parent;
> > +	u8 *pos;
> > +	int ret;
> > +
> > +	ret = tpm_try_get_ops(chip);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = tpm2_start_auth_session(chip);
> > +	if (ret)
> > +		goto err_ops;
> > +
> > +	if (key->key.parent == TPM2_RH_NULL) {
> > +		ret = tpm2_load_context(chip, chip->null_key_context,
> > &offset,
> > +					&parent);
> > +		if (ret) {
> > +			ret = -EIO;
> > +			goto err_auth;
> > +		}
> > +	} else {
> > +		parent = key->key.parent;
> > +	}
>
> Do we need braces here?


I think I added them because checkpatch complained me about not having
them. So I guess the rule is that if any branch has braces, all of them
should have...
>
> > +
> > +	ret = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
> > +	if (ret < 0)
> > +		goto err_parent;
> > +
> > +	tpm_buf_append_name(chip, &buf, parent, NULL);
> > +	tpm_buf_append_hmac_session(chip, &buf,
> > TPM2_SA_CONTINUE_SESSION |
> > +				    TPM2_SA_ENCRYPT, NULL, 0);
> > +	tpm_buf_append(&buf, key->key.blob, key->key.blob_len);
> > +	if (buf.flags & TPM_BUF_OVERFLOW) {
> > +		ret = -E2BIG;
> > +		goto err_buf;
> > +	}
> > +	tpm_buf_fill_hmac_session(chip, &buf);
> > +	ret = tpm_transmit_cmd(chip, &buf, 4, "RSA key loading");
> > +	ret = tpm_buf_check_hmac_response(chip, &buf, ret);
> > +	if (ret) {
> > +		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_RSA_DECRYPT);
> > +	tpm_buf_append_name(chip, &buf, key_handle, NULL);
> > +	tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_DECRYPT,
> > NULL, 0);
> > +	tpm_buf_append_u16(&buf, in_len);
> > +	tpm_buf_append(&buf, in, in_len);
> > +	tpm_buf_append_u16(&buf, TPM_ALG_NULL);
> > +	tpm_buf_append_u16(&buf, 0);
> > +	tpm_buf_fill_hmac_session(chip, &buf);
> > +	ret = tpm_transmit_cmd(chip, &buf, 4, "RSA key decrypting");
> > +	ret = tpm_buf_check_hmac_response(chip, &buf, ret);
> > +	if (ret) {
> > +		ret = -EIO;
> > +		goto err_blob;
> > +	}
> > +
> > +	pos = buf.data + TPM_HEADER_SIZE + 4;
> > +	decrypted_len = be16_to_cpup((__be16 *)pos);
> > +	pos += 2;
> > +
> > +	if (params->out_len < decrypted_len) {
> > +		ret = -EMSGSIZE;
> > +		goto err_blob;
> > +	}
> > +
> > +	memcpy(out, pos, decrypted_len);
> > +	ret = decrypted_len;
> > +
> > +err_blob:
> > +	tpm2_flush_context(chip, key_handle);
> > +
> > +err_buf:
> > +	tpm_buf_destroy(&buf);
> > +
> > +err_parent:
> > +	if (key->key.parent == TPM2_RH_NULL)
> > +		tpm2_flush_context(chip, parent);
> > +
> > +err_auth:
> > +	if (ret < 0)
> > +		tpm2_end_auth_session(chip);
> > +
> > +err_ops:
> > +	tpm_put_ops(chip);
> > +	return ret;
> > +}
> > +
> > +static int tpm2_key_rsa_decrypt(struct tpm_chip *chip, struct tpm2_key_rsa
> > *key,
> > +				struct kernel_pkey_params *params,
> > +				const void *in, void *out)
> > +{
> > +	const u8 *ptr;
> > +	int out_len;
> > +	u8 *work;
> > +	int ret;
> > +
> > +	work = kzalloc(params->out_len, GFP_KERNEL);
> > +	if (!work)
> > +		return -ENOMEM;
>
> Maybe use ERR_PTR() here and couple of more places

Hmm... but the function returns 'int'?

BR, Jarkko