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

diff --git a/docs/libcurl/curl_global_trace.md b/docs/libcurl/curl_global_trace.md
index f78f2ca0b06f2c9df4096d9947fbfccae644b3f3..e4cf8e05a56427ccca4de33530e51385e8a7ee7d 100644 (file)
--- 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

diff --git a/lib/curl_trc.c b/lib/curl_trc.c
index 1400707a7ad8af24d1f42133d8e1c018454c54de..dd3cd7bdd543aa554d809adee00a277730f69bce 100644 (file)
--- 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);
 

diff --git a/lib/curl_trc.h b/lib/curl_trc.h
index b4ae8e5314cd424b7efb2e04233f0e36b95ce35d..fc0e6dedcf9606fc00aae4c52eee45410b4471d0 100644 (file)
--- 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;

diff --git a/lib/doh.c b/lib/doh.c
index 776a96fea8866fb36ec59968cf6476fcd607ec6d..6d39f6ff0ebaec9b4d489c0c787ccfa68199a150 100644 (file)
--- 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");
   }
 }
 

diff --git a/lib/httpsrr.c b/lib/httpsrr.c
index a05a5cb5c15c86e498779214dd6820c8f7c2337b..441fe350b117fa56a8f791e159c20a89d8352d0f 100644 (file)
--- 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);
 }
 

diff --git a/lib/multi.c b/lib/multi.c
index 0732c7bfbae1ce41cc90fde6d07f7aea33b9d0df..77cb306d31ae1f8955967a1d24b678de86abc747 100644 (file)
--- 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;
-
-    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);
+    /* 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);
+      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 {

diff --git a/lib/urldata.h b/lib/urldata.h
index b0c7af23441588b5e316de4505b82123948a9383..5d6b3c2fdf4ae4628a1974e1de92e39a20f06bd9 100644 (file)
--- 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);
 
 /*

diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index fc15cbc399c6d867217b9490ba1b411c003d9829..2e001ec3c8ef91d26f31cf861605b764a7656c22 100644 (file)
--- 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 \

diff --git a/tests/data/test2117 b/tests/data/test2117
new file mode 100644 (file)
index 0000000..a5b800b
--- /dev/null
+++ 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>