Remi Gacogne [Thu, 23 Jun 2022 10:36:17 +0000 (12:36 +0200)]
dnsdist: Fix the number of concurrent queries on a backend TCP conn
When we are in the process of sending a query to the backend, that
query is no longer accounted in the "queued" queries nor it is in
the "queued" responses, but we need to take it into account.
Otherwise we might be sending two concurrent queries to a backend
that does not support out-of-order processing (increasing our
latency), or even worse to one that does not support pipelining.
Remi Gacogne [Tue, 28 Jun 2022 08:32:01 +0000 (10:32 +0200)]
dnsdist: Fix a bug in SetEDNSOptionAction
The DNS parser has already converted the "TTL" of the OPT record to
the host byte order before providing to us, and unfortunately we do
not want that for the meta-OPT record, where the TTL is used to encode
the extended rcode, the EDNS version and the DO bits, amongst other
things.
In other places we do parse the TTL from the DNS payload ourselves
and thus do not need to worry about that conversion, but here we
need to convert the value back to the network byte order.
Remi Gacogne [Wed, 19 Oct 2022 11:30:07 +0000 (13:30 +0200)]
dnsdist: Only IXFR queries can contain a SOA
So the "single SOA" response is only valid for IXFR, not AXFR.
This is the second issue spotted by Håkan Lindqvist in this pull
request, many, many thanks for that :)
Remi Gacogne [Wed, 19 Oct 2022 09:20:00 +0000 (11:20 +0200)]
dnsdist: Properly handle single-SOA XFR responses
From rfc1995 section 2 "Brief Description of the Protocol":
"If an IXFR query with the same or newer version number than that of the server is received, it is replied to with a single SOA record of the server's current version, just as in AXFR."
Until now we considered such a message to be an unfinished response to the pending {A,I}XFR, waiting for more DNS messages to come up and keeping the connection open for as long as the remote host was willing to accept that.
This causes an issue for servers keeping the connection open for a very long time, like ixfrdist.
Remi Gacogne [Fri, 25 Nov 2022 17:34:17 +0000 (18:34 +0100)]
dnsdist: Ignore unclean TLS session shutdown
OpenSSL 3.0 "helpfully" treats an unclean TLS session shutdown as an
error, flooding our logs and killing TLS session resumption. We do
not care about a possible "truncation attack" since we already know
how many bytes we are supposed to get, so we can ignore this.
Remi Gacogne [Tue, 27 Dec 2022 16:01:55 +0000 (17:01 +0100)]
dnsdist: Prevent an underflow of the TCP d_queued counter
By incrementing it _before_ writing to the pipe, and decrementing
it in case of an error, we prevent a very possible underflow from
occurring if the reader manages to decrement before we can return
from write and increment it.
Remi Gacogne [Mon, 16 Jan 2023 14:28:02 +0000 (15:28 +0100)]
dnsdist: Skip invalid OCSP files after issuing a warning
Contrary to certificates and keys, OCSP files are never required to
provide a working DoT or DoH service, so it's better to start even
if would not load all, or any, OCSP files.
Sander Hoentjen [Mon, 20 Feb 2023 15:51:07 +0000 (16:51 +0100)]
dnsdist-protocols.hh: include <cstdint>
This fixes building dnsdist with gcc13:
```
In file included from dnsdist-protocols.cc:26:
dnsdist-protocols.hh:32:8: error: use of enum 'typeenum' without previous declaration
32 | enum typeenum : uint8_t
| ^~~~~~~~
dnsdist-protocols.hh:32:19: error: 'uint8_t' was not declared in this scope
32 | enum typeenum : uint8_t
| ^~~~~~~
dnsdist-protocols.hh:25:1: note: 'uint8_t' is defined in header '<cstdint>'; did you forget to '#include <cstdint>'?
24 | #include <vector>
+++ |+#include <cstdint>
25 | #include <string>
```
Remi Gacogne [Mon, 12 Dec 2022 14:42:57 +0000 (15:42 +0100)]
dnsdist: Disable the send wrappers in our CI
The way the send wrappers are implemented, reading the data _after_
it has been sent, cause them to report a data race that does not
exist with existing implementations:
- we call `send()` from thread 1 to send a query to a backend, never
touching the data or associated metadata again from that thread
- we get a response from the backend in a different thread, thread 2,
which will then access the metadata and sometimes (truncated UDP
answers following a DoH query) even modify the data itself
- ASAN and TSAN complain because the wrapper might still be reading
the data after the UDP datagram has been sent, which is effectively
a race, but it does not really make any sense for an actual
implementation of `send()` to do that.
We work around that by disabling the `send()` wrappers in our CI,
for the dnsdist regression tests only, via `intercept_send=0`.
Remi Gacogne [Thu, 1 Dec 2022 13:34:19 +0000 (14:34 +0100)]
Restrict permissions for GITHUB_TOKEN in our workflows
Added using https://github.com/step-security/secure-workflows
For more information see:
- https://github.com/ossf/scorecard/blob/d8fefc9b246db3600c777e9d60d441d7c386ce1d/docs/checks.md#token-permissions
- https://github.blog/changelog/2021-04-20-github-actions-control-permissions-for-github_token/
Remi Gacogne [Wed, 4 May 2022 16:38:22 +0000 (18:38 +0200)]
dnsdist: Fix invalid proxy protocol payload on a DoH TC to TCP retry
dnsdist forwards incoming DoH queries to its backend over UDP, and
retry over TCP if the response is truncated (TC=1).
When the proxy protocol is used between dnsdist and its backend, the
second query, over TCP, needs to take into account that the proxy
protocol payload has already been handled. This was not properly done
in that exact case because the proxy protocol payload length was not
propagated to the code handling the TCP communication, leading to
the query ID being edited at the wrong offset in the packet and thus
to an invalid proxy protocol payload.
Remi Gacogne [Mon, 21 Mar 2022 09:27:30 +0000 (10:27 +0100)]
dnsdist-1.7.x: Only allocate the health-check mplexer when needed
When health-checking is disabled, or when a check delay longer than one
second is used, there is no need to allocate a new multiplexer object
every second.
dnsdist: Properly use eBPF when the DynBlock is not set
When the DynBlock rule does not set a specific action we use the
default one, set with `setDynBlocksAction()`, so we should follow
the same logic when determining whether to insert an eBPF block.