httpsrr: DoH with HTTPS, fix response handling

Fix handling of DoH response that only asks for HTTPS records. Add test 2117 for checking that a HTTPS-RR resolve is processed, even though the actual answer is invalid. Closes #22372
2026-07-26 05:37:20 +03:00 · 2026-07-23 11:20:19 +02:00 · 2026-07-23 11:20:19 +02:00 · a954d87f0b
commit a954d87f0b
parent 474ebb5247
9 changed files with 155 additions and 44 deletions
--- a/docs/libcurl/curl_global_trace.md
+++ b/docs/libcurl/curl_global_trace.md
@ -101,6 +101,11 @@ trace.

 Tracing of DNS operations to resolve hostnames and HTTPS records.

+## `doh`
+
+Tracing of DoH operations (DNS over HTTPS) to resolve hostnames and
+HTTPS records.
+
 ## `lib-ids`

 Adds transfer and connection identifiers as prefix to every call to
@ -116,10 +121,6 @@ connection. The command line tool `curl`uses the same format for its
 transfers but have no own way to identify in trace output which transfer
 a trace event is connected to.

-## `doh`
-
-Former name for DNS-over-HTTP operations. Now an alias for `dns`.
-
 ## `multi`

 Traces multi operations managing transfers' state changes and sockets poll
--- a/lib/curl_trc.c
+++ b/lib/curl_trc.c
@ -219,6 +219,12 @@ struct curl_trc_feat Curl_trc_feat_dns = {
  "DNS",
  CURL_LOG_LVL_NONE,
 };
