From 9c4b3c26fab0647c0a03789e64d6ae0699855078 Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 28 Apr 2023 10:17:37 +1000 Subject: [PATCH] fix(net): Reject nodes using ZClassic ports, and warn if configured with those ports (#6567) * Reject nodes using the ZClassic default ports * Always check regtest and other coin ports, even if no network is supplied * Warn if Zebra is configured with ports from other coins * Allow unspecified addresses and ports for inbound listeners --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- zebra-network/src/peer.rs | 5 ++- zebra-network/src/peer/priority.rs | 43 +++++++++++++++++------- zebra-network/src/peer_set/initialize.rs | 22 ++++++------ 3 files changed, 45 insertions(+), 25 deletions(-) diff --git a/zebra-network/src/peer.rs b/zebra-network/src/peer.rs index 52291d64..13498e9b 100644 --- a/zebra-network/src/peer.rs +++ b/zebra-network/src/peer.rs @@ -28,4 +28,7 @@ pub use error::{ErrorSlot, HandshakeError, PeerError, SharedPeerError}; pub use handshake::{ConnectedAddr, ConnectionInfo, Handshake, HandshakeRequest}; pub use load_tracked_client::LoadTrackedClient; pub use minimum_peer_version::MinimumPeerVersion; -pub use priority::{AttributePreference, PeerPreference}; +pub use priority::{ + address_is_valid_for_inbound_listeners, address_is_valid_for_outbound_connections, + AttributePreference, PeerPreference, +}; diff --git a/zebra-network/src/peer/priority.rs b/zebra-network/src/peer/priority.rs index d50130e7..f754c8bb 100644 --- a/zebra-network/src/peer/priority.rs +++ b/zebra-network/src/peer/priority.rs @@ -88,7 +88,7 @@ impl PeerPreference { /// used to permanently reject entire [`MetaAddr`]s. /// /// [`MetaAddr`]: crate::meta_addr::MetaAddr -fn address_is_valid_for_outbound_connections( +pub fn address_is_valid_for_outbound_connections( peer_addr: &SocketAddr, network: impl Into>, ) -> Result<(), &'static str> { @@ -105,30 +105,47 @@ fn address_is_valid_for_outbound_connections( ); } - // Ignore ports used by similar networks: Flux/ZelCash and misconfigured Zcash. + address_is_valid_for_inbound_listeners(peer_addr, network) +} + +/// Is the supplied [`SocketAddr`] valid for inbound listeners on `network`? +/// +/// This is used to check Zebra's configured Zcash listener port. +pub fn address_is_valid_for_inbound_listeners( + listener_addr: &SocketAddr, + network: impl Into>, +) -> Result<(), &'static str> { + // TODO: make private IP addresses an error unless a debug config is set (#3117) + + // Ignore ports used by potentially compatible nodes: misconfigured Zcash ports. if let Some(network) = network.into() { - if peer_addr.port() == network.default_port() { + if listener_addr.port() == network.default_port() { return Ok(()); } - if peer_addr.port() == 8232 { + if listener_addr.port() == 8232 { return Err( "invalid peer port: port is for Mainnet, but this node is configured for Testnet", ); - } else if peer_addr.port() == 18232 { + } else if listener_addr.port() == 18232 { return Err( "invalid peer port: port is for Testnet, but this node is configured for Mainnet", ); - } else if peer_addr.port() == 18344 { - return Err( - "invalid peer port: port is for Regtest, but Zebra does not support that network", - ); - } else if [16125, 26125].contains(&peer_addr.port()) { - // 16125/26125 is used by Flux/ZelCash, which uses the same network magic numbers as Zcash, - // so we have to reject it by port - return Err("invalid peer port: port is for a non-Zcash network"); } } + // Ignore ports used by potentially compatible nodes: other coins and unsupported Zcash regtest. + if listener_addr.port() == 18344 { + return Err( + "invalid peer port: port is for Regtest, but Zebra does not support that network", + ); + } else if [8033, 18033, 16125, 26125].contains(&listener_addr.port()) { + // These coins use the same network magic numbers as Zcash, so we have to reject them by port: + // - ZClassic: 8033/18033 + // https://github.com/ZclassicCommunity/zclassic/blob/504362bbf72400f51acdba519e12707da44138c2/src/chainparams.cpp#L130 + // - Flux/ZelCash: 16125/26125 + return Err("invalid peer port: port is for a non-Zcash coin"); + } + Ok(()) } diff --git a/zebra-network/src/peer_set/initialize.rs b/zebra-network/src/peer_set/initialize.rs index 4abf769b..21993ea6 100644 --- a/zebra-network/src/peer_set/initialize.rs +++ b/zebra-network/src/peer_set/initialize.rs @@ -28,13 +28,16 @@ use tower::{ }; use tracing_futures::Instrument; -use zebra_chain::{chain_tip::ChainTip, parameters::Network}; +use zebra_chain::chain_tip::ChainTip; use crate::{ address_book_updater::AddressBookUpdater, constants, meta_addr::{MetaAddr, MetaAddrChange}, - peer::{self, HandshakeRequest, MinimumPeerVersion, OutboundConnectorRequest, PeerPreference}, + peer::{ + self, address_is_valid_for_inbound_listeners, HandshakeRequest, MinimumPeerVersion, + OutboundConnectorRequest, PeerPreference, + }, peer_set::{set::MorePeers, ActiveConnectionCounter, CandidateSet, ConnectionTracker, PeerSet}, AddressBook, BoxError, Config, Request, Response, }; @@ -465,17 +468,14 @@ async fn limit_initial_peers( #[instrument(skip(config), fields(addr = ?config.listen_addr))] pub(crate) async fn open_listener(config: &Config) -> (TcpListener, SocketAddr) { // Warn if we're configured using the wrong network port. - use Network::*; - let wrong_net = match config.network { - Mainnet => Testnet, - Testnet => Mainnet, - }; - if config.listen_addr.port() == wrong_net.default_port() { + if let Err(wrong_addr) = + address_is_valid_for_inbound_listeners(&config.listen_addr, config.network) + { warn!( - "We are configured with port {} for {:?}, but that port is the default port for {:?}. The default port for {:?} is {}.", - config.listen_addr.port(), + "We are configured with address {} on {:?}, but it could cause network issues. \ + The default port for {:?} is {}. Error: {wrong_addr:?}", + config.listen_addr, config.network, - wrong_net, config.network, config.network.default_port(), );