[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@xxxxxxxxxxxxxxx # v3.7+
Reported-by: Maximilian Stahlberg <maximilian.stahlberg@xxxxxxxxxxxxx>
Closes: https://lore.kernel.org/r/9b05b89cf296b8a6392bbe6327228c1bc2f05c49.camel@xxxxxxxxxxxxx
Signed-off-by: Laveesh Bansal <laveeshb@xxxxxxxxxxxxxxxxx>
---
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