From ea6415539d2793fd0cfcb70a8fde344a7bac3f9b Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 20 Jun 2024 15:56:31 +1000 Subject: [PATCH] fix(gateway): don't panic on max port range in NAT table (#5459) In our NAT table on the gateway, we try to first pick the external port as the one on the packet that we want to translate. This makes that port mapping consistent between NAT sessions in the majority of cases. In case the port is taken, we iterate through two chained `Range`s that end up cycling the entire port range. [`RangeFrom`](https://doc.rust-lang.org/std/ops/struct.RangeFrom.html) has a somewhat unexpected behaviour in regards to exhaustived ranges: They panic when trying to access the next element. To avoid this, we explicitly end the first range at `u16::MAX` which makes it an empty range in case the source port is `u16::MAX`. --- rust/connlib/tunnel/proptest-regressions/peer/nat_table.txt | 2 ++ rust/connlib/tunnel/src/peer/nat_table.rs | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/rust/connlib/tunnel/proptest-regressions/peer/nat_table.txt b/rust/connlib/tunnel/proptest-regressions/peer/nat_table.txt index 987e1d174..ba658bb1c 100644 --- a/rust/connlib/tunnel/proptest-regressions/peer/nat_table.txt +++ b/rust/connlib/tunnel/proptest-regressions/peer/nat_table.txt @@ -7,3 +7,5 @@ cc 1e03c37afda9f91fb4bf23e693921b59530eaefdb95094db03ac8d1e448b68d4 # shrinks to input = _TranslatesBackAndForthUdpPacketArgs { packet: Ipv4(ConvertibleIpv4Packet { buf: Owned([0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 69, 0, 0, 28, 0, 0, 0, 0, 64, 17, 122, 210, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 8, 255, 222]) }), outside_dst: 0.0.0.0, response_delay: 9223372036854758677.681955028s } cc 95107af217cd84a6eacda3264751e15825f7fb92d7cc70378baaf2f7a0958b1c # shrinks to input = _TranslatesBackAndForthUdpPacketArgs { packet: Ipv4(ConvertibleIpv4Packet { buf: Owned([0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 69, 0, 0, 28, 0, 0, 0, 0, 64, 17, 122, 210, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 8, 255, 222]) }), outside_dst: ::ffff:0.0.0.0, response_delay: 0 } cc 6937709663669c298021f536f9f921964928d1ab8640068da13db6e1cde363a4 # shrinks to input = _TranslatesBackAndForthUdpPacketArgs { packet: Ipv4(ConvertibleIpv4Packet { buf: Owned([0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 69, 0, 0, 28, 0, 0, 0, 0, 64, 17, 122, 210, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 8, 255, 222]) }), outside_dst: 0.0.0.0, response_delay: 60 } +cc 483e1d1fa3f453bd4890de5449f284490511e59c9a206cfbc051204a41323495 # shrinks to input = _TranslatesBackAndForthPacketArgs { packet: Ipv6(ConvertibleIpv6Packet { buf: Owned([96, 0, 0, 0, 0, 20, 6, 64, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 255, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 255, 0, 0, 0, 0, 255, 255, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 80, 0, 0, 128, 175, 101, 0, 0]) }), outside_dst: ::ffff:0.0.0.0, response_delay: 0 } +cc 9937e167496adf2b059df0c2b48c103de8f338b401c585299ae51f7db3e7706e # shrinks to input = _CanHandleMultiplePacketsArgs { packet1: Ipv4(ConvertibleIpv4Packet { buf: Owned([0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 69, 0, 0, 28, 0, 0, 192, 0, 64, 17, 186, 209, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 8, 255, 222]) }), outside_dst1: 0.0.0.0, packet2: Ipv4(ConvertibleIpv4Packet { buf: Owned([0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 69, 0, 0, 28, 0, 0, 192, 0, 64, 17, 186, 209, 0, 0, 0, 0, 0, 0, 0, 0, 255, 255, 0, 0, 0, 8, 255, 222]) }), outside_dst2: 0.0.0.0 } diff --git a/rust/connlib/tunnel/src/peer/nat_table.rs b/rust/connlib/tunnel/src/peer/nat_table.rs index d7c1171ef..828d840f1 100644 --- a/rust/connlib/tunnel/src/peer/nat_table.rs +++ b/rust/connlib/tunnel/src/peer/nat_table.rs @@ -67,7 +67,7 @@ impl NatTable { // Find the first available public port, starting from the port of the to-be-mapped packet. // This will re-assign the same port in most cases, even after the mapping expires. - let outside = (src.value()..) + let outside = (src.value()..=u16::MAX) .chain(1..src.value()) .map(|p| (src.with_value(p), outside_dst)) .find(|outside| !self.table.contains_right(outside))