rec: Log when going Bogus because of a missing SOA in authority
A missing SOA in the authority section of negative (NXDOMAIN, NODATA)
answers in a DNSSEC-secure zone currently leads to a Bogus result,
because the needed NSEC/NSEC3 could not be validated.
Aki Tuomi [Sun, 19 Jul 2020 15:06:14 +0000 (18:06 +0300)]
misc.cc: Resize hostname to final size in getCarbonHostname()
In 5c21b47fbc35ddcb8d939eb8541c6c3bad1080a8 we change how
hostname is allocated. We allocate getMaxHostNameSize for string,
then give the raw buffer for gethostname function, but forget to
resize the string into actual result length, causing the carbon
output to include trailing NUL bytes after hostname.
rec: Validate cached DNSKEYs against the DSs, not the RRSIGs only
DNSKEYs might be cached in a non-validated state ("Indeterminate")
when the DNSSEC mode is set to "Process" and the initial query did
not ask for validation.
We would then validate the DNSKEY records against the RRSIGs, like
for regular records, but not against the DSs.
rec: A ServFail while retrieving DS/DNSKEY records is just that
Before that commit, failing to get the DS or DNSKEY records needed
during validation because of a network issue would trigger a Bogus
DNSSEC validation result because validation could not be performed,
but that should just be a Server Failure instead.
This is especially an issue because the Bogus result would get
inserted into the cache and could stay there for as long as
'max-cache-bogus-ttl' seconds.
Remi Gacogne [Wed, 17 Jun 2020 12:49:55 +0000 (14:49 +0200)]
rec: Copy the negative cache entry before validating it
Otherwise, in the unlikely case that:
- we need to go to the network in order to validate, for example to
get or a DNSKEY ;
- the negative cache cleaning is run at that exact moment ;
- and the entry we have a pointer to gets wiped during that cleanup
we might trigger a heap-based use-after-free (read), possibly leading
to a crash if the memory has been reused already.
Otto Moerbeek [Fri, 5 Jun 2020 08:37:28 +0000 (10:37 +0200)]
Add/modify tests. Also re-check for the cache case. It *is* a bit
unsettling that case causes an ImmediateServFailException, but I do
not like to touch the general flow right now. That would be required
to make the CNAME cache case more similar to the non-cached case.
Otto Moerbeek [Wed, 3 Jun 2020 07:07:56 +0000 (09:07 +0200)]
Correct depth increments.
With the introduction of qname minimization, a function
doResolveNoQNameMinimization() was introduced. This function is
called by doResolve() with depth incremented. Due to the recursive
nature of the resursor algortihm (Nomen est Omen) we end up
incrementing the depth too much. This prompted a review of the other
places depth was incremented, and I believe it should only be done
when calling doResolve(). Especially the case "+ 2" in the getAddrs()
call looks strange to me, as the doResolve() calls in getAddrs()
already call doResolve() with depth + 1.
This fixes #9184 and likely other cases of deep recursion caused
by long CNAME chains.
Remi Gacogne [Mon, 25 May 2020 09:33:19 +0000 (11:33 +0200)]
rec: Defer the NOD lookup until after the response has been sent
If the NOD lookup is slow, for example because the destination
authoritative server is down, doing the NOD lookup before the response
has been sent increases the latency a lot.
This commit moves the actual NOD lookup after the response has been
sent, so we can still use the existing mthread (we might actually need
to do a proper DNS resolution to find the target authoritative server)
without keeping the client waiting.
Remi Gacogne [Tue, 19 May 2020 14:46:33 +0000 (16:46 +0200)]
Fix compilation on systems that do not define HOST_NAME_MAX
On FreeBSD at least, HOST_NAME_MAX is not defined and we need to
use sysconf() to get the value at runtime instead.
Based on a work done by @RvdE to make the recursor compile on
FreeBSD (many thanks!).
Sander Hoentjen [Mon, 16 Dec 2019 21:44:43 +0000 (22:44 +0100)]
Fix build with gcc-10
From an e-mail from Jeff Law <law@redhat.com>:
Subject: Minor problem in pdns, dnsdist and pdns-recursor packages in Fedora
[ All three packages have embedded copies of the same problematic code
and the same patch fixes all three. ]
Red Hat's compiler team continues to try and be proactive in identifying
issues that will arise as a result of the introduction of a new GCC
release into Fedora each spring.
You're being contacted because a package you maintain in Fedora is going
to fail to build with gcc-10 in the spring. Yes, I know that's a few
months away, but it's far easier to fix this stuff proactively now than
wait.
Fixing it now also means that your package will continue to be built
with testing versions of gcc-10 as we proceed through the development
process thus allowing additional issues to be caught early.
Your particular package will fail due to an uninstantiated template for
AsyncLoader<Request>. These kinds of problems are relatively common due to
changes in the tuning of the inliner for gcc-10:
> BUILDSTDERR: /usr/bin/ld: webserver.o: in function `WebServer::serveConnection(std::shared_ptr<Socket>) const':
> BUILDSTDERR: /builddir/build/BUILD/pdns-4.2.1/pdns/webserver.cc:373: undefined reference to `YaHTTP::AsyncLoader<YaHTTP::Request>::feed(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)'
> BUILDSTDERR: collect2: error: ld returned 1 exit status
> BUILDSTDERR: make[3]: *** [Makefile:2751: ixfrdist] Error 1
>
The attached patch arranges for an instance to be instantiated when
compiling reqresp.cpp and is sufficient to fix this problem. The
choice of reqresp.cpp fairly arbitrary IIRC.
Ideally you'll with upstream to get this fixed, but a Fedora patch is
clearly OK as well. I'll install the attached fix into Fedora in a
week or so if I haven't heard from you.
Don't read potentially uninitalized memory if gethostname() failed
If the buffer is smaller than `HOST_NAME_MAX` (64 on Linux but up to
255 bytes in POSIX, which FreeBSD, MacOS etc honor) gethostname()
might return -1 without null-terminating the buffer, causing an
out-of-bounds read.
As we look for the first '.' using `strchr()`, replacing it with a
null byte, we also have a one-byte out-of-bounds write which might
result in a crash or, albeit very unlikely, arbitrary code execution.
Otto Moerbeek [Mon, 10 Feb 2020 13:31:41 +0000 (14:31 +0100)]
Introduce an explicit refreshFromConf arg to RPZIXFRTracker.
Always load and store the rpz refresh value from and to the zone.
That we we can easily decide which value to use: if an explicit
refreshFromConf value is set, use that one, otherwise use the one
in the rpz zone.
projects / thirdparty / pdns.git / log
