The INSIST in isc_radix_insert() checks node->data[RADIX_V4] and
node->node_num[RADIX_V4] twice due to a copy-paste error, never
verifying the RADIX_V6 fields.
Fix the second pair to check RADIX_V6.
Backport of MR !11664
Merge branch 'backport-ondrej/fix-copy-paste-error-checking-RADIX_V4-instead-of-RADIX_V6-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11686
The INSIST in isc_radix_insert() checks node->data[RADIX_V4] and
node->node_num[RADIX_V4] twice due to a copy-paste error, never
verifying the RADIX_V6 fields.
Fix the second pair to check RADIX_V6.
(cherry picked from commit 3f15f2d9e5)
After KeyTrap, the temporal DNSSEC were originally hard errors that
caused validation failures even if the records had another valid
signature. This has been changed and the RRSIGs outside of the
inception and expiration time are not counted as hard errors. However,
these errors are not even counted as validation attempts, so excessive
number of expired RRSIGs would cause some non-cryptograhic extra work
for the validator. This has been fixed and the temporal errors are
correctly counted as validation attempts.
Closes#5760
Backport of MR !11589
Merge branch 'backport-5760-count-DNSSEC-temporal-errors-as-validation-attempts-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11763
After KeyTrap, the temporal DNSSEC were originally hard errors that
caused validation failures even if the records had another valid
signature. This has been changed and the RRSIGs outside of the
inception and expiration time are not counted as hard errors. However,
these errors are not even counted as validation attempts, so excessive
number of expired RRSIGs would cause some non-cryptograhic extra work
for the validator. This has been fixed and the temporal errors are
correctly counted as validation attempts.
(cherry picked from commit 6ba57a1f0f)
An authenticated DDNS client could bypass update-policy per-type record limits
(e.g. TXT(3)) by including padding records in the UPDATE message that are
silently skipped during processing in the main branch.
As BIND 9.20 is not affected, only backport the test.
Closes#5799
Backport of MR !11708
Merge branch 'backport-5799-fix-counter-desync-in-SSU-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11760
The prescan and main update loops in DNS UPDATE processing both used the
same counter to index the maxbytype[] quota array. The prescan loop
always incremented the counter, but the main loop had 14 continue paths
that skipped the increment. This allowed an authenticated DDNS client to
craft an UPDATE message with padding records (e.g. CNAME+A pairs that
trigger CNAME-conflict skips) to shift the counter and read wrong quota
entries, bypassing per-type record limits entirely.
Fix by incrementing the counter unconditionally at the start of each
iteration in the main loop.
(cherry picked from commit bac40394d5)
The :iscman:`named` process could terminate unexpectedly when
processing a catalog zone ACL in an APL resource record that
was completely empty. This has been fixed.
Closes#5801
Backport of MR !11740
Merge branch 'backport-5801-catz-empty-apl-rr-bug-fix-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11759
Allow empty APL records because RFC 3123 (Section 4) says "zero or
more items". This fixes processing of a catalog zone ACL (which is
based on APL records) when the zone contains an empty APL record or
when a zone update arrives which creates an empty APL record.
(cherry picked from commit 35b8af229e)
The "publish" job has no dependencies on other jobs, so nothing prevents
it from being accidentally started before the scheduled publication
date. Although publication still requires confirmation via an SSH
connection to a dedicated, locked-down runner, performing that action
prematurely may have drastic consequences. Therefore, it is worth
implementing additional safeguards.
Add an extra check to the "publish" job to ensure it can only be run on
the scheduled publication day. In exceptional circumstances, this check
can be overridden by setting the FORCE_PUBLICATION CI variable to any
non-empty value.
(cherry picked from commit ce977f53b9)
The "merge-tag" and "update-stable-tag" jobs currently use the
"manual_release_job_qa" YAML anchor, which makes them depend on the
"staging" job. Meanwhile, both of these jobs require the tag they were
created for to be public for them to work. While this is harmless, as
these jobs will simply fail if they are run too early, it still makes
sense for them to depend on the "publish" job instead, if only to reduce
confusion in the pipeline view. Adjust the "needs" key for the
"merge-tag" and "update-stable-tag" jobs accordingly.
(cherry picked from commit 722290dce6)
The commit.txt file produced by each Cloudsmith build job is required to
run the corresponding publication job. Therefore, the artifact lifetime
for the former must be long enough to prevent the file from expiring
before the publication job is run. Set the lifetime of the artifacts
created by Cloudsmith build jobs to one month to ensure that the
publication jobs can access them.
(cherry picked from commit ce09f8d0f8)
Setting "artifacts: false" for the dependency on the "publish-private"
job prevents the url-*.txt files produced by that job from being pulled
from GitLab when the jobs that build EVN & -S Cloudsmith packages are
run, effectively breaking the latter. Fix by making these jobs depend
on the artifacts of the "publish-private" job.
(cherry picked from commit b36f17238b)
The "nsec3-delegation" test was added in a release branch, before commit
e40db975d9 introduced the current system
test naming convention. Rename the test to comply with that convention.
Backport of MR !11753
Merge branch 'backport-michal/rename-nsec3-delegation-test-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11754
The "nsec3-delegation" test was added in a release branch, before commit
e40db975d9 introduced the current system
test naming convention. Rename the test to comply with that convention.
(cherry picked from commit 48bf3d3e65)
Fixed a crash that could occur when running rndc reconfig to change a zone's update policy (e.g., from allow-update to update-policy) while DNS UPDATE requests were being processed for that zone.
ISC would like to thank Vitaly Simonovich for bringing this issue to our attention.
Fixes#5817
Backport of MR !11707
Merge branch 'backport-5817-fix-crash-via-SSU-table-desynchronization-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11738
Race rndc reconfig (toggling between allow-update and update-policy)
against a stream of DNS UPDATEs for 5 seconds and verify that named
does not crash.
Before the fix, the race between send_update() and update_action()
reading the SSU table independently could trigger an assertion
failure (INSIST) when the zone's update policy changed between the
two reads.
(cherry picked from commit c503b6eee8)
Pass the SSU table through the update event struct from
send_update() to update_action() instead of reading it from the
zone twice. If rndc reconfig changed the zone's update policy
between the two reads (e.g., from allow-update to update-policy),
send_update() would skip the maxbytype allocation but
update_action() would see a non-NULL ssutable, triggering
INSIST(ssutable == NULL || maxbytype != NULL) and crashing named.
The ssutable reference is now taken once in send_update() and
transferred to update_action() via the event struct, ensuring
both functions see the same value.
(cherry picked from commit c172416559)
Calling `rndc modzone` didn't work properly for a zone hat was configured in
the configuration file. It could crash if BIND 9 was built without LMDB or if
there was already an NZF file for the zone. In addition, `rndc modzone` failed
in subsequent attempts. These problems are now fixed.
Closes#5826
Merge branch '5826-fix-modzone-issues-ytatuya' into 'bind-9.20'
See merge request isc-projects/bind9!11743
If a zone is in named.conf, not originally added by rndc addzone,
rndc modzone for that zone succeeds once, but subsequent modzone
attempts fail. This is because do_modzone removes the zone config
from global or view options, but it would fail due to 'not found'
once the config is removed.
The fix is to ensure re-adding the updated zone config to the
global or view options. This also works as a more complete fix
for the issue 85453d3 atempted to solve, ensuring rndc showzone
shows the latest config: it now works for multple attemps of
modzone, and with named that is not built with LMDB.
The change in this commit relies on UNCONST in a few places.
That's not clean, but 'add/mod/delzone' generally seems to
need it (for example, delete_zoneconf uses it to modify the list
of zones). In that sense, this change follows the convention
(for a longer term, there may have to be a better API so that we
can modify config obtions that were once parsed).
This reverts commit 85453d393d.
This commit doesn't seem to be a complete solution of what
it appears to fix: showzone succeeds and shows the modified
config after first modzone, but subsequent attempts of modzone
fail (though not because of the commit being reverted), let
alone showing the correct new config.
Revering the change for now, and will provide a more comprehensive
fix in the next commit.
If named is built without LMDB and has a zone in named.conf,
then rndc modzone for that zone triggers an assertion failure
unless there's already an NZF file. This is because load_nzf
doesn't create 'nzf_config' when NZF is missing, while a valid
nzf_config is assumed in do_modzone when it tries to add the
modified zone config to add_parser.
The crash is fixed by skipping the call to cfg_parser_mapadd when
nzf_config is NULL. Skipping it should be okay since the config stored
in add_parser would be needed only for subsequently deleting a zone by
rndc delzone when the zone was originally added by rndc addzone, but
in this case the zone was not 'added'. Checking if nzf_config is NULL
before using it also seems to be consistent with other parts of the
implementation.
A helper macro that returns the current value of a pointer and sets
it to NULL in one expression, useful for transferring ownership in
designated initializers.
Backport of MR !11724
Merge branch 'backport-ondrej/TAKE_OWNERSHIP-macro-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11736
A helper macro that returns the current value of a pointer and sets
it to NULL in one expression, useful for transferring ownership in
designated initializers.
(cherry picked from commit 0f3be0beb8)
The usage still said the default NSEC3 iterations is 10, but this
has been 0 for a while.
Backport of MR !11727
Merge branch 'backport-matthijs-dnssec-signzone-help-nsec3iter-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11734
dns_view_flushnode() was called in the delete_expired() async
callback, which runs after the query that detected the NTA expiry.
This created a race: the query would proceed with stale cached data
from the NTA period before the flush had a chance to run, resulting
in transient SERVFAIL with EDE 22 (No Reachable Authority).
Skip dns_view_flushnode() in the older branches as the solutions for
older branches are too complicated and this was not a critical bug.
Also simplify the expiry comparison in delete_expired() to a direct
pointer comparison (nta == pval) instead of comparing expiry
timestamps.
Backport of MR !11729
Merge branch 'backport-ondrej/refactor-nta-using-RCU-delete-order-fix-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11730
When an NTA already exists for a name, the old code retrieved
and reused the existing NTA object, then reset its timer via
settimer(). This is incorrect because isc_timer_start() and
isc_timer_stop() require the timer to be manipulated from its
owning loop (enforced by REQUIRE(timer->loop == isc_loop()) in
lib/isc/timer.c), and the caller may be running on a different
loop than the one that created the original NTA.
Instead, delete the old NTA (shutting down its timer on the
correct loop) and insert a fresh one that is owned by the
current loop.
dns_view_flushnode() was called in the delete_expired() async
callback, which runs after the query that detected the NTA expiry.
This created a race: the query would proceed with stale cached data
from the NTA period before the flush had a chance to run, resulting
in transient SERVFAIL with EDE 22 (No Reachable Authority).
Skip dns_view_flushnode() in the older branches as the solutions for
older branches are too complicated and this was not a critical bug.
(cherry picked from commit da8e1c956a)
When a configured NTA for a name expired, any possibly cached
data for the name (with "insecure" DNSSEC validation result)
was not flushed from the resolver's cache. This has been fixed.
Closes#5747
Backport of MR !11597
Merge branch 'backport-5747-nta-expiry-cache-flush-bug-fix-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11715
When NTA expires the name's node should be flushed from the view's
cache as it's done when the NTA is manually removed using a rndc
command.
(cherry picked from commit 1899a3318c)
Move the write to fctx->vresult after LOCK(&fctx->lock). The field was
being set before acquiring the lock, but dns_resolver_logfetch() reads
it under the same lock from another thread.
Backport of MR !11717
Merge branch 'backport-ondrej/fix-data-race-on-fctx-result-in-validated-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11721
Move the write to fctx->vresult after LOCK(&fctx->lock). The field was
being set before acquiring the lock, but dns_resolver_logfetch() reads
it under the same lock from another thread.
(cherry picked from commit a2bd833909)
The SRTT (Smoothed Round-Trip Time) update for remote servers was not
atomic — concurrent callers could each read the same value and one
update would be silently lost. Additionally, the aging decay applied
once per second could run multiple times if several threads entered the
function simultaneously.
Use compare-and-swap loops for the SRTT update and for the aging
timestamp to ensure no updates are lost.
Backport of MR !11718
Merge branch 'backport-ondrej/fix-non-atomic-srtt-aging-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11723
The SRTT update loaded the old value, computed a new one, and stored it
back as separate operations. Two concurrent callers could each read the
same old value and one update would be silently lost.
Use a CAS loop for the read-modify-write on entry->srtt. For the aging
path, also CAS on entry->lastage to prevent multiple threads from aging
the same entry in the same second.
(cherry picked from commit 4d15494b94)
A 'dns_dtenv_t' pointer is passed to an async function without taking
a reference first, which can potentially cause a use-after-free error.
Take a reference, then detach in the async function.
Closes#5820
Backport of MR !11705
Merge branch 'backport-5820-dns_dtenv-reference-bug-fix-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!11714
The 'env' pointer is passed to an async function without taking
a reference first, which can potentially cause a use-after-free
error. Take a reference, then detach in the async function.
(cherry picked from commit 48d7401f0d)
