feat(gateway): deprecate the NAT64 module - #8383
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
thomaseizinger
force-pushed
the
chore/feature-flag-icmp-error
branch
from
March 24, 2025 06:23
4db6d37 to
fdfbf6b
Compare
thomaseizinger
marked this pull request as ready for review
March 24, 2025 08:42
Member
Author
|
@jamilbk I tested this locally with the compose setup and github.com. Trying to clone over SSH by forcing IPv6 did not work and instantly failed with a "Network unreachable" error. This shows that SSH can correctly decode our ICMP error and will fall back to IPv4 in a happy-eyeballs scenario. |
thomaseizinger
changed the title
jamilbk
reviewed
Mar 26, 2025
Comment on lines
+454
to
+460
| // TODO: Should we use the source IP of the packet? | ||
| let icmp_error = match dst { | ||
| IpAddr::V4(inside_dst) => icmpv4_unreachable(inside_dst, self.client_tun.v4, packet)?, | ||
| IpAddr::V6(inside_dst) => icmpv6_unreachable(inside_dst, self.client_tun.v6, packet)?, | ||
| }; | ||
|
|
||
| Ok(TranslateOutboundResult::DestinationUnreachable(icmp_error)) |
Member
There was a problem hiding this comment.
Hm, yeah I would think you want the source address here to be the Gateway's tunnel IP address right? That's the "sender" after all.
Member
Author
There was a problem hiding this comment.
I see your point, I'll try and see if that works.
Member
Author
There was a problem hiding this comment.
Yep works but doesn't make a difference in my testing.
jamilbk
reviewed
Mar 26, 2025
thomaseizinger
enabled auto-merge
March 27, 2025 00:47
jamilbk
approved these changes
Mar 27, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
At present, the Gateway implements a NAT64 conversion that can convert IPv4 packets to IPv6 and vice versa. Doing this efficiently creates a fair amount of complexity within our
ip-packetcrate. In addition, routing ICMP errors back through our NAT is also complicated by this because we may have to translate the packet embedded in the ICMP error as well.The NAT64 module was originally conceived as a result of the new stub resolver-based DNS architecture. When the Client resolves IPs for a domain, it doesn't know whether the domain will actually resolve to IPv4 AND IPv6 addresses so it simply assigns 4 of each to every domain. Thus, when receiving an IPv6 packet for such a DNS resource, the Gateway may only have IPv4 addresses available and can therefore not route the packet (unless it translates it).
This problem is not novel. In fact, an IP being unroutable or a particular route disappearing happens all the time on the Internet. ICMP was conceived to handle this problem and it is doing a pretty good job at it. We can make use of that and simply return an ICMP unreachable error back to the client whenever it picks an IP that we cannot map to one that we resolved.
In this PR, we leave all of the NAT64 code intact and only add a feature-flag that - when active - sends aforementioned ICMP error. While offline (and thus also for our tests), the feature-flag evaluates to false. It is however set to
truein the backend, meaning on staging and later in production, we will send these ICMP errors.Once this is rolled out and indeed proving to be working as intended, we can simplify our codebase and rip out the NAT64 module. At that point, we will also have to adapt the test-suite.