Repository navigation
feat(gateway): translate TimeExceeded ICMP messages - #9812
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
thomaseizinger
commented
Jul 10, 2025
thomaseizinger
left a comment
Member
Author
There was a problem hiding this comment.
@Intuinewin I've given this a review, I think it looks good so far. Thank you! Only a few minor things that need tweaking which should also fix CI. Would you mind opening a new PR against the contrib-icmp-ttl branch to address these?
thomaseizinger
force-pushed
the
contrib-icmp-ttl
branch
from
July 11, 2025 09:12
afab19c to
6b8a9e4
Compare
thomaseizinger
force-pushed
the
contrib-icmp-ttl
branch
from
July 11, 2025 10:12
10f891b to
38d8209
Compare
Member
Author
|
Currently blocked by #9834. |
Member
Author
@Intuinewin Does this match the behaviour in your testing? I guess it is better than not working at all but I do wonder whether there is a way we can make |
This was referenced Jul 11, 2025
thomaseizinger
force-pushed
the
contrib-icmp-ttl
branch
from
July 12, 2025 06:55
38d8209 to
cb32c7d
Compare
thomaseizinger
marked this pull request as ready for review
July 12, 2025 06:57
jamilbk
approved these changes
Jul 12, 2025
thomaseizinger
force-pushed
the
contrib-icmp-ttl
branch
from
July 12, 2025 20:18
939ef77 to
9bab4cd
Compare
This branch was successfully deployed
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
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.
In the DNS resource NAT table, we track parts of the layer 4 protocol of the connection in order to map packets back to the correct proxy IP in case multiple DNS names resolve to the same real IP. The involvement of layer 4 means we need to perform some packet inspection in case we receive ICMP errors from an upstream router.
Presently, the only ICMP error we handle here is destination unreachable. Those are generated e.g. when we are trying to contact an IPv6 address but we don't have an IPv6 egress interface. An additional error that we want to handle here is "time exceeded":
Time exceeded is sent when the TTL of a packet reaches 0. Typically, TTLs are set high enough such that the packet makes it to its destination. When using tools such as
tracepathhowever, the TTL is specifically only incremented one-by-one in order to resolve the exact hops a packet is taking to a destination. Without handling the time exceeded ICMP error, usingtracepaththrough Firezone is broken because the packets get dropped at the DNS resource NAT.With this PR, we generalise the functionality of detecting destination unreachable ICMP errors to also handle time-exceeded errors, allowing tools such as
tracepathto somewhat work:We say "somewhat work" because due to the NAT that is in place for DNS resources, the output does not disclose the intermediary hops beyond the Gateway.
Co-authored-by: Antoine Labarussias antoinelabarussias@gmail.com