-
Notifications
You must be signed in to change notification settings - Fork 4
Make upstream DNS work (nicely) #20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
+2,399
−2,122
Conversation
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
This implements DNS name compression (see the decription added in
dns/encode.cpp `encode_name()` for details).
This reduces the sizes of nearly all query responses
(because every response answer includes the question, and then uses the
name again for the response), and in some cases significantly so.
This is rather important for Session Router all of our names are 52 byte
pubkeys (plus the 4 or 5 byte tld), and so we are potentially running up
against the DNS 512-byte max message size.
(We should also enable EDNS to allow longer messages, but that is left
here as a FIXME and not yet implemented).
An example shows how the compression helps:
For example, an AAAA query for `localhost.sesh` responds with an answer
of:
;; QUESTION SECTION:
;localhost.sesh. IN AAAA
;; ANSWER SECTION:
localhost.sesh. 10 IN CNAME sh6tnpf84s885m8ygsjw7g8qjuo1jk7ydufiog8sjdtgkhb3w8iy.sesh.
sh6tnpf84s885m8ygsjw7g8qjuo1jk7ydufiog8sjdtgkhb3w8iy.sesh. 10 IN AAAA fd2e:7365:7368::1
The repeated question doesn't compress anything, of course, but once you
hit the answer, you start getting savings:
The repeated `localhost.sesh` in the first answer gets compressed from
16 bytes (without compression) to a 2 byte pointer (back to the same
address in the question).
The first PUBKEY.sesh (in the CNAME target) gets slightly reduced by
being able to encode the trailing `sesh` from 6 bytes uncompressed
(4+"sesh"+\0) to a 2-byte pointer (again back
into the question, pointing just at the sesh tld rather than the entire
lokinet.sesh value).
The pubkey.sesh. in the second answers gets hugely reduced: an
uncompressed 59 bytes (52+"pubkey"+4+"sesh"+0) becomes a simple 2-byte
pointer to the same name in the previous answer line.
For some queries like SRV records the savings are even potentially even
larger, especially when there are multiple SRV entries for a .sesh
address.
An empty name was getting encoded as two \0's instead of just one.
This adds support for EDNS requests, allowing for larger responses. As part of that, it adds support for handling DNS cookies, which are a sort of pseudo-mac over DNS requests to prevent out-of-path attackers from being able to forge responses.
- Greatly simplify the DNS code into a dns::Handler + dns::Listener
instead of 14 (approximately, it's hard to tell) intermixed classes
doing various things in needlessly complicated ways.
- Move all of the actual DNS record logic from tun.cpp (which was more
than half of tun.cpp!) into dns/handler.cpp.
- Remove DNSInterceptor (and l3-intercept config option), as it doesn't
work reliably even in current stable Lokinet. The only platform that
might actually need it is Android (eventually) but it can either be
resurrected or (more likely) an Android-specific interceptor built.
Other platforms have better alternatives than attempting IP packet
interception.
- Implement TCP DNS requests, so that we can successful return queries
that might not fit in a UDP packet (even with EDNS).
- Implement dns truncation bit, which is meant to tell an application
that the response is too large and it needs to retry via TCP (hence
the above addition):
- Catch the exception and set the truncation flag when we overrun the
reply buffer. Previously we were just not responding.
- Make sure we can't return sliced RR records in a truncated reply by
throwing away all the answer section if we run out of space.
- A TXT lookup for path.PUBKEY.{sesh,snode} now returns info about your
path in a TXT record.
- Replace the stdev calculation for paths with a jitter calculation as
that is probably slightly more relevant. (jitter = mean absolute
difference between sequential pings)
- Refactor the current_path methods to return more raw info rather that
stringifying early, deferring stringification to later parts where
needed (session_router, and now DNS). Also adds ping and expiry info
into it.
Example TXT info:
;; QUESTION SECTION:
;path.55fxrybf3jtausbnmxpgwcsz9t8qkf5pr8t5f4xyto4omjrkorpy.snode. IN TXT
;; ANSWER SECTION:
path.55fxrybf3jtausbnmxpgwcsz9t8qkf5pr8t5f4xyto4omjrkorpy.snode. 0 IN TXT "d=out;
path=55fxro86pzfka36f19qagzsuogk6qwek1g8hdgkx5menjn1s6iso@199.127.61.170
55fxne4863o4r4ohntbnx1baqupfgmghdw4sas5zbnxso4a9d1oy@172.93.103.156
55fxr5s76pmfaqfrhxbyfgnkcueok8j6a8c6fci9ts8axmwgmaoo@206.221.176.9
55fxrybf3jtausbnmxpgwcsz9t8qkf5pr8t5f4xy" "to4omjrkorpy@23.88.6.250;
ttl=1160; p=116; pj=0.714; pr=8; pt=0; pT=0"
where:
- `d` is `in`/`out`/`none` showing the session direction.
When `d` is not `none` this is followed by:
- `path=...` space-separated PUBKEY@IP values along the path
- `ttl=...` seconds remaining before this path expires
- `p=...` average path ping time (in milliseconds)
- `pj=...` average path jitter (milliseconds, with three decimal points)
- `pr=...` number of ping responses
- `pt=...` number of ping timeouts
- `pT=...` number of recent ping timeouts (i.e. 3 means the last 3 pings
have timed out).
This solves issues during destruction where we could get a segfault or a deadlock because of race conditions between connection close and link endpoint destruction. Basically what happened was: - during Router stop, link_endpoint gets destroyed. - that destruction closes all the connections, firing the on_conn_closed callbacks during destruction. - those callbacks were capturing a shared_ptr to the closed connection in a lambda job pushed onto the router loop - that router loop job can't run yet because we're already inside a router job doing the shutdown that triggered all of this - we get "router is stopped" - the queued cleanup code fires. There were multiple things wrong here: - trying to call `selected_alpn` (or `remote()`) would crash or deadlock because the network loop was gone, but those accessors go through a call_get. - the cleanup job had an invalid `this` pointer because link_endpoint was gone. - the cleanup job was also capturing a shared_ptr<Connection> to a Connection tied to the link_endpoint's loop, and so attempting to destroy that lambda (even if we early return) could crash or deadlock because the loop that handles the Connection destructor isn't around anymore. This reworks it to fix it: - adds a shared_ptr canary we capture to make sure we are still alive, to early-return from the lambda if `this` isn't valid anymore. - captures all the conn details we need in the lambda, rather than capturing a shared_ptr to get it out of inside the lambda. - changes all the "is the conn pointer the same" logic to use the reference id rather than pointer (since we can't keep the conn pointer anymore).
This make upstream requests via unbound work again.
- Make use of libunbound's event API to attach unbound requests directly
to our event loop.
- Properly rewrite the response from libunbound to apply the upstream
EDNS and whatnot
- Delete l3-intercept from config -- the code was already gone, this just removes
the option from the config as well.
- Rename `[dns]:bind` to `[dns]:listen`
- Allow an explicit empty `listen=` to disable DNS. (This is
potentially useful for reachable remote clients that do not need to
establish outbound sessions, i.e. pure hidden services).
- Set upstream defaults to Quad9. They already were using 9.9.9.10, but
this changes them to use all four addresses (primary+alt ipv4+ipv6),
and switches to the malware-blocking 9.9.9.9 rather than the raw .10.
- rename [dns]:add-hosts to [dns]:unbound-hosts, and:
- make it default to the system hosts file
- make it non-multi because unbound only supports one (and previously
specifying multiple would end up only using the last one)
- replace the "undeclared" handler with `[dns]:unbound` so that you now use:
`unbound=some-unbound-option: value`
rather than polluting the [dns] section with unbound options.
- Use a thread for background processing
- Fix DNS startup failure not failing router startup
- Global log info listening DNS addresses
- Add (and test) example of enabling DNSSEC. Not enabled by default
because we don't want to have to ship a root.key file, but if you have
one, this works.
- Fix OPT RR missing from forwarded unbound responses, and fix the
missing name inside it.
- Fix DO/AD flag handling in unbound responses doing an extra (unwanted)
byte swap.
6d4b384 to
4667df1
Compare
This was referenced Nov 19, 2025
tewinget
approved these changes
Nov 25, 2025
62a61a5
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.
This make upstream requests via unbound work again.
[dns]:bindto[dns]:listenlisten=to disable DNS. (This is potentially useful for reachable remote clients that do not need to establish outbound sessions, i.e. pure hidden services).[dns]:unboundso that you now use:unbound=some-unbound-option: valuerather than polluting the [dns] section with unbound options.