diff mbox series

[01/12] X.509: Make certificate parser public

Message ID e3d7c94d89e09a6985ac2bf0a6d192b007f454bf.1695921657.git.lukas@wunner.de (mailing list archive)
State Changes Requested
Delegated to: Herbert Xu
Headers show
Series PCI device authentication | expand

Commit Message

Lukas Wunner Sept. 28, 2023, 5:32 p.m. UTC 
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>
---
 crypto/asymmetric_keys/x509_parser.h | 37 +----------------------
 include/keys/x509-parser.h           | 44 ++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 36 deletions(-)
 create mode 100644 include/keys/x509-parser.h

Comments

Ilpo Järvinen Oct. 3, 2023, 7:57 a.m. UTC | #1 
On Thu, 28 Sep 2023, 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>
> ---
>  crypto/asymmetric_keys/x509_parser.h | 37 +----------------------
>  include/keys/x509-parser.h           | 44 ++++++++++++++++++++++++++++
>  2 files changed, 45 insertions(+), 36 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 a299c9c56f40..a7ef43c39002 100644
> --- a/crypto/asymmetric_keys/x509_parser.h
> +++ b/crypto/asymmetric_keys/x509_parser.h
> @@ -5,40 +5,7 @@
>   * Written by David Howells (dhowells@redhat.com)
>   */
>  
> -#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>
>  
>  /*
>   * selftest.c
> @@ -52,8 +19,6 @@ static inline int fips_signature_selftest(void) { return 0; }
>  /*
>   * x509_cert_parser.c
>   */
> -extern void x509_free_certificate(struct x509_certificate *cert);
> -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..7c2ebc84791f
> --- /dev/null
> +++ b/include/keys/x509-parser.h
> @@ -0,0 +1,44 @@
> +/* 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)
> + */

Please add the include guard #ifndef + #define.

Other than that, this looks okay,

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Jonathan Cameron Oct. 3, 2023, 3:13 p.m. UTC | #2 
On Thu, 28 Sep 2023 19:32:32 +0200
Lukas Wunner <lukas@wunner.de> 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>
Got to where this is used now in my review and it makes sense there.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  crypto/asymmetric_keys/x509_parser.h | 37 +----------------------
>  include/keys/x509-parser.h           | 44 ++++++++++++++++++++++++++++
>  2 files changed, 45 insertions(+), 36 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 a299c9c56f40..a7ef43c39002 100644
> --- a/crypto/asymmetric_keys/x509_parser.h
> +++ b/crypto/asymmetric_keys/x509_parser.h
> @@ -5,40 +5,7 @@
>   * Written by David Howells (dhowells@redhat.com)
>   */
>  
> -#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>
>  
>  /*
>   * selftest.c
> @@ -52,8 +19,6 @@ static inline int fips_signature_selftest(void) { return 0; }
>  /*
>   * x509_cert_parser.c
>   */
> -extern void x509_free_certificate(struct x509_certificate *cert);
> -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..7c2ebc84791f
> --- /dev/null
> +++ b/include/keys/x509-parser.h
> @@ -0,0 +1,44 @@
> +/* 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)
> + */
> +
> +#include <crypto/public_key.h>
> +#include <keys/asymmetric-type.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);
Dan Williams Oct. 6, 2023, 6:47 p.m. UTC | #3 
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>

Looks good to me:

Reviewed-by: Dan Williams <dan.j.williams@intel.com>
diff mbox series

Patch

diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h
index a299c9c56f40..a7ef43c39002 100644
--- a/crypto/asymmetric_keys/x509_parser.h
+++ b/crypto/asymmetric_keys/x509_parser.h
@@ -5,40 +5,7 @@ 
  * Written by David Howells (dhowells@redhat.com)
  */
 
-#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>
 
 /*
  * selftest.c
@@ -52,8 +19,6 @@  static inline int fips_signature_selftest(void) { return 0; }
 /*
  * x509_cert_parser.c
  */
-extern void x509_free_certificate(struct x509_certificate *cert);
-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..7c2ebc84791f
--- /dev/null
+++ b/include/keys/x509-parser.h
@@ -0,0 +1,44 @@ 
+/* 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)
+ */
+
+#include <crypto/public_key.h>
+#include <keys/asymmetric-type.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);