When a cached name held both a CNAME and records of another type — one stale,
the other still fresh — named with serve-stale could return the expired set
instead of the fresh one, in either direction. It now prefers whichever is fresh.
Merge branch 'ondrej/fix-serve_stale-cname-and-type' into 'main'
See merge request isc-projects/bind9!12282
With serve-stale enabled, stale rdataset headers are kept at a node so
they can be served as a last resort. The find loop, however, accepted a
stale CNAME or stale record of the requested type as a final answer and
broke out of the iteration early, returning stale data even when a fresh
header for the same name appeared later in the scan. Treat STALE(found)
like a missing answer so the loop keeps looking and only falls back to
the stale header when no fresh answer is found.
In rare cases the :iscman:`named` process could terminate unexpectedly
when processing authorized DNS UPDATE messages in quick procession
which are updating a zone with inline-signing enabled. This has been
fixed.
Closes#5816
Merge branch '5816-inline-signing-concurrent-update-fix' into 'main'
See merge request isc-projects/bind9!11982
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.
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.
This new check floods the server with DNS UPDATE messages for an
'inline-signing yes; sig-signing-signatures 1;' zone to see if
it manages to process the updates correctly.
Small cleanup. Found with ninja -C build-dir/ scan-build.
Patch submitted by Tim Rühsen.
Closes#6163
Merge branch '6163-double-initialization-copy-tuple' into 'main'
See merge request isc-projects/bind9!12292
Internal refactoring of the cache database (qpcache) with no functional
change. The slab headers that hold cached rdatasets are now reference
counted and own their memory context and node reference directly, so a
header can outlive the cleaning of its node and be reclaimed
independently of it. Building on that, the per-type slabtop container is
folded into the slab header itself, removing a level of indirection and
one allocation per cached type.
Merge branch 'ondrej/slabheader-reference-counting' into 'main'
See merge request isc-projects/bind9!12285
When the cache is over its size limit, qpcache_miss() runs LRU eviction
while add() is still in progress. Eviction removes a header together
with its RRSIG/covered 'related' partner, so it could free a header the
add still needs -- the new header, the partner it was just paired with,
or the one about to be displaced for max-types-per-name. With 'related'
now a counted reference, that became a use-after-free reachable under
sustained load.
Run the eviction last, after the new header is linked, bound and any
over-limit header removed, and skip the new header and its partner in
the eviction loop (the partner is marked visited so the SIEVE hand still
advances). Per-header removal is factored into header_delete(),
expire_header() and flush_node().
Cache headers are now unlinked from their node as soon as they expire,
so a slabheader is never left in the "ancient, awaiting cleanup" state
that DNS_SLABHEADERATTR_ANCIENT tracked. Drop the attribute, rename
mark_ancient() to header_delete() to reflect that it now removes the
header rather than flagging it (keying idempotency on list membership),
and remove the ancient RRset statistics counter that recorded the state,
which is now always zero; the rdataset statistics array shrinks to
match.
The rdataset-level 'ancient' flag and 'rndc dumpdb -expired' are
unaffected: expiry is derived from the entry's TTL when the rdataset is
bound, not from the slabheader attribute.
qpcache_find, qpcache_findrdataset, and find_headers each repeated the
same logic to pick a header and its RRSIG out of a slabheader and its
'related' link. Pull it into store_headers(), and rename check_header()
to invalid_header() since it returns true when the header should be
skipped.
With headers removed eagerly in mark_ancient there is at most one header
per type at a node, so the separate per-type dns_slabtop container no
longer earns its keep. Fold its fields onto the header -- the link into
the node's list, the RRSIG/covered related pairing, and the SIEVE-LRU
state -- and link headers directly into the node, dropping a level of
indirection and an allocation per cached type.
Now that a bound rdataset keeps references to both the slabheader and
the node, an ancient header (and the slabtop it leaves empty) can be
removed from the cache immediately in mark_ancient, instead of being
parked on a per-node dirty list and reclaimed only once the node becomes
unreferenced. This drops the dirty list and the clean_cache_* machinery
entirely.
Because the cache structure can now change under a node that still has
external references, the all-rdatasets iterator no longer walks the live
slabtop list: allrdatasets() binds every matching rdataset up front and
the iterator works from that snapshot, cloning out each entry.
Now that the slabheader carries its own memory context, free its
noqname/closest proofs from slabheader_destroy rather than reaching
through the owning node's deletedata method. That was the method's last
caller, so remove dns_db_deletedata entirely; the cache bookkeeping it
performed (rrset statistics and the dirty list) becomes a plain helper
called wherever a header leaves the cache.
The bound rdataset now keeps its own node reference instead of reaching
it through the slabheader, and each slabheader carries its own memory
context so it can free itself once its reference count reaches zero.
The noqname/closest proof rdatasets are views into a slabheader's proof
slabs, so they now hold a reference to that slabheader too, keeping the
proof slab (and the cloned owner name): alive for as long as the proof
rdataset is. Keeping the node on the rdataset means node access stays
valid for the life of the rdataset, independent of the slabheader's own
node pointer.
Instead of only reference counting the enclosing qpcnode, add the
reference counting directly to the slabheaders. This will allow us to
simplify the data model, as the slabheaders reference is now incremented
when we bind the rdataset and decremented when we disassociate the
rdataset. This will allow us to remove the slabheader from the slabtop
directly instead of waiting for qpcnode to become completely
unreferenced.
Follow the method split with a data split: give the proof
pseudo-rdataset its own member in the dns_rdataset union, carrying its
own node, instead of overloading the slab member. The proof methods
now read proof.raw/proof.node independently of the real
slab.raw/slab.noqname/slab.closest, so the two share no state and the
proof path is fully self-contained.
The noqname/closest proof pseudo-rdatasets share the slab data layout
but have a different lifecycle: they reference a bare proof slab owned
by the parent header and are torn down via dns_slabheader_freeproof(),
not the rdataset's own teardown. Give them a dedicated
dns_rdataslab_proof_rdatasetmethods table with their own
disassociate/clone so the proof path can diverge from the real
bound-rdataset path -- a prerequisite for reference counting the real
headers without it ever reaching the proof slabs.
With serve-stale enabled, stale rdataset headers are kept at a node so
they can be served as a last resort. The find loop, however, accepted a
stale CNAME or stale record of the requested type as a final answer and
broke out of the iteration early, returning stale data even when a fresh
header for the same name appeared later in the scan. Treat STALE(found)
like a missing answer so the loop keeps looking and only falls back to
the stale header when no fresh answer is found.
Callbacks for libuv stream reads do not signal zero-length reads as a
failure signal but rather as EAGAIN/EWOULDBLOCK. This can trigger an
assertion when a zero-length read is pushed onto a PROXYv2 endpoint that
has not yet processed the headers as it expects a non-NULL region of
positive length.
Closes#6140
Merge branch '6140-proxyv2-assertion-on-zero-byte-read' into 'main'
See merge request isc-projects/bind9!12261
Callbacks for libuv stream reads do not signal zero-length reads as a
failure signal but rather as EAGAIN/EWOULDBLOCK. This can trigger an
assertion when a zero-length read is pushed onto a PROXYv2 endpoint that
has not yet processed the headers as it expects a non-NULL region of
positive length.
The third-party comparison is right at the 0.4 % threshold, making it
fail quite often. The current range of observed values ranges from
0.3-0.5 %. Raise the threshold to 0.5 %.
The recent-named comparison produces values ranging from 0.05 to 0.12 %,
but appears to be more sensitive to time of the day when the test runs.
Raise the threshold to 0.15 %.
Both results are stable within the specified ranges across the last
three releases in both main and bind-9.20 series.
Merge branch 'nicki/respdiff-threshold-bump' into 'main'
See merge request isc-projects/bind9!12286
The third-party comparison is right at the 0.4 % threshold, making it
fail quite often. The current range of observed values ranges from
0.3-0.5 %. Raise the threshold to 0.5 %.
The recent-named comparison produces values ranging from 0.05 to 0.12 %,
but appears to be more sensitive to time of the day when the test runs.
Raise the threshold to 0.15 %.
Both results are stable within the specified ranges across the last
three releases in both main and bind-9.20 series.
When on an ``rndc reconfig`` the DNSSEC policy changes such that it changes the expected ``CDNSKEY`` and/or ``CDS`` records in the zone, the RRset should
be updated accordingly. This did not happen when removing digests from the configuration, or setting `cdnskey no;`. This has been fixed.
Closes#6166
Merge branch '6166-reconfig-delete-cds' into 'main'
See merge request isc-projects/bind9!12265
With dnssec-signzone smart-signing (-S), the CDS and CDSNKEY are
derived from the key timing metadata and the configuration options (-G).
The test has specific test cases that smart signing (with the fix)
interferes with. Therefor, disable smart-signing in the cds system test.
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;'.
When on an 'rndc reconfig' the DNSSEC policy changes such that it
changes the expected CDNSKEY/CDS records in the zone, the RRset should
be updated accordingly.
Add a test case where we reconfigure a zone with a policy such that
these records should be removed, and on a second reconfigure add
them back again.
Note the test deliberately adds a different CDS digest on the
second reconfigure.
Some of the tests in `dnssec/tests_validation.py` worked by iterating through the response message looking for failure conditions, such as excessively high TTL values. In some cases, previous changes caused additional data not to be returned. Since there was nothing to iterate, the tests still "passed".
Tests that don't make sense anymore have been removed. Other tests that iterate through responses have been updated with checks to ensure that the responses actually do contain data.
Merge branch 'each-cleanup-validation-test' into 'main'
See merge request isc-projects/bind9!12269
Some of the tests in in dnssec/tests_validation.py worked by iterating
through the response message looking for failure conditions, such as
excessively high TTL values. In some cases, previous changes caused
additional data not to be returned. Since there was nothing to
iterate, the tests still "passed".
Tests that don't make sense anymore have been removed. Other tests that
iterate through responses have been updated with checks to ensure that
the responses actually do contain data.
expand on the isctest.check.empty_answer() function, adding
empty_authority(), empty_additional(), has_answer(), has_authority(),
and has_additional().
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.
Closes#5971
Merge branch '5971-wildcard-noqname-mismatch' into 'main'
See merge request isc-projects/bind9!12256
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
Under `forward-first` policy in a forwarding zone BIND could accept NS above the forward zone apex from negative responses. This has been fixed.
ISC would like to thank Qifan Zhang, of Palo Alto Networks, for the report.
Closes#5937
Merge branch '5937-fix-forward-first-referral-bailiwick-v2' into 'main'
See merge request isc-projects/bind9!12154
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.
Add a system test covering authority-section NS referrals returned by
configured forwarders under forward first.
The test verifies that a forwarder for fwd.hack cannot install the
parent hack zone cut and redirect resolution for the sibling zone
sibling.hack.
Since internal communications are now Zulip based, CI jobs now target
Zulip instead of Mattermost. The `MATTERMOST_WEBHOOK_URL` environment
variable is no longer needed, scripts now use `ZULIP_SERVER_URL` and
`ZULIP_API_KEY`.
In order to harmonize Zulip messaging, `message_zulip.py` is used where
curl calls to the webhook were previously used.
Merge branch 'andoni/mattermost-to-zulip-migration' into 'main'
See merge request isc-projects/bind9!12199
Since internal communications are now Zulip based, CI jobs now target
Zulip instead of Mattermost. The MATTERMOST_WEBHOOK_URL environment
variable is no longer needed, scripts now use ZULIP_SERVER_URL and
ZULIP_API_KEY.
In order to harmonize Zulip messaging, message_zulip.py is used where
curl calls to the webhook were previously used.
When forwarding DNS queries, the CD bit is cleared on the first query, and the CD bit is only used as a fallback if the first query fails. However, due to a logic bug this could lead to an unbounded loop re-sending the same message, until the maximum query count is hit. This has been fixed.
Closes#5804
Merge branch '5804-resend-loop-forwarder-cd' into 'main'
See merge request isc-projects/bind9!12133
Commit `36cf1c6a5bf943ad718ddba9fbe6ea97810e3bc2` introduces the
`DNS_FETCHOPT_TRYCD` flag which enables, when sending a query to a
forwarder, the forwarder to validate the answer (CD=0). The crux is
that if for some reason the forwarder returns SERVFAIL, we can retry the
same query and disable the forwarder validation (CD=1) so the resolver
can attempt validation itself (or detect it's bogus).
The logic was to first set `DNS_FETCHOPT_TRYCD` to the query options but
not on the message (so CD=0), and, when getting a SERVFAIL answer, if
the option `DNS_FETCHOPT_TRYCD` was set, to also set it into the
message. However, there was no way to know if this was the first (or
second) query because the original message is discarded when getting the
answer. This can lead to an unbounded loop re-sending the same message
again and again (until the global query count stops it).
This is fixed by using two separate flags `DNS_FETCHOPT_TRYNOCD`, set on
the query options for the very first query, then, if it SERVFAIL,
check if `DNS_FETCHOPT_TRYNOCD` is set but `DNS_FETCHOPT_TRYCD` is not.
In this case, we know we're about to send the second query. If it also
fails, `DNS_FETCHOPT_TRYCD` will be set anyway, so there is no point
retrying. This breaks the unbounded loop.
Run malicious server: resend_loop/ans2/ans.py
Start BIND: ns1
Send single query to test.com
OBSERVED BEHAVIOR
The malicious server receives tens of thousands of resend packets
within seconds. CPU usage of the named worker thread remains elevated
(50–100% of one core) until the default fetch timeout (~10 seconds)
terminates the request. Instrumentation during testing confirmed that
isc_counter_used(fctx->qc) remains constant (value 1) throughout the
entire resend loop.
When caching delegation NS data, only use A/AAAA glue records if the resolver has the corresponding IPv4/IPv6 dispatcher configured. If IPv4 or IPv6 is disabled, ignore glue for that family and fall back to caching the nameserver name if there is no glue from the other supported family.
Merge branch 'colin/glues-supported-stack' into 'main'
See merge request isc-projects/bind9!11889
When caching delegation NS data, only use A/AAAA glue records if the
resolver has the corresponding IPv4/IPv6 dispatcher configured. If IPv4
or IPv6 is disabled, ignore glue for that family and fall back to
caching the nameserver name if there is no glue from the other supported
family.
The new `cache_delegns` system test is covering delegation NS caching
with dual-stack resolver, IPv4-only, IPv6-only configurations. It also
set up an authoritative sever with zones with A-only, AAAA-only, and
dual-stack glue, which are all queried, and checks the delegation
database dump to confirm that the cached delegation data correspond to
the resolver configuration.
The `geoip metro` and `geoip metrocode` configuration options has been
removed as metro code are deprecated from MaxMind library.
Merge branch 'colin/remove-geoip-metro' into 'main'
See merge request isc-projects/bind9!12217