In TlsClientHandshaker, only insert data into cache if all required states are present.
ApplicationState is now optional to TlsClientHandshaker because not all applications have it.
gfe-relnote: unused code. not protected.
PiperOrigin-RevId: 307719344
Change-Id: Ib4e359c6a13c9b7b805dca7a1cfd3b1bf89af6ef
diff --git a/quic/core/http/quic_spdy_client_session.cc b/quic/core/http/quic_spdy_client_session.cc
index 07b9c7a..65751b9 100644
--- a/quic/core/http/quic_spdy_client_session.cc
+++ b/quic/core/http/quic_spdy_client_session.cc
@@ -191,7 +191,7 @@
return std::make_unique<QuicCryptoClientStream>(
server_id_, this,
crypto_config_->proof_verifier()->CreateDefaultContext(), crypto_config_,
- this);
+ this, /*has_application_state = */ true);
}
bool QuicSpdyClientSession::IsAuthorized(const std::string& /*authority*/) {
diff --git a/quic/core/http/quic_spdy_client_session_test.cc b/quic/core/http/quic_spdy_client_session_test.cc
index dc72a73..eea081c 100644
--- a/quic/core/http/quic_spdy_client_session_test.cc
+++ b/quic/core/http/quic_spdy_client_session_test.cc
@@ -173,6 +173,9 @@
}
std::unique_ptr<QuicCryptoServerConfig> crypto_config =
crypto_test_utils::CryptoServerConfigForTesting();
+ if (connection_->version().handshake_protocol == PROTOCOL_TLS1_3) {
+ SSL_CTX_clear_options(crypto_config->ssl_ctx(), SSL_OP_NO_TICKET);
+ }
crypto_test_utils::HandshakeWithFakeServer(
&config, crypto_config.get(), &helper_, &alarm_factory_, connection_,
stream, AlpnForVersion(connection_->version()));
diff --git a/quic/core/quic_crypto_client_handshaker_test.cc b/quic/core/quic_crypto_client_handshaker_test.cc
index 92de76f..7e941c2 100644
--- a/quic/core/quic_crypto_client_handshaker_test.cc
+++ b/quic/core/quic_crypto_client_handshaker_test.cc
@@ -136,11 +136,13 @@
{version_})),
session_(connection_, false),
crypto_client_config_(std::make_unique<InsecureProofVerifier>()),
- client_stream_(new QuicCryptoClientStream(server_id_,
- &session_,
- nullptr,
- &crypto_client_config_,
- &proof_handler_)),
+ client_stream_(
+ new QuicCryptoClientStream(server_id_,
+ &session_,
+ nullptr,
+ &crypto_client_config_,
+ &proof_handler_,
+ /*has_application_state = */ false)),
handshaker_(server_id_,
client_stream_,
&session_,
diff --git a/quic/core/quic_crypto_client_stream.cc b/quic/core/quic_crypto_client_stream.cc
index 538d828..6f2ffcf 100644
--- a/quic/core/quic_crypto_client_stream.cc
+++ b/quic/core/quic_crypto_client_stream.cc
@@ -32,7 +32,8 @@
QuicSession* session,
std::unique_ptr<ProofVerifyContext> verify_context,
QuicCryptoClientConfig* crypto_config,
- ProofHandler* proof_handler)
+ ProofHandler* proof_handler,
+ bool has_application_state)
: QuicCryptoClientStreamBase(session) {
DCHECK_EQ(Perspective::IS_CLIENT, session->connection()->perspective());
switch (session->connection()->version().handshake_protocol) {
@@ -44,7 +45,7 @@
case PROTOCOL_TLS1_3:
handshaker_ = std::make_unique<TlsClientHandshaker>(
server_id, this, session, std::move(verify_context), crypto_config,
- proof_handler);
+ proof_handler, has_application_state);
break;
case PROTOCOL_UNSUPPORTED:
QUIC_BUG << "Attempting to create QuicCryptoClientStream for unknown "
diff --git a/quic/core/quic_crypto_client_stream.h b/quic/core/quic_crypto_client_stream.h
index 5ec1b01..d095deb 100644
--- a/quic/core/quic_crypto_client_stream.h
+++ b/quic/core/quic_crypto_client_stream.h
@@ -187,7 +187,8 @@
QuicSession* session,
std::unique_ptr<ProofVerifyContext> verify_context,
QuicCryptoClientConfig* crypto_config,
- ProofHandler* proof_handler);
+ ProofHandler* proof_handler,
+ bool has_application_state);
QuicCryptoClientStream(const QuicCryptoClientStream&) = delete;
QuicCryptoClientStream& operator=(const QuicCryptoClientStream&) = delete;
diff --git a/quic/core/tls_client_handshaker.cc b/quic/core/tls_client_handshaker.cc
index 4b6c2af..f16fdd5 100644
--- a/quic/core/tls_client_handshaker.cc
+++ b/quic/core/tls_client_handshaker.cc
@@ -52,7 +52,8 @@
QuicSession* session,
std::unique_ptr<ProofVerifyContext> verify_context,
QuicCryptoClientConfig* crypto_config,
- QuicCryptoClientStream::ProofHandler* proof_handler)
+ QuicCryptoClientStream::ProofHandler* proof_handler,
+ bool has_application_state)
: TlsHandshaker(stream, session),
session_(session),
server_id_(server_id),
@@ -63,6 +64,7 @@
user_agent_id_(crypto_config->user_agent_id()),
pre_shared_key_(crypto_config->pre_shared_key()),
crypto_negotiated_params_(new QuicCryptoNegotiatedParameters),
+ has_application_state_(has_application_state),
tls_connection_(crypto_config->ssl_ctx(), this) {}
TlsClientHandshaker::~TlsClientHandshaker() {
@@ -501,11 +503,25 @@
}
void TlsClientHandshaker::InsertSession(bssl::UniquePtr<SSL_SESSION> session) {
+ if (!received_transport_params_) {
+ QUIC_BUG << "Transport parameters isn't received";
+ return;
+ }
if (session_cache_ == nullptr) {
QUIC_DVLOG(1) << "No session cache, not inserting a session";
return;
}
- session_cache_->Insert(server_id_, std::move(session), nullptr, nullptr);
+ if (has_application_state_ && !received_application_state_) {
+ // Application state is not received yet. cache the sessions.
+ if (cached_tls_sessions_[0] != nullptr) {
+ cached_tls_sessions_[1] = std::move(cached_tls_sessions_[0]);
+ }
+ cached_tls_sessions_[0] = std::move(session);
+ return;
+ }
+ session_cache_->Insert(server_id_, std::move(session),
+ received_transport_params_.get(),
+ received_application_state_.get());
}
void TlsClientHandshaker::WriteMessage(EncryptionLevel level,
@@ -522,9 +538,17 @@
void TlsClientHandshaker::OnApplicationState(
std::unique_ptr<ApplicationState> application_state) {
received_application_state_ = std::move(application_state);
- if (session_cache_ != nullptr) {
- // TODO(renjietang): cache the TLS session ticket and insert them together.
- session_cache_->Insert(server_id_, nullptr, nullptr,
+ // At least one tls session is cached before application state is received. So
+ // insert now.
+ if (session_cache_ != nullptr && cached_tls_sessions_[0] != nullptr) {
+ if (cached_tls_sessions_[1] != nullptr) {
+ // Insert the older session first.
+ session_cache_->Insert(server_id_, std::move(cached_tls_sessions_[1]),
+ received_transport_params_.get(),
+ received_application_state_.get());
+ }
+ session_cache_->Insert(server_id_, std::move(cached_tls_sessions_[0]),
+ received_transport_params_.get(),
received_application_state_.get());
}
}
diff --git a/quic/core/tls_client_handshaker.h b/quic/core/tls_client_handshaker.h
index 0128c8b..7acec4f 100644
--- a/quic/core/tls_client_handshaker.h
+++ b/quic/core/tls_client_handshaker.h
@@ -34,7 +34,8 @@
QuicSession* session,
std::unique_ptr<ProofVerifyContext> verify_context,
QuicCryptoClientConfig* crypto_config,
- QuicCryptoClientStream::ProofHandler* proof_handler);
+ QuicCryptoClientStream::ProofHandler* proof_handler,
+ bool has_application_state);
TlsClientHandshaker(const TlsClientHandshaker&) = delete;
TlsClientHandshaker& operator=(const TlsClientHandshaker&) = delete;
@@ -165,8 +166,14 @@
bool allow_empty_alpn_for_tests_ = false;
+ const bool has_application_state_;
+
TlsClientConnection tls_connection_;
+ // If |has_application_state_|, stores the tls session tickets before
+ // application state is received. The latest one is put in the front.
+ bssl::UniquePtr<SSL_SESSION> cached_tls_sessions_[2] = {};
+
std::unique_ptr<TransportParameters> received_transport_params_ = nullptr;
std::unique_ptr<ApplicationState> received_application_state_ = nullptr;
};
diff --git a/quic/core/tls_handshaker_test.cc b/quic/core/tls_handshaker_test.cc
index 6103743..f774e28 100644
--- a/quic/core/tls_handshaker_test.cc
+++ b/quic/core/tls_handshaker_test.cc
@@ -250,7 +250,8 @@
session,
crypto_test_utils::ProofVerifyContextForTesting(),
&crypto_config_,
- &proof_handler_)) {}
+ &proof_handler_,
+ /*has_application_state = */ false)) {}
~TestQuicCryptoClientStream() override = default;
