Bug 1150114 - Allow PrintableString to match UTF8String in name constraints checking. r=briansmith, a=sledru
authorDavid Keeler <dkeeler@mozilla.com>
Wed, 08 Apr 2015 16:17:39 -0700
changeset 260205 a00d0de3202f
parent 260204 630336da65f2
child 260210 69173cc17556
push id721
push userjlund@mozilla.com
push date2015-04-21 23:03 +0000
treeherdermozilla-release@d27c9211ebb3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbriansmith, sledru
bugs1150114
milestone38.0
Bug 1150114 - Allow PrintableString to match UTF8String in name constraints checking. r=briansmith, a=sledru
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
@@ -129,22 +129,28 @@ Result SearchNames(const Input* subjectA
                    FallBackToSearchWithinSubject fallBackToCommonName,
                    /*out*/ MatchResult& match);
 Result SearchWithinRDN(Reader& rdn,
                        GeneralNameType referenceIDType,
                        Input referenceID,
                        FallBackToSearchWithinSubject fallBackToEmailAddress,
                        FallBackToSearchWithinSubject fallBackToCommonName,
                        /*in/out*/ MatchResult& match);
-Result SearchWithinAVA(Reader& rdn,
-                       GeneralNameType referenceIDType,
-                       Input referenceID,
-                       FallBackToSearchWithinSubject fallBackToEmailAddress,
-                       FallBackToSearchWithinSubject fallBackToCommonName,
-                       /*in/out*/ MatchResult& match);
+Result MatchAVA(Input type,
+                uint8_t valueEncodingTag,
+                Input presentedID,
+                GeneralNameType referenceIDType,
+                Input referenceID,
+                FallBackToSearchWithinSubject fallBackToEmailAddress,
+                FallBackToSearchWithinSubject fallBackToCommonName,
+                /*in/out*/ MatchResult& match);
+Result ReadAVA(Reader& rdn,
+               /*out*/ Input& type,
+               /*out*/ uint8_t& valueTag,
+               /*out*/ Input& value);
 void MatchSubjectPresentedIDWithReferenceID(GeneralNameType presentedIDType,
                                             Input presentedID,
                                             GeneralNameType referenceIDType,
                                             Input referenceID,
                                             /*in/out*/ MatchResult& match);
 
 Result MatchPresentedIDWithReferenceID(GeneralNameType presentedIDType,
                                        Input presentedID,
@@ -483,21 +489,25 @@ Result
 SearchWithinRDN(Reader& rdn,
                 GeneralNameType referenceIDType,
                 Input referenceID,
                 FallBackToSearchWithinSubject fallBackToEmailAddress,
                 FallBackToSearchWithinSubject fallBackToCommonName,
                 /*in/out*/ MatchResult& match)
 {
   do {
-    Result rv = der::Nested(rdn, der::SEQUENCE, [&](Reader& r) {
-      return SearchWithinAVA(r, referenceIDType, referenceID,
-                             fallBackToEmailAddress, fallBackToCommonName,
-                             match);
-    });
+    Input type;
+    uint8_t valueTag;
+    Input value;
+    Result rv = ReadAVA(rdn, type, valueTag, value);
+    if (rv != Success) {
+      return rv;
+    }
+    rv = MatchAVA(type, valueTag, value, referenceIDType, referenceID,
+                  fallBackToEmailAddress, fallBackToCommonName, match);
     if (rv != Success) {
       return rv;
     }
   } while (!rdn.AtEnd());
 
   return Success;
 }
 
@@ -511,36 +521,23 @@ SearchWithinRDN(Reader& rdn,
 //
 // DirectoryString ::= CHOICE {
 //       teletexString           TeletexString (SIZE (1..MAX)),
 //       printableString         PrintableString (SIZE (1..MAX)),
 //       universalString         UniversalString (SIZE (1..MAX)),
 //       utf8String              UTF8String (SIZE (1..MAX)),
 //       bmpString               BMPString (SIZE (1..MAX)) }
 Result
-SearchWithinAVA(Reader& rdn,
-                GeneralNameType referenceIDType,
-                Input referenceID,
-                FallBackToSearchWithinSubject fallBackToEmailAddress,
-                FallBackToSearchWithinSubject fallBackToCommonName,
-                /*in/out*/ MatchResult& match)
+MatchAVA(Input type, uint8_t valueEncodingTag, Input presentedID,
+         GeneralNameType referenceIDType,
+         Input referenceID,
+         FallBackToSearchWithinSubject fallBackToEmailAddress,
+         FallBackToSearchWithinSubject fallBackToCommonName,
+         /*in/out*/ MatchResult& match)
 {
-  // AttributeTypeAndValue ::= SEQUENCE {
-  //   type     AttributeType,
-  //   value    AttributeValue }
-  //
-  // AttributeType ::= OBJECT IDENTIFIER
-  //
-  // AttributeValue ::= ANY -- DEFINED BY AttributeType
-  Reader type;
-  Result rv = der::ExpectTagAndGetValue(rdn, der::OIDTag, type);
-  if (rv != Success) {
-    return rv;
-  }
-
   // Try to match the  CN as a DNSName or an IPAddress.
   //
   // id-at-commonName        AttributeType ::= { id-at 3 }
   //
   // -- Naming attributes of type X520CommonName:
   // --   X520CommonName ::= DirectoryName (SIZE (1..ub-common-name))
   // --
   // -- Expanded to avoid parameterized type:
@@ -551,29 +548,22 @@ SearchWithinAVA(Reader& rdn,
   //       utf8String        UTF8String      (SIZE (1..ub-common-name)),
   //       bmpString         BMPString       (SIZE (1..ub-common-name)) }
   //
   // python DottedOIDToCode.py id-at-commonName 2.5.4.3
   static const uint8_t id_at_commonName[] = {
     0x55, 0x04, 0x03
   };
   if (fallBackToCommonName == FallBackToSearchWithinSubject::Yes &&
-      type.MatchRest(id_at_commonName)) {
+      InputsAreEqual(type, Input(id_at_commonName))) {
     // We might have previously found a match. Now that we've found another CN,
     // we no longer consider that previous match to be a match, so "forget" about
     // it.
     match = MatchResult::NoNamesOfGivenType;
 
-    uint8_t valueEncodingTag;
-    Input presentedID;
-    rv = der::ReadTagAndGetValue(rdn, valueEncodingTag, presentedID);
-    if (rv != Success) {
-      return rv;
-    }
-
     // PrintableString is a subset of ASCII that contains all the characters
     // allowed in CN-IDs except '*'. Although '*' is illegal, there are many
     // real-world certificates that are encoded this way, so we accept it.
     //
     // In the case of UTF8String, we rely on the fact that in UTF-8 the octets in
     // a multi-byte encoding of a code point are always distinct from ASCII. Any
     // non-ASCII byte in a UTF-8 string causes us to fail to match. We make no
     // attempt to detect or report malformed UTF-8 (e.g. incomplete or overlong
@@ -628,33 +618,30 @@ SearchWithinAVA(Reader& rdn,
   //
   // EmailAddress ::=     IA5String (SIZE (1..ub-emailaddress-length))
   //
   // python DottedOIDToCode.py id-emailAddress 1.2.840.113549.1.9.1
   static const uint8_t id_emailAddress[] = {
     0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x09, 0x01
   };
   if (fallBackToEmailAddress == FallBackToSearchWithinSubject::Yes &&
-      type.MatchRest(id_emailAddress)) {
+      InputsAreEqual(type, Input(id_emailAddress))) {
     if (referenceIDType == GeneralNameType::rfc822Name &&
         match == MatchResult::Match) {
       // We already found a match; we don't need to match another one
       return Success;
     }
-    Input presentedID;
-    rv = der::ExpectTagAndGetValue(rdn, der::IA5String, presentedID);
-    if (rv != Success) {
-      return rv;
+    if (valueEncodingTag != der::IA5String) {
+      return Result::ERROR_BAD_DER;
     }
     return MatchPresentedIDWithReferenceID(GeneralNameType::rfc822Name,
                                            presentedID, referenceIDType,
                                            referenceID, match);
   }
 
-  rdn.SkipToEnd();
   return Success;
 }
 
 void
 MatchSubjectPresentedIDWithReferenceID(GeneralNameType presentedIDType,
                                        Input presentedID,
                                        GeneralNameType referenceIDType,
                                        Input referenceID,
@@ -1275,16 +1262,42 @@ MatchPresentedIPAddressWithConstraint(In
     }
     foundMatch =
       ((presentedByte ^ constraintAddressByte) & constraintMaskByte) == 0;
   } while (foundMatch && !presented.AtEnd());
 
   return Success;
 }
 
+// AttributeTypeAndValue ::= SEQUENCE {
+//   type     AttributeType,
+//   value    AttributeValue }
+//
+// AttributeType ::= OBJECT IDENTIFIER
+//
+// AttributeValue ::= ANY -- DEFINED BY AttributeType
+Result
+ReadAVA(Reader& rdn,
+        /*out*/ Input& type,
+        /*out*/ uint8_t& valueTag,
+        /*out*/ Input& value)
+{
+  return der::Nested(rdn, der::SEQUENCE, [&](Reader& ava) -> Result {
+    Result rv = der::ExpectTagAndGetValue(ava, der::OIDTag, type);
+    if (rv != Success) {
+      return rv;
+    }
+    rv = der::ReadTagAndGetValue(ava, valueTag, value);
+    if (rv != Success) {
+      return rv;
+    }
+    return Success;
+  });
+}
+
 // Names are sequences of RDNs. RDNS are sets of AVAs. That means that RDNs are
 // unordered, so in theory we should match RDNs with equivalent AVAs that are
 // in different orders. Within the AVAs are DirectoryNames that are supposed to
 // be compared according to LDAP stringprep normalization rules (e.g.
 // normalizing whitespace), consideration of different character encodings,
 // etc. Indeed, RFC 5280 says we MUST deal with all of that.
 //
 // In practice, many implementations, including NSS, only match Names in a way
@@ -1304,22 +1317,27 @@ MatchPresentedIPAddressWithConstraint(In
 //     identically to the encoding used in the subject field or
 //     subjectAltName extension.  If not, then name constraints stated as
 //     excludedSubtrees will not match and invalid paths will be accepted
 //     and name constraints expressed as permittedSubtrees will not match
 //     and valid paths will be rejected.  To avoid acceptance of invalid
 //     paths, CAs SHOULD state name constraints for distinguished names as
 //     permittedSubtrees wherever possible.
 //
-// Consequently, we implement the comparison in the simplest possible way. For
-// permittedSubtrees, we rely on implementations to follow that MUST-level
-// requirement for compatibility. For excludedSubtrees, we simply prohibit any
-// non-empty directoryName constraint to ensure we are not being too lenient.
-// We support empty DirectoryName constraints in excludedSubtrees so that a CA
-// can say "Do not allow any DirectoryNames in issued certificates."
+// For permittedSubtrees, the MUST-level requirement is relaxed for
+// compatibility in the case of PrintableString and UTF8String. That is, if a
+// name constraint has been encoded using UTF8String and the presented ID has
+// been encoded with a PrintableString (or vice-versa), they are considered to
+// match if they are equal everywhere except for the tag identifying the
+// encoding. See bug 1150114.
+//
+// For excludedSubtrees, we simply prohibit any non-empty directoryName
+// constraint to ensure we are not being too lenient. We support empty
+// DirectoryName constraints in excludedSubtrees so that a CA can say "Do not
+// allow any DirectoryNames in issued certificates."
 Result
 MatchPresentedDirectoryNameWithConstraint(NameConstraintsSubtrees subtreesType,
                                           Input presentedID,
                                           Input directoryNameConstraint,
                                           /*out*/ bool& matches)
 {
   Reader constraintRDNs;
   Result rv = der::ExpectTagAndGetValueAtEnd(directoryNameConstraint,
@@ -1351,27 +1369,59 @@ MatchPresentedDirectoryNameWithConstrain
     if (constraintRDNs.AtEnd()) {
       matches = true;
       return Success;
     }
     if (presentedRDNs.AtEnd()) {
       matches = false;
       return Success;
     }
-    Input constraintRDN;
+    Reader constraintRDN;
     rv = der::ExpectTagAndGetValue(constraintRDNs, der::SET, constraintRDN);
     if (rv != Success) {
       return rv;
     }
-    Input presentedRDN;
+    Reader presentedRDN;
     rv = der::ExpectTagAndGetValue(presentedRDNs, der::SET, presentedRDN);
     if (rv != Success) {
       return rv;
     }
-    if (!InputsAreEqual(constraintRDN, presentedRDN)) {
+    while (!constraintRDN.AtEnd() && !presentedRDN.AtEnd()) {
+      Input constraintType;
+      uint8_t constraintValueTag;
+      Input constraintValue;
+      rv = ReadAVA(constraintRDN, constraintType, constraintValueTag,
+                   constraintValue);
+      if (rv != Success) {
+        return rv;
+      }
+      Input presentedType;
+      uint8_t presentedValueTag;
+      Input presentedValue;
+      rv = ReadAVA(presentedRDN, presentedType, presentedValueTag,
+                   presentedValue);
+      if (rv != Success) {
+        return rv;
+      }
+      // TODO (bug 1155767): verify that if an AVA is a PrintableString it
+      // consists only of characters valid for PrintableStrings.
+      bool avasMatch =
+        InputsAreEqual(constraintType, presentedType) &&
+        InputsAreEqual(constraintValue, presentedValue) &&
+        (constraintValueTag == presentedValueTag ||
+         (constraintValueTag == der::Tag::UTF8String &&
+          presentedValueTag == der::Tag::PrintableString) ||
+         (constraintValueTag == der::Tag::PrintableString &&
+          presentedValueTag == der::Tag::UTF8String));
+      if (!avasMatch) {
+        matches = false;
+        return Success;
+      }
+    }
+    if (!constraintRDN.AtEnd() || !presentedRDN.AtEnd()) {
       matches = false;
       return Success;
     }
   }
 }
 
 // RFC 5280 says:
 //
--- a/security/pkix/test/gtest/pkixnames_tests.cpp
+++ b/security/pkix/test/gtest/pkixnames_tests.cpp
@@ -2416,26 +2416,111 @@ static const NameConstraintParams NAME_C
     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
   },
+
+  /////////////////////////////////////////////////////////////////////////////
+  // DirectoryName name constraint tests
+
+  { // One AVA per RDN
+    RDN(OU("Example Organization")) + RDN(CN("example.com")), NO_SAN,
+    GeneralSubtree(DirectoryName(Name(RDN(OU("Example Organization")) +
+                                      RDN(CN("example.com"))))),
+    Success, Result::ERROR_CERT_NOT_IN_NAME_SPACE
+  },
+  { // RDNs can have multiple AVAs.
+    RDN(OU("Example Organization") + CN("example.com")), NO_SAN,
+    GeneralSubtree(DirectoryName(Name(RDN(OU("Example Organization") +
+                                          CN("example.com"))))),
+    Success, Result::ERROR_CERT_NOT_IN_NAME_SPACE
+  },
+  { // The constraint is a prefix of the subject DN.
+    RDN(OU("Example Organization")) + RDN(CN("example.com")), NO_SAN,
+    GeneralSubtree(DirectoryName(Name(RDN(OU("Example Organization"))))),
+    Success, Result::ERROR_CERT_NOT_IN_NAME_SPACE
+  },
+  { // The name constraint is not a prefix of the subject DN.
+    // Note that for excludedSubtrees, we simply prohibit any non-empty
+    // directoryName constraint to ensure we are not being too lenient.
+    RDN(OU("Other Example Organization")) + RDN(CN("example.com")), NO_SAN,
+    GeneralSubtree(DirectoryName(Name(RDN(OU("Example Organization")) +
+                                      RDN(CN("example.com"))))),
+    Result::ERROR_CERT_NOT_IN_NAME_SPACE, Result::ERROR_CERT_NOT_IN_NAME_SPACE
+  },
+  { // Same as the previous one, but one RDN with multiple AVAs.
+    RDN(OU("Other Example Organization") + CN("example.com")), NO_SAN,
+    GeneralSubtree(DirectoryName(Name(RDN(OU("Example Organization") +
+                                          CN("example.com"))))),
+    Result::ERROR_CERT_NOT_IN_NAME_SPACE, Result::ERROR_CERT_NOT_IN_NAME_SPACE
+  },
+  { // With multiple AVAs per RDN in the subject DN, the constraint is not a
+    // prefix of the subject DN.
+    RDN(OU("Example Organization") + CN("example.com")), NO_SAN,
+    GeneralSubtree(DirectoryName(Name(RDN(OU("Example Organization"))))),
+    Result::ERROR_CERT_NOT_IN_NAME_SPACE, Result::ERROR_CERT_NOT_IN_NAME_SPACE
+  },
+  { // The subject DN RDN has multiple AVAs, but the name constraint has only
+    // one AVA per RDN.
+    RDN(OU("Example Organization") + CN("example.com")), NO_SAN,
+    GeneralSubtree(DirectoryName(Name(RDN(OU("Example Organization")) +
+                                      RDN(CN("example.com"))))),
+    Result::ERROR_CERT_NOT_IN_NAME_SPACE, Result::ERROR_CERT_NOT_IN_NAME_SPACE
+  },
+  { // The name constraint RDN has multiple AVAs, but the subject DN has only
+    // one AVA per RDN.
+    RDN(OU("Example Organization")) + RDN(CN("example.com")), NO_SAN,
+    GeneralSubtree(DirectoryName(Name(RDN(OU("Example Organization") +
+                                          CN("example.com"))))),
+    Result::ERROR_CERT_NOT_IN_NAME_SPACE, Result::ERROR_CERT_NOT_IN_NAME_SPACE
+  },
+  { // In this case, the constraint uses a different encoding from the subject.
+    // We consider them to match because we allow UTF8String and
+    // PrintableString to compare equal when their contents are equal.
+    RDN(OU("Example Organization", der::UTF8String)) + RDN(CN("example.com")),
+    NO_SAN, GeneralSubtree(DirectoryName(Name(RDN(OU("Example Organization",
+                                                     der::PrintableString)) +
+                                              RDN(CN("example.com"))))),
+    Success, Result::ERROR_CERT_NOT_IN_NAME_SPACE
+  },
+  { // Same as above, but with UTF8String/PrintableString switched.
+    RDN(OU("Example Organization", der::PrintableString)) + RDN(CN("example.com")),
+    NO_SAN, GeneralSubtree(DirectoryName(Name(RDN(OU("Example Organization",
+                                                     der::UTF8String)) +
+                                              RDN(CN("example.com"))))),
+    Success, Result::ERROR_CERT_NOT_IN_NAME_SPACE
+  },
+  { // If the contents aren't the same, then they shouldn't match.
+    RDN(OU("Other Example Organization", der::UTF8String)) + RDN(CN("example.com")),
+    NO_SAN, GeneralSubtree(DirectoryName(Name(RDN(OU("Example Organization",
+                                                     der::PrintableString)) +
+                                              RDN(CN("example.com"))))),
+    Result::ERROR_CERT_NOT_IN_NAME_SPACE, Result::ERROR_CERT_NOT_IN_NAME_SPACE
+  },
+  { // Only UTF8String and PrintableString are considered equivalent.
+    RDN(OU("Example Organization", der::PrintableString)) + RDN(CN("example.com")),
+    NO_SAN, GeneralSubtree(DirectoryName(Name(RDN(OU("Example Organization",
+                                                     der::TeletexString)) +
+                                              RDN(CN("example.com"))))),
+    Result::ERROR_CERT_NOT_IN_NAME_SPACE, Result::ERROR_CERT_NOT_IN_NAME_SPACE
+  },
 };
 
 class pkixnames_CheckNameConstraints
   : public ::testing::Test
   , public ::testing::WithParamInterface<NameConstraintParams>
 {
 };
 
 TEST_P(pkixnames_CheckNameConstraints,
-       NameConstraintsEnforcedforDirectlyIssuedEndEntity)
+       NameConstraintsEnforcedForDirectlyIssuedEndEntity)
 {
   // Test that name constraints are enforced on a certificate directly issued by
   // this certificate.
 
   const NameConstraintParams& param(GetParam());
 
   ByteString certDER(CreateCert(param.subject, param.subjectAltName));
   ASSERT_FALSE(ENCODING_FAILED(certDER));
--- a/security/pkix/test/lib/pkixtestutil.cpp
+++ b/security/pkix/test/lib/pkixtestutil.cpp
@@ -668,26 +668,26 @@ CN(const ByteString& value, uint8_t enco
   // python DottedOIDToCode.py --tlv id-at-commonName 2.5.4.3
   static const uint8_t tlv_id_at_commonName[] = {
     0x06, 0x03, 0x55, 0x04, 0x03
   };
   return AVA(tlv_id_at_commonName, encodingTag, value);
 }
 
 ByteString
-OU(const ByteString& value)
+OU(const ByteString& value, uint8_t encodingTag)
 {
   // id-at OBJECT IDENTIFIER ::= { joint-iso-ccitt(2) ds(5) 4 }
   // id-at-organizationalUnitName AttributeType ::= { id-at 11 }
   // 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);
+  return AVA(tlv_id_at_organizationalUnitName, encodingTag, 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[] = {
--- a/security/pkix/test/lib/pkixtestutil.h
+++ b/security/pkix/test/lib/pkixtestutil.h
@@ -132,23 +132,23 @@ ByteString CN(const ByteString&, uint8_t
 
 inline ByteString
 CN(const char* value, uint8_t encodingTag = 0x0c /*UTF8String*/)
 {
   return CN(ByteString(reinterpret_cast<const uint8_t*>(value),
                        std::strlen(value)), encodingTag);
 }
 
-ByteString OU(const ByteString&);
+ByteString OU(const ByteString&, uint8_t encodingTag = 0x0c /*UTF8String*/);
 
 inline ByteString
-OU(const char* value)
+OU(const char* value, uint8_t encodingTag = 0x0c /*UTF8String*/)
 {
   return OU(ByteString(reinterpret_cast<const uint8_t*>(value),
-                       std::strlen(value)));
+                       std::strlen(value)), encodingTag);
 }
 
 ByteString emailAddress(const ByteString&);
 
 inline ByteString
 emailAddress(const char* value)
 {
   return emailAddress(ByteString(reinterpret_cast<const uint8_t*>(value),
@@ -216,16 +216,24 @@ template <size_t L>
 inline ByteString
 DNSName(const char (&bytes)[L])
 {
   return DNSName(ByteString(reinterpret_cast<const uint8_t*>(&bytes),
                             L - 1));
 }
 
 inline ByteString
+DirectoryName(const ByteString& name)
+{
+  // (2 << 6) means "context-specific", (1 << 5) means "constructed", and 4 is
+  // the DirectoryName tag.
+  return TLV((2 << 6) | (1 << 5) | 4, name);
+}
+
+inline ByteString
 IPAddress()
 {
   // (2 << 6) means "context-specific", 7 is the GeneralName tag.
   return TLV((2 << 6) | 7, ByteString());
 }
 
 template <size_t L>
 inline ByteString