diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e217e8b59a9..cf28b3da872 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -42,7 +42,13 @@ jobs: save-if: false - name: Run all tests - run: cargo test --package "$CRATE" --all-features + run: | + if [ "$CRATE" = "libp2p-upnp" ]; then + # Run integration tests serially to avoid multiple IGD mock servers running in parallel. + cargo test --package "$CRATE" --all-features -- --test-threads 1 + else + cargo test --package "$CRATE" --all-features + fi - name: Check if we compile without any features activated run: cargo build --package "$CRATE" --no-default-features diff --git a/Cargo.lock b/Cargo.lock index 5c4fe86f441..f55c895fec5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -310,11 +310,12 @@ checksum = "1505bd5d3d116872e7271a6d4e16d81d0c8570876c8de68093a09ac269d8aac0" [[package]] name = "attohttpc" -version = "0.24.1" +version = "0.30.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8d9a9bf8b79a749ee0b911b91b671cc2b6c670bdbc7e3dfd537576ddc94bb2a2" +checksum = "16e2cdb6d5ed835199484bb92bb8b3edd526effe995c61732580439c1a67e2e9" dependencies = [ - "http 0.2.12", + "base64", + "http", "log", "url", ] @@ -362,7 +363,7 @@ dependencies = [ "axum-core", "bytes", "futures-util", - "http 1.3.1", + "http", "http-body", "http-body-util", "hyper", @@ -395,7 +396,7 @@ dependencies = [ "async-trait", "bytes", "futures-util", - "http 1.3.1", + "http", "http-body", "http-body-util", "mime", @@ -1630,7 +1631,7 @@ dependencies = [ "fnv", "futures-core", "futures-sink", - "http 1.3.1", + "http", "indexmap 2.9.0", "slab", "tokio", @@ -1812,17 +1813,6 @@ dependencies = [ "windows-link", ] -[[package]] -name = "http" -version = "0.2.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "601cbb57e577e2f5ef5be8e7b83f0f63994f25aa94d673e54a92d5c516d101f1" -dependencies = [ - "bytes", - "fnv", - "itoa", -] - [[package]] name = "http" version = "1.3.1" @@ -1841,7 +1831,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1efedce1fb8e6913f23e0c92de8e62cd5b772a67e7b3946df930a62566c93184" dependencies = [ "bytes", - "http 1.3.1", + "http", ] [[package]] @@ -1852,7 +1842,7 @@ checksum = "b021d93e26becf5dc7e1b75b1bed1fd93124b374ceb73f43d4d4eafec896a64a" dependencies = [ "bytes", "futures-core", - "http 1.3.1", + "http", "http-body", "pin-project-lite", ] @@ -1886,7 +1876,7 @@ dependencies = [ "futures-channel", "futures-core", "h2", - "http 1.3.1", + "http", "http-body", "httparse", "httpdate", @@ -1905,7 +1895,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2d191583f3da1305256f22463b9bb0471acad48a4e534a5218b9963e9c1f59b2" dependencies = [ "futures-util", - "http 1.3.1", + "http", "hyper", "hyper-util", "rustls", @@ -1956,7 +1946,7 @@ dependencies = [ "futures-channel", "futures-core", "futures-util", - "http 1.3.1", + "http", "http-body", "hyper", "ipnet", @@ -2155,15 +2145,15 @@ dependencies = [ [[package]] name = "igd-next" -version = "0.16.1" +version = "0.16.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d06464e726471718db9ad3fefc020529fabcde03313a0fc3967510e2db5add12" +checksum = "516893339c97f6011282d5825ac94fc1c7aad5cad26bdc2d0cee068c0bf97f97" dependencies = [ "async-trait", "attohttpc", "bytes", "futures", - "http 1.3.1", + "http", "http-body-util", "hyper", "hyper-util", @@ -3196,13 +3186,15 @@ dependencies = [ [[package]] name = "libp2p-upnp" -version = "0.6.0" +version = "0.6.1" dependencies = [ "futures", "futures-timer", "igd-next", "libp2p-core", "libp2p-swarm", + "libp2p-swarm-test", + "mock-igd", "tokio", "tracing", ] @@ -3526,6 +3518,19 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "mock-igd" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e48bc9bd248c63b9efa3902b4e88027e2dffc86bef5fc92e0b0c17d2cb1b941a" +dependencies = [ + "axum", + "socket2 0.5.9", + "thiserror 1.0.69", + "tokio", + "tracing", +] + [[package]] name = "moka" version = "0.12.10" @@ -3926,7 +3931,7 @@ checksum = "91cf61a1868dacc576bf2b2a1c3e9ab150af7272909e80085c3173384fe11f76" dependencies = [ "async-trait", "futures-core", - "http 1.3.1", + "http", "opentelemetry 0.27.1", "opentelemetry-proto", "opentelemetry_sdk 0.27.1", @@ -4675,7 +4680,7 @@ dependencies = [ "encoding_rs", "futures-core", "h2", - "http 1.3.1", + "http", "http-body", "http-body-util", "hyper", @@ -5515,7 +5520,7 @@ dependencies = [ "async-trait", "base64", "futures", - "http 1.3.1", + "http", "indexmap 2.9.0", "parking_lot", "paste", @@ -5801,7 +5806,7 @@ dependencies = [ "base64", "bytes", "h2", - "http 1.3.1", + "http", "http-body", "http-body-util", "hyper", @@ -5864,7 +5869,7 @@ dependencies = [ "bitflags 2.9.0", "bytes", "futures-util", - "http 1.3.1", + "http", "http-body", "http-body-util", "http-range-header", @@ -5889,7 +5894,7 @@ dependencies = [ "bitflags 2.9.0", "bytes", "futures-util", - "http 1.3.1", + "http", "http-body", "iri-string", "pin-project-lite", diff --git a/Cargo.toml b/Cargo.toml index 104025d12f1..37e3367d206 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -108,7 +108,7 @@ libp2p-swarm-test = { version = "0.6.0", path = "swarm-test" } libp2p-tcp = { version = "0.44.1", path = "transports/tcp" } libp2p-tls = { version = "0.6.2", path = "transports/tls" } libp2p-uds = { version = "0.43.1", path = "transports/uds" } -libp2p-upnp = { version = "0.6.0", path = "protocols/upnp" } +libp2p-upnp = { version = "0.6.1", path = "protocols/upnp" } libp2p-webrtc = { version = "0.9.0-alpha.2", path = "transports/webrtc" } libp2p-webrtc-utils = { version = "0.4.0", path = "misc/webrtc-utils" } libp2p-webrtc-websys = { version = "0.4.0", path = "transports/webrtc-websys" } diff --git a/protocols/upnp/CHANGELOG.md b/protocols/upnp/CHANGELOG.md index ed693bcf93f..5ed988e9518 100644 --- a/protocols/upnp/CHANGELOG.md +++ b/protocols/upnp/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.6.1 + +- Add integration tests. See [PR 6264](https://github.com/libp2p/rust-libp2p/pull/6264) + ## 0.6.0 - Change `Event::NewExternalAddr` and `Event::ExpiredExternalAddr` from tuple variants to struct variants @@ -31,7 +35,7 @@ ## 0.4.0 - update igd-next to 0.15.1. - See [PR XXXX](https://github.com/libp2p/rust-libp2p/pull/XXXX). + See [PR 5625](https://github.com/libp2p/rust-libp2p/pull/5625). diff --git a/protocols/upnp/Cargo.toml b/protocols/upnp/Cargo.toml index fb120cfff71..c1205693736 100644 --- a/protocols/upnp/Cargo.toml +++ b/protocols/upnp/Cargo.toml @@ -3,7 +3,7 @@ name = "libp2p-upnp" edition.workspace = true rust-version.workspace = true description = "UPnP support for libp2p transports" -version = "0.6.0" +version = "0.6.1" license = "MIT" repository = "https://github.com/libp2p/rust-libp2p" keywords = ["peer-to-peer", "libp2p", "networking"] @@ -19,6 +19,11 @@ libp2p-swarm = { workspace = true } tokio = { workspace = true, default-features = false, features = ["rt"], optional = true } tracing = { workspace = true } +[dev-dependencies] +libp2p-swarm-test = { path = "../../swarm-test" } +tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } +mock-igd = "0.1" + [features] tokio = ["igd-next/aio_tokio", "dep:tokio"] diff --git a/protocols/upnp/src/behaviour.rs b/protocols/upnp/src/behaviour.rs index 157cd01e06c..e80f770c326 100644 --- a/protocols/upnp/src/behaviour.rs +++ b/protocols/upnp/src/behaviour.rs @@ -273,14 +273,32 @@ pub struct Behaviour { /// Pending behaviour events to be emitted. pending_events: VecDeque, + + /// Relaxes certain checks to enable testing in environments without real UPnP infrastructure. + test_mode: bool, } impl Default for Behaviour { fn default() -> Self { Self { - state: GatewayState::Searching(crate::tokio::search_gateway()), + state: GatewayState::Searching(crate::tokio::search_gateway(None)), mappings: Default::default(), pending_events: VecDeque::new(), + test_mode: false, + } + } +} + +impl Behaviour { + /// Creates a new `Behaviour` with `test_mode` enabled. + pub fn new_with_test_mode() -> Self { + Self { + test_mode: true, + // Shorten the timeout to speed up `GatewayNotFound` tests. + state: GatewayState::Searching(crate::tokio::search_gateway(Some( + Duration::from_secs(1), + ))), + ..Default::default() } } } @@ -317,7 +335,8 @@ impl NetworkBehaviour for Behaviour { listener_id, addr: multiaddr, }) => { - let Ok((addr, protocol)) = multiaddr_to_socketaddr_protocol(multiaddr.clone()) + let Ok((addr, protocol)) = + multiaddr_to_socketaddr_protocol(multiaddr.clone(), self.test_mode) else { tracing::debug!("multiaddress not supported for UPnP {multiaddr}"); return; @@ -440,7 +459,7 @@ impl NetworkBehaviour for Behaviour { GatewayState::Searching(ref mut fut) => match Pin::new(fut).poll(cx) { Poll::Ready(Ok(result)) => match result { Ok(gateway) => { - if !is_addr_global(gateway.external_addr) { + if !self.test_mode && !is_addr_global(gateway.external_addr) { self.state = GatewayState::NonRoutableGateway(gateway.external_addr); tracing::debug!( @@ -614,27 +633,33 @@ impl NetworkBehaviour for Behaviour { /// /// Fails if the given [`Multiaddr`] does not begin with an IP /// protocol encapsulating a TCP or UDP port. +/// +/// When `test_mode` is true, the private IP check is skipped to allow testing with +/// non-private addresses like 127.0.0.1. fn multiaddr_to_socketaddr_protocol( addr: Multiaddr, + test_mode: bool, ) -> Result<(SocketAddr, PortMappingProtocol), ()> { let mut iter = addr.into_iter(); match iter.next() { // Idg only supports Ipv4. - Some(multiaddr::Protocol::Ip4(ipv4)) if ipv4.is_private() => match iter.next() { - Some(multiaddr::Protocol::Tcp(port)) => { - return Ok(( - SocketAddr::V4(SocketAddrV4::new(ipv4, port)), - PortMappingProtocol::TCP, - )); - } - Some(multiaddr::Protocol::Udp(port)) => { - return Ok(( - SocketAddr::V4(SocketAddrV4::new(ipv4, port)), - PortMappingProtocol::UDP, - )); + Some(multiaddr::Protocol::Ip4(ipv4)) if test_mode || ipv4.is_private() => { + match iter.next() { + Some(multiaddr::Protocol::Tcp(port)) => { + return Ok(( + SocketAddr::V4(SocketAddrV4::new(ipv4, port)), + PortMappingProtocol::TCP, + )); + } + Some(multiaddr::Protocol::Udp(port)) => { + return Ok(( + SocketAddr::V4(SocketAddrV4::new(ipv4, port)), + PortMappingProtocol::UDP, + )); + } + _ => {} } - _ => {} - }, + } _ => {} } Err(()) diff --git a/protocols/upnp/src/tokio.rs b/protocols/upnp/src/tokio.rs index 67ef52f9608..cfe21f4e27c 100644 --- a/protocols/upnp/src/tokio.rs +++ b/protocols/upnp/src/tokio.rs @@ -18,7 +18,7 @@ // FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER // DEALINGS IN THE SOFTWARE. -use std::{error::Error, net::IpAddr}; +use std::{error::Error, net::IpAddr, time::Duration}; use futures::{ channel::{mpsc, oneshot}, @@ -90,14 +90,30 @@ pub(crate) struct Gateway { pub(crate) external_addr: IpAddr, } -pub(crate) fn search_gateway() -> oneshot::Receiver>> { +/// Searches for a UPnP gateway on the network and returns a channel to receive the result. +/// +/// # Arguments +/// +/// * `timeout` - Optional timeout duration for the gateway search. If `None`, the default timeout +/// from [`igd_next::SearchOptions`] is used. +pub(crate) fn search_gateway( + timeout: Option, +) -> oneshot::Receiver>> { let (search_result_sender, search_result_receiver) = oneshot::channel(); let (events_sender, mut task_receiver) = mpsc::channel(10); let (mut task_sender, events_queue) = mpsc::channel(0); tokio::spawn(async move { - let gateway = match igd_next::aio::tokio::search_gateway(SearchOptions::default()).await { + let search_options = if timeout.is_some() { + SearchOptions { + timeout, + ..Default::default() + } + } else { + SearchOptions::default() + }; + let gateway = match igd_next::aio::tokio::search_gateway(search_options).await { Ok(gateway) => gateway, Err(err) => { let _ = search_result_sender.send(Err(err.into())); diff --git a/protocols/upnp/tests/upnp.rs b/protocols/upnp/tests/upnp.rs new file mode 100644 index 00000000000..15b1dae05d4 --- /dev/null +++ b/protocols/upnp/tests/upnp.rs @@ -0,0 +1,237 @@ +// Copyright 2023 Protocol Labs. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the "Software"), +// to deal in the Software without restriction, including without limitation +// the rights to use, copy, modify, merge, publish, distribute, sublicense, +// and/or sell copies of the Software, and to permit persons to whom the +// Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +//! Integration tests for the `UPnP` network behaviour. +//! +//! NOTE: If an IGD device exists in the test environment, results may be unintended. + +use std::time::Duration; + +use libp2p_swarm::{Swarm, SwarmEvent}; +use libp2p_swarm_test::SwarmExt; +use libp2p_upnp as upnp; +use mock_igd::{Action, MockIgdServer, Protocol, Responder}; + +/// Mock external IP address used for testing. +/// This address is from the TEST-NET-3 range (203.0.113.0/24) reserved for testing (RFC 5737). +const MOCK_EXTERNAL_IP: &str = "203.0.113.42"; + +/// Test that port mapping succeeds when the gateway responds successfully. +#[tokio::test] +async fn port_mapping_success() { + // Start the mock IGD server with SSDP enabled + let igd_server = MockIgdServer::builder() + .with_ssdp() + .start() + .await + .expect("Failed to start mock IGD server"); + + // Register mock behaviors + igd_server + .mock( + Action::GetExternalIPAddress, + Responder::success().with_external_ip(MOCK_EXTERNAL_IP.parse().unwrap()), + ) + .await; + + igd_server + .mock( + Action::add_port_mapping().with_protocol(Protocol::TCP), + Responder::success(), + ) + .await; + + // Use test_mode to bypass IP address validations for testing. + let mut swarm = Swarm::new_ephemeral_tokio(|_| upnp::tokio::Behaviour::new_with_test_mode()); + + swarm.listen().await; + + // Wait for UPnP events + tokio::time::timeout(Duration::from_secs(5), async { + loop { + if let SwarmEvent::Behaviour(upnp_event) = swarm.next_swarm_event().await { + match upnp_event { + upnp::Event::NewExternalAddr { external_addr, .. } => { + assert!( + external_addr.to_string().contains(MOCK_EXTERNAL_IP), + "External address should contain the mock external IP" + ); + break; + } + event => panic!("Unexpected event: {:?}", event), + } + } + } + }) + .await + .unwrap(); + + // Verify that the IGD server received the expected requests + let requests = igd_server.received_requests().await; + assert!( + requests + .iter() + .any(|r| r.action_name == "GetExternalIPAddress"), + "Should have received GetExternalIPAddress request" + ); + assert!( + requests.iter().any(|r| r.action_name == "AddPortMapping"), + "Should have received AddPortMapping request" + ); + + igd_server.shutdown(); +} + +/// Test that port mapping failure is handled gracefully. +#[tokio::test] +async fn port_mapping_failure() { + // Start the mock IGD server with SSDP enabled + let igd_server = MockIgdServer::builder() + .with_ssdp() + .start() + .await + .expect("Failed to start mock IGD server"); + + // Register mock behaviors + igd_server + .mock( + Action::GetExternalIPAddress, + Responder::success().with_external_ip(MOCK_EXTERNAL_IP.parse().unwrap()), + ) + .await; + + // Error 718 = ConflictInMappingEntry + igd_server + .mock( + Action::add_port_mapping().with_protocol(Protocol::TCP), + Responder::error(718, "ConflictInMappingEntry"), + ) + .await; + + // Use test_mode to bypass IP address validations for testing. + let mut swarm = Swarm::new_ephemeral_tokio(|_| upnp::tokio::Behaviour::new_with_test_mode()); + + swarm.listen().await; + + let result = tokio::time::timeout(Duration::from_secs(5), async { + loop { + if let SwarmEvent::Behaviour(upnp_event) = swarm.next_swarm_event().await { + // We expect no UPnP event due to the port mapping error + panic!("Unexpected UPnP event: {upnp_event:?}"); + } + } + }) + .await; + + // Timeout is expected since mapping fails and no success event is emitted + assert!( + result.is_err(), + "Expected timeout since port mapping should fail" + ); + + // Verify that the IGD server received the AddPortMapping request + let requests = igd_server.received_requests().await; + assert!( + requests.iter().any(|r| r.action_name == "AddPortMapping"), + "Should have received AddPortMapping request" + ); + + igd_server.shutdown(); +} + +/// Test that the behaviour emits NonRoutableGateway when the gateway returns a private IP address. +#[tokio::test] +async fn non_routable_gateway() { + // Start the mock IGD server with SSDP enabled + let igd_server = MockIgdServer::builder() + .with_ssdp() + .start() + .await + .expect("Failed to start mock IGD server"); + + // Register mock behaviors + igd_server + .mock( + Action::GetExternalIPAddress, + // Use a private IP address (192.168.0.1) to simulate a non-routable gateway. + // This should trigger the NonRoutableGateway event. + Responder::success().with_external_ip("192.168.0.1".parse().unwrap()), + ) + .await; + + // Don't use test_mode so that the private IP address check remains active. + let mut swarm = Swarm::new_ephemeral_tokio(|_| upnp::tokio::Behaviour::default()); + + swarm.listen().await; + + // Wait for UPnP events + tokio::time::timeout(Duration::from_secs(5), async { + loop { + if let SwarmEvent::Behaviour(upnp_event) = swarm.next_swarm_event().await { + match upnp_event { + // The gateway returned a private IP, so it should be deemed non-routable. + upnp::Event::NonRoutableGateway => { + break; + } + event => panic!("Unexpected event: {:?}", event), + } + } + } + }) + .await + .unwrap(); + + // Verify that the IGD server received the expected requests + let requests = igd_server.received_requests().await; + assert!( + requests + .iter() + .any(|r| r.action_name == "GetExternalIPAddress"), + "Should have received GetExternalIPAddress request" + ); + + igd_server.shutdown(); +} + +/// Test that the behaviour emits GatewayNotFound when no gateway is available. +#[tokio::test] +async fn gateway_not_found() { + // No mock server - SSDP discovery should fail + let mut swarm = Swarm::new_ephemeral_tokio(|_| upnp::tokio::Behaviour::new_with_test_mode()); + + swarm.listen().await; + + // Wait for GatewayNotFound event + tokio::time::timeout(Duration::from_secs(5), async { + loop { + if let SwarmEvent::Behaviour(upnp_event) = swarm.next_swarm_event().await { + match upnp_event { + upnp::Event::GatewayNotFound => { + // Expected event received - test passes + break; + } + event => panic!("Unexpected UPnP event: {:?}", event), + } + } + } + }) + .await + .unwrap(); +}