diff options
| author | David Lönnhager <david.l@mullvad.net> | 2024-09-03 17:34:00 +0200 |
|---|---|---|
| committer | David Lönnhager <david.l@mullvad.net> | 2024-09-04 17:00:03 +0200 |
| commit | bb90f4820be908970f980750cf36178f83efdffa (patch) | |
| tree | 5cf548ac35e1f678c4d3dc8a41b1f067de43818e | |
| parent | a9ff9d512becc85d2607948e4517c1ae6a680dec (diff) | |
| download | mullvadvpn-bb90f4820be908970f980750cf36178f83efdffa.tar.xz mullvadvpn-bb90f4820be908970f980750cf36178f83efdffa.zip | |
Filter only on reassembled packets in PF
This fixes an issue of fragments being blocked by PF, causing
instability and timeouts
| -rw-r--r-- | Cargo.lock | 4 | ||||
| -rw-r--r-- | talpid-core/Cargo.toml | 2 | ||||
| -rw-r--r-- | talpid-core/src/firewall/macos.rs | 16 |
3 files changed, 18 insertions, 4 deletions
diff --git a/Cargo.lock b/Cargo.lock index ec64baf113..e75f8957f6 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 a5a25aad59..2390f107f6 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 51ae7ef708..60cecf11de 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<Vec<pfctl::ScrubRule>> { + // 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(()) } } |