check_deadlock() aborted any fetch whose name equals the owner of a
chaining rdataset, assuming nothing the validator needs can live at an
alias. That is true for a CNAME, but a DNAME aliases only the names
below its owner: with a DNAME at a zone apex, the DNSKEY signing the
DNAME lives at the owner name itself, so every answer synthesized from
a signed apex DNAME failed validation whenever that key was not
already validated in the cache.
Chaining CNAMEs, including those synthesized from a DNAME, still cover
the self-join case the check was added for.
(cherry picked from commit 057ae0adb8)
Ignore NSEC3 records that failed in the sub-validator when determining
whether an insecure delegation is legitimate.
Fixes: isc-projects/bind9#5970
(cherry picked from commit 1bd458d3ba)
An unvalidated NXDOMAIN (e.g. from a CD=1 query) marked every RRset at
the name ancient without checking trust, evicting DNSSEC-validated data.
Keep the cache unchanged when any existing RRset is already secure.
dns_ncache_add() now returns DNS_R_UNCHANGED for the rejected add;
negcache() serves a matching cached negative or the queried type, else
SERVFAIL (never the unrelated RRset the add bound), and rctx_ncache()
forwards it so the fetch fails fast.
(cherry picked from commit a7a90eb9d8)
We were triggering an assertion when trying to copy a private record
to a buffer for modifying. Extend the private type detection and
copy the contents after we have rejected invalid private records.
(cherry picked from commit 7182a03b82)
Reject referrals from root/global forwarders, where there is no narrower
forward-zone apex for name_external() to enforce.
(cherry picked from commit 18f4db8f1b)
Apply the existing name_external() bailiwick check to NS RRsets
processed as referrals in rctx_authority_negative(), and enforce the
same check again in rctx_referral() before caching or following the
delegation.
This prevents a forward-first forwarder from installing a parent
zone-cut above the configured forward zone via an authority-section
NS RRset.
(cherry picked from commit 492d11fa39)
tostruct_nid() and tostruct_l64() read the 16-bit preference with the
non-consuming uint16_fromregion() and then memmove()'d the whole
remaining region -- still ten octets, still anchored at the preference
-- into the eight-octet nid[]/l64[] arrays. That folded the preference
into the first two locator octets and stored two octets past the end of
the array. tostruct_l32() shares the root cause: it read the 32-bit
locator from the same unconsumed offset, so the value was built from the
preference plus the first two locator octets.
Consume the two preference octets first, matching the sibling
tostruct_lp(), and assert the expected framing on the fixed-size types.
(cherry picked from commit 5aa279bbea)
find_zone_keys() collects every matching private key into a local list,
hands the first DNS_MAXZONEKEYS keys to the caller, and frees the rest.
On overflow it destroyed only the first surplus key before breaking out
of the loop, so any keys after it stayed linked on the local list and
were lost when the function returned.
Unlink and destroy every list entry, transferring a key to the caller
only while under the limit. No entry is left behind, so a zone with
more than DNS_MAXZONEKEYS matching keys no longer leaks memory on each
signing attempt.
(cherry picked from commit a957dd13fa)
The idle timeout that bounds how long a reused outgoing TCP/TLS
connection is held open for reuse was only tunable through the 'named -T
tcpidletimeout' developer hook added earlier on this branch. Make it a
proper configuration option, tcp-reuse-timeout (options block, in units
of 100 milliseconds like the other tcp-*-timeout options), and drop the
-T hook.
(cherry picked from commit 477130cf8e)
When 'ISC_R_TIMEDOUT' is received in 'tcp_recv()', it times out the
oldest response in the active responses queue, and only after that it
checks whether other active responses have also timed out. So when
setting a timeout value for a read operation after a successful
connection, it makes sense to take the timeout value from the oldest
response in the active queue too, because, theoretically, the responses
can have different timeout values, e.g. when the TCP dispatch is shared.
Currently 'resp' is always NULL. Previously when connect and read
timeouts were not separated in dispatch this affected only logging, but
now since we are setting a new timeout after a successful connection,
we need to choose a suitable response from the active queue.
(cherry picked from commit e61ba5865f)
(partially cherry-picked from commit 64ffbe82c0)
The dns_dispatch_gettcp() function is used for finding an existing TCP
connection that can be reused for sending a query from a specified local
address to a specified remote address. The logic for matching the
provided <local address, remote address> tuple to one of the existing
TCP connections is implemented in the dispatch_match() function:
- if the examined TCP connection already has a libuv handle assigned,
it means the connection has already been established; therefore,
compare the provided <local address, remote address> tuple against
the corresponding address tuple for the libuv handle associated with
the connection,
- if the examined TCP connection does not yet have a libuv handle
assigned, it means the connection has not yet been established;
therefore, compare the provided <local address, remote address>
tuple against the corresponding address tuple that the TCP
connection was originally created for.
This logic limits TCP connection reuse potential as the libuv handle
assigned to an existing dispatch object may have a more specific local
<address, port> tuple associated with it than the local <address, port>
tuple that the dispatch object was originally created for. That's
because the local address for outgoing connections can be set to a
wildcard <address, port> tuple (indicating that the caller does not care
what source <address, port> tuple will be used for establishing the
connection, thereby delegating the task of picking it to the operating
system) and then get "upgraded" to a specific <address, port> tuple when
the socket is bound (and a libuv handle gets associated with it). When
another dns_dispatch_gettcp() caller then tries to look for an existing
TCP connection to the same peer and passes a wildcard address in the
local part of the tuple, the function will not match that request to a
previously-established TCP connection (unless isc_nmhandle_localaddr()
returns a wildcard address as well).
Simplify dispatch_match() so that the libuv handle associated with an
existing dispatch object is not examined for the purpose of matching it
to the provided <local address, remote address> tuple; instead, always
examine the <local address, remote address> tuple that the dispatch
object was originally created for. This enables reuse of TCP
connections created without providing a specific local socket address
while still preventing other connections (created for a specific local
socket address) from being inadvertently shared.
(cherry picked from commit 086c325ad3)
A reused TCP/TLS dispatch with no outstanding responses was left in the
reuse pool with no read pending, so a peer closing the idle connection
went unnoticed: the socket lingered in CLOSE-WAIT and the dead dispatch
was later handed to a new query, which failed and the fetch timed out.
Keep a read pending on an idle connected dispatch, bounded by an idle
timeout, so the close is seen promptly and the connection is dropped
from the pool instead of reused.
The idle read may only be (re)armed while the dispatch is still
connected; arming it on a dispatch that is already shutting down
re-reads a dying handle and double-schedules a netmgr job.
On shutdown, close the connection as soon as the dispatch reaches its
terminal state instead of waiting for the last reference to drop, so an
unexpected read (or a peer-side close) cannot leave the socket in
CLOSE-WAIT while a reference still lingers.
(cherry picked from commit febeac215d)
Named was accepting DS records for sibling zones when it shouldn't
have. Only DS rrsets that match the delegation name should have
been accepted.
Remove ds_name from struct respctx as it is no longer useful.
(cherry picked from commit 123934931f)
Add a check that an NSEC record being used as a proof of nonexistence
for a given name is not signed by a name lower in the DNS hierarchy than
the one in question.
Fixes: isc-projects/bind9#5876
(cherry picked from commit c5f99f3508)
Eviction of an entry owned by another loop was bounced to that loop via
isc_async_run(), so a queued list removal could run after the cache had
freed its LRU lists. Use a single mutex-guarded LRU list instead, removing
entries synchronously under the lock, and let each entry hold its own
memory-context reference so the RCU free never touches a gone loop.
(cherry picked from commit bb43ecaf51)
A response that failed the signature check with a missing or unexpected
TSIG used to set nextitem, so the resolver kept reading the dispatch for
another response. When the response was truncated with the TSIG cut off
the end of the wire, no further response ever arrived and the fetch
stalled until resolver-query-timeout.
Treat an unauthenticated response like every other signature-check
failure and finish the fetch immediately. A response carrying a missing
or bogus TSIG now yields SERVFAIL instead of being skipped in favour of
a later one; the cookie system test that fed a spoofed TSIG response is
updated to expect that. The unauthenticated data is still never
returned.
(cherry picked from commit 2327277f90)
The dns_update_signaturesinc() updates zone signatures in chunks,
keeping its current state in 'zone->rss_state'. When a zone shuts
down, the signature update process is canceled, and all the data
in the state is not freed.
Create a new dns_update_state_clear() function which can be called
from dns_zone_free() to free the memory.
(cherry picked from commit 5e12906669)
When the zone is shutting down jump straight to cleanup, otherwise
an assertion failure is possible, e.g. because zone->loop can be
already NULL.
(cherry picked from commit b56e3cd2ee)
When a DNS UPDATE messages is received, the zone_send_secureserial()
function can schedule a new receive_secure_serial() call with a new
'rss' object before a previous one had a chance to be fully processed.
This can cause an assertion failure in receive_secure_serial() with a
new 'rss' object (when the old one was rescheduled because it got
DNS_R_CONTINUE from dns_update_signaturesinc()). In other words:
1. receive_secure_serial() called with rss, sets zone->rss = rss,
reschedules because of DNS_R_CONTINUE
2. receive_secure_serial() called with rss_new, INSIST fails because
zone->rss != rss_new), i.e. this was called before the old 'rss'
was fully processed
Change the code logic by introducing a new 'rss_next' field and making
sure the old 'rss' is complete before starting processing the new
one.
(cherry picked from commit 5118f3bad6)
dns_rbtnodechain_next() returns DNS_R_NEWORIGIN rather than
ISC_R_SUCCESS whenever the successor lies in a different RBT origin,
which is the case when the queried name's predecessor is an empty
non-terminal with children. Treating that as a failure made
wildcard_blocked() miss the empty non-terminal that must block the
wildcard, so the expansion proceeded and later tripped an assertion.
activeempty() already handled both results.
When adding to dnssec-policy:
cdnskey no;
cds-digest-types { };
and then reconfig the server, named must remove existing CDS and CDNSKEY
records. Note this already worked when adding CDS digest, or setting
'cdnskey yes;', but not when digests were removed from the list, or
when setting 'cdnskey no;'.
(cherry picked from commit 1c4c2cba8b)
A positive wildcard answer, and the NSEC3 proof that the requested
name doesn't exist in the zone, must both be from the same zone.
Otherwise, an NSEC3 from an ancestor zone could be used to interfere
with validation.
We now retrieve the signer name from a wildcard response's signature.
An NSEC3 record cannot be used as a NOQNAME proof for the wildcard
unless it exactly matches the name one level above the NSEC3.
Fixes: isc-projects/bind9#5971
(cherry picked from commit 45c9bd2603)
Offloaded work used two different mechanisms: a per-loop isc_helper
thread for CPU-bound crypto (DNSSEC validation, message signature
checks) and the process-global libuv thread pool for blocking I/O (zone
load and dump, inbound transfer apply). Neither could cancel a queued
task, and the two disagreed about exclusive mode — the helper paused
with its loop under isc_loopmgr_pause() but the libuv pool did not, so
blocking offloaded work kept running while a loop held the exclusive
lock.
Unify both behind isc_work: each loop gets its own worker thread per
lane — FAST for short, bounded tasks and SLOW for long, blocking ones —
fed by a private queue. Separate lanes keep a short crypto task off the
path of a multi-second zone dump once both run on per-loop workers;
every lane parks with isc_loopmgr_pause() so exclusive mode now quiesces
offloaded work too; and a still-queued task can be canceled before it
starts (isc_work_cancel). isc_helper is removed and its callers select a
lane.
(cherry picked from commit a5f13b3410)
When the connect callback's result is ISC_R_SUCCESS and the callback
changes the result because of some condition, the 'xfr' should not
be detached, because it now belongs to the receive callback.
Detach the reference only if the callback's result is non-success.
(cherry picked from commit fb27599b58)
The geoip2.c:match_string() function can incorrectly return 'true'
when matching strings of different lengths (i.e. it matches a
substring). Return 'false' when the lengths of the matched strings
are different.
(cherry picked from commit 8448136b16)
When a Dynamic Update is received that removes the DNSKEY (or CDNSKEY,
or CDS) RRset, remove all records except the ones that are in use
for signing for the zone (with dnssec-policy).
(cherry picked from commit 97b9c9c823)
A resolver that validated DNSSEC accepted RSA DNSKEYs of any modulus
size up to OpenSSL's compile-time ceiling, and accepted any public
exponent the wire format could carry. RSA verification cost grows
sharply with the modulus length, so an authoritative server could
publish an oversized DNSKEY to make each signature check on the
resolver many times more expensive than for a normally sized key.
The intended verify-time cap had no effect because the helper it called
returned the public-exponent bit length rather than the modulus bit
length, so the test was always satisfied. Replace it with an honest
modulus-range check and a stricter exponent check that accepts only odd
exponents in the closed range [3, 2^32 + 1] (covering every Fermat
prime up to F5 and the odd intermediate values seen in deployed keys),
reject anything outside those bounds at every RSA key load path so an
invalid key never reaches the verifier, and keep the same checks at the
verifier as a backstop against future load paths.
(cherry picked from commit 8b2c490811)
A new ISC_ATTR_WARN_UNUSED_RESULT macro now defines
__attribute__((warn_unused_result)) and is used for
dns_rdata_fromstruct().
(cherry picked from commit 07c8c1d242)
The in some functions implementing RFC 5011 key maintenance, the
results of dns_rdata_fromstruct() were not checked. This has been
fixed.
(cherry picked from commit 175d418e28)
In MR !9740, we introduced an optimization that reduces memory usage
by processing rdatas in batches during AXFR.
The maximum batch size is 128, but the batch size was allowed to grow
beyond that limit if all rdatas in a batch were for the same name, as
that allows a more efficient optimization.
This optimization could theoretically allow the batch size arbitrarily
for a sufficient large zone transfer. Since synthetic tests don't show
any performance improvement from the optimization, this MR removes it.
(cherry picked from commit 7502e73333)
A query for an RRSIG is handled as a subset of ANY, so rctx_answer_any()
filters out records that do not match the queried type. When every
record was filtered out (an answer carrying only unrelated types), the
function still returned success with nothing cached, and the fetch then
waited for a validator that was never started until the backstop fetch
timer fired ~12s later. Treat an all-filtered answer as a broken
response, matching how non-meta types already reject a reply with no
usable record.
(cherry picked from commit 938b58a809)
The resolver turned a CNAME response to an RRSIG or NSEC query into
FORMERR inside rctx_answer_cname(). That is redundant -- every caller
already copes with a DNS_R_CNAME or DNS_R_DNAME result -- and it is the
wrong layer, because the resolver cannot tell a legitimate alias from a
broken one. Drop it; a CNAME for one of these types now flows back as
an ordinary alias.
The case that must be stopped lives in the validator. While proving an
unsigned CNAME insecure, proveunsecure() fetches the DS for the CNAME's
own name; because fetches are shared, that fetch re-enters and stalls on
the in-flight fetch the validator is waiting for, deadlocking for about
twelve seconds (GL#5878). Unlike the resolver, the validator knows it
is validating an alias, so check_chaining() now aborts a fetch whose
name matches the chaining rdataset's owner: it cannot advance the chain
and would only self-join.
(cherry picked from commit d0c6219d66)
dns_resolver_createfetch() guarded against fetch loops by comparing the
raw request name/type/domain before any fetch context existed. Move the
check after the context is obtained and run it against the context
itself, and only when we joined an already in-flight context
(!new_fctx) that is also an ancestor in the parent chain. That is the
real loop condition: the new fetch would block waiting on a fetch that
is itself waiting on us. A newly created context waits on nothing, so it
proceeds, bounded by the fetch depth limit and the complementary ADB
loop detection.
(cherry picked from commit 0e04671b65)
Two TSIG-authenticated TKEY DELETE queries for the same dynamic key,
arriving on different worker loops, could each enter
dns_tsigkey_delete() and cause over-decrementing the key refcount.
This has been fixed by making dns_tsigkey_delete() idempotent.
(cherry picked from commit 5c8dcd4419)
Matthijs Mekking authored 2 months ago
CNAME and other record types cannot coexist. DNSSEC records are the
exceptions to this rule.
If the answer contains a name with a CNAME, remove existing RRsets at
the same name from the cache.
If the answer contains a name without a CNAME, remove the CNAME RRset
at the same name from the cache.
(manually picked from commit 69a560fff1)
A signature cannot cover a meta-type (NONE, ANY, AXFR, IXFR, MAILB,
MAILA, OPT, TSIG, TKEY); previously such records were cached by the
recursive resolver and collided with negative-cache entries on the
same owner name, corrupting the QP-trie cache.
Assisted-by: Claude:claude-opus-4-7
(cherry picked from commit c28ba9c3c6)
Once the walk reaches the root, splitting one more label off would
trip an internal assertion and abort named. Stop cleanly with
ISC_R_NOTFOUND so the dispatcher cancels the fetch. Only reachable
through misconfiguration (root configured as a primary with parental
agents, or a parent zone that NODATAs its own NS).
Assisted-by: Claude:claude-opus-4-7
Manually edited to resolve conflicts.
(cherry picked from commit 141e8110f7)
The retry path in resquery_send() that flipped DNS_FETCHOPT_TCP on a
query whose dispatch had already been bound as UDP in fctx_query() had
no effect on the transport actually used, but did leave a stale TCP
bit visible to downstream consumers (dnstap framing, cookie checks,
the AUTHORITY-NS spoofability guard).
The ineffective code has been removed from resquery_send(). The
TCP fallback functionality will be corrected and restored in the
BIND 9.22.
Assisted-by: Claude:claude-opus-4-7
(cherry picked from commit 01523a078a)
Until now, the dispatcher silently dropped UDP responses from the
expected peer that carried the wrong DNS message id and kept listening
for the correct id to arrive within the read timeout. An off-path
attacker who knows the destination address and source port of an
outgoing fetch could exploit that quiet retry window to flood the
resolver with guessed responses; with a gigabit link the per-query
success probability grows linearly with the number of guesses that
arrive before the legitimate answer or the timeout.
Treat any such mismatch as a possible spoofing attempt and let the
resolver immediately retry the same query over TCP, the same control
path the truncation handler already uses.
Add a resolver statistics counter - exposed as 'queries retried over TCP
after a response with mismatched query id' in rndc stats and
'MismatchTCP' in the statistics channel
Assisted-by: Claude:claude-opus-4-7
(cherry picked from commit 11bca1051f)
Bouncing the offload itself to the target loop let the after-work
callback fire on the target thread and run the user's done callback
before the calling thread had published *dctxp / *lctxp. Enqueue on
the calling loop and bounce only the done callback instead, so the
publish is sequenced before the cross-thread hand-off by construction
and cannot be reintroduced by reordering the entry-point body.
(cherry picked from commit 8ae464d552)
The resolver marked every NS RR's glue from a referral for caching with
no aggregate bound, so a parent server returning many NS RRs and many
glue addresses per NS could inflate cache memory long beyond what
resolution can ever use.
Truncate each glue rdataset to DELEG_MAX_GLUES_PER_NS (20) A and 20 AAAA
records before marking it for caching. The NS RRset itself is still
cached in full, bounded by max-records-per-type.
The CHECK_FOR_GLUE_IN_ANSWER macro defaulted to 0 and was never enabled
by the build system, leaving check_answer() and the answer-section glue
scan in rctx_referral() as dead code. Drop them so the surrounding
referral-cache path is easier to reason about.
Ensure that we don't attempt an ACL match for answer addresses
when handling a class-CHAOS zone. This is an additional line of
defense for YWH-PGM40640-74.
(cherry picked from commit e62673c765b52307c800e86f0185fe52b573c145)
NOTIFY and UPDATE messages must specify a data class in the
QUESTION/ZONE section. NONE and ANY are meta-classes and not
appropriate here. Return FORMERR if either is used.
Rejecting messages with a query class of NONE addresses YWH-PGM40640-72,
YWH-PGM40640-82, and YWH-PGM40640-83. Rejecting messages with a query
class of ANY addresses YWH-PGM40640-87, YWH-PGM40640-88, and
YWH-PGM40640-117.
Fixes: isc-projects/bind9#5778Fixes: isc-projects/bind9#5782Fixes: isc-projects/bind9#5783Fixes: isc-projects/bind9#5797Fixes: isc-projects/bind9#5798Fixes: isc-projects/bind9#5853
(cherry picked from commit c66a1b1e1bfd6c79d7b9bc8d4a59e69f4faa1563)
