Revert of QUIC - Race Cert Verification with host resolution if certs are

net/quic/chromium/crypto/proof_verifier_chromium.cc

 // Copyright 2013 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/quic/chromium/crypto/proof_verifier_chromium.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 60 matching lines @@
       const std::string& server_config,
       QuicVersion quic_version,
       base::StringPiece chlo_hash,
       const std::vector<std::string>& certs,
       const std::string& cert_sct,
       const std::string& signature,
       std::string* error_details,
       std::unique_ptr<ProofVerifyDetails>* verify_details,
       std::unique_ptr<ProofVerifierCallback> callback);
 
-  // Starts the certificate chain verification of |certs|.  If |QUIC_PENDING| is
-  // returned, then |callback| will be invoked asynchronously when the
-  // verification completes.
-  QuicAsyncStatus VerifyCertChain(
-      const std::string& hostname,
-      const std::vector<std::string>& certs,
-      std::string* error_details,
-      std::unique_ptr<ProofVerifyDetails>* verify_details,
-      std::unique_ptr<ProofVerifierCallback> callback);
-
  private:
   enum State {
     STATE_NONE,
     STATE_VERIFY_CERT,
     STATE_VERIFY_CERT_COMPLETE,
   };
 
-  // Convert |certs| to |cert_|(X509Certificate). Returns true if successful.
-  bool GetX509Certificate(const vector<string>& certs,
-                          std::string* error_details,
-                          std::unique_ptr<ProofVerifyDetails>* verify_details);
-
-  // Start the cert verification.
-  QuicAsyncStatus VerifyCert(
-      const string& hostname,
-      const uint16_t port,
-      std::string* error_details,
-      std::unique_ptr<ProofVerifyDetails>* verify_details,
-      std::unique_ptr<ProofVerifierCallback> callback);
-
   int DoLoop(int last_io_result);
   void OnIOComplete(int result);
   int DoVerifyCert(int result);
   int DoVerifyCertComplete(int result);
 
   bool VerifySignature(const std::string& signed_data,
                        QuicVersion quic_version,
                        StringPiece chlo_hash,
                        const std::string& signature,
                        const std::string& cert);
@@ skipping 20 matching lines @@
   std::unique_ptr<ProofVerifyDetailsChromium> verify_details_;
   std::string error_details_;
 
   // X509Certificate from a chain of DER encoded certificates.
   scoped_refptr<X509Certificate> cert_;
 
   // |cert_verify_flags| is bitwise OR'd of CertVerifier::VerifyFlags and it is
   // passed to CertVerifier::Verify.
   int cert_verify_flags_;
 
-  // If set to true, enforces policy checking in DoVerifyCertComplete().
-  bool enforce_policy_checking_;
-
   State next_state_;
 
   base::TimeTicks start_time_;
 
   BoundNetLog net_log_;
 
   DISALLOW_COPY_AND_ASSIGN(Job);
 };
 
 ProofVerifierChromium::Job::Job(
     ProofVerifierChromium* proof_verifier,
     CertVerifier* cert_verifier,
     CTPolicyEnforcer* ct_policy_enforcer,
     TransportSecurityState* transport_security_state,
     CTVerifier* cert_transparency_verifier,
     int cert_verify_flags,
     const BoundNetLog& net_log)
     : proof_verifier_(proof_verifier),
       verifier_(cert_verifier),
       policy_enforcer_(ct_policy_enforcer),
       transport_security_state_(transport_security_state),
       cert_transparency_verifier_(cert_transparency_verifier),
       cert_verify_flags_(cert_verify_flags),
-      enforce_policy_checking_(true),
       next_state_(STATE_NONE),
       start_time_(base::TimeTicks::Now()),
       net_log_(net_log) {
   CHECK(proof_verifier_);
   CHECK(verifier_);
   CHECK(policy_enforcer_);
   CHECK(transport_security_state_);
   CHECK(cert_transparency_verifier_);
 }
 
@@ skipping 27 matching lines @@
   error_details->clear();
 
   if (STATE_NONE != next_state_) {
     *error_details = "Certificate is already set and VerifyProof has begun";
     DLOG(DFATAL) << *error_details;
     return QUIC_FAILURE;
   }
 
   verify_details_.reset(new ProofVerifyDetailsChromium);
 
