Merge branch 'stop-arping-leaking-in-tunnel-ip-on-linux-des-1434'

CHANGELOG.md

@@ -58,6 +58,10 @@ Line wrap the file at 100 chars.                                              Th
   leak (as easily) when the VPN tunnel is up. Previously, WSL would leak while in the blocked or
   connecting state, or while lockdown mode was active.
 
+#### Linux
+- Prevent attackers able to send ARP requests to the device running Mullvad from figuring out
+  the in-tunnel IP. Fixes 2024 audit issue `MLLVD-CR-24-03`.
+
 
 ## [2024.7] - 2024-10-30
 This release is identical to 2024.7-beta1.

README.md

@@ -136,12 +136,20 @@ See [this](Release.md) for instructions on how to make a new release.
     * Set to `"pass"` to add logging to rules allowing packets.
     * Set to `"drop"` to add logging to rules blocking packets.
 
-* `TALPID_FIREWALL_DONT_SET_SRC_VALID_MARK` - Forces the daemon to not set `src_valid_mark` config
-    on Linux. The kernel config option is set because otherwise strict reverse path filtering may
-    prevent relay traffic from reaching the daemon. If `rp_filter` is set to `1` on the interface
+* `TALPID_FIREWALL_DONT_SET_SRC_VALID_MARK` - Set this variable to `1` to stop the daemon from
+    setting the `net.ipv4.conf.all.src_valid_mark` kernel parameter to `1` on Linux when a tunnel
+    is established.
+    The kernel config parameter is set by default, because otherwise strict reverse path filtering
+    may prevent relay traffic from reaching the daemon. If `rp_filter` is set to `1` on the interface
     that will be receiving relay traffic, and `src_valid_mark` is not set to `1`, the daemon will
     not be able to receive relay traffic.
 
+* `TALPID_FIREWALL_DONT_SET_ARP_IGNORE` - Set this variable to `1` to stop the daemon from
+    setting the `net.ipv4.conf.all.arp_ignore` kernel parameter to `2` on Linux when a tunnel
+    is established.
+    The kernel config parameter is set by default, because otherwise an attacker who can send ARP
+    requests to the device running Mullvad can figure out the in-tunnel IP.
+
 * `TALPID_DNS_MODULE` - Allows changing the method that will be used for DNS configuration.
   By default this is automatically detected, but you can set it to one of the options below to
   choose a specific method.

talpid-core/src/firewall/linux.rs

@@ -21,6 +21,7 @@ use talpid_types::net::{
 const MANGLE_CHAIN_PRIORITY: i32 = libc::NF_IP_PRI_MANGLE;
 const PREROUTING_CHAIN_PRIORITY: i32 = libc::NF_IP_PRI_CONNTRACK + 1;
 const PROC_SYS_NET_IPV4_CONF_SRC_VALID_MARK: &str = "/proc/sys/net/ipv4/conf/all/src_valid_mark";
+const PROC_SYS_NET_IPV4_CONF_ARP_IGNORE: &str = "/proc/sys/net/ipv4/conf/all/arp_ignore";
 
 pub type Result<T> = std::result::Result<T, Error>;
 
@@ -76,6 +77,11 @@ static DONT_SET_SRC_VALID_MARK: LazyLock<bool> = LazyLock::new(|| {
         .map(|v| v != "0")
         .unwrap_or(false)
 });
+static DONT_SET_ARP_IGNORE: LazyLock<bool> = LazyLock::new(|| {
+    env::var("TALPID_FIREWALL_DONT_SET_ARP_IGNORE")
+        .map(|v| v != "0")
+        .unwrap_or(false)
+});
 
 #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
 enum Direction {
@@ -134,14 +140,29 @@ impl Firewall {
     fn apply_kernel_config(policy: &FirewallPolicy) {
         if *DONT_SET_SRC_VALID_MARK {
             log::debug!("Not setting src_valid_mark");
-            return;
-        }
-
-        if let FirewallPolicy::Connecting { .. } = policy {
+        } else if let FirewallPolicy::Connecting { .. } = policy {
             if let Err(err) = set_src_valid_mark_sysctl() {
                 log::error!("Failed to apply src_valid_mark: {}", err);
             }
         }
+
+        // When we have a tunnel with an IP configured, we configure the system
+        // to not reply to arp requests for this tunnel IP *on other interfaces*.
+        // By default, Linux responds to incoming arp requests for any IP configured on any
+        // interface on the system. This makes it possible to via ARP-pinging figure out the
+        // VPN in-tunnel IP by spamming ARP requests to any physical interface on the device.
+        //
+        // We never store the initial value and restore it. We deem the default value
+        // to be too relaxed and don't see any reason why anyone would want a more relaxed
+        // setting than the one we are setting here.
+        if *DONT_SET_ARP_IGNORE {
+            log::debug!("Not setting arp_ignore");
+        } else if let FirewallPolicy::Connecting { .. } | FirewallPolicy::Connected { .. } = policy
+        {
+            if let Err(err) = lock_down_arp_ignore_sysctl() {
+                log::error!("Failed to apply arp_ignore: {}", err);
+            }
+        }
     }
 
     fn send_and_process(batch: &FinalizedBatch) -> Result<()> {
@@ -1054,6 +1075,21 @@ fn set_src_valid_mark_sysctl() -> io::Result<()> {
     fs::write(PROC_SYS_NET_IPV4_CONF_SRC_VALID_MARK, b"1")
 }
 
+/// If the `net.ipv4.conf.all.arp_ignore` setting is below 2, sets it to 2.
+///
+/// 2 means: reply only if the target IP address is local address configured on the incoming
+/// interface and both with the sender's IP address are part from same subnet on this interface.
+fn lock_down_arp_ignore_sysctl() -> io::Result<()> {
+    // Should be safe to treat the content as a string, since it should always be a number.
+    let current_arp_ignore = fs::read_to_string(PROC_SYS_NET_IPV4_CONF_ARP_IGNORE)?;
+    match current_arp_ignore.trim() {
+        "0" | "1" => fs::write(PROC_SYS_NET_IPV4_CONF_ARP_IGNORE, b"2")?,
+        "2" => (),
+        _ => log::trace!("Not locking down arp_ignore since it is set to {current_arp_ignore}"),
+    }
+    Ok(())
+}
+
 /// Tables that are no longer used but need to be deleted due to upgrades.
 /// This can be removed when upgrades from 2023.3 are no longer supported.
 fn batch_deprecated_tables(batch: &mut Batch) {