Do not reuse tokens received in NEW_TOKEN frames for different connection attempts by:
1) add source address token to QuicClientSessionCache,
2) Clear token after use.
Protected by FLAGS_quic_reloadable_flag_quic_tls_use_token_in_session_cache.
PiperOrigin-RevId: 411536820
diff --git a/quic/core/crypto/quic_client_session_cache.cc b/quic/core/crypto/quic_client_session_cache.cc
index 64c3ffd..97f7ee5 100644
--- a/quic/core/crypto/quic_client_session_cache.cc
+++ b/quic/core/crypto/quic_client_session_cache.cc
@@ -87,6 +87,12 @@
state->application_state =
std::make_unique<ApplicationState>(*iter->second->application_state);
}
+ if (GetQuicReloadableFlag(quic_tls_use_token_in_session_cache) &&
+ !iter->second->token.empty()) {
+ state->token = iter->second->token;
+ // Clear token after use.
+ iter->second->token.clear();
+ }
return state;
}
@@ -102,6 +108,18 @@
}
}
+void QuicClientSessionCache::OnNewTokenReceived(const QuicServerId& server_id,
+ absl::string_view token) {
+ if (token.empty()) {
+ return;
+ }
+ auto iter = cache_.Lookup(server_id);
+ if (iter == cache_.end()) {
+ return;
+ }
+ iter->second->token = token;
+}
+
void QuicClientSessionCache::RemoveExpiredEntries(QuicWallTime now) {
auto iter = cache_.begin();
while (iter != cache_.end()) {
diff --git a/quic/core/crypto/quic_client_session_cache.h b/quic/core/crypto/quic_client_session_cache.h
index 667594f..38678af 100644
--- a/quic/core/crypto/quic_client_session_cache.h
+++ b/quic/core/crypto/quic_client_session_cache.h
@@ -13,6 +13,10 @@
namespace quic {
+namespace test {
+class QuicClientSessionCachePeer;
+} // namespace test
+
// QuicClientSessionCache maps from QuicServerId to information used to resume
// TLS sessions for that server.
class QUIC_EXPORT_PRIVATE QuicClientSessionCache : public SessionCache {
@@ -32,6 +36,9 @@
void ClearEarlyData(const QuicServerId& server_id) override;
+ void OnNewTokenReceived(const QuicServerId& server_id,
+ absl::string_view token) override;
+
void RemoveExpiredEntries(QuicWallTime now) override;
void Clear() override;
@@ -39,6 +46,8 @@
size_t size() const { return cache_.Size(); }
private:
+ friend class test::QuicClientSessionCachePeer;
+
struct QUIC_EXPORT_PRIVATE Entry {
Entry();
Entry(Entry&&);
@@ -56,6 +65,7 @@
bssl::UniquePtr<SSL_SESSION> sessions[2];
std::unique_ptr<TransportParameters> params;
std::unique_ptr<ApplicationState> application_state;
+ std::string token; // An opaque string received in NEW_TOKEN frame.
};
// Creates a new entry and insert into |cache_|.
diff --git a/quic/core/crypto/quic_crypto_client_config.h b/quic/core/crypto/quic_crypto_client_config.h
index 694e068..1351729 100644
--- a/quic/core/crypto/quic_crypto_client_config.h
+++ b/quic/core/crypto/quic_crypto_client_config.h
@@ -49,6 +49,9 @@
// client received from the server at the application layer that the client
// needs to remember when performing a 0-RTT handshake.
std::unique_ptr<ApplicationState> application_state = nullptr;
+
+ // Opaque token received in NEW_TOKEN frame if any.
+ std::string token;
};
// SessionCache is an interface for managing storing and retrieving
@@ -80,6 +83,10 @@
// associated with |server_id|.
virtual void ClearEarlyData(const QuicServerId& server_id) = 0;
+ // Called when NEW_TOKEN frame is received.
+ virtual void OnNewTokenReceived(const QuicServerId& server_id,
+ absl::string_view token) = 0;
+
// Called to remove expired entries.
virtual void RemoveExpiredEntries(QuicWallTime now) = 0;
diff --git a/quic/core/http/end_to_end_test.cc b/quic/core/http/end_to_end_test.cc
index ee891dc..5d2d884 100644
--- a/quic/core/http/end_to_end_test.cc
+++ b/quic/core/http/end_to_end_test.cc
@@ -47,6 +47,7 @@
#include "quic/test_tools/qpack/qpack_encoder_test_utils.h"
#include "quic/test_tools/qpack/qpack_test_utils.h"
#include "quic/test_tools/quic_client_peer.h"
+#include "quic/test_tools/quic_client_session_cache_peer.h"
#include "quic/test_tools/quic_config_peer.h"
#include "quic/test_tools/quic_connection_peer.h"
#include "quic/test_tools/quic_dispatcher_peer.h"
@@ -1702,6 +1703,61 @@
server_thread_->Resume();
client_->Disconnect();
+
+ // Regression test for b/206087883.
+ // Mock server crash.
+ StopServer();
+
+ // The handshake fails due to idle timeout.
+ client_->Connect();
+ ASSERT_FALSE(client_->client()->WaitForOneRttKeysAvailable());
+ client_->WaitForWriteToFlush();
+ client_->WaitForResponse();
+ ASSERT_FALSE(client_->client()->connected());
+ EXPECT_THAT(client_->connection_error(), IsError(QUIC_NETWORK_IDLE_TIMEOUT));
+
+ // Server restarts.
+ server_writer_ = new PacketDroppingTestWriter();
+ StartServer();
+
+ // Client re-connect.
+ client_->Connect();
+ ASSERT_TRUE(client_->client()->WaitForOneRttKeysAvailable());
+ client_->WaitForWriteToFlush();
+ client_->WaitForResponse();
+ ASSERT_TRUE(client_->client()->connected());
+ client_session = GetClientSession();
+ ASSERT_TRUE(client_session);
+ EXPECT_FALSE(client_session->EarlyDataAccepted());
+ EXPECT_FALSE(client_->client()->EarlyDataAccepted());
+ server_thread_->Pause();
+ server_session = GetServerSession();
+ server_connection = GetServerConnection();
+ if (!GetQuicReloadableFlag(quic_tls_use_token_in_session_cache)) {
+ // Address token is reused.
+ if (server_session != nullptr && server_connection != nullptr) {
+ // Verify address is validated via validating token received in INITIAL
+ // packet.
+ EXPECT_FALSE(server_connection->GetStats()
+ .address_validated_via_decrypting_packet);
+ EXPECT_TRUE(server_connection->GetStats().address_validated_via_token);
+ } else {
+ ADD_FAILURE() << "Missing server connection";
+ }
+ } else {
+ // Verify address token is only used once.
+ if (server_session != nullptr && server_connection != nullptr) {
+ // Verify address is validated via decrypting packet.
+ EXPECT_TRUE(server_connection->GetStats()
+ .address_validated_via_decrypting_packet);
+ EXPECT_FALSE(server_connection->GetStats().address_validated_via_token);
+ } else {
+ ADD_FAILURE() << "Missing server connection";
+ }
+ }
+ server_thread_->Resume();
+
+ client_->Disconnect();
}
TEST_P(EndToEndTest, AddressTokenRefreshedByServer) {
@@ -1721,8 +1777,16 @@
client_->Disconnect();
- std::string old_address_token =
- client_crypto_config->LookupOrCreate(server_id)->source_address_token();
+ QuicClientSessionCache* session_cache = static_cast<QuicClientSessionCache*>(
+ client_crypto_config->mutable_session_cache());
+ std::string old_address_token;
+ if (GetQuicReloadableFlag(quic_tls_use_token_in_session_cache)) {
+ old_address_token =
+ QuicClientSessionCachePeer::GetToken(session_cache, server_id);
+ } else {
+ old_address_token =
+ client_crypto_config->LookupOrCreate(server_id)->source_address_token();
+ }
ASSERT_TRUE(!old_address_token.empty());
SetQuicReloadableFlag(quic_add_cached_network_parameters_to_address_token,
@@ -1750,8 +1814,14 @@
client_->Disconnect();
- std::string new_address_token =
- client_crypto_config->LookupOrCreate(server_id)->source_address_token();
+ std::string new_address_token;
+ if (GetQuicReloadableFlag(quic_tls_use_token_in_session_cache)) {
+ new_address_token =
+ QuicClientSessionCachePeer::GetToken(session_cache, server_id);
+ } else {
+ new_address_token =
+ client_crypto_config->LookupOrCreate(server_id)->source_address_token();
+ }
ASSERT_TRUE(!new_address_token.empty());
ASSERT_NE(new_address_token, old_address_token);
}
@@ -1772,8 +1842,16 @@
client_->Disconnect();
- std::string old_address_token =
- client_crypto_config->LookupOrCreate(server_id)->source_address_token();
+ QuicClientSessionCache* session_cache = static_cast<QuicClientSessionCache*>(
+ client_crypto_config->mutable_session_cache());
+ std::string old_address_token;
+ if (GetQuicReloadableFlag(quic_tls_use_token_in_session_cache)) {
+ old_address_token =
+ QuicClientSessionCachePeer::GetToken(session_cache, server_id);
+ } else {
+ old_address_token =
+ client_crypto_config->LookupOrCreate(server_id)->source_address_token();
+ }
ASSERT_TRUE(!old_address_token.empty());
// Pause the server thread again to blackhole packets from client.
@@ -1782,10 +1860,17 @@
EXPECT_FALSE(client_->client()->WaitForOneRttKeysAvailable());
EXPECT_FALSE(client_->client()->connected());
- std::string new_address_token =
- client_crypto_config->LookupOrCreate(server_id)->source_address_token();
- // TODO(b/206087883): This currently fails, fix the client and uncomment it.
- // ASSERT_TRUE(new_address_token.empty());
+ std::string new_address_token;
+ if (GetQuicReloadableFlag(quic_tls_use_token_in_session_cache)) {
+ new_address_token =
+ QuicClientSessionCachePeer::GetToken(session_cache, server_id);
+ // Verify address token gets cleared.
+ ASSERT_TRUE(new_address_token.empty());
+ } else {
+ new_address_token =
+ client_crypto_config->LookupOrCreate(server_id)->source_address_token();
+ ASSERT_FALSE(new_address_token.empty());
+ }
server_thread_->Resume();
}
diff --git a/quic/core/quic_dispatcher_test.cc b/quic/core/quic_dispatcher_test.cc
index d4af035..199270e 100644
--- a/quic/core/quic_dispatcher_test.cc
+++ b/quic/core/quic_dispatcher_test.cc
@@ -454,7 +454,8 @@
ParsedClientHello parsed_chlo;
parsed_chlo.alpns = {ExpectedAlpn()};
parsed_chlo.sni = TestHostname();
- if (address_token_.has_value()) {
+ if (address_token_.has_value() &&
+ !GetQuicReloadableFlag(quic_tls_use_token_in_session_cache)) {
parsed_chlo.retry_token = *address_token_;
}
return parsed_chlo;
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h
index 4c3a5e8..518b7c3 100644
--- a/quic/core/quic_flags_list.h
+++ b/quic/core/quic_flags_list.h
@@ -115,6 +115,8 @@
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_conservative_cwnd_and_pacing_gains, false)
// If true, validate that peer owns the new address once the server detects peer migration or is probed from that address, and also apply anti-amplification limit while sending to that address.
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_server_reverse_validate_new_path3, true)
+// If true, when client attempts TLS resumption, use token in session_cache_ instead of cached_states_ in QuicCryptoClientConfig.
+QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_tls_use_token_in_session_cache, true)
// When receiving STOP_SENDING, send a RESET_STREAM with a matching error code.
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_match_ietf_reset_code, true)
// When the flag is true, exit STARTUP after the same number of loss events as PROBE_UP.
diff --git a/quic/core/tls_client_handshaker.cc b/quic/core/tls_client_handshaker.cc
index 211dfde..c8306a8 100644
--- a/quic/core/tls_client_handshaker.cc
+++ b/quic/core/tls_client_handshaker.cc
@@ -42,10 +42,12 @@
has_application_state_(has_application_state),
crypto_config_(crypto_config),
tls_connection_(crypto_config->ssl_ctx(), this, session->GetSSLConfig()) {
- std::string token =
- crypto_config->LookupOrCreate(server_id)->source_address_token();
- if (!token.empty()) {
- session->SetSourceAddressTokenToSend(token);
+ if (!GetQuicReloadableFlag(quic_tls_use_token_in_session_cache)) {
+ std::string token =
+ crypto_config->LookupOrCreate(server_id)->source_address_token();
+ if (!token.empty()) {
+ session->SetSourceAddressTokenToSend(token);
+ }
}
if (crypto_config->tls_signature_algorithms().has_value()) {
SSL_set1_sigalgs_list(ssl(),
@@ -125,6 +127,10 @@
}
if (cached_state_) {
SSL_set_session(ssl(), cached_state_->tls_session.get());
+ if (GetQuicReloadableFlag(quic_tls_use_token_in_session_cache) &&
+ !cached_state_->token.empty()) {
+ session()->SetSourceAddressTokenToSend(cached_state_->token);
+ }
}
// Start the handshake.
@@ -434,9 +440,15 @@
if (token.empty()) {
return;
}
- QuicCryptoClientConfig::CachedState* cached =
- crypto_config_->LookupOrCreate(server_id_);
- cached->set_source_address_token(token);
+ if (GetQuicReloadableFlag(quic_tls_use_token_in_session_cache)) {
+ if (session_cache_ != nullptr) {
+ session_cache_->OnNewTokenReceived(server_id_, token);
+ }
+ } else {
+ QuicCryptoClientConfig::CachedState* cached =
+ crypto_config_->LookupOrCreate(server_id_);
+ cached->set_source_address_token(token);
+ }
}
void TlsClientHandshaker::SetWriteSecret(
diff --git a/quic/test_tools/quic_client_session_cache_peer.h b/quic/test_tools/quic_client_session_cache_peer.h
new file mode 100644
index 0000000..6f0c667
--- /dev/null
+++ b/quic/test_tools/quic_client_session_cache_peer.h
@@ -0,0 +1,28 @@
+// Copyright (c) 2021 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.
+
+#ifndef QUICHE_QUIC_TEST_TOOLS_QUIC_CLIENT_SESSION_CACHE_PEER_H_
+#define QUICHE_QUIC_TEST_TOOLS_QUIC_CLIENT_SESSION_CACHE_PEER_H_
+
+#include "quic/core/crypto/quic_client_session_cache.h"
+
+namespace quic {
+namespace test {
+
+class QuicClientSessionCachePeer {
+ public:
+ static std::string GetToken(QuicClientSessionCache* cache,
+ const QuicServerId& server_id) {
+ auto iter = cache->cache_.Lookup(server_id);
+ if (iter == cache->cache_.end()) {
+ return {};
+ }
+ return iter->second->token;
+ }
+};
+
+} // namespace test
+} // namespace quic
+
+#endif // QUICHE_QUIC_TEST_TOOLS_QUIC_CLIENT_SESSION_CACHE_PEER_H_
diff --git a/quic/test_tools/simple_session_cache.cc b/quic/test_tools/simple_session_cache.cc
index d2f5cf8..2d23673 100644
--- a/quic/test_tools/simple_session_cache.cc
+++ b/quic/test_tools/simple_session_cache.cc
@@ -48,6 +48,7 @@
}
state->transport_params =
std::make_unique<TransportParameters>(*it->second.params);
+ state->token = it->second.token;
return state;
}
@@ -56,6 +57,15 @@
// do anything here.
}
+void SimpleSessionCache::OnNewTokenReceived(const QuicServerId& server_id,
+ absl::string_view token) {
+ auto it = cache_entries_.find(server_id);
+ if (it == cache_entries_.end()) {
+ return;
+ }
+ it->second.token = token;
+}
+
void SimpleSessionCache::RemoveExpiredEntries(QuicWallTime /*now*/) {
// The simple session cache does not support removing expired entries.
}
diff --git a/quic/test_tools/simple_session_cache.h b/quic/test_tools/simple_session_cache.h
index 54bb0c7..6b4111e 100644
--- a/quic/test_tools/simple_session_cache.h
+++ b/quic/test_tools/simple_session_cache.h
@@ -31,6 +31,8 @@
QuicWallTime now,
const SSL_CTX* ctx) override;
void ClearEarlyData(const QuicServerId& server_id) override;
+ void OnNewTokenReceived(const QuicServerId& server_id,
+ absl::string_view token) override;
void RemoveExpiredEntries(QuicWallTime now) override;
void Clear() override;
@@ -39,6 +41,7 @@
bssl::UniquePtr<SSL_SESSION> session;
std::unique_ptr<TransportParameters> params;
std::unique_ptr<ApplicationState> application_state;
+ std::string token;
};
std::map<QuicServerId, Entry> cache_entries_;
};
