network: select egress traffic by destination-port range

aci_edge_sandboxes/README.md

@@ -645,17 +645,18 @@ and deny rules; deny rules take precedence. Each rule lists destination
 networks (`to`, each a CIDR with optional `except` sub-networks) and destination
 `ports` (protocol, `port`, optional `endPort`). An empty `to` matches every
 destination, and an empty `ports` matches every protocol and port. A `tcp` or
-`udp` entry without a `port` matches every port of that protocol, an `icmp`
-entry matches ICMP alone, and `any` matches every protocol or, with a port,
-TCP and UDP on that port.
+`udp` entry without a `port` matches every port of that protocol, one with a
+`port` and an `endPort` matches every port from `port` through `endPort`,
+an `icmp` entry matches ICMP alone, and `any` matches every protocol or, with a
+port or a port range, TCP and UDP on those ports.
 
 Rules are expanded into OpenVMM rules exactly: exceptions are subtracted from
-their networks, port ranges become one rule per port, and protocol `any` with a
-port becomes a TCP and a UDP rule. IPv6 networks and rules that would expand to
-more than 256 OpenVMM rules are rejected. Egress denied without allow rules,
-with ingress denied, attaches no network device; otherwise the guest gets
-`10.0.0.2/24` behind the profile's NAT gateway `10.0.0.1`, which also serves
-DNS.
+their networks, a port range becomes one OpenVMM range rule for each network,
+and protocol `any` with a port or a port range becomes a TCP and a UDP rule.
+IPv6 networks and rules that would expand to more than 256 OpenVMM rules are
+rejected. Egress denied without allow rules, with ingress denied, attaches no
+network device; otherwise the guest gets `10.0.0.2/24` behind the profile's NAT
+gateway `10.0.0.1`, which also serves DNS.
 
 ### Idempotence and concurrency
 

aci_edge_sandboxes/model/src/model.rs

@@ -288,6 +288,18 @@ impl NetworkRule {
         self
     }
 
+    /// Restricts the rule to the ports of `protocol` from `port` through `end_port`, both
+    /// included.
+    #[must_use]
+    pub fn on_port_range(mut self, protocol: Protocol, port: u16, end_port: u16) -> Self {
+        self.ports.push(NetworkPort {
+            protocol,
+            port: Some(port),
+            end_port: Some(end_port),
+        });
+        self
+    }
+
     /// Restricts the rule to `protocol` on every port.
     #[must_use]
     pub fn on_protocol(mut self, protocol: Protocol) -> Self {
@@ -806,6 +818,14 @@ mod tests {
             .unwrap(),
             rule
         );
+        // A port range carries its inclusive last port as `endPort`.
+        let rule = NetworkRule::to("192.0.2.1").on_port_range(Protocol::Tcp, 8000, 8010);
+        let json = serde_json::json!({
+            "to": [{ "cidr": "192.0.2.1" }],
+            "ports": [{ "protocol": "tcp", "port": 8000, "endPort": 8010 }]
+        });
+        assert_eq!(serde_json::to_value(&rule).unwrap(), json);
+        assert_eq!(serde_json::from_value::<NetworkRule>(json).unwrap(), rule);
     }
 
     #[test]

aci_edge_sandboxes/src/openvmm/network.rs

@@ -2,9 +2,9 @@
 //!
 //! OpenVMM's portable profile enforces an egress default plus IPv4 allow and deny rules that
 //! match a network and, optionally, one protocol (TCP, UDP, or ICMP) and, for TCP or UDP, one
-//! port; deny rules take precedence. Contract rules are expanded into that form exactly:
-//! exceptions are subtracted from their networks, port ranges become one rule per port, and `any`
-//! protocol with a port becomes a TCP and a UDP rule. Rules that cannot be expressed exactly are
+//! port or an inclusive range of ports; deny rules take precedence. Contract rules are expanded
+//! into that form exactly: exceptions are subtracted from their networks, and `any` protocol with
+//! a port or a port range becomes a TCP and a UDP rule. Rules that cannot be expressed exactly are
 //! rejected.
 
 use crate::cidr::{Cidr, Ipv4Cidr};
@@ -109,49 +109,53 @@ fn expand_rule(rule: &NetworkRule, field: &str, output: &mut Vec<String>) -> Res
         networks.extend(remainder);
     }
 
