In TlsServerHandshaker, do not call ProofSourceHandle::SelectCertificate if QUIC connection has disconnected.
Protected by FLAGS_quic_reloadable_flag_quic_tls_no_select_cert_if_disconnected.
PiperOrigin-RevId: 410354467
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h
index af9c02c..176f87c 100644
--- a/quic/core/quic_flags_list.h
+++ b/quic/core/quic_flags_list.h
@@ -65,6 +65,8 @@
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_discard_initial_packet_with_key_dropped, true)
// If true, do not bundle 2nd ACK with connection close if there is an ACK queued.
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_single_ack_in_packet2, false)
+// If true, do not call ProofSourceHandle::SelectCertificate if QUIC connection has disconnected.
+QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_tls_no_select_cert_if_disconnected, true)
// If true, do not count bytes sent/received on the alternative path into the bytes sent/received on the default path.
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_count_bytes_on_alternative_path_seperately, true)
// If true, do not re-arm PTO while sending application data during handshake.
diff --git a/quic/core/tls_server_handshaker.cc b/quic/core/tls_server_handshaker.cc
index d35b923..15eed38 100644
--- a/quic/core/tls_server_handshaker.cc
+++ b/quic/core/tls_server_handshaker.cc
@@ -210,6 +210,10 @@
if (session->connection()->context()->tracer) {
tls_connection_.EnableInfoCallback();
}
+
+ if (no_select_cert_if_disconnected_) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_tls_no_select_cert_if_disconnected, 1, 2);
+ }
}
TlsServerHandshaker::~TlsServerHandshaker() { CancelOutstandingCallbacks(); }
@@ -965,6 +969,13 @@
? std::string(alps_result.alps_buffer.get(), alps_result.alps_length)
: std::string();
+ if (no_select_cert_if_disconnected_ &&
+ !session()->connection()->connected()) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_tls_no_select_cert_if_disconnected, 2, 2);
+ select_cert_status_ = QUIC_FAILURE;
+ return ssl_select_cert_error;
+ }
+
const QuicAsyncStatus status = proof_source_handle_->SelectCertificate(
session()->connection()->self_address().Normalized(),
session()->connection()->peer_address().Normalized(),
diff --git a/quic/core/tls_server_handshaker.h b/quic/core/tls_server_handshaker.h
index 57aa652..01cf19b 100644
--- a/quic/core/tls_server_handshaker.h
+++ b/quic/core/tls_server_handshaker.h
@@ -377,6 +377,8 @@
last_received_cached_network_params_;
bool cert_matched_sni_ = false;
+ const bool no_select_cert_if_disconnected_ =
+ GetQuicReloadableFlag(quic_tls_no_select_cert_if_disconnected);
};
} // namespace quic
diff --git a/quic/core/tls_server_handshaker_test.cc b/quic/core/tls_server_handshaker_test.cc
index 3e707b2..214866c 100644
--- a/quic/core/tls_server_handshaker_test.cc
+++ b/quic/core/tls_server_handshaker_test.cc
@@ -26,6 +26,7 @@
#include "quic/test_tools/failing_proof_source.h"
#include "quic/test_tools/fake_proof_source.h"
#include "quic/test_tools/fake_proof_source_handle.h"
+#include "quic/test_tools/quic_config_peer.h"
#include "quic/test_tools/quic_test_utils.h"
#include "quic/test_tools/simple_session_cache.h"
#include "quic/test_tools/test_certificates.h"
@@ -95,6 +96,10 @@
ON_CALL(*this, MaybeCreateProofSourceHandle())
.WillByDefault(testing::Invoke(
this, &TestTlsServerHandshaker::RealMaybeCreateProofSourceHandle));
+
+ ON_CALL(*this, OverrideQuicConfigDefaults(_))
+ .WillByDefault(testing::Invoke(
+ this, &TestTlsServerHandshaker::RealOverrideQuicConfigDefaults));
}
MOCK_METHOD(std::unique_ptr<ProofSourceHandle>,
@@ -102,6 +107,9 @@
(),
(override));
+ MOCK_METHOD(void, OverrideQuicConfigDefaults, (QuicConfig * config),
+ (override));
+
void SetupProofSourceHandle(
FakeProofSourceHandle::Action select_cert_action,
FakeProofSourceHandle::Action compute_signature_action,
@@ -142,6 +150,10 @@
return TlsServerHandshaker::MaybeCreateProofSourceHandle();
}
+ void RealOverrideQuicConfigDefaults(QuicConfig* config) {
+ return TlsServerHandshaker::OverrideQuicConfigDefaults(config);
+ }
+
// Owned by TlsServerHandshaker.
FakeProofSourceHandle* fake_proof_source_handle_ = nullptr;
ProofSource* proof_source_ = nullptr;
@@ -1023,6 +1035,44 @@
EXPECT_FALSE(server_handshaker_->received_client_cert());
}
+TEST_P(TlsServerHandshakerTest, CloseConnectionBeforeSelectCert) {
+ InitializeServerWithFakeProofSourceHandle();
+ server_handshaker_->SetupProofSourceHandle(
+ /*select_cert_action=*/FakeProofSourceHandle::Action::
+ FAIL_SYNC_DO_NOT_CHECK_CLOSED,
+ /*compute_signature_action=*/FakeProofSourceHandle::Action::
+ FAIL_SYNC_DO_NOT_CHECK_CLOSED);
+
+ EXPECT_CALL(*server_handshaker_, OverrideQuicConfigDefaults(_))
+ .WillOnce(testing::Invoke([](QuicConfig* config) {
+ QuicConfigPeer::SetReceivedMaxUnidirectionalStreams(config,
+ /*max_streams=*/0);
+ }));
+
+ EXPECT_CALL(*server_connection_,
+ CloseConnection(QUIC_ZERO_RTT_RESUMPTION_LIMIT_REDUCED, _, _))
+ .WillOnce(testing::Invoke(
+ [this](QuicErrorCode error, const std::string& details,
+ ConnectionCloseBehavior connection_close_behavior) {
+ server_connection_->ReallyCloseConnection(
+ error, details, connection_close_behavior);
+ ASSERT_FALSE(server_connection_->connected());
+ }));
+
+ AdvanceHandshakeWithFakeClient();
+ if (!GetQuicReloadableFlag(quic_tls_no_select_cert_if_disconnected)) {
+ // SelectCertificate is called when flag is false.
+ EXPECT_FALSE(server_handshaker_->fake_proof_source_handle()
+ ->all_select_cert_args()
+ .empty());
+ return;
+ }
+
+ EXPECT_TRUE(server_handshaker_->fake_proof_source_handle()
+ ->all_select_cert_args()
+ .empty());
+}
+
} // namespace
} // namespace test
} // namespace quic
diff --git a/quic/test_tools/fake_proof_source_handle.cc b/quic/test_tools/fake_proof_source_handle.cc
index 46a1d09..0b2e06f 100644
--- a/quic/test_tools/fake_proof_source_handle.cc
+++ b/quic/test_tools/fake_proof_source_handle.cc
@@ -80,7 +80,9 @@
const std::vector<uint8_t>& quic_transport_params,
const absl::optional<std::vector<uint8_t>>& early_data_context,
const QuicSSLConfig& ssl_config) {
- QUICHE_CHECK(!closed_);
+ if (select_cert_action_ != Action::FAIL_SYNC_DO_NOT_CHECK_CLOSED) {
+ QUICHE_CHECK(!closed_);
+ }
all_select_cert_args_.push_back(SelectCertArgs(
server_address, client_address, ssl_capabilities, hostname, client_hello,
alpn, alps, quic_transport_params, early_data_context, ssl_config));
@@ -90,7 +92,8 @@
select_cert_op_.emplace(delegate_, callback_, select_cert_action_,
all_select_cert_args_.back(), dealyed_ssl_config_);
return QUIC_PENDING;
- } else if (select_cert_action_ == Action::FAIL_SYNC) {
+ } else if (select_cert_action_ == Action::FAIL_SYNC ||
+ select_cert_action_ == Action::FAIL_SYNC_DO_NOT_CHECK_CLOSED) {
callback()->OnSelectCertificateDone(
/*ok=*/false,
/*is_sync=*/true, nullptr, /*handshake_hints=*/absl::string_view(),
@@ -121,7 +124,9 @@
uint16_t signature_algorithm,
absl::string_view in,
size_t max_signature_size) {
- QUICHE_CHECK(!closed_);
+ if (compute_signature_action_ != Action::FAIL_SYNC_DO_NOT_CHECK_CLOSED) {
+ QUICHE_CHECK(!closed_);
+ }
all_compute_signature_args_.push_back(
ComputeSignatureArgs(server_address, client_address, hostname,
signature_algorithm, in, max_signature_size));
@@ -132,7 +137,9 @@
compute_signature_action_,
all_compute_signature_args_.back());
return QUIC_PENDING;
- } else if (compute_signature_action_ == Action::FAIL_SYNC) {
+ } else if (compute_signature_action_ == Action::FAIL_SYNC ||
+ compute_signature_action_ ==
+ Action::FAIL_SYNC_DO_NOT_CHECK_CLOSED) {
callback()->OnComputeSignatureDone(/*ok=*/false, /*is_sync=*/true,
/*signature=*/"", /*details=*/nullptr);
return QUIC_FAILURE;
diff --git a/quic/test_tools/fake_proof_source_handle.h b/quic/test_tools/fake_proof_source_handle.h
index b4203e8..bb2c60e 100644
--- a/quic/test_tools/fake_proof_source_handle.h
+++ b/quic/test_tools/fake_proof_source_handle.h
@@ -25,6 +25,8 @@
// Handle the operation asynchronously. Fail the operation when the caller
// calls CompletePendingOperation().
FAIL_ASYNC,
+ // Similar to FAIL_SYNC, but do not QUICHE_CHECK(!closed_) when invoked.
+ FAIL_SYNC_DO_NOT_CHECK_CLOSED,
};
// |delegate| must do cert selection and signature synchronously.
