Allow multiple connections from the same Peer ID (#8215)

* if the value of settings_pack::allow_multiple_connections_per_ip is true, then skip the check for the same pid

* add new setting

* No change to ABI

* Update test file

* Remove invalid tests in test_peer_list.cpp as allow_multiple_connections_per_pid has no effect on peer_list

* Restore the test file

* add new test

* add new test

* fix peer_list.hpp

* Delete the currently unnecessary fields

* Merge test code

Author: lzhzh1

Makefile

@@ -753,6 +753,7 @@ SIM_SOURCES = \
   setup_dht.hpp \
   setup_swarm.cpp \
   setup_swarm.hpp \
+  test_allow_multiple_connections_per_pid.cpp \
   test_auto_manage.cpp \
   test_checking.cpp \
   test_dht.cpp \

include/libtorrent/settings_pack.hpp

@@ -1046,6 +1046,14 @@ namespace aux {
 			// NOCOW flag on the download directory, which will cause all files
 			// created within it to have the NOCOW flag set.
 			disk_disable_copy_on_write,
+			
+			// determines if connections from the same Peer ID as existing
+			// connections should be rejected or not. Typically, we
+			// only establish a single connection with each peer. If
+			// a peer has multiple IP addresses, enabling this feature
+			// may improve transfer efficiency, but it may also
+			// increase network load.
+			allow_multiple_connections_per_pid,
 
 			max_bool_setting_internal
 		};

simulation/Jamfile

@@ -65,4 +65,5 @@ run test_save_resume.cpp ;
 run test_error_handling.cpp ;
 run test_timeout.cpp ;
 run test_peer_connection.cpp ;
+run test_allow_multiple_connections_per_pid.cpp ;
 

simulation/fake_peer.hpp

@@ -172,6 +172,13 @@ struct fake_peer
 			});
 	}
 
+	// Set a fixed peer-id to use instead of random bytes
+	void set_peer_id(lt::peer_id const& pid)
+	{
+		m_fixed_pid = pid;
+		m_use_fixed_pid = true;
+	}
+
 private:
 
 	void send_simple_msg(std::uint8_t const msg_code)
@@ -201,7 +208,14 @@ struct fake_peer
 		int const len = sizeof(handshake) - 1;
 		memcpy(m_out_buffer.data(), handshake, len);
 		memcpy(&m_out_buffer[28], ih.data(), 20);
-		lt::aux::random_bytes({&m_out_buffer[48], 20});
+		if (m_use_fixed_pid)
+		{
+			memcpy(&m_out_buffer[48], m_fixed_pid.data(), 20);
+		}
+		else
+		{
+			lt::aux::random_bytes({&m_out_buffer[48], 20});
+		}
 
 		TORRENT_ASSERT(!m_writing);
 		m_writing = true;
@@ -320,6 +334,10 @@ struct fake_peer
 	// socket
 	bool m_writing = false;
 
+	// fixed peer-id to use instead of random bytes
+	lt::peer_id m_fixed_pid;
+	bool m_use_fixed_pid = false;
+
 	std::vector<char> m_send_buffer;
 };
 

simulation/test_allow_multiple_connections_per_pid.cpp

