https-rr: implementation improvements

- fold DoH and async HTTPS-RR handling into common code.
  have common cleanups, etc. Have a CURLcode result in async
  handling to allow HTTPS RR parsing to fail.
- keep target, ipv4hints, ipv6hints, port and echconfig also
  when resolving via cares. We need to know `target` and `port`
  when evaluating possible ALPN candidates to not go astray.
- add CURL_TRC_DNS for tracing DNS operations
- replace DoH specific tracing with DNS, use doh as alias
  for dns in curl_global_tracea()

Closes #16132

docs/libcurl/curl_global_trace.md

@@ -97,9 +97,13 @@ In order to find out all components involved in a transfer, run it with "all"
 configured. You can then see all names involved in your libcurl version in the
 trace.
 
+## `dns`
+
+Tracing of DNS operations to resolve hostnames and HTTPS records.
+
 ## `doh`
 
-Tracing of DNS-over-HTTP operations to resolve hostnames.
+Former name for DNS-over-HTTP operations. Now an alias for `dns`.
 
 ## `read`
 

lib/asyn-ares.c

@@ -419,14 +419,14 @@ CURLcode Curl_resolver_is_resolved(struct Curl_easy *data,
        them */
     res->temp_ai = NULL;
 
+    result = res->result;
     if(!data->state.async.dns)
       result = Curl_resolver_error(data);
-    else {
+    if(!result) {
       *dns = data->state.async.dns;
 #ifdef USE_HTTPSRR_ARES
       {
-        struct Curl_https_rrinfo *lhrr =
+        struct Curl_https_rrinfo *lhrr = Curl_httpsrr_dup_move(&res->hinfo);
-          Curl_memdup(&res->hinfo, sizeof(struct Curl_https_rrinfo));
         if(!lhrr)
           result = CURLE_OUT_OF_MEMORY;
         else

lib/asyn-thread.c

@@ -585,17 +585,19 @@ CURLcode Curl_resolver_is_resolved(struct Curl_easy *data,
   Curl_mutex_release(&td->tsd.mutx);
 
   if(done) {
+    CURLcode result = td->result;
     getaddrinfo_complete(data);
 
-    if(!data->state.async.dns) {
+    if(!result && !data->state.async.dns)
-      CURLcode result = Curl_resolver_error(data);
+      result = Curl_resolver_error(data);
+
+    if(result) {
       destroy_async_data(data);
       return result;
     }
 #ifdef USE_HTTPSRR_ARES
     {
-      struct Curl_https_rrinfo *lhrr =
+      struct Curl_https_rrinfo *lhrr = Curl_httpsrr_dup_move(&td->hinfo);
-        Curl_memdup(&td->hinfo, sizeof(struct Curl_https_rrinfo));
       if(!lhrr) {
         destroy_async_data(data);
         return CURLE_OUT_OF_MEMORY;

lib/asyn.h

@@ -60,6 +60,7 @@ struct thread_data {
   timediff_t interval_end;
   struct curltime start;
   struct thread_sync_data tsd;
+  CURLcode result; /* CURLE_OK or error handling response */
 #if defined(USE_HTTPSRR) && defined(USE_ARES)
   struct Curl_https_rrinfo hinfo;
   ares_channel channel;
@@ -74,6 +75,7 @@ struct thread_data {
   struct Curl_addrinfo *temp_ai; /* intermediary result while fetching c-ares
                                     parts */
   int last_status;
+  CURLcode result; /* CURLE_OK or error handling response */
 #ifndef HAVE_CARES_GETADDRINFO
   struct curltime happy_eyeballs_dns_time; /* when this timer started, or 0 */
 #endif

lib/curl_trc.c

@@ -176,6 +176,11 @@ struct curl_trc_feat Curl_trc_feat_write = {
   "WRITE",
   CURL_LOG_LVL_NONE,
 };
+struct curl_trc_feat Curl_trc_feat_dns = {
+  "DNS",
+  CURL_LOG_LVL_NONE,
+};
+
 
 void Curl_trc_read(struct Curl_easy *data, const char *fmt, ...)
 {
@@ -199,6 +204,17 @@ void Curl_trc_write(struct Curl_easy *data, const char *fmt, ...)
   }
 }
 
+void Curl_trc_dns(struct Curl_easy *data, const char *fmt, ...)
+{
+  DEBUGASSERT(!strchr(fmt, '\n'));
+  if(Curl_trc_ft_is_verbose(data, &Curl_trc_feat_dns)) {
+    va_list ap;
+    va_start(ap, fmt);
+    trc_infof(data, &Curl_trc_feat_dns, fmt, ap);
+    va_end(ap);
+  }
+}
+
 #ifndef CURL_DISABLE_FTP
 struct curl_trc_feat Curl_trc_feat_ftp = {
   "FTP",
@@ -284,11 +300,11 @@ struct trc_feat_def {
 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_FTP
   { &Curl_trc_feat_ftp,       TRC_CT_PROTOCOL },
 #endif
 #ifndef CURL_DISABLE_DOH
-  { &Curl_doh_trc,            TRC_CT_NETWORK },
 #endif
 #ifndef CURL_DISABLE_SMTP
   { &Curl_trc_feat_smtp,      TRC_CT_PROTOCOL },
@@ -395,6 +411,10 @@ static CURLcode trc_opt(const char *config)
       trc_apply_level_by_category(TRC_CT_NETWORK, lvl);
     else if(Curl_str_casecompare(&out, "proxy"))
       trc_apply_level_by_category(TRC_CT_PROXY, lvl);
+    else if(Curl_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);
 

lib/curl_trc.h

@@ -86,6 +86,8 @@ void Curl_trc_write(struct Curl_easy *data,
                     const char *fmt, ...) CURL_PRINTF(2, 3);
 void Curl_trc_read(struct Curl_easy *data,
                    const char *fmt, ...) CURL_PRINTF(2, 3);
+void Curl_trc_dns(struct Curl_easy *data,
+                  const char *fmt, ...) CURL_PRINTF(2, 3);
 
 #ifndef CURL_DISABLE_FTP
 extern struct curl_trc_feat Curl_trc_feat_ftp;
@@ -121,6 +123,9 @@ void Curl_trc_ws(struct Curl_easy *data,
 #define CURL_TRC_READ(data, ...) \
   do { if(Curl_trc_ft_is_verbose(data, &Curl_trc_feat_read)) \
          Curl_trc_read(data, __VA_ARGS__); } while(0)
+#define CURL_TRC_DNS(data, ...) \
+  do { if(Curl_trc_ft_is_verbose(data, &Curl_trc_feat_dns)) \
+         Curl_trc_dns(data, __VA_ARGS__); } while(0)
 
 #ifndef CURL_DISABLE_FTP
 #define CURL_TRC_FTP(data, ...) \
@@ -149,6 +154,7 @@ void Curl_trc_ws(struct Curl_easy *data,
 #define CURL_TRC_CF Curl_trc_cf_infof
 #define CURL_TRC_WRITE Curl_trc_write
 #define CURL_TRC_READ  Curl_trc_read
+#define CURL_TRC_DNS   Curl_trc_dns
 
 #ifndef CURL_DISABLE_FTP
 #define CURL_TRC_FTP   Curl_trc_ftp
@@ -174,6 +180,7 @@ struct curl_trc_feat {
 };
 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;
 
 #define Curl_trc_is_verbose(data) \
             ((data) && (data)->set.verbose && \

lib/doh.c

@@ -71,10 +71,6 @@ static const char *doh_strerror(DOHcode code)
   return "bad error code";
 }
 
-struct curl_trc_feat Curl_doh_trc = {
-  "DoH",
-  CURL_LOG_LVL_NONE,
-};
 #endif /* !CURL_DISABLE_VERBOSE_STRINGS */
 
 /* @unittest 1655
@@ -281,7 +277,7 @@ static CURLcode doh_run_probe(struct Curl_easy *data,
      the gcc typecheck helpers */
   doh->state.internal = TRUE;
 #ifndef CURL_DISABLE_VERBOSE_STRINGS
-  doh->state.feat = &Curl_doh_trc;
+  doh->state.feat = &Curl_trc_feat_dns;
 #endif
   ERROR_CHECK_SETOPT(CURLOPT_URL, url);
   ERROR_CHECK_SETOPT(CURLOPT_DEFAULT_PROTOCOL, "https");
@@ -305,7 +301,7 @@ static CURLcode doh_run_probe(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_doh_trc))
+  if(Curl_trc_ft_is_verbose(data, &Curl_trc_feat_dns))
     ERROR_CHECK_SETOPT(CURLOPT_VERBOSE, 1L);
   if(data->set.no_signal)
     ERROR_CHECK_SETOPT(CURLOPT_NOSIGNAL, 1L);
@@ -1087,12 +1083,14 @@ static CURLcode doh_test_alpn_escapes(void)
 }
 #endif
 
-static CURLcode doh_resp_decode_httpsrr(unsigned char *cp, size_t len,
+static CURLcode doh_resp_decode_httpsrr(struct Curl_easy *data,
+                                        unsigned char *cp, size_t len,
                                         struct Curl_https_rrinfo **hrr)
 {
   uint16_t pcode = 0, plen = 0;
   struct Curl_https_rrinfo *lhrr = NULL;
   char *dnsname = NULL;
+  CURLcode result = CURLE_OUT_OF_MEMORY;
 
 #ifdef DEBUGBUILD
   /* a few tests of escaping, should not be here but ok for now */
@@ -1116,44 +1114,9 @@ static CURLcode doh_resp_decode_httpsrr(unsigned char *cp, size_t len,
     plen = doh_get16bit(cp, 2);
     cp += 4;
     len -= 4;
-    switch(pcode) {
+    result = Curl_httpsrr_set(data, lhrr, pcode, cp, plen);
-    case HTTPS_RR_CODE_ALPN:
+    if(result)
-      if(Curl_httpsrr_decode_alpn(cp, plen, lhrr->alpns) != CURLE_OK)
+      goto err;
-        goto err;
-      break;
-    case HTTPS_RR_CODE_NO_DEF_ALPN:
-      lhrr->no_def_alpn = TRUE;
-      break;
-    case HTTPS_RR_CODE_IPV4:
-      if(!plen)
-        goto err;
-      lhrr->ipv4hints = Curl_memdup(cp, plen);
-      if(!lhrr->ipv4hints)
-        goto err;
-      lhrr->ipv4hints_len = (size_t)plen;
-      break;
-    case HTTPS_RR_CODE_ECH:
-      if(!plen)
-        goto err;
-      lhrr->echconfiglist = Curl_memdup(cp, plen);
-      if(!lhrr->echconfiglist)
-        goto err;
-      lhrr->echconfiglist_len = (size_t)plen;
-      break;
-    case HTTPS_RR_CODE_IPV6:
-      if(!plen)
-        goto err;
-      lhrr->ipv6hints = Curl_memdup(cp, plen);
-      if(!lhrr->ipv6hints)
-        goto err;
-      lhrr->ipv6hints_len = (size_t)plen;
-      break;
-    case HTTPS_RR_CODE_PORT:
-      lhrr->port = doh_get16bit(cp, 0);
-      break;
-    default:
-      break;
-    }
     if(plen > 0 && plen <= len) {
       cp += plen;
       len -= plen;
@@ -1163,10 +1126,9 @@ static CURLcode doh_resp_decode_httpsrr(unsigned char *cp, size_t len,
   *hrr = lhrr;
   return CURLE_OK;
 err:
-  Curl_safefree(lhrr->target);
+  Curl_httpsrr_cleanup(lhrr);
-  Curl_safefree(lhrr->echconfiglist);
   Curl_safefree(lhrr);
-  return CURLE_OUT_OF_MEMORY;
+  return result;
 }
 
 # ifdef DEBUGBUILD
@@ -1256,8 +1218,8 @@ CURLcode Curl_doh_is_resolved(struct Curl_easy *data,
       struct Curl_addrinfo *ai;
 
 
-      if(Curl_trc_ft_is_verbose(data, &Curl_doh_trc)) {
+      if(Curl_trc_ft_is_verbose(data, &Curl_trc_feat_dns)) {
-        infof(data, "[DoH] hostname: %s", dohp->host);
+        CURL_TRC_DNS(data, "hostname: %s", dohp->host);
         doh_show(data, &de);
       }
 
@@ -1291,8 +1253,8 @@ CURLcode Curl_doh_is_resolved(struct Curl_easy *data,
 #ifdef USE_HTTPSRR
     if(de.numhttps_rrs > 0 && result == CURLE_OK && *dnsp) {
       struct Curl_https_rrinfo *hrr = NULL;
-      result = doh_resp_decode_httpsrr(de.https_rrs->val, de.https_rrs->len,
+      result = doh_resp_decode_httpsrr(data, de.https_rrs->val,
-                                       &hrr);
+                                       de.https_rrs->len, &hrr);
       if(result) {
         infof(data, "Failed to decode HTTPS RR");
         return result;

lib/doh.h

@@ -167,8 +167,6 @@ UNITTEST void de_init(struct dohentry *d);
 UNITTEST void de_cleanup(struct dohentry *d);
 #endif
 
-extern struct curl_trc_feat Curl_doh_trc;
-
 #else /* if DoH is disabled */
 #define Curl_doh(a,b,c,d) NULL
 #define Curl_doh_is_resolved(x,y) CURLE_COULDNT_RESOLVE_HOST

lib/hostip.c

@@ -1081,10 +1081,7 @@ static void hostcache_unlink_entry(void *entry)
     Curl_freeaddrinfo(dns->addr);
 #ifdef USE_HTTPSRR
     if(dns->hinfo) {
-      free(dns->hinfo->target);
+      Curl_httpsrr_cleanup(dns->hinfo);
-      free(dns->hinfo->ipv4hints);
-      free(dns->hinfo->echconfiglist);
-      free(dns->hinfo->ipv6hints);
       free(dns->hinfo);
     }
 #endif

lib/httpsrr.c

@@ -31,6 +31,7 @@
 #include "httpsrr.h"
 #include "connect.h"
 #include "sendf.h"
+#include "strdup.h"
 
 /* The last 3 #include files should be in this order */
 #include "curl_printf.h"
@@ -91,44 +92,92 @@ err:
   return CURLE_BAD_CONTENT_ENCODING;
 }
 
+CURLcode Curl_httpsrr_set(struct Curl_easy *data,
+                          struct Curl_https_rrinfo *hi,
+                          uint16_t rrkey, const uint8_t *val, size_t vlen)
+{
+  switch(rrkey) {
+  case HTTPS_RR_CODE_ALPN: /* str_list */
+    Curl_httpsrr_decode_alpn(val, vlen, hi->alpns);
+    CURL_TRC_DNS(data, "HTTPS RR ALPN: %u %u %u %u",
+                 hi->alpns[0], hi->alpns[1], hi->alpns[2], hi->alpns[3]);
+    break;
+  case HTTPS_RR_CODE_NO_DEF_ALPN:
+    hi->no_def_alpn = TRUE;
+    CURL_TRC_DNS(data, "HTTPS RR no-def-alpn");
+    break;
+  case HTTPS_RR_CODE_IPV4: /* addr4 list */
+    if(!vlen)
+      return CURLE_BAD_FUNCTION_ARGUMENT;
+    hi->ipv4hints = Curl_memdup(val, vlen);
+    if(!hi->ipv4hints)
+      return CURLE_OUT_OF_MEMORY;
+    hi->ipv4hints_len = vlen;
+    CURL_TRC_DNS(data, "HTTPS RR IPv4");
+    break;
+  case HTTPS_RR_CODE_ECH:
+    if(!vlen)
+      return CURLE_BAD_FUNCTION_ARGUMENT;
+    hi->echconfiglist = Curl_memdup(val, vlen);
+    if(!hi->echconfiglist)
+      return CURLE_OUT_OF_MEMORY;
+    hi->echconfiglist_len = vlen;
+    CURL_TRC_DNS(data, "HTTPS RR ECH");
+    break;
+  case HTTPS_RR_CODE_IPV6: /* addr6 list */
+    if(!vlen)
+      return CURLE_BAD_FUNCTION_ARGUMENT;
+    hi->ipv6hints = Curl_memdup(val, vlen);
+    if(!hi->ipv6hints)
+      return CURLE_OUT_OF_MEMORY;
+    hi->ipv6hints_len = vlen;
+    CURL_TRC_DNS(data, "HTTPS RR IPv6");
+    break;
+  case HTTPS_RR_CODE_PORT:
+    if(vlen != 2)
+      return CURLE_BAD_FUNCTION_ARGUMENT;
+    hi->port = (unsigned short)((val[0] << 8) | val[1]);
+    CURL_TRC_DNS(data, "HTTPS RR port %u", hi->port);
+    break;
+  default:
+    CURL_TRC_DNS(data, "HTTPS RR unknown code");
+    break;
+  }
+  return CURLE_OK;
+}
+
+struct Curl_https_rrinfo *
+Curl_httpsrr_dup_move(struct Curl_https_rrinfo *rrinfo)
+{
+  struct Curl_https_rrinfo *dup = Curl_memdup(rrinfo, sizeof(*rrinfo));
+  if(dup)
+    memset(rrinfo, 0, sizeof(*rrinfo));
+  return dup;
+}
+
+void Curl_httpsrr_cleanup(struct Curl_https_rrinfo *rrinfo)
+{
+  Curl_safefree(rrinfo->target);
+  Curl_safefree(rrinfo->echconfiglist);
+  Curl_safefree(rrinfo->ipv4hints);
+  Curl_safefree(rrinfo->ipv6hints);
+}
+
+
 #ifdef USE_ARES
 
-static void httpsrr_opt(struct Curl_easy *data,
+static CURLcode httpsrr_opt(struct Curl_easy *data,
-                        const ares_dns_rr_t *rr,
+                            const ares_dns_rr_t *rr,
-                        ares_dns_rr_key_t key, size_t idx)
+                            ares_dns_rr_key_t key, size_t idx)
 {
   size_t len = 0;
   const unsigned char *val = NULL;
   unsigned short code;
   struct thread_data *res = &data->state.async.thdata;
   struct Curl_https_rrinfo *hi = &res->hinfo;
-  code  = ares_dns_rr_get_opt(rr, key, idx, &val, &len);
 
-  switch(code) {
+  code  = ares_dns_rr_get_opt(rr, key, idx, &val, &len);
-  case HTTPS_RR_CODE_ALPN: /* str_list */
+  return Curl_httpsrr_set(data, hi, code, val, len);
-    Curl_httpsrr_decode_alpn(val, len, hi->alpns);
-    infof(data, "HTTPS RR ALPN: %u %u %u %u",
-          hi->alpns[0], hi->alpns[1], hi->alpns[2], hi->alpns[3]);
-    break;
-  case HTTPS_RR_CODE_NO_DEF_ALPN:
-    infof(data, "HTTPS RR no-def-alpn");
-    break;
-  case HTTPS_RR_CODE_IPV4: /* addr4 list */
-    infof(data, "HTTPS RR IPv4");
-    break;
-  case HTTPS_RR_CODE_ECH:
-    infof(data, "HTTPS RR ECH");
-    break;
-  case HTTPS_RR_CODE_IPV6: /* addr6 list */
-    infof(data, "HTTPS RR IPv6");
-    break;
-  case HTTPS_RR_CODE_PORT:
-    infof(data, "HTTPS RR port");
-    break;
-  default:
-    infof(data, "HTTPS RR unknown code");
-    break;
-  }
 }
 
 void Curl_dnsrec_done_cb(void *arg, ares_status_t status,
@@ -136,6 +185,7 @@ void Curl_dnsrec_done_cb(void *arg, ares_status_t status,
                          const ares_dns_record_t *dnsrec)
 {
   struct Curl_easy *data = arg;
+  CURLcode result = CURLE_OK;
   size_t i;
 #ifdef CURLRES_ARES
   struct thread_data *res = &data->state.async.thdata;
@@ -147,6 +197,7 @@ void Curl_dnsrec_done_cb(void *arg, ares_status_t status,
     return;
 
   for(i = 0; i < ares_dns_record_rr_cnt(dnsrec, ARES_SECTION_ANSWER); i++) {
+    const char *target;
     size_t opt;
     const ares_dns_rr_t *rr =
       ares_dns_record_rr_get_const(dnsrec, ARES_SECTION_ANSWER, i);
@@ -154,12 +205,26 @@ void Curl_dnsrec_done_cb(void *arg, ares_status_t status,
       continue;
     /* When SvcPriority is 0, the SVCB record is in AliasMode. Otherwise, it
        is in ServiceMode */
-    infof(data, "HTTPS RR priority: %u",
+    target = ares_dns_rr_get_str(rr, ARES_RR_HTTPS_TARGET);
-          ares_dns_rr_get_u16(rr, ARES_RR_HTTPS_PRIORITY));
+    if(target && target[0]) {
+      res->hinfo.target = strdup(target);
+      if(!res->hinfo.target) {
+        result = CURLE_OUT_OF_MEMORY;
+        goto out;
+      }
+      CURL_TRC_DNS(data, "HTTPS RR target: %s", res->hinfo.target);
+    }
+    CURL_TRC_DNS(data, "HTTPS RR priority: %u",
+                 ares_dns_rr_get_u16(rr, ARES_RR_HTTPS_PRIORITY));
     for(opt = 0; opt < ares_dns_rr_get_opt_cnt(rr, ARES_RR_HTTPS_PARAMS);
-        opt++)
+        opt++) {
-      httpsrr_opt(data, rr, ARES_RR_HTTPS_PARAMS, opt);
+      result = httpsrr_opt(data, rr, ARES_RR_HTTPS_PARAMS, opt);
+      if(result)
+        break;
+    }
   }
+out:
+  res->result = result;
 }
 
 #endif /* USE_ARES */

lib/httpsrr.h

@@ -35,6 +35,8 @@
 #define CURL_MAXLEN_host_name 253
 #define MAX_HTTPSRR_ALPNS 4
 
+struct Curl_easy;
+
 struct Curl_https_rrinfo {
   /*
    * Fields from HTTPS RR. The only mandatory fields are priority and target.
@@ -53,7 +55,15 @@ struct Curl_https_rrinfo {
   uint16_t priority;
   bool no_def_alpn; /* keytag = 2 */
 };
-#endif
+
+CURLcode Curl_httpsrr_set(struct Curl_easy *data,
+                          struct Curl_https_rrinfo *hi,
+                          uint16_t rrkey, const uint8_t *val, size_t vlen);
+
+struct Curl_https_rrinfo *
+Curl_httpsrr_dup_move(struct Curl_https_rrinfo *rrinfo);
+
+void Curl_httpsrr_cleanup(struct Curl_https_rrinfo *rrinfo);
 
 /*
  * Code points for DNS wire format SvcParams as per RFC 9460
@@ -68,9 +78,12 @@ struct Curl_https_rrinfo {
 CURLcode Curl_httpsrr_decode_alpn(const unsigned char *cp, size_t len,
                                   unsigned char *alpns);
 
-#if defined(USE_ARES) && defined(USE_HTTPSRR)
+#if defined(USE_ARES)
 void Curl_dnsrec_done_cb(void *arg, ares_status_t status,
                          size_t timeouts,
                          const ares_dns_record_t *dnsrec);
-#endif
+
+#endif /* USE_ARES */
+#endif /* USE_HTTPSRR */
+
 #endif /* HEADER_CURL_HTTPSRR_H */

scripts/singleuse.pl

@@ -47,6 +47,7 @@ my %wl = (
     'Curl_creader_def_close' => 'internal api',
     'Curl_creader_def_read' => 'internal api',
     'Curl_creader_def_total_length' => 'internal api',
+    'Curl_trc_dns' => 'internal api',
 );
 
 my %api = (