Add policies support to VerifyCertificateChain().

net/cert/internal/verify_certificate_chain.h

Index: net/cert/internal/verify_certificate_chain.h
diff --git a/net/cert/internal/verify_certificate_chain.h b/net/cert/internal/verify_certificate_chain.h
index 8d1f044805192df088776b66f6a9b9f6f6c9a50e..09a9d6b6408c067f9f82062186a147be20333947 100644
--- a/net/cert/internal/verify_certificate_chain.h
+++ b/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"
@@ -31,57 +31,104 @@ enum class KeyPurpose {
 };
 
 // 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:
+// -----------------------------------------
+// Additional responsibilities of the caller
+// -----------------------------------------
 //
-//  * 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
-//
-// 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).
 //
-//      * 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
+//      * 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
+//
+//     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
 // ---------
@@ -91,11 +138,29 @@ enum class KeyPurpose {
 //     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:
 //
 //  * Signature:             No
 //  * Validity (expiration): No
@@ -104,17 +169,23 @@ 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

[mattm, 2017/05/25 23:21:49]

yes?

[eroman, 2017/05/26 18:49:59]

The "No" is because it is not specified in rfc 5937. I have been trying to match
5937's notion of trust anchor constraints here, which in essence just does a
preprocessing step and changes the inputs to RFC 5937.

I have some ideas on improving this, but don't want to do so in this (which is
why I left the the others as "Not currently, TODO").

 //  * 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,
+    bool initial_explicit_policy,
+    const std::set<der::Input>& user_initial_policy_set,
+    bool initial_policy_mapping_inhibit,
+    bool initial_any_policy_inhibit,

[mattm, 2017/05/25 23:21:49]

maybe use enums instead of passing a bunch of bools?

[eroman, 2017/05/26 18:49:59]

sgtm

+    CertPathErrors* errors);

[mattm, 2017/05/25 23:21:49]

Add a todo for outputting the final valid_policy_tree?

[eroman, 2017/05/26 18:49:59]

Good idea.

I will go ahead and add "user-constrained-policy-set" as an output which will be
useful in some other CLs.
I don't think valid_policy_tree itself is worth outputting (or calculating for
that matter).
Let me see how well just the set works.

 
 // TODO(crbug.com/634443): Move exported errors to a central location?
 extern CertErrorId kValidityFailedNotAfter;