feat(connlib): clients make use of DNS mangling on gateways #5049

conectado · 2024-05-20T20:50:25Z

This PR is the "client-side" of things for #4994. Up until now, when a user wanted to connect to a DNS resource, we would establish a connection to the gateway and pass along the domain we are trying to access. The gateway would resolve that domain and send the response back to the client, allowing them to finally send a DNS response.

Now, we instantly assign and respond with 4x A and 4x AAAA records to any query for one of our DNS resources. Upon the first IP packet for one of these "proxy IPs", we select a gateway, establish a connection and send our proxy IPs along. The gateway then performs the necessary mangling and NATing of all packets. See #5354 for details.

vercel · 2024-05-20T20:50:27Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
firezone	⬜️ Ignored (Inspect)	Visit Preview		Jun 20, 2024 5:40am

github-actions · 2024-05-20T20:52:12Z

Terraform Cloud Plan Output

Plan: 15 to add, 23 to change, 15 to destroy.

Terraform Cloud Plan

github-actions · 2024-05-20T21:01:16Z

Performance Test Results

TCP

Test Name	Received/s	Sent/s	Retransmits
direct-tcp-client2server	232.6 MiB (-1%)	235.3 MiB (-0%)	270 (-22%)
direct-tcp-server2client	240.2 MiB (+2%)	241.7 MiB (+2%)	324 (-7%)
relayed-tcp-client2server	236.6 MiB (-3%)	237.4 MiB (-3%)	408 (+29%)
relayed-tcp-server2client	245.0 MiB (+3%)	246.2 MiB (+2%)	842 (+36%)

UDP

Test Name	Total/s	Jitter	Lost
direct-udp-client2server	500.0 MiB (-0%)	0.04ms (+58%)	44.99% (+2%)
direct-udp-server2client	500.0 MiB (+0%)	0.01ms (-46%)	24.84% (-10%)
relayed-udp-client2server	500.0 MiB (+0%)	0.03ms (-24%)	54.05% (+1%)
relayed-udp-server2client	500.0 MiB (-0%)	0.02ms (-66%)	36.32% (+5%)

