Remi Gacogne [Thu, 17 Aug 2023 09:19:15 +0000 (11:19 +0200)]
dnsdist: Properly record self-answered UDP responses with recvmmsg
Responses sent directly from dnsdist, without reaching out to a backend
(self-generated and cache hits answers, mostly) where not properly
accounted for in frontend metrics, ring buffer entries and latency
computation when recvmmsg/sendmmsg support was enabled via
`setUDPMultipleMessagesVectorSize()`.
Remi Gacogne [Wed, 3 May 2023 13:02:34 +0000 (15:02 +0200)]
dnsdist: Fix cache hit and miss metrics with DoH queries
Since we do two lookups for DoH queries forwarded over UDP (first
TCP then UDP), we need to be careful to only record a cache miss
in our last attempt.
dnsdist: Properly handle reconnection failure for backend UDP sockets
We try to reconnect our UDP sockets toward backends on some kind of
network errors that indicate a topology change, but we need to be
careful to handle the case where we actually fail to reconnect, as
we end up with no remaining sockets to use.
This commit properly deals with this case by pausing the thread handling
UDP responses from the backend, instead of having it enter a busy loop,
and by attempting to reconnect if we get a `bad file number` error when
trying to send a UDP datagram to the backend.
Remi Gacogne [Wed, 8 Mar 2023 17:25:30 +0000 (18:25 +0100)]
YaHTTP: Prevent integer overflow on very large chunks
If the chunk_size is very close to the maximum value of an integer,
we trigger an integer overflow when checking if we have a trailing
newline after the payload.
Reported by OSS-Fuzz as:
https://oss-fuzz.com/testcase-detail/6439610474692608
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=56804
Remi Gacogne [Mon, 12 Jun 2023 09:04:51 +0000 (11:04 +0200)]
dnsdist: Remove a racy test in the AsynchronousHolder unit tests
We are adding an expired event so the worker thread of the
AsynchronousHolder can pick it up immediately, even before we come
back from the call to push(), which leads to a racy test.
This was observed on GitHub Actions when running with TSAN:
```
FAIL: testrunner
================
Running 170 test cases...
test-dnsdistasync.cc(156): error: in "test_dnsdistasync/test_AddingExpiredEvent": check !holder->empty() has failed
*** 1 failure is detected in the test module "unit"
FAIL testrunner (exit status: 201)
```
Remi Gacogne [Tue, 13 Jun 2023 12:08:56 +0000 (14:08 +0200)]
dnsdist: Increment the "dyn blocked" counter for eBPF blocks as well
Regular, userspace blocks increment the "dyn blocked" counter for every
dropped query. The eBPF blocks are executed in kernelspace and thus do
not increment that counter at all, which makes it challenging for
reporting to do its job. On the other hand we want our eBPF code to
be as efficient as possible since it is used when performance really
matters.
This commit updates the counter when a eBPF dynamic block is removed,
which is a compromise between the performance impact and a slight
reporting delay.
Remi Gacogne [Wed, 28 Jun 2023 13:23:35 +0000 (15:23 +0200)]
Work around Red Hat 8 pooping the bed in OpenSSL's headers
The openssl/kdf.h header on EL8 is invalid because someone backported
a work-in-progress feature to an older OpenSSL branch and did not
bother to backport the fixes that were added later.
Red Hat declined to fix their mess and helpfully suggested we do the
work instead in https://bugzilla.redhat.com/show_bug.cgi?id=2215856
dnsdist: Fix a crash when X-Forwarded-For overrides the initial source IP
When both the processing of X-Forwarded-For DNS-over-https headers
(`trustForwardedForHeader=true`) and a maximum number of concurrent
TCP connections per client (`setMaxTCPConnectionsPerClient()`) are
enabled, dnsdist could crash because of an uncaught exception:
```
dnsdist[X]: terminate called after throwing an instance of 'std::runtime_error'
dnsdist[X]: what(): DOH thread failed to launch: map::at
```
This was caused by the TCP connection being first accounted for with the
initial source IP (from the upstream HTTP proxy) but later released using
the IP extracted from the X-Forwarded-For header, leading to an unexpected
failure to locate the corresponding entry in the map.
We might not actually want to enforce the maximum number of concurrent
TCP connections per client when X-Forwarded-For processing is enabled,
though, because we usually want to rate limit the actual client and
not the HTTP proxy, but X-Forwarded-For being set per HTTP query, instead
of per-connection, makes that pretty much impossible at our level since
the same connection from the HTTP proxy can be reused for several clients.
The proxy protocol would be a better option to enforce that limit.
Remi Gacogne [Mon, 15 May 2023 12:10:55 +0000 (14:10 +0200)]
dnsdist: Stop setting SO_REUSEADDR on outgoing UDP client sockets
`SO_REUSEADDR` is useful on TCP server sockets to allow binding quickly
after restarting the process without waiting `TIME_WAIT` seconds, or
to allow some port reuse on BSD. It also allows reusing a port more
quickly for TCP client sockets.
For UDP sockets, however, Linux allows two sockets to be bound to the
same address and port, and will distribute all packets to the most
recent socket, which is very unexpected, to say the least.
Remi Gacogne [Thu, 11 May 2023 13:27:07 +0000 (15:27 +0200)]
dnsdist: Properly set the size of the UDP health-check response
We forgot to resize the response buffer to what we actually got,
so the initial buffer size (512) was mistakenly used later on.
Technically this should not be an issue as the buffer is large
enough, but that prevents us from reporting that the response
was broken if it not large enough for a DNS header, for example.
Remi Gacogne [Thu, 11 May 2023 13:07:01 +0000 (15:07 +0200)]
dnsdist: Account for the health-check run time between two runs
We used to wait one full second between every run, which only makes
sense if the runs are not taking a long time. But as soon as we have
at least one check timing out, the run is taking roughly the time
of the longest timeout configured, so after this commit we:
- do not wait at all if the last run took more than a full second
- wait one second minus the elapsed time of the last run otherwise
Otto Moerbeek [Tue, 21 Mar 2023 12:34:35 +0000 (13:34 +0100)]
rec and dnsdist: fix a case of potential unaligned header access
I addded an argument to ageDNSPacket to circumvent having to do it in
two places in rec.
I am also wondering if the break in ageDNSPakcet() is right.
I suspect we want to continue with other records even if we see an OPT
(which does not *have* to be the last as far as I know)
Remi Gacogne [Thu, 16 Mar 2023 13:18:49 +0000 (14:18 +0100)]
auth: Prevent a race during the processing of SVC auto-hints
When `svc-autohints` is enabled, the content of SVCB and HTTPS records
is modified in `PacketHandler::doAdditionalProcessing()` to expand
the IPv4 and IPv6 with their actual values.
This causes an issue because the content of these records might be
shared between threads, via the record cache, and one thread could
be trying to read from the internal `std::set` while a second thread
is altering it, leading to a data race and possibly to memory corruption
and a crash.
This is correctly detected by TSAN:
```
WARNING: ThreadSanitizer: data race (pid=102795)
Write of size 8 at 0x7b3400010350 by thread T33:
#0 operator delete(void*) <null> (pdns_server+0x211b7c) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
#1 std::__new_allocator<std::_Rb_tree_node<SvcParam>>::deallocate(std::_Rb_tree_node<SvcParam>*, unsigned long) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/new_allocator.h:158:2 (pdns_server+0x33fc78) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
#2 std::allocator_traits<std::allocator<std::_Rb_tree_node<SvcParam>>>::deallocate(std::allocator<std::_Rb_tree_node<SvcParam>>&, std::_Rb_tree_node<SvcParam>*, unsigned long) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/alloc_traits.h:496:13 (pdns_server+0x33fc78)
#3 std::_Rb_tree<SvcParam, SvcParam, std::_Identity<SvcParam>, std::less<SvcParam>, std::allocator<SvcParam>>::_M_put_node(std::_Rb_tree_node<SvcParam>*) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/stl_tree.h:565:9 (pdns_server+0x33fc78)
#4 std::_Rb_tree<SvcParam, SvcParam, std::_Identity<SvcParam>, std::less<SvcParam>, std::allocator<SvcParam>>::_M_drop_node(std::_Rb_tree_node<SvcParam>*) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/stl_tree.h:632:2 (pdns_server+0x33fc78)
#5 std::_Rb_tree<SvcParam, SvcParam, std::_Identity<SvcParam>, std::less<SvcParam>, std::allocator<SvcParam>>::_M_erase_aux(std::_Rb_tree_const_iterator<SvcParam>) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/stl_tree.h:2495:7 (pdns_server+0x33fc78)
#6 std::_Rb_tree<SvcParam, SvcParam, std::_Identity<SvcParam>, std::less<SvcParam>, std::allocator<SvcParam>>::erase[abi:cxx11](std::_Rb_tree_const_iterator<SvcParam>) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/stl_tree.h:1197:2 (pdns_server+0x33fc78)
#7 std::set<SvcParam, std::less<SvcParam>, std::allocator<SvcParam>>::erase[abi:cxx11](std::_Rb_tree_const_iterator<SvcParam>) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/stl_set.h:655:21 (pdns_server+0x33fc78)
#8 SVCBBaseRecordContent::setHints(SvcParam::SvcParamKey const&, std::vector<ComboAddress, std::allocator<ComboAddress>> const&) /work/pdns/pdns/dnsrecords.cc:768:14 (pdns_server+0x33fc78)
#9 PacketHandler::doAdditionalProcessing(DNSPacket&, std::unique_ptr<DNSPacket, std::default_delete<DNSPacket>>&) /work/pdns/pdns/packethandler.cc:565:16 (pdns_server+0x4ed330) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
#10 PacketHandler::doQuestion(DNSPacket&) /work/pdns/pdns/packethandler.cc:1794:5 (pdns_server+0x4f79b4) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
PowerDNS#11 PacketHandler::question(DNSPacket&) /work/pdns/pdns/packethandler.cc:1175:10 (pdns_server+0x4f649a) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
PowerDNS#12 MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::distribute(int) /work/pdns/pdns/./distributor.hh:220:14 (pdns_server+0x260f70) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
PowerDNS#13 MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::MultiThreadDistributor(int)::'lambda'()::operator()() const /work/pdns/pdns/./distributor.hh:179:25 (pdns_server+0x260b31) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
PowerDNS#14 void std::__invoke_impl<void, MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::MultiThreadDistributor(int)::'lambda'()>(std::__invoke_other, MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::MultiThreadDistributor(int)::'lambda'()&&) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/invoke.h:61:14 (pdns_server+0x260b31)
PowerDNS#15 std::__invoke_result<MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::MultiThreadDistributor(int)::'lambda'()>::type std::__invoke<MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::MultiThreadDistributor(int)::'lambda'()>(MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::MultiThreadDistributor(int)::'lambda'()&&) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/invoke.h:96:14 (pdns_server+0x260b31)
PowerDNS#16 void std:thread:_Invoker<std::tuple<MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::MultiThreadDistributor(int)::'lambda'()>>::_M_invoke<0ul>(std::_Index_tuple<0ul>) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/std_thread.h:258:13 (pdns_server+0x260b31)
PowerDNS#17 std:thread:_Invoker<std::tuple<MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::MultiThreadDistributor(int)::'lambda'()>>::operator()() /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/std_thread.h:265:11 (pdns_server+0x260b31)
PowerDNS#18 std:thread:_State_impl<std:thread:_Invoker<std::tuple<MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::MultiThreadDistributor(int)::'lambda'()>>>::_M_run() /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/std_thread.h:210:13 (pdns_server+0x260b31)
PowerDNS#19 execute_native_thread_routine /usr/src/debug/gcc/gcc/libstdc++-v3/src/c++11/thread.cc:82:18 (libstdc++.so.6+0xd72c2) (BuildId: 6fe66a2d539a78c993bd2d377e00fad389220963)
SUMMARY: ThreadSanitizer: data race (/work/pdns-rgacogne/pdns/pdns_server+0x211b7c) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593) in operator delete(void*)
```
To prevent this issue, this commit clones the content of SVCB/HTTPS
records before modifying the copy. the drawback is that we need to
do this operation every single time we process them.
projects / thirdparty / pdns.git / log
