Add NoNat action

mullvad · Oct 1, 2024 · 678ee52 · 678ee52
1 parent d5f127e
commit 678ee52
Show file tree

Hide file tree

Showing 7 changed files with 102 additions and 34 deletions.
diff --git a/src/lib.rs b/src/lib.rs
@@ -366,15 +366,17 @@ impl PfCtl {
         utils::copy_anchor_name(anchor, &mut pfioc_rule.anchor[..])?;
         rule.try_copy_to(&mut pfioc_rule.rule)?;
 
-        // register NAT address in newly created address pool
-        let nat_to = rule.get_nat_to();
         let pool_ticket = utils::get_pool_ticket(self.fd())?;
-        utils::add_pool_address(self.fd(), nat_to.ip(), pool_ticket)?;
 
-        // copy address pool in pf_rule
-        let nat_pool = nat_to.ip().to_pool_addr_list()?;
-        pfioc_rule.rule.rpool.list = unsafe { nat_pool.to_palist() };
-        nat_to.port().try_copy_to(&mut pfioc_rule.rule.rpool)?;
+        if let Some(nat_to) = rule.get_nat_to() {
+            // register NAT address in newly created address pool
+            utils::add_pool_address(self.fd(), nat_to.ip(), pool_ticket)?;
+
+            // copy address pool in pf_rule
+            let nat_pool = nat_to.ip().to_pool_addr_list()?;
+            pfioc_rule.rule.rpool.list = unsafe { nat_pool.to_palist() };
+            nat_to.port().try_copy_to(&mut pfioc_rule.rule.rpool)?;
+        }
 
         // set tickets
         pfioc_rule.pool_ticket = pool_ticket;

diff --git a/src/rule/mod.rs b/src/rule/mod.rs
@@ -11,7 +11,10 @@ use crate::{
     ffi, Error, ErrorInternal, Result,
 };
 use ipnetwork::IpNetwork;
-use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
+use std::{
+    net::{IpAddr, Ipv4Addr, Ipv6Addr},
+    ops::Deref,
+};
 
 mod addr_family;
 pub use self::addr_family::*;
@@ -172,27 +175,41 @@ pub struct NatRule {
     from: Endpoint,
     #[builder(default)]
     to: Endpoint,
-    nat_to: NatEndpoint,
 }
 
 impl NatRule {
     /// Returns the `AddrFamily` this rule matches against. Returns an `InvalidRuleCombination`
     /// error if this rule has an invalid combination of address families.
     fn get_af(&self) -> Result<AddrFamily> {
         let endpoint_af = compatible_af(self.from.get_af(), self.to.get_af())?;
-        let nat_af = compatible_af(endpoint_af, self.nat_to.0.get_af())?;
-        compatible_af(self.af, nat_af)
+        if let Some(nat_to) = self.get_nat_to() {
+            let nat_af = compatible_af(endpoint_af, nat_to.0.get_af())?;
+            compatible_af(self.af, nat_af)
+        } else {
+            compatible_af(self.af, endpoint_af)
+        }
     }
 
     /// Accessor for `nat_to`
-    pub fn get_nat_to(&self) -> Endpoint {
-        self.nat_to.0
+    pub fn get_nat_to(&self) -> Option<NatEndpoint> {
+        match self.action {
+            NatRuleAction::Nat { nat_to } => Some(nat_to),
+            NatRuleAction::NoNat => None,
+        }
     }
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct NatEndpoint(Endpoint);
 
+impl Deref for NatEndpoint {
+    type Target = Endpoint;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
 impl From<Ip> for NatEndpoint {
     fn from(ip: Ip) -> Self {
         // Default NAT port range

diff --git a/src/rule/rule_action.rs b/src/rule/rule_action.rs
@@ -6,7 +6,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use crate::ffi;
+use crate::{ffi, NatEndpoint};
 
 /// Enum describing what should happen to a packet that matches a filter rule.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
@@ -61,13 +61,15 @@ impl From<DropAction> for u32 {
 /// Enum describing what should happen to a packet that matches a NAT rule.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub enum NatRuleAction {
-    Nat,
+    Nat { nat_to: NatEndpoint },
+    NoNat,
 }
 
 impl From<NatRuleAction> for u8 {
     fn from(rule_action: NatRuleAction) -> Self {
         match rule_action {
-            NatRuleAction::Nat => ffi::pfvar::PF_NAT as u8,
+            NatRuleAction::Nat { .. } => ffi::pfvar::PF_NAT as u8,
+            NatRuleAction::NoNat => ffi::pfvar::PF_NONAT as u8,
         }
     }
 }

diff --git a/src/transaction.rs b/src/transaction.rs
@@ -198,15 +198,17 @@ impl Transaction {
         utils::copy_anchor_name(anchor, &mut pfioc_rule.anchor[..])?;
         rule.try_copy_to(&mut pfioc_rule.rule)?;
 
-        // register NAT address in newly created address pool
-        let nat_to = rule.get_nat_to();
         let pool_ticket = utils::get_pool_ticket(fd)?;
-        utils::add_pool_address(fd, nat_to.ip(), pool_ticket)?;
 
-        // copy address pool in pf_rule
-        let nat_pool = nat_to.ip().to_pool_addr_list()?;
-        pfioc_rule.rule.rpool.list = unsafe { nat_pool.to_palist() };
-        nat_to.port().try_copy_to(&mut pfioc_rule.rule.rpool)?;
+        if let Some(nat_to) = rule.get_nat_to() {
+            // register NAT address in newly created address pool
+            utils::add_pool_address(fd, nat_to.ip(), pool_ticket)?;
+
+            // copy address pool in pf_rule
+            let nat_pool = nat_to.ip().to_pool_addr_list()?;
+            pfioc_rule.rule.rpool.list = unsafe { nat_pool.to_palist() };
+            nat_to.port().try_copy_to(&mut pfioc_rule.rule.rpool)?;
+        }
 
         // set tickets
         pfioc_rule.pool_ticket = pool_ticket;

diff --git a/tests/helper/pfcli.rs b/tests/helper/pfcli.rs
@@ -16,7 +16,10 @@ pub fn is_enabled() -> bool {
         false
     } else {
         let stderr = str_from_stdout(&output.stderr);
-        panic!("Invalid output from pfctl ({}), stdout:\n{str}\nstderr:\n{stderr}", output.status);
+        panic!(
+            "Invalid output from pfctl ({}), stdout:\n{str}\nstderr:\n{stderr}",
+            output.status
+        );
     }
 }
 

diff --git a/tests/nat_rules.rs b/tests/nat_rules.rs
@@ -4,19 +4,16 @@ mod helper;
 
 use crate::helper::pfcli;
 use assert_matches::assert_matches;
-use pfctl::Port;
 use std::net::{Ipv4Addr, Ipv6Addr};
 
 static ANCHOR_NAME: &str = "pfctl-rs.integration.testing.nat-rules";
 
 fn nat_rule(dest: pfctl::Ip, nat_to: pfctl::Ip) -> pfctl::NatRule {
     pfctl::NatRuleBuilder::default()
-        .action(pfctl::NatRuleAction::Nat)
+        .action(pfctl::NatRuleAction::Nat {
+            nat_to: nat_to.into(),
+        })
         .to(pfctl::Endpoint::new(dest, 1234))
-        .nat_to(pfctl::Endpoint::new(
-            nat_to,
-            Port::Range(32768, 49151, pfctl::PortRangeModifier::Inclusive),
-        ))
         .build()
         .unwrap()
 }
@@ -35,6 +32,22 @@ fn nat_rule_ipv6() -> pfctl::NatRule {
     )
 }
 
+fn nonat_rule(dest: pfctl::Ip) -> pfctl::NatRule {
+    pfctl::NatRuleBuilder::default()
+        .action(pfctl::NatRuleAction::NoNat)
+        .to(pfctl::Endpoint::new(dest, 1234))
+        .build()
+        .unwrap()
+}
+
+fn nonat_rule_ipv4() -> pfctl::NatRule {
+    nonat_rule(pfctl::Ip::from(Ipv4Addr::new(127, 0, 0, 1)))
+}
+
+fn nonat_rule_ipv6() -> pfctl::NatRule {
+    nonat_rule(pfctl::Ip::from(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 1)))
+}
+
 fn before_each() {
     pfctl::PfCtl::new()
         .unwrap()
@@ -69,3 +82,23 @@ test!(add_nat_rule_ipv6 {
         &["nat inet6 from any to ::1 port = 1234 -> ::2"]
     );
 });
+
+test!(add_nonat_rule_ipv4 {
+    let mut pf = pfctl::PfCtl::new().unwrap();
+    let rule = nonat_rule_ipv4();
+    assert_matches!(pf.add_nat_rule(ANCHOR_NAME, &rule), Ok(()));
+    assert_eq!(
+        pfcli::get_nat_rules(ANCHOR_NAME),
+        &["no nat inet from any to 127.0.0.1 port = 1234"]
+    );
+});
+
+test!(add_nonat_rule_ipv6 {
+    let mut pf = pfctl::PfCtl::new().unwrap();
+    let rule = nonat_rule_ipv6();
+    assert_matches!(pf.add_nat_rule(ANCHOR_NAME, &rule), Ok(()));
+    assert_eq!(
+        pfcli::get_nat_rules(ANCHOR_NAME),
+        &["no nat inet6 from any to ::1 port = 1234"]
+    );
+});
diff --git a/tests/transaction.rs b/tests/transaction.rs
@@ -68,12 +68,18 @@ fn get_filter_rules() -> Vec<pfctl::FilterRule> {
 
 fn get_nat_rules() -> Vec<pfctl::NatRule> {
     let rule1 = pfctl::NatRuleBuilder::default()
-        .action(pfctl::NatRuleAction::Nat)
+        .action(pfctl::NatRuleAction::Nat {
+            nat_to: Ipv4Addr::new(127, 0, 0, 1).into(),
+        })
         .to(Ipv4Addr::new(1, 2, 3, 4))
-        .nat_to(Ipv4Addr::new(127, 0, 0, 1))
         .build()
         .unwrap();
-    vec![rule1]
+    let rule2 = pfctl::NatRuleBuilder::default()
+        .action(pfctl::NatRuleAction::NoNat)
+        .to(Ipv4Addr::new(1, 3, 3, 7))
+        .build()
+        .unwrap();
+    vec![rule1, rule2]
 }
 
 fn get_redirect_rules() -> Vec<pfctl::RedirectRule> {
@@ -160,7 +166,10 @@ fn verify_redirect_rules(anchor: &str) {
 fn verify_nat_rules(anchor: &str) {
     assert_eq!(
         get_nat_rules_filtered(anchor, |rule| rule.contains("nat")),
-        &["nat inet from any to 1.2.3.4 -> 127.0.0.1",]
+        &[
+            "nat inet from any to 1.2.3.4 -> 127.0.0.1",
+            "no nat inet from any to 1.3.3.7",
+        ]
     );
 }