Fixes 17

.github/workflows/cmake.yml

@@ -44,7 +44,7 @@ jobs:
     - name: Test
       run: make -C ${{github.workspace}}/ test ARGS="--verbose"
 
-    - uses: actions/upload-artifact@v4
+    - uses: actions/upload-artifact@v7
       if: ${{ success() || failure() }}
       with:
         name: robot-logs-${{ matrix.compiler }}

.gitignore

@@ -20,3 +20,5 @@ report.html
 custom_curl/
 valgrind-*.log
 tests/robot/__pycache__
+compile_commands.json
+.cache/clangd

CMakeLists.txt

@@ -27,14 +27,26 @@ if (NOT CMAKE_INSTALL_BINDIR)
   set(CMAKE_INSTALL_BINDIR bin)
 endif()
 
-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")
+string(APPEND CMAKE_C_FLAGS " -Wall -Wextra -Wpedantic -Wstrict-aliasing -Wformat=2 -Wunused -Wno-variadic-macros")
+string(APPEND CMAKE_C_FLAGS " -Wnull-dereference -Wshadow -Wconversion -Wsign-conversion -Wfloat-conversion -Wundef")
+string(APPEND CMAKE_C_FLAGS " -Wimplicit-fallthrough -Wstrict-overflow=2 -Wredundant-decls -Wdouble-promotion")
+string(APPEND CMAKE_C_FLAGS " -fstack-protector-strong -D_FORTIFY_SOURCE=3 -fPIE")
+
 set(CMAKE_C_FLAGS_DEBUG "-gdwarf-4 -DDEBUG")
 set(CMAKE_C_FLAGS_RELEASE "-O2")
 
+set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fPIE -Wl,-z,relro,-z,now")
+
+if (CMAKE_C_COMPILER_ID MATCHES GNU)
+string(APPEND CMAKE_C_FLAGS " -Warray-bounds=2 -Wduplicated-cond -Wduplicated-branches -Wrestrict")
+endif()
+if (CMAKE_C_COMPILER_ID MATCHES Clang)
+string(APPEND CMAKE_C_FLAGS " -Wno-format-nonliteral -Wno-double-promotion")
+endif()
 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")
+string(APPEND CMAKE_C_FLAGS " -Wno-gnu-zero-variadic-macro-arguments -Wno-gnu-folding-constant")
 endif()
 
 set(SERVICE_EXTRA_OPTIONS "")
@@ -116,7 +128,12 @@ 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=*,-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,-misc-include-cleaner,-llvmlibc-restrict-system-libc-headers,-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling")
+    set(CLANG_TIDY_CHECKS "-checks=*,-readability-identifier-length,-altera-unroll-loops,-concurrency-mt-unsafe,")
+    string(APPEND CLANG_TIDY_CHECKS "-bugprone-easily-swappable-parameters,-*magic-numbers,-hicpp-signed-bitwise,")
+    string(APPEND CLANG_TIDY_CHECKS "-readability-function-cognitive-complexity,-altera-id-dependent-backward-branch,")
+    string(APPEND CLANG_TIDY_CHECKS "-misc-include-cleaner,-llvmlibc-restrict-system-libc-headers,")
+    string(APPEND CLANG_TIDY_CHECKS "-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling")
+    set(DO_CLANG_TIDY "${CLANG_TIDY_EXE}" "-fix" "-fix-errors" "${CLANG_TIDY_CHECKS}")
   endif()
 else()
   message(STATUS "Not using clang-tidy.")

development_build_with_http3.sh

@@ -7,9 +7,9 @@ echo "WARNING !!!"
 echo
 echo "Use only for development and testing!"
 echo "It is highly highly not recommended, to use in production!"
-echo "This script was based on: https://github.com/curl/curl/blob/curl-8_12_1/docs/HTTP3.md"
+echo "This script was based on: https://github.com/curl/curl/blob/curl-8_19_0/docs/HTTP3.md"
 echo
-echo "Extra packages suggested to be installed: autoconf libtool"
+echo "Extra packages suggested to be installed: pkg-config pkgconf autoconf automake libtool"
 echo
 
 sleep 5
@@ -22,14 +22,14 @@ cd custom_curl
 
 ###
 
