Unify DoH URI making throughout the codebase

This commit adds new function isc_nm_http_makeuri() which is supposed
to unify DoH URI construction throughout the codebase.

It handles IPv6 addresses, hostnames, and IPv6 addresses given as
hostnames properly, and replaces similar ad-hoc code in the codebase.

diff --git a/bin/tests/test_client.c b/bin/tests/test_client.c
index a0caead0c5a020924ac40a7ef88a1e26202e73aa..e74bc267c98f90c08e16d627829ffcb334492164 100644 (file)
--- a/bin/tests/test_client.c
+++ b/bin/tests/test_client.c
@@ -392,35 +392,6 @@ connect_cb(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) {
        isc_nm_send(handle, &message, send_cb, NULL);
 }
 
-#if HAVE_LIBNGHTTP2
-static void
-sockaddr_to_url(isc_sockaddr_t *sa, const bool https, char *outbuf,
-               size_t outbuf_len, const char *append) {
-       uint16_t sa_port;
-       char saddr[INET6_ADDRSTRLEN] = { 0 };
-       int sa_family;
-
-       if (sa == NULL || outbuf == NULL || outbuf_len == 0) {
-               return;
-       }
-
-       sa_family = ((struct sockaddr *)&sa->type.sa)->sa_family;
-
-       sa_port = ntohs(sa_family == AF_INET ? sa->type.sin.sin_port
-                                            : sa->type.sin6.sin6_port);
-       inet_ntop(sa_family,
-                 sa_family == AF_INET
-                         ? (struct sockaddr *)&sa->type.sin.sin_addr
-                         : (struct sockaddr *)&sa->type.sin6.sin6_addr,
-                 saddr, sizeof(saddr));
-
-       snprintf(outbuf, outbuf_len, "%s://%s%s%s:%u%s",
-                https ? "https" : "http", sa_family == AF_INET ? "" : "[",
-                saddr, sa_family == AF_INET ? "" : "]", sa_port,
-                append ? append : "");
-}
-#endif
-
 static void
 run(void) {
        switch (protocol) {
@@ -449,8 +420,8 @@ run(void) {
                bool is_post = (protocol == HTTPS_POST ||
                                protocol == HTTP_POST);
                char req_url[256];
-               sockaddr_to_url(&sockaddr_remote, is_https, req_url,
-                               sizeof(req_url), DEFAULT_DOH_PATH);
+               isc_nm_http_makeuri(is_https, &sockaddr_remote, NULL, 0,
+                                   DEFAULT_DOH_PATH, req_url, sizeof(req_url));
                if (is_https) {
                        isc_tlsctx_createclient(&tls_ctx);
                }

diff --git a/lib/isc/include/isc/netmgr.h b/lib/isc/include/isc/netmgr.h
index f9433754bd4aca35e9a7a14b34b6457cf4efc0ef..3c5aff9e2b3e1d452be19f713fe4a68004a3b2a8 100644 (file)
--- a/lib/isc/include/isc/netmgr.h
+++ b/lib/isc/include/isc/netmgr.h
@@ -549,6 +549,21 @@ isc_nm_is_http_handle(isc_nmhandle_t *handle);
 
 bool
 isc_nm_http_path_isvalid(const char *path);
+
+void
+isc_nm_http_makeuri(const bool https, const isc_sockaddr_t *sa,
+                   const char *hostname, const uint16_t http_port,
+                   const char *abs_path, char *outbuf,
+                   const size_t outbuf_len);
+/*%<
+ * Makes a URI connection string out of na isc_sockaddr_t object 'sa'
+ * or the specified 'hostname' and 'http_port'.
+ *
+ * Requires:
+ * \li 'abs_path' is a valid absolute HTTP path string;
+ * \li 'outbuf' is a valid pointer to a buffer which will get the result;
+ * \li 'outbuf_len' is a size of the result buffer and is greater than zero.
+ */
 #endif /* HAVE_LIBNGHTTP2 */
 
 void

diff --git a/lib/isc/netmgr/http.c b/lib/isc/netmgr/http.c
index 6a8a4128c22a566e9e47060599bbea314ff6f83a..abca92731cb8474866ba006492e83d2c6cc1e000 100644 (file)
--- a/lib/isc/netmgr/http.c
+++ b/lib/isc/netmgr/http.c
@@ -3081,6 +3081,66 @@ isc__nmhandle_http_keepalive(isc_nmhandle_t *handle, bool value) {
        }
 }
 
+void
+isc_nm_http_makeuri(const bool https, const isc_sockaddr_t *sa,
+                   const char *hostname, const uint16_t http_port,
+                   const char *abs_path, char *outbuf,
+                   const size_t outbuf_len) {
+       char saddr[INET6_ADDRSTRLEN] = { 0 };
+       int family;
+       bool ipv6_addr = false;
+       struct sockaddr_in6 sa6;
+       uint16_t host_port = http_port;
+       const char *host = NULL;
+
+       REQUIRE(outbuf != NULL);
+       REQUIRE(outbuf_len != 0);
+       REQUIRE(isc_nm_http_path_isvalid(abs_path));
+
+       /* If hostname is specified, use that. */
+       if (hostname != NULL && hostname[0] != '\0') {
+               /*
+                * The host name could be an IPv6 address. If so,
+                * wrap it between [ and ].
+                */
+               if (inet_pton(AF_INET6, hostname, &sa6) == 1 &&
+                   hostname[0] != '[') {
+                       ipv6_addr = true;
+               }
+               host = hostname;
+       } else {
+               /*
+                * A hostname was not specified; build one from
+                * the given IP address.
+                */
+               INSIST(sa != NULL);
+               family = ((const struct sockaddr *)&sa->type.sa)->sa_family;
+               host_port = ntohs(family == AF_INET ? sa->type.sin.sin_port
+                                                   : sa->type.sin6.sin6_port);
+               ipv6_addr = family == AF_INET6;
+               (void)inet_ntop(
+                       family,
+                       family == AF_INET
+                               ? (const struct sockaddr *)&sa->type.sin.sin_addr
+                               : (const struct sockaddr *)&sa->type.sin6
+                                         .sin6_addr,
+                       saddr, sizeof(saddr));
+               host = saddr;
+       }
+
+       /*
+        * If the port number was not specified, the default
+        * depends on whether we're using encryption or not.
+        */
+       if (host_port == 0) {
+               host_port = https ? 443 : 80;
+       }
+
+       (void)snprintf(outbuf, outbuf_len, "%s://%s%s%s:%u%s",
+                      https ? "https" : "http", ipv6_addr ? "[" : "", host,
+                      ipv6_addr ? "]" : "", host_port, abs_path);
+}
+
 /*
  * DoH GET Query String Scanner-less Recursive Descent Parser/Verifier
  *

diff --git a/lib/isc/tests/doh_test.c b/lib/isc/tests/doh_test.c
index 1442610233511bcbe1fd11d819816e1840068c48..eb94f1cef34596d7d2daf4eff7ef7f945741e8c8 100644 (file)
--- a/lib/isc/tests/doh_test.c
+++ b/lib/isc/tests/doh_test.c
@@ -375,27 +375,7 @@ thread_local size_t nwrites = NWRITES;
 static void
 sockaddr_to_url(isc_sockaddr_t *sa, const bool https, char *outbuf,
                size_t outbuf_len, const char *append) {
-       uint16_t port;
-       char saddr[INET6_ADDRSTRLEN] = { 0 };
-       int family;
-
-       if (sa == NULL || outbuf == NULL || outbuf_len == 0) {
-               return;
-       }
-
-       family = ((struct sockaddr *)&sa->type.sa)->sa_family;
-
-       port = ntohs(family == AF_INET ? sa->type.sin.sin_port
-                                      : sa->type.sin6.sin6_port);
-       inet_ntop(family,
-                 family == AF_INET
-                         ? (struct sockaddr *)&sa->type.sin.sin_addr
-                         : (struct sockaddr *)&sa->type.sin6.sin6_addr,
-                 saddr, sizeof(saddr));
-
-       snprintf(outbuf, outbuf_len, "%s://%s%s%s:%u%s",
-                https ? "https" : "http", family == AF_INET ? "" : "[", saddr,
-                family == AF_INET ? "" : "]", port, append ? append : "");
+       isc_nm_http_makeuri(https, sa, NULL, 0, append, outbuf, outbuf_len);
 }
 
 static isc_quota_t *
@@ -2085,6 +2065,122 @@ doh_path_validation(void **state) {
        assert_true(isc_nm_http_path_isvalid("/123"));
 }
 
+static void
+doh_connect_makeuri(void **state) {
+       struct in_addr localhostv4 = { ntohl(INADDR_LOOPBACK) };
+       isc_sockaddr_t sa;
+       char uri[256];
+       UNUSED(state);
+
+       /* Firstly, test URI generation using isc_sockaddr_t */
+       isc_sockaddr_fromin(&sa, &localhostv4, 0);
+       uri[0] = '\0';
+       isc_nm_http_makeuri(true, &sa, NULL, 0, DOH_PATH, uri, sizeof(uri));
+       assert_true(strcmp("https://127.0.0.1:443/dns-query", uri) == 0);
+
+       uri[0] = '\0';
+       isc_nm_http_makeuri(false, &sa, NULL, 0, DOH_PATH, uri, sizeof(uri));
+       assert_true(strcmp("http://127.0.0.1:80/dns-query", uri) == 0);
+
+       /*
+        * The port value should be ignored, because we can get one from
+        * the isc_sockaddr_t object.
+        */
+       uri[0] = '\0';
+       isc_nm_http_makeuri(true, &sa, NULL, 44343, DOH_PATH, uri, sizeof(uri));
+       assert_true(strcmp("https://127.0.0.1:443/dns-query", uri) == 0);
+
+       uri[0] = '\0';
+       isc_nm_http_makeuri(false, &sa, NULL, 8080, DOH_PATH, uri, sizeof(uri));
+       assert_true(strcmp("http://127.0.0.1:80/dns-query", uri) == 0);
+
+       /* IPv6 */
+       isc_sockaddr_fromin6(&sa, &in6addr_loopback, 0);
+       uri[0] = '\0';
+       isc_nm_http_makeuri(true, &sa, NULL, 0, DOH_PATH, uri, sizeof(uri));
+       assert_true(strcmp("https://[::1]:443/dns-query", uri) == 0);
+
+       uri[0] = '\0';
+       isc_nm_http_makeuri(false, &sa, NULL, 0, DOH_PATH, uri, sizeof(uri));
+       assert_true(strcmp("http://[::1]:80/dns-query", uri) == 0);
+
+       /*
+        * The port value should be ignored, because we can get one from
+        * the isc_sockaddr_t object.
+        */
+       uri[0] = '\0';
+       isc_nm_http_makeuri(true, &sa, NULL, 44343, DOH_PATH, uri, sizeof(uri));
+       assert_true(strcmp("https://[::1]:443/dns-query", uri) == 0);
+
+       uri[0] = '\0';
+       isc_nm_http_makeuri(false, &sa, NULL, 8080, DOH_PATH, uri, sizeof(uri));
+       assert_true(strcmp("http://[::1]:80/dns-query", uri) == 0);
+
+       /* Try to set the port numbers. */
+       isc_sockaddr_setport(&sa, 44343);
+       uri[0] = '\0';
+       isc_nm_http_makeuri(true, &sa, NULL, 0, DOH_PATH, uri, sizeof(uri));
+       assert_true(strcmp("https://[::1]:44343/dns-query", uri) == 0);
+
+       isc_sockaddr_setport(&sa, 8080);
+       uri[0] = '\0';
+       isc_nm_http_makeuri(false, &sa, NULL, 0, DOH_PATH, uri, sizeof(uri));
+       assert_true(strcmp("http://[::1]:8080/dns-query", uri) == 0);
+
+       /*
+        * Try to make a URI using a hostname and a port number. The
+        * isc_sockaddr_t object will be ignored.
+        */
+       isc_sockaddr_any(&sa);
+       uri[0] = '\0';
+       isc_nm_http_makeuri(true, &sa, "example.com", 0, DOH_PATH, uri,
+                           sizeof(uri));
+       assert_true(strcmp("https://example.com:443/dns-query", uri) == 0);
+
+       uri[0] = '\0';
+       isc_nm_http_makeuri(false, &sa, "example.com", 0, DOH_PATH, uri,
+                           sizeof(uri));
+       assert_true(strcmp("http://example.com:80/dns-query", uri) == 0);
+
+       /* Try to set the port numbers. */
+       isc_sockaddr_setport(&sa, 443);
+       uri[0] = '\0';
+       isc_nm_http_makeuri(true, &sa, "example.com", 44343, DOH_PATH, uri,
+                           sizeof(uri));
+       assert_true(strcmp("https://example.com:44343/dns-query", uri) == 0);
+
+       isc_sockaddr_setport(&sa, 80);
+       uri[0] = '\0';
+       isc_nm_http_makeuri(false, &sa, "example.com", 8080, DOH_PATH, uri,
+                           sizeof(uri));
+       assert_true(strcmp("http://example.com:8080/dns-query", uri) == 0);
+
+       /* IPv4 as the hostname - nothing fancy here */
+       uri[0] = '\0';
+       isc_nm_http_makeuri(false, NULL, "127.0.0.1", 8080, DOH_PATH, uri,
+                           sizeof(uri));
+       assert_true(strcmp("http://127.0.0.1:8080/dns-query", uri) == 0);
+
+       uri[0] = '\0';
+       isc_nm_http_makeuri(true, NULL, "127.0.0.1", 44343, DOH_PATH, uri,
+                           sizeof(uri));
+       assert_true(strcmp("https://127.0.0.1:44343/dns-query", uri) == 0);
+
+       /*
+        * A peculiar edge case: IPv6 given as the hostname (notice
+        * the brackets)
+        */
+       uri[0] = '\0';
+       isc_nm_http_makeuri(false, NULL, "::1", 8080, DOH_PATH, uri,
+                           sizeof(uri));
+       assert_true(strcmp("http://[::1]:8080/dns-query", uri) == 0);
+
+       uri[0] = '\0';
+       isc_nm_http_makeuri(true, NULL, "[::1]", 44343, DOH_PATH, uri,
+                           sizeof(uri));
+       assert_true(strcmp("https://[::1]:44343/dns-query", uri) == 0);
+}
+
 int
 main(void) {
        const struct CMUnitTest tests_short[] = {
@@ -2098,6 +2194,8 @@ main(void) {
                                                NULL),
                cmocka_unit_test_setup_teardown(doh_path_validation, NULL,
                                                NULL),
+               cmocka_unit_test_setup_teardown(doh_connect_makeuri, NULL,
+                                               NULL),
                cmocka_unit_test_setup_teardown(doh_noop_POST, nm_setup,
                                                nm_teardown),
                cmocka_unit_test_setup_teardown(doh_noop_GET, nm_setup,
@@ -2121,6 +2219,8 @@ main(void) {
                                                NULL),
                cmocka_unit_test_setup_teardown(doh_path_validation, NULL,
                                                NULL),
+               cmocka_unit_test_setup_teardown(doh_connect_makeuri, NULL,
+                                               NULL),
                cmocka_unit_test_setup_teardown(doh_noop_POST, nm_setup,
                                                nm_teardown),
                cmocka_unit_test_setup_teardown(doh_noop_GET, nm_setup,