diff --git a/nano/core_test/network.cpp b/nano/core_test/network.cpp index a6940eb74f..ba6e6023e8 100644 --- a/nano/core_test/network.cpp +++ b/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 (keepalive.header.network) = nano::networks::nano_dev_network; - node1.network.inbound (keepalive, std::make_shared (node1)); - ASSERT_EQ (0, node1.stats.count (nano::stat::type::message, nano::stat::detail::invalid_network)); const_cast (keepalive.header.network) = nano::networks::invalid; - node1.network.inbound (keepalive, std::make_shared (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 (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) diff --git a/nano/node/bootstrap/bootstrap_server.cpp b/nano/node/bootstrap/bootstrap_server.cpp index 9c898589c4..5a5529666e 100644 --- a/nano/node/bootstrap/bootstrap_server.cpp +++ b/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) { diff --git a/nano/node/common.cpp b/nano/node/common.cpp index f4cbc7a144..ddfa9cb2ab 100644 --- a/nano/node/common.cpp +++ b/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; diff --git a/nano/node/network.cpp b/nano/node/network.cpp index 0b2ed57909..3e4ca6d85d 100644 --- a/nano/node/network.cpp +++ b/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 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), diff --git a/nano/node/transport/tcp.cpp b/nano/node/transport/tcp.cpp index c0d6259a23..2b678096d2 100644 --- a/nano/node/transport/tcp.cpp +++ b/nano/node/transport/tcp.cpp @@ -631,9 +631,10 @@ void nano::transport::tcp_channels::start_tcp_receive_node_id (std::shared_ptrdata (), 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_ptrnetwork_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); {