Bugfix: correctly check for magic bytes and network in message header

The network bytes (magic bytes), the first 2 byts of the header, were not
correctly checked. They were checked in the network class inbound function
but that function is called for realtime messages after the handshake is
completed. It is not called for bootstrap messages not handshake messages.

Also the version checking was not done properly so I am doing it now
properly immediately after the magic bytes tests.

The checks are done at the following 3 places:
* nano::bootstrap_server::receive_header_action
* nano::transport::tcp_channels::start_tcp_receive_node_id
* nano::message_parser::deserialize_buffer

The last one is likely only used by UDP and test scripts that use UDP.
So we should be able to remove the last one when we remove the UDP code.

nano/core_test/network.cpp

@@ -1249,17 +1249,41 @@ TEST (network, loopback_channel)
 }
 
 // Ensure the network filters messages with the incorrect magic number
-TEST (network, filter)
+TEST (network, filter_invalid_network_bytes)
 {
-	nano::system system{ 1 };
+	nano::system system{ 2 };
 	auto & node1 = *system.nodes[0];
+	auto & node2 = *system.nodes[1];
+
+	// find the comms channel that goes from node2 to node1
+	auto channel = node2.network.find_channel (node1.network.endpoint ());
+	ASSERT_NE (nullptr, channel);
+
+	// send a keepalive, from node2 to node1, with the wrong network bytes
 	nano::keepalive keepalive{ nano::dev::network_params.network };
-	const_cast<nano::networks &> (keepalive.header.network) = nano::networks::nano_dev_network;
-	node1.network.inbound (keepalive, std::make_shared<nano::transport::channel_loopback> (node1));
-	ASSERT_EQ (0, node1.stats.count (nano::stat::type::message, nano::stat::detail::invalid_network));
 	const_cast<nano::networks &> (keepalive.header.network) = nano::networks::invalid;
-	node1.network.inbound (keepalive, std::make_shared<nano::transport::channel_loopback> (node1));
-	ASSERT_EQ (1, node1.stats.count (nano::stat::type::message, nano::stat::detail::invalid_network));
+	channel->send (keepalive);
+
+	ASSERT_TIMELY (5s, 1 == node1.stats.count (nano::stat::type::message, nano::stat::detail::invalid_network));
+}
+
+// Ensure the network filters messages with the incorrect minimum version
+TEST (network, filter_invalid_version_using)
+{
+	nano::system system{ 2 };
+	auto & node1 = *system.nodes[0];
+	auto & node2 = *system.nodes[1];
+
+	// find the comms channel that goes from node2 to node1
+	auto channel = node2.network.find_channel (node1.network.endpoint ());
+	ASSERT_NE (nullptr, channel);
+
+	// send a keepalive, from node2 to node1, with the wrong version_using
+	nano::keepalive keepalive{ nano::dev::network_params.network };
+	const_cast<uint8_t &> (keepalive.header.version_using) = nano::dev::network_params.network.protocol_version_min - 1;
+	channel->send (keepalive);
+
+	ASSERT_TIMELY (5s, 1 == node1.stats.count (nano::stat::type::message, nano::stat::detail::outdated_version));
 }
 
 TEST (network, fill_keepalive_self)

nano/node/bootstrap/bootstrap_server.cpp

@@ -168,6 +168,18 @@ void nano::bootstrap_server::receive_header_action (boost::system::error_code co
 		nano::message_header header (error, type_stream);
 		if (!error)
 		{
+			if (header.network != node->network_params.network.current_network)
+			{
+				node->stats.inc (nano::stat::type::message, nano::stat::detail::invalid_network);
+				return;
+			}
+
+			if (header.version_using < node->network_params.network.protocol_version_min)
+			{
+				node->stats.inc (nano::stat::type::message, nano::stat::detail::outdated_version);
+				return;
+			}
+
 			auto this_l (shared_from_this ());
 			switch (header.type)
 			{

nano/node/common.cpp

@@ -337,6 +337,12 @@ void nano::message_parser::deserialize_buffer (uint8_t const * buffer_a, std::si
 		nano::message_header header (error, stream);
 		if (!error)
 		{
+			if (header.network != network.current_network)
+			{
+				status = parse_status::invalid_header;
+				return;
+			}
+
 			if (header.version_using < network.protocol_version_min)
 			{
 				status = parse_status::outdated_version;

nano/node/network.cpp

@@ -15,14 +15,9 @@ nano::network::network (nano::node & node_a, uint16_t port_a) :
 	id (nano::network_constants::active_network),
 	syn_cookies (node_a.network_params.network.max_peers_per_ip),
 	inbound{ [this] (nano::message const & message, std::shared_ptr<nano::transport::channel> const & channel) {
-		if (message.header.network == id)
-		{
-			process_message (message, channel);
-		}
-		else
-		{
-			this->node.stats.inc (nano::stat::type::message, nano::stat::detail::invalid_network);
-		}
+		debug_assert (message.header.network == node.network_params.network.current_network);
+		debug_assert (message.header.version_using >= node.network_params.network.protocol_version_min);
+		process_message (message, channel);
 	} },
 	buffer_container (node_a.stats, nano::network::buffer_size, 4096), // 2Mb receive buffer
 	resolver (node_a.io_ctx),

nano/node/transport/tcp.cpp

@@ -631,9 +631,10 @@ void nano::transport::tcp_channels::start_tcp_receive_node_id (std::shared_ptr<n
 					auto error (false);
 					nano::bufferstream stream (receive_buffer_a->data (), size_a);
 					nano::message_header header (error, stream);
+					// the header type should in principle be checked after checking the network bytes and the version numbers, I will not change it here since the benefits do not outweight the difficulties
 					if (!error && header.type == nano::message_type::node_id_handshake)
 					{
-						if (header.version_using >= node_l->network_params.network.protocol_version_min)
+						if (header.network == node_l->network_params.network.current_network && header.version_using >= node_l->network_params.network.protocol_version_min)
 						{
 							nano::node_id_handshake message (error, stream, header);
 							if (!error && message.response && message.query)
@@ -704,6 +705,16 @@ void nano::transport::tcp_channels::start_tcp_receive_node_id (std::shared_ptr<n
 						}
 						else
 						{
+							// error handling, either the networks bytes or the version is wrong
+							if (header.network == node_l->network_params.network.current_network)
+							{
+								node_l->stats.inc (nano::stat::type::message, nano::stat::detail::invalid_network);
+							}
+							else
+							{
+								node_l->stats.inc (nano::stat::type::message, nano::stat::detail::outdated_version);
+							}
+
 							// Version of channel is not high enough, just abort. Don't fallback to udp, instead cleanup attempt
 							cleanup_node_id_handshake_socket (endpoint_a);
 							{