-  // Converts |certs| to |cert_|.
-  if (!GetX509Certificate(certs, error_details, verify_details))
+  if (certs.empty()) {
+    *error_details = "Failed to create certificate chain. Certs are empty.";
+    DLOG(WARNING) << *error_details;
+    verify_details_->cert_verify_result.cert_status = CERT_STATUS_INVALID;
+    *verify_details = std::move(verify_details_);
     return QUIC_FAILURE;
+  }
+
+  // Convert certs to X509Certificate.
+  vector<StringPiece> cert_pieces(certs.size());
+  for (unsigned i = 0; i < certs.size(); i++) {
+    cert_pieces[i] = base::StringPiece(certs[i]);
+  }
+  cert_ = X509Certificate::CreateFromDERCertChain(cert_pieces);
+  if (!cert_.get()) {
+    *error_details = "Failed to create certificate chain";
+    DLOG(WARNING) << *error_details;
+    verify_details_->cert_verify_result.cert_status = CERT_STATUS_INVALID;
+    *verify_details = std::move(verify_details_);
+    return QUIC_FAILURE;
+  }
 
   if (!cert_sct.empty()) {
     // Note that this is a completely synchronous operation: The CT Log Verifier
     // gets all the data it needs for SCT verification and does not do any
     // external communication.
     cert_transparency_verifier_->Verify(cert_.get(), std::string(), cert_sct,
                                         &verify_details_->ct_verify_result,
                                         net_log_);
   }
 
   // We call VerifySignature first to avoid copying of server_config and
   // signature.
