Large improvement pack

.github/workflows/cmake.yml

@@ -22,7 +22,7 @@ jobs:
       run: sudo apt-get update
 
     - name: Setup Dependencies
-      run: sudo apt-get install cmake libc-ares-dev libcurl4-openssl-dev libev-dev build-essential clang-tidy dnsutils python3-pip python3-venv valgrind ${{ matrix.compiler }}
+      run: sudo apt-get install cmake libc-ares-dev libcurl4-openssl-dev libev-dev libsystemd-dev build-essential clang-tidy dnsutils python3-pip python3-venv valgrind ${{ matrix.compiler }}
 
     - name: Setup Python Virtual Environment
       run: python3 -m venv ${{github.workspace}}/venv

.gitignore

@@ -18,3 +18,4 @@ output.xml
 report.html
 custom_curl/
 valgrind-*.log
+tests/robot/__pycache__

CMakeLists.txt

@@ -1,6 +1,8 @@
 cmake_minimum_required(VERSION 3.7)
 project(HttpsDnsProxy C)
 
+include(CheckIncludeFile)
+
 # FUNCTIONS
 
 # source: https://stackoverflow.com/a/27990434
@@ -25,15 +27,19 @@ if (NOT CMAKE_INSTALL_BINDIR)
   set(CMAKE_INSTALL_BINDIR bin)
 endif()
 
-set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wextra --pedantic -Wno-strict-aliasing -Wno-variadic-macros")
+set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wextra -Wpedantic -Wstrict-aliasing -Wformat=2 -Wunused -Wno-variadic-macros -Wnull-dereference -Wshadow -Wconversion -Wsign-conversion -Wfloat-conversion -Wimplicit-fallthrough")
 set(CMAKE_C_FLAGS_DEBUG "-gdwarf-4 -DDEBUG")
 set(CMAKE_C_FLAGS_RELEASE "-O2")
 
-if ((CMAKE_C_COMPILER_ID MATCHES GNU   AND CMAKE_C_COMPILER_VERSION VERSION_GREATER_EQUAL 9) OR
-    (CMAKE_C_COMPILER_ID MATCHES Clang AND CMAKE_C_COMPILER_VERSION VERSION_GREATER_EQUAL 10))
+if (((CMAKE_C_COMPILER_ID MATCHES GNU   AND CMAKE_C_COMPILER_VERSION VERSION_GREATER_EQUAL  9) AND
+     (CMAKE_C_COMPILER_ID MATCHES GNU   AND CMAKE_C_COMPILER_VERSION VERSION_LESS          14)) OR
+    ( CMAKE_C_COMPILER_ID MATCHES Clang AND CMAKE_C_COMPILER_VERSION VERSION_GREATER_EQUAL 10))
 set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wno-gnu-zero-variadic-macro-arguments -Wno-gnu-folding-constant")
 endif()
 
+set(SERVICE_EXTRA_OPTIONS "")
+set(SERVICE_TYPE "simple")
+
 # VERSION
 # It is possible to define external default value like: cmake -DSW_VERSION=1.2-custom
 
@@ -81,6 +87,15 @@ include_directories(
   ${LIBCARES_INCLUDE_DIR} ${LIBCURL_INCLUDE_DIR}
   ${LIBEV_INCLUDE_DIR} src)
 
+check_include_file("systemd/sd-daemon.h" HAVE_SD_DAEMON_H)
+
+if(HAVE_SD_DAEMON_H)
+  message(STATUS "Using libsystemd")
+  add_definitions(-DHAS_LIBSYSTEMD=1)
+  set(LIBS ${LIBS} systemd)
+  set(SERVICE_TYPE "notify")
+endif()
+
 # CLANG TIDY
 
 option(USE_CLANG_TIDY "Use clang-tidy during compilation" ON)
@@ -95,7 +110,7 @@ if(USE_CLANG_TIDY)
     message(STATUS "clang-tidy not found.")
   else()
     message(STATUS "clang-tidy found: ${CLANG_TIDY_EXE}")
