Skip to content

Drop packets with invalid NAT requirements in flow-filter #1341

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
qmonnet merged 2 commits into from
Mar 20, 2026
+166 −36
Merged

Drop packets with invalid NAT requirements in flow-filter #1341

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a9f08cd
test(flow-filter): Add helper to fake flow session info for packet
qmonnet Mar 13, 2026
954346e
feat(flow-filter): Filter packets with invalid NAT requirements
qmonnet Mar 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions flow-filter/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,17 @@ impl FlowFilter {
None
}
Some(VpcdLookupResult::Single(dst_data)) => {
// Check NAT requirements are sensible
if self
.check_nat_requirements(packet, &dst_data, true)
.is_err()
{
debug!(
"{nfi}: Invalid NAT requirements found for flow {tuple}, dropping packet"
);
packet.done(DoneReason::Filtered);
return;
}
Self::set_nat_requirements(packet, &dst_data);
Some(dst_data.vpcd)
}
Expand Down Expand Up @@ -270,6 +281,14 @@ impl FlowFilter {
// requirement.
if data_set.len() == 1 {
let dst_data = data_set.iter().next().unwrap_or_else(|| unreachable!());
// Check NAT requirements are sensible - no need to check flow availability, we know we
// don't have an active flow if we reached that point.
if self
.check_nat_requirements(packet, dst_data, false)
.is_err()
{
return None;
}
Self::set_nat_requirements(packet, dst_data);
return Some(first_vpcd);
}
Expand Down Expand Up @@ -313,6 +332,38 @@ impl FlowFilter {
None
}

/// Check if the packet has valid NAT requirements.
fn check_nat_requirements<Buf: PacketBufferMut>(
&self,
packet: &Packet<Buf>,
dst_data: &RemoteData,
needs_flow_verif: bool,
) -> Result<(), ()> {
if needs_flow_verif && packet.active_flow_info().is_some() {
return Ok(());
}

// We have no valid flow table entry for the packet: in this case, some NAT requirements are
// not supported.
let nfi = &self.name;
if matches!(dst_data.dst_nat_req, Some(NatRequirement::Stateful)) {
debug!(
"{nfi}: Packet requires destination NAT with masquerade, but packet does not contain flow-info"
);
return Err(());
}
if matches!(
dst_data.src_nat_req,
Some(NatRequirement::PortForwarding(_))
) {
debug!(
"{nfi}: Packet requires source NAT with port forwarding, but packet does not contain flow-info"
);
return Err(());
}
Ok(())
}

/// Set NAT requirements on the packet based on the remote data object.
fn set_nat_requirements<Buf: PacketBufferMut>(packet: &mut Packet<Buf>, data: &RemoteData) {
if data.requires_stateful_nat() {
Expand Down
151 changes: 115 additions & 36 deletions flow-filter/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,32 @@ fn create_test_icmp_v4_packet(
packet
}

fn fake_flow_session<Buf: PacketBufferMut>(
packet: &mut Packet<Buf>,
dst_vpcd: VpcDiscriminant,
set_nat_state: bool,
set_port_fw_state: bool,
) {
// Create flow_info with dst_vpcd and NAT info and attach it to the packet
let flow_info = FlowInfo::new(std::time::Instant::now() + std::time::Duration::from_secs(60));
let mut binding = flow_info.locked.write().unwrap();
binding.dst_vpcd = Some(Box::new(dst_vpcd));
if set_nat_state {
// Content should be a NatFlowState object but we can't include it in this crate without
// introducing a circular dependency; just use a bool, as we don't attempt to downcast
// it anyway.
binding.nat_state = Some(Box::new(true));
}
if set_port_fw_state {
// Content should be a PortFwState object but we can't include it in this crate without
// introducing a circular dependency; just use a bool, as we don't attempt to downcast
// it anyway.
binding.port_fw_state = Some(Box::new(true));
}
drop(binding);
packet.meta_mut().flow_info = Some(Arc::new(flow_info));
}

#[test]
fn test_flow_filter_packet_allowed() {
// Setup table
Expand Down Expand Up @@ -930,9 +956,9 @@ fn test_flow_filter_table_check_nat_requirements() {
"70.0.0.10".parse().unwrap(),
);
let packet_out = flow_filter.process([packet].into_iter()).next().unwrap();
assert!(!packet_out.is_done(), "{:?}", packet_out.get_done());
assert_eq!(packet_out.meta().dst_vpcd, Some(vpcd(vni2.into())));
assert!(needs_masquerade(&packet_out));
// These are invalid NAT requirements because we cannot currently initiate a connection towards
// an expose using masquerading, and here there is no flow info attached to packet.
assert_eq!(packet_out.get_done(), Some(DoneReason::Filtered));

// src: stateful NAT, dst: default (no NAT)
let packet = create_test_packet(
Expand Down Expand Up @@ -1055,9 +1081,9 @@ fn test_flow_filter_table_check_stateful_nat_plus_peer_forwarding() {
2000,
);
let packet_out = flow_filter.process([packet].into_iter()).next().unwrap();
assert!(!packet_out.is_done(), "{:?}", packet_out.get_done());
assert_eq!(packet_out.meta().dst_vpcd, Some(vni1.into()));
assert!(needs_masquerade(&packet_out));
// We don't have a flow-info for the packet, and cannot initiate the connection towards an
// expose using stateful NAT.
assert_eq!(packet_out.get_done(), Some(DoneReason::Filtered));

// VPC 2 to VPC 1, outside of port forwarding port range: reverse stateful NAT
let packet = create_test_ipv4_udp_packet_with_ports(
Expand All @@ -1068,9 +1094,9 @@ fn test_flow_filter_table_check_stateful_nat_plus_peer_forwarding() {
123,
);
let packet_out = flow_filter.process([packet].into_iter()).next().unwrap();
assert!(!packet_out.is_done(), "{:?}", packet_out.get_done());
assert_eq!(packet_out.meta().dst_vpcd, Some(vni1.into()));
assert!(needs_masquerade(&packet_out));
// We don't have a flow-info for the packet, and cannot initiate the connection towards an
// expose using stateful NAT.
assert_eq!(packet_out.get_done(), Some(DoneReason::Filtered));

// VPC 2 to VPC 1, inside of port forwarding range: port forwarding
let packet = create_test_ipv4_udp_packet_with_ports(
Expand All @@ -1094,17 +1120,7 @@ fn test_flow_filter_table_check_stateful_nat_plus_peer_forwarding() {
2000,
456,
);
// Create flow_info with dst_vpcd and stateful NAT info and attach it to the packet
let flow_info = FlowInfo::new(std::time::Instant::now() + std::time::Duration::from_secs(60));
let mut binding = flow_info.locked.write().unwrap();
binding.dst_vpcd = Some(Box::new(VpcDiscriminant::from(vni2)));
// Content should be a NatFlowState object but we can't include it in this crate without
// introducing a circular dependency; just use a bool, as we don't attempt to downcast it
// anyway.
binding.nat_state = Some(Box::new(true));
drop(binding);
packet.meta_mut().flow_info = Some(Arc::new(flow_info));

fake_flow_session(&mut packet, vni2.into(), true, false);
let packet_out = flow_filter.process([packet].into_iter()).next().unwrap();
assert!(!packet_out.is_done(), "{:?}", packet_out.get_done());
assert_eq!(packet_out.meta().dst_vpcd, Some(vni2.into()));
Expand All @@ -1119,21 +1135,39 @@ fn test_flow_filter_table_check_stateful_nat_plus_peer_forwarding() {
2000,
456,
);
// Create flow_info with dst_vpcd and port forwarding info and attach it to the packet
let flow_info = FlowInfo::new(std::time::Instant::now() + std::time::Duration::from_secs(60));
let mut binding = flow_info.locked.write().unwrap();
binding.dst_vpcd = Some(Box::new(VpcDiscriminant::from(vni2)));
// Content should be a PortFwState object but we can't include it in this crate without
// introducing a circular dependency; just use a bool, as we don't attempt to downcast it
// anyway.
binding.port_fw_state = Some(Box::new(true));
drop(binding);
packet.meta_mut().flow_info = Some(Arc::new(flow_info));

fake_flow_session(&mut packet, vni2.into(), false, true);
let packet_out = flow_filter.process([packet].into_iter()).next().unwrap();
assert!(!packet_out.is_done(), "{:?}", packet_out.get_done());
assert_eq!(packet_out.meta().dst_vpcd, Some(vni2.into()));
assert!(needs_port_forwarding(&packet_out));

// VPC 2 to VPC 1, outside of port forwarding port range, with flow_info attached for stateful NAT: reverse stateful NAT
let mut packet = create_test_ipv4_udp_packet_with_ports(
Some(vni2.into()),
"5.0.0.10".parse().unwrap(),
"100.0.0.27".parse().unwrap(),
456,
123,
);
fake_flow_session(&mut packet, vni1.into(), true, false);
let packet_out = flow_filter.process([packet].into_iter()).next().unwrap();
assert!(!packet_out.is_done(), "{:?}", packet_out.get_done());
assert_eq!(packet_out.meta().dst_vpcd, Some(vni1.into()));
assert!(needs_masquerade(&packet_out));

// VPC 2 to VPC 1, outside of port forwarding IP range, with flow_info attached for stateful NAT: reverse stateful NAT
let mut packet = create_test_ipv4_udp_packet_with_ports(
Some(vni2.into()),
"5.0.0.10".parse().unwrap(),
"100.0.0.4".parse().unwrap(),
456,
2000,
);
fake_flow_session(&mut packet, vni1.into(), true, false);
let packet_out = flow_filter.process([packet].into_iter()).next().unwrap();
assert!(!packet_out.is_done(), "{:?}", packet_out.get_done());
assert_eq!(packet_out.meta().dst_vpcd, Some(vni1.into()));
assert!(needs_masquerade(&packet_out));
}

#[test]
Expand Down Expand Up @@ -1247,7 +1281,7 @@ fn test_flow_filter_protocol_aware_port_forwarding() {
assert!(needs_port_forwarding(&packet_out));

// UDP packet inside port forwarding range: TCP-only port forwarding is filtered out,
// only stateful NAT remains -> stateful NAT (not dropped!)
// only stateful NAT remains (destination NAT), with no flow table entry -> drop packet
let packet = create_test_ipv4_udp_packet_with_ports(
Some(vni2.into()),
"5.0.0.10".parse().unwrap(),
Expand All @@ -1256,6 +1290,19 @@ fn test_flow_filter_protocol_aware_port_forwarding() {
3000,
);
let packet_out = flow_filter.process([packet].into_iter()).next().unwrap();
assert_eq!(packet_out.get_done(), Some(DoneReason::Filtered));

// UDP packet inside port forwarding range: TCP-only port forwarding is filtered out,
// only stateful NAT remains, with flow table entry -> stateful NAT (not dropped!)
let mut packet = create_test_ipv4_udp_packet_with_ports(
Some(vni2.into()),
"5.0.0.10".parse().unwrap(),
"100.0.0.27".parse().unwrap(),
456,
3000,
);
fake_flow_session(&mut packet, vni1.into(), true, false);
let packet_out = flow_filter.process([packet].into_iter()).next().unwrap();
assert!(!packet_out.is_done(), "{:?}", packet_out.get_done());
assert_eq!(packet_out.meta().dst_vpcd, Some(vni1.into()));
assert!(needs_masquerade(&packet_out));
Expand Down Expand Up @@ -1533,8 +1580,9 @@ fn test_flow_filter_table_from_overlay_masquerade_port_forwarding_private_ips_ov

// VPC-2 -> VPC-1: 192.168.90.100:22 -> 192.168.50.7:6789
//
// Must use port forwarding even though we have overlap on source IP/port with masquerading
// rule, because of unambiguous destination
// Overlap on source IP/port with masquerading rule, the destination is unambiguous and we find
// a source NAT port forwarding requirement, but there's no associated flow table entry and we
// cannot initiate a port forwarding session on the source side, so we drop
let packet = create_test_ipv4_tcp_packet_with_ports(
Some(vpcd(vni2.into())),
"192.168.90.100".parse().unwrap(),
Expand All @@ -1543,6 +1591,22 @@ fn test_flow_filter_table_from_overlay_masquerade_port_forwarding_private_ips_ov
6789,
);
let packet_out = flow_filter.process([packet].into_iter()).next().unwrap();
assert_eq!(packet_out.get_done(), Some(DoneReason::Filtered));

// VPC-2 -> VPC-1: 192.168.90.100:22 -> 192.168.50.7:6789
//
// Must use port forwarding even though we have overlap on source IP/port with masquerading
// rule, because of unambiguous destination, and we have a flow table entry so source-side port
// forwarding is allowed
let mut packet = create_test_ipv4_tcp_packet_with_ports(
Some(vpcd(vni2.into())),
"192.168.90.100".parse().unwrap(),
"192.168.50.7".parse().unwrap(),
22,
6789,
);
fake_flow_session(&mut packet, vni1.into(), false, true);
let packet_out = flow_filter.process([packet].into_iter()).next().unwrap();
assert!(!packet_out.is_done(), "{:?}", packet_out.get_done());
assert_eq!(packet_out.meta().dst_vpcd, Some(vpcd(vni1.into())));
assert!(needs_port_forwarding(&packet_out));
Expand Down Expand Up @@ -1825,9 +1889,10 @@ fn test_flow_filter_table_from_overlay_masquerade_port_forwarding_private_ips_ov

// Test with packets

// VPC-1 -> VPC-2, outside of port forwarding range
// VPC-1 -> VPC-2, outside of port forwarding range, no flow info attached
//
// Only masquerading applies.
// Only masquerading applies but we cannot initiate the connection towards the expose using
// masquerading.
let packet = create_test_ipv4_tcp_packet_with_ports(
Some(vpcd(vni1.into())),
"7.7.7.7".parse().unwrap(),
Expand All @@ -1836,6 +1901,20 @@ fn test_flow_filter_table_from_overlay_masquerade_port_forwarding_private_ips_ov
5678,
);
let packet_out = flow_filter.process([packet].into_iter()).next().unwrap();
assert_eq!(packet_out.get_done(), Some(DoneReason::Filtered));

// VPC-1 -> VPC-2, outside of port forwarding range, with flow info attached
//
// Only masquerading applies.
let mut packet = create_test_ipv4_tcp_packet_with_ports(
Some(vpcd(vni1.into())),
"7.7.7.7".parse().unwrap(),
"10.0.0.1".parse().unwrap(),
1234,
5678,
);
fake_flow_session(&mut packet, vni2.into(), true, false);
let packet_out = flow_filter.process([packet].into_iter()).next().unwrap();
assert!(!packet_out.is_done(), "{:?}", packet_out.get_done());
assert_eq!(packet_out.meta().dst_vpcd, Some(vpcd(vni2.into())));
assert!(needs_masquerade(&packet_out));
Expand Down
Loading