Merge branch 'fix-flaky-lan-lockdown-test'

6 files changed, 53 insertions, 132 deletions

diff --git a/test/test-manager/src/tests/helpers.rs b/test/test-manager/src/tests/helpers.rs
index 86cbecb50c..da50679a26 100644
--- a/test/test-manager/src/tests/helpers.rs
+++ b/test/test-manager/src/tests/helpers.rs
@@ -121,7 +121,7 @@ pub async fn send_guest_probes(
     rpc: ServiceClient,
     interface: String,
     destination: SocketAddr,
-) -> Result<ProbeResult, Error> {
+) -> ProbeResult {
     const MONITOR_DURATION: Duration = Duration::from_secs(8);
 
     let pktmon = start_packet_monitor(
@@ -162,7 +162,7 @@ pub async fn send_guest_probes(
         }
     }
 
-    Ok(result)
+    result
 }
 
 /// Send one probe per transport protocol to `destination` without running a packet monitor
@@ -509,8 +509,8 @@ pub fn get_app_env() -> HashMap<String, String> {
 ///
 /// # Note
 /// This function does not handle bridges and multihop configurations (currently). There is no
-/// particular reason for this other than it not being needed at the time, so feel free to extend this
-/// function :).
+/// particular reason for this other than it not being needed at the time, so feel free to extend
+/// this function :).
 pub async fn constrain_to_relay(
     mullvad_client: &mut MullvadProxyClient,
     query: RelayQuery,
diff --git a/test/test-manager/src/tests/settings.rs b/test/test-manager/src/tests/settings.rs
index 6d1ade18c7..bd52800499 100644
--- a/test/test-manager/src/tests/settings.rs
+++ b/test/test-manager/src/tests/settings.rs
@@ -3,11 +3,9 @@ use super::{
     helpers::{connect_and_wait, send_guest_probes},
     Error, TestContext,
 };
-use crate::{assert_tunnel_state, vm::network::DUMMY_LAN_INTERFACE_IP};
 
 use mullvad_management_interface::MullvadProxyClient;
-use mullvad_types::states::TunnelState;
-use std::net::{IpAddr, SocketAddr};
+use std::net::SocketAddr;
 use test_macro::test_function;
 use test_rpc::ServiceClient;
 
@@ -22,12 +20,9 @@ pub async fn test_lan(
     rpc: ServiceClient,
     mut mullvad_client: MullvadProxyClient,
 ) -> Result<(), Error> {
-    let lan_destination = SocketAddr::new(IpAddr::V4(DUMMY_LAN_INTERFACE_IP), 1234);
-
-    // Connect
-    //
-
-    connect_and_wait(&mut mullvad_client).await?;
+    // Take care not to use some bogus IP in the guest's subnet, lest we just send ARP requests
+    // These will fail if there's no actual host present
+    let lan_destination = "10.1.2.3:1234".parse().unwrap();
 
     // Disable LAN sharing
     //
@@ -39,6 +34,11 @@ pub async fn test_lan(
         .await
         .expect("failed to disable LAN sharing");
 
+    // Connect
+    //
+
+    connect_and_wait(&mut mullvad_client).await?;
+
     // Ensure LAN is not reachable
     //
 
@@ -46,7 +46,7 @@ pub async fn test_lan(
 
     let default_interface = rpc.get_default_interface().await?;
     let detected_probes =
-        send_guest_probes(rpc.clone(), default_interface.clone(), lan_destination).await?;
+        send_guest_probes(rpc.clone(), default_interface.clone(), lan_destination).await;
     assert!(
         detected_probes.none(),
         "observed unexpected outgoing LAN packets: {detected_probes:?}"
@@ -67,8 +67,7 @@ pub async fn test_lan(
 
     log::info!("Test whether outgoing LAN traffic is blocked");
 
-    let detected_probes =
-        send_guest_probes(rpc.clone(), default_interface, lan_destination).await?;
+    let detected_probes = send_guest_probes(rpc.clone(), default_interface, lan_destination).await;
     assert!(
         detected_probes.all(),
         "did not observe all outgoing LAN packets: {detected_probes:?}"
@@ -96,19 +95,11 @@ pub async fn test_lockdown(
     rpc: ServiceClient,
     mut mullvad_client: MullvadProxyClient,
 ) -> Result<(), Error> {
-    let lan_destination: SocketAddr = SocketAddr::new(IpAddr::V4(DUMMY_LAN_INTERFACE_IP), 1337);
+    // Take care not to use some bogus IP in the guest's subnet, lest we just send ARP requests
+    // These will fail if there's no actual host present
+    let lan_destination = "10.1.2.3:1234".parse().unwrap();
     let inet_destination: SocketAddr = "1.1.1.1:1337".parse().unwrap();
 
-    log::info!("Verify tunnel state: disconnected");
-    assert_tunnel_state!(&mut mullvad_client, TunnelState::Disconnected { .. });
-
-    // Enable lockdown mode
-    //
-    mullvad_client
-        .set_block_when_disconnected(true)
-        .await
-        .expect("failed to enable lockdown mode");
-
     // Disable LAN sharing
     //
 
@@ -119,20 +110,27 @@ pub async fn test_lockdown(
         .await
         .expect("failed to disable LAN sharing");
 
+    // Enable lockdown mode
+    //
+    mullvad_client
+        .set_block_when_disconnected(true)
+        .await
+        .expect("failed to enable lockdown mode");
+
     // Ensure all destinations are unreachable
     //
 
     let default_interface = rpc.get_default_interface().await?;
 
     let detected_probes =
-        send_guest_probes(rpc.clone(), default_interface.clone(), lan_destination).await?;
+        send_guest_probes(rpc.clone(), default_interface.clone(), lan_destination).await;
     assert!(
         detected_probes.none(),
         "observed outgoing packets to LAN: {detected_probes:?}"
     );
 
     let detected_probes =
-        send_guest_probes(rpc.clone(), default_interface.clone(), inet_destination).await?;
+        send_guest_probes(rpc.clone(), default_interface.clone(), inet_destination).await;
     assert!(
         detected_probes.none(),
         "observed outgoing packets to internet: {detected_probes:?}"
@@ -152,14 +150,14 @@ pub async fn test_lockdown(
     //
 
     let detected_probes =
-        send_guest_probes(rpc.clone(), default_interface.clone(), lan_destination).await?;
+        send_guest_probes(rpc.clone(), default_interface.clone(), lan_destination).await;
     assert!(
         detected_probes.all(),
         "did not observe some outgoing packets: {detected_probes:?}"
     );
 
     let detected_probes =
-        send_guest_probes(rpc.clone(), default_interface.clone(), inet_destination).await?;
+        send_guest_probes(rpc.clone(), default_interface.clone(), inet_destination).await;
     assert!(
         detected_probes.none(),
         "observed outgoing packets to internet: {detected_probes:?}"
@@ -180,19 +178,11 @@ pub async fn test_lockdown(
 
     // Send traffic outside the tunnel to sanity check that the internet is *not* reachable via non-
     // tunnel interfaces.
-    let detected_probes =
-        send_guest_probes(rpc.clone(), default_interface, inet_destination).await?;
+    let detected_probes = send_guest_probes(rpc.clone(), default_interface, inet_destination).await;
     assert!(
         detected_probes.none(),
         "observed outgoing packets to internet: {detected_probes:?}"
     );
 
-    // Disable lockdown mode
-    //
-    mullvad_client
-        .set_block_when_disconnected(false)
-        .await
-        .expect("failed to disable lockdown mode");
-
     Ok(())
 }
diff --git a/test/test-manager/src/tests/tunnel_state.rs b/test/test-manager/src/tests/tunnel_state.rs
index edbe595092..f75bc79498 100644
--- a/test/test-manager/src/tests/tunnel_state.rs
+++ b/test/test-manager/src/tests/tunnel_state.rs
@@ -5,10 +5,7 @@ use super::{
     },
     ui, Error, TestContext,
 };
-use crate::{
-    assert_tunnel_state, tests::helpers::ping_sized_with_timeout,
-    vm::network::DUMMY_LAN_INTERFACE_IP,
-};
+use crate::{assert_tunnel_state, tests::helpers::ping_sized_with_timeout};
 
 use mullvad_management_interface::MullvadProxyClient;
 use mullvad_relay_selector::query::builder::RelayQueryBuilder;
@@ -20,10 +17,7 @@ use mullvad_types::{
     states::TunnelState,
     CustomTunnelEndpoint,
 };
-use std::{
-    net::{IpAddr, SocketAddr},
-    time::Duration,
-};
+use std::{net::SocketAddr, time::Duration};
 use talpid_types::net::{Endpoint, TransportProtocol, TunnelEndpoint, TunnelType};
 use test_macro::test_function;
 use test_rpc::ServiceClient;
@@ -143,7 +137,7 @@ pub async fn test_disconnected_state(
         .expect("failed to obtain non-tun interface");
 
     let detected_probes =
-        send_guest_probes(rpc.clone(), non_tunnel_interface, inet_destination).await?;
+        send_guest_probes(rpc.clone(), non_tunnel_interface, inet_destination).await;
     assert!(
         detected_probes.all(),
         "did not see (all) outgoing packets to destination: {detected_probes:?}",
@@ -181,9 +175,11 @@ pub async fn test_connecting_state(
     mut mullvad_client: MullvadProxyClient,
 ) -> Result<(), Error> {
     let inet_destination = "1.1.1.1:1337".parse().unwrap();
-    let lan_destination: SocketAddr = SocketAddr::new(IpAddr::V4(DUMMY_LAN_INTERFACE_IP), 1337);
+    // Take care not to use some bogus IP in the guest's subnet, lest we just send ARP requests
+    // These will fail if there's no actual host present
+    let lan_destination = "10.1.2.3:1234".parse().unwrap();
     let inet_dns = "1.1.1.1:53".parse().unwrap();
-    let lan_dns: SocketAddr = SocketAddr::new(IpAddr::V4(DUMMY_LAN_INTERFACE_IP), 53);
+    let lan_dns = "10.1.2.3:53".parse().unwrap();
 
     log::info!("Verify tunnel state: disconnected");
     assert_tunnel_state!(&mut mullvad_client, TunnelState::Disconnected { .. });
@@ -225,25 +221,25 @@ pub async fn test_connecting_state(
 
     assert!(
         send_guest_probes(rpc.clone(), non_tunnel_interface.clone(), inet_destination)
-            .await?
+            .await
             .none(),
         "observed unexpected outgoing packets (inet)"
     );
     assert!(
         send_guest_probes(rpc.clone(), non_tunnel_interface.clone(), lan_destination)
-            .await?
+            .await
             .none(),
         "observed unexpected outgoing packets (lan)"
     );
     assert!(
         send_guest_probes(rpc.clone(), non_tunnel_interface.clone(), inet_dns)
-            .await?
+            .await
             .none(),
         "observed unexpected outgoing packets (DNS, inet)"
     );
     assert!(
         send_guest_probes(rpc.clone(), non_tunnel_interface, lan_dns)
-            .await?
+            .await
             .none(),
         "observed unexpected outgoing packets (DNS, lan)"
     );
@@ -262,9 +258,11 @@ pub async fn test_error_state(
     mut mullvad_client: MullvadProxyClient,
 ) -> Result<(), Error> {
     let inet_destination = "1.1.1.1:1337".parse().unwrap();
-    let lan_destination: SocketAddr = SocketAddr::new(IpAddr::V4(DUMMY_LAN_INTERFACE_IP), 1337);
+    // Take care not to use some bogus IP in the guest's subnet, lest we just send ARP requests
+    // These will fail if there's no actual host present
+    let lan_destination = "10.1.2.3:1234".parse().unwrap();
     let inet_dns = "1.1.1.1:53".parse().unwrap();
-    let lan_dns: SocketAddr = SocketAddr::new(IpAddr::V4(DUMMY_LAN_INTERFACE_IP), 53);
+    let lan_dns = "10.1.2.3:53".parse().unwrap();
 
     log::info!("Verify tunnel state: disconnected");
     assert_tunnel_state!(&mut mullvad_client, TunnelState::Disconnected { .. });
@@ -303,25 +301,25 @@ pub async fn test_error_state(
 
     assert!(
         send_guest_probes(rpc.clone(), default_interface.clone(), inet_destination)
-            .await?
+            .await
             .none(),
         "observed unexpected outgoing packets (inet)"
     );
     assert!(
         send_guest_probes(rpc.clone(), default_interface.clone(), lan_destination)
-            .await?
+            .await
             .none(),
         "observed unexpected outgoing packets (lan)"
     );
     assert!(
         send_guest_probes(rpc.clone(), default_interface.clone(), inet_dns)
-            .await?
+            .await
             .none(),
         "observed unexpected outgoing packets (DNS, inet)"
     );
     assert!(
         send_guest_probes(rpc.clone(), default_interface, lan_dns)
-            .await?
+            .await
             .none(),
         "observed unexpected outgoing packets (DNS, lan)"
     );
@@ -389,7 +387,7 @@ pub async fn test_connected_state(
         .await
         .expect("failed to find non-tun interface");
 
-    let detected_probes = send_guest_probes(rpc.clone(), nontun_iface, inet_destination).await?;
+    let detected_probes = send_guest_probes(rpc.clone(), nontun_iface, inet_destination).await;
     assert!(
         detected_probes.none(),
         "observed unexpected outgoing packets: {detected_probes:?}"
diff --git a/test/test-manager/src/vm/network/linux.rs b/test/test-manager/src/vm/network/linux.rs
index f54d218b2f..c767a35072 100644
--- a/test/test-manager/src/vm/network/linux.rs
+++ b/test/test-manager/src/vm/network/linux.rs
@@ -25,15 +25,6 @@ pub const BRIDGE_NAME: &str = "br-mullvadtest";
 /// TAP interface used by the guest
 pub const TAP_NAME: &str = "tap-mullvadtest";
 
-/// Pingable dummy LAN interface (name)
-pub const DUMMY_LAN_INTERFACE_NAME: &str = "lan-mullvadtest";
-/// Pingable dummy LAN interface (IP)
-pub const DUMMY_LAN_INTERFACE_IP: Ipv4Addr = Ipv4Addr::new(172, 29, 1, 200);
-/// Pingable dummy interface with public IP (name)
-pub const DUMMY_INET_INTERFACE_NAME: &str = "net-mullvadtest";
-/// Pingable dummy interface with public IP (IP)
-pub const DUMMY_INET_INTERFACE_IP: Ipv4Addr = Ipv4Addr::new(1, 3, 3, 7);
-
 // Private key of the wireguard remote peer on host.
 const CUSTOM_TUN_REMOTE_PRIVKEY: &str = "gLvQuyqazziyf+pUCAFUgTnWIwn6fPE5MOReOqPEGHU=";
 // Public key of the wireguard remote peer on host.
@@ -47,9 +38,9 @@ data_encoding_macro::base64_array!(
     "pub const CUSTOM_TUN_LOCAL_PRIVKEY" = "mPue6Xt0pdz4NRAhfQSp/SLKo7kV7DW+2zvBq0N9iUI="
 );
 
-/// "Real" (non-tunnel) IP of the wireguard remote peer as defined in `setup-network.sh`.
+/// "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, 200);
+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;
@@ -134,28 +125,6 @@ table ip mullvad_test_nat {{
     ))
     .await?;
 
-    log::debug!("Set up pingable hosts");
-
-    run_ip_cmd(["link", "add", DUMMY_LAN_INTERFACE_NAME, "type", "dummy"]).await?;
-    run_ip_cmd([
-        "addr",
-        "add",
-        "dev",
-        DUMMY_LAN_INTERFACE_NAME,
-        &DUMMY_LAN_INTERFACE_IP.to_string(),
-    ])
-    .await?;
-
-    run_ip_cmd(["link", "add", DUMMY_INET_INTERFACE_NAME, "type", "dummy"]).await?;
-    run_ip_cmd([
-        "addr",
-        "add",
-        "dev",
-        DUMMY_INET_INTERFACE_NAME,
-        &DUMMY_INET_INTERFACE_IP.to_string(),
-    ])
-    .await?;
-
     log::debug!("Create WireGuard peer");
 
     create_local_wireguard_peer().await?;
diff --git a/test/test-manager/src/vm/network/macos.rs b/test/test-manager/src/vm/network/macos.rs
index ef815f203a..78653df41c 100644
--- a/test/test-manager/src/vm/network/macos.rs
+++ b/test/test-manager/src/vm/network/macos.rs
@@ -4,9 +4,6 @@ use anyhow::{anyhow, Context, Result};
 use futures::future::{self, Either};
 use tokio::{io::AsyncWriteExt, process::Command};
 
-/// Pingable dummy LAN interface (IP)
-pub const DUMMY_LAN_INTERFACE_IP: Ipv4Addr = Ipv4Addr::new(192, 168, 64, 254);
-
 // Private key of the wireguard remote peer on host.
 const CUSTOM_TUN_REMOTE_PRIVKEY: &str = "gLvQuyqazziyf+pUCAFUgTnWIwn6fPE5MOReOqPEGHU=";
 // Public key of the wireguard remote peer on host.
@@ -50,41 +47,9 @@ pub async fn setup_test_network() -> Result<()> {
         .await
         .context("Failed to create WireGuard interface")?;
 
-    // A bit of trickery to detect when the bridge is available.
-    tokio::spawn(async move {
-        for _ in 0..30 {
-            let Ok(interface) = find_vm_bridge() else {
-                tokio::time::sleep(Duration::from_secs(1)).await;
-                continue;
-            };
-            match create_dummy_interface(interface).await {
-                Ok(_) => log::debug!("Created dummy interface"),
-                Err(error) => log::error!("Failed to create dummy interface: {error}"),
-            }
-            return;
-        }
-        log::error!("Failed to create dummy interface: timed out");
-    });
-
     Ok(())
 }
 
-async fn create_dummy_interface(interface: String) -> Result<()> {
-    let mut cmd = Command::new("/usr/bin/sudo");
-    cmd.args([
-        "/sbin/ifconfig",
-        &interface,
-        "alias",
-        &DUMMY_LAN_INTERFACE_IP.to_string(),
-    ]);
-    let output = cmd.output().await.context("Create dummy interface")?;
-    if output.status.success() {
-        Ok(())
-    } else {
-        Err(anyhow!("ifconfig failed: {:?}", output.status.code()))
-    }
-}
-
 /// 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.
diff --git a/test/test-manager/src/vm/network/mod.rs b/test/test-manager/src/vm/network/mod.rs
index 944e241013..de055376b0 100644
--- a/test/test-manager/src/vm/network/mod.rs
+++ b/test/test-manager/src/vm/network/mod.rs
@@ -12,8 +12,7 @@ pub use macos as platform;
 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, DUMMY_LAN_INTERFACE_IP,
-    NON_TUN_GATEWAY,
+    CUSTOM_TUN_REMOTE_REAL_PORT, CUSTOM_TUN_REMOTE_TUN_ADDR, NON_TUN_GATEWAY,
 };
 
 /// Port on NON_TUN_GATEWAY that hosts a SOCKS5 server