This change makes the zone table lock-free for reads. Previously, the
zone table used a red-black tree, which is not thread safe, so the hot
read path acquired both the per-view mutex and the per-zonetable
rwlock. (The double locking was to fix to cleanup races on shutdown.)
One visible difference is that zones are not necessarily shut down
promptly: it depends on when the qp-trie garbage collector cleans up
the zone table. The `catz` system test checks several times that zones
have been deleted; the test now checks for zones to be removed from
the server configuration, instead of being fully shut down. The catz
test does not churn through enough zones to trigger a gc, so the zones
are not fully detached until the server exits.
After this change, it is still possible to improve the way we handle
changes to the zone table, for instance, batching changes, or better
compaction heuristics.
Revert refcount debug tracing (commit a8b29f0365), there are better
ways to do it.
Use the dns_qpmethods_t typedef where appropriate.
Some stylistic improvements.
Cleanup the remnants of MS Compiler bits from <isc/refcount.h>, printing
the information in named/main.c, and cleanup some comments about Windows
that no longer apply.
The bits in picohttpparser.{h,c} were left out, because it's not our
code.
Instead of marking the unused entities with UNUSED(x) macro in the
function body, use a `ISC_ATTR_UNUSED` attribute macro that expans to
C23 [[maybe_unused]] or __attribute__((__unused__)) as fallback.
stop and restart the server in the 'tsiggss' test, in order
to confirm that GSS negotiated TSIG keys are saved and restored
when named loads.
added logging to dns_tsigkey_createfromkey() to indicate whether
a key has been statically configured, generated via GSS negotiation,
or restored from a file.
Add some qp-trie tracing macros which can be enabled by a
developer. These print a message when a leaf is attached or
detached, indicating which part of the qp-trie implementation
did so. The refcount methods must now return the refcount value
so it can be printed by the trace macros.
The first working multi-threaded qp-trie was stuck with an unpleasant
trade-off:
* Use `isc_rwlock`, which has acceptable write performance, but
terrible read scalability because the qp-trie made all accesses
through a single lock.
* Use `liburcu`, which has great read scalability, but terrible
write performance, because I was relying on `rcu_synchronize()`
which is rather slow. And `liburcu` is LGPL.
To get the best of both worlds, we need our own scalable read side,
which we now have with `isc_qsbr`. And we need to modify the write
side so that it is not blocked by readers.
Better write performance requires an async cleanup function like
`call_rcu()`, instead of the blocking `rcu_synchronize()`. (There
is no blocking cleanup in `isc_qsbr`, because I have concluded
that it would be an attractive nuisance.)
Until now, all my multithreading qp-trie designs have been based
around two versions, read-only and mutable. This is too few to
work with asynchronous cleanup. The bare minimum (as in epoch
based reclamation) is three, but it makes more sense to support an
arbitrary number. Doing multi-version support "properly" makes
fewer assumptions about how safe memory reclamation works, and it
makes snapshots and rollbacks simpler.
To avoid making the memory management even more complicated, I
have introduced a new kind of "packed reader node" to anchor the
root of a version of the trie. This is simpler because it re-uses
the existing chunk lifetime logic - see the discussion under
"packed reader nodes" in `qp_p.h`.
I have also made the chunk lifetime logic simpler. The idea of a
"generation" is gone; instead, chunks are either mutable or
immutable. And the QSBR phase number is used to indicate when a
chunk can be reclaimed.
Instead of the `shared_base` flag (which was basically a one-bit
reference count, with a two version limit) the base array now has a
refcount, which replaces the confusing ad-hoc lifetime logic with
something more familiar and systematic.
Ensure dns_qpkey_fromname() and dns_qpkey_toname() are inverses.
Excercise a single-threaded dns_qp_t with a fixed set of random keys
and a small chunk size. Use the table of names to ensure that the trie
is behaving as expected. This is (in effect) randomized testing like
the `qpmulti` unit test, but making use of coverage-guided fuzzing
and (in principle) test case minimization.
After deprecating the operating system limits settings (coresize,
datasize, files and stacksize), mark them as ancient and remove the code
that sets the values from config.
Since this is very sensitive code which has often had security
problems in many DNS implementations, it needs a decent amount of
validation. This fuzzer ensures that the new code has the same output
as the old code, and that it doesn't take longer than a second.
The benchmark uses the fuzzer's copy of the old dns_name_fromwire()
code to compare a number of scenarios: many compression pointers, many
labels, long labels, random data, with/without downcasing.
All we need for compression is a very small hash set of compression
offsets, because most of the information we need (the previously added
names) can be found in the message using the compression offsets.
This change combines dns_compress_find() and dns_compress_add() into
one function dns_compress_name() that both finds any existing suffix,
and adds any new prefix to the table. The old split led to performance
problems caused by duplicate names in the compression context.
Compression contexts are now either small or large, which the caller
chooses depending on the expected size of the message. There is no
dynamic resizing.
There is a behaviour change: compression now acts on all the labels in
each name, instead of just the last few.
A small benchmark suggests this is about 2x faster.
Access to the source tree is not available with oss_fuzz. Have
fuzz/dns_message_checksig build and populate a key directory for
the fuzzer to use. This contains a key pair and a zone file which
has the public key from the key pair. Clean it up on shutdown.
Previously stack with buffer for test dns message went out of scope
before the message was processed. For fuzz testing its better to avoid
allocation, so let's avoid allocations completely and use simplest
possible static buffer.
Fixes: #3565
dns_message_checksig is called in a number of scenarios
* on requests and responses
* on multiple opcodes
* with and without signatures
* with TSIG signatures
* with SIG(0) signatures
* with and without configured TSIG keys
* with and without KEY records being present
* signing performed now, in the future and in the past
we use the first two octets of the seed to configure the calling
environment with the remainder of the seed being the rdata of the
TSIG/SIG(0) record.
It's wasteful to use 20 bytes and a pointer indirection to represent
two bits of information, so turn the struct into an enum. And change
the names of the enumeration constants to make the intent more clear.
This change introduces some inline functions into another header,
which confuses `gcovr` when it is trying to collect code coverage
statistics. So, in the CI job, copy more header files into a directory
where `gcovr` looks for them.
There was a proposal in the late 1990s that it might, but it turned
out to be unworkable. See RFC 6891, Extension Mechanisms for
DNS (EDNS(0)), section 5, Extended Label Types.
The remnants of the code that supported this in BIND are redundant.
From an attacker's point of view, a VLA declaration is essentially a
primitive for performing arbitrary arithmetic on the stack pointer. If
the attacker can control the size of a VLA they have a very powerful
tool for causing memory corruption.
To mitigate this kind of attack, and the more general class of stack
clash vulnerabilities, C compilers insert extra code when allocating a
VLA to probe the growing stack one page at a time. If these probes hit
the stack guard page, the program will crash.
From the point of view of a C programmer, there are a few things to
consider about VLAs:
* If it is important to handle allocation failures in a controlled
manner, don't use VLAs. You can use VLAs if it is OK for
unreasonable inputs to cause an uncontrolled crash.
* If the VLA is known to be smaller than some known fixed size,
use a fixed size array and a run-time check to ensure it is large
enough. This will be more efficient than the compiler's stack
probes that need to cope with arbitrary-size VLAs.
* If the VLA might be large, allocate it on the heap. The heap
allocator can allocate multiple pages in one shot, whereas the
stack clash probes work one page at a time.
Most of the existing uses of VLAs in BIND are in test code where they
are benign, but there was one instance in `named`, in the GSS-TSIG
verification code, which has now been removed.
This commit adjusts the style guide and the C compiler flags to allow
VLAs in test code but not elsewhere.
... along with dns_rdataclass_fromtext and dns_rdatatype_fromtext
Most of the test binary is modified named-rrchecker. Main differences:
- reads single RR and exists
- does not refuse meta classes and rr types
We actually do have some fromtext code for meta-things so erroring out
in named-rrchecker would prevent us from testing this code.
Corpus has examples of all currently supported RR types. I did not do
any minimization.
In future use command
diff -U0 \
<(sed -n -e 's/^.*fromtext_\(.*\)(.*$/\1/p' lib/dns/code.h | \
sort) \
<(ls fuzz/dns_rdata_fromtext.in/)
to check for missing RR types.
This commit converts the license handling to adhere to the REUSE
specification. It specifically:
1. Adds used licnses to LICENSES/ directory
2. Add "isc" template for adding the copyright boilerplate
3. Changes all source files to include copyright and SPDX license
header, this includes all the C sources, documentation, zone files,
configuration files. There are notes in the doc/dev/copyrights file
on how to add correct headers to the new files.
4. Handle the rest that can't be modified via .reuse/dep5 file. The
binary (or otherwise unmodifiable) files could have license places
next to them in <foo>.license file, but this would lead to cluttered
repository and most of the files handled in the .reuse/dep5 file are
system test files.
Unify the header guard style and replace the inconsistent include guards
with #pragma once.
The #pragma once is widely and very well supported in all compilers that
BIND 9 supports, and #pragma once was already in use in several new or
refactored headers.
Using simpler method will also allow us to automate header guard checks
as this is simpler to programatically check.
For reference, here are the reasons for the change taken from
Wikipedia[1]:
> In the C and C++ programming languages, #pragma once is a non-standard
> but widely supported preprocessor directive designed to cause the
> current source file to be included only once in a single compilation.
>
> Thus, #pragma once serves the same purpose as include guards, but with
> several advantages, including: less code, avoidance of name clashes,
> and sometimes improvement in compilation speed. On the other hand,
> #pragma once is not necessarily available in all compilers and its
> implementation is tricky and might not always be reliable.
1. https://en.wikipedia.org/wiki/Pragma_once
Replace most "master/slave" terminology in tests with the preferred
"primary/secondary", with the following exceptions:
- When testing the old syntax
- When master is used in master file and master file format terms
- When master is used in hostmaster or postmaster terms
- When master used in legacy domain names (for example in dig.batch)
- When there is no replacement (for example default-masters)
Remove the dynamic registration of result codes. Convert isc_result_t
from unsigned + #defines into 32-bit enum type in grand unified
<isc/result.h> header. Keep the existing values of the result codes
even at the expense of the description and identifier tables being
unnecessary large.
Additionally, add couple of:
switch (result) {
[...]
default:
break;
}
statements where compiler now complains about missing enum values in the
switch statement.
The last remaining defines needed for platforms without NAME_MAX and
PATH_MAX (I'm looking at you, GNU Hurd) were moved to isc/dir.h where
it's prevalently used.
The Makefile.tests was modifying global AM_CFLAGS and LDADD and could
accidentally pull /usr/include to be listed before the internal
libraries, which is known to cause problems if the headers from the
previous version of BIND 9 has been installed on the build machine.
The dns_message_create() function cannot soft fail (as all memory
allocations either succeed or cause abort), so we change the function to
return void and cleanup the calls.
