diff --git a/include/libtorrent/kademlia/node.hpp b/include/libtorrent/kademlia/node.hpp index aacc62f87..230e18a4d 100644 --- a/include/libtorrent/kademlia/node.hpp +++ b/include/libtorrent/kademlia/node.hpp @@ -192,7 +192,7 @@ typedef std::map dht_mutable_table_t; public: node_impl(alert_dispatcher* alert_disp, udp_socket_interface* sock - , dht_settings const& settings, node_id nid, address const& external_address + , libtorrent::dht_settings const& settings, node_id nid, address const& external_address , dht_observer* observer); virtual ~node_impl() {} @@ -268,7 +268,7 @@ public: void status(libtorrent::session_status& s); - dht_settings const& settings() const { return m_settings; } + libtorrent::dht_settings const& settings() const { return m_settings; } protected: @@ -277,7 +277,7 @@ protected: bool lookup_torrents(sha1_hash const& target, entry& reply , char* tags) const; - dht_settings const& m_settings; + libtorrent::dht_settings const& m_settings; private: typedef libtorrent::mutex mutex_t; diff --git a/include/libtorrent/kademlia/rpc_manager.hpp b/include/libtorrent/kademlia/rpc_manager.hpp index 0d769221c..9426769dc 100644 --- a/include/libtorrent/kademlia/rpc_manager.hpp +++ b/include/libtorrent/kademlia/rpc_manager.hpp @@ -49,6 +49,8 @@ POSSIBILITY OF SUCH DAMAGE. namespace libtorrent { namespace aux { struct session_impl; } } +namespace libtorrent { struct dht_settings; } + namespace libtorrent { namespace dht { @@ -79,7 +81,7 @@ public: // returns true if the node needs a refresh // if so, id is assigned the node id to refresh - bool incoming(msg const&, node_id* id); + bool incoming(msg const&, node_id* id, libtorrent::dht_settings const& settings); time_duration tick(); bool invoke(entry& e, udp::endpoint target diff --git a/include/libtorrent/session_settings.hpp b/include/libtorrent/session_settings.hpp index 9955b5b24..ca3170b97 100644 --- a/include/libtorrent/session_settings.hpp +++ b/include/libtorrent/session_settings.hpp @@ -1406,6 +1406,7 @@ namespace libtorrent , extended_routing_table(true) , aggressive_lookups(true) , privacy_lookups(false) + , enforce_node_id(false) {} // the maximum number of peers to send in a @@ -1475,6 +1476,11 @@ namespace libtorrent // when set, perform lookups in a way that is slightly more expensive, but which // minimizes the amount of information leaked about you. bool privacy_lookups; + + // when set, node's whose IDs that are not correctly generated based on its external + // IP are ignored. When a query arrives from such node, an error message is returned + // with a message saying "invalid node ID". + bool enforce_node_id; }; #ifndef TORRENT_DISABLE_ENCRYPTION diff --git a/src/kademlia/node.cpp b/src/kademlia/node.cpp index 5668d582e..fc37e302b 100644 --- a/src/kademlia/node.cpp +++ b/src/kademlia/node.cpp @@ -224,17 +224,8 @@ void node_impl::incoming(msg const& m) char y = *(y_ent->string_ptr()); lazy_entry const* ext_ip = m.message.dict_find_string("ip"); - if (ext_ip && ext_ip->string_length() == 4) - { - // this node claims we use the wrong node-ID! - address_v4::bytes_type b; - memcpy(&b[0], ext_ip->string_ptr(), 4); - if (m_observer) - m_observer->set_external_address(address_v4(b) - , m.addr.address()); - } #if TORRENT_USE_IPV6 - else if (ext_ip && ext_ip->string_length() == 16) + if (ext_ip && ext_ip->string_length() >= 16) { // this node claims we use the wrong node-ID! address_v6::bytes_type b; @@ -242,15 +233,23 @@ void node_impl::incoming(msg const& m) if (m_observer) m_observer->set_external_address(address_v6(b) , m.addr.address()); - } + } else #endif + if (ext_ip && ext_ip->string_length() >= 4) + { + address_v4::bytes_type b; + memcpy(&b[0], ext_ip->string_ptr(), 4); + if (m_observer) + m_observer->set_external_address(address_v4(b) + , m.addr.address()); + } switch (y) { case 'r': { node_id id; - if (m_rpc.incoming(m, &id)) + if (m_rpc.incoming(m, &id, m_settings)) refresh(id, boost::bind(&nop)); break; } @@ -682,22 +681,22 @@ void node_impl::incoming_request(msg const& m, entry& e) } e["ip"] = endpoint_to_bytes(m.addr); -/* - // if this nodes ID doesn't match its IP, tell it what - // its IP is with an error - // don't enforce this yet - if (!verify_id(id, m.addr.address())) - { - incoming_error(e, "invalid node ID"); - return; - } -*/ + char const* query = top_level[0]->string_cstr(); lazy_entry const* arg_ent = top_level[1]; node_id id(top_level[2]->string_ptr()); + // if this nodes ID doesn't match its IP, tell it what + // its IP is with an error + // don't enforce this yet + if (m_settings.enforce_node_id && !verify_id(id, m.addr.address())) + { + incoming_error(e, "invalid node ID"); + return; + } + m_table.heard_about(id, m.addr); entry& reply = e["r"]; diff --git a/src/kademlia/rpc_manager.cpp b/src/kademlia/rpc_manager.cpp index 52dbc0cf6..8ffefc098 100644 --- a/src/kademlia/rpc_manager.cpp +++ b/src/kademlia/rpc_manager.cpp @@ -47,6 +47,7 @@ POSSIBILITY OF SUCH DAMAGE. #include #include #include +#include // for dht_settings #include #include // time() @@ -267,7 +268,7 @@ void rpc_manager::unreachable(udp::endpoint const& ep) // defined in node.cpp void incoming_error(entry& e, char const* msg, int error_code = 203); -bool rpc_manager::incoming(msg const& m, node_id* id) +bool rpc_manager::incoming(msg const& m, node_id* id, libtorrent::dht_settings const& settings) { INVARIANT_CHECK; @@ -337,12 +338,21 @@ bool rpc_manager::incoming(msg const& m, node_id* id) return false; } + node_id nid = node_id(node_id_ent->string_ptr()); + if (settings.enforce_node_id && !verify_id(nid, m.addr.address())) + { + entry e; + incoming_error(e, "invalid node ID"); + m_sock->send_packet(e, m.addr, 0); + return false; + } + #ifdef TORRENT_DHT_VERBOSE_LOGGING TORRENT_LOG(rpc) << "[" << o->m_algorithm.get() << "] Reply with transaction id: " << tid << " from " << m.addr; #endif o->reply(m); - *id = node_id(node_id_ent->string_ptr()); + *id = nid; int rtt = total_milliseconds(now - o->sent()); diff --git a/test/test_dht.cpp b/test/test_dht.cpp index 47a951011..b28de2a57 100644 --- a/test/test_dht.cpp +++ b/test/test_dht.cpp @@ -119,7 +119,7 @@ void send_dht_msg(node_impl& node, char const* msg, udp::endpoint const& ep , char const* target = 0, entry const* value = 0 , bool scrape = false, bool seed = false , std::string const key = std::string(), std::string const sig = std::string() - , int seq = -1, char const* cas = 0) + , int seq = -1, char const* cas = 0, sha1_hash const* nid = NULL) { // we're about to clear out the backing buffer // for this lazy_entry, so we better clear it now @@ -129,7 +129,8 @@ void send_dht_msg(node_impl& node, char const* msg, udp::endpoint const& ep e["t"] = t; e["y"] = "q"; entry::dictionary_type& a = e["a"].dict(); - a["id"] = generate_next().to_string(); + if (nid == NULL) a["id"] = generate_next().to_string(); + else a["id"] = nid->to_string(); if (info_hash) a["info_hash"] = std::string(info_hash, 20); if (name) a["n"] = name; if (!token.empty()) a["token"] = token; @@ -320,6 +321,7 @@ int test_main() dht_settings sett; sett.max_torrents = 4; sett.max_dht_items = 4; + sett.enforce_node_id = false; address ext = address::from_string("236.0.0.1"); mock_socket s; print_alert ad; @@ -361,13 +363,11 @@ int test_main() dht::key_desc_t err_desc[] = { {"y", lazy_entry::string_t, 1, 0}, - {"e", lazy_entry::list_t, 2, 0}, - {"r", lazy_entry::dict_t, 0, key_desc_t::parse_children}, - {"id", lazy_entry::string_t, 20, key_desc_t::last_child}, + {"e", lazy_entry::list_t, 2, 0} }; fprintf(stderr, "msg: %s\n", print_entry(response).c_str()); - ret = dht::verify_message(&response, err_desc, parsed, 4, error_string, sizeof(error_string)); + ret = dht::verify_message(&response, err_desc, parsed, 2, error_string, sizeof(error_string)); TEST_CHECK(ret); if (ret) { @@ -498,6 +498,69 @@ int test_main() fprintf(stderr, " invalid get_peers response: %s\n", error_string); } + // ====== test node ID enforcement ====== + + // enable node_id enforcement + sett.enforce_node_id = true; + + // this is one of the test vectors from: + // http://libtorrent.org/dht_sec.html + source = udp::endpoint(address::from_string("124.31.75.21"), 20); + node_id nid = to_hash("1712f6c70c5d6a4ec8a88e4c6ab4c28b95eee401"); + send_dht_msg(node, "find_node", source, &response, "10", 0, 0, std::string() + , 0, "0101010101010101010101010101010101010101", 0, false, false, std::string(), std::string(), -1, 0, &nid); + + dht::key_desc_t nodes_desc[] = { + {"y", lazy_entry::string_t, 1, 0}, + {"r", lazy_entry::dict_t, 0, key_desc_t::parse_children}, + {"id", lazy_entry::string_t, 20, key_desc_t::last_child}, + }; + + fprintf(stderr, "msg: %s\n", print_entry(response).c_str()); + ret = dht::verify_message(&response, nodes_desc, parsed, 3, error_string, sizeof(error_string)); + TEST_CHECK(ret); + if (ret) + { + TEST_CHECK(parsed[0]->string_value() == "r"); + } + else + { + fprintf(stderr, "msg: %s\n", print_entry(response).c_str()); + fprintf(stderr, " invalid error response: %s\n", error_string); + } + + // verify that we reject invalid node IDs + // this is now an invalid node-id for 'source' + nid[0] = 0x18; + send_dht_msg(node, "find_node", source, &response, "10", 0, 0, std::string() + , 0, "0101010101010101010101010101010101010101", 0, false, false, std::string(), std::string(), -1, 0, &nid); + + ret = dht::verify_message(&response, err_desc, parsed, 2, error_string, sizeof(error_string)); + TEST_CHECK(ret); + if (ret) + { + TEST_CHECK(parsed[0]->string_value() == "e"); + if (parsed[1]->list_at(0)->type() == lazy_entry::int_t + && parsed[1]->list_at(1)->type() == lazy_entry::string_t) + { + TEST_CHECK(parsed[1]->list_at(1)->string_value() == "invalid node ID"); + } + else + { + fprintf(stderr, "msg: %s\n", print_entry(response).c_str()); + TEST_ERROR("invalid error response"); + } + } + else + { + fprintf(stderr, "msg: %s\n", print_entry(response).c_str()); + fprintf(stderr, " invalid error response: %s\n", error_string); + } + + sett.enforce_node_id = false; + +// =========================== + bloom_filter<256> test; for (int i = 0; i < 256; ++i) {