QUIC - Race Cert Verification with host resolution if certs are

net/quic/quic_stream_factory.cc

 // Copyright (c) 2012 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/quic_stream_factory.h"
 
 #include <openssl/aead.h>
 
 #include <algorithm>
 #include <tuple>
@@ skipping 16 matching lines @@
 #include "crypto/openssl_util.h"
 #include "net/base/ip_address.h"
 #include "net/base/net_errors.h"
 #include "net/cert/cert_verifier.h"
 #include "net/cert/ct_verifier.h"
 #include "net/dns/host_resolver.h"
 #include "net/dns/single_request_host_resolver.h"
 #include "net/http/bidirectional_stream_impl.h"
 #include "net/quic/bidirectional_stream_quic_impl.h"
 #include "net/quic/crypto/channel_id_chromium.h"
+#include "net/quic/crypto/proof_verifier.h"
 #include "net/quic/crypto/proof_verifier_chromium.h"
 #include "net/quic/crypto/properties_based_quic_server_info.h"
 #include "net/quic/crypto/quic_random.h"
 #include "net/quic/crypto/quic_server_info.h"
 #include "net/quic/port_suggester.h"
 #include "net/quic/quic_chromium_alarm_factory.h"
 #include "net/quic/quic_chromium_connection_helper.h"
 #include "net/quic/quic_chromium_packet_reader.h"
 #include "net/quic/quic_chromium_packet_writer.h"
 #include "net/quic/quic_client_promised_info.h"
@@ skipping 113 matching lines @@
   QuicConfig config;
   config.SetIdleConnectionStateLifetime(
       QuicTime::Delta::FromSeconds(idle_connection_timeout_seconds),
       QuicTime::Delta::FromSeconds(idle_connection_timeout_seconds));
   config.SetConnectionOptionsToSend(connection_options);
   return config;
 }
 
 }  // namespace
 
