Re: [PATCH v2 01/18] X.509: Make certificate parser public

Alistair Francis
2024-07-10T02:46:47Z

On Sun, 2024-06-30 at 21:36 +0200, Lukas Wunner wrote:
> The upcoming support for PCI device authentication with CMA-SPDM
> (PCIe r6.1 sec 6.31) requires validating the Subject Alternative Name
> in X.509 certificates.
>
> High-level functions for X.509 parsing such as key_create_or_update()
> throw away the internal, low-level struct x509_certificate after
> extracting the struct public_key and public_key_signature from it.
> The Subject Alternative Name is thus inaccessible when using those
> functions.
>
> Afford CMA-SPDM access to the Subject Alternative Name by making
> struct
> x509_certificate public, together with the functions for parsing an
> X.509 certificate into such a struct and freeing such a struct.
>
> The private header file x509_parser.h previously included
> <linux/time.h>
> for the definition of time64_t.  That definition was since moved to
> <linux/time64.h> by commit 361a3bf00582 ("time64: Add time64.h header
> and define struct timespec64"), so adjust the #include directive as
> part
> of the move to the new public header file <keys/x509-parser.h>.
>
> No functional change intended.
>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

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

Alistair

> ---
>  crypto/asymmetric_keys/x509_parser.h | 40 +--------------------
>  include/keys/x509-parser.h           | 53
> ++++++++++++++++++++++++++++
>  2 files changed, 54 insertions(+), 39 deletions(-)
>  create mode 100644 include/keys/x509-parser.h
>
> diff --git a/crypto/asymmetric_keys/x509_parser.h
> b/crypto/asymmetric_keys/x509_parser.h
> index 0688c222806b..39f1521b773d 100644
> --- a/crypto/asymmetric_keys/x509_parser.h
> +++ b/crypto/asymmetric_keys/x509_parser.h
> @@ -5,49 +5,11 @@
>   * Written by David Howells (dhowells@redhat.com)
>   */
>
> -#include <linux/cleanup.h>
> -#include <linux/time.h>
> -#include <crypto/public_key.h>
> -#include <keys/asymmetric-type.h>
> -
> -struct x509_certificate {
> - struct x509_certificate *next;
> - struct x509_certificate *signer; /* Certificate that
> signed this one */
> - struct public_key *pub; /* Public
> key details */
> - struct public_key_signature *sig; /* Signature
> parameters */
> - char *issuer; /* Name of
> certificate issuer */
> - char *subject; /* Name of
> certificate subject */
> - struct asymmetric_key_id *id; /* Issuer + Serial
> number */
> - struct asymmetric_key_id *skid; /* Subject +
> subjectKeyId (optional) */
> - time64_t valid_from;
> - time64_t valid_to;
> - const void *tbs; /* Signed data */
> - unsigned tbs_size; /* Size of signed
> data */
> - unsigned raw_sig_size; /* Size of signature
> */
> - const void *raw_sig; /* Signature data */
> - const void *raw_serial; /* Raw serial number
> in ASN.1 */
> - unsigned raw_serial_size;
> - unsigned raw_issuer_size;
> - const void *raw_issuer; /* Raw issuer name
> in ASN.1 */
> - const void *raw_subject; /* Raw subject name
> in ASN.1 */
> - unsigned raw_subject_size;
> - unsigned raw_skid_size;
> - const void *raw_skid; /* Raw subjectKeyId
> in ASN.1 */
> - unsigned index;
> - bool seen; /* Infinite
> recursion prevention */
> - bool verified;
> - bool self_signed; /* T if self-signed
> (check unsupported_sig too) */
> - bool unsupported_sig; /* T if signature
> uses unsupported crypto */
> - bool blacklisted;
> -};
> +#include <keys/x509-parser.h>
>
>  /*
>   * x509_cert_parser.c
>   */
> -extern void x509_free_certificate(struct x509_certificate *cert);
> -DEFINE_FREE(x509_free_certificate, struct x509_certificate *,
> -     if (!IS_ERR(_T)) x509_free_certificate(_T))
> -extern struct x509_certificate *x509_cert_parse(const void *data,
> size_t datalen);
>  extern int x509_decode_time(time64_t *_t,  size_t hdrlen,
>       unsigned char tag,
>       const unsigned char *value, size_t
> vlen);
> diff --git a/include/keys/x509-parser.h b/include/keys/x509-parser.h
> new file mode 100644
> index 000000000000..37436a5c7526
> --- /dev/null
> +++ b/include/keys/x509-parser.h
> @@ -0,0 +1,53 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/* X.509 certificate parser
> + *
> + * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@redhat.com)
> + */
> +
> +#ifndef _KEYS_X509_PARSER_H
> +#define _KEYS_X509_PARSER_H
> +
> +#include <crypto/public_key.h>
> +#include <keys/asymmetric-type.h>
> +#include <linux/cleanup.h>
> +#include <linux/time64.h>
> +
> +struct x509_certificate {
> + struct x509_certificate *next;
> + struct x509_certificate *signer; /* Certificate that
> signed this one */
> + struct public_key *pub; /* Public
> key details */
> + struct public_key_signature *sig; /* Signature
> parameters */
> + char *issuer; /* Name of
> certificate issuer */
> + char *subject; /* Name of
> certificate subject */
> + struct asymmetric_key_id *id; /* Issuer + Serial
> number */
> + struct asymmetric_key_id *skid; /* Subject +
> subjectKeyId (optional) */
> + time64_t valid_from;
> + time64_t valid_to;
> + const void *tbs; /* Signed data */
> + unsigned tbs_size; /* Size of signed
> data */
> + unsigned raw_sig_size; /* Size of signature
> */
> + const void *raw_sig; /* Signature data */
> + const void *raw_serial; /* Raw serial number
> in ASN.1 */
> + unsigned raw_serial_size;
> + unsigned raw_issuer_size;
> + const void *raw_issuer; /* Raw issuer name
> in ASN.1 */
> + const void *raw_subject; /* Raw subject name
> in ASN.1 */
> + unsigned raw_subject_size;
> + unsigned raw_skid_size;
> + const void *raw_skid; /* Raw subjectKeyId
> in ASN.1 */
> + unsigned index;
> + bool seen; /* Infinite
> recursion prevention */
> + bool verified;
> + bool self_signed; /* T if self-signed
> (check unsupported_sig too) */
> + bool unsupported_sig; /* T if signature
> uses unsupported crypto */
> + bool blacklisted;
> +};
> +
> +struct x509_certificate *x509_cert_parse(const void *data, size_t
> datalen);
> +void x509_free_certificate(struct x509_certificate *cert);
> +
> +DEFINE_FREE(x509_free_certificate, struct x509_certificate *,
> +     if (!IS_ERR(_T)) x509_free_certificate(_T))
> +
> +#endif /* _KEYS_X509_PARSER_H */