-    // `None` matches every protocol; a protocol without a port matches each of its ports.
-    let mut selectors: Vec<Option<(&str, Option<u16>)>> = Vec::new();
+    let mut selectors = Vec::new();
     if rule.ports.is_empty() {
-        selectors.push(None);
+        selectors.push(Selector::Everything);
     }
     for (index, port) in rule.ports.iter().enumerate() {
-        let protocols: &[&str] = match port.protocol {
+        let protocols: &[&'static str] = match port.protocol {
             Protocol::Tcp => &["tcp"],
             Protocol::Udp => &["udp"],
             Protocol::Icmp => &["icmp"],
             Protocol::Any => &["tcp", "udp"],
         };
         let Some(start) = port.port else {
+            if port.end_port.is_some() {
+                return unsupported(format!(".ports[{index}].endPort requires port"));
+            }
             if port.protocol == Protocol::Any {
-                selectors.push(None);
+                selectors.push(Selector::Everything);
             } else {
-                selectors.extend(protocols.iter().map(|protocol| Some((*protocol, None))));
+                selectors.extend(protocols.iter().copied().map(Selector::Protocol));
             }
             continue;
         };
         if port.protocol == Protocol::Icmp {
             return unsupported(format!(".ports[{index}]: ICMP has no ports"));
         }
         let end = port.end_port.unwrap_or(start);
-        if usize::from(end - start) >= MAX_RULES {
-            return unsupported(format!(
-                ".ports[{index}]: the openvmm backend accepts port ranges of at most {MAX_RULES} \
-                 ports"
-            ));
-        }
-        for protocol in protocols {
-            for number in start..=end {
-                selectors.push(Some((protocol, Some(number))));
-            }
+        if end < start {
+            return unsupported(format!(".ports[{index}].endPort must not be below port"));
         }
+        selectors.extend(
+            protocols
+                .iter()
+                .map(|&protocol| Selector::Ports(protocol, start, end)),
+        );
     }
 
     for network in &networks {
         for selector in &selectors {
-            output.push(match selector {
-                None => network.to_string(),
-                Some((protocol, None)) => format!("{network}:{protocol}"),
-                Some((protocol, Some(port))) => format!("{network}:{protocol}:{port}"),
+            output.push(match *selector {
+                Selector::Everything => network.to_string(),
+                Selector::Protocol(protocol) => format!("{network}:{protocol}"),
+                Selector::Ports(protocol, start, end) if start == end => {
+                    format!("{network}:{protocol}:{start}")
+                }
+                Selector::Ports(protocol, start, end) => {
+                    format!("{network}:{protocol}:{start}-{end}")
+                }
             });
             if output.len() > MAX_RULES * 4 {
                 return unsupported(format!(" expands to more than {MAX_RULES} OpenVMM rules"));
@@ -161,6 +165,17 @@ fn expand_rule(rule: &NetworkRule, field: &str, output: &mut Vec<String>) -> Res
     Ok(())
 }
 
+/// The traffic that one OpenVMM rule selects at its destination network.
+#[derive(Clone, Copy)]
+enum Selector {
+    /// Every protocol and port.
+    Everything,
+    /// Every port of TCP or UDP, or every ICMP message.
+    Protocol(&'static str),
+    /// The TCP or UDP ports from the first through the last, both included.
+    Ports(&'static str, u16, u16),
+}
+
 fn ipv4(cidr: &Cidr) -> Ipv4Cidr {
     cidr.ipv4().expect("an IPv4 network")
 }
@@ -238,10 +253,8 @@ mod tests {
         assert_eq!(
             rules(&arguments, "--network-egress-allow"),
             [
-                "10.0.0.0/32:tcp:80",
-                "10.0.0.0/32:tcp:81",
-                "10.0.0.2/31:tcp:80",
-                "10.0.0.2/31:tcp:81",
+                "10.0.0.0/32:tcp:80-81",
+                "10.0.0.2/31:tcp:80-81",
                 "192.0.2.7/32:tcp:53",
                 "192.0.2.7/32:udp:53",
                 "198.51.100.0/24",
@@ -326,6 +339,32 @@ mod tests {
         );
     }
 
+    #[test]
+    fn port_ranges_expand_to_one_rule_per_network_and_protocol() {
+        let egress = EgressPolicy::new(Access::Deny)
+            .with_allow(NetworkRule::to("192.0.2.7").on_port_range(Protocol::Tcp, 8000, 8010))
+            .with_allow(NetworkRule::to("192.0.2.7").on_port_range(Protocol::Udp, 5000, 5010))
+            .with_allow(NetworkRule::to("198.51.100.0/24").on_port_range(Protocol::Any, 1, 65535))
+            // A range of one port is that port's rule.
+            .with_allow(NetworkRule::to("203.0.113.9").on_port_range(Protocol::Tcp, 443, 443))
+            .with_deny(NetworkRule::to("192.0.2.7").on_port(Protocol::Tcp, 8005));
+        let arguments = network_arguments(Some(&policy(egress)), "10.0.0.2/24").unwrap();
+        assert_eq!(
+            rules(&arguments, "--network-egress-allow"),
+            [
+                "192.0.2.7/32:tcp:8000-8010",
+                "192.0.2.7/32:udp:5000-5010",
+                "198.51.100.0/24:tcp:1-65535",
+                "198.51.100.0/24:udp:1-65535",
+                "203.0.113.9/32:tcp:443",
+            ]
+        );
+        assert_eq!(
+            rules(&arguments, "--network-egress-deny"),
+            ["192.0.2.7/32:tcp:8005"]
+        );
+    }
+
     #[test]
     fn inexpressible_rules_are_rejected() {
         let port = |protocol, port, end_port| NetworkRule {
@@ -339,7 +378,9 @@ mod tests {
         for rule in [
             NetworkRule::to("2001:db8::/32"),
             port(Protocol::Icmp, Some(8), None),
-            port(Protocol::Tcp, Some(1), Some(1024)),
+            // Structural validation rejects these first, unless the backend is called directly.
+            port(Protocol::Tcp, Some(90), Some(80)),
+            port(Protocol::Udp, None, Some(80)),
             NetworkRule {
                 to: (0..300)
                     .map(|index| NetworkPeer::new(format!("10.0.{}.0/24", index % 256)))
@@ -350,6 +391,17 @@ mod tests {
                     end_port: Some(2),
                 }],
             },
+            // 129 separate ranges of both TCP and UDP need 258 OpenVMM rules.
+            NetworkRule {
+                to: vec![NetworkPeer::new("10.0.0.0/8")],
+                ports: (0..129)
+                    .map(|index| NetworkPort {
+                        protocol: Protocol::Any,
+                        port: Some(3 * index + 1),
+                        end_port: Some(3 * index + 2),
+                    })
+                    .collect(),
+            },
         ] {
             let egress = EgressPolicy::new(Access::Deny).with_allow(rule.clone());
             assert_eq!(

aci_edge_sandboxes/tests/openvmm_e2e.rs

@@ -861,6 +861,93 @@ fn network_protocol_selectors_are_enforced() {
     }
 }
 
+#[test]
+#[ignore = "requires a hypervisor and NVX guest artifacts; run scripts/nvx.py test-aci-edge-sandboxes"]
+fn network_port_ranges_are_enforced() {
+    // The TCP probe targets port 53 of the gateway, so a range admits or blocks it when the range
+    // contains port 53: at its first or last port, or inside it, but not from the next port up
+    // or through the next port down, and a UDP range never does.
+    let (nvx, backend) = client("ranges");
+    let gateway = || NetworkRule::to("10.0.0.1");
+    let allow = |rule: NetworkRule| NetworkPolicy {
+        egress: EgressPolicy::new(Access::Deny).with_allow(rule),
+        ..NetworkPolicy::deny_all()
+    };
+    // The address-only allow rule admits the probe, so only deny precedence can block it.
+    let deny = |rule: NetworkRule| NetworkPolicy {
+        egress: EgressPolicy::new(Access::Deny)
+            .with_allow(gateway())
+            .with_deny(rule),
+        ..NetworkPolicy::deny_all()
+    };
+    let cases: [(&str, NetworkPolicy, &[u8]); 10] = [
+        (
+            "allow a tcp range from the port",
+            allow(gateway().on_port_range(Protocol::Tcp, 53, 60)),
+            b"reached\n",
+        ),
+        (
+            "allow a tcp range through the port",
+            allow(gateway().on_port_range(Protocol::Tcp, 40, 53)),
+            b"reached\n",
+        ),
+        (
+            "allow an any range around the port",
+            allow(gateway().on_port_range(Protocol::Any, 50, 60)),
+            b"reached\n",
+        ),
+        (
+            "allow a tcp range from the next port",
+            allow(gateway().on_port_range(Protocol::Tcp, 54, 60)),
+            b"blocked\n",
+        ),
+        (
+            "allow a tcp range through the previous port",
+            allow(gateway().on_port_range(Protocol::Tcp, 40, 52)),
+            b"blocked\n",
+        ),
+        (
+            "allow every udp port",
+            allow(gateway().on_port_range(Protocol::Udp, 1, 65535)),
+            b"blocked\n",
+        ),
+        (
+            "allow a tcp range around a denied port",
+            NetworkPolicy {
+                egress: EgressPolicy::new(Access::Deny)
+                    .with_allow(gateway().on_port_range(Protocol::Tcp, 50, 60))
+                    .with_deny(gateway().on_port(Protocol::Tcp, 53)),
+                ..NetworkPolicy::deny_all()
+            },
+            b"blocked\n",
+        ),
+        (
+            "deny a tcp range around the port",
+            deny(gateway().on_port_range(Protocol::Tcp, 50, 60)),
+            b"blocked\n",
+        ),
+        (
+            "deny a tcp range from the next port",
+            deny(gateway().on_port_range(Protocol::Tcp, 54, 60)),
+            b"reached\n",
+        ),
+        (
+            "deny every udp port",
+            deny(gateway().on_port_range(Protocol::Udp, 1, 65535)),
+            b"reached\n",
+        ),
+    ];
+    for (name, policy, expected) in cases {
+        let sandbox = started(
+            &nvx,
+            &backend,
+            &ProvisionRequest::new().with_network(policy),
+        );
+        let output = shell(&nvx, id(&sandbox), GATEWAY_TCP_PROBE);
+        assert_eq!(output.stdout, expected, "{name}: {output:?}");
+    }
+}
+
 #[test]
 #[ignore = "requires a hypervisor and NVX guest artifacts; run scripts/nvx.py test-aci-edge-sandboxes"]
 fn openvmm_lifecycle_on_a_real_hypervisor() {

aci_edge_sandboxes/tests/openvmm_fake.rs

@@ -1359,8 +1359,14 @@ fn filesystem_and_network_policies_reach_openvmm() {
         .with_network(NetworkPolicy {
             egress: EgressPolicy::new(Access::Deny)
                 .with_allow(NetworkRule::to("192.0.2.0/24").on_port(Protocol::Tcp, 443))
+                .with_allow(NetworkRule::to("192.0.2.0/24").on_port_range(
+                    Protocol::Tcp,
+                    8000,
+                    8010,
+                ))
                 .with_allow(NetworkRule::to("192.0.2.9").on_protocol(Protocol::Icmp))
-                .with_deny(NetworkRule::to("192.0.2.0/24").on_protocol(Protocol::Udp)),
+                .with_deny(NetworkRule::to("192.0.2.0/24").on_protocol(Protocol::Udp))
+                .with_deny(NetworkRule::to("192.0.2.7").on_port(Protocol::Tcp, 8005)),
             ..NetworkPolicy::deny_all()
         });
     let sandbox_id = nvx.provision(&request).unwrap().sandbox_id;
@@ -1381,9 +1387,16 @@ fn filesystem_and_network_policies_reach_openvmm() {
     assert_eq!(value("--network-egress"), ["deny"]);
     assert_eq!(
         value("--network-egress-allow"),
-        ["192.0.2.0/24:tcp:443", "192.0.2.9/32:icmp"]
+        [
+            "192.0.2.0/24:tcp:443",
+            "192.0.2.0/24:tcp:8000-8010",
+            "192.0.2.9/32:icmp"
+        ]
+    );
+    assert_eq!(
+        value("--network-egress-deny"),
+        ["192.0.2.0/24:udp", "192.0.2.7/32:tcp:8005"]
     );
-    assert_eq!(value("--network-egress-deny"), ["192.0.2.0/24:udp"]);
     let command_line = &value("--cmdline")[0];
     let maps: Vec<&str> = command_line
         .split(' ')