In order to make zone.c more readable, we are splitting it up in
separate source files. This moves the zonemgr to its own file
("zonemgr.c").
Since this code accesses the zone structure directly, move the
'struct dns_zonemgr' and its prerequisites to "zone_p.h".
The helper functions 'forward_cancel()', 'zone_xfrdone()',
'zmgr_start_xfrin_ifquota()', and 'zmgr_resume_xfrs() need to be
internally accessible to both source files.
Note: This commit does not compile.
The name 'isdelegation()' was confusing. This function is not checking
whether this message is a delegation, but whether the denial of
existence proofs in this message is a proof of a referral to an
unsigned zone.
The name 'is_unsecure_referral()' is more appropriate.
The isdelegation() was changed to return an isc_result_t because the
idea was to have a separate return value DNS_R_NSEC3ITERRANGE to signal
to the caller we could not verify the proof because of too many
iterations in the NSEC3 record, or perhaps ISC_R_UNEXPECTED for a more
generic cause that verification was not done.
But this would make error handling more fragile and all we care about
is whether we can reliably say the NS bit was not set.
If we can not reliably say so, we have to treat it as an insecure
referrral.
Since the answer is either yes or no, we can revert back to returning
a boolean value.
Replicating CI failures requires the developer to piece together the
sanitizer flags by hand, reducing ergonomics.
Fix this problem by embedding the relevant settings to the executables.
Symbol resolution still needs manual intervention by setting the env
variable `*SAN_SYMBOLIZER_PATH`. However, this doesn't affect any behavior.
When the xfrin_recv_done() function decides to retry the transfer
using AXFR because of a previous error, it calls the xfrin_reset()
function which calls dns_db_closeversion() on 'xfr->ver'. The problem
is that the ixfr processing of a previous message could be still
in process in a worker thread, which then can use freed 'xfr->ver'.
If there is an ongoing worker thread delay the AXFR retry until after
the worker thread has finished its work.
When receiving NSEC records via IXFR, the node was not marked with
havensec because the condition checked the uninitialized output
rdataset type instead of the input rdataset type. This caused
queries for empty non-terminal names in NSEC-signed zones received
via IXFR to return the zone apex NSEC instead of the correct
covering NSEC record.
The bug was introduced in f4b4f030.
Replace the separate pointer+length field pairs in dns_rdata_nsec3_t
(salt/salt_length, next/next_length, typebits/len) and
dns_rdata_nsec3param_t (salt/salt_length) with isc_region_t. This
makes the structs self-describing and eliminates a class of
length-mismatch bugs.
The dns_zone_setnsec3param() signature is updated to take
isc_region_t *salt instead of separate saltlen and salt arguments.
Function signatures for dns_nsec3_addnsec3, dns_db_getnsec3parameters,
and related internal functions still use separate pointer+length pairs
and should be updated in a follow-up.
dns_zone_getloadtime(), dns_zone_getexpiretime(),
dns_zone_getrefreshtime(), and dns_zone_getrefreshkeytime()
cannot fail, so return void instead of ISC_R_SUCCESS.
The caller is supposed to hold the zone lock for 'inline_raw()' and
'inline_secure()', but when adding 'REQUIRE(LOCKED_ZONE(zone));' to
these functions it turned out to be not always the case.
In order to make zone.c more readable, we are splitting it up in
separate source files. This moves the set and get functions to its
own file ("zoneproperties.c").
Since this code accesses the zone structure directly, move the
'struct dns_zone' and its prerequisites to "zone_p.h".
The helper functions 'inline_raw()', 'inline_secure()',
'dns_zone_setview_helper()', 'zone_settimer(), 'set_resigntime()', and
'zone_freedbargs()' need to be internally accessible to both source
files.
A few set/get functions remain in zone.c for now:
- dns_zone_getserial
- dns_zone_getversion
- dns_zone_setviewcommit
- dns_zone_setviewrevert
- dns_zone_get_rpz_num
- dns_zone_set_parentcatz
- dns_zone_get_parentcatz
- dns_zone_setrawdata
- dns_zone_setskr
- dns_zone_getskrbundle
- dns_zone_setnsec3param
- dns_zone_setoption
- dns_zone_getoptions
- dns_zone_getrequesttransporttype
- dns_zone_getredirecttype
- dns__zone_getnotifyctx
- dns_zone_getgluecachestats
- dns_zone_setplugins
- dns_zone_setserial
- dns_zone_getxfr
- dns_zone_getkeystores
When `dns_view_bestzonecut()` is called with a NULL `delegsetp`, it
calls `bestzonecut_zone()` with a NULL `rdataset` pointer but there is a
non-guarded de-reference of the `rdataset` pointer in
`bestzonecut_zone()`.
In practice, the only current situation where `dns_view_bestzonecut()`
is called with NULL `delegsetp` is from a case of `seek_ds()` _and_ the
non-guarded dereference occurs only if there is a static-stub local
zone matching the zonecut `seek_ds()` is looking for. It's unclear if
such flow is actually possible.
The `rdataset` is now always valid inside `dns_view_bestzonecut()`. (It
was initially set only if `delegsetp` was set to avoid extra works in
the qpzone, which can be skipped when `rdataset` is NULL, but this
doesn't really make a difference, considering we are in a slow path
considering the result wasn't found in this case.)
The local variable `zfname` was released in the cleanup part of the
function if not NULL, but it turns out it is now always NULL at that
point.
The flow can get to that part only in two cases: either `zfname` is not
NULL, and then it's ownership is moved to a different variable (thus, it
is now NULL), or `zfname` is already NULL.
Removing the bit of deadcode releasing it.
After gss_accept_sec_context() succeeds, the GSS context is passed
to dst_key_fromgssapi() which transfers ownership to the dst_key.
If a subsequent operation fails (dst_key_fromgssapi itself,
dns_tsigkey_createfromkey, or dns_tsigkeyring_add), the cleanup
label frees the dst_key but only if it was created. If the failure
happened before dst_key_fromgssapi, the GSS context was orphaned.
Delete the GSS context in the cleanup path when it was not
transferred to a dst_key.
When gss_accept_sec_context() completes successfully but
gss_display_name() returns an empty principal, the GSS context
was leaked — it was neither stored in a key nor deleted.
Delete the context and reject with BADKEY in this case. This
should only occur due to a GSS library bug, since a completed
context should always have a valid principal.
Use pre-increment (++ring->generated) instead of post-increment
(ring->generated++) so the comparison against DNS_TSIG_MAXGENERATEDKEYS
happens after counting the new key. With post-increment, one extra key
beyond the limit was allowed before eviction kicked in.
Check for existing non-expired TSIG keys before accepting a new
GSS-API negotiation. Per RFC 3645 Section 4.1.1:
- If a key exists and has not expired, reject with BADNAME
- If a key exists but has expired, delete it and start fresh
Previously, an expired GSS key would permanently block
re-negotiation for that name until the server was restarted.
Use BADKEY rather than BADNAME to avoid creating an oracle for
key name enumeration by unauthenticated attackers.
The dns_rdatavec_subtractrdataset function would copy the old header
using memmove but the old header includes fields such as trust and
reference counts that are atomic.
While the values of those fields were never used, it did cause a benign
race condition. This commit refactors dns_rdatavec_subtractrdataset and
dns_rdatavec_merge not to use memmove.
Now that we track the references at the vecheader level, binding an
rdataset is no longer guaranteed to keep its node alive. Therefore
remove the node pointer from the rdataset, and instead decide whether
glue is required by explicitely passing the owner name to addglue.
This commit adds a level of indirection to the signing operations.
Instead of being intrusive, the qpz_heap will keep track of which
headers must be resigned through a hashmap.
The intent is to make dns_vecheader_t entirely self-contained. In
particular, the ownership structure between the heap and the headers is
flipped. Before, the headers would "own" the heap, now the heap owns
the header.
Change setsigningtime to take the node of the header being changed.
Done to facilitate further refactoring that will remove the header
pointer from vecheader.
This commit changes the deregistration of vecheaders from the heap to
go through a private api instead of the dyndb public one. This is safe
since vecheader is only used by qpzone.
This is done in order to facilitate further refactoring.
The <sys/endian.h> header has existed in macOS since around ~26. This
causes the `htobeNN`/`htoleNN` macros to be redefined in <isc/endian.h>
in terms of <libkern/OSByteOrder.h> when other system headers include
<sys/endian.h>.
Fix this issue by using checking for the existence of <sys/endian.h> in
meson and including it according to the probe result.
Now that TTL-based cleaning has been removed, the dns_expire_ttl enum
value, its switch case in expireheader(), and the deletettl stats counter
(text, XML, JSON) are all dead code. Remove them so the stats channel
no longer reports a permanently-zero counter.
Instead of doing a full sweep of all names and entries before dumping,
expire stale entries lazily as they are encountered during the dump
iteration. This aligns with the QPcache approach of avoiding separate
TTL-based cleaning passes.
dns_adb_flush() retains its explicit full sweep since it needs to
force-expire everything.
Lower the hard floor for max-cache-size from 2 MB to 8 MB to support
resource-constrained environments (e.g. CPE devices) while remaining
safe for LRU-only eviction.
Extract the inline max-cache-size logic from configure_view() into
reusable helpers: configure_max_cache_size(), default_max_cache_size(),
max_cache_size_as_percent(), and sanitized_max_cache_size().
Move DNS_CACHE_MINSIZE and DNS_ADB_MINADBSIZE to public headers and
remove the SIZE_AS_PERCENT sentinel.
Since TTL-based cache cleaning has been removed, an unlimited
max-cache-size would eventually exhaust system memory.
Both 'max-cache-size unlimited;' and 'max-cache-size 0;' now fall
back to the default value (90% of physical memory for recursive
views).
The heaps have been removed, so the separate heap memory context
(hmctx) is no longer needed. Remove it from both dns_cache and
dns_qpcache, along with the HeapMemInUse statistics.
The experiments show that the SIEVE-LRU based mechanism is good enough
as the only mechanism for cleaning up the expired entries from the
cache.
This simplifies the internal logic and memory usage of the cache.
The disadvantage is that the cache use will organically grow until it
hits the overmem cleaning mechanism.
The advantage is that the measurements show that BIND 9 is well behaved
even with 512 MB cache under heavy load.
dns_rdataslab_fromrdataset() set .expire to rdataset->ttl, but the
only consumer (qpcache_addrdataset) immediately overwrote it with
now + rdataset->ttl. Remove the redundant initialization and set the
expire time only once.
When a resolver+auth server has a delegation on a local zone and has a
glue, the glue can only be for in-domain NS.
In this case, when the resolver is looking at the zonecut,
`dns_view_bestzonecut()` synthesizes a delegset from an NS rdataset
found in the local zone (the delegation inside auth zone), and ignores
the glues if any.
As a result, the delegset will contain a single delegation of type
DNS_DELEGTYPE_NS_NAMES, which leads to an ADB fetch. But it's actually an
in-memory fetch, because in this case, the fetch will immediately find
the A/AAAA glues from the local zone.
An alternative approach (not chosen here) would be to make
`dns_view_bestzonecut()`, when converting an NS rdataset into a
`dns_deleg_t`, check for glues for the delegation in the auth zone, and
add those in the `dns_deleg_t`. The delegation would be of type
DNS_DELEGTYPE_NS_GLUES which would avoid the ADB name lookup.
However, that's extra code, extra logic and complexities, for a lookup
that will be done in memory anyway, just a bit later. So for now, this
is not implemented that way.
The test is added, however, to confirm that there is no attempt from the
resolver to get the NS fron the child zone.
because the cache no longer stores delegation (parent-side) NS rrsets,
and authoritative (child-side) NS rrsets don't affect recursion,
it no longer makes sense for qpcache_find() to look for NS rrsets
and return DNS_R_DELEGATION. that code has been removed.
the cache still does search for covering DNAME records. the
check_zonecut() function has been renamed to check_dname() for clarity.
related changes:
- one test case has been removed from the mirror system test, because it
tested the behavior of a cached delegation.
- query_checkrrl() and rpz_rrset_find() have been updated so they no
longer expect cache responses to have DNS_R_DELEGATION response codes.
The get_nameservers path in rctx_nextserver() is only reachable from
rctx_referral(), which already detaches fctx->delegset. Assert that
it is NULL rather than redundantly detaching it, since
dns_view_bestzonecut() requires *delegsetp == NULL.
If both dns_view_bestzonecut() and dns_deleg_fromrdataset() fail,
delegset stays NULL. Passing it to ns_query_recurse() would crash
on the REQUIRE(DNS_DELEGSET_VALID(delegset)) in createfetch().
Return ISC_R_NOTFOUND instead, which lets the caller handle the
failure gracefully.
When resume_dslookup() receives ISC_R_SHUTTINGDOWN or ISC_R_CANCELED,
frdataset (fctx->nsrrset) was not disassociated. While fctx__destroy()
eventually cleans it up, leaving it associated keeps the underlying DB
node referenced longer than necessary.
When a referral lookup is triggered by a QMIN query, it should be
exempt from the fetches-per-zone limit just as the QMIN query itself
is.
Also restart the test server between the fetches-per-server and
fetches-per-zone tests so that leftover statistics from the former
do not pollute the latter.
Another fix is because zone spills and general query drops are no longer
in a strict >= relation (on a parent-centric resolver), so check that
both counters are non-zero instead.
Stop storing the NS referral into the main cache when processing a
negative response. These records are already cached in the delegation
database and are not needed elsewhere.
Update dnssec tests that relied on parent-side NS RRsets being
returned in recursive query responses.
When `query.c` finds a zonecut in the main cache (e.g. from stale NS
records), it must still use the correct delegation for recursion. Look
up the delegation DB via `dns_view_bestzonecut()` first; fall back to
`dns_deleg_fromrdataset()` only if no delegation is found.
This might also be done inside `query_lookup()` instead, with the `qctx`
holding a delegset property, but that approach needs further work to
avoid breakage and it is not clear so far if there would be other use
case of it. Current approach is simpler for now.
The resolver now uses glue addresses from `dns_deleg_t` objects stored
in the delegation database. The main cache is still used for ADB A/AAAA
lookups when no glue is available for a nameserver name.
The resolver's `fctx_getaddresses()` is refactored to, for each
delegation of the delegation set, try to get the address-based finds,
then nameserver name lookups. (Later, the logic to handle DELEG
`include-delegparm=` will be hooked there too.)
Add a new ADB API function that creates a find from a list of addresses
rather than by looking up nameserver names. This enables the resolver
to handle address-based delegations (NS-based with glues or DELEG with
addresses) and name-based delegations uniformly (i.e. the list of finds
from ADB is handled the same way no matter the type of the delegation).
