diff --git a/Makefile b/Makefile index 5ab49dc97e2..05876e28230 100644 --- a/Makefile +++ b/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 \ diff --git a/include/libtorrent/settings_pack.hpp b/include/libtorrent/settings_pack.hpp index b3d6f33743d..6f6da822be5 100644 --- a/include/libtorrent/settings_pack.hpp +++ b/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 }; diff --git a/simulation/Jamfile b/simulation/Jamfile index 479664cc6b4..30928dc6530 100644 --- a/simulation/Jamfile +++ b/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 ; diff --git a/simulation/fake_peer.hpp b/simulation/fake_peer.hpp index 981460b027b..9d9044823e3 100644 --- a/simulation/fake_peer.hpp +++ b/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 m_send_buffer; }; diff --git a/simulation/test_allow_multiple_connections_per_pid.cpp b/simulation/test_allow_multiple_connections_per_pid.cpp new file mode 100644 index 00000000000..759619e3874 --- /dev/null +++ b/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 disconnects; + std::vector 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, 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 ses = std::make_shared(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(sim, peer1_ip); + auto peer2 = std::make_unique(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(a); + if (pd) result.disconnects.push_back(pd->error); + + auto* pa = lt::alert_cast(a); + if (pa) result.connects.push_back(pa->endpoint); + + if (lt::alert_cast(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 const& errors) +{ + for (auto const& err : errors) + if (err == lt::errors::duplicate_peer_id) + return true; + return false; +} + +bool has_connected(std::vector 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"))); +} diff --git a/src/bt_peer_connection.cpp b/src/bt_peer_connection.cpp index 87fc694277d..81d048d118e 100644 --- a/src/bt_peer_connection.cpp +++ b/src/bt_peer_connection.cpp @@ -3534,28 +3534,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] == '-') diff --git a/src/settings_pack.cpp b/src/settings_pack.cpp index cfe695e2765..ceeea86459a 100644 --- a/src/settings_pack.cpp +++ b/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