@@ -0,0 +1,170 @@
+/*
+
+
+
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions
+are met:
+
+    * Redistributions of source code must retain the above copyright
+      notice, this list of conditions and the following disclaimer.
+    * Redistributions in binary form must reproduce the above copyright
+      notice, this list of conditions and the following disclaimer in
+      the documentation and/or other materials provided with the distribution.
+    * Neither the name of the author nor the names of its
+      contributors may be used to endorse or promote products derived
+      from this software without specific prior written permission.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
+LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+POSSIBILITY OF SUCH DAMAGE.
+
+*/
+
+#include "libtorrent/session.hpp"
+#include "libtorrent/torrent_handle.hpp"
+#include "libtorrent/settings_pack.hpp"
+#include "libtorrent/alert_types.hpp"
+#include "libtorrent/disabled_disk_io.hpp"
+#include "libtorrent/torrent_flags.hpp"
+#include "settings.hpp"
+#include "fake_peer.hpp"
+#include "utils.hpp"
+#include "test_utils.hpp"
+#include "setup_transfer.hpp"
+#include "create_torrent.hpp"
+#include "simulator/simulator.hpp"
+#include "simulator/utils.hpp"
+
+namespace {
+
+struct test_result
+{
+	std::vector<lt::error_code> disconnects;
+	std::vector<lt::tcp::endpoint> connects;
+};
+
+test_result test_allow_multiple_connections_per_pid(bool allow
+	, lt::peer_id const& pid
+	, char const* peer1_ip
+	, char const* peer2_ip)
+{
+	// setup the simulation
+	sim::default_config cfg;
+	sim::simulation sim{cfg};
+	auto ios = std::make_unique<sim::asio::io_context>(sim, lt::make_address_v4("50.0.0.1"));
+	lt::session_proxy zombie;
+
+	// setup settings pack
+	lt::session_params sp;
+	sp.settings = settings();
+	sp.settings.set_int(lt::settings_pack::alert_mask
+		, lt::alert_category::all & ~lt::alert_category::stats);
+	sp.settings.set_bool(lt::settings_pack::allow_multiple_connections_per_pid, allow);
+	sp.disk_io_constructor = lt::disabled_disk_io_constructor;
+
+	// create session
+	std::shared_ptr<lt::session> ses = std::make_shared<lt::session>(sp, *ios);
+
+	// add torrent
+	lt::add_torrent_params params = ::create_torrent(0, false);
+	lt::sha1_hash const info_hash = params.ti->info_hash();
+	params.flags &= ~lt::torrent_flags::auto_managed;
+	params.flags &= ~lt::torrent_flags::paused;
+	ses->async_add_torrent(std::move(params));
+
+	// create two fake peers with the same peer-id but different IPs
+	auto peer1 = std::make_unique<fake_peer>(sim, peer1_ip);
+	auto peer2 = std::make_unique<fake_peer>(sim, peer2_ip);
+	peer1->set_peer_id(pid);
+	peer2->set_peer_id(pid);
+
+	test_result result;
+
+	// set up a timer to fire later, to shut down
+	sim::timer t2(sim, lt::seconds(5)
+		, [&](boost::system::error_code const&)
+	{
+		zombie = ses->abort();
+		ses.reset();
+	});
+
+	print_alerts(*ses, [&](lt::session&, lt::alert const* a)
+	{
+		auto* pd = lt::alert_cast<lt::peer_disconnected_alert>(a);
+		if (pd) result.disconnects.push_back(pd->error);
+
+		auto* pa = lt::alert_cast<lt::peer_connect_alert>(a);
+		if (pa) result.connects.push_back(pa->endpoint);
+
+		if (lt::alert_cast<lt::add_torrent_alert>(a))
+		{
+			// both peers connect immediately after the torrent is added
+			peer1->connect_to(ep("50.0.0.1", 6881), info_hash);
+			peer2->connect_to(ep("50.0.0.1", 6881), info_hash);
+		}
+	});
+
+	sim.run();
+
+	return result;
+}
+
+bool has_duplicate_peer_id_error(std::vector<lt::error_code> const& errors)
+{
+	for (auto const& err : errors)
+		if (err == lt::errors::duplicate_peer_id)
+			return true;
+	return false;
+}
+
+bool has_connected(std::vector<lt::tcp::endpoint> const& endpoints
+	, lt::address_v4 const& addr)
+{
+	for (auto const& ep : endpoints)
+		if (ep.address() == addr)
+			return true;
+	return false;
+}
+
+} // anonymous namespace
+
+// allow_multiple_connections_per_pid = false:
+// a second connection from a different IP but the same peer-id should be rejected
+TORRENT_TEST(allow_multiple_connections_per_pid_false)
+{
+	lt::peer_id pid;
+	std::fill(pid.data(), pid.data() + 20, char(0xAA));
+
+	auto result = test_allow_multiple_connections_per_pid(false, pid
+		, "60.0.0.1", "60.0.0.2");
+
+	// verify that a duplicate_peer_id disconnect occurred
+	TEST_CHECK(has_duplicate_peer_id_error(result.disconnects));
+}
+
+// allow_multiple_connections_per_pid = true:
+// a second connection from a different IP but the same peer-id should be allowed
+TORRENT_TEST(allow_multiple_connections_per_pid_true)
+{
+	lt::peer_id pid;
+	std::fill(pid.data(), pid.data() + 20, char(0xBB));
+
+	auto result = test_allow_multiple_connections_per_pid(true, pid
+		, "60.0.0.3", "60.0.0.4");
+
+	// verify no duplicate_peer_id error occurred
+	TEST_CHECK(!has_duplicate_peer_id_error(result.disconnects));
+
+	// verify that the second peer connected successfully
+	TEST_CHECK(has_connected(result.connects, lt::make_address_v4("60.0.0.4")));
+}

src/bt_peer_connection.cpp

@@ -3518,28 +3518,31 @@ namespace {
 			std::copy(recv_buffer.begin(), recv_buffer.begin() + 20, pid.data());
 
 			// now, let's see if this connection should be closed
-			peer_connection* p = t->find_peer(pid);
-			if (p)
-			{
-				TORRENT_ASSERT(p->pid() == pid);
-				// we found another connection with the same peer-id
-				// which connection should be closed in order to be
-				// sure that the other end closes the same connection?
-				// the peer with greatest peer-id is the one allowed to
-				// initiate connections. So, if our peer-id is greater than
-				// the others, we should close the incoming connection,
-				// if not, we should close the outgoing one.
-				if ((pid < m_our_peer_id) == is_outgoing())
-				{
-					p->disconnect(errors::duplicate_peer_id, operation_t::bittorrent);
-				}
-				else
+			if (!t->settings().get_bool(settings_pack::allow_multiple_connections_per_pid))
+			{
+				peer_connection* p = t->find_peer(pid);
+				if (p)
 				{
-					disconnect(errors::duplicate_peer_id, operation_t::bittorrent);
-					return;
+					TORRENT_ASSERT(p->pid() == pid);
+					// we found another connection with the same peer-id
+					// which connection should be closed in order to be
+					// sure that the other end closes the same connection?
+					// the peer with greatest peer-id is the one allowed to
+					// initiate connections. So, if our peer-id is greater than
+					// the others, we should close the incoming connection,
+					// if not, we should close the outgoing one.
+					if ((pid < m_our_peer_id) == is_outgoing())
+					{
+						p->disconnect(errors::duplicate_peer_id, operation_t::bittorrent);
+					}
+					else
+					{
+						disconnect(errors::duplicate_peer_id, operation_t::bittorrent);
+						return;
+					}
 				}
 			}
-
+			
 			set_pid(pid);
 			m_client_version = identify_client(pid);
 			if (pid[0] == '-' && pid[1] == 'B' && pid[2] == 'C' && pid[7] == '-')

src/settings_pack.cpp

@@ -242,6 +242,7 @@ constexpr int DISK_WRITE_MODE = settings_pack::enable_os_cache;
 		SET(socks5_udp_send_local_ep, false, nullptr),
 		SET(proxy_send_host_in_connect, false, nullptr),
 		SET(disk_disable_copy_on_write, false, nullptr),
+		SET(allow_multiple_connections_per_pid, false, nullptr),
 	}});
 
 	CONSTEXPR_SETTINGS