Deprecate quic_dont_pad_chlo Deprecate gfe2_reloadable_flag_quic_dont_pad_chlo PiperOrigin-RevId: 325867913 Change-Id: I2ca12a94ff2f03537f3e3f3af83a0a199569a242
diff --git a/quic/core/crypto/crypto_server_test.cc b/quic/core/crypto/crypto_server_test.cc index 58d39b8..17ab10b 100644 --- a/quic/core/crypto/crypto_server_test.cc +++ b/quic/core/crypto/crypto_server_test.cc
@@ -507,22 +507,6 @@ CheckRejectReasons(kRejectReasons, QUICHE_ARRAYSIZE(kRejectReasons)); } -TEST_P(CryptoServerTest, TooSmall) { - if (GetQuicReloadableFlag(quic_dont_pad_chlo)) { - // This test validates that non-padded CHLOs are rejected, it cannot pass - // when the padding is no longer required. - // TODO(dschinazi) remove this test when we deprecate quic_dont_pad_chlo. - return; - } - ShouldFailMentioning( - "too small", crypto_test_utils::CreateCHLO( - {{"PDMD", "X509"}, {"VER\0", client_version_string_}})); - - const HandshakeFailureReason kRejectReasons[] = { - SERVER_CONFIG_INCHOATE_HELLO_FAILURE}; - CheckRejectReasons(kRejectReasons, QUICHE_ARRAYSIZE(kRejectReasons)); -} - TEST_P(CryptoServerTest, BadSourceAddressToken) { // Invalid source-address tokens should be ignored. // clang-format off
diff --git a/quic/core/crypto/quic_crypto_client_config.cc b/quic/core/crypto/quic_crypto_client_config.cc index 1b1c595..c110f8e 100644 --- a/quic/core/crypto/quic_crypto_client_config.cc +++ b/quic/core/crypto/quic_crypto_client_config.cc
@@ -68,8 +68,7 @@ : proof_verifier_(std::move(proof_verifier)), session_cache_(std::move(session_cache)), ssl_ctx_(TlsClientConnection::CreateSslCtx( - GetQuicRestartFlag(quic_enable_zero_rtt_for_tls_v2))), - disable_chlo_padding_(GetQuicReloadableFlag(quic_dont_pad_chlo)) { + GetQuicRestartFlag(quic_enable_zero_rtt_for_tls_v2))) { DCHECK(proof_verifier_.get()); SetDefaults(); } @@ -419,12 +418,7 @@ QuicReferenceCountedPointer<QuicCryptoNegotiatedParameters> out_params, CryptoHandshakeMessage* out) const { out->set_tag(kCHLO); - // TODO(rch): Remove this when we remove quic_use_chlo_packet_size flag. - if (pad_inchoate_hello_ && !disable_chlo_padding_) { - out->set_minimum_size(kClientHelloMinimumSize); - } else { - out->set_minimum_size(1); - } + out->set_minimum_size(1); // Server name indication. We only send SNI if it's a valid domain name, as // per the spec. @@ -509,11 +503,7 @@ FillInchoateClientHello(server_id, preferred_version, cached, rand, /* demand_x509_proof= */ true, out_params, out); - if (pad_full_hello_ && !disable_chlo_padding_) { - out->set_minimum_size(kClientHelloMinimumSize); - } else { - out->set_minimum_size(1); - } + out->set_minimum_size(1); const CryptoHandshakeMessage* scfg = cached->GetServerConfig(); if (!scfg) {
diff --git a/quic/core/crypto/quic_crypto_client_config.h b/quic/core/crypto/quic_crypto_client_config.h index e355c5e..3d90757 100644 --- a/quic/core/crypto/quic_crypto_client_config.h +++ b/quic/core/crypto/quic_crypto_client_config.h
@@ -395,10 +395,6 @@ bool pad_full_hello() const { return pad_full_hello_; } void set_pad_full_hello(bool new_value) { pad_full_hello_ = new_value; } - void set_disable_chlo_padding(bool disabled) { - disable_chlo_padding_ = disabled; - } - private: // Sets the members to reasonable, default values. void SetDefaults(); @@ -466,7 +462,6 @@ // other means of verifying the client. bool pad_inchoate_hello_ = true; bool pad_full_hello_ = true; - bool disable_chlo_padding_; }; } // namespace quic
diff --git a/quic/core/crypto/quic_crypto_client_config_test.cc b/quic/core/crypto/quic_crypto_client_config_test.cc index c995e8a..88b3eb6 100644 --- a/quic/core/crypto/quic_crypto_client_config_test.cc +++ b/quic/core/crypto/quic_crypto_client_config_test.cc
@@ -158,12 +158,7 @@ quiche::QuicheStringPiece alpn; EXPECT_TRUE(msg.GetStringPiece(kALPN, &alpn)); EXPECT_EQ("hq", alpn); - - if (GetQuicReloadableFlag(quic_dont_pad_chlo)) { - EXPECT_EQ(msg.minimum_size(), 1u); - } else { - EXPECT_EQ(msg.minimum_size(), 1024u); - } + EXPECT_EQ(msg.minimum_size(), 1u); } TEST_F(QuicCryptoClientConfigTest, InchoateChloIsNotPadded) {
diff --git a/quic/core/crypto/quic_crypto_server_config.cc b/quic/core/crypto/quic_crypto_server_config.cc index c07b8d3..5835e94 100644 --- a/quic/core/crypto/quic_crypto_server_config.cc +++ b/quic/core/crypto/quic_crypto_server_config.cc
@@ -1211,16 +1211,6 @@ const CryptoHandshakeMessage& client_hello = client_hello_state->client_hello; ClientHelloInfo* info = &(client_hello_state->info); - if (GetQuicReloadableFlag(quic_dont_pad_chlo)) { - QUIC_RELOADABLE_FLAG_COUNT_N(quic_dont_pad_chlo, 1, 2); - } else { - if (validate_chlo_size_ && client_hello.size() < kClientHelloMinimumSize) { - helper.ValidationComplete(QUIC_CRYPTO_INVALID_VALUE_LENGTH, - "Client hello too small", nullptr); - return; - } - } - if (client_hello.GetStringPiece(kSNI, &info->sni) && !QuicHostnameUtils::IsValidSNI(info->sni)) { helper.ValidationComplete(QUIC_INVALID_CRYPTO_MESSAGE_PARAMETER,
diff --git a/quic/core/http/end_to_end_test.cc b/quic/core/http/end_to_end_test.cc index b6d62e8..32aed2b 100644 --- a/quic/core/http/end_to_end_test.cc +++ b/quic/core/http/end_to_end_test.cc
@@ -4739,7 +4739,6 @@ ASSERT_TRUE(Initialize()); return; } - SetQuicReloadableFlag(quic_dont_pad_chlo, true); SetQuicReloadableFlag(quic_dispatcher_legacy_version_encapsulation, true); client_config_.SetClientConnectionOptions(QuicTagVector{kQLVE}); ASSERT_TRUE(Initialize()); @@ -4764,7 +4763,6 @@ ASSERT_TRUE(Initialize()); return; } - SetQuicReloadableFlag(quic_dont_pad_chlo, true); SetQuicReloadableFlag(quic_dispatcher_legacy_version_encapsulation, true); client_config_.SetClientConnectionOptions(QuicTagVector{kQLVE}); constexpr auto kCustomParameter = @@ -4789,7 +4787,6 @@ } client_supported_versions_.insert(client_supported_versions_.begin(), QuicVersionReservedForNegotiation()); - SetQuicReloadableFlag(quic_dont_pad_chlo, true); SetQuicReloadableFlag(quic_dispatcher_legacy_version_encapsulation, true); client_config_.SetClientConnectionOptions(QuicTagVector{kQLVE}); ASSERT_TRUE(Initialize()); @@ -4809,7 +4806,6 @@ return; } SetPacketLossPercentage(30); - SetQuicReloadableFlag(quic_dont_pad_chlo, true); SetQuicReloadableFlag(quic_dispatcher_legacy_version_encapsulation, true); client_config_.SetClientConnectionOptions(QuicTagVector{kQLVE}); // Disable blackhole detection as this test is testing loss recovery.
diff --git a/quic/core/http/quic_spdy_client_session.cc b/quic/core/http/quic_spdy_client_session.cc index 066adb9..c5ef0c4 100644 --- a/quic/core/http/quic_spdy_client_session.cc +++ b/quic/core/http/quic_spdy_client_session.cc
@@ -42,9 +42,6 @@ if (config()->HasClientRequestedIndependentOption(kQLVE, Perspective::IS_CLIENT)) { connection()->EnableLegacyVersionEncapsulation(server_id_.host()); - // Legacy Version Encapsulation needs CHLO padding to be disabled. - // TODO(dschinazi) remove this line once we deprecate quic_dont_pad_chlo. - crypto_config_->set_disable_chlo_padding(true); } QuicSpdyClientSessionBase::Initialize(); }
diff --git a/quic/core/quic_dispatcher.cc b/quic/core/quic_dispatcher.cc index f171610..7275adf 100644 --- a/quic/core/quic_dispatcher.cc +++ b/quic/core/quic_dispatcher.cc
@@ -662,21 +662,20 @@ BufferEarlyPacket(*packet_info); break; } - if (GetQuicReloadableFlag(quic_dont_pad_chlo)) { - QUIC_RELOADABLE_FLAG_COUNT_N(quic_dont_pad_chlo, 2, 2); - // We only apply this check for versions that do not use the IETF - // invariant header because those versions are already checked in - // QuicDispatcher::MaybeDispatchPacket. - if (packet_info->version_flag && - !packet_info->version.HasIetfInvariantHeader() && - crypto_config()->validate_chlo_size() && - packet_info->packet.length() < kMinClientInitialPacketLength) { - QUIC_DVLOG(1) << "Dropping CHLO packet which is too short, length: " - << packet_info->packet.length(); - QUIC_CODE_COUNT(quic_drop_small_chlo_packets); - break; - } + + // We only apply this check for versions that do not use the IETF + // invariant header because those versions are already checked in + // QuicDispatcher::MaybeDispatchPacket. + if (packet_info->version_flag && + !packet_info->version.HasIetfInvariantHeader() && + crypto_config()->validate_chlo_size() && + packet_info->packet.length() < kMinClientInitialPacketLength) { + QUIC_DVLOG(1) << "Dropping CHLO packet which is too short, length: " + << packet_info->packet.length(); + QUIC_CODE_COUNT(quic_drop_small_chlo_packets); + break; } + if (GetQuicReloadableFlag(quic_dispatcher_legacy_version_encapsulation)) { QUIC_RELOADABLE_FLAG_COUNT_N( quic_dispatcher_legacy_version_encapsulation, 3, 3);
diff --git a/quic/core/quic_dispatcher_test.cc b/quic/core/quic_dispatcher_test.cc index 282bef7..8d06ce5 100644 --- a/quic/core/quic_dispatcher_test.cc +++ b/quic/core/quic_dispatcher_test.cc
@@ -587,7 +587,6 @@ // is not currently supported in QuicDispatcher. return; } - SetQuicReloadableFlag(quic_dont_pad_chlo, true); SetQuicReloadableFlag(quic_dispatcher_legacy_version_encapsulation, true); QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 1); QuicConnectionId server_connection_id = TestConnectionId(); @@ -1376,14 +1375,6 @@ } TEST_P(QuicDispatcherTestAllVersions, DoNotProcessSmallPacket) { - if (!version_.HasIetfInvariantHeader() && - !GetQuicReloadableFlag(quic_dont_pad_chlo)) { - // When quic_dont_pad_chlo is false, we only drop small packets when using - // IETF_QUIC_LONG_HEADER_PACKET. When quic_dont_pad_chlo is true, we drop - // small packets for all versions. - // TODO(dschinazi) remove this early return when we deprecate the flag. - return; - } CreateTimeWaitListManager(); QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 1);