Make resolving HTTPS DNS records reliable:#21175
Conversation
- allow to specify when they are wanted on starting a resolve - match dns cache entries accordingly. An entry which never tried to get HTTPS-RRs is no answer for a resolve that wants it. - fix late arrivals of resolve answers to match the "async" records that started them - if it still exists. - provide for multiple "async" resolves in a transfer at the same time. We may need to resolve an IP interface while the main connection resolve has not finished yet. - allow lookup of HTTPS-RR information as soon as it is available, even if A/AAAA queries are still ongoing. For this, the "async" infrastructure is changed: - Defined bits for DNS queries `CURL_DNSQ_A`, `CURL_DNSQ_AAAA` and `CURL_DNSQ_HTTPS`. These replace `ip_version` which says nothing about HTTPS. Use them in dns cache entries for matching. - enhance the `async->id` to be a unique `uint32_t` for resolves inside one multi. This is weak, as the id may wrap around. However it is combined with the `mid` of the easy handle, making collisions highly unlikely. `data->state.async` is only accessed in few places where the mid/async-id match is performed. - vtls: for ECH supporting TLS backends (openssl, rustls, wolfssl), retrieve the HTTPS-RR information from the dns connection filter. Delay the connect if the HTTPS-RR is needed, but has not been resolved yet. The implementation of all this is complete for the threaded resolver. c-ares resolver and DoH do not take advantage of all new async features yet. To be done in separate PRs. Details: c-ares: cleanup settings and initialisation. Any ares channel is only being created on starting a resolve and propagating operations in setopt.c to the channel are not helpful. Changed threaded+ares pollset handling so that they do not overwrite each others `ASYNC_NAME` timeouts. Add trace name 'threads' for tracing thread queue and pool used by threaded resolver.
- do not query HTTPS for ip addresses - do not AAAA ipv4 addresses when A records are queried as well
There was a problem hiding this comment.
Pull request overview
This PR refactors curl’s DNS resolution “async” infrastructure to make HTTPS RR (SVCB/HTTPS) handling more reliable and to support multiple concurrent resolve operations per transfer, enabling TLS/ECH backends to consume HTTPS-RR data as soon as it becomes available.
Changes:
- Replace
ip_version-based resolve/cache matching withCURL_DNSQ_*(A/AAAA/HTTPS) query bitmasks and track per-entry “queries requested” vs “responses available”. - Introduce per-multi unique-ish
resolv_idplumbing so multiple async resolves can run concurrently and late results can be matched correctly. - Update vtls backends (OpenSSL/rustls/wolfSSL) to fetch HTTPS-RR via the DNS connection filter and optionally delay connect until HTTPS-RR is resolved for ECH.
Reviewed changes
Copilot reviewed 39 out of 39 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/libtest/lib655.c | Improves resolver-start callback test diagnostics by enabling verbose debug tracing. |
| lib/vtls/wolfssl.c | Switch ECH HTTPS-RR sourcing to cf-dns and add “delay connect until HTTPS-RR arrives” logic. |
| lib/vtls/rustls.c | Switch ECH HTTPS-RR sourcing to cf-dns and add “delay connect until HTTPS-RR arrives” logic. |
| lib/vtls/openssl.c | Switch ECH HTTPS-RR sourcing to cf-dns and add “delay connect until HTTPS-RR arrives” logic. |
| lib/urldata.h | Removes per-easy next_async_id (IDs now come from multi). |
| lib/url.c | Uses new resolve cleanup entry point on easy close. |
| lib/thrdqueue.h | Simplifies tracing API signature for thread queue trace. |
| lib/thrdqueue.c | Routes thread queue tracing through new threads trace feature. |
| lib/thrdpool.h | Simplifies tracing API signature for thread pool trace. |
| lib/thrdpool.c | Routes thread pool tracing through new threads trace feature. |
| lib/socks.c | Updates SOCKS resolver usage to new dns_queries + resolv_id APIs. |
| lib/setopt.c | Stops pushing DNS options immediately into a live ares channel (channel now created per-resolve). |
| lib/request.c | Moves resolve cleanup from DoH-specific close to generic resolve teardown on hard reset. |
| lib/multihandle.h | Adds last_resolv_id counter used to assign resolve IDs. |
| lib/multi.c | Uses new “shutdown all resolves” entry point during multi done. |
| lib/httpsrr.h | Adds mandatory/complete flags, updates setter signatures, and adds tracing helper. |
| lib/httpsrr.c | Adds verbose HTTPS-RR tracing and updates parsing helpers to new signatures/flags. |
| lib/hostip6.c | Updates sync resolver to use dns_queries to decide PF_UNSPEC vs PF_INET/PF_INET6. |
| lib/hostip4.c | Updates sync resolver signature to accept dns_queries (unused for IPv4-only path). |
| lib/hostip.h | Introduces CURL_DNSQ_* flags, new resolve APIs (IDs, destroy/shutdown/all), and query-string helper. |
| lib/hostip.c | Core resolve refactor: multiple async resolves per easy, per-resolve IDs, cache matching by query bits, announce callback factoring. |
| lib/ftp.c | Updates FTP resolve call sites to new dns_queries API. |
| lib/easy.c | Relies on Curl_req_hard_reset() to clear resolve state during curl_easy_reset(). |
| lib/doh.h | Plumbs DoH result-taking by resolv_id/async context; removes exported Curl_doh_close(). |
| lib/doh.c | Matches late DoH replies by resolve ID; adds per-query DoH issuance based on dns_queries. |
| lib/dnscache.h | Extends cache entries to track dns_queries and dns_responses; updates API signatures accordingly. |
| lib/dnscache.c | Implements query/response-based cache matching; adds helper to attach HTTPS-RR to entries. |
| lib/curl_trc.h | Declares new threads trace feature (USE_THREADS builds). |
| lib/curl_trc.c | Defines/registers new threads trace feature. |
| lib/connect.c | Requests HTTPS-RR resolution for FIRSTSOCKET when USE_HTTPSRR is enabled. |
| lib/cf-socket.c | Updates bind-local resolution to new dns_queries API. |
| lib/cf-https-connect.c | Removes dead/placeholder “need_https_rr” block in HTTPS CONNECT filter. |
| lib/cf-dns.h | Extends cf-dns APIs to accept dns_queries and exposes Curl_conn_dns_resolved_https(). |
| lib/cf-dns.c | Tracks resolve IDs in cf-dns and supports reading HTTPS-RR from in-flight async resolve. |
| lib/asyn.h | Reshapes async API to be per-resolve (id-based) and adds HTTPS-RR “knows/get” hooks. |
| lib/asyn-thrdd.c | Threaded resolver refactor: per-query tracking, resolve-id matching, optional HTTPS-RR ares subquery. |
| lib/asyn-base.c | Adds shared async time-left helper and new Curl_ares_timeout_ms() helper. |
| lib/asyn-ares.c | Refactors ares setup to be per-resolve; updates callbacks to use async context and query bits. |
| docs/libcurl/curl_global_trace.md | Documents new threads trace feature. |
Comments suppressed due to low confidence (2)
lib/doh.c:1177
- doh_resp_decode_httpsrr() never sets the new
Curl_https_rrinfo.completeflag, so Curl_httpsrr_trace() will log "not available" and any code relying oncompletewill treat the parsed HTTPS-RR as incomplete. Setlhrr->complete = TRUE(or!result) once parsing finishes successfully (and ideally call Curl_httpsrr_trace() after setting it, not inside the loop).
lhrr->port = -1; /* until set */
while(len >= 4) {
pcode = doh_get16bit(cp, 0);
plen = doh_get16bit(cp, 2);
cp += 4;
len -= 4;
if(pcode < expected_min_pcode || plen > len) {
result = CURLE_WEIRD_SERVER_REPLY;
goto err;
}
result = Curl_httpsrr_set(lhrr, pcode, cp, plen);
if(result)
goto err;
Curl_httpsrr_trace(data, lhrr);
cp += plen;
len -= plen;
expected_min_pcode = pcode + 1;
}
DEBUGASSERT(!len);
*hrr = lhrr;
return CURLE_OK;
lib/vtls/rustls.c:1006
- init_config_builder_ech() declares
struct Curl_dns_entry *dns = NULL;and unlinks it in cleanup, but dns is never assigned anymore after switching to Curl_conn_dns_get_https(). This will trigger an unused-variable warning (and the unlink is dead code). Please remove the dns variable and the corresponding cleanup block.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- make https_rrinfo member `port` and `uint16_t` and add bit `port_set`. Uses less space and simplifies initial struct values and checks. - openssl.c: use const where const is due - wolfssl.c: use const where const is due, use consistent ifdef checks
|
augment review |
🤖 Augment PR SummarySummary: This PR refactors libcurl’s async resolver plumbing to make HTTPS (SVCB/HTTPS-RR) DNS record resolution more reliable and consumable earlier, primarily to support ECH configuration discovery. Changes:
Technical Notes: Threaded resolver functionality appears complete; c-ares and DoH are partially adapted and further feature parity work is deferred to follow-up PRs. 🤖 Was this summary useful? React with 👍 or 👎 |
For this, the "async" infrastructure is changed:
CURL_DNSQ_A,CURL_DNSQ_AAAAandCURL_DNSQ_HTTPS. These replaceip_versionwhich says nothing about HTTPS. Use them in dns cache entries for matching.async->idto be a uniqueuint32_tfor resolves inside one multi. This is weak, as the id may wrap around. However it is combined with themidof the easy handle, making collisions highly unlikely.data->state.asyncis only accessed in few places where the mid/async-id match is performed.The implementation of all this is complete for the threaded resolver. c-ares resolver and DoH do not take advantage of all new async features yet. To be done in separate PRs.
Details:
c-ares: cleanup settings and initialisation. Any ares channel is only being created on starting a resolve and propagating operations in setopt.c to the channel are not helpful.
Changed threaded+ares pollset handling so that they do not overwrite each others
ASYNC_NAMEtimeouts.Add trace name 'threads' for tracing thread queue and pool used by threaded resolver.