diff --git a/beacon_node/lighthouse_network/src/discovery/mod.rs b/beacon_node/lighthouse_network/src/discovery/mod.rs index a8c87523a54..446be66f0e7 100644 --- a/beacon_node/lighthouse_network/src/discovery/mod.rs +++ b/beacon_node/lighthouse_network/src/discovery/mod.rs @@ -1240,7 +1240,7 @@ mod tests { let spec = Arc::new(ChainSpec::default()); let keypair = secp256k1::Keypair::generate(); let mut config = NetworkConfig::default(); - config.set_listening_addr(network_utils::listen_addr::ListenAddress::unused_v4_ports()); + config.set_listening_addr(network_utils::listen_addr::ListenAddress::zero_v4_ports()); let config = Arc::new(config); let enr_key: CombinedKey = CombinedKey::from_secp256k1(&keypair); let next_fork_digest = [0; 4]; diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 26dd3b6642e..04f9e8e3dbe 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -17,6 +17,7 @@ use execution_layer::DEFAULT_JWT_FILE; use http_api::TlsConfig; use lighthouse_network::{Enr, Multiaddr, NetworkConfig, PeerIdSerialized, multiaddr::Protocol}; use network_utils::listen_addr::ListenAddress; +use network_utils::listen_addr::compute_listen_ports; use sensitive_url::SensitiveUrl; use std::collections::HashSet; use std::fmt::Debug; @@ -1005,12 +1006,6 @@ pub fn parse_listening_addresses(cli_args: &ArgMatches) -> Result Result Result { // A single ipv4 address was provided. Set the ports - - // use zero ports if required. If not, use the given port. - let tcp_port = use_zero_ports - .then(network_utils::unused_port::unused_tcp4_port) - .transpose()? - .unwrap_or(port); - // use zero ports if required. If not, use the specific discovery port. If none given, use - // the tcp port. - let disc_port = use_zero_ports - .then(network_utils::unused_port::unused_udp4_port) - .transpose()? - .or(maybe_disc_port) - .unwrap_or(tcp_port); - // use zero ports if required. If not, use the specific quic port. If none given, use - // the tcp port + 1. - let quic_port = use_zero_ports - .then(network_utils::unused_port::unused_udp4_port) - .transpose()? - .or(maybe_quic_port) - .unwrap_or(if tcp_port == 0 { 0 } else { tcp_port + 1 }); + let (tcp_port, disc_port, quic_port) = + compute_listen_ports(use_zero_ports, port, maybe_disc_port, maybe_quic_port); ListenAddress::V4(network_utils::listen_addr::ListenAddr { addr: ipv4, @@ -1078,44 +1044,13 @@ pub fn parse_listening_addresses(cli_args: &ArgMatches) -> Result Self { + // Used for testing + pub fn zero_v4_ports() -> Self { ListenAddress::V4(ListenAddr { addr: Ipv4Addr::UNSPECIFIED, - disc_port: crate::unused_port::unused_udp4_port().unwrap(), - quic_port: crate::unused_port::unused_udp4_port().unwrap(), - tcp_port: crate::unused_port::unused_tcp4_port().unwrap(), + disc_port: 0, + quic_port: 0, + tcp_port: 0, }) } - pub fn unused_v6_ports() -> Self { + pub fn zero_v6_ports() -> Self { ListenAddress::V6(ListenAddr { addr: Ipv6Addr::UNSPECIFIED, - disc_port: crate::unused_port::unused_udp6_port().unwrap(), - quic_port: crate::unused_port::unused_udp6_port().unwrap(), - tcp_port: crate::unused_port::unused_tcp6_port().unwrap(), + disc_port: 0, + quic_port: 0, + tcp_port: 0, }) } } + +/// Compute all beacon listening ports (TCP, discovery UDP, QUIC UDP) at once. +/// Returns a tuple of (tcp_port, disc_port, quic_port). +/// +/// When `use_zero_ports` is true, all ports are set to 0 (OS assigns ephemeral ports). +/// Otherwise: +/// - TCP port uses the provided `tcp_port` +/// - Discovery port defaults to TCP port (TCP and UDP can share the same port number) +/// - QUIC port defaults to TCP port + 1 (to avoid conflict with discovery UDP) +pub fn compute_listen_ports( + use_zero_ports: bool, + tcp_port: u16, + maybe_disc_port: Option, + maybe_quic_port: Option, +) -> (u16, u16, u16) { + if use_zero_ports { + return (0, 0, 0); + } + + let disc_port = maybe_disc_port.unwrap_or(tcp_port); // udp / tcp can listen to same port + + // Handle QUIC port with overflow safety + let quic_port = maybe_quic_port.unwrap_or_else(|| { + if tcp_port == 0 || tcp_port == u16::MAX { + // If tcp_port is 0 or MAX, set quic_port to 0 + 0 + } else { + tcp_port.wrapping_add(1) + } + }); + + (tcp_port, disc_port, quic_port) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_compute_listen_ports_with_zero_ports_flag() { + // When use_zero_ports is true, all ports should be 0 regardless of input + assert_eq!(compute_listen_ports(true, 9000, None, None), (0, 0, 0)); + } + + #[test] + fn test_compute_listen_ports_default_behavior() { + // Default behavior: disc_port = tcp_port, quic_port = tcp_port + 1 + let (tcp, disc, quic) = compute_listen_ports(false, 9000, None, None); + assert_eq!(tcp, 9000); + assert_eq!(disc, 9000); // Discovery defaults to TCP port (UDP and TCP can listen to same port) + assert_eq!(quic, 9001); // QUIC defaults to TCP port + 1 + } + + #[test] + fn test_compute_listen_ports_with_explicit_ports() { + // Explicit discovery port should be used + let (tcp, disc, quic) = compute_listen_ports(false, 9000, Some(8000), Some(7000)); + assert_eq!(tcp, 9000); + assert_eq!(disc, 8000); // Explicit discovery port + assert_eq!(quic, 7000); // Explicit QUIC port + } + + #[test] + fn test_compute_listen_ports_with_zero_tcp_port() { + // Edge case: tcp_port = 0 without use_zero_ports flag + // QUIC and discovery ports should also be 0 + assert_eq!(compute_listen_ports(false, 0, None, None), (0, 0, 0)); + } + + #[test] + fn test_compute_listen_ports_max_port_overflow() { + // Edge case: tcp_port = u16::MAX (65535) + // QUIC should be 0 to avoid overflow panic + let (tcp, disc, quic) = compute_listen_ports(false, u16::MAX, None, None); + assert_eq!(tcp, u16::MAX); + assert_eq!(disc, u16::MAX); + assert_eq!(quic, 0); // u16::MAX would overflow, so we use 0 + } +} diff --git a/common/network_utils/src/unused_port.rs b/common/network_utils/src/unused_port.rs index 212ae963e3c..7226328ae31 100644 --- a/common/network_utils/src/unused_port.rs +++ b/common/network_utils/src/unused_port.rs @@ -26,21 +26,6 @@ pub fn unused_tcp4_port() -> Result { zero_port(Transport::Tcp, IpVersion::Ipv4) } -/// A convenience wrapper over [`zero_port`]. -pub fn unused_udp4_port() -> Result { - zero_port(Transport::Udp, IpVersion::Ipv4) -} - -/// A convenience wrapper over [`zero_port`]. -pub fn unused_tcp6_port() -> Result { - zero_port(Transport::Tcp, IpVersion::Ipv6) -} - -/// A convenience wrapper over [`zero_port`]. -pub fn unused_udp6_port() -> Result { - zero_port(Transport::Udp, IpVersion::Ipv6) -} - /// A bit of hack to find an unused port. /// /// Does not guarantee that the given port is unused after the function exits, just that it was @@ -97,3 +82,31 @@ fn find_unused_port(transport: Transport, socket_addr: SocketAddr) -> Result Result { + let addr = std::net::SocketAddr::new(std::net::Ipv4Addr::LOCALHOST.into(), 0); + TcpListener::bind(addr).map_err(|e| format!("Failed to bind TCPv4 listener: {:?}", e)) +} + +/// Bind a TCPv6 listener on localhost with an ephemeral port (port 0) and return it. +/// Safe against TOCTOU: the socket remains open and reserved by the OS. +pub fn bind_tcp6_any() -> Result { + let addr = std::net::SocketAddr::new(std::net::Ipv6Addr::LOCALHOST.into(), 0); + TcpListener::bind(addr).map_err(|e| format!("Failed to bind TCPv6 listener: {:?}", e)) +} + +/// Bind a UDPv4 socket on localhost with an ephemeral port (port 0) and return it. +/// Safe against TOCTOU: the socket remains open and reserved by the OS. +pub fn bind_udp4_any() -> Result { + let addr = std::net::SocketAddr::new(std::net::Ipv4Addr::LOCALHOST.into(), 0); + UdpSocket::bind(addr).map_err(|e| format!("Failed to bind UDPv4 socket: {:?}", e)) +} + +/// Bind a UDPv6 socket on localhost with an ephemeral port (port 0) and return it. +/// Safe against TOCTOU: the socket remains open and reserved by the OS. +pub fn bind_udp6_any() -> Result { + let addr = std::net::SocketAddr::new(std::net::Ipv6Addr::LOCALHOST.into(), 0); + UdpSocket::bind(addr).map_err(|e| format!("Failed to bind UDPv6 socket: {:?}", e)) +} diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index 207324ea33f..bb556f6fe7a 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -11,9 +11,6 @@ use beacon_node::{ }; use beacon_processor::BeaconProcessorConfig; use lighthouse_network::PeerId; -use network_utils::unused_port::{ - unused_tcp4_port, unused_tcp6_port, unused_udp4_port, unused_udp6_port, -}; use std::fs::File; use std::io::{Read, Write}; use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; @@ -36,6 +33,12 @@ const DUMMY_ENR_TCP_PORT: u16 = 7777; const DUMMY_ENR_UDP_PORT: u16 = 8888; const DUMMY_ENR_QUIC_PORT: u16 = 9999; +// Fixed test ports for config-only assertions (no actual bind occurs in these tests). +const TEST_TCP4_PORT: u16 = 39001; +const TEST_TCP6_PORT: u16 = 39011; +const TEST_UDP4_PORT: u16 = 39002; +const TEST_UDP6_PORT: u16 = 39012; + const _: () = assert!(DUMMY_ENR_QUIC_PORT != 0 && DUMMY_ENR_TCP_PORT != 0 && DUMMY_ENR_UDP_PORT != 0); @@ -967,7 +970,7 @@ fn network_port_flag_over_ipv4() { ); }); - let port = unused_tcp4_port().expect("Unable to find unused port."); + let port = TEST_TCP4_PORT; CommandLineTest::new() .flag("port", Some(port.to_string().as_str())) .flag("allow-insecure-genesis-sync", None) @@ -1004,7 +1007,7 @@ fn network_port_flag_over_ipv6() { ); }); - let port = unused_tcp4_port().expect("Unable to find unused port."); + let port = TEST_TCP4_PORT; CommandLineTest::new() .flag("listen-address", Some("::1")) .flag("port", Some(port.to_string().as_str())) @@ -1054,8 +1057,8 @@ fn network_port_flag_over_ipv4_and_ipv6() { ); }); - let port = unused_tcp4_port().expect("Unable to find unused port."); - let port6 = unused_tcp6_port().expect("Unable to find unused port."); + let port = TEST_TCP4_PORT; + let port6 = TEST_TCP6_PORT; CommandLineTest::new() .flag("listen-address", Some("127.0.0.1")) .flag("listen-address", Some("::1")) @@ -1305,11 +1308,41 @@ fn private_flag() { } #[test] fn zero_ports_flag() { + // Test that --zero-ports sets all ports to 0 CommandLineTest::new() .run_with_zero_port() .with_config(|config| { + // HTTP and metrics ports should be 0 assert_eq!(config.http_api.listen_port, 0); assert_eq!(config.http_metrics.listen_port, 0); + + // Network ports should all be 0 + assert_eq!( + config.network.listen_addrs().v4().map(|listen_addr| ( + listen_addr.tcp_port, + listen_addr.disc_port, + listen_addr.quic_port + )), + Some((0, 0, 0)) + ); + }); + + // Test that --zero-ports overrides explicit non-zero port + CommandLineTest::new() + .flag("port", Some("9000")) + .flag("zero-ports", None) + .flag("allow-insecure-genesis-sync", None) + .run() + .with_config(|config| { + // Even though port=9000 was specified, --zero-ports should override + assert_eq!( + config.network.listen_addrs().v4().map(|listen_addr| ( + listen_addr.tcp_port, + listen_addr.disc_port, + listen_addr.quic_port + )), + Some((0, 0, 0)) + ); }); } #[test] @@ -1406,8 +1439,8 @@ fn enr_tcp6_port_flag() { fn enr_match_flag_over_ipv4() { let addr = "127.0.0.2".parse::().unwrap(); - let udp4_port = unused_udp4_port().expect("Unable to find unused port."); - let tcp4_port = unused_tcp4_port().expect("Unable to find unused port."); + let udp4_port = TEST_UDP4_PORT; + let tcp4_port = TEST_TCP4_PORT; CommandLineTest::new() .flag("enr-match", None) @@ -1437,8 +1470,8 @@ fn enr_match_flag_over_ipv6() { const ADDR: &str = "::1"; let addr = ADDR.parse::().unwrap(); - let udp6_port = unused_udp6_port().expect("Unable to find unused port."); - let tcp6_port = unused_tcp6_port().expect("Unable to find unused port."); + let udp6_port = TEST_UDP6_PORT; + let tcp6_port = TEST_TCP6_PORT; CommandLineTest::new() .flag("enr-match", None) @@ -1467,13 +1500,13 @@ fn enr_match_flag_over_ipv6() { fn enr_match_flag_over_ipv4_and_ipv6() { const IPV6_ADDR: &str = "::1"; - let udp6_port = unused_udp6_port().expect("Unable to find unused port."); - let tcp6_port = unused_tcp6_port().expect("Unable to find unused port."); + let udp6_port = TEST_UDP6_PORT; + let tcp6_port = TEST_TCP6_PORT; let ipv6_addr = IPV6_ADDR.parse::().unwrap(); const IPV4_ADDR: &str = "127.0.0.1"; - let udp4_port = unused_udp4_port().expect("Unable to find unused port."); - let tcp4_port = unused_tcp4_port().expect("Unable to find unused port."); + let udp4_port = TEST_UDP4_PORT; + let tcp4_port = TEST_TCP4_PORT; let ipv4_addr = IPV4_ADDR.parse::().unwrap(); CommandLineTest::new() diff --git a/lighthouse/tests/boot_node.rs b/lighthouse/tests/boot_node.rs index 38111ca0efd..2cec36575f2 100644 --- a/lighthouse/tests/boot_node.rs +++ b/lighthouse/tests/boot_node.rs @@ -4,7 +4,6 @@ use crate::exec::{CommandLineTestExec, CompletedTest}; use clap::ArgMatches; use clap_utils::get_eth2_network_config; use lighthouse_network::{Enr, discovery::ENR_FILENAME}; -use network_utils::unused_port::unused_udp4_port; use std::fs::File; use std::io::Write; use std::net::Ipv4Addr; @@ -15,6 +14,9 @@ use tempfile::TempDir; const IP_ADDRESS: &str = "192.168.2.108"; +// Fixed test port for config-only assertions (no actual bind occurs in these tests). +const TEST_UDP4_PORT: u16 = 39102; + /// Returns the `lighthouse boot_node` command. fn base_cmd() -> Command { let lighthouse_bin = env!("CARGO_BIN_EXE_lighthouse"); @@ -61,7 +63,7 @@ fn enr_address_arg() { #[test] fn port_flag() { - let port = unused_udp4_port().unwrap(); + let port = TEST_UDP4_PORT; CommandLineTest::new() .flag("port", Some(port.to_string().as_str())) .run_with_ip() @@ -133,7 +135,7 @@ fn boot_nodes_flag() { #[test] fn enr_port_flag() { - let port = unused_udp4_port().unwrap(); + let port = TEST_UDP4_PORT; CommandLineTest::new() .flag("enr-port", Some(port.to_string().as_str())) .run_with_ip()