Deprecate --gfe2_reloadable_flag_quic_tls_use_per_handshaker_proof_source.
PiperOrigin-RevId: 369310089
Change-Id: Ia507325a32b18a8b686f0124b9bbc26a6e1ce37e
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h
index 6d84230..287f176 100644
--- a/quic/core/quic_flags_list.h
+++ b/quic/core/quic_flags_list.h
@@ -69,7 +69,6 @@
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_testonly_default_false, false)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_testonly_default_true, true)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_tls_use_normalized_sni_for_cert_selectioon, true)
-QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_tls_use_per_handshaker_proof_source, true)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_unified_iw_options, false)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_use_connection_id_on_default_path, false)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_use_encryption_level_context, true)
diff --git a/quic/core/tls_server_handshaker.cc b/quic/core/tls_server_handshaker.cc
index fd9cc55..3336b7b 100644
--- a/quic/core/tls_server_handshaker.cc
+++ b/quic/core/tls_server_handshaker.cc
@@ -47,8 +47,6 @@
QUIC_DVLOG(1) << "CancelPendingOperation. is_signature_pending="
<< (signature_callback_ != nullptr);
if (signature_callback_) {
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_tls_use_per_handshaker_proof_source, 3,
- 3);
signature_callback_->Cancel();
signature_callback_ = nullptr;
}
@@ -119,35 +117,6 @@
return success ? QUIC_SUCCESS : QUIC_FAILURE;
}
-TlsServerHandshaker::SignatureCallback::SignatureCallback(
- TlsServerHandshaker* handshaker)
- : handshaker_(handshaker) {
- QUICHE_DCHECK(!handshaker_->use_proof_source_handle_);
-}
-
-void TlsServerHandshaker::SignatureCallback::Run(
- bool ok,
- std::string signature,
- std::unique_ptr<ProofSource::Details> details) {
- if (handshaker_ == nullptr) {
- return;
- }
- if (ok) {
- handshaker_->cert_verify_sig_ = std::move(signature);
- handshaker_->proof_source_details_ = std::move(details);
- }
- int last_expected_ssl_error = handshaker_->expected_ssl_error();
- handshaker_->set_expected_ssl_error(SSL_ERROR_WANT_READ);
- handshaker_->signature_callback_ = nullptr;
- if (last_expected_ssl_error == SSL_ERROR_WANT_PRIVATE_KEY_OPERATION) {
- handshaker_->AdvanceHandshakeFromCallback();
- }
-}
-
-void TlsServerHandshaker::SignatureCallback::Cancel() {
- handshaker_ = nullptr;
-}
-
TlsServerHandshaker::DecryptCallback::DecryptCallback(
TlsServerHandshaker* handshaker)
: handshaker_(handshaker) {}
@@ -217,13 +186,9 @@
}
void TlsServerHandshaker::CancelOutstandingCallbacks() {
- if (use_proof_source_handle_ && proof_source_handle_) {
+ if (proof_source_handle_) {
proof_source_handle_->CancelPendingOperation();
}
- if (signature_callback_) {
- signature_callback_->Cancel();
- signature_callback_ = nullptr;
- }
if (ticket_decryption_callback_) {
ticket_decryption_callback_->Cancel();
ticket_decryption_callback_ = nullptr;
@@ -232,7 +197,6 @@
std::unique_ptr<ProofSourceHandle>
TlsServerHandshaker::MaybeCreateProofSourceHandle() {
- QUICHE_DCHECK(use_proof_source_handle_);
return std::make_unique<DefaultProofSourceHandle>(this, proof_source_);
}
@@ -610,33 +574,19 @@
uint16_t sig_alg,
absl::string_view in) {
QUICHE_DCHECK_EQ(expected_ssl_error(), SSL_ERROR_WANT_READ);
- if (use_proof_source_handle_) {
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_tls_use_per_handshaker_proof_source, 2,
- 3);
- QuicAsyncStatus status = proof_source_handle_->ComputeSignature(
- session()->connection()->self_address(),
- session()->connection()->peer_address(), cert_selection_hostname(),
- sig_alg, in, max_out);
- if (status == QUIC_PENDING) {
- set_expected_ssl_error(SSL_ERROR_WANT_PRIVATE_KEY_OPERATION);
- if (async_op_timer_.has_value()) {
- QUIC_CODE_COUNT(
- quic_tls_server_computing_signature_while_another_op_pending);
- }
- async_op_timer_ = QuicTimeAccumulator();
- async_op_timer_->Start(now());
- }
- return PrivateKeyComplete(out, out_len, max_out);
- }
- signature_callback_ = new SignatureCallback(this);
- proof_source_->ComputeTlsSignature(
+ QuicAsyncStatus status = proof_source_handle_->ComputeSignature(
session()->connection()->self_address(),
session()->connection()->peer_address(), cert_selection_hostname(),
- sig_alg, in, std::unique_ptr<SignatureCallback>(signature_callback_));
- if (signature_callback_) {
+ sig_alg, in, max_out);
+ if (status == QUIC_PENDING) {
set_expected_ssl_error(SSL_ERROR_WANT_PRIVATE_KEY_OPERATION);
- return ssl_private_key_retry;
+ if (async_op_timer_.has_value()) {
+ QUIC_CODE_COUNT(
+ quic_tls_server_computing_signature_while_another_op_pending);
+ }
+ async_op_timer_ = QuicTimeAccumulator();
+ async_op_timer_->Start(now());
}
return PrivateKeyComplete(out, out_len, max_out);
}
@@ -682,7 +632,6 @@
QUIC_DVLOG(1) << "OnComputeSignatureDone. ok:" << ok
<< ", is_sync:" << is_sync
<< ", len(signature):" << signature.size();
- QUICHE_DCHECK(use_proof_source_handle_);
if (ok) {
cert_verify_sig_ = std::move(signature);
proof_source_details_ = std::move(details);
@@ -804,24 +753,21 @@
// EarlySelectCertCallback can be called twice from BoringSSL: If the first
// call returns ssl_select_cert_retry, when cert selection completes,
// SSL_do_handshake will call it again.
- if (use_proof_source_handle_) {
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_tls_use_per_handshaker_proof_source, 1,
- 3);
- if (select_cert_status_.has_value()) {
- // This is the second call, return the result directly.
- QUIC_DVLOG(1) << "EarlySelectCertCallback called to continue handshake, "
- "returning directly. success:"
- << (select_cert_status_.value() == QUIC_SUCCESS);
- return (select_cert_status_.value() == QUIC_SUCCESS)
- ? ssl_select_cert_success
- : ssl_select_cert_error;
- }
- // This is the first call.
- select_cert_status_ = QUIC_PENDING;
- proof_source_handle_ = MaybeCreateProofSourceHandle();
+ if (select_cert_status_.has_value()) {
+ // This is the second call, return the result directly.
+ QUIC_DVLOG(1) << "EarlySelectCertCallback called to continue handshake, "
+ "returning directly. success:"
+ << (select_cert_status_.value() == QUIC_SUCCESS);
+ return (select_cert_status_.value() == QUIC_SUCCESS)
+ ? ssl_select_cert_success
+ : ssl_select_cert_error;
}
+ // This is the first call.
+ select_cert_status_ = QUIC_PENDING;
+ proof_source_handle_ = MaybeCreateProofSourceHandle();
+
if (!pre_shared_key_.empty()) {
// TODO(b/154162689) add PSK support to QUIC+TLS.
QUIC_BUG(quic_bug_10341_6)
@@ -852,64 +798,6 @@
QUIC_LOG(INFO) << "No hostname indicated in SNI";
}
- if (use_proof_source_handle_) {
- std::string error_details;
- if (!ProcessTransportParameters(client_hello, &error_details)) {
- CloseConnection(QUIC_HANDSHAKE_FAILED, error_details);
- return ssl_select_cert_error;
- }
- OverrideQuicConfigDefaults(session()->config());
- session()->OnConfigNegotiated();
-
- auto set_transport_params_result = SetTransportParameters();
- if (!set_transport_params_result.success) {
- QUIC_LOG(ERROR) << "Failed to set transport parameters";
- return ssl_select_cert_error;
- }
-
- const QuicAsyncStatus status = proof_source_handle_->SelectCertificate(
- session()->connection()->self_address(),
- session()->connection()->peer_address(), cert_selection_hostname(),
- absl::string_view(
- reinterpret_cast<const char*>(client_hello->client_hello),
- client_hello->client_hello_len),
- AlpnForVersion(session()->version()),
- set_transport_params_result.quic_transport_params,
- set_transport_params_result.early_data_context);
-
- QUICHE_DCHECK_EQ(status, select_cert_status().value());
-
- if (status == QUIC_PENDING) {
- set_expected_ssl_error(SSL_ERROR_PENDING_CERTIFICATE);
- if (async_op_timer_.has_value()) {
- QUIC_CODE_COUNT(
- quic_tls_server_selecting_cert_while_another_op_pending);
- }
- async_op_timer_ = QuicTimeAccumulator();
- async_op_timer_->Start(now());
- return ssl_select_cert_retry;
- }
-
- if (status == QUIC_FAILURE) {
- return ssl_select_cert_error;
- }
-
- return ssl_select_cert_success;
- }
-
- QuicReferenceCountedPointer<ProofSource::Chain> chain =
- proof_source_->GetCertChain(session()->connection()->self_address(),
- session()->connection()->peer_address(),
- cert_selection_hostname());
- if (!chain || chain->certs.empty()) {
- QUIC_LOG(ERROR) << "No certs provided for host. raw:" << hostname_
- << ", normalized:" << crypto_negotiated_params_->sni;
- return ssl_select_cert_error;
- }
-
- CryptoBuffers cert_buffers = chain->ToCryptoBuffers();
- tls_connection_.SetCertChain(cert_buffers.value);
-
std::string error_details;
if (!ProcessTransportParameters(client_hello, &error_details)) {
CloseConnection(QUIC_HANDSHAKE_FAILED, error_details);
@@ -918,13 +806,38 @@
OverrideQuicConfigDefaults(session()->config());
session()->OnConfigNegotiated();
- if (!SetTransportParameters().success) {
+ auto set_transport_params_result = SetTransportParameters();
+ if (!set_transport_params_result.success) {
QUIC_LOG(ERROR) << "Failed to set transport parameters";
return ssl_select_cert_error;
}
- QUIC_DLOG(INFO) << "Set " << chain->certs.size() << " certs for server "
- << "with hostname " << hostname_;
+ const QuicAsyncStatus status = proof_source_handle_->SelectCertificate(
+ session()->connection()->self_address(),
+ session()->connection()->peer_address(), cert_selection_hostname(),
+ absl::string_view(
+ reinterpret_cast<const char*>(client_hello->client_hello),
+ client_hello->client_hello_len),
+ AlpnForVersion(session()->version()),
+ set_transport_params_result.quic_transport_params,
+ set_transport_params_result.early_data_context);
+
+ QUICHE_DCHECK_EQ(status, select_cert_status().value());
+
+ if (status == QUIC_PENDING) {
+ set_expected_ssl_error(SSL_ERROR_PENDING_CERTIFICATE);
+ if (async_op_timer_.has_value()) {
+ QUIC_CODE_COUNT(quic_tls_server_selecting_cert_while_another_op_pending);
+ }
+ async_op_timer_ = QuicTimeAccumulator();
+ async_op_timer_->Start(now());
+ return ssl_select_cert_retry;
+ }
+
+ if (status == QUIC_FAILURE) {
+ return ssl_select_cert_error;
+ }
+
return ssl_select_cert_success;
}
@@ -934,8 +847,6 @@
const ProofSource::Chain* chain) {
QUIC_DVLOG(1) << "OnSelectCertificateDone. ok:" << ok
<< ", is_sync:" << is_sync;
- QUICHE_DCHECK(use_proof_source_handle_);
-
select_cert_status_ = QUIC_FAILURE;
if (ok) {
if (chain && !chain->certs.empty()) {
diff --git a/quic/core/tls_server_handshaker.h b/quic/core/tls_server_handshaker.h
index 7850d35..738bfe9 100644
--- a/quic/core/tls_server_handshaker.h
+++ b/quic/core/tls_server_handshaker.h
@@ -88,11 +88,8 @@
protected:
// Creates a proof source handle for selecting cert and computing signature.
- // Only called when |use_proof_source_handle_| is true.
virtual std::unique_ptr<ProofSourceHandle> MaybeCreateProofSourceHandle();
- bool use_proof_source_handle() const { return use_proof_source_handle_; }
-
// Hook to allow the server to override parts of the QuicConfig based on SNI
// before we generate transport parameters.
virtual void OverrideQuicConfigDefaults(QuicConfig* config);
@@ -186,21 +183,6 @@
std::unique_ptr<ProofSource::Details> details) override;
private:
- class QUIC_EXPORT_PRIVATE SignatureCallback
- : public ProofSource::SignatureCallback {
- public:
- explicit SignatureCallback(TlsServerHandshaker* handshaker);
- void Run(bool ok,
- std::string signature,
- std::unique_ptr<ProofSource::Details> details) override;
-
- // If called, Cancel causes the pending callback to be a no-op.
- void Cancel();
-
- private:
- TlsServerHandshaker* handshaker_;
- };
-
class QUIC_EXPORT_PRIVATE DecryptCallback
: public ProofSource::DecryptCallback {
public:
@@ -309,7 +291,6 @@
std::unique_ptr<ProofSourceHandle> proof_source_handle_;
ProofSource* proof_source_;
- SignatureCallback* signature_callback_ = nullptr;
// State to handle potentially asynchronous session ticket decryption.
// |ticket_decryption_callback_| points to the non-owned callback that was
@@ -346,8 +327,6 @@
QuicReferenceCountedPointer<QuicCryptoNegotiatedParameters>
crypto_negotiated_params_;
TlsServerConnection tls_connection_;
- const bool use_proof_source_handle_ =
- GetQuicReloadableFlag(quic_tls_use_per_handshaker_proof_source);
const bool use_normalized_sni_for_cert_selection_ =
GetQuicReloadableFlag(quic_tls_use_normalized_sni_for_cert_selectioon);
const QuicCryptoServerConfig* crypto_config_; // Unowned.
diff --git a/quic/core/tls_server_handshaker_test.cc b/quic/core/tls_server_handshaker_test.cc
index 2c7d415..e146e0b 100644
--- a/quic/core/tls_server_handshaker_test.cc
+++ b/quic/core/tls_server_handshaker_test.cc
@@ -400,10 +400,6 @@
}
TEST_P(TlsServerHandshakerTest, HandshakeWithAsyncSelectCertSuccess) {
- if (!GetQuicReloadableFlag(quic_tls_use_per_handshaker_proof_source)) {
- return;
- }
-
InitializeServerWithFakeProofSourceHandle();
server_handshaker_->SetupProofSourceHandle(
/*select_cert_action=*/FakeProofSourceHandle::Action::DELEGATE_ASYNC,
@@ -426,10 +422,6 @@
}
TEST_P(TlsServerHandshakerTest, HandshakeWithAsyncSelectCertFailure) {
- if (!GetQuicReloadableFlag(quic_tls_use_per_handshaker_proof_source)) {
- return;
- }
-
InitializeServerWithFakeProofSourceHandle();
server_handshaker_->SetupProofSourceHandle(
/*select_cert_action=*/FakeProofSourceHandle::Action::FAIL_ASYNC,
@@ -449,10 +441,6 @@
}
TEST_P(TlsServerHandshakerTest, HandshakeWithAsyncSelectCertAndSignature) {
- if (!GetQuicReloadableFlag(quic_tls_use_per_handshaker_proof_source)) {
- return;
- }
-
InitializeServerWithFakeProofSourceHandle();
server_handshaker_->SetupProofSourceHandle(
/*select_cert_action=*/FakeProofSourceHandle::Action::DELEGATE_ASYNC,
@@ -507,10 +495,6 @@
}
TEST_P(TlsServerHandshakerTest, CancelPendingSelectCert) {
- if (!GetQuicReloadableFlag(quic_tls_use_per_handshaker_proof_source)) {
- return;
- }
-
InitializeServerWithFakeProofSourceHandle();
server_handshaker_->SetupProofSourceHandle(
/*select_cert_action=*/FakeProofSourceHandle::Action::DELEGATE_ASYNC,
@@ -557,10 +541,6 @@
}
TEST_P(TlsServerHandshakerTest, HostnameForCertSelectionAndComputeSignature) {
- if (!GetQuicReloadableFlag(quic_tls_use_per_handshaker_proof_source)) {
- return;
- }
-
// Client uses upper case letters in hostname. It is considered valid by
// QuicHostnameUtils::IsValidSNI, but it should be normalized for cert
// selection.
