fix ACL to firewall rule translation for IPv4-only or IPv6-only destinations

Cargo.toml

@@ -11,7 +11,7 @@ resolver = "2"
 
 [workspace.dependencies]
 # internal crates
-defguard_common = { path = "./crates/defguard_common", version = "1.5.2" }
+defguard_common = { path = "./crates/defguard_common", version = "1.6.2" }
 defguard_core = { path = "./crates/defguard_core", version = "0.0.0" }
 defguard_event_logger = { path = "./crates/defguard_event_logger", version = "0.0.0" }
 defguard_event_router = { path = "./crates/defguard_event_router", version = "0.0.0" }

crates/defguard_common/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "defguard_common"
-version = "1.6.1"
+version = "1.6.2"
 edition.workspace = true
 license-file.workspace = true
 homepage.workspace = true

crates/defguard_core/src/enterprise/db/models/acl.rs

@@ -179,14 +179,37 @@ pub struct AclRuleInfo<I = NoId> {
     pub protocols: Vec<Protocol>,
 }
 
+/// Constructs a [`String`] of comma-separated addresses.
+fn format_destination(destination: &[IpNetwork]) -> String {
+    match destination {
+        [] => String::new(),
+        d => d
+            .iter()
+            .map(|a| {
+                let mut addr_string = a.to_string() + ", ";
+                if a.is_ipv4() {
+                    addr_string = addr_string.replace("/32", "");
+                };
+                addr_string
+            })
+            .collect::<String>(),
+    }
+}
+/// Constructs a [`String`] of comma-separated ports and port ranges.
+fn format_ports(ports: &[PortRange]) -> String {
+    ports
+        .iter()
+        .map(ToString::to_string)
+        .collect::<Vec<_>>()
+        .join(", ")
+}
+
 impl<I> AclRuleInfo<I> {
     /// Constructs a [`String`] of comma-separated addresses and address ranges.
     pub(crate) fn format_destination(&self) -> String {
-        // process single addresses
-        let addrs = match &self.destination {
-            d if d.is_empty() => String::new(),
-            d => d.iter().map(|a| a.to_string() + ", ").collect::<String>(),
-        };
+        // process single addresses and subnets
+        let addrs = format_destination(&self.destination);
+
         // process address ranges
         let ranges = match &self.destination_ranges {
             r if r.is_empty() => String::new(),
@@ -195,8 +218,8 @@ impl<I> AclRuleInfo<I> {
             }),
         };
 
-        // remove full mask from resulting string
-        let destination = (addrs + &ranges).replace("/32", "");
+        // combine resulting strings
+        let destination = addrs + &ranges;
         if destination.is_empty() {
             destination
         } else {
@@ -207,11 +230,7 @@ impl<I> AclRuleInfo<I> {
 
     /// Constructs a [`String`] of comma-separated ports and port ranges.
     pub(crate) fn format_ports(&self) -> String {
-        self.ports
-            .iter()
-            .map(ToString::to_string)
-            .collect::<Vec<_>>()
-            .join(", ")
+        format_ports(&self.ports)
     }
 }
 
@@ -1365,11 +1384,9 @@ pub struct AclAliasInfo<I = NoId> {
 impl<I> AclAliasInfo<I> {
     /// Constructs a [`String`] of comma-separated addresses and address ranges
     pub fn format_destination(&self) -> String {
-        // process single addresses
-        let addrs = match &self.destination {
-            d if d.is_empty() => String::new(),
-            d => d.iter().map(|a| a.to_string() + ", ").collect::<String>(),
-        };
+        // process single addresses and subnets
+        let addrs = format_destination(&self.destination);
+
         // process address ranges
         let ranges = match &self.destination_ranges {
             r if r.is_empty() => String::new(),
@@ -1378,8 +1395,8 @@ impl<I> AclAliasInfo<I> {
             }),
         };
 
-        // remove full mask from resulting string
-        let destination = (addrs + &ranges).replace("/32", "");
+        // combine resulting strings
+        let destination = addrs + &ranges;
         if destination.is_empty() {
             destination
         } else {
@@ -1390,11 +1407,7 @@ impl<I> AclAliasInfo<I> {
 
     /// Constructs a [`String`] of comma-separated ports and port ranges
     pub fn format_ports(&self) -> String {
-        self.ports
-            .iter()
-            .map(ToString::to_string)
-            .collect::<Vec<_>>()
-            .join(", ")
+        format_ports(&self.ports)
     }
 }
 

crates/defguard_core/src/enterprise/firewall/mod.rs

@@ -55,8 +55,8 @@ pub async fn generate_firewall_rules_from_acls(
     let location = WireguardNetwork::find_by_id(&mut *conn, location_id)
         .await?
         .ok_or(ModelError::NotFound)?;
-    let has_ipv4_addresses = location.address.iter().any(IpNetwork::is_ipv4);
-    let has_ipv6_addresses = location.address.iter().any(IpNetwork::is_ipv6);
+    let location_has_ipv4_addresses = location.address.iter().any(IpNetwork::is_ipv4);
+    let location_has_ipv6_addresses = location.address.iter().any(IpNetwork::is_ipv6);
 
     // convert each ACL into a corresponding `FirewallRule`s
     for acl in acl_rules {
@@ -148,16 +148,17 @@ pub async fn generate_firewall_rules_from_acls(
 
         // skip creating default firewall rules if given ACL includes only destination aliases and no manual destination config
         // at this point component aliases have been added to the manual config so they don't need to be handled separately
-        let has_no_manual_destination = dest_addrs_v4.is_empty()
-            && dest_addrs_v6.is_empty()
+        let has_v4_destination = !dest_addrs_v4.is_empty();
+        let has_v6_destination = !dest_addrs_v6.is_empty();
+        let has_no_manual_destination = !(has_v4_destination || has_v6_destination)
             && destination_ports.is_empty()
             && protocols.is_empty();
         let has_destination_aliases = !destination_aliases.is_empty();
         let is_destination_alias_only_rule = has_destination_aliases && has_no_manual_destination;
 
         if !is_destination_alias_only_rule {
             let comment = format!("ACL {} - {}", acl.id, acl.name);
-            if has_ipv4_addresses {
+            if location_has_ipv4_addresses && has_v4_destination {
                 // create IPv4 rules
                 let ipv4_rules = create_rules(
                     acl.id,
@@ -174,7 +175,7 @@ pub async fn generate_firewall_rules_from_acls(
                 deny_rules.push(ipv4_rules.1);
             }
 
-            if has_ipv6_addresses {
+            if location_has_ipv6_addresses && has_v6_destination {
                 // create IPv6 rules
                 let ipv6_rules = create_rules(
                     acl.id,
@@ -222,7 +223,7 @@ pub async fn generate_firewall_rules_from_acls(
                 "ACL {} - {}, ALIAS {} - {}",
                 acl.id, acl.name, alias.id, alias.name
             );
-            if has_ipv4_addresses {
+            if location_has_ipv4_addresses {
                 // create IPv4 rules
                 let ipv4_rules = create_rules(
                     alias.id,
@@ -239,7 +240,7 @@ pub async fn generate_firewall_rules_from_acls(
                 deny_rules.push(ipv4_rules.1);
             }
 
-            if has_ipv6_addresses {
+            if location_has_ipv6_addresses {
                 // create IPv6 rules
                 let ipv6_rules = create_rules(
                     alias.id,