From 8fe58b2dd4cd939bd2da61b50c2f485e2f2f21d2 Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Wed, 8 Jan 2025 13:56:40 +0100 Subject: [PATCH] Fix bridge ip logic --- test/test-manager/src/main.rs | 17 +++++---- test/test-manager/src/tests/access_methods.rs | 6 ++-- .../src/tests/audits/cve_2019_14899.rs | 6 ++-- .../src/tests/audits/mllvd_cr_24_03.rs | 35 +++++++++---------- test/test-manager/src/tests/config.rs | 8 ++--- test/test-manager/src/tests/dns.rs | 20 +++++++---- .../src/tests/relay_ip_overrides.rs | 10 +++--- test/test-manager/src/tests/tunnel.rs | 8 ++--- test/test-manager/src/vm/network/linux.rs | 5 +-- test/test-manager/src/vm/network/macos.rs | 20 +++++++---- test/test-manager/src/vm/network/mod.rs | 13 +++---- 11 files changed, 82 insertions(+), 66 deletions(-) diff --git a/test/test-manager/src/main.rs b/test/test-manager/src/main.rs index b72ae6a495cc..af96337d167d 100644 --- a/test/test-manager/src/main.rs +++ b/test/test-manager/src/main.rs @@ -295,7 +295,15 @@ async fn main() -> Result<()> { .await .context("Failed to run provisioning for VM")?; - let bridge_ip = instance.get_ip().to_owned(); + #[cfg(target_os = "macos")] + let IpAddr::V4(guest_ip) = instance.get_ip().to_owned() else { + panic!("Expected bridge IP to be version 4, but was version 6.") + }; + let (bridge_name, bridge_ip) = vm::network::bridge( + #[cfg(target_os = "macos")] + &guest_ip, + )?; + TEST_CONFIG.init(tests::config::TestConfig::new( account, artifacts_dir, @@ -312,10 +320,7 @@ async fn main() -> Result<()> { .gui_package_path .map(|path| path.file_name().unwrap().to_string_lossy().into_owned()), mullvad_host, - vm::network::bridge( - #[cfg(target_os = "macos")] - &bridge_ip, - )?, + bridge_name, bridge_ip, test_rpc::meta::Os::from(vm_config.os_type), openvpn_certificate, @@ -324,7 +329,7 @@ async fn main() -> Result<()> { // For convenience, spawn a SOCKS5 server that is reachable for tests that need it let socks = socks_server::spawn(SocketAddr::new( - crate::vm::network::NON_TUN_GATEWAY.into(), + TEST_CONFIG.host_bridge_ip.into(), crate::vm::network::SOCKS5_PORT, )) .await?; diff --git a/test/test-manager/src/tests/access_methods.rs b/test/test-manager/src/tests/access_methods.rs index 747ebd23e590..bead7767f27f 100644 --- a/test/test-manager/src/tests/access_methods.rs +++ b/test/test-manager/src/tests/access_methods.rs @@ -14,6 +14,8 @@ use talpid_types::net::proxy::CustomProxy; use test_macro::test_function; use test_rpc::ServiceClient; +use crate::tests::config::TEST_CONFIG; + use super::TestContext; /// Assert that API traffic can be proxied via a custom Shadowsocks proxy. @@ -48,7 +50,7 @@ async fn test_access_method_socks5_remote( _rpc: ServiceClient, mullvad_client: MullvadProxyClient, ) -> anyhow::Result<()> { - use crate::vm::network::{NON_TUN_GATEWAY, SOCKS5_PORT}; + use crate::vm::network::SOCKS5_PORT; use std::net::SocketAddr; use talpid_types::net::proxy::Socks5Remote; log::info!("Testing SOCKS5 (Remote) access method"); @@ -57,7 +59,7 @@ async fn test_access_method_socks5_remote( // The remote SOCKS5 proxy is assumed to be running on the test manager. On // which port it listens to is defined as a constant in the `test-manager` // crate. - let endpoint = SocketAddr::from((NON_TUN_GATEWAY, SOCKS5_PORT)); + let endpoint = SocketAddr::from((TEST_CONFIG.host_bridge_ip, SOCKS5_PORT)); let access_method = Socks5Remote::new(endpoint); log::info!("Testing SOCKS5-proxy: {access_method:?}"); assert_access_method_works(mullvad_client, access_method.clone()) diff --git a/test/test-manager/src/tests/audits/cve_2019_14899.rs b/test/test-manager/src/tests/audits/cve_2019_14899.rs index 71872c071686..d723b84dd11b 100644 --- a/test/test-manager/src/tests/audits/cve_2019_14899.rs +++ b/test/test-manager/src/tests/audits/cve_2019_14899.rs @@ -42,8 +42,8 @@ use test_rpc::ServiceClient; use tokio::{task::yield_now, time::sleep}; use crate::{ - tests::{helpers, TestContext}, - vm::network::{linux::TAP_NAME, NON_TUN_GATEWAY}, + tests::{config::TEST_CONFIG, helpers, TestContext}, + vm::network::linux::TAP_NAME, }; /// The port number we set in the malicious packet. @@ -67,7 +67,7 @@ pub async fn test_cve_2019_14899_mitigation( let victim_tunnel_interface = helpers::get_tunnel_interface(&mut mullvad_client) .await .context("Failed to find tunnel interface")?; - let victim_gateway_ip = NON_TUN_GATEWAY; + let victim_gateway_ip = TEST_CONFIG.host_bridge_ip; // Create a raw socket which let's us send custom ethernet packets log::info!("Creating raw socket"); diff --git a/test/test-manager/src/tests/audits/mllvd_cr_24_03.rs b/test/test-manager/src/tests/audits/mllvd_cr_24_03.rs index bf743bcc676d..d82945d809c9 100644 --- a/test/test-manager/src/tests/audits/mllvd_cr_24_03.rs +++ b/test/test-manager/src/tests/audits/mllvd_cr_24_03.rs @@ -1,9 +1,10 @@ #![cfg(target_os = "linux")] //! Test mitigation for mllvd_cr_24_03 //! -//! By sending an ARP request for the in-tunnel IP address to any network interface on the device running Mullvad, it -//! will respond and confirm that it owns this address. This means someone on the LAN or similar can figure out the -//! device's in-tunnel IP, and potentially also make an educated guess that they are using Mullvad at all. +//! By sending an ARP request for the in-tunnel IP address to any network interface on the device +//! running Mullvad, it will respond and confirm that it owns this address. This means someone on +//! the LAN or similar can figure out the device's in-tunnel IP, and potentially also make an +//! educated guess that they are using Mullvad at all. //! //! # Setup //! @@ -12,26 +13,23 @@ //! Network adjacent attacker: test-manager //! //! # Procedure -//! Have test-runner connect to relay. Let test-manager know about the test-runner's private in-tunnel IP (such that -//! we don't have to enumerate all possible private IPs). +//! Have test-runner connect to relay. Let test-manager know about the test-runner's private +//! in-tunnel IP (such that we don't have to enumerate all possible private IPs). //! -//! Have test-manager invoke the `arping` command targeting the bridge network between test-manager <-> test-runner. -//! If `arping` times out without a reply, it will exit with a non-0 exit code. If it got a reply from test-runner, it -//! will exit with code 0. +//! Have test-manager invoke the `arping` command targeting the bridge network between test-manager +//! <-> test-runner. If `arping` times out without a reply, it will exit with a non-0 exit code. If +//! it got a reply from test-runner, it will exit with code 0. //! //! Note that only linux was susceptible to this vulnerability. -use std::ffi::OsStr; -use std::process::Output; +use std::{ffi::OsStr, process::Output}; use anyhow::bail; use mullvad_management_interface::MullvadProxyClient; use test_macro::test_function; use test_rpc::ServiceClient; -use crate::tests::helpers::*; -use crate::tests::TestContext; -use crate::vm::network::bridge; +use crate::tests::{config::TEST_CONFIG, helpers::*, TestContext}; #[test_function(target_os = "linux")] pub async fn test_mllvd_cr_24_03( @@ -40,8 +38,9 @@ pub async fn test_mllvd_cr_24_03( mut mullvad_client: MullvadProxyClient, ) -> anyhow::Result<()> { // Get the bridge network between manager and runner. This will be used when invoking `arping`. - let bridge = bridge()?; - // Connect runner to a relay. After this point we will be able to acquire the runner's private in-tunnel IP. + let bridge = &TEST_CONFIG.host_bridge_name; + // Connect runner to a relay. After this point we will be able to acquire the runner's private + // in-tunnel IP. connect_and_wait(&mut mullvad_client).await?; // Get the private ip address let in_tunnel_ip = { @@ -55,12 +54,12 @@ pub async fn test_mllvd_cr_24_03( "-i", "1", "-I", - &bridge, + bridge, &in_tunnel_ip.to_string(), ]) .await?; - // If arping exited with code 0, it means the runner replied to the ARP request, implying the runner leaked its - // private in-tunnel IP! + // If arping exited with code 0, it means the runner replied to the ARP request, implying the + // runner leaked its private in-tunnel IP! if let Some(0) = malicious_arping.status.code() { log::error!("{}", String::from_utf8(malicious_arping.stdout)?); bail!("ARP leak detected") diff --git a/test/test-manager/src/tests/config.rs b/test/test-manager/src/tests/config.rs index e25e60db33b2..b7b7b9f20701 100644 --- a/test/test-manager/src/tests/config.rs +++ b/test/test-manager/src/tests/config.rs @@ -1,6 +1,4 @@ -use std::net::IpAddr; -use std::sync::OnceLock; -use std::{ops::Deref, path::Path}; +use std::{net::Ipv4Addr, ops::Deref, path::Path, sync::OnceLock}; use test_rpc::meta::Os; pub static TEST_CONFIG: TestConfigContainer = TestConfigContainer::new(); @@ -36,7 +34,7 @@ pub struct TestConfig { pub mullvad_host: String, pub host_bridge_name: String, - pub host_bridge_ip: IpAddr, + pub host_bridge_ip: Ipv4Addr, pub os: Os, /// The OpenVPN CA certificate to use with the the installed Mullvad App. pub openvpn_certificate: OpenVPNCertificate, @@ -53,7 +51,7 @@ impl TestConfig { ui_e2e_tests_filename: Option, mullvad_host: String, host_bridge_name: String, - host_bridge_ip: IpAddr, + host_bridge_ip: Ipv4Addr, os: Os, openvpn_certificate: OpenVPNCertificate, ) -> Self { diff --git a/test/test-manager/src/tests/dns.rs b/test/test-manager/src/tests/dns.rs index 69f98450caf1..28db345cf7c4 100644 --- a/test/test-manager/src/tests/dns.rs +++ b/test/test-manager/src/tests/dns.rs @@ -28,9 +28,9 @@ use crate::{ }, vm::network::{ CUSTOM_TUN_GATEWAY, CUSTOM_TUN_LOCAL_PRIVKEY, CUSTOM_TUN_LOCAL_TUN_ADDR, - CUSTOM_TUN_REMOTE_PUBKEY, CUSTOM_TUN_REMOTE_REAL_ADDR, CUSTOM_TUN_REMOTE_REAL_PORT, - CUSTOM_TUN_REMOTE_TUN_ADDR, NON_TUN_GATEWAY, + CUSTOM_TUN_REMOTE_PUBKEY, CUSTOM_TUN_REMOTE_REAL_PORT, CUSTOM_TUN_REMOTE_TUN_ADDR, }, + TEST_CONFIG, }; /// How long to wait for expected "DNS queries" to appear @@ -358,20 +358,28 @@ pub async fn test_dns_config_custom_private( rpc: ServiceClient, mut mullvad_client: MullvadProxyClient, ) -> anyhow::Result<()> { - log::debug!("Setting custom DNS resolver to {NON_TUN_GATEWAY}"); + log::debug!( + "Setting custom DNS resolver to {}", + TEST_CONFIG.host_bridge_ip + ); mullvad_client .set_dns_options(settings::DnsOptions { default_options: settings::DefaultDnsOptions::default(), custom_options: settings::CustomDnsOptions { - addresses: vec![IpAddr::V4(NON_TUN_GATEWAY)], + addresses: vec![IpAddr::V4(TEST_CONFIG.host_bridge_ip)], }, state: settings::DnsState::Custom, }) .await .context("failed to configure DNS server")?; - run_dns_config_non_tunnel_test(&rpc, &mut mullvad_client, IpAddr::V4(NON_TUN_GATEWAY)).await + run_dns_config_non_tunnel_test( + &rpc, + &mut mullvad_client, + IpAddr::V4(TEST_CONFIG.host_bridge_ip), + ) + .await } /// Test whether the expected custom DNS works for public IPs. @@ -646,7 +654,7 @@ async fn connect_local_wg_relay(mullvad_client: &mut MullvadProxyClient) -> Resu .await?; let peer_addr: SocketAddr = SocketAddr::new( - IpAddr::V4(CUSTOM_TUN_REMOTE_REAL_ADDR), + IpAddr::V4(TEST_CONFIG.host_bridge_ip), CUSTOM_TUN_REMOTE_REAL_PORT, ); diff --git a/test/test-manager/src/tests/relay_ip_overrides.rs b/test/test-manager/src/tests/relay_ip_overrides.rs index 3805412ee92a..eb2e36d2f857 100644 --- a/test/test-manager/src/tests/relay_ip_overrides.rs +++ b/test/test-manager/src/tests/relay_ip_overrides.rs @@ -5,8 +5,8 @@ use super::{ TestContext, }; use crate::{ - vm, - vm::network::linux::{NON_TUN_GATEWAY, TEST_SUBNET}, + tests::config::TEST_CONFIG, + vm::{self, network::linux::TEST_SUBNET}, }; use anyhow::{anyhow, bail, ensure, Context}; use futures::FutureExt; @@ -88,7 +88,7 @@ pub async fn test_wireguard_ip_override( mullvad_client .set_relay_override(RelayOverride { hostname, - ipv4_addr_in: Some(NON_TUN_GATEWAY), + ipv4_addr_in: Some(TEST_CONFIG.host_bridge_ip), ipv6_addr_in: None, }) .await?; @@ -144,7 +144,7 @@ pub async fn test_openvpn_ip_override( mullvad_client .set_relay_override(RelayOverride { hostname, - ipv4_addr_in: Some(NON_TUN_GATEWAY), + ipv4_addr_in: Some(TEST_CONFIG.host_bridge_ip), ipv6_addr_in: None, }) .await?; @@ -229,7 +229,7 @@ pub async fn test_bridge_ip_override( mullvad_client .set_relay_override(RelayOverride { hostname, - ipv4_addr_in: Some(NON_TUN_GATEWAY), + ipv4_addr_in: Some(TEST_CONFIG.host_bridge_ip), ipv6_addr_in: None, }) .await?; diff --git a/test/test-manager/src/tests/tunnel.rs b/test/test-manager/src/tests/tunnel.rs index 162de63f615d..bc15ccb8a3bb 100644 --- a/test/test-manager/src/tests/tunnel.rs +++ b/test/test-manager/src/tests/tunnel.rs @@ -625,7 +625,7 @@ pub async fn test_remote_socks_bridge( bridge_type: BridgeType::Custom, normal: BridgeConstraints::default(), custom: Some(CustomProxy::Socks5Remote(Socks5Remote::new(( - crate::vm::network::NON_TUN_GATEWAY, + TEST_CONFIG.host_bridge_ip, crate::vm::network::SOCKS5_PORT, )))), }) @@ -703,10 +703,8 @@ pub async fn test_local_socks_bridge( rpc: ServiceClient, mut mullvad_client: MullvadProxyClient, ) -> Result<(), Error> { - let remote_addr = SocketAddr::from(( - crate::vm::network::NON_TUN_GATEWAY, - crate::vm::network::SOCKS5_PORT, - )); + let remote_addr = + SocketAddr::from((TEST_CONFIG.host_bridge_ip, crate::vm::network::SOCKS5_PORT)); let socks_server = rpc .start_tcp_forward("127.0.0.1:0".parse().unwrap(), remote_addr) .await diff --git a/test/test-manager/src/vm/network/linux.rs b/test/test-manager/src/vm/network/linux.rs index f81131119cea..f5ed5236557f 100644 --- a/test/test-manager/src/vm/network/linux.rs +++ b/test/test-manager/src/vm/network/linux.rs @@ -38,9 +38,6 @@ data_encoding_macro::base64_array!( "pub const CUSTOM_TUN_LOCAL_PRIVKEY" = "mPue6Xt0pdz4NRAhfQSp/SLKo7kV7DW+2zvBq0N9iUI=" ); -/// "Real" (non-tunnel) IP of the wireguard remote peer on the host -#[allow(dead_code)] -pub const CUSTOM_TUN_REMOTE_REAL_ADDR: Ipv4Addr = Ipv4Addr::new(172, 29, 1, 1); /// Port of the wireguard remote peer as defined in `setup-network.sh`. #[allow(dead_code)] pub const CUSTOM_TUN_REMOTE_REAL_PORT: u16 = 51820; @@ -53,7 +50,7 @@ pub const CUSTOM_TUN_REMOTE_TUN_ADDR: Ipv4Addr = Ipv4Addr::new(192, 168, 15, 1); pub const CUSTOM_TUN_GATEWAY: Ipv4Addr = CUSTOM_TUN_REMOTE_TUN_ADDR; /// Gateway of the non-tunnel interface. #[allow(dead_code)] -pub const NON_TUN_GATEWAY: Ipv4Addr = Ipv4Addr::new(172, 29, 1, 1); +pub(super) const NON_TUN_GATEWAY: Ipv4Addr = Ipv4Addr::new(172, 29, 1, 1); /// Name of the wireguard interface on the host pub const CUSTOM_TUN_INTERFACE_NAME: &str = "wg-relay0"; diff --git a/test/test-manager/src/vm/network/macos.rs b/test/test-manager/src/vm/network/macos.rs index 0e52a33caa5d..1dbcb4b596c1 100644 --- a/test/test-manager/src/vm/network/macos.rs +++ b/test/test-manager/src/vm/network/macos.rs @@ -53,16 +53,24 @@ pub async fn setup_test_network() -> Result<()> { /// A hack to find the Tart bridge interface using `NON_TUN_GATEWAY`. /// It should be possible to retrieve this using the virtualization framework instead, /// but that requires an entitlement. -pub(crate) fn find_vm_bridge(bridge_ip: &IpAddr) -> Result { +pub(crate) fn find_vm_bridge(guest_ip: &Ipv4Addr) -> Result<(String, Ipv4Addr)> { for addr in nix::ifaddrs::getifaddrs().unwrap() { if !addr.interface_name.starts_with("bridge") { continue; } - if let Some(address) = addr.address.as_ref().and_then(|addr| addr.as_sockaddr_in()) { - let interface_ip = SocketAddrV4::from(*address).ip(); - if interface_ip == bridge_ip { - return Ok(addr.interface_name.to_owned()); - } + let Some(address) = addr.address.as_ref().and_then(|addr| addr.as_sockaddr_in()) else { + continue; + }; + let address = SocketAddrV4::from(*address).ip(); + let Some(netmask) = addr.netmask.as_ref().and_then(|addr| addr.as_sockaddr_in()) else { + continue; + }; + let netmask = SocketAddrV4::from(*netmask).ip(); + let Ok(ip_v4_network) = ipnetwork::Ipv4Network::with_netmask(address, netmask) else { + continue; + }; + if ip_v4_network.contains(guest_ip) { + return Ok((addr.interface_name.to_owned(), address)); } } diff --git a/test/test-manager/src/vm/network/mod.rs b/test/test-manager/src/vm/network/mod.rs index 5a72264b4567..c9a314726529 100644 --- a/test/test-manager/src/vm/network/mod.rs +++ b/test/test-manager/src/vm/network/mod.rs @@ -1,7 +1,6 @@ // #[cfg(target_os = "linux")] pub mod linux; -#[cfg(target_os = "macos")] -use std::net::IpAddr; +use std::net::Ipv4Addr; #[cfg(target_os = "linux")] pub use linux as platform; @@ -14,19 +13,21 @@ pub use macos as platform; // Import shared constants and functions pub use platform::{ CUSTOM_TUN_GATEWAY, CUSTOM_TUN_INTERFACE_NAME, CUSTOM_TUN_LOCAL_PRIVKEY, - CUSTOM_TUN_LOCAL_TUN_ADDR, CUSTOM_TUN_REMOTE_PUBKEY, CUSTOM_TUN_REMOTE_REAL_ADDR, - CUSTOM_TUN_REMOTE_REAL_PORT, CUSTOM_TUN_REMOTE_TUN_ADDR, NON_TUN_GATEWAY, + CUSTOM_TUN_LOCAL_TUN_ADDR, CUSTOM_TUN_REMOTE_PUBKEY, CUSTOM_TUN_REMOTE_REAL_PORT, + CUSTOM_TUN_REMOTE_TUN_ADDR, }; /// Port on NON_TUN_GATEWAY that hosts a SOCKS5 server pub const SOCKS5_PORT: u16 = 54321; /// Get the name of the bridge interface between the test-manager and the test-runner. -pub fn bridge(#[cfg(target_os = "macos")] bridge_ip: &IpAddr) -> anyhow::Result { +pub fn bridge( + #[cfg(target_os = "macos")] bridge_ip: &Ipv4Addr, +) -> anyhow::Result<(String, Ipv4Addr)> { #[cfg(target_os = "macos")] { crate::vm::network::macos::find_vm_bridge(bridge_ip) } #[cfg(not(target_os = "macos"))] - Ok(platform::BRIDGE_NAME.to_owned()) + Ok((platform::BRIDGE_NAME.to_owned(), platform::NON_TUN_GATEWAY)) }