-  if (!signature.empty() &&
-      !VerifySignature(server_config, quic_version, chlo_hash, signature,
+  if (!VerifySignature(server_config, quic_version, chlo_hash, signature,
                        certs[0])) {
     *error_details = "Failed to verify signature of server config";
     DLOG(WARNING) << *error_details;
     verify_details_->cert_verify_result.cert_status = CERT_STATUS_INVALID;
     *verify_details = std::move(verify_details_);
     return QUIC_FAILURE;
   }
 
-  DCHECK(enforce_policy_checking_);
-  return VerifyCert(hostname, port, error_details, verify_details,
-                    std::move(callback));
-}
-
-QuicAsyncStatus ProofVerifierChromium::Job::VerifyCertChain(
-    const string& hostname,
-    const vector<string>& certs,
-    std::string* error_details,
-    std::unique_ptr<ProofVerifyDetails>* verify_details,
-    std::unique_ptr<ProofVerifierCallback> callback) {
-  DCHECK(error_details);
-  DCHECK(verify_details);
-  DCHECK(callback);
-
-  error_details->clear();
-
-  if (STATE_NONE != next_state_) {
-    *error_details = "Certificate is already set and VerifyCertChain has begun";
-    DLOG(DFATAL) << *error_details;
-    return QUIC_FAILURE;
-  }
-
-  verify_details_.reset(new ProofVerifyDetailsChromium);
-
-  // Converts |certs| to |cert_|.
-  if (!GetX509Certificate(certs, error_details, verify_details))
-    return QUIC_FAILURE;
-
-  enforce_policy_checking_ = false;
-  // |port| is not needed because |enforce_policy_checking_| is false.
-  return VerifyCert(hostname, /*port=*/0, error_details, verify_details,
-                    std::move(callback));
-}
-
-bool ProofVerifierChromium::Job::GetX509Certificate(
-    const vector<string>& certs,
-    std::string* error_details,
-    std::unique_ptr<ProofVerifyDetails>* verify_details) {
-  if (certs.empty()) {
-    *error_details = "Failed to create certificate chain. Certs are empty.";
-    DLOG(WARNING) << *error_details;
-    verify_details_->cert_verify_result.cert_status = CERT_STATUS_INVALID;
-    *verify_details = std::move(verify_details_);
-    return false;
-  }
-
-  // Convert certs to X509Certificate.
-  vector<StringPiece> cert_pieces(certs.size());
-  for (unsigned i = 0; i < certs.size(); i++) {
-    cert_pieces[i] = base::StringPiece(certs[i]);
-  }
-  cert_ = X509Certificate::CreateFromDERCertChain(cert_pieces);
-  if (!cert_.get()) {
-    *error_details = "Failed to create certificate chain";
-    DLOG(WARNING) << *error_details;
-    verify_details_->cert_verify_result.cert_status = CERT_STATUS_INVALID;
-    *verify_details = std::move(verify_details_);
-    return false;
-  }
-  return true;
-}
-
-QuicAsyncStatus ProofVerifierChromium::Job::VerifyCert(
-    const string& hostname,
-    const uint16_t port,
-    std::string* error_details,
-    std::unique_ptr<ProofVerifyDetails>* verify_details,
-    std::unique_ptr<ProofVerifierCallback> callback) {
   hostname_ = hostname;
   port_ = port;
 
   next_state_ = STATE_VERIFY_CERT;
   switch (DoLoop(OK)) {
     case OK:
       *verify_details = std::move(verify_details_);
       return QUIC_SUCCESS;
     case ERR_IO_PENDING:
       callback_ = std::move(callback);
@@ skipping 58 matching lines @@
 
   const CertVerifyResult& cert_verify_result =
       verify_details_->cert_verify_result;
   const CertStatus cert_status = cert_verify_result.cert_status;
   verify_details_->ct_verify_result.ct_policies_applied = result == OK;
   verify_details_->ct_verify_result.ev_policy_compliance =
       ct::EVPolicyCompliance::EV_POLICY_DOES_NOT_APPLY;
 
   // If the connection was good, check HPKP and CT status simultaneously,
   // but prefer to treat the HPKP error as more serious, if there was one.
-  if (enforce_policy_checking_ &&
-      (result == OK ||
+  if ((result == OK ||
        (IsCertificateError(result) && IsCertStatusMinorError(cert_status)))) {
     if ((cert_verify_result.cert_status & CERT_STATUS_IS_EV)) {
       ct::EVPolicyCompliance ev_policy_compliance =
           policy_enforcer_->DoesConformToCTEVPolicy(
               cert_verify_result.verified_cert.get(),
               SSLConfigService::GetEVCertsWhitelist().get(),
               verify_details_->ct_verify_result.verified_scts, net_log_);
       verify_details_->ct_verify_result.ev_policy_compliance =
           ev_policy_compliance;
       if (ev_policy_compliance !=
@@ skipping 169 matching lines @@
   }
   const ProofVerifyContextChromium* chromium_context =
       reinterpret_cast<const ProofVerifyContextChromium*>(verify_context);
   std::unique_ptr<Job> job(
       new Job(this, cert_verifier_, ct_policy_enforcer_,
               transport_security_state_, cert_transparency_verifier_,
               chromium_context->cert_verify_flags, chromium_context->net_log));
   QuicAsyncStatus status = job->VerifyProof(
       hostname, port, server_config, quic_version, chlo_hash, certs, cert_sct,
       signature, error_details, verify_details, std::move(callback));
-  if (status == QUIC_PENDING)
+  if (status == QUIC_PENDING) {
     active_jobs_.insert(job.release());
+  }
   return status;
 }
 
-QuicAsyncStatus ProofVerifierChromium::VerifyCertChain(
-    const std::string& hostname,
-    const std::vector<std::string>& certs,
-    const ProofVerifyContext* verify_context,
-    std::string* error_details,
-    std::unique_ptr<ProofVerifyDetails>* verify_details,
-    std::unique_ptr<ProofVerifierCallback> callback) {
-  if (!verify_context) {
-    *error_details = "Missing context";
-    return QUIC_FAILURE;
-  }
-  const ProofVerifyContextChromium* chromium_context =
-      reinterpret_cast<const ProofVerifyContextChromium*>(verify_context);
-  std::unique_ptr<Job> job(
-      new Job(this, cert_verifier_, ct_policy_enforcer_,
-              transport_security_state_, cert_transparency_verifier_,
-              chromium_context->cert_verify_flags, chromium_context->net_log));
-  QuicAsyncStatus status = job->VerifyCertChain(
-      hostname, certs, error_details, verify_details, std::move(callback));
-  if (status == QUIC_PENDING)
-    active_jobs_.insert(job.release());
-  return status;
-}
-
 void ProofVerifierChromium::OnJobComplete(Job* job) {
   active_jobs_.erase(job);
   delete job;
 }
 
 }  // namespace net