fix Allowed IPs validation & normalize Allowed IPs for routing configuration

new-ui/src/pages/full/OverviewPage/components/UpdateTunnelModal/UpdateTunnelModal.tsx

@@ -21,13 +21,10 @@ import { Snackbar } from '../../../../../shared/providers/snackbar/snackbar';
 import { api } from '../../../../../shared/rust-api/api';
 import { ThemeSpacing } from '../../../../../shared/types';
 import { isPresent } from '../../../../../shared/utils/isPresent';
-import {
-  patternValidIpV6WithMask,
-  patternValidIpWithMask,
-} from '../../../../../shared/utils/patterns';
 import {
   allowedIpsSchema,
   endpointSchema,
+  interfaceAddressesSchema,
   optionalWireguardKeySchema,
   wireguardKeySchema,
 } from '../../../../../shared/utils/zod';
@@ -67,15 +64,7 @@ export const UpdateTunnelModal = () => {
 
 const formSchema = z.object({
   name: z.string().trim().min(1, 'Field is required'),
-  address: z.string().refine((value) => {
-    if (!value) return false;
-    return value
-      .split(',')
-      .map((ip) => ip.trim())
-      .every(
-        (ip) => patternValidIpWithMask.test(ip) || patternValidIpV6WithMask.test(ip),
-      );
-  }, 'Field is invalid'),
+  address: interfaceAddressesSchema,
   prvkey: wireguardKeySchema,
   pubkey: wireguardKeySchema,
   server_pubkey: wireguardKeySchema,

new-ui/src/pages/full/TunnelWizardPage/steps/GeneralInformationStep/GeneralInformationStep.tsx

@@ -17,23 +17,12 @@ import { formChangeLogic } from '../../../../../shared/formLogic';
 import { Snackbar } from '../../../../../shared/providers/snackbar/snackbar';
 import { api } from '../../../../../shared/rust-api/api';
 import { ThemeSpacing } from '../../../../../shared/types';
-import {
-  patternValidIpV6WithMask,
-  patternValidIpWithMask,
-} from '../../../../../shared/utils/patterns';
+import { interfaceAddressesSchema } from '../../../../../shared/utils/zod';
 import { useTunnelWizardStore } from '../../hooks/useTunnelWizardStore';
 
 const formSchema = z.object({
   name: z.string().trim().min(1, 'Field is required'),
-  address: z.string().refine((value) => {
-    if (value) {
-      const ips = value.split(',').map((ip) => ip.trim());
-      return ips.every(
-        (ip) => patternValidIpWithMask.test(ip) || patternValidIpV6WithMask.test(ip),
-      );
-    }
-    return false;
-  }, 'Field is invalid'),
+  address: interfaceAddressesSchema,
 });
 
 type FormFields = z.infer<typeof formSchema>;

new-ui/src/shared/utils/patterns.ts

@@ -70,11 +70,6 @@ export const patternValidDomain =
 export const patternValidIp =
   /^(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$/;
 
-export const patternValidIpWithMask =
-  /^(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)(?:\/(?:[0-9]|[1-2][0-9]|3[0-2]))?$/;
-
-export const cidrRegex =
-  /^(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}\/\d{1,2}|[0-9a-fA-F:.]+\/\d{1,3})$/;
 // Regular expression to match a WireGuard endpoint. A bare IPv4 literal must
 // include a port (a port-less IP is almost always a mistake), while domain names
 // and localhost may omit it. IPv6 endpoints are validated separately via
@@ -86,9 +81,6 @@ export const patternValidEndpoint =
 export const patternValidIpV6 =
   /^(([a-f0-9]{1,4}:){7}|::([a-f0-9]{1,4}:){0,6}|([a-f0-9]{1,4}:){1}:([a-f0-9]{1,4}:){0,5}|([a-f0-9]{1,4}:){2}:([a-f0-9]{1,4}:){0,4}|([a-f0-9]{1,4}:){3}:([a-f0-9]{1,4}:){0,3}|([a-f0-9]{1,4}:){4}:([a-f0-9]{1,4}:){0,2}|([a-f0-9]{1,4}:){5}:([a-f0-9]{1,4}:){0,1})([a-f0-9]{1,4}|(((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([0-9]{1,2}))\.){3}((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([0-9]{1,2})))$/;
 
-export const patternValidIpV6WithMask =
-  /^(([a-f0-9]{1,4}:){7}|::([a-f0-9]{1,4}:){0,6}|([a-f0-9]{1,4}:){1}:([a-f0-9]{1,4}:){0,5}|([a-f0-9]{1,4}:){2}:([a-f0-9]{1,4}:){0,4}|([a-f0-9]{1,4}:){3}:([a-f0-9]{1,4}:){0,3}|([a-f0-9]{1,4}:){4}:([a-f0-9]{1,4}:){0,2}|([a-f0-9]{1,4}:){5}:([a-f0-9]{1,4}:){0,1})([a-f0-9]{1,4}|(((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([0-9]{1,2}))\.){3}((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([0-9]{1,2})))(?:\/(?:[0-9]|[1-9][0-9]|1[01][0-9]|12[0-8]))?$/;
-
 // Reuse pattern from above to support format [ipv6]:port
 export const patternValidIpV6WithPort =
   /^\[((([a-f0-9]{1,4}:){7}|::([a-f0-9]{1,4}:){0,6}|([a-f0-9]{1,4}:){1}:([a-f0-9]{1,4}:){0,5}|([a-f0-9]{1,4}:){2}:([a-f0-9]{1,4}:){0,4}|([a-f0-9]{1,4}:){3}:([a-f0-9]{1,4}:){0,3}|([a-f0-9]{1,4}:){4}:([a-f0-9]{1,4}:){0,2}|([a-f0-9]{1,4}:){5}:([a-f0-9]{1,4}:){0,1})([a-f0-9]{1,4}|(((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([0-9]{1,2}))\.){3}((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([0-9]{1,2})))(\/128)?)\]:(\d{1,5})$/;

new-ui/src/shared/utils/zod.ts

@@ -1,6 +1,5 @@
 import { z } from 'zod';
 import {
-  cidrRegex,
   patternValidEndpoint,
   patternValidIpV6WithPort,
   patternValidWireguardKey,
@@ -36,11 +35,23 @@ export const optionalWireguardKeySchema = z
   .string()
   .refine((v) => !v || patternValidWireguardKey.test(v), 'Invalid WireGuard key');
 
-// Comma-separated list of CIDR ranges; an empty value is allowed.
-export const allowedIpsSchema = z.string().refine((v) => {
-  if (!v) return true;
-  return v
+const ipOrCidrSchema = z.union([z.ipv4(), z.ipv6(), z.cidrv4(), z.cidrv6()]);
+
+const isValidIpList = (value: string) =>
+  value
     .split(',')
-    .map((s) => s.trim())
-    .every((cidr) => cidrRegex.test(cidr));
-}, 'Invalid CIDR notation');
+    .map((ip) => ip.trim())
+    .every((ip) => ipOrCidrSchema.safeParse(ip).success);
+
+// A required comma-separated list of interface addresses or CIDR ranges.
+export const interfaceAddressesSchema = z
+  .string()
+  .refine((value) => Boolean(value) && isValidIpList(value), 'Field is invalid');
+
+// Comma-separated list of allowed IP addresses or CIDR ranges; an empty value is allowed.
+export const allowedIpsSchema = z
+  .string()
+  .refine(
+    (value) => !value || isValidIpList(value),
+    'Invalid IP address or CIDR notation',
+  );

src-tauri/cli/Cargo.toml

@@ -13,6 +13,7 @@ tonic-prost-build.workspace = true
 [dependencies]
 clap.workspace = true
 common = { package = "defguard-client-common", path = "../common" }
+defguard-client-proto = { path = "../client-proto" }
 defguard_wireguard_rs = { workspace = true, features = ["check_dependencies"] }
 dirs-next.workspace = true
 prost.workspace = true

src-tauri/cli/src/bin/dg.rs

@@ -11,6 +11,7 @@ use std::{
 
 use clap::{builder::FalseyValueParser, command, value_parser, Arg, Command};
 use common::{dns_borrow, find_free_tcp_port, get_interface_name};
+use defguard_client_proto::conversions::normalize_allowed_ips;
 #[cfg(not(target_os = "macos"))]
 use defguard_wireguard_rs::Kernel;
 #[cfg(target_os = "macos")]
@@ -243,7 +244,7 @@ async fn connect(config: CliConfig, ifname: String, trigger: Arc<Notify>) -> Res
         .collect::<Vec<_>>();
     debug!("Parsed assigned IPs: {addresses:?}");
 
-    let config = InterfaceConfiguration {
+    let mut config = InterfaceConfiguration {
         name: config.instance_info.name.clone(),
         prvkey: config.private_key.to_string(),
         addresses,
@@ -252,6 +253,7 @@ async fn connect(config: CliConfig, ifname: String, trigger: Arc<Notify>) -> Res
         mtu: None,
         fwmark: None,
     };
+    normalize_allowed_ips(&mut config);
     let configure_interface_result = wgapi.configure_interface(&config);
 
     configure_interface_result.expect("Failed to configure WireGuard interface");

src-tauri/client-proto/Cargo.toml

@@ -17,6 +17,7 @@ serde.workspace = true
 serde_with = "3.11"
 tonic.workspace = true
 tonic-prost.workspace = true
+tracing.workspace = true
 
 defguard_wireguard_rs.workspace = true
 

src-tauri/client-proto/src/conversions.rs

@@ -1,16 +1,62 @@
 use std::{
+    collections::HashSet,
+    mem::take,
+    net::{IpAddr, Ipv4Addr, Ipv6Addr},
     str::FromStr,
     time::{Duration, UNIX_EPOCH},
 };
 
 use defguard_wireguard_rs::{
     host::Host, key::Key, net::IpAddrMask, peer::Peer, InterfaceConfiguration,
 };
-
 use tonic::Status;
+use tracing::debug;
 
 use crate::defguard::client::v1::{InterfaceConfig, InterfaceData, Peer as ProtoPeer};
 
+/// Truncates host bits from a peer allowed IP.
+///
+/// This runs before `WGApi` classifies default routes. In particular, a non-canonical `/0` must
+/// become an unspecified address so it takes the default-route loop-prevention path.
+#[must_use]
+fn truncate_to_network(mut allowed_ip: IpAddrMask) -> IpAddrMask {
+    let max_cidr = if allowed_ip.address.is_ipv4() {
+        Ipv4Addr::BITS
+    } else {
+        Ipv6Addr::BITS
+    };
+
+    // Unreachable via `FromStr`, which rejects an out-of-range cidr, but `IpAddrMask::new` and the
+    // public `cidr` field don't. Bail out rather than let `mask()` underflow its shift.
+    if allowed_ip.cidr as u32 > max_cidr {
+        debug!("Leaving allowed IP {allowed_ip} unnormalized, its cidr exceeds {max_cidr}");
+        return allowed_ip;
+    }
+
+    allowed_ip.address = match (allowed_ip.address, allowed_ip.mask()) {
+        (IpAddr::V4(address), IpAddr::V4(mask)) => {
+            IpAddr::V4(Ipv4Addr::from(u32::from(address) & u32::from(mask)))
+        }
+        (IpAddr::V6(address), IpAddr::V6(mask)) => {
+            IpAddr::V6(Ipv6Addr::from(u128::from(address) & u128::from(mask)))
+        }
+        _ => return allowed_ip,
+    };
+    allowed_ip
+}
+
+/// Normalizes and deduplicates peer allowed IPs before they reach `WGApi`.
+pub fn normalize_allowed_ips(config: &mut InterfaceConfiguration) {
+    for peer in &mut config.peers {
+        let mut seen = HashSet::new();
+        peer.allowed_ips = take(&mut peer.allowed_ips)
+            .into_iter()
+            .map(truncate_to_network)
+            .filter(|allowed_ip| seen.insert(allowed_ip.clone()))
+            .collect();
+    }
+}
+
 impl From<InterfaceConfiguration> for InterfaceConfig {
     fn from(config: InterfaceConfiguration) -> Self {
         Self {
@@ -179,6 +225,93 @@ mod tests {
         peer
     }
 
+    #[test]
+    fn test_truncate_to_network_clears_ipv4_host_bits() {
+        let allowed_ip = "172.16.0.1/24".parse::<IpAddrMask>().unwrap();
+
+        assert_eq!(
+            truncate_to_network(allowed_ip),
+            "172.16.0.0/24".parse::<IpAddrMask>().unwrap()
+        );
+    }
+
+    #[test]
+    fn test_truncate_to_network_keeps_ipv4_host_route() {
+        let allowed_ip = "172.16.0.1/32".parse::<IpAddrMask>().unwrap();
+
+        assert_eq!(truncate_to_network(allowed_ip.clone()), allowed_ip);
+    }
+
+    #[test]
+    fn test_truncate_to_network_keeps_canonical_address() {
+        let allowed_ip = "172.16.0.0/24".parse::<IpAddrMask>().unwrap();
+
+        assert_eq!(truncate_to_network(allowed_ip.clone()), allowed_ip);
+    }
+
+    #[test]
+    fn test_truncate_to_network_handles_ipv4_default_route() {
+        let allowed_ip = "10.0.0.1/0".parse::<IpAddrMask>().unwrap();
+
+        assert_eq!(
+            truncate_to_network(allowed_ip),
+            "0.0.0.0/0".parse::<IpAddrMask>().unwrap()
+        );
+    }
+
+    #[test]
+    fn test_truncate_to_network_clears_ipv6_host_bits() {
+        let allowed_ip = "2001:db8::1/96".parse::<IpAddrMask>().unwrap();
+
+        assert_eq!(
+            truncate_to_network(allowed_ip),
+            "2001:db8::/96".parse::<IpAddrMask>().unwrap()
+        );
+    }
+
+    #[test]
+    fn test_truncate_to_network_preserves_invalid_ipv4_cidr() {
+        let allowed_ip = IpAddrMask::new("172.16.0.1".parse().unwrap(), 33);
+
+        assert_eq!(truncate_to_network(allowed_ip.clone()), allowed_ip);
+    }
+
+    #[test]
+    fn test_truncate_to_network_preserves_invalid_ipv6_cidr() {
+        let allowed_ip = IpAddrMask::new("2001:db8::1".parse().unwrap(), 129);
+
+        assert_eq!(truncate_to_network(allowed_ip.clone()), allowed_ip);
+    }
+
+    #[test]
+    fn test_normalize_allowed_ips_deduplicates_after_masking() {
+        let mut peer = sample_peer();
+        peer.allowed_ips = ["172.16.0.1/24", "172.16.0.2/24", "10.0.0.0/24"]
+            .into_iter()
+            .map(|allowed_ip| allowed_ip.parse().unwrap())
+            .collect();
+        let mut config = InterfaceConfiguration {
+            name: "wg0".into(),
+            prvkey: String::new(),
+            addresses: vec!["10.0.0.1/24".parse().unwrap()],
+            port: 0,
+            peers: vec![peer],
+            mtu: None,
+            fwmark: None,
+        };
+
+        normalize_allowed_ips(&mut config);
+
+        assert_eq!(
+            config.peers[0].allowed_ips,
+            ["172.16.0.0/24", "10.0.0.0/24"]
+                .into_iter()
+                .map(|allowed_ip| allowed_ip.parse().unwrap())
+                .collect::<Vec<_>>()
+        );
+        assert_eq!(config.addresses, vec!["10.0.0.1/24".parse().unwrap()]);
+    }
+
     #[test]
     fn test_host_to_interface_data() {
         let secret = EphemeralSecret::random();