Remove unneeded qp*_ref/unref and qpcache_ref/unref calls by skipping
the extra ref/unref on the database in qp*node_release(). This makes
the extra ref/unref in detachnode() unnecessary as the reference on the
database will always only go down when releasing the database nodes.
ISC_LOG_WARNING was used while ISC_LOG_DEBUG(3) was implied.
Closes#5145
Merge branch '5145-artem-fix-wrong-logging-severity-in-do_nsfetch' into 'main'
See merge request isc-projects/bind9!10017
The text that stale-cache-enable is set to no has no effect on
max-cache-ttl, but on max-stale-ttl.
Closes#5181
Merge branch '5181-max-stale-ttl-typo-arm' into 'main'
See merge request isc-projects/bind9!10108
Remove code in the QP zone database to handle failures of `dns_qp_insert()` which can't actually happen.
Closes#5171
Merge branch '5171-qpzone-insert-checks' into 'main'
See merge request isc-projects/bind9!10088
in some places there were checks for failures of dns_qp_insert()
after dns_qp_getname(). such failures could only happen if another
thread inserted a node between the two calls, and that can't happen
because the calls are serialized with dns_qpmulti_write(). we can
simplify the code and just add an INSIST.
When converting SVCB records to text representation `named` now uses named
`SvcParamKeys` values unless backward-compatible mode is activated,
in which case the values which were not defined initially in
RFC9460 and were added later (see [1]) are converted to opaque
"keyNNNN" syntax, like, for example, "key7" instead of "dohpath".
Also a new `+[no]svcparamkeycompat` option is implemented for `dig`,
which enables the backward-compatible mode and uses the opaque
syntax, if required for interoperability with other software or
scripts. By default, the compatibility mode is disabled.
[1] https://www.iana.org/assignments/dns-svcb/dns-svcb.xhtmlCloses#5156
Merge branch '5156-svcparamkey-use-named-keys-by-default' into 'main'
See merge request isc-projects/bind9!10085
This check in the nsupdate system test expects the opaque
representation of the "dohpath" Service Parameter Key. Use
the +svcparamkeycompat dig option to enable it.
The new +[no]svcparamkeycompat option for dig enables the
backward-compatible mode for the Service Parameter Keys'
(SvcParamKeys) representation format. See the previous commit
for more information.
When converting SVCB records to text representation use named
SvcParamKeys values unless backward-compatible mode is activated,
in which case the values which were not defined initially in
RFC9460 and were added later (see [1]) are converted to opaque
"keyN" syntax, like, for example, "key7" instead of "dohpath".
[1] https://www.iana.org/assignments/dns-svcb/dns-svcb.xhtml
Co-authored-by: sdomi <ja@sdomi.pl>
This commit does several changes to isc_symtab:
1. Rewrite the isc_symtab to internally use isc_hashmap instead of
hand-stiched hashtable.
2. Create a new isc_symtab_define_and_return() api, which returns
the already defined symvalue on ISC_R_EXISTS; this allows users
of the API to skip the isc_symtab_lookup()+isc_symtab_define()
calls and directly call isc_symtab_define_and_return().
3. Merge isccc_symtab into isc_symtab - the only missing function
was isccc_symtab_foreach() that was merged into isc_symtab API.
4. Add full set of unit tests for the isc_symtab API.
Closes#5103
Merge branch '5103-use-isc_hashmap-for-isc_symtab' into 'main'
See merge request isc-projects/bind9!9921
This commit does several changes to isc_symtab:
1. Rewrite the isc_symtab to internally use isc_hashmap instead of
hand-stiched hashtable.
2. Create a new isc_symtab_define_and_return() api, which returns
the already defined symvalue on ISC_R_EXISTS; this allows users
of the API to skip the isc_symtab_lookup()+isc_symtab_define()
calls and directly call isc_symtab_define_and_return().
3. Merge isccc_symtab into isc_symtab - the only missing function
was isccc_symtab_foreach() that was merged into isc_symtab API.
4. Add full set of unit tests for the isc_symtab API.
When processing a query with the "checking disabled" bit set (CD=1), `named` stores the unvalidated result in the cache, marked "pending". When the same query is sent with CD=0, the cached data is validated, and either accepted as an answer, or ejected from the cache as invalid. This deferred validation was not attempted for DS and DNSKEY records if they had no cached signatures, causing spurious validation failures. We now complete the deferred validation in this scenario.
Also, if deferred validation fails, we now re-query the data to find out whether the zone has been corrected since the invalid data was cached.
Closes#5066
Merge branch '5066-fix-strip-dnssec-rrsigs' into 'main'
See merge request isc-projects/bind9!10104
If a deferred validation on data that was originally queried with
CD=1 fails, we now repeat the query, since the zone data may have
changed in the meantime.
When a query is made with CD=1, we store the result in the
cache marked pending so that it can be validated later, at
which time it will either be accepted as an answer or removed
from the cache as invalid. Deferred validation was not
attempted when there were no cached RRSIGs for DNSKEY and
DS. We now complete the deferred validation in this scenario.
An incorrect optimization caused "CNAME and other data" errors not to be detected if certain types were at the same node as a CNAME. This has been fixed.
Closes#5150
Merge branch '5150-cname-and-other-data-check-not-applied-to-all-types' into 'main'
See merge request isc-projects/bind9!10033
prio_type was being used in the wrong place to optimize cname_and_other.
We have to first exclude and accepted types and we also have to
determine that the record exists before we can check if we are at
a point where a later CNAME cannot appear.
This is a complement to the already present system test "stress" test.
Merge branch 'mnowak/generate-tsan-unit-stress-tests' into 'main'
See merge request isc-projects/bind9!9474
GitLab CI Runner's $builds_dir variable is set to "/builds" by default.
For technical reasons, the FreeBSD Runners, using the "instance"
executor, sets the path differently.
The value of $CI_PROJECT_DIR is based on $builds_dir, so if the
generate-stress-test-configs.py script generates jobs with
$CI_PROJECT_DIR (or variables like $INSTALL_PATH that are based on it)
evaluated, it is calcified to whatever was the value in the particular
environment, disregarding the FreeBSD "instance" executor specifics in
the child pipeline.
Instead of evaluating $CI_PROJECT_DIR in the script, evaluate it in the
runtime environment.
Merge branch 'mnowak/fix-CI_PROJECT_DIR-variable-evaluation' into 'main'
See merge request isc-projects/bind9!10075
GitLab CI Runner's $builds_dir variable is set to "/builds" by default.
For technical reasons, the FreeBSD Runners, using the "instance"
executor, sets the path differently.
The value of $CI_PROJECT_DIR is based on $builds_dir, so if the
generate-stress-test-configs.py script generates jobs with
$CI_PROJECT_DIR (or variables like $INSTALL_PATH that are based on it)
evaluated, it is calcified to whatever was the value in the particular
environment, disregarding the FreeBSD "instance" executor specifics in
the child pipeline.
Instead of evaluating $CI_PROJECT_DIR in the script, evaluate it in the
runtime environment.
Instead of having many node_lock_count * sizeof(<member>) arrays, pack all
the members into a qpcache_bucket_t that is cacheline aligned to prevent
false sharing between RWLocks.
Merge branch 'ondrej/prevent-nodelock-false-sharing' into 'main'
See merge request isc-projects/bind9!10072
Instead of using on hash of the name modulo number of the buckets,
assign the locknum randomly with isc_random_uniform(). This makes
the locknum assignment aligned with qpcache and allows the bucket
number to be non-prime in the future.
Reduce the number of qpzone_ref() and qpzone_unref() calls in
qpzone_detachnode() by relying on the call_rcu to delay
the destruction of the lock buckets.
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.
Instead of having many node_lock_count * sizeof(<member>) arrays, pack
all the members into a qpcache_bucket_t struct that is cacheline aligned
and have a single array of those.
Additionaly, make both the head and the tail of isc_queue_t padded, not
just the head, to prevent false sharing of the lock-free structure with
the lock that follows it.
In some cases, the numeric identifier doesn't correspond to the
directory name (i.e. `resolver` server in `shutdown` test, which is
supposed to be 10.53.0.3). These are typically servers that shouldn't be
auto-started by the runner, thus avoiding the typical `*ns<X>` name.
Support these server by allowing a fallback initialization with custom
numeric identifier in case it can't be parsed from the directory name.
The start()/stop() functions can be used in the pytests in the same way
as start_server and stop_server functions were used in shell tests. Note
that the servers obtained through the servers fixture are still started
and stopped by the test runner at the start and end of the test. This
makes these functions mostly useful for restarting the server(s)
mid-test.
Previously, these functions have been provided as fixtures. This was
limiting re-use, because it wasn't possible to call these outside of
tests / other fixtures without passing these utility functions around.
Move them into isctest.run package instead.
When query logging is enabled, named will now include the destination
address port in the logged message.
Closes#5060
Merge branch '5060-log-query-incoming-port' into 'main'
See merge request isc-projects/bind9!9972
When query logging is enabled, named will now include the destination
address port in the logged message.
Example messages for before and after this change:
before: client @0x7608b2026000 10.53.0.1#52136 (example.test): query: example.test IN A +E(0)K (10.53.0.1)
after: client @0x729bf5c26000 10.53.0.1#35976 (example.test): query: example.test IN A +E(0)K (10.53.0.1#53)
The ANS servers were not to written to handle NS queries at the QNAME, resulting in gratuitous protocol errors that will break tests when NS requests are made for the QNAME: i.e., NXDOMAIN for NS vs data for expected type, CNAME not being returned for all query types.
Prerequisite for !9155Closes#5062
Merge branch '5062-fix-ans-servers-ns-at-qname' into 'main'
See merge request isc-projects/bind9!9786
The ANS servers were not to written to handle NS queries at the
QNAME resulting in gratuitious protocol errors that will break tests
when NS requests are made for the QNAME.
Print the expiration time of the stale RRsets in the cache dump.
Merge branch 'ondrej/print-expiration-time-of-stale-records' into 'main'
See merge request isc-projects/bind9!10057
In #1870, the expiration time of ANCIENT records were printed, but
actually the ancient records are very short lived, and the information
carries a little value.
Instead of printing the expiration of ANCIENT records, print the
expiration time of STALE records.
Under rare circumstances, the RRSet that expired at the time of
the query could be returned with TTL far in the future. This
has been fixed.
As a side-effect, the expiration time of expired RRSets are no
longer printed out in the cache dump.
Closes#5094
Merge branch '5094-fix-timestamp-in-ttl' into 'main'
See merge request isc-projects/bind9!10048
The original .ttl field was actually used as TTL in the dns_qpzone unit.
Restore the field by adding it to union with the .expire struct member
and cleanup all the code that added or subtracted 'now' from the ttl
field as that was misleading as 'now' would be always 0 for qpzone
database.
The find_coveringnsec() was getting the 'now' from two sources -
search->now and separate now argument. Things like this are ticking
bombs, remove the extra 'now' argument and use single source of 'now'.
When the mark_ancient() helper function was introduced, couple of places
with duplicate (or almost duplicate) code was missed. Move the
mark_ancient() function closer to the top of the file, and correctly use
it in places that mark the header as ANCIENT.
If we know that the header has ZEROTTL set, the server should never send
stale records for it and the TTL should never be anything else than 0.
The comment was already there, but the code was not matching the
comment.
The old name was misleading as it never meant time-to-live, e.g. number
of seconds from now when the header should expire. The true meaning was
an expiration time e.g. now + ttl. This was the original design bug
that caused the slip when we assigned header->ttl to rdataset->ttl.
Because the name was matching, nobody has questioned the correctness of
the code both during the MR review and during the numerous re-reviews
when we were searching for the cause of the 54 year TTL.
When the header has been marked as ANCIENT, but the ttl hasn't been
reset (this happens in couple of places), the rdataset TTL would be
set to the header timestamp instead to a reasonable TTL value.
Since this header has been already expired (ANCIENT is set), set the
rdataset TTL to 0 and don't reuse this field to print the expiration
time when dumping the cache. Instead of printing the time, we now
just print 'expired (awaiting cleanup'.
ADB responses were not being validated, allowing spoofed responses to be accepted and used for further lookups. This should not be possible when the servers for the zone are in a signed zone, except with CD=1 requests or when glue is needed. This has been fixed.
Closes#5066
Merge branch '5066-validate-adb-fetches' into 'main'
See merge request isc-projects/bind9!10052
Named was failing to recover when spoofed nameserver address from
a signed zone for a peer zone were returned to a previous CD=1
query. Validate non-glue interior server addresses before using them.
When there are parent and child zones on the same server, the DNSKEY
lookup was failing as the pending record we are validating is needed
to fetch the DNSKEY records. This change allows that to happen.
The caller is already setting STARTATZONE when the name being looked
up is a subdomain of the current domain.
The address lookups from ADB were not being validated, allowing
spoofed responses to be accepted and used for other lookups.
Validate the answers except when CD=1 is set in the triggering
request. Separate ADB names looked up with CD=1 from those without
CD=1, to prevent the use of unvalidated answers in the normal lookup
case (CD=0). Set the TTL on unvalidated (pending) responses to
ADB_CACHE_MINIMUM when adding them to the ADB.
The search for the deepest known zone cut in the cache could improperly reject a node if it contained any stale data, regardless of whether it was the NS RRset that was stale.
Closes#5155
Merge branch '5155-fix-findzonecut' into 'main'
See merge request isc-projects/bind9!10047
the search for the deepest known zone cut in the cache could
improperly reject a node containing stale data, even if the
NS rdataset wasn't the data that was stale.
this change also improves the efficiency of the search by
stopping it when both NS and RRSIG(NS) have been found.
We need to avoid double-triggering of post-merge jobs in the following
scenario:
1. A private MR gets merged into the private BIND 9 repository.
2. This merge operation triggers a "push" pipeline in the private
repository, which correctly runs post-merge jobs, e.g. to set MR
metadata in the private project.
3. When a release is published, a script is run to change the
automatically assigned milestone value ("Not released yet") to
something else.
4. Shortly afterwards, the result of the merge from step 1 is merged
back into a maintenance branch in the public repository.
5. The push operation triggers another "push" pipeline, this time in
the public project.
At this point there are two problems:
- If the script is dumb (like it currently is), it will extract the
merge request ID from the merge commit description and change the
milestone for a merge request in the wrong project namespace.
- Even if the script was fixed to extract and use the correct GitLab
project reference, it would reset the milestone for the merge
request in the private repository back to "Not released yet" - while
the milestone set in step 3 should be retained.
An alternative would be to change the order of operations so that
post-release milestoning happens at a later stage, while also fixing the
script to correctly follow cross-project references, but that approach
seems more fragile than simply failing on all cross-project pushes. The
rule to enforce is: each project should only take care of its own
post-merge tasks.
This MR adds an extra CVE checklist step suggested by @pspacek after
CVE-2024-0760. See [Mattermost][1] for context.
It also syncs checklist steps with their corresponding links to the
explanations wiki page as these have been out-of-sync since !9307.
[1]: https://mattermost.isc.org/isc/pl/156npi4bwtr6pechbj6sfs6jjo
Merge branch 'michal/update-cve-checklist' into 'main'
See merge request isc-projects/bind9!9949
With shallow fetching working reliably in pygit2 1.17.0+, there is no
longer any need for GitLab CI runners to clone the BIND 9 repository
with a fixed depth of 1000 during every "danger" CI job as Hazard is now
able to fetch remote refs with an arbitrary depth, controlled by the
HAZARD_FETCH_DEPTH environment variable. The latter can be defined via
GitLab project's CI settings and adjusted as needed over time, without
the need to update .gitlab-ci.yml every time its value needs to be
changed.
Merge branch 'michal/use-default-cloning-depth-for-the-danger-ci-job' into 'main'
See merge request isc-projects/bind9!9946
With shallow fetching working reliably in pygit2 1.17.0+, there is no
longer any need for GitLab CI runners to clone the BIND 9 repository
with a fixed depth of 1000 during every "danger" CI job as Hazard is now
able to fetch remote refs with an arbitrary depth, controlled by the
HAZARD_FETCH_DEPTH environment variable. The latter can be defined via
GitLab project's CI settings and adjusted as needed over time, without
the need to update .gitlab-ci.yml every time its value needs to be
changed.
Clean up the pattern in the newref() and decref() functions in QP databases. Replace the `db_nodelock_t` structure with plain reference counting for every active database node in QPDB.
Related to #5134
Merge branch '5134-refactor-decref-functions-in-qpdb' into 'main'
See merge request isc-projects/bind9!10006
Change the names of the node reference counting functions
and add comments to make the mechanism easier to understand:
- newref() and decref() are now called qpcnode_acquire()/
qpznode_acquire() and qpcnode_release()/qpznode_release()
respectively; this reflects the fact that they modify both
the internal and external reference counters for a node.
- qpcnode_newref() and qpznode_newref() are now called
qpcnode_erefs_increment() and qpznode_erefs_increment(), and
qpcnode_decref() and qpznode_decref() are now called
qpcnode_erefs_decrement() and qpznode_erefs_decrement(),
to reflect that they only increase and decrease the node's
external reference counters, not internal.
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.
The origin_node in qpcache was always NULL, so we can remove the
getoriginode() function and origin_node pointer as the
dns_db_getoriginnode() correctly returns ISC_R_NOTFOUND when the
function is not implemented.
Cleanup the pattern in the decref() functions in both qpcache.c and
qpzone.c, so it follows the similar patter as we already have in
newref() function.
Changes introducing the support of extended DNS error code 1 and 2 uses
SHA-1 digest for some tests which break FIPS platform. The digest itself
was irrelevant, another digest is used.
Merge branch 'colin/fix-fips-9807' into 'main'
See merge request isc-projects/bind9!10002
Changes !9948 introducing the support of extended DNS error code 1 and 2
uses SHA-1 digest for some tests which break FIPS platform. The digest
itself was irrelevant, another digest is used.
Instead of mixing the dns_resolver and dns_validator units directly with
the EDE code, split-out the dns_ede functionality into own separate
compilation unit and hide the implementation details behind abstraction.
Additionally, the new dns_edelist_t doesn't have to be copied into all
responses as those are attached to the fetch context, but it could be
only passed by reference.
This makes the dns_ede implementation simpler to use, although sligtly
more complicated on the inside.
Closes#5141
Merge branch '5141-ede-mem-fix-v2' into 'main'
See merge request isc-projects/bind9!10016
In order to avoid to loop to find the next position to store an EDE in
a dns_edectx_t, add a "nextede" state which holds the next available
position.
Also, in order ot avoid to loop to find if an EDE is already existing in
a dns_edectx_t, and avoid a duplicate, use a bitmap to immediately know
if the EDE is there or not.
Those both changes applies for adding or copying EDE.
Also make the direction of dns_ede_copy more explicit/avoid errors by
making "edectx_from" a const pointer.
Migrate tests cases in client_test code which were exclusively testing
code which is now all wrapped inside ede compilation unit. Those are
testing maximum number of EDE, duplicate EDE as well as truncation of
text of an EDE.
Also add coverage for the copy of EDE from an edectx to another one, as
well as checking the assertion of the maximum EDE info code which can be
used.
Instead of mixing the dns_resolver and dns_validator units directly with
the EDE code, split-out the dns_ede functionality into own separate
compilation unit and hide the implementation details behind abstraction.
Additionally, the EDE codes are directly copied into the ns_client
buffers by passing the EDE context to dns_resolver_createfetch().
This makes the dns_ede implementation simpler to use, although sligtly
more complicated on the inside.
Co-authored-by: Colin Vidal <colin@isc.org>
Co-authored-by: Ondřej Surý <ondrej@isc.org>
Reduce the amount of artifacts stored by running make clean at the end
of unit and system test run. If any of the previous commands fail, the
runner will stop executing the commands in `script` immediately, so the
cleanup only happens if none of the previous commands failed.
The build artifacts from unit and system tests are re-used anywhere and
should be safe to throw away immediately.
Merge branch 'nicki/reduce-ci-artifacts' into 'main'
See merge request isc-projects/bind9!10015
Reduce the amount of artifacts stored by running make clean at the end
of unit and system test run. If any of the previous commands fail, the
runner will stop executing the commands in `script` immediately, so the
cleanup only happens if none of the previous commands failed.
The build artifacts from unit and system tests are re-used anywhere and
should be safe to throw away immediately. Same for respdiff.
The prior regex didn't match the actual names we use for release
branches in the private repo. This caused the merged-metadata job to not
be created upon merging to a release branch, resulting in the private MR
not being properly milestoned.
Use the correct regex along with protecting the v9.*-release branches in
the gitlab UI so that they have access to the token used to perform the
required API operations.
Merge branch 'nicki/ci-fix-post-merge-in-private-repo' into 'main'
See merge request isc-projects/bind9!10003
The prior regex didn't match the actual names we use for release
branches in the private repo. This caused the merged-metadata job to not
be created upon merging to a release branch, resulting in the private MR
not being properly milestoned.
Use the correct regex along with protecting the v9.*-release branches in
the gitlab UI so that they have access to the token used to perform the
required API operations.
The iana.org link is gone.
Merge branch 'mnowak/linkcheck-fix-draft-icann-dnssec-keymgmt-01.txt-link' into 'main'
See merge request isc-projects/bind9!10010
Validation pipeline: https://gitlab.isc.org/isc-projects/bind9/-/pipelines/160984
Prerequisites:
- [x] isc-private/devops!11
- [x] https://gitlab.isc.org/isc-projects/bind9-qa/-/merge_requests/9
Things to consider:
- FreeBSD DoH jobs are not added because Flamethrower queries always timeout.
- This adds 15 more CI jobs:
- Linux (AWS autoscaler): `(auth + recursive + RPZ) * (DoH + DoT) * (amd64 + arm64) = 12`
- FreeBSD (one FreeBSD runner): `(auth + recursive + RPZ) * (DoT) * (amd64) = 3`
- Autoscaler is not yet present on FreeBSD. Adding 3 CI jobs (i.e., DoT) run serially adds 3 hours to the pipeline runtime. Should we add just one FreeBSD DoT job to limit the runtime?
- DoH/DoT performance is slightly lower than pure TCP, so the threshold for the test to pass must be lowered by 5-10% (see isc-private/bind-qa!40).
Merge branch 'mnowak/stress-test-with-doh-dot' into 'main'
See merge request isc-projects/bind9!5800
Add DoH and DoT stress test jobs. The DoH scenario on FreeBSD is omitted
because all Flamethrower's DoH queries timeout on this platform.
Since the response rate of DoT queries is lower than that of DoH and
TCP, the expected TCP response rate is 80%.
Due to the large number of similar stress test configurations, the
"util/generate-stress-test-configs.py" script now generates them as part
of a downstream pipeline. The script is expected to be run exclusively
within the CI environment, which sources all environmental variables and
files.
This refactoring brought the following changes:
- To start a stress test immediately and not wait for artifacts of the
autoreconf job, run the "autoreconf -fi" command as part of every job.
- Drop the BIND_STRESS_TEST_* variables as they were rarely used and
conflicted with mode and platform selection in the configuration
generator.
- Most pipelines now include a few short, randomly selected stress test
jobs. To schedule all stress tests, set the ALL_BIND_STRESS_TESTS
environmental variable, push a tag to CI, or run a scheduled pipeline.
- Set the BIND_STRESS_TESTS_RUN_TIME environmental variable to pick the
stress test runtime of your choosing, set the BIND_STRESS_TESTS_RATE
environmental variable to set different than the default query rate.
- Job timeout is set to 30 minutes plus stress test runtime in minutes.
Extended DNS error 22 (No reachable authority) was previously detected when `fctx_expired` fired. It turns out this function is used as a "safety net" and the timeout detection should be caught earlier.
It was working though, because of another issue fixed by !9927. But then, the recursive request timed out detection occurs before `fctx_expired` making impossible to raise the EDE 22 error.
This fixes the problem by triggering the EDE 22 in the part of the code detecting the (TCP or UDP) time out and taking the decision to cancel the whole fetch (i.e. There is no other server to attempt to contact).
Note this is not targeting users (no release note) because there is no release versions of BIND between !9927 and this changes. Thus a release note would be confusing.
Closes#5137
Merge branch '5137-ede22' into 'main'
See merge request isc-projects/bind9!9985
When an EDE code is added to a message, the code is converted early in a
big-endian order so it can be memcpy-ed directly in the EDE buffer that
will go on the wire.
This previous change forget to update debug logs which still assume the
EDE code was in host byte order. Add a separate variable to
differentiate both and avoid ambiguities
When EDE 3 (stale answer) was added the serve-stale tests were checking
for those exclusively, i.e. grepping for no "EDE" in the dig output when
no stale answer was expected.
However, some stale tests disable stale answers and make the
authoritative server unresponsive, effectively triggering a timed out
request thus an EDE 22. Update those tests so they still tests the
absence of EDE 3 error, but also the presence of EDE 22.
This re-do a previously existing EDE 22 system test as well as add
another one making sure the timed out flow detection works also on UDP
when the resolver is contacting the authoritative server. (the existing
test was using TCP to contact the authoritative servers).
Extended DNS error 22 (No reachable authority) was previously detected
when `fctx_expired` fired. It turns out this function is used as a
"safety net" and the timeout detection should be caught earlier.
It was working though, because of another issue fixed by !9927. Since
this change, the recursive request timed out detection occurs before
`fctx_expired` so EDE 22 is not added to the response message anymore.
The fix of the problem is to add the EDE 22 code in two situations:
- When the dispatch code timed out (rctx_timedout) the resolver code
checks various properties to figure out if it needs to make another
fetch attempt. One of the paramters if the fetch expiration time. If
it expires, the whole recursion is canceled, so it now adds the EDE 22
code.
- If the fetch expiration time doesn't expires in the case above (and
other parameters allows it) a new fetch attempt is made (fctx_query).
But before the new request is actually made, the fetch expiration time
is re-checked. It might then has elapsed, and the whole recursion is
canceled. So it now also adds the EDE 22 code here as well.
The new command is `rndc memprof`. The memory profiling status is also
reported inside `rndc status`. The status also shows whether named can
toggle memory profiling or not and if the server is built with jemalloc.
Closes#4759
Merge branch '4759-add-a-trigger-to-dump-jeprof-data-or-memory-statistics' into 'main'
See merge request isc-projects/bind9!9370
The new command is `rndc memprof`. The memory profiling status is also
reported inside `rndc status`. The status also shows whether named can
toggle memory profiling or not and if the server is built with jemalloc.
The changelog job is supposed to test that the text from GitLab MR
title&description is valid rst syntax and can be built with sphinx. In
49128fc1, the way gitchangelog generates entries was changed - it no
longer writes to the changelog file, but generates output on stdout
instead. Ensure the generated notes is actually written to (some)
rendered file which is part of the docs so that the subsequent sphinx
build attempts to render the note.
Merge branch 'nicki/ci-fix-changelog-job' into 'main'
See merge request isc-projects/bind9!9804
The changelog job is supposed to test that the text from GitLab MR
title&description is valid rst syntax and can be built with sphinx. In
49128fc1, the way gitchangelog generates entries was changed - it no
longer writes to the changelog file, but generates output on stdout
instead. Ensure the generated notes is actually written to (some)
rendered file which is part of the docs so that the subsequent sphinx
build attempts to render the note.
Add support for EDE codes 1 & 2 which might occurs during DNSSEC validation in case of unsupported RRSIG algorithm or DNSKEY digest.
See #2715
Merge branch '2715-ede-unsupported-digest-alg' into 'main'
See merge request isc-projects/bind9!9948
A DNSSEC validation can fail in the case where multiple DNSKEY are
available for a zone and none of them are supported, but for different
reasons: one has a DS record in the parent zone using an unsupported
digest while the other one uses an unsupported encryption algorithm.
Add a specific test case covering this flow and making sure that two
extended DNS error are provided: code 1 and 2, each of them highlighting
unsupported algorithm and digest.
Add support for EDE codes 1 (Unsupported DNSKEY Algorithm) and 2
(Unsupported DS Digest Type) which might occurs during DNSSEC
validation in case of unsupported DNSKEY algorithm or DS digest type.
Because DNSSEC internally kicks off various fetches, we need to copy
all encountered extended errors from fetch responses to the fetch
context. Upon an event, the errors from the fetch context are copied
to the client response.
The minimal dnspython version to run this test is 2.5.0.
Merge branch 'mnowak/pytest_rewrite_cipher-suites' into 'main'
See merge request isc-projects/bind9!8662
The servers are setup and torn down once per each test module. All the
logs and server state persists between individual tests within the same
module. The servers fixture representing these servers should be
module-wide as well.
The base image tends to have a rather old dnspython version and when
used with pylint and mypy it produces errors about newer dnspython
features the old version does not know about.
$ mypy "bin/tests/system/isctest/"
bin/tests/system/isctest/query.py:55: error: Unexpected keyword argument "verify" for "tls" [call-arg]
/usr/lib/python3/dist-packages/dns/query.py:958: note: "tls" defined here
$ pylint --rcfile $CI_PROJECT_DIR/.pylintrc --disable=wrong-import-position $(git ls-files 'bin/tests/system/*.py' | grep -vE 'ans\.py')
************* Module isctest.query
bin/tests/system/isctest/query.py:55:11: E1123: Unexpected keyword argument 'verify' in function call (unexpected-keyword-arg)
When explicitly set to True, the "verify" argument lets dnspython verify
certificates used for the connection. As most certificates in the system
test will inevitably be self-signed, the "verify" argument defaults to
False.
The "verify" argument is present in dnspython since the version 2.5.0.
A number of result codes are obsolete and can be removed. Others, including `ISC_R_NOMEMORY`, are still checked in various places even though they can't occur any longer. These have been cleaned up.
Merge branch 'each-cleanup-results' into 'main'
See merge request isc-projects/bind9!9942
ISCCC_R_SYNTAX, ISCCC_R_EXPIRED, and ISCCC_R_CLOCKSKEW have the
same usage and text formats as DNS_R_SYNTAX, DNS_R_EXPIRED and
DNS_R_CLOCKSCREW respectively. this was originally done because
result codes were defined in separate libraries, and some tool
might be linked with libisccc but not libdns. as the result codes
are now defined in only one place, there's no need to retain the
duplicates.
building BIND without crypto support is no longer possible.
consequently this result code is never sent, and therefore we
don't need code in calling functions to handle it.
the isc_mem allocation functions can no longer fail; as a result,
ISC_R_NOMEMORY is now rarely used: only when an external library
such as libjson-c or libfstrm could return NULL. (even in
these cases, arguably we should assert rather than returning
ISC_R_NOMEMORY.)
code and comments that mentioned ISC_R_NOMEMORY have been
cleaned up, and the following functions have been changed to
type void, since (in most cases) the only value they could
return was ISC_R_SUCCESS:
- dns_dns64_create()
- dns_dyndb_create()
- dns_ipkeylist_resize()
- dns_kasp_create()
- dns_kasp_key_create()
- dns_keystore_create()
- dns_order_create()
- dns_order_add()
- dns_peerlist_new()
- dns_tkeyctx_create()
- dns_view_create()
- dns_zone_setorigin()
- dns_zone_setfile()
- dns_zone_setstream()
- dns_zone_getdbtype()
- dns_zone_setjournal()
- dns_zone_setkeydirectory()
- isc_lex_openstream()
- isc_portset_create()
- isc_symtab_create()
(the exception is dns_view_create(), which could have returned
other error codes in the event of a crypto library failure when
calling isc_file_sanitize(), but that should be a RUNTIME_CHECK
anyway.)
Adjust the limit of maximum disagreements in respdiff results based on
recent pipeline results.
The respdiff and respdiff:asan seem to have almost identical results,
typically around 0.07 % of differences with ocassional spikes up to
around 0.11 %. Similar results are for respdiff:tsan, perhaps with more
common spikes with values up to around 0.12 %. Set the limit to 0.15 %
to allow for some tolerance due to network conditions, time of day etc.
The respdiff:third-party has a slightly higher disagreements average,
with typical values being around 0.12 %. Set the limit to 0.2 %.
Exceeding either of those values should be quite clear indication that
some resolution behaviour has changed, since the values appear to be
very stable within the newly configured limits.
Merge branch 'nicki/ci-respdiff-limits' into 'main'
See merge request isc-projects/bind9!9950
Adjust the limit of maximum disagreements in respdiff results based on
recent pipeline results.
The respdiff and respdiff:asan seem to have almost identical results,
typically around 0.07 % of differences with ocassional spikes up to
around 0.11 %. Similar results are for respdiff:tsan, perhaps with more
common spikes with values up to around 0.12 %. Set the limit to 0.15 %
to allow for some tolerance due to network conditions, time of day etc.
The respdiff:third-party has a slightly higher disagreements average,
with typical values being around 0.12 %. Set the limit to 0.2 %.
Exceeding either of those values should be quite clear indication that
some resolution behaviour has changed, since the values appear to be
very stable within the newly configured limits.
There was confusion about whether the interval was calculated from
the validity period provided on the command line (with -s and -e),
or from the signature being replaced.
Add text to clarify that the interval is calculated from the new
validity period.
Closes#5128
Merge branch '5128-clarify-dnssec-signzone-interval' into 'main'
See merge request isc-projects/bind9!9955
There was confusion about whether the interval was calculated from
the validity period provided on the command line (with -s and -e),
or from the signature being replaced.
Add text to clarify that the interval is calculated from the new
validity period.
In the case when `dnssec-signzone` is called on an already signed zone, and the private key file is unavailable, a signature that needs to be refreshed may be dropped without being able to generate a replacement. This has been fixed.
Closes#5126
Merge branch '5126-dnssec-signzone-retain-rrsig-if-key-is-offline' into 'main'
See merge request isc-projects/bind9!9951
Track inside the dns_dnsseckey structure whether we have seen the
private key, or if this key only has a public key file.
If the key only has a public key file, or a DNSKEY reference in the
zone, mark the key 'pubkey'. In dnssec-signzone, if the key only
has a public key available, consider the key to be offline. Any
signatures that should be refreshed for which the key is not available,
retain the signature.
So in the code, 'expired' becomes 'refresh', and the new 'expired'
is only used to determine whether we need to keep the signature if
the corresponding key is not available (retaining the signature if
it is not expired).
In the 'keysthatsigned' function, we can remove:
- key->force_publish = false;
- key->force_sign = false;
because they are redundant ('dns_dnsseckey_create' already sets these
values to false).
Add a test case for the scenario below.
There is a case when signing a zone with dnssec-signzone where the
private key file is moved outside the key directory (for offline
ksk purposes), and then the zone is resigned. The signature of the
DNSKEY needs refreshing, but is not expired.
Rather than removing the signature without having a valid replacement,
leave the signature in the zone (despite it needs to be refreshed).
If the generated status output exceeds 4096 it was silently truncated, now we output that the status was truncated.
Closes#4180
Merge branch '4180-possible-truncation-in-dns_keymgr_status' into 'main'
See merge request isc-projects/bind9!9905
Extended DNS error mechanism (EDE) may have several errors raised during a DNS resolution. `named` is now able to add up to three EDE codes in a DNS response. In the case of duplicate error codes, only the first one will be part of the DNS response.
Closes#5085
Merge branch '5085-multiple-ede' into 'main'
See merge request isc-projects/bind9!9952
Extended DNS error mechanism (EDE) enables to have several EDE raised
during a DNS resolution (typically, a DNSSEC query will do multiple
fetches which each of them can have an error). Add support to up to 3
EDE errors in an DNS response. If duplicates occur (two EDEs with the
same code, the extra text is not compared), only the first one will be
part of the DNS answer.
Because the maximum number of EDE is statically fixed, `ns_client_t`
object own a static vector of `DNS_DE_MAX_ERRORS` (instead of a linked
list, for instance). The array can be fully filled (all slots point to
an allocated `dns_ednsopt_t` object) or partially filled (or
empty). In such case, the first NULL slot means there is no more EDE
objects.
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.
Merge branch 'aram/dispatch-tcp_connected-fix' into 'main'
See merge request isc-projects/bind9!9927
Since the read timeout now works, the resolver time outs from the
dispatch level instead of from the "hung fetch" timer, and so the
EDE value in 'fctx_expired()' is not being set. Remove the expected
EDE value from the test.
When TCP is used, 'fctx_query()' adds one second to the rtt
(round-trip time) value, but there's a bug when the decision
about using TCP is made already after the calculation. Move the
block of the code which looks up the peers list to decide
whether to use TCP into a place that's before the rtt calculation
is performed. This commit doesn't add or remove any code, it just
moves the code and adds a comment block.
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.
Prevent lock contention among many worker threads referring to the same database node at the same time. This would improve zone and cache database performance for the heavily contended database nodes.
Closes#5130
Merge branch '5130-reduce-lock-contention-in-decrement-reference' into 'main'
See merge request isc-projects/bind9!9963
Previously, this function always acquires a node write lock if it
might need node cleanup in case the reference decrements to 0. In
fact, the lock is unnecessary if the reference is larger than 1 and it
can be optimized as an "easy" case. This optimization could even be
"necessary". In some extreme cases, many worker threads could repeat
acquring and releasing the reference on the same node, resulting in
severe lock contention for nothing (as the ref wouldn't decrement to 0
in most cases). This change would prevent noticeable performance
drop like query timeout for such cases.
Co-authored-by: JINMEI Tatuya <jtatuya@infoblox.com>
Co-authored-by: Ondřej Surý <ondrej@isc.org>
Shutdown the fetch context immediately after the last fetch has been canceled from
that particular fetch context.
Merge branch 'ondrej/shutdown-the-fetch-context-early' into 'main'
See merge request isc-projects/bind9!9958
Currently, the fetch context will continue running even when the last
fetch (response) has been removed from the context, so named can process
and cache the answer. This can lead to a situation where the number of
outgoing recursing clients exceeds the the configured number for
recursive-clients.
Be more stringent about the recursive-clients limit and shutdown the
fetch context immediately after the last fetch has been canceled from
that particular fetch context.
Resolver under heavy-load could exhaust the memory available for storing
the information in the Address Database (ADB) effectively evicting already
stored information in the ADB. The memory used to retrieve and provide
information from the ADB is now not a subject of the same memory limits
that are applied for storing the information in the Address Database.
Closes#5127
Merge branch '5127-change-ADB-memory-split' into 'main'
See merge request isc-projects/bind9!9954
Address Database (ADB) shares the memory for the short lived ADB
objects (finds, fetches, addrinfo) and the long lived ADB
objects (names, entries, namehooks). This could lead to a situation
where the resolver-heavy load would force evict ADB objects from the
database to point where ADB is completely empty, leading to even more
resolver-heavy load.
Make the short lived ADB objects use the other memory context that we
already created for the hashmaps. This makes the ADB overmem condition
to not be triggered by the ongoing resolver fetches.
The network manager layer has two different timers with their
own timeout values for TCP connections: connect timeout and read
timeout. Separate the connect and the read TCP timeouts in the
dispatch module too.
Closes#5009
Merge branch '5009-dispatch-separate-connect-and-read-timeouts' into 'main'
See merge request isc-projects/bind9!9698
In some places there was a limitation of the maximum timeout
value of INT16_MAX, which is only about 32 seconds. Refactor
the code to remove the limitation.
The network manager layer has two different timers with their
own timeout values for TCP connections: connect timeout and read
timeout. Separate the connect and the read TCP timeouts in the
dispatch module too.
struct fetchctx does have several fields which are now unused or confusing, removing those.
Merge branch 'colin/remove-fctx-validator' into 'main'
See merge request isc-projects/bind9!9945
struct fetchctx does have a list of pending validators as well as a
pointer to the HEAD validator. Remove the validator pointer to avoid
confusion, as there is no perticular reasons to have it directly
accessible outside of the list.
Fix DNS-over-HTTP(S) implementation issues that arise under heavy
query load. Optimize resource usage for :iscman:`named` instances
that accept queries over DNS-over-HTTP(S).
Previously, :iscman:`named` would process all incoming HTTP/2 data
at once, which could overwhelm the server, especially when dealing
with clients that send requests but don't wait for responses. That
has been fixed. Now, :iscman:`named` handles HTTP/2 data in smaller
chunks and throttles reading until the remote side reads the
response data. It also throttles clients that send too many requests
at once.
Additionally, :iscman:`named` now carefully processes data sent by
some clients, which can be considered "flooding." It logs these
clients and drops connections from them.
:gl:`#4795`
In some cases, :iscman:`named` could leave DNS-over-HTTP(S)
connections in the `CLOSE_WAIT` state indefinitely. That also has
been fixed. ISC would like to thank JF Billaud for thoroughly
investigating the issue and verifying the fix.
:gl:`#5083`
See https://gitlab.isc.org/isc-projects/bind9/-/issues/4795
Closes https://gitlab.isc.org/isc-projects/bind9/-/issues/5083
Merge branch 'artem-improve-doh-resource-usage' into 'v9.21.4-release'
See merge request isc-private/bind9!732
We started using isc_nm_bad_request() more actively throughout
codebase. In the case of HTTP/2 it can lead to a large count of
useless "Bad Request" messages in the BIND log, as often we attempt to
send such request over effectively finished HTTP/2 sessions.
This commit fixes that.
A call to isc_nm_read_stop() would always stop reading timer even in
manual timer control mode which was added with StreamDNS in mind. That
looks like an omission that happened due to how timers are controlled
in StreamDNS where we always stop the timer before pausing reading
anyway (see streamdns_on_complete_dnsmessage()). That would not work
well for HTTP, though, where we might want pause reading without
stopping the timer in the case we want to split incoming data into
multiple chunks to be processed independently.
I suppose that it happened due to NM refactoring in the middle of
StreamDNS development (at the time isc_nm_cancelread() and
isc_nm_pauseread() were removed), as the StreamDNS code seems to be
written as if timers are not stoping during a call to
isc_nm_read_stop().
This commit introduces manual read timer control as used by StreamDNS
and its underlying transports. Before that, DoH code would rely on the
timer control provided by TCP, which would reset the timer any time
some data arrived. Now, the timer is restarted only when a full DNS
message is processed in line with other DNS transports.
That change is required because we should not stop the timer when
reading from the network is paused due to throttling. We need a way to
drop timed-out clients, particularly those who refuse to read the data
we send.
This commit adds logic to make code better protected against clients
that send valid HTTP/2 data that is useless from a DNS server
perspective.
Firstly, it adds logic that protects against clients who send too
little useful (=DNS) data. We achieve that by adding a check that
eventually detects such clients with a nonfavorable useful to
processed data ratio after the initial grace period. The grace period
is limited to processing 128 KiB of data, which should be enough for
sending the largest possible DNS message in a GET request and then
some. This is the main safety belt that would detect even flooding
clients that initially behave well in order to fool the checks server.
Secondly, in addition to the above, we introduce additional checks to
detect outright misbehaving clients earlier:
The code will treat clients that open too many streams (50) without
sending any data for processing as flooding ones; The clients that
managed to send 1.5 KiB of data without opening a single stream or
submitting at least some DNS data will be treated as flooding ones.
Of course, the behaviour described above is nothing else but
heuristical checks, so they can never be perfect. At the same time,
they should be reasonable enough not to drop any valid clients,
realatively easy to implement, and have negligible computational
overhead.
Initially, our DNS-over-HTTP(S) implementation would try to process as
much incoming data from the network as possible. However, that might
be undesirable as we might create too many streams (each effectively
backed by a ns_client_t object). That is too forgiving as it might
overwhelm the server and trash its memory allocator, causing high CPU
and memory usage.
Instead of doing that, we resort to processing incoming data using a
chunk-by-chunk processing strategy. That is, we split data into small
chunks (currently 256 bytes) and process each of them
asynchronously. However, we can process more than one chunk at
once (up to 4 currently), given that the number of HTTP/2 streams has
not increased while processing a chunk.
That alone is not enough, though. In addition to the above, we should
limit the number of active streams: these streams for which we have
received a request and started processing it (the ones for which a
read callback was called), as it is perfectly fine to have more opened
streams than active ones. In the case we have reached or surpassed the
limit of active streams, we stop reading AND processing the data from
the remote peer. The number of active streams is effectively decreased
only when responses associated with the active streams are sent to the
remote peer.
Overall, this strategy is very similar to the one used for other
stream-based DNS transports like TCP and TLS.
When answering queries, don't add data to the additional section if the answer has more than 13 names in the RDATA. This limits the number of lookups into the database(s) during a single client query, reducing query processing load.
See isc-projects/bind9#5034
Merge branch '5034-security-limit-additional' into 'v9.21.4-release'
See merge request isc-private/bind9!750
Limit the number of records appended to ADDITIONAL section to the names
that have less than 14 records in the RDATA. This limits the number
of the lookups into the database(s) during single client query.
Also don't append any additional data to ANY queries. The answer to ANY
is already big enough.
Instead of running the whole resolver/ns4 server with -T noaa flag,
use it only for the part where it is actually needed. The -T noaa
could interfere with other parts of the test because the answers don't
have the authoritative-answer bit set, and we could have false
positives (or false negatives) in the test because the authoritative
server doesn't follow the DNS protocol for all the tests in the resolver
system test.
All the database implementations share the same names for the methods
implementing the database. That has some advantages like knowing what
to expect, but it turns out that any time such method shows up in any
kind of tracing - be it perf record, backtrace or anything else that
uses symbol names, it is very hard to distinguish whether the find()
belongs to qpcache, qpzone, builtin or sdlz implementation.
Make at least the names for qpzone and qpcache unique.
A bug in the qpzone database could trigger a crash when querying for a deleted name, or a newly-added empty non-terminal name, in an NSEC3-signed zone. This has been fixed.
Closes#5108
Merge branch '5108-nsec3-empty-node' into 'main'
See merge request isc-projects/bind9!9928
there was a database bug in which dns_db_find() could get a partial
match for the query name, but still set foundname to match the full
query name. this triggered an assertion when query_addwildcardproof()
assumed that foundname would be shorter.
the database bug has been fixed, but in case it happens again, we
can just copy the name instead of splitting it. we will also log a
warning that the closest-encloser name was invalid.
when adding a new NSEC3 record, dns_nsec3_addnsec3() uses a
dbiterator to seek to the newly created node and then find its
predecessor. dbiterators in the qpzone use snapshots, so changes
to the database are not reflected in an already-existing iterator.
consequently, when we add a new node, we have to create a new iterator
before we can seek to it.
this test adds a record with empty non-terminal nodes above it. this
has also been observed to trigger the crash in NSEC3 zones.
NOTE: the test currently fails, because while there is no crash, the
query results are not as expected. when we add a node below an ENT,
receive_secure_serial() gets DNS_R_PARTIALMATCH, and the signed
zone is never updated. this is not a regression from fixing the
crash bug; it's a separate inline-signing bug.
test that there's no crash when querying for a newly-deleted node.
(incidentally also renamed ns3/named.conf.in to ns3/named1.conf.in,
because named2.conf.in does exist, and they should match.)
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().
Commit b121f02eac renamed the top-level
"primaries" block in bin/named/config.c to "remote-servers". This
configuration block lists the primary servers used for an IANA root zone
mirror when no primary servers are explicitly specified for it in the
configuration. However, the relevant part of the named_zone_configure()
function only looks for a top-level "primaries" block and not for any of
its synonyms. As a result, configuring an IANA root zone mirror with
just:
zone "." {
type mirror;
};
now results in a cryptic fatal error on startup:
loading configuration: not found
exiting (due to fatal error)
Fix by using the correct top-level block name in named_zone_configure().
Response policy zones (RPZ) and catalog zones were not working correctly if they had an $INCLUDE statement defined. This has been fixed.
Closes#5111
Merge branch '5111-includes-disable-rpz-and-catz-fix' into 'main'
See merge request isc-projects/bind9!9930
The code in zone_startload() disables RPZ and CATZ for a zone if
dns_master_loadfile() returns anything other than ISC_R_SUCCESS,
which makes sense, but it's an error because zone_startload() can
also return DNS_R_SEENINCLUDE upon success when the zone had an
$INCLUDE statement.
Add performance tests of DoH using the GET protocol to nightly pipelines.
Merge branch 'nicki/ci-shotgun-doh-get' into 'main'
See merge request isc-projects/bind9!9926
Adjust number of zones down to 23 to match those present when testing in FIPS mode.
Closes#5097
Merge branch '5097-checking-startup-notify-rate-limit-fails-on-ol-8-fips' into 'main'
See merge request isc-projects/bind9!9919
Debug messages logging network traffic now include information about both sides of each communication channel rather than just one of them.
Closes#4345
Merge branch '4345-log-both-from-and-to-socket-in-debug-messages' into 'main'
See merge request isc-projects/bind9!8349
Ensure the log prefixes passed to the dns_message_logpacketfrom()
function by its callers do not include the word "from" as the latter is
now emitted by the logfmtpacket() helper function.
Ensure the log prefixes passed to the dns_message_logpacketfromto()
function by its callers do not include the words "from" or "to" as those
are now emitted by the logfmtpacket() helper function.
Move dns_dispentry_getlocaladdress() calls around so that they are not
only invoked when dnstap support is compiled in. This function calls
isc_nmhandle_localaddr(), which may issue a system call, but only if the
ISC_SOCKET_DETAILS preprocessor macro is set at compile time.
Pass the value extracted by dns_dispentry_getlocaladdress() to
dns_message_logpacketfromto() so that it gets logged, adding useful
information to the relevant debug messages.
Since dns_message_logpacket() only takes a single socket address as a
parameter (and it is always the sending socket's address), rename it to
dns_message_logpacketfrom() so that its name better conveys its purpose
and so that the difference in purpose between this function and
dns_message_logpacketfromto() becomes more apparent.
Since dns_message_logfmtpacket() needs to be provided with both "from"
and "to" socket addresses, rename it to dns_message_logpacketfromto() so
that its name better conveys its purpose. Clean up the code comments
for that function.
Change the function prototype for dns_message_logfmtpacket() so that it
takes two isc_sockaddr_t parameters: one for the sending side and
another one for the receiving side. This enables debug messages to be
more precise.
Also adjust the function prototype for logfmtpacket() accordingly.
Unlike dns_message_logfmtpacket(), this function must not require both
'from' and 'to' parameters to be non-NULL as it is still going to be
used by dns_message_logpacket(), which only provides a single socket
address. Adjust its log format to handle both of these cases properly.
Adjust both dns_message_logfmtpacket() call sites accordingly, without
actually providing the second socket address yet. (This causes the
revised REQUIRE() assertion in dns_message_logfmtpacket() to fail; the
issue will be addressed in a separate commit.)
Both existing callers of the dns_message_logfmtpacket() function set the
argument passed as 'style' to &dns_master_style_comment. To simplify
these call sites, drop the 'style' parameter from the prototype for
dns_message_logfmtpacket() and use a fixed value of
&dns_master_style_comment in the function's body instead.
All callers of the logfmtpacket() helper function require the argument
passed as 'address' to be non-NULL. Meanwhile, the 'newline' and
'space' local variables in logfmtpacket() are only set to values
different than their initial values if the 'address' parameter is NULL.
Replace the 'newline' and 'space' local variables in logfmtpacket() with
fixed strings to improve code readability.
Enable extracting the exact address/port that a local wildcard/TCP socket is bound to, improving the accuracy of dnstap logging and providing more information in debug logs produced by system tests. Since this requires issuing an extra system call on some hot paths, this new feature is only enabled when the ``ISC_SOCKET_DETAILS`` preprocessor macro is set at compile time.
Closes#4344
Merge branch '4344-enable-extraction-of-exact-local-socket-addresses' into 'main'
See merge request isc-projects/bind9!8348
Extracting the exact address that each wildcard/TCP socket is bound to
locally requires issuing the getsockname() system call, which libuv
exposes via its uv_*_getsockname() functions. This is only required for
detailed logging and comes at a noticeable performance cost, so it
should not happen by default. However, it is useful for debugging
certain problems (e.g. cryptic system test failures), so a convenient
way of enabling that behavior should exist.
Update isc_nmhandle_localaddr() so that it calls uv_*_getsockname() when
the ISC_SOCKET_DETAILS preprocessor macro is set at compile time.
Ensure proper handling of sockets that wrap other sockets.
Set the new ISC_SOCKET_DETAILS macro by default when --enable-developer
is passed to ./configure. This enables detailed logging in the system
tests run in GitLab CI without affecting performance in non-development
BIND 9 builds.
Note that setting the ISC_SOCKET_DETAILS preprocessor macro at compile
time enables all callers of isc_nmhandle_localaddr() to extract the
exact address of a given local socket, which results e.g. in dnstap
captures containing more accurate information.
Mention the new preprocessor macro in the section of the ARM that
discusses why exact socket addresses may not be logged by default.
This MR is a prerequisite for !8348.
It intentionally does not have a changelog entry associated with it, to
prevent making a false impression of improving connection reuse **for
existing code**. It will only make a difference once !8348 gets merged
(and even then, only if the new `ISC_SOCKET_DETAILS` macro will be set
during build). That's because `isc_nmhandle_localaddr()` currently
simply returns `handle->local` and its return value will only be set to
the actual address the socket is bound to with !8348 in place.
Note that `dns_dispatch_gettcp()` is currently only used by the
`dns_request` API, so this MR's potential for introducing new breakage
is relatively low.
Closes#4693
Merge branch '4693-improve-reuse-of-outgoing-tcp-connections' into 'main'
See merge request isc-projects/bind9!8972
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.
This change ensures that SNI extension is used in outgoing connections over TLS (e.g. for DoT and DoH) when applicable.
Closes#5099
Merge branch 'artem-outgoing-tls-sni-support' into 'main'
See merge request isc-projects/bind9!9923
This commit ensures that BIND enables TLS SNI support for outgoing DoT
connections (when possible) in order to improve compatibility with
other DNS server software.
This commit adds support for setting SNI hostnames in outgoing
connections over TLS.
Most of the changes are related to either adapting the code to accept
and extra argument in *connect() functions and a couple of changes to
the TLS Stream to actually make use of the new SNI hostname
information.
ThreadSanitizer has trouble understanding that gluelist->glue is
constant after it is assigned to the slabheader with cmpxchg. Help
ThreadSanitizer to understand the code by using CMM_STORE_SHARED and
CMM_LOAD_SHARED on gluelist->glue.
Merge branch 'ondrej/hint-tsan-in-addglue' into 'main'
See merge request isc-projects/bind9!9929
Previously, we had an ISC_CONSTEXPR macro that was expanded to either
`constexpr` or `static const`, depending on compiler support. To make
the code cleaner, move `constexpr` support detection to Autoconf; if
`constexpr` support is missing from the compiler, define `constexpr` as
`static const` in config.h.
Merge branch 'ondrej/rename-ISC_CONSTEXPR-to-constexpr-shim' into 'main'
See merge request isc-projects/bind9!9924
Previously, we had an ISC_CONSTEXPR macro that was expanded to either
`constexpr` or `static const`, depending on compiler support. To make
the code cleaner, move `constexpr` support detection to Autoconf; if
`constexpr` support is missing from the compiler, define `constexpr` as
`static const` in config.h.
Since BIND 9 headers are not longer public, there's no reason to keep
the ISC_LANG_BEGINDECL and ISC_LANG_ENDDECL macros to support including
them from C++ projects.
Merge branch 'ondrej/remove-C++-header-support' into 'main'
See merge request isc-projects/bind9!9925
Since BIND 9 headers are not longer public, there's no reason to keep
the ISC_LANG_BEGINDECL and ISC_LANG_ENDDECL macros to support including
them from C++ projects.
For performance reasons, the returned GLUE records are cached on the first use. The current implementation could randomly cause a performance drop and increased memory use. This has been fixed.
Closes#5064
Merge branch '5064-rewrite-glue-cache-in-qpzone' into 'main'
See merge request isc-projects/bind9!9831
This is a second attempt to rewrite the GLUE cache to not use per
database version hash table. Instead of keeping a hash table indexed by
the node, use a directly linked list of GLUE records for each
slabheader. This was attempted before, but there was a data race caused
by the fact that the thread cleaning the GLUE records could be slower
than accessing the slab headers again and reinitializing the wait-free
stack.
The improved design builds on the previous design, but adds a new
dns_gluelist structure that has a pointer to the database version.
If a dns_gluelist belonging to a different (old) version is detected, it
is just detached from the slabheader and left for the closeversion() to
clean it up later.
coccinelle v1.1 trips over a superfluous isc_mem_get() NULL check in
tests/libtest/ns.c and reports the following failure in CI:
EXN: Failure("rule starting on line 26: already tagged token:\nC code context\nFile \"./tests/libtest/ns.c\", line 350, column 1, charpos = 7939\n around = 'if',\n whole content = \tif (qctx != NULL) {") in ./tests/libtest/ns.c
coccinelle v1.1 reports the following failure:
EXN: Failure("./lib/dns/sdlz.c: 172: try to delete an expanded token: unsigned") in ./lib/dns/sdlz.c
coccinelle v1.2 reports the following failure:
EXN: Failure("./util/models.c: 21: try to delete an expanded token: unsigned") in ./util/models.c
DLV is long gone, so we can remove design documentation around DLV, related command line options (that were already a hard failure), and some DLV related test remnants.
Merge branch 'matthijs-remove-dlv-remnants' into 'main'
See merge request isc-projects/bind9!9888
DLV is long gone, so we can remove design documentation around DLV,
related command line options (that were already a hard failure),
and some DLV related test remnants.
The top blocks 'primaries' and 'parental-agents' are no longer preferred and should be renamed to 'remote-servers'. The zone statements 'parental-agents' and 'primaries' are still used, and may refer to any 'remote-servers' top block.
Closes#4544
Merge branch '4544-primaries-block-documentation-issues' into 'main'
See merge request isc-projects/bind9!9822
The 'remote-servers' named.conf reference conflicts with the standard
term from the glossary. Rename the standard term to server-list to
make the docs build.
Add back the top blocks 'parental-agents', 'primaries', and 'masters'
to the configuration. Do not document them as so many names for the
same clause is confusing.
This has a slight negative side effect that a top block 'primaries'
can be referred to with a zone statement 'parental-agents' for example,
but that shouldn't be a big issue.
Having zone statements that are also top blocks is confusing, and if
we want to add more in the future (which I suspect will be for
generalized notifications, multi-signer), we need to duplicate a lot
of code.
Remove top blocks 'parental-agents' and 'primaries' and just have one
top block 'remote-servers' that you can refer to with zone statements.
Fix the loop terminating condition to get consistent sample sizes and increase the minimum number of samples from 20 to 40.
Closes#5091
Merge branch '5091-investigate-checking-startup-notify-rate-limit-failure' into 'main'
See merge request isc-projects/bind9!9894
The terminating conditions for the startup notify test would
occasionally get ~20 records or get +10 seconds of records due to
a bad terminating condition. Additionally 20 samples lead to test
failures. Fix the terminating condition to use the correct conditional
(-eq -> -ge) and increase the minimum number of log entries to
average over to 40.
gcovr has issues with processing files produced as part of a BIND 9
build with tracing support enabled (--enable-tracing). Depending on the
gcovr version used, these issues may result in either warnings or
failures being reported by that tool. Disable tracing support for
gcovr-enabled builds to work around these issues.
Include the recent changes such as:
- changes to running system tests
- gitlab development workflow
- changelog and release note process
Closes#5045
Merge branch '5045-update-contributing' into 'main'
See merge request isc-projects/bind9!9784
After the rndc reload command finished, we might have queried the
database zone sooner than it was reloaded because rndc reloads zones
asynchronously if no specific zone was provided. We should wait for "all
zones loaded" in the ns1 log to be sure.
Closes#5075
Merge branch '5075-database-rndc-reload-ensure-all-zones-loaded' into 'main'
See merge request isc-projects/bind9!9829
After the rndc reload command finished, we might have queried the
database zone sooner than it was reloaded because rndc reloads zones
asynchronously if no specific zone was provided. We should wait for "all
zones loaded" in the ns1 log to be sure.
The `sortlist` option, which was deprecated in BIND 9.20, has now been removed.
Closes#4665
Merge branch '4665-remove-sortlist' into 'main'
See merge request isc-projects/bind9!9839
this commit removes the deprecated "sortlist" option. the option
is now marked as ancient; it is a fatal error to use it in
named.conf.
the sortlist system test has been removed, and other tests that
referenced the option have been modified.
the enabling functions, dns_message_setsortorder() and
dns_rdataset_towiresorted(), have also been removed.
Check that zones with AAAA records are served by IPv6 servers and that zones with A records are served by IPv4 servers.
Sometimes, IPv6 services are accidentally misconfigured and zones with IPv6 (AAAA) address records are not served by DNS servers with IPv6 addresses, which means they need to use translation devices to look up those IPv6 addresses. The reverse is also sometimes true: zones with A records are not resolvable over IPv4 when they should be.
To prevent this, BIND now looks for these misconfigured zones and issues a warning if they are found.
Closes#4370
Merge branch '4370-check-that-a-zone-is-served-by-ipv6-servers-if-it-has-aaaa-records' into 'main'
See merge request isc-projects/bind9!8393
This checks that "zone has A records but is not served by IPv4
servers" and "zone has AAAA records but is not served by IPv6
servers" are emitted when they should be and not when they shouldn't
be.
named-checkzone will now, as part of the zone's integrity checks,
look to see if there are A or AAAA records being served and if so
check that the nameservers have A or AAAA records respectively.
These are a sometimes overlooked checks that, if not met, can mean
that a service that is supposed to reachable over IPv6 will not be
resolvable when the recursive resolver is IPv6 only. Similarly for
IPv4 servers when there are IPv4 only resolvers.
Some files used logmodule names that had been copied in from elsewhere; these have now been given module names of their own. Also, the RBT and RBTDB logmodules have been removed, since they are now unused.
Merge branch 'each-cleanup-logmodules' into 'main'
See merge request isc-projects/bind9!9895
- remove obsolete DNS_LOGMODULE_RBT and DNS_LOGMODULE_RBTDB
- correct the misuse of the wrong log modules in dns/rpz.c and
dns/catz.c, and add DNS_LOGMODULE_RPZ and DNS_LOGMODULE_CATZ
to support them.
These options have been deprecated in 9.19 in favor of the 'trust-anchors' option and are now being removed.
Closes#5080
Merge branch '5080-remove-trusted-and-managed-keys' into 'main'
See merge request isc-projects/bind9!9855
The style guide now mentions clang-format, doesn't parenthesize return values, and no longer calls for backward compatibility in public function names.
Merge branch 'each-style-update' into 'main'
See merge request isc-projects/bind9!9892
`shutdown_trigger_close_cb` is not called in the main loop since
queued events in the `loop->async_trigger`, including loop teardown
(shutdown_server) are processed first, before the `uv_close` callback
is executed..
In order to pass the information to the queued events, it is necessary
to set the flag earlier in the process and not wait for the `uv_close`
callback to trigger.
The December releases suffer from the ns2/managed1.conf file not being
in the mkeys extra_artifacts. This manifests only when pytest is run
with the --setup-only option, which is the case in the
cross-version-config-tests CI job. The original issue is fixed in !9815,
but the fix will be effective only when subsequent releases are out.
The #4666 issue removed the "fixed" value for the "rrset-order" option
which is still present in the December release system test and which the
current named can't handle. This will be addressed when when the January
9.21 release is published.
The #4482 issue removed the "dnssec-must-be-secure" feature.
If the RETERR define is only used once in a file, just drop the macro.
Merge branch 'matthijs-remove-single-use-define-reterr' into 'main'
See merge request isc-projects/bind9!9871
This allows easier identification of which burst is which in
named.run.
Merge branch 'marka-use-different-burst-name-for-forensics' into 'main'
See merge request isc-projects/bind9!9881
This subtest exercises static stub behaviour when server-addresses has an address. This was misidentified in the description.
Closes!9799
Merge branch 'marka-fix-stub-subtest-description' into 'main'
See merge request isc-projects/bind9!9799
Search directive from resolv.conf had a maximum of 8 domains. Any
more were ignored. Do not ignore them anymore; iterate over any
number of domains.
Test resolv.conf support by checking the first and last domain in
the search list. Ignore the domains between; just ensure that the
last domain in the configuration is the last domain parsed.
The line after an unknown directive in resolv.conf could accidentally be skipped, potentially affecting dig, host, nslookup, nsupdate, or delv. This has been fixed.
Closes#5084
Merge branch '5084-plain-unknown-keyword-in-resolv-conf-not-handled-propely' into 'main'
See merge request isc-projects/bind9!9865
Update to the new unit test framework.
Add a test for an unknown directive without any arguments.
Add test for an unknown directive without arguments, followed
by a search directive.
Only call eatline() to skip to the next line if we're not
already at the end of a line when parsing an unknown directive.
We were accidentally skipping the next line when there was only
a single unknown directive on the current line.
The DNS_R_MUSTBESECURE lost its meaning with removal of
dnssec-must-be-secure option, so replace the few remaining (and a bit
confusing) use of this result code with DNS_R_NOVALIDSIG.
The dnssec-must-be-secure feature was added in the early days of BIND 9
and DNSSEC and it makes sense only as a debugging feature. There are no
reasons to keep this feature in the production code anymore.
Remove the feature to simplify the code.
Remove the "fixed" value from the "rrset-order" option and from the
autoconf script.
Closes#4666
Merge branch '4666-remote-rrset-order-fixed' into 'main'
See merge request isc-projects/bind9!9852
Commit af7db89513 as part of #4141 was supposed to apply the 'max-recursion-queries' quota to validator queries, but the counter was never actually passed on to 'dns_resolver_createfetch()'. This has been fixed, and the global query counter ('max-query-count', per client request) is now also added.
Related to #4980
Merge branch '4980-pass-counters-in-validator-createfetch' into 'main'
See merge request isc-projects/bind9!9856
Commit af7db89513 as part of #4141 was
supposed to apply the 'max-recursion-queries' quota to validator
queries, but the counter was never actually passed on to
dns_resolver_createfetch(). This has been fixed, and the global query
counter ('max-query-count', per client request) is now also added.
Upstream code doesn't do regular releases, so we need to regularly
sync the code from the upstream repository. This is synchronization up
to the commit f8d0513 from Jan 29, 2024.
When the `attach-cache` option is used in the `options` block with an arbitrary name, it causes all views to use the same cache. Previously, this configuration caused the cache to be deleted and a new cache created every time the server was reconfigured. This has been fixed.
Closes#5061
Merge branch '5061-attach-cache-fix' into 'main'
See merge request isc-projects/bind9!9787
when the attach-cache option is used in the options block
with an arbitrary name, it causes all views to use the same
cache. however, previously, this could cause the cache to be
deleted and a new cache created every time the server was
reconfigured. this did *not* occur when attach-cache was
used at the view level to refer back to another view's cache.
in this commit we correct the problem by checking for
pre-existing caches during reconfiguration, and moving
them from the old server cache list to the new cache list
before cleaning up and freeing the old cache list.
While implementing the global limit 'max-query-count', initially I
thought adding the variable to the resolver structure. But the limit
is per client request so it was moved to the view structure (and
counter in ns_query structure). However, I forgot to remove the
variable from the resolver structure again. This commit fixes that.
When returning zone statistics counters, the statistics sets are now attached while the zone is locked. This addresses Coverity warnings CID 468720, 468728 and 468729.
Closes#4934
Merge branch '4934-lock-and-attach-when-return-zone-stats' into 'main'
See merge request isc-projects/bind9!9488
Commit 4b3d0c6600 has removed them, but
did not remove few traces in documentation and help. Remove them from
remaining places.
Merge branch 'main-memory-flags-size-mctx' into 'main'
See merge request isc-projects/bind9!9606
`dnssec-signzone` was using revoked keys for signing RRsets other than DNSKEY. This has been corrected.
Closes#5070
Merge branch '5070-dnssec-signzone-fix-revoke' into 'main'
See merge request isc-projects/bind9!9800
If ECDSAP256SHA256_SUPPORTED or ECDSAP384SHA384_SUPPORTED variables were
not present in the environment, os.environ would raise KeyError that is
not being handled in the decorator. Use os.getenv() instead.
To mitigate DNS flood attacks over a single TCP connection, we throttle the connection when the other side does not read the data. Throttling should only occur on server-side sockets, but erroneously also happened for nsupdate, which acts as a client. When nsupdate started throttling the connection, it never attempts to read again. This has been fixed.
Closes#4910
Merge branch '4910-nsupdate-hangs-when-processing-large-update' into 'main'
See merge request isc-projects/bind9!9709
The root cause is the fix for CVE-2024-0760 (part 3), which resets
the TCP connection on a failed send. Specifically commit
4b7c61381f stops reading on the socket
because the TCP connection is throttling.
When the tcpdns_send_cb callback thinks about restarting reading
on the socket, this fails because the socket is a client socket.
And nsupdate is a client and is using the same netmgr code.
This commit removes the requirement that the socket must be a server
socket, allowing reading on the socket again after being throttled.
The configuration option 'max-query-count' sets how many outgoing queries per client request is allowed. The existing 'max-recursion-queries' is the number of permissible queries for a single name and is reset on every CNAME redirection. This new option is a global limit on the client request. The default is 200.
This allows us to send a bit more queries while looking up a single name. The default for 'max-recursion-queries' is changed from 32 to 50.
Closes#4980Closes#4921
Merge branch '4980-global-limit-outgoing-queries' into 'main'
See merge request isc-projects/bind9!9737
Changing the default for max-recursion-queries from 100 to 32 was too
strict in some cases, especially lookups in reverse IPv6 trees started
to fail more frequently. From issue #4921 it looks like 50 is a better
default.
Now that we have 'max-query-count' as a global limit of outgoing queries
per client request, we can increase the default for
'max-recursion-queries' again, as the number of recursive queries is
no longer bound by the multiple of 'max-recursion-queries' and
'max-query-restarts'.
This adds a new test directory specifically for CAMP attacks. This first
test in this test directory follows multiple CNAME chains, restarting
the max-recursion-queries counter, but should bail when the global
maximum quota max-query-count is reached.
Add another option to configure how many outgoing queries per
client request is allowed. The existing 'max-recursion-queries' is
per restart, this one is a global limit.
The ns2/managed1.conf file is created by the setup.sh script. Then, in
the tests.sh script it is moved to ns2/managed.conf. The latter file
name is in mkeys extra_artifacts, but the former one is not. This is a
problem when pytest is started with the --setup-only option as it only
runs the setup.sh script (e.g., in the cross-version-config-tests CI
job) and thus failing the "Unexpected files found" assertion.
Merge branch 'mnowak/mkeys-add-ns2-managed1-conf-to-extra-artifacts' into 'main'
See merge request isc-projects/bind9!9815
The ns2/managed1.conf file is created by the setup.sh script. Then, in
the tests.sh script it is moved to ns2/managed.conf. The latter file
name is in mkeys extra_artifacts, but the former one is not. This is a
problem when pytest is started with the --setup-only option as it only
runs the setup.sh script (e.g., in the cross-version-config-tests CI
job) and thus failing the "Unexpected files found" assertion.
Previously, the update policy rules check was moved earlier in the
sequence, and the keep rule match pointers were kept to maintain the
ability to verify maximum records by type.
However, these pointers can become invalid if server reloading
or reconfiguration occurs before update completion. To prevent
this issue, extract the maximum records by type value immediately
during processing and only keep the copy of the values instead of the
full ssurule.
The automatically-generated grammar for named.conf clauses that may or may not be enabled at compile time will now include the same comment, regardless of whether or not they are. Previously, the grammar didn't include a comment if an option was enabled, but said "not configured" if it was disabled. Now, in both cases, it will say "optional (only available if configured)".
Closes#4960
Merge branch '4960-optional-grammar' into 'main'
See merge request isc-projects/bind9!9579
the generated grammar for named.conf clauses that may or may not be
enabled at compile time will now print the same comment regardless of
whether or not they are.
previously, the grammar didn't print a comment if an option was enabled,
but printed "not configured" if it was disabled. now, in both cases,
it will say "optional (only available if configured)".
as an incidental fix, clarified the documentation for "named-checkconf -n".
When the tests were added, the files were generated without FIPS
compatibility in mind. That made the tests fail on recent OpenSSL
versions in FIPS mode.
So, the files were regenerated on a FIPS compliant system using the
following stanza:
```
$ openssl dhparam -out <file> 3072
```
Apparently, the old files are not valid for FIPS starting with OpneSSL
3.1.X release series as "FIPS 140-3 compliance changes" are mentioned
in the [changelog](https://openssl-library.org/news/openssl-3.1-notes/).
Closes#5074.
Merge branch '5074-fips-compatible-dhparams' into 'main'
See merge request isc-projects/bind9!9807
When the tests were added, the files were generated without FIPS
compatibility in mind. That made the tests fail on recent OpenSSL
versions in FIPS mode.
So, the files were regenerated on a FIPS compliant system using the
following stanza:
$ openssl dhparam -out <file> 3072
Apparently, the old files are not valid for FIPS starting with OpneSSL
3.1.X release series as "FIPS 140-3 compliance changes" are mentioned
in the changelog:
https://openssl-library.org/news/openssl-3.1-notes/
When the resolver is trying to query an authority server and eventually timed out, a SERVFAIL answer is given to the client. Add the Extended DNS Error Code 22 - No Reachable Authority to the response.
Closes#2268
Merge branch '2268/ede-no-reachable-authority' into 'main'
See merge request isc-projects/bind9!9743
Add support for Extended DNS Errors (EDE) error 22: No reachable
authority. This occurs when after a timeout delay when the resolver is
trying to query an authority server.
The log message would not be ever reached, because the IPv6 API is
always considered to be complete. Just remove the dead code.
Closes#5068
Merge branch '5068-remove-dead-code-from-ns_interface' into 'main'
See merge request isc-projects/bind9!9798
When release notes are generated, the text is wrapped and line breaks
are inserted into each paragraph (sourced from the commit message's
body). Prevent line breaks after hyphens, as these are often used for
option names. This makes it possible to easily find the options
afterwards.
Merge branch 'nicki/gitchangelog-dont-break-on-hyphens' into 'main'
See merge request isc-projects/bind9!9801
When release notes are generated, the text is wrapped and line breaks
are inserted into each paragraph (sourced from the commit message's
body). Prevent line breaks after hyphens, as these are often used for
option names. This makes it possible to easily find the options
afterwards.
Add extra log messages about TCP connection management.
Merge branch 'tcp-accept-extended-logging' into 'main'
See merge request isc-projects/bind9!9089
The SERVFAIL cache doesn't have a memory bound and the
cleaning of the old SERVFAIL cache entries was implemented
only in opportunistic manner. Improve the memory cleaning
of the SERVFAIL cache to be more aggressive, so it doesn't
consume a lot of memory in the case the server encounters
many SERVFAILs at once.
Closes#5025
Merge branch '5025-improve-badcache-cleaning' into 'main'
See merge request isc-projects/bind9!9760
The lame-ttl processing was overriden to be disabled in the config,
but the code related to the lame-ttl was still kept in the resolver
code. More importantly, the DNS_RESOLVER_BADCACHETTL() macro would
cause the entries in the resolver badcache to be always cached for at
least 30 seconds even if the lame-ttl would be set to 0.
Remove the dns_badcache code from the dns_resolver unit, so we save some
processing time and memory in the resolver code.
Instead of cleaning the dns_badcache opportunistically, add per-loop
LRU, so each thread-loop can clean the expired entries. This also
allows removal of the atomic operations as the badcache entries are now
immutable, instead of updating the badcache entry in place, the old
entry is now deleted from the hashtable and the LRU list, and the new
entry is inserted in the LRU.
Instead of various probing, just assume that IPv6 is universally available
and cleanup the various checks and defines that we have accumulated over
the years.
Merge branch 'ondrej/cleanup-IPv6-networking-support' into 'main'
See merge request isc-projects/bind9!9360
The code that listens on individual interfaces is now stable and doesn't
require any changes. The code that would bind to IPv6 wildcard address
and then use IPv6 pktinfo structure to get the source address is not
going to be completed, so it's better to just remove the dead cruft.
In 2024, it is reasonable to assume that IPv4 and IPv6 is always
available on a socket() level. We still keep the option to enable or
disable each IP version individually, as the routing might be broken or
undesirable for one of the versions.
The nslookup system test checks the count of resolved addresses in
the CNAME tests using a 'grep' match on the hostname, and ignoring
lines containing the 'canonical name' string. In order to protect
the check from intermittent failures like the 'address in use' warning
message, which then automatically resolves after a retry, edit the
'grep' matching string to also ignore the comments (as the mentioned
warning message is a comment which contains the hostname).
Closes#4948
Merge branch '4948-nslookup-test-fix' into 'main'
See merge request isc-projects/bind9!9523
The nslookup system test checks the count of resolved addresses in
the CNAME tests using a 'grep' match on the hostname, and ignoring
lines containing the 'canonical name' string. In order to protect
the check from intermittent failures like the 'address in use' warning
message, which then automatically resolves after a retry, edit the
'grep' matching string to also ignore the comments (as the mentioned
warning message is a comment which contains the hostname).
There was a data race dns_validator_cancel() was called when the
offloaded operations were in progress. Make dns_validator_cancel()
respect the data ownership and only set new .canceling variable when
the offloaded operations are in progress. The cancel operation would
then finish when the offloaded work passes the ownership back to the
respective thread.
Closes#4926
Merge branch '4926-fix-data-race-in-dns_validator' into 'main'
See merge request isc-projects/bind9!9470
There was a data race dns_validator_cancel() was called when the
offloaded operations were in progress. Make dns_validator_cancel()
respect the data ownership and only set new .shuttingdown variable when
the offloaded operations are in progress. The cancel operation would
then finish when the offloaded work passes the ownership back to the
respective thread.
In some cases (there is evidence only when XoT was used) `named` failed
to try the next primary server in the list when the previous one was
marked as unreachable. This has been fixed.
Closes#5038
Merge branch '5038-xfr-primary-next-fix' into 'main'
See merge request isc-projects/bind9!9781
Add test cases which check that when a XoT primary server is
unreachable or is already marked as unreachble then the next
primary server in the list is used.
Previously a ISC_R_CANCELED result code switch-case has been added to
the zone.c:zone_xfrdone() function, which did two things:
1. Schedule a new zone transfer if there's a scheduled force reload of
the zone.
2. Reset the primaries list.
This proved to be not a well-thought change and causes problems,
because the ISC_R_CANCELED code is used not only when the whole transfer
is canceled, but also when, for example, a particular primary server is
unreachable, and named still needs to continue the transfer process by
trying the next server, which it now no longer does in some cases. To
solve this issue, three changes are made:
1. Make sure dns_zone_refresh() runs on the zone's loop, so that the
sequential calls of dns_zone_stopxfr() and dns_zone_forcexfr()
functions (like done in 'rndc retransfer -force') run in intended
order and don't race with each other.
2. Since starting the new transfer is now guaranteed to run after the
previous transfer is shut down (see the previous change), remove the
special handling of the ISC_R_CANCELED case, and let the default
handler to handle it like before. This will bring back the ability to
try the next primary if the current one was interrupted with a
ISC_R_CANCELED result code.
3. Change the xfrin.c:xfrin_shutdown() function to pass the
ISC_R_SHUTTINGDOWN result code instead of ISC_R_CANCELED, as it makes
more sense.
Remove special code which creates default listeners, and use the normal named.conf configuration parser instead. This removes unneeded code and makes the built-in configuration text provide a true primary source of defaults. This change should be transparent to end-users and should not cause any visible change.
Closes#1424
Merge branch '1424-listen-builtin-config' into 'main'
See merge request isc-projects/bind9!2663
Stop using ns_listenlist_default() to set the default listen-on
and listen-on-v6 configuration. Instead, configure these options
using the default values in config.c.
The DLZ modules are poorly maintained as we only ensure they can still
be compiled, the DLZ interface is blocking, so anything that blocks the
query to the database blocks the whole server and they should not be
used except in testing. The DLZ interface itself is going to be scheduled
for removal.
The DLZ modules now live in https://gitlab.isc.org/isc-projects/dlz-modules
repository.
Closes#4865
Merge branch '4865-remove-contributed-DLZ-modules' into 'main'
See merge request isc-projects/bind9!9349
The DLZ modules are poorly maintained as we only ensure they can still
be compiled, the DLZ interface is blocking, so anything that blocks the
query to the database blocks the whole server and they should not be
used except in testing. The DLZ interface itself should be scheduled
for removal.
Add a new 'crypto' log module that will be used for a low-level
cryptographic operations. The DNS related cryptography logs
are still logged in the 'dns/crypto' module.
Merge branch 'ondrej/add-ISC_LOGMODULE_CRYPTO' into 'main'
See merge request isc-projects/bind9!9287
The libisc now includes sizeable chunks of cryptography, but the crypto
log module was missing. Add the new ISC_LOGMODULE_CRYPTO to libisc and
use it in the isc_tls error logging.
Some tests may leave artifacts in the .libs directory. Ignore this
directory when detecting expected artifacts.
Closes#5055
Merge branch '5055-ignore-libs-artifacts' into 'main'
See merge request isc-projects/bind9!9766
Add a none parameter to named configuration option `query-source` (respectively `query-source-v6`) which forbid usage of IPv4 (respectively IPv6) addresses when named is doing an upstream query.
Closes#4981 Turning-off upstream IPv6 queries while still listening to downstream queries on IPv6.
Merge branch 'colin/querysource-none' into 'main'
See merge request isc-projects/bind9!9727
This change adds a "none" parameter to the query-source[-v6]
options in named.conf, which forbid the usage of IPv4 or IPv6
addresses when doing upstream queries.
The new log message is emitted when adding or updating an RRset
fails due to exceeding the max-records-per-type limit. The log includes
the owner name and type, corresponding zone name, and the limit value.
It will be emitted on loading a zone file, inbound zone transfer
(both AXFR and IXFR), handling a DDNS update, or updating a cache DB.
It's especially helpful in the case of zone transfer, since the
secondary side doesn't have direct access to the offending zone data.
It could also be used for max-types-per-name, but this change
doesn't implement it yet as it's much less likely to happen
in practice.
Merge branch 'helpful-log-on-toomanyrecords' into 'main'
See merge request isc-projects/bind9!9509
DNS_LOGMODULE_RBTDB was simply inappropriate, and this
log message is actually dependent on db implementation
details, so DNS_LOGMODULE_DB would be the best choice.
The new log message is emitted when adding or updating an RRset
fails due to exceeding the max-records-per-type limit. The log includes
the owner name and type, corresponding zone name, and the limit value.
It will be emitted on loading a zone file, inbound zone transfer
(both AXFR and IXFR), handling a DDNS update, or updating a cache DB.
It's especially helpful in the case of zone transfer, since the
secondary side doesn't have direct access to the offending zone data.
It could also be used for max-types-per-name, but this change
doesn't implement it yet as it's much less likely to happen
in practice.
`dig` fails to parse a valid (as far as I can tell, and accepted by `kdig` and `Wireshark`) `SVCB` record with a `dohpath` URI template containing a `{&dns}`, like `dohpath=/some/path?key=value{&dns}"`. If the URI template contains a `{?dns}` instead `dig` is happy, but my understanding of rfc9461 and section 1.2. "Levels and Expression Types" of rfc6570 is that `{&dns}` is valid.
See for example section 1.2. "Levels and Expression Types" of rfc6570.
Note that Peter van Dijk suggested that `{dns}` and `{dns,someothervar}` might be valid forms as well, so my patch might be too restrictive, although it's anyone's guess how DoH clients would handle complex templates.
Closes https://gitlab.isc.org/isc-projects/bind9/-/issues/4922
Merge branch 'svcb-dohpath-uri-template' into 'main'
See merge request isc-projects/bind9!9455
The 'dns' variable in dohpath can be in various forms ({?dns},
{dns}, {&dns} etc.). To check for a valid dohpath it ends up
being simpler to just parse the URI template rather than looking
for all the various forms if substring.
by logging SSL_CTX_use_certificate_chain_file and SSL_CTX_use_PrivateKey_file errors individually.
Closes#5008
Merge branch '5008-provide-more-visibility-into-ssl-errors' into 'main'
See merge request isc-projects/bind9!9683
On some slow systems, the test might intermittently fail due to inherent
timing issues. In our CI, this most often happens in the
system:gcc:8fips:amd64 jobs.
Closes#3098
Merge branch '3098-allow-re-run-of-mkeys-test' into 'main'
See merge request isc-projects/bind9!9732
On some slow systems, the test might intermittently fail due to inherent
timing issues. In our CI, this most often happens in the
system:gcc:8fips:amd64 jobs.
The cross-version-config-tests job has never functioned in CI because
the testing framework changed after the testing was completed. To run
the new "named" binary using the old configurations, paths in the test
framework must be updated to point to the location of the new binaries.
Closes#4977
Merge branch '4977-fix-cross-version-config-tests' into 'main'
See merge request isc-projects/bind9!9702
The cross-version-config-tests job has never functioned in CI because
the testing framework changed after the testing was completed. To run
the new "named" binary using the old configurations, paths in the test
framework must be updated to point to the location of the new binaries.
Add query counters for DoT, DoH, unencrypted DoH and their proxied
counterparts. The new protocols do not update their respective TCP/UDP
transport counter and is now for TCP/UDP over plain 53 only.
Closes#598
Merge branch '598-wishlist-statistics-for-dns-over-tcp-and-tls' into 'main'
See merge request isc-projects/bind9!9585
Add query counters for DoT, DoH, unencrypted DoH and their proxied
counterparts. The protocols don't increment TCP/UDP counters anymore
since they aren't the same as plain DNS-over-53.
The usage of port and tls arguments in *-source and *-source-v6 named configuration options has been previously removed. Remove various configuration check deprecating usage of those arguments.
Merge branch 'colin/querysource-check-cleanup' into 'main'
See merge request isc-projects/bind9!9738
The usage of port and tls arguments in *-source and *-source-v6 named
configuration options has been previously removed. Remove
configuration check deprecating usage of those arguments.
Reintroduce logic to apply diffs when the number of pending tuples is
above 128. The previous strategy of accumulating all the tuples and
pushing them at the end leads to excessive memory consumption during
transfer.
This effectively reverts half of e3892805d6Closes#4986
Merge branch '4986-axfr-chunked-insert' into 'main'
See merge request isc-projects/bind9!9740
Reintroduce logic to apply diffs when the number of pending tuples is
above 128. The previous strategy of accumulating all the tuples and
pushing them at the end leads to excessive memory consumption during
transfer.
This effectively reverts half of e3892805d6
Fix alpine build by removing the need for `LargestIntegralType` use in `tests/isc/time_test.c`.
Merge branch 'alessio/fix-time-test' into 'main'
See merge request isc-projects/bind9!9758
Avoids using functions that require LargestIntegralType arguments in
time_test to resolve import issues on Alpine Linux. Using size_t instead
wasn't an option due to compatibility issues with 32-bit architectures.
'dig -F' and 'delv -F' can now be used to select FIPS mode at runtime.
Closes#5046
Merge branch '5046-enable-runtime-selection-of-fips-mode-in-dig' into 'main'
See merge request isc-projects/bind9!9754
To enable FIPS mode at runtime use 'delv -F'. The operating system
must have FIPS support available for this to succeed. This is on
by default when built with --enable-fips-mode.
To enable FIPS mode at runtime use 'dig -F'. The operating system
must have FIPS support available for this to succeed. This is on
by default when built with --enable-fips-mode.
Fix a unit test build failure on the i686 platform.
Merge branch 'bug/main/unittest-isc-time-32b' into 'main'
See merge request isc-projects/bind9!9753
Fixes unittest build failure on i686 platform.
In file included from tests/isc/time_test.c:24:
tests/isc/time_test.c: In function ‘run_test_isc_time_formatISO8601TZms_test’:
tests/isc/time_test.c:284:32: error: passing argument 2 of ‘_assert_in_set’ from incompatible pointer type [-Wincompatible-pointer-types]
284 | assert_in_set(buf[23], plus_minus, sizeof(plus_minus));
| ^~~~~~~~~~
| |
| size_t * {aka unsigned int *}
/usr/include/cmocka.h:2369:64: note: expected ‘const long long unsigned int *’ but argument is of type ‘size_t *’ {aka ‘unsigned int *’}
2369 | const LargestIntegralType value, const LargestIntegralType values[],
Re-split format strings that had been poorly split by multiple
clang-format runs using different versions of clang-format.
Closes#5043
Merge branch '5043-re-split-format-strings-lib-ns-update-c' into 'main'
See merge request isc-projects/bind9!9752
* Add new clang-format option to remove redundant semicolons
* Add new clang-format option to remove redundant parentheses
Merge branch 'ondrej/code-style-2024' into 'main'
See merge request isc-projects/bind9!9749
Add an 'initial-ds' entry to bind.keys for the new root key, ID
38696, which is scheduled for publication in January 2025.
Closes#4896
Merge branch '4896-update-bind-keys' into 'main'
See merge request isc-projects/bind9!9422
Keeping the Known Issues as part of the rendered docs has the issue that
the list can't be updated on the official docs website until the next
release. This is unpractical is a high-priority issue is discovered
shortly after a release. Keep the Known Issues in wiki and simply link
to the list from the rendered docs. The wiki article can be updated at
any time as needed.
Two `REQUIRE` assertions were accidentally deleted by the
dbf230650f commit earlier.
Bring them back.
Merge branch 'aram/dns_xfrin_create-bring-back-requires' into 'main'
See merge request isc-projects/bind9!9729
When canceling the ADB find, the lock on the find gets released for
a brief period of time to be locked again inside adbname lock. During
the brief period that the ADB find is unlocked, it can get canceled by
other means removing it from the adbname list which in turn causes
assertion failure due to a double removal from the adbname list.
This has been fixed.
Closes#5024
Merge branch '5024-fix-crash-in-dns_adb_cancelfind' into 'main'
See merge request isc-projects/bind9!9722
When canceling the ADB find, the lock on the find gets released for
a brief period of time to be locked again inside adbname lock. During
the brief period that the ADB find is unlocked, it can get canceled by
other means removing it from the adbname list which in turn causes
assertion failure due to a double removal from the adbname list.
Recheck if the find->adbname is still valid after acquiring the lock
again and if not just skip the double removal. Additionally, attach to
the adbname as in the worst case, the adbname might also cease to exist
if the scheduler would block this particular thread for a longer period
of time invalidating the lock we are going to acquire and release.
Some omissions of !9426 discovered during the backports
Merge branch 'nicki/extra-artifacts-fixups' into 'main'
See merge request isc-projects/bind9!9739
When a test is re-run by the flaky plugin, the TestReport outcomes
collected in the pytest_runtest_makereport() hook should be overriden.
Each of the setup/call/teardown phases is reported again and since we
care about the overall outcome, their respective results should be
overriden so that only the outcome from the final test (re)run gets
reported.
Prior to this change, it lead to a situation where an extra_artifact
generated during the test might be ignored. This was caused because the
check was skipped, since the test was incorrectly considered as "failed"
in the case where the test would fail on the first run, but pass on a
subsequent flaky rerun.
Remove the RBTDB database implementation, and only leave the QPDB
based implementations of zone and cache databases. This means
it's no longer possible to choose the RBTDB to be default at the
compilation time and it's not possible to configure RBTDB as the
database backend in the configuration file.
Closes#5027
Merge branch '5027-remove-rbtdb-implementation' into 'main'
See merge request isc-projects/bind9!9733
QPDB is now a default implementation for both cache and zone. Remove
the venerable RBTDB database implementation, so we can fast-track the
changes to the database without having to implement the design changes
to both QPDB and RBTDB and this allows us to be more aggressive when
refactoring the database design.
``clean.sh`` scripts have been replaced by lists of expected artifacts for each system test module. The list is defined using the custom ``pytest.mark.extra_artifacts`` mark, which can use both filenames and globs.
Closes#4261
Merge branch '4261-add-pytest-fixture-checking-test-artifacts' into 'main'
See merge request isc-projects/bind9!9426
Prior to introducing the pytest runner, clean.sh files were used as a
list of files that the test is expected to leave around as artifacts and
check that no extra files were created.
With the pytest runner, those scripts are no longer used, but the
ability to detect extraneous files is still useful. Add a new
"extra_artifacts" mark which can be used for the same purpose.
There is a data race between the statistics channel, which uses
`dns_zone_getxfr()` to get a reference to `zone->xfr`, and the creation
of `zone->xfr`, because the latter happens outside of a zone lock.
Split the `dns_xfrin_create()` function into two parts to separate the
zone transfer starting part from the zone transfer object creation part.
This allows us to attach the new object to a local variable first, then
attach it to `zone->xfr` under a lock, and only then start the transfer.
Closes#5011
Merge branch '5011-dns_zone_getxfr-race-fix' into 'main'
See merge request isc-projects/bind9!9716
There is a data race between the statistics channel, which uses
`dns_zone_getxfr()` to get a reference to `zone->xfr`, and the creation
of `zone->xfr`, because the latter happens outside of a zone lock.
Split the `dns_xfrin_create()` function into two parts to separate the
zone tranfer startring part from the zone transfer object creation part.
This allows us to attach the new object to a local variable first, then
attach it to `zone->xfr` under a lock, and only then start the transfer.
Originally, the dns_dbversion_t was typedef'ed to void type. This
allowed some flexibility, but using (void *) just removes any
type-checking that C might have. Instead of using:
typedef void dns_dbversion_t;
use a trick to define the type to non-existing structure:
typedef struct dns_dbversion dns_dbversion_t;
This allows the C compilers to employ the type-checking while the
structure itself doesn't have to be ever defined because the actual
'storage' is never accessed using dns_dbversion_t type.
Merge branch 'ondrej/non-void-dns_dbversion_t' into 'main'
See merge request isc-projects/bind9!9724
Originally, the dns_dbversion_t was typedef'ed to void type. This
allowed some flexibility, but using (void *) just removes any
type-checking that C might have. Instead of using:
typedef void dns_dbversion_t;
use a trick to define the type to non-existing structure:
typedef struct dns_dbversion dns_dbversion_t;
This allows the C compilers to employ the type-checking while the
structure itself doesn't have to be ever defined because the actual
'storage' is never accessed using dns_dbversion_t type.
Remove named_g_sessionkey and named_g_sessionkeyname as they are declared and cleaned up but otherwise are unused.
Closes#5023
Merge branch '5023-remove-named_g_sessionkey-as-it-is-unused' into 'main'
See merge request isc-projects/bind9!9720
Originally, the dns_dbnode_t was typedef'ed to void type. This allowed
some flexibility, but using (void *) just removes any type-checking that
C might have. Instead of using:
typedef void dns_dbnode_t;
use a trick to define the type to non-existing structure:
typedef struct dns_dbnode dns_dbnode_t;
This allows the C compilers to employ the type-checking while the
structure itself doesn't have to be ever defined because the actual
'storage' is never accessed using dns_dbnode_t type.
Merge branch 'ondrej/non-void-dns_dbnode_t' into 'main'
See merge request isc-projects/bind9!9719
Originally, the dns_dbnode_t was typedef'ed to void type. This allowed
some flexibility, but using (void *) just removes any type-checking that
C might have. Instead of using:
typedef void dns_dbnode_t;
use a trick to define the type to non-existing structure:
typedef struct dns_dbnode dns_dbnode_t;
This allows the C compilers to employ the type-checking while the
structure itself doesn't have to be ever defined because the actual
'storage' is never accessed using dns_dbnode_t type.
The query-source option currently allows the address to be specified in two ways, either as every other X-source option, or as an
"address" key-value pair.
This merge request extends the `parse_sockaddrsub` config parsing function so that it can parse the query-source option. It also removes the separate config parsing function for `query-source`.
Closes#4961
Merge branch '4961-query-source-parsing-refactor' into 'main'
See merge request isc-projects/bind9!9551
The query-source option has the slight quirk of allowing the address to
be specified in two ways, either as every other source option, or as an
"address" key-value pair.
For this reason, it had a separate parsing function from other X-source
options, but it is possible to extend the parsing of other X-sources to
be generic and also handle query-source.
This commit just does that.
The isc/crypto.h now directly includes the OpenSSL headers (evp.h) and
any application that includes that header also needs to have
OPENSSL_CFLAGS in the Makefile.am. Adjust the required automake files
as needed.
Merge branch 'ondrej/add-missing-OPENSSL_CFLAGS' into 'main'
See merge request isc-projects/bind9!9713
The isc/crypto.h now directly includes the OpenSSL headers (evp.h) and
any application that includes that header also needs to have
OPENSSL_CFLAGS in the Makefile.am. Adjust the required automake files
as needed.
The zone transfer summary will now print the expire option value in the zone transfer summary.
Closes#5013
Merge branch '5013-print-expire-option-in-transfer-summary' into 'main'
See merge request isc-projects/bind9!9694
Add missing checks for `$FEATURETEST --have-fips-dh` in notify system test to match those in setup.sh.
Closes#5015
Merge branch '5015-tls-notify-checks-fail-on-ol-8-fips' into 'main'
See merge request isc-projects/bind9!9707
The tool 'dnssec-ksr' now allows for KSK generation, as well as planned KSK rollovers. When signing a bundle from a Key Signing Request (KSR), only the key that is active in that time frame is being used for signing. Also, the CDS and CDNSKEY records are now added and removed at the correct time.
Closes#4697Closes#4705
Merge branch '4705-dnssec-ksr-only-sign-with-active-ksks' into 'main'
See merge request isc-projects/bind9!9452
dnssec-ksr can now sign KSR files with multiple KSKs. A planned KSK
rollover is supported, meaning the KSR will first be signed with
one KSK and later with another. The timing metadata for CDS and
CDNSKEY records are also taken into account, so these records are
only published when the time is between "SyncPublish" and "SyncDelete".
Add a test case for Offline KSK where during the lifespan of the Signed
Key Response a KSK rollover happens. Ensure that the correct DNSKEY,
CDNSKEY, and CDS records are published at the right times.
When the zone is initially signed, the CDNSKEY/CDS RRset is not
immediately published. The DNSKEY and signatures must propagate first.
Adjust the test to allow for this case.
Add an option to dnssec-ksr keygen, -o, to create KSKs instead of ZSKs.
This way, we can create a set of KSKS for a given period too.
For KSKs we also need to set timing metadata, including "SyncPublish"
and "SyncDelete". This functionality already exists in keymgr.c so
let's make the function accessible.
Replace dnssec-keygen calls with dnssec-ksr keygen for KSK in the
ksr system test and check keys for created KSKs as well. This requires
a slight modification of the check_keys function to take into account
KSK timings and metadata.
In the ksr system test, the 'test_ksr_twotone' case may fail if there are two keys with the same keytag (but different algorithms), because one key is expected to be signing and the other is not.
Switch to regular expression matching and include the algorithm in the search string.
Closes#5017
Merge branch '5017-unexpected-match-ksr-twotone-again' into 'main'
See merge request isc-projects/bind9!9701
In the ksr system test, the test_ksr_twotone case may fail if there
are two keys with the same keytag (but different algorithms), because
one key is expected to be signing and the other is not.
Switch to regular expression matching and include the algorithm in the
search string.
With the introduction of the generated changelog, the CHANGES file
became a symlink to doc/arm/changelog.rst. After the changes made in
!9549, the changelog file transitioned from being a wholly generated
file to one that includes versioned changelog files, which are
themselves generated. However, while implementing !9549, we overlooked
that the CHANGES file is copied to a release directory on an FTP server
and contains just "include" directives, not the changelog itself.
Therefore, in the same fashion as the "RELEASE-NOTES*.html" file, create
a "CHANGELOG*.html" file that redirects to the Changelog appendix of the
ARM.
Closes#5000
Merge branch '5000-provide-correct-changelog-on-ftp' into 'main'
See merge request isc-projects/bind9!9690
With the introduction of the generated changelog, the CHANGES file
became a symlink to doc/arm/changelog.rst. After the changes made in
!9549, the changelog file transitioned from being a wholly generated
file to one that includes versioned changelog files, which are
themselves generated. However, while implementing !9549, we overlooked
that the CHANGES file is copied to a release directory on an FTP server
and contains just "include" directives, not the changelog itself.
Therefore, in the same fashion as the "RELEASE-NOTES*.html" file, create
a "CHANGELOG*.html" file that redirects to the Changelog appendix of the
ARM.
Configuration files in system tests which require some variables (e.g.
port numbers) filled in during test setup, can now use jinja2 templates
when `jinja2` python package is available.
Any `*.j2` file found within the system test directory will be
automatically rendered with the environment variables into a file
without the `.j2` extension by the pytest runner. E.g.
`ns1/named.conf.j2` will become `ns1/named.conf` during test setup. To
avoid automatic rendering, use `.j2.manual` extension and render the
files manually at test time.
New `templates` pytest fixture has been added. Its `render()` function
can be used to render a template with custom test variables. This can be
useful to fill in different config options during the test. With
advanced jinja2 template syntax, it can also be used to include/omit
entire sections of the config file rather than using `named1.conf.in`,
`named2.conf.in` etc.
Closes#4938
Merge branch '4938-use-jinja2-templates-in-system-tests' into 'main'
See merge request isc-projects/bind9!9587
Configuration files in system tests which require some variables (e.g.
port numbers) filled in during test setup, can now use jinja2 templates
when `jinja2` python package is available.
Any `*.j2` file found within the system test directory will be
automatically rendered with the environment variables into a file
without the `.j2` extension by the pytest runner. E.g.
`ns1/named.conf.j2` will become `ns1/named.conf` during test setup. To
avoid automatic rendering, use `.j2.manual` extension and render the
files manually at test time.
New `templates` pytest fixture has been added. Its `render()` function
can be used to render a template with custom test variables. This can be
useful to fill in different config options during the test. With
advanced jinja2 template syntax, it can also be used to include/omit
entire sections of the config file rather than using `named1.conf.in`,
`named2.conf.in` etc.
The `Report-Channel` and `ZONEVERSION` EDNS options can now be sent using `dig +ednsopt=report-channel` (or `dig +ednsopt=rc` for short), and `dig +ednsopt=zoneversion`.
Several other EDNS option names, including `DAU`, `DHU`, `N3U`, and `CHAIN`, are now displayed correctly in text and YAML formats. Also, an inconsistency has been corrected: the `TCP-KEEPALIVE` option is now spelled with a hyphen in both text and YAML formats; previously, text format used a space.
Merge branch 'each-add-missing-edns-opts' into 'main'
See merge request isc-projects/bind9!9691
some EDNS option names, including DAU, DHU, N3U, and CHAIN,
were not printed in dns_message_pseudosectiontotext() or
_psuedosectiontoyaml(); they were displayed as unknown options.
this has been corrected.
that code was also refactored to use switch instead of if/else,
and to look up the option code names in a table to prevent
inconsistencies between the two formats. one such inconsistency
was corrected: the "TCP-KEEPALIVE" option is now always printed
with a hyphen, instead of being "TCP KEEPALIVE" when not using
YAML. the keepalive system test has been updated to expect this.
EDNS options that print DNS names (i.e., CHAIN and Report-Channel)
now enclose them in quotation marks to ensure YAML correctness.
the auth system test has been updated to expect this when grepping
for Report-Channel options.
Report-Channel and ZONEVERSION EDNS options can now be sent
using `dig +ednsopt=report-channel` (or `dig +ednsopt=rc` for
short), and `dig +ednsopt=zoneversion`.
Use a stricter hazard check which ensures the audience tag is present in
the MR title and is one of the known values. This prevents siuations
where incorrect audience is accidentally used, resulting in a missing
changelog entry or a release note.
Coverity Scan reported a new issue for the ksr system test. There is allegedly a null pointer dereference (FORWARD_NULL) in check_keys().
This popped up because previously we set 'retired' to 0 in case of unlimited lifetime, but we changed it to None.
It is actually a false positive, because if lifetime is unlimited there will be only one key in 'keys'.
However, the code would be better if we always initialized 'active' and if it is not the first key and retired is set, set the successor key's active time to the retire time of the predecessor key.
Closes#5004
Merge branch '5004-cid-510858-ksr-check-keys' into 'main'
See merge request isc-projects/bind9!9687
Coverity Scan reported a new issue for the ksr system test. There
is allegedly a null pointer dereference (FORWARD_NULL) in check_keys().
This popped up because previously we set 'retired' to 0 in case of
unlimited lifetime, but we changed it to None.
It is actually a false positive, because if lifetime is unlimited
there will be only one key in 'keys'.
However, the code would be better if we always initialized 'active'
and if it is not the first key and retired is set, set the successor
key's active time to the retire time of the predecessor key.
A test may fail if the key id is shorter than 5 digits. Add a leading space to the expected strings which start with the key tag to avoid the issue.
Closes#5002
Merge branch '5002-unexpected-match-ksr-twotwone' into 'main'
See merge request isc-projects/bind9!9688
The test_ksr_twotwone may fail if the key id is shorter than 5 digits.
Add a leading space to the expected strings which start with the key
tag to avoid the issue.
Make system tests symlinks and logged test names consistent across pytest versions.
Merge branch 'nicki/pytest-v8-compat' into 'main'
See merge request isc-projects/bind9!9071
The pytest collection mechanism has been overhauled in pytest 8.0.0,
resulting in a different node tree when collecting the tests. Ensure the
paths / names we're using that are derived from the node tree are
consistent across different pytest versions.
Particularly, this has affected the convenience symlink name (which is
supposed to be in the form of e.g. dns64_sh_dns64 for the dns64 module
and tests_sh_dns64.py module) and the test name that's logged at the
start of the test, which is supposed to include the system test
directory relative to the root system test directory as well as the
module name (e.g. dns64/tests_sh_dns64.py).
Related https://github.com/pytest-dev/pytest/issues/7777
Notifies configured to use TLS will now be sent over TLS, instead of plaintext UDP or TCP.
Also, failing to load the TLS configuration for notify now also results in an error.
Closes#4821
Merge branch '4821-notify-over-tls' into 'main'
See merge request isc-projects/bind9!9407
This allows for dispatch to use existing TCP/HTTPS/TLS etc. streams without accidentally using an unexpected transport.
Closes#4989
Merge branch '4989-fix-transport-use-with-dispatch' into 'main'
See merge request isc-projects/bind9!9633
Dispatch needs to know the transport that is being used over the
TCP connection to correctly allow for it to be reused. Add a
transport parameter to dns_dispatch_createtcp and dns_dispatch_gettcp
and use it when selecting a TCP socket for reuse.
Add new `send-report-channel` and `log-report-channel` options. `send-report-channel` specifies an agent domain, to which error reports can be sent by querying a specially constructed name within the agent domain. EDNS Report-Channel options will be added to outgoing authoritative responses, to inform clients where to send such queries in the event of a problem.
If a zone is configured which matches the agent domain and has `log-report-channel` set to `yes`, error-reporting queries will be logged at level `info` to the `dns-reporting-agent` logging channel.
Closes#3659
Merge branch '3659-implement-draft-ietf-dnsop-dns-error-reporting' into 'main'
See merge request isc-projects/bind9!7036
RFC 9567 section 8.1 specifies that the agent domain cannot
be a subdomain of the domain it is reporting on. therefore,
in addition to making it illegal to configure that at the
zone level, we also need to disable send-report-channel for
any zone for which the global send-report-channel value is
a subdomain.
we also now warn if send-report-channel is configured
globally to a zone that we host, but that zone doesn't
have log-report-channel set.
the logging of error-report queries is no longer activated by
the view's "send-report-channel" option; that now only configures
the agent-domain value that is to be sent in authoritative
responses. the warning that was logged when "send-agent-domain"
was set to a value that is not a locally configured zone has
been removed.
error-report logging is now activated by the presence of an
authoritative zone with the "log-report-channel" option set to
"yes". this is not permitted in the root zone.
NOTE: a zone with "log-report-channel yes;" should contain a
"*._er" wildcard, but that requirement is not yet enforced.
add a boolean "log-report-channel" option for primary and
secondary zones, which sets the DNS_ZONEOPT_LOGREPORTS zone
flag. this option is not yet functional.
If send-report-channel is set at the zone level, it will
be stored in the zone object and used instead of the
view-level agent-domain when constructing the EDNS
Report-Channel option.
This commit adds support for the EDNS Report-Channel option,
which is returned in authoritative responses when EDNS is in use.
"send-report-channel" sets the Agent-Domain value that will be
included in EDNS Report-Channel options. This is configurable at
the options/view level; the value is a DNS name. Setting the
Agent-Domain to the root zone (".") disables the option.
When this value has been set, incoming queries matchng the form
_er.<qtype>.<qname>.<extended-error-code>._er.<agent-domain>/TXT
will be logged to the dns-reporting-agent channel at INFO level.
(Note: error reporting queries will only be accepted if sent via
TCP or with a good server cookie. If neither is present, named
returns BADCOOKIE to complete the DNS COOKIE handshake, or TC=1
to switch the client to TCP.)
This logs how named determines if an update request is granted or denied when using update-policy.
Closes#4751
Merge branch '4751-provide-debug-level-logging-of-update-policy' into 'main'
See merge request isc-projects/bind9!9074
These are logged to the update category at debug level 99 and
have the following form.
update-policy: using: signer=ddns-key.example.nil, name=updated.example.nil, addr=10.53.0.1, tcp=0, type=A, target=
update-policy: trying: grant zonesub-key.example.nil zonesub TXT
update-policy: next rule: signer does not match identity
update-policy: trying: grant ddns-key.example.nil zonesub ANY
update-policy: matched: grant ddns-key.example.nil zonesub ANY
or
update-policy: using: signer=restricted.example.nil, name=example.nil, addr=10.53.0.1, tcp=0, type=TXT, target=
update-policy: trying: grant zonesub-key.example.nil zonesub TXT
update-policy: next rule: signer does not match identity
update-policy: trying: grant ddns-key.example.nil zonesub ANY
update-policy: next rule: signer does not match identity
update-policy: trying: grant restricted.example.nil zonesub ANY
update-policy: next rule: name/subdomain mismatch
update-policy: no match found
where 'using:' is the calling parameters of dns_ssutable_checkrules,
'trying:' in the rule bing evaluated, "next rule:" is the reason
the rule does not match, "matched:" repeats the matched rule, and
no match found is reported when te set of rules is exhausted.
When working with key timestamps, ensure we correctly set the UTC
timezone in order for the tests to work consistently regardless of the
local time setting.
Closes#4999
Merge branch '4999-pytest-kasp-use-utc-timezone' into 'main'
See merge request isc-projects/bind9!9673
Use a different timezone via the TZ variable in at least one of the
system test jobs in order to detect possible issues with timezone
handling in python.
When working with key timestamps, ensure we correctly set the UTC
timezone in order for the tests to work consistently regardless of the
local time setting.
When DSCP support was removed, parsing of hostnames in rndc.conf was accidentally broken, resulting in an assertion failure. This has been fixed.
Closes#4991
Merge branch '4991-rndc-fix-parsing-hostnames' into 'main'
See merge request isc-projects/bind9!9669
When DSCP was removed the parsing of hostnames was accidentally
broken resulting in an assertion failure. Call cfg_parse_tuple
rather than using custom code in parse_sockaddrnameport.
With Sphinx 8.1.0, footnotes can't stand on their own and have to be
referenced from somewhere, otherwise build fails, e.g.:
doc/dnssec-guide/signing.rst:1470: WARNING: Footnote [#] is not referenced. [ref.footnote]
Merge branch 'mnowak/fix-dnssec-guide-footnote' into 'main'
See merge request isc-projects/bind9!9663
With Sphinx 8.1.0, footnotes can't stand on their own and have to be
referenced from somewhere, otherwise build fails, e.g.:
doc/dnssec-guide/signing.rst:1470: WARNING: Footnote [#] is not referenced. [ref.footnote]
Options of the form `[+-]option=<value>` failed to display the value on the printed command line. This has been fixed.
Closes#4993
Merge branch '4993-dig-restore-command-line-values' into 'main'
See merge request isc-projects/bind9!9653
FIPS build of libisc failed from some missing definitions.
Closes#4994
Merge branch '4994-crypto-c-174-7-error-implicit-declaration-of-function-isc_fips_mode' into 'main'
See merge request isc-projects/bind9!9655
HTTP URI of doc/arm/requirements.txt on the "main" branch is the source
of PyPI packages in CI base image. To make it consistent in all
maintained CI branches on RTD, use the HTTP URI as well.
This change is non-material on the "main" branch, but ensures the right
packages on stable branches where for RTD we currently leverage outdated
packages because we failed to update doc/arm/requirements.txt there.
Merge branch 'mnowak/rtd-always-resource-requirements-txt-from-main' into 'main'
See merge request isc-projects/bind9!9630
HTTP URI of doc/arm/requirements.txt on the "main" branch is the source
of PyPI packages in CI base image. To make it consistent in all
maintained CI branches on RTD, use the HTTP URI as well.
This change is non-material on the "main" branch, but ensures the right
packages on stable branches where for RTD we currently leverage outdated
packages because we failed to update doc/arm/requirements.txt there.
With Sphinx 8.1.0, footnotes can't stand on their own and have
referenced from somewhere:
/builds/isc-projects/bind9/doc/arm/general.rst:439: WARNING: Footnote [#] is not referenced. [ref.footnote]
/builds/isc-projects/bind9/doc/arm/general.rst:441: WARNING: Footnote [#] is not referenced. [ref.footnote]
/builds/isc-projects/bind9/doc/arm/general.rst:445: WARNING: Footnote [#] is not referenced. [ref.footnote]
/builds/isc-projects/bind9/doc/arm/general.rst:457: WARNING: Footnote [#] is not referenced. [ref.footnote]
Enforcing pylint standards and default for our test code seems
counter-productive. Since most of the newly added code are tests or is
test-related, encountering these checks rarely make us refactor the code
in other ways and we just disable these checks individually. Code that
is too complex or convoluted will be pointed out in reviews anyways.
BIND 9.20.0 broke `recursive-clients 0;`. This has now been fixed.
Closes#4987
Merge branch '4987-fix-recursive-clients-0' into 'main'
See merge request isc-projects/bind9!9621
Move all test cases from tests.sh to tests_ksr.py. The only test that
is not moved is the check that key id's match expected keys. The
shell-based system test checks two earlier set environment variables
against each other that has become redundant in the pytest variant,
because we now check the signed key response against a list of keys
and for each key we take into account the timing metadata. So we
already ensure that each published key is in the correct key bundle.
Write initial pytest kasp library. This contains everything that is
required for testing Offline KSK functionality with pytest.
This includes:
- addtime: adding a value to a timing metadata
- get_timing_metdata: retrieve timing metadata from keyfile
- get_metadata/get_keystate: retrieve metadata from statefile
- get_keytag: retrieve keytag from base keyfile string
- get_keyrole: get key role from statefile
- dnskey_equals: compare DNSKEY record from file against a string
- cds_equals: compare CDS derived from file against a string
- zone_is_signed: wait until a zone is completely signed
- dnssec_verify: verify a DNSSEC signed zone with dnssec-verify
- check_dnssecstatus: check rndc dnssec -status output
- check_signatures: check that signatures for a given RRset are correct
- check_dnskeys: check that the published DNSKEY RRset is correct
- check_cds: check that the published CDS RRset is correct
- check_apex: check SOA, DNSKEY, CDNSKEY, and CDS RRset
- check_subdomain: check an RRset below the apex
Unify libcrypto initialization and explicit digest fetching in a single place.
It will remove the remaining implicit fetching and deduplicate explicit
fetching inside the codebase. Initialization has been moved in to ensure
OpenSSL cleanup is done only after fetched contextes are destroyed.
Merge branch 'aydin/libdns-explicit-fetch' into 'main'
See merge request isc-projects/bind9!9288
Unify libcrypto initialization and explicit digest fetching in a single
place and move relevant code to the isc__crypto namespace instead of
isc__tls.
It will remove the remaining implicit fetching and deduplicate explicit
fetching inside the codebase.
The <openssl/hmac.h> header was unused and including the
header might cause build failure when OpenSSL doesn't have
Engines support enabled.
See https://fedoraproject.org/wiki/Changes/OpensslDeprecateEngine
Removes unused hmac includes after Remove OpenSSL Engine support
(commit ef7aba70) removed engine support.
Merge branch 'main-openssl-engine' into 'main'
See merge request isc-projects/bind9!9228
The <openssl/hmac.h> header was unused and including the
header might cause build failure when OpenSSL doesn't have
Engines support enabled.
See https://fedoraproject.org/wiki/Changes/OpensslDeprecateEngine
Removes unused hmac includes after Remove OpenSSL Engine support
(commit ef7aba7072) removed engine
support.
The unit test doh_test tends do fail quite often due to exceeding run
time limit in the unit:clang:freebsd14:amd64 job. Use a retry on gitlab
level to alleviate the issue until a better fix is available.
Related #4924
Merge branch '4924-retry-doh_test-freebsd14' into 'main'
See merge request isc-projects/bind9!9578
The unit test doh_test tends do fail quite often due to exceeding run
time limit in the unit:clang:freebsd14:amd64 job. Use a retry on gitlab
level to alleviate the issue until a better fix is available.
CookieOption with new .server/.client attributes (rather than .data) was
added to dnspython. Adjust the code to use the new attributes if
available and fall back to the old code for dnspython<2.7.0
compatibility.
The performance improvement for finding the NSEC3 closest encloser when generating authoritative responses could cause servers to return incorrect NSEC3 records in some cases. This has been fixed.
Closes#4950
Merge branch '4950-bind-logs-expected-covering-nsec3-got-an-exact-match' into 'main'
See merge request isc-projects/bind9!9610
maxlabels is the suffix length that corresponds to the latest
NXDOMAIN response. minlabels is the suffix length that corresponds
to longest found existing name.
When `rndc recursing` is used to dump the list of recursing clients, it now indicates whether a query was sent via UDP, TCP, TLS, or HTTP.
Closes#4971
Merge branch '4971-recursing-show-client-transport' into 'main'
See merge request isc-projects/bind9!9590
there was no system test that exercised 'rndc recursing'; a
simple one has now been added; it confirms that the number of
recursing clients reported by 'rndc stats' is in agreement with
the list returned by 'rndc recursing'.
Otherwise the "statistics-channels" option in doc/misc/options and
doc/man/named.conf.5in is marked as "not configured" (contrary to what
we have in release tarballs as they were build on a different image that
has libjson-c and libxml2 in it).
Caused by #4895 that made the option dependant on libjson-c or libxml2
presence in the build image.
When running shotgun tests on tagged releases, the increased number of
jobs may cause the shotgun pipeline to take longer than 50 minutes to
finish.
Merge branch 'nicki/increase-shotgun-pipeline-timeout' into 'main'
See merge request isc-projects/bind9!9599
Prior to doing key management, BIND 9 will check if the key files on disk match the expected keys. If key files for previously observed keys have become unavailable, this will prevent the internal key manager from running.
Merge branch '4763-do-not-roll-if-key-files-are-missing' into 'main'
See merge request isc-projects/bind9!9337
In a multi-signer setup, removing DNSKEY records from the zone should
not be treated as a key that previously exists in the keyring, thus
blocking the keymgr. Add a test case to make sure.
Test that if a key to be purged is in the keyring, it does not
prevent the keymgr from running. Normally a key that is in the keyring
should be available again on the next run, but that is not true for
a key that can be purged.
In addition, fix some wait_for_log calls, by adding the missing
'|| ret=1' parts.
Some test cases were working but for the wrong reasons. These started
to fail when I implemented the first approach for #4763, where the
existence of a DNSKEY together with an empty keyring is suspicious and
would prevent the keymgr from running.
These are:
1. kasp: The multisigner-model2.kasp zone has ZSKs from other providers
in the zone, but not yet its own keys. Pregenerate signing keys and
add them to the unsigned zone as well.
2. kasp: The dynamic-signed-inline-signing.kasp zone has a key generated
and added in the raw version of the zone. But the key file is stored
outside the key-directory for the given zone. Add '-K keys' to the
dnssec-keygen command.
Prior to running the keymgr, first make sure that existing keys
are present in the new keylist. If not, treat this as an operational
error where the keys are made offline (temporarily), possibly unwanted.
In this specific case the key files are temporary unavailable, for
example because of an operator error, or a mount failure). In such
cases, BIND should not try to roll over these keys.
This commit ensures that the port is set before attempting a UDP
query. Before that a situation could appear when previous query have
completed over a different transport (that uses a dedicated port) and
then a UDP query will be attempted over the port of the previous
transport.
Closes: #4984.
Merge branch 'artem-debian-bug-1059582' into 'main'
See merge request isc-projects/bind9!9618
This commit ensures that the port is set before attempting a UDP
query. Before that a situation could appear when previous query have
completed over a different transport (that uses a dedicated port) and
then a UDP query will be attempted over the port of the previous
transport.
This release note was missing due to a malformed Merge Request title.
The text is not copied verbatim, but changed to something more release
note-like.
Merge branch '4460-add-missing-release-note' into 'main'
See merge request isc-projects/bind9!9598
This release note was missing due to a malformed Merge Request title.
The text is not copied verbatim, but changed to something more release
note-like.
Revert "fix: chg: Improve performance when looking for the closest encloser when returning NSEC3 proofs"
This reverts merge request !9436Closes#4950
Merge branch 'revert-78d48f7a' into 'main'
See merge request isc-projects/bind9!9611
In two places, after linking the client to the manager's
"recursing-clients" list using the check_recursionquota()
function, the query.c module fails to unlink it on error
paths. Fix the bugs by unlinking the client from the list.
Merge branch 'aram/unlink-recursing-clients-on-error-paths' into 'main'
See merge request isc-projects/bind9!9586
Rename check_recursionquota() to acquire_recursionquota(), and
implement a new function called release_recursionquota() to
reverse the action. It helps with decreasing code duplication.
In two places, after linking the client to the manager's
"recursing-clients" list using the check_recursionquota()
function, the query.c module fails to unlink it on error
paths. Fix the bugs by unlinking the client from the list.
Also make sure that unlinking happens before detaching the
client's handle, as it is the logically correct order, e.g.
in case if it's the last handle and ns__client_reset_cb()
can be called because of the detachment.
The dns_zone_getxfrintime() function fails to lock the zone before
accessing its 'xfrintime' structure member, which can cause a data
race between soa_query() and the statistics channel. Add the missing
locking/unlocking pair, like it's done in numerous other similar
functions.
Closes#4976
Merge branch '4976-zone-xfrintime-data-race-fix' into 'main'
See merge request isc-projects/bind9!9591
The dns_zone_getxfrintime() function fails to lock the zone before
accessing its 'xfrintime' structure member, which can cause a data
race between soa_query() and the statistics channel. Add the missing
locking/unlocking pair, like it's done in numerous other similar
functions.
The 'nodetach' member is a leftover from the times when non-zero
'stale-answer-client-timeout' values were supported, and currently
is always 'false'. Clean up the member and its usage.
Merge branch 'aram/cleanup-ns-client-nodetach' into 'main'
See merge request isc-projects/bind9!9592
The 'nodetach' member is a leftover from the times when non-zero
'stale-answer-client-timeout' values were supported, and currently
is always 'false'. Clean up the member and its usage.
The outgoing UDP sockets enabled `SO_REUSEADDR` that allows sharing of the UDP sockets, but with one big caveat - the socket that was opened the last would get all traffic. The dispatch code would ignore the invalid responses in the dns_dispatch, but this could lead to unexpected results.
Merge branch 'ondrej/fix-outgoing-UDP-port-selection' into 'main'
See merge request isc-projects/bind9!9569
Currently, the outgoing UDP sockets have enabled
SO_REUSEADDR (SO_REUSEPORT on BSDs) which allows multiple UDP sockets to
bind to the same address+port. There's one caveat though - only a
single (the last one) socket is going to receive all the incoming
traffic. This in turn could lead to incoming DNS message matching to
invalid dns_dispatch and getting dropped.
Disable setting the SO_REUSEADDR on the outgoing UDP sockets. This
needs to be done explicitly because `uv_udp_open()` silently enables the
option on the socket.
When matching the received Query ID in the TCP connection,
an invalid received Query ID can very rarely cause assertion
failure.
Closes#4952
Merge branch '4952-check-whether-QID-is-not-deleted' into 'main'
See merge request isc-projects/bind9!9580
When matching the TCP dispatch responses, we should skip the responses
that do not belong to our TCP connection. This can happen with faulty
upstream server that sends invalid QID back to us.
The dns_dispatch_add() function registers the 'resp' entry in
'disp->mgr->qids' hash table with 'resp->port' being 0, but in
tcp_recv_success(), when looking up an entry in the hash table
after a successfully received data the port is used, so if the
local port was set (i.e. it was not 0) it fails to find the
entry and results in an unexpected error.
Set the 'resp->port' to the given local port value extracted from
'disp->local'.
Closes#4969
Merge branch '4969-dispatch-tcp-source-port-bug-fix' into 'main'
See merge request isc-projects/bind9!9576
The dns_dispatch_add() function registers the 'resp' entry in
'disp->mgr->qids' hash table with 'resp->port' being 0, but in
tcp_recv_success(), when looking up an entry in the hash table
after a successfully received data the port is used, so if the
local port was set (i.e. it was not 0) it fails to find the
entry and results in an unexpected error.
Set the 'resp->port' to the given local port value extracted from
'disp->local'.
The configuration option `print-time` can now be set to `iso8601-tzinfo` in order to use the ISO 8601 timestamp with timezone information when logging. This is used as a default for `named -g`.
Closes#4963
Merge branch '4963-provide-timezone-information-in-log-timestamps' into 'main'
See merge request isc-projects/bind9!9563
This commit adds support for timestamps in iso8601 format with timezone
when logging. This is exposed through the iso8601-tzinfo printtime
suboption.
It also makes the new logging format the default for -g output,
hopefully removing the need for custom timestamp parsing in scripts.
The `dns.query.udp` and `dns.query.tcp` methods are [prone to timeouts](https://gitlab.isc.org/isc-projects/bind9/-/jobs/4785053); their `isctest.query` equivalents should be used in system tests instead.
Merge branch 'mnowak/convert-dns-query-udp-and-tcp-to-isctest-query' into 'main'
See merge request isc-projects/bind9!9560
This commit nulls all type fields for the clausedef lists that are
declared ancient, and removes the corresponding cfg_type_t and parsing
functions when they are found to be unused after the change.
Among others, it removes some leftovers from #1913.
Closes#4962
Merge branch '4962-null-clausedef-types-for-ancient-options' into 'main'
See merge request isc-projects/bind9!9552
This commit nulls all type fields for the clausedef lists that are
declared ancient, and removes the corresponding cfg_type_t and parsing
functions when they are found to be unused after the change.
When `sig-validity-interval` was obsoleted, the text that the signature validity interval should be multiples of the SOA expire interval was removed. Restore this text to the description of the `signatures-validity` option.
Closes#4951
Merge branch '4951-document-signatures-validity-soa-expire' into 'main'
See merge request isc-projects/bind9!9566
The example.com zone file given in the "Configurations and Zone Files"
chapter has an SOA expire of 3 weeks, which is not a multiple of
the default signatures-validity value. Adjust the SOA expire so that
it is much lower than the signatures-validity default.
When `sig-validity-interval` was obsoleted, the text that the signature
validity interval should be multiples of the SOA expire interval was
removed. Restore this text to the description of the
`signatures-validity` option.
Static-stub addresses and addresses from other sources were being
mixed together, resulting in static-stub queries going to addresses
not specified in the configuration, or alternatively, static-stub
addresses being used instead of the correct server addresses.
Closes#4850
Merge branch '4850-add-an-additional-class-of-names-to-adb' into 'main'
See merge request isc-projects/bind9!9314
Static-stub address and addresses from other sources where being
mixed together resulting in static-stub queries going to addresses
not specified in the configuration or alternatively static-stub
addresses being used instead of the real addresses.
As the relaxed memory ordering doesn't ensure any memory
synchronization, it is possible that the increment will succeed even
in the case when it should not - there is a race between
atomic_fetch_sub(..., acq_rel) and atomic_fetch_add(..., relaxed).
Only the result is consistent, but the previous value for both calls
could be same when both calls are executed at the same time.
Merge branch 'ondrej/use-release-memory-ordering-for-reference-counting' into 'main'
See merge request isc-projects/bind9!9460
As the relaxed memory ordering doesn't ensure any memory
synchronization, it is possible that the increment will succeed even
in the case when it should not - there is a race between
atomic_fetch_sub(..., acq_rel) and atomic_fetch_add(..., relaxed).
Only the result is consistent, but the previous value for both calls
could be same when both calls are executed at the same time.
2024-09-30 11:03:01 +02:00
1426 changed files with 33570 additions and 55686 deletions
# cross-testrun files as there is no need to use that feature in CI.
PYTEST_ADDOPTS:"-p no:cacheprovider"
# Default platforms to run "stress" tests on
BIND_STRESS_TEST_OS:linux
BIND_STRESS_TEST_ARCH:amd64
HYPOTHESIS_PROFILE:"ci"
# Some jobs may clean up the build artifacts unless this is set to 0.
CLEAN_BUILD_ARTIFACTS_ON_SUCCESS:1
default:
# Allow all running CI jobs to be automatically canceled when a new
# version of a branch is pushed.
@@ -123,8 +122,8 @@ stages:
# Alpine Linux
.alpine-3.20-amd64:&alpine_3_20_amd64_image
image:"$CI_REGISTRY_IMAGE:alpine-3.20-amd64"
.alpine-3.21-amd64:&alpine_3_21_amd64_image
image:"$CI_REGISTRY_IMAGE:alpine-3.21-amd64"
<<:*linux_amd64
# Oracle Linux
@@ -171,16 +170,16 @@ stages:
# Fedora
.tsan-fedora-40-amd64:&tsan_fedora_40_amd64_image
image:"$CI_REGISTRY_IMAGE:tsan-fedora-40-amd64"
.tsan-fedora-41-amd64:&tsan_fedora_41_amd64_image
image:"$CI_REGISTRY_IMAGE:tsan-fedora-41-amd64"
<<:*linux_amd64
.fedora-40-amd64:&fedora_40_amd64_image
image:"$CI_REGISTRY_IMAGE:fedora-40-amd64"
.fedora-41-amd64:&fedora_41_amd64_image
image:"$CI_REGISTRY_IMAGE:fedora-41-amd64"
<<:*linux_amd64
.fedora-40-arm64:&fedora_40_arm64_image
image:"$CI_REGISTRY_IMAGE:fedora-40-arm64"
.fedora-41-arm64:&fedora_41_arm64_image
image:"$CI_REGISTRY_IMAGE:fedora-41-arm64"
<<:*linux_arm64
# Ubuntu
@@ -206,15 +205,15 @@ stages:
### QCOW2 Image Templates
.freebsd-13-amd64:&freebsd_13_amd64_image
image:"freebsd-13.3-x86_64"
image:"freebsd-13.4-x86_64"
<<:*libvirt_amd64
.freebsd-14-amd64:&freebsd_14_amd64_image
image:"freebsd-14.1-x86_64"
image:"freebsd-14.2-x86_64"
<<:*libvirt_amd64
.openbsd-amd64:&openbsd_amd64_image
image:"openbsd-7.5-x86_64"
image:"openbsd-7.6-x86_64"
<<:*libvirt_amd64
### Job Templates
@@ -316,9 +315,7 @@ stages:
- *configure
- *check_readline_setup
- make -j${BUILD_PARALLEL_JOBS:-1} -k all V=1
- test -z "${BUILD_CONTRIB}" || for DIR in contrib/dlz/modules/*; do test -f "${DIR}/Makefile" && CFLAGS="${CFLAGS} -Werror" make -C "${DIR}"; done
- test -z "${RUN_MAKE_INSTALL}" || make DESTDIR="${INSTALL_PATH}" install
- test -z "${RUN_MAKE_INSTALL}" -o -z "${BUILD_CONTRIB}" || for DIR in contrib/dlz/modules/*; do test -f "${DIR}/Makefile" && make -C "${DIR}" DESTDIR="${INSTALL_PATH}" install; done
- test -z "${RUN_MAKE_INSTALL}" || DESTDIR="${INSTALL_PATH}" sh util/check-make-install
- if [[ "${CFLAGS}" == *"-fsanitize=address"* ]]; then ( ! grep -F AddressSanitizer config.log ); fi
- test -z "${CROSS_COMPILATION}" || grep -F -A 1 "checking whether we are cross compiling" config.log | grep -q "result.*yes"
@@ -363,6 +360,7 @@ stages:
needs:
- job:ci-variables
artifacts:true
timeout:2h
.system_test_common:&system_test_common
<<:*default_triggering_rules
@@ -378,6 +376,7 @@ stages:
- >
"$PYTEST" --junit-xml="$CI_PROJECT_DIR"/junit.xml -n "$TEST_PARALLEL_JOBS" | tee pytest.out.txt
- '( ! grep -F "grep: warning:" pytest.out.txt )'
- test "$CLEAN_BUILD_ARTIFACTS_ON_SUCCESS" -eq 0 || ( cd ../../.. && make clean >/dev/null 2>&1 )
after_script:
- test -n "${OUT_OF_TREE_WORKSPACE}" && cd "${OUT_OF_TREE_WORKSPACE}"
- *display_pytest_failures
@@ -432,6 +431,7 @@ stages:
- test -n "${OUT_OF_TREE_WORKSPACE}" && cd "${OUT_OF_TREE_WORKSPACE}"
script:
- make -j${TEST_PARALLEL_JOBS:-1} -k unit V=1
- test "$CLEAN_BUILD_ARTIFACTS_ON_SUCCESS" -eq 0 || make clean >/dev/null 2>&1
after_script:
- test -d bind-* && cd bind-*
- REALSOURCEDIR="$PWD"
@@ -468,10 +468,12 @@ stages:
junit:junit.xml
.docs:&docs_job
variables:
DOC_MAKE_TARGET:doc
stage:docs
script:
- *configure
- make -j${BUILD_PARALLEL_JOBS:-1} -k doc V=1
- make -j${BUILD_PARALLEL_JOBS:-1} -k ${DOC_MAKE_TARGET} V=1
- find doc/man/ -maxdepth 1 -name "*.[0-9]" -exec mandoc -T lint "{}" \; | ( ! grep -v -e "skipping paragraph macro. sp after" -e "unknown font, skipping request. ft C" -e "input text line longer than 80 bytes" )
- [ ] [:link:][step_deputy] **(IM)** Pick a Deputy Incident Manager
- [ ] [:link:][step_respond] **(IM)** Respond to the bug reporter
- [ ] [:link:][step_public_mrs] **(SwEng)** Ensure there are no public merge requests which inadvertently disclose the issue
- [ ] [:link:][step_coordinate_cve_id] **(SwEng)** Check if we need to coordinate with other vendors (an industry-wide CVE identifier might be necessary)
- [ ] [:link:][step_assign_cve_id] **(IM)** Assign a CVE identifier
- [ ] [:link:][step_note_cve_info] **(SwEng)** Update this issue with the assigned CVE identifier, the CVSS score, and CWE category
- [ ] [:link:][step_note_cve_info] **(SwEng)** Update this issue with the assigned CVE identifier, the CVSS score, and the CWE category
- [ ] [:link:][step_versions_affected] **(SwEng)** Determine the range of product versions affected (including the Subscription Edition)
- [ ] [:link:][step_workarounds] **(SwEng)** Determine whether workarounds for the problem exist
- [ ] [:link:][step_coordinate] **(SwEng)** If necessary, coordinate with other parties
- [ ] [:link:][step_earliest_prepare] **(Support)** Prepare "earliest" notification text
- [ ] [:link:][step_earliest_send] **(Support)** Update "earliest" notification ticket in support portal Earliest queue which will notify earliest customers
- [ ] [:link:][step_advisory_mr] **(Support)** Create a merge request for the Security Advisory and include all readily available information in it
@@ -48,6 +48,7 @@ confidential!
- [ ] [:link:][step_backports] **(SwEng)** Prepare backports of the merge request addressing the problem for all affected (and still maintained) branches of a given product
- [ ] [:link:][step_finish_advisory] **(Support)** Finish preparing the Security Advisory
- [ ] [:link:][step_meta_issue] **(QA)** Create (or update) the private issue containing links to fixes & reproducers for all CVEs fixed in a given release cycle
- [ ] [:link:][step_coordinate_check] **(SwEng)** Make sure other vendors are able to release on the date that was previously agreed upon
- [ ] [:link:][step_merge_fixes] **(QA)** Merge the CVE fixes in CVE identifier order
- [ ] [:link:][step_patches] **(QA)** Prepare a standalone patch for the last stable release of each affected (and still maintained) product branch
- [ ] [:link:][step_asn_releases] **(QA)** Prepare ASN releases (as outlined in the Release Checklist)
@@ -68,15 +69,15 @@ confidential!
### On the Day of Public Disclosure
- [ ] [:link:][step_clearance] **(IM)** Grant QA & Marketing clearance to proceed with public release
- [ ] [:link:][step_publish] **(QA/Marketing)** Publish the releases (as outlined in the release checklist)
- [ ] [:link:][step_matrix] **(Support)** (BIND 9 only) Add the new CVEs to the vulnerability matrix in the Knowledge Base
- [ ] [:link:][step_publish_advisory] **(Support)** Bump Document Version for the Security Advisory and publish it in the Knowledge Base
- [ ] [:link:][step_publish] **(QA/Marketing)** Publish the releases (as outlined in the release checklist)
- [ ] [:link:][step_notifications] **(First IM)** Send notification emails to third parties
- [ ] [:link:][step_mitre] **(First IM)** Advise MITRE about the disclosed CVEs
# Copyright (C) Internet Systems Consortium, Inc. ("ISC")
#
# SPDX-License-Identifier: MPL-2.0
#
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, you can obtain one at https://mozilla.org/MPL/2.0/.
#
# See the COPYRIGHT file distributed with this work for additional
# information regarding copyright ownership.
rm -f */named.memstats
rm -f */named.run
rm -f */named.conf
rm -f dig.out.test*
rm -f ns2/example.com.bk
rm -f ns2/example.net.bk
rm -f ns*/managed-keys.bind* ns*/*mkeys*
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Ondřej Surý
Artem Boldariev
Artem Boldariev
Matthijs Mekking
Evan Hunt
Michal Nowak
Arаm Sаrgsyаn
Alessio Podda
Mark Andrews
Andoni Duarte
Nicki KřížekandMichal Nowak
Petr Špaček
Michał Kępień
Colin Vidal
Aydın Mercan
JINMEI TatuyaandOndřej Surý
Pavel BřezinaandOndřej Surý
Petr MenšíkandMark Andrews
Colin VidalandColin Vidal
Remi GacogneandMark Andrews
Timo EisenmannandMark Andrews
Suzanne GoldlustandNicki Křížek