Implement test for audit ticket `MUL-02-002 WP2`

7 files changed, 166 insertions, 82 deletions

diff --git a/test/connection-checker/src/net.rs b/test/connection-checker/src/net.rs
index 2e17423933..ff45993408 100644
--- a/test/connection-checker/src/net.rs
+++ b/test/connection-checker/src/net.rs
@@ -31,7 +31,7 @@ pub fn send_tcp(opt: &Opt, destination: SocketAddr) -> eyre::Result<()> {
 
     let mut stream = std::net::TcpStream::from(sock);
     stream
-        .write_all(b"hello there")
+        .write_all(PAYLOAD)
         .wrap_err(eyre!("Failed to send message to {destination}"))?;
 
     Ok(())
@@ -56,7 +56,7 @@ pub fn send_udp(_opt: &Opt, destination: SocketAddr) -> Result<(), eyre::Error>
 
     let std_socket = std::net::UdpSocket::from(sock);
     std_socket
-        .send_to(b"Hello there!", destination)
+        .send_to(PAYLOAD, destination)
         .wrap_err(eyre!("Failed to send message to {destination}"))?;
 
     Ok(())
diff --git a/test/test-manager/src/network_monitor.rs b/test/test-manager/src/network_monitor.rs
index 87a8193e29..ddd35edc7e 100644
--- a/test/test-manager/src/network_monitor.rs
+++ b/test/test-manager/src/network_monitor.rs
@@ -25,6 +25,7 @@ pub struct ParsedPacket {
     pub source: SocketAddr,
     pub destination: SocketAddr,
     pub protocol: IpNextHeaderProtocol,
+    pub payload: Vec<u8>,
 }
 
 impl PacketCodec for Codec {
@@ -74,9 +75,9 @@ impl Codec {
 
         let mut source = SocketAddr::new(IpAddr::V4(packet.get_source()), 0);
         let mut destination = SocketAddr::new(IpAddr::V4(packet.get_destination()), 0);
+        let mut payload = vec![];
 
         let protocol = packet.get_next_level_protocol();
-
         match protocol {
             IpHeaderProtocols::Tcp => {
                 let seg = TcpPacket::new(packet.payload()).or_else(|| {
@@ -85,6 +86,7 @@ impl Codec {
                 })?;
                 source.set_port(seg.get_source());
                 destination.set_port(seg.get_destination());
+                payload = seg.payload().to_vec();
             }
             IpHeaderProtocols::Udp => {
                 let seg = UdpPacket::new(packet.payload()).or_else(|| {
@@ -93,6 +95,7 @@ impl Codec {
                 })?;
                 source.set_port(seg.get_source());
                 destination.set_port(seg.get_destination());
+                payload = seg.payload().to_vec();
             }
             IpHeaderProtocols::Icmp => {}
             proto => log::debug!("ignoring v4 packet, transport/protocol type {proto}"),
@@ -102,6 +105,7 @@ impl Codec {
             source,
             destination,
             protocol,
+            payload,
         })
     }
 
@@ -113,6 +117,7 @@ impl Codec {
 
         let mut source = SocketAddr::new(IpAddr::V6(packet.get_source()), 0);
         let mut destination = SocketAddr::new(IpAddr::V6(packet.get_destination()), 0);
+        let mut payload = vec![];
 
         let protocol = packet.get_next_header();
         match protocol {
@@ -123,6 +128,7 @@ impl Codec {
                 })?;
                 source.set_port(seg.get_source());
                 destination.set_port(seg.get_destination());
+                payload = seg.payload().to_vec();
             }
             IpHeaderProtocols::Udp => {
                 let seg = UdpPacket::new(packet.payload()).or_else(|| {
@@ -131,6 +137,7 @@ impl Codec {
                 })?;
                 source.set_port(seg.get_source());
                 destination.set_port(seg.get_destination());
+                payload = seg.payload().to_vec();
             }
             IpHeaderProtocols::Icmpv6 => {}
             proto => log::debug!("ignoring v6 packet, transport/protocol type {proto}"),
@@ -140,12 +147,14 @@ impl Codec {
             source,
             destination,
             protocol,
+            payload,
         })
     }
 }
 
-#[derive(Debug)]
-pub struct MonitorUnexpectedlyStopped(());
+#[derive(Debug, thiserror::Error)]
+#[error("Packet monitor stopped unexpectedly")]
+pub struct MonitorUnexpectedlyStopped;
 
 pub struct PacketMonitor {
     handle: tokio::task::JoinHandle<Result<MonitorResult, MonitorUnexpectedlyStopped>>,
@@ -297,7 +306,7 @@ async fn start_packet_monitor_for_interface(
                         }
                         _ => {
                             log::error!("lost packet stream");
-                            break Err(MonitorUnexpectedlyStopped(()));
+                            break Err(MonitorUnexpectedlyStopped);
                         }
                     }
                 }
diff --git a/test/test-manager/src/tests/helpers.rs b/test/test-manager/src/tests/helpers.rs
index 3652d1f15a..2dff6eccdd 100644
--- a/test/test-manager/src/tests/helpers.rs
+++ b/test/test-manager/src/tests/helpers.rs
@@ -5,13 +5,17 @@ use crate::network_monitor::{
 use anyhow::{anyhow, bail, ensure, Context};
 use futures::StreamExt;
 use mullvad_management_interface::{client::DaemonEvent, MullvadProxyClient};
+use mullvad_relay_selector::{
+    query::RelayQuery, GetRelay, RelaySelector, SelectorConfig, WireguardConfig,
+};
 use mullvad_types::{
     constraints::Constraint,
     location::Location,
     relay_constraints::{
-        BridgeSettings, GeographicLocationConstraint, LocationConstraint, RelaySettings,
+        BridgeSettings, GeographicLocationConstraint, LocationConstraint, RelayConstraints,
+        RelaySettings,
     },
-    relay_list::{Relay, RelayList},
+    relay_list::Relay,
     states::TunnelState,
 };
 use pcap::Direction;
@@ -499,27 +503,49 @@ pub fn get_app_env() -> HashMap<String, String> {
     ])
 }
 
