In quic, use transport params to announce the support of handshake_done frame. protected by gfe2_reloadable_flag_quic_support_handshake_done_in_t050. PiperOrigin-RevId: 316678561 Change-Id: Ice2b7d65a0422bdab383e84ed75c7edc31488a53
diff --git a/quic/core/crypto/transport_parameters.cc b/quic/core/crypto/transport_parameters.cc index f6d03b5..330a14d 100644 --- a/quic/core/crypto/transport_parameters.cc +++ b/quic/core/crypto/transport_parameters.cc
@@ -56,6 +56,7 @@ kInitialRoundTripTime = 0x3127, kGoogleConnectionOptions = 0x3128, kGoogleUserAgentId = 0x3129, + kGoogleSupportHandshakeDone = 0x312A, // Only used in T050. kGoogleQuicParam = 18257, // Used for non-standard Google-specific params. kGoogleQuicVersion = 18258, // Used to transmit version and supported_versions. @@ -120,6 +121,8 @@ return "google_connection_options"; case TransportParameters::kGoogleUserAgentId: return "user_agent_id"; + case TransportParameters::kGoogleSupportHandshakeDone: + return "support_handshake_done"; case TransportParameters::kGoogleQuicParam: return "google"; case TransportParameters::kGoogleQuicVersion: @@ -152,6 +155,7 @@ case TransportParameters::kInitialRoundTripTime: case TransportParameters::kGoogleConnectionOptions: case TransportParameters::kGoogleUserAgentId: + case TransportParameters::kGoogleSupportHandshakeDone: case TransportParameters::kGoogleQuicParam: case TransportParameters::kGoogleQuicVersion: return true; @@ -465,6 +469,9 @@ rv += " " + TransportParameterIdToString(kGoogleUserAgentId) + " \"" + user_agent_id.value() + "\""; } + if (support_handshake_done) { + rv += " " + TransportParameterIdToString(kGoogleSupportHandshakeDone); + } if (google_quic_params) { rv += " " + TransportParameterIdToString(kGoogleQuicParam); } @@ -512,7 +519,8 @@ kMinActiveConnectionIdLimitTransportParam, kVarInt62MaxValue), max_datagram_frame_size(kMaxDatagramFrameSize), - initial_round_trip_time_us(kInitialRoundTripTime) + initial_round_trip_time_us(kInitialRoundTripTime), + support_handshake_done(false) // Important note: any new transport parameters must be added // to TransportParameters::AreValid, SerializeTransportParameters and // ParseTransportParameters, TransportParameters's custom copy constructor, the @@ -546,6 +554,7 @@ initial_round_trip_time_us(other.initial_round_trip_time_us), google_connection_options(other.google_connection_options), user_agent_id(other.user_agent_id), + support_handshake_done(other.support_handshake_done), custom_parameters(other.custom_parameters) { if (other.preferred_address) { preferred_address = std::make_unique<TransportParameters::PreferredAddress>( @@ -589,6 +598,7 @@ rhs.initial_round_trip_time_us.value() && google_connection_options == rhs.google_connection_options && user_agent_id == rhs.user_agent_id && + support_handshake_done == rhs.support_handshake_done && custom_parameters == rhs.custom_parameters)) { return false; } @@ -748,6 +758,7 @@ kIntegerParameterLength + // initial_round_trip_time_us kTypeAndValueLength + // google_connection_options kTypeAndValueLength + // user_agent_id + kTypeAndValueLength + // support_handshake_done kTypeAndValueLength + // google kTypeAndValueLength + // google-version kGreaseParameterLength; // GREASE @@ -969,6 +980,17 @@ } } + // Google-specific support handshake done. + if (in.support_handshake_done) { + if (!WriteTransportParameterId( + &writer, TransportParameters::kGoogleSupportHandshakeDone, + version) || + !WriteTransportParameterLength(&writer, /*length=*/0, version)) { + QUIC_BUG << "Failed to write support_handshake_done for " << in; + return false; + } + } + // Google-specific non-standard parameter. if (in.google_quic_params) { const QuicData& serialized_google_quic_params = @@ -1330,6 +1352,13 @@ } out->user_agent_id = std::string(value_reader.ReadRemainingPayload()); break; + case TransportParameters::kGoogleSupportHandshakeDone: + if (out->support_handshake_done) { + *error_details = "Received a second support_handshake_done"; + return false; + } + out->support_handshake_done = true; + break; case TransportParameters::kGoogleQuicParam: { if (out->google_quic_params) { *error_details = "Received a second Google parameter";
diff --git a/quic/core/crypto/transport_parameters.h b/quic/core/crypto/transport_parameters.h index 8baeb21..093bb09 100644 --- a/quic/core/crypto/transport_parameters.h +++ b/quic/core/crypto/transport_parameters.h
@@ -202,6 +202,9 @@ // Google-specific user agent identifier. quiche::QuicheOptional<std::string> user_agent_id; + // Google-specific handshake done support. This is only used for T050. + bool support_handshake_done; + // Transport parameters used by Google QUIC but not IETF QUIC. This is // serialized into a TransportParameter struct with a TransportParameterId of // kGoogleQuicParamId.
diff --git a/quic/core/crypto/transport_parameters_test.cc b/quic/core/crypto/transport_parameters_test.cc index 8267c8b..3991953 100644 --- a/quic/core/crypto/transport_parameters_test.cc +++ b/quic/core/crypto/transport_parameters_test.cc
@@ -44,6 +44,7 @@ const uint8_t kFakePreferredStatelessResetTokenData[16] = { 0x80, 0x81, 0x82, 0x83, 0x84, 0x85, 0x86, 0x87, 0x88, 0x89, 0x8A, 0x8B, 0x8C, 0x8D, 0x8E, 0x8F}; +const bool kFakeSupportHandshakeDone = true; const auto kCustomParameter1 = static_cast<TransportParameters::TransportParameterId>(0xffcd); @@ -281,6 +282,7 @@ orig_params.initial_round_trip_time_us.set_value(kFakeInitialRoundTripTime); orig_params.google_connection_options = CreateFakeGoogleConnectionOptions(); orig_params.user_agent_id = CreateFakeUserAgentId(); + orig_params.support_handshake_done = kFakeSupportHandshakeDone; orig_params.custom_parameters[kCustomParameter1] = kCustomParameter1Value; orig_params.custom_parameters[kCustomParameter2] = kCustomParameter2Value; @@ -313,6 +315,7 @@ orig_params.initial_round_trip_time_us.set_value(kFakeInitialRoundTripTime); orig_params.google_connection_options = CreateFakeGoogleConnectionOptions(); orig_params.user_agent_id = CreateFakeUserAgentId(); + orig_params.support_handshake_done = kFakeSupportHandshakeDone; orig_params.custom_parameters[kCustomParameter1] = kCustomParameter1Value; orig_params.custom_parameters[kCustomParameter2] = kCustomParameter2Value; @@ -487,7 +490,7 @@ TEST_P(TransportParametersTest, ParseClientParams) { // clang-format off const uint8_t kClientParamsOld[] = { - 0x00, 0x76, // length of the parameters array that follows + 0x00, 0x7A, // length of the parameters array that follows // max_idle_timeout 0x00, 0x01, // parameter id 0x00, 0x02, // length @@ -553,6 +556,9 @@ 0x31, 0x29, // parameter id 0x00, 0x08, // length 'F', 'a', 'k', 'e', 'U', 'A', 'I', 'D', // value + // support_handshake_done + 0x31, 0x2A, // parameter id + 0x00, 0x00, // value // Google version extension 0x47, 0x52, // parameter id 0x00, 0x04, // length @@ -624,6 +630,9 @@ 0x71, 0x29, // parameter id 0x08, // length 'F', 'a', 'k', 'e', 'U', 'A', 'I', 'D', // value + // support_handshake_done + 0x71, 0x2A, // parameter id + 0x00, // length // Google version extension 0x80, 0x00, 0x47, 0x52, // parameter id 0x04, // length @@ -679,6 +688,7 @@ new_params.google_connection_options.value()); ASSERT_TRUE(new_params.user_agent_id.has_value()); EXPECT_EQ(CreateFakeUserAgentId(), new_params.user_agent_id.value()); + EXPECT_TRUE(new_params.support_handshake_done); } TEST_P(TransportParametersTest, @@ -846,7 +856,7 @@ TEST_P(TransportParametersTest, ParseServerParams) { // clang-format off const uint8_t kServerParamsOld[] = { - 0x00, 0xcf, // length of parameters array that follows + 0x00, 0xd3, // length of parameters array that follows // original_destination_connection_id 0x00, 0x00, // parameter id 0x00, 0x08, // length @@ -929,6 +939,9 @@ 'A', 'L', 'P', 'N', // value 'E', 'F', 'G', 0x00, 'H', 'I', 'J', 0xff, + // support_handshake_done + 0x31, 0x2A, // parameter id + 0x00, 0x00, // value // Google version extension 0x47, 0x52, // parameter id 0x00, 0x0d, // length @@ -1020,6 +1033,9 @@ 'A', 'L', 'P', 'N', // value 'E', 'F', 'G', 0x00, 'H', 'I', 'J', 0xff, + // support_handshake_done + 0x71, 0x2A, // parameter id + 0x00, // length // Google version extension 0x80, 0x00, 0x47, 0x52, // parameter id 0x0d, // length @@ -1090,6 +1106,7 @@ EXPECT_EQ(CreateFakeGoogleConnectionOptions(), new_params.google_connection_options.value()); EXPECT_FALSE(new_params.user_agent_id.has_value()); + EXPECT_TRUE(new_params.support_handshake_done); } TEST_P(TransportParametersTest, ParseServerParametersRepeated) {
diff --git a/quic/core/http/end_to_end_test.cc b/quic/core/http/end_to_end_test.cc index 03c83d2..abe9399 100644 --- a/quic/core/http/end_to_end_test.cc +++ b/quic/core/http/end_to_end_test.cc
@@ -201,6 +201,8 @@ SetQuicReloadableFlag(quic_donot_change_queued_ack, true); SetQuicReloadableFlag(quic_fix_last_inflight_packets_sent_time, true); SetQuicReloadableFlag(quic_fix_server_pto_timeout, true); + + SetQuicReloadableFlag(quic_support_handshake_done_in_t050, true); } ~EndToEndTest() override { QuicRecyclePort(server_address_.port()); } @@ -579,7 +581,7 @@ TEST_P(EndToEndTest, HandshakeConfirmed) { ASSERT_TRUE(Initialize()); - if (!version_.HasHandshakeDone()) { + if (!version_.UsesTls()) { return; } EXPECT_EQ(kFooResponseBody, client_->SendSynchronousRequest("/foo")); @@ -1146,9 +1148,6 @@ // brutal. SetPacketLossPercentage(5); ASSERT_TRUE(Initialize()); - if (version_.UsesTls() && !version_.HasHandshakeDone()) { - return; - } EXPECT_TRUE(client_->client()->WaitForHandshakeConfirmed()); SetPacketLossPercentage(30); @@ -1168,9 +1167,6 @@ // Regression test for b/80090281. TEST_P(EndToEndTest, LargePostWithPacketLossAndAlwaysBundleWindowUpdates) { ASSERT_TRUE(Initialize()); - if (version_.UsesTls() && !version_.HasHandshakeDone()) { - return; - } EXPECT_TRUE(client_->client()->WaitForHandshakeConfirmed()); server_thread_->WaitForCryptoHandshakeConfirmed(); @@ -1202,9 +1198,6 @@ // b/10126687 is fixed, losing handshake packets is pretty brutal. SetPacketLossPercentage(5); ASSERT_TRUE(Initialize()); - if (version_.UsesTls() && !version_.HasHandshakeDone()) { - return; - } EXPECT_TRUE(client_->client()->WaitForHandshakeConfirmed()); SetPacketLossPercentage(10); client_writer_->set_fake_blocked_socket_percentage(10); @@ -1223,9 +1216,6 @@ TEST_P(EndToEndTest, LargePostNoPacketLossWithDelayAndReordering) { ASSERT_TRUE(Initialize()); - if (version_.UsesTls() && !version_.HasHandshakeDone()) { - return; - } EXPECT_TRUE(client_->client()->WaitForHandshakeConfirmed()); // Both of these must be called when the writer is not actively used. SetPacketSendDelay(QuicTime::Delta::FromMilliseconds(2)); @@ -2413,9 +2403,6 @@ // demonstrates that retransmissions do not break this functionality. SetPacketLossPercentage(5); ASSERT_TRUE(Initialize()); - if (version_.UsesTls() && !version_.HasHandshakeDone()) { - return; - } // Wait for the server SHLO before upping the packet loss. EXPECT_TRUE(client_->client()->WaitForHandshakeConfirmed()); SetPacketLossPercentage(30); @@ -3789,9 +3776,6 @@ TEST_P(EndToEndTest, ResetStreamOnTtlExpires) { ASSERT_TRUE(Initialize()); - if (version_.UsesTls() && !version_.HasHandshakeDone()) { - return; - } EXPECT_TRUE(client_->client()->WaitForHandshakeConfirmed()); SetPacketLossPercentage(30);
diff --git a/quic/core/quic_config.cc b/quic/core/quic_config.cc index fa06bcb..68638e4 100644 --- a/quic/core/quic_config.cc +++ b/quic/core/quic_config.cc
@@ -447,6 +447,7 @@ initial_stream_flow_control_window_bytes_(kSFCW, PRESENCE_OPTIONAL), initial_session_flow_control_window_bytes_(kCFCW, PRESENCE_OPTIONAL), connection_migration_disabled_(kNCMR, PRESENCE_OPTIONAL), + support_handshake_done_(0, PRESENCE_OPTIONAL), alternate_server_address_ipv6_(kASAD, PRESENCE_OPTIONAL), alternate_server_address_ipv4_(kASAD, PRESENCE_OPTIONAL), stateless_reset_token_(kSRST, PRESENCE_OPTIONAL), @@ -832,6 +833,19 @@ return connection_migration_disabled_.HasReceivedValue(); } +void QuicConfig::SetSupportHandshakeDone() { + support_handshake_done_.SetSendValue(1); +} + +bool QuicConfig::HandshakeDoneSupported() const { + return support_handshake_done_.HasSendValue() && + support_handshake_done_.GetSendValue() > 0; +} + +bool QuicConfig::PeerSupportsHandshakeDone() const { + return support_handshake_done_.HasReceivedValue(); +} + void QuicConfig::SetIPv6AlternateServerAddressToSend( const QuicSocketAddress& alternate_server_address_ipv6) { if (!alternate_server_address_ipv6.host().IsIPv6()) { @@ -1154,6 +1168,7 @@ params->disable_active_migration = connection_migration_disabled_.HasSendValue() && connection_migration_disabled_.GetSendValue() != 0; + params->support_handshake_done = HandshakeDoneSupported(); if (alternate_server_address_ipv6_.HasSendValue() || alternate_server_address_ipv4_.HasSendValue()) { @@ -1304,6 +1319,9 @@ if (params.disable_active_migration) { connection_migration_disabled_.SetReceivedValue(1u); } + if (params.support_handshake_done) { + support_handshake_done_.SetReceivedValue(1u); + } active_connection_id_limit_.SetReceivedValue( params.active_connection_id_limit.value());
diff --git a/quic/core/quic_config.h b/quic/core/quic_config.h index 4f302b6..6f041b8 100644 --- a/quic/core/quic_config.h +++ b/quic/core/quic_config.h
@@ -382,6 +382,11 @@ void SetDisableConnectionMigration(); bool DisableConnectionMigration() const; + // Support handshake done. + void SetSupportHandshakeDone(); + bool HandshakeDoneSupported() const; + bool PeerSupportsHandshakeDone() const; + // IPv6 alternate server address. void SetIPv6AlternateServerAddressToSend( const QuicSocketAddress& alternate_server_address_ipv6); @@ -566,6 +571,10 @@ // Uses the disable_active_migration transport parameter in IETF QUIC. QuicFixedUint32 connection_migration_disabled_; + // Whether handshake done is supported. Only used in T050. + // Uses the support_handshake_done transport parameter in IETF QUIC. + QuicFixedUint32 support_handshake_done_; + // Alternate server addresses the client could connect to. // Uses the preferred_address transport parameter in IETF QUIC. // Note that when QUIC_CRYPTO is in use, only one of the addresses is sent.
diff --git a/quic/core/quic_config_test.cc b/quic/core/quic_config_test.cc index 4aa9619..16c7c56 100644 --- a/quic/core/quic_config_test.cc +++ b/quic/core/quic_config_test.cc
@@ -561,6 +561,7 @@ config_.ReceivedMaxBidirectionalStreams()); EXPECT_FALSE(config_.DisableConnectionMigration()); + EXPECT_FALSE(config_.PeerSupportsHandshakeDone()); // The following config shouldn't be processed because of resumption. EXPECT_FALSE(config_.HasReceivedStatelessResetToken()); @@ -584,6 +585,7 @@ params.initial_max_streams_bidi.set_value(2 * kDefaultMaxStreamsPerConnection); params.disable_active_migration = true; + params.support_handshake_done = true; EXPECT_THAT(config_.ProcessTransportParameters( params, SERVER, /* is_resumption = */ false, &error_details), @@ -618,6 +620,7 @@ config_.ReceivedMaxBidirectionalStreams()); EXPECT_TRUE(config_.DisableConnectionMigration()); + EXPECT_TRUE(config_.PeerSupportsHandshakeDone()); ASSERT_TRUE(config_.HasReceivedStatelessResetToken()); ASSERT_TRUE(config_.HasReceivedMaxAckDelayMs());
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc index 8e85af9..b22dc96 100644 --- a/quic/core/quic_connection.cc +++ b/quic/core/quic_connection.cc
@@ -320,7 +320,8 @@ idle_network_detector_(this, clock_->ApproximateNow(), &arena_, - alarm_factory_) { + alarm_factory_), + support_handshake_done_(version().HasHandshakeDone()) { QUIC_DLOG(INFO) << ENDPOINT << "Created connection with server connection ID " << server_connection_id << " and version: " << ParsedQuicVersionToString(version()); @@ -539,6 +540,9 @@ SetNetworkTimeouts(config.max_time_before_crypto_handshake(), config.max_idle_time_before_crypto_handshake()); } + if (config.HandshakeDoneSupported()) { + support_handshake_done_ = true; + } sent_packet_manager_.SetFromConfig(config); if (config.HasReceivedBytesForConnectionId() && @@ -1586,7 +1590,13 @@ } bool QuicConnection::OnHandshakeDoneFrame(const QuicHandshakeDoneFrame& frame) { - DCHECK(connected_ && VersionHasIetfQuicFrames(transport_version())); + DCHECK(connected_); + if (!support_handshake_done_) { + CloseConnection(IETF_QUIC_PROTOCOL_VIOLATION, + "Handshake done frame is unsupported", + ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); + return false; + } if (perspective_ == Perspective::IS_SERVER) { CloseConnection(IETF_QUIC_PROTOCOL_VIOLATION,
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h index 8e3608b..7339b57 100644 --- a/quic/core/quic_connection.h +++ b/quic/core/quic_connection.h
@@ -1670,6 +1670,9 @@ bool blackhole_detection_disabled_ = false; + // True if this connection supports handshake done frame. + bool support_handshake_done_; + const bool use_idle_network_detector_ = GetQuicReloadableFlag(quic_use_idle_network_detector);
diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc index 1cba589..726f1e5 100644 --- a/quic/core/quic_framer.cc +++ b/quic/core/quic_framer.cc
@@ -1013,6 +1013,9 @@ return 0; } break; + case HANDSHAKE_DONE_FRAME: + // HANDSHAKE_DONE has no payload. + break; default: RaiseError(QUIC_INVALID_FRAME_DATA); QUIC_BUG << "QUIC_INVALID_FRAME_DATA"; @@ -2986,6 +2989,19 @@ } break; } + case HANDSHAKE_DONE_FRAME: { + // HANDSHAKE_DONE has no payload. + QuicHandshakeDoneFrame handshake_done_frame; + QUIC_DVLOG(2) << ENDPOINT << "Processing handshake done frame " + << handshake_done_frame; + if (!visitor_->OnHandshakeDoneFrame(handshake_done_frame)) { + QUIC_DVLOG(1) << ENDPOINT + << "Visitor asked to stop further processing."; + // Returning true since there was no parsing error. + return true; + } + break; + } default: set_detailed_error("Illegal frame type.");
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc index 763b291..ad7fce3 100644 --- a/quic/core/quic_session.cc +++ b/quic/core/quic_session.cc
@@ -118,7 +118,11 @@ connection_->SetSessionNotifier(this); connection_->SetDataProducer(this); connection_->SetFromConfig(config_); - + if (GetQuicReloadableFlag(quic_support_handshake_done_in_t050) && + version().UsesTls() && !version().HasHandshakeDone() && + perspective_ == Perspective::IS_CLIENT) { + config_.SetSupportHandshakeDone(); + } // On the server side, version negotiation has been done by the dispatcher, // and the server session is created with the right version. if (perspective() == Perspective::IS_SERVER) { @@ -1545,8 +1549,13 @@ << ENDPOINT << "Handshake completes without cipher suite negotiation."; QUIC_BUG_IF(!config_.negotiated()) << ENDPOINT << "Handshake completes without parameter negotiation."; - if (connection()->version().HasHandshakeDone() && + if ((connection()->version().HasHandshakeDone() || + (GetQuicReloadableFlag(quic_support_handshake_done_in_t050) && + config_.PeerSupportsHandshakeDone())) && perspective_ == Perspective::IS_SERVER) { + if (!connection()->version().HasHandshakeDone()) { + QUIC_RELOADABLE_FLAG_COUNT(quic_support_handshake_done_in_t050); + } // Server sends HANDSHAKE_DONE to signal confirmation of the handshake // to the client. control_frame_manager_.WriteOrBufferHandshakeDone();
diff --git a/quic/core/quic_types.h b/quic/core/quic_types.h index 753f86e..39e1a87 100644 --- a/quic/core/quic_types.h +++ b/quic/core/quic_types.h
@@ -227,6 +227,8 @@ STOP_WAITING_FRAME = 6, PING_FRAME = 7, CRYPTO_FRAME = 8, + // TODO(b/157935330): stop hard coding this when deprecate T050. + HANDSHAKE_DONE_FRAME = 9, // STREAM and ACK frames are special frames. They are encoded differently on // the wire and their values do not need to be stable. @@ -248,7 +250,6 @@ MESSAGE_FRAME, NEW_TOKEN_FRAME, RETIRE_CONNECTION_ID_FRAME, - HANDSHAKE_DONE_FRAME, NUM_FRAME_TYPES };
diff --git a/quic/test_tools/quic_config_peer.cc b/quic/test_tools/quic_config_peer.cc index b380642..52325d1 100644 --- a/quic/test_tools/quic_config_peer.cc +++ b/quic/test_tools/quic_config_peer.cc
@@ -132,5 +132,10 @@ config->max_datagram_frame_size_.SetReceivedValue(max_datagram_frame_size); } +// static +void QuicConfigPeer::DisableSupportHandshakeDone(QuicConfig* config) { + config->support_handshake_done_.SetSendValue(0); +} + } // namespace test } // namespace quic
diff --git a/quic/test_tools/quic_config_peer.h b/quic/test_tools/quic_config_peer.h index a351907..109bd64 100644 --- a/quic/test_tools/quic_config_peer.h +++ b/quic/test_tools/quic_config_peer.h
@@ -76,6 +76,7 @@ static void SetReceivedMaxDatagramFrameSize(QuicConfig* config, uint64_t max_datagram_frame_size); + static void DisableSupportHandshakeDone(QuicConfig* config); }; } // namespace test