Add DNS-over-QUIC (RFC 9250) up- and downstream - #3000
Conversation
81fc949 to
103729c
Compare
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
103729c to
48e0271
Compare
|
Conflicts have been resolved. |
80d53ea to
964bf88
Compare
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
DoQ is the last IETF standards-track encrypted DNS transport we did not speak, and the cheapest one to add: the DNS message rides a QUIC stream with the same 2-byte length prefix DoT already uses, so on top of the OpenSSL QUIC stack the DoH3 work brought in there is no HTTP layer at all. Both directions land together because they share that stack. 1. Outbound: `doq://host` becomes a `dns.upstreams` scheme (the `quic://` spelling AdGuard and dnsproxy configs use is accepted as an alias), defaulting to UDP port 853. The exchange is fail-closed exactly like DoT/DoH/DoH3 - a bad chain, a reset stream or a timeout drops the query so dnsmasq fails over rather than downgrading to plaintext. Per RFC 9250 Sec. 4.2.1 the Message ID goes out as 0 and dnsmasq's own ID is restored on the answer. 2. Inbound: `dns.doq` (default on, like `dns.dot`) brings up a listener on UDP/853 - the same port number DoT uses on TCP, which does not collide. It is a single-threaded event loop in the shape of the DoT listener: OpenSSL owns the QUIC transport while every in-flight query is a small non-blocking state machine, so a slow resolve never stalls another connection and a flood costs a bounded state record rather than a thread. Queries are attributed to the real downstream client through the same private-EDNS handoff as DoT/DoH, and answers are padded per RFC 8467 when the client asked for it. 3. The socket, handshake and timer plumbing the DoH3 client already carried moves into `quic_common.c`, so both QUIC clients share one context, one trust store and one fail-closed verify instead of duplicating them. DoQ is gated on the new `HAVE_QUIC` (OpenSSL >= 4.0) rather than `HAVE_HTTP3`, so a build without nghttp3 still speaks it. The two inbound reactors share a small pool of loopback sockets to dnsmasq (`dotdoh_loopback_take`/`_give`, capped at 16) rather than opening one per DoQ stream and per DoT connection. dnsmasq forks a child per TCP connection, so the old shape made the number of children scale with client behaviour: DoQ forked one per query, and a client opening a DoT connection per query forked one per query too - a burst of those exhausts dnsmasq's child slots, after which queries are read and never answered. Measured over 1000 queries per transport, pooling cuts DoQ's server-side overhead from ~4.7 ms to ~1.37 ms (in line with DoT and DoH) and its core latency from ~1.1 ms to ~0.65 ms, and a connection-per-query DoT client now completes instead of stalling. A pooled socket that dnsmasq closed after its own keep-alive limit is detected on checkout, and DoQ retries once on a fresh socket, as the DoT path already did. Only a socket sitting at a message boundary may go back into the pool. Both reactors can tear a connection down mid-exchange - the DoT deadline sweep and thread shutdown run through the same `conn_free()` - and pooling a socket with a half-written query or an unread answer would leave it out of step, so the next borrower would read the previous one's answer as its own. DoT therefore tracks an explicit idle flag and closes anything else, and `dotdoh_loopback_take()` treats readable bytes, EOF and errors alike as fatal: dnsmasq only ever writes an answer to a query we sent, so anything readable on an idle socket means exactly that desynchronisation. A signal-interrupted poll() is not evidence either way and keeps the socket. Worth calling out on the hardening side: QUIC address validation (Retry) stays on so we cannot be used to amplify towards a forged source, 0-RTT is explicitly disabled because a replayed early-data query would be answered and logged twice, only the `doq` ALPN is accepted, and connections, per-source connections, concurrent streams and per-connection queries are each capped. RFC 9250 Sec. 5.5.2 makes `edns-tcp-keepalive` a protocol error on DoQ, so a query carrying it closes the connection. One wrinkle needed solving: OpenSSL's QUIC API exposes the peer address but no per-connection *local* address, and the listener is wildcard-bound, so - unlike DoT and DoH - we cannot simply read off which of our addresses the client reached. Conveying nothing is not an option: the answer would then be built from the interface of our own loopback handoff, so `pi.hole` would resolve to 127.0.0.1 for every DoQ client, and a CNAME chain reaching it would write that into the shared cache record. We therefore ask the kernel which source address it would use to reach that peer - the address it would itself put on a reply datagram, and so the one a plain-DNS answer is built from - and convey that. Signed-off-by: DL6ER <dl6er@dl6er.de>
|
Conflicts have been resolved. |
a95ae28 to
68e7551
Compare
There was a problem hiding this comment.
Pull request overview
Adds full DNS-over-QUIC (DoQ, RFC 9250) support to FTL in both directions: outbound encrypted upstreams (doq:// plus quic:// alias) and an inbound DoQ listener on UDP/853. This builds on the existing encrypted-DNS plumbing by refactoring shared QUIC client transport code into a common module, and expands CI coverage with new DoQ end-to-end and regression tests.
Changes:
- Add inbound DoQ server (event-driven QUIC reactor) and outbound DoQ upstream client/pool, plus
dns.doqconfig. - Refactor shared QUIC socket/handshake/timer logic into
quic_common.*for reuse by DoH3 and DoQ clients. - Extend tests (BATS + shim + regression C + python client) for DoQ behavior (ALPN, msgid=0, padding, attribution), and adjust CI behavior on riscv64.
Reviewed changes
Copilot reviewed 32 out of 33 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test_final.bats | Adjust expected config write counts, including riscv64-specific behavior after skipping encrypted-DNS suites. |
| test/run.sh | Add DoQ readiness marker cleanup and skip encrypted-DNS test suites on riscv64. |
| test/pihole.toml | Document encrypted upstreamCA relevance for additional encrypted schemes and add dns.doq option. |
| test/dotdoh.bats | Add outbound DoQ proxy-path tests (resolve, msgid=0, padding, quic:// alias, fail-closed). |
| test/dotdoh_shim.py | Add optional aioquic-based DoQ server and new logging for DoQ msgid/proto. |
| test/dotdoh_server.bats | Add inbound DoQ server end-to-end tests (listener up, resolve, attribution, multiplexing, ALPN reject, malformed query, IPv6, pi.hole localization). |
| test/dotdoh_regression.c | Add URI parsing tests for doq/quic schemes and new EDNS option removal tests. |
| test/dotdoh_query.py | Add aioquic-driven DoQ client commands for inbound server tests (including multi-stream and ALPN refusal). |
| src/signals.c | Add thread name entry for inbound DoQ worker. |
| src/enums.h | Add DOTDOH_DOQ thread enum. |
| src/dotdoh/upstream_uri.h | Add UST_DOQ upstream type. |
| src/dotdoh/upstream_uri.c | Parse doq:// and quic:// as DoQ, default port 853, and enforce DoQ pathless URIs. |
| src/dotdoh/server.h | Expose loopback socket pooling API and DoQ inbound thread entrypoint. |
| src/dotdoh/server.c | Implement shared pool of loopback sockets to reduce dnsmasq TCP-child churn (used by DoT/DoQ). |
| src/dotdoh/quic_common.h | New shared QUIC client transport interface (ctx, timers, UDP connect, handshake, verify name binding). |
| src/dotdoh/quic_common.c | New shared QUIC client transport implementation, with OpenSSL>=4.0 gating. |
| src/dotdoh/quic_client.h | Remove duplicated QUIC global init API (now in quic_common). |
| src/dotdoh/quic_client.c | Switch DoH3 QUIC client to shared quic_common.* primitives. |
| src/dotdoh/proxy.c | Route UST_DOQ via new DoQ pool; initialize shared QUIC client context; include doq/quic schemes in encrypted-upstream count. |
| src/dotdoh/edns_pad.h | Add edns_has_option() and edns_remove_option() declarations for DoQ-related option stripping. |
| src/dotdoh/edns_pad.c | Implement generic EDNS option detection and removal with padding absorption when possible. |
| src/dotdoh/dot_server.c | Integrate loopback socket pool reuse for DoT and make pooling safe across teardown. |
| src/dotdoh/doq_server.c | New inbound DoQ QUIC reactor on UDP/853 with caps, attribution, padding, and protocol checks. |
| src/dotdoh/doq_client.h | New outbound DoQ pool interface mirroring existing upstream pool APIs. |
| src/dotdoh/doq_client.c | New outbound DoQ exchange implementation (msgid=0 on wire, fail-closed, keepalive option stripping). |
| src/dotdoh/CMakeLists.txt | Add new DoQ and QUIC-common sources to dotdoh object library. |
| src/dnsmasq_interface.c | Start inbound DoQ thread when dns.doq is enabled. |
| src/config/validator.c | Update upstream URI validation comments to include the new encrypted schemes. |
| src/config/config.h | Add dns.doq config item to the config struct. |
| src/config/config.c | Add dns.doq config item definition and update upstreamCA help text for new encrypted schemes. |
| src/CMakeLists.txt | Introduce HAVE_QUIC gating for DoQ based on OpenSSL QUIC availability independent of nghttp3. |
| src/api/docs/content/specs/config.yaml | Extend API config schema defaults to include dns.doq. |
| .gitignore | Ignore generated dotdoh_regression binary. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
`dns.dot` and `dns.doq` were booleans with the port hardcoded to 853 in `dot_server.c` and `doq_server.c`. Both become `CONF_UINT16` defaulting to `853`, with `0` disabling the listener. The keys keep their names, so whether a listener runs and which port it uses stay one setting - the same shape `dns.port` already has for plain DNS. The defaults are the assignments from RFC 7858, Sec. 3.1 (TCP/853 for DoT) and RFC 9250, Sec. 4.1.1 (UDP/853 for DoQ). The two still do not collide, as one is TCP and the other UDP. Each listener reads its port once when its thread starts instead of dereferencing `config` at every use, so the bind and the log line cannot disagree should the configuration be replaced underneath. The DoQ socket drops `SO_REUSEADDR`. It bought nothing on UDP, which has no `TIME_WAIT` to work around, but Linux does let two UDP sockets share an address once both set it - so `dns.doq = 53` would have bound alongside dnsmasq and quietly taken a share of its datagrams. Without the option that bind fails with `EADDRINUSE` and says so in the log, which is what the DoT listener has always done on TCP. There is deliberately no boolean-compatibility shim. Neither key has shipped in a release, and the TOML reader treats a type mismatch as an absent key. Anyone who set `dns.dot = false` while testing #3000 therefore gets the listener back on 853, with only a `DEBUG_CONFIG` line to say so, and `pihole.toml` is rewritten with the number. `dns.doh` stays a boolean, as it has no port of its own and rides `webserver.port`. `test_suite.bats` gains CLI assertions that both keys read back as `853` and that a boolean is rejected as a 16-bit unsigned integer. Neither reads nor rejected writes touch `pihole.toml`, so the write counters in `test_final.bats` are unaffected. No automated test binds a non-default port, as that needs an FTL restart mid-suite. Signed-off-by: DL6ER <dl6er@dl6er.de>
… alias RFC 9250, Sec. 4.2 gives each DNS query its own bidirectional stream. Bytes following that one message were discarded silently (`s->have = 0`), which is indistinguishable from a well-formed stream and would let a non-conformant client smuggle a second message past us. Treat them as the protocol violation they are and fail the stream instead. Not requiring the client's FIN before answering stays deliberate: the client is obliged to send it, but there is nothing to gain from withholding an answer we can already produce. `parse_upstream_uri()` has always accepted `quic://` as an alias for `doq://` (the spelling AdGuard and dnsproxy configs use), but the `dns.upstreamCA` help text and both `validate_upstreams()` comments listed only the other four schemes, so a copied `quic://` URI looked unsupported. Signed-off-by: DL6ER <dl6er@dl6er.de>
What does this implement/fix?
This adds DNS-over-QUIC (RFC 9250) in both directions, on top of the inbound DoT/DoH server in #2989 (this PR is stacked on
new/dotdoh-serverand should be reviewed after it).DoQ is the last IETF standards-track encrypted DNS transport we did not speak, and it is by far the cheapest one left to add: the DNS message rides a QUIC stream with the same 2-byte length prefix DoT already uses, so on top of the OpenSSL QUIC stack the DoH3 work brought in there is no HTTP layer at all. Up- and downstream land together because they share that stack.
Outbound.
doq://hostbecomes adns.upstreamsscheme, defaulting to UDP port 853; thequic://spelling AdGuard and dnsproxy configs use is accepted as an alias so a copy-pasted resolver address just works. The exchange is fail-closed exactly like DoT/DoH/DoH3 - a bad chain, a reset stream or a timeout drops the query so dnsmasq fails over to the next server rather than downgrading to plaintext. Per RFC 9250 Sec. 4.2.1 the Message ID goes out as 0 and dnsmasq's own ID is restored on the answer, which is what several public DoQ resolvers enforce.Inbound.
dns.doq(default on, likedns.dot) brings up a listener on UDP/853 - the same port number DoT uses on TCP, which does not collide because the transports differ. It is a single-threaded event loop in the shape of the DoT listener: OpenSSL owns the QUIC transport (handshake, loss recovery, flow control, address validation) while every in-flight query is a small non-blocking state machine, so a slow resolve never stalls another connection and a flood costs a bounded state record rather than a thread stack. Queries are attributed to the real downstream client through the same private-EDNS handoff DoT and DoH use, and answers are padded per RFC 8467 when the client asked for it.Shared plumbing. The socket, handshake and timer code the DoH3 client already carried moves into
quic_common.c, so both QUIC clients share one context, one trust store and one fail-closed verify instead of two copies. DoQ is gated on a newHAVE_QUIC(OpenSSL >= 4.0) rather thanHAVE_HTTP3, so a build without nghttp3 still speaks DoQ.On hardening: QUIC address validation (Retry) stays on so we cannot be used to amplify towards a forged source, 0-RTT is explicitly disabled because a replayed early-data query would be answered and logged twice, only the
doqALPN is accepted, and connections, per-source connections, concurrent streams and per-connection queries are each capped. RFC 9250 Sec. 5.5.2 makesedns-tcp-keepalivea protocol error on DoQ, so a query carrying it closes the connection.One wrinkle needed solving. OpenSSL's QUIC API exposes the peer address but no per-connection local address, and the listener is wildcard-bound, so - unlike DoT and DoH - we cannot simply read off which of our addresses a client reached. Conveying nothing is not an option: the answer is then built from the interface of our own loopback handoff, so
pi.holeresolves to 127.0.0.1 for every DoQ client, and a CNAME chain reaching it writes that into the shared cache record other clients are served from. So we ask the kernel which source address it would use to reach that peer - the address it would itself put on a reply datagram, and therefore the one a plain-DNS answer is built from - and convey that as the private EDNS destination option. On a normal LAN this is exactly the address the client used. Should OpenSSL ever expose the real per-connection local address, this becomes a one-line change.How to test the change during review
Automated coverage (all of it runs in CI):
test/dotdoh_regression.c-doq://andquic://URI parsing: defaults,sni@ippinning, bracketed IPv6, and the rejections (empty host, port 0/out of range, a path,doqx://not prefix-matchingdoq).test/dotdoh.bats- outbound: a query resolves over the DoQ proxy path against the aioquic DoQ server intest/dotdoh_shim.py(port 8854), the forwarded query is padded (RFC 8467), and every forwarded query carries Message ID 0 (RFC 9250). That the answer is still accepted proves we map dnsmasq's ID back.test/dotdoh_server.bats- inbound: the listener comes up on UDP/853, a query resolves, it is attributed to the real downstream client (sourced from127.0.0.4, which no other test uses, so the API evidence can only come from this query), several queries multiplex over one connection, a client not offering thedoqALPN is refused, a malformed query does not wedge the reactor, an IPv6 query resolves, and - the one that pins the destination hint down -pi.hole/AAAAover a v4 DoQ transport returns NODATA rather than leaking::1. I verified that last one discriminates by suppressing the hint and re-running: it fails withexpected NODATA but got 1 answer record(s)while its DoT twin still passes.Manual, against a running Pi-hole:
/api/queriesmust show thedoq://upstream, not127.47.11.N. Cross-check the resolver itself withkdig +quic @94.140.14.14 A example.com.dns.doq = trueand a validwebserver.tls.cert, query the Pi-hole from another machine:q -t A example.com quic://<pi-hole-ip>works as a second client.doq://upstream at a host presenting the wrong certificate and confirm the query is dropped (dnsmasq fails over) rather than answered.pi.holeover DoQ - from another machine,kdig +quic @<pi-hole-ip> -p 853 A pi.holemust return the Pi-hole address that machine reaches, not 127.0.0.1.nghttp3. The build must still reportDoQ (DNS-over-QUIC) support: YESandHTTP/3 support: NO, and DoQ must work. Against an OpenSSL older than 4.0 the build must succeed withDoQ (DNS-over-QUIC) support: NO;-Werrormakes that configuration easy to break, so it is worth checking.Related issue or feature (if applicable): N/A
Pull request in docs with documentation (if applicable): N/A
By submitting this pull request, I confirm the following:
git rebase)Checklist:
developmentbranch.