Re: [PATCH v2 03/18] X.509: Move certificate length retrieval into new helper

Alistair Francis
2024-07-10T02:49:49Z

On Sun, 2024-06-30 at 21:38 +0200, Lukas Wunner wrote:
> The upcoming in-kernel SPDM library (Security Protocol and Data
> Model,
> https://www.dmtf.org/dsp/DSP0274) needs to retrieve the length from
> ASN.1 DER-encoded X.509 certificates.
>
> Such code already exists in x509_load_certificate_list(), so move it
> into a new helper for reuse by SPDM.
>
> Export the helper so that SPDM can be tristate.  (Some upcoming users
> of
> the SPDM libray may be modular, such as SCSI and ATA.)
>
> No functional change intended.
>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  crypto/asymmetric_keys/x509_loader.c | 38 +++++++++++++++++++-------
> --
>  include/keys/asymmetric-type.h       |  2 ++
>  2 files changed, 28 insertions(+), 12 deletions(-)
>
> diff --git a/crypto/asymmetric_keys/x509_loader.c
> b/crypto/asymmetric_keys/x509_loader.c
> index a41741326998..25ff027fad1d 100644
> --- a/crypto/asymmetric_keys/x509_loader.c
> +++ b/crypto/asymmetric_keys/x509_loader.c
> @@ -4,28 +4,42 @@
>  #include <linux/key.h>
>  #include <keys/asymmetric-type.h>
>
> +ssize_t x509_get_certificate_length(const u8 *p, unsigned long
> buflen)
> +{
> + ssize_t plen;
> +
> + /* Each cert begins with an ASN.1 SEQUENCE tag and must be
> more
> + * than 256 bytes in size.
> + */
> + if (buflen < 4)
> + return -EINVAL;
> +
> + if (p[0] != 0x30 &&
> +     p[1] != 0x82)
> + return -EINVAL;
> +
> + plen = (p[2] << 8) | p[3];
> + plen += 4;
> + if (plen > buflen)
> + return -EINVAL;
> +
> + return plen;
> +}
> +EXPORT_SYMBOL_GPL(x509_get_certificate_length);
> +
>  int x509_load_certificate_list(const u8 cert_list[],
>          const unsigned long list_size,
>          const struct key *keyring)
>  {
>   key_ref_t key;
>   const u8 *p, *end;
> - size_t plen;
> + ssize_t plen;
>
>   p = cert_list;
>   end = p + list_size;
>   while (p < end) {
> - /* Each cert begins with an ASN.1 SEQUENCE tag and
> must be more
> - * than 256 bytes in size.
> - */
> - if (end - p < 4)
> - goto dodgy_cert;
> - if (p[0] != 0x30 &&
> -     p[1] != 0x82)
> - goto dodgy_cert;
> - plen = (p[2] << 8) | p[3];
> - plen += 4;
> - if (plen > end - p)
> + plen = x509_get_certificate_length(p, end - p);
> + if (plen < 0)
>   goto dodgy_cert;
>
>   key = key_create_or_update(make_key_ref(keyring, 1),
> diff --git a/include/keys/asymmetric-type.h
> b/include/keys/asymmetric-type.h
> index 69a13e1e5b2e..e2af07fec3c6 100644
> --- a/include/keys/asymmetric-type.h
> +++ b/include/keys/asymmetric-type.h
> @@ -84,6 +84,8 @@ extern struct key *find_asymmetric_key(struct key
> *keyring,
>          const struct
> asymmetric_key_id *id_2,
>          bool partial);
>
> +ssize_t x509_get_certificate_length(const u8 *p, unsigned long
> buflen);
> +
>  int x509_load_certificate_list(const u8 cert_list[], const unsigned
> long list_size,
>          const struct key *keyring);
>