[EXTERNAL] [PATCH v2 2/6] lib: Expand asn1_encode_integer() to variable size integers
Bharat Bhushan
bbhushan2 at marvell.com
Tue May 21 05:36:06 UTC 2024
> -----Original Message-----
> From: Jarkko Sakkinen <jarkko at kernel.org>
> Sent: Tuesday, May 21, 2024 8:46 AM
> To: Herbert Xu <herbert at gondor.apana.org.au>
> Cc: linux-integrity at vger.kernel.org; keyrings at vger.kernel.org;
> Andreas.Fuchs at infineon.com; James Prestwood <prestwoj at gmail.com>;
> David Woodhouse <dwmw2 at infradead.org>; Eric Biggers
> <ebiggers at kernel.org>; James Bottomley
> <James.Bottomley at hansenpartnership.com>; Jarkko Sakkinen
> <jarkko at kernel.org>; David S. Miller <davem at davemloft.net>; open
> list:CRYPTO API <linux-crypto at vger.kernel.org>; open list <linux-
> kernel at vger.kernel.org>; Andrew Morton <akpm at linux-foundation.org>;
> James Bottomley <James.Bottomley at HansenPartnership.com>; Mimi Zohar
> <zohar at linux.ibm.com>; David Howells <dhowells at redhat.com>; Paul Moore
> <paul at paul-moore.com>; James Morris <jmorris at namei.org>; Serge E. Hallyn
> <serge at hallyn.com>; open list:SECURITY SUBSYSTEM <linux-security-
> module at 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 at 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
>
More information about the Linux-security-module-archive
mailing list