From bb90f4820be908970f980750cf36178f83efdffa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20L=C3=B6nnhager?= Date: Tue, 3 Sep 2024 17:34:00 +0200 Subject: [PATCH 1/2] Filter only on reassembled packets in PF This fixes an issue of fragments being blocked by PF, causing instability and timeouts --- Cargo.lock | 4 ++-- talpid-core/Cargo.toml | 2 +- talpid-core/src/firewall/macos.rs | 16 +++++++++++++++- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ec64baf1138d..e75f8957f610 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2818,9 +2818,9 @@ dependencies = [ [[package]] name = "pfctl" -version = "0.5.0" +version = "0.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3eef3d868ab90cdd1a3814668982f248fbb8849922f2617142e398e7d037a81e" +checksum = "10824597de43012ff83ed8b044fd3aeeb3b93107e80303a86649c86742e27c5a" dependencies = [ "derive_builder", "ioctl-sys 0.8.0", diff --git a/talpid-core/Cargo.toml b/talpid-core/Cargo.toml index a5a25aad5934..2390f107f68f 100644 --- a/talpid-core/Cargo.toml +++ b/talpid-core/Cargo.toml @@ -47,7 +47,7 @@ duct = "0.13" [target.'cfg(target_os = "macos")'.dependencies] async-trait = "0.1" duct = "0.13" -pfctl = "0.5.0" +pfctl = "0.6.0" subslice = "0.2" system-configuration = "0.5.1" hickory-proto = "0.24.1" diff --git a/talpid-core/src/firewall/macos.rs b/talpid-core/src/firewall/macos.rs index 51ae7ef70843..60cecf11de06 100644 --- a/talpid-core/src/firewall/macos.rs +++ b/talpid-core/src/firewall/macos.rs @@ -154,11 +154,21 @@ impl Firewall { new_filter_rules.push(drop_all_rule); let mut anchor_change = pfctl::AnchorChange::new(); + anchor_change.set_scrub_rules(Self::get_scrub_rules()?); anchor_change.set_filter_rules(new_filter_rules); anchor_change.set_redirect_rules(self.get_dns_redirect_rules(policy)?); self.pf.set_rules(ANCHOR_NAME, anchor_change) } + fn get_scrub_rules() -> Result> { + // Filter only reassembled packets. Without this, PF will filter based on individual + // fragments, which may not have complete transport-layer headers. + let scrub_rule = pfctl::ScrubRuleBuilder::default() + .action(pfctl::ScrubRuleAction::Scrub) + .build()?; + Ok(vec![scrub_rule]) + } + fn get_dns_redirect_rules( &mut self, policy: &FirewallPolicy, @@ -773,14 +783,18 @@ impl Firewall { .try_add_anchor(ANCHOR_NAME, pfctl::AnchorKind::Filter)?; self.pf .try_add_anchor(ANCHOR_NAME, pfctl::AnchorKind::Redirect)?; + self.pf + .try_add_anchor(ANCHOR_NAME, pfctl::AnchorKind::Scrub)?; Ok(()) } fn remove_anchor(&mut self) -> Result<()> { self.pf - .try_remove_anchor(ANCHOR_NAME, pfctl::AnchorKind::Filter)?; + .try_remove_anchor(ANCHOR_NAME, pfctl::AnchorKind::Scrub)?; self.pf .try_remove_anchor(ANCHOR_NAME, pfctl::AnchorKind::Redirect)?; + self.pf + .try_remove_anchor(ANCHOR_NAME, pfctl::AnchorKind::Filter)?; Ok(()) } } From 5740b9f9dd6d63e42bc3ba05a41ac238fea04dea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20L=C3=B6nnhager?= Date: Tue, 3 Sep 2024 17:36:13 +0200 Subject: [PATCH 2/2] Update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b79e0e42308d..c0d2497f0711 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,8 @@ Line wrap the file at 100 chars. Th #### macOS - Exclude programs when executed using a relative path from a shell. - Reduce packet loss when using split tunneling. +- Don't block fragmented packets in the PF firewall. Fixes various issues relating to connecting + (and general instability) when IP fragmentation is present. ## [2024.5] - 2024-09-03