From 6fa34faa00229306d3a2fd84b664570b0327d61e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20L=C3=B6nnhager?= Date: Thu, 3 Oct 2024 12:53:09 +0200 Subject: [PATCH 1/3] Do not duplicate incoming ICMP --- talpid-core/src/split_tunnel/macos/tun.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/talpid-core/src/split_tunnel/macos/tun.rs b/talpid-core/src/split_tunnel/macos/tun.rs index 0e260a151762..2482fadcb7b0 100644 --- a/talpid-core/src/split_tunnel/macos/tun.rs +++ b/talpid-core/src/split_tunnel/macos/tun.rs @@ -19,10 +19,11 @@ use pnet_packet::{ udp::MutableUdpPacket, MutablePacket, Packet, }; +use talpid_types::net::{ALLOWED_LAN_NETS, ALLOWED_LAN_MULTICAST_NETS}; use std::{ ffi::{c_uint, CStr}, io::{self, IoSlice, Write}, - net::{Ipv4Addr, Ipv6Addr}, + net::{Ipv4Addr, Ipv6Addr, IpAddr}, }; use talpid_routing::RouteManagerHandle; use tokio::{ @@ -676,6 +677,9 @@ async fn handle_incoming_data_v4( log::trace!("Dropping packet to VPN IP on default interface"); return; } + if is_non_vpn_destination(IpAddr::from(ip.get_destination())) { + return; + } fix_ipv4_checksums(&mut ip, None, Some(vpn_addr)); @@ -698,6 +702,9 @@ async fn handle_incoming_data_v6( log::trace!("Dropping packet to VPN IP on default interface"); return; } + if is_non_vpn_destination(IpAddr::from(ip.get_destination())) { + return; + } fix_ipv6_checksums(&mut ip, None, Some(vpn_addr)); @@ -710,6 +717,15 @@ async fn handle_incoming_data_v6( } } +/// Packets routed outside of the split tunneling interface should not be duplicated on the VPN +/// utun. As a shortcut we do not duplicate any private IPs. +fn is_non_vpn_destination(ip: IpAddr) -> bool { + ALLOWED_LAN_NETS + .iter() + .chain(ALLOWED_LAN_MULTICAST_NETS.iter()) + .any(|net| net.contains(ip)) +} + // Recalculate L3 and L4 checksums. Silently fail on error fn fix_ipv4_checksums( ip: &mut MutableIpv4Packet<'_>, From 3f095b37e228bc729195699a7ab081b0016ec1e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20L=C3=B6nnhager?= Date: Fri, 4 Oct 2024 10:28:05 +0200 Subject: [PATCH 2/3] Do not kill states for allowed endpoint --- talpid-core/src/firewall/macos.rs | 9 +++++++++ talpid-core/src/firewall/mod.rs | 14 ++++++++++++++ talpid-core/src/split_tunnel/macos/tun.rs | 14 +++++++------- 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/talpid-core/src/firewall/macos.rs b/talpid-core/src/firewall/macos.rs index 5f674f2935c8..73308d8dd923 100644 --- a/talpid-core/src/firewall/macos.rs +++ b/talpid-core/src/firewall/macos.rs @@ -119,6 +119,15 @@ impl Firewall { } } + if let Some(endpoint) = policy.allowed_endpoint() { + // Keep states to the allowed endpoint. + // Note that we're not taking into account allowed clients here, because it's highly + // impractical. + if endpoint.endpoint.address == remote_address { + return Ok(false); + } + } + let Some(peer) = policy.peer_endpoint().map(|endpoint| endpoint.endpoint) else { // If there's no peer, there's also no tunnel. We have no states to preserve return Ok(true); diff --git a/talpid-core/src/firewall/mod.rs b/talpid-core/src/firewall/mod.rs index 17f74b17eec9..09349728092b 100644 --- a/talpid-core/src/firewall/mod.rs +++ b/talpid-core/src/firewall/mod.rs @@ -143,6 +143,20 @@ impl FirewallPolicy { } } + /// Return the allowed endpoint, if available + pub fn allowed_endpoint(&self) -> Option<&AllowedEndpoint> { + match self { + FirewallPolicy::Connecting { + allowed_endpoint, .. + } + | FirewallPolicy::Blocked { + allowed_endpoint: Some(allowed_endpoint), + .. + } => Some(allowed_endpoint), + _ => None, + } + } + /// Return tunnel metadata, if available pub fn tunnel(&self) -> Option<&crate::tunnel::TunnelMetadata> { match self { diff --git a/talpid-core/src/split_tunnel/macos/tun.rs b/talpid-core/src/split_tunnel/macos/tun.rs index 2482fadcb7b0..90581c736cc0 100644 --- a/talpid-core/src/split_tunnel/macos/tun.rs +++ b/talpid-core/src/split_tunnel/macos/tun.rs @@ -19,13 +19,13 @@ use pnet_packet::{ udp::MutableUdpPacket, MutablePacket, Packet, }; -use talpid_types::net::{ALLOWED_LAN_NETS, ALLOWED_LAN_MULTICAST_NETS}; use std::{ ffi::{c_uint, CStr}, io::{self, IoSlice, Write}, - net::{Ipv4Addr, Ipv6Addr, IpAddr}, + net::{IpAddr, Ipv4Addr, Ipv6Addr}, }; use talpid_routing::RouteManagerHandle; +use talpid_types::net::{ALLOWED_LAN_MULTICAST_NETS, ALLOWED_LAN_NETS}; use tokio::{ io::{AsyncReadExt, AsyncWriteExt}, sync::broadcast, @@ -677,7 +677,8 @@ async fn handle_incoming_data_v4( log::trace!("Dropping packet to VPN IP on default interface"); return; } - if is_non_vpn_destination(IpAddr::from(ip.get_destination())) { + if is_private_ip(IpAddr::from(ip.get_source())) { + // Drop packets from private IPs return; } @@ -702,7 +703,8 @@ async fn handle_incoming_data_v6( log::trace!("Dropping packet to VPN IP on default interface"); return; } - if is_non_vpn_destination(IpAddr::from(ip.get_destination())) { + if is_private_ip(IpAddr::from(ip.get_source())) { + // Drop packets from private IPs return; } @@ -717,9 +719,7 @@ async fn handle_incoming_data_v6( } } -/// Packets routed outside of the split tunneling interface should not be duplicated on the VPN -/// utun. As a shortcut we do not duplicate any private IPs. -fn is_non_vpn_destination(ip: IpAddr) -> bool { +fn is_private_ip(ip: IpAddr) -> bool { ALLOWED_LAN_NETS .iter() .chain(ALLOWED_LAN_MULTICAST_NETS.iter()) From fa2c577754af0a3e8218f52d94cd75e1e112e726 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20L=C3=B6nnhager?= Date: Thu, 3 Oct 2024 14:18:46 +0200 Subject: [PATCH 3/3] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 610890b2456d..5e7cbb2bbafc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ Line wrap the file at 100 chars. Th #### macOS - Fix Apple services not working by forcing stray connections out through the VPN tunnel. The "bypass" toggle has been removed. +- Fix packets being duplicated on LAN when split tunneling is enabled. ## [2024.6-beta1] - 2024-09-26