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

@@ -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,
+};

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<Option<Network>>,
 ) -> 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<Option<Network>>,
+) -> 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(())
 }

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