…way` (#5160) To encode that clients always have both ipv4 and ipv6 and they are the only allowed source ips for any given client, into the type, we split those into their specific fields in the `ClientOnGateway` struct and update tests accordingly. Furthermore, these will be used for the DNS refactor for ipv6-in-ipv4 and ipv4-in-ipv6 to set the source ip of outgoing packets, without having to do additional routing or mappings. There will be more notes on this on the corresponding PR #5049 . --------- Co-authored-by: Thomas Eizinger <thomas@eizinger.io>

jamilbk · 2024-06-02T17:55:03Z

Leaving a note here to delay merging/releasing this until 2 weeks after announcement is made to current users. Planning to send that out this week.

thomaseizinger

Did an early pass. Looks pretty good already! Curious to hear whether my new tests found any bugs!

rust/connlib/shared/src/messages.rs

rust/connlib/tunnel/src/gateway.rs

rust/connlib/tunnel/src/lib.rs

rust/connlib/tunnel/src/peer.rs

rust/connlib/tunnel/src/client.rs

thomaseizinger · 2024-06-05T03:41:03Z

Leaving a note here to delay merging/releasing this until 2 weeks after announcement is made to current users. Planning to send that out this week.

@jamilbk We discussed an idea today that would allow us to make a comparatively easy, backwards-compatible change to the gateway that allows old and new clients to connect at the same time. We can roll this out early, and tell everyone to upgrade. After a week or two, we can then merge the client changes which then require a new gateway.

This should make deployment easier because nothing has to be upgraded in lockstep.

jamilbk · 2024-06-05T04:47:29Z

Leaving a note here to delay merging/releasing this until 2 weeks after announcement is made to current users. Planning to send that out this week.

@jamilbk We discussed an idea today that would allow us to make a comparatively easy, backwards-compatible change to the gateway that allows old and new clients to connect at the same time. We can roll this out early, and tell everyone to upgrade. After a week or two, we can then merge the client changes which then require a new gateway.

This should make deployment easier because nothing has to be upgraded in lockstep.

This would be amazing. Let's opt for that if we can to avoid production disruptions.

thomaseizinger · 2024-06-05T05:17:46Z

Leaving a note here to delay merging/releasing this until 2 weeks after announcement is made to current users. Planning to send that out this week.

@jamilbk We discussed an idea today that would allow us to make a comparatively easy, backwards-compatible change to the gateway that allows old and new clients to connect at the same time. We can roll this out early, and tell everyone to upgrade. After a week or two, we can then merge the client changes which then require a new gateway.
This should make deployment easier because nothing has to be upgraded in lockstep.

This would be amazing. Let's opt for that if we can to avoid production disruptions.

The plan is:

Make the deserialisation of the message in the gateway handle both messages, new and old (i.e. with IPs and without)
In case of old, default to 0 IPs
Make the behaviour in the gateway such that no addresses means no translation, i.e. the current behaviour

What I would like to avoid is having a state of main that doesn't allow us to release. Hence, we would split this PR into two:

Only build the new packet translation logic in the gateway with the above behaviour to allow old clients to function. Compatibility tests and manual testing should give us high-confidence that this indeed works.
Make a 2nd, stacked PR that includes all the client changes. CI in that PR should tell us whether the new behaviour works as intended.

We shouldn't merge PR (1) until CI in PR (2) is green. In fact, I think we should continue to develop the new feature in this PR until we know it is working. Then we can see if a PR only changing the gateway also passes CI.

The window inbetween merging these two PRs is essentially the timeframe that customers have to upgrade their gateways (assuming we make a release soon-ish after each PR merges).

Currently, the transition for sending ICMP packets does not explicitly state whether we want to send an IPv4 or an IPv6 packet. Being explicit about this makes things a bit easier to understand. It may also simplify the adaption of the tests for #5049.

jamilbk · 2024-06-05T05:31:35Z

The window inbetween merging these two PRs is essentially the timeframe that customers have to upgrade their gateways (assuming we make a release soon-ish after each PR merges).

Got it. Aiming for 2 weeks for this one, so ideally the first PR will land end of this week / early next (Gateway 1.1), we'll make the announcement, and the 2nd PR would sometime end of June (Clients 1.1).

That means I probably need to refactor CI to split off the client releases from elixir/gateway releases. #4397

cc @conectado

With #5049, connlib will start mangling and translating ICMP requests which means we can no longer rely on the ICMP request emitted by the gateway to have the same sequence number and identifier as originally generated by the client. In the end, that is actually also not a property we care about. What we do care about is that an ICMP request results in an ICMP reply and that _those_ have a matching sequence number and identifier. Additionally, every ICMP request arriving at the gateway should target the correct resource. For CIDR resources, we already now which IP that should be. For DNS resources, it has to be one of the resolved IPs for the domain.

With #5049, we will allocate a fixed set of 4 IPs per DNS resource on the client. In order to ensure that this always works correctly, increase the number of resolved IPs to at most 6.

rust/connlib/tunnel/src/tests.rs

thomaseizinger · 2024-06-19T23:44:23Z

rust/connlib/tunnel/src/client.rs

+        self.peers.insert(
+            GatewayOnClient::new(gateway_id, &ips, HashSet::from([resource_id])),
+            &[],
+        );


@conectado Because we need access to awaiting_connection_details (which we remove in this function now) in order to create the GatewayOnClient in peers, I had to move the creation of the peer into this function.

This is not a working commit, though it does compiles. With this refactor, each time a DNS query for one of our resources is seen we return a unique set of ips for it. Might be the case we have seen the resource before, that means we return forever the same one. Now we don't initiate a connection when we see the query but we defer that until a later time since mangling will be done in the gateway. remove dns_internal_ips send intent based on ip refactor(connlib): remove translation on client peers refactor(connlib): keep track of the fqdn and ips in the awaiting connection details and send that up to the gateway chore(connlib): remove unused function refactor(connlib): have the gateway translate from the proxy ips to the real ip addresses for DNS refactor(connlib): do nat propperly with source port and translate destination ip refactor(connlib): expire nat table after not seeing packet for 1 min fix(connlib): delete the correct table entry when expirying nat feat(connlib): add initial ipv4-to-ipv6 translation methods feat(connlib): add ipv6 to ipv4 header translation for non-fragmented packets chore(connlib): remove unused functions fix(connlib): set translation ips in gateways for reuse connections refactor(connlib): add padding for packets to be able to convert them feat(connlib): implement ipv6 to ipv4 conversion with icmpv6 to icmpv4 still missing fix(connlib): icmp conversion feat(connlib): set ipv4 for ipv6 translation if ipv6 is empty and vice-versa chore(connlib): split allowed_ips into ipv4 and ipv6 in `ClientOnGateway` To encode into the types that clients always have both ipv4 and ipv6 and they are the only allowed source ips for any given client, we split those into their specific fields in the `ClientOnGateway` struct and update tests accordingly. Furthermore, these will be used for the DNS refactor for ipv6-in-ipv4 and ipv4-in-ipv6 to set the source ip of outgoing packets, without having to do additional routing or mappings. There will be more notes on this on the corresponding PR. fix(connlib): don't shorten packets after decapsulation fix(connlib): poll_read to account for additional 20 bytes fix(connlib): payload_mut for convertable ip packets fix(connlib): correctly set source instead of destination for translate_source fix(connlib): ipv4 to ipv6 payload length calculation fix(connlib): icmp echo and reply fix(rust): to_owned for convertable packets chore(connlib): add todo chore(connlib): remove stray dbgs fix(connlib): always ensure source before transforming packets fix(connlib): obtain proxy ips fix(connlib): send proxy ips for reusing resource feat(connlib): refresh mapping after 30 seconds of no incoming traffic fix semantic conflicts fix(connlib) get proptests to compile encode new test logic fix(connlib): don't generate gaps for proxy-ips remove unused dependency fix(connlib): fix rust warnings fix(connlib): clippy lints rename DnsMap to StubDns move dns_resources within stub_dns rename peer transform functions to keep same scheme fix typo rename StubDns to StubResolver simplify ip mapping fix gateway compilation on windows fix warning message unify decapsulate functions in the gateway unify decapsulate functions in the gateway enable deserialization of old message format add backward compatibility test make gateway handle backward compatible messages Fix compile errors Fix itertools dep Merge `pick_dns_resource` into `dns_resource_by_domain` Use `sorted_by_key` apply pr feedback address PR feedback simplify nat port assign Construct `outside` only once Use `inspect_err` instead of `match` Update NAT logs Only log if NAT entry was present Fix typos Fix warning Minor updates to tests Make errors around unsupported IP protocols more obvious Rename functions for consistency Ad some more docs Add some basic proptests for NAT table Fix test

github-actions bot added the kind/refactor Code refactoring label May 20, 2024

conectado force-pushed the refactor/dns-translation-on-gateway branch from 279f273 to 885d0c2 Compare May 20, 2024 21:16

conectado mentioned this pull request May 22, 2024

refactor(connlib): remove concept of "allowed IPs" from Peer #5077

Closed

thomaseizinger mentioned this pull request May 22, 2024

test(connlib): add RoamClient transition to state machine tests #5060

Closed

conectado force-pushed the refactor/dns-translation-on-gateway branch from 051462f to 5fb602f Compare May 28, 2024 04:16

conectado mentioned this pull request May 30, 2024

chore(connlib): split allowed_ips into ipv4 and ipv6 in ClientOnGateway #5160

Merged

conectado force-pushed the refactor/dns-translation-on-gateway branch from 03e0c32 to f5df55f Compare May 31, 2024 04:58

thomaseizinger reviewed Jun 3, 2024

View reviewed changes

conectado force-pushed the refactor/dns-translation-on-gateway branch from f5df55f to 55fe1aa Compare June 3, 2024 18:44

This was referenced Jun 4, 2024

test(connlib): refactor checked ICMP properties #5207

Merged

test(connlib): make the packet source an explicit input #5217

Merged

thomaseizinger mentioned this pull request Jun 5, 2024

test(connlib): generate up to 6 resolved IPs #5218

Merged

conectado force-pushed the refactor/dns-translation-on-gateway branch from 25a4532 to aa47f9b Compare June 6, 2024 02:58

thomaseizinger mentioned this pull request Jun 6, 2024

test(connlib): model overlapping DNS resources #5262

Closed

conectado mentioned this pull request Jun 7, 2024

STUN through Firezone #5274

Open

thomaseizinger reviewed Jun 10, 2024

View reviewed changes

rust/connlib/tunnel/src/tests.rs Outdated Show resolved Hide resolved

conectado force-pushed the refactor/dns-translation-on-gateway branch 2 times, most recently from 92864ca to 3425f6a Compare June 10, 2024 21:55

conectado force-pushed the refactor/dns-translation-on-gateway branch from dc58fc1 to 3b3594e Compare June 12, 2024 23:19

thomaseizinger force-pushed the refactor/dns-translation-on-gateway branch from d239369 to 340fc2a Compare June 13, 2024 02:44

conectado mentioned this pull request Jun 13, 2024

feat(gateway): NAT & mangling for DNS resources #5354

Merged

thomaseizinger mentioned this pull request Jun 13, 2024

Refactor DNS resources #4994

Open

thomaseizinger force-pushed the refactor/dns-translation-on-gateway branch 2 times, most recently from 9b76ed1 to 6b73955 Compare June 14, 2024 05:56

thomaseizinger changed the base branch from main to refactor/gateway-only-support-dns-translation June 14, 2024 05:56

thomaseizinger changed the title ~~refactor(connlib): dns translation on gateway~~ feat(connlib): clients make use of DNS mangling on gateways Jun 14, 2024

conectado force-pushed the refactor/gateway-only-support-dns-translation branch from deac45a to ab202c9 Compare June 15, 2024 02:00

conectado force-pushed the refactor/dns-translation-on-gateway branch from 6b73955 to 1f054d5 Compare June 15, 2024 04:30

thomaseizinger force-pushed the refactor/gateway-only-support-dns-translation branch from ab202c9 to 6aa74b1 Compare June 17, 2024 00:54

thomaseizinger force-pushed the refactor/dns-translation-on-gateway branch 2 times, most recently from 94b9ca3 to 043b684 Compare June 17, 2024 02:00

thomaseizinger mentioned this pull request Jun 17, 2024

refactor(connlib): tidy-up tech-debt around mangling of forwarded DNS queries #5391

Open

conectado force-pushed the refactor/gateway-only-support-dns-translation branch from 1fa85b5 to 3528a69 Compare June 19, 2024 00:52

thomaseizinger mentioned this pull request Jun 19, 2024

fix(connlib): mark ips as outdated instead of removing them from DNS internal mapping #5436

Closed

Base automatically changed from refactor/gateway-only-support-dns-translation to main June 19, 2024 23:30

thomaseizinger force-pushed the refactor/dns-translation-on-gateway branch from cba6435 to 56eeaeb Compare June 19, 2024 23:41

thomaseizinger reviewed Jun 19, 2024

View reviewed changes

thomaseizinger marked this pull request as ready for review June 20, 2024 01:27

thomaseizinger force-pushed the refactor/dns-translation-on-gateway branch from 56eeaeb to 98f567a Compare June 20, 2024 01:30

conectado and others added 8 commits June 20, 2024 15:38

refactor(gateway): support translation of dns

d26cafd

Fix bad index into buffer

f2a46e3

Make ensure_allowed on client match gateway-side design

33af47e

Make handle_timeout pure

dd3a80a

Remove DNS_REFRESH_INTERVAL const

c0e386a

Remove next_mangled_dns_expire field

ead8da9

Align test implementation of DNS resources with CIDR resources

fe6cced

thomaseizinger force-pushed the refactor/dns-translation-on-gateway branch from 98f567a to fe6cced Compare June 20, 2024 05:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(connlib): clients make use of DNS mangling on gateways #5049

feat(connlib): clients make use of DNS mangling on gateways #5049

conectado commented May 20, 2024 •

edited by thomaseizinger

vercel bot commented May 20, 2024 •

edited

github-actions bot commented May 20, 2024 •

edited

github-actions bot commented May 20, 2024 •

edited

jamilbk commented Jun 2, 2024

thomaseizinger left a comment •

edited

thomaseizinger commented Jun 5, 2024

jamilbk commented Jun 5, 2024

thomaseizinger commented Jun 5, 2024

jamilbk commented Jun 5, 2024

thomaseizinger Jun 19, 2024

feat(connlib): clients make use of DNS mangling on gateways #5049

Are you sure you want to change the base?

feat(connlib): clients make use of DNS mangling on gateways #5049

Conversation

conectado commented May 20, 2024 • edited by thomaseizinger

vercel bot commented May 20, 2024 • edited

github-actions bot commented May 20, 2024 • edited

Terraform Cloud Plan Output

github-actions bot commented May 20, 2024 • edited

Performance Test Results

TCP

UDP

jamilbk commented Jun 2, 2024

thomaseizinger left a comment • edited

Choose a reason for hiding this comment

thomaseizinger commented Jun 5, 2024

jamilbk commented Jun 5, 2024

thomaseizinger commented Jun 5, 2024

jamilbk commented Jun 5, 2024

thomaseizinger Jun 19, 2024

Choose a reason for hiding this comment

conectado commented May 20, 2024 •

edited by thomaseizinger

vercel bot commented May 20, 2024 •

edited

github-actions bot commented May 20, 2024 •

edited

github-actions bot commented May 20, 2024 •

edited

thomaseizinger left a comment •

edited