+#ifndef CURL_DISABLE_DOH
+struct curl_trc_feat Curl_trc_feat_doh = {
+  "DoH",
+  CURL_LOG_LVL_NONE,
+};
+#endif
 struct curl_trc_feat Curl_trc_feat_timer = {
  "TIMER",
  CURL_LOG_LVL_NONE,
@ -530,6 +536,9 @@ static struct trc_feat_def trc_feats[] = {
  { &Curl_trc_feat_read,      TRC_CT_NONE },
  { &Curl_trc_feat_write,     TRC_CT_NONE },
  { &Curl_trc_feat_dns,       TRC_CT_NETWORK },
+#ifndef CURL_DISABLE_DOH
+  { &Curl_trc_feat_doh,       TRC_CT_NETWORK },
+#endif
  { &Curl_trc_feat_timer,     TRC_CT_NETWORK },
 #ifdef USE_THREADS
  { &Curl_trc_feat_threads,   TRC_CT_NONE },
@ -652,10 +661,6 @@ static CURLcode trc_opt(const char *config)
      trc_apply_level_by_category(TRC_CT_NETWORK, lvl);
    else if(curlx_str_casecompare(&out, "proxy"))
      trc_apply_level_by_category(TRC_CT_PROXY, lvl);
-    else if(curlx_str_casecompare(&out, "doh")) {
-      struct Curl_str dns = { "dns", 3 };
-      trc_apply_level_by_name(&dns, lvl);
-    }
    else
      trc_apply_level_by_name(&out, lvl);

--- a/lib/curl_trc.h
+++ b/lib/curl_trc.h
@ -307,6 +307,9 @@ extern struct curl_trc_feat Curl_trc_feat_multi;
 extern struct curl_trc_feat Curl_trc_feat_read;
 extern struct curl_trc_feat Curl_trc_feat_write;
 extern struct curl_trc_feat Curl_trc_feat_dns;
+#ifndef CURL_DISABLE_DOH
+extern struct curl_trc_feat Curl_trc_feat_doh;
+#endif
 extern struct curl_trc_feat Curl_trc_feat_timer;
 #ifdef USE_THREADS
 extern struct curl_trc_feat Curl_trc_feat_threads;
--- a/lib/doh.c
+++ b/lib/doh.c
@ -217,8 +217,8 @@ static void doh_print_buf(struct Curl_easy *data,
 * This looks up the probe response at its meta CURL_EZM_DOH_PROBE
 * and copies the response body over to the struct at the master's
 * meta at CURL_EZM_DOH_MASTER. */
-static void doh_probe_done(struct Curl_easy *data,
-                           struct Curl_easy *doh, CURLcode result)
+static void doh_probe_done(struct Curl_easy *doh,
+                           struct Curl_easy *master, CURLcode result)
 {
  struct Curl_resolv_async *async = NULL;
  struct doh_probes *dohp = NULL;
@ -227,13 +227,14 @@ static void doh_probe_done(struct Curl_easy *data,

  doh_req = Curl_meta_get(doh, CURL_EZM_DOH_PROBE);
  if(!doh_req) {
+    /* transfer `doh` is not a DoH probe. */
    DEBUGASSERT(0);
    return;
  }

-  async = Curl_async_get(data, doh_req->resolv_id);
+  async = Curl_async_get(master, doh_req->resolv_id);
  if(!async) {
-    CURL_TRC_DNS(data, "[%u] ignoring outdated DoH response",
+    CURL_TRC_DNS(master, "[%u] ignoring outdated DoH response",
                 doh_req->resolv_id);
    return;
  }
@ -246,7 +247,7 @@ static void doh_probe_done(struct Curl_easy *data,
  /* We really should have found the slot where to store the response */
  if(i >= DOH_SLOT_COUNT) {
    DEBUGASSERT(0);
-    failf(data, "DoH: unknown sub request done");
+    failf(master, "DoH: unknown sub request done");
    return;
  }

@ -267,8 +268,8 @@ static void doh_probe_done(struct Curl_easy *data,
    infof(doh, "DoH request %s", curl_easy_strerror(result));

  if(!dohp->pending) {
-    /* DoH completed, run the transfer picking up the results */
-    Curl_multi_mark_dirty(data);
+    /* DoH completed, run master to act on results */
+    Curl_multi_mark_dirty(master);
  }
 }

@ -344,7 +345,7 @@ static CURLcode doh_probe_run(struct Curl_easy *data,

  /* pass in the struct pointer via a local variable to please coverity and
     the gcc typecheck helpers */
-  VERBOSE(doh->state.feat = &Curl_trc_feat_dns);
+  VERBOSE(doh->state.feat = &Curl_trc_feat_doh);
  ERROR_CHECK_SETOPT(CURLOPT_URL, url);
  ERROR_CHECK_SETOPT(CURLOPT_DEFAULT_PROTOCOL, "https");
  ERROR_CHECK_SETOPT(CURLOPT_WRITEFUNCTION, doh_probe_write_cb);
@ -367,7 +368,7 @@ static CURLcode doh_probe_run(struct Curl_easy *data,
  ERROR_CHECK_SETOPT(CURLOPT_SHARE, (CURLSH *)data->share);
  if(data->set.err && data->set.err != stderr)
    ERROR_CHECK_SETOPT(CURLOPT_STDERR, data->set.err);
-  if(Curl_trc_ft_is_verbose(data, &Curl_trc_feat_dns))
+  if(Curl_trc_ft_is_verbose(data, &Curl_trc_feat_doh))
    ERROR_CHECK_SETOPT(CURLOPT_VERBOSE, 1L);
  if(data->set.no_signal)
    ERROR_CHECK_SETOPT(CURLOPT_NOSIGNAL, 1L);
@ -423,6 +424,7 @@ static CURLcode doh_probe_run(struct Curl_easy *data,

  doh->state.internal = TRUE;
  doh->master_mid = data->mid; /* master transfer of this one */
+  doh->sub_xfer_done = doh_probe_done;

  result = Curl_meta_set(doh, CURL_EZM_DOH_PROBE, doh_req, doh_probe_dtor);
  doh_req = NULL; /* call took ownership */
@ -479,9 +481,6 @@ CURLcode Curl_doh(struct Curl_easy *data,

  dohp->host = async->hostname;
  dohp->port = async->port;
-  /* We are making sub easy handles and want to be called back when
-   * one is done. */
-  data->sub_xfer_done = doh_probe_done;

  /* create IPv4 DoH request */
  if(async->dns_queries & CURL_DNSQ_A) {
@ -1207,7 +1206,8 @@ CURLcode Curl_doh_take_result(struct Curl_easy *data,
  if(!dohp)
    return CURLE_OUT_OF_MEMORY;

-  if(dohp->probe_resp[DOH_SLOT_IPV4].probe_mid == UINT32_MAX &&
+  if(CURL_DNSQ_IS_ADDR(async->dns_queries) &&
+     dohp->probe_resp[DOH_SLOT_IPV4].probe_mid == UINT32_MAX &&
     dohp->probe_resp[DOH_SLOT_IPV6].probe_mid == UINT32_MAX) {
    failf(data, "Could not DoH-resolve: %s", dohp->host);
    return async->for_proxy ?
@ -1237,7 +1237,9 @@ CURLcode Curl_doh_take_result(struct Curl_easy *data,
      if(rc[slot] && (rc[slot] != DOH_DNS_NXDOMAIN))
        negative = FALSE;
      if(rc[slot]) {
-        CURL_TRC_DNS(data, "DoH: %s type %s for %s", doh_strerror(rc[slot]),
+        CURL_TRC_DNS(data, "[%s] [DoH] error: %s type %s for %s",
+                     Curl_resolv_query_str(async->dns_queries),
+                     doh_strerror(rc[slot]),
                     doh_type2name(p->dnstype), dohp->host);
      }
    } /* next slot */
@ -1247,7 +1249,7 @@ CURLcode Curl_doh_take_result(struct Curl_easy *data,
        /* we have an address, of one kind or other */
        struct Curl_addrinfo *ai;

-        if(Curl_trc_ft_is_verbose(data, &Curl_trc_feat_dns)) {
+        if(Curl_trc_ft_is_verbose(data, &Curl_trc_feat_doh)) {
          CURL_TRC_DNS(data, "hostname: %s", dohp->host);
          doh_show(data, &de);
        }
@ -1283,6 +1285,7 @@ CURLcode Curl_doh_take_result(struct Curl_easy *data,
      /* Now add and HTTPSRR information if we have */
      struct Curl_https_rrinfo *hrr = NULL;

+      CURL_TRC_DNS(data, "[HTTPS] got %d records", de.numhttps_rrs);
      if(de.numhttps_rrs > 0 && result == CURLE_OK) {
        result = doh_resp_decode_httpsrr(data, de.https_rrs->val,
                                         de.https_rrs->len, &hrr);
@ -1338,11 +1341,12 @@ static void doh_close(struct Curl_easy *data,
                     doh->probe_resp[slot].probe_mid));
        continue;
      }
+      probe_data->sub_xfer_done = NULL; /* No longer interested in result */
      /* data->multi might already be reset at this time */
      Curl_multi_remove_handle(data->multi, probe_data);
      Curl_close(&probe_data);
    }
-    data->sub_xfer_done = NULL;
+    CURL_TRC_DNS(data, "[DoH] probe done");
  }
 }

--- a/lib/httpsrr.c
+++ b/lib/httpsrr.c
@ -98,15 +98,15 @@ void Curl_httpsrr_trace(struct Curl_easy *data,
  CURLcode result;

  if(!rr) {
-    CURL_TRC_DNS(data, "[HTTPS-RR] not available");
+    CURL_TRC_DNS(data, "[HTTPS] no record available");
    return;
  }
  curlx_dyn_init(&tmp, 1024);
  result = Curl_httpsrr_print(&tmp, rr);
  if(!result)
-    CURL_TRC_DNS(data, "HTTPS-RR: %s", curlx_dyn_ptr(&tmp));
+    CURL_TRC_DNS(data, "[HTTPS] record: %s", curlx_dyn_ptr(&tmp));
  else
-    CURL_TRC_DNS(data, "Error printing HTTPS-RR information");
+    CURL_TRC_DNS(data, "[HTTPS] error printing information");
  curlx_dyn_free(&tmp);
 }

--- a/lib/multi.c
+++ b/lib/multi.c
@ -2445,20 +2445,16 @@ static void handle_completed(struct Curl_multi *multi,
                             CURLcode result)
 {
  if(data->master_mid != UINT32_MAX) {
-    /* A sub transfer, not for msgsent to application */
-    struct Curl_easy *mdata;
+    /* A sub transfer, not for msgsent to application. Is anyone still
+     * interested in processing its results? */
+    if(data->sub_xfer_done) {
+      struct Curl_easy *master = Curl_multi_get_easy(multi, data->master_mid);

-    CURL_TRC_M(data, "sub xfer done for master %u", data->master_mid);
-    mdata = Curl_multi_get_easy(multi, data->master_mid);
-    if(mdata) {
-      if(mdata->sub_xfer_done)
-        mdata->sub_xfer_done(mdata, data, result);
+      CURL_TRC_M(data, "sub xfer done for master %u", data->master_mid);
+      if(master)
+        data->sub_xfer_done(data, master, result);
      else
-        CURL_TRC_M(data, "master easy %u without sub_xfer_done callback.",
-                   data->master_mid);
-    }
-    else {
-      CURL_TRC_M(data, "master easy %u already gone.", data->master_mid);
+        CURL_TRC_M(data, "master easy %u already gone.", data->master_mid);
    }
  }
  else {
--- a/lib/urldata.h
+++ b/lib/urldata.h
@ -1192,10 +1192,10 @@ struct UserDefined {
 #define IS_MIME_POST(a) FALSE
 #endif

-/* callback that gets called when a sub easy (data->master_mid set) is
-   DONE. Called on the master easy. */
-typedef void multi_sub_xfer_done_cb(struct Curl_easy *master_easy,
-                                    struct Curl_easy *sub_easy,
+/* callback that gets called when the transfer `data` is done and
+ * `data->master_mid` is set to an existing easy handle. */
+typedef void multi_sub_xfer_done_cb(struct Curl_easy *data,
+                                    struct Curl_easy *master,
                                    CURLcode result);

 /*
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@ -254,7 +254,7 @@ test2072 test2073 test2074 test2075 test2076 test2077 test2078 test2079 \
 test2080 test2081 test2082 test2083 test2084 test2085 test2086 test2087 \
 test2088 test2089 test2090 test2091 test2092 test2093 \
 test2100 test2101 test2102 test2103 test2104 test2105 test2106 test2107 \
-test2108 test2109 test2110 test2113 test2114 test2115 test2116 \
+test2108 test2109 test2110 test2113 test2114 test2115 test2116 test2117 \
 \
 test2200 test2201 test2202 test2203 test2204 test2205 test2206 test2207 \
 test2208 \
--- a/tests/data/test2117
+++ b/tests/data/test2117
@ -0,0 +1,102 @@
+<?xml version="1.0" encoding="US-ASCII"?>
+<testcase>
+<info>
+<keywords>
+HTTP
+HTTP GET
+DOH
+httpsrr
+</keywords>
+</info>
+
+# Server-side
+<reply>
+
+# This is the DoH response for foo.example.com A 127.0.0.1. It will be sent
+# for all DoH requests, so the AAAA and HTTPS resolves will fail with wrong
+# data. But it does verify that the responses are acted upon.
+# This requires that the test server is accessible at that address!
+
+<data1 base64="yes">
+SFRUUC8xLjEgMjAwIE9LCkRhdGU6IFRodSwgMDkgTm92IDIwMTAgMTQ6NDk6MDAgR01UClNlcnZl
+cjogdGVzdC1zZXJ2ZXIvZmFrZQpDb25uZWN0aW9uOiBjbG9zZQpDb250ZW50LVR5cGU6IGFwcGxp
+Y2F0aW9uL2Rucy1tZXNzYWdlCkNvbnRlbnQtTGVuZ3RoOiA0OQoKAAABAAABAAEAAAAAA2Zvbwdl
+eGFtcGxlA2NvbQAAAQABwAwAAQABAAAANwAEfwAAAQ==
+</data1>
+<data>
+HTTP/1.1 200 OK
+Date: Tue, 09 Nov 2010 14:49:00 GMT
+Server: test-server/fake
+Last-Modified: Tue, 13 Jun 2000 12:10:00 GMT
+ETag: "21025-dc7-39462498"
+Accept-Ranges: bytes
+Content-Length: 6
+Connection: close
+Content-Type: text/html
+Funny-head: yesyes
+
+-foo-
+</data>
+</reply>
+
+# Client-side
+<client>
+<server>
+http
+https
+</server>
+
+# requires Debug so that it can use the DoH server without https
+# requires IPv6 so that we can assume and compare both DoH requests
+
+<features>
+Debug
+DoH
+IPv6
+ECH
+HTTPSRR
+</features>
+<name>
+HTTP GET using DoH with mandatory HTTPS-RR response
+</name>
+<command>
+https://foo.example.com:%HTTPSPORT/%TESTNUMBER --insecure --ech true --connect-timeout 20 --doh-insecure --doh-url https://%HOSTIP:%HTTPSPORT/%TESTNUMBER0001
+</command>
+</client>
+
+# Verify data after the test has been "shot"
+<verify>
+
+# To make the test ignore the order of the two outgoing DoH requests, strip
+# the family byte
+
+<strippart>
+s/com\x00\x00(\x1c|\x01)/com-00-00!/g;
+</strippart>
+<protocol crlf="yes">
+POST /%TESTNUMBER0001 HTTP/1.1
+Host: %HOSTIP:%HTTPSPORT
+Accept: */*
+Content-Type: application/dns-message
+Content-Length: 47
+
+%hex[%00%00%01%00%00%01%00%00%00%00%00%00%06_%HTTPSPORT%06_https%03foo%07example%03com%00%00A%00%01]hex%POST /%TESTNUMBER0001 HTTP/1.1
+Host: %HOSTIP:%HTTPSPORT
+Accept: */*
+Content-Type: application/dns-message
+Content-Length: 33
+
+%hex[%00%00%01%00%00%01%00%00%00%00%00%00%03foo%07example%03com-00-00!%00%01]hex%POST /%TESTNUMBER0001 HTTP/1.1
+Host: %HOSTIP:%HTTPSPORT
+Accept: */*
+Content-Type: application/dns-message
+Content-Length: 33
+
+%hex[%00%00%01%00%00%01%00%00%00%00%00%00%03foo%07example%03com-00-00!%00%01]hex%GET /%TESTNUMBER HTTP/1.1
+Host: foo.example.com:%HTTPSPORT
+User-Agent: curl/%VERSION
+Accept: */*
+
+</protocol>
+</verify>
+</testcase>