-/// Return a filtered version of the daemon's relay list.
+/// Constrain the daemon to only select the relay selected with `query` when establishing all
+/// future tunnels (until relay settings are updated, see [`set_relay_settings`]). Returns the
+/// selected [`Relay`] for future reference.
 ///
-/// * `mullvad_client` - An interface to the Mullvad daemon.
-/// * `critera` - A function used to determine which relays to return.
-pub async fn filter_relays<Filter>(
+/// # 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 :).
+pub async fn constrain_to_relay(
     mullvad_client: &mut MullvadProxyClient,
-    criteria: Filter,
-) -> Result<Vec<Relay>, Error>
-where
-    Filter: Fn(&Relay) -> bool,
-{
-    let relay_list: RelayList = mullvad_client
-        .get_relay_locations()
-        .await
-        .map_err(|error| Error::Daemon(format!("Failed to obtain relay list: {}", error)))?;
+    query: RelayQuery,
+) -> anyhow::Result<Relay> {
+    /// Convert the result of invoking the relay selector to a relay constraint.
+    fn convert_to_relay_constraints(
+        selected_relay: GetRelay,
+    ) -> anyhow::Result<(Relay, RelayConstraints)> {
+        match selected_relay {
+            GetRelay::Wireguard {
+                inner: WireguardConfig::Singlehop { exit },
+                ..
+            }
+            | GetRelay::OpenVpn { exit, .. } => {
+                let location = into_constraint(&exit)?;
+                let relay_constraints = RelayConstraints {
+                    location,
+                    ..Default::default()
+                };
+                Ok((exit, relay_constraints))
+            }
+            unsupported => bail!("Can not constrain to a {unsupported:?}"),
+        }
+    }
+
+    // Construct a relay selector with up-to-date information from the runnin daemon's relay list
+    let relay_list = mullvad_client.get_relay_locations().await?;
+    let relay_selector = RelaySelector::from_list(SelectorConfig::default(), relay_list);
+    // Select an(y) appropriate relay for the given query and constrain the daemon to only connect
+    // to that specific relay (when connecting).
+    let relay = relay_selector.get_relay_by_query(query)?;
+    let (exit, relay_constraints) = convert_to_relay_constraints(relay)?;
+    set_relay_settings(mullvad_client, RelaySettings::Normal(relay_constraints)).await?;
 
-    Ok(relay_list
-        .relays()
-        .filter(|relay| criteria(relay))
-        .cloned()
-        .collect())
+    Ok(exit)
 }
 
 /// Convenience function for constructing a constraint from a given [`Relay`].
@@ -527,7 +553,7 @@ where
 /// # Panics
 ///
 /// The relay must have a location set.
-pub fn into_constraint(relay: &Relay) -> Constraint<LocationConstraint> {
+pub fn into_constraint(relay: &Relay) -> anyhow::Result<Constraint<LocationConstraint>> {
     relay
         .location
         .as_ref()
@@ -537,16 +563,12 @@ pub fn into_constraint(relay: &Relay) -> Constraint<LocationConstraint> {
                  city_code,
                  ..
              }| {
-                GeographicLocationConstraint::Hostname(
-                    country_code.to_string(),
-                    city_code.to_string(),
-                    relay.hostname.to_string(),
-                )
+                GeographicLocationConstraint::hostname(country_code, city_code, &relay.hostname)
             },
         )
         .map(LocationConstraint::Location)
         .map(Constraint::Only)
-        .expect("relay is missing location")
+        .ok_or(anyhow!("relay is missing location"))
 }
 
 /// Ping monitoring made easy!
diff --git a/test/test-manager/src/tests/install.rs b/test/test-manager/src/tests/install.rs
index cb1ddf58c5..f0e0942af7 100644
--- a/test/test-manager/src/tests/install.rs
+++ b/test/test-manager/src/tests/install.rs
@@ -281,6 +281,7 @@ pub async fn test_installation_idempotency(
     // Connect to any relay. This forces the daemon to enter a secured target state
     connect_and_wait(&mut mullvad_client)
         .await
+        .map(|_| ()) // Discard the new tunnel state
         .or_else(|error| match error {
             Error::UnexpectedErrorState(_) => Ok(()),
             err => Err(err),
diff --git a/test/test-manager/src/tests/tunnel.rs b/test/test-manager/src/tests/tunnel.rs
index a0acbf2114..594c930ef0 100644
--- a/test/test-manager/src/tests/tunnel.rs
+++ b/test/test-manager/src/tests/tunnel.rs
@@ -6,10 +6,11 @@ use super::{
     Error, TestContext,
 };
 use crate::{
-    network_monitor::{start_packet_monitor, MonitorOptions},
-    tests::helpers::login_with_retries,
+    network_monitor::{start_packet_monitor, MonitorOptions, ParsedPacket},
+    tests::helpers::{login_with_retries, ConnChecker},
 };
 
+use anyhow::{bail, ensure};
 use mullvad_management_interface::MullvadProxyClient;
 use mullvad_relay_selector::query::builder::RelayQueryBuilder;
 use mullvad_types::{
@@ -19,6 +20,7 @@ use mullvad_types::{
         RelaySettings, SelectedObfuscation, TransportPort, Udp2TcpObfuscationSettings,
         WireguardConstraints,
     },
+    states::TunnelState,
     wireguard,
 };
 use std::net::SocketAddr;
@@ -787,3 +789,86 @@ 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
+    // FIXME: This needs to be kept in sync with the magic payload string defined in `connection_cheker::net`.
+    // An easy fix would be to make the payload for `ConnCheck` configurable.
+    let unique_identifier = b"Hello there!";
+    let identify_rogue_packet = move |packet: &ParsedPacket| {
+        packet
+            .payload
+            .windows(unique_identifier.len())
+            .any(|window| window == unique_identifier)
+    };
+    let rogue_packet_monitor =
+        start_packet_monitor(identify_rogue_packet, MonitorOptions::default()).await;
+
+    // Step 3 - Start the rogue program which will try to leak traffic to the chosen relay endpoint
+    let mut checker = ConnChecker::new(rpc.clone(), mullvad_client.clone(), target_endpoint);
+    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.unwrap();
+
+    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"
+    );
+
+    // 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(())
+}
diff --git a/test/test-manager/src/tests/tunnel_state.rs b/test/test-manager/src/tests/tunnel_state.rs
index 5e69b33b7a..edbe595092 100644
--- a/test/test-manager/src/tests/tunnel_state.rs
+++ b/test/test-manager/src/tests/tunnel_state.rs
@@ -11,12 +11,12 @@ use crate::{
 };
 
 use mullvad_management_interface::MullvadProxyClient;
+use mullvad_relay_selector::query::builder::RelayQueryBuilder;
 use mullvad_types::{
     constraints::Constraint,
     relay_constraints::{
         GeographicLocationConstraint, LocationConstraint, RelayConstraints, RelaySettings,
     },
-    relay_list::{Relay, RelayEndpointData},
     states::TunnelState,
     CustomTunnelEndpoint,
 };
@@ -340,40 +340,21 @@ pub async fn test_connected_state(
     _: TestContext,
     rpc: ServiceClient,
     mut mullvad_client: MullvadProxyClient,
-) -> Result<(), Error> {
+) -> anyhow::Result<()> {
     let inet_destination = "1.1.1.1:1337".parse().unwrap();
 
     // Set relay to use
-    //
-
     log::info!("Select relay");
-
-    let relay_filter = |relay: &Relay| {
-        relay.active && matches!(relay.endpoint_data, RelayEndpointData::Wireguard(_))
-    };
-
-    let relay = helpers::filter_relays(&mut mullvad_client, relay_filter)
-        .await?
-        .pop()
-        .unwrap();
-
-    let relay_settings = RelaySettings::Normal(RelayConstraints {
-        location: helpers::into_constraint(&relay),
-        ..Default::default()
-    });
-
-    set_relay_settings(&mut mullvad_client, relay_settings)
-        .await
-        .expect("failed to update relay settings");
+    let relay = helpers::constrain_to_relay(
+        &mut mullvad_client,
+        RelayQueryBuilder::new().wireguard().build(),
+    )
+    .await?;
 
     // Connect
-    //
-
     connect_and_wait(&mut mullvad_client).await?;
 
     // Verify that endpoint was selected
-    //
-
     match mullvad_client.get_tunnel_state().await? {
         TunnelState::Connected {
             endpoint:
diff --git a/test/test-manager/src/tests/ui.rs b/test/test-manager/src/tests/ui.rs
index ebd6fb3ee9..cce7cdd990 100644
--- a/test/test-manager/src/tests/ui.rs
+++ b/test/test-manager/src/tests/ui.rs
@@ -1,9 +1,6 @@
 use super::{config::TEST_CONFIG, helpers, Error, TestContext};
 use mullvad_management_interface::MullvadProxyClient;
-use mullvad_types::{
-    relay_constraints::{RelayConstraints, RelaySettings},
-    relay_list::{Relay, RelayEndpointData},
-};
+use mullvad_relay_selector::query::builder::RelayQueryBuilder;
 use std::{
     collections::BTreeMap,
     fmt::Debug,
@@ -85,25 +82,14 @@ pub async fn test_ui_tunnel_settings(
     _: TestContext,
     rpc: ServiceClient,
     mut mullvad_client: MullvadProxyClient,
-) -> Result<(), Error> {
+) -> anyhow::Result<()> {
     // tunnel-state.spec precondition: a single WireGuard relay should be selected
     log::info!("Select WireGuard relay");
-    let entry = helpers::filter_relays(&mut mullvad_client, |relay: &Relay| {
-        relay.active && matches!(relay.endpoint_data, RelayEndpointData::Wireguard(_))
-    })
-    .await?
-    .pop()
-    .unwrap();
-
-    // The test expects us to be disconnected and logged in but to have a specific relay selected
-    let relay_settings = RelaySettings::Normal(RelayConstraints {
-        location: helpers::into_constraint(&entry),
-        ..Default::default()
-    });
-
-    helpers::set_relay_settings(&mut mullvad_client, relay_settings)
-        .await
-        .expect("failed to update relay settings");
+    let entry = helpers::constrain_to_relay(
+        &mut mullvad_client,
+        RelayQueryBuilder::new().wireguard().build(),
+    )
+    .await?;
 
     let ui_result = run_test_env(
         &rpc,