Replace custom DNS servers used in the "qmin" system test with new code
based on the isctest.asyncserver module. The revised code employs zone
files and a limited amount of custom logic, which massively improves
test readability and maintainability, extends logging, and fixes
non-compliant replies sent by some of the custom servers in response to
certain queries (e.g. AA=0 in authoritative empty non-terminal
responses, non-glue address records in ADDITIONAL section).
Backport of MR !10195
Merge branch 'backport-michal/qmin-asyncserver-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10275
The vulture tool seems to be unable to follow how the parent classes
defined in bin/tests/system/qmin/qmin_ans.py use mandatory properties
specified by child classes in bin/tests/system/qmin/ans*/ans.py. Make
the tool ignore not just ans.py servers, but also *_ans.py utility
modules above the ansX/ subdirectories to prevent false positives about
unused code from causing CI pipeline failures.
(cherry picked from commit dfd37918d6913b783ead915d608b5951386f5974)
Some versions of the Hypothesis Python library - notably the one
included in stock OS repositories for Ubuntu 20.04 Focal Fossa - cause a
.hypothesis file to be created in a Python script's working directory
when the hypothesis module is present in its import chain. Ignore such
files by adding them to the list of expected test artifacts to prevent
pytest teardown checks from failing due to these files appearing in the
file system after running system tests.
(cherry picked from commit f413ddbe5f2edfdeedc41603dcd2afe105ed2844)
Commit 6c010a5644324947c8c13b5600cd8d988ae7684f caused the PYTHONPATH
environment variable to be set for ans.py servers started using
start.pl. However, no system test has actually used the new
isctest.asyncserver module since that change was applied, so it has not
been noticed until now that including the source directory in PYTHONPATH
is only sufficient for in-tree builds. Include the build directory
instead of the source directory in the PYTHONPATH environment variable
set for ans.py servers started by start.pl so that they work correctly
for both in-tree and out-of-tree builds.
(cherry picked from commit a799dd04adc08a062ec9961a026573abcc7c9181)
Replace custom DNS servers used in the "qmin" system test with new code
based on the isctest.asyncserver module. The revised code employs zone
files and a limited amount of custom logic, which massively improves
test readability and maintainability, extends logging, and fixes
non-compliant replies sent by some of the custom servers in response to
certain queries (e.g. AA=0 in authoritative empty non-terminal
responses, non-glue address records in ADDITIONAL section).
(cherry picked from commit 7faa34c6ee40653eeec23ef2df8093564cfc1891)
The check fails with the following error for some time:
403 Client Error: Forbidden for url: https://dl.acm.org/doi/10.1145/1315245.1315298
(cherry picked from commit 1ab889ee21c6b39b12a80a7637c7081817a365b1)
When `-T cookiealwaysvalid` is passed to `named`, DNS cookie checks for
the incoming queries always pass, given they are structurally correct.
Backport of MR !10232
Merge branch 'backport-aram/new-named-minus-T-option-of-cookiealwaysvalid-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10264
Add a check in the "cookie" system test to make sure that the new
'-T cookiealwaysvalid' option works.
(cherry picked from commit 4e75a20b6a63c3840559accc9df2af2d50a747f6)
When -T cookiealwaysvalid is passed to named, DNS cookie checks for
the incoming queries always pass, given they are structurally correct.
(cherry picked from commit 807ef8545d2e06c77826f3b2ac3f1cb7a7413dad)
A performance improvement for finding the closest encloser when generating authoritative responses from NSEC3 zones was previously reverted after a bug was found that could trigger an assertion failure. (See #4460, #4950, and #5108 for details.) The bug has now been fixed, and the performance improvement has been restored.
Fixes#5204
Backport of MR !9610
Backport of MR !9928
Merge branch '5108-nsec3-empty-node-bind-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10034
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 an empty node was found, the result was treated as a partial match,
but foundname could still contain the name of the empty node instead of
its parent.
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().
maxlabels is the suffix length that corresponds to the latest
NXDOMAIN response. minlabels is the suffix length that corresponds
to longest found existing name.
(cherry picked from commit 67f31c504679dfcd9f1231037afa56da01e40d36)
Add missing locks in dns_zone_getxfrsource4 et al. Addresses CID 468706, 468708, 468741, 468742, 468785, and 468778.
Cleanup dns_zone_setxfrsource4 et al to now return void.
Remove double copies with dns_zone_getprimaryaddr and dns_zone_getsourceaddr.
Closes#4933
Backport of MR !9485
Merge branch 'backport-4933-add-missing-locks-when-returning-addresses-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10259
Add missing locks in dns_zone_getxfrsource4 et al. Addresses CID
468706, 468708, 468741, 468742, 468785 and 468778.
Cleanup dns_zone_setxfrsource4 et al to now return void.
Remove double copies with dns_zone_getprimaryaddr and dns_zone_getsourceaddr.
(cherry picked from commit d0a59277fb13023d3aff5c1d4d91506a850365ee)
The `I:checking that lifting the limit will allow everything to get
cached (20)` test was failing due to the TTL of the records being
too short for the elapsed time of the test. Raise the TTL to fix
this and adjust other tests as needed.
Closes#5206
Backport of MR !10177
Merge branch 'backport-5206-tune-last-sub-test-of-reclimit-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10249
The 'I:checking that lifting the limit will allow everything to get
cached (20)' test was failing due to the TTL of the records being
too short for the elapsed time of the test. Raise the TTL to fix
this and adjust other tests as needed.
(cherry picked from commit 1a58bd211357ccd366b70e51d3cadaa7fc5aad15)
The `step()` function (used for stepping to the prececessor or successor of a database node) could overlook a node if there was an rdataset that was marked IGNORE because it had been rolled back, covering an active rdataset under it.
Closes#5170
Backport of MR !10103
Merge branch 'backport-5170-step-ignores-rollback-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10257
the db_test unit test now looks up an empty nonterminal node
to exercise the behavior of the step() function in qpzone.
(cherry picked from commit ecde0ea2d719153c84fca19eaeeeeb6a01c10c1a)
check that a database rollback works and the correct
(original) data is found on lookup.
(cherry picked from commit 7d98aba3ac9189b88d54ac0a690e625d27950e1a)
the step() function (used for stepping to the prececessor or
successor of a database node) could overlook a node because
there was an rdataset marked IGNORE because it had been rolled
back, covering an active rdataset under it.
(cherry picked from commit 24eaff7adc30c3cde22c5926369c3729ad12ae15)
When a key is revoked, its key ID changes due to the inclusion of the "revoked" flag. A collision between this changed key ID
and an unrelated public-only key could cause a crash in `dnssec-signzone`.
Closes#5231
Backport of MR !10233
Merge branch 'backport-5231-fix-keyid-collision-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10256
when a key is revoked its key ID changes, due to the inclusion
of the "revoke" flag. a collision between this changed key ID and
that of an unrelated public-only key could cause a crash in
dnssec-signzone.
(cherry picked from commit 9cfe9f5eb787f6c42eee87fc79f5fd38218090c4)
Dig +showbadvers now displays the received BADVERS message and
continues the EDNS version negotiation. Previously to see the
BADVERS message +noednsneg had to be specified which terminated the
EDNS negotiation. Additionally the specified EDNS value (+edns=value)
is now used when making all the initial queries with +trace. i.e EDNS
version negotiation will be performed with each server when performing
the trace.
Closes#5234
Backport of MR !10234
Merge branch 'backport-5234-have-dig-display-the-badvers-message-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10247
Add "+showbadvers" to display the BADVERS response similarly
to "+showbadcookie". Additionally reset the EDNS version to
the requested version in "dig +trace" so that EDNS version
negotiation can be tested at all levels of the trace rather
that just when requesting the root nameservers.
(cherry picked from commit 6c271f63281ca2263ebbd7ad7f6788bc4449d279)
Support was added for EDE codes 20 (Not Authoritative) when client requests recursion (RD) but the server has recursion disabled.
RFC 8914 mention EDE 20 should also be returned if the client doesn't have the RD bit set (and recursion is needed) but it doesn't apply for
BIND as BIND would try to resolve from the "deepest" referral in AUTHORITY section. For example, if the client asks for "www.isc.org/A" but the server only knows the root domain, it will return NOERROR but no answer for "www.isc.og/A", just the list of other servers to ask.
See #1836
Backport of MR !10228
Merge branch 'backport-1836-not-authoritative-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10243
Extended DNS Error message EDE 20 (Not Authoritative) is now sent when
client request recursion (RD) but the server has recursion disabled.
RFC 8914 mention EDE 20 should also be returned if the client doesn't
have the RD bit set (and recursion is needed) but it doesn't apply for
BIND as BIND would try to resolve from the "deepest" referral in
AUTHORITY section. For example, if the client asks for "www.isc.org/A"
but the server only knows the root domain, it will returns NOERROR but
no answer for "www.isc.og/A", just the list of other servers to ask.
(cherry picked from commit 24ffbdcfea32b7f3c3feceba23cfc4bf474a1fa3)
Support was added for EDE codes 7 (Signature Expired) and 8 (Signature Not Yet Valid) which might occur during DNSSEC validation.
See #2715
Backport of MR !10225
Merge branch 'backport-2715-expired-future-keys-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10242
Add DNSSEC system tests to cover extended DNS error 7 (Signature
Expired) and 8 (Signature Not Yet Valid).
(cherry picked from commit e763d6637f54fcd079f4ab17120c0c53aa4adecc)
Extended DNS Error messages EDE 7 (expired key) and EDE 8 (validity
period of the key not yet started) are now sent in case of such DNSSEC
validation failures.
Refactor the existing validator extended error APIs in order to make it
easy to have a consisdent extra info (with domain/type) in the various
use case (i.e. when the EDE depends on validator state,
validate_extendederror or when the EDE doesn't depend of any state but
can be called directly in a specific flow).
(cherry picked from commit 334ea1269fc04b764be8e8ebf33d8c9c0036026c)
Closes#5229
Backport of MR !10238
Merge branch 'backport-5229-ksr-system-test-can-fail-on-key-collision-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10240
When generating new key pairs, one test checks if existing keys that
match the time bundle are selected, rather than extra keys being
generated. Part of the test is to check the verbose output, counting
the number of "Selecting" and "Generating" occurences. But if there
is a key collision, the ksr tool will output that the key already
exists and includes the substring "already exists, or might collide
with another key upon revokation. Generating a new key".
So substract by one the generated counter if there is a "collide"
occurrence.
(cherry picked from commit 8b3d2e5633183205fda5121329caf35e71200167)
Acquire the database reference in the detachnode() to prevent the last
reference to be release while the NODE_LOCK being locked. The NODE_LOCK
is locked/unlocked inside the RCU critical section, thus it is most
probably this should not pose a problem as the database uses call_rcu
memory reclamation, but this it is still safer to acquire the reference
before releasing the node.
Closes#5194
Backport of MR !10155
Merge branch 'backport-5194-fix-assertion-failure-while-reference-counting-qpdb-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10156
Acquire the database refernce in the detachnode() to prevent the last
reference to be release while the NODE_LOCK being locked. The NODE_LOCK
is locked/unlocked inside the RCU critical section, thus it is most
probably this should not pose a problem as the database uses call_rcu
memory reclamation, but this it is still safer to acquire the reference
before releasing the node.
(cherry picked from commit d1ef6a93c112137ab0682afb9a3240d47285d408)
The text that stale-cache-enable is set to no has no effect on
max-cache-ttl, but on max-stale-ttl.
Closes#5181
Backport of MR !10108
Merge branch 'backport-5181-max-stale-ttl-typo-arm-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10115
The text that stale-cache-enable is set to no has no effect on
max-cache-ttl, but on max-stale-ttl.
(cherry picked from commit b017d9fe6715c8eee3a0344395c81136d4dde8c8)
This reverts commit 67255da4b376f65138b299dcd5eb6a3b7f9735a9, reversing
changes made to 74c9ff384e695d1b27fa365d1fee84576f869d4c.
Closes#5169
Backport of MR !10224
Merge branch 'backport-5169-revert-qpzone-delete-dead-nodes-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10226
This reverts commit 67255da4b376f65138b299dcd5eb6a3b7f9735a9, reversing
changes made to 74c9ff384e695d1b27fa365d1fee84576f869d4c.
(cherry picked from commit 1e4695510aee2d27bf6f5f14dc8564357d737aa3)
When `dns_remote_done()` is true, calling `dns_remote_curraddr()` asserts.
Add a `dns_remote_curraddr()` check before calling `dns_remote_curraddr()`.
Closes#5215
Backport of MR !10222
Merge branch 'backport-5215-assert-in-dns_remote_curraddr-fix-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10223
When dns_remote_done() is true, calling dns_remote_curraddr() asserts.
Add a dns_remote_curraddr() check before calling dns_remote_curraddr().
(cherry picked from commit 6cd9e4f67c48ce9178600aba7fe91266b914e713)
