From ff63557f582ee5d7f6afbc11e5c80e9d5077d18b Mon Sep 17 00:00:00 2001 From: arvidn Date: Sun, 7 May 2017 20:43:55 -0400 Subject: [PATCH] bind upnp requests to correct local address --- ChangeLog | 1 + include/libtorrent/http_connection.hpp | 4 ++-- include/libtorrent/natpmp.hpp | 2 +- include/libtorrent/upnp.hpp | 16 +++++++-------- src/session_impl.cpp | 28 +++++++++++++++----------- src/upnp.cpp | 28 +++++++++++++------------- test/test_upnp.cpp | 4 ++-- 7 files changed, 43 insertions(+), 40 deletions(-) diff --git a/ChangeLog b/ChangeLog index d016afc51..3693722b3 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,4 @@ + * bind upnp requests to correct local address * save resume data when removing web seeds * fix proxying of https connections * fix race condition in disk I/O storage class diff --git a/include/libtorrent/http_connection.hpp b/include/libtorrent/http_connection.hpp index c1049e234..563f933a3 100644 --- a/include/libtorrent/http_connection.hpp +++ b/include/libtorrent/http_connection.hpp @@ -104,7 +104,7 @@ struct TORRENT_EXTRA_EXPORT http_connection std::string m_sendbuffer; void get(std::string const& url, time_duration timeout = seconds(30) - , int prio = 0, aux::proxy_settings const* ps = 0, int handle_redirects = 5 + , int prio = 0, aux::proxy_settings const* ps = NULL, int handle_redirects = 5 , std::string const& user_agent = std::string() , address const& bind_addr = address_v4::any() , int resolve_flags = 0, std::string const& auth_ = std::string() @@ -114,7 +114,7 @@ struct TORRENT_EXTRA_EXPORT http_connection ); void start(std::string const& hostname, int port - , time_duration timeout, int prio = 0, aux::proxy_settings const* ps = 0 + , time_duration timeout, int prio = 0, aux::proxy_settings const* ps = NULL , bool ssl = false, int handle_redirect = 5 , address const& bind_addr = address_v4::any() , int resolve_flags = 0 diff --git a/include/libtorrent/natpmp.hpp b/include/libtorrent/natpmp.hpp index cc1a2a710..0fa0d5949 100644 --- a/include/libtorrent/natpmp.hpp +++ b/include/libtorrent/natpmp.hpp @@ -75,7 +75,7 @@ public: void close(); private: - + boost::shared_ptr self() { return shared_from_this(); } void update_mapping(int i, mutex::scoped_lock& l); diff --git a/include/libtorrent/upnp.hpp b/include/libtorrent/upnp.hpp index a5538cded..0682622b1 100644 --- a/include/libtorrent/upnp.hpp +++ b/include/libtorrent/upnp.hpp @@ -187,13 +187,13 @@ public: // portmap_alert_ respectively. If The mapping fails immediately, the return value // is -1, which means failure. There will not be any error alert notification for // mappings that fail with a -1 return value. - int add_mapping(protocol_type p, int external_port, int local_port); + int add_mapping(protocol_type p, int external_port, tcp::endpoint local_ep); // This function removes a port mapping. ``mapping_index`` is the index that refers // to the mapping you want to remove, which was returned from add_mapping(). void delete_mapping(int mapping_index); - bool get_mapping(int mapping_index, int& local_port, int& external_port, int& protocol) const; + bool get_mapping(int mapping_index, tcp::endpoint& local_ep, int& external_port, int& protocol) const; void discover_device(); void close(); @@ -261,11 +261,10 @@ private: global_mapping_t() : protocol(none) , external_port(0) - , local_port(0) {} int protocol; int external_port; - int local_port; + tcp::endpoint local_ep; }; struct mapping_t @@ -273,7 +272,6 @@ private: enum action_t { action_none, action_add, action_delete }; mapping_t() : action(action_none) - , local_port(0) , external_port(0) , protocol(none) , failcount(0) @@ -282,11 +280,11 @@ private: // the time the port mapping will expire time_point expires; - int action; - - // the local port for this mapping. If this is set + // the local port for this mapping. The port is set // to 0, the mapping is not in use - int local_port; + tcp::endpoint local_ep; + + int action; // the external (on the NAT router) port // for the mapping. This is the port we diff --git a/src/session_impl.cpp b/src/session_impl.cpp index e2bba7227..1078e918f 100644 --- a/src/session_impl.cpp +++ b/src/session_impl.cpp @@ -2250,7 +2250,8 @@ retry: if ((mask & 2) && m_upnp) { if (m_tcp_mapping[1] != -1) m_upnp->delete_mapping(m_tcp_mapping[1]); - m_tcp_mapping[1] = m_upnp->add_mapping(upnp::tcp, tcp_port, tcp_port); + m_tcp_mapping[1] = m_upnp->add_mapping(upnp::tcp, tcp_port + , tcp::endpoint(m_listen_interface.address(), tcp_port)); #ifdef TORRENT_USE_OPENSSL if (m_ssl_tcp_mapping[1] != -1) { @@ -2258,7 +2259,7 @@ retry: m_ssl_tcp_mapping[1] = -1; } if (ssl_port > 0) m_ssl_tcp_mapping[1] = m_upnp->add_mapping(upnp::tcp - , ssl_port, ssl_port); + , ssl_port, tcp::endpoint(m_listen_interface.address(), ssl_port)); #endif } } @@ -5994,7 +5995,8 @@ retry: void session_impl::maybe_update_udp_mapping(int const nat, bool const ssl , int const local_port, int const external_port) { - int local, external, protocol; + int external, protocol; + tcp::endpoint local_ep; #ifdef TORRENT_USE_OPENSSL int* mapping = ssl ? m_ssl_udp_mapping : m_udp_mapping; #else @@ -6003,6 +6005,7 @@ retry: #endif if (nat == 0 && m_natpmp) { + int local = 0; if (mapping[nat] != -1) { if (m_natpmp->get_mapping(mapping[nat], local, external, protocol)) @@ -6014,23 +6017,24 @@ retry: m_natpmp->delete_mapping(mapping[nat]); } mapping[nat] = m_natpmp->add_mapping(natpmp::udp - , local_port, external_port); + , external_port, local_port); return; } else if (nat == 1 && m_upnp) { if (mapping[nat] != -1) { - if (m_upnp->get_mapping(mapping[nat], local, external, protocol)) + if (m_upnp->get_mapping(mapping[nat], local_ep, external, protocol)) { // we already have a mapping. If it's the same, don't do anything - if (local == local_port && external == external_port && protocol == natpmp::udp) + if (local_ep.port() == local_port && external == external_port && protocol == natpmp::udp) return; } m_upnp->delete_mapping(mapping[nat]); } + local_ep.port(local_port); mapping[nat] = m_upnp->add_mapping(upnp::udp - , local_port, external_port); + , external_port, local_ep); return; } } @@ -6786,13 +6790,13 @@ retry: if (m_udp_socket.is_open()) { m_udp_mapping[1] = m_upnp->add_mapping(upnp::udp - , m_listen_interface.port(), m_listen_interface.port()); + , m_listen_interface.port(), m_listen_interface); } #ifdef TORRENT_USE_OPENSSL if (m_ssl_udp_socket.is_open() && ssl_port > 0) { m_ssl_udp_mapping[1] = m_upnp->add_mapping(upnp::udp - , ssl_port, ssl_port); + , ssl_port, tcp::endpoint(m_listen_interface.address(), ssl_port)); } #endif return m_upnp.get(); @@ -6803,9 +6807,9 @@ retry: { int ret = 0; if (m_upnp) ret = m_upnp->add_mapping(static_cast(t), external_port - , local_port); - if (m_natpmp) ret = m_natpmp->add_mapping(static_cast(t), external_port - , local_port); + , tcp::endpoint(m_listen_interface.address(), local_port)); + if (m_natpmp) ret = m_natpmp->add_mapping(static_cast(t) + , external_port, local_port); return ret; } diff --git a/src/upnp.cpp b/src/upnp.cpp index b71704ca3..e4dc73526 100644 --- a/src/upnp.cpp +++ b/src/upnp.cpp @@ -166,7 +166,7 @@ void upnp::discover_device_impl(mutex::scoped_lock& l) } // returns a reference to a mapping or -1 on failure -int upnp::add_mapping(upnp::protocol_type p, int external_port, int local_port) +int upnp::add_mapping(upnp::protocol_type p, int external_port, tcp::endpoint local_ep) { // external port 0 means _every_ port TORRENT_ASSERT(external_port != 0); @@ -175,8 +175,8 @@ int upnp::add_mapping(upnp::protocol_type p, int external_port, int local_port) char msg[500]; snprintf(msg, sizeof(msg), "adding port map: [ protocol: %s ext_port: %u " - "local_port: %u ] %s", (p == tcp?"tcp":"udp"), external_port - , local_port, m_disabled ? "DISABLED": ""); + "local_ep: %s ] %s", (p == tcp?"tcp":"udp"), external_port + , print_endpoint(local_ep).c_str(), m_disabled ? "DISABLED": ""); log(msg, l); if (m_disabled) return -1; @@ -192,7 +192,7 @@ int upnp::add_mapping(upnp::protocol_type p, int external_port, int local_port) mapping_it->protocol = p; mapping_it->external_port = external_port; - mapping_it->local_port = local_port; + mapping_it->local_ep = local_ep; int mapping_index = mapping_it - m_mappings.begin(); @@ -209,7 +209,7 @@ int upnp::add_mapping(upnp::protocol_type p, int external_port, int local_port) m.action = mapping_t::action_add; m.protocol = p; m.external_port = external_port; - m.local_port = local_port; + m.local_ep = local_ep; if (!d.service_namespace.empty()) update_map(d, mapping_index, l); } @@ -227,8 +227,8 @@ void upnp::delete_mapping(int mapping) char msg[500]; snprintf(msg, sizeof(msg), "deleting port map: [ protocol: %s ext_port: %u " - "local_port: %u ]", (m.protocol == tcp?"tcp":"udp"), m.external_port - , m.local_port); + "local_ep: %s ]", (m.protocol == tcp?"tcp":"udp"), m.external_port + , print_endpoint(m.local_ep).c_str()); log(msg, l); if (m.protocol == none) return; @@ -246,13 +246,13 @@ void upnp::delete_mapping(int mapping) } } -bool upnp::get_mapping(int index, int& local_port, int& external_port, int& protocol) const +bool upnp::get_mapping(int index, tcp::endpoint& local_ep, int& external_port, int& protocol) const { TORRENT_ASSERT(index < int(m_mappings.size()) && index >= 0); if (index >= int(m_mappings.size()) || index < 0) return false; global_mapping_t const& m = m_mappings[index]; if (m.protocol == none) return false; - local_port = m.local_port; + local_ep = m.local_ep; external_port = m.external_port; protocol = m.protocol; return true; @@ -537,7 +537,7 @@ void upnp::on_reply(udp::endpoint const& from, char* buffer { mapping_t m; m.action = mapping_t::action_add; - m.local_port = j->local_port; + m.local_ep = j->local_ep; m.external_port = j->external_port; m.protocol = j->protocol; d.mapping.push_back(m); @@ -700,9 +700,9 @@ void upnp::create_port_mapping(http_connection& c, rootdevice& d, int i) "" , soap_action, d.service_namespace.c_str(), d.mapping[i].external_port , (d.mapping[i].protocol == udp ? "UDP" : "TCP") - , d.mapping[i].local_port + , d.mapping[i].local_ep.port() , local_endpoint.c_str() - , m_user_agent.c_str(), local_endpoint.c_str(), d.mapping[i].local_port + , m_user_agent.c_str(), local_endpoint.c_str(), d.mapping[i].local_ep.port() , d.lease_duration, soap_action); post(d, soap, soap_action, l); @@ -772,7 +772,7 @@ void upnp::update_map(rootdevice& d, int i, mutex::scoped_lock& l) , boost::bind(&upnp::create_port_mapping, self(), _1, boost::ref(d), i))); d.upnp_connection->start(d.hostname, d.port - , seconds(10), 1); + , seconds(10), 1, NULL, false, 5, m.local_ep.address()); } else if (m.action == mapping_t::action_delete) { @@ -783,7 +783,7 @@ void upnp::update_map(rootdevice& d, int i, mutex::scoped_lock& l) , boost::ref(d), i, _5), true, default_max_bottled_buffer_size , boost::bind(&upnp::delete_port_mapping, self(), boost::ref(d), i))); d.upnp_connection->start(d.hostname, d.port - , seconds(10), 1); + , seconds(10), 1, NULL, false, 5, m.local_ep.address()); } m.action = mapping_t::action_none; diff --git a/test/test_upnp.cpp b/test/test_upnp.cpp index 447bab5a8..97f06e2f0 100644 --- a/test/test_upnp.cpp +++ b/test/test_upnp.cpp @@ -182,8 +182,8 @@ void run_upnp_test(char const* root_filename, char const* router_model, char con std::cerr << "router: " << upnp_handler->router_model() << std::endl; TEST_EQUAL(upnp_handler->router_model(), router_model); - int mapping1 = upnp_handler->add_mapping(upnp::tcp, 500, 500); - int mapping2 = upnp_handler->add_mapping(upnp::udp, 501, 501); + int mapping1 = upnp_handler->add_mapping(upnp::tcp, 500, ep("127.0.0.1", 500)); + int mapping2 = upnp_handler->add_mapping(upnp::udp, 501, ep("127.0.0.1", 501)); for (int i = 0; i < 40; ++i) {