Merge branch 'add-e2e-tests-for-arp-ping-leak-des-1438'

7 files changed, 207 insertions, 108 deletions

diff --git a/test/test-manager/src/tests/cve_2019_14899.rs b/test/test-manager/src/tests/audits/cve_2019_14899.rs
index 79687b646a..71872c0716 100644
--- a/test/test-manager/src/tests/cve_2019_14899.rs
+++ b/test/test-manager/src/tests/audits/cve_2019_14899.rs
@@ -1,4 +1,16 @@
 #![cfg(target_os = "linux")]
+//! Test mitigation for cve-2019-14899
+//!
+//! The vulnerability allowed a malicious router to learn the victims private mullvad tunnel IP.
+//! It is performed by sending a TCP packet to the victim with SYN and ACK flags set.
+//!
+//! If the destination_addr of the packet was the same as the private IP, the victims computer
+//! would respond to the packet with the RST flag set.
+//!
+//! This test simply gets the private tunnel IP from the test runner and sends the SYN/ACK packet
+//! targeted to that address. If the guest does not respond, the test passes.
+//!
+//! Note that only linux was susceptible to this vulnerability.
 
 use std::{
     convert::Infallible,
@@ -30,27 +42,13 @@ use test_rpc::ServiceClient;
 use tokio::{task::yield_now, time::sleep};
 
 use crate::{
-    tests::helpers,
+    tests::{helpers, TestContext},
     vm::network::{linux::TAP_NAME, NON_TUN_GATEWAY},
 };
 
-use super::TestContext;
-
 /// The port number we set in the malicious packet.
 const MALICIOUS_PACKET_PORT: u16 = 12345;
 
-/// Test mitigation for cve-2019-14899.
-///
-/// The vulnerability allowed a malicious router to learn the victims private mullvad tunnel IP.
-/// It is performed by sending a TCP packet to the victim with SYN and ACK flags set.
-///
-/// If the destination_addr of the packet was the same as the private IP, the victims computer
-/// would respond to the packet with the RST flag set.
-///
-/// This test simply gets the private tunnel IP from the test runner and sends the SYN/ACK packet
-/// targeted to that address. If the guest does not respond, the test passes.
-///
-/// Note that only linux was susceptible to this vulnerability.
 #[test_function(target_os = "linux")]
 pub async fn test_cve_2019_14899_mitigation(
     _: TestContext,
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
new file mode 100644
index 0000000000..bf743bcc67
--- /dev/null
+++ b/test/test-manager/src/tests/audits/mllvd_cr_24_03.rs
@@ -0,0 +1,81 @@
+#![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.
+//!
+//! # Setup
+//!
+//! Victim: test-runner
+//!
+//! 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-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 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;
+
+#[test_function(target_os = "linux")]
+pub async fn test_mllvd_cr_24_03(
+    _: TestContext,
+    rpc: ServiceClient,
+    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.
+    connect_and_wait(&mut mullvad_client).await?;
+    // Get the private ip address
+    let in_tunnel_ip = {
+        let vpn_interface = get_tunnel_interface(&mut mullvad_client).await?;
+        rpc.get_interface_ip(vpn_interface).await?
+    };
+    // Invoke arping
+    let malicious_arping = arping([
+        "-w",
+        "5",
+        "-i",
+        "1",
+        "-I",
+        &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 let Some(0) = malicious_arping.status.code() {
+        log::error!("{}", String::from_utf8(malicious_arping.stdout)?);
+        bail!("ARP leak detected")
+    }
+    // test runner did not respond to ARP request, leak mitigation seems to work!
+    Ok(())
+}
+
+/// Invoke `arping` on test-manager.
+async fn arping<I, S>(args: I) -> std::io::Result<Output>
+where
+    I: IntoIterator<Item = S>,
+    S: AsRef<OsStr>,
+{
+    let mut arping = tokio::process::Command::new("arping");
+    arping.args(args);
+    arping.output().await
+}
diff --git a/test/test-manager/src/tests/audits/mod.rs b/test/test-manager/src/tests/audits/mod.rs
new file mode 100644
index 0000000000..c580c5c47c
--- /dev/null
+++ b/test/test-manager/src/tests/audits/mod.rs
@@ -0,0 +1,4 @@
+//! This module collects tests for old audit issues to prevent any potential regression.
+pub mod cve_2019_14899;
+pub mod mllvd_cr_24_03;
+pub mod mul_02_002;
diff --git a/test/test-manager/src/tests/audits/mul_02_002.rs b/test/test-manager/src/tests/audits/mul_02_002.rs
new file mode 100644
index 0000000000..c0571eaacc
--- /dev/null
+++ b/test/test-manager/src/tests/audits/mul_02_002.rs
@@ -0,0 +1,98 @@
+//! Test mitigation for MUL-02-002-WP2
+//!
+//! Fail to leak traffic to verify that mitigation "Firewall allows deanonymization by eavesdropper" works.
+//!
+//! # Vulnerability
+//! 1. Connect to a relay on port 443. Record this relay's IP address (the new gateway of the
+//!    client)
+//! 2. Start listening for unencrypted traffic on the outbound network interface
+//!    (Choose some human-readable, identifiable payload to look for in the outgoing TCP packets)
+//! 3. Start a rogue program which performs a GET request\* containing the payload defined in step 2
+//! 4. The network snooper started in step 2 should now be able to observe the network request
+//!    containing the identifiable payload being sent unencrypted over the wire
+//!
+//! \* or something similar, as long as it generates some traffic containing UDP and/or TCP packets
+//!    with the correct payload.
+
+use anyhow::{bail, ensure};
+use mullvad_management_interface::MullvadProxyClient;
+use mullvad_relay_selector::query::builder::{RelayQueryBuilder, TransportProtocol};
+use mullvad_types::states::TunnelState;
+use test_macro::test_function;
+use test_rpc::ServiceClient;
+
+use crate::network_monitor::{start_packet_monitor, MonitorOptions, ParsedPacket};
+use crate::tests::helpers::{
+    connect_and_wait, constrain_to_relay, disconnect_and_wait, ConnChecker,
+};
+use crate::tests::TestContext;
+
+#[test_function]
+pub async fn test_mul_02_002(
+    _: TestContext,
+    rpc: ServiceClient,
+    mut mullvad_client: MullvadProxyClient,
+) -> anyhow::Result<()> {
+    // Step 1 - Choose a relay
+    constrain_to_relay(
+        &mut mullvad_client,
+        RelayQueryBuilder::new()
+            .openvpn()
+            .transport_protocol(TransportProtocol::Tcp)
+            .port(443)
+            .build(),
+    )
+    .await?;
+
+    // Step 1.5 - Temporarily connect to the relay to get the target endpoint
+    let tunnel_state = connect_and_wait(&mut mullvad_client).await?;
+    let TunnelState::Connected { endpoint, .. } = tunnel_state else {
+        bail!("Expected tunnel state to be `Connected` - instead it was {tunnel_state:?}");
+    };
+    disconnect_and_wait(&mut mullvad_client).await?;
+    let target_endpoint = endpoint.endpoint.address;
+
+    // Step 2 - Start a network monitor snooping the outbound network interface for some
+    // identifiable payload
+    let unique_identifier = "Hello there!";
+    let identify_rogue_packet = move |packet: &ParsedPacket| {
+        packet
+            .payload
+            .windows(unique_identifier.len())
+            .any(|window| window == unique_identifier.as_bytes())
+    };
+    let rogue_packet_monitor =
+        start_packet_monitor(identify_rogue_packet, MonitorOptions::default()).await;
+
+    // Step 3 - Start the rogue program which will try to leak the unique identifier payload
+    // to the chosen relay endpoint
+    let mut checker = ConnChecker::new(rpc.clone(), mullvad_client.clone(), target_endpoint);
+    checker.payload(unique_identifier);
+    let mut conn_artist = checker.spawn().await?;
+    // Before proceeding, assert that the method of detecting identifiable packets work.
+    conn_artist.check_connection().await?;
+    let monitor_result = rogue_packet_monitor.into_result().await?;
+
+    log::info!("Checking that the identifiable payload was detectable without encryption");
+    ensure!(
+        !monitor_result.packets.is_empty(),
+        "Did not observe rogue packets! The method seems to be broken"
+    );
+    log::info!("The identifiable payload was detected! (that's good)");
+
+    // Step 4 - Finally, connect to a tunnel and assert that no outgoing traffic contains the
+    // payload in plain text.
+    connect_and_wait(&mut mullvad_client).await?;
+    let rogue_packet_monitor =
+        start_packet_monitor(identify_rogue_packet, MonitorOptions::default()).await;
+    conn_artist.check_connection().await?;
+    let monitor_result = rogue_packet_monitor.into_result().await?;
+
+    log::info!("Checking that the identifiable payload was not detected");
+    ensure!(
+        monitor_result.packets.is_empty(),
+        "Observed rogue packets! The tunnel seems to be leaking traffic"
+    );
+
+    Ok(())
+}
diff --git a/test/test-manager/src/tests/helpers.rs b/test/test-manager/src/tests/helpers.rs
index 0c3f16ccb3..84945aef79 100644
--- a/test/test-manager/src/tests/helpers.rs
+++ b/test/test-manager/src/tests/helpers.rs
@@ -168,12 +168,15 @@ pub async fn using_mullvad_exit(rpc: &ServiceClient) -> bool {
 }
 
 /// Get VPN tunnel interface name
-pub async fn get_tunnel_interface(client: &mut MullvadProxyClient) -> Option<String> {
-    match client.get_tunnel_state().await.ok()? {
+pub async fn get_tunnel_interface(client: &mut MullvadProxyClient) -> anyhow::Result<String> {
+    match client.get_tunnel_state().await? {
         TunnelState::Connecting { endpoint, .. } | TunnelState::Connected { endpoint, .. } => {
-            endpoint.tunnel_interface
+            let Some(tunnel_interface) = endpoint.tunnel_interface else {
+                bail!("Unknown tunnel interface");
+            };
+            Ok(tunnel_interface)
         }
-        _ => None,
+        _ => bail!("Tunnel is not up"),
     }
 }
 
diff --git a/test/test-manager/src/tests/mod.rs b/test/test-manager/src/tests/mod.rs
index 14c2a1e3a5..ca24cd63bf 100644
--- a/test/test-manager/src/tests/mod.rs
+++ b/test/test-manager/src/tests/mod.rs
@@ -1,7 +1,7 @@
 mod access_methods;
 mod account;
+mod audits;
 pub mod config;
-mod cve_2019_14899;
 mod daita;
 mod dns;
 mod helpers;
diff --git a/test/test-manager/src/tests/tunnel.rs b/test/test-manager/src/tests/tunnel.rs
index fde8c73049..d1b6e65585 100644
--- a/test/test-manager/src/tests/tunnel.rs
+++ b/test/test-manager/src/tests/tunnel.rs
@@ -7,11 +7,11 @@ use super::{
     Error, TestContext,
 };
 use crate::{
-    network_monitor::{start_packet_monitor, MonitorOptions, ParsedPacket},
-    tests::helpers::{login_with_retries, ConnChecker},
+    network_monitor::{start_packet_monitor, MonitorOptions},
+    tests::helpers::login_with_retries,
 };
 
-use anyhow::{bail, ensure, Context};
+use anyhow::Context;
 use mullvad_management_interface::MullvadProxyClient;
 use mullvad_relay_selector::query::builder::RelayQueryBuilder;
 use mullvad_types::{
@@ -20,7 +20,6 @@ use mullvad_types::{
         self, BridgeConstraints, BridgeSettings, BridgeType, OpenVpnConstraints, RelayConstraints,
         RelaySettings, TransportPort, WireguardConstraints,
     },
-    states::TunnelState,
     wireguard,
 };
 use std::net::SocketAddr;
@@ -833,87 +832,3 @@ pub async fn test_establish_tunnel_without_api(
     // Profit
     Ok(())
 }
-
-/// Fail to leak traffic to verify that mitigation for MUL-02-002-WP2
-/// ("Firewall allows deanonymization by eavesdropper") works.
-///
-/// # Vulnerability
-/// 1. Connect to a relay on port 443. Record this relay's IP address (the new gateway of the
-///    client)
-/// 2. Start listening for unencrypted traffic on the outbound network interface
-/// (Choose some human-readable, identifiable payload to look for in the outgoing TCP packets)
-/// 3. Start a rogue program which performs a GET request* containing the payload defined in step 2
-/// 4. The network snooper started in step 2 should now be able to observe the network request
-///    containing the identifiable payload being sent unencrypted over the wire
-///
-/// * or something similiar, as long as it generates some traffic containing UDP and/or TCP packets
-/// with the correct payload.
-#[test_function]
-pub async fn test_mul_02_002(
-    _: TestContext,
-    rpc: ServiceClient,
-    mut mullvad_client: MullvadProxyClient,
-) -> anyhow::Result<()> {
-    // Step 1 - Choose a relay
-    helpers::constrain_to_relay(
-        &mut mullvad_client,
-        RelayQueryBuilder::new()
-            .openvpn()
-            .transport_protocol(TransportProtocol::Tcp)
-            .port(443)
-            .build(),
-    )
-    .await?;
-
-    // Step 1.5 - Temporarily connect to the relay to get the target endpoint
-    let tunnel_state = helpers::connect_and_wait(&mut mullvad_client).await?;
-    let TunnelState::Connected { endpoint, .. } = tunnel_state else {
-        bail!("Expected tunnel state to be `Connected` - instead it was {tunnel_state:?}");
-    };
-    helpers::disconnect_and_wait(&mut mullvad_client).await?;
-    let target_endpoint = endpoint.endpoint.address;
-
-    // Step 2 - Start a network monitor snooping the outbound network interface for some
-    // identifiable payload
-    let unique_identifier = "Hello there!";
-    let identify_rogue_packet = move |packet: &ParsedPacket| {
-        packet
-            .payload
-            .windows(unique_identifier.len())
-            .any(|window| window == unique_identifier.as_bytes())
-    };
-    let rogue_packet_monitor =
-        start_packet_monitor(identify_rogue_packet, MonitorOptions::default()).await;
-
-    // Step 3 - Start the rogue program which will try to leak the unique identifier payload
-    // to the chosen relay endpoint
-    let mut checker = ConnChecker::new(rpc.clone(), mullvad_client.clone(), target_endpoint);
-    checker.payload(unique_identifier);
-    let mut conn_artist = checker.spawn().await?;
-    // Before proceeding, assert that the method of detecting identifiable packets work.
-    conn_artist.check_connection().await?;
-    let monitor_result = rogue_packet_monitor.into_result().await?;
-
-    log::info!("Checking that the identifiable payload was detectable without encryption");
-    ensure!(
-        !monitor_result.packets.is_empty(),
-        "Did not observe rogue packets! The method seems to be broken"
-    );
-    log::info!("The identifiable payload was detected! (that's good)");
-
-    // Step 4 - Finally, connect to a tunnel and assert that no outgoing traffic contains the
-    // payload in plain text.
-    helpers::connect_and_wait(&mut mullvad_client).await?;
-    let rogue_packet_monitor =
-        start_packet_monitor(identify_rogue_packet, MonitorOptions::default()).await;
-    conn_artist.check_connection().await?;
-    let monitor_result = rogue_packet_monitor.into_result().await?;
-
-    log::info!("Checking that the identifiable payload was not detected");
-    ensure!(
-        monitor_result.packets.is_empty(),
-        "Observed rogue packets! The tunnel seems to be leaking traffic"
-    );
-
-    Ok(())
-}