From 4c271ab5f4bb184b81ccd8bbbc9ada60e3597151 Mon Sep 17 00:00:00 2001 From: baranyaib90 <5031516+baranyaib90@users.noreply.github.com> Date: Mon, 23 Mar 2026 22:43:54 +0100 Subject: [PATCH 1/9] Revert "Prevent use-after-free in TCP client removal" This reverts commit 442815e1d055ac6ad9da845fc9014e2c652a5719. There should not be any use-after-free. --- src/dns_listener_tcp.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/dns_listener_tcp.c b/src/dns_listener_tcp.c index c7e5e23..8e8ffc3 100644 --- a/src/dns_listener_tcp.c +++ b/src/dns_listener_tcp.c @@ -95,17 +95,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; } } From 276ae59994324c26bc02fbd0afabbcccef8a17bc Mon Sep 17 00:00:00 2001 From: baranyaib90 <5031516+baranyaib90@users.noreply.github.com> Date: Mon, 23 Mar 2026 22:42:59 +0100 Subject: [PATCH 2/9] Reverting code changes and minor improvements - tighten TCP accept/read/send error handling and non-blocking socket setup - improve logging for dropped TCP clients and response failures - fix UDP bind cleanup - correct address-list buffer bounds checks in dns_poller - refine DNS truncation handling and related response logic --- src/dns_listener_tcp.c | 67 ++++++++++++++++++++++++------------------ src/dns_listener_udp.c | 1 - src/dns_poller.c | 4 +-- src/dns_truncate.c | 22 +++++++------- src/https_client.c | 4 ++- src/logging.c | 7 ++--- src/main.c | 4 ++- src/ring_buffer.c | 6 ++-- 8 files changed, 64 insertions(+), 51 deletions(-) diff --git a/src/dns_listener_tcp.c b/src/dns_listener_tcp.c index 8e8ffc3..da94cf0 100644 --- a/src/dns_listener_tcp.c +++ b/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 #include #include @@ -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 }; @@ -140,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; @@ -190,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; } @@ -219,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; + } + + // 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 (client_sock == -1 && errno != EAGAIN && errno != EWOULDBLOCK) { - ELOG("Failed to accept TCP client: %s", strerror(errno)); + 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; } @@ -325,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; @@ -335,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; } @@ -351,7 +361,7 @@ 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; } @@ -359,29 +369,30 @@ static void tcp_respond(dns_listener_t *self, struct sockaddr *raddr, // 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); } diff --git a/src/dns_listener_udp.c b/src/dns_listener_udp.c index 2cf6689..74db7dc 100644 --- a/src/dns_listener_udp.c +++ b/src/dns_listener_udp.c @@ -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); } diff --git a/src/dns_poller.c b/src/dns_poller.c index 5b6cd47..f3d75e4 100644 --- a/src/dns_poller.c +++ b/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; } diff --git a/src/dns_truncate.c b/src/dns_truncate.c index 7c439e9..a784a2c 100644 --- a/src/dns_truncate.c +++ b/src/dns_truncate.c @@ -110,17 +110,19 @@ static void truncate_to_size_limit(char *buf, size_t *buflen, const uint16_t siz } ares_dns_record_destroy(dnsrec); - if (new_resp != NULL) { - if (new_resp_len < old_size) { - memcpy(buf, new_resp, new_resp_len); - *buflen = new_resp_len; - buf[2] |= 0x02; // set truncation flag - ILOG("%04hX: DNS response size truncated from %u to %u to keep %u limit", - tx_id, old_size, new_resp_len, size_limit); - } - ares_free_string(new_resp); - new_resp = NULL; + if (new_resp == NULL) { + return; } + + if (new_resp_len < old_size) { + memcpy(buf, new_resp, new_resp_len); + *buflen = new_resp_len; + buf[2] |= 0x02; // set truncation flag + ILOG("%04hX: DNS response size truncated from %u to %u to keep %u limit", + tx_id, old_size, new_resp_len, size_limit); + } + + ares_free_string(new_resp); } void dns_truncate_for_udp(const char *dns_req, size_t dns_req_len, diff --git a/src/https_client.c b/src/https_client.c index 22196bb..3129296 100644 --- a/src/https_client.c +++ b/src/https_client.c @@ -265,9 +265,11 @@ static void https_set_request_version(https_client_t *client, http_version_str(http_version_int), easy_code, curl_easy_strerror(easy_code)); if (client->opt->use_http_version == 3) { - ELOG("Try to run application without -q argument!"); + ELOG("Try to run application without -q argument! Falling back to HTTP/2 version."); + client->opt->use_http_version = 2; } else if (client->opt->use_http_version == 2) { ELOG("Try to run application with -x argument! Falling back to HTTP/1.1 version."); + client->opt->use_http_version = 1; } } } diff --git a/src/logging.c b/src/logging.c index f19db00..30dee55 100644 --- a/src/logging.c +++ b/src/logging.c @@ -19,7 +19,6 @@ static int loglevel = LOG_ERROR; // NOLINT(cppcoreguidelines static ev_timer logging_timer; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) static ev_signal sigusr2; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) static ev_async flight_recorder_async; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) -static struct ev_loop *logging_loop = NULL; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) static struct ring_buffer * flight_recorder = NULL; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) static const char * const SeverityStr[] = { @@ -54,17 +53,15 @@ static void logging_flight_recorder_dump_async_cb(struct ev_loop __attribute__(( logging_flight_recorder_dump(); } -static void logging_flight_recorder_dump_cb(struct ev_loop __attribute__((unused)) *loop, +static void logging_flight_recorder_dump_cb(struct ev_loop *loop, ev_signal __attribute__((__unused__)) *w, int __attribute__((__unused__)) revents) { // Signal handler: just trigger async watcher to defer to main loop // This ensures fprintf() is called outside of signal context - ev_async_send(logging_loop, &flight_recorder_async); + ev_async_send(loop, &flight_recorder_async); } void logging_events_init(struct ev_loop *loop) { - logging_loop = loop; - /* don't start timer if we will never write messages that are not flushed */ if (loglevel < LOG_FLUSH_LEVEL) { DLOG("starting periodic log flush timer"); diff --git a/src/main.c b/src/main.c index b94a9c3..9d956d7 100644 --- a/src/main.c +++ b/src/main.c @@ -141,8 +141,9 @@ int main(int argc, char *argv[]) { exit(0); // asking for help is not a problem case OPR_VERSION: { printf("%s\n", sw_version()); + CURLcode init_res = curl_global_init(CURL_GLOBAL_DEFAULT); // needed to ensure, that curl_version*() calls will work properly! curl_version_info_data *curl_ver = curl_version_info(CURLVERSION_NOW); - if (curl_ver != NULL) { + if (init_res == CURLE_OK && curl_ver != NULL) { printf("Using: ev/%d.%d c-ares/%s %s\n", ev_version_major(), ev_version_minor(), ares_version(NULL), curl_version()); @@ -151,6 +152,7 @@ int main(int argc, char *argv[]) { curl_ver->features & CURL_VERSION_HTTP3 ? "HTTP3 " : "", curl_ver->features & CURL_VERSION_HTTPS_PROXY ? "HTTPS-proxy " : "", curl_ver->features & CURL_VERSION_IPV6 ? "IPv6" : ""); + curl_global_cleanup(); exit(0); } else { printf("\nFailed to get curl version info!\n"); diff --git a/src/ring_buffer.c b/src/ring_buffer.c index b2d232e..f356d1a 100644 --- a/src/ring_buffer.c +++ b/src/ring_buffer.c @@ -5,7 +5,9 @@ #include "ring_buffer.h" -#define MAX_LOG_ENTRY_SIZE 8192 +enum { +MAX_LOG_ENTRY_SIZE = 8192 +}; struct ring_buffer { @@ -23,13 +25,11 @@ void ring_buffer_init(struct ring_buffer **rbp, uint32_t size) } struct ring_buffer *rb = (struct ring_buffer *)calloc(1, sizeof(struct ring_buffer)); if (!rb) { - *rbp = NULL; return; } rb->storage = (char**)calloc(size, sizeof(char*)); if (!rb->storage) { free((void*) rb); - *rbp = NULL; return; } rb->size = size; From b1e5e32e8af3176c20aca469d94abbde5d07a8ae Mon Sep 17 00:00:00 2001 From: baranyaib90 <5031516+baranyaib90@users.noreply.github.com> Date: Sat, 11 Apr 2026 23:42:18 +0200 Subject: [PATCH 3/9] Update HTTP3 development build script --- development_build_with_http3.sh | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/development_build_with_http3.sh b/development_build_with_http3.sh index 39833c6..475c02f 100755 --- a/development_build_with_http3.sh +++ b/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 .. From c2a3d6ed141ed8dcea6049daa2dff79a2da3e59e Mon Sep 17 00:00:00 2001 From: baranyaib90 <5031516+baranyaib90@users.noreply.github.com> Date: Sat, 11 Apr 2026 23:05:24 +0200 Subject: [PATCH 4/9] Visual Studio Code development document added With suggestion to use clangd for IntelliSense purpose. --- .gitignore | 2 ++ vscode_development_setup.md | 64 +++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 vscode_development_setup.md diff --git a/.gitignore b/.gitignore index 3320a6c..300d808 100644 --- a/.gitignore +++ b/.gitignore @@ -20,3 +20,5 @@ report.html custom_curl/ valgrind-*.log tests/robot/__pycache__ +compile_commands.json +.cache/clangd diff --git a/vscode_development_setup.md b/vscode_development_setup.md new file mode 100644 index 0000000..2a8b8ba --- /dev/null +++ b/vscode_development_setup.md @@ -0,0 +1,64 @@ +# VS Code Development Setup for https_dns_proxy + +This document provides setup instructions for developing the https_dns_proxy project in Visual Studio Code with full language support and IntelliSense for this CMake-based C project. + +## Platform Support + +### Linux (Native) +Development works natively on Linux with all features enabled. Install the required tools on your Linux system and open the project directly in VS Code. + +### Windows via Remote SSH +Connect to a remote Linux machine (server, WSL, or VM) from Windows using VS Code's Remote SSH extension: +- Install the "Remote - SSH" extension in VS Code on Windows +- Connect to your Linux development environment +- Open the https_dns_proxy project from the remote machine +- All tools and dependencies run on the Linux machine + +### Windows via Remote WSL +Use Windows Subsystem for Linux (WSL) directly from Windows: +- Install WSL 2 on Windows (Ubuntu 24.04 LTS or newer recommended) +- Install the "Remote - WSL" extension in VS Code on Windows +- Open the project in VS Code's WSL environment +- All development happens within the WSL Linux environment + +## Prerequisites +- **VS Code** installed (with Remote extensions if using Windows) +- **CMake** installed +- **Clangd** installed (for language support) +- **VS Code Clangd extension** installed + +## Setup Steps + +1. **Generate compile_commands.json** + - This file is required by Clangd to understand the compilation flags for each source file. + - Run the following command in the project root: + ``` + cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=ON . + ``` + - This will create `compile_commands.json` in the project root. + +2. **Install Clangd Extension** + - Open VS Code (locally on Linux, or through Remote - SSH/WSL from Windows) + - Go to Extensions (Ctrl+Shift+X) + - Search for "clangd" and install the official Clangd extension by LLVM + - The extension will automatically work in remote environments + +3. **Verify Configuration** + - Ensure the project is open in VS Code + - Clangd should automatically detect `compile_commands.json` and provide language features + - Hover over functions/types to see IntelliSense and go-to-definition + +4. **Restart Language Server (if needed)** + - In VS Code, open Command Palette (Ctrl+Shift+P) + - Type "Clangd: Restart language server" and select it + +## Additional Commands +- **Switch to Clangd**: Use "C/C++: Select Language Server" (Ctrl+Shift+P) and choose Clangd +- **View Clangd Logs**: "Clangd: Show logs" in Command Palette +- **Rebuild IntelliSense**: "Clangd: Reload language server" in Command Palette + +## Troubleshooting +- If Clangd doesn't work, ensure `compile_commands.json` exists in the project root +- Check VS Code settings for Clangd configuration (View > Command Palette > "Preferences: Open Settings") +- Set `clangd.path` in settings if Clangd is not in your system PATH +- For remote development, ensure all tools are installed on the target Linux machine/WSL environment From 80e756d83c87234cf59b608d2bca4c1754e5a9ce Mon Sep 17 00:00:00 2001 From: baranyaib90 <5031516+baranyaib90@users.noreply.github.com> Date: Tue, 5 May 2026 22:45:35 +0200 Subject: [PATCH 5/9] Uplift github actions/upload-artifact --- .github/workflows/cmake.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/cmake.yml b/.github/workflows/cmake.yml index caea824..09a7d25 100644 --- a/.github/workflows/cmake.yml +++ b/.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 }} From 8716c87969c76e0ec79af08173895b2f5c91ff92 Mon Sep 17 00:00:00 2001 From: baranyaib90 <5031516+baranyaib90@users.noreply.github.com> Date: Sun, 12 Apr 2026 23:41:31 +0200 Subject: [PATCH 6/9] CMakeLists.txt: More strict compilation for security Also: - breaking long lines for readability - set gcc/clang specific warnings --- CMakeLists.txt | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 11a5903..a09cc4b 100644 --- a/CMakeLists.txt +++ b/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.") From 6b4de7304f5dfb27d6ebd51492550e99601d07bc Mon Sep 17 00:00:00 2001 From: baranyaib90 <5031516+baranyaib90@users.noreply.github.com> Date: Tue, 7 Apr 2026 22:40:44 +0200 Subject: [PATCH 7/9] Small dns_poller optimalizations --- src/main.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/main.c b/src/main.c index 9d956d7..25fdac0 100644 --- a/src/main.c +++ b/src/main.c @@ -268,15 +268,14 @@ int main(int argc, char *argv[]) { logging_events_init(loop); - dns_poller_t dns_poller; - uint8_t using_dns_poller = 0; - char hostname[255] = {0}; // Domain names shouldn't exceed 253 chars. + dns_poller_t * dns_poller = NULL; if (!proxy_supports_name_resolution(opt.curl_proxy)) { + char hostname[255] = {0}; // Domain names shouldn't exceed 253 chars. if (hostname_from_url(opt.resolver_url, hostname, sizeof(hostname))) { - using_dns_poller = 1; + dns_poller = (dns_poller_t *)calloc(1, sizeof(dns_poller_t)); doh_proxy_await_bootstrap(proxy); doh_proxy_set_on_ready(proxy, systemd_notify_ready, NULL); - dns_poller_init(&dns_poller, loop, opt.bootstrap_dns, + dns_poller_init(dns_poller, loop, opt.bootstrap_dns, opt.bootstrap_dns_polling_interval, opt.source_addr, hostname, opt.ipv4 ? AF_INET : AF_UNSPEC, @@ -294,8 +293,10 @@ int main(int argc, char *argv[]) { ev_run(loop, 0); DLOG("loop breaked"); - if (using_dns_poller) { - dns_poller_cleanup(&dns_poller); + if (dns_poller != NULL) { + dns_poller_cleanup(dns_poller); + free(dns_poller); + dns_poller = NULL; } logging_events_cleanup(loop); From a7b4b1de17d412389c56cc4d6bfe342f67276863 Mon Sep 17 00:00:00 2001 From: baranyaib90 <5031516+baranyaib90@users.noreply.github.com> Date: Sat, 11 Apr 2026 23:04:02 +0200 Subject: [PATCH 8/9] Added resplver IP override and port change support Some polishing in doh_proxy.c --- src/dns_listener_udp.c | 2 +- src/doh_proxy.c | 82 +++++++++++++++++---------------- src/doh_proxy.h | 22 +++++---- src/main.c | 102 ++++++++++++++++++++++++++++++----------- src/options.c | 10 +++- src/options.h | 3 ++ 6 files changed, 143 insertions(+), 78 deletions(-) diff --git a/src/dns_listener_udp.c b/src/dns_listener_udp.c index 74db7dc..561e69c 100644 --- a/src/dns_listener_udp.c +++ b/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) { diff --git a/src/doh_proxy.c b/src/doh_proxy.c index f8a8fef..3ba694e 100644 --- a/src/doh_proxy.c +++ b/src/doh_proxy.c @@ -1,43 +1,42 @@ #include +#include #include #include #include "dns_common.h" +#include "dns_poller.h" #include "doh_proxy.h" #include "logging.h" struct doh_proxy { struct ev_loop *loop; https_client_t *client; - const char *resolver_url; stat_t *stat; - // CURLOPT_RESOLVE entries (one slist node, "host:443:ip1,ip2,..."). NULL - // until the first successful resolver update arrives. + // CURLOPT_RESOLVE entries (one slist node, "host:443:ip1,ip2,..."). + // NULL until the first successful resolver update arrives. struct curl_slist *resolv; + const char *resolver_url; + uint16_t resolver_port; + // True until the first successful resolver update completes. While set, // inbound DNS requests are dropped (we'd otherwise leak through libcurl's // fallback resolver and risk a recursion through our own listener). uint8_t awaiting_bootstrap; - - doh_proxy_ready_fn on_ready; - void *on_ready_ctx; - uint8_t on_ready_fired; -}; + doh_proxy_bootstrap_done_cb bootstrap_done_cb; +} __attribute__((aligned(64))); // Per-request transient state. Lives from doh_proxy_handle_request to // https_resp_cb, when the response (or failure) returns from libcurl. -// NOLINTNEXTLINE(altera-struct-pack-align) typedef struct { doh_proxy_t *proxy; dns_listener_t *listener; - uint16_t tx_id; ev_tstamp start_tstamp; struct sockaddr_storage raddr; char *dns_req; size_t dns_req_len; -} doh_request_t; +} __attribute__((packed)) __attribute__((aligned(128))) doh_request_t; doh_proxy_t * doh_proxy_create(struct ev_loop *loop, https_client_t *client, @@ -50,25 +49,28 @@ doh_proxy_t * doh_proxy_create(struct ev_loop *loop, p->loop = loop; p->client = client; p->resolver_url = resolver_url; + p->resolver_port = 443; p->stat = stat; return p; } -void doh_proxy_await_bootstrap(doh_proxy_t *p) { - p->awaiting_bootstrap = 1; +void doh_proxy_await_bootstrap(doh_proxy_t *p, doh_proxy_bootstrap_done_cb cb) { + if (p != NULL) { + p->awaiting_bootstrap = 1; + p->bootstrap_done_cb = cb; + } } -void doh_proxy_set_on_ready(doh_proxy_t *p, doh_proxy_ready_fn cb, void *cb_ctx) { - p->on_ready = cb; - p->on_ready_ctx = cb_ctx; +void doh_proxy_set_port(doh_proxy_t *p, uint16_t port) { + if (p != NULL) { + p->resolver_port = port; + } } -static void fire_on_ready(doh_proxy_t *p) { - if (p->on_ready_fired || !p->on_ready) { - return; +void doh_proxy_set_resolv(doh_proxy_t *p, const char *buf) { + if (p != NULL) { + p->resolv = curl_slist_append(NULL, buf); } - p->on_ready_fired = 1; - p->on_ready(p->on_ready_ctx); } // Returns 1 if `addr_list` is a (possibly equal, possibly proper) subset of @@ -111,19 +113,21 @@ void doh_proxy_handle_resolver_update(const char *hostname, void *ctx, return; } - char buf[255 + (sizeof(":443:") - 1) + POLLER_ADDR_LIST_SIZE]; + char buf[HOSTNAME_BUFFER_SIZE + 1 + PORT_STR_LENGTH + 1 + POLLER_ADDR_LIST_SIZE]; memset(buf, 0, sizeof(buf)); - if (strlen(hostname) > 254) { FLOG("Hostname too long."); } - int ip_start = snprintf(buf, sizeof(buf) - 1, "%s:443:", hostname); + if (strlen(hostname) > 254) { FLOG("Hostname too long"); } + int ip_start = snprintf(buf, sizeof(buf) - 1, "%s:%u:", hostname, p->resolver_port); if (ip_start < 0) { abort(); // must be impossible } (void)snprintf(buf + ip_start, sizeof(buf) - 1 - (uint32_t)ip_start, "%s", addr_list); if (p->resolv && p->resolv->data) { - char *old_addr_list = strstr(p->resolv->data, ":443:"); + char port_colon[10]; + (void)snprintf(port_colon, sizeof(port_colon), ":%u:", p->resolver_port); + char * old_addr_list = strstr(p->resolv->data, port_colon); if (old_addr_list) { - old_addr_list += sizeof(":443:") - 1; + old_addr_list += strlen(port_colon); if (!addr_list_reduced(addr_list, old_addr_list)) { DLOG("DNS server IP address unchanged (%s).", buf + ip_start); free((void*)addr_list); @@ -142,7 +146,7 @@ void doh_proxy_handle_resolver_update(const char *hostname, void *ctx, if (p->awaiting_bootstrap) { p->awaiting_bootstrap = 0; - fire_on_ready(p); + p->bootstrap_done_cb(); } } @@ -153,16 +157,17 @@ static void doh_response_cb(void *data, char *buf, size_t buflen) { return; } doh_proxy_t *p = req->proxy; - DLOG("Received response for id: %hX, len: %zu", req->tx_id, buflen); + const uint16_t req_id = ntohs(*((uint16_t*)req->dns_req)); + DLOG("Received response for id: %04hX, len: %zu", req_id, buflen); if (buf != NULL) { // NULL on timeout / DNS failure / similar. if (buflen < DNS_HEADER_LENGTH) { - WLOG("%04hX: Malformed response received, too short: %u", req->tx_id, buflen); + WLOG("%04hX: Malformed response received, too short: %u", req_id, buflen); } else { - const uint16_t response_id = ntohs(*((uint16_t*)buf)); - if (req->tx_id != response_id) { - WLOG("DNS request and response IDs are not matching: %hX != %hX", - req->tx_id, response_id); + const uint16_t resp_id = ntohs(*((uint16_t*)buf)); + if (req_id != resp_id) { + WLOG("DNS request and response IDs are not matching: %04hX != %04hX", + req_id, resp_id); } else { req->listener->respond(req->listener, (struct sockaddr*)&req->raddr, req->dns_req, req->dns_req_len, buf, buflen); @@ -184,22 +189,21 @@ void doh_proxy_handle_request(void *ctx, dns_listener_t *listener, char *dns_req, size_t dns_req_len) { doh_proxy_t *p = (doh_proxy_t *)ctx; - uint16_t tx_id = ntohs(*((uint16_t*)dns_req)); - DLOG("Received request for id: %hX, len: %zu", tx_id, dns_req_len); + const uint16_t req_id = ntohs(*((uint16_t*)dns_req)); + DLOG("Received request for id: %04hX, len: %zu", req_id, dns_req_len); if (p->awaiting_bootstrap) { - WLOG("%04hX: Query received before bootstrapping is completed, discarding.", tx_id); + WLOG("%04hX: Query received before bootstrapping is completed, discarding.", req_id); free(dns_req); return; } doh_request_t *req = (doh_request_t *)calloc(1, sizeof(doh_request_t)); if (req == NULL) { - FLOG("%04hX: Out of mem", tx_id); + FLOG("%04hX: Out of mem", req_id); } req->proxy = p; req->listener = listener; - req->tx_id = tx_id; req->dns_req = dns_req; req->dns_req_len = dns_req_len; // raddr length depends on family; sockaddr_storage holds either. Copy what @@ -216,7 +220,7 @@ void doh_proxy_handle_request(void *ctx, dns_listener_t *listener, } https_client_fetch(p->client, p->resolver_url, req->dns_req, dns_req_len, - p->resolv, req->tx_id, doh_response_cb, req); + p->resolv, req_id, doh_response_cb, req); } void doh_proxy_destroy(doh_proxy_t *p) { diff --git a/src/doh_proxy.h b/src/doh_proxy.h index b7e3a69..a0dd2f4 100644 --- a/src/doh_proxy.h +++ b/src/doh_proxy.h @@ -4,10 +4,14 @@ #include #include "dns_listener.h" -#include "dns_poller.h" #include "https_client.h" #include "stat.h" +enum { + HOSTNAME_BUFFER_SIZE = 256, // To store max hostname length per RFC1035 2.3.4 + PORT_STR_LENGTH = 5, // Port shouldn't exceed 5 chars: "65535" +}; + // The DoH proxy core. Owns the per-request lifecycle (allocate state on // inbound, hand to the HTTPS client, route the response back to the // originating listener, free) and the curl resolve list driven by the @@ -15,9 +19,8 @@ typedef struct doh_proxy doh_proxy_t; // Optional callback invoked the first time the proxy is ready to serve -// requests (after bootstrap completes, if bootstrap was required). Called -// at most once. -typedef void (*doh_proxy_ready_fn)(void *ctx); +// requests (after bootstrap completes,). Called at most once. +typedef void (*doh_proxy_bootstrap_done_cb)(void); doh_proxy_t * doh_proxy_create(struct ev_loop *loop, https_client_t *client, @@ -27,11 +30,14 @@ doh_proxy_t * doh_proxy_create(struct ev_loop *loop, // Mark the proxy as awaiting bootstrap. Until the first successful resolver // update, inbound DNS requests will be dropped (libcurl would otherwise fall // back to gethostbyname() and may deadlock if our resolver depends on us). -void doh_proxy_await_bootstrap(doh_proxy_t *p); +// Optionally set a one-shot "ready" notifier (e.g. systemd_notify_ready). +// Fires when the first resolver update completes. +void doh_proxy_await_bootstrap(doh_proxy_t *p, doh_proxy_bootstrap_done_cb cb); + +void doh_proxy_set_port(doh_proxy_t *p, uint16_t port); -// Set a one-shot "ready" notifier (e.g. systemd_notify_ready). Fires when -// bootstrap completes, or never if await_bootstrap was never called. -void doh_proxy_set_on_ready(doh_proxy_t *p, doh_proxy_ready_fn cb, void *cb_ctx); +// Set static resolv list (for when no polling is used). +void doh_proxy_set_resolv(doh_proxy_t *p, const char *buf); // dns_request_fn — pass to dns_*_listener_create as the request callback. // `ctx` must be a doh_proxy_t *. diff --git a/src/main.c b/src/main.c index 25fdac0..cbac43b 100644 --- a/src/main.c +++ b/src/main.c @@ -2,10 +2,10 @@ // (C) 2016 Aaron Drew #include -#include #include #include #include +#include #include #include #include @@ -29,28 +29,54 @@ static int is_ipv4_address(char *str) { return inet_pton(AF_INET, str, &addr) == 1; } -static int hostname_from_url(const char* url_in, - char* hostname, const size_t hostname_len) { - int res = 0; +enum url_type { + URL_TYPE_ERROR, + URL_TYPE_IP, + URL_TYPE_HOSTNAME, +}; + +static enum url_type parse_url(const char* url_in, + char* hostname, const size_t hostname_len, + uint16_t *port) { + enum url_type res = URL_TYPE_ERROR; CURLU *url = curl_url(); if (url != NULL) { CURLUcode rc = curl_url_set(url, CURLUPART_URL, url_in, 0); if (rc == CURLUE_OK) { + // Extract port first + char *port_str = NULL; + rc = curl_url_get(url, CURLUPART_PORT, &port_str, 0); + DLOG("CURLUPART_PORT: %s, %s", curl_url_strerror(rc), port_str ? port_str : "NULL"); + if (rc == CURLUE_OK && port_str != NULL && strlen(port_str) > 0) { + char *endptr = NULL; + unsigned long value = strtoul(port_str, &endptr, 10); + if (*endptr == '\0' && value <= 65535UL) { + *port = (uint16_t)value; + } + } + curl_free(port_str); + + // Extract host char *host = NULL; rc = curl_url_get(url, CURLUPART_HOST, &host, 0); + DLOG("CURLUPART_HOST: %s, %s", curl_url_strerror(rc), host ? host : "NULL"); if (rc == CURLUE_OK && host != NULL) { const size_t host_len = strlen(host); - if (hostname_len > 0 && - host_len < hostname_len && - host[0] != '[' && host[host_len-1] != ']' && // skip IPv6 address - !is_ipv4_address(host)) { - strncpy(hostname, host, hostname_len-1); - hostname[hostname_len-1] = '\0'; - res = 1; // success + if (host_len > 0) { + if ((host[0] == '[' && host[host_len-1] == ']') || // IPv6 address + is_ipv4_address(host)) { + res = URL_TYPE_IP; + } else if (host_len < hostname_len) { + (void)snprintf(hostname, hostname_len, "%s", host); + res = URL_TYPE_HOSTNAME; + } } } curl_free(host); } + else { + DLOG("Error parsing URL '%s': %s", url_in, curl_url_strerror(rc)); + } curl_url_cleanup(url); } return res; @@ -69,7 +95,7 @@ static void sigpipe_cb(struct ev_loop __attribute__((__unused__)) *loop, ELOG("Received SIGPIPE. Ignoring."); } -static void systemd_notify_ready(void __attribute__((__unused__)) *unused) { +static void systemd_notify_ready(void) { #if HAS_LIBSYSTEMD == 1 static uint8_t called_once = 0; if (called_once != 0) { @@ -270,24 +296,44 @@ int main(int argc, char *argv[]) { dns_poller_t * dns_poller = NULL; if (!proxy_supports_name_resolution(opt.curl_proxy)) { - char hostname[255] = {0}; // Domain names shouldn't exceed 253 chars. - if (hostname_from_url(opt.resolver_url, hostname, sizeof(hostname))) { - dns_poller = (dns_poller_t *)calloc(1, sizeof(dns_poller_t)); - doh_proxy_await_bootstrap(proxy); - doh_proxy_set_on_ready(proxy, systemd_notify_ready, NULL); - dns_poller_init(dns_poller, loop, opt.bootstrap_dns, - opt.bootstrap_dns_polling_interval, opt.source_addr, - hostname, - opt.ipv4 ? AF_INET : AF_UNSPEC, - doh_proxy_handle_resolver_update, proxy); - ILOG("DNS polling initialized for '%s'", hostname); - } else { - ILOG("Resolver prefix '%s' doesn't appear to contain a " - "hostname. DNS polling disabled.", opt.resolver_url); - systemd_notify_ready(NULL); + char hostname[HOSTNAME_BUFFER_SIZE] = {0}; + uint16_t port = 443; // Default for HTTPS + const enum url_type url_type = parse_url(opt.resolver_url, hostname, sizeof(hostname), + &port); + switch (url_type) { + case URL_TYPE_HOSTNAME: + doh_proxy_set_port(proxy, port); + if (opt.resolver_ip == NULL) { + dns_poller = (dns_poller_t *)calloc(1, sizeof(dns_poller_t)); + doh_proxy_await_bootstrap(proxy, systemd_notify_ready); + dns_poller_init(dns_poller, loop, opt.bootstrap_dns, + opt.bootstrap_dns_polling_interval, opt.source_addr, + hostname, opt.ipv4 ? AF_INET : AF_UNSPEC, + doh_proxy_handle_resolver_update, proxy); + ILOG("DNS polling initialized for '%s'", hostname); + } else { + const size_t resolv_buf_len = strlen(hostname) + 1 + PORT_STR_LENGTH + 1 + strlen(opt.resolver_ip) + 1; + char * resolv_buf = (char *)calloc(resolv_buf_len, sizeof(char)); + (void)snprintf(resolv_buf, resolv_buf_len, "%s:%u:%s", hostname, port, opt.resolver_ip); + doh_proxy_set_resolv(proxy, resolv_buf); + ILOG("DNS polling disabled. Using static IP '%s'", resolv_buf); + free(resolv_buf); + systemd_notify_ready(); + } + break; + case URL_TYPE_IP: + doh_proxy_set_port(proxy, port); + ILOG("Resolver prefix '%s' doesn't appear to contain a hostname. " + "DNS polling disabled.", opt.resolver_url); + systemd_notify_ready(); + break; + case URL_TYPE_ERROR: // fallthrough + default: + FLOG("Failed to parse resolver URL: %s", opt.resolver_url); + break; } } else { - systemd_notify_ready(NULL); + systemd_notify_ready(); } ev_run(loop, 0); diff --git a/src/options.c b/src/options.c index 1016341..fa04d2c 100644 --- a/src/options.c +++ b/src/options.c @@ -38,6 +38,7 @@ void options_init(struct Options *opt) { opt->bootstrap_dns_polling_interval = 120; opt->ipv4 = 0; opt->resolver_url = "https://dns.google/dns-query"; + opt->resolver_ip = NULL; opt->curl_proxy = NULL; opt->source_addr = NULL; opt->use_http_version = DEFAULT_HTTP_VERSION; @@ -59,7 +60,7 @@ int parse_int(char * str) { enum OptionsParseResult options_parse_args(struct Options *opt, int argc, char **argv) { int c = 0; - while ((c = getopt(argc, argv, "a:c:p:T:du:g:b:i:4r:e:t:l:vxqm:L:s:S:C:F:hV")) != -1) { + while ((c = getopt(argc, argv, "a:c:p:T:du:g:b:i:4r:R:e:t:l:vxqm:L:s:S:C:F:hV")) != -1) { switch (c) { case 'a': // listen_addr opt->listen_addr = optarg; @@ -94,6 +95,9 @@ enum OptionsParseResult options_parse_args(struct Options *opt, int argc, char * case 'r': // resolver url prefix opt->resolver_url = optarg; break; + case 'R': // resolver ip address + opt->resolver_ip = optarg; + break; case 't': // curl http proxy opt->curl_proxy = optarg; break; @@ -226,7 +230,8 @@ void options_show_usage(int __attribute__((unused)) argc, char **argv) { options_init(&defaults); printf("Usage: %s [-a ] [-p ] [-T ]\n", argv[0]); printf(" [-b ] [-i ] [-4]\n"); - printf(" [-r ] [-t ] [-S ] [-x] [-q] [-C ] [-c ]\n"); + printf(" [-r ] [-R ] [-t ] [-S ]\n"); + printf(" [-x] [-q] [-C ] [-c ]\n"); printf(" [-d] [-u ] [-g ] \n"); printf(" [-v]+ [-l ] [-s ] [-F ] [-V] [-h]\n"); printf("\n DNS server\n"); @@ -249,6 +254,7 @@ void options_show_usage(int __attribute__((unused)) argc, char **argv) { printf("\n HTTPS client\n"); printf(" -r resolver_url The HTTPS path to the resolver URL. (Default: %s)\n", defaults.resolver_url); + printf(" -R resolver_ip The IP address of the resolver, instead of using DNS resolution.\n"); printf(" -t proxy_server Optional HTTP proxy. e.g. socks5://127.0.0.1:1080\n"); printf(" Remote name resolution will be used if the protocol\n"); printf(" supports it (http, https, socks4a, socks5h), otherwise\n"); diff --git a/src/options.h b/src/options.h index 6d0a9d3..30784d2 100644 --- a/src/options.h +++ b/src/options.h @@ -39,6 +39,9 @@ struct Options { // Resolver URL prefix to use. Must start with https://. const char *resolver_url; + // Resolver IP address + const char *resolver_ip; + // Optional http proxy if required. // e.g. "socks5://127.0.0.1:1080" const char *curl_proxy; From 373cab9a8e4fc8271e2e4c186d9c58574a83fe5d Mon Sep 17 00:00:00 2001 From: baranyaib90 <5031516+baranyaib90@users.noreply.github.com> Date: Fri, 1 May 2026 15:47:45 +0200 Subject: [PATCH 9/9] Truncation 2.0 Details in huge comment in code --- src/dns_truncate.c | 140 +++++++++++++++++------------ tests/robot/functional_tests.robot | 40 +++++---- tests/robot/valgrind.supp | 1 - 3 files changed, 107 insertions(+), 74 deletions(-) diff --git a/src/dns_truncate.c b/src/dns_truncate.c index a784a2c..babe65f 100644 --- a/src/dns_truncate.c +++ b/src/dns_truncate.c @@ -3,6 +3,7 @@ #include #include #include +#include #include "dns_common.h" #include "dns_truncate.h" @@ -13,14 +14,19 @@ // RFC1035 4.2.1 default of 512 if the request can't be parsed or the OPT // advertises a smaller size (RFC6891 4.3 mandates that values below 512 // MUST be treated as 512). +// Using c-ares' DNS parser for convenience and robustness, since the client's +// request can not be trusted. static uint16_t get_edns_udp_size(const char *dns_req, const size_t dns_req_len) { ares_dns_record_t *dnsrec = NULL; - ares_status_t parse_status = ares_dns_parse((const unsigned char *)dns_req, dns_req_len, 0, &dnsrec); + ares_status_t parse_status = ares_dns_parse((const unsigned char *)dns_req, dns_req_len, + ARES_DNS_PARSE_AN_BASE_RAW | ARES_DNS_PARSE_NS_BASE_RAW, // for faster parsing + &dnsrec); if (parse_status != ARES_SUCCESS) { - WLOG("Failed to parse DNS request: %s", ares_strerror((int)parse_status)); + const uint16_t req_id = ntohs(*((uint16_t*)dns_req)); + WLOG("%04hX: Failed to parse DNS request: %s", req_id, ares_strerror((int)parse_status)); return DNS_SIZE_LIMIT; } - const uint16_t tx_id = ares_dns_record_get_id(dnsrec); + const uint16_t req_id = ares_dns_record_get_id(dnsrec); uint16_t udp_size = 0; const size_t record_count = ares_dns_record_rr_cnt(dnsrec, ARES_SECTION_ADDITIONAL); for (size_t i = 0; i < record_count; ++i) { @@ -28,89 +34,107 @@ static uint16_t get_edns_udp_size(const char *dns_req, const size_t dns_req_len) if (ares_dns_rr_get_type(rr) == ARES_REC_TYPE_OPT) { udp_size = ares_dns_rr_get_u16(rr, ARES_RR_OPT_UDP_SIZE); if (udp_size > 0) { - DLOG("%04hX: Found EDNS0 UDP buffer size: %u", tx_id, udp_size); + DLOG("%04hX: Found EDNS0 UDP buffer size: %u", req_id, udp_size); } break; } } ares_dns_record_destroy(dnsrec); if (udp_size < DNS_SIZE_LIMIT) { - DLOG("%04hX: EDNS0 UDP buffer size %u overruled to %d", tx_id, udp_size, DNS_SIZE_LIMIT); + DLOG("%04hX: EDNS0 UDP buffer size %u overruled to %d", req_id, udp_size, DNS_SIZE_LIMIT); return DNS_SIZE_LIMIT; } return udp_size; } -static void truncate_to_size_limit(char *buf, size_t *buflen, const uint16_t size_limit) { +/* + * @brief Truncates a DNS response in-place to a skeleton packet to force an immediate TCP fallback. + * + * @param buf Pointer to the raw DNS message buffer. + * @param buflen The actual size of the data currently in the buffer. + * Will be set to the new truncated size after processing. + * @param size_limit The desired maximum size (e.g. 512). + * + * @section reasoning Architectural Reasoning & RFC Compliance: + * + * 1. TC Bit Enforcement (RFC 1035): + * Sets the Truncation bit (buf[2] |= 0x02) unconditionally when payload data is cleared. + * According to RFC 1035, the primary directive given to a resolver when it catches a packet + * with TC = 1 is that it must discard the UDP response data and immediately retry the query + * over a reliable transport (TCP). Because the client throws away the packet anyway, returning + * an empty data section completely satisfies the protocol's intent. + * + * 2. Total Section Cleardown (Deterministic Atomicity & RFC 2181): + * RFC 2181, Section 5.2, introduces the concept of RRSet Atomicity, stating that all records + * belonging to the same name, class, and type must be treated as a single cohesive unit. + * Instead of complex, error-prone progressive backtracking loops that risk partial RRSet exposure + * (which can cause intermediary resolvers to incorrectly cache incomplete data), this engine + * clears the ANCount, NSCount, and non-OPT ARCount fields to 0. Wiping all records + * uniformly ensures zero data corruption risk, as a set of 0 records cannot violate atomicity. + * + * 3. EDNS0/OPT Preservation (RFC 6891): + * The OPT pseudo-RR (Type 41) is critical for extended error tracking, cookies, and DNSSEC signaling. + * RFC 6891 mandates that OPT records should be preserved in truncated messages if they were present + * in the request. This function scans the Additional section, locates the OPT record, and uses + * memmove() to safely relocate it to sit directly flush against the end of the Question section, + * preserving it in the truncated response stream. + * + * 4. Trusted Data Assumption: + * DoH resolver response is considered trusted input, so assuming that it complies with RFCs + * and is well-formed. + * + */ +static void truncate_to_size_limit(uint8_t *buf, size_t *buflen, size_t size_limit) { const size_t old_size = *buflen; buf[2] |= 0x02; // anyway: set truncation flag ares_dns_record_t *dnsrec = NULL; - ares_status_t status = ares_dns_parse((const unsigned char *)buf, *buflen, 0, &dnsrec); + ares_status_t status = ares_dns_parse((const unsigned char *)buf, *buflen, + ARES_DNS_PARSE_AN_BASE_RAW | ARES_DNS_PARSE_NS_BASE_RAW, // for faster parsing + &dnsrec); if (status != ARES_SUCCESS) { - WLOG("Failed to parse DNS response: %s", ares_strerror((int)status)); + const uint16_t resp_id = ntohs(*((uint16_t*)buf)); + WLOG("%04hX: Failed to parse DNS response: %s", resp_id, ares_strerror((int)status)); return; } - const uint16_t tx_id = ares_dns_record_get_id(dnsrec); + const uint16_t resp_id = ares_dns_record_get_id(dnsrec); - // NOTE: according to current c-ares implementation, removing first or last elements are the fastest! + // NOTE: according to current c-ares implementation, removing last element is the fastest! - // remove every additional and authority record - while (ares_dns_record_rr_cnt(dnsrec, ARES_SECTION_ADDITIONAL) > 0) { - status = ares_dns_record_rr_del(dnsrec, ARES_SECTION_ADDITIONAL, 0); + // Remove every answer and authority record + for (size_t i = ares_dns_record_rr_cnt(dnsrec, ARES_SECTION_ANSWER); i > 0; i--) { + status = ares_dns_record_rr_del(dnsrec, ARES_SECTION_ANSWER, i - 1); if (status != ARES_SUCCESS) { - WLOG("%04hX: Could not remove additional record: %s", tx_id, ares_strerror((int)status)); + WLOG("%04hX: Could not remove answer record: %s", resp_id, ares_strerror((int)status)); } } - while (ares_dns_record_rr_cnt(dnsrec, ARES_SECTION_AUTHORITY) > 0) { - status = ares_dns_record_rr_del(dnsrec, ARES_SECTION_AUTHORITY, 0); + for (size_t i = ares_dns_record_rr_cnt(dnsrec, ARES_SECTION_AUTHORITY); i > 0; i--) { + status = ares_dns_record_rr_del(dnsrec, ARES_SECTION_AUTHORITY, i - 1); if (status != ARES_SUCCESS) { - WLOG("%04hX: Could not remove authority record: %s", tx_id, ares_strerror((int)status)); + WLOG("%04hX: Could not remove authority record: %s", resp_id, ares_strerror((int)status)); } } - - // rough estimate to reach size limit - size_t answers = ares_dns_record_rr_cnt(dnsrec, ARES_SECTION_ANSWER); - size_t answers_to_keep = ((size_limit - DNS_HEADER_LENGTH) * answers) / old_size; - answers_to_keep = answers_to_keep > 0 ? answers_to_keep : 1; // try to keep 1 answer - - // remove answer records until fit size limit or running out of answers - unsigned char *new_resp = NULL; - size_t new_resp_len = 0; - for (uint8_t g = 0; g < UINT8_MAX; ++g) { // endless loop guard - status = ares_dns_write(dnsrec, &new_resp, &new_resp_len); - if (status != ARES_SUCCESS) { - WLOG("%04hX: Failed to create truncated DNS response: %s", tx_id, ares_strerror((int)status)); - new_resp = NULL; // just to be sure - break; - } - if (new_resp_len < size_limit || answers == 0) { - break; + // Remove every additional record except OPT + for (size_t i = ares_dns_record_rr_cnt(dnsrec, ARES_SECTION_ADDITIONAL); i > 0; i--) { + const ares_dns_rr_t *rr = ares_dns_record_rr_get(dnsrec, ARES_SECTION_ADDITIONAL, i - 1); + if (ares_dns_rr_get_type(rr) == ARES_REC_TYPE_OPT) { + continue; // skip removing OPT, removing records before will be unoptimal } - if (new_resp_len >= old_size) { - WLOG("%04hX: Truncated DNS response size larger or equal to original: %u >= %u", - tx_id, new_resp_len, old_size); // impossible? + status = ares_dns_record_rr_del(dnsrec, ARES_SECTION_ADDITIONAL, i - 1); + if (status != ARES_SUCCESS) { + WLOG("%04hX: Could not remove additional record: %s", resp_id, ares_strerror((int)status)); } - ares_free_string(new_resp); - new_resp = NULL; + } - DLOG("%04hX: DNS response size truncated from %u to %u but to keep %u limit reducing answers from %u to %u", - tx_id, old_size, new_resp_len, size_limit, answers, answers_to_keep); + unsigned char *new_resp = NULL; + size_t new_resp_len = 0; + status = ares_dns_write(dnsrec, &new_resp, &new_resp_len); - while (answers > answers_to_keep) { - status = ares_dns_record_rr_del(dnsrec, ARES_SECTION_ANSWER, answers - 1); - if (status != ARES_SUCCESS) { - WLOG("%04hX: Could not remove answer record: %s", tx_id, ares_strerror((int)status)); - break; - } - --answers; - } - answers = ares_dns_record_rr_cnt(dnsrec, ARES_SECTION_ANSWER); // update to be sure! - answers_to_keep /= 2; - } ares_dns_record_destroy(dnsrec); - if (new_resp == NULL) { + if (status != ARES_SUCCESS || new_resp == NULL || new_resp_len == 0) { + WLOG("%04hX: Failed to create truncated DNS response: %s (new_resp=%p, new_resp_len=%zu)", + resp_id, ares_strerror((int)status), new_resp, new_resp_len); return; } @@ -119,7 +143,7 @@ static void truncate_to_size_limit(char *buf, size_t *buflen, const uint16_t siz *buflen = new_resp_len; buf[2] |= 0x02; // set truncation flag ILOG("%04hX: DNS response size truncated from %u to %u to keep %u limit", - tx_id, old_size, new_resp_len, size_limit); + resp_id, old_size, new_resp_len, size_limit); } ares_free_string(new_resp); @@ -132,10 +156,10 @@ void dns_truncate_for_udp(const char *dns_req, size_t dns_req_len, } const uint16_t udp_size = get_edns_udp_size(dns_req, dns_req_len); if (*resp_len <= udp_size) { - uint16_t tx_id = ntohs(*((uint16_t*)dns_req)); + uint16_t req_id = ntohs(*((uint16_t*)dns_req)); DLOG("%04hX: DNS response size %zu larger than %d but EDNS0 UDP buffer size %u allows it", - tx_id, *resp_len, DNS_SIZE_LIMIT, udp_size); + req_id, *resp_len, DNS_SIZE_LIMIT, udp_size); return; } - truncate_to_size_limit(resp, resp_len, udp_size); + truncate_to_size_limit((uint8_t*)resp, resp_len, udp_size); } diff --git a/tests/robot/functional_tests.robot b/tests/robot/functional_tests.robot index 5e9125c..3b3791a 100644 --- a/tests/robot/functional_tests.robot +++ b/tests/robot/functional_tests.robot @@ -1,4 +1,5 @@ *** Settings *** + Documentation Simple functional tests for https_dns_proxy Library OperatingSystem Library Process @@ -7,15 +8,18 @@ Library DnsTcpClient.py *** Variables *** + ${BINARY_PATH} ${CURDIR}/../../https_dns_proxy ${PORT} 55353 *** Settings *** + Test Teardown Stop Proxy *** Keywords *** + Common Test Setup Set Test Variable &{expected_logs} loop destroyed=1 # last log line Set Test Variable @{error_logs} [F] # any fatal error @@ -68,7 +72,6 @@ Stop Proxy END Should Be Equal As Integers ${result.rc} 0 - Start Dig [Arguments] ${domain}=google.com ${handle} = Start Process dig +timeout\=${dig_timeout} +retry\=${dig_retry} @{dig_options} @127.0.0.1 -p ${PORT} ${domain} @@ -100,7 +103,6 @@ Run Dig Parallel Stop Dig ${handle} END - Large Response Test [Documentation] https://dnscheck.tools/#more # use large buffer not to fragment UDP response, and ask for TXT response @@ -109,18 +111,17 @@ Large Response Test Should Match Regexp ${dig_output} MSG SIZE\\s+rcvd: 4\\d{3}$ # expecting more than 4k large response Verify Truncation - [Arguments] ${domain} ${udp_buffer_size} ${result_bytes_min} ${result_bytes_max} ${expect}=${None} - # ask for TXT response - Set Test Variable @{dig_options} +notcp +ignore +bufsize=${udp_buffer_size} -t txt + [Arguments] ${domain} ${result_bytes_min} ${result_bytes_max} ${expect}=${None} ${dig_output} = Run Dig ${domain} ${expect} Should Contain ${dig_output} flags: qr tc # expecting response to be ${result_bytes_min} byte (could be flaky) - @{res} = Should Match Regexp ${dig_output} MSG SIZE\\s+rcvd: (\\d+)$ # expecting more than 4k large response + @{res} = Should Match Regexp ${dig_output} MSG SIZE\\s+rcvd: (\\d+)$ Should Be True ${res}[1] >= ${result_bytes_min} Should Be True ${res}[1] <= ${result_bytes_max} *** Test Cases *** + Handle Unbound Server Does Not Support HTTP/1.1 Start Proxy -x -r https://doh.mullvad.net/dns-query # resolver uses Unbound Run Keyword And Expect Error 9 != 0 # timeout exit code @@ -184,23 +185,32 @@ Send TCP Requests Fragmented Close Tcp Client Connection -Truncate UDP Small +No Truncate UDP Small Start Proxy - Wait Until Keyword Succeeds 5x 200ms - # too small buffer will be overridden to 512, so expecting more than 300 bytes - ... Verify Truncation microsoft.com 256 300 512 + # too small buffer will be overridden to 512, so no truncation + Set Test Variable @{dig_options} @{dig_options} +ignore +bufsize=256 -t TXT +dnssec + ${dig_output} = Run Dig facebook.com + Should Contain ${dig_output} flags: qr rd ra; # no tr flag! + @{res} = Should Match Regexp ${dig_output} MSG SIZE\\s+rcvd: (\\d+)$ + Should Be True ${res}[1] >= 256 + Should Be True ${res}[1] <= 512 Truncate UDP Large Start Proxy - Wait Until Keyword Succeeds 5x 200ms - # expecting more than 1500 byte large response - ... Verify Truncation microsoft.com 2000 1500 2000 + # response would be ~4500 byte, has to be dropped because of RRSet Atomicity (RFC 2181, Sec 5.2) + Set Test Variable @{dig_options} @{dig_options} +ignore +bufsize=4096 -t txt + Verify Truncation microsoft.com 20 100 ANSWER: 0 Truncate UDP Impossible Start Proxy - Wait Until Keyword Succeeds 5x 200ms # the only TXT answer record has to be dropped to met limit - ... Verify Truncation txtfill4096.test.dnscheck.tools 4096 12 100 ANSWER: 0 + Set Test Variable @{dig_options} @{dig_options} +ignore +bufsize=4096 -t txt + Verify Truncation txtfill4096.test.dnscheck.tools 12 100 ANSWER: 0 + +Valgrind Resource Leak Check Truncation + Start Proxy With Valgrind + Set Test Variable @{dig_options} @{dig_options} +ignore +bufsize=4096 -t txt + Verify Truncation txtfill4096.test.dnscheck.tools 12 100 ANSWER: 0 Source Address Binding [Documentation] Test -S flag binds both HTTPS and bootstrap DNS to source address diff --git a/tests/robot/valgrind.supp b/tests/robot/valgrind.supp index 8ace591..c19713c 100644 --- a/tests/robot/valgrind.supp +++ b/tests/robot/valgrind.supp @@ -10,7 +10,6 @@ Memcheck:Leak match-leak-kinds: reachable ... - fun:ldap_int_sasl_init fun:ldap_int_initialize fun:ldap_get_option fun:curl_version