Bug 1037324: Delegate additional name constraint selection to the TrustDomain in mozilla::pkix, r=cviecco
authorBrian Smith <brian@briansmith.org>
Thu, 10 Jul 2014 22:38:59 -0700
changeset 215614 440f68cba022b72dd87c1c84de1579cb8295d4eb
parent 215613 f98f0f5ba507095aac4335fa705e235484f2541a
child 215615 f1eae42835bbbd59ede333a49c40d6894ae69abe
push id515
push userraliiev@mozilla.com
push dateMon, 06 Oct 2014 12:51:51 +0000
treeherdermozilla-release@267c7a481bef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscviecco
bugs1037324
milestone33.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 1037324: Delegate additional name constraint selection to the TrustDomain in mozilla::pkix, r=cviecco
security/apps/AppTrustDomain.cpp
security/certverifier/NSSCertDBTrustDomain.cpp
security/pkix/include/pkix/pkixtypes.h
security/pkix/lib/pkixbuild.cpp
security/pkix/lib/pkixcheck.cpp
security/pkix/lib/pkixcheck.h
security/pkix/test/gtest/pkixbuild_tests.cpp
--- a/security/apps/AppTrustDomain.cpp
+++ b/security/apps/AppTrustDomain.cpp
@@ -108,17 +108,19 @@ AppTrustDomain::FindIssuer(const SECItem
   //    message, passing each one to checker.Check.
   ScopedCERTCertList
     candidates(CERT_CreateSubjectCertList(nullptr, CERT_GetDefaultCertDB(),
                                           &encodedIssuerName, time, true));
   if (candidates) {
     for (CERTCertListNode* n = CERT_LIST_HEAD(candidates);
          !CERT_LIST_END(n, candidates); n = CERT_LIST_NEXT(n)) {
       bool keepGoing;
-      SECStatus srv = checker.Check(n->cert->derCert, keepGoing);
+      SECStatus srv = checker.Check(n->cert->derCert,
+                                    nullptr/*additionalNameConstraints*/,
+                                    keepGoing);
       if (srv != SECSuccess) {
         return SECFailure;
       }
       if (!keepGoing) {
         break;
       }
     }
   }
--- a/security/certverifier/NSSCertDBTrustDomain.cpp
+++ b/security/certverifier/NSSCertDBTrustDomain.cpp
@@ -56,30 +56,81 @@ NSSCertDBTrustDomain::NSSCertDBTrustDoma
   , mOCSPCache(ocspCache)
   , mPinArg(pinArg)
   , mOCSPGetConfig(ocspGETConfig)
   , mCheckChainCallback(checkChainCallback)
   , mBuiltChain(builtChain)
 {
 }
 
+// E=igca@sgdn.pm.gouv.fr,CN=IGC/A,OU=DCSSI,O=PM/SGDN,L=Paris,ST=France,C=FR
+static const uint8_t ANSSI_SUBJECT_DATA[] =
+                       "\x30\x81\x85\x31\x0B\x30\x09\x06\x03\x55\x04"
+                       "\x06\x13\x02\x46\x52\x31\x0F\x30\x0D\x06\x03"
+                       "\x55\x04\x08\x13\x06\x46\x72\x61\x6E\x63\x65"
+                       "\x31\x0E\x30\x0C\x06\x03\x55\x04\x07\x13\x05"
+                       "\x50\x61\x72\x69\x73\x31\x10\x30\x0E\x06\x03"
+                       "\x55\x04\x0A\x13\x07\x50\x4D\x2F\x53\x47\x44"
+                       "\x4E\x31\x0E\x30\x0C\x06\x03\x55\x04\x0B\x13"
+                       "\x05\x44\x43\x53\x53\x49\x31\x0E\x30\x0C\x06"
+                       "\x03\x55\x04\x03\x13\x05\x49\x47\x43\x2F\x41"
+                       "\x31\x23\x30\x21\x06\x09\x2A\x86\x48\x86\xF7"
+                       "\x0D\x01\x09\x01\x16\x14\x69\x67\x63\x61\x40"
+                       "\x73\x67\x64\x6E\x2E\x70\x6D\x2E\x67\x6F\x75"
+                       "\x76\x2E\x66\x72";
+
+static const SECItem ANSSI_SUBJECT = {
+  siBuffer,
+  const_cast<uint8_t*>(ANSSI_SUBJECT_DATA),
+  sizeof(ANSSI_SUBJECT_DATA) - 1
+};
+
+static const uint8_t PERMIT_FRANCE_GOV_NAME_CONSTRAINTS_DATA[] =
+                       "\x30\x5D" // SEQUENCE (length=93)
+                       "\xA0\x5B" // permittedSubtrees (length=91)
+                       "\x30\x05\x82\x03" ".fr"
+                       "\x30\x05\x82\x03" ".gp"
+                       "\x30\x05\x82\x03" ".gf"
+                       "\x30\x05\x82\x03" ".mq"
+                       "\x30\x05\x82\x03" ".re"
+                       "\x30\x05\x82\x03" ".yt"
+                       "\x30\x05\x82\x03" ".pm"
+                       "\x30\x05\x82\x03" ".bl"
+                       "\x30\x05\x82\x03" ".mf"
+                       "\x30\x05\x82\x03" ".wf"
+                       "\x30\x05\x82\x03" ".pf"
+                       "\x30\x05\x82\x03" ".nc"
+                       "\x30\x05\x82\x03" ".tf";
+
+static const SECItem PERMIT_FRANCE_GOV_NAME_CONSTRAINTS = {
+  siBuffer,
+  const_cast<uint8_t*>(PERMIT_FRANCE_GOV_NAME_CONSTRAINTS_DATA),
+  sizeof(PERMIT_FRANCE_GOV_NAME_CONSTRAINTS_DATA) - 1
+};
+
 SECStatus
 NSSCertDBTrustDomain::FindIssuer(const SECItem& encodedIssuerName,
                                  IssuerChecker& checker, PRTime time)
 {
   // TODO: NSS seems to be ambiguous between "no potential issuers found" and
   // "there was an error trying to retrieve the potential issuers."
   ScopedCERTCertList
     candidates(CERT_CreateSubjectCertList(nullptr, CERT_GetDefaultCertDB(),
                                           &encodedIssuerName, time, true));
   if (candidates) {
     for (CERTCertListNode* n = CERT_LIST_HEAD(candidates);
          !CERT_LIST_END(n, candidates); n = CERT_LIST_NEXT(n)) {
+      const SECItem* additionalNameConstraints = nullptr;
+      // TODO: Use CERT_CompareName or equivalent
+      if (SECITEM_ItemsAreEqual(&encodedIssuerName, &ANSSI_SUBJECT)) {
+        additionalNameConstraints = &PERMIT_FRANCE_GOV_NAME_CONSTRAINTS;
+      }
       bool keepGoing;
-      SECStatus srv = checker.Check(n->cert->derCert, keepGoing);
+      SECStatus srv = checker.Check(n->cert->derCert,
+                                    additionalNameConstraints, keepGoing);
       if (srv != SECSuccess) {
         return SECFailure;
       }
       if (!keepGoing) {
         break;
       }
     }
   }
--- a/security/pkix/include/pkix/pkixtypes.h
+++ b/security/pkix/include/pkix/pkixtypes.h
@@ -141,18 +141,26 @@ public:
   virtual SECStatus GetCertTrust(EndEntityOrCA endEntityOrCA,
                                  const CertPolicyId& policy,
                                  const SECItem& candidateCertDER,
                          /*out*/ TrustLevel* trustLevel) = 0;
 
   class IssuerChecker
   {
   public:
+    // potentialIssuerDER is the complete DER encoding of the certificate to
+    // be checked as a potential issuer.
+    //
+    // If additionalNameConstraints is not nullptr then it must point to an
+    // encoded NameConstraints extension value; in that case, those name
+    // constraints will be checked in addition to any any name constraints
+    // contained in potentialIssuerDER.
     virtual SECStatus Check(const SECItem& potentialIssuerDER,
-                            /*out*/ bool& keepGoing) = 0;
+               /*optional*/ const SECItem* additionalNameConstraints,
+                    /*out*/ bool& keepGoing) = 0;
   protected:
     IssuerChecker();
     virtual ~IssuerChecker();
   private:
     IssuerChecker(const IssuerChecker&) /*= delete*/;
     void operator=(const IssuerChecker&) /*= delete*/;
   };
 
--- a/security/pkix/lib/pkixbuild.cpp
+++ b/security/pkix/lib/pkixbuild.cpp
@@ -60,16 +60,17 @@ public:
     , stapledOCSPResponse(stapledOCSPResponse)
     , subCACount(subCACount)
     , result(SEC_ERROR_LIBRARY_FAILURE)
     , resultWasSet(false)
   {
   }
 
   SECStatus Check(const SECItem& potentialIssuerDER,
+                  /*optional*/ const SECItem* additionalNameConstraints,
                   /*out*/ bool& keepGoing);
 
   Result CheckResult() const;
 
 private:
   TrustDomain& trustDomain;
   const BackCert& subject;
   const PRTime time;
@@ -125,16 +126,17 @@ PathBuildingStep::CheckResult() const
   }
   PR_SetError(result, 0);
   return MapSECStatus(SECFailure);
 }
 
 // The code that executes in the inner loop of BuildForward
 SECStatus
 PathBuildingStep::Check(const SECItem& potentialIssuerDER,
+                        /*optional*/ const SECItem* additionalNameConstraints,
                         /*out*/ bool& keepGoing)
 {
   BackCert potentialIssuer(potentialIssuerDER, EndEntityOrCA::MustBeCA,
                            &subject);
   Result rv = potentialIssuer.Init();
   if (rv != Success) {
     return RecordResult(PR_GetError(), keepGoing);
   }
@@ -152,19 +154,30 @@ PathBuildingStep::Check(const SECItem& p
                               &prev->GetSubjectPublicKeyInfo()) &&
         SECITEM_ItemsAreEqual(&potentialIssuer.GetSubject(),
                               &prev->GetSubject())) {
       // XXX: error code
       return RecordResult(SEC_ERROR_UNKNOWN_ISSUER, keepGoing);
     }
   }
 