-    set(DO_CLANG_TIDY "${CLANG_TIDY_EXE}" "-fix" "-fix-errors" "-checks=*,-cert-err34-c,-readability-identifier-length,-altera-unroll-loops,-bugprone-easily-swappable-parameters,-concurrency-mt-unsafe,-*magic-numbers,-hicpp-signed-bitwise,-readability-function-cognitive-complexity,-altera-id-dependent-backward-branch,-google-readability-todo,-misc-include-cleaner")
+    set(DO_CLANG_TIDY "${CLANG_TIDY_EXE}" "-fix" "-fix-errors" "-checks=*,-cert-err34-c,-readability-identifier-length,-altera-unroll-loops,-bugprone-easily-swappable-parameters,-concurrency-mt-unsafe,-*magic-numbers,-hicpp-signed-bitwise,-readability-function-cognitive-complexity,-altera-id-dependent-backward-branch,-google-readability-todo,-misc-include-cleaner,-cast-align")
   endif()
 else()
   message(STATUS "Not using clang-tidy.")
@@ -132,7 +147,6 @@ endif()
 
 install(TARGETS ${TARGET_NAME} DESTINATION ${CMAKE_INSTALL_BINDIR})
 
-set(SERVICE_EXTRA_OPTIONS "")
 if(IS_DIRECTORY "/etc/munin/plugins" AND
    IS_DIRECTORY "/etc/munin/plugin-conf.d")
   set(SERVICE_EXTRA_OPTIONS "-s 300")

README.md

@@ -2,7 +2,7 @@
 
 `https_dns_proxy` is a light-weight DNS<-->HTTPS, non-caching translation
 proxy for the [RFC 8484][rfc-8484] DNS-over-HTTPS standard. It receives
