fix issue in self-connection detection introduced with the change to generate unique peer-ids for each connection. Now, the torrent keeps track of all of our peer-ids generated for outgoing (bittorrent) connections, and we check them against incoming peers' peer-ids

include/libtorrent/bt_peer_connection.hpp

@@ -108,6 +108,8 @@ namespace libtorrent {
 
 		~bt_peer_connection() override;
 
+		peer_id our_pid() const override { return m_our_peer_id; }
+
 #if !defined TORRENT_DISABLE_ENCRYPTION
 		bool supports_encryption() const
 		{ return m_encrypted; }
@@ -414,7 +416,7 @@ namespace libtorrent {
 		std::string m_client_version;
 
 		// the peer ID we advertise for ourself
-		peer_id m_our_peer_id;
+		peer_id const m_our_peer_id;
 
 		// this is a queue of ranges that describes
 		// where in the send buffer actual payload

include/libtorrent/peer_connection.hpp

@@ -146,6 +146,7 @@ namespace aux {
 		std::shared_ptr<aux::socket_type> s;
 		tcp::endpoint endp;
 		torrent_peer* peerinfo;
+		peer_id our_peer_id;
 	};
 
 	struct TORRENT_EXTRA_EXPORT peer_connection_hot_members

include/libtorrent/peer_connection_interface.hpp

@@ -52,6 +52,7 @@ namespace libtorrent {
 		virtual void disconnect(error_code const& ec
 			, operation_t op, int error = 0) = 0;
 		virtual peer_id const& pid() const = 0;
+		virtual peer_id our_pid() const = 0;
 		virtual void set_holepunch_mode() = 0;
 		virtual torrent_peer* peer_info_struct() const = 0;
 		virtual void set_peer_info(torrent_peer* pi) = 0;

include/libtorrent/torrent.hpp

@@ -387,6 +387,10 @@ namespace libtorrent {
 		bt_peer_connection* find_peer(tcp::endpoint const& ep) const;
 		peer_connection* find_peer(peer_id const& pid);
 
+		// checks to see if this peer id is used in one of our own outgoing
+		// connections.
+		bool is_self_connection(peer_id const& pid) const;
+
 		void on_resume_data_checked(status_t status, storage_error const& error);
 		void on_force_recheck(status_t status, storage_error const& error);
 		void on_piece_hashed(piece_index_t piece, sha1_hash const& piece_hash
@@ -1433,6 +1437,11 @@ namespace libtorrent {
 		aux::handler_storage<64> m_deferred_handler_storage;
 #endif
 
+		// these are the peer IDs we've used for our outgoing peer connections for
+		// this torrent. If we get an incoming peer claiming to have one of these,
+		// it's a connection to ourself, and we should reject it.
+		std::set<peer_id> m_outgoing_pids;
+
 		// for torrents who have a bandwidth limit, this is != 0
 		// and refers to a peer_class in the session.
 		peer_class_t m_peer_class{0};

include/libtorrent/vector_utils.hpp

@@ -48,8 +48,8 @@ namespace libtorrent {
 		return i;
 	}
 
-	template<class T>
-	void sorted_insert(std::vector<T>& container, T v)
+	template <typename T, typename U>
+	void sorted_insert(std::vector<T>& container, U v)
 	{
 		auto i = std::lower_bound(container.begin(), container.end(), v);
 		container.insert(i, v);

include/libtorrent/web_connection_base.hpp

@@ -75,6 +75,8 @@ namespace libtorrent {
 
 		bool in_handshake() const override;
 
+		peer_id our_pid() const override { return peer_id(); }
+
 		// the following functions appends messages
 		// to the send buffer
 		void write_choke() override {}

simulation/libsimulator

@@ -1 +1 @@
-Subproject commit ffaccc122691d10131a95bbe085ab325f57a602b
+Subproject commit e4fb5e171a6ed885e7de3192153ba77835d23dee

simulation/test_swarm.cpp

@@ -44,6 +44,7 @@ POSSIBILITY OF SUCH DAMAGE.
 #include "setup_transfer.hpp" // for ep()
 #include "fake_peer.hpp"
 
+#include "simulator/nat.hpp"
 #include "simulator/queue.hpp"
 #include "utils.hpp"
 
@@ -462,6 +463,60 @@ TORRENT_TEST(dead_peers)
 	TEST_EQUAL(num_connect_timeout, 3);
 }
 
+// the address 50.0.0.1 sits behind a NAT. All of its outgoing connections have
+// their source address rewritten to 51.51.51.51
+struct nat_config : sim::default_config
+{
+	nat_config() : m_nat_hop(std::make_shared<nat>(addr("51.51.51.51"))) {}
+
+	sim::route outgoing_route(lt::address ip) override
+	{
+		// This is extremely simplistic. It will simply alter the percieved source
+		// IP of the connecting client.
+		sim::route r;
+		if (ip == addr("50.0.0.1")) r.append(m_nat_hop);
+		return r;
+	}
+	std::shared_ptr<nat> m_nat_hop;
+};
+
+TORRENT_TEST(self_connect)
+{
+	int num_self_connection_disconnects = 0;
+
+	nat_config network_cfg;
+	sim::simulation sim{network_cfg};
+
+	setup_swarm(1, swarm_test::download, sim
+		// add session
+		, [](lt::settings_pack& p) {
+			p.set_bool(settings_pack::enable_incoming_utp, false);
+			p.set_bool(settings_pack::enable_outgoing_utp, false);
+		}
+		// add torrent
+		, [](lt::add_torrent_params& params) {
+			// this is our own address and listen port, just to make sure we get
+			// ourself as a peer (which normally happens one way or another in the
+			// wild)
+			params.peers.assign({ep("50.0.0.1", 6881)});
+		}
+		// on alert
+		, [&](lt::alert const* a, lt::session&) {
+			auto* e = alert_cast<peer_disconnected_alert>(a);
+			if (e
+				&& e->op == operation_t::bittorrent
+				&& e->error == error_code(errors::self_connection))
+			{
+				++num_self_connection_disconnects;
+			}
+		}
+		// terminate
+		, [](int t, lt::session&) -> bool
+		{ return t > 100; });
+
+	TEST_EQUAL(num_self_connection_disconnects, 1);
+}
+
 TORRENT_TEST(delete_files)
 {
 	std::string save_path;

src/bt_peer_connection.cpp

@@ -155,7 +155,7 @@ namespace {
 		, m_rc4_encrypted(false)
 		, m_recv_buffer(peer_connection::m_recv_buffer)
 #endif
-		, m_our_peer_id(aux::generate_peer_id(*pack.sett))
+		, m_our_peer_id(pack.our_peer_id)
 	{
 #ifndef TORRENT_DISABLE_LOGGING
 		peer_log(peer_log_alert::info, "CONSTRUCT", "bt_peer_connection");
@@ -3180,6 +3180,12 @@ namespace {
 				if (max_out_request_queue() > 50) max_out_request_queue(50);
 			}
 
+			if (t->is_self_connection(pid))
+			{
+				disconnect(errors::self_connection, operation_t::bittorrent);
+				return;
+			}
+
 #ifndef TORRENT_DISABLE_EXTENSIONS
 			for (auto i = m_extensions.begin()
 				, end(m_extensions.end()); i != end;)

src/peer_connection.cpp

@@ -4303,6 +4303,13 @@ namespace libtorrent {
 		}
 
 		std::shared_ptr<torrent> t = m_torrent.lock();
+
+		if (ec == errors::self_connection)
+		{
+			// don't try to connect to ourself again
+			if (m_peer_info && t) t->ban_peer(m_peer_info);
+		}
+
 		if (m_connecting)
 		{
 			m_counters.inc_stats_counter(counters::num_peers_half_open, -1);
@@ -6108,9 +6115,6 @@ namespace libtorrent {
 
 		if (m_remote == m_socket->local_endpoint(ec))
 		{
-			// if the remote endpoint is the same as the local endpoint, we're connected
-			// to ourselves
-			if (m_peer_info && t) t->ban_peer(m_peer_info);
 			disconnect(errors::self_connection, operation_t::bittorrent, 1);
 			return;
 		}

src/session_impl.cpp

@@ -2941,19 +2941,21 @@ namespace aux {
 #endif
 		}
 
-		peer_connection_args pack;
-		pack.ses = this;
-		pack.sett = &m_settings;
-		pack.stats_counters = &m_stats_counters;
-		pack.disk_thread = &m_disk_thread;
-		pack.ios = &m_io_service;
-		pack.tor = std::weak_ptr<torrent>();
-		pack.s = s;
-		pack.endp = endp;
-		pack.peerinfo = nullptr;
+		peer_connection_args pack{
+			this
+			, &m_settings
+			, &m_stats_counters
+			, &m_disk_thread
+			, &m_io_service
+			, std::weak_ptr<torrent>()
+			, s
+			, endp
+			, nullptr
+			, aux::generate_peer_id(m_settings)
+		};
 
 		std::shared_ptr<peer_connection> c
-			= std::make_shared<bt_peer_connection>(pack);
+			= std::make_shared<bt_peer_connection>(std::move(pack));
 
 		if (!c->is_disconnecting())
 		{

src/torrent.cpp

@@ -1963,11 +1963,14 @@ bool is_downloading_state(int const st)
 		return nullptr;
 	}
 
+	bool torrent::is_self_connection(peer_id const& pid) const
+	{
+		return m_outgoing_pids.count(pid) > 0;
+	}
+
 	void torrent::on_resume_data_checked(status_t const status
 		, storage_error const& error) try
 	{
-		// hold a reference until this function returns
-
 #if TORRENT_USE_ASSERTS
 		TORRENT_ASSERT(m_outstanding_check_files);
 		m_outstanding_check_files = false;
@@ -4412,6 +4415,7 @@ bool is_downloading_state(int const st)
 		// have been destructed
 		if (m_peer_list) m_peer_list->clear();
 		m_connections.clear();
+		m_outgoing_pids.clear();
 		m_peers_to_disconnect.clear();
 		m_num_uploads = 0;
 		m_num_connecting = 0;
@@ -5454,6 +5458,12 @@ bool is_downloading_state(int const st)
 		TORRENT_ASSERT(std::count(m_peers_to_disconnect.begin()
 			, m_peers_to_disconnect.end(), p) == 0);
 
+		auto it = m_outgoing_pids.find(p->our_pid());
+		if (it != m_outgoing_pids.end())
+		{
+			m_outgoing_pids.erase(it);
+		}
+
 		// only schedule the peer for actual removal if in fact
 		// we can be sure peer_connection will be kept alive until
 		// the deferred function is called. If a peer_connection
@@ -6018,24 +6028,27 @@ bool is_downloading_state(int const st)
 			return;
 		}
 
+		peer_connection_args pack{
+			&m_ses
+			, &settings()
+			, &m_ses.stats_counters()
+			, &m_ses.disk_thread()
+			, &m_ses.get_io_service()
+			, shared_from_this()
+			, s
+			, a
+			, &web->peer_info
+			, aux::generate_peer_id(settings())
+		};
+
 		std::shared_ptr<peer_connection> c;
-		peer_connection_args pack;
-		pack.ses = &m_ses;
-		pack.sett = &settings();
-		pack.stats_counters = &m_ses.stats_counters();
-		pack.disk_thread = &m_ses.disk_thread();
-		pack.ios = &m_ses.get_io_service();
-		pack.tor = shared_from_this();
-		pack.s = s;
-		pack.endp = a;
-		pack.peerinfo = &web->peer_info;
 		if (web->type == web_seed_entry::url_seed)
 		{
-			c = std::make_shared<web_peer_connection>(pack, *web);
+			c = std::make_shared<web_peer_connection>(std::move(pack), *web);
 		}
 		else if (web->type == web_seed_entry::http_seed)
 		{
-			c = std::make_shared<http_seed_connection>(pack, *web);
+			c = std::make_shared<http_seed_connection>(std::move(pack), *web);
 		}
 		if (!c) return;
 
@@ -6630,18 +6643,21 @@ bool is_downloading_state(int const st)
 #endif
 		}
 
-		peer_connection_args pack;
-		pack.ses = &m_ses;
-		pack.sett = &settings();
-		pack.stats_counters = &m_ses.stats_counters();
-		pack.disk_thread = &m_ses.disk_thread();
-		pack.ios = &m_ses.get_io_service();
-		pack.tor = shared_from_this();
-		pack.s = s;
-		pack.endp = a;
-		pack.peerinfo = peerinfo;
+		peer_id const our_pid = aux::generate_peer_id(settings());
+		peer_connection_args pack{
+			&m_ses
+			, &settings()
+			, &m_ses.stats_counters()
+			, &m_ses.disk_thread()
+			, &m_ses.get_io_service()
+			, shared_from_this()
+			, s
+			, a
+			, peerinfo
+			, our_pid
+		};
 
-		std::shared_ptr<peer_connection> c = std::make_shared<bt_peer_connection>(pack);
+		auto c = std::make_shared<bt_peer_connection>(std::move(pack));
 
 #if TORRENT_USE_ASSERTS
 		c->m_in_constructor = false;
@@ -6672,6 +6688,7 @@ bool is_downloading_state(int const st)
 		sorted_insert(m_connections, c.get());
 		TORRENT_TRY
 		{
+			m_outgoing_pids.insert(our_pid);
 			m_ses.insert_peer(c);
 			need_peer_list();
 			m_peer_list->set_connection(peerinfo, c.get());
@@ -7760,6 +7777,8 @@ bool is_downloading_state(int const st)
 #if TORRENT_USE_INVARIANT_CHECKS
 	void torrent::check_invariant() const
 	{
+		TORRENT_ASSERT(m_connections.size() >= m_outgoing_pids.size());
+
 		// the piece picker and the file progress states are supposed to be
 		// created in sync
 		TORRENT_ASSERT(has_picker() == !m_file_progress.empty());

test/test_peer_list.cpp

@@ -63,6 +63,7 @@ struct mock_peer_connection
 		, m_tp(nullptr)
 		, m_remote(remote)
 		, m_local(ep("127.0.0.1", 8080))
+		, m_our_id(nullptr)
 		, m_disconnect_called(false)
 		, m_torrent(*tor)
 	{
@@ -102,6 +103,7 @@ struct mock_peer_connection
 	tcp::endpoint m_remote;
 	tcp::endpoint m_local;
 	peer_id m_id;
+	peer_id m_our_id;
 	bool m_disconnect_called;
 	mock_torrent& m_torrent;
 
@@ -111,6 +113,7 @@ struct mock_peer_connection
 	void disconnect(error_code const& ec
 		, operation_t op, int error = 0) override;
 	peer_id const& pid() const override { return m_id; }
+	peer_id our_pid() const override { return m_our_id; }
 	void set_holepunch_mode() override {}
 	torrent_peer* peer_info_struct() const override { return m_tp; }
 	void set_peer_info(torrent_peer* pi) override { m_tp = pi; }