[PATCH 0/2] X.509: Accept a certificate with an empty subject

[PATCH 0/2] X.509: Accept a certificate with an empty subject

[Laveesh Bansal]

Patch 1 fixes loading a certificate with an empty Subject, which iwd
hits with an eduroam server certificate [1]. Patch 2 adds a KUnit test
for it.

I developed this series using an LLM coding assistant.

[1] https://lore.kernel.org/r/9b05b89cf296b8a6392bbe6327228c1bc2f05c49.camel@uni-bremen.de


Laveesh Bansal (2):
  X.509: Accept a certificate with an empty subject
  X.509: Add KUnit tests for the certificate parser

 crypto/asymmetric_keys/Kconfig            |  11 ++
 crypto/asymmetric_keys/Makefile           |   1 +
 crypto/asymmetric_keys/x509.asn1          |   8 +-
 crypto/asymmetric_keys/x509_parser_test.c | 160 ++++++++++++++++++++++
 4 files changed, 179 insertions(+), 1 deletion(-)
 create mode 100644 crypto/asymmetric_keys/x509_parser_test.c

-- 
2.54.0

[PATCH 1/2] X.509: Accept a certificate with an empty subject

[Laveesh Bansal]

A certificate with an empty Subject, which RFC 5280 allows, fails to
load: add_key() returns -EBADMSG. iwd hits this with an eduroam server
certificate.

The subject is parsed with the Name rule. The ASN.1 compiler turns a
SEQUENCE OF into a loop that runs at least once, so the first RDN is
mandatory and an empty SEQUENCE runs out of data in the decoder.

Give the subject its own rule, a SEQUENCE OF a one-member CHOICE, so the
first RDN can be absent. Anything that is not a SET is still rejected.
The issuer keeps the Name rule, as RFC 5280 requires it to be non-empty.

Fixes: c26fd69fa009 ("X.509: Add a crypto key parser for binary (DER) X.509 certificates")
Cc: stable@vger.kernel.org # v3.7+
Reported-by: Maximilian Stahlberg <maximilian.stahlberg@uni-bremen.de>
Closes: https://lore.kernel.org/r/9b05b89cf296b8a6392bbe6327228c1bc2f05c49.camel@uni-bremen.de
Signed-off-by: Laveesh Bansal <laveeshb@laveeshbansal.com>
---
Tested in QEMU/KVM guests on arm64 and x86_64 (cbbddb6b08cc, 7.3.0) with
KASAN, lockdep and debug lists on. Each cell is the add_key() result
before / after the patch, the same on both.

  case                                        before / after
  empty Subject                               EBADMSG / ok
  the reporter's certificate                  EBADMSG / ok
  Subject with 1 or 4 RDNs                    ok / ok
  empty Issuer, truncated, garbage            EBADMSG / EBADMSG
  malformed Subject (a member that is
        not a SET, or an empty SET)           EBADMSG / EBADMSG
  chain: empty Subject, CA-restricted ring    EBADMSG / ok
  chain: empty Subject, signed by another CA  EBADMSG / ENOKEY

The built-in keyring and PKCS#7 messages share this parser and now accept
such certificates too (tested on arm64). I didn't find any lookup that can
hit an empty Subject. The only one that compares Subjects is the dn:
fallback, and it would need an empty Issuer, which is still rejected.

Letting the compiler accept an empty SEQUENCE OF would need an opt-in,
since Extensions must stay non-empty. The CHOICE is that opt-in for the
subject alone; x509_akid.asn1 already uses the same shape for
GeneralNames.
 crypto/asymmetric_keys/x509.asn1 | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/crypto/asymmetric_keys/x509.asn1 b/crypto/asymmetric_keys/x509.asn1
