Add policies support to VerifyCertificateChain().

Support is compliant with RFC 5280 and supports all the policy
extensions specified therein:

 * Inhibit Any Policy
 * Policy Constraints
 * Policies
 * Policy Mappings

Testing is done solely using the PKITS test suite, which has fairly good
coverage of these extensions:

  4.8 (Certificate Policies)
  4.9 (Require Explicit Policy)
  4.10 (Policy Mappings)
  4.11 (Inhibit Policy Mapping)
  4.12 (Inhibit Any Policy)

BUG=634456,634453,634452

Review-Url: https://codereview.chromium.org/2903283002
Cr-Commit-Position: refs/heads/master@{#476513}

net/cert/internal/nist_pkits_unittest.cc

@@ -57,15 +57,18 @@ PkitsTestInfo::PkitsTestInfo() {
 }
 
 void PkitsTestInfo::SetInitialExplicitPolicy(bool b) {
-  initial_explicit_policy = b;
+  initial_explicit_policy =
+      b ? InitialExplicitPolicy::kTrue : InitialExplicitPolicy::kFalse;
 }
 
 void PkitsTestInfo::SetInitialPolicyMappingInhibit(bool b) {
-  initial_policy_mapping_inhibit = b;
+  initial_policy_mapping_inhibit = b ? InitialPolicyMappingInhibit::kTrue
+                                     : InitialPolicyMappingInhibit::kFalse;
 }
 
 void PkitsTestInfo::SetInitialInhibitAnyPolicy(bool b) {
-  initial_inhibit_any_policy = b;
+  initial_inhibit_any_policy =
+      b ? InitialAnyPolicyInhibit::kTrue : InitialAnyPolicyInhibit::kFalse;
 }
 
 PkitsTestInfo::~PkitsTestInfo() = default;

net/cert/internal/nist_pkits_unittest.h

@@ -46,13 +46,15 @@ struct PkitsTestInfo {
   std::set<der::Input> initial_policy_set;
 
   // The value of "initial-explicit-policy".
-  bool initial_explicit_policy = false;
+  InitialExplicitPolicy initial_explicit_policy = InitialExplicitPolicy::kFalse;
 
   // The value of "initial-policy-mapping-inhibit".
-  bool initial_policy_mapping_inhibit = false;
+  InitialPolicyMappingInhibit initial_policy_mapping_inhibit =
+      InitialPolicyMappingInhibit::kFalse;
 
   // The value of "initial-inhibit-any-policy".
-  bool initial_inhibit_any_policy = false;
+  InitialAnyPolicyInhibit initial_inhibit_any_policy =
+      InitialAnyPolicyInhibit::kFalse;
 
   // This is the time when PKITS was published.
   der::GeneralizedTime time = {2011, 4, 15, 0, 0, 0};

net/cert/internal/path_builder.cc

@@ -604,9 +604,12 @@ void CertPathBuilder::DoGetNextPathComplete() {
 
   // Verify the entire certificate chain.
   auto result_path = base::MakeUnique<ResultPath>();
-  VerifyCertificateChain(next_path_.certs, next_path_.last_cert_trust,
-                         signature_policy_, time_, key_purpose_,
-                         &result_path->errors);
+  // TODO(eroman): don't pass placeholder for policy.
+  VerifyCertificateChain(
+      next_path_.certs, next_path_.last_cert_trust, signature_policy_, time_,
+      key_purpose_, InitialExplicitPolicy::kFalse, {AnyPolicy()},
+      InitialPolicyMappingInhibit::kFalse, InitialAnyPolicyInhibit::kFalse,
+      nullptr /*user_constrained_policy_set*/, &result_path->errors);
   bool verify_result = !result_path->errors.ContainsHighSeverityErrors();
 
   DVLOG(1) << "CertPathBuilder VerifyCertificateChain result = "

net/cert/internal/test_helpers.cc

@@ -125,7 +125,8 @@ der::Input SequenceValueFromString(const std::string* s) {
   return ::testing::AssertionSuccess();
 }
 
-VerifyCertChainTest::VerifyCertChainTest() = default;
+VerifyCertChainTest::VerifyCertChainTest()
+    : user_initial_policy_set{AnyPolicy()} {}
 VerifyCertChainTest::~VerifyCertChainTest() = default;
 
 bool VerifyCertChainTest::HasHighSeverityErrors() const {

net/cert/internal/test_helpers.h

@@ -95,6 +95,16 @@ struct VerifyCertChainTest {
   // The Key Purpose to use when verifying the chain.
   KeyPurpose key_purpose = KeyPurpose::ANY_EKU;
 
+  InitialExplicitPolicy initial_explicit_policy = InitialExplicitPolicy::kFalse;
+
+  std::set<der::Input> user_initial_policy_set;
+
+  InitialPolicyMappingInhibit initial_policy_mapping_inhibit =
+      InitialPolicyMappingInhibit::kFalse;
+
+  InitialAnyPolicyInhibit initial_any_policy_inhibit =
+      InitialAnyPolicyInhibit::kFalse;
+
   // The expected errors/warnings from verification (as a string).
   std::string expected_errors;
 

net/cert/internal/verify_certificate_chain.h

@@ -5,7 +5,7 @@
 #ifndef NET_CERT_INTERNAL_VERIFY_CERTIFICATE_CHAIN_H_
 #define NET_CERT_INTERNAL_VERIFY_CERTIFICATE_CHAIN_H_
 
-#include <vector>
+#include <set>
 
 #include "base/compiler_specific.h"
 #include "base/memory/ref_counted.h"
@@ -30,72 +30,160 @@ enum class KeyPurpose {
   CLIENT_AUTH,
 };
 
+enum class InitialExplicitPolicy {
+  kFalse,
+  kTrue,
+};
+
+enum class InitialPolicyMappingInhibit {
+  kFalse,
+  kTrue,
+};
+
+enum class InitialAnyPolicyInhibit {
+  kFalse,
+  kTrue,
+};
+
 // VerifyCertificateChain() verifies an ordered certificate path in accordance
-// with RFC 5280 (with some modifications [1]).
+// with RFC 5280's "Certification Path Validation" algorithm (section 6).
 //
-// [1] Deviations from RFC 5280:
+// -----------------------------------------
+// Deviations from RFC 5280
+// -----------------------------------------
 //
-//   * If Extended Key Usage appears on intermediates it is treated as a
-//     restriction on subordinate certificates.
+//   * If Extended Key Usage appears on intermediates, it is treated as
+//     a restriction on subordinate certificates.
 //
-// The caller is responsible for additionally checking:
-//
-//  * The end-entity's KeyUsage before using its SPKI.
-//  * The end-entity's name/subjectAltName (note that name constraints from
-//    intermediates will have already been applied, so just need to check
-//    the end-entity for a match).
-//  * Policies
+// -----------------------------------------
+// Additional responsibilities of the caller
+// -----------------------------------------
 //
-// WARNING: This implementation is in progress, and is currently incomplete.
-// Consult an OWNER before using it.
+// After successful path verification, the caller is responsible for
+// subsequently checking:
 //
-// TODO(eroman): Take a CertPath instead of ParsedCertificateList +
-//               TrustAnchor.
+//  * The end-entity's KeyUsage before using its SPKI.
+//  * The end-entity's name/subjectAltName. Name constraints from intermediates
+//    will have already been applied, so it is sufficient to check the
+//    end-entity for a match.
 //
 // ---------
 // Inputs
 // ---------
 //
-//   cert_chain:
-//     A non-empty chain of N DER-encoded certificates, listed in the
-//     "forward" direction. The first certificate is the target certificate to
-//     verify, and the last certificate has trustedness given by
-//     |last_cert_trust|.
+//   certs:
+//     A non-empty chain of DER-encoded certificates, listed in the
+//     "forward" direction. The first certificate is the target
+//     certificate to verify, and the last certificate has trustedness
+//     given by |last_cert_trust| (generally a trust anchor).
+//
+//      * certs[0] is the target certificate to verify.
+//      * certs[i+1] holds the certificate that issued cert_chain[i].
+//      * certs[N-1] the root certificate
 //
-//      * cert_chain[0] is the target certificate to verify.
-//      * cert_chain[i+1] holds the certificate that issued cert_chain[i].
-//      * cert_chain[N-1] the root certificate
+//     Note that THIS IS NOT identical in meaning to the same named
+//     "certs" input defined in RFC 5280 section 6.1.1.a. The differences
+//     are:
+//
+//      * The order of certificates is reversed
+//      * In RFC 5280 "certs" DOES NOT include the trust anchor
 //
 //   last_cert_trust:
-//     Trustedness of certs.back(). The trustedness of certs.back() MUST BE
-//     decided by the caller -- this function takes it purely as an input.
-//     Moreover, the CertificateTrust can be used to specify trust anchor
-//     constraints [1]
+//     Trustedness of |certs.back()|. The trustedness of |certs.back()|
+//     MUST BE decided by the caller -- this function takes it purely as
+//     an input. Moreover, the CertificateTrust can be used to specify
+//     trust anchor constraints.
+//
+//     This combined with |certs.back()| (the root certificate) fills a
+//     similar role to "trust anchor information" defined in RFC 5280
+//     section 6.1.1.d.
 //
 //   signature_policy:
 //     The policy to use when verifying signatures (what hash algorithms are
 //     allowed, what length keys, what named curves, etc).
 //
 //   time:
-//     The UTC time to use for expiration checks.
+//     The UTC time to use for expiration checks. This is equivalent to
+//     the input from RFC 5280 section 6.1.1:
 //
-//   key_purpose:
+//       (b)  the current date/time.
+//
+//   required_key_purpose:
 //     The key purpose that the target certificate needs to be valid for.
 //
+//   user_initial_policy_set:
+//     This is equivalent to the same named input in RFC 5280 section
+//     6.1.1:
+//
+//       (c)  user-initial-policy-set: A set of certificate policy
+//            identifiers naming the policies that are acceptable to the
+//            certificate user. The user-initial-policy-set contains the
+//            special value any-policy if the user is not concerned about
+//            certificate policy.
+//
+//   initial_policy_mapping_inhibit:
+//     This is equivalent to the same named input in RFC 5280 section
+//     6.1.1:
+//
+//       (e)  initial-policy-mapping-inhibit, which indicates if policy
+//            mapping is allowed in the certification path.
+//
+//   initial_explicit_policy:
+//     This is equivalent to the same named input in RFC 5280 section
+//     6.1.1:
+//
+//       (f)  initial-explicit-policy, which indicates if the path must be
+//            valid for at least one of the certificate policies in the
+//            user-initial-policy-set.
+//
+//   initial_any_policy_inhibit:
+//     This is equivalent to the same named input in RFC 5280 section
+//     6.1.1:
+//
+//       (g)  initial-any-policy-inhibit, which indicates whether the
+//            anyPolicy OID should be processed if it is included in a
+//            certificate.
+//
 // ---------
 // Outputs
 // ---------
+//
+//   user_constrained_policy_set:
+//     Can be null. If non-null, |user_constrained_policy_set| will be filled
+//     with the matching policies (intersected with user_initial_policy_set).
+//     This is equivalent to the same named output in X.509 section 10.2.
+//     Note that it is OK for this to point to input user_initial_policy_set.
+//
 //   errors:
 //     Must be non-null. The set of errors/warnings encountered while
 //     validating the path are appended to this structure. If verification
 //     failed, then there is guaranteed to be at least 1 high severity error
 //     written to |errors|.
 //
-// [1] Conceptually VerifyCertificateChain() sets RFC 5937's
-// "enforceTrustAnchorConstraints" to true. And one specifies whether to
-// interpret a root certificate as having trust anchor constraints through the
-// |last_cert_trust| parameter. The constraints are just a subset of the
-// extensions present in the certificate:
+// -------------------------
+// Trust Anchor constraints
+// -------------------------
+//
+// Conceptually, VerifyCertificateChain() sets RFC 5937's
+// "enforceTrustAnchorConstraints" to true.
+//
+// One specifies trust anchor constraints using the |last_cert_trust|
+// parameter in conjunction with extensions appearing in |certs.back()|.
+//
+// The trust anchor |certs.back()| is always passed as a certificate to
+// this function, however the manner in which that certificate is
+// interpreted depends on |last_cert_trust|:
+//
+// TRUSTED_ANCHOR:
+//
+// No properties from the root certificate, other than its Subject and
+// SPKI, are checked during verification. This is the usual
+// interpretation for a "trust anchor".
+//
+// TRUSTED_ANCHOR_WITH_CONSTRAINTS:
+//
+// Only a subset of extensions and properties from the certificate are checked,
+// as described by RFC 5937.
 //
 //  * Signature:             No
 //  * Validity (expiration): No
@@ -104,17 +192,24 @@ enum class KeyPurpose {
 //  * Basic constraints:     Yes, but only the pathlen (CA=false is accepted)
 //  * Name constraints:      Yes
 //  * Certificate policies:  Not currently, TODO(crbug.com/634453)
+//  * Policy Mappings:       No
 //  * inhibitAnyPolicy:      Not currently, TODO(crbug.com/634453)
 //  * PolicyConstraints:     Not currently, TODO(crbug.com/634452)
 //
 // The presence of any other unrecognized extension marked as critical fails
 // validation.
-NET_EXPORT void VerifyCertificateChain(const ParsedCertificateList& certs,
-                                       const CertificateTrust& last_cert_trust,
-                                       const SignaturePolicy* signature_policy,
-                                       const der::GeneralizedTime& time,
-                                       KeyPurpose required_key_purpose,
-                                       CertPathErrors* errors);
+NET_EXPORT void VerifyCertificateChain(
+    const ParsedCertificateList& certs,
+    const CertificateTrust& last_cert_trust,
+    const SignaturePolicy* signature_policy,
+    const der::GeneralizedTime& time,
+    KeyPurpose required_key_purpose,
+    InitialExplicitPolicy initial_explicit_policy,
+    const std::set<der::Input>& user_initial_policy_set,
+    InitialPolicyMappingInhibit initial_policy_mapping_inhibit,
+    InitialAnyPolicyInhibit initial_any_policy_inhibit,
+    std::set<der::Input>* user_constrained_policy_set,
+    CertPathErrors* errors);
 
 // TODO(crbug.com/634443): Move exported errors to a central location?
 extern CertErrorId kValidityFailedNotAfter;

net/cert/internal/verify_certificate_chain_pkits_unittest.cc

@@ -67,12 +67,19 @@ class VerifyCertificateChainPkitsTestDelegate {
 
     SimpleSignaturePolicy signature_policy(1024);
 
+    std::set<der::Input> user_constrained_policy_set;
+
     CertPathErrors path_errors;
-    VerifyCertificateChain(input_chain, CertificateTrust::ForTrustAnchor(),
-                           &signature_policy, info.time, KeyPurpose::ANY_EKU,
-                           &path_errors);
+    VerifyCertificateChain(
+        input_chain, CertificateTrust::ForTrustAnchor(), &signature_policy,
+        info.time, KeyPurpose::ANY_EKU, info.initial_explicit_policy,
+        info.initial_policy_set, info.initial_policy_mapping_inhibit,
+        info.initial_inhibit_any_policy, &user_constrained_policy_set,
+        &path_errors);
     bool did_succeed = !path_errors.ContainsHighSeverityErrors();
 
+    EXPECT_EQ(info.user_constrained_policy_set, user_constrained_policy_set);
+
     //  TODO(crbug.com/634443): Test errors on failure?
     if (info.should_validate != did_succeed) {
       ASSERT_EQ(info.should_validate, did_succeed)
@@ -221,6 +228,21 @@ INSTANTIATE_TYPED_TEST_CASE_P(VerifyCertificateChain,
 INSTANTIATE_TYPED_TEST_CASE_P(VerifyCertificateChain,
                               PkitsTest07KeyUsage,
                               VerifyCertificateChainPkitsTestDelegate);
+INSTANTIATE_TYPED_TEST_CASE_P(VerifyCertificateChain,
+                              PkitsTest08CertificatePolicies,
+                              VerifyCertificateChainPkitsTestDelegate);
+INSTANTIATE_TYPED_TEST_CASE_P(VerifyCertificateChain,
+                              PkitsTest09RequireExplicitPolicy,
+                              VerifyCertificateChainPkitsTestDelegate);
+INSTANTIATE_TYPED_TEST_CASE_P(VerifyCertificateChain,
+                              PkitsTest10PolicyMappings,
+                              VerifyCertificateChainPkitsTestDelegate);
+INSTANTIATE_TYPED_TEST_CASE_P(VerifyCertificateChain,
+                              PkitsTest11InhibitPolicyMapping,
+                              VerifyCertificateChainPkitsTestDelegate);
+INSTANTIATE_TYPED_TEST_CASE_P(VerifyCertificateChain,
+                              PkitsTest12InhibitAnyPolicy,
+                              VerifyCertificateChainPkitsTestDelegate);
 INSTANTIATE_TYPED_TEST_CASE_P(VerifyCertificateChain,
                               PkitsTest13NameConstraints,
                               VerifyCertificateChainPkitsTestDelegate);
@@ -232,8 +254,4 @@ INSTANTIATE_TYPED_TEST_CASE_P(VerifyCertificateChain,
 // PkitsTest05VerifyingPathswithSelfIssuedCertificates,
 // PkitsTest14DistributionPoints, PkitsTest15DeltaCRLs
 
-// TODO(mattm): Certificate Policies support: PkitsTest08CertificatePolicies,
-// PkitsTest09RequireExplicitPolicy PkitsTest10PolicyMappings,
-// PkitsTest11InhibitPolicyMapping, PkitsTest12InhibitAnyPolicy
-
 }  // namespace net

net/cert/internal/verify_certificate_chain_unittest.cc

@@ -19,8 +19,13 @@ class VerifyCertificateChainDelegate {
     SimpleSignaturePolicy signature_policy(1024);
 
     CertPathErrors errors;
-    VerifyCertificateChain(test.chain, test.last_cert_trust, &signature_policy,
-                           test.time, test.key_purpose, &errors);
+    // TODO(eroman): Check user_constrained_policy_set.
+    VerifyCertificateChain(
+        test.chain, test.last_cert_trust, &signature_policy, test.time,
+        test.key_purpose, test.initial_explicit_policy,
+        test.user_initial_policy_set, test.initial_policy_mapping_inhibit,
+        test.initial_any_policy_inhibit,
+        nullptr /*user_constrained_policy_set*/, &errors);
     EXPECT_EQ(test.expected_errors, errors.ToDebugString(test.chain))
         << "Test file: " << test_file_path;
   }

net/third_party/nist-pkits/generate_tests.py

@@ -688,6 +688,17 @@ TEST_OVERRIDES = {
              user_constrained_policy_set=[]),
   ],
 
+  '4.10.7': [ # Invalid Mapping From anyPolicy Test7 
+    # Procedure: Validate Invalid Mapping From anyPolicy Test7 EE using the
+    # default settings or open and verify Signed Test Message 6.2.2.100 using
+    # the default settings.
+    #
+    # Expected Result: The path should not validate successfully since the
+    # intermediate certificate includes a policy mapping extension in which
+    # anyPolicy appears as an issuerDomainPolicy. 
+    TestInfo(False, user_constrained_policy_set=[]),
+  ],
+
   '4.10.8': [ # Invalid Mapping To anyPolicy Test8
     # Procedure: Validate Invalid Mapping To anyPolicy Test8 EE using the
     # default settings or open and verify Signed Test Message 6.2.2.101 using
@@ -696,8 +707,6 @@ TEST_OVERRIDES = {
     # Expected Result: The path should not validate successfully since the
     # intermediate certificate includes a policy mapping extension in which
     # anyPolicy appears as an subjectDomainPolicy.
-    #
-    # TODO(eroman): What should user_constrained_policy_set be?
     TestInfo(False, user_constrained_policy_set=[]),
   ],
 

net/third_party/nist-pkits/pkits_testcases-inl.h

@@ -2113,6 +2113,7 @@ WRAPPED_TYPED_TEST_P(PkitsTest10PolicyMappings,
                               "MappingFromanyPolicyCACRL"};
   PkitsTestInfo info;
   info.should_validate = false;
+  info.SetUserConstrainedPolicySet("");
 
   this->RunTest(certs, crls, info);
 }