From 7b076d5170f3299b1933f990e8b35777083c1809 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Mon, 16 Sep 2019 19:20:23 +0000 Subject: [PATCH] p2p: fix bans taking port into account --- src/p2p/net_node.h | 6 +++--- src/p2p/net_node.inl | 8 ++++---- src/p2p/net_node_common.h | 6 +++--- src/rpc/core_rpc_server.cpp | 6 +++--- tests/unit_tests/ban.cpp | 22 ++++++++++++++++++++-- 5 files changed, 33 insertions(+), 15 deletions(-) diff --git a/src/p2p/net_node.h b/src/p2p/net_node.h index 255a1fc1f..d7e2e91f5 100644 --- a/src/p2p/net_node.h +++ b/src/p2p/net_node.h @@ -271,7 +271,7 @@ namespace nodetool virtual bool block_subnet(const epee::net_utils::ipv4_network_subnet &subnet, time_t seconds = P2P_IP_BLOCKTIME); virtual bool unblock_subnet(const epee::net_utils::ipv4_network_subnet &subnet); virtual bool is_host_blocked(const epee::net_utils::network_address &address, time_t *seconds) { CRITICAL_REGION_LOCAL(m_blocked_hosts_lock); return !is_remote_host_allowed(address, seconds); } - virtual std::map get_blocked_hosts() { CRITICAL_REGION_LOCAL(m_blocked_hosts_lock); return m_blocked_hosts; } + virtual std::map get_blocked_hosts() { CRITICAL_REGION_LOCAL(m_blocked_hosts_lock); return m_blocked_hosts; } virtual std::map get_blocked_subnets() { CRITICAL_REGION_LOCAL(m_blocked_hosts_lock); return m_blocked_subnets; } virtual void add_used_stripe_peer(const typename t_payload_net_handler::connection_context &context); @@ -484,11 +484,11 @@ namespace nodetool std::map m_network_zones; - std::map m_conn_fails_cache; + std::map m_conn_fails_cache; epee::critical_section m_conn_fails_cache_lock; epee::critical_section m_blocked_hosts_lock; // for both hosts and subnets - std::map m_blocked_hosts; + std::map m_blocked_hosts; std::map m_blocked_subnets; epee::critical_section m_host_fails_score_lock; diff --git a/src/p2p/net_node.inl b/src/p2p/net_node.inl index 97a18b519..24c87cef8 100644 --- a/src/p2p/net_node.inl +++ b/src/p2p/net_node.inl @@ -166,7 +166,7 @@ namespace nodetool const time_t now = time(nullptr); // look in the hosts list - auto it = m_blocked_hosts.find(address); + auto it = m_blocked_hosts.find(address.host_str()); if (it != m_blocked_hosts.end()) { if (now >= it->second) @@ -224,7 +224,7 @@ namespace nodetool limit = std::numeric_limits::max(); else limit = now + seconds; - m_blocked_hosts[addr] = limit; + m_blocked_hosts[addr.host_str()] = limit; // drop any connection to that address. This should only have to look into // the zone related to the connection, but really make sure everything is @@ -254,7 +254,7 @@ namespace nodetool bool node_server::unblock_host(const epee::net_utils::network_address &address) { CRITICAL_REGION_LOCAL(m_blocked_hosts_lock); - auto i = m_blocked_hosts.find(address); + auto i = m_blocked_hosts.find(address.host_str()); if (i == m_blocked_hosts.end()) return false; m_blocked_hosts.erase(i); @@ -1342,7 +1342,7 @@ namespace nodetool bool node_server::is_addr_recently_failed(const epee::net_utils::network_address& addr) { CRITICAL_REGION_LOCAL(m_conn_fails_cache_lock); - auto it = m_conn_fails_cache.find(addr); + auto it = m_conn_fails_cache.find(addr.host_str()); if(it == m_conn_fails_cache.end()) return false; diff --git a/src/p2p/net_node_common.h b/src/p2p/net_node_common.h index 239814c2c..e0046cd86 100644 --- a/src/p2p/net_node_common.h +++ b/src/p2p/net_node_common.h @@ -58,7 +58,7 @@ namespace nodetool virtual bool for_connection(const boost::uuids::uuid&, std::function f)=0; virtual bool block_host(const epee::net_utils::network_address &address, time_t seconds = 0)=0; virtual bool unblock_host(const epee::net_utils::network_address &address)=0; - virtual std::map get_blocked_hosts()=0; + virtual std::map get_blocked_hosts()=0; virtual std::map get_blocked_subnets()=0; virtual bool add_host_fail(const epee::net_utils::network_address &address)=0; virtual void add_used_stripe_peer(const t_connection_context &context)=0; @@ -114,9 +114,9 @@ namespace nodetool { return true; } - virtual std::map get_blocked_hosts() + virtual std::map get_blocked_hosts() { - return std::map(); + return std::map(); } virtual std::map get_blocked_subnets() { diff --git a/src/rpc/core_rpc_server.cpp b/src/rpc/core_rpc_server.cpp index 529cdbf2d..7192db122 100644 --- a/src/rpc/core_rpc_server.cpp +++ b/src/rpc/core_rpc_server.cpp @@ -1985,12 +1985,12 @@ namespace cryptonote PERF_TIMER(on_get_bans); auto now = time(nullptr); - std::map blocked_hosts = m_p2p.get_blocked_hosts(); - for (std::map::const_iterator i = blocked_hosts.begin(); i != blocked_hosts.end(); ++i) + std::map blocked_hosts = m_p2p.get_blocked_hosts(); + for (std::map::const_iterator i = blocked_hosts.begin(); i != blocked_hosts.end(); ++i) { if (i->second > now) { COMMAND_RPC_GETBANS::ban b; - b.host = i->first.host_str(); + b.host = i->first; b.ip = 0; uint32_t ip; if (epee::string_tools::get_ip_int32_from_string(ip, b.host)) diff --git a/tests/unit_tests/ban.cpp b/tests/unit_tests/ban.cpp index 17fba90c6..b710f9226 100644 --- a/tests/unit_tests/ban.cpp +++ b/tests/unit_tests/ban.cpp @@ -36,6 +36,7 @@ #include "cryptonote_protocol/cryptonote_protocol_handler.inl" #define MAKE_IPV4_ADDRESS(a,b,c,d) epee::net_utils::ipv4_network_address{MAKE_IP(a,b,c,d),0} +#define MAKE_IPV4_ADDRESS_PORT(a,b,c,d,e) epee::net_utils::ipv4_network_address{MAKE_IP(a,b,c,d),e} #define MAKE_IPV4_SUBNET(a,b,c,d,e) epee::net_utils::ipv4_network_subnet{MAKE_IP(a,b,c,d),e} namespace cryptonote { @@ -94,10 +95,10 @@ typedef nodetool::node_server hosts = server.get_blocked_hosts(); + std::map hosts = server.get_blocked_hosts(); for (auto rec: hosts) { - if (rec.first == address) + if (rec.first == address.host_str()) { if (t) *t = rec.second; @@ -240,5 +241,22 @@ TEST(ban, subnet) ASSERT_TRUE(server.get_blocked_subnets().size() == 0); } +TEST(ban, ignores_port) +{ + time_t seconds; + test_core pr_core; + cryptonote::t_cryptonote_protocol_handler cprotocol(pr_core, NULL); + Server server(cprotocol); + cprotocol.set_p2p_endpoint(&server); + + ASSERT_FALSE(is_blocked(server,MAKE_IPV4_ADDRESS_PORT(1,2,3,4,5))); + ASSERT_TRUE(server.block_host(MAKE_IPV4_ADDRESS_PORT(1,2,3,4,5), std::numeric_limits::max() - 1)); + ASSERT_TRUE(is_blocked(server,MAKE_IPV4_ADDRESS_PORT(1,2,3,4,5))); + ASSERT_TRUE(is_blocked(server,MAKE_IPV4_ADDRESS_PORT(1,2,3,4,6))); + ASSERT_TRUE(server.unblock_host(MAKE_IPV4_ADDRESS_PORT(1,2,3,4,5))); + ASSERT_FALSE(is_blocked(server,MAKE_IPV4_ADDRESS_PORT(1,2,3,4,5))); + ASSERT_FALSE(is_blocked(server,MAKE_IPV4_ADDRESS_PORT(1,2,3,4,6))); +} + namespace nodetool { template class node_server>; } namespace cryptonote { template class t_cryptonote_protocol_handler; }