Bug 1089104: Add support for TeletexString-encoded CN-IDs to CheckCertHostname, r=keeler

--HG--
extra : rebase_source : 320794deae857a574f509b7277ea64576abd37b3

security/pkix/lib/pkixder.h

@@ -68,6 +68,7 @@ enum Tag
   SEQUENCE = UNIVERSAL | CONSTRUCTED | 0x10, // 0x30
   SET = UNIVERSAL | CONSTRUCTED | 0x11, // 0x31
   PrintableString = UNIVERSAL | 0x13,
+  TeletexString = UNIVERSAL | 0x14,
   UTCTime = UNIVERSAL | 0x17,
   GENERALIZED_TIME = UNIVERSAL | 0x18,
 };

security/pkix/lib/pkixnames.cpp

@@ -379,7 +379,9 @@ SearchWithinAVA(Reader& rdn,
     return rv;
   }
 
-  // We only support printableString and utf8String.
+  // 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
@@ -387,12 +389,21 @@ SearchWithinAVA(Reader& rdn,
   // attempt to detect or report malformed UTF-8 (e.g. incomplete or overlong
   // encodings of code points, or encodings of invalid code points).
   //
-  // We could trivially add support for teletexString if needed, but RFC 5280
-  // deprecated it. universalString and bmpString are also deprecated, and they
-  // are a little harder to support because they are not single-byte ASCII
-  // superset encodings.
+  // TeletexString is supported as long as it does not contain any escape
+  // sequences, which are not supported. We'll reject escape sequences as
+  // invalid characters in names, which means we only accept strings that are
+  // in the default character set, which is a superset of ASCII. Note that NSS
+  // actually treats TeletexString as ISO-8859-1. Many certificates that have
+  // wildcard CN-IDs (e.g. "*.example.com") use TeletexString because
+  // PrintableString is defined to not allow '*' and because, at one point in
+  // history, UTF8String was too new to use for compatibility reasons.
+  //
+  // UniversalString and BMPString are also deprecated, and they are a little
+  // harder to support because they are not single-byte ASCII superset
+  // encodings, so we don't bother.
   if (valueEncodingTag != der::PrintableString &&
-      valueEncodingTag != der::UTF8String) {
+      valueEncodingTag != der::UTF8String &&
+      valueEncodingTag != der::TeletexString) {
     return Success;
   }
 

security/pkix/test/gtest/pkixnames_tests.cpp

@@ -22,6 +22,7 @@
  * limitations under the License.
  */
 #include "pkix/pkix.h"
+#include "pkixder.h"
 #include "pkixgtest.h"
 #include "pkixtestutil.h"
 
@@ -1072,6 +1073,44 @@ static const uint8_t ipv6_addr_str[] =
 // DNSNAMES_VALIDITY, and not here.
 static const CheckCertHostnameParams CHECK_CERT_HOSTNAME_PARAMS[] =
 {
+  // This is technically illegal. PrintableString is defined in such a way that
+  // '*' is not an allowed character, but there are many real-world certificates
+  // that are encoded this way.
+  WITHOUT_SAN("foo.example.com", RDN(CN("*.example.com", der::PrintableString)),
+              Success),
+  WITHOUT_SAN("foo.example.com", RDN(CN("*.example.com", der::UTF8String)),
+              Success),
+
+  // Many certificates use TeletexString when encoding wildcards in CN-IDs
+  // because PrintableString is defined as not allowing '*' and UTF8String was,
+  // at one point in history, considered too new to depend on for compatibility.
+  // We accept TeletexString-encoded CN-IDs when they don't contain any escape
+  // sequences. The reference I used for the escape codes was
+  // https://tools.ietf.org/html/rfc1468. The escaping mechanism is actually
+  // pretty complex and these tests don't even come close to testing all the
+  // possibilities.
+  WITHOUT_SAN("foo.example.com", RDN(CN("*.example.com", der::TeletexString)),
+              Success),
+  // "ESC ( B" ({0x1B,0x50,0x42}) is the escape code to switch to ASCII, which
+  // is redundant because it already the default.
+  WITHOUT_SAN("foo.example.com",
+              RDN(CN("\x1B(B*.example.com", der::TeletexString)),
+              Result::ERROR_BAD_CERT_DOMAIN),
+  WITHOUT_SAN("foo.example.com",
+              RDN(CN("*.example\x1B(B.com", der::TeletexString)),
+              Result::ERROR_BAD_CERT_DOMAIN),
+  WITHOUT_SAN("foo.example.com",
+              RDN(CN("*.example.com\x1B(B", der::TeletexString)),
+              Result::ERROR_BAD_CERT_DOMAIN),
+  // "ESC $ B" ({0x1B,0x24,0x42}) is the escape code to switch to
+  // JIS X 0208-1983 (a Japanese character set).
+  WITHOUT_SAN("foo.example.com",
+              RDN(CN("\x1B$B*.example.com", der::TeletexString)),
+              Result::ERROR_BAD_CERT_DOMAIN),
+  WITHOUT_SAN("foo.example.com",
+              RDN(CN("*.example.com\x1B$B", der::TeletexString)),
+              Result::ERROR_BAD_CERT_DOMAIN),
+
   // Match a DNSName SAN entry with a redundant (ignored) matching CN-ID.
   WITH_SAN("a", RDN(CN("a")), DNSName("a"), Success),
   // Match a DNSName SAN entry when there is an CN-ID that doesn't match.

security/pkix/test/lib/pkixtestutil.cpp

@@ -611,7 +611,7 @@ AVA(const uint8_t (&type)[N], uint8_t directoryStringType,
 }
 
 ByteString
-CN(const ByteString& value)
+CN(const ByteString& value, uint8_t encodingTag)
 {
   // id-at OBJECT IDENTIFIER ::= { joint-iso-ccitt(2) ds(5) 4 }
   // id-at-commonName        AttributeType ::= { id-at 3 }
@@ -619,7 +619,7 @@ CN(const ByteString& value)
   static const uint8_t tlv_id_at_commonName[] = {
     0x06, 0x03, 0x55, 0x04, 0x03
   };
-  return AVA(tlv_id_at_commonName, der::UTF8String, value);
+  return AVA(tlv_id_at_commonName, encodingTag, value);
 }
 
 ByteString

security/pkix/test/lib/pkixtestutil.h

@@ -112,13 +112,13 @@ mozilla::pkix::Time YMDHMS(int16_t year, int16_t month, int16_t day,
 
 ByteString TLV(uint8_t tag, const ByteString& value);
 
-ByteString CN(const ByteString&);
+ByteString CN(const ByteString&, uint8_t encodingTag = 0x0c /*UTF8String*/);
 
 inline ByteString
-CN(const char* value)
+CN(const char* value, uint8_t encodingTag = 0x0c /*UTF8String*/)
 {
   return CN(ByteString(reinterpret_cast<const uint8_t*>(value),
-                       std::strlen(value)));
+                       std::strlen(value)), encodingTag);
 }
 
 ByteString OU(const ByteString&);