+class QuicStreamFactory::CertVerifierJob {
+ public:
+  class ProofVerifierCallbackImpl : public ProofVerifierCallback {

[Ryan Hamilton, 2016/07/07 00:24:36]

Hm. I'm surprised this is using the ProofVerifier instead of the using the
CertVerifier directly. Looking at the code, I guess it's non-trivial to get the
certs into the right form and then there are a bunch of check we need to do. OK,
makes sense.

[ramant (doing other things), 2016/07/07 16:18:20]

Wondered about the same (whether to duplicate some of the ProofVerifierChromium
code or not).
Acknowledged.

+   public:
+    explicit ProofVerifierCallbackImpl(CertVerifierJob* job) : job_(job) {}
+
+    ~ProofVerifierCallbackImpl() override {}
+
+    void Run(bool ok,
+             const std::string& error_details,
+             std::unique_ptr<ProofVerifyDetails>* details) override {
+      if (job_ == nullptr)
+        return;
+      job_->verify_callback_ = nullptr;
+      job_->OnComplete();
+    }
+
+    void Cancel() { job_ = nullptr; }
+
+   private:
+    CertVerifierJob* job_;
+  };
+
+  CertVerifierJob(const QuicServerId& server_id,
+                  int cert_verify_flags,
+                  const BoundNetLog& net_log)
+      : server_id_(server_id),
+        verify_callback_(nullptr),
+        verify_context_(base::WrapUnique(
+            new ProofVerifyContextChromium(cert_verify_flags, net_log))),
+        net_log_(net_log),
+        weak_factory_(this) {}
+
+  ~CertVerifierJob() {
+    if (verify_callback_)
+      verify_callback_->Cancel();
+  }
+
+  QuicAsyncStatus Run(QuicCryptoClientConfig* crypto_config,
+                      const CompletionCallback& callback) {
+    QuicCryptoClientConfig::CachedState* cached =
+        crypto_config->LookupOrCreate(server_id_);
+    ProofVerifierCallbackImpl* verify_callback =
+        new ProofVerifierCallbackImpl(this);
+    // Pass empty chlo_hash, signature and cert_sct so that only certificate is
+    // verified.
+    QuicAsyncStatus status = crypto_config->proof_verifier()->VerifyProof(
+        server_id_.host(), server_id_.port(), cached->server_config(),
+        QUIC_VERSION_25, /*chlo_hash=*/"", cached->certs(),

[Ryan Hamilton, 2016/07/07 00:24:36]

hard-coding a quic version here seems scary.

[ramant (doing other things), 2016/07/07 16:18:20]

Deleted it.

Done.

+        /*cert_sct=*/"", /*signature=*/"", verify_context_.get(),
+        &verify_error_details_, &verify_details_, verify_callback);

[Ryan Hamilton, 2016/07/07 00:24:36]

Wow, there are a lot of empty arguments. I'd be inclined to add a new method to
the ProofVerifier just for this purpose.

[ramant (doing other things), 2016/07/07 16:18:20]

Done.

+    if (status == QUIC_PENDING) {
+      verify_callback_ = verify_callback;
+      callback_ = callback;
+    } else {
+      delete verify_callback;
+    }
+    return status;
+  }
+
+  void OnComplete() {
+    if (!callback_.is_null())
+      callback_.Run(OK);
+  }
+
+  const QuicServerId& server_id() const { return server_id_; }
+
+ private:
+  QuicServerId server_id_;
+  ProofVerifierCallbackImpl* verify_callback_;
+  std::unique_ptr<ProofVerifyContext> verify_context_;
+  std::unique_ptr<ProofVerifyDetails> verify_details_;
+  std::string verify_error_details_;
+  const BoundNetLog net_log_;
+  CompletionCallback callback_;
+  base::WeakPtrFactory<CertVerifierJob> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(CertVerifierJob);
+};
+
 // Responsible for creating a new QUIC session to the specified server, and
 // for notifying any associated requests when complete.
 class QuicStreamFactory::Job {
  public:
   Job(QuicStreamFactory* factory,
       HostResolver* host_resolver,
       const QuicSessionKey& key,
       bool was_alternative_service_recently_broken,
       int cert_verify_flags,
       QuicServerInfo* server_info,
@@ skipping 445 matching lines @@
     int threshold_public_resets_post_handshake,
     int threshold_timeouts_with_open_streams,
     int socket_receive_buffer_size,
     bool delay_tcp_race,
     int max_server_configs_stored_in_properties,
     bool close_sessions_on_ip_change,
     bool disable_quic_on_timeout_with_open_streams,
     int idle_connection_timeout_seconds,
     bool migrate_sessions_on_network_change,
     bool migrate_sessions_early,
+    bool race_cert_verification,
     const QuicTagVector& connection_options,
     bool enable_token_binding)
     : require_confirmation_(true),
       net_log_(net_log),
       host_resolver_(host_resolver),
       client_socket_factory_(client_socket_factory),
       http_server_properties_(http_server_properties),
       transport_security_state_(transport_security_state),
       cert_transparency_verifier_(cert_transparency_verifier),
       quic_crypto_client_stream_factory_(quic_crypto_client_stream_factory),
@@ skipping 33 matching lines @@
       delay_tcp_race_(delay_tcp_race),
       yield_after_packets_(kQuicYieldAfterPacketsRead),
       yield_after_duration_(QuicTime::Delta::FromMilliseconds(
           kQuicYieldAfterDurationMilliseconds)),
       close_sessions_on_ip_change_(close_sessions_on_ip_change),
       migrate_sessions_on_network_change_(
           migrate_sessions_on_network_change &&
           NetworkChangeNotifier::AreNetworkHandlesSupported()),
       migrate_sessions_early_(migrate_sessions_early &&
                               migrate_sessions_on_network_change_),
+      race_cert_verification_(race_cert_verification),
       port_seed_(random_generator_->RandUint64()),
       check_persisted_supports_quic_(true),
       has_initialized_data_(false),
       num_push_streams_created_(0),
       status_(OPEN),
       task_runner_(nullptr),
       ssl_config_service_(ssl_config_service),
       weak_factory_(this) {
   if (ssl_config_service_.get())
     ssl_config_service_->AddObserver(this);
@@ skipping 47 matching lines @@
   CloseAllSessions(ERR_ABORTED, QUIC_CONNECTION_CANCELLED);
   while (!all_sessions_.empty()) {
     delete all_sessions_.begin()->first;
     all_sessions_.erase(all_sessions_.begin());
   }
   while (!active_jobs_.empty()) {
     const QuicServerId server_id = active_jobs_.begin()->first;
     STLDeleteElements(&(active_jobs_[server_id]));
     active_jobs_.erase(server_id);
   }
+  while (!active_cert_verifier_jobs_.empty()) {
+    delete active_cert_verifier_jobs_.begin()->second;
+    active_cert_verifier_jobs_.erase(active_cert_verifier_jobs_.begin());

[Ryan Hamilton, 2016/07/07 00:24:36]

I wonder if the value in the map could be an std::unique_ptr?

[ramant (doing other things), 2016/07/07 16:18:20]

Done.

+  }
   if (ssl_config_service_.get())
     ssl_config_service_->RemoveObserver(this);
   if (migrate_sessions_on_network_change_) {
     NetworkChangeNotifier::RemoveNetworkObserver(this);
   } else if (close_sessions_on_ip_change_) {
     NetworkChangeNotifier::RemoveIPAddressObserver(this);
   }
 }
 
 void QuicStreamFactory::set_require_confirmation(bool require_confirmation) {
@@ skipping 116 matching lines @@
     if (!ContainsKey(quic_supported_servers_at_startup_, destination)) {
       // If there is no entry for QUIC, consider that as a new server and
       // don't wait for Cache thread to load the data for that server.
       load_from_disk_cache = false;
     }
     if (load_from_disk_cache && CryptoConfigCacheIsEmpty(server_id)) {
       quic_server_info = quic_server_info_factory_->GetForServer(server_id);
     }
   }
 
+  StartCertVerifyJob(server_id, cert_verify_flags, net_log);
+
   QuicSessionKey key(destination, server_id);
   std::unique_ptr<Job> job(
       new Job(this, host_resolver_, key, WasQuicRecentlyBroken(server_id),
               cert_verify_flags, quic_server_info, net_log));
   int rv = job->Run(base::Bind(&QuicStreamFactory::OnJobComplete,
                                base::Unretained(this), job.get()));
   if (rv == ERR_IO_PENDING) {
     active_requests_[request] = server_id;
     job_requests_map_[server_id].insert(request);
     active_jobs_[server_id].insert(job.release());
@@ skipping 112 matching lines @@
   for (Job* other_job : active_jobs_[server_id]) {
     if (other_job != job)
       other_job->Cancel();
   }
 
   STLDeleteElements(&(active_jobs_[server_id]));
   active_jobs_.erase(server_id);
   job_requests_map_.erase(server_id);
 }
 
+void QuicStreamFactory::OnCertVerifyJobComplete(CertVerifierJob* job, int rv) {
+  const QuicServerId server_id(job->server_id());
+  delete job;
+  active_cert_verifier_jobs_.erase(server_id);
+}
+
 std::unique_ptr<QuicHttpStream> QuicStreamFactory::CreateFromSession(
     QuicChromiumClientSession* session) {
   return std::unique_ptr<QuicHttpStream>(
       new QuicHttpStream(session->GetWeakPtr()));
 }
 
 QuicChromiumClientSession::QuicDisabledReason
 QuicStreamFactory::QuicDisabledReason(uint16_t port) const {
   if (max_number_of_lossy_connections_ > 0 &&
       number_of_lossy_connections_.find(port) !=
@@ skipping 508 matching lines @@
   // TODO(rtenneti): crbug.com/498823 - delete active_sessions_.empty() check.
   if (active_sessions_.empty())
     return false;
   return ContainsKey(active_sessions_, server_id);
 }
 
 bool QuicStreamFactory::HasActiveJob(const QuicServerId& server_id) const {
   return ContainsKey(active_jobs_, server_id);
 }
 
+bool QuicStreamFactory::HasActiveCertVerifierJob(
+    const QuicServerId& server_id) const {
+  return ContainsKey(active_cert_verifier_jobs_, server_id);
+}
+
 int QuicStreamFactory::ConfigureSocket(DatagramClientSocket* socket,
                                        IPEndPoint addr,
                                        NetworkHandle network) {
   if (enable_non_blocking_io_ &&
       client_socket_factory_ == ClientSocketFactory::GetDefaultFactory()) {
 #if defined(OS_WIN)
     static_cast<UDPClientSocket*>(socket)->UseNonBlockingIO();
 #endif
   }
 
@@ skipping 187 matching lines @@
       alternative_service);
 }
 
 bool QuicStreamFactory::CryptoConfigCacheIsEmpty(
     const QuicServerId& server_id) {
   QuicCryptoClientConfig::CachedState* cached =
       crypto_config_.LookupOrCreate(server_id);
   return cached->IsEmpty();
 }
 
+void QuicStreamFactory::StartCertVerifyJob(const QuicServerId& server_id,
+                                           int cert_verify_flags,
+                                           const BoundNetLog& net_log) {
+  if (!race_cert_verification_)
+    return;
+  QuicCryptoClientConfig::CachedState* cached =
+      crypto_config_.LookupOrCreate(server_id);
+  if (!cached || cached->certs().empty() ||
+      HasActiveCertVerifierJob(server_id)) {
+    return;
+  }
+  std::unique_ptr<CertVerifierJob> cert_verifier_job(
+      new CertVerifierJob(server_id, cert_verify_flags, net_log));
+  QuicAsyncStatus status = cert_verifier_job->Run(
+      &crypto_config_,
+      base::Bind(&QuicStreamFactory::OnCertVerifyJobComplete,
+                 base::Unretained(this), cert_verifier_job.get()));
+  if (status == QUIC_PENDING)
+    active_cert_verifier_jobs_[server_id] = cert_verifier_job.release();
+}
+
 void QuicStreamFactory::InitializeCachedStateInCryptoConfig(
     const QuicServerId& server_id,
     const std::unique_ptr<QuicServerInfo>& server_info,
     QuicConnectionId* connection_id) {
   QuicCryptoClientConfig::CachedState* cached =
       crypto_config_.LookupOrCreate(server_id);
   if (cached->has_server_designated_connection_id())
     *connection_id = cached->GetNextServerDesignatedConnectionId();
 
   if (!cached->IsEmpty())
@@ skipping 101 matching lines @@
   // Since the session was active, there's no longer an
   // HttpStreamFactoryImpl::Job running which can mark it broken, unless the TCP
   // job also fails. So to avoid not using QUIC when we otherwise could, we mark
   // it as recently broken, which means that 0-RTT will be disabled but we'll
   // still race.
   http_server_properties_->MarkAlternativeServiceRecentlyBroken(
       alternative_service);
 }
 
 }  // namespace net