-  rv = CheckNameConstraints(potentialIssuer, requiredEKUIfPresent);
-  if (rv != Success) {
-    return RecordResult(PR_GetError(), keepGoing);
+  if (potentialIssuer.GetNameConstraints()) {
+    rv = CheckNameConstraints(*potentialIssuer.GetNameConstraints(),
+                              subject, requiredEKUIfPresent);
+    if (rv != Success) {
+       return RecordResult(PR_GetError(), keepGoing);
+    }
+  }
+
+  if (additionalNameConstraints) {
+    rv = CheckNameConstraints(*additionalNameConstraints, subject,
+                              requiredEKUIfPresent);
+    if (rv != Success) {
+       return RecordResult(PR_GetError(), keepGoing);
+    }
   }
 
   // RFC 5280, Section 4.2.1.3: "If the keyUsage extension is present, then the
   // subject public key MUST NOT be used to verify signatures on certificates
   // or CRLs unless the corresponding keyCertSign or cRLSign bit is set."
   rv = BuildForward(trustDomain, potentialIssuer, time, KeyUsage::keyCertSign,
                     requiredEKUIfPresent, requiredPolicy, nullptr, subCACount);
   if (rv != Success) {
--- a/security/pkix/lib/pkixcheck.cpp
+++ b/security/pkix/lib/pkixcheck.cpp
@@ -407,106 +407,34 @@ CheckBasicConstraints(EndEntityOrCA endE
 inline void
 PORT_FreeArena_false(PLArenaPool* arena) {
   // PL_FreeArenaPool can't be used because it doesn't actually free the
   // memory, which doesn't work well with memory analysis tools
   return PORT_FreeArena(arena, PR_FALSE);
 }
 
 Result
-CheckNameConstraints(const BackCert& cert, KeyPurposeId requiredEKUIfPresent)
+CheckNameConstraints(const SECItem& encodedNameConstraints,
+                     const BackCert& firstChild,
+                     KeyPurposeId requiredEKUIfPresent)
 {
-  // These hardcoded consts are to handle a post certificate creation
-  // name constraints. In this case for ANSSI.
-  static const char constraintFranceGov[] =
-                                     "\x30\x5D" /* sequence len 93*/
-                                     "\xA0\x5B" /* element len 91 */
-                                     "\x30\x05" /* sequence len 5 */
-                                     "\x82\x03" /* entry len 3 */
-                                     ".fr"
-                                     "\x30\x05\x82\x03" /* sequence len 5, entry len 3 */
-                                     ".gp"
-                                     "\x30\x05\x82\x03"
-                                     ".gf"
-                                     "\x30\x05\x82\x03"
-                                     ".mq"
-                                     "\x30\x05\x82\x03"
-                                     ".re"
-                                     "\x30\x05\x82\x03"
-                                     ".yt"
-                                     "\x30\x05\x82\x03"
-                                     ".pm"
-                                     "\x30\x05\x82\x03"
-                                     ".bl"
-                                     "\x30\x05\x82\x03"
-                                     ".mf"
-                                     "\x30\x05\x82\x03"
-                                     ".wf"
-                                     "\x30\x05\x82\x03"
-                                     ".pf"
-                                     "\x30\x05\x82\x03"
-                                     ".nc"
-                                     "\x30\x05\x82\x03"
-                                     ".tf";
-
-  /* The stringified value for the subject is:
-     E=igca@sgdn.pm.gouv.fr,CN=IGC/A,OU=DCSSI,O=PM/SGDN,L=Paris,ST=France,C=FR
-   */
-  static const char rawANSSISubject[] =
-                                 "\x30\x81\x85\x31\x0B\x30\x09\x06\x03\x55\x04"
-                                 "\x06\x13\x02\x46\x52\x31\x0F\x30\x0D\x06\x03"
-                                 "\x55\x04\x08\x13\x06\x46\x72\x61\x6E\x63\x65"
-                                 "\x31\x0E\x30\x0C\x06\x03\x55\x04\x07\x13\x05"
-                                 "\x50\x61\x72\x69\x73\x31\x10\x30\x0E\x06\x03"
-                                 "\x55\x04\x0A\x13\x07\x50\x4D\x2F\x53\x47\x44"
-                                 "\x4E\x31\x0E\x30\x0C\x06\x03\x55\x04\x0B\x13"
-                                 "\x05\x44\x43\x53\x53\x49\x31\x0E\x30\x0C\x06"
-                                 "\x03\x55\x04\x03\x13\x05\x49\x47\x43\x2F\x41"
-                                 "\x31\x23\x30\x21\x06\x09\x2A\x86\x48\x86\xF7"
-                                 "\x0D\x01\x09\x01\x16\x14\x69\x67\x63\x61\x40"
-                                 "\x73\x67\x64\x6E\x2E\x70\x6D\x2E\x67\x6F\x75"
-                                 "\x76\x2E\x66\x72";
-
-  const SECItem ANSSI_SUBJECT = {
-    siBuffer,
-    reinterpret_cast<uint8_t *>(const_cast<char *>(rawANSSISubject)),
-    sizeof(rawANSSISubject) - 1
-  };
-
-  const SECItem PERMIT_FRANCE_GOV_NC = {
-    siBuffer,
-    reinterpret_cast<uint8_t *>(const_cast<char *>(constraintFranceGov)),
-    sizeof(constraintFranceGov) - 1
-  };
-
-  const SECItem* nameConstraintsToUse = cert.GetNameConstraints();
-
-  if (!nameConstraintsToUse) {
-    if (SECITEM_ItemsAreEqual(&cert.GetSubject(), &ANSSI_SUBJECT)) {
-      nameConstraintsToUse = &PERMIT_FRANCE_GOV_NC;
-    } else {
-      return Success;
-    }
-   }
-
   ScopedPtr<PLArenaPool, PORT_FreeArena_false>
     arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
   if (!arena) {
     return MapSECStatus(SECFailure);
   }
 
   // Owned by arena
   const CERTNameConstraints* constraints =
-    CERT_DecodeNameConstraintsExtension(arena.get(), nameConstraintsToUse);
+    CERT_DecodeNameConstraintsExtension(arena.get(), &encodedNameConstraints);
   if (!constraints) {
     return MapSECStatus(SECFailure);
   }
 
-  for (const BackCert* child = cert.childCert; child;
-       child = child->childCert) {
+  for (const BackCert* child = &firstChild; child; child = child->childCert) {
     ScopedPtr<CERTCertificate, CERT_DestroyCertificate>
       nssCert(CERT_NewTempCertificate(CERT_GetDefaultCertDB(),
                                       const_cast<SECItem*>(&child->GetDER()),
                                       nullptr, false, true));
     if (!nssCert) {
       return MapSECStatus(SECFailure);
     }
 
--- a/security/pkix/lib/pkixcheck.h
+++ b/security/pkix/lib/pkixcheck.h
@@ -36,14 +36,15 @@ Result CheckIssuerIndependentProperties(
           const BackCert& cert,
           PRTime time,
           KeyUsage requiredKeyUsageIfPresent,
           KeyPurposeId requiredEKUIfPresent,
           const CertPolicyId& requiredPolicy,
           unsigned int subCACount,
           /*optional out*/ TrustLevel* trustLevel = nullptr);
 
-Result CheckNameConstraints(const BackCert& cert,
+Result CheckNameConstraints(const SECItem& encodedNameConstraints,
+                            const BackCert& firstChild,
                             KeyPurposeId requiredEKUIfPresent);
 
 } } // namespace mozilla::pkix
 
 #endif // mozilla_pkix__pkixcheck_h
--- a/security/pkix/test/gtest/pkixbuild_tests.cpp
+++ b/security/pkix/test/gtest/pkixbuild_tests.cpp
@@ -130,17 +130,19 @@ private:
   {
     ScopedCERTCertList
       candidates(CERT_CreateSubjectCertList(nullptr, CERT_GetDefaultCertDB(),
                                             &encodedIssuerName, time, true));
     if (candidates) {
       for (CERTCertListNode* n = CERT_LIST_HEAD(candidates);
            !CERT_LIST_END(n, candidates); n = CERT_LIST_NEXT(n)) {
         bool keepGoing;
-        SECStatus srv = checker.Check(n->cert->derCert, keepGoing);
+        SECStatus srv = checker.Check(n->cert->derCert,
+                                      nullptr/*additionalNameConstraints*/,
+                                      keepGoing);
         if (srv != SECSuccess) {
           return SECFailure;
         }
         if (!keepGoing) {
           break;
         }
       }
     }