index feb9573cacce..d22f7bad9721 100644
--- a/crypto/asymmetric_keys/x509.asn1
+++ b/crypto/asymmetric_keys/x509.asn1
@@ -17,7 +17,7 @@ TBSCertificate ::= SEQUENCE {
 	signature		AlgorithmIdentifier ({ x509_note_sig_algo }),
 	issuer			Name ({ x509_note_issuer }),
 	validity		Validity,
-	subject			Name ({ x509_note_subject }),
+	subject			SubjectName ({ x509_note_subject }),
 	subjectPublicKeyInfo	SubjectPublicKeyInfo,
 	issuerUniqueID    [ 1 ]	IMPLICIT UniqueIdentifier OPTIONAL,
 	subjectUniqueID   [ 2 ]	IMPLICIT UniqueIdentifier OPTIONAL,
@@ -34,6 +34,12 @@ AlgorithmIdentifier ::= SEQUENCE {
 
 Name ::= SEQUENCE OF RelativeDistinguishedName
 
+-- RFC 5280 4.1.2.6 allows an empty subject.  The CHOICE lets the first RDN be
+-- absent, and its COND_FAIL still rejects a member that is not a SET.
+SubjectName ::= SEQUENCE OF CHOICE {
+	rdn	RelativeDistinguishedName
+}
+
 RelativeDistinguishedName ::= SET OF AttributeValueAssertion
 
 AttributeValueAssertion ::= SEQUENCE {
-- 
2.54.0

[PATCH 2/2] X.509: Add KUnit tests for the certificate parser

[Laveesh Bansal]

Add a KUnit test for x509_cert_parse() that builds a small certificate
in memory with different Subject and Issuer names. It checks that a
certificate with an empty Subject parses, and that a Subject with a
member that is not a SET and an empty Issuer are rejected.

Signed-off-by: Laveesh Bansal <laveeshb@laveeshbansal.com>
---
Without patch 1, x509_empty_subject fails and the other three cases pass;
with it, all four pass (arm64 QEMU/KVM guest, cbbddb6b08cc, 7.3.0).
 crypto/asymmetric_keys/Kconfig            |  11 ++
 crypto/asymmetric_keys/Makefile           |   1 +
 crypto/asymmetric_keys/x509_parser_test.c | 160 ++++++++++++++++++++++
 3 files changed, 172 insertions(+)
 create mode 100644 crypto/asymmetric_keys/x509_parser_test.c

diff --git a/crypto/asymmetric_keys/Kconfig b/crypto/asymmetric_keys/Kconfig
index c73979b2e98e..ca2c11f82e32 100644
--- a/crypto/asymmetric_keys/Kconfig
+++ b/crypto/asymmetric_keys/Kconfig
@@ -126,4 +126,15 @@ config VERIFY_PEFILE_KUNIT_TEST
 
 	  If unsure, say N.
 
+config X509_PARSER_KUNIT_TEST
+	tristate "KUnit tests for X.509 certificate parsing" if !KUNIT_ALL_TESTS
+	depends on KUNIT && X509_CERTIFICATE_PARSER
+	default KUNIT_ALL_TESTS
+	help
+	  Enable KUnit tests for the X.509 certificate parser.  The tests build
+	  small certificates in memory and check which Subject and Issuer
+	  names x509_cert_parse() accepts.
+
+	  If unsure, say N.
+
 endif # ASYMMETRIC_KEY_TYPE
diff --git a/crypto/asymmetric_keys/Makefile b/crypto/asymmetric_keys/Makefile
index 35172685c533..1ce91737ac89 100644
--- a/crypto/asymmetric_keys/Makefile
+++ b/crypto/asymmetric_keys/Makefile
@@ -22,6 +22,7 @@ x509_key_parser-y := \
 	x509_cert_parser.o \
 	x509_loader.o \
 	x509_public_key.o
+obj-$(CONFIG_X509_PARSER_KUNIT_TEST) += x509_parser_test.o
 obj-$(CONFIG_FIPS_SIGNATURE_SELFTEST) += x509_selftest.o
 x509_selftest-y += selftest.o
 x509_selftest-$(CONFIG_FIPS_SIGNATURE_SELFTEST_RSA) += selftest_rsa.o
diff --git a/crypto/asymmetric_keys/x509_parser_test.c b/crypto/asymmetric_keys/x509_parser_test.c
new file mode 100644
index 000000000000..c005b6fe68a6
--- /dev/null
+++ b/crypto/asymmetric_keys/x509_parser_test.c
@@ -0,0 +1,160 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit tests for the X.509 certificate parser.
+ *
+ * The cases build a minimal certificate in memory, vary the Subject or Issuer
+ * name, and call x509_cert_parse().  The key and signature are dummies, so
+ * Issuer and Subject must differ or the parser tries to verify the signature
+ * as a self-signed certificate.
+ */
+#include <kunit/test.h>
+#include <linux/cleanup.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include "x509_parser.h"
+
+/* Name DERs: empty, CN=x, CN=y, and one whose member is not a SET. */
+static const u8 name_empty[] = { 0x30, 0x00 };
+static const u8 name_cn[] = {
+	0x30, 0x0c, 0x31, 0x0a, 0x30, 0x08, 0x06, 0x03, 0x55, 0x04, 0x03,
+	0x0c, 0x01, 0x78
+};
+
+static const u8 name_other_cn[] = {
+	0x30, 0x0c, 0x31, 0x0a, 0x30, 0x08, 0x06, 0x03, 0x55, 0x04, 0x03,
+	0x0c, 0x01, 0x79
+};
+
+static const u8 name_not_a_set[] = { 0x30, 0x03, 0x02, 0x01, 0x00 };
+
+static const u8 cert_version[] = { 0xa0, 0x03, 0x02, 0x01, 0x02 };
+static const u8 cert_serial[] = { 0x02, 0x01, 0x01 };
+/* sha256WithRSAEncryption, NULL parameters */
+static const u8 cert_sig_algo[] = {
+	0x30, 0x0d, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01,
+	0x0b, 0x05, 0x00
+};
+
+/* 2026-01-01 to 2036-01-01 */
+static const u8 cert_validity[] = {
+	0x30, 0x1e,
+	0x17, 0x0d, '2', '6', '0', '1', '0', '1', '0', '0', '0', '0', '0', '0', 'Z',
+	0x17, 0x0d, '3', '6', '0', '1', '0', '1', '0', '0', '0', '0', '0', '0', 'Z'
+};
+
+/* rsaEncryption with a dummy 4-byte key; the key is not parsed here */
+static const u8 cert_spki[] = {
+	0x30, 0x16,
+	0x30, 0x0d, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01,
+	0x01, 0x05, 0x00,
+	0x03, 0x05, 0x00, 0x01, 0x02, 0x03, 0x04
+};
+
+static const u8 cert_signature[] = { 0x03, 0x05, 0x00, 0x01, 0x02, 0x03, 0x04 };
+
+/* Write tag, length (up to 255) and @len bytes of @val at @out. */
+static u8 *put_tlv(u8 *out, u8 tag, const u8 *val, size_t len)
+{
+	*out++ = tag;
+	if (len > 127)
+		*out++ = 0x81;
+	*out++ = len;
+	memcpy(out, val, len);
+	return out + len;
+}
+
+static u8 *put_raw(u8 *out, const u8 *val, size_t len)
+{
+	memcpy(out, val, len);
+	return out + len;
+}
+
+static struct x509_certificate *parse_cert(struct kunit *test,
+					   const u8 *issuer, size_t issuer_len,
+					   const u8 *subject, size_t subject_len)
+{
+	/* The parsed certificate keeps pointers into this buffer. */
+	u8 *cert = kunit_kzalloc(test, 256, GFP_KERNEL);
+	u8 tbs[160], body[256], *p = tbs, *q = body;
+
+	KUNIT_ASSERT_NOT_NULL(test, cert);
+	/* Keep tbs[] and body[] large enough for the fixed parts plus the names. */
+	KUNIT_ASSERT_LE(test, issuer_len + subject_len, 64);
+
+	p = put_raw(p, cert_version, sizeof(cert_version));
+	p = put_raw(p, cert_serial, sizeof(cert_serial));
+	p = put_raw(p, cert_sig_algo, sizeof(cert_sig_algo));
+	p = put_raw(p, issuer, issuer_len);
+	p = put_raw(p, cert_validity, sizeof(cert_validity));
+	p = put_raw(p, subject, subject_len);
+	p = put_raw(p, cert_spki, sizeof(cert_spki));
+
+	q = put_tlv(q, 0x30, tbs, p - tbs);
+	q = put_raw(q, cert_sig_algo, sizeof(cert_sig_algo));
+	q = put_raw(q, cert_signature, sizeof(cert_signature));
+	p = put_tlv(cert, 0x30, body, q - body);
+
+	return x509_cert_parse(cert, p - cert);
+}
+
+/* RFC 5280 allows an empty Subject, so the certificate must parse. */
+static void x509_empty_subject(struct kunit *test)
+{
+	struct x509_certificate *cert __free(x509_free_certificate) =
+		parse_cert(test, name_other_cn, sizeof(name_other_cn),
+			   name_empty, sizeof(name_empty));
+
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, cert);
+	/* raw_subject_size counts the contents, not the tag and length. */
+	KUNIT_EXPECT_EQ(test, cert->raw_subject_size, 0);
+}
+
+/* Control: the same certificate with a one-RDN Subject parses. */
+static void x509_named_subject(struct kunit *test)
+{
+	struct x509_certificate *cert __free(x509_free_certificate) =
+		parse_cert(test, name_other_cn, sizeof(name_other_cn),
+			   name_cn, sizeof(name_cn));
+
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, cert);
+	KUNIT_EXPECT_EQ(test, cert->raw_subject_size, sizeof(name_cn) - 2);
+}
+
+/* A Subject member that is not a SET must be rejected, not looped over. */
+static void x509_subject_not_a_set(struct kunit *test)
+{
+	struct x509_certificate *cert __free(x509_free_certificate) =
+		parse_cert(test, name_other_cn, sizeof(name_other_cn),
+			   name_not_a_set, sizeof(name_not_a_set));
+
+	KUNIT_EXPECT_EQ(test, PTR_ERR_OR_ZERO(cert), -EBADMSG);
+}
+
+/* The Issuer must not be empty. */
+static void x509_empty_issuer(struct kunit *test)
+{
+	struct x509_certificate *cert __free(x509_free_certificate) =
+		parse_cert(test, name_empty, sizeof(name_empty),
+			   name_cn, sizeof(name_cn));
+
+	KUNIT_EXPECT_EQ(test, PTR_ERR_OR_ZERO(cert), -EBADMSG);
+}
+
+static struct kunit_case x509_parser_cases[] = {
+	KUNIT_CASE(x509_empty_subject),
+	KUNIT_CASE(x509_named_subject),
+	KUNIT_CASE(x509_subject_not_a_set),
+	KUNIT_CASE(x509_empty_issuer),
+	{}
+};
+
+static struct kunit_suite x509_parser_suite = {
+	.name = "x509_parser",
+	.test_cases = x509_parser_cases,
+};
+
+kunit_test_suite(x509_parser_suite);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("KUnit tests for the X.509 certificate parser");
-- 
2.54.0

Re: [PATCH 1/2] X.509: Accept a certificate with an empty subject

[Eric Biggers]

On Wed, Nov 11, 2026 at 11:42:45PM -0800, Laveesh Bansal wrote:
> A certificate with an empty Subject, which RFC 5280 allows, fails to
> load: add_key() returns -EBADMSG. iwd hits this with an eduroam server
> certificate.
> 
> The subject is parsed with the Name rule. The ASN.1 compiler turns a
> SEQUENCE OF into a loop that runs at least once, so the first RDN is
> mandatory and an empty SEQUENCE runs out of data in the decoder.
> 
> Give the subject its own rule, a SEQUENCE OF a one-member CHOICE, so the
> first RDN can be absent. Anything that is not a SET is still rejected.
> The issuer keeps the Name rule, as RFC 5280 requires it to be non-empty.
> 
> Fixes: c26fd69fa009 ("X.509: Add a crypto key parser for binary (DER) X.509 certificates")
> Cc: stable@vger.kernel.org # v3.7+
> Reported-by: Maximilian Stahlberg <maximilian.stahlberg@uni-bremen.de>
> Closes: https://lore.kernel.org/r/9b05b89cf296b8a6392bbe6327228c1bc2f05c49.camel@uni-bremen.de
> Signed-off-by: Laveesh Bansal <laveeshb@laveeshbansal.com>

This problem (and many others) could also be solved if iwd switched to
use a userspace crypto library instead of misusing the kernel APIs.

- Eric