-git clone --depth 1 -b openssl-3.1.4+quic https://github.com/quictls/openssl
+git clone --depth 1 -b openssl-3.5.6 https://github.com/openssl/openssl
 cd openssl
-./config enable-tls1_3 --prefix=$INSTALL_DIR
-make -j build_libs
+./config --prefix=$INSTALL_DIR --libdir=lib
+make -j
 make install_dev
 cd ..
 
-git clone --depth 1 -b v1.1.0 https://github.com/ngtcp2/nghttp3
+git clone --depth 1 -b v1.15.0 https://github.com/ngtcp2/nghttp3
 cd nghttp3
 git submodule update --init
 autoreconf -fi
@@ -38,26 +38,26 @@ make -j
 make install
 cd ..
 
-git clone --depth 1 -b v1.2.0 https://github.com/ngtcp2/ngtcp2
+git clone --depth 1 -b v1.22.0 https://github.com/ngtcp2/ngtcp2
 cd ngtcp2
 autoreconf -fi
-./configure PKG_CONFIG_PATH=$INSTALL_DIR/lib64/pkgconfig:$INSTALL_DIR/lib64/pkgconfig LDFLAGS="-Wl,-rpath,$INSTALL_DIR/lib64" --prefix=$INSTALL_DIR --enable-lib-only --with-openssl
+./configure PKG_CONFIG_PATH=$INSTALL_DIR/lib/pkgconfig LDFLAGS="-Wl,-rpath,$INSTALL_DIR/lib" --prefix=$INSTALL_DIR --enable-lib-only --with-openssl
 make -j
 make install
 cd ..
 
-git clone --depth 1 -b v1.64.0 https://github.com/nghttp2/nghttp2
+git clone --depth 1 -b v1.68.1 https://github.com/nghttp2/nghttp2
 cd nghttp2
 autoreconf -fi
-./configure PKG_CONFIG_PATH=$INSTALL_DIR/lib64/pkgconfig:$INSTALL_DIR/lib64/pkgconfig LDFLAGS="-Wl,-rpath,$INSTALL_DIR/lib64" --prefix=$INSTALL_DIR --enable-lib-only --with-openssl
+./configure PKG_CONFIG_PATH=$INSTALL_DIR/lib/pkgconfig LDFLAGS="-Wl,-rpath,$INSTALL_DIR/lib" --prefix=$INSTALL_DIR --enable-lib-only --with-openssl
 make -j
 make install
 cd ..
 
-git clone --depth 1 -b curl-8_12_1 https://github.com/curl/curl
+git clone --depth 1 -b curl-8_19_0 https://github.com/curl/curl
 cd curl
 autoreconf -fi
-LDFLAGS="-Wl,-rpath,$INSTALL_DIR/lib64" ./configure --with-openssl=$INSTALL_DIR --with-nghttp2=$INSTALL_DIR --with-nghttp3=$INSTALL_DIR --with-ngtcp2=$INSTALL_DIR --prefix=$INSTALL_DIR
+LDFLAGS="-Wl,-rpath,$INSTALL_DIR/lib" ./configure PKG_CONFIG_PATH=$INSTALL_DIR/lib/pkgconfig --with-openssl=$INSTALL_DIR --with-nghttp2=$INSTALL_DIR --with-nghttp3=$INSTALL_DIR --with-ngtcp2=$INSTALL_DIR --prefix=$INSTALL_DIR --without-libpsl
 make -j
 make install
 cd ..

src/dns_listener_tcp.c

@@ -1,6 +1,3 @@
-//NOLINTNEXTLINE(bugprone-reserved-identifier,cert-dcl37-c,cert-dcl51-cpp)
-#define _GNU_SOURCE  // needed for having accept4()
-
 #include <arpa/inet.h>
 #include <errno.h>
 #include <fcntl.h>
@@ -34,7 +31,8 @@
 enum {
   LISTEN_BACKLOG  =   5,
   IDLE_TIMEOUT_S  = 120,  // "two minutes" according to RFC1035 4.2.2
-  RESEND_DELAY_US = 500,  // 0.0005 sec
+  RESPONSE_SEND_ATTEMPTS = 50,   // 0.025 sec max wait
+  RESPONSE_SEND_DELAY_US = 500,  // 0.0005 sec
   TCP_DNS_MAX_PAYLOAD = UINT16_MAX - sizeof(uint16_t),  // Max after 2-byte length prefix
 };
 
