Deprecate gfe2_reloadable_flag_quic_use_proof_source_get_cert_chains PiperOrigin-RevId: 895448563
diff --git a/quiche/common/quiche_feature_flags_list.h b/quiche/common/quiche_feature_flags_list.h index 559f978..f950acf 100755 --- a/quiche/common/quiche_feature_flags_list.h +++ b/quiche/common/quiche_feature_flags_list.h
@@ -66,7 +66,6 @@ QUICHE_FLAG(bool, quiche_reloadable_flag_quic_testonly_default_false, false, false, "A testonly reloadable flag that will always default to false.") QUICHE_FLAG(bool, quiche_reloadable_flag_quic_testonly_default_true, true, true, "A testonly reloadable flag that will always default to true.") QUICHE_FLAG(bool, quiche_reloadable_flag_quic_update_max_datagram, false, true, "If true, updates the maximum datagram size after flushing pending packets.") -QUICHE_FLAG(bool, quiche_reloadable_flag_quic_use_proof_source_get_cert_chains, true, true, "When true, quic::TlsServerHandshaker will use ProofSource::GetCertChains() instead of ProofSource::GetCertChain()") QUICHE_FLAG(bool, quiche_reloadable_flag_quic_use_received_client_addresses_cache, true, true, "If true, use a LRU cache to record client addresses of packets received on server's original address.") QUICHE_FLAG(bool, quiche_restart_flag_quic_dispatcher_close_connection_on_invalid_ack, false, false, "An invalid ack is an ack that the peer sent for a packet that was not sent by the dispatcher. If true, the dispatcher will close the connection if it receives an invalid ack.") QUICHE_FLAG(bool, quiche_restart_flag_quic_shed_tls_handshake_config, false, false, "If true, QUIC connections will call SSL_set_shed_handshake_config to drop BoringSSL handshake state after the handshake finishes in order to save memory.")
diff --git a/quiche/quic/core/crypto/proof_source.h b/quiche/quic/core/crypto/proof_source.h index b66a18b..698ca36 100644 --- a/quiche/quic/core/crypto/proof_source.h +++ b/quiche/quic/core/crypto/proof_source.h
@@ -344,20 +344,11 @@ // // When called asynchronously(is_sync=false), this method will be responsible // to continue the handshake from where it left off. - // - // Callers that pass a `LocalSSLConfig` in `ssl_config` must use the result of - // `DoesOnSelectCertificateDoneExpectChains()` to decide which fields to - // populate. virtual void OnSelectCertificateDone(bool ok, bool is_sync, SSLConfig ssl_config, absl::string_view ticket_encryption_key, bool cert_matched_sni) = 0; - // Returns true when `OnSelectCertificateDone()` reads the - // `LocalSSLConfig::chains` field. Otherwise, it may read - // `LocalSSLConfig::chain`. - virtual bool DoesOnSelectCertificateDoneExpectChains() const = 0; - // Called when a ProofSourceHandle::ComputeSignature operation completes. virtual void OnComputeSignatureDone( bool ok, bool is_sync, std::string signature,
diff --git a/quiche/quic/core/tls_server_handshaker.cc b/quiche/quic/core/tls_server_handshaker.cc index a0fa3c8..b22d855 100644 --- a/quiche/quic/core/tls_server_handshaker.cc +++ b/quiche/quic/core/tls_server_handshaker.cc
@@ -115,39 +115,17 @@ return QUIC_FAILURE; } - if (handshaker_->DoesOnSelectCertificateDoneExpectChains()) { - QUIC_RELOADABLE_FLAG_COUNT_N(quic_use_proof_source_get_cert_chains, 1, 2); - - ProofSource::CertChainsResult cert_chains_result = - proof_source_->GetCertChains(server_address, client_address, hostname); - - handshaker_->OnSelectCertificateDone( - /*ok=*/true, /*is_sync=*/true, - ProofSourceHandleCallback::LocalSSLConfig(cert_chains_result.chains, - QuicDelayedSSLConfig()), - /*ticket_encryption_key=*/absl::string_view(), - /*cert_matched_sni=*/cert_chains_result.chains_match_sni); - if (!handshaker_->select_cert_status().has_value()) { - QUIC_BUG(select_cert_status_valueless_after_sync_select_cert); - // Return success to continue the handshake. - return QUIC_SUCCESS; - } - return *handshaker_->select_cert_status(); - } - - bool cert_matched_sni; - quiche::QuicheReferenceCountedPointer<ProofSource::Chain> chain = - proof_source_->GetCertChain(server_address, client_address, hostname, - &cert_matched_sni); + ProofSource::CertChainsResult cert_chains_result = + proof_source_->GetCertChains(server_address, client_address, hostname); handshaker_->OnSelectCertificateDone( /*ok=*/true, /*is_sync=*/true, - ProofSourceHandleCallback::LocalSSLConfig{chain.get(), - QuicDelayedSSLConfig()}, - /*ticket_encryption_key=*/absl::string_view(), cert_matched_sni); + ProofSourceHandleCallback::LocalSSLConfig(cert_chains_result.chains, + QuicDelayedSSLConfig()), + /*ticket_encryption_key=*/absl::string_view(), + /*cert_matched_sni=*/cert_chains_result.chains_match_sni); if (!handshaker_->select_cert_status().has_value()) { - QUIC_BUG(quic_bug_12423_1) - << "select_cert_status() has no value after a synchronous select cert"; + QUIC_BUG(select_cert_status_valueless_after_sync_select_cert); // Return success to continue the handshake. return QUIC_SUCCESS; } @@ -243,8 +221,6 @@ QuicCryptoServerStreamBase(session), proof_source_(crypto_config->proof_source()), proof_verifier_(crypto_config->proof_verifier()), - use_proof_source_get_cert_chains_( - GetQuicReloadableFlag(quic_use_proof_source_get_cert_chains)), pre_shared_key_(crypto_config->pre_shared_key()), crypto_negotiated_params_(new QuicCryptoNegotiatedParameters), tls_connection_(crypto_config->ssl_ctx(), this, session->GetSSLConfig()), @@ -1156,22 +1132,13 @@ if (ok) { if (auto* local_config = std::get_if<LocalSSLConfig>(&ssl_config); local_config != nullptr) { - if (!use_proof_source_get_cert_chains_ && local_config->chain && - !local_config->chain->certs.empty()) { - tls_connection_.AddCertChain( - local_config->chain->ToCryptoBuffers().value, - local_config->chain->trust_anchor_id); - select_cert_status_ = QUIC_SUCCESS; - } else if (use_proof_source_get_cert_chains_ && - !local_config->chains.empty() && - // Cert selection fails when there are no chains with certs. - absl::c_any_of(local_config->chains, - [](const quiche::QuicheReferenceCountedPointer< - ProofSource::Chain> absl_nonnull& chain) { - return !chain->certs.empty(); - })) { - QUIC_RELOADABLE_FLAG_COUNT_N(quic_use_proof_source_get_cert_chains, 2, - 2); + if (!local_config->chains.empty() && + // Cert selection fails when there are no chains with certs. + absl::c_any_of(local_config->chains, + [](const quiche::QuicheReferenceCountedPointer< + ProofSource::Chain> absl_nonnull& chain) { + return !chain->certs.empty(); + })) { for (const quiche::QuicheReferenceCountedPointer< ProofSource::Chain> absl_nonnull& chain : local_config->chains) {
diff --git a/quiche/quic/core/tls_server_handshaker.h b/quiche/quic/core/tls_server_handshaker.h index 7267aaf..aece463 100644 --- a/quiche/quic/core/tls_server_handshaker.h +++ b/quiche/quic/core/tls_server_handshaker.h
@@ -208,9 +208,6 @@ void OnSelectCertificateDone(bool ok, bool is_sync, SSLConfig ssl_config, absl::string_view ticket_encryption_key, bool cert_matched_sni) override; - bool DoesOnSelectCertificateDoneExpectChains() const override { - return use_proof_source_get_cert_chains_; - } void OnComputeSignatureDone( bool ok, bool is_sync, std::string signature, @@ -265,9 +262,7 @@ // Close the handle. Cancel the pending signature operation, if any. void CloseHandle() override; - // Delegates to `proof_source_->GetCertChains()` when - // `handshaker_->use_proof_source_get_cert_chains()` is true. Otherwise, - // delegates to `proof_source_->GetCertChain()`. + // Delegates to `proof_source_->GetCertChains()`. // // Returns `QUIC_SUCCESS` or `QUIC_FAILURE`. Never returns `QUIC_PENDING`. QuicAsyncStatus SelectCertificate( @@ -402,10 +397,6 @@ // True if new ALPS codepoint in the ClientHello. bool alps_new_codepoint_received_ = false; - // The value of the reloadable flag `quic_use_proof_source_get_cert_chains` at - // the time of construction. - const bool use_proof_source_get_cert_chains_; - // nullopt means select cert hasn't started. std::optional<QuicAsyncStatus> select_cert_status_;
diff --git a/quiche/quic/core/tls_server_handshaker_test.cc b/quiche/quic/core/tls_server_handshaker_test.cc index 9e03220..fa33070 100644 --- a/quiche/quic/core/tls_server_handshaker_test.cc +++ b/quiche/quic/core/tls_server_handshaker_test.cc
@@ -75,15 +75,12 @@ struct TestParams { ParsedQuicVersion version; bool disable_resumption; - bool enable_get_cert_chains; }; ABSL_ATTRIBUTE_UNUSED // Used by ::testing::PrintToStringParamName(). std::string PrintToString(const TestParams& p) { return absl::StrCat(ParsedQuicVersionToString(p.version), "AndResumption", - (p.disable_resumption ? "Disabled" : "Enabled"), - "AndGetCertChains", - (p.enable_get_cert_chains ? "Enabled" : "Disabled")); + (p.disable_resumption ? "Disabled" : "Enabled")); } // Constructs test permutations. @@ -91,10 +88,7 @@ std::vector<TestParams> params; for (const auto& version : AllSupportedVersionsWithTls()) { for (bool disable_resumption : {false, true}) { - for (bool enable_get_cert_chains : {false, true}) { - params.push_back( - TestParams{version, disable_resumption, enable_get_cert_chains}); - } + params.push_back(TestParams{version, disable_resumption}); } } return params; @@ -165,15 +159,14 @@ FakeProofSourceHandle::Action compute_signature_action, QuicDelayedSSLConfig dealyed_ssl_config = QuicDelayedSSLConfig()) { EXPECT_CALL(*this, MaybeCreateProofSourceHandle()) - .WillOnce( - [this, select_cert_action, compute_signature_action, - dealyed_ssl_config]() { - auto handle = std::make_unique<FakeProofSourceHandle>( - proof_source_, this, select_cert_action, - compute_signature_action, dealyed_ssl_config); - fake_proof_source_handle_ = handle.get(); - return handle; - }); + .WillOnce([this, select_cert_action, compute_signature_action, + dealyed_ssl_config]() { + auto handle = std::make_unique<FakeProofSourceHandle>( + proof_source_, this, select_cert_action, compute_signature_action, + dealyed_ssl_config); + fake_proof_source_handle_ = handle.get(); + return handle; + }); } FakeProofSourceHandle* fake_proof_source_handle() { @@ -255,8 +248,6 @@ supported_versions_({GetParam().version}) { SetQuicFlag(quic_disable_server_tls_resumption, GetParam().disable_resumption); - SetQuicReloadableFlag(quic_use_proof_source_get_cert_chains, - GetParam().enable_get_cert_chains); client_crypto_config_ = std::make_unique<QuicCryptoClientConfig>( crypto_test_utils::ProofVerifierForTesting(), @@ -758,41 +749,23 @@ } TEST_P(TlsServerHandshakerTest, SelectCertificateCallsGetCertChains) { - if (GetQuicReloadableFlag(quic_use_proof_source_get_cert_chains)) { - EXPECT_CALL(*proof_source_, GetCertChains(_, _, kServerHostname)); - // Although the default implementation of `ProofSource::GetCertChains()` - // uses `ProofSource::GetCertChain()`, we're using a `FakeProofSource` that - // delegates to a `TestProofSource`, which does not have this behavior. - EXPECT_CALL(*proof_source_, GetCertChain).Times(0); - } else { - EXPECT_CALL(*proof_source_, GetCertChains).Times(0); - EXPECT_CALL(*proof_source_, GetCertChain(_, _, kServerHostname, _)); - } + EXPECT_CALL(*proof_source_, GetCertChains(_, _, kServerHostname)); + // Although the default implementation of `ProofSource::GetCertChains()` uses + // `ProofSource::GetCertChain()`, we're using a `FakeProofSource` that + // delegates to a `TestProofSource`, which does not have this behavior. + EXPECT_CALL(*proof_source_, GetCertChain).Times(0); CompleteCryptoHandshake(); ExpectHandshakeSuccessful(); } TEST_P(TlsServerHandshakerTest, ZeroCertChainsFailsHandshake) { - if (GetQuicReloadableFlag(quic_use_proof_source_get_cert_chains)) { - EXPECT_CALL(*proof_source_, GetCertChains(_, _, kServerHostname)) - .WillOnce(Return(ProofSource::CertChainsResult{ - .chains_match_sni = false, - .chains = {}, - })); - EXPECT_CALL(*proof_source_, GetCertChain).Times(0); - } else { - EXPECT_CALL(*proof_source_, GetCertChain) - .WillOnce([&](const QuicSocketAddress&, const QuicSocketAddress&, - const std::string&, bool* cert_matched_sni) - -> quiche::QuicheReferenceCountedPointer< - ProofSource::Chain> { - *cert_matched_sni = false; - return // quiche::QuicheReferenceCountedPointer<ProofSource::Chain>( - nullptr; - }); - EXPECT_CALL(*proof_source_, GetCertChains).Times(0); - } + EXPECT_CALL(*proof_source_, GetCertChains(_, _, kServerHostname)) + .WillOnce(Return(ProofSource::CertChainsResult{ + .chains_match_sni = false, + .chains = {}, + })); + EXPECT_CALL(*proof_source_, GetCertChain).Times(0); AdvanceHandshakeWithFakeClient(); @@ -812,41 +785,33 @@ // this test would incorrectly exercise `FakeProofSourceHandle` instead of // `DefaultProofSourceHandle`. - if (GetQuicReloadableFlag(quic_use_proof_source_get_cert_chains)) { - // Return chains that claim not to match SNI. (Whether the certificate in - // `quic::test::kTestCertificate` actually matches `kServerHostname` is - // irrelevant.) - EXPECT_CALL(*proof_source_, GetCertChains(_, _, kServerHostname)) - .WillOnce(Return(ProofSource::CertChainsResult{ - .chains_match_sni = false, - .chains = - std::vector< - quiche::QuicheReferenceCountedPointer<ProofSource::Chain>>{ - quiche::QuicheReferenceCountedPointer<ProofSource::Chain>( - new ProofSource::Chain( - /*certs=*/{std::string( - quic::test::kTestCertificate)}, - /*trust_anchor_id=*/"")), - quiche::QuicheReferenceCountedPointer<ProofSource::Chain>( - new ProofSource::Chain( - /*certs=*/{std::string( - quic::test::kTestCertificate)}, - /*trust_anchor_id=*/"")), - }, - })); + // Return chains that claim not to match SNI. (Whether the certificate in + // `quic::test::kTestCertificate` actually matches `kServerHostname` is + // irrelevant.) + EXPECT_CALL(*proof_source_, GetCertChains(_, _, kServerHostname)) + .WillOnce(Return(ProofSource::CertChainsResult{ + .chains_match_sni = false, + .chains = + std::vector< + quiche::QuicheReferenceCountedPointer<ProofSource::Chain>>{ + quiche::QuicheReferenceCountedPointer<ProofSource::Chain>( + new ProofSource::Chain( + /*certs=*/{std::string(quic::test::kTestCertificate)}, + /*trust_anchor_id=*/"")), + quiche::QuicheReferenceCountedPointer<ProofSource::Chain>( + new ProofSource::Chain( + /*certs=*/{std::string(quic::test::kTestCertificate)}, + /*trust_anchor_id=*/"")), + }, + })); - EXPECT_CALL(*proof_source_, GetCertChain).Times(0); + EXPECT_CALL(*proof_source_, GetCertChain).Times(0); - EXPECT_CALL(*server_handshaker_, OnSelectCertificateDone( - /*ok=*/true, /*is_sync=*/_, - /*ssl_config=*/_, - /*ticket_encryption_key=*/_, - /*cert_matched_sni=*/false)); - } else { - EXPECT_CALL(*proof_source_, GetCertChains).Times(0); - EXPECT_CALL(*proof_source_, GetCertChain(_, _, kServerHostname, _)); - EXPECT_CALL(*server_handshaker_, OnSelectCertificateDone); - } + EXPECT_CALL(*server_handshaker_, OnSelectCertificateDone( + /*ok=*/true, /*is_sync=*/_, + /*ssl_config=*/_, + /*ticket_encryption_key=*/_, + /*cert_matched_sni=*/false)); CompleteCryptoHandshake(); } @@ -866,54 +831,43 @@ // this test would incorrectly exercise `FakeProofSourceHandle` instead of // `DefaultProofSourceHandle`. - if (GetQuicReloadableFlag(quic_use_proof_source_get_cert_chains)) { - using LocalSSLConfig = ProofSourceHandleCallback::LocalSSLConfig; + using LocalSSLConfig = ProofSourceHandleCallback::LocalSSLConfig; - std::vector<quiche::QuicheReferenceCountedPointer<ProofSource::Chain>> - chains = { - quiche::QuicheReferenceCountedPointer<ProofSource::Chain>( - new ProofSource::Chain( - /*certs=*/{std::string(quic::test::kTestCertificate)}, - /*trust_anchor_id=*/"\x11\x22\x33")), - quiche::QuicheReferenceCountedPointer<ProofSource::Chain>( - new ProofSource::Chain( - /*certs=*/{std::string(quic::test::kTestCertificate)}, - /*trust_anchor_id=*/"")), - }; - EXPECT_CALL(*proof_source_, GetCertChains(_, _, kServerHostname)) - .WillOnce(Return(ProofSource::CertChainsResult{ - .chains_match_sni = true, - .chains = chains, - })); - EXPECT_CALL(*proof_source_, GetCertChain).Times(0); - // `DefaultProofSourceHandle::SelectCertificate()` should pick only the - // chains that claim to match SNI. - EXPECT_CALL(*server_handshaker_, - OnSelectCertificateDone( - /*ok=*/true, /*is_sync=*/_, - VariantWith<LocalSSLConfig>( - AllOf(Field(&LocalSSLConfig::chain, IsNull()), - Field(&LocalSSLConfig::chains, - ElementsAre(chains[0], chains[1])), - Field(&LocalSSLConfig::delayed_ssl_config, - Eq(QuicDelayedSSLConfig())))), - /*ticket_encryption_key=*/_, - /*cert_matched_sni=*/true)); - } else { - EXPECT_CALL(*proof_source_, GetCertChains).Times(0); - EXPECT_CALL(*proof_source_, GetCertChain(_, _, kServerHostname, _)); - EXPECT_CALL(*server_handshaker_, OnSelectCertificateDone( - /*ok=*/true, /*is_sync=*/_, - /*ssl_config=*/_, - /*ticket_encryption_key=*/_, - /*cert_matched_sni=*/false)); - } + std::vector<quiche::QuicheReferenceCountedPointer<ProofSource::Chain>> + chains = { + quiche::QuicheReferenceCountedPointer<ProofSource::Chain>( + new ProofSource::Chain( + /*certs=*/{std::string(quic::test::kTestCertificate)}, + /*trust_anchor_id=*/"\x11\x22\x33")), + quiche::QuicheReferenceCountedPointer<ProofSource::Chain>( + new ProofSource::Chain( + /*certs=*/{std::string(quic::test::kTestCertificate)}, + /*trust_anchor_id=*/"")), + }; + EXPECT_CALL(*proof_source_, GetCertChains(_, _, kServerHostname)) + .WillOnce(Return(ProofSource::CertChainsResult{ + .chains_match_sni = true, + .chains = chains, + })); + EXPECT_CALL(*proof_source_, GetCertChain).Times(0); + // `DefaultProofSourceHandle::SelectCertificate()` should pick only the + // chains that claim to match SNI. + EXPECT_CALL( + *server_handshaker_, + OnSelectCertificateDone( + /*ok=*/true, /*is_sync=*/_, + VariantWith<LocalSSLConfig>(AllOf( + Field(&LocalSSLConfig::chain, IsNull()), + Field(&LocalSSLConfig::chains, ElementsAre(chains[0], chains[1])), + Field(&LocalSSLConfig::delayed_ssl_config, + Eq(QuicDelayedSSLConfig())))), + /*ticket_encryption_key=*/_, + /*cert_matched_sni=*/true)); CompleteCryptoHandshake(); ExpectHandshakeSuccessful(); - EXPECT_EQ(client_stream()->MatchedTrustAnchorIdForTesting(), - GetQuicReloadableFlag(quic_use_proof_source_get_cert_chains)); + EXPECT_TRUE(client_stream()->MatchedTrustAnchorIdForTesting()); } // Test that `DefaultProofSourceHandle::SelectCertificate()` passes through @@ -934,54 +888,43 @@ // this test would incorrectly exercise `FakeProofSourceHandle` instead of // `DefaultProofSourceHandle`. - if (GetQuicReloadableFlag(quic_use_proof_source_get_cert_chains)) { - using LocalSSLConfig = ProofSourceHandleCallback::LocalSSLConfig; + using LocalSSLConfig = ProofSourceHandleCallback::LocalSSLConfig; - std::vector<quiche::QuicheReferenceCountedPointer<ProofSource::Chain>> - chains = { - quiche::QuicheReferenceCountedPointer<ProofSource::Chain>( - new ProofSource::Chain( - /*certs=*/{std::string(quic::test::kTestCertificate)}, - /*trust_anchor_id=*/"\x07\x08")), - quiche::QuicheReferenceCountedPointer<ProofSource::Chain>( - new ProofSource::Chain( - /*certs=*/{std::string(quic::test::kTestCertificate)}, - /*trust_anchor_id=*/"\x11\x22\x33")), - }; - EXPECT_CALL(*proof_source_, GetCertChains(_, _, kServerHostname)) - .WillOnce(Return(ProofSource::CertChainsResult{ - .chains_match_sni = true, - .chains = chains, - })); - EXPECT_CALL(*proof_source_, GetCertChain).Times(0); - // `DefaultProofSourceHandle::SelectCertificate()` should pick only the - // chains that claim to match SNI. - EXPECT_CALL(*server_handshaker_, - OnSelectCertificateDone( - /*ok=*/true, /*is_sync=*/_, - VariantWith<LocalSSLConfig>( - AllOf(Field(&LocalSSLConfig::chain, IsNull()), - Field(&LocalSSLConfig::chains, - ElementsAre(chains[0], chains[1])), - Field(&LocalSSLConfig::delayed_ssl_config, - Eq(QuicDelayedSSLConfig())))), - /*ticket_encryption_key=*/_, - /*cert_matched_sni=*/true)); - } else { - EXPECT_CALL(*proof_source_, GetCertChains).Times(0); - EXPECT_CALL(*proof_source_, GetCertChain(_, _, kServerHostname, _)); - EXPECT_CALL(*server_handshaker_, OnSelectCertificateDone( - /*ok=*/true, /*is_sync=*/_, - /*ssl_config=*/_, - /*ticket_encryption_key=*/_, - /*cert_matched_sni=*/false)); - } + std::vector<quiche::QuicheReferenceCountedPointer<ProofSource::Chain>> + chains = { + quiche::QuicheReferenceCountedPointer<ProofSource::Chain>( + new ProofSource::Chain( + /*certs=*/{std::string(quic::test::kTestCertificate)}, + /*trust_anchor_id=*/"\x07\x08")), + quiche::QuicheReferenceCountedPointer<ProofSource::Chain>( + new ProofSource::Chain( + /*certs=*/{std::string(quic::test::kTestCertificate)}, + /*trust_anchor_id=*/"\x11\x22\x33")), + }; + EXPECT_CALL(*proof_source_, GetCertChains(_, _, kServerHostname)) + .WillOnce(Return(ProofSource::CertChainsResult{ + .chains_match_sni = true, + .chains = chains, + })); + EXPECT_CALL(*proof_source_, GetCertChain).Times(0); + // `DefaultProofSourceHandle::SelectCertificate()` should pick only the + // chains that claim to match SNI. + EXPECT_CALL( + *server_handshaker_, + OnSelectCertificateDone( + /*ok=*/true, /*is_sync=*/_, + VariantWith<LocalSSLConfig>(AllOf( + Field(&LocalSSLConfig::chain, IsNull()), + Field(&LocalSSLConfig::chains, ElementsAre(chains[0], chains[1])), + Field(&LocalSSLConfig::delayed_ssl_config, + Eq(QuicDelayedSSLConfig())))), + /*ticket_encryption_key=*/_, + /*cert_matched_sni=*/true)); CompleteCryptoHandshake(); ExpectHandshakeSuccessful(); - EXPECT_EQ(client_stream()->MatchedTrustAnchorIdForTesting(), - GetQuicReloadableFlag(quic_use_proof_source_get_cert_chains)); + EXPECT_TRUE(client_stream()->MatchedTrustAnchorIdForTesting()); } // Test that the handshake fails when all chains returned by @@ -1002,58 +945,44 @@ // this test would incorrectly exercise `FakeProofSourceHandle` instead of // `DefaultProofSourceHandle`. - if (GetQuicReloadableFlag(quic_use_proof_source_get_cert_chains)) { - using LocalSSLConfig = ProofSourceHandleCallback::LocalSSLConfig; + using LocalSSLConfig = ProofSourceHandleCallback::LocalSSLConfig; - std::vector<quiche::QuicheReferenceCountedPointer<ProofSource::Chain>> - chains = { - quiche::QuicheReferenceCountedPointer<ProofSource::Chain>( - new ProofSource::Chain( - /*certs=*/{std::string(quic::test::kTestCertificate)}, - /*trust_anchor_id=*/"\x07\x08")), - quiche::QuicheReferenceCountedPointer<ProofSource::Chain>( - new ProofSource::Chain( - /*certs=*/{std::string(quic::test::kTestCertificate)}, - /*trust_anchor_id=*/"\x42")), - }; - EXPECT_CALL(*proof_source_, GetCertChains(_, _, kServerHostname)) - .WillOnce(Return(ProofSource::CertChainsResult{ - .chains_match_sni = true, - .chains = chains, - })); - EXPECT_CALL(*proof_source_, GetCertChain).Times(0); - // `DefaultProofSourceHandle::SelectCertificate()` should pick only the - // chains that claim to match SNI. - EXPECT_CALL(*server_handshaker_, - OnSelectCertificateDone( - /*ok=*/true, /*is_sync=*/_, - VariantWith<LocalSSLConfig>( - AllOf(Field(&LocalSSLConfig::chain, IsNull()), - Field(&LocalSSLConfig::chains, - ElementsAre(chains[0], chains[1])), - Field(&LocalSSLConfig::delayed_ssl_config, - Eq(QuicDelayedSSLConfig())))), - /*ticket_encryption_key=*/_, - /*cert_matched_sni=*/true)); - AdvanceHandshakeWithFakeClient(); + std::vector<quiche::QuicheReferenceCountedPointer<ProofSource::Chain>> + chains = { + quiche::QuicheReferenceCountedPointer<ProofSource::Chain>( + new ProofSource::Chain( + /*certs=*/{std::string(quic::test::kTestCertificate)}, + /*trust_anchor_id=*/"\x07\x08")), + quiche::QuicheReferenceCountedPointer<ProofSource::Chain>( + new ProofSource::Chain( + /*certs=*/{std::string(quic::test::kTestCertificate)}, + /*trust_anchor_id=*/"\x42")), + }; + EXPECT_CALL(*proof_source_, GetCertChains(_, _, kServerHostname)) + .WillOnce(Return(ProofSource::CertChainsResult{ + .chains_match_sni = true, + .chains = chains, + })); + EXPECT_CALL(*proof_source_, GetCertChain).Times(0); + // `DefaultProofSourceHandle::SelectCertificate()` should pick only the + // chains that claim to match SNI. + EXPECT_CALL( + *server_handshaker_, + OnSelectCertificateDone( + /*ok=*/true, /*is_sync=*/_, + VariantWith<LocalSSLConfig>(AllOf( + Field(&LocalSSLConfig::chain, IsNull()), + Field(&LocalSSLConfig::chains, ElementsAre(chains[0], chains[1])), + Field(&LocalSSLConfig::delayed_ssl_config, + Eq(QuicDelayedSSLConfig())))), + /*ticket_encryption_key=*/_, + /*cert_matched_sni=*/true)); + AdvanceHandshakeWithFakeClient(); - EXPECT_FALSE(client_stream()->one_rtt_keys_available()); - EXPECT_FALSE(client_stream()->encryption_established()); - EXPECT_FALSE(server_stream()->one_rtt_keys_available()); - EXPECT_FALSE(server_stream()->encryption_established()); - - } else { - EXPECT_CALL(*proof_source_, GetCertChains).Times(0); - EXPECT_CALL(*proof_source_, GetCertChain(_, _, kServerHostname, _)); - EXPECT_CALL(*server_handshaker_, OnSelectCertificateDone( - /*ok=*/true, /*is_sync=*/_, - /*ssl_config=*/_, - /*ticket_encryption_key=*/_, - /*cert_matched_sni=*/false)); - - CompleteCryptoHandshake(); - ExpectHandshakeSuccessful(); - } + EXPECT_FALSE(client_stream()->one_rtt_keys_available()); + EXPECT_FALSE(client_stream()->encryption_established()); + EXPECT_FALSE(server_stream()->one_rtt_keys_available()); + EXPECT_FALSE(server_stream()->encryption_established()); EXPECT_FALSE(client_stream()->MatchedTrustAnchorIdForTesting()); } @@ -1065,8 +994,10 @@ // Send a zero-length ClientHello from client to server. char bogus_handshake_message[] = { // Handshake struct (RFC 8446 appendix B.3) - 1, // HandshakeType client_hello - 0, 0, 0, // uint24 length + 1, // HandshakeType client_hello + 0, + 0, + 0, // uint24 length }; // Install a packet flusher such that the packets generated by
diff --git a/quiche/quic/test_tools/fake_proof_source_handle.cc b/quiche/quic/test_tools/fake_proof_source_handle.cc index fc33158..56cee5c 100644 --- a/quiche/quic/test_tools/fake_proof_source_handle.cc +++ b/quiche/quic/test_tools/fake_proof_source_handle.cc
@@ -121,31 +121,16 @@ } QUICHE_DCHECK(select_cert_action_ == Action::DELEGATE_SYNC); - if (GetQuicReloadableFlag(quic_use_proof_source_get_cert_chains)) { - ProofSource::CertChainsResult chains_result = - delegate_->GetCertChains(server_address, client_address, hostname); - const bool ok = !chains_result.chains.empty(); - callback_->OnSelectCertificateDone( - ok, /*is_sync=*/true, - ProofSourceHandleCallback::LocalSSLConfig( - absl::MakeConstSpan(chains_result.chains), delayed_ssl_config_), - /*ticket_encryption_key=*/absl::string_view(), - /*cert_matched_sni=*/chains_result.chains_match_sni); - return ok ? QUIC_SUCCESS : QUIC_FAILURE; - } - bool cert_matched_sni; - quiche::QuicheReferenceCountedPointer<ProofSource::Chain> chain = - delegate_->GetCertChain(server_address, client_address, hostname, - &cert_matched_sni); - - bool ok = chain && !chain->certs.empty(); + ProofSource::CertChainsResult chains_result = + delegate_->GetCertChains(server_address, client_address, hostname); + const bool ok = !chains_result.chains.empty(); callback_->OnSelectCertificateDone( ok, /*is_sync=*/true, - ProofSourceHandleCallback::LocalSSLConfig{chain.get(), - delayed_ssl_config_}, + ProofSourceHandleCallback::LocalSSLConfig( + absl::MakeConstSpan(chains_result.chains), delayed_ssl_config_), /*ticket_encryption_key=*/absl::string_view(), - /*cert_matched_sni=*/cert_matched_sni); + /*cert_matched_sni=*/chains_result.chains_match_sni); return ok ? QUIC_SUCCESS : QUIC_FAILURE; } @@ -222,37 +207,20 @@ callback_->OnSelectCertificateDone( /*ok=*/false, /*is_sync=*/false, - callback_->DoesOnSelectCertificateDoneExpectChains() - ? ProofSourceHandleCallback::LocalSSLConfig( - /*chains=*/{}, delayed_ssl_config_) - : ProofSourceHandleCallback::LocalSSLConfig(/*chain=*/nullptr, - delayed_ssl_config_), + ProofSourceHandleCallback::LocalSSLConfig( + /*chains=*/{}, delayed_ssl_config_), /*ticket_encryption_key=*/absl::string_view(), /*cert_matched_sni=*/false); } else if (action_ == Action::DELEGATE_ASYNC) { - if (GetQuicReloadableFlag(quic_use_proof_source_get_cert_chains)) { - ProofSource::CertChainsResult chains_result = delegate_->GetCertChains( - args_.server_address, args_.client_address, args_.hostname); - const bool ok = !chains_result.chains.empty(); - callback_->OnSelectCertificateDone( - ok, /*is_sync=*/false, - ProofSourceHandleCallback::LocalSSLConfig( - absl::MakeConstSpan(chains_result.chains), delayed_ssl_config_), - /*ticket_encryption_key=*/absl::string_view(), - /*cert_matched_sni=*/chains_result.chains_match_sni); - } else { - bool cert_matched_sni; - quiche::QuicheReferenceCountedPointer<ProofSource::Chain> chain = - delegate_->GetCertChain(args_.server_address, args_.client_address, - args_.hostname, &cert_matched_sni); - bool ok = chain && !chain->certs.empty(); - callback_->OnSelectCertificateDone( - ok, /*is_sync=*/false, - ProofSourceHandleCallback::LocalSSLConfig{chain.get(), - delayed_ssl_config_}, - /*ticket_encryption_key=*/absl::string_view(), - /*cert_matched_sni=*/cert_matched_sni); - } + ProofSource::CertChainsResult chains_result = delegate_->GetCertChains( + args_.server_address, args_.client_address, args_.hostname); + const bool ok = !chains_result.chains.empty(); + callback_->OnSelectCertificateDone( + ok, /*is_sync=*/false, + ProofSourceHandleCallback::LocalSSLConfig( + absl::MakeConstSpan(chains_result.chains), delayed_ssl_config_), + /*ticket_encryption_key=*/absl::string_view(), + /*cert_matched_sni=*/chains_result.chains_match_sni); } else { QUIC_BUG(quic_bug_10139_1) << "Unexpected action: " << static_cast<int>(action_);