Deprecate quic_abstract_connection_id_generator. Deletes a lot of code! PiperOrigin-RevId: 481146465
diff --git a/quiche/quic/core/quic_dispatcher.cc b/quiche/quic/core/quic_dispatcher.cc index e82ddce..da0af71 100644 --- a/quiche/quic/core/quic_dispatcher.cc +++ b/quiche/quic/core/quic_dispatcher.cc
@@ -359,68 +359,6 @@ ProcessHeader(&packet_info); } -absl::optional<QuicConnectionId> QuicDispatcher::MaybeReplaceServerConnectionId( - const QuicConnectionId& server_connection_id, - const ParsedQuicVersion& version) { - if (GetQuicRestartFlag(quic_abstract_connection_id_generator)) { - // If the Dispatcher doesn't map the original connection ID, then using a - // connection ID generator that isn't deterministic may break the handshake - // and will certainly drop all 0-RTT packets. - QUIC_RESTART_FLAG_COUNT(quic_abstract_connection_id_generator); - return connection_id_generator_.MaybeReplaceConnectionId( - server_connection_id, version); - } - const uint8_t server_connection_id_length = server_connection_id.length(); - if (server_connection_id_length == expected_server_connection_id_length_) { - return absl::optional<QuicConnectionId>(); - } - QUICHE_DCHECK(version.AllowsVariableLengthConnectionIds()); - QuicConnectionId new_connection_id; - if (server_connection_id_length < expected_server_connection_id_length_) { - new_connection_id = ReplaceShortServerConnectionId( - version, server_connection_id, expected_server_connection_id_length_); - // Verify that ReplaceShortServerConnectionId is deterministic. - QUICHE_DCHECK_EQ( - new_connection_id, - ReplaceShortServerConnectionId(version, server_connection_id, - expected_server_connection_id_length_)); - } else { - new_connection_id = ReplaceLongServerConnectionId( - version, server_connection_id, expected_server_connection_id_length_); - // Verify that ReplaceLongServerConnectionId is deterministic. - QUICHE_DCHECK_EQ( - new_connection_id, - ReplaceLongServerConnectionId(version, server_connection_id, - expected_server_connection_id_length_)); - } - QUICHE_DCHECK_EQ(expected_server_connection_id_length_, - new_connection_id.length()); - - QUIC_DLOG(INFO) << "Replacing incoming connection ID " << server_connection_id - << " with " << new_connection_id; - return new_connection_id; -} - -QuicConnectionId QuicDispatcher::ReplaceShortServerConnectionId( - const ParsedQuicVersion& /*version*/, - const QuicConnectionId& server_connection_id, - uint8_t expected_server_connection_id_length) const { - QUICHE_DCHECK_LT(server_connection_id.length(), - expected_server_connection_id_length); - return QuicUtils::CreateReplacementConnectionId( - server_connection_id, expected_server_connection_id_length); -} - -QuicConnectionId QuicDispatcher::ReplaceLongServerConnectionId( - const ParsedQuicVersion& /*version*/, - const QuicConnectionId& server_connection_id, - uint8_t expected_server_connection_id_length) const { - QUICHE_DCHECK_GT(server_connection_id.length(), - expected_server_connection_id_length); - return QuicUtils::CreateReplacementConnectionId( - server_connection_id, expected_server_connection_id_length); -} - namespace { constexpr bool IsSourceUdpPortBlocked(uint16_t port) { // These UDP source ports have been observed in large scale denial of service @@ -1275,7 +1213,8 @@ const QuicSocketAddress self_address, const QuicSocketAddress peer_address) { absl::optional<QuicConnectionId> server_connection_id = - MaybeReplaceServerConnectionId(original_connection_id, version); + connection_id_generator_.MaybeReplaceConnectionId(original_connection_id, + version); const bool replaced_connection_id = server_connection_id.has_value(); if (!replaced_connection_id) { server_connection_id = original_connection_id;
diff --git a/quiche/quic/core/quic_dispatcher.h b/quiche/quic/core/quic_dispatcher.h index 9cfc3b0..a8e6e72 100644 --- a/quiche/quic/core/quic_dispatcher.h +++ b/quiche/quic/core/quic_dispatcher.h
@@ -181,31 +181,6 @@ // otherwise, returns false and the packet needs further processing. virtual bool MaybeDispatchPacket(const ReceivedPacketInfo& packet_info); - // Generate a connection ID with a length that is expected by the dispatcher. - // Called only when |server_connection_id| is shorter than - // |expected_connection_id_length|. - // Note that this MUST produce a deterministic result (calling this method - // with two connection IDs that are equal must produce the same result). - // Note that this is not used in general operation because our default - // |expected_server_connection_id_length| is 8, and the IETF specification - // requires clients to use an initial length of at least 8. However, we - // allow disabling that requirement via - // |allow_short_initial_server_connection_ids_|. - virtual QuicConnectionId ReplaceShortServerConnectionId( - const ParsedQuicVersion& version, - const QuicConnectionId& server_connection_id, - uint8_t expected_server_connection_id_length) const; - - // Generate a connection ID with a length that is expected by the dispatcher. - // Called only when |server_connection_id| is longer than - // |expected_connection_id_length|. - // Note that this MUST produce a deterministic result (calling this method - // with two connection IDs that are equal must produce the same result). - virtual QuicConnectionId ReplaceLongServerConnectionId( - const ParsedQuicVersion& version, - const QuicConnectionId& server_connection_id, - uint8_t expected_server_connection_id_length) const; - // Values to be returned by ValidityChecks() to indicate what should be done // with a packet. Fates with greater values are considered to be higher // priority. ValidityChecks should return fate based on the priority order @@ -342,11 +317,6 @@ // element of the vector is returned. std::string SelectAlpn(const std::vector<std::string>& alpns); - // Check if the client-generated server connection ID needs to be replaced. - absl::optional<QuicConnectionId> MaybeReplaceServerConnectionId( - const QuicConnectionId& server_connection_id, - const ParsedQuicVersion& version); - // Sends public/stateless reset packets with no version and unknown // connection ID according to the packet's size. virtual void MaybeResetPacketsWithNoVersion(
diff --git a/quiche/quic/core/quic_dispatcher_test.cc b/quiche/quic/core/quic_dispatcher_test.cc index ed5ea0c..ffa0b22 100644 --- a/quiche/quic/core/quic_dispatcher_test.cc +++ b/quiche/quic/core/quic_dispatcher_test.cc
@@ -63,6 +63,9 @@ namespace test { namespace { +const QuicConnectionId kReturnConnectionId{ + {0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07}}; + class TestQuicSpdyServerSession : public QuicServerSessionBase { public: TestQuicSpdyServerSession(const QuicConfig& config, @@ -432,8 +435,7 @@ const QuicConnectionId& server_connection_id, const QuicConnectionId& client_connection_id, std::unique_ptr<QuicCryptoClientConfig> client_crypto_config) { - if (expect_generator_is_called_ && - GetQuicRestartFlag(quic_abstract_connection_id_generator)) { + if (expect_generator_is_called_) { // Can't replace the connection ID in early gQUIC versions! ASSERT_TRUE(version.AllowsVariableLengthConnectionIds() || generated_connection_id_ == absl::nullopt); @@ -533,8 +535,6 @@ bool expect_generator_is_called_ = true; absl::optional<QuicConnectionId> generated_connection_id_; // Constant to set generated_connection_id to when needed. - QuicConnectionId return_connection_id_{ - {0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07}}; MockConnectionIdGenerator connection_id_generator_; std::unique_ptr<NiceMock<TestDispatcher>> dispatcher_; MockTimeWaitListManager* time_wait_list_manager_; @@ -594,26 +594,18 @@ QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 1); QuicConnectionId original_connection_id, new_connection_id; if (long_connection_id) { - if (GetQuicRestartFlag(quic_abstract_connection_id_generator)) { - original_connection_id = TestConnectionIdNineBytesLong(1); - new_connection_id = return_connection_id_; - EXPECT_CALL(connection_id_generator_, - MaybeReplaceConnectionId(original_connection_id, version_)) - .WillOnce(Return(new_connection_id)); - } else { - original_connection_id = QuicConnectionId( - {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09}); - new_connection_id = - QuicConnectionId({0x6c, 0x6b, 0x4b, 0xad, 0x8d, 0x00, 0x24, 0xd8}); - } + original_connection_id = TestConnectionIdNineBytesLong(1); + new_connection_id = kReturnConnectionId; + EXPECT_CALL(connection_id_generator_, + MaybeReplaceConnectionId(original_connection_id, version_)) + .WillOnce(Return(new_connection_id)); + } else { original_connection_id = TestConnectionId(); new_connection_id = original_connection_id; - if (GetQuicRestartFlag(quic_abstract_connection_id_generator)) { - EXPECT_CALL(connection_id_generator_, - MaybeReplaceConnectionId(original_connection_id, version_)) - .WillOnce(Return(absl::nullopt)); - } + EXPECT_CALL(connection_id_generator_, + MaybeReplaceConnectionId(original_connection_id, version_)) + .WillOnce(Return(absl::nullopt)); } QuicConfig client_config = DefaultQuicConfig(); // Add a 2000-byte custom parameter to increase the length of the CHLO. @@ -1037,10 +1029,7 @@ QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 1); QuicConnectionId bad_connection_id = TestConnectionIdNineBytesLong(2); - generated_connection_id_ = - (GetQuicRestartFlag(quic_abstract_connection_id_generator)) - ? return_connection_id_ - : QuicUtils::CreateReplacementConnectionId(bad_connection_id); + generated_connection_id_ = kReturnConnectionId; EXPECT_CALL(*dispatcher_, CreateQuicSession(*generated_connection_id_, _, client_address, @@ -1068,10 +1057,7 @@ QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 1); QuicConnectionId bad_connection_id = EmptyQuicConnectionId(); - generated_connection_id_ = - GetQuicRestartFlag(quic_abstract_connection_id_generator) - ? return_connection_id_ - : QuicUtils::CreateReplacementConnectionId(bad_connection_id); + generated_connection_id_ = kReturnConnectionId; // Disable validation of invalid short connection IDs. dispatcher_->SetAllowShortInitialServerConnectionIds(true); @@ -1118,10 +1104,7 @@ }))); ProcessFirstFlight(client_address, TestConnectionId(1)); - generated_connection_id_ = - GetQuicRestartFlag(quic_abstract_connection_id_generator) - ? return_connection_id_ - : QuicUtils::CreateReplacementConnectionId(bad_connection_id); + generated_connection_id_ = kReturnConnectionId; EXPECT_CALL(*dispatcher_, CreateQuicSession(*generated_connection_id_, _, client_address, Eq(ExpectedAlpn()), _, _)) @@ -2258,12 +2241,12 @@ dispatcher_->OnConnectionClosed(TestConnectionId(2), QuicErrorCode::QUIC_NO_ERROR, "detail", quic::ConnectionCloseSource::FROM_SELF); - // QUICHE_BUG fires when connection1 tries to remove TestConnectionId(2) - // again from the session_map. - EXPECT_QUICHE_BUG(dispatcher_->OnConnectionClosed( - TestConnectionId(1), QuicErrorCode::QUIC_NO_ERROR, - "detail", quic::ConnectionCloseSource::FROM_SELF), - "Missing session for cid"); + // QUICHE_BUG fires when connection1 tries to remove TestConnectionId(2) + // again from the session_map. + EXPECT_QUICHE_BUG(dispatcher_->OnConnectionClosed( + TestConnectionId(1), QuicErrorCode::QUIC_NO_ERROR, + "detail", quic::ConnectionCloseSource::FROM_SELF), + "Missing session for cid"); } TEST_P(QuicDispatcherSupportMultipleConnectionIdPerConnectionTest, @@ -2452,11 +2435,9 @@ // When CHLO arrives, a new session should be created, and all packets // buffered should be delivered to the session. - if (GetQuicRestartFlag(quic_abstract_connection_id_generator)) { - EXPECT_CALL(connection_id_generator_, - MaybeReplaceConnectionId(conn_id, version_)) - .WillOnce(Return(absl::nullopt)); - } + EXPECT_CALL(connection_id_generator_, + MaybeReplaceConnectionId(conn_id, version_)) + .WillOnce(Return(absl::nullopt)); EXPECT_CALL(*dispatcher_, CreateQuicSession(conn_id, _, client_addr_, Eq(ExpectedAlpn()), _, Eq(ParsedClientHelloForTest()))) @@ -2490,11 +2471,9 @@ data_connection_map_[conn_id].pop_back(); // When CHLO arrives, a new session should be created, and all packets // buffered should be delivered to the session. - if (GetQuicRestartFlag(quic_abstract_connection_id_generator)) { - EXPECT_CALL(connection_id_generator_, - MaybeReplaceConnectionId(conn_id, version_)) - .WillOnce(Return(absl::nullopt)); - } + EXPECT_CALL(connection_id_generator_, + MaybeReplaceConnectionId(conn_id, version_)) + .WillOnce(Return(absl::nullopt)); EXPECT_CALL(*dispatcher_, CreateQuicSession(conn_id, _, client_addr_, Eq(ExpectedAlpn()), _, _)) .WillOnce(Return(ByMove(CreateSession( @@ -2540,11 +2519,9 @@ for (size_t i = 1; i <= kNumConnections; ++i) { QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 20000 + i); QuicConnectionId conn_id = TestConnectionId(i); - if (GetQuicRestartFlag(quic_abstract_connection_id_generator)) { - EXPECT_CALL(connection_id_generator_, - MaybeReplaceConnectionId(conn_id, version_)) - .WillOnce(Return(absl::nullopt)); - } + EXPECT_CALL(connection_id_generator_, + MaybeReplaceConnectionId(conn_id, version_)) + .WillOnce(Return(absl::nullopt)); EXPECT_CALL(*dispatcher_, CreateQuicSession(conn_id, _, client_address, Eq(ExpectedAlpn()), _, _)) .WillOnce(Return(ByMove(CreateSession( @@ -2591,11 +2568,9 @@ // When CHLO arrives, a new session should be created, and all packets // buffered should be delivered to the session. - if (GetQuicRestartFlag(quic_abstract_connection_id_generator)) { - EXPECT_CALL(connection_id_generator_, - MaybeReplaceConnectionId(conn_id, version_)) - .WillOnce(Return(absl::nullopt)); - } + EXPECT_CALL(connection_id_generator_, + MaybeReplaceConnectionId(conn_id, version_)) + .WillOnce(Return(absl::nullopt)); EXPECT_CALL(*dispatcher_, CreateQuicSession(conn_id, _, client_addr_, Eq(ExpectedAlpn()), _, _)) .Times(1) // Only triggered by 1st CHLO. @@ -2661,12 +2636,9 @@ kMaxNumSessionsToCreate + kDefaultMaxConnectionsInStore + 1; for (uint64_t conn_id = 1; conn_id <= kNumCHLOs; ++conn_id) { if (conn_id <= kMaxNumSessionsToCreate) { - if (GetQuicRestartFlag(quic_abstract_connection_id_generator)) { - EXPECT_CALL( - connection_id_generator_, - MaybeReplaceConnectionId(TestConnectionId(conn_id), version_)) - .WillOnce(Return(absl::nullopt)); - } + EXPECT_CALL(connection_id_generator_, + MaybeReplaceConnectionId(TestConnectionId(conn_id), version_)) + .WillOnce(Return(absl::nullopt)); EXPECT_CALL(*dispatcher_, CreateQuicSession(TestConnectionId(conn_id), _, client_addr_, Eq(ExpectedAlpn()), _, @@ -2704,11 +2676,9 @@ for (uint64_t conn_id = kMaxNumSessionsToCreate + 1; conn_id <= kMaxNumSessionsToCreate + kDefaultMaxConnectionsInStore; ++conn_id) { - if (GetQuicRestartFlag(quic_abstract_connection_id_generator)) { - EXPECT_CALL(connection_id_generator_, - MaybeReplaceConnectionId(TestConnectionId(conn_id), version_)) - .WillOnce(Return(absl::nullopt)); - } + EXPECT_CALL(connection_id_generator_, + MaybeReplaceConnectionId(TestConnectionId(conn_id), version_)) + .WillOnce(Return(absl::nullopt)); EXPECT_CALL(*dispatcher_, CreateQuicSession(TestConnectionId(conn_id), _, client_addr_, Eq(ExpectedAlpn()), _,
diff --git a/quiche/quic/core/quic_flags_list.h b/quiche/quic/core/quic_flags_list.h index 20f5594..6078d31 100644 --- a/quiche/quic/core/quic_flags_list.h +++ b/quiche/quic/core/quic_flags_list.h
@@ -85,8 +85,6 @@ QUIC_FLAG(quic_reloadable_flag_quic_conservative_cwnd_and_pacing_gains, false) // QuicConnection uses a library to generate connection IDs QUIC_FLAG(quic_reloadable_flag_quic_connection_uses_abstract_connection_id_generator, true) -// QuicDispatcher uses a library to generate connection IDs -QUIC_FLAG(quic_restart_flag_quic_abstract_connection_id_generator, true) // When true, defaults to BBR congestion control instead of Cubic. QUIC_FLAG(quic_reloadable_flag_quic_default_to_bbr, false) // When true, support draft-ietf-quic-v2-01