Prevent reconnecting to excluded peers with sufficient score.

nano/core_test/network.cpp

@@ -1060,32 +1060,32 @@ 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));
 	}
 	// The oldest one must have been removed
 	ASSERT_EQ (max_size + 1, excluded_peers.size ());
 	auto & peers_by_endpoint (excluded_peers.peers.get<nano::peer_exclusion::tag_endpoint> ());
-	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<std::chrono::seconds> (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<nano::node> (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);
+}

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<std::mutex> guard (node0->network.excluded_peers.mutex);
+		return node0->network.excluded_peers.peers.get<nano::peer_exclusion::tag_endpoint> ().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<std::mutex> guard (node0->network.excluded_peers.mutex);
+		return node0->network.excluded_peers.peers.get<nano::peer_exclusion::tag_endpoint> ().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<std::mutex> guard (node->network.excluded_peers.mutex);
+		return node->network.excluded_peers.peers.get<nano::peer_exclusion::tag_endpoint> ().count (address);
+	});
+}
 }
-}

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);

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;

nano/lib/stats.hpp

@@ -294,6 +294,7 @@ class stat final
 		tcp_accept_success,
 		tcp_accept_failure,
 		tcp_write_drop,
+		tcp_excluded,
 
 		// ipc
 		invocations,

nano/node/bootstrap/bootstrap_attempt.cpp

@@ -321,6 +321,11 @@ void nano::bootstrap_attempt_legacy::attempt_restart_check (nano::unique_lock<st
 			if (score >= 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::seconds> (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 ();

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<nano::socket> socket_a)
 {
-	auto connection (std::make_shared<nano::bootstrap_server> (socket_a, node.shared ()));
+	if (!node.network.excluded_peers.check (socket_a->remote_endpoint ()))
 	{
+		auto connection (std::make_shared<nano::bootstrap_server> (socket_a, node.shared ()));
 		nano::lock_guard<std::mutex> 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 ()

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<bool> ("network_keepalive", network_keepalive_logging_value);
 	toml.get<bool> ("network_node_id_handshake", network_node_id_handshake_logging_value);
 	toml.get<bool> ("network_telemetry_logging", network_telemetry_logging_value);
+	toml.get<bool> ("network_rejected_logging", network_rejected_logging_value);
 	toml.get<bool> ("node_lifetime_tracing", node_lifetime_tracing_value);
 	toml.get<bool> ("insufficient_work", insufficient_work_logging_value);
 	toml.get<bool> ("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;

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 };

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<tag_endpoint> ());
-	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<std::mutex> guard (mutex);
 	auto & peers_by_endpoint (peers.get<tag_endpoint> ());
-	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<std::mutex> guard (mutex);
-	peers.get<tag_endpoint> ().erase (endpoint_a);
+	peers.get<tag_endpoint> ().erase (endpoint_a.address ());
 }
 
 size_t nano::peer_exclusion::limited_size (size_t const network_peers_count_a) const

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<nano::tcp_endpoint> ().address ()) address;
 		uint64_t score;
 	};
 
@@ -32,7 +32,7 @@ class peer_exclusion final
 		mi::ordered_non_unique<mi::tag<tag_exclusion>,
 			mi::member<peer_exclusion::item, std::chrono::steady_clock::time_point, &peer_exclusion::item::exclude_until>>,
 		mi::hashed_unique<mi::tag<tag_endpoint>,
-			mi::member<peer_exclusion::item, nano::tcp_endpoint, &item::endpoint>>>>;
+			mi::member<peer_exclusion::item, decltype(peer_exclusion::item::address), &peer_exclusion::item::address>>>>;
 	// 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<container_info_component> collect_container_info (peer_exclusion const & excluded_peers, const std::string & name);

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);
 	}