Improve QuicCryptoServerStreamBase functions for querying resumption status
QuicCryptoServerStreamBase::ZeroRttAttempted is currently only used in one
place, which is to query whether or not the client attempted to resume the
crypto connection. In QUIC Crypto, the only way to resume a connection is if
it's a 0-RTT connection, but in TLS, a resumption can occur without 0-RTT.
Given this difference, it makes sense to change the semantics (and name) of
ZeroRttAttempted to match its use (and simplify its implementation for the
TLS handshake). This CL also implements the previously unimplemented
TlsServerHandshaker::ResumptionAttempted.
QuicCryptoServerStreamBase is also missing a function to query whether the
connection was actually a resumption, so this CL adds the unused
IsResumption function.
No behavior changes: renames a method, adds new unused method, not flag protected
PiperOrigin-RevId: 314368205
Change-Id: I985fbc278493f6f104db9b509d83103e37bca48f
diff --git a/quic/core/quic_crypto_server_stream.cc b/quic/core/quic_crypto_server_stream.cc
index 66f8229..c7dd2d9 100644
--- a/quic/core/quic_crypto_server_stream.cc
+++ b/quic/core/quic_crypto_server_stream.cc
@@ -298,6 +298,11 @@
num_handshake_messages_with_server_nonces_ == 0;
}
+bool QuicCryptoServerStream::IsResumption() const {
+ // QUIC Crypto doesn't have a non-0-RTT resumption mode.
+ return IsZeroRtt();
+}
+
int QuicCryptoServerStream::NumServerConfigUpdateMessagesSent() const {
return num_server_config_update_messages_sent_;
}
@@ -307,7 +312,7 @@
return previous_cached_network_params_.get();
}
-bool QuicCryptoServerStream::ZeroRttAttempted() const {
+bool QuicCryptoServerStream::ResumptionAttempted() const {
return zero_rtt_attempted_;
}
diff --git a/quic/core/quic_crypto_server_stream.h b/quic/core/quic_crypto_server_stream.h
index 52d4874..3aaf0ef 100644
--- a/quic/core/quic_crypto_server_stream.h
+++ b/quic/core/quic_crypto_server_stream.h
@@ -35,9 +35,10 @@
void SendServerConfigUpdate(
const CachedNetworkParameters* cached_network_params) override;
bool IsZeroRtt() const override;
+ bool IsResumption() const override;
+ bool ResumptionAttempted() const override;
int NumServerConfigUpdateMessagesSent() const override;
const CachedNetworkParameters* PreviousCachedNetworkParams() const override;
- bool ZeroRttAttempted() const override;
void SetPreviousCachedNetworkParams(
CachedNetworkParameters cached_network_params) override;
void OnPacketDecrypted(EncryptionLevel level) override;
diff --git a/quic/core/quic_crypto_server_stream_base.h b/quic/core/quic_crypto_server_stream_base.h
index cdf12a3..540b7a4 100644
--- a/quic/core/quic_crypto_server_stream_base.h
+++ b/quic/core/quic_crypto_server_stream_base.h
@@ -62,8 +62,17 @@
virtual void SendServerConfigUpdate(
const CachedNetworkParameters* cached_network_params) = 0;
+ // Returns true if the connection was a successful 0-RTT resumption.
virtual bool IsZeroRtt() const = 0;
- virtual bool ZeroRttAttempted() const = 0;
+
+ // Returns true if the connection was the result of a resumption handshake,
+ // whether 0-RTT or not.
+ virtual bool IsResumption() const = 0;
+
+ // Returns true if the client attempted a resumption handshake, whether or not
+ // the resumption actually occurred.
+ virtual bool ResumptionAttempted() const = 0;
+
virtual const CachedNetworkParameters* PreviousCachedNetworkParams()
const = 0;
virtual void SetPreviousCachedNetworkParams(
diff --git a/quic/core/quic_crypto_server_stream_test.cc b/quic/core/quic_crypto_server_stream_test.cc
index debfc3b..9ff7dc0 100644
--- a/quic/core/quic_crypto_server_stream_test.cc
+++ b/quic/core/quic_crypto_server_stream_test.cc
@@ -226,7 +226,7 @@
// Do a first handshake in order to prime the client config with the server's
// information.
AdvanceHandshakeWithFakeClient();
- EXPECT_FALSE(server_stream()->ZeroRttAttempted());
+ EXPECT_FALSE(server_stream()->ResumptionAttempted());
// Now do another handshake, hopefully in 0-RTT.
QUIC_LOG(INFO) << "Resetting for 0-RTT handshake attempt";
@@ -247,7 +247,7 @@
client_connection_, client_stream(), server_connection_, server_stream());
EXPECT_EQ(1, client_stream()->num_sent_client_hellos());
- EXPECT_TRUE(server_stream()->ZeroRttAttempted());
+ EXPECT_TRUE(server_stream()->ResumptionAttempted());
}
TEST_F(QuicCryptoServerStreamTest, FailByPolicy) {
diff --git a/quic/core/tls_server_handshaker.cc b/quic/core/tls_server_handshaker.cc
index 555bc66..cdc89d8 100644
--- a/quic/core/tls_server_handshaker.cc
+++ b/quic/core/tls_server_handshaker.cc
@@ -137,6 +137,14 @@
return false;
}
+bool TlsServerHandshaker::IsResumption() const {
+ return SSL_session_reused(ssl());
+}
+
+bool TlsServerHandshaker::ResumptionAttempted() const {
+ return ticket_received_;
+}
+
int TlsServerHandshaker::NumServerConfigUpdateMessagesSent() const {
// SCUP messages aren't supported when using the TLS handshake.
return 0;
@@ -147,11 +155,6 @@
return nullptr;
}
-bool TlsServerHandshaker::ZeroRttAttempted() const {
- // TODO(nharper): Support 0-RTT with TLS 1.3 in QUIC.
- return false;
-}
-
void TlsServerHandshaker::SetPreviousCachedNetworkParams(
CachedNetworkParameters /*cached_network_params*/) {}
@@ -470,6 +473,7 @@
DCHECK(proof_source_->GetTicketCrypter());
if (!ticket_decryption_callback_) {
+ ticket_received_ = true;
ticket_decryption_callback_ = new DecryptCallback(this);
proof_source_->GetTicketCrypter()->Decrypt(
in, std::unique_ptr<DecryptCallback>(ticket_decryption_callback_));
diff --git a/quic/core/tls_server_handshaker.h b/quic/core/tls_server_handshaker.h
index c62dbbb..85c1b86 100644
--- a/quic/core/tls_server_handshaker.h
+++ b/quic/core/tls_server_handshaker.h
@@ -40,9 +40,10 @@
void SendServerConfigUpdate(
const CachedNetworkParameters* cached_network_params) override;
bool IsZeroRtt() const override;
+ bool IsResumption() const override;
+ bool ResumptionAttempted() const override;
int NumServerConfigUpdateMessagesSent() const override;
const CachedNetworkParameters* PreviousCachedNetworkParams() const override;
- bool ZeroRttAttempted() const override;
void SetPreviousCachedNetworkParams(
CachedNetworkParameters cached_network_params) override;
void OnPacketDecrypted(EncryptionLevel level) override;
@@ -181,6 +182,11 @@
// |decrypted_session_ticket_| contains the decrypted session ticket after the
// callback has run but before it is passed to BoringSSL.
std::vector<uint8_t> decrypted_session_ticket_;
+ // |ticket_received_| tracks whether we received a resumption ticket from the
+ // client. It does not matter whether we were able to decrypt said ticket or
+ // if we actually resumed a session with it - the presence of this ticket
+ // indicates that the client attempted a resumption.
+ bool ticket_received_ = false;
std::string hostname_;
std::string cert_verify_sig_;
diff --git a/quic/core/tls_server_handshaker_test.cc b/quic/core/tls_server_handshaker_test.cc
index a71338c..d91df58 100644
--- a/quic/core/tls_server_handshaker_test.cc
+++ b/quic/core/tls_server_handshaker_test.cc
@@ -383,6 +383,8 @@
CompleteCryptoHandshake();
ExpectHandshakeSuccessful();
EXPECT_FALSE(client_stream()->IsResumption());
+ EXPECT_FALSE(server_stream()->IsResumption());
+ EXPECT_FALSE(server_stream()->ResumptionAttempted());
// Now do another handshake
InitializeServer();
@@ -390,6 +392,8 @@
CompleteCryptoHandshake();
ExpectHandshakeSuccessful();
EXPECT_TRUE(client_stream()->IsResumption());
+ EXPECT_TRUE(server_stream()->IsResumption());
+ EXPECT_TRUE(server_stream()->ResumptionAttempted());
}
TEST_F(TlsServerHandshakerTest, ResumptionWithAsyncDecryptCallback) {
@@ -411,6 +415,8 @@
CompleteCryptoHandshake();
ExpectHandshakeSuccessful();
EXPECT_TRUE(client_stream()->IsResumption());
+ EXPECT_TRUE(server_stream()->IsResumption());
+ EXPECT_TRUE(server_stream()->ResumptionAttempted());
}
TEST_F(TlsServerHandshakerTest, ResumptionWithFailingDecryptCallback) {
@@ -426,6 +432,8 @@
CompleteCryptoHandshake();
ExpectHandshakeSuccessful();
EXPECT_FALSE(client_stream()->IsResumption());
+ EXPECT_FALSE(server_stream()->IsResumption());
+ EXPECT_TRUE(server_stream()->ResumptionAttempted());
}
TEST_F(TlsServerHandshakerTest, ResumptionWithFailingAsyncDecryptCallback) {
@@ -448,6 +456,8 @@
CompleteCryptoHandshake();
ExpectHandshakeSuccessful();
EXPECT_FALSE(client_stream()->IsResumption());
+ EXPECT_FALSE(server_stream()->IsResumption());
+ EXPECT_TRUE(server_stream()->ResumptionAttempted());
}
TEST_F(TlsServerHandshakerTest, HandshakeFailsWithFailingProofSource) {
