Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(356)

Unified Diff: net/quic/chromium/quic_stream_factory.cc

Issue 2120703003: QUIC - Race Cert Verification with host resolution if certs are (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Fix for buildbot failure. Created 4 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « net/quic/chromium/quic_stream_factory.h ('k') | net/quic/chromium/quic_stream_factory_test.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/quic/chromium/quic_stream_factory.cc
diff --git a/net/quic/chromium/quic_stream_factory.cc b/net/quic/chromium/quic_stream_factory.cc
index 09fb8e62bf91078fe9640b0b56a2b41e25de43c7..b7739f9d7ab9702fff97745a8646e69d1f7fa198 100644
--- a/net/quic/chromium/quic_stream_factory.cc
+++ b/net/quic/chromium/quic_stream_factory.cc
@@ -40,6 +40,7 @@
#include "net/quic/chromium/quic_chromium_connection_helper.h"
#include "net/quic/chromium/quic_chromium_packet_reader.h"
#include "net/quic/chromium/quic_chromium_packet_writer.h"
+#include "net/quic/core/crypto/proof_verifier.h"
#include "net/quic/core/crypto/properties_based_quic_server_info.h"
#include "net/quic/core/crypto/quic_random.h"
#include "net/quic/core/crypto/quic_server_info.h"
@@ -167,6 +168,88 @@ QuicConfig InitializeQuicConfig(const QuicTagVector& connection_options,
} // namespace
+// Responsible for verifying the certificates saved in
+// QuicCryptoClientConfig, and for notifying any associated requests when
+// complete. Results from cert verification are ignored.
+class QuicStreamFactory::CertVerifierJob {
+ public:
+ // ProofVerifierCallbackImpl is passed as the callback method to
+ // VerifyCertChain. The ProofVerifier calls this class with the result of cert
+ // verification when verification is performed asynchronously.
+ class ProofVerifierCallbackImpl : public ProofVerifierCallback {
+ 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();
+ }
+
+ // Starts verification of certs cached in the |crypto_config|.
+ QuicAsyncStatus Run(QuicCryptoClientConfig* crypto_config,
+ const CompletionCallback& callback) {
+ QuicCryptoClientConfig::CachedState* cached =
+ crypto_config->LookupOrCreate(server_id_);
+ ProofVerifierCallbackImpl* verify_callback =
+ new ProofVerifierCallbackImpl(this);
+ QuicAsyncStatus status = crypto_config->proof_verifier()->VerifyCertChain(
+ server_id_.host(), cached->certs(), verify_context_.get(),
+ &verify_error_details_, &verify_details_,
+ std::unique_ptr<ProofVerifierCallback>(verify_callback));
+ if (status == QUIC_PENDING) {
+ verify_callback_ = verify_callback;
+ callback_ = 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 {
@@ -364,9 +447,8 @@ void QuicStreamFactory::Job::CancelWaitForDataReadyCallback() {
int QuicStreamFactory::Job::DoResolveHost() {
// Start loading the data now, and wait for it after we resolve the host.
- if (server_info_) {
+ if (server_info_)
server_info_->Start();
- }
io_state_ = STATE_RESOLVE_HOST_COMPLETE;
dns_resolution_start_time_ = base::TimeTicks::Now();
@@ -388,9 +470,8 @@ int QuicStreamFactory::Job::DoResolveHostComplete(int rv) {
// Inform the factory of this resolution, which will set up
// a session alias, if possible.
- if (factory_->OnResolution(key_, address_list_)) {
+ if (factory_->OnResolution(key_, address_list_))
return OK;
- }
if (server_info_)
io_state_ = STATE_LOAD_SERVER_INFO;
@@ -469,14 +550,12 @@ int QuicStreamFactory::Job::DoConnect() {
return rv;
}
- if (!session_->connection()->connected()) {
+ if (!session_->connection()->connected())
return ERR_CONNECTION_CLOSED;
- }
session_->StartReading();
- if (!session_->connection()->connected()) {
+ if (!session_->connection()->connected())
return ERR_QUIC_PROTOCOL_ERROR;
- }
bool require_confirmation = factory_->require_confirmation() ||
was_alternative_service_recently_broken_;
@@ -485,8 +564,9 @@ int QuicStreamFactory::Job::DoConnect() {
base::Bind(&QuicStreamFactory::Job::OnIOComplete, GetWeakPtr()));
if (!session_->connection()->connected() &&
- session_->error() == QUIC_PROOF_INVALID)
+ session_->error() == QUIC_PROOF_INVALID) {
return ERR_QUIC_HANDSHAKE_FAILED;
+ }
return rv;
}
@@ -503,9 +583,8 @@ int QuicStreamFactory::Job::DoResumeConnect() {
int QuicStreamFactory::Job::DoConnectComplete(int rv) {
if (session_ && session_->error() == QUIC_CRYPTO_HANDSHAKE_STATELESS_REJECT) {
num_sent_client_hellos_ += session_->GetNumSentClientHellos();
- if (num_sent_client_hellos_ >= QuicCryptoClientStream::kMaxClientHellos) {
+ if (num_sent_client_hellos_ >= QuicCryptoClientStream::kMaxClientHellos)
return ERR_QUIC_HANDSHAKE_FAILED;
- }
// The handshake was rejected statelessly, so create another connection
// to resume the handshake.
io_state_ = STATE_CONNECT;
@@ -634,6 +713,7 @@ QuicStreamFactory::QuicStreamFactory(
bool migrate_sessions_early,
bool allow_server_migration,
bool force_hol_blocking,
+ bool race_cert_verification,
const QuicTagVector& connection_options,
bool enable_token_binding)
: require_confirmation_(true),
@@ -690,6 +770,7 @@ QuicStreamFactory::QuicStreamFactory(
migrate_sessions_on_network_change_),
allow_server_migration_(allow_server_migration),
force_hol_blocking_(force_hol_blocking),
+ race_cert_verification_(race_cert_verification),
port_seed_(random_generator_->RandUint64()),
check_persisted_supports_quic_(true),
has_initialized_data_(false),
@@ -757,6 +838,8 @@ QuicStreamFactory::~QuicStreamFactory() {
STLDeleteElements(&(active_jobs_[server_id]));
active_jobs_.erase(server_id);
}
+ while (!active_cert_verifier_jobs_.empty())
+ active_cert_verifier_jobs_.erase(active_cert_verifier_jobs_.begin());
if (ssl_config_service_.get())
ssl_config_service_->RemoveObserver(this);
if (migrate_sessions_on_network_change_) {
@@ -888,11 +971,12 @@ int QuicStreamFactory::Create(const QuicServerId& server_id,
// 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)) {
+ if (load_from_disk_cache && CryptoConfigCacheIsEmpty(server_id))
quic_server_info = quic_server_info_factory_->GetForServer(server_id);
- }
}
+ ignore_result(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),
@@ -953,9 +1037,8 @@ bool QuicStreamFactory::OnResolution(const QuicSessionKey& key,
const AddressList& address_list) {
const QuicServerId& server_id(key.server_id());
DCHECK(!HasActiveSession(server_id));
- if (disable_connection_pooling_) {
+ if (disable_connection_pooling_)
return false;
- }
for (const IPEndPoint& address : address_list) {
if (!ContainsKey(ip_aliases_, address))
continue;
@@ -1025,6 +1108,10 @@ void QuicStreamFactory::OnJobComplete(Job* job, int rv) {
job_requests_map_.erase(server_id);
}
+void QuicStreamFactory::OnCertVerifyJobComplete(CertVerifierJob* job, int rv) {
+ active_cert_verifier_jobs_.erase(job->server_id());
+}
+
std::unique_ptr<QuicHttpStream> QuicStreamFactory::CreateFromSession(
QuicChromiumClientSession* session) {
return std::unique_ptr<QuicHttpStream>(
@@ -1140,9 +1227,8 @@ void QuicStreamFactory::OnSessionGoingAway(QuicChromiumClientSession* session) {
DCHECK_EQ(session, active_sessions_[server_id]);
// Track sessions which have recently gone away so that we can disable
// port suggestions.
- if (session->goaway_received()) {
+ if (session->goaway_received())
gone_away_aliases_.insert(*it);
- }
active_sessions_.erase(server_id);
ProcessGoingAwaySession(session, server_id, true);
@@ -1151,9 +1237,8 @@ void QuicStreamFactory::OnSessionGoingAway(QuicChromiumClientSession* session) {
if (!aliases.empty()) {
const IPEndPoint peer_address = session->connection()->peer_address();
ip_aliases_[peer_address].erase(session);
- if (ip_aliases_[peer_address].empty()) {
+ if (ip_aliases_[peer_address].empty())
ip_aliases_.erase(peer_address);
- }
}
session_aliases_.erase(session);
}
@@ -1258,9 +1343,8 @@ void QuicStreamFactory::OnSessionConnectTimeout(
QuicChromiumClientSession* session) {
const AliasSet& aliases = session_aliases_[session];
- if (aliases.empty()) {
+ if (aliases.empty())
return;
- }
for (const QuicSessionKey& key : aliases) {
const QuicServerId& server_id = key.server_id();
@@ -1272,9 +1356,8 @@ void QuicStreamFactory::OnSessionConnectTimeout(
const IPEndPoint peer_address = session->connection()->peer_address();
ip_aliases_[peer_address].erase(session);
- if (ip_aliases_[peer_address].empty()) {
+ if (ip_aliases_[peer_address].empty())
ip_aliases_.erase(peer_address);
- }
QuicSessionKey key = *aliases.begin();
session_aliases_.erase(session);
Job* job = new Job(this, host_resolver_, session, key);
@@ -1370,9 +1453,8 @@ NetworkHandle QuicStreamFactory::FindAlternateNetwork(
NetworkChangeNotifier::NetworkList network_list;
NetworkChangeNotifier::GetConnectedNetworks(&network_list);
for (NetworkHandle new_network : network_list) {
- if (new_network != old_network) {
+ if (new_network != old_network)
return new_network;
- }
}
return NetworkChangeNotifier::kInvalidNetworkHandle;
}
@@ -1580,6 +1662,11 @@ 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) {
@@ -1668,15 +1755,13 @@ int QuicStreamFactory::CreateSession(
// Passing in kInvalidNetworkHandle binds socket to default network.
int rv = ConfigureSocket(socket.get(), addr,
NetworkChangeNotifier::kInvalidNetworkHandle);
- if (rv != OK) {
+ if (rv != OK)
return rv;
- }
- if (enable_port_selection) {
+ if (enable_port_selection)
DCHECK_LE(1u, port_suggester->call_count());
- } else {
+ else
DCHECK_EQ(0u, port_suggester->call_count());
- }
if (!helper_.get()) {
helper_.reset(
@@ -1790,6 +1875,29 @@ bool QuicStreamFactory::CryptoConfigCacheIsEmpty(
return cached->IsEmpty();
}
+QuicAsyncStatus QuicStreamFactory::StartCertVerifyJob(
+ const QuicServerId& server_id,
+ int cert_verify_flags,
+ const BoundNetLog& net_log) {
+ if (!race_cert_verification_)
+ return QUIC_FAILURE;
+ QuicCryptoClientConfig::CachedState* cached =
+ crypto_config_.LookupOrCreate(server_id);
+ if (!cached || cached->certs().empty() ||
+ HasActiveCertVerifierJob(server_id)) {
+ return QUIC_FAILURE;
+ }
+ 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] = std::move(cert_verifier_job);
+ return status;
+}
+
void QuicStreamFactory::InitializeCachedStateInCryptoConfig(
const QuicServerId& server_id,
const std::unique_ptr<QuicServerInfo>& server_info,
« no previous file with comments | « net/quic/chromium/quic_stream_factory.h ('k') | net/quic/chromium/quic_stream_factory_test.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698