doh: cleanups and extended HTTPS RR code

In preparation for using HTTPS outside of ECH, the parser now also
extracts the port number.

Plus other minor cleanups.

Closes #16007

diff --git a/lib/doh.c b/lib/doh.c
index a74f9447ce841d576da259814debf7bce5728df8..6f814f296da7ad647358f4833dea620c941640c7 100644 (file)
--- a/lib/doh.c
+++ b/lib/doh.c
@@ -410,12 +410,6 @@ struct Curl_addrinfo *Curl_doh(struct Curl_easy *data,
   struct doh_probes *dohp;
   struct connectdata *conn = data->conn;
   size_t i;
-#ifdef USE_HTTPSRR
-  /* for now, this is only used when ECH is enabled */
-# ifdef USE_ECH
-  char *qname = NULL;
-# endif
-#endif
   *waitp = FALSE;
   (void)hostname;
   (void)port;
@@ -463,28 +457,27 @@ struct Curl_addrinfo *Curl_doh(struct Curl_easy *data,
 
 #ifdef USE_HTTPSRR
   /*
-   * TODO: Figure out the conditions under which we want to make
-   * a request for an HTTPS RR when we are not doing ECH. For now,
-   * making this request breaks a bunch of DoH tests, e.g. test2100,
-   * where the additional request does not match the pre-cooked data
-   * files, so there is a bit of work attached to making the request
-   * in a non-ECH use-case. For the present, we will only make the
-   * request when ECH is enabled in the build and is being used for
-   * the curl operation.
+   * TODO: Figure out the conditions under which we want to make a request for
+   * an HTTPS RR when we are not doing ECH. For now, making this request
+   * breaks a bunch of DoH tests, e.g. test2100, where the additional request
+   * does not match the pre-cooked data files, so there is a bit of work
+   * attached to making the request in a non-ECH use-case. For the present, we
+   * will only make the request when ECH is enabled in the build and is being
+   * used for the curl operation.
    */
 # ifdef USE_ECH
-  if(data->set.tls_ech & CURLECH_ENABLE
-     || data->set.tls_ech & CURLECH_HARD) {
-    if(port == 443)
-      qname = strdup(hostname);
-    else
+  if(data->set.tls_ech & (CURLECH_ENABLE|CURLECH_HARD)) {
+    char *qname = NULL;
+    if(port != PORT_HTTPS) {
       qname = aprintf("_%d._https.%s", port, hostname);
-    if(!qname)
-      goto error;
+      if(!qname)
+        goto error;
+    }
     result = doh_run_probe(data, &dohp->probe[DOH_SLOT_HTTPS_RR],
-                           DNS_TYPE_HTTPS, qname, data->set.str[STRING_DOH],
+                           DNS_TYPE_HTTPS,
+                           qname ? qname : hostname, data->set.str[STRING_DOH],
                            data->multi, dohp->req_hds);
-    Curl_safefree(qname);
+    free(qname);
     if(result)
       goto error;
     dohp->pending++;
@@ -1081,12 +1074,12 @@ static CURLcode doh_decode_rdata_name(unsigned char **buf, size_t *remaining,
   return CURLE_OK;
 }
 
-static CURLcode doh_decode_rdata_alpn(unsigned char *rrval, size_t len,
+static CURLcode doh_decode_rdata_alpn(unsigned char *cp, size_t len,
                                       char **alpns)
 {
   /*
-   * spec here is as per draft-ietf-dnsop-svcb-https, section-7.1.1
-   * encoding is catenated list of strings each preceded by a one
+   * spec here is as per RFC 9460, section-7.1.1
+   * encoding is a concatenated list of strings each preceded by a one
    * octet length
    * output is comma-sep list of the strings
    * implementations may or may not handle quoting of comma within
@@ -1095,25 +1088,21 @@ static CURLcode doh_decode_rdata_alpn(unsigned char *rrval, size_t len,
    * backslash - same goes for a backslash character, and of course
    * we need to use two backslashes in strings when we mean one;-)
    */
-  int remaining = (int) len;
   char *oval;
-  size_t i;
-  unsigned char *cp = rrval;
   struct dynbuf dval;
 
   if(!alpns)
     return CURLE_OUT_OF_MEMORY;
   Curl_dyn_init(&dval, DYN_DOH_RESPONSE);
-  remaining = (int)len;
-  cp = rrval;
-  while(remaining > 0) {
+  while(len > 0) {
     size_t tlen = (size_t) *cp++;
+    size_t i;
 
     /* if not 1st time, add comma */
-    if(remaining != (int)len && Curl_dyn_addn(&dval, ",", 1))
+    if(Curl_dyn_len(&dval) && Curl_dyn_addn(&dval, ",", 1))
       goto err;
-    remaining--;
-    if(tlen > (size_t)remaining)
+    len--;
+    if(tlen > len)
       goto err;
     /* add escape char if needed, clunky but easier to read */
     for(i = 0; i != tlen; i++) {
@@ -1124,7 +1113,7 @@ static CURLcode doh_decode_rdata_alpn(unsigned char *rrval, size_t len,
       if(Curl_dyn_addn(&dval, cp++, 1))
         goto err;
     }
-    remaining -= (int)tlen;
+    len -= tlen;
   }
   /* this string is always null terminated */
   oval = Curl_dyn_ptr(&dval);
@@ -1161,11 +1150,9 @@ static CURLcode doh_test_alpn_escapes(void)
 }
 #endif
 
-static CURLcode doh_resp_decode_httpsrr(unsigned char *rrval, size_t len,
+static CURLcode doh_resp_decode_httpsrr(unsigned char *cp, size_t len,
                                         struct Curl_https_rrinfo **hrr)
 {
-  size_t remaining = len;
-  unsigned char *cp = rrval;
   uint16_t pcode = 0, plen = 0;
   struct Curl_https_rrinfo *lhrr = NULL;
   char *dnsname = NULL;
@@ -1175,73 +1162,74 @@ static CURLcode doh_resp_decode_httpsrr(unsigned char *rrval, size_t len,
   if(doh_test_alpn_escapes() != CURLE_OK)
     return CURLE_OUT_OF_MEMORY;
 #endif
+  if(len <= 2)
+    return CURLE_BAD_FUNCTION_ARGUMENT;
   lhrr = calloc(1, sizeof(struct Curl_https_rrinfo));
   if(!lhrr)
     return CURLE_OUT_OF_MEMORY;
-  lhrr->val = Curl_memdup(rrval, len);
-  if(!lhrr->val)
-    goto err;
-  lhrr->len = len;
-  if(remaining <= 2)
-    goto err;
-  lhrr->priority = (uint16_t)((cp[0] << 8) + cp[1]);
+  lhrr->priority = doh_get16bit(cp, 0);
   cp += 2;
-  remaining -= (uint16_t)2;
-  if(doh_decode_rdata_name(&cp, &remaining, &dnsname) != CURLE_OK)
+  len -= 2;
+  if(doh_decode_rdata_name(&cp, &len, &dnsname) != CURLE_OK)
     goto err;
   lhrr->target = dnsname;
-  while(remaining >= 4) {
-    pcode = (uint16_t)((*cp << 8) + (*(cp + 1)));
-    cp += 2;
-    plen = (uint16_t)((*cp << 8) + (*(cp + 1)));
-    cp += 2;
-    remaining -= 4;
-    if(pcode == HTTPS_RR_CODE_ALPN) {
+  lhrr->port = -1; /* until set */
+  while(len >= 4) {
+    pcode = doh_get16bit(cp, 0);
+    plen = doh_get16bit(cp, 2);
+    cp += 4;
+    len -= 4;
+    switch(pcode) {
+    case HTTPS_RR_CODE_ALPN:
       if(doh_decode_rdata_alpn(cp, plen, &lhrr->alpns) != CURLE_OK)
         goto err;
-    }
-    if(pcode == HTTPS_RR_CODE_NO_DEF_ALPN)
+      break;
+    case HTTPS_RR_CODE_NO_DEF_ALPN:
       lhrr->no_def_alpn = TRUE;
-    else if(pcode == HTTPS_RR_CODE_IPV4) {
+      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;
-    }
-    else if(pcode == HTTPS_RR_CODE_ECH) {
+      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;
-    }
-    else if(pcode == HTTPS_RR_CODE_IPV6) {
+      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 <= remaining) {
+    if(plen > 0 && plen <= len) {
       cp += plen;
-      remaining -= plen;
+      len -= plen;
     }
   }
-  DEBUGASSERT(!remaining);
+  DEBUGASSERT(!len);
   *hrr = lhrr;
   return CURLE_OK;
 err:
-  if(lhrr) {
-    Curl_safefree(lhrr->target);
-    Curl_safefree(lhrr->echconfiglist);
-    Curl_safefree(lhrr->val);
-    Curl_safefree(lhrr->alpns);
-    Curl_safefree(lhrr);
-  }
+  Curl_safefree(lhrr->target);
+  Curl_safefree(lhrr->echconfiglist);
+  Curl_safefree(lhrr->alpns);
+  Curl_safefree(lhrr);
   return CURLE_OUT_OF_MEMORY;
 }
 

diff --git a/lib/hostip.c b/lib/hostip.c
index 110595c9e6e30f4d60c536cb5e7c3fec80c32d9f..c036049a61a9ebc00eab69b32b69f0f3b5d212c8 100644 (file)
--- a/lib/hostip.c
+++ b/lib/hostip.c
@@ -1079,18 +1079,11 @@ static void hostcache_unlink_entry(void *entry)
     Curl_freeaddrinfo(dns->addr);
 #ifdef USE_HTTPSRR
     if(dns->hinfo) {
-      if(dns->hinfo->target)
-        free(dns->hinfo->target);
-      if(dns->hinfo->alpns)
-        free(dns->hinfo->alpns);
-      if(dns->hinfo->ipv4hints)
-        free(dns->hinfo->ipv4hints);
-      if(dns->hinfo->echconfiglist)
-        free(dns->hinfo->echconfiglist);
-      if(dns->hinfo->ipv6hints)
-        free(dns->hinfo->ipv6hints);
-      if(dns->hinfo->val)
-        free(dns->hinfo->val);
+      free(dns->hinfo->target);
+      free(dns->hinfo->alpns);
+      free(dns->hinfo->ipv4hints);
+      free(dns->hinfo->echconfiglist);
+      free(dns->hinfo->ipv6hints);
       free(dns->hinfo);
     }
 #endif

diff --git a/lib/hostip.h b/lib/hostip.h
index b1c5ecb2e1b72e287de3a546af6ea435b54c3abe..3a7f327aebf854c8093b1695ea4519bf60d33e43 100644 (file)
--- a/lib/hostip.h
+++ b/lib/hostip.h
@@ -67,28 +67,21 @@ struct Curl_hash *Curl_global_host_cache_init(void);
 #define CURL_MAXLEN_host_name 253
 
 struct Curl_https_rrinfo {
-  size_t len; /* raw encoded length */
-  unsigned char *val; /* raw encoded octets */
   /*
-   * fields from HTTPS RR, with the mandatory fields
-   * first (priority, target), then the others in the
-   * order of the keytag numbers defined at
-   * https://datatracker.ietf.org/doc/html/rfc9460#section-14.3.2
+   * Fields from HTTPS RR. The only mandatory fields are priority and target.
+   * See https://datatracker.ietf.org/doc/html/rfc9460#section-14.3.2
    */
-  uint16_t priority;
   char *target;
   char *alpns; /* keytag = 1 */
-  bool no_def_alpn; /* keytag = 2 */
-  /*
-   * we do not support ports (keytag = 3) as we do not support
-   * port-switching yet
-   */
   unsigned char *ipv4hints; /* keytag = 4 */
   size_t ipv4hints_len;
   unsigned char *echconfiglist; /* keytag = 5 */
   size_t echconfiglist_len;
   unsigned char *ipv6hints; /* keytag = 6 */
   size_t ipv6hints_len;
+  int port; /* -1 means not set */
+  uint16_t priority;
+  bool no_def_alpn; /* keytag = 2 */
 };
 #endif