Replace the hysteretic hi_water/lo_water switch with a stochastic
check: always false below lo_water, always true at or above hi_water,
linearly ramped probability in between. This spreads cache cleaning
across many inserts instead of triggering a thundering herd once the
hi_water mark is crossed (which causes every addrdataset to enter the
LRU purge path simultaneously and serializes lookups behind the node
write locks).
The is_overmem atomic and its stores are no longer needed and are
removed. The existing tests that asserted specific hysteretic state
transitions are simplified to check only the deterministic boundaries.
(cherry picked from commit ee24d2a1c3361dcc1c48fb29bb2e0b91bc3405e8)
Previously, the user of dns_dispatch API had to first call
dns_dispatch_gettcp() and if that failed create a new TCP dispatch with
dns_dispatch_createtcp(). This has been changed and the TCP connection
reuse happens transparently inside dns_dispatch_createtcp(). There are
separate buckets for dns_resolver, dns_request and dns_xfrin units, so
these don't get mixed together.
(cherry picked from commit d5ee86b799)
dns_rdataslab_fromrdataset(), dns_rdataslab_merge() and
dns_rdataslab_subtract() summed per-record storage into an
unsigned int with no upper-bound check. An RRset whose total
encoded size exceeds DNS_RDATA_MAXLENGTH cannot fit in a DNS
message and is unservable; building its in-memory representation
only burns memory on data that will fail at response time, and at
the upper bound the running sum could in theory wrap.
Cap the running total at DNS_RDATA_MAXLENGTH and return ISC_R_NOSPACE
when exceeded. Update the qpdb cache memory-purge test to use a
record size that fits within the new limit.
Assisted-by: Claude:claude-opus-4-7
(cherry picked from commit f9d24b1b85)
The wire-format RSA DNSKEY parser was the only key path with no upper
bound on the public exponent — opensslrsa_parse and opensslrsa_fromlabel
already cap at RSA_MAX_PUBEXP_BITS. An attacker-controlled DNSKEY could
therefore force a validator to compute s^e mod n with e up to ~|n| bits,
amplifying every verify by ~120x for typical 2048-bit moduli (OpenSSL
itself only caps the exponent for moduli above 3072 bits). Apply the
same bit-count cap to wire-format keys.
Assisted-by: Claude:claude-opus-4-7
(cherry picked from commit ab8c1a77e0)
Add a REQUIRE(isc_loop() == loop) assertion to isc_work_enqueue()
to strictly enforce that work is enqueued from the loop it is
assigned to. This loudly prohibits cross-thread queue manipulation
before it inevitably turns into a concurrency debugging nightmare.
(cherry picked from commit f1311d2d19)
Adds text and wire format unit tests to verify the newly enforced
maximum NSEC3 hash length constraints. These tests ensure that hash
lengths up to the 39-byte maximum are accepted, while larger sizes
correctly fail.
(cherry picked from commit e83a182056)
NSEC3 hashes are required to fit within a single DNS label. Since there
are 5 bits per label byte without pad characters, the maximum hash size
is floor(63*5/8) (39 bytes).
This patch enforces this maximum length for unknown algorithms, while
strictly enforcing the exact expected digest length for known algorithms
like SHA-1.
(cherry picked from commit 3801d0ebbf)
Add two short records to example.com.db that cause assertion failures
when converted to wire form.
The checks added to tests.sh are technically not required: the relevant
assertion failures are already hit when the zone is transferred out of
ns1.
Update the relevant unit tests with 1-byte records.
Co-authored-by: Mark Andrews <marka@isc.org>
(cherry picked from commit ce1d68cbc5)
This commit implements a batch update function for qpzone. The main
reason for this is speed: using addrdataset would cause a qp transaction
per rrdataset added, leading to a substantial slowdown compared to
RBTDB. The new API results in a qp transaction per applied diff.
(cherry picked from commit da53708dcb)
These functions don't need to be called from multiple places and
by making them static we will detect when they are not added to the
list functions to be tested.
(cherry picked from commit 22d664aa15)
The `assert_in_range()` function was deprecated in favor of
`assert_int_in_range()` and `assert_uint_in_range()`. Add compatibility
shims for cmocka<2.0.0 and use the new functions.
(cherry picked from commit 6843a4bd9a)
This is a new seek function for dbiterator that is meant to find an
NSEC3 node in a zone database. The difference with dns_dbiterator_seek
is that if the node does not exist, this seek function will point the
iterator to the next NSEC3 name.
(cherry picked from commit 41159e9062)
previously, there were over 40 separate definitions of CHECK macros, of
which most used "goto cleanup", and the rest "goto failure" or "goto
out". there were another 10 definitions of RETERR, of which most were
identical to CHECK, but some simply returned a result code instead of
jumping to a cleanup label.
this has now been standardized throughout the code base: RETERR is for
returning an error code in the case of an error, and CHECK is for jumping
to a cleanup tag, which is now always called "cleanup". both macros are
defined in isc/util.h.
(cherry picked from commit 52bba5cc34)
Since we are using system routines for randomness, there's no point
in spending time and run the statistical suite for testing PRNG.
(cherry picked from commit 90b3def5e9)
When generating a new key, dnssec-keygen checks for possible
key ID collisions with existing keys. The dnssec.c:findmatchingkeys()
function, which is supposed to get the list of the existing keys,
fails to do that for the existing KEY rrtype keys (i.e. generated
using 'dnssec-keygen -T KEY') because it doesn't pass down to the
dst_key_fromnamedfile() -> dst_key_read_public() functions the type
of the keys it's interested in. Fix the issue by introducing a new
function parameter which tells in which type of keys the caller is
currently interested in.
(cherry picked from commit 49b7ce9a54)
It was discovered in an upcoming academic paper that a xoshiro128**
internal state can be recovered by an external 3rd party allowing to
predict UDP ports and DNS IDs in the outgoing queries. This could lead
to an attacker spoofing the DNS answers with great efficiency and
poisoning the DNS cache.
Change the internal random generator to system CSPRNG with buffering to
avoid excessive syscalls.
Thanks Omer Ben Simhon and Amit Klein of Hebrew University of Jerusalem
for responsibly reporting this to us. Very cool research!
(cherry picked from commit cffcab9d5f)
The dns_qpmulti_memusage() causes assertion failure when called on
freshly created qpmulti instance because the qp->usage hasn't been
allocated yet.
(cherry picked from commit b2f653b332)
The beauty and horrors of the C - the compiler properly detects variable
shadowing, but you can freely shadow a standard function 'free()' with
variable called 'free'. And if you reference 'free()' just as 'free'
you get the function pointer which means you can do also pointer
arithmetics, so 'free > 0' is always valid even when you delete the
local variable.
Replace the local variables 'free' with a name that doesn't shadow the
'free()' function to prevent future hard to detect bugs.
(cherry picked from commit 855960ce46)
We need disable clang-format here to preserve the brackets around
the string concatenation to prevent -Wstring-concatenation -Werror
breaking the build.
(cherry picked from commit eeafcee7ad)
Coverity flagged a potential divide by zero error in collect in
qpmulti.c when the elapsed time is zero but that is only called
once the elapsed time is greater than or equal to RUNTIME (1/4
second) so INSIST this is the case.
(cherry picked from commit 081dbb1108)
In a previous change, the "algorithm" value passed to
dns_tsigkey_create() was changed from a DNS name to an integer;
the name was then chosen from a table of known algorithms. A
side effect of this change was that a query using an unknown TSIG
algorithm was no longer handled correctly, and could trigger an
assertion failure. This has been corrected.
The dns_tsigkey struct now stores the signing algorithm
as dst_algorithm_t value 'alg' instead of as a dns_name,
but retains an 'algname' field, which is used only when the
algorithm is DST_ALG_UNKNOWN. This allows the name of the
unrecognized algorithm name to be returned in a BADKEY
response.
(cherry picked from commit decf461d68846d6754c1f64790c3f9006d158a1d)
The pthread-based implementation of the isc_rwlock_tryupgrade()
function always returns ISC_R_LOCKBUSY. Fix the test by adding
conditional checks.
(cherry picked from commit c27659bc62)
We were failing to account for the length byte before the OID.
See RFC 4034.
Algorithm number 254 is reserved for private use and will never be
assigned to a specific algorithm. The public key area in the DNSKEY
RR and the signature area in the RRSIG RR begin with an unsigned
length byte followed by a BER encoded Object Identifier (ISO OID) of
that length. The OID indicates the private algorithm in use, and the
remainder of the area is whatever is required by that algorithm.
Entities should only use OIDs they control to designate their private
algorithms.
(cherry picked from commit ca7355b7d0)
Previously changed mem_test (!10320) introduces a test which checks for
the value of `__FILE__`, which is different if the build is done
out-of-tree or not, even though this is not relevant for the test (only
the base filename is). This result in a broken test for out-of-tree
builds. Fix this by changing the way the "grep" is done in the test,
ignoring the optional path prefix in the filename.
(cherry picked from commit b7ae514b3b)
When allocating memory under -m trace|record, the __FILE__ pointer is
stored, so it can be printed out later in order to figure out in which
file an allocation leaked. (among others, like the line number).
However named crashes when called with -m record and using a plugin
leaking memory. The reason is that plugins are unloaded earlier than
when the leaked allocations are dumped (obviously, as it's done as late
as possible). In such circumstances, __FILE__ is dangling because the
dynamically loaded library (the plugin) is not in memory anymore.
Fix the crash by systematically copying the __FILE__ string
instead of copying the pointer. Of course, this make each allocation to
consume a bit more memory (and longer, as it needs to calculate the
length of __FILE__) but this occurs only under -m trace|record debugging
flags.
In term of unit test, because grepping in C is not fun, and because the
whole "syntax" of the dump output is tested in other tests, this simply
search for a substring in the whole buffer to make sure the expected
allocations are found.
(cherry picked from commit 4eb2cd364a)
Unit test for isc_netaddr_masktoprefixlen were missing IPv6 mask cases.
Add those and few other IPv4 cases. Also, the test is refactored in
order to make it easy to add new cases.
(cherry picked from commit ad2c3fa4f7)
The string literal initialalising compressed was too big for the
array as it has an unwanted NUL terminator. This is allowed for
in C for historical reasons but produces a warning with some
compilers. Adjust the declaration to include the NUL and adjust
the users to pass in an adjusted size which excludes the NUL rather
than sizeof(compressed).
(cherry picked from commit 6a6b6be824)
when a requested name is found in the QP trie during a lookup, but its
records have been marked as nonexistent by a previous deletion, then
it's treated as a partial match, but the foundname could be left
pointing to the original qname rather than the parent. this could
lead to an assertion failure in query_findclosestnsec3().
the db_test unit test now looks up an empty nonterminal node
to exercise the behavior of the step() function in qpzone.
(cherry picked from commit ecde0ea2d7)
DNSKEY, KEY, RRSIG and SIG constraints have been relaxed to allow
empty key and signature material after the algorithm identifier for
PRIVATEOID and PRIVATEDNS. It is arguable whether this falls within
the expected use of these types as no key material is shared and
the signatures are ineffective but these are private algorithms and
they can be totally insecure.
(cherry picked from commit b048190e23)
Instead of having many node_lock_count * sizeof(<member>) arrays, pack
all the members into a qpzone_bucket_t that is cacheline aligned and have
a single array of those.
(cherry picked from commit 6dcc398726)
This removes the db_nodelock_t structure and changes the node_locks
array to be composed only of isc_rwlock_t pointers. The .reference
member has been moved to qpdb->references in addition to
common.references that's external to dns_db API users. The .exiting
members has been completely removed as it has no use when the reference
counting is used correctly.
(cherry picked from commit 431513d8b3)
