Bug 921896: Check name constraints in insanity::pkix, r=cviecco, r=keeler

--HG--
extra : rebase_source : 6d3e77670a5553b477a881609cc30f5f4140294c
extra : source : 2545cd47894a95323b718eb4f82be6d744019c7a

security/insanity/lib/pkixbuild.cpp

@@ -41,6 +41,7 @@ BackCert::Init()
   const SECItem* dummyEncodedSubjectKeyIdentifier = nullptr;
   const SECItem* dummyEncodedAuthorityKeyIdentifier = nullptr;
   const SECItem* dummyEncodedAuthorityInfoAccess = nullptr;
+  const SECItem* dummyEncodedSubjectAltName = nullptr;
 
   for (const CERTCertExtension* ext = *exts; ext; ext = *++exts) {
     const SECItem** out = nullptr;
@@ -51,7 +52,9 @@ BackCert::Init()
       switch (ext->id.data[2]) {
         case 14: out = &dummyEncodedSubjectKeyIdentifier; break; // bug 965136
         case 15: out = &encodedKeyUsage; break;
+        case 17: out = &dummyEncodedSubjectAltName; break; // bug 970542
         case 19: out = &encodedBasicConstraints; break;
+        case 30: out = &encodedNameConstraints; break;
         case 35: out = &dummyEncodedAuthorityKeyIdentifier; break; // bug 965136
         case 37: out = &encodedExtendedKeyUsage; break;
       }
@@ -109,7 +112,8 @@ BuildForwardInner(TrustDomain& trustDomain,
 {
   PORT_Assert(potentialIssuerCertToDup);
 
-  BackCert potentialIssuer(potentialIssuerCertToDup, &subject);
+  BackCert potentialIssuer(potentialIssuerCertToDup, &subject,
+                           BackCert::ExcludeCN);
   Result rv = potentialIssuer.Init();
   if (rv != Success) {
     return rv;
@@ -137,6 +141,11 @@ BuildForwardInner(TrustDomain& trustDomain,
     return rv;
   }
 
+  rv = CheckNameConstraints(potentialIssuer);
+  if (rv != Success) {
+    return rv;
+  }
+
   unsigned int newSubCACount = subCACount;
   if (endEntityOrCA == MustBeCA) {
     newSubCACount = subCACount + 1;
@@ -260,7 +269,15 @@ BuildCertChain(TrustDomain& trustDomain,
   // The only non-const operation on the cert we are allowed to do is
   // CERT_DupCertificate.
 
-  BackCert cert(certToDup, nullptr);
+  // XXX: Support the legacy use of the subject CN field for indicating the
+  // domain name the certificate is valid for.
+  BackCert::ConstrainedNameOptions cnOptions
+    = endEntityOrCA == MustBeEndEntity &&
+      requiredEKUIfPresent == SEC_OID_EXT_KEY_USAGE_SERVER_AUTH
+    ? BackCert::IncludeCN
+    : BackCert::ExcludeCN;
+
+  BackCert cert(certToDup, nullptr, cnOptions);
   Result rv = cert.Init();
   if (rv != Success) {
     return SECFailure;

security/insanity/lib/pkixcheck.cpp

@@ -182,6 +182,66 @@ CheckBasicConstraints(const BackCert& cert,
   return Success;
 }
 
+Result
+BackCert::GetConstrainedNames(/*out*/ const CERTGeneralName** result)
+{
+  if (!constrainedNames) {
+    if (!GetArena()) {
+      return FatalError;
+    }
+
+    constrainedNames =
+      CERT_GetConstrainedCertificateNames(nssCert, arena.get(),
+                                          cnOptions == IncludeCN);
+    if (!constrainedNames) {
+      return MapSECStatus(SECFailure);
+    }
+  }
+
+  *result = constrainedNames;
+  return Success;
+}
+
+// 4.2.1.10. Name Constraints
+Result
+CheckNameConstraints(BackCert& cert)
+{
+  if (!cert.encodedNameConstraints) {
+    return Success;
+  }
+
+  PLArenaPool* arena = cert.GetArena();
+  if (!arena) {
+    return FatalError;
+  }
+
+  // Owned by arena
+  const CERTNameConstraints* constraints =
+    CERT_DecodeNameConstraintsExtension(arena, cert.encodedNameConstraints);
+  if (!constraints) {
+    return MapSECStatus(SECFailure);
+  }
+
+  for (BackCert* prev = cert.childCert; prev; prev = prev->childCert) {
+    const CERTGeneralName* names = nullptr;
+    Result rv = prev->GetConstrainedNames(&names);
+    if (rv != Success) {
+      return rv;
+    }
+    PORT_Assert(names);
+    CERTGeneralName* currentName = const_cast<CERTGeneralName*>(names);
+    do {
+      rv = MapSECStatus(CERT_CheckNameSpace(arena, constraints, currentName));
+      if (rv != Success) {
+        return rv;
+      }
+      currentName = CERT_GetNextGeneralName(currentName);
+    } while (currentName != names);
+  }
+
+  return Success;
+}
+
 // 4.2.1.12. Extended Key Usage (id-ce-extKeyUsage)
 // 4.2.1.12. Extended Key Usage (id-ce-extKeyUsage)
 Result

security/insanity/lib/pkixcheck.h

@@ -32,6 +32,8 @@ Result CheckExtensions(BackCert& certExt,
                        SECOidTag requiredEKUIfPresent,
                        unsigned int depth);
 
+Result CheckNameConstraints(BackCert& cert);
+
 } } // namespace insanity::pkix
 
 #endif // insanity__pkixcheck_h

security/insanity/lib/pkixutil.h

@@ -81,13 +81,22 @@ MapSECStatus(SECStatus srv)
 class BackCert
 {
 public:
+  // ExcludeCN means that GetConstrainedNames won't include the subject CN in
+  // its results. IncludeCN means that GetConstrainedNames will include the
+  // subject CN in its results.
+  enum ConstrainedNameOptions { ExcludeCN = 0, IncludeCN = 1 };
+
   // nssCert and childCert must be valid for the lifetime of BackCert
-  BackCert(CERTCertificate* nssCert, BackCert* childCert)
+  BackCert(CERTCertificate* nssCert, BackCert* childCert,
+           ConstrainedNameOptions cnOptions)
     : encodedBasicConstraints(nullptr)
     , encodedExtendedKeyUsage(nullptr)
     , encodedKeyUsage(nullptr)
+    , encodedNameConstraints(nullptr)
     , childCert(childCert)
     , nssCert(nssCert)
+    , constrainedNames(nullptr)
+    , cnOptions(cnOptions)
   {
   }
 
@@ -96,11 +105,18 @@ public:
   const SECItem* encodedBasicConstraints;
   const SECItem* encodedExtendedKeyUsage;
   const SECItem* encodedKeyUsage;
+  const SECItem* encodedNameConstraints;
 
   BackCert* const childCert;
 
   const CERTCertificate* GetNSSCert() const { return nssCert; }
 
+  // Returns the names that should be considered when evaluating name
+  // constraints. The list is constructed lazily and cached. The result is a
+  // weak reference; do not try to free it, and do not hold long-lived
+  // references to it.
+  Result GetConstrainedNames(/*out*/ const CERTGeneralName** result);
+
   // This is the only place where we should be dealing with non-const
   // CERTCertificates.
   Result PrependNSSCertToList(CERTCertList* results);
@@ -111,6 +127,8 @@ private:
   CERTCertificate* nssCert;
 
   ScopedPLArenaPool arena;
+  CERTGeneralName* constrainedNames;
+  ConstrainedNameOptions cnOptions;
 
   BackCert(const BackCert&) /* = delete */;
   void operator=(const BackCert&); /* = delete */;