From 527dbc96a37e182021cab28babf08e15bbbd119a Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Fri, 20 Mar 2026 10:42:39 +0000 Subject: [PATCH 1/3] refactor(config): Rename VpcExposeNat.proto to proto_restriction We plan to remove type L4Protocol and use instead an Option. In that context, having "None" for a NAT object doesn't mean that NAT rules should apply to no protocol at all, but rather that they are not restricted to any L4 protocol. In other words, the field will designate a "L4 protocol restriction" rather than a "L4 protocol covered by the rule". Rename the field to .proto_restriction to reflect that. Signed-off-by: Quentin Monnet --- config/src/converters/k8s/config/expose.rs | 2 +- config/src/display.rs | 2 +- config/src/external/overlay/vpcpeering.rs | 8 ++++---- config/src/utils/overlap.rs | 2 +- flow-filter/src/tables.rs | 2 +- nat/src/portfw/portfwtable/setup.rs | 6 +++++- nat/src/stateful/apalloc/setup.rs | 5 ++++- 7 files changed, 17 insertions(+), 10 deletions(-) diff --git a/config/src/converters/k8s/config/expose.rs b/config/src/converters/k8s/config/expose.rs index 74f8fb1d2..687212c54 100644 --- a/config/src/converters/k8s/config/expose.rs +++ b/config/src/converters/k8s/config/expose.rs @@ -238,7 +238,7 @@ fn set_port_ranges( nat.as_range.remove(target_prefix); } - nat.proto = match proto { + nat.proto_restriction = match proto { Some(GatewayAgentPeeringsPeeringExposeNatPortForwardPortsProto::Tcp) => L4Protocol::Tcp, Some(GatewayAgentPeeringsPeeringExposeNatPortForwardPortsProto::Udp) => L4Protocol::Udp, Some(GatewayAgentPeeringsPeeringExposeNatPortForwardPortsProto::KopiumEmpty) | None => { diff --git a/config/src/display.rs b/config/src/display.rs index 4842ff7ed..e990eaa79 100644 --- a/config/src/display.rs +++ b/config/src/display.rs @@ -78,7 +78,7 @@ impl Display for VpcExpose { if !nat.as_range.is_empty() { write!(f, "{SEP} as:")?; nat.as_range.iter().for_each(|pfx| { - let _ = write!(f, " {pfx} proto: {:?} NAT:{}", nat.proto, &nat.config); + let _ = write!(f, " {pfx} proto: {:?} NAT:{}", nat.proto_restriction, &nat.config); }); carriage = true; } diff --git a/config/src/external/overlay/vpcpeering.rs b/config/src/external/overlay/vpcpeering.rs index 47e3b1d3e..378bbdc3b 100644 --- a/config/src/external/overlay/vpcpeering.rs +++ b/config/src/external/overlay/vpcpeering.rs @@ -38,7 +38,7 @@ pub struct VpcExposeNat { pub as_range: PrefixPortsSet, pub not_as: PrefixPortsSet, pub config: VpcExposeNatConfig, - pub proto: L4Protocol, + pub proto_restriction: L4Protocol, } impl VpcExposeNat { @@ -48,7 +48,7 @@ impl VpcExposeNat { as_range: PrefixPortsSet::new(), not_as: PrefixPortsSet::new(), config, - proto: L4Protocol::default(), + proto_restriction: L4Protocol::default(), } } @@ -151,7 +151,7 @@ impl VpcExpose { Some(nat) if nat.is_port_forwarding() => { nat.config = VpcExposeNatConfig::PortForwarding(options); if let Some(proto) = proto { - nat.proto = proto; + nat.proto_restriction = proto; } } Some(_) => { @@ -163,7 +163,7 @@ impl VpcExpose { let mut nat = VpcExposeNat::from_config(VpcExposeNatConfig::PortForwarding(options)); if let Some(proto) = proto { - nat.proto = proto; + nat.proto_restriction = proto; } self.nat = Some(nat); } diff --git a/config/src/utils/overlap.rs b/config/src/utils/overlap.rs index c4cf28bbc..eb179cdd3 100644 --- a/config/src/utils/overlap.rs +++ b/config/src/utils/overlap.rs @@ -52,7 +52,7 @@ fn port_forwarding_with_distinct_l4_protocols( && expose_right.has_port_forwarding() && let Some(nat_left) = expose_left.nat.as_ref() && let Some(nat_right) = expose_right.nat.as_ref() - && nat_left.proto.intersection(&nat_right.proto).is_none() + && nat_left.proto_restriction.intersection(&nat_right.proto_restriction).is_none() { true } else { diff --git a/flow-filter/src/tables.rs b/flow-filter/src/tables.rs index 052edff1e..29ee2bf83 100644 --- a/flow-filter/src/tables.rs +++ b/flow-filter/src/tables.rs @@ -471,7 +471,7 @@ pub(crate) enum NatRequirement { impl NatRequirement { pub(crate) fn from_nat(nat: &VpcExposeNat) -> NatRequirement { - match (&nat.config, nat.proto) { + match (&nat.config, nat.proto_restriction) { (VpcExposeNatConfig::Stateful(_), _) => NatRequirement::Stateful, (VpcExposeNatConfig::Stateless(_), _) => NatRequirement::Stateless, (VpcExposeNatConfig::PortForwarding(_), proto) => NatRequirement::PortForwarding(proto), diff --git a/nat/src/portfw/portfwtable/setup.rs b/nat/src/portfw/portfwtable/setup.rs index 7268fec0a..0d37c6d0a 100644 --- a/nat/src/portfw/portfwtable/setup.rs +++ b/nat/src/portfw/portfwtable/setup.rs @@ -13,7 +13,11 @@ use net::ip::NextHeader; use net::packet::VpcDiscriminant; fn port_fw_proto(expose: &VpcExpose) -> L4Protocol { - expose.nat.as_ref().unwrap_or_else(|| unreachable!()).proto + expose + .nat + .as_ref() + .unwrap_or_else(|| unreachable!()) + .proto_restriction } fn expose_to_portfw_rule( diff --git a/nat/src/stateful/apalloc/setup.rs b/nat/src/stateful/apalloc/setup.rs index 0cf430805..0499ed07d 100644 --- a/nat/src/stateful/apalloc/setup.rs +++ b/nat/src/stateful/apalloc/setup.rs @@ -199,7 +199,10 @@ fn find_masquerade_portfw_overlap<'a>( for pf_expose in port_forwarding_exposes { let pf_nat = pf_expose.nat.as_ref().unwrap_or_else(|| unreachable!()); - let Some(relevant_proto) = expose_nat.proto.intersection(&pf_nat.proto) else { + let Some(relevant_proto) = expose_nat + .proto_restriction + .intersection(&pf_nat.proto_restriction) + else { // No overlap on L4 protocols, so no overlap for prefixes and ports. continue; }; From 4b446bbc3b7f078da228575799e65d26dbcc5f93 Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Fri, 20 Mar 2026 11:49:09 +0000 Subject: [PATCH 2/3] refactor(config,flow-filter,lpm,nat): Remove struct L4Protocol Drop struct L4Protocol for NAT's protocol restrictions and use an Option instead, because... ??? Signed-off-by: Quentin Monnet --- config/src/converters/k8s/config/expose.rs | 13 +++++-- config/src/external/overlay/vpcpeering.rs | 43 ++++++++++++++++------ config/src/utils/overlap.rs | 8 +++- flow-filter/src/lib.rs | 15 ++++---- flow-filter/src/setup.rs | 13 ++++--- flow-filter/src/tables.rs | 27 +++++++++----- flow-filter/src/tests.rs | 27 +++++++------- lpm/src/prefix/with_ports.rs | 20 ---------- nat/src/portfw/portfwtable/setup.rs | 18 ++++++--- nat/src/stateful/apalloc/setup.rs | 31 +++++++--------- 10 files changed, 120 insertions(+), 95 deletions(-) diff --git a/config/src/converters/k8s/config/expose.rs b/config/src/converters/k8s/config/expose.rs index 687212c54..32ebba272 100644 --- a/config/src/converters/k8s/config/expose.rs +++ b/config/src/converters/k8s/config/expose.rs @@ -8,7 +8,8 @@ use k8s_intf::gateway_agent_crd::{ GatewayAgentPeeringsPeeringExposeIps, GatewayAgentPeeringsPeeringExposeNat, GatewayAgentPeeringsPeeringExposeNatPortForwardPortsProto, }; -use lpm::prefix::{L4Protocol, PortRange, Prefix, PrefixString, PrefixWithOptionalPorts}; +use lpm::prefix::{PortRange, Prefix, PrefixString, PrefixWithOptionalPorts}; +use net::ip::NextHeader; use crate::converters::k8s::FromK8sConversionError; use crate::converters::k8s::config::SubnetMap; @@ -239,10 +240,14 @@ fn set_port_ranges( } nat.proto_restriction = match proto { - Some(GatewayAgentPeeringsPeeringExposeNatPortForwardPortsProto::Tcp) => L4Protocol::Tcp, - Some(GatewayAgentPeeringsPeeringExposeNatPortForwardPortsProto::Udp) => L4Protocol::Udp, + Some(GatewayAgentPeeringsPeeringExposeNatPortForwardPortsProto::Tcp) => { + Some(NextHeader::TCP) + } + Some(GatewayAgentPeeringsPeeringExposeNatPortForwardPortsProto::Udp) => { + Some(NextHeader::UDP) + } Some(GatewayAgentPeeringsPeeringExposeNatPortForwardPortsProto::KopiumEmpty) | None => { - L4Protocol::Any + None } }; diff --git a/config/src/external/overlay/vpcpeering.rs b/config/src/external/overlay/vpcpeering.rs index 378bbdc3b..3b466f9d8 100644 --- a/config/src/external/overlay/vpcpeering.rs +++ b/config/src/external/overlay/vpcpeering.rs @@ -5,9 +5,9 @@ use crate::utils::{check_private_prefixes_dont_overlap, check_public_prefixes_dont_overlap}; use lpm::prefix::{ - IpRangeWithPorts, L4Protocol, Prefix, PrefixPortsSet, PrefixWithOptionalPorts, - PrefixWithPortsSize, + IpRangeWithPorts, Prefix, PrefixPortsSet, PrefixWithOptionalPorts, PrefixWithPortsSize, }; +use net::ip::NextHeader; use std::collections::BTreeMap; use std::ops::Bound::{Excluded, Unbounded}; use std::time::Duration; @@ -38,7 +38,7 @@ pub struct VpcExposeNat { pub as_range: PrefixPortsSet, pub not_as: PrefixPortsSet, pub config: VpcExposeNatConfig, - pub proto_restriction: L4Protocol, + pub proto_restriction: Option, } impl VpcExposeNat { @@ -48,7 +48,7 @@ impl VpcExposeNat { as_range: PrefixPortsSet::new(), not_as: PrefixPortsSet::new(), config, - proto_restriction: L4Protocol::default(), + proto_restriction: None, } } @@ -66,6 +66,31 @@ impl VpcExposeNat { pub fn is_port_forwarding(&self) -> bool { matches!(self.config, VpcExposeNatConfig::PortForwarding(_)) } + + #[must_use] + pub fn l4_proto_common_restrictions( + left: &Option, + right: &Option, + ) -> Option> { + match (left, right) { + (Some(left), Some(right)) if left == right => Some(vec![*left]), + (Some(_), Some(_)) => None, + (None, Some(single)) | (Some(single), None) => Some(vec![*single]), + (None, None) => Some(vec![NextHeader::TCP, NextHeader::UDP]), + } + } + + #[must_use] + pub fn l4_proto_restriction_applies_to_proto( + restriction: &Option, + proto: &Option, + ) -> bool { + match (restriction, proto) { + (None, _) => true, + (Some(_), None) => false, + (Some(restriction), Some(proto)) => restriction == proto, + } + } } fn empty_set() -> &'static PrefixPortsSet { @@ -144,15 +169,13 @@ impl VpcExpose { pub fn make_port_forwarding( mut self, idle_timeout: Option, - proto: Option, + proto_restriction: Option, ) -> Result { let options = VpcExposePortForwarding { idle_timeout }; match self.nat.as_mut() { Some(nat) if nat.is_port_forwarding() => { nat.config = VpcExposeNatConfig::PortForwarding(options); - if let Some(proto) = proto { - nat.proto_restriction = proto; - } + nat.proto_restriction = proto_restriction; } Some(_) => { return Err(ConfigError::Invalid(format!( @@ -162,9 +185,7 @@ impl VpcExpose { None => { let mut nat = VpcExposeNat::from_config(VpcExposeNatConfig::PortForwarding(options)); - if let Some(proto) = proto { - nat.proto_restriction = proto; - } + nat.proto_restriction = proto_restriction; self.nat = Some(nat); } } diff --git a/config/src/utils/overlap.rs b/config/src/utils/overlap.rs index eb179cdd3..44fd5d0d2 100644 --- a/config/src/utils/overlap.rs +++ b/config/src/utils/overlap.rs @@ -1,8 +1,8 @@ // SPDX-License-Identifier: Apache-2.0 // Copyright Open Network Fabric Authors -use crate::ConfigError; use crate::external::overlay::vpcpeering::VpcExpose; +use crate::{ConfigError, external::overlay::vpcpeering::VpcExposeNat}; use lpm::prefix::{IpRangeWithPorts, PrefixPortsSet, PrefixWithPortsSize}; pub fn check_private_prefixes_dont_overlap( @@ -52,7 +52,11 @@ fn port_forwarding_with_distinct_l4_protocols( && expose_right.has_port_forwarding() && let Some(nat_left) = expose_left.nat.as_ref() && let Some(nat_right) = expose_right.nat.as_ref() - && nat_left.proto_restriction.intersection(&nat_right.proto_restriction).is_none() + && VpcExposeNat::l4_proto_common_restrictions( + &nat_left.proto_restriction, + &nat_right.proto_restriction, + ) + .is_none() { true } else { diff --git a/flow-filter/src/lib.rs b/flow-filter/src/lib.rs index 3a941928b..d5e20f553 100644 --- a/flow-filter/src/lib.rs +++ b/flow-filter/src/lib.rs @@ -14,12 +14,13 @@ //! peerings, get dropped. use crate::tables::{NatRequirement, RemoteData, VpcdLookupResult}; +use config::external::overlay::vpcpeering::VpcExposeNat; use indenter::indented; -use lpm::prefix::L4Protocol; use net::buffer::PacketBufferMut; use net::flows::FlowStatus; use net::flows::flow_info_item::ExtractRef; use net::headers::{Transport, TryIp, TryTransport}; +use net::ip::NextHeader; use net::packet::{DoneReason, Packet, VpcDiscriminant}; use pipeline::{NetworkFunction, PipelineData}; use std::collections::HashSet; @@ -313,7 +314,7 @@ fn deal_with_multiple_matches( let Some(NatRequirement::PortForwarding(requirement_proto)) = d.src_nat_req else { return false; }; - requirement_proto.intersection(&packet_proto).is_some() + VpcExposeNat::l4_proto_restriction_applies_to_proto(&requirement_proto, &packet_proto) }) { set_nat_requirements(packet, dst_data); @@ -325,7 +326,7 @@ fn deal_with_multiple_matches( let Some(NatRequirement::PortForwarding(req_proto)) = d.dst_nat_req else { return false; }; - req_proto.intersection(&packet_proto).is_some() + VpcExposeNat::l4_proto_restriction_applies_to_proto(&req_proto, &packet_proto) }) && data_set .iter() .any(|d| d.dst_nat_req == Some(NatRequirement::Stateful)) @@ -460,10 +461,10 @@ fn set_nat_requirements_from_flow_info( } } -pub(crate) fn get_l4_proto(packet: &Packet) -> L4Protocol { +pub(crate) fn get_l4_proto(packet: &Packet) -> Option { match packet.try_transport() { - Some(Transport::Tcp(_)) => L4Protocol::Tcp, - Some(Transport::Udp(_)) => L4Protocol::Udp, - _ => L4Protocol::Any, + Some(Transport::Tcp(_)) => Some(NextHeader::TCP), + Some(Transport::Udp(_)) => Some(NextHeader::UDP), + _ => None, } } diff --git a/flow-filter/src/setup.rs b/flow-filter/src/setup.rs index 67725aab5..c53a71ad2 100644 --- a/flow-filter/src/setup.rs +++ b/flow-filter/src/setup.rs @@ -541,7 +541,8 @@ mod tests { use super::*; use config::external::overlay::vpc::{Vpc, VpcTable}; use config::external::overlay::vpcpeering::{VpcExpose, VpcManifest, VpcPeeringTable}; - use lpm::prefix::{L4Protocol, PortRange, Prefix, PrefixWithPortsSize}; + use lpm::prefix::{PortRange, Prefix, PrefixWithPortsSize}; + use net::ip::NextHeader; use net::packet::VpcDiscriminant; use net::vxlan::Vni; use std::collections::BTreeSet; @@ -876,7 +877,7 @@ mod tests { name: "manifest2".to_string(), exposes: vec![ VpcExpose::empty() - .make_port_forwarding(None, Some(L4Protocol::Tcp)) + .make_port_forwarding(None, Some(NextHeader::TCP)) .unwrap() .ip(PrefixWithOptionalPorts::new( "1.0.0.1/32".into(), @@ -888,7 +889,7 @@ mod tests { )) .unwrap(), VpcExpose::empty() - .make_port_forwarding(None, Some(L4Protocol::Udp)) + .make_port_forwarding(None, Some(NextHeader::UDP)) .unwrap() .ip(PrefixWithOptionalPorts::new( "1.0.0.1/32".into(), @@ -950,7 +951,7 @@ mod tests { set.contains(&RemoteData::new( vpcd, None, - Some(NatRequirement::PortForwarding(L4Protocol::Tcp)), + Some(NatRequirement::PortForwarding(Some(NextHeader::TCP))), )), "{set:?}" ); @@ -958,7 +959,7 @@ mod tests { set.contains(&RemoteData::new( vpcd, None, - Some(NatRequirement::PortForwarding(L4Protocol::Udp)), + Some(NatRequirement::PortForwarding(Some(NextHeader::UDP))), )), "{set:?}" ); @@ -1054,7 +1055,7 @@ mod tests { HashSet::from([RemoteData::new( vpcd(200), None, - Some(NatRequirement::PortForwarding(L4Protocol::Any)), + Some(NatRequirement::PortForwarding(None)), )]), ); diff --git a/flow-filter/src/tables.rs b/flow-filter/src/tables.rs index 29ee2bf83..2999ce13c 100644 --- a/flow-filter/src/tables.rs +++ b/flow-filter/src/tables.rs @@ -6,8 +6,9 @@ use config::ConfigError; use config::external::overlay::vpcpeering::{VpcExposeNat, VpcExposeNatConfig}; use lpm::prefix::range_map::DisjointRangesBTreeMap; -use lpm::prefix::{L4Protocol, PortRange, Prefix}; +use lpm::prefix::{PortRange, Prefix}; use lpm::trie::{IpPortPrefixTrie, ValueWithAssociatedRanges}; +use net::ip::NextHeader; use net::packet::VpcDiscriminant; use std::collections::{HashMap, HashSet}; use std::fmt::Debug; @@ -466,7 +467,7 @@ impl PortRangeMap { pub(crate) enum NatRequirement { Stateless, Stateful, - PortForwarding(L4Protocol), + PortForwarding(Option), } impl NatRequirement { @@ -474,7 +475,9 @@ impl NatRequirement { match (&nat.config, nat.proto_restriction) { (VpcExposeNatConfig::Stateful(_), _) => NatRequirement::Stateful, (VpcExposeNatConfig::Stateless(_), _) => NatRequirement::Stateless, - (VpcExposeNatConfig::PortForwarding(_), proto) => NatRequirement::PortForwarding(proto), + (VpcExposeNatConfig::PortForwarding(_), proto_restriction) => { + NatRequirement::PortForwarding(proto_restriction) + } } } } @@ -509,10 +512,13 @@ impl RemoteData { || self.dst_nat_req == Some(NatRequirement::Stateless) } - pub(crate) fn requires_port_forwarding(&self, packet_proto: L4Protocol) -> bool { + pub(crate) fn requires_port_forwarding(&self, packet_proto: Option) -> bool { for requirement in [self.src_nat_req, self.dst_nat_req] { - if let Some(NatRequirement::PortForwarding(req_proto)) = requirement - && packet_proto.intersection(&req_proto).is_some() + if let Some(NatRequirement::PortForwarding(req_proto_restriction)) = requirement + && VpcExposeNat::l4_proto_restriction_applies_to_proto( + &req_proto_restriction, + &packet_proto, + ) { return true; } @@ -521,10 +527,13 @@ impl RemoteData { } // Determines whether the NAT requirements object covers a given L4 protocol. - pub(crate) fn applies_to(&self, packet_proto: L4Protocol) -> bool { + pub(crate) fn applies_to(&self, packet_proto: Option) -> bool { for requirement in [self.src_nat_req, self.dst_nat_req] { - if let Some(NatRequirement::PortForwarding(req_proto)) = requirement - && req_proto.intersection(&packet_proto).is_none() + if let Some(NatRequirement::PortForwarding(req_proto_restriction)) = requirement + && !VpcExposeNat::l4_proto_restriction_applies_to_proto( + &req_proto_restriction, + &packet_proto, + ) { return false; } diff --git a/flow-filter/src/tests.rs b/flow-filter/src/tests.rs index f5a74eed8..5d0f9e1fd 100644 --- a/flow-filter/src/tests.rs +++ b/flow-filter/src/tests.rs @@ -8,7 +8,7 @@ use crate::{ use config::external::overlay::Overlay; use config::external::overlay::vpc::{Vpc, VpcTable}; use config::external::overlay::vpcpeering::{VpcExpose, VpcManifest, VpcPeering, VpcPeeringTable}; -use lpm::prefix::{L4Protocol, PortRange, Prefix, PrefixWithOptionalPorts}; +use lpm::prefix::{PortRange, Prefix, PrefixWithOptionalPorts}; use net::buffer::{PacketBufferMut, TestBuffer}; use net::flows::FlowInfo; use net::headers::{Net, TryHeadersMut, TryIpMut}; @@ -312,12 +312,12 @@ fn test_flow_filter_multiple_matches_no_dst_vpcd() { RemoteData::new( vpcd(200), None, - Some(NatRequirement::PortForwarding(L4Protocol::Tcp)), // This rule is for TCP + Some(NatRequirement::PortForwarding(Some(NextHeader::TCP))), // This rule is for TCP ), RemoteData::new( vpcd(300), None, - Some(NatRequirement::PortForwarding(L4Protocol::Tcp)), // This rule is for TCP + Some(NatRequirement::PortForwarding(Some(NextHeader::TCP))), // This rule is for TCP ), ])), Prefix::from("10.0.0.0/24"), @@ -1171,7 +1171,7 @@ fn test_flow_filter_protocol_aware_port_forwarding() { .as_range("100.0.0.0/24".into()) .unwrap(), VpcExpose::empty() - .make_port_forwarding(None, Some(L4Protocol::Tcp)) // TCP-only port forwarding + .make_port_forwarding(None, Some(NextHeader::TCP)) // TCP-only port forwarding .unwrap() .ip(PrefixWithOptionalPorts::new( "1.0.0.27/32".into(), @@ -1264,7 +1264,8 @@ fn test_flow_filter_protocol_aware_port_forwarding() { #[test] #[traced_test] fn test_flow_filter_protocol_any_port_forwarding() { - // Test that L4Protocol::Any port forwarding works for both TCP and UDP packets. + // Test that port forwarding with None as protocol restriction works for both TCP and UDP + // packets. let vni1 = vni(100); let vni2 = vni(200); @@ -1277,7 +1278,7 @@ fn test_flow_filter_protocol_any_port_forwarding() { .add(Vpc::new("vpc2", "VPC02", vni2.as_u32()).unwrap()) .unwrap(); - // Port forwarding with L4Protocol::Any (default) + // Port forwarding with None as protocol restriction (default) let mut peering_table = VpcPeeringTable::new(); peering_table .add(VpcPeering::with_default_group( @@ -1381,7 +1382,7 @@ fn test_flow_filter_table_from_overlay_masquerade_port_forwarding_private_ips_ov name: "vpc2".to_string(), exposes: vec![ VpcExpose::empty() - .make_port_forwarding(None, Some(L4Protocol::Tcp)) + .make_port_forwarding(None, Some(NextHeader::TCP)) .unwrap() .ip(PrefixWithOptionalPorts::new( "192.168.90.100/32".into(), // 192.168.90.100 used privately for VPC02 @@ -1393,7 +1394,7 @@ fn test_flow_filter_table_from_overlay_masquerade_port_forwarding_private_ips_ov )) .unwrap(), VpcExpose::empty() - .make_port_forwarding(None, Some(L4Protocol::Udp)) + .make_port_forwarding(None, Some(NextHeader::UDP)) .unwrap() .ip(PrefixWithOptionalPorts::new( "192.168.90.100/32".into(), // 192.168.90.100 used privately for VPC02 @@ -1405,7 +1406,7 @@ fn test_flow_filter_table_from_overlay_masquerade_port_forwarding_private_ips_ov )) .unwrap(), VpcExpose::empty() - .make_port_forwarding(None, Some(L4Protocol::Tcp)) + .make_port_forwarding(None, Some(NextHeader::TCP)) .unwrap() .ip(PrefixWithOptionalPorts::new( "192.168.90.100/32".into(), // 192.168.90.100 used privately for VPC02 @@ -1589,7 +1590,7 @@ fn test_flow_filter_table_from_overlay_masquerade_port_forwarding_private_ips_ov name: "vpc2".to_string(), exposes: vec![ VpcExpose::empty() - .make_port_forwarding(None, Some(L4Protocol::Tcp)) + .make_port_forwarding(None, Some(NextHeader::TCP)) .unwrap() .ip(PrefixWithOptionalPorts::new( "192.168.90.0/24".into(), // Contains 192.168.90.100 used privately from VPC 2 @@ -1601,7 +1602,7 @@ fn test_flow_filter_table_from_overlay_masquerade_port_forwarding_private_ips_ov )) .unwrap(), VpcExpose::empty() - .make_port_forwarding(None, Some(L4Protocol::Udp)) + .make_port_forwarding(None, Some(NextHeader::UDP)) .unwrap() .ip(PrefixWithOptionalPorts::new( "192.168.90.0/24".into(), // Contains 192.168.90.100 used privately from VPC 2 @@ -1613,7 +1614,7 @@ fn test_flow_filter_table_from_overlay_masquerade_port_forwarding_private_ips_ov )) .unwrap(), VpcExpose::empty() - .make_port_forwarding(None, Some(L4Protocol::Tcp)) + .make_port_forwarding(None, Some(NextHeader::TCP)) .unwrap() .ip(PrefixWithOptionalPorts::new( "192.168.90.100/32".into(), // 192.168.90.100 used privately from VPC 2 @@ -1790,7 +1791,7 @@ fn test_flow_filter_table_from_overlay_masquerade_port_forwarding_private_ips_ov name: "vpc2".to_string(), exposes: vec![ VpcExpose::empty() - .make_port_forwarding(None, Some(L4Protocol::Tcp)) + .make_port_forwarding(None, Some(NextHeader::TCP)) .unwrap() .ip(PrefixWithOptionalPorts::new( "1.0.0.1/32".into(), diff --git a/lpm/src/prefix/with_ports.rs b/lpm/src/prefix/with_ports.rs index fcf8cd44c..6434fef72 100644 --- a/lpm/src/prefix/with_ports.rs +++ b/lpm/src/prefix/with_ports.rs @@ -681,26 +681,6 @@ impl Display for PrefixWithOptionalPorts { } } -#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Hash)] -pub enum L4Protocol { - Tcp, - Udp, - #[default] - Any, -} - -impl L4Protocol { - #[must_use] - pub fn intersection(&self, other: &L4Protocol) -> Option { - match (self, other) { - (L4Protocol::Any, other_proto) => Some(*other_proto), - (self_proto, L4Protocol::Any) => Some(*self_proto), - (self_proto, other_proto) if self_proto == other_proto => Some(*self_proto), - _ => None, - } - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/nat/src/portfw/portfwtable/setup.rs b/nat/src/portfw/portfwtable/setup.rs index 0d37c6d0a..3777b3a1b 100644 --- a/nat/src/portfw/portfwtable/setup.rs +++ b/nat/src/portfw/portfwtable/setup.rs @@ -8,11 +8,11 @@ use crate::portfw::{PortFwEntry, PortFwKey, PortFwTableError}; use config::ConfigError; use config::external::overlay::vpc::{Peering, Vpc, VpcTable}; use config::external::overlay::vpcpeering::VpcExpose; -use lpm::prefix::{L4Protocol, PrefixWithOptionalPorts}; +use lpm::prefix::PrefixWithOptionalPorts; use net::ip::NextHeader; use net::packet::VpcDiscriminant; -fn port_fw_proto(expose: &VpcExpose) -> L4Protocol { +fn port_fw_proto(expose: &VpcExpose) -> Option { expose .nat .as_ref() @@ -67,16 +67,22 @@ fn vpc_port_fw_peering( for expose in peering.local.port_forwarding_exposes() { let remote_vpc_vni = vpc_table.get_remote_vni(peering); let src_vpc = VpcDiscriminant::from_vni(remote_vpc_vni); - match port_fw_proto(expose) { - L4Protocol::Tcp => { + let proto = port_fw_proto(expose); + match proto { + Some(NextHeader::TCP) => { let rule = expose_to_portfw_rule(expose, NextHeader::TCP, src_vpc, dst_vpc)?; rules.push(rule); } - L4Protocol::Udp => { + Some(NextHeader::UDP) => { let rule = expose_to_portfw_rule(expose, NextHeader::UDP, src_vpc, dst_vpc)?; rules.push(rule); } - L4Protocol::Any => { + Some(_) => { + return Err(PortFwTableError::Unsupported(format!( + "Unsupported protocol {proto:?}", + ))); + } + None => { let rule = expose_to_portfw_rule(expose, NextHeader::TCP, src_vpc, dst_vpc)?; rules.push(rule); diff --git a/nat/src/stateful/apalloc/setup.rs b/nat/src/stateful/apalloc/setup.rs index 0499ed07d..c4196fd2f 100644 --- a/nat/src/stateful/apalloc/setup.rs +++ b/nat/src/stateful/apalloc/setup.rs @@ -10,12 +10,10 @@ use crate::stateful::allocator_writer::StatefulNatConfig; use crate::stateful::{NatAllocator, NatIp}; use config::ConfigError; use config::external::overlay::vpc::Peering; -use config::external::overlay::vpcpeering::{VpcExpose, VpcManifest}; +use config::external::overlay::vpcpeering::{VpcExpose, VpcExposeNat, VpcManifest}; use config::utils::collapse_prefixes_peering; use lpm::prefix::range_map::DisjointRangesBTreeMap; -use lpm::prefix::{ - IpPrefix, L4Protocol, PortRange, Prefix, PrefixPortsSet, PrefixWithOptionalPorts, -}; +use lpm::prefix::{IpPrefix, PortRange, Prefix, PrefixPortsSet, PrefixWithOptionalPorts}; use net::ip::NextHeader; use net::packet::VpcDiscriminant; use std::collections::{BTreeMap, BTreeSet}; @@ -199,21 +197,19 @@ fn find_masquerade_portfw_overlap<'a>( for pf_expose in port_forwarding_exposes { let pf_nat = pf_expose.nat.as_ref().unwrap_or_else(|| unreachable!()); - let Some(relevant_proto) = expose_nat - .proto_restriction - .intersection(&pf_nat.proto_restriction) - else { + let Some(relevant_protos) = VpcExposeNat::l4_proto_common_restrictions( + &expose_nat.proto_restriction, + &pf_nat.proto_restriction, + ) else { // No overlap on L4 protocols, so no overlap for prefixes and ports. continue; }; let ranges_intersection = pf_expose.ips.intersection_prefixes_and_ports(&expose.ips); - match relevant_proto { - L4Protocol::Tcp => reserve_sets.tcp.extend(ranges_intersection), - L4Protocol::Udp => reserve_sets.udp.extend(ranges_intersection), - L4Protocol::Any => { - reserve_sets.tcp.extend(ranges_intersection.clone()); - reserve_sets.udp.extend(ranges_intersection); - } + if relevant_protos.contains(&NextHeader::TCP) { + reserve_sets.tcp.extend(ranges_intersection.clone()); + } + if relevant_protos.contains(&NextHeader::UDP) { + reserve_sets.udp.extend(ranges_intersection); } } reserve_sets @@ -398,7 +394,8 @@ fn create_ipv6_bitmap_mappings( mod tests { use super::{ReserveSets, find_masquerade_portfw_overlap}; use config::external::overlay::vpcpeering::VpcExpose; - use lpm::prefix::{L4Protocol, PortRange, PrefixPortsSet, PrefixWithOptionalPorts}; + use lpm::prefix::{PortRange, PrefixPortsSet, PrefixWithOptionalPorts}; + use net::ip::NextHeader; fn prefix_with_ports(s: &str, start: u16, end: u16) -> PrefixWithOptionalPorts { PrefixWithOptionalPorts::new(s.into(), Some(PortRange::new(start, end).unwrap())) @@ -460,7 +457,7 @@ mod tests { .unwrap() .ip("10.0.0.0/24".into()); let pf_expose = VpcExpose::empty() - .make_port_forwarding(None, Some(L4Protocol::Tcp)) // TCP only + .make_port_forwarding(None, Some(NextHeader::TCP)) // TCP only .unwrap() .ip(prefix_with_ports("10.0.0.0/24", 8080, 8090)); let pf_exposes_vec = vec![&pf_expose]; From 734c1d5d19f3abc9cf42b79f3a1044f8c56d4fef Mon Sep 17 00:00:00 2001 From: Quentin Monnet Date: Fri, 20 Mar 2026 11:50:43 +0000 Subject: [PATCH 3/3] feat(config): Prevent non-L4 NextHeader use in NAT proto restrictions We recently dropped the L4Protocol type to qualify the L4 restrictions for a NAT set up, in favour of an Option. This forces us to be more careful when handling the field, because NextHeader can be any IP's inner header, not just TCP or UDP. To prevent unexpected usage of VpcExposeNat.proto_restriction, in particular to prevent callers to randomly assign any existing NextHeader variant, make the field private, and provide a setter than can fail. Also add assertions where relevant, to make sure we only handle TCP or UDP with that field. Signed-off-by: Quentin Monnet --- config/src/converters/k8s/config/expose.rs | 3 +- config/src/display.rs | 7 +++- config/src/external/overlay/vpcpeering.rs | 46 +++++++++++++++++++++- config/src/utils/overlap.rs | 4 +- flow-filter/src/tables.rs | 2 +- nat/src/portfw/portfwtable/setup.rs | 2 +- nat/src/stateful/apalloc/setup.rs | 4 +- 7 files changed, 58 insertions(+), 10 deletions(-) diff --git a/config/src/converters/k8s/config/expose.rs b/config/src/converters/k8s/config/expose.rs index 32ebba272..04fa241cf 100644 --- a/config/src/converters/k8s/config/expose.rs +++ b/config/src/converters/k8s/config/expose.rs @@ -239,7 +239,7 @@ fn set_port_ranges( nat.as_range.remove(target_prefix); } - nat.proto_restriction = match proto { + let proto_restriction = match proto { Some(GatewayAgentPeeringsPeeringExposeNatPortForwardPortsProto::Tcp) => { Some(NextHeader::TCP) } @@ -250,6 +250,7 @@ fn set_port_ranges( None } }; + nat.set_proto_restriction(proto_restriction)?; vpc_exposes.push(vpc_expose_clone); } diff --git a/config/src/display.rs b/config/src/display.rs index e990eaa79..0b83507f4 100644 --- a/config/src/display.rs +++ b/config/src/display.rs @@ -78,7 +78,12 @@ impl Display for VpcExpose { if !nat.as_range.is_empty() { write!(f, "{SEP} as:")?; nat.as_range.iter().for_each(|pfx| { - let _ = write!(f, " {pfx} proto: {:?} NAT:{}", nat.proto_restriction, &nat.config); + let _ = write!( + f, + " {pfx} proto: {:?} NAT:{}", + nat.proto_restriction(), + &nat.config + ); }); carriage = true; } diff --git a/config/src/external/overlay/vpcpeering.rs b/config/src/external/overlay/vpcpeering.rs index 3b466f9d8..258e6da1b 100644 --- a/config/src/external/overlay/vpcpeering.rs +++ b/config/src/external/overlay/vpcpeering.rs @@ -4,6 +4,7 @@ //! Dataplane configuration model: vpc peering use crate::utils::{check_private_prefixes_dont_overlap, check_public_prefixes_dont_overlap}; +use crate::{ConfigError, ConfigResult}; use lpm::prefix::{ IpRangeWithPorts, Prefix, PrefixPortsSet, PrefixWithOptionalPorts, PrefixWithPortsSize, }; @@ -38,7 +39,7 @@ pub struct VpcExposeNat { pub as_range: PrefixPortsSet, pub not_as: PrefixPortsSet, pub config: VpcExposeNatConfig, - pub proto_restriction: Option, + proto_restriction: Option, } impl VpcExposeNat { @@ -67,11 +68,40 @@ impl VpcExposeNat { matches!(self.config, VpcExposeNatConfig::PortForwarding(_)) } + #[must_use] + pub fn proto_restriction(&self) -> Option { + self.proto_restriction + } + + pub fn set_proto_restriction(&mut self, proto_restriction: Option) -> ConfigResult { + match proto_restriction { + Some(NextHeader::TCP | NextHeader::UDP) => { + self.proto_restriction = proto_restriction; + Ok(()) + } + None => { + self.proto_restriction = None; + Ok(()) + } + Some(_) => Err(ConfigError::InternalFailure(format!( + "Trying to set invalid L4 protocol restriction for NAT: {proto_restriction:?}" + ))), + } + } + + /// Returns the common L4 protocol restrictions between two NAT configurations. + /// + /// # Panics + /// + /// Panics if either `left` or `right` contains a protocol other than TCP or UDP. #[must_use] pub fn l4_proto_common_restrictions( left: &Option, right: &Option, ) -> Option> { + assert!(left.is_none_or(|pr| matches!(pr, NextHeader::TCP | NextHeader::UDP))); + assert!(right.is_none_or(|pr| matches!(pr, NextHeader::TCP | NextHeader::UDP))); + match (left, right) { (Some(left), Some(right)) if left == right => Some(vec![*left]), (Some(_), Some(_)) => None, @@ -81,10 +111,17 @@ impl VpcExposeNat { } #[must_use] + /// Returns whether the L4 protocol restriction applies to the given protocol. + /// + /// # Panics + /// + /// Panics if `restriction` contains a protocol other than TCP or UDP. pub fn l4_proto_restriction_applies_to_proto( restriction: &Option, proto: &Option, ) -> bool { + assert!(restriction.is_none_or(|pr| matches!(pr, NextHeader::TCP | NextHeader::UDP))); + match (restriction, proto) { (None, _) => true, (Some(_), None) => false, @@ -99,7 +136,6 @@ fn empty_set() -> &'static PrefixPortsSet { &EMPTY_SET } -use crate::{ConfigError, ConfigResult}; #[derive(Clone, Debug, Default, PartialEq)] pub struct VpcExpose { pub default: bool, @@ -171,6 +207,12 @@ impl VpcExpose { idle_timeout: Option, proto_restriction: Option, ) -> Result { + if proto_restriction.is_some_and(|pr| !matches!(pr, NextHeader::TCP | NextHeader::UDP)) { + return Err(ConfigError::Invalid(format!( + "Trying to set invalid L4 protocol restriction for NAT: {proto_restriction:?}" + ))); + } + let options = VpcExposePortForwarding { idle_timeout }; match self.nat.as_mut() { Some(nat) if nat.is_port_forwarding() => { diff --git a/config/src/utils/overlap.rs b/config/src/utils/overlap.rs index 44fd5d0d2..a25d7570c 100644 --- a/config/src/utils/overlap.rs +++ b/config/src/utils/overlap.rs @@ -53,8 +53,8 @@ fn port_forwarding_with_distinct_l4_protocols( && let Some(nat_left) = expose_left.nat.as_ref() && let Some(nat_right) = expose_right.nat.as_ref() && VpcExposeNat::l4_proto_common_restrictions( - &nat_left.proto_restriction, - &nat_right.proto_restriction, + &nat_left.proto_restriction(), + &nat_right.proto_restriction(), ) .is_none() { diff --git a/flow-filter/src/tables.rs b/flow-filter/src/tables.rs index 2999ce13c..19adb4c1c 100644 --- a/flow-filter/src/tables.rs +++ b/flow-filter/src/tables.rs @@ -472,7 +472,7 @@ pub(crate) enum NatRequirement { impl NatRequirement { pub(crate) fn from_nat(nat: &VpcExposeNat) -> NatRequirement { - match (&nat.config, nat.proto_restriction) { + match (&nat.config, nat.proto_restriction()) { (VpcExposeNatConfig::Stateful(_), _) => NatRequirement::Stateful, (VpcExposeNatConfig::Stateless(_), _) => NatRequirement::Stateless, (VpcExposeNatConfig::PortForwarding(_), proto_restriction) => { diff --git a/nat/src/portfw/portfwtable/setup.rs b/nat/src/portfw/portfwtable/setup.rs index 3777b3a1b..d0d35ec87 100644 --- a/nat/src/portfw/portfwtable/setup.rs +++ b/nat/src/portfw/portfwtable/setup.rs @@ -17,7 +17,7 @@ fn port_fw_proto(expose: &VpcExpose) -> Option { .nat .as_ref() .unwrap_or_else(|| unreachable!()) - .proto_restriction + .proto_restriction() } fn expose_to_portfw_rule( diff --git a/nat/src/stateful/apalloc/setup.rs b/nat/src/stateful/apalloc/setup.rs index c4196fd2f..66fefec2e 100644 --- a/nat/src/stateful/apalloc/setup.rs +++ b/nat/src/stateful/apalloc/setup.rs @@ -198,8 +198,8 @@ fn find_masquerade_portfw_overlap<'a>( for pf_expose in port_forwarding_exposes { let pf_nat = pf_expose.nat.as_ref().unwrap_or_else(|| unreachable!()); let Some(relevant_protos) = VpcExposeNat::l4_proto_common_restrictions( - &expose_nat.proto_restriction, - &pf_nat.proto_restriction, + &expose_nat.proto_restriction(), + &pf_nat.proto_restriction(), ) else { // No overlap on L4 protocols, so no overlap for prefixes and ports. continue;