@@ -95,17 +93,13 @@ static void remove_client(struct tcp_client_s * client) {
 
   close(client->sock);
 
-  // Save next pointer before freeing. Safe because this is single-threaded
-  // event loop - no callbacks can run during this function.
-  struct tcp_client_s *next = client->next;
-
   if (d->clients == client) {
-    d->clients = next;
+    d->clients = client->next;
   }
   else {
     for (struct tcp_client_s * cur = d->clients; cur != NULL; cur = cur->next) {
       if (cur->next == client) {
-        cur->next = next;
+        cur->next = client->next;
         break;
       }
     }
@@ -144,11 +138,11 @@ static void read_cb(struct ev_loop __attribute__((unused)) *loop,
   ssize_t len = recv(w->fd, buf, DNS_REQUEST_BUFFER_SIZE, 0);
   if (len <= 0) {
     if (len == 0 || errno == ECONNRESET) {
-      DLOG_CLIENT("Connection closed");
+      DLOG_CLIENT("TCP client closed connection");
     } else if (errno == EAGAIN || errno == EWOULDBLOCK) {
       return;
     } else {
-      WLOG_CLIENT("Read error: %s", strerror(errno));
+      WLOG_CLIENT("Read error: %s (%d), dropping client", strerror(errno), errno);
     }
     remove_client(client);
     return;
@@ -194,12 +188,13 @@ static void read_cb(struct ev_loop __attribute__((unused)) *loop,
   uint8_t request_received = 0;
   while (get_dns_request(client, &dns_req, &req_size)) {
     if (req_size < DNS_HEADER_LENGTH) {
-      WLOG_CLIENT("Malformed request received, too short: %u", req_size);
+      WLOG_CLIENT("Malformed request received, too short: %u, dropping client", req_size);
       free(dns_req);
       remove_client(client);
       return;
     }
 
+    DLOG_CLIENT("Requested %04hX", ntohs(*((uint16_t*)dns_req)));
     d->cb(d->cb_data, &d->base, (struct sockaddr*)&client->raddr, dns_req, req_size);
     request_received = 1;
   }
@@ -223,16 +218,27 @@ static void accept_cb(struct ev_loop __attribute__((unused)) *loop,
   struct sockaddr_storage client_addr;
   socklen_t client_addr_len = sizeof(client_addr);
 
-  int client_sock = accept(w->fd, (struct sockaddr *)&client_addr, &client_addr_len);
-  if (client_sock != -1) {
-    // Set non-blocking mode for macOS compatibility (Linux accept4 does this atomically)
-    int flags = fcntl(client_sock, F_GETFL, 0);
-    if (flags != -1) {
-      fcntl(client_sock, F_SETFL, flags | O_NONBLOCK);
+  // NOLINTNEXTLINE(android-cloexec-accept)
+  const int client_sock = accept(w->fd, (struct sockaddr *)&client_addr, &client_addr_len);
+  if (client_sock == -1) {
+    if (errno != EAGAIN && errno != EWOULDBLOCK) {
+      ELOG("Failed to accept TCP client: %s (%d)", strerror(errno), errno);
     }
+    return;
   }
-  if (client_sock == -1 && errno != EAGAIN && errno != EWOULDBLOCK) {
-    ELOG("Failed to accept TCP client: %s", strerror(errno));
+
+  // Set non-blocking mode for macOS compatibility (Linux accept4 does this atomically)
+  const int flags = fcntl(client_sock, F_GETFL, 0);
+  if (flags == -1) {
+    ELOG("Error getting TCP client socket flags: %s (%d), dropping client",
+         strerror(errno), errno);
+    close(client_sock);
+    return;
+  }
+  if (fcntl(client_sock, F_SETFL, flags | O_NONBLOCK) == -1) {
+    ELOG("Error setting TCP client socket to non-blocking: %s (%d), dropping client",
+         strerror(errno), errno);
+    close(client_sock);
     return;
   }
 
@@ -329,6 +335,7 @@ static void tcp_respond(dns_listener_t *self, struct sockaddr *raddr,
     WLOG("Malformed response received, invalid length: %u", resp_len);
     return;
   }
+  const uint16_t response_id = ntohs(*((uint16_t*)resp));
 
   // find client data
   struct tcp_client_s *client = NULL;
@@ -339,7 +346,6 @@ static void tcp_respond(dns_listener_t *self, struct sockaddr *raddr,
     }
   }
   if (client == NULL) {
-    uint16_t response_id = ntohs(*((uint16_t*)resp));
     WLOG("Could not find client, can not send DNS response: %04hX", response_id);
     return;
   }
@@ -355,37 +361,38 @@ static void tcp_respond(dns_listener_t *self, struct sockaddr *raddr,
   uint16_t resp_size = htons((uint16_t)resp_len);
   ssize_t len = send(client->sock, &resp_size, sizeof(uint16_t), MSG_MORE | MSG_NOSIGNAL);
   if (len != sizeof(uint16_t)) {
-    WLOG_CLIENT("Send error: %s, len: %d", strerror(errno), len);
+    WLOG_CLIENT("Send error: %s (%d), len: %d, dropping client", strerror(errno), errno, len);
     remove_client(client);
     return;
   }
 
   // send the response
   ssize_t sent = 0;
   int attempts = 0;
-  for (; attempts < 50; ++attempts)  // 25ms max wait
+  for (; attempts < RESPONSE_SEND_ATTEMPTS; ++attempts)
   {
     len = send(client->sock, resp + sent, resp_len - (size_t)sent, MSG_NOSIGNAL);
-    if (len < 0) {
+    if (len > 0) {
+      sent += len;
+      if (sent == (ssize_t)resp_len) {
+        break;
+      }
+    } else if (len < 0) {
       if (errno != EAGAIN && errno != EWOULDBLOCK) {
-        WLOG_CLIENT("Send error: %s", strerror(errno));
+        WLOG_CLIENT("Send error: %s (%d), dropping client", strerror(errno), errno);
         remove_client(client);
         return;
       }
-      // EAGAIN/EWOULDBLOCK - socket buffer full, retry after delay
-      continue;
-    }
-    sent += len;
-    if (sent == (ssize_t)resp_len) {
-      break;
     }
-    usleep(RESEND_DELAY_US);
+    usleep(RESPONSE_SEND_DELAY_US);
   }
   if (sent != (ssize_t)resp_len) {
-    WLOG_CLIENT("Send timeout after %d attempts, sent %zd/%zu bytes", attempts, sent, resp_len);
+    WLOG_CLIENT("Send timeout after %d attempts, sent %zd/%zu bytes, dropping client",
+                attempts, sent, resp_len);
     remove_client(client);
     return;
   }
+  DLOG_CLIENT("Responded %04hX", response_id);
 
   ev_timer_again(d->loop, &client->timer_watcher);
 }

src/dns_listener_udp.c

@@ -21,7 +21,7 @@ typedef struct dns_listener_udp_s {
 
   dns_request_fn cb;
   void *cb_data;
-} dns_listener_udp_t;
+} __attribute__((aligned(128))) dns_listener_udp_t;
 
 // Creates and binds a listening UDP socket for incoming requests.
 static int get_listen_sock(struct addrinfo *listen_addrinfo) {
@@ -44,7 +44,6 @@ static int get_listen_sock(struct addrinfo *listen_addrinfo) {
 
   int res = bind(sock, listen_addrinfo->ai_addr, listen_addrinfo->ai_addrlen);
   if (res < 0) {
-    close(sock);
     FLOG("Error binding on %s:%d UDP: %s (%d)", ipstr, port,
          strerror(errno), errno);
   }

src/dns_poller.c

@@ -57,7 +57,7 @@ static char *get_addr_listing(struct ares_addrinfo_node * nodes) {
     const char *res = NULL;
 
     // Check that we have space for at least one character plus null terminator
-    if (pos >= list + POLLER_ADDR_LIST_SIZE - 1) {
+    if ((pos - list) >= POLLER_ADDR_LIST_SIZE - 1) {
       DLOG("Not enough space for more addresses");
       break;
     }
@@ -81,7 +81,7 @@ static char *get_addr_listing(struct ares_addrinfo_node * nodes) {
     if (res != NULL) {
       pos += strlen(pos);
       // Check we have room for the comma and null terminator
-      if (pos >= list + POLLER_ADDR_LIST_SIZE - 1) {
+      if ((pos - list) >= POLLER_ADDR_LIST_SIZE - 1) {
         DLOG("Not enough space for comma separator");
         break;
       }