diff --git a/nano/core_test/network.cpp b/nano/core_test/network.cpp index 94bd1db94d..d66cafb097 100644 --- a/nano/core_test/network.cpp +++ b/nano/core_test/network.cpp @@ -1060,10 +1060,9 @@ TEST (peer_exclusion, validate) nano::peer_exclusion excluded_peers; size_t fake_peers_count = 10; auto max_size = excluded_peers.limited_size (fake_peers_count); - auto address (boost::asio::ip::address_v6::loopback ()); for (auto i = 0; i < max_size + 2; ++i) { - nano::tcp_endpoint endpoint (address, i); + nano::tcp_endpoint endpoint (boost::asio::ip::address_v6::v4_mapped (boost::asio::ip::address_v4 (i)), 0); ASSERT_FALSE (excluded_peers.check (endpoint)); ASSERT_EQ (1, excluded_peers.add (endpoint, fake_peers_count)); ASSERT_FALSE (excluded_peers.check (endpoint)); @@ -1071,21 +1070,22 @@ TEST (peer_exclusion, validate) // The oldest one must have been removed ASSERT_EQ (max_size + 1, excluded_peers.size ()); auto & peers_by_endpoint (excluded_peers.peers.get ()); - ASSERT_EQ (peers_by_endpoint.end (), peers_by_endpoint.find (nano::tcp_endpoint (address, 0))); + nano::tcp_endpoint oldest (boost::asio::ip::address_v6::v4_mapped (boost::asio::ip::address_v4 (0x0)), 0); + ASSERT_EQ (peers_by_endpoint.end (), peers_by_endpoint.find (oldest.address ())); auto to_seconds = [](std::chrono::steady_clock::time_point const & timepoint) { return std::chrono::duration_cast (timepoint.time_since_epoch ()).count (); }; - nano::tcp_endpoint first (address, 1); - ASSERT_NE (peers_by_endpoint.end (), peers_by_endpoint.find (first)); - nano::tcp_endpoint second (address, 2); + nano::tcp_endpoint first (boost::asio::ip::address_v6::v4_mapped (boost::asio::ip::address_v4 (0x1)), 0); + ASSERT_NE (peers_by_endpoint.end (), peers_by_endpoint.find (first.address ())); + nano::tcp_endpoint second (boost::asio::ip::address_v6::v4_mapped (boost::asio::ip::address_v4 (0x2)), 0); ASSERT_EQ (false, excluded_peers.check (second)); - ASSERT_NEAR (to_seconds (std::chrono::steady_clock::now () + excluded_peers.exclude_time_hours), to_seconds (peers_by_endpoint.find (second)->exclude_until), 2); + ASSERT_NEAR (to_seconds (std::chrono::steady_clock::now () + excluded_peers.exclude_time_hours), to_seconds (peers_by_endpoint.find (second.address ())->exclude_until), 2); ASSERT_EQ (2, excluded_peers.add (second, fake_peers_count)); - ASSERT_EQ (peers_by_endpoint.end (), peers_by_endpoint.find (first)); - ASSERT_NEAR (to_seconds (std::chrono::steady_clock::now () + excluded_peers.exclude_time_hours), to_seconds (peers_by_endpoint.find (second)->exclude_until), 2); + ASSERT_EQ (peers_by_endpoint.end (), peers_by_endpoint.find (first.address ())); + ASSERT_NEAR (to_seconds (std::chrono::steady_clock::now () + excluded_peers.exclude_time_hours), to_seconds (peers_by_endpoint.find (second.address ())->exclude_until), 2); ASSERT_EQ (3, excluded_peers.add (second, fake_peers_count)); - ASSERT_NEAR (to_seconds (std::chrono::steady_clock::now () + excluded_peers.exclude_time_hours * 3 * 2), to_seconds (peers_by_endpoint.find (second)->exclude_until), 2); + ASSERT_NEAR (to_seconds (std::chrono::steady_clock::now () + excluded_peers.exclude_time_hours * 3 * 2), to_seconds (peers_by_endpoint.find (second.address ())->exclude_until), 2); ASSERT_EQ (max_size, excluded_peers.size ()); // Clear many entries if there are a low number of peers @@ -1105,3 +1105,39 @@ TEST (peer_exclusion, validate) ASSERT_EQ (sizeof (decltype (excluded_peers.peers)::value_type), child_info.sizeof_element); } } + +TEST (network, tcp_no_connect_excluded_peers) +{ + nano::system system (1); + auto node0 (system.nodes[0]); + ASSERT_EQ (0, node0->network.size ()); + auto node1 (std::make_shared (system.io_ctx, nano::get_available_port (), nano::unique_path (), system.alarm, system.logging, system.work)); + node1->start (); + system.nodes.push_back (node1); + auto endpoint1 (node1->network.endpoint ()); + auto endpoint1_tcp (nano::transport::map_endpoint_to_tcp (endpoint1)); + while (!node0->network.excluded_peers.check (endpoint1_tcp)) + { + node0->network.excluded_peers.add (endpoint1_tcp, 1); + } + ASSERT_EQ (0, node0->stats.count (nano::stat::type::tcp, nano::stat::detail::tcp_excluded)); + node1->network.merge_peer (node0->network.endpoint ()); + ASSERT_TIMELY (5s, node0->stats.count (nano::stat::type::tcp, nano::stat::detail::tcp_excluded) >= 1); + ASSERT_EQ (nullptr, node0->network.find_channel (endpoint1)); + + // Should not actively reachout to excluded peers + ASSERT_TRUE (node0->network.reachout (endpoint1, true)); + + // Erasing from excluded peers should allow a connection + node0->network.excluded_peers.remove (endpoint1_tcp); + ASSERT_FALSE (node0->network.excluded_peers.check (endpoint1_tcp)); + + // Manually cleanup previous attempt + node1->network.cleanup (std::chrono::steady_clock::now ()); + node1->network.syn_cookies.purge (std::chrono::steady_clock::now ()); + + // Ensure a successful connection + ASSERT_EQ (0, node0->network.size ()); + node1->network.merge_peer (node0->network.endpoint ()); + ASSERT_TIMELY (5s, node0->network.size () == 1); +} diff --git a/nano/core_test/node_telemetry.cpp b/nano/core_test/node_telemetry.cpp index d4eed98945..8a2eab219a 100644 --- a/nano/core_test/node_telemetry.cpp +++ b/nano/core_test/node_telemetry.cpp @@ -718,6 +718,8 @@ TEST (node_telemetry, disable_metrics) } } +namespace nano +{ TEST (node_telemetry, remove_peer_different_genesis) { nano::system system (1); @@ -738,6 +740,10 @@ TEST (node_telemetry, remove_peer_different_genesis) ASSERT_EQ (node0->stats.count (nano::stat::type::message, nano::stat::detail::node_id_handshake, nano::stat::dir::out), 1); ASSERT_EQ (node1->stats.count (nano::stat::type::message, nano::stat::detail::node_id_handshake, nano::stat::dir::out), 1); + system.poll_until_true (3s, [&node0, address = node1->network.endpoint ().address ()]() -> bool { + nano::lock_guard guard (node0->network.excluded_peers.mutex); + return node0->network.excluded_peers.peers.get ().count (address); + }); } TEST (node_telemetry, remove_peer_different_genesis_udp) @@ -763,10 +769,12 @@ TEST (node_telemetry, remove_peer_different_genesis_udp) ASSERT_EQ (0, node0->network.size ()); ASSERT_EQ (0, node1->network.size ()); + system.poll_until_true (3s, [&node0, address = node1->network.endpoint ().address ()]() -> bool { + nano::lock_guard guard (node0->network.excluded_peers.mutex); + return node0->network.excluded_peers.peers.get ().count (address); + }); } -namespace nano -{ TEST (node_telemetry, remove_peer_invalid_signature) { nano::system system; @@ -788,5 +796,9 @@ TEST (node_telemetry, remove_peer_invalid_signature) node->network.process_message (telemetry_ack, channel); ASSERT_TIMELY (10s, node->stats.count (nano::stat::type::telemetry, nano::stat::detail::invalid_signature) > 0); + system.poll_until_true (3s, [&node, address = channel->get_endpoint ().address ()]() -> bool { + nano::lock_guard guard (node->network.excluded_peers.mutex); + return node->network.excluded_peers.peers.get ().count (address); + }); +} } -} \ No newline at end of file diff --git a/nano/core_test/toml.cpp b/nano/core_test/toml.cpp index 0239107919..73398eb444 100644 --- a/nano/core_test/toml.cpp +++ b/nano/core_test/toml.cpp @@ -202,6 +202,7 @@ TEST (toml, daemon_config_deserialize_defaults) ASSERT_EQ (conf.node.logging.network_timeout_logging_value, defaults.node.logging.network_timeout_logging_value); ASSERT_EQ (conf.node.logging.node_lifetime_tracing_value, defaults.node.logging.node_lifetime_tracing_value); ASSERT_EQ (conf.node.logging.network_telemetry_logging_value, defaults.node.logging.network_telemetry_logging_value); + ASSERT_EQ (conf.node.logging.network_rejected_logging_value, defaults.node.logging.network_rejected_logging_value); ASSERT_EQ (conf.node.logging.rotation_size, defaults.node.logging.rotation_size); ASSERT_EQ (conf.node.logging.single_line_record_value, defaults.node.logging.single_line_record_value); ASSERT_EQ (conf.node.logging.stable_log_filename, defaults.node.logging.stable_log_filename); @@ -469,6 +470,7 @@ TEST (toml, daemon_config_deserialize_no_defaults) network_message = true network_node_id_handshake = true network_telemetry_logging = true + network_rejected_logging = true network_packet = true network_publish = true network_timeout = true @@ -607,6 +609,7 @@ TEST (toml, daemon_config_deserialize_no_defaults) ASSERT_NE (conf.node.logging.network_message_logging_value, defaults.node.logging.network_message_logging_value); ASSERT_NE (conf.node.logging.network_node_id_handshake_logging_value, defaults.node.logging.network_node_id_handshake_logging_value); ASSERT_NE (conf.node.logging.network_telemetry_logging_value, defaults.node.logging.network_telemetry_logging_value); + ASSERT_NE (conf.node.logging.network_rejected_logging_value, defaults.node.logging.network_rejected_logging_value); ASSERT_NE (conf.node.logging.network_packet_logging_value, defaults.node.logging.network_packet_logging_value); ASSERT_NE (conf.node.logging.network_publish_logging_value, defaults.node.logging.network_publish_logging_value); ASSERT_NE (conf.node.logging.network_timeout_logging_value, defaults.node.logging.network_timeout_logging_value); diff --git a/nano/lib/stats.cpp b/nano/lib/stats.cpp index c039f42066..c51d597731 100644 --- a/nano/lib/stats.cpp +++ b/nano/lib/stats.cpp @@ -628,6 +628,8 @@ std::string nano::stat::detail_to_string (uint32_t key) case nano::stat::detail::tcp_write_drop: res = "tcp_write_drop"; break; + case nano::stat::detail::tcp_excluded: + res = "tcp_excluded"; case nano::stat::detail::unreachable_host: res = "unreachable_host"; break; diff --git a/nano/lib/stats.hpp b/nano/lib/stats.hpp index a76aef5df1..2dff45ae5e 100644 --- a/nano/lib/stats.hpp +++ b/nano/lib/stats.hpp @@ -294,6 +294,7 @@ class stat final tcp_accept_success, tcp_accept_failure, tcp_write_drop, + tcp_excluded, // ipc invocations, diff --git a/nano/node/bootstrap/bootstrap_attempt.cpp b/nano/node/bootstrap/bootstrap_attempt.cpp index 590960fbb7..57227144b8 100644 --- a/nano/node/bootstrap/bootstrap_attempt.cpp +++ b/nano/node/bootstrap/bootstrap_attempt.cpp @@ -321,6 +321,11 @@ void nano::bootstrap_attempt_legacy::attempt_restart_check (nano::unique_lock= nano::peer_exclusion::score_limit) { node->logger.always_log (boost::str (boost::format ("Adding peer %1% to excluded peers list with score %2% after %3% seconds bootstrap attempt") % endpoint_frontier_request % score % std::chrono::duration_cast (std::chrono::steady_clock::now () - attempt_start).count ())); + auto channel = node->network.find_channel (nano::transport::map_tcp_to_endpoint (endpoint_frontier_request)); + if (channel != nullptr) + { + node->network.erase (*channel); + } } lock_a.unlock (); stop (); diff --git a/nano/node/bootstrap/bootstrap_server.cpp b/nano/node/bootstrap/bootstrap_server.cpp index d4f98f8868..2da045e538 100644 --- a/nano/node/bootstrap/bootstrap_server.cpp +++ b/nano/node/bootstrap/bootstrap_server.cpp @@ -64,12 +64,21 @@ size_t nano::bootstrap_listener::connection_count () void nano::bootstrap_listener::accept_action (boost::system::error_code const & ec, std::shared_ptr socket_a) { - auto connection (std::make_shared (socket_a, node.shared ())); + if (!node.network.excluded_peers.check (socket_a->remote_endpoint ())) { + auto connection (std::make_shared (socket_a, node.shared ())); nano::lock_guard lock (mutex); connections[connection.get ()] = connection; connection->receive (); } + else + { + node.stats.inc (nano::stat::type::tcp, nano::stat::detail::tcp_excluded); + if (node.config.logging.network_rejected_logging ()) + { + node.logger.try_log ("Rejected connection from excluded peer ", socket_a->remote_endpoint ()); + } + } } boost::asio::ip::tcp::endpoint nano::bootstrap_listener::endpoint () diff --git a/nano/node/logging.cpp b/nano/node/logging.cpp index 66bf4e1d23..8934e2028b 100644 --- a/nano/node/logging.cpp +++ b/nano/node/logging.cpp @@ -134,6 +134,7 @@ nano::error nano::logging::serialize_toml (nano::tomlconfig & toml) const toml.put ("network_keepalive", network_keepalive_logging_value, "Log keepalive related messages.\ntype:bool"); toml.put ("network_node_id_handshake", network_node_id_handshake_logging_value, "Log node-id handshake related messages.\ntype:bool"); toml.put ("network_telemetry", network_telemetry_logging_value, "Log telemetry related messages.\ntype:bool"); + toml.put ("network_rejected", network_rejected_logging_value, "Log message when a connection is rejected.\ntype:bool"); toml.put ("node_lifetime_tracing", node_lifetime_tracing_value, "Log node startup and shutdown messages.\ntype:bool"); toml.put ("insufficient_work", insufficient_work_logging_value, "Log if insufficient work is detected.\ntype:bool"); toml.put ("log_ipc", log_ipc_value, "Log IPC related activity.\ntype:bool"); @@ -166,6 +167,7 @@ nano::error nano::logging::deserialize_toml (nano::tomlconfig & toml) toml.get ("network_keepalive", network_keepalive_logging_value); toml.get ("network_node_id_handshake", network_node_id_handshake_logging_value); toml.get ("network_telemetry_logging", network_telemetry_logging_value); + toml.get ("network_rejected_logging", network_rejected_logging_value); toml.get ("node_lifetime_tracing", node_lifetime_tracing_value); toml.get ("insufficient_work", insufficient_work_logging_value); toml.get ("log_ipc", log_ipc_value); @@ -201,6 +203,7 @@ nano::error nano::logging::serialize_json (nano::jsonconfig & json) const json.put ("network_keepalive", network_keepalive_logging_value); json.put ("network_node_id_handshake", network_node_id_handshake_logging_value); json.put ("network_telemetry_logging", network_telemetry_logging_value); + json.put ("network_rejected_logging", network_rejected_logging_value); json.put ("node_lifetime_tracing", node_lifetime_tracing_value); json.put ("insufficient_work", insufficient_work_logging_value); json.put ("log_ipc", log_ipc_value); @@ -358,6 +361,11 @@ bool nano::logging::network_telemetry_logging () const return network_logging () && network_telemetry_logging_value; } +bool nano::logging::network_rejected_logging () const +{ + return network_logging () && network_rejected_logging_value; +} + bool nano::logging::node_lifetime_tracing () const { return node_lifetime_tracing_value; diff --git a/nano/node/logging.hpp b/nano/node/logging.hpp index e42a7aba53..c52290b08c 100644 --- a/nano/node/logging.hpp +++ b/nano/node/logging.hpp @@ -53,6 +53,7 @@ class logging final bool network_keepalive_logging () const; bool network_node_id_handshake_logging () const; bool network_telemetry_logging () const; + bool network_rejected_logging () const; bool node_lifetime_tracing () const; bool insufficient_work_logging () const; bool upnp_details_logging () const; @@ -77,6 +78,7 @@ class logging final bool network_keepalive_logging_value{ false }; bool network_node_id_handshake_logging_value{ false }; bool network_telemetry_logging_value{ false }; + bool network_rejected_logging_value{ false }; bool node_lifetime_tracing_value{ false }; bool insufficient_work_logging_value{ true }; bool log_ipc_value{ true }; diff --git a/nano/node/peer_exclusion.cpp b/nano/node/peer_exclusion.cpp index c43f15625e..2eba6927af 100644 --- a/nano/node/peer_exclusion.cpp +++ b/nano/node/peer_exclusion.cpp @@ -17,11 +17,12 @@ uint64_t nano::peer_exclusion::add (nano::tcp_endpoint const & endpoint_a, size_ } debug_assert (peers.size () <= size_max); auto & peers_by_endpoint (peers.get ()); - auto existing (peers_by_endpoint.find (endpoint_a)); + auto address = endpoint_a.address (); + auto existing (peers_by_endpoint.find (address)); if (existing == peers_by_endpoint.end ()) { // Insert new endpoint - auto inserted (peers.emplace (peer_exclusion::item{ std::chrono::steady_clock::steady_clock::now () + exclude_time_hours, endpoint_a, 1 })); + auto inserted (peers.emplace (peer_exclusion::item{ std::chrono::steady_clock::steady_clock::now () + exclude_time_hours, address, 1 })); (void)inserted; debug_assert (inserted.second); result = 1; @@ -50,7 +51,7 @@ bool nano::peer_exclusion::check (nano::tcp_endpoint const & endpoint_a) bool excluded (false); nano::lock_guard guard (mutex); auto & peers_by_endpoint (peers.get ()); - auto existing (peers_by_endpoint.find (endpoint_a)); + auto existing (peers_by_endpoint.find (endpoint_a.address ())); if (existing != peers_by_endpoint.end () && existing->score >= score_limit) { if (existing->exclude_until > std::chrono::steady_clock::now ()) @@ -68,7 +69,7 @@ bool nano::peer_exclusion::check (nano::tcp_endpoint const & endpoint_a) void nano::peer_exclusion::remove (nano::tcp_endpoint const & endpoint_a) { nano::lock_guard guard (mutex); - peers.get ().erase (endpoint_a); + peers.get ().erase (endpoint_a.address ()); } size_t nano::peer_exclusion::limited_size (size_t const network_peers_count_a) const diff --git a/nano/node/peer_exclusion.hpp b/nano/node/peer_exclusion.hpp index e7013d39d9..97b09d2f0a 100644 --- a/nano/node/peer_exclusion.hpp +++ b/nano/node/peer_exclusion.hpp @@ -16,7 +16,7 @@ class peer_exclusion final public: item () = delete; std::chrono::steady_clock::time_point exclude_until; - nano::tcp_endpoint endpoint; + decltype (std::declval ().address ()) address; uint64_t score; }; @@ -32,7 +32,7 @@ class peer_exclusion final mi::ordered_non_unique, mi::member>, mi::hashed_unique, - mi::member>>>; + mi::member>>>; // clang-format on private: @@ -52,6 +52,9 @@ class peer_exclusion final size_t limited_size (size_t const) const; size_t size () const; + friend class node_telemetry_remove_peer_different_genesis_Test; + friend class node_telemetry_remove_peer_different_genesis_udp_Test; + friend class node_telemetry_remove_peer_invalid_signature_Test; friend class peer_exclusion_validate_Test; }; std::unique_ptr collect_container_info (peer_exclusion const & excluded_peers, const std::string & name); diff --git a/nano/node/telemetry.cpp b/nano/node/telemetry.cpp index 8ed405e8d0..1015557a62 100644 --- a/nano/node/telemetry.cpp +++ b/nano/node/telemetry.cpp @@ -110,6 +110,9 @@ bool nano::telemetry::verify_message (nano::telemetry_ack const & message_a, nan if (remove_channel) { + // Add to peer exclusion list + network.excluded_peers.add (channel_a.get_tcp_endpoint (), network.size ()); + // Disconnect from peer with incorrect telemetry data network.erase (channel_a); } diff --git a/nano/node/transport/tcp.cpp b/nano/node/transport/tcp.cpp index f38343d98f..9837e70341 100644 --- a/nano/node/transport/tcp.cpp +++ b/nano/node/transport/tcp.cpp @@ -275,31 +275,34 @@ void nano::transport::tcp_channels::process_message (nano::message const & messa { node.network.process_message (message_a, channel); } - else if (!node_id_a.is_zero ()) + else if (!node.network.excluded_peers.check (endpoint_a)) { - // Add temporary channel - socket_a->set_writer_concurrency (nano::socket::concurrency::multi_writer); - auto temporary_channel (std::make_shared (node, socket_a)); - debug_assert (endpoint_a == temporary_channel->get_tcp_endpoint ()); - temporary_channel->set_node_id (node_id_a); - temporary_channel->set_network_version (message_a.header.version_using); - temporary_channel->set_last_packet_received (std::chrono::steady_clock::now ()); - temporary_channel->set_last_packet_sent (std::chrono::steady_clock::now ()); - temporary_channel->temporary = true; - debug_assert (type_a == nano::bootstrap_server_type::realtime || type_a == nano::bootstrap_server_type::realtime_response_server); - // Don't insert temporary channels for response_server - if (type_a == nano::bootstrap_server_type::realtime) + if (!node_id_a.is_zero ()) { - insert (temporary_channel, socket_a, nullptr); + // Add temporary channel + socket_a->set_writer_concurrency (nano::socket::concurrency::multi_writer); + auto temporary_channel (std::make_shared (node, socket_a)); + debug_assert (endpoint_a == temporary_channel->get_tcp_endpoint ()); + temporary_channel->set_node_id (node_id_a); + temporary_channel->set_network_version (message_a.header.version_using); + temporary_channel->set_last_packet_received (std::chrono::steady_clock::now ()); + temporary_channel->set_last_packet_sent (std::chrono::steady_clock::now ()); + temporary_channel->temporary = true; + debug_assert (type_a == nano::bootstrap_server_type::realtime || type_a == nano::bootstrap_server_type::realtime_response_server); + // Don't insert temporary channels for response_server + if (type_a == nano::bootstrap_server_type::realtime) + { + insert (temporary_channel, socket_a, nullptr); + } + node.network.process_message (message_a, temporary_channel); + } + else + { + // Initial node_id_handshake request without node ID + debug_assert (message_a.header.type == nano::message_type::node_id_handshake); + debug_assert (type_a == nano::bootstrap_server_type::undefined); + node.stats.inc (nano::stat::type::message, nano::stat::detail::node_id_handshake, nano::stat::dir::in); } - node.network.process_message (message_a, temporary_channel); - } - else - { - // Initial node_id_handshake request without node ID - debug_assert (message_a.header.type == nano::message_type::node_id_handshake); - debug_assert (type_a == nano::bootstrap_server_type::undefined); - node.stats.inc (nano::stat::type::message, nano::stat::detail::node_id_handshake, nano::stat::dir::in); } } } @@ -367,7 +370,7 @@ bool nano::transport::tcp_channels::reachout (nano::endpoint const & endpoint_a) { auto tcp_endpoint (nano::transport::map_endpoint_to_tcp (endpoint_a)); // Don't overload single IP - bool error = max_ip_connections (tcp_endpoint); + bool error = node.network.excluded_peers.check (tcp_endpoint) || max_ip_connections (tcp_endpoint); if (!error && !node.flags.disable_tcp_realtime) { // Don't keepalive to nodes that already sent us something @@ -440,7 +443,7 @@ void nano::transport::tcp_channels::ongoing_keepalive () for (auto i (0); i <= random_count; ++i) { auto tcp_endpoint (node.network.udp_channels.bootstrap_peer (node.network_params.protocol.protocol_version_min)); - if (tcp_endpoint != invalid_endpoint && find_channel (tcp_endpoint) == nullptr) + if (tcp_endpoint != invalid_endpoint && find_channel (tcp_endpoint) == nullptr && !node.network.excluded_peers.check (tcp_endpoint)) { start_tcp (nano::transport::map_tcp_to_endpoint (tcp_endpoint)); }