Add announce_port support

[0xThiebaut]

The announce_port setting permits to overwrite the port passed along to trackers as the &port= parameter. If left as the default, the listening port is used. This setting is only meant for very special cases where a seed's listening port differs from the effectively exposed port (e.g., through external NAT-PMP).

This PR would be complementary to the #7726 listen_on_proxy setting.

[0xThiebaut]

Example use-case

To secure outbound connections, a proxy server on the same subnet can be used to tunnel requests through a VPN (e.g., using pufferffish/wireproxy). Some VPNs support NAT-PMP (e.g., ProtonVPN) which could technically allow the proxy server to perform dynamic port forwarding back.

Through the announce_port setting, a tracker can be informed which NAT-PMP port has been made available through the proxy while the proxy has been configured to direct requests from the announced port to the actual listening port.

[arvidn]

I think it would be good to have some test coverage of this feature. Maybe you could add a test case here:

https://github.com/arvidn/libtorrent/blob/RC_2_0/simulation/test_tracker.cpp#L384-L410

[0xThiebaut]

I think it would be good to have some test coverage of this feature. Maybe you could add a test case here:

https://github.com/arvidn/libtorrent/blob/RC_2_0/simulation/test_tracker.cpp#L384-L410

A simple test coverage has been added to ensure announce_ip and announce_port are properly reflected in the tracker URL.

[arvidn]

looks good. would you mind adding a line to Changelog too?

[0xThiebaut]

looks good. would you mind adding a line to Changelog too?

Done as well, thanks!

[arvidn]

should this feature affect the port we announce to the DHT?
the current default is to use "implied port", i.e. the source port of the DHT message. This works since we accept uTP connections on the same port.

[arvidn]

and I imagine it should not affect local service discovery announcements (i.e. multicast on the local network)

[0xThiebaut]

The announce_port feature is intended for cases where additional appliances are located between local and remote peers. The local network discovery is by definition local so I agree it should not be affected by the announce_port setting. A future boolean setting could be introduced should anyone manage to find a use-case requiring announce_port overrides for local network discovery.

For DHT/uTP there can however be appliances between the local and remote peers; It would make sense to support overriding the announced port. I'm not familiar with DHT/uTP, do they currently use the same listening port as the TCP listener but for UDP?

[arvidn]

I'm not familiar with DHT/uTP, do they currently use the same listening port as the TCP listener but for UDP?

Yes, libtorrent attempts to open the listen socket for TCP and UDP on the same port. The UDP socket is used for DHT, uTP (peer connections) and UDP trackers.

[0xThiebaut]

My current understanding would be as follow:

For UDP trackers I think we are covered as it uses req.listen_port which announce_port overrides.

libtorrent/src/udp_tracker_connection.cpp

Line 732 in 6a2c1ee

aux::write_uint16(req.listen_port, out); // port

For uTP (peer connections) I don't believe there is a notion of "announcing"; it is merely the transport protocol and does not define which ports are used.

For DHT we need to ensure that announce_port indeed overrides the announced port. I believe we would need to do so in the session implementation and, if announce_port is set, make sure to clear the dht::announce::implied_port flag while overriding the port argument.

libtorrent/src/session_impl.cpp

Lines 6217 to 6221 in 6a2c1ee

	void session_impl::dht_announce(sha1_hash const& info_hash, int port, dht::announce_flags_t const flags)
	{
	if (!m_dht) return;
	m_dht->announce(info_hash, port, flags, std::bind(&on_dht_get_peers, std::ref(m_alerts), info_hash, _1));
	}

Would that make sense?

[0xThiebaut]

Another DHT option is to do it at node-level where the logic already replaces 0 with the listening port; Although doing it at session-level would match the TCP announce_port variant.

libtorrent/src/kademlia/node.cpp

Lines 458 to 467 in 6a2c1ee

	if (listen_port == 0 && m_observer != nullptr)
	{
	listen_port = m_observer->get_listen_port(
	(flags & announce::ssl_torrent) ? aux::transport::ssl : aux::transport::plaintext
	, m_sock);
	}
	get_peers(info_hash, std::move(f)
	, std::bind(&announce_fun, _1, std::ref(*this)
	, listen_port, info_hash, flags), flags);

[arvidn]

I can land this PR, and if you think the DHT behavior also should change, you could make another PR. I think having this logic in the session would make sense. It might also make sense to update the documentation for the setting to make it clear that it doesn't affect local service discovery and whether or not it affects DHT announces

[0xThiebaut]

I'll update the PR today to reflect that announce_port does not affect local discovery. I'll also apply the DHT change as I would rather avoid complex documentation with non-intuitive edge-cases.

[0xThiebaut]

I might be misunderstanding the relationship between a session and a torrent but wouldn't the following bypass the announce_port applied in the session's session_impl::dht_announce? Based on my limited understanding I would have expected the torrent to call m_ses.dht_announce() instead of m_ses.dht()->announce().

libtorrent/src/torrent.cpp

Lines 2816 to 2820 in e5fe95f

	m_torrent_file->info_hashes().for_each([&](sha1_hash const& ih, protocol_version v)
	{
	m_ses.dht()->announce(ih, 0, flags
	, std::bind(&torrent::on_dht_announce_response_disp, self, v, _1));
	});

[arvidn]

there's only one DHT node (technically, one per network interface you have). And to smooth-out the network traffic, torrents announces are staggered. The session loops over all the torrents and announces them one at a time, at a pace making them all announce once per 15 minutes (or whatever the setting has been set to).

[0xThiebaut]

So on top of applying announce_port in session_impl::dht_announce we also need to add it to torrent::dht_announce as both functions determine a port to use? Or we move the logic to the underlying dht_tracker::announce which is used by both the session_impl and torrent?

Apologies if I am missing something.

[arvidn]

thanks for your patience. I just did one more review pass

[arvidn]

thanks!