bug 1143085 - allow subject alternative name extensions to be empty for compatibility r=briansmith a=kwierso

4 files changed, 59 insertions, 9 deletions

diff --git a/lib/mozpkix/lib/pkixnames.cpp b/lib/mozpkix/lib/pkixnames.cpp
index 49ce5593d..4ecb8198e 100644
--- a/lib/mozpkix/lib/pkixnames.cpp
+++ b/lib/mozpkix/lib/pkixnames.cpp
@@ -348,8 +348,10 @@ SearchNames(/*optional*/ const Input* subjectAltName,
       return rv;
     }
 
-    // do { ... } while(...) because subjectAltName isn't allowed to be empty.
-    do {
+    // According to RFC 5280, "If the subjectAltName extension is present, the
+    // sequence MUST contain at least one entry." For compatibility reasons, we
+    // do not enforce this. See bug 1143085.
+    while (!altNames.AtEnd()) {
       GeneralNameType presentedIDType;
       Input presentedID;
       rv = ReadGeneralName(altNames, presentedIDType, presentedID);
@@ -371,7 +373,7 @@ SearchNames(/*optional*/ const Input* subjectAltName,
           presentedIDType == GeneralNameType::iPAddress) {
         fallBackToCommonName = FallBackToSearchWithinSubject::No;
       }
-    } while (!altNames.AtEnd());
+    }
   }
 
   if (referenceIDType == GeneralNameType::nameConstraints) {
diff --git a/lib/mozpkix/test/gtest/pkixnames_tests.cpp b/lib/mozpkix/test/gtest/pkixnames_tests.cpp
index 8adcb96d4..29cefe2c4 100644
--- a/lib/mozpkix/test/gtest/pkixnames_tests.cpp
+++ b/lib/mozpkix/test/gtest/pkixnames_tests.cpp
@@ -1354,7 +1354,11 @@ static const CheckCertHostnameParams CHECK_CERT_HOSTNAME_PARAMS[] =
 
   // http://tools.ietf.org/html/rfc5280#section-4.2.1.6: "If the subjectAltName
   // extension is present, the sequence MUST contain at least one entry."
-  WITH_SAN("a", RDN(CN("a")), ByteString(), Result::ERROR_BAD_DER),
+  // However, for compatibility reasons, this is not enforced. See bug 1143085.
+  // This case is treated as if the extension is not present (i.e. name
+  // matching falls back to the subject CN).
+  WITH_SAN("a", RDN(CN("a")), ByteString(), Success),
+  WITH_SAN("a", RDN(CN("b")), ByteString(), Result::ERROR_BAD_CERT_DOMAIN),
 
   // http://tools.ietf.org/html/rfc5280#section-4.1.2.6 says "If subject naming
   // information is present only in the subjectAltName extension (e.g., a key
@@ -2217,11 +2221,6 @@ static const NameConstraintParams NAME_CONSTRAINT_PARAMS[] =
   { RDN(CN("b.example.com")), NO_SAN, GeneralSubtree(DNSName("a.example.com")),
     Result::ERROR_CERT_NOT_IN_NAME_SPACE, Success
   },
-  { // Empty SAN is rejected
-    RDN(CN("a.example.com")), ByteString(),
-    GeneralSubtree(DNSName("a.example.com")),
-    Result::ERROR_BAD_DER, Result::ERROR_BAD_DER
-  },
   { // DNSName CN-ID match is detected when there is a SAN w/o any DNSName or
     // IPAddress
     RDN(CN("a.example.com")), RFC822Name("foo@example.com"),
@@ -2392,6 +2391,34 @@ static const NameConstraintParams NAME_CONSTRAINT_PARAMS[] =
     GeneralSubtree(RFC822Name(".uses_underscore.example.com")),
     Success, Result::ERROR_CERT_NOT_IN_NAME_SPACE
   },
+
+  /////////////////////////////////////////////////////////////////////////////
+  // Name constraint tests that relate to having an empty SAN. According to RFC
+  // 5280 this isn't valid, but we allow it for compatibility reasons (see bug
+  // 1143085).
+  { // For DNSNames, we fall back to the subject CN.
+    RDN(CN("a.example.com")), ByteString(),
+    GeneralSubtree(DNSName("a.example.com")),
+    Success, Result::ERROR_CERT_NOT_IN_NAME_SPACE
+  },
+  { // For RFC822Names, we do not fall back to the subject emailAddress.
+    // This new implementation seems to conform better to the standards for
+    // RFC822 name constraints, by only applying the name constraints to
+    // emailAddress names in the certificate subject if there is no
+    // subjectAltName extension in the cert.
+    // In this case, the presence of the (empty) SAN extension means that RFC822
+    // name constraints are not enforced on the emailAddress attributes of the
+    // subject.
+    RDN(emailAddress("a@example.com")), ByteString(),
+    GeneralSubtree(RFC822Name("a@example.com")),
+    Success, Success
+  },
+  { // Compare this to the case where there is no SAN (i.e. the name
+    // constraints are enforced, because the extension is not present at all).
+    RDN(emailAddress("a@example.com")), NO_SAN,
+    GeneralSubtree(RFC822Name("a@example.com")),
+    Success, Result::ERROR_CERT_NOT_IN_NAME_SPACE
+  },
 };
 
 class pkixnames_CheckNameConstraints
diff --git a/lib/mozpkix/test/lib/pkixtestutil.cpp b/lib/mozpkix/test/lib/pkixtestutil.cpp
index 586e79c83..03fc5a443 100644
--- a/lib/mozpkix/test/lib/pkixtestutil.cpp
+++ b/lib/mozpkix/test/lib/pkixtestutil.cpp
@@ -685,6 +685,18 @@ OU(const ByteString& value)
   return AVA(tlv_id_at_organizationalUnitName, der::UTF8String, value);
 }
 
+ByteString
+emailAddress(const ByteString& value)
+{
+  // id-emailAddress AttributeType ::= { pkcs-9 1 }
+  // python DottedOIDToCode.py --tlv id-emailAddress 1.2.840.113549.1.9.1
+  static const uint8_t tlv_id_emailAddress[] = {
+    0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x09, 0x01
+  };
+
+  return AVA(tlv_id_emailAddress, der::IA5String, value);
+}
+
 // RelativeDistinguishedName ::=
 //   SET SIZE (1..MAX) OF AttributeTypeAndValue
 //
diff --git a/lib/mozpkix/test/lib/pkixtestutil.h b/lib/mozpkix/test/lib/pkixtestutil.h
index 5536cba7f..6adc5f697 100644
--- a/lib/mozpkix/test/lib/pkixtestutil.h
+++ b/lib/mozpkix/test/lib/pkixtestutil.h
@@ -146,6 +146,15 @@ OU(const char* value)
                        std::strlen(value)));
 }
 
+ByteString emailAddress(const ByteString&);
+
+inline ByteString
+emailAddress(const char* value)
+{
+  return emailAddress(ByteString(reinterpret_cast<const uint8_t*>(value),
+                                 std::strlen(value)));
+}
+
 // RelativeDistinguishedName ::=
 //   SET SIZE (1..MAX) OF AttributeTypeAndValue
 //