Deprecate quic_google_transport_param_omit_old Deprecate gfe2_restart_flag_quic_google_transport_param_omit_old PiperOrigin-RevId: 327087161 Change-Id: I10cd15ce6ad6e59d230198fbf10c805ec4fc9361
diff --git a/quic/core/crypto/transport_parameters.cc b/quic/core/crypto/transport_parameters.cc index 60f6c26..71a9dfb 100644 --- a/quic/core/crypto/transport_parameters.cc +++ b/quic/core/crypto/transport_parameters.cc
@@ -12,8 +12,6 @@ #include "third_party/boringssl/src/include/openssl/digest.h" #include "third_party/boringssl/src/include/openssl/sha.h" -#include "net/third_party/quiche/src/quic/core/crypto/crypto_framer.h" -#include "net/third_party/quiche/src/quic/core/crypto/crypto_handshake_message.h" #include "net/third_party/quiche/src/quic/core/quic_connection_id.h" #include "net/third_party/quiche/src/quic/core/quic_data_reader.h" #include "net/third_party/quiche/src/quic/core/quic_data_writer.h" @@ -58,7 +56,8 @@ kGoogleUserAgentId = 0x3129, kGoogleSupportHandshakeDone = 0x312A, // Only used in T050. kGoogleKeyUpdateNotYetSupported = 0x312B, - kGoogleQuicParam = 0x4751, // Used for non-standard Google-specific params. + // 0x4751 was used for non-standard Google-specific parameters encoded as a + // Google QUIC_CRYPTO CHLO, it has been replaced by individual parameters. kGoogleQuicVersion = 0x4752, // Used to transmit version and supported_versions. @@ -128,8 +127,6 @@ return "support_handshake_done"; case TransportParameters::kGoogleKeyUpdateNotYetSupported: return "key_update_not_yet_supported"; - case TransportParameters::kGoogleQuicParam: - return "google"; case TransportParameters::kGoogleQuicVersion: return "google-version"; case TransportParameters::kMinAckDelay: @@ -164,7 +161,6 @@ case TransportParameters::kGoogleUserAgentId: case TransportParameters::kGoogleSupportHandshakeDone: case TransportParameters::kGoogleKeyUpdateNotYetSupported: - case TransportParameters::kGoogleQuicParam: case TransportParameters::kGoogleQuicVersion: case TransportParameters::kMinAckDelay: return true; @@ -485,9 +481,6 @@ if (key_update_not_yet_supported) { rv += " " + TransportParameterIdToString(kGoogleKeyUpdateNotYetSupported); } - if (google_quic_params) { - rv += " " + TransportParameterIdToString(kGoogleQuicParam); - } for (const auto& kv : custom_parameters) { rv += " 0x" + quiche::QuicheTextUtils::Hex(static_cast<uint32_t>(kv.first)); rv += "="; @@ -580,10 +573,6 @@ preferred_address = std::make_unique<TransportParameters::PreferredAddress>( *other.preferred_address); } - if (other.google_quic_params) { - google_quic_params = - std::make_unique<CryptoHandshakeMessage>(*other.google_quic_params); - } } bool TransportParameters::operator==(const TransportParameters& rhs) const { @@ -626,21 +615,15 @@ } if ((!preferred_address && rhs.preferred_address) || - (preferred_address && !rhs.preferred_address) || - (!google_quic_params && rhs.google_quic_params) || - (google_quic_params && !rhs.google_quic_params)) { + (preferred_address && !rhs.preferred_address)) { return false; } - bool address = true; - if (preferred_address && rhs.preferred_address) { - address = (*preferred_address == *rhs.preferred_address); + if (preferred_address && rhs.preferred_address && + *preferred_address != *rhs.preferred_address) { + return false; } - bool google_quic = true; - if (google_quic_params && rhs.google_quic_params) { - google_quic = (*google_quic_params == *rhs.google_quic_params); - } - return address && google_quic; + return true; } bool TransportParameters::operator!=(const TransportParameters& rhs) const { @@ -783,7 +766,6 @@ kTypeAndValueLength + // user_agent_id kTypeAndValueLength + // support_handshake_done kTypeAndValueLength + // key_update_not_yet_supported - kTypeAndValueLength + // google kTypeAndValueLength + // google-version kGreaseParameterLength; // GREASE @@ -805,11 +787,6 @@ for (const auto& kv : in.custom_parameters) { max_transport_param_length += kTypeAndValueLength + kv.second.length(); } - // Google-specific non-standard parameter. - if (in.google_quic_params) { - max_transport_param_length += - in.google_quic_params->GetSerialized().length(); - } out->resize(max_transport_param_length); QuicDataWriter writer(out->size(), reinterpret_cast<char*>(out->data())); @@ -1027,20 +1004,6 @@ } } - // Google-specific non-standard parameter. - if (in.google_quic_params) { - const QuicData& serialized_google_quic_params = - in.google_quic_params->GetSerialized(); - if (!WriteTransportParameterId( - &writer, TransportParameters::kGoogleQuicParam, version) || - !WriteTransportParameterStringPiece( - &writer, serialized_google_quic_params.AsStringPiece(), version)) { - QUIC_BUG << "Failed to write Google params of length " - << serialized_google_quic_params.length() << " for " << in; - return false; - } - } - // Google-specific version extension. static_assert(sizeof(QuicVersionLabel) == sizeof(uint32_t), "bad length"); uint64_t google_version_length = sizeof(in.version); @@ -1402,14 +1365,6 @@ } out->key_update_not_yet_supported = true; break; - case TransportParameters::kGoogleQuicParam: { - if (out->google_quic_params) { - *error_details = "Received a second Google parameter"; - return false; - } - out->google_quic_params = - CryptoFramer::ParseMessage(value_reader.ReadRemainingPayload()); - } break; case TransportParameters::kGoogleQuicVersion: { if (!value_reader.ReadUInt32(&out->version)) { *error_details = "Failed to read Google version extension version";
diff --git a/quic/core/crypto/transport_parameters.h b/quic/core/crypto/transport_parameters.h index 8034dd1..bf8c607 100644 --- a/quic/core/crypto/transport_parameters.h +++ b/quic/core/crypto/transport_parameters.h
@@ -8,13 +8,14 @@ #include <memory> #include <vector> -#include "net/third_party/quiche/src/quic/core/crypto/crypto_handshake_message.h" #include "net/third_party/quiche/src/quic/core/quic_connection_id.h" #include "net/third_party/quiche/src/quic/core/quic_data_reader.h" #include "net/third_party/quiche/src/quic/core/quic_data_writer.h" +#include "net/third_party/quiche/src/quic/core/quic_tag.h" #include "net/third_party/quiche/src/quic/core/quic_types.h" #include "net/third_party/quiche/src/quic/core/quic_versions.h" #include "net/third_party/quiche/src/quic/platform/api/quic_containers.h" +#include "net/third_party/quiche/src/quic/platform/api/quic_socket_address.h" #include "net/third_party/quiche/src/common/platform/api/quiche_optional.h" #include "net/third_party/quiche/src/common/platform/api/quiche_string_piece.h" @@ -213,11 +214,6 @@ // yet been implemented. This will be removed once we implement it. bool key_update_not_yet_supported; - // Transport parameters used by Google QUIC but not IETF QUIC. This is - // serialized into a TransportParameter struct with a TransportParameterId of - // kGoogleQuicParamId. - std::unique_ptr<CryptoHandshakeMessage> google_quic_params; - // Validates whether transport parameters are valid according to // the specification. If the transport parameters are not valid, this method // will write a human-readable error message to |error_details|.
diff --git a/quic/core/crypto/transport_parameters_test.cc b/quic/core/crypto/transport_parameters_test.cc index a87ff1b..0578e29 100644 --- a/quic/core/crypto/transport_parameters_test.cc +++ b/quic/core/crypto/transport_parameters_test.cc
@@ -189,27 +189,6 @@ EXPECT_TRUE(orig_params == new_params); EXPECT_FALSE(orig_params != new_params); - // Test comparison on CryptoHandshakeMessage. - orig_params.google_quic_params = std::make_unique<CryptoHandshakeMessage>(); - const std::string kTestString = "test string"; - orig_params.google_quic_params->SetStringPiece(42, kTestString); - const uint32_t kTestValue = 12; - orig_params.google_quic_params->SetValue(1337, kTestValue); - EXPECT_NE(orig_params, new_params); - EXPECT_FALSE(orig_params == new_params); - EXPECT_TRUE(orig_params != new_params); - - new_params.google_quic_params = std::make_unique<CryptoHandshakeMessage>(); - new_params.google_quic_params->SetStringPiece(42, kTestString); - new_params.google_quic_params->SetValue(1337, kTestValue + 1); - EXPECT_NE(orig_params, new_params); - EXPECT_FALSE(orig_params == new_params); - EXPECT_TRUE(orig_params != new_params); - new_params.google_quic_params->SetValue(1337, kTestValue); - EXPECT_EQ(orig_params, new_params); - EXPECT_TRUE(orig_params == new_params); - EXPECT_FALSE(orig_params != new_params); - // Test comparison on CustomMap orig_params.custom_parameters[kCustomParameter1] = kCustomParameter1Value; orig_params.custom_parameters[kCustomParameter2] = kCustomParameter2Value; @@ -1246,42 +1225,6 @@ EmptyQuicConnectionId()); } -TEST_P(TransportParametersTest, CryptoHandshakeMessageRoundtrip) { - TransportParameters orig_params; - orig_params.perspective = Perspective::IS_CLIENT; - orig_params.version = kFakeVersionLabel; - orig_params.max_udp_payload_size.set_value(kMaxPacketSizeForTest); - - orig_params.google_quic_params = std::make_unique<CryptoHandshakeMessage>(); - const std::string kTestString = "test string"; - orig_params.google_quic_params->SetStringPiece(42, kTestString); - const uint32_t kTestValue = 12; - orig_params.google_quic_params->SetValue(1337, kTestValue); - - std::vector<uint8_t> serialized; - ASSERT_TRUE(SerializeTransportParameters(version_, orig_params, &serialized)); - - TransportParameters new_params; - std::string error_details; - ASSERT_TRUE(ParseTransportParameters(version_, Perspective::IS_CLIENT, - serialized.data(), serialized.size(), - &new_params, &error_details)) - << error_details; - EXPECT_TRUE(error_details.empty()); - ASSERT_NE(new_params.google_quic_params.get(), nullptr); - EXPECT_EQ(new_params.google_quic_params->tag(), - orig_params.google_quic_params->tag()); - quiche::QuicheStringPiece test_string; - EXPECT_TRUE(new_params.google_quic_params->GetStringPiece(42, &test_string)); - EXPECT_EQ(test_string, kTestString); - uint32_t test_value; - EXPECT_THAT(new_params.google_quic_params->GetUint32(1337, &test_value), - IsQuicNoError()); - EXPECT_EQ(test_value, kTestValue); - EXPECT_EQ(kFakeVersionLabel, new_params.version); - EXPECT_EQ(kMaxPacketSizeForTest, new_params.max_udp_payload_size.value()); -} - TEST_P(TransportParametersTest, VeryLongCustomParameter) { // Ensure we can handle a 70KB custom parameter on both send and receive. size_t custom_value_length = 70000;
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc index 189deac..706ee2d 100644 --- a/quic/core/http/quic_spdy_session_test.cc +++ b/quic/core/http/quic_spdy_session_test.cc
@@ -109,8 +109,7 @@ EXPECT_TRUE( session()->config()->FillTransportParameters(&transport_parameters)); error = session()->config()->ProcessTransportParameters( - transport_parameters, CLIENT, /* is_resumption = */ false, - &error_details); + transport_parameters, /* is_resumption = */ false, &error_details); } else { CryptoHandshakeMessage msg; session()->config()->ToHandshakeMessage(&msg, transport_version());
diff --git a/quic/core/http/quic_spdy_stream_test.cc b/quic/core/http/quic_spdy_stream_test.cc index c9ed83c..81de165 100644 --- a/quic/core/http/quic_spdy_stream_test.cc +++ b/quic/core/http/quic_spdy_stream_test.cc
@@ -95,8 +95,7 @@ EXPECT_TRUE( session()->config()->FillTransportParameters(&transport_parameters)); error = session()->config()->ProcessTransportParameters( - transport_parameters, CLIENT, /* is_resumption = */ false, - &error_details); + transport_parameters, /* is_resumption = */ false, &error_details); } else { CryptoHandshakeMessage msg; session()->config()->ToHandshakeMessage(&msg, transport_version());
diff --git a/quic/core/quic_config.cc b/quic/core/quic_config.cc index 0fd26a9..23597e2 100644 --- a/quic/core/quic_config.cc +++ b/quic/core/quic_config.cc
@@ -1228,17 +1228,6 @@ params->google_connection_options = connection_options_.GetSendValues(); } - if (!GetQuicRestartFlag(quic_google_transport_param_omit_old)) { - if (!params->google_quic_params) { - params->google_quic_params = std::make_unique<CryptoHandshakeMessage>(); - } - initial_round_trip_time_us_.ToHandshakeMessage( - params->google_quic_params.get()); - connection_options_.ToHandshakeMessage(params->google_quic_params.get()); - } else { - QUIC_RESTART_FLAG_COUNT_N(quic_google_transport_param_omit_old, 1, 3); - } - if (GetQuicReloadableFlag(quic_send_key_update_not_yet_supported)) { QUIC_RELOADABLE_FLAG_COUNT(quic_send_key_update_not_yet_supported); params->key_update_not_yet_supported = true; @@ -1251,7 +1240,6 @@ QuicErrorCode QuicConfig::ProcessTransportParameters( const TransportParameters& params, - HelloType hello_type, bool is_resumption, std::string* error_details) { if (!is_resumption && params.original_destination_connection_id.has_value()) { @@ -1377,26 +1365,6 @@ params.google_connection_options.value()); } - if (!GetQuicRestartFlag(quic_google_transport_param_omit_old)) { - const CryptoHandshakeMessage* peer_params = params.google_quic_params.get(); - if (peer_params != nullptr && !google_params_already_parsed) { - QuicErrorCode error = initial_round_trip_time_us_.ProcessPeerHello( - *peer_params, hello_type, error_details); - if (error != QUIC_NO_ERROR) { - DCHECK(!error_details->empty()); - return error; - } - error = connection_options_.ProcessPeerHello(*peer_params, hello_type, - error_details); - if (error != QUIC_NO_ERROR) { - DCHECK(!error_details->empty()); - return error; - } - } - } else { - QUIC_RESTART_FLAG_COUNT_N(quic_google_transport_param_omit_old, 2, 3); - } - received_custom_transport_parameters_ = params.custom_parameters; if (!is_resumption) {
diff --git a/quic/core/quic_config.h b/quic/core/quic_config.h index 1c329dc..4132987 100644 --- a/quic/core/quic_config.h +++ b/quic/core/quic_config.h
@@ -485,14 +485,11 @@ // if something prevents them from being written (e.g. a value is too large). bool FillTransportParameters(TransportParameters* params) const; - // ProcessTransportParameters reads from |params| which was received from a - // peer operating as a |hello_type|. It processes values for ICSL, MIDS, CFCW, - // and SFCW and sets the corresponding members of this QuicConfig. - // If |is_resumption|, some configs will not be processed. + // ProcessTransportParameters reads from |params| which were received from a + // peer. If |is_resumption|, some configs will not be processed. // On failure, it returns a QuicErrorCode and puts a detailed error in // |*error_details|. QuicErrorCode ProcessTransportParameters(const TransportParameters& params, - HelloType hello_type, bool is_resumption, std::string* error_details);
diff --git a/quic/core/quic_config_test.cc b/quic/core/quic_config_test.cc index 9a4d951..dad2423 100644 --- a/quic/core/quic_config_test.cc +++ b/quic/core/quic_config_test.cc
@@ -422,7 +422,7 @@ std::string error_details = "foobar"; EXPECT_THAT(config_.ProcessTransportParameters( - params, SERVER, /* is_resumption = */ false, &error_details), + params, /* is_resumption = */ false, &error_details), IsQuicNoError()); EXPECT_EQ("", error_details); EXPECT_EQ(quic::QuicTime::Delta::FromSeconds(60), @@ -441,14 +441,14 @@ params.min_ack_delay_us.set_value(25 * kNumMicrosPerMilli + 1); std::string error_details = "foobar"; EXPECT_THAT(config_.ProcessTransportParameters( - params, SERVER, /* is_resumption = */ false, &error_details), + params, /* is_resumption = */ false, &error_details), IsError(IETF_QUIC_PROTOCOL_VIOLATION)); EXPECT_EQ("MinAckDelay is greater than MaxAckDelay.", error_details); params.max_ack_delay.set_value(25 /*ms*/); params.min_ack_delay_us.set_value(25 * kNumMicrosPerMilli); EXPECT_THAT(config_.ProcessTransportParameters( - params, SERVER, /* is_resumption = */ false, &error_details), + params, /* is_resumption = */ false, &error_details), IsQuicNoError()); EXPECT_TRUE(error_details.empty()); } @@ -536,7 +536,7 @@ std::string error_details; EXPECT_THAT(config_.ProcessTransportParameters( - params, SERVER, /* is_resumption = */ true, &error_details), + params, /* is_resumption = */ true, &error_details), IsQuicNoError()) << error_details; @@ -596,7 +596,7 @@ params.support_handshake_done = true; EXPECT_THAT(config_.ProcessTransportParameters( - params, SERVER, /* is_resumption = */ false, &error_details), + params, /* is_resumption = */ false, &error_details), IsQuicNoError()) << error_details; @@ -669,7 +669,7 @@ params.disable_active_migration = true; std::string error_details; EXPECT_THAT(config_.ProcessTransportParameters( - params, SERVER, /* is_resumption = */ false, &error_details), + params, /* is_resumption = */ false, &error_details), IsQuicNoError()); EXPECT_TRUE(config_.DisableConnectionMigration()); }
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc index cb55eb0..73c371d 100644 --- a/quic/core/quic_session.cc +++ b/quic/core/quic_session.cc
@@ -1680,13 +1680,7 @@ const TransportParameters& params, bool is_resumption, std::string* error_details) { - HelloType hello_type; - if (perspective_ == Perspective::IS_CLIENT) { - hello_type = SERVER; - } else { - hello_type = CLIENT; - } - return config_.ProcessTransportParameters(params, hello_type, is_resumption, + return config_.ProcessTransportParameters(params, is_resumption, error_details); }
diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc index e8dbce5..f9dd5ac 100644 --- a/quic/core/quic_session_test.cc +++ b/quic/core/quic_session_test.cc
@@ -96,8 +96,7 @@ EXPECT_TRUE( session()->config()->FillTransportParameters(&transport_parameters)); error = session()->config()->ProcessTransportParameters( - transport_parameters, CLIENT, /* is_resumption = */ false, - &error_details); + transport_parameters, /* is_resumption = */ false, &error_details); } else { CryptoHandshakeMessage msg; session()->config()->ToHandshakeMessage(&msg, transport_version());
diff --git a/quic/core/tls_client_handshaker.cc b/quic/core/tls_client_handshaker.cc index 5465fc6..093e2dc 100644 --- a/quic/core/tls_client_handshaker.cc +++ b/quic/core/tls_client_handshaker.cc
@@ -206,9 +206,6 @@ if (!user_agent_id_.empty()) { params.user_agent_id = user_agent_id_; } - if (!GetQuicRestartFlag(quic_google_transport_param_omit_old)) { - params.google_quic_params->SetStringPiece(kUAID, user_agent_id_); - } // Notify QuicConnectionDebugVisitor. session()->connection()->OnTransportParametersSent(params);
diff --git a/quic/core/tls_server_handshaker.cc b/quic/core/tls_server_handshaker.cc index e8024e0..66d8cc3 100644 --- a/quic/core/tls_server_handshaker.cc +++ b/quic/core/tls_server_handshaker.cc
@@ -303,16 +303,9 @@ return false; } ProcessAdditionalTransportParameters(client_params); - if (!session()->user_agent_id().has_value()) { - if (client_params.user_agent_id.has_value()) { - session()->SetUserAgentId(client_params.user_agent_id.value()); - } else if (client_params.google_quic_params) { - quiche::QuicheStringPiece user_agent_id; - client_params.google_quic_params->GetStringPiece(kUAID, &user_agent_id); - if (!user_agent_id.empty()) { - session()->SetUserAgentId(user_agent_id.data()); - } - } + if (!session()->user_agent_id().has_value() && + client_params.user_agent_id.has_value()) { + session()->SetUserAgentId(client_params.user_agent_id.value()); } return true;