feat(connlib): expand single-label queries using search-domain#8378
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
PR Overview
This PR adds support for expanding single-label DNS queries using a configurable account-wide search domain, enabling queries like "app" to be automatically expanded into FQDNs (e.g. "app.example.com").
- Introduces an optional search_domain field to the Interface message and configuration structures.
- Updates the stub resolver and associated tests to properly expand single-label queries using the search domain.
- Propagates the search_domain configuration through client state and simulation strategies.
Reviewed Changes
| File | Description |
|---|---|
| rust/connlib/tunnel/src/tests/stub_portal.rs | Extracts search domains from DNS resources for test generation. |
| rust/connlib/tunnel/src/tests/reference.rs | Adds handling for single-label queries in DNS query transitions. |
| rust/connlib/tunnel/src/dns.rs | Expands single-label queries using search-domain in the resolver. |
| rust/connlib/tunnel/src/tests/sim_client.rs | Propagates search_domain through simulation client strategies. |
| .github/workflows/_rust.yml | Updates workflow grep checks to include search-domain logs. |
| rust/connlib/tunnel/src/tests/sut.rs | Passes search_domain to test harness functions. |
| rust/connlib/tunnel/src/messages.rs | Includes search_domain field within the Interface message. |
| rust/connlib/tunnel/src/client.rs | Updates interface configuration and resolver settings to include search_domain. |
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
rust/connlib/tunnel/src/client.rs:1048
- [nitpick] The log message attribute 'search_domains' is inconsistent with the field name 'search_domain'. Consider renaming it to 'search_domain' for consistency.
tracing::trace!(upstream_dns = ?config.upstream_dns, search_domains = ?config.search_domain, ipv4 = %config.ipv4, ipv6 = %config.ipv6, "Received interface configuration from portal");
thomaseizinger
force-pushed
the
feat/match-search-domains
branch
from
4823113 to
fd0a9d6
Compare
thomaseizinger
force-pushed
the
feat/match-search-domains
branch
from
fd0a9d6 to
f479bd1
Compare
thomaseizinger
force-pushed
the
feat/match-search-domains
branch
from
f479bd1 to
757b383
Compare
There was a problem hiding this comment.
PR Overview
This PR extends the functionality to support search domains for expanding single-label DNS queries into FQDNs, making DNS resolution behavior consistent across platforms. Key changes include:
- Propagation of an optional search_domain field in configuration messages (e.g. Interface messages).
- Modifications in the DNS resolver logic to expand single-label queries using the search domain.
- Updates to tests, client code, and DNS control modules (Windows/Linux) to utilize and pass along the new search_domain field.
Reviewed Changes
| File | Description |
|---|---|
| rust/connlib/tunnel/src/tests/stub_portal.rs | Added generation of possible_search_domains and passing optional search domain in test strategy. |
| rust/connlib/tunnel/src/tests/reference.rs | Introduced functions to expand single-label queries using the search domain when present. |
| rust/connlib/tunnel/src/dns.rs | Updated resource matching logic to expand single-label DNS queries using search_domain; added setter. |
| rust/connlib/tunnel/src/client.rs | Introduced LLMNR constants and handling for LLMNR DNS queries; integrated search_domain in interface. |
| rust/headless-client/src/dns_control/windows.rs | Updated DNS control for Windows to accept and ignore (currently) search_domain parameter. |
| rust/headless-client/src/dns_control/linux.rs | Updated DNS control for Linux to pass search_domain to resolvectl and etc_resolv_conf utilities. |
| rust/connlib/tunnel/src/tests/sim_client.rs | Propagated search_domain parameter in test client state creation and DNS query handling. |
| Other files (messages, lib, callbacks, ipc_service, etc.) | Adjusted data structures and function signatures to include search_domain where applicable. |
Copilot reviewed 19 out of 19 changed files in this pull request and generated no comments.
| // Take the original qname out of the message. | ||
| // DNS queries should always respond for the exact same qname that was queried, even if we expanded a single-label domain. | ||
| let qname = message |
There was a problem hiding this comment.
This is interesting to point out. Previously, we took the parsed domain name from the query in passed it in separately to this function (despite the message being the query again).
Now that we turn single-label domains into FQDN, this actually broke the tests because upon querying for app, connlib suddenly returned app.example.com in the records.
I believe that is wrong and we should only ever return the exact domain name that was queried for, even if we internally expanded it to a FQDN.
| #[serde(default)] | ||
| pub upstream_dns: Vec<DnsServer>, | ||
| #[serde(default)] | ||
| pub search_domain: Option<DomainName>, |
There was a problem hiding this comment.
@jamilbk This is the field you need to set in the init message.
thomaseizinger
force-pushed
the
feat/match-search-domains
branch
from
916fcca to
f58a405
Compare
thomaseizinger
force-pushed
the
feat/match-search-domains
branch
from
f58a405 to
f40f68f
Compare
|
@jamilbk I've split out the commits a bit, should be reasonably easy to follow. |
thomaseizinger
force-pushed
the
feat/match-search-domains
branch
from
f40f68f to
17d412c
Compare
| let Some(datagram) = packet.as_udp() else { | ||
| tracing::debug!(?packet, "Not a UDP packet"); | ||
|
|
||
| return; | ||
| }; |
There was a problem hiding this comment.
Technically, we are also meant to support TCP for LLMNR queries but I didn't want to bother with that for now. Our responses never exceed the max UDP payload and we don't forward LLMNR queries to upstream resolvers because they are sent via multicast anyway and are only meant for the local network.
thomaseizinger
force-pushed
the
feat/match-search-domains
branch
from
17d412c to
1c3f9cb
Compare
thomaseizinger
force-pushed
the
feat/match-search-domains
branch
from
1c3f9cb to
58667d3
Compare
jamilbk
left a comment
jamilbk
left a comment
There was a problem hiding this comment.
Here it is on Apple: https://developer.apple.com/documentation/networkextension/nednssettings/searchdomains
Should be a matter of threading it up to onSetInterfaceConfig and we're done.
|
I may take a stab at this later today or over the weekend. |
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
I tried this by hacking a static value in and it did set it on the resolver but only for "scoped queries". The primary resolver at the top of Might need some more research to figure out what that actually does and how it interacts. |
|
Merging this to make further dev easier. It worked with a hard-coded domain and CI is green so I see little risk in merging this. |
Reverts part of #8378 so that our OS-native expansion takes effect on all platforms. --------- Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
Search domains are a way of performing a DNS lookup without typing the full-qualified domain name. For example, with a search domain of
example.com, performing a DNS query forappwill automatically expand the query toapp.example.com. At present, this doesn't work with Firezone because there is no way to configure an account-wide search-domain.With this PR, we extend the
Interfacemessage sent by the portal to also include an optionalsearch_domainfield that must be a valid domain name. If set,connlib's DNS stub resolver will now append this domain to all single-label queries and match the resulting domain against all active DNS resource.On Linux - with
systemd-resolvedas the DNS backend - we need to set the search domain on the TUN interface as well and enable LLMNR in order to be able to intercept these queries.resolvedexpands the query for us, however, meaning with this configuration, we don't actually receive a single-label query inconnlib. Instead, we directly seeapp.example.comwhen we typehost appordig +search appand haveexample.comas our search domain.MacOS has a similar system but with a different fallack. There, the operating system will first try all configured search domains on the system (typically just the ones set prior to Firezone starting), and send queries for FQDN to all resolvers. If none of the resolvers (including Firezone's stub resolver) return results, it sends the single-label query directly to the primary resolver. To handle this case, Firezone needs to know about the search-domain and expand it itself when it receives the single-label query. In the future, we may want to look into how we can configure MacOS such that it performs this expansion for us.
On Windows and Android, queries for a single-label domain will be directly sent to Firezone's stub resolver where we then hit the same codepath as explained above.
Specifically, the way this codepath works is that if we receive a single-label query AND we have a search-domain set, we expand it and match that particular query against our list of resources. In every other case, we continue on with the single-label domain.
Related: #8365
Fixes: #8377