-regular (UDP) DNS requests and issues them via DoH.
+regular (UDP or TCP) DNS requests and issues them via DoH.
 
 [Google's DNS-over-HTTPS][google-doh] service is default, but
 [Cloudflare's service][cloudflare-doh] also works with trivial commandline flag
@@ -48,6 +48,7 @@ Depends on `c-ares (>=1.11.0)`, `libcurl (>=7.66.0)`, `libev (>=4.25)`.
 On Debian-derived systems those are libc-ares-dev,
 libcurl4-{openssl,nss,gnutls}-dev and libev-dev respectively.
 On Redhat-derived systems those are c-ares-devel, libcurl-devel and libev-devel.
+On systems with systemd it is recommended to have libsystemd development package installed.
 
 On MacOS, you may run into issues with curl headers. Others have had success when first installing curl with brew.
 ```
@@ -57,7 +58,7 @@ brew link curl --force
 
 On Ubuntu
 ```
-apt-get install cmake libc-ares-dev libcurl4-openssl-dev libev-dev build-essential
+apt-get install cmake libc-ares-dev libcurl4-openssl-dev libev-dev libsystemd-dev build-essential
 ```
 
 If all pre-requisites are met, you should be able to build with:
@@ -158,7 +159,7 @@ docker run --name "https-dns-proxy" -p 5053:5053/udp  \
 Just run it as a daemon and point traffic at it. Commandline flags are:
 
 ```
-Usage: ./https_dns_proxy [-a <listen_addr>] [-p <listen_port>]
+Usage: ./https_dns_proxy [-a <listen_addr>] [-p <listen_port>] [-T <tcp_client_limit>]
         [-b <dns_servers>] [-i <polling_interval>] [-4]
         [-r <resolver_url>] [-t <proxy_server>] [-x] [-q] [-C <ca_path>] [-c <dscp_codepoint>]
         [-d] [-u <user>] [-g <group>]
@@ -167,6 +168,8 @@ Usage: ./https_dns_proxy [-a <listen_addr>] [-p <listen_port>]
  DNS server
   -a listen_addr         Local IPv4/v6 address to bind to. (Default: 127.0.0.1)
   -p listen_port         Local port to bind to. (Default: 5053)
+  -T tcp_client_limit    Number of TCP clients to serve.
+                         (Default: 20, Disabled: 0, Min: 1, Max: 200)
 
  DNS client
   -b dns_servers         Comma-separated IPv4/v6 addresses and ports (addr:port)
@@ -189,6 +192,9 @@ Usage: ./https_dns_proxy [-a <listen_addr>] [-p <listen_port>]
   -q                     Use HTTP/3 (QUIC) only.
   -m max_idle_time       Maximum idle time in seconds allowed for reusing a HTTPS connection.
                          (Default: 118, Min: 0, Max: 3600)
+  -L conn_loss_time      Time in seconds to tolerate connection timeouts of reused connections.
+                         This option mitigates half-open TCP connection issue (e.g. WAN IP change).
+                         (Default: 15, Min: 5, Max: 60)
   -C ca_path             Optional file containing CA certificates.
   -c dscp_codepoint      Optional DSCP codepoint to set on upstream HTTPS server
                          connections. (Min: 0, Max: 63)

https_dns_proxy.service.in

@@ -6,11 +6,13 @@ Before=nss-lookup.target
 After=network.target
 
 [Service]
-Type=simple
+Type=${SERVICE_TYPE}
 DynamicUser=yes
-Restart=on-failure
 ExecStart=${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_BINDIR}/https_dns_proxy \
   -v -v ${SERVICE_EXTRA_OPTIONS}
+Restart=on-failure
+RestartSec=5
+TimeoutStartSec=20
 TimeoutStopSec=10
 
 [Install]

munin/https_dns_proxy.plugin

@@ -11,6 +11,8 @@ graph_scale no
 graph_args --base 1000 --lower-limit 0
 requests.label Requests
 responses.label Responses
+tcprequests.label TcpRequests
+tcpresponses.label TcpResponses
 
 multigraph https_dns_proxy_latency
 graph_title HTTPS DNS proxy - latency
@@ -19,6 +21,7 @@ graph_category network
 graph_scale no
 graph_args --base 1000 --lower-limit 0
 latency.label Latency
+tcplatency.label TcpLatency
 
 multigraph https_dns_proxy_connections
 graph_title HTTPS DNS proxy - connections
@@ -40,7 +43,7 @@ EOM
 esac
 
 log_lines=$(journalctl --unit https_dns_proxy.service --output cat --since '6 minutes ago')
-pattern='stat\.c:[0-9]+ ([0-9]+) ([0-9]+) ([0-9]+) ([0-9]+) ([0-9]+) ([0-9]+) ([0-9]+) ([0-9]+)$'
+pattern='stat\.c:[0-9]+ ([0-9]+) ([0-9]+) ([0-9]+) ([0-9]+) ([0-9]+) ([0-9]+) ([0-9]+) ([0-9]+) ([0-9]+) ([0-9]+) ([0-9]+) ([0-9]+) ([0-9]+)$'
 
 # match log lines with pattern (last match will be used)
 IFS='
@@ -53,18 +56,26 @@ for line in $log_lines; do
   fi
 done
 
-latency='U'
 if [ -n "${stat[3]}" ] && \
    [ -n "${stat[2]}" ] && \
    [ "${stat[2]}" -gt "0" ]; then
   latency=$((${stat[3]} / ${stat[2]}))
 fi
 
+if [ -n "${stat[11]}" ] && \
+   [ -n "${stat[10]}" ] && \
+   [ "${stat[10]}" -gt "0" ]; then
+  tcplatency=$((${stat[11]} / ${stat[10]}))
+fi
+
 echo "multigraph https_dns_proxy_count"
 echo "requests.value ${stat[1]:-U}"
 echo "responses.value ${stat[2]:-U}"
+echo "tcprequests.value ${stat[9]:-U}"
+echo "tcpresponses.value ${stat[10]:-U}"
 echo "multigraph https_dns_proxy_latency"
-echo "latency.value ${latency}"
+echo "latency.value ${latency:-0}"
+echo "tcplatency.value ${tcplatency:-0}"
 echo "multigraph https_dns_proxy_connections"
 echo "opened.value ${stat[6]:-U}"
 echo "closed.value ${stat[7]:-U}"

src/dns_poller.c

@@ -7,12 +7,12 @@
 static void sock_cb(struct ev_loop __attribute__((unused)) *loop,
                     ev_io *w, int revents) {
   dns_poller_t *d = (dns_poller_t *)w->data;
-  ares_process_fd(d->ares, (revents & EV_READ) ? w->fd : ARES_SOCKET_BAD,
-                  (revents & EV_WRITE) ? w->fd : ARES_SOCKET_BAD);
+  ares_process_fd(d->ares, (revents & EV_READ)  ? w->fd : ARES_SOCKET_BAD,
+                           (revents & EV_WRITE) ? w->fd : ARES_SOCKET_BAD);
 }
 
 static struct ev_io * get_io_event(dns_poller_t *d, int sock) {
-  for (int i = 0; i < d->io_events_count; i++) {
+  for (unsigned i = 0; i < d->io_events_count; i++) {
     if (d->io_events[i].fd == sock) {
       return &d->io_events[i];
     }
@@ -35,7 +35,7 @@ static void sock_state_cb(void *data, int fd, int read, int write) {
   // reserve and start new event on unused slot
   io_event_ptr = get_io_event(d, 0);
   if (!io_event_ptr) {
-    FLOG("c-ares needed more IO event handler, than the number of provided nameservers: %d", d->io_events_count);
+    FLOG("c-ares needed more IO event handler, than the number of provided nameservers: %u", d->io_events_count);
   }
   DLOG("Reserved new io event: %p", io_event_ptr);
   // NOLINTNEXTLINE(clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling)
@@ -44,21 +44,43 @@ static void sock_state_cb(void *data, int fd, int read, int write) {
   ev_io_start(d->loop, io_event_ptr);
 }
 
-static char *get_addr_listing(char** addr_list, const int af) {
+static char *get_addr_listing(struct ares_addrinfo_node * nodes) {
   char *list = (char *)calloc(1, POLLER_ADDR_LIST_SIZE);
-  char *pos = list;
   if (list == NULL) {
     FLOG("Out of mem");
   }
-  for (int i = 0; addr_list[i]; i++) {
-    const char *res = ares_inet_ntop(af, addr_list[i], pos,
-                                     list + POLLER_ADDR_LIST_SIZE - 1 - pos);
+  char *pos = list;
+  unsigned ipv4 = 0;
+  unsigned ipv6 = 0;
+
+  for (struct ares_addrinfo_node *node = nodes; node != NULL; node = node->ai_next) {
+    const char *res = NULL;
+
+    if (node->ai_family == AF_INET) {
+      res = ares_inet_ntop(AF_INET, (const void *)&((struct sockaddr_in *)node->ai_addr)->sin_addr,
+                           pos, (ares_socklen_t)(list + POLLER_ADDR_LIST_SIZE - 1 - pos));
+      ipv4++;
+    } else if (node->ai_family == AF_INET6) {
+      res = ares_inet_ntop(AF_INET6, (const void *)&((struct sockaddr_in6 *)node->ai_addr)->sin6_addr,
+                           pos, (ares_socklen_t)(list + POLLER_ADDR_LIST_SIZE - 1 - pos));
+      ipv6++;
+    } else {
+      WLOG("Unhandled address family: %d", node->ai_family);
+      continue;
+    }
+
     if (res != NULL) {
       pos += strlen(pos);
       *pos = ',';
       pos++;
+    } else {
+      DLOG("Not enough space left for further IP addresses");  // test with POLLER_ADDR_LIST_SIZE = 10 value
+      break;
     }
   }
+
+  DLOG("Received %u IPv4 and %u IPv6 addresses", ipv4, ipv6);
+
   if (pos == list) {
     free((void*)list);
     list = NULL;
@@ -69,19 +91,20 @@ static char *get_addr_listing(char** addr_list, const int af) {
 }
 
 static void ares_cb(void *arg, int status, int __attribute__((unused)) timeouts,
-                    struct hostent *h) {
+                    struct ares_addrinfo *result) {
   dns_poller_t *d = (dns_poller_t *)arg;
   d->request_ongoing = 0;
   ev_tstamp interval = 5;  // retry by default after some time
 
   if (status != ARES_SUCCESS) {
     WLOG("DNS lookup of '%s' failed: %s", d->hostname, ares_strerror(status));
-  } else if (!h || h->h_length < 1) {
+  } else if (!result || result->nodes == NULL) {
     WLOG("No hosts found for '%s'", d->hostname);
   } else {
     interval = d->polling_interval;
-    d->cb(d->hostname, d->cb_data, get_addr_listing(h->h_addr_list, h->h_addrtype));
+    d->cb(d->hostname, d->cb_data, get_addr_listing(result->nodes));
   }
+  ares_freeaddrinfo(result);
 
   if (status != ARES_EDESTRUCTION) {
     DLOG("DNS poll interval changed to: %.0lf", interval);
@@ -97,18 +120,23 @@ static ev_tstamp get_timeout(dns_poller_t *d)
     struct timeval tv;
     struct timeval *tvp = ares_timeout(d->ares, &max_tv, &tv);
     // NOLINTNEXTLINE(bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions)
-    ev_tstamp after = tvp->tv_sec + tvp->tv_usec * 1e-6;
-    return after ? after : 0.1;
+    ev_tstamp after = (double)tvp->tv_sec + (double)tvp->tv_usec * 1e-6;
+    return after > 0.1 ? after : 0.1;
 }
 
 static void timer_cb(struct ev_loop __attribute__((unused)) *loop,
                      ev_timer *w, int __attribute__((unused)) revents) {
   dns_poller_t *d = (dns_poller_t *)w->data;
 
   if (d->request_ongoing) {
-    // process query timeouts
-    DLOG("Processing DNS queries");
+    DLOG("Processing DNS query timeouts");
+#if ARES_VERSION_MAJOR >= 1 && ARES_VERSION_MINOR >= 34
+    ares_process_fds(d->ares, NULL, 0, ARES_PROCESS_FLAG_NONE);
+#elif ARES_VERSION_MAJOR >= 1 && ARES_VERSION_MINOR >= 28
+    ares_process_fd(d->ares, ARES_SOCKET_BAD, ARES_SOCKET_BAD);
+#else
     ares_process(d->ares, NULL, NULL);
+#endif
   } else {
     DLOG("Starting DNS query");
     // Cancel any pending queries before making new ones. c-ares can't be depended on to
@@ -117,7 +145,14 @@ static void timer_cb(struct ev_loop __attribute__((unused)) *loop,
     // free memory tied up by any "zombie" queries.
     ares_cancel(d->ares);
     d->request_ongoing = 1;
-    ares_gethostbyname(d->ares, d->hostname, d->family, ares_cb, d);
+
+    struct ares_addrinfo_hints hints;
+    memset(&hints, 0, sizeof(hints));  // NOLINT(clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling)
+    hints.ai_flags  = ARES_AI_CANONNAME;
+    hints.ai_family = d->family;
+    hints.ai_socktype = SOCK_STREAM;
+
+    ares_getaddrinfo(d->ares, d->hostname, "https", &hints, ares_cb, d);
   }
 
   if (d->request_ongoing) {  // need to re-check, it might change!
@@ -170,8 +205,8 @@ void dns_poller_init(dns_poller_t *d, struct ev_loop *loop,
   d->timer.data = d;
   ev_timer_start(d->loop, &d->timer);
 
-  int nameservers = 1;
-  for (int i = 0; bootstrap_dns[i]; i++) {
+  unsigned nameservers = 1;
+  for (unsigned i = 0; bootstrap_dns[i]; i++) {
     if (bootstrap_dns[i] == ',') {
       nameservers++;
     }
@@ -181,7 +216,7 @@ void dns_poller_init(dns_poller_t *d, struct ev_loop *loop,
   if (!d->io_events) {
     FLOG("Out of mem");
   }
-  for (int i = 0; i < nameservers; i++) {
+  for (unsigned i = 0; i < nameservers; i++) {
     d->io_events[i].data = d;
   }
   d->io_events_count = nameservers;

src/dns_poller.h

@@ -30,7 +30,7 @@ typedef struct {
 
   ev_timer timer;
   ev_io *io_events;
-  int io_events_count;
+  unsigned io_events_count;
 } dns_poller_t;
 
 // Initializes c-ares and starts a timer for periodic DNS resolution on the