Add errors per ResultPath for CertPathBuilder.

net/cert/internal/verify_certificate_chain.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/cert/internal/verify_certificate_chain.h"
 
 #include <memory>
 
 #include "base/logging.h"
 #include "net/cert/internal/cert_errors.h"
@@ skipping 437 matching lines @@
 
 }  // namespace
 
 // This implementation is structured to mimic the description of certificate
 // path verification given by RFC 5280 section 6.1.
 bool VerifyCertificateChain(const ParsedCertificateList& certs,
                             const TrustAnchor* trust_anchor,
                             const SignaturePolicy* signature_policy,
                             const der::GeneralizedTime& time,
                             CertErrors* errors) {
-  DCHECK(trust_anchor);
   DCHECK(signature_policy);
   DCHECK(errors);
 
   // An empty chain is necessarily invalid.
   if (certs.empty()) {
     errors->Add(kChainIsEmpty);
     return false;
   }
 
+  if (!trust_anchor) {

[mattm, 2016/08/31 19:49:49]

does something hit this currently? or just to be more consistent with the
handling of empty chain?

[eroman, 2016/08/31 21:46:50]

Good question, let me run a test to find out.

The way in which this came up is when I was changing the code in path builder
[1], it tests for a null trust anchor, which implies it can reach there with a
null trust anchor.

I didn't think too hard if that is actually the case, but proceeding under the
assumption that it is, I figured it would be simpler to move the error emitting
inside of VerifyCertificateChain() rather than run the risk of crashing.

I am not totally sold on this approach though, so happy to change it based on
your thoughts. WDYT?

[eroman, 2016/08/31 21:55:37]

Ran the tests -- nothing reaches this.

... which suggests that either I don't need it, or I should add some test. I
will probably just remove this since it is weird.

[eroman, 2016/09/01 03:44:51]

Done -- removed

+    errors->Add(kNullTrustAnchor);
+    return false;
+  }
+
   // Will contain a NameConstraints for each previous cert in the chain which
   // had nameConstraints. This corresponds to the permitted_subtrees and
   // excluded_subtrees state variables from RFC 5280.
   std::vector<const NameConstraints*> name_constraints_list;
 
   // |working_spki| is an amalgamation of 3 separate variables from RFC 5280:
   //    * working_public_key
   //    * working_public_key_algorithm
   //    * working_public_key_parameters
   //
@@ skipping 106 matching lines @@
                        "Not permitted by name constraints");
 DEFINE_CERT_ERROR_TYPE(kSubjectDoesNotMatchIssuer,
                        "subject does not match issuer");
 DEFINE_CERT_ERROR_TYPE(kSignatureVerificationFailed,
                        "Signature verification failed");
 DEFINE_CERT_ERROR_TYPE(kValidityFailedNotAfter, "Time is after notAfter");
 DEFINE_CERT_ERROR_TYPE(kValidityFailedNotBefore, "Time is before notBefore");
 DEFINE_CERT_ERROR_TYPE(kSignatureAlgorithmsDifferentEncoding,
                        "Certificate.signatureAlgorithm is encoded differently "
                        "than TBSCertificate.signature");
+DEFINE_CERT_ERROR_TYPE(kNullTrustAnchor, "Missing trust anchor");
 
 }  // verify_certificate_chain_errors
 
 }  // namespace net