bug 1143085 - allow subject alternative name extensions to be empty for compatibility r=briansmith a=kwierso
authorDavid Keeler <dkeeler@mozilla.com>
Mon, 16 Mar 2015 14:00:33 -0700
changeset 263413 8c039a4e032de4d22ae98d843e819ca75c9ac520
parent 263412 4510d411652770a9f23cb5031cd25a8bc6ab47c8
child 263414 6d94c4cf9813cd8feac6b3e668439054ae94559c
push id4718
push userraliiev@mozilla.com
push dateMon, 11 May 2015 18:39:53 +0000
treeherdermozilla-beta@c20c4ef55f08 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbriansmith, kwierso
bugs1143085
milestone39.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
bug 1143085 - allow subject alternative name extensions to be empty for compatibility r=briansmith a=kwierso
security/pkix/lib/pkixnames.cpp
security/pkix/test/gtest/pkixnames_tests.cpp
security/pkix/test/lib/pkixtestutil.cpp
security/pkix/test/lib/pkixtestutil.h
--- a/security/pkix/lib/pkixnames.cpp
+++ b/security/pkix/lib/pkixnames.cpp
@@ -343,18 +343,20 @@ SearchNames(/*optional*/ const Input* su
   if (subjectAltName) {
     Reader altNames;
     rv = der::ExpectTagAndGetValueAtEnd(*subjectAltName, der::SEQUENCE,
                                         altNames);
     if (rv != Success) {
       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);
       if (rv != Success) {
         return rv;
       }
 
       rv = MatchPresentedIDWithReferenceID(presentedIDType, presentedID,
@@ -366,17 +368,17 @@ SearchNames(/*optional*/ const Input* su
       if (referenceIDType != GeneralNameType::nameConstraints &&
           match == MatchResult::Match) {
         return Success;
       }
       if (presentedIDType == GeneralNameType::dNSName ||
           presentedIDType == GeneralNameType::iPAddress) {
         fallBackToCommonName = FallBackToSearchWithinSubject::No;
       }
-    } while (!altNames.AtEnd());
+    }
   }
 
   if (referenceIDType == GeneralNameType::nameConstraints) {
     rv = CheckPresentedIDConformsToConstraints(GeneralNameType::directoryName,
                                                subject, referenceID);
     if (rv != Success) {
       return rv;
     }
--- a/security/pkix/test/gtest/pkixnames_tests.cpp
+++ b/security/pkix/test/gtest/pkixnames_tests.cpp
@@ -1349,17 +1349,21 @@ static const CheckCertHostnameParams CHE
   // Duplicate DNSName.
   WITH_SAN("a", RDN(CN("foo")), DNSName("a") + DNSName("a"), Success),
   // After an invalid DNSName.
   WITH_SAN("b", RDN(CN("foo")), DNSName("!") + DNSName("b"),
            Result::ERROR_BAD_DER),
 
   // 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
   // bound only to an email address or URI), then the subject name MUST be an
   // empty sequence and the subjectAltName extension MUST be critical." So, we
   // have to support an empty subject. We don't enforce that the SAN must be
   // critical or even that there is a SAN when the subject is empty, though.
   WITH_SAN("a", ByteString(), DNSName("a"), Success),
@@ -2212,21 +2216,16 @@ static const NameConstraintParams NAME_C
     Success, Success
   },
   { RDN(CN("a.example.com")), NO_SAN, GeneralSubtree(DNSName("a.example.com")),
     Success, Result::ERROR_CERT_NOT_IN_NAME_SPACE
   },
   { 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"),
     GeneralSubtree(DNSName("a.example.com")),
     Success, Result::ERROR_CERT_NOT_IN_NAME_SPACE
   },
   { // DNSName CN-ID mismatch is detected when there is a SAN w/o any DNSName
     // or IPAddress
@@ -2387,16 +2386,44 @@ static const NameConstraintParams NAME_C
   { ByteString(), RFC822Name("uses_underscore@example.com"),
     GeneralSubtree(RFC822Name("example.com")),
     Success, Result::ERROR_CERT_NOT_IN_NAME_SPACE
   },
   { ByteString(), RFC822Name("a@a.uses_underscore.example.com"),
     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
   : public ::testing::Test
   , public ::testing::WithParamInterface<NameConstraintParams>
 {
 };
 
--- a/security/pkix/test/lib/pkixtestutil.cpp
+++ b/security/pkix/test/lib/pkixtestutil.cpp
@@ -680,16 +680,28 @@ OU(const ByteString& value)
   // python DottedOIDToCode.py --tlv id-at-organizationalUnitName 2.5.4.11
   static const uint8_t tlv_id_at_organizationalUnitName[] = {
     0x06, 0x03, 0x55, 0x04, 0x0b
   };
 
   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
 //
 ByteString
 RDN(const ByteString& avas)
 {
   return TLV(der::SET, avas);
 }
--- a/security/pkix/test/lib/pkixtestutil.h
+++ b/security/pkix/test/lib/pkixtestutil.h
@@ -141,16 +141,25 @@ ByteString OU(const ByteString&);
 
 inline ByteString
 OU(const char* value)
 {
   return OU(ByteString(reinterpret_cast<const uint8_t*>(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
 //
 ByteString RDN(const ByteString& avas);
 
 // Name ::= CHOICE { -- only one possibility for now --
 //   rdnSequence  RDNSequence }
 //