Add DNSSEC system tests covering extended DNS error 9. Because BIND9 won't
returned signed records (RRSIG) when provided a signed zone without DNSKEY
records, this test involves a new mock DNS server which loads a
fully signed zone and return associated RRSIG to its RR, but also return NODATA
when the query type is DNSKEY. This enable to run a resolver trying to validate
a domain, but failing because it can't get the DNSKEY, thus expecting the EDE 9
message.
Extended DNS Error 9 (Missing DNSKEY) is now sent when a validating resolver
attempts to validate a response but can't get the DNSKEY from the authoritative
server.
The EDE 9 is sent only in the proveunsecure flow because we want to
send it only in the case there has to be a validation, so we need to
make sure the parent domain has a DS record to the children.
The check looks for logs that are not present, fails to make the
possible failure visible, and fails to bump the check enumerator:
I:checking that log-report-channel zones fail if '*._er/TXT' is missing (129)
grep: test.out4.129: No such file or directory
grep: test.out4.129: No such file or directory
I:checking that raw zone with bad class is handled (129)
The issue appeared in #3659.
Merge branch 'mnowak/checkzone-test-fix' into 'main'
See merge request isc-projects/bind9!10286
The check looks for logs that are not present, fails to make the
possible failure visible, and fails to bump the check enumerator:
I:checking that log-report-channel zones fail if '*._er/TXT' is missing (129)
grep: test.out4.129: No such file or directory
grep: test.out4.129: No such file or directory
I:checking that raw zone with bad class is handled (129)
Lines starting with A or NSEC are expected but not matched with the
OpenBSD grep. Extended regular expressions with direct use of
parentheses and the pipe symbol is more appropriate.
I:checking RRSIG query from cache (154)
I:failed
The issue appeared in #4805.
Merge branch 'mnowak/openbsd-grep-fix' into 'main'
See merge request isc-projects/bind9!10285
Lines starting with A or NSEC are expected but not matched with the
OpenBSD grep. Extended regular expressions with direct use of
parentheses and the pipe symbol is more appropriate.
I:checking RRSIG query from cache (154)
I:failed
When query responses timed out, the resolver could incorrectly increase the regular responses counters, even if no response was received. This has been fixed.
Closes#5193
Merge branch '5193-resolver-statistics-counters-fix' into 'main'
See merge request isc-projects/bind9!10227
When a response times out the fctx_cancelquery() function
incorrectly calculates it in the 'dns_resstatscounter_queryrtt5'
counter (i.e. >=1600 ms). To avoid this, the rctx_timedout()
function should make sure that 'rctx->finish' is NULL. And in order
to adjust the RTT values for the timed out server, 'rctx->no_response'
should be true. Update the rctx_timedout() function to make those
changes.
The resquery_response() function increases the response counter without
checking if the response was successful. Increase the counter only when
the result indicates success.
This branch started off as `michal/upforwd-asyncserver`. It quickly
turned out that the critical `asyncserver.py` change that was needed for
the `upforwd` system test was for the server to be able to read multiple
TCP queries on a single connection. As currently present in `main`,
`asyncserver.py` closes every client connection after servicing a single
query. Retaining that behavior would cause the `upforwd` system test to
fail and, in general, capturing all data sent by a client seems more
useful in tests than just closing connections quickly. `asyncserver.py`
can always be extended in the future (e.g. by adding a new
`ResponseAction` that the networking code would react to) to reinstate
the original behavior, if it turns out to be necessary.
While working on changing that particular `asyncserver.py` behavior, I
noticed a couple of other deficiencies in the TCP connection handling
code, so I started addressing them. One thing led to another and before
I noticed, enough changes were applied to be worth doing a separate
merge request, particularly given that the actual rewrite of
`upforwd/ans4/ans.pl` using `asyncserver.py` is trivial once the
required changes to `asyncserver.py` itself are applied.
Merge branch 'michal/asyncserver-tcp-improvements' into 'main'
See merge request isc-projects/bind9!10276
Instead of closing every incoming TCP connection after handling a single
query, continue receiving queries on each TCP connection until the
client disconnects itself. When coupled with response dropping, this
enables silently receiving all incoming data, simulating an unresponsive
server.
A TCP DNS client may send its queries in chunks, causing
StreamReader.read() to return less data than previously declared by the
client as the DNS message length; even the two-octet DNS message length
itself may be split up into two single-octet transmissions. Sending
data in chunks is valid client behavior that should not be treated as an
error. Add a new helper method for reading TCP data in a loop, properly
distinguishing between chunked queries and client disconnections. Use
the new method for reading all TCP data from clients.
A TCP peer may reset the connection at any point, but asyncserver.py
currently only handles connection resets when it is sending data to the
client. Handle connection resets during reading in the same way.
Add a helper class, Peer, which holds the <host, port> tuple of a
connection endpoint and gets pretty-printed when formatted as a string.
This enables passing instances of this new class directly to logging
functions, eliminating the need for the AsyncDnsServer._format_peer()
helper method.
The false positive rate is about 10-20 % when evaluating shotgun results
from a single run. Attempt to reduce the false positive rate by allowing
a re-run of failed jobs.
Merge branch 'nicki/ci-shotgun-reduce-false-positives' into 'main'
See merge request isc-projects/bind9!10271
The false positive rate is about 10-20 % when evaluating shotgun results
from a single run. Attempt to reduce the false positive rate by allowing
a re-run of failed jobs.
While there is a slight risk that barely noticable decreases in
performance might slip by more easily in MRs, they'd still likely pop up
during nightly or pre-release testing.
Also increase the tolerance threshold for DoH latency comparisons, as
those tests often experience increased jitter in the tail end latencies.
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).
Merge branch 'michal/qmin-asyncserver' into 'main'
See merge request isc-projects/bind9!10195
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.
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.
Commit 6c010a5644 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.
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).
There was some code in dns_resolver unit meant to keep compatibility with BIND 8 breaking the DNS protocol. These should not be needed anymore.
Merge branch 'ondrej/resolver-bind-8-cleanup' into 'main'
See merge request isc-projects/bind9!10270
There were kludges to help process responses from authoritative servers
giving RRs in wrong sections (mentioning BIND 8). These should just go
away and such responses should not be processed.
When `-T cookiealwaysvalid` is passed to `named`, DNS cookie checks for
the incoming queries always pass, given they are structurally correct.
Merge branch 'aram/new-named-minus-T-option-of-cookiealwaysvalid' into 'main'
See merge request isc-projects/bind9!10232
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
Merge branch '4933-add-missing-locks-when-returning-addresses' into 'main'
See merge request isc-projects/bind9!9485
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.
- `dns_rdatatype_ismulti()` returns true if a given type can have
multiple answers: ANY, RRSIG, or SIG.
- `dns_rdatatype_issig()` returns true for a signature: RRSIG or SIG.
- `dns_rdatatype_isaddr()` returns true for an address: A or AAAA.
- `dns_rdatatype_isalias()` returns true for an alias: CNAME or DNAME.
Code has been modified to use these functions where applicable.
These and all similar functions (e.g., `dns_rdatatype_ismeta()`, `dns_rdatatype_issingleton()`, etc) are now `static inline` functions defined in `rdata.h`.
Merge branch 'each-rdatatype-functions' into 'main'
See merge request isc-projects/bind9!10216
- dns_rdatatype_ismulti() returns true if a given type can have
multiple answers: ANY, RRSIG, or SIG.
- dns_rdatatype_issig() returns true for a signature: RRSIG or SIG.
- dns_rdatatype_isaddr() returns true for an address: A or AAAA.
- dns_rdatatype_isalias() returns true for an alias: CNAME or DNAME.
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
Merge branch '5170-step-ignores-rollback' into 'main'
See merge request isc-projects/bind9!10103
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.
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
Merge branch '5231-fix-keyid-collision' into 'main'
See merge request isc-projects/bind9!10233
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.
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
Merge branch '5206-tune-last-sub-test-of-reclimit' into 'main'
See merge request isc-projects/bind9!10177
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.
When performing QNAME minimization, `named` now sends an NS query for the original query name, before sending the final query. This prevents the parent zone from learning the original query type, in the event that the query name is a delegation point.
For example, when looking up an address record for `example.com`, NS queries are now sent to the servers for both `com` and `example.com`, before the address query is sent to the servers for `example.com`. Previously, an address query would have been sent to the servers for `com`.
Closes#4805
Merge branch '4805-missing-qname-ns-query-when-using-qname-minimisation' into 'main'
See merge request isc-projects/bind9!9155
When performing QNAME minimization, named now sends an NS
query for the original QNAME, to prevent the parent zone from
receiving the QTYPE.
For example, when looking up example.com/A, we now send NS queries
for both com and example.com before sending the A query to the
servers for example.com. Previously, an A query for example.com
would have been sent to the servers for com.
Several system tests needed to be adjusted for the new query pattern:
- Some queries in the serve-stale test were sent to the wrong server.
- The synthfromdnssec test could fail due to timing issues; this
has been addressed by adding a 1-second delay.
- The cookie test could fail due to the a change in the count of
TSIG records received in the "check that missing COOKIE with a
valid TSIG signed response does not trigger TCP fallback" test case.
- The GL #4652 regression test case in the chain system test depends
on a particular query order, which no longer occurs when QNAME
minimization is active. We now disable qname-minimization
for that test.
If a timeout occurs when sending a QMIN query, QNAME
minimization should be disabled. This now causes a hard
failure in strict mode, or a fallback to non-minimized queries
in relaxed mode.
The calling fetch has already called fcount_incr() for this zone;
calling it again for a QMIN query results in double counting.
When resuming after a QMIN query is answered, however, we do now
ensure before continuing that the fetches-per-zone limit has not
been exceeded.
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
Merge branch '5234-have-dig-display-the-badvers-message' into 'main'
See merge request isc-projects/bind9!10234
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.
If the `max-clients-per-query` option is set to a lower value than `clients-per-query`, the value is adjusted to match `clients-per-query`.
Closes#5224
Merge branch '5224-raise-max-clients-per-query-to-be-at-least' into 'main'
See merge request isc-projects/bind9!10241
In the case where 'clients-per-query' is larger than
'max-clients-per-query', raise 'max-clients-per-query' so that
'clients-per-query' equals 'max-clients-per-query' and log a warning
that this is what happened.
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
Merge branch '1836-not-authoritative' into 'main'
See merge request isc-projects/bind9!10228
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.
Support was added for EDE codes 7 (Signature Expired) and 8 (Signature Not Yet Valid) which might occur during DNSSEC validation.
See #2715
Merge branch '2715-expired-future-keys' into 'main'
See merge request isc-projects/bind9!10225
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).
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.
Assigning value "NULL" to "newstr", but that stored value is overwritten
before it can be used.
Setting "newstr" to NULL does not have any effect, so the line can
safely be removed.
Closes#5227
Merge branch '5227-cid-544147' into 'main'
See merge request isc-projects/bind9!10239
Assigning value "NULL" to "newstr", but that stored value is overwritten
before it can be used.
Setting "newstr" to NULL does not have any effect, so the line can
safely be removed.
When `dns_remote_done()` is true, calling `dns_remote_curraddr()` asserts.
Add a `dns_remote_curraddr()` check before calling `dns_remote_curraddr()`.
Closes#5215
Merge branch '5215-assert-in-dns_remote_curraddr-fix' into 'main'
See merge request isc-projects/bind9!10222
This MR changes custom attach/detach implementation with refcount macros, replaces isc_mem_destroy() with isc_mem_detach(), and does various small cleanups.
Merge branch 'ondrej/cleanup-isc_mem-api' into 'main'
See merge request isc-projects/bind9!9456
The isc_mem API is one of the most commonly used APIs that didn't
used ISC_REFCOUNT_DECL and ISC_REFCOUNT_IMPL macros. Replace the
implementation of isc_mem_attach(), isc_mem_detach() and
isc_mem_destroy() with the respective macros.
This also removes the legacy isc_mem_destroy() functionality that would
check whether all references had been detached from the memory context
as it doesn't work reliably when using the call_rcu() API. Instead of
doing this individually, call isc_mem_checkdestroyed(stderr) from the
isc_mem_destroy() macro to keep the extra check that all contexts were
freed when the program is exiting.
ZONEMD digests RRSIG records and potentially digests SIG record. Add digests
methods for both record types.
Closes#5219
Merge branch '5219-add-digest-methods-for-sig-and-rrsig' into 'main'
See merge request isc-projects/bind9!10217
ZONEMD needs to be able to digest SIG and RRSIG records. The signer
field can be compressed in SIG so we need to call dns_name_digest().
While for RRSIG the records the signer field is not compressed the
canonical form has the signer field downcased (RFC 4034, 6.2). This
also implies that compare_rrsig needs to downcase the signer field
during comparison.
The qpcache_findzonecut() accepts two "foundnames": 'foundname' and
'dcname' could be NULL. Originally, when 'dcname' would be NULL, the
'dcname' would be set to 'foundname' which basically means that we were
copying the .ndata over itself for no apparent reason.
Merge branch 'ondrej/refactor-qpcache_findzonecut' into 'main'
See merge request isc-projects/bind9!10049
The qpcache_findzonecut() accepts two "foundnames": 'foundname' and
'dcname' could be NULL. Originally, when 'dcname' would be NULL, the
'dcname' would be set to 'foundname'. Then code like this was present:
result = find_deepest_zonecut(&search, node, nodep, foundname,
rdataset,
sigrdataset DNS__DB_FLARG_PASS);
dns_name_copy(foundname, dcname);
Which basically means that we are copying the .ndata over itself for no
apparent reason. Cleanup the dcname vs foundname usage.
Co-authored-by: Evan Hunt <each@isc.org>
Co-authored-by: Ondřej Surý <ondrej@isc.org>
isc_iterated_hash didn't work in offloaded threads as the per thread
initialisation has not been done. This has been fixed.
Closes#5214
Merge branch '5214-call-isc__iterated_hash_initialize-in-isc__work_cb' into 'main'
See merge request isc-projects/bind9!10206
When a system test is run with the `USE_RR` environment variable set to 1, an `rr` trace is now correctly generated for each instance of `named`.
Closes#5079
Merge branch '5079-fix-rr' into 'main'
See merge request isc-projects/bind9!10197
when running a system test with the USE_RR environment
variable set to 1, an rr trace is generated for named.
because rr wasn't run using libtool --mode=execute, the
trace would actually be generated for the wrapper script
generated by libtool, not for the actual named binary.
IPv6 Advanced Socket API (:rfc:`3542`) is a hard requirement, remove the
autoconf check to speed up the ./configure run a little bit.
Merge branch 'ondrej/remove-check-for-mandatory-IPv6' into 'main'
See merge request isc-projects/bind9!10201
This merge request addresses several key performance bottlenecks in the DoH (DNS over HTTPS) implementation by introducing significant optimizations and improvements.
### Key Improvements
1. **Simplification and Optimisation of `http_do_bio()` Function**:
- The code flow in the `http_do_bio()` function has been significantly simplified.
2. **Flushing HTTP Write Buffer on Outgoing DNS Messages**:
- The buffer is flushed and a send operation is performed when there is an outgoing DNS message.
3. **Bumping Active Streams Processing Limit**:
- The total number of active streams has been increased to 60% of the total streams limit.
These changes collectively enhance the performance and reliability of the DoH implementation, making it more efficient and robust for handling high-load scenarios, particularly noticeable in long runs (>= 1h) of `stress:long:rpz:doh+udp:linux:*` tests. It improves perf. for tests for BIND 9.18, but it likely will have a positive but less pronounced effect on newer versions as well.
In essence, the merge request fixes three bottlenecks stacked upon each other.
*It is a logical continuation of the merge requests !10109.* !10109, unfortunately, did not completely [address the performance drop in 9.18](https://gitlab.isc.org/isc-projects/bind9/-/pipelines/221545) for longer runs of the stress test. This merge request [addresses that](https://gitlab.isc.org/isc-projects/bind9/-/pipelines/223661).
**P.S.**
The origin of the fixes is, in fact, the branch in !10193. So this MR is a ... *forward port* of them.
Merge branch 'artem-doh-performance-drop-post-fix' into 'main'
See merge request isc-projects/bind9!10192
This commit bumps the total number of active streams (= the opened
streams for which a request is received, but response is not ready) to
60% of the total streams limit.
The previous limit turned out to be too tight as revealed by
longer (≥1h) runs of "stress:long:rpz:doh+udp:linux:*" tests.
Previously, the code would try to avoid sending any data regardless of
what it is unless:
a) The flush limit is reached;
b) There are no sends in flight.
This strategy is used to avoid too numerous send requests with little
amount of data. However, it has been proven to be too aggressive and,
in fact, harms performance in some cases (e.g., on longer (≥1h) runs
of "stress:long:rpz:doh+udp:linux:*").
Now, additionally to the listed cases, we also:
c) Flush the buffer and perform a send operation when there is an
outgoing DNS message passed to the code (which is indicated by the
presence of a send callback).
That helps improve performance for "stress:long:rpz:doh+udp:linux:*"
tests.
Previously, a function for continuing IO processing on the next UV
tick was introduced (http_do_bio_async()). The intention behind this
function was to ensure that http_do_bio() is eventually called at
least once in the future. However, the current implementation allows
queueing multiple such delayed requests needlessly. There is currently
no need for these excessive requests as http_do_bio() can requeue them
if needed. At the same time, each such request can lead to a memory
allocation, particularly in BIND 9.18.
This commit ensures that the number of enqueued delayed IO processing
requests never exceeds one in order to avoid potentially bombarding IO
threads with the delayed requests needlessly.
This commit significantly simplifies the code flow in the
http_do_bio() function, which is responsible for processing incoming
and outgoing HTTP/2 data. It seems that the way it was structured
before was indirectly caused by the presence of the missing callback
calls bug, fixed in 8b8f4d500d.
The change introduced by this commit is known to remove a bottleneck
and allows reproducible and measurable performance improvement for
long runs (>= 1h) of "stress:long:rpz:doh+udp:linux:*" tests.
Additionally, it fixes a similar issue with potentially missing send
callback calls processing and hardens the code against use-after-free
errors related to the session object (they can potentially occur).
Cleanup short list macros from <isc/util.h>, remove two unused headers, move locking macros to respective headers and use only the C11 static assertion.
Merge branch 'ondrej/cleanup-short-macros' into 'main'
See merge request isc-projects/bind9!10196
Previously, a gcc < 4.6 shim for _Static_assert() was included. Such an
old compiler is not supported now anyway, so the macro variant has been
removed in favor of a single definition using _Static_assert().
Previously, the LOCK()/UNLOCK() and friends macros were defined in the
isc/util.h header. Those macros were moved to their respective headers
as those would have to be included anyway if that particular lock was in
use.
Formerly, isc/util.h would pull a few extra headers (isc/list.h,
isc/attributes.h, isc/result.h and errno.h). These includes were
removed in favor of including them directly when used.
The short convenience list macros were used very sparingly and
inconsistenly in the code base. As the consistency is prefered over
the convenience, all shortened list macro were removed in favor of
their ISC_LIST API targets.
When querying zone transfers information from the statistics channel there was a rare possibility that `named` could terminate unexpectedly if a zone transfer was in a state when transferring from all the available primary servers had failed earlier. This has been fixed.
Closes#5198
Merge branch '5198-dns_remote_curraddr-bug-fix' into 'main'
See merge request isc-projects/bind9!10182
When all the addresses were already iterated over, the
dns_remote_curraddr() function asserts. So before calling it,
dns_zone_getprimaryaddr() now checks the address list using the
dns_remote_done() function. This also means that instead of
returning 'isc_sockaddr_t' it now returns 'isc_result_t' and
writes the primary's address into the provided pointer only when
returning success.
Logging initialization check is now redundant as there is a default global log context created during libisc's constructor.
`isc_log` calls can safely be made at any time outside libisc's constructor.
Merge branch 'aydin/remove-log-check' into 'main'
See merge request isc-projects/bind9!10186
Disabling dynamic tags ensures the Clang symbolizer creates a valid TSAN
report. For consistency, also add the option to gcc:tsan so they are
both on the same footing.
Closes#5149
Merge branch '5149-fix-tsan-flags' into 'main'
See merge request isc-projects/bind9!10185
Disabling new dynamic ELF tags ensures the Clang symbolizer creates
valid TSAN reports. For consistency, also add the option to gcc:tsan so
they are both on the same footing.
The fix in !10172 was incomplete; there were still some code paths in the resolver that could set the dns_fetchresponse `result` field to the wrong value when the rdataset type was CNAME or DNAME.
Closes#5201
Merge branch '5201-eresult-fix' into 'main'
See merge request isc-projects/bind9!10178
Since algorithm fetching is handled purely in libisc, FIPS mode toggling
can be purely done in within the library instead of provider fetching in
the binary for OpenSSL >=3.0.
Disabling FIPS mode isn't a realistic requirement and isn't done
anywhere in the codebase. Make the FIPS mode toggle enable-only to
reflect the situation.
Merge branch 'aydin/fips-in-crypto' into 'main'
See merge request isc-projects/bind9!9920
Since algorithm fetching is handled purely in libisc, FIPS mode toggling
can be purely done in within the library instead of provider fetching in
the binary for OpenSSL >=3.0.
Disabling FIPS mode isn't a realistic requirement and isn't done
anywhere in the codebase. Make the FIPS mode toggle enable-only to
reflect the situation.
Execute DNS Shotgun performance tests on the regular MRs and compare the changes they introduce against the MR diff base. The results are evaluated automatically - the shotgun jobs will fail if thresholds for CPU/memory/latency difference is exceeded.
Merge branch 'nicki/ci-shotgun-eval' into 'main'
See merge request isc-projects/bind9!10127
The keyword rules allows more flexible and complex conditions when
deciding whether to create the job and also makes it possible run tweak
variables or job properties depending on arbitraty rules. Since it's
not possible to combine only/except and rules together, replace all
uses of only/except to avoid any potential future issues.
If the shotgun tests are executed for MRs, compare it against the MR's
base rather than the previous release. Only fail the job in case the
performance drops (pass on performance improvements).
Note that start_in optimization was removed, since it isn't properly
supported with rules as of February 2025
(https://gitlab.com/gitlab-org/gitlab/-/issues/424203). Without this
optimization, container test images are likely to be re-built
unnecessarily when testing different protocols. A workaround for the
.gitlab-ci.yml exists, but the extra complexity doesn't seem justified.
The container image builds might change or be optimized in the future,
so let's just go with the build duplication for now.
Answers to an "ANY" query which were processed by the RPZ "passthru"
policy had the response-policy's `max-policy-ttl` value unexpectedly
applied. This has been fixed.
Closes#5187
Merge branch '5187-rpz-passthru-any-type-ttl-bug-fix' into 'main'
See merge request isc-projects/bind9!10176
Answers to an "ANY" query which are processed by the RPZ "passthru"
policy have the response-policy's 'max-policy-ttl' value unexpectedly
applied. Do not change the records' TTL when RPZ uses a policy which
does not alter the answer.
Previously, in some cases, the resolver could return rdatasets of type CNAME or DNAME without the result code being set to `DNS_R_CNAME` or `DNS_R_DNAME`. This could trigger an assertion failure in the ADB. The resolver error has been fixed.
Closes#5201
Merge branch '5201-adb-cname-error' into 'main'
See merge request isc-projects/bind9!10172
when the caching of a negative record failed because of the
presence of a positive one, ncache_adderesult() could override
this to ISC_R_SUCCESS. this could cause CNAME and DNAME responses
to be handled incorrectly. ncache_adderesult() now sets the result
code correctly in such cases.
When a server name turns out to be a CNAME or DNAME, the ADB does not use it, but the `dns_adbname` structure still stored a copy of the target name. This is unnecessary and the code has been removed.
Merge branch 'each-remove-adb-target' into 'main'
See merge request isc-projects/bind9!10149
the target name parameter to dns_adb_createfind() was always passed as
NULL, so we can safely remove it.
relatedly, the 'target' field in the dns_adbname structure was never
referenced after being set. the 'expire_target' field was used, but
only as a way to check whether an ADB name represents a CNAME or DNAME,
and that information can be stored as a single flag.
The dual-stack-servers configuration option was not working as expected; the specified servers were not being used when they should have been, leading to resolution failures. This has been fixed.
Closes#5019
Merge branch '5019-dual-stack-servers-wasn-t-working-in-all-cases' into 'main'
See merge request isc-projects/bind9!9708
Now that fctx_try is being called when adb returns DNS_ADB_NOMOREADDRESSES
we don't need these priming queries for the dual-stack-servers test
to succeed.
Named was stopping nameserver address resolution attempts too soon
when dual stack servers are configured. Dual stack servers are
used when there are *not* addresses for the server in a particular
address family so find->status == DNS_ADB_NOMOREADDRESSES is not a
sufficient stopping condition when dual stack servers are available.
Call fctx_try to see if the alternate servers can be used.
DNSKEY, KEY, RRSIG and SIG constraints have been relaxed to allow empty key and signature material after the algorithm identifier for PRIVATEOID and PRIVATEDNS. It is arguable whether this falls within the expected use of these types as no key material is shared and the signatures are ineffective but these are private algorithms and they can be totally insecure.
Closes#5167
Merge branch '5167-relax-private-dnskey-constraints' into 'main'
See merge request isc-projects/bind9!10083
DNSKEY, KEY, RRSIG and SIG constraints have been relaxed to allow
empty key and signature material after the algorithm identifier for
PRIVATEOID and PRIVATEDNS. It is arguable whether this falls within
the expected use of these types as no key material is shared and
the signatures are ineffective but these are private algorithms and
they can be totally insecure.
The `NS_QUERY_DONE_BEGIN` and `NS_QUERY_DONE_SEND` plugin hooks could cause a reference leak if they returned `NS_HOOK_RETURN` without cleaning up the query context properly.
Closes#2094
Merge branch '2094-plugin-reference-leak' into 'main'
See merge request isc-projects/bind9!9971
if the NS_QUERY_DONE_BEGIN or NS_QUERY_DONE_SEND hook is
used in a plugin and returns NS_HOOK_RETURN, some of the
cleanup in ns_query_done() can be skipped over, leading
to reference leaks that can cause named to hang on shut
down.
this has been addressed by adding more housekeeping
code after the cleanup: tag in ns_query_done().
dnssec-signzone could dereference a NULL key pointer when resigning a zone. This has been fixed.
Closes#5192
Merge branch '5192-dnssec-signzone-needs-to-check-for-a-null-key-when-setting-offline' into 'main'
See merge request isc-projects/bind9!10161
Several functions in the `dns_name` module have had parameters removed, that were rarely or never used:
- `dns_name_fromtext()` and `dns_name_concatenate()` no longer take a target buffer.
- `dns_name_towire()` no longer takes a compression offset pointer; this is now part of the compression context.
- `dns_name_towire()` with a `NULL` compression context will copy name data directly into a buffer with no processing.
Merge branch 'each-simplify-names' into 'main'
See merge request isc-projects/bind9!10152
previously, dns_name_fromtext() took both a target name and an
optional target buffer parameter, which could override the name's
dedicated buffer. this interface is unnecessarily complex.
we now have two functions, dns_name_fromtext() to convert text
into a dns_name that has a dedicated buffer, and dns_name_wirefromtext()
to convert text into uncompressed DNS wire format and append it to a
target buffer.
in cases where it really is necessary to have both, we can use
dns_name_fromtext() to load the dns_name, then dns_name_towire()
to append the wire format to the target buffer.
dns_name_fromtext() stores the converted name in the 'name'
passed to it, and optionally also copies it in wire format to
a buffer 'target'. this makes the interface unnecessarily
complex, and could be simplified by having a different function
for each purpose. as a first step, remove uses of the target
buffer in calls to dns_name_fromtext() where it wasn't actually
needed.
the target buffer passed to dns_name_concatenate() was never
used (except for one place in dig, where it wasn't actually
needed, and has already been removed in a prior commit).
we can safely remove the parameter.
this parameter was added as a (minor) optimization for
cases where dns_name_towire() is run repeatedly with the
same compression context, as when rendering all of the rdatas
in an rdataset. it is currently only used in one place.
we now simplify the interface by removing the extra parameter.
the compression offset value is now part of the compression
context, and can be activated when needed by calling
dns_compress_setmultiuse(). multiuse mode is automatically
deactivated by any subsequent call to dns_compress_permitted().
The `dns_rdataslab_fromrdataset()` function creates a slab from an rdataset. If the source rdataset already uses a slab, then no processing is necessary; we can just copy the existing slab to a new location.
Closes#5188
Merge branch '5188-optimize-makeslab' into 'main'
See merge request isc-projects/bind9!10162
the dns_rdataslab_fromrdataset() function creates a slab
from an rdataset. if the source rdataset already uses a slab,
then no processing is necessary; we can just copy the existing
slab to a new location.
Previously, if a new counter was added to the hashtable
while dumping recursing clients via the `rndc recursing`
command, and `fetches-per-zone` was enabled, an assertion
failure could occur. This has been fixed.
Closes#5200
Merge branch '5200-destroy-iterator-inside-the-rwlock' into 'main'
See merge request isc-projects/bind9!10164
Previously, the hashmap iterator for fetches-per-zone was destroy
outside the rwlock. This could lead to an assertion failure due to a
timing race with the internal rehashing of the hashmap table as the
rehashing process requires no iterators to be running when rehashing the
hashmap table. This has been fixed by moving the destruction of the
iterator inside the read locked section.
The memory used to internally store the DNS names has been reduced.
Merge branch 'ondrej/experiment-no-offsets-in-dns_name' into 'main'
See merge request isc-projects/bind9!10140
The third argument to set_offsets() was only used in
dns_name_fromregion() and not really needed. We can remove the third
argument and then manually check whether the last label is root label.
There was just a single use of passing an extra buffer to
dns_name_downcase() which have been replaced by simple call to
isc_ascii_lowercase() and the 'target' argument from dns_name_downcase()
function has been removed.
The MAKE_EMPTY() macro was clearing up the output variable in case of
the failure. However, this was breaking the usual design pattern that
the output variables are left in indeterminate state or we don't touch
them at all when a failure occurs. Remove the macro and change the
dns_name_downcase() to not touch the name contents until success.
There was a back-and-forth between static arrays and the pointers to the
offsets. Since we are now only using the static arrays, we can cleanup
the usage of the pointers that would previously point either to the
static array or name->offsets if available.
The offsets were meant to speed-up the repeated dns_name operations, but
it was experimentally proven that there's actually no real-world
benefit. Remove the offsets and labels fields from the dns_name and the
static offsets fields to save 128 bytes from the fixedname in favor of
calculating labels and offsets only when needed.
The old symtab implementation should have been removed in !9921, but it wasn't. This MR addresses that.
Merge branch 'alessio/cleanup-symtab-orphan-files' into 'main'
See merge request isc-projects/bind9!10122
The DNS header shows if a message has multiple questions or invalid
NOTIFY sections. We can drop these messages early, right after parsing
the question. This matches RFC 9619 for multi-question messages and
Unbound's handling of NOTIFY. We still parse the question to include it in
our FORMERR response.
Add drop_msg_early() function to check for these conditions:
- Messages with more than one question, as required by RFC 9619
- NOTIFY query messages containing answer sections (like Unbound)
- NOTIFY messages containing authority sections (like Unbound)
Closes#5158, #3656
Merge branch '5158-early-formerr-on-bad-notify-or-bad-qdcount' into 'main'
See merge request isc-projects/bind9!10056
The DNS header shows if a message has multiple questions or invalid
NOTIFY sections. We can drop these messages early, right after parsing
the question. This matches RFC 9619 for multi-question messages and
Unbound's handling of NOTIFY.
To further add further robustness, we include an additional check for
unknown opcodes, and also drop those messages early.
Add early_sanity_check() function to check for these conditions:
- Messages with more than one question, as required by RFC 9619
- NOTIFY query messages containing answer sections (like Unbound)
- NOTIFY messages containing authority sections (like Unbound)
- Unknown opcodes.
The zone file for example3 (ns1/example3.db) can be modified in the
upforwd test as example3 is updated as part of the test. Whether
the zone is written out or not by the end of the test is timing
dependent. Rename ns1/example3.db to ns1/example3.db.in and copy it to
ns1/example3.db in setup so we don't trigger post test changes checks.
Closes#5180
Merge branch '5180-create-example3-in-setup' into 'main'
See merge request isc-projects/bind9!10160
The zone file for example3 (ns1/example3.db) can be modified in the
upforwd test as example3 is updated as part of the test. Whether
the zone is written out or not by the end of the test is timing
dependent. Rename ns1/example3.db to ns1/example3.db.in and copy
it to ns1/example3.db in setup so we don't trigger post test changes
checks.
A change in 6aba56ae8 (checking whether a rejected RRset was identical
to the data it would have replaced, so that we could still cache a
signature) inadvertently introduced cases where processing of a
response would continue when previously it would have been skipped.
Closes#5197
Merge branch '5197-cache_name-logic-error' into 'main'
See merge request isc-projects/bind9!10157
A change in 6aba56ae8 (checking whether a rejected RRset was identical
to the data it would have replaced, so that we could still cache a
signature) inadvertently introduced cases where processing of a
response would continue when previously it would have been skipped.
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
Merge branch '5194-fix-assertion-failure-while-reference-counting-qpdb' into 'main'
See merge request isc-projects/bind9!10155
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.
Instead of relying on unreliable order of execution of the library
constructors and destructors, move them to individual binaries. The
advantage is that the execution time and order will remain constant and
will not depend on the dynamic load dependency solver.
Merge branch 'ondrej/move-the-constructors-destructors-to-binaries' into 'main'
See merge request isc-projects/bind9!10069
As the default_call_rcu_thread can't be forced to flush all the work
during the executable shutdown, create one call_rcu_thread explicitly
and assign it to the all created threads.
This allows this explicit call_rcu_thread to be unassociated from the
main thread and freed before the executable destructor exits.
Instead of relying on unreliable order of execution of the library
constructors and destructors, move them to individual binaries. The
advantage is that the execution time and order will remain constant and
will not depend on the dynamic load dependency solver.
This requires more work, but that was mitigated by a simple requirement,
any executable using libisc and libdns, must include <isc/lib.h> and
<dns/lib.h> respectively (in this particular order). In turn, these two
headers must not be included from within any library as they contain
inlined functions marked with constructor/destructor attributes.
Previously, active resolver fetches were only dumped when the `fetches-per-zone` configuration option was enabled. Now, active resolver fetches are dumped along with the number of `clients-per-server` counters per resolver fetch.
Merge branch 'ondrej/make-dns_resolver_dumpfetches-dump-fetches' into 'main'
See merge request isc-projects/bind9!10107
Previously, the dns_resolver_dumpfetches() would go over the fetch
counters. Alas, because of the earlier optimization, the fetch counters
would be increased only when fetches-per-zone was not 0, otherwise the
whole counting was skipped for performance reasons.
Instead of using the auxiliary fetch counters hash table, use the real
hash table that stores the fetch contexts to dump the ongoing fetches to
the recursing file.
Additionally print more information about the fetch context like start
and expiry times, number of fetch responses, number of queries and count
of allowed and dropped fetches.
Previously, a data race could cause a newly created fetch context for a new client to be used
before it had been fully initialized, which would cause the query to become stuck; queries for the same
data would be either paused indefinitely or dropped because of
the `clients-per-query` limit. This has been fixed.
Closes#5053
Merge branch '5053-fetch-context-create-data-race' into 'main'
See merge request isc-projects/bind9!10146
The order of the fetch context hash table rwlock and the individual
fetch context was reversed when calling the release_fctx() function.
This was causing a problem when iterating the hash table, and thus the
ordering has been corrected in a way that the hash table rwlock is now
always locked on the outside and the fctx lock is the interior lock.
In the next commit, we need to know whether the timer has been started
or stopped. Add isc_timer_running() function that returns true if the
timer has been started.
Update Sphinx-related Python packages to their current versions pulled
in by "pip install sphinx-rtd-theme" run in a fresh virtual environment.
Merge branch 'michal/update-sphinx-and-sphinx_rtd_theme' into 'main'
See merge request isc-projects/bind9!10138
With RPZ in use, `named` could terminate unexpectedly because of a race condition when a reconfiguration command was received using `rndc`. This has been fixed.
Closes#5146
Merge branch '5146-rpz-reconfig-bug-fix' into 'main'
See merge request isc-projects/bind9!10079
After a reconfiguration the old view can be left without a valid
'rpzs' member, because when the RPZ is not changed during the named
reconfiguration 'rpzs' "migrate" from the old view into the new
view, so when a query resumes it can find that 'qctx->view->rpzs'
is NULL which query_resume() currently doesn't expect to happen if
it's recursing and 'qctx->rpz_st' is not NULL.
Fix the issue by adding a NULL-check. In order to not split the log
message to two different log messages depending on whether
'qctx->view->rpzs' is NULL or not, change the message to not log
the RPZ policy's "version" which is just a runtime counter and is
most likely not very useful for the users.
The isc_counter_create() doesn't need the return value (it was always
ISC_R_SUCCESS), use the macros to implement the reference counting,
little style cleanup, and expand the unit test.
Merge branch 'ondrej/cleanup-isc_counter-unit' into 'main'
See merge request isc-projects/bind9!10126
The isc_counter_create() doesn't need the return value (it was always
ISC_R_SUCCESS), use the macros to implement the reference counting,
little style cleanup, and expand the unit test.
Previously, when parsing responses, named incorrectly rejected responses without matching RRSIG records for NSEC/DS/NSEC3 records in the authority section. This rejection, if appropriate, should have been left for the validator to determine and has been fixed.
Closes#5185
Merge branch '5185-remove-rrsig-check-from-dns_message_parse' into 'main'
See merge request isc-projects/bind9!10125
Checking whether the authority section is properly signed should
be left to the validator. Checking in getsection (dns_message_parse)
was way too early and resulted in resolution failures of lookups
that should have otherwise succeeded.
Previously a hard-coded limitation of maximum two key or message
verification checks were introduced when checking the message's
SIG(0) signature. It was done in order to protect against possible
DoS attacks. The logic behind choosing the number 2 was that more
than a single key should only be required during key rotations, and
in that case two keys are enough. But later it became apparent that
there are other use cases too where even more keys are required, see
issue number #5050 in GitLab.
This change introduces two new configuration options for the views,
`sig0key-checks-limit` and `sig0message-checks-limit`, which define how
many keys are allowed to be checked to find a matching key, and how
many message verifications are allowed to take place once a matching
key has been found. The latter protects against expensive cryptographic
operations when there are keys with colliding tags and algorithm
numbers, with default being 2, and the former protects against a bit
less expensive key parsing operations and defaults to 16.
Closes#5050
Merge branch '5050-sig0-let-considering-more-than-two-keys' into 'main'
See merge request isc-projects/bind9!9967
Previously a hard-coded limitation of maximum two key or message
verification checks were introduced when checking the message's
SIG(0) signature. It was done in order to protect against possible
DoS attacks. The logic behind choosing the number two was that more
than one key should only be required only during key rotations, and
in that case two keys are enough. But later it became apparent that
there are other use cases too where even more keys are required, see
issue number #5050 in GitLab.
This change introduces two new configuration options for the views,
sig0key-checks-limit and sig0message-checks-limit, which define how
many keys are allowed to be checked to find a matching key, and how
many message verifications are allowed to take place once a matching
key has been found. The latter protects against expensive cryptographic
operations when there are keys with colliding tags and algorithm
numbers, with default being 2, and the former protects against a bit
less expensive key parsing operations and defaults to 16.
Running jobs which were entered into the isc_quota queue is the
responsibility of the isc_quota_release() function, which, when
releasing a previously acquired quota, checks whether the queue
is empty, and if it's not, it runs a job from the queue without touching
the 'quota->used' counter. This mechanism is susceptible to a possible
hangup of a newly queued job in case when between the time a decision
has been made to queue it (because used >= max) and the time it was
actually queued, the last quota was released. Since there is no more
quotas to be released (unless arriving in the future), the newly
entered job will be stuck in the queue.
Fix the issue by adding checks in both isc_quota_release() and
isc_quota_acquire_cb() to make sure that the described hangup does
not happen. Also see code comments.
Closes#4965
Merge branch '4965-isc_quota-bug-fix' into 'main'
See merge request isc-projects/bind9!10082
Running jobs which were entered into the isc_quota queue is the
responsibility of the isc_quota_release() function, which, when
releasing a previously acquired quota, checks whether the queue
is empty, and if it's not, it runs a job from the queue without touching
the 'quota->used' counter. This mechanism is susceptible to a possible
hangup of a newly queued job in case when between the time a decision
has been made to queue it (because used >= max) and the time it was
actually queued, the last quota was released. Since there is no more
quotas to be released (unless arriving in the future), the newly
entered job will be stuck in the queue.
Fix the wrong memory ordering for 'quota->used', as the relaxed
ordering doesn't ensure that data modifications made by one thread
are visible in other threads.
Add checks in both isc_quota_release() and isc_quota_acquire_cb()
to make sure that the described hangup does not happen. Also see
code comments.
A new option 'min-transfer-rate-in <bytes> <minutes>' has been added
to the view and zone configurations. It can abort incoming zone
transfers which run very slowly due to network related issues, for
example. The default value is set to 10240 bytes in 5 minutes.
Closes#3914
Merge branch '3914-detect-and-restart-stalled-zone-transfers' into 'main'
See merge request isc-projects/bind9!9098
Expose the average transfer rate (in bytes-per-second) during the
last full 'min-transfer-rate-in <bytes> <minutes>' minutes interval.
If no such interval has passed yet, then the overall average rate is
reported instead.
Add a new big zone, run a zone transfer in slow mode, and check
whether the zone transfer gets canceled because 100000 bytes are
not transferred in 5 seconds (as it's running in slow mode).
This new option sets a minimum amount of transfer rate for
an incoming zone transfer that will abort a transfer, which
for some network related reasons run very slowly.
The cache has been updated so that if new data is rejected - for example, because there was already existing data at a higher trust level - then its covering RRSIG will also be rejected.
Closes#5132
Merge branch '5132-improve-cd-behavior' into 'main'
See merge request isc-projects/bind9!9999
Add a new dns_rdataset_equals() function to check whether two
rdatasets are equal in DNSSEC terms.
When an rdataset being cached is rejected because its trust
level is lower than the existing rdataset, we now check to see
whether the rejected data was identical to the existing data.
This allows us to cache a potentially useful RRSIG when handling
CD=1 queries, while still rejecting RRSIGs that would definitely
have resulted in a validation failure.
Rdata slabs used in the QP databases are usually prepended with a slab header, but are sometimes "raw", containing only the rdata and no header. Previously, to allow for them to be used both ways, functions that operated on them took a `reservelen` argument, which would be set to either the header length or to zero, and skipped over that many bytes at the beginning of the buffer. Most such functions were never used on the raw form. To make the code clearer, each of these functions now operates on full slabs with headers, and an alternate "raw" version of the function has been added in cases where that was needed.
In addition, the `dns_rdataslab_merge()` and `_subtract()` functions have been rewritten for clarity and efficiency, and a minor bug has been fixed in `dns_rdataslab_equal()` and `_equalx()`, which could cause an incorrect result if both slabs being compared had zero length.
Merge branch 'each-refactor-rdataslab' into 'main'
See merge request isc-projects/bind9!10084
The "raw" version of the header was used for the noqname and the closest
proofs to save around 152 bytes of the dns_slabheader_t while bringing
an additional complexity. Remove the raw version of the dns_slabheader
API at the slight expense of having unused dns_slabheader_t data sitting
in front of the proofs.
reduce the number of rdata comparisons needed by walking
through the original slab once to determine whether the rdata
in it is duplicated in the slab to be subtracted, and then
write out the rdatas that aren't. previously, this was
done twice: once when determining the size of the target buffer
and then again when copying data into it.
when merging two rdata slabs, we now check once to see
whether an item in the new slab has a duplicate in the
old. previously this was done twice; once to determine the
size of the target buffer required, and then again when
copying the data into it.
we also minimize the number of rdata comparisons necessary,
by remembering which items in the old slab have already been
found to be duplicates.
The function name dns_slabheader_fromrdataset() was too similar
to dns_rdataslab_fromrdataset(). Instead, we now have an rdataset
method 'getheader' which is implemented for slab-type rdatasets.
A new NOHEADER rdataset attribute is set for rdatasets using
raw slabs (i.e., noqname and closest encloser proofs); when
called on rdatasets with that flag set, dns_rdataset_getheader()
returns NULL.
when dns_rdataslab_fromrdataset() is run, in addition to
allocating space for a slab header, it also partially
initializes it, setting the type match rdataset->type and
rdataset->covers, the trust to rdataset->trust, and the TTL to
rdataset->ttl.
there are now two functions for creating an rdataslab from an
rdataset: dns_rdataslab_fromrdataset() creates a full slab (including
space for a slab header), and dns_rdataslab_raw_fromrdataset() creates
a raw slab.
- there are now two functions for getting rdataslab size:
dns_rdataslab_size() is for full slabs and dns_rdataslab_sizeraw()
for raw slabs. there is no longer a need for a reservelen parameter.
- dns_rdataslab_count() also no longer takes a reservelen parameter.
(currently it's never used for raw slabs, so there is no _countraw()
function.)
- dns_rdataslab_rdatasize() has been removed, because
dns_rdataslab_sizeraw() can do the same thing.
- dns_rdataslab_merge() and dns_rdataslab_subtract() both take
slabheader parameters instead of character buffers, and the
reservelen parameter has been removed.
if both rdataslabs being compared have zero length, return true.
also, since these functions are only ever called on slabheaders
with sizeof(dns_slabheader_t) as the reserve length, we can
simplify the API: remove the reservelen argument, and pass the
slabs as type dns_slabheader_t * instead of unsigned char *.
The dns_slabheader object uses the 'next' pointer for two purposes.
In the first header for any given type, 'next' points to the first
header for the next type. But 'down' points to the next header of
the same type, and in that record, 'next' points back up.
This design made the code confusing to read. We now use a union
so that the 'next' pointer can also be called 'up'.
The value returned by http_send_outgoing() is not used anywhere, so we
make it not return anything (void). Probably it is an omission from
older times.
When handling outgoing data, there were a couple of rarely executed
code paths that would not take into account that the callback MUST be
called.
It could lead to potential memory leaks and consequent shutdown hangs.
This commit changes the way how the number of active HTTP streams is
calculated and allows it to scale with the values of the maximum
amount of streams per connection, instead of effectively capping at
STREAM_CLIENTS_PER_CONN.
The original limit, which is intended to define the pipelining limit
for TCP/DoT. However, it appeared to be too restrictive for DoH, as it
works quite differently and implements pipelining at protocol level by
the means of multiplexing multiple streams. That renders each stream
to be effectively a separate connection from the point of view of the
rest of the codebase.
Previously we would limit the amount of incoming data to process based
solely on the presence of not completed send requests. That worked,
however, it was found to severely degrade performance in certain
cases, as was revealed during extended testing.
Now we switch to keeping track of how much data is in flight (or ready
to be in flight) and limit the amount of processed incoming data when
the amount of in flight data surpasses the given threshold, similarly
to like we do in other transports.
In the qpzone implementation of `dns_db_closeversion()`, if there are changed nodes that have no remaining data, delete them.
Closes#5169
Merge branch '5169-qpzone-delete-dead-nodes' into 'main'
See merge request isc-projects/bind9!10089
Removed some code from the cache database implementation that was left over from before it and the zone database implementation were separated.
Merge branch 'each-qpcache-refactor' into 'main'
See merge request isc-projects/bind9!9991
Database versions are not used in cache databases. Some places in
qpcache.c required the version argument to be NULL; others marked it
as UNUSED. Unify all cases to require version to be NULL.
Add new related_headers() function that simplifies the code
flow in qpcache_findrdataset(). Also use check_stale_header() function
to remove code duplication.
Add a helper function both_headers() that unifies the slabheader
matching for simple type: it returns true when both the type and
the matching RRSIG have been found.
The new maybe_update_headers() function unifies the LRU updates to the
slabheaders that was scattered all over the place. More calls to update
headers after bindrdatasets() were also added for completeness.
This removes code duplication between the dual bindrdataset() calls. It
also unifies the handling as there were small differences between the
calls: one variant was checking for !NEGATIVE(found) condition and one
wasn't, and it is technically ok to do the check for all variants.
The check_stale_header() function now updates header_prev directly
so it doesn't have to be handled in the outer loop; it's always
set to the correct value of the previous header in the chain.
some code was left in the cache database implementation after
it was separated from the zone database, and can be cleaned up
and refactored now:
- the DNS_SLABHEADERATTR_IGNORE flag is never set in the cache
- support for loading the cache from was removed, but the add()
function still had a 'loading' flag that's always false
- two different macros were used for checking the
DNS_SLABHEADERATTR_NONEXISTENT flag - EXISTS() and NONEXISTENT().
it's clearer to just use EXISTS().
- the cache doesn't support versions, so it isn't necessary to
walk down the 'down' pointer chain when iterating through the
cache or looking for a header to update. 'down' now only points
to records that are deleted from the cache but have not yet been
purged from memory. this allows us to simplify both the iterator
and the add() function.
The host command was extended to also query for the HTTPS RR type by default.
Merge branch 'feature/main/host-rr-https' into 'main'
See merge request isc-projects/bind9!8642
Unless explicitly specified type from host command, do fourth query for
type HTTPS RR. It is expected it will become more common and some
systems already query that record for every name.
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.
An exit path in the dns_dispatch_add() function fails to get out of
the RCU critical section when returning early. Add the missing
rcu_read_unlock() call.
Merge branch 'aram/add-missing-rcu_read_unlock-in-dns_dispatch_add' into 'main'
See merge request isc-projects/bind9!9561
An exit path in the dns_dispatch_add() function fails to get out of
the RCU critical section when returning early. Add the missing
rcu_read_unlock() call.
On Linux, the system administrator can use Control Group ``cgroup``
mechanism to limit the amount of available memory to the process. This
limit will be honoured when calculating the percentage-based values.
Merge branch 'ondrej/use-uv_get_available_memory-doc' into 'main'
See merge request isc-projects/bind9!9556
On Linux, the system administrator can use Control Group ``cgroup``
mechanism to limit the amount of available memory to the process. This
limit will be honoured when calculating the percentage-based values.
Add the new record type WALLET (262). This provides a mapping from a domain name to a cryptographic currency wallet. Multiple mappings can exist if multiple records exist.
Closes#4947
Merge branch '4947-add-wallet-type-to-named' into 'main'
See merge request isc-projects/bind9!9521
The 'rndc dumpdb' command wasn't reporting errors which
occurred when starting up the database dump process by named,
like, for example, a permission denied error for the
'dump-file' file. This has been fixed. Note, however, that
'rndc dumpdb' performs asynchronous writes, so errors can
also occur during the dumping process, which will not be
reported back to 'rndc', but which will still be logged by
named.
Closes#4944
Merge branch '4944-rndc-dumpdb-do-not-ignore-errors' into 'main'
See merge request isc-projects/bind9!9547
Check that with a preexisting dump file without a write permission
no actual dump happens, and rndc returns an error result.
Also add a couple of missing entries in the cleaning script.
The named_server_dumpdb() function, which is called when a 'rndc dumpdb'
command is issued, returns a 'isc_result_t' result code and it has been
always ignored since its introduction in
eb8713ed94, where it was still called
ns_server_dumpdb(). The orignal reasoning is not preserved, but it could
have been also a simple copy-paste mistake, as there are commands, which
return 'void' and require manually setting 'result = ISC_R_SUCCESS;', as
it was done here. Anyway, named will now return the actual result, and
'rndc' will report an error, when the 'dumpdb' command fails.
Keep the changelog for each version in a separate source file to avoid
rebase conflicts when handling the -S version.
---
Related !9528Closes#4946
Merge branch 'nicki/reorganize-changelog-docfiles-9.21' into 'main'
See merge request isc-projects/bind9!9549
Since the changes aren't tracked in the single changelog.rst file,
generate the changelog to stdout instead, so it can be easily redirected
to the proper file.
This change uses uv_get_available_memory() if available with fallback
to uv_get_constrained_memory() with fallback to uv_get_total_memory().
Merge branch 'ondrej/use-uv_get_available_memory' into 'main'
See merge request isc-projects/bind9!9527
This change uses uv_get_total_memory() to get the memory available to
BIND 9 with possible modification by uv_get_constrained_memory() if the
libuv version is recent enough to honour constraints created by
f.e. cgroups.
Due to the recent improvements to the TCP processing, much higher loads
can be handled by BIND9 without causing client timeouts. The updated
parameters give us useful data for both cold and hot cache testing.
Merge branch 'nicki/increase-tcp-dot-shotgun-load' into 'main'
See merge request isc-projects/bind9!9494
Due to the recent improvements to the TCP processing, much higher loads
can be handled by BIND9 without causing client timeouts. The updated
parameters give us useful data for both cold and hot cache testing.
The ans4 server wasn't shutting down cleanly sometimes. Check that read returns the
expected value for the message length or exit read loop.
Closes#4301
Merge branch '4301-check-read-value-in-ans4' into 'main'
See merge request isc-projects/bind9!9537
The shutdown system test sends queries when named is shutting down, not
in an attempt to get answers but to destabilize the server into a crash.
With isctest.query.udp() defaulting to try up to ten times with a
ten-second timeout to get a response we don't care about from a likely
terminated server, we make the test run much longer than needed because
of retries and long timeouts.
Also, see isc-projects/bind9#4943.
Merge branch 'mnowak/shutdown-downgrade-timeout-and-attempts-arguments' into 'main'
See merge request isc-projects/bind9!9507
The shutdown system test sends queries when named is shutting down, not
in an attempt to get answers but to destabilize the server into a crash.
With isctest.query.udp() defaulting to try up to ten times with a
ten-second timeout to get a response we don't care about from a likely
terminated server, we make the test run much longer than needed because
of retries and long timeouts.
Instead of cooking up our own code for getting the number of available
CPUs for named to use, make use of uv_available_parallelism() from
libuv >= 1.44.0.
Merge branch 'ondrej/use-uv_available_parallelism-if-available' into 'main'
See merge request isc-projects/bind9!9524
The OpenBSD doesn't have sysctlbyname(), but sysctl() can be used to
read the number of online/available CPUs by reading following MIB(s):
[CTL_HW, HW_NCPUONLINE] with fallback to [CTL_HW, HW_NCPU].
Cleanup various checks and cleanups that are available on the all
platforms like sysctlbyname() and various related <sys/*.h> headers
that are either defined in POSIX or available on Linux and all BSDs.
Instead of cooking up our own code for getting the number of available
CPUs for named to use, make use of uv_available_parallelism() from
libuv >= 1.44.0.
Incoming transfers that took longer than 30 seconds would stop reading from the TCP stream and the incoming transfer would be indefinitely stuck causing BIND 9 to hang during shutdown.
This has been fixed and the `max-transfer-time-in` and `max-transfer-idle-in` timeouts are now honoured.
Closes#4949
Merge branch '4949-fix-ignored-and-invalid-dispatch-timeout-in-dns_xfrin' into 'main'
See merge request isc-projects/bind9!9531
The dns_dispatch_add() call in the dns_xfrin unit had hardcoded 30
second limit. This meant that any incoming transfer would be stopped in
it didn't finish within 30 seconds limit. Additionally, dns_xfrin
callback was ignoring the return value from dns_dispatch_getnext() when
restarting the reading from the TCP stream; this could cause transfers
to get stuck waiting for a callback that would never come due to the
dns_dispatch having already been shut down.
Call the dns_dispatch_add() without a timeout and properly handle the
result code from the dns_dispatch_getnext().
The dns_dispatch_add() has timeout parameter that could not be 0 (for
not timeout). Modify the dns_dispatch implementation to accept a zero
timeout for cases where the timeouts are undesirable because they are
managed externally.
The issue of long-running operations potentially blocking query resolution has been fixed. Revert this temporary workaround and restore the number of threadpool threads.
Related #4898
Merge branch '4898-remove-workaround-and-note' into 'main'
See merge request isc-projects/bind9!9530
Log a query response summary using the new category `responses`. Logging can be controlled by the option `responselog` and `rndc responselog`.
Closes#459
Merge branch '459-log-query-response-status' into 'main'
See merge request isc-projects/bind9!9449
Query and response log shares the same flags. Move flags logging out of
log_query to share it with log_response. Use buffer instead of snprintf
to fill flags a bit faster.
Signed-off-by: Petr Menšík <pemensik@redhat.com>
Remove answer flag from log, log instead count of records for each
message section. Include EDNS version and few flags of response. Add
also status of result.
Still does not include body of responses rrset.
when the QP cache was adapted from the RBTDB, some variable names weren't changed, and still incorrectly reference the RBT.
Merge branch 'each-qpcache-nits' into 'main'
See merge request isc-projects/bind9!9515
when the QP cache was adapted from the RBT database, some names
weren't changed. this could be confusing, so let's change them now.
also, we no longer need to include rbt.h.
DNSRPS was the API for a commercial implementation of Response-Policy
Zones that was supposedly better. However, it was never open-sourced
and has only ever been available from a single vendor. This goes against
the principle that the open-source edition of BIND 9 should contain only
features that are generally available and universal.
This commit removes the DNSRPS implementation from BIND 9. It may be
reinstated in the subscription edition if there's enough interest from
customers, but it would have to be rewritten as a plugin (hook) instead
of hard-wiring it again in so many places.
Merge branch 'ondrej/remove-DNSRPS-from-open-source-edition' into 'main'
See merge request isc-projects/bind9!9358
DNSRPS was the API for a commercial implementation of Response-Policy
Zones that was supposedly better. However, it was never open-sourced
and has only ever been available from a single vendor. This goes against
the principle that the open-source edition of BIND 9 should contain only
features that are generally available and universal.
This commit removes the DNSRPS implementation from BIND 9. It may be
reinstated in the subscription edition if there's enough interest from
customers, but it would have to be rewritten as a plugin (hook) instead
of hard-wiring it again in so many places.
Addressed several build and test errors when DNSRPS is enabled.
Merge branch 'each-dnsrps-cleanup' into 'main'
See merge request isc-projects/bind9!9374
Some 'notify' and 'xfer-in' related log messages were logged at the
'general' category instead of their own category. This has been fixed.
Closes#2730
Merge branch '2730-logging-category-for-notify-and-xfer-related-messages' into 'main'
See merge request isc-projects/bind9!9451
If the operating system UDP queue gets full and the outgoing UDP sending
starts to be delayed, BIND 9 could exhibit memory spikes as it tries to
enqueue all the outgoing UDP messages. Try a bit harder to deliver the
outgoing UDP messages synchronously and if that fails, drop the outgoing
DNS message that would get queued up and then timeout on the client side.
Closes#4930
Merge branch '4930-limit-the-UDP-send-queue' into 'main'
See merge request isc-projects/bind9!9506
If the operating system UDP queue gets full and the outgoing UDP sending
starts to be delayed, BIND 9 could exhibit memory spikes as it tries to
enqueue all the outgoing UDP messages. As those are not going to be
delivered anyway (as we argued when we stopped enlarging the operating
system send and receive buffers), try to send the UDP messages directly
using `uv_udp_try_send()` and if that fails, drop the outgoing UDP
message.
We currently set SO_INCOMING_CPU incorrectly, and testing by Ondrej
shows that fixing the issue by setting affinities is worse than letting
the kernel schedule threads without constraints. So we should not set
SO_INCOMING_CPU anymore.
Closes#4936
Merge branch '4936-remove-so-incoming-cpu' into 'main'
See merge request isc-projects/bind9!9497
We currently set SO_INCOMING_CPU incorrectly, and testing by Ondrej
shows that fixing the issue and setting affinities is worse than letting
the kernel schedule threads without constraints. So we should not set
SO_INCOMING_CPU anymore.
The new ForwardOnlyFail statistics channel counter indicates the
number of queries failed due to bad forwarders for 'forward only'
zones.
Related to #1793
Merge branch 'aram/add-new-stats-channel-counter-forwardonlyfail' into 'main'
See merge request isc-projects/bind9!9498
When resolving a zone with a 'forward only' policy, and
finding out that all the forwarders are marked as "bad",
the 'ServerQuota' counter of the statistics channel was
incorrectly increased. This has been fixed.
Closes#1793
Merge branch '1793-serverquota-counter-bug-with-forward-only' into 'main'
See merge request isc-projects/bind9!9493
Check that the fix in the previous commit works and that the
'ServerQuota' counter in the statistics channel is still unset
after a SERVFAIL result in a 'forward only' zone.
The 'all_spilled' local variable in resolver.c:fctx_getaddresses()
is 'true' by default, and only becomes false when there is at least
one successfully found NS address. However, when a 'forward only;'
configuration is used, the code jumps over the part where it looks
for NS addresses and doesn't reset the 'all_spilled' to false, which
results in incorretly increased 'serverquota' statistics variable,
and also in invalid return error code from the function. The result
code error didn't make any differences, because all codes other than
'ISC_R_SUCCESS' or 'DNS_R_WAIT' were treated in the same way, and
the result code was never logged anywhere.
Set the default value of 'all_spilled' to 'false', and only make it
'true' before actually starting to look up NS addresses.
Silence Coverity CID 468757 and 468767 (DATA RACE read not locked) by converting dnssec-signzone to use atomics for statistics counters rather than using a lock.
Closes#4939
Merge branch '4939-remove-stats-lock-from-dnssec-signzone' into 'main'
See merge request isc-projects/bind9!9496
Silence Coverity CID 468757 and 468767 (DATA RACE read not locked)
by converting dnssec-signzone to use atomics for statistics counters
rather than using a lock. This should be marginally faster than
using the lock as well when statistics are requested.
As part of the KeyTrap \[CVE-2023-50387\] mitigation, the DNSSEC CPU-intensive operations were offloaded to a separate threadpool that we use to run other tasks that could affect the networking latency.
If that threadpool is running some long-running tasks like RPZ, catalog zone processing, or zone file operations, it would delay DNSSEC validations to a point where the resolving signed DNS records would fail.
Split the CPU-intensive and long-running tasks into separate threadpools in a way that the long-running tasks don't block the CPU-intensive operations.
Closes#4898
Merge branch '4898-move-offloaded-DNSSEC-to-own-threads' into 'main'
See merge request isc-projects/bind9!9473
Currently, the isc_work API is overloaded. It runs both the
CPU-intensive operations like DNSSEC validations and long-term tasks
like RPZ processing, CATZ processing, zone file loading/dumping and few
others.
Under specific circumstances, when many large zones are being loaded, or
RPZ zones processed, this stops the CPU-intensive tasks and the DNSSEC
validation is practically stopped until the long-running tasks are
finished.
As this is undesireable, this commit moves the CPU-intensive operations
from the isc_work API to the isc_helper API that only runs fast memory
cleanups now.
Add an extra thread that can be used to offload operations that would
affect latency, but are not long-running tasks; those are handled by
isc_work API.
Each isc_loop now has matching isc_helper thread that also built on top
of uv_loop. In fact, it matches most of the isc_loop functionality, but
only the `isc_helper_run()` asynchronous call is exposed.
When verifying a message in an offloaded thread there is a race with
the worker thread which writes to the same buffer. Clone the message
buffer before offloading.
Closes#4929
Merge branch '4929-data-race-in-dns_dnssec_verifymessage-memmove' into 'main'
See merge request isc-projects/bind9!9481
When verifying a message in an offloaded thread there is a race with
the worker thread which writes to the same buffer. Clone the message
buffer before offloading.
Remove the use of "port" when configuring query-source(-v6),
transfer-source(-v6), notify-source(-v6), parental-source(-v6),
etc. Remove the use of source ports for parental-agents.
Also remove the deprecated options use-{v4,v6}-udp-ports and
avoid-{v4,v6}udp-ports.
Closes#3843
Merge branch '3843-remove-deprecated-source-port-options' into 'main'
See merge request isc-projects/bind9!9469
Remove the use of "port" when configuring query-source(-v6),
transfer-source(-v6), notify-source(-v6), parental-source(-v6),
etc. Remove the use of source ports for parental-agents.
Also remove the deprecated options use-{v4,v6}-udp-ports and
avoid-{v4,v6}udp-ports.
When the libxml2 and libjson-c libraries are not supported, the statistics channel can't return anything useful, so it is now disabled. Use of `statistics-channel` in `named.conf` is a fatal error.
Closes#4895
Merge branch '4895-link-style-sheet-to-libxml2-support' into 'main'
See merge request isc-projects/bind9!9423
The `statschannel` system test failed if only one of `libxml2` or `json-c` is
available / configured as checks were being run against the non available
statistics page.
Closes#4919
Merge branch '4919-fix-statschannel-system-test' into 'main'
See merge request isc-projects/bind9!9454
Future port the missing TCP4Clients/TCP6Clients documentation entry.
Commit abc47f5ce4 added two new statistics
counters without documenting them. Add the missing counter descriptions
to the ARM.
(cherry picked from commit cb79240091)
Merge branch 'aydin/tcpclient-doc' into 'main'
See merge request isc-projects/bind9!9078
Commit abc47f5ce4 added two new statistics
counters without documenting them. Add the missing counter descriptions
to the ARM.
(cherry picked from commit cb79240091)
This change allows fallback from an IXFR failure to AXFR when the reason is `DNS_R_TOOMANYRECORDS`. This is because this error condition could be temporary only in an intermediate version of IXFR transactions and it's possible that the latest version of the zone doesn't have that condition. In such a case, the secondary would never be able to update the zone (even if it could) without this fallback.
This fallback behavior is particularly useful with the recently introduced `max-records-per-type` and `max-types-per-name` options: the primary may not have these limitations and may temporarily introduce "too many" records, breaking IXFR. If the primary side subsequently deletes these records, this fallback will help recover the zone transfer failure automatically; without it, the secondary side would first need to increase the limit, which requires more operational overhead and has its own adverse effect.
Closes#4928
Merge branch 'fallback-ixfr-to-axfr-on-toomanyrecords' into 'main'
See merge request isc-projects/bind9!9333
This change allows fallback from an IXFR failure to AXFR when the
reason is DNS_R_TOOMANYRECORDS. This is because this error condition
could be temporary only in an intermediate version of IXFR
transactions and it's possible that the latest version of the zone
doesn't have that condition. In such a case, the secondary would never
be able to update the zone (even if it could) without this fallback.
This fallback behavior is particularly useful with the recently
introduced max-records-per-type and max-types-per-name options:
the primary may not have these limitations and may temporarily
introduce "too many" records, breaking IXFR. If the primary side
subsequently deletes these records, this fallback will help recover
the zone transfer failure automatically; without it, the secondary
side would first need to increase the limit, which requires more
operational overhead and has its own adverse effect.
This change also fixes a minor glitch that DNS_R_TOOMANYRECORDS wasn't
logged in xfrin_fail.
The named process could terminate unexpectedly when processing ACL. This has been fixed.
Closes#4908
Merge branch '4908-acl-assertion-failure' into 'main'
See merge request isc-projects/bind9!9458
The rcu_xchg_pointer() function can be used outside of a critical
section, and usually must be followed by a synchronize_rcu() or
call_rcu() call to detach from the resource, unless if there are
some guarantees in place because of our own reference counting.
`named-checkconf` now takes "-n" to ignore "not configured" errors. This allows named-checkconf to check the syntax of configurations from other builds which have support for more options.
Merge branch '4913-add-option-to-named-checkconf-to-override-notconfigured-flag' into 'main'
See merge request isc-projects/bind9!9446
named-checkconf now takes "-n" to ignore "not configured" errors.
This allows named-checkconf to check the syntax of configurations
from other builds which have support for more options.
This file was initially created for unit testing, but later code was added to generate the file. The static file should have been removed from the git repo.
Closes#4916
Merge branch '4916-skr-unit-test-rm-test-file' into 'main'
See merge request isc-projects/bind9!9450
This file was initially created for unit testing, but later code was
added to generate the file. The static file should have been removed
from the git repo.
The statements that already exist in the grammar can't be created with
the namedconf:statement. Use a plain definition list for these
statements and add a manual anchor for each one so links to them can be
created.
Avoid using the :any: syntax in the definition lists, as that just
creates a link to the duplicate and completely unrelated statement,
which just makes the documentation more confusing.
Introduce this temporary workaround to reduce the impact of long-running
tasks in offload threads which can block the resolution of queries.
Related isc-projects/bind9#4898
Merge branch '4898-workaround-double-threadpool-threads' into 'v9.21.1-release'
See merge request isc-private/bind9!740
If the ZSK has unlimited lifetime, the timing metadata "Inactive" and "Delete" cannot be found and is treated as an error, preventing the zone to be signed. This has been fixed.
Closes#4914
Merge branch '4914-offline-ksk-zsk-lifetime-unlimited-bug' into 'main'
See merge request isc-projects/bind9!9447
Under rare circumstances, named could terminate unexpectedly
when validating a DNSKEY resource record if the validation
was canceled in the meantime. This has been fixed.
Closesisc-projects/bind9#4911
Merge branch '4911-assertion-failure-in-validate_dnskey_dsset_done' into 'v9.21.1-release'
See merge request isc-private/bind9!731
If the ZSK has lifetime unlimited, the timing metadata "Inactive" and
"Delete" cannot be found and is treated as an error. Fix by allowing
these metadata to not exist.
When a validator is already shut down, val->name becomes NULL. We
need to process and keep the ISC_R_CANCELED or ISC_R_SHUTTINGDOWN
result code before calling validate_async_done(), otherwise, when it
is called with the hardcoded DNS_R_NOVALIDSIG result code, it can
cause an assetion failure when val->name (being NULL) is used in
proveunsecure().
Delay relinquishing root privileges until the control channel has been configured, for the benefit of systems that require root to use privileged port numbers. This mostly affects systems without fine-grained privilege systems (i.e., other than Linux).
Closes#4793
Merge branch '4793-bind-9-19-24-not-listening-to-rndc-port-953-on-localhost' into 'main'
Closes#4793
See merge request isc-projects/bind9!9123
On systems where root access is needed to configure privileged
ports, we don't want to fully relinquish root privileges until
after the control channel (which typically runs on port 953) has
been established.
named_os_changeuser() now takes a boolean argument 'permanent'.
This allows us to switch the effective userid temporarily with
named_os_changeuser(false) and restore it with named_os_restoreuser(),
before permanently dropping privileges with named_os_changeuser(true).
Administrators may wish to constrain the set of cores that BIND 9 runs on via the 'taskset', 'cpuset' or 'numactl' programs (or equivalent on other O/S).
If the admin has used taskset, the `named` will now follow to automatically use the given number of CPUs rather than the system wide count.
Closes#4884
Merge branch '4884-use-cpuset-to-get-number-of-cpus' into 'main'
Closes#4884
See merge request isc-projects/bind9!9398
Administrators may wish to constrain the set of cores that BIND 9 runs
on via the 'taskset', 'cpuset' or 'numactl' programs (or equivalent on
other O/S), for example to achieve higher (or more stable) performance
by more closely associating threads with individual NIC rx queues. If
the admin has used taskset, it follows that BIND ought to
automatically use the given number of CPUs rather than the system wide
count.
Co-Authored-By: Ray Bellis <ray@isc.org>
With max-recursion-queries set to 50 the resolver system test was
unstable in the "checking query resolution for a domain with a valid
glueless delegation chain" check as ns1 replied with SERVFAIL.
Closes#4897
Merge branch '4897-resolver-ns1-max-recursion-queries-100' into 'main'
Closes#4897
See merge request isc-projects/bind9!9435
With max-recursion-queries set to 50 the resolver system test was
unstable in the "checking query resolution for a domain with a valid
glueless delegation chain" check as ns1 replied with SERVFAIL.
Use the fact that the database returns the longest matching part of the requested name to find the required NSEC3 record. If there are multiple versions present in the database we may have to search further.
Closes#4460
Merge branch '4460-auth-nsec3-many-labels' into 'main'
Closes#4460
See merge request isc-projects/bind9!9436
Return partial match from dns_db_find/dns_db_find when requested
to short circuit the closest encloser discover process. Most of the
time this will be the actual closest encloser but may not be when
there yet to be committed / cleaned up versions of the zone with
names below the actual closest encloser.
Move lock earlier in the call sequence to address access without lock report.
```
1559 /*
1560 * Caller must be holding the fctx lock.
1561 */
CID 468796: (#1 of 1): Data race condition (MISSING_LOCK)
1. missing_lock: Accessing fctx->state without holding lock fetchctx.lock. Elsewhere, fetchctx.state is written to with fetchctx.lock held 2 out of 2 times.
1562 REQUIRE(fctx->state == fetchstate_done);
1563
1564 FCTXTRACE("sendevents");
1565
1566 LOCK(&fctx->lock);
1567
```
Closes#4902
Merge branch '4902-accessing-fctx-state-without-holding-lock' into 'main'
Closes#4902
See merge request isc-projects/bind9!9427
fctx->state should be read with the lock held.
1559 /*
1560 * Caller must be holding the fctx lock.
1561 */
CID 468796: (#1 of 1): Data race condition (MISSING_LOCK)
1. missing_lock: Accessing fctx->state without holding lock fetchctx.lock.
Elsewhere, fetchctx.state is written to with fetchctx.lock held 2 out of 2 times.
1562 REQUIRE(fctx->state == fetchstate_done);
1563
1564 FCTXTRACE("sendevents");
1565
1566 LOCK(&fctx->lock);
1567
find->status is a private field of dns_adbfind_t so it now has an accessor function and has been made atomic.
Closes#4802
Merge branch '4802-address-potential-tsan-issue-with-find-status' into 'main'
Closes#4802
See merge request isc-projects/bind9!9137
Fetches generated automatically as a result of 'prefetch' are now
exempt from the 'fetches-per-zone' and 'fetches-per-server' quotas.
This should help in maintaining the cache from which query responses
can be given.
Closes#4219
Merge branch '4219-exempt-good-queries-from-fetch-limits' into 'main'
Closes#4219
See merge request isc-projects/bind9!9095
Give prefetches a free pass through the quota so that the cache
entries for popular zones could be updated successfully even if the
quota for is already reached.
Give prefetches a free pass through the quota so that the cache entry
for a popular zone could be updated successfully even if the quota for
it is already reached.
The `malloc_usable_size()` can return size larger than originally allocated and when these sizes disagree the fortifier enabled by `_FORTIFY_SOURCE=3` detects overflow and stops the `named` execution abruptly. Stop using these convenience functions as they are primary used for introspection-only.
Closes#4880
Merge branch '4880-dont-use-malloc_usable_size' into 'main'
Closes#4880
See merge request isc-projects/bind9!9400
Although the nanual page of malloc_usable_size says:
Although the excess bytes can be over‐written by the application
without ill effects, this is not good programming practice: the
number of excess bytes in an allocation depends on the underlying
implementation.
it looks like the premise is broken with _FORTIFY_SOURCE=3 on newer
systems and it might return a value that causes program to stop with
"buffer overflow" detected from the _FORTIFY_SOURCE. As we do have own
implementation that tracks the allocation size that we can use to track
the allocation size, we can stop relying on this introspection function.
Also the newer manual page for malloc_usable_size changed the NOTES to:
The value returned by malloc_usable_size() may be greater than the
requested size of the allocation because of various internal
implementation details, none of which the programmer should rely on.
This function is intended to only be used for diagnostics and
statistics; writing to the excess memory without first calling
realloc(3) to resize the allocation is not supported. The returned
value is only valid at the time of the call.
Remove usage of both malloc_usable_size() and malloc_size() to be on the
safe size and only use the internal size tracking mechanism when
jemalloc is not available.
The cross-version-config-tests job fails when a system test is removed
from the upcoming release. To avoid this, remove the system test also
from the $BIND_BASELINE_VERSION.
See the failure mode at https://gitlab.isc.org/isc-projects/bind9/-/jobs/4668947.
Merge branch 'mnowak/remove-dialup-from-cross-version-config-tests-job' into 'main'
See merge request isc-projects/bind9!9413
The cross-version-config-tests job fails when a system test is removed
from the upcoming release. To avoid this, remove the system test also
from the $BIND_BASELINE_VERSION.
This supports bit-for-bit reproducibility of built documentation.
Closes#4886
Merge branch 'issue-4886/order-preserving-documentation-tags' into 'main'
Closes#4886
See merge request isc-projects/bind9!9399
The 'set' datatype in Python does not provide iteration-order
guarantees related to insertion-order. That means that its
usage in the 'split_csv' helper function during documentation
build can produce nondeterministic results.
That is non-desirable for two reasons: it means that the
documentation output may appear to vary unnecessarily between
builds, and secondly there could be loss-of-information in cases
where tag order in the source documentation is significant.
This patch implements order-preserving de-duplication of tags,
allowing authors to specify tags using intentional priority
ordering, while also removing tags that appear more than once.
1. Set milestone to 'Not released yet' after merge
We will set milestone to actual version number when we actually tag a
particular version. This will get rid of mass MR reassignment when we
do last minute changes to a release plan etc.
2. Adjust No CHANGES and Release Notes MR labels to match gitchangelog
workflow.
Previously CI job for the autobackport bot inherited "interruptible:
true" global configuration. This caused premature termination of the job
when another merge was finished before the autobackport job ran to
completion.
The `dialup` and `heartbeat-interval` options have been removed, along with all code implementing them. Using these options is now a fatal error.
Closes#4237
Merge branch '4237-remove-dialup' into 'main'
Closes#4237
See merge request isc-projects/bind9!8160
The new 'request-ixfr-max-diffs' configuration option sets the
maximum number of incoming incremental zone transfer (IXFR) differences,
exceeding which triggers a full zone transfer (AXFR).
Closes#4389
Merge branch '4389-request-ixfr-max-diffs' into 'main'
Closes#4389
See merge request isc-projects/bind9!9094
Configure a maximum of 3 allowed differences and add 5 new records.
Check that named detected that the differences exceed the allowed
limit and successfully retries with AXFR.
This limits the maximum number of received incremental zone
transfer differences for a secondary server. Upon reaching the
confgiured limit, the secondary aborts IXFR and initiates a full
zone transfer (AXFR).
It is useful when multiple signers are being used
to sign a zone to able to specify a restricted
range of range of key tags that will be used by an
operator to sign the zone. This adds controls to
named (dnssec-policy), dnssec-signzone, dnssec-keyfromlabel and
dnssec-ksr (dnssec-policy) to specify such ranges.
Closes#4830
Merge branch '4830-support-restricted-key-tag-range-when-generating-new-keys' into 'main'
Closes#4830
See merge request isc-projects/bind9!9258
A new argument has been added to dnssec-keygen and dnssec-keyfromlabel
to restrict the tag value of key generated / imported to a particular
range. This is intended to be used by multi-signers.
Co-authored-by: Suzanne Goldlust <sgoldlust@isc.org>
With the release of BIND 9.19.25 this workaround is not needed anymore.
This reverts commit c58dd6e78f.
Merge branch 'mnowak/revert-workaround-temporary-cross-version-config-tests-limitation' into 'main'
See merge request isc-projects/bind9!9065
If there is an algorithm rollover and two keys of different algorithm share the same keytags, then there is a possibility that if we check that a key matches a specific state, we are checking against the wrong key. This has been fixed by not only checking for matching key tag but also key algorithm.
Closes#4878
Merge branch '4878-fix-algorithm-rollover-keytag-conflict-bug' into 'main'
Closes#4878
See merge request isc-projects/bind9!9381
If there is an algorithm rollover and two keys of different algorithm
share the same keytags, then there is a possibility that if we check
that a key matches a specific state, we are checking against the wrong
key.
Fix this by not only checking for matching key id but also key
algorithm.
If there is a keytag conflict between keys with different algorithms,
we need to supply what key algorithm is used so we can get the right
public key.
For clarity, print the algorithm on the found keys after 'check_keys'.
Add a new configuration option `offline-ksk` to enable Offline KSK key management. Signed Key Response (SKR) files created with `dnssec-ksr` (or other program) can now be imported into `named` with the new `rndc skr -import` command. Rather than creating new DNSKEY, CDS and CDNSKEY records and generating signatures covering these types, these records are loaded from the currently active bundle from the imported SKR.
The implementation is loosely based on: https://www.iana.org/dnssec/archive/files/draft-icann-dnssec-keymgmt-01.txtCloses#1128
Merge branch '1128-offline-ksk-rndc-import-skr' into 'main'
Closes#1128
See merge request isc-projects/bind9!9119
Some things we no longer want to do when we are in offline-ksk mode.
1. Don't check for inactive and private keys if the key is a KSK.
2. Don't update the TTL of DNSKEY, CDS and CDNSKEY RRset, these come
from the SKR.
With offline-ksk enabled, we don't run the keymgr because the key
timings are determined by the SKR. We do update the key states but
we derive them from the timing metadata.
Then, we can skip a other tasks in offline-ksk mode, like DS checking
at the parent and CDS synchronization, because the CDS and CDNSKEY
RRsets also come from the SKR.
This added source code stores SKR data. It is loosely based on:
https://www.iana.org/dnssec/archive/files/draft-icann-dnssec-keymgmt-01.txt
A SKR contains a list of signed DNSKEY RRsets. Each change in data
should be stored in a separate bundle. So if the RRSIG is refreshed that
means it is stored in the next bundle. Likewise, if there is a new ZSK
pre-published, it is in the next bundle.
In addition (not mentioned in the draft), each bundle may contain
signed CDS and CDNSKEY RRsets.
Each bundle has an inception time. These will determine when we need
to re-sign or re-key the zone.
Test importing a Signed Key Response. Files should be loaded and once
loaded the correct bundle should be used. Alsoe test cases where the
bundle is not the first bundle in the SKR.
Add the code and documentation required to provide KSR import using
rndc. This is just the command, and the feature is at this point in
time still not implemented.
Prepare the system test for tests that require a server to import
created SKR files. This means the test script needs adjustments to
take into account the directory test files are located.
In addition, the check_keys function is renamed to ksr_check_keys
because the name clashes with check_keys from kasp.sh. It also has an
extra parameter added, offset, that can be used to check ksr files that
are created in the past or future.
Add a new configuration option to enable Offline KSK key management.
Offline KSK cannot work with CSK because it splits how keys with the
KSK and ZSK role operate. Therefore, one key cannot have both roles.
Add a configuration check to ensure this.
This add the command line arguments: `-d` (debug), `-l` (list tests) and `-t test` (run this test) to the unit tests.
Closes#4579
Merge branch '4579-restore-the-ability-to-select-individual-unit-tests' into 'main'
Closes#4579
See merge request isc-projects/bind9!9384
A very rare assertion failure can be triggered when the incoming transfer is either forcefully shut down or it is finished during printing the details about the statistics channel. This has been fixed.
Closes#4860
Merge branch '4860-destroy-xfrin-timers-on-the-loop' into 'main'
Closes#4860
See merge request isc-projects/bind9!9336
There are few places where we attach/detach from the dns_xfrin object
while running on a different thread than the zone's assigned thread -
xfrin_xmlrender() in the statschannel and dns_zone_stopxfr() to name the
two places where it happens now. In the rare case, when the incoming
transfer completes (or shuts down) in the brief period between the other
thread attaches and detaches from the dns_xfrin, the isc_timer_destroy()
calls would be called by the last thread calling the xfrin_detach().
In the worst case, it would be this other thread causing assertion
failure. Move the isc_timer_destroy() call to xfrin_end() function
which is always called on the right thread and to match this move
isc_timer_create() to xfrin_start() - although this other change makes
no difference.
named now prints its initial working directory during startup and the
changed working directory when loading or reloading its configuration
file if it has a valid 'directory' option defined.
Closes#4731
Merge branch '4731-log-workdir-full-path' into 'main'
Closes#4731
See merge request isc-projects/bind9!9362
named now prints its initial working directory during startup and the
changed working directory when loading or reloading its configuration
file if it has a valid 'directory' option defined.
It looks like that all supported systems now have support for
_POSIX_SAVED_IDS, so it's safe to use setegid() and setegid() because
those will not change saved used/group IDs.
Closes#4862
Merge branch '4862-just-use-seteuid-and-setegid' into 'main'
Closes#4862
See merge request isc-projects/bind9!9363
It looks like that all supported systems now have support for
_POSIX_SAVED_IDS, so it's safe to use setegid() and setegid() because
those will not change saved used/group IDs.
If the machine running the job is extra loaded (or extra slow), the
print test in the badcache unit test would fail because the TTL would be
59 (possibly even lower) and the test expects the badcache.out to only
have TTL 60. Refactor the test to check for the expected strings and
check whether the TTL is in the expected range.
Closes#4861
Merge branch '4861-allow-TTL-skew-in-badcache_test' into 'main'
Closes#4861
See merge request isc-projects/bind9!9338
If the machine running the job is extra loaded (or extra slow), the
print test in the badcache unit test would fail because the TTL would be
59 (possibly even lower) and the test expects the badcache.out to only
have TTL 60. Refactor the test to check for the expected strings and
check whether the TTL is in the expected range.
Instead of juggling different logging context, use one single logging
context that gets initialized in the libisc constructor and destroyed in
the libisc destructor.
Closes#4848
Merge branch '4848-refactor-isc_log-api-to-use-single-global-logging-context' into 'main'
Closes#4848
See merge request isc-projects/bind9!9301
The new
isc_log_createandusechannel() function combines following calls:
isc_log_createchannel()
isc_log_usechannel()
calls into a single call that cannot fail and therefore can be used in
places where we know this cannot fail thus simplifying the error
handling.
Remove the complicated mechanism that could be (in theory) used by
external libraries to register new categories and modules with
statically defined lists in <isc/log.h>. This is similar to what we
have done for <isc/result.h> result codes. All the libraries are now
internal to BIND 9, so we don't need to provide a mechanism to register
extra categories and modules.
The isc_log_write1() and isc_log_vwrite1() functions were meant to
de-duplicate the messages sent to the isc_log subsystem. However, they
were never used in an entire code base and the whole mechanism around it
was complicated and very inefficient. Just remove those, there are
better ways to deduplicate syslog messages inside syslog daemons now.
Add isc_logconfig_get() function to get the current logconfig and use
the getter to replace most of the little dancing around setting up
logging in the tools. Thus:
isc_log_create(mctx, &lctx, &logconfig);
isc_log_setcontext(lctx);
dns_log_setcontext(lctx);
...
...use lcfg...
...
isc_log_destroy();
is now only:
logconfig = isc_logconfig_get(lctx);
...use lcfg...
For thread-safety, isc_logconfig_get() should be surrounded by RCU read
lock, but since we never use isc_logconfig_get() in threaded context,
the only place where it is actually used (but not really needed) is
named_log_init().
Instead of juggling different logging context, use one single logging
context that gets initialized in the libisc constructor and destroyed in
the libisc destructor.
The application is still responsible for creating the logging
configuration before using the isc_log API.
This patch is first in the series in a way that it is transparent for
the users of the isc_log API as the isc_log_create() and
isc_log_destroy() are now thin shims that emulate the previous
functionality, but it isc_log_create() will always return internal
isc__lctx pointer and isc_log_destroy() will actually not destroy the
internal isc__lctx context.
Signed-off-by: Ondřej Surý <ondrej@isc.org>
On FreeBSD, the TCP connection callback could spuriously receive
ISC_R_CONNECTIONRESET even when connection to the loopback interface.
Skip the other checks in such case and graciously shutdown the TCP
connection.
Closes#4849
Merge branch '4849-dispatch_test-fails-in-connected_shutdown-on-freebsd' into 'main'
Closes#4849
See merge request isc-projects/bind9!9303
On FreeBSD, the TCP connection callback could spuriously receive
ISC_R_CONNECTIONRESET even when connection to the loopback interface.
Skip the other checks in such case and graciously shutdown the TCP
connection.
Update Internal_use_only-CVE.md checklist changing some tasks and responsibilities reflecting the new ASN process with RT vs SF. Support will now be sending ASNs to customers, although Marketing will announce the new releases (both publicly and to support customers).
Merge branch 'vicky-main-patch-57381' into 'main'
See merge request isc-projects/bind9!9307
When manually handling the release notes (due to rst markup, fixups
etc.), the different MR number for backports causes needless friction.
Remove the reference from release notes and keep it only in changelog
which isn't manually redacted.
To reduce the friction when handling the release notes, it is preferable
to have the sections sorted by issue number, rather than merge order.
Fallback to commit subject line if unavailable (e.g. for changelog
entries).
The shim implementation of setresuid() was wrong - there was a copy and
paste error and it was calling setresgid() instead. This only affects
NetBSD because Linux, FreeBSD and OpenBSD have setresuid() and
setresgid() implementation available from the system library.
Closes#4862
Merge branch '4862-fix-setresuid-shim' into 'main'
Closes#4862
See merge request isc-projects/bind9!9359
The shim implementation of setresuid() was wrong - there was a copy and
paste error and it was calling setresgid() instead. This only affects
NetBSD because Linux, FreeBSD and OpenBSD have setresuid() and
setresgid() implementation available from the system library.
The test_traffic_json and test_traffic_xml occasionally fail when
running under TSAN. This happens in CI and is most likely a result of
some instability that doesn't seem to be easily reproduced.
Closes#4598
Merge branch '4598-mark-statschannel-test-flaky' into 'main'
Closes#4598
See merge request isc-projects/bind9!9293
The test_traffic_json and test_traffic_xml occasionally fail when
running under TSAN. This happens in CI and is most likely a result of
some instability that doesn't seem to be easily reproduced.
Log canceled resolver queries (e.g. when shutting down a hung
fetch) in DEBUG3 level instead of DEBUG1 which is used for the
"unrecognized" result codes.
Closes#4797
Merge branch '4797-rpz_rewrite-add-ISC_R_CANCELED-processing' into 'main'
Closes#4797
See merge request isc-projects/bind9!9148
In rpz system tests, we could leak file if the applying the updates has
failed. Add the missing fclose() before returning.
Merge branch 'ondrej/add-missing-flose-to-rpz-testlib' into 'main'
See merge request isc-projects/bind9!9317
Instead of directly using the result of dirfd() in the unlinkat() call,
check whether the returned file descriptor is actually valid. That
doesn't really change the logic as the unlinkat() would fail with
invalid descriptor anyway, but this is cleaner and will report the right
error returned directly by dirfd() instead of EBADF from unlinkat().
Closes#4853
Merge branch '4853-check-result-of-dirfd-in-isc_log' into 'main'
Closes#4853
See merge request isc-projects/bind9!9316
Instead of directly using the result of dirfd() in the unlinkat() call,
check whether the returned file descriptor is actually valid. That
doesn't really change the logic as the unlinkat() would fail with
invalid descriptor anyway, but this is cleaner and will report the right
error returned directly by dirfd() instead of EBADF from unlinkat().
The getifaddr() works fine for years, so we don't have to
keep the callback to parse /proc/net/if_inet6 anymore.
Closes#4852
Merge branch '4852-handle-errors-from-rewind' into 'main'
Closes#4852
See merge request isc-projects/bind9!9315
The clang-scan 19 has reported that we are ignoring errno after the call
to rewind(). As we don't really care about the result, just silence the
error, the whole code will be removed in the development version anyway
as it is not needed.
New version of clang (19) has introduced a stricter checks when mixing
integer (and float types) with enums. In this case, we used enum {}
as C17 doesn't have constexpr yet. Change the time conversion constants
to be static const unsigned int instead of enum values.
Closes#4845
Merge branch '4845-change-NS_PER_SEC-type-from-enum-to-integer' into 'main'
Closes#4845
See merge request isc-projects/bind9!9313
The contexpr introduced in C23 standard makes perfect sense to be used
instead of preprocessor macros - the symbols are kept, etc. Define
ISC_CONSTEXPR to be `constexpr` for C23 and `static const` for the older
C standards. Use the newly introduced macro for the NS_PER_SEC and
friends time constants.
New version of clang (19) has introduced a stricter checks when mixing
integer (and float types) with enums. In this case, we used enum {}
as C17 doesn't have constexpr yet. Change the time conversion constants
to be static const unsigned int instead of enum values.
The new TSAN images, the TSAN-enabled images install libraries to
opt/tsan, synchronize the configure options and CFLAGS between gcc:tsan
and clang:tsan images and set the PKG_CONFIG_PATH to /opt/tsan/lib.
Additionally, drop Debian bullseye that's EOL now.
Merge branch 'ondrej/use-staging-tsan-images' into 'main'
See merge request isc-projects/bind9!9324
The new TSAN images, the TSAN-enabled images install libraries to
/opt/tsan, synchronize the configure options and CFLAGS between gcc:tsan
and clang:tsan images and set the PKG_CONFIG_PATH to /opt/tsan/lib.
Check if 'lctx->logconfig' is NULL before using it in isc_log_doit(),
because it's possible that isc_log_destroy() was already called, e.g.
when a 'call_rcu' function wants to log a message during shutdown.
Closes#4842
Merge branch '4842-shutdown-crash-in-isc_log_doit' into 'main'
Closes#4842
See merge request isc-projects/bind9!9297
Check if 'lctx->logconfig' is NULL before using it in isc_log_doit(),
because it's possible that isc_log_destroy() was already called, e.g.
when a 'call_rcu' function wants to log a message during shutdown.
GCC 11.1+ emits a note during compilation when there are 64-bit
atomic fields in a structure, because it fixed a compiler bug
by changing the alignment of such fields, which caused ABI change.
Add -Wno-psabi to CFLAGS for such builds in order to silence the
warning. That shouldn't be a problem since we don't expose our
structures to the outside.
Closes#4841
Merge branch '4841-alignment-of-_Atomic-long-long-unsigned-int' into 'main'
Closes#4841
See merge request isc-projects/bind9!9319
GCC 11.1+ emits a note during compilation when there are 64-bit
atomic fields in a structure, because it fixed a compiler bug
by changing the alignment of such fields, which caused ABI change.
Add -Wno-psabi to CFLAGS for such builds in order to silence the
warning. That shouldn't be a problem since we don't expose our
structures to the outside.
The build system ensures it is always included for every source file.
Merge branch 'aydin/no-include-config-h' into 'main'
See merge request isc-projects/bind9!9320
Instead of keeping the whole array of test_node_t objects, just keep the
hashvalues and generated the rest of the test_node_t on the fly. The
test still works this way and the file size has been reduced from 2M to
90k.
Closes#4851
Merge branch '4851-generate-problematic-isc_hashmap-test-data' into 'main'
Closes#4851
See merge request isc-projects/bind9!9318
Instead of keeping the whole array of test_node_t objects, just keep the
hashvalues and generated the rest of the test_node_t on the fly. The
test still works this way and the file size has been reduced from 2M to
90k.
Change the placement of the attributes to match the existing usage in
other places (after the declaration).
Merge branch 'ondrej/ISC_CONSTRUCTOR-style' into 'main'
See merge request isc-projects/bind9!9291
Since changelog entries are now generated from MR title&description,
they aren't sanity checked during a regular docs build. If these contain
special sequences that will be interpreted by sphinx, it might result in
breakage that would have to be amended manually.
Add a CI check to test a doc build with changelog after the MR is merged
to ensure that the docs can be built when generating changelog from
pristine git contents.
Related #4847
Merge branch 'nicki/add-changelog-entry-check' into 'main'
See merge request isc-projects/bind9!9294
Some distributions (notably, debian bookworm) have deprecated the
`python` interpreter in favor of `python3`. Since our scripts are
python3 anyway, use the proper numbered version in shebang to make
scripts easily executable.
Since changelog entries are now generated from MR title&description,
they aren't sanity checked during a regular docs build. If these contain
special sequences that will be interpreted by sphinx, it might result in
breakage that would have to be amended manually.
Add a CI check to test a doc build with changelog after the MR is merged
to ensure that the docs can be built when generating changelog from
pristine git contents.
When the round robin hashing reorders the map entries on deletion, we
were adjusting the iterator table size only when the reordering was
happening at the internal table boundary. The iterator table size had
to be reduced by one to prevent seeing the entry that resized on
position [0] twice because it migrated to [iter->size - 1] position.
However, the same thing could happen when the same entry migrates a
second time from [iter->size - 1] to [iter->size - 2] position (and so
on) because the check that we are manipulating the entry just in the [0]
position was insufficient. Instead of checking the position [pos == 0],
we now check that the [pos % iter->size == 0], thus ignoring all the
entries that might have moved back to the end of the internal table.
Closes#4838
Merge branch '4838-fix-assertion-failure-in-hashmap-deletion-iterator' into 'main'
Closes#4838
See merge request isc-projects/bind9!9292
When iterating through the old internal hashmap table, skip all the
nodes that have been already migrated to the new table. We know that
all positions with index less than .hiter are NULL.
When the round robin hashing reorders the map entries on deletion, we
were adjusting the iterator table size only when the reordering was
happening at the internal table boundary. The iterator table size had
to be reduced by one to prevent seeing the entry that resized on
position [0] twice because it migrated to [iter->size - 1] position.
However, the same thing could happen when the same entry migrates a
second time from [iter->size - 1] to [iter->size - 2] position (and so
on) because the check that we are manipulating the entry just in the [0]
position was insufficient. Instead of checking the position [pos == 0],
we now check that the [pos % iter->size == 0], thus ignoring all the
entries that might have moved back to the end of the internal table.
Add second iterator test with a well-known configuration of the nodes
array that causes the last element of the array to migrate two times to
the previous node.
Ensure the issue number in changelog isn't accidentally removed for backport MRs.
Merge branch 'nicki/fix-gitchangelog-replacement-regexs' into 'main'
See merge request isc-projects/bind9!9295
Prior to this change, the issue number could be accidentally removed by
the `Backport of` text, depending on the order of the MR description
contents. Ensure all the removals for text in MR descriptions happen
first, and only then run the replacement regex for issue number, which
appends it to the end of the last non-empty line (which will no longer
be removed).
The only removals that happen after the replacement are guaranteed to
always happen after the end of MR description, since they're
auto-generated by gitlab when the merge commit is created, thus won't
affect the line with the issue number.
Also remove the needless isc-private/bind9 replacement. References
to private MRs are already removed by the very first regex.
As we now setup the logging very early, parsing the default config would
always print warnings about experimental (and possibly deprecated)
options in the default config. This would even mess with commands like
`named -V` and it is also wrong to warn users about using experimental
options in the default config, because they can't do anything about
this. Add CFG_PCTX_NODEPRECATED and CFG_PCTX_NOEXPERIMENTAL options
that we can pass to cfg parser and silence the early warnings caused by
using experimental options in the default config.
Merge branch 'ondrej/silence-warnings-from-default-config' into 'main'
See merge request isc-projects/bind9!9304
As we now setup the logging very early, parsing the default config would
always print warnings about experimental (and possibly deprecated)
options in the default config. This would even mess with commands like
`named -V` and it is also wrong to warn users about using experimental
options in the default config, because they can't do anything about
this. Add CFG_PCTX_NODEPRECATED and CFG_PCTX_NOEXPERIMENTAL options
that we can pass to cfg parser and silence the early warnings caused by
using experimental options in the default config.
The code in conftools/ directory hasn't been touched since 2000.
Nobody knows what it does and nobody even knows how to build it
or test it. Just remove the whole directory.
Merge branch 'ondrej/remove-outdated-perllib-integration' into 'main'
See merge request isc-projects/bind9!9302
The code in conftools/ directory hasn't been touched since 2000.
Nobody knows what it does and nobody even knows how to build it
or test it. Just remove the whole directory.
OpenSSL has added support for deterministic ECDSA (RFC 6979) with
version 3.2.
Use it by default as it removes arguably its most fragile side of ECDSA.
The derandomization doesn't pose a risk for DNS usecases and is allowed by FIPS 186-5.
Closes https://gitlab.isc.org/isc-projects/bind9/-/issues/299
Merge branch '299-change-ecdsa-to-deterministic-usage-elliptic-curve-digital-signature-algorithm-rfc-6979' into 'main'
Closes#299
See merge request isc-projects/bind9!9128
OpenSSL has added support for deterministic ECDSA (RFC 6979) with
version 3.2.
Use it by default as derandomization doesn't pose a risk for DNS
usecases and is allowed by FIPS 186-5.
During a reconfiguration named wasn't reconfiguring catalog zones'
member zones. This has been fixed.
Closesisc-projects/bind9#4733
Merge branch '4733-fix-catz-member-zones-reconfigure' into 'v9.21.0-release'
See merge request isc-private/bind9!695
This condition in catalog zone processing functions is checked only
for sanity, so there is no meaning for a log message that normally
shouldn't be ever logged.
Catalog zone member zones should be reconfigured as all the other
zones during a reconfiguration. Test it by checking whether the newly
added allow-query setting affects a member zone.
The fcount_incr() was not increasing counter->count when force was set
to true, but fcount_decr() would try to decrease the counter leading to
underflow and assertion failure. Swap the order of the arguments in the
condition, so the !force is evaluated after incrementing the .count.
Closes#4846
Merge branch '4786-forced-fcount_incr-should-still-increment-count-and-allowed-fix' into 'main'
Closes#4846
See merge request isc-projects/bind9!9298
The fcount_incr() was not increasing counter->count when force was set
to true, but fcount_decr() would try to decrease the counter leading to
underflow and assertion failure. Swap the order of the arguments in the
condition, so the !force is evaluated after incrementing the .count.
LaTeX in CI and on ReadTheDocs [fails][1] to render a PDF version of ARM if
the Changelog section is included. The running theory is that the
verbatim section of more than twenty thousand lines is too big to meet
LaTeX self-imposed constraints, and it fails with:
! TeX capacity exceeded, sorry [main memory size=5000000].
Or it just hangs if extra_mem_bot=30000000 is set in
/etc/texmf/texmf.d/01main_memory_bump.cnf:
! Dimension too large.
\fb@put@frame ...p \ifdim \dimen@ >\ht \@tempboxa
\fb@putboxa #1\fb@afterfra...
l.56913 \end{sphinxVerbatim}
Make each BIND 9 release a separate code block to work around the issue.
Further split up the sections for some exceptionally large releases, for
the same reason.
[1]: https://gitlab.isc.org/isc-projects/bind9/-/jobs/4584011
Merge branch 'mnowak/fix-arm-changelog-section' into 'main'
See merge request isc-projects/bind9!9266
LaTeX in CI and on ReadTheDocs fails to render a PDF version of ARM if
the Changelog section is included. The running theory is that the
verbatim section of more than twenty thousand lines is too big to meet
LaTeX self-imposed constraints, and it fails with:
! TeX capacity exceeded, sorry [main memory size=5000000].
Or it just hangs if extra_mem_bot=30000000 is set in
/etc/texmf/texmf.d/01main_memory_bump.cnf:
! Dimension too large.
\fb@put@frame ...p \ifdim \dimen@ >\ht \@tempboxa
\fb@putboxa #1\fb@afterfra...
l.56913 \end{sphinxVerbatim}
Make each BIND 9 release a separate code block to work around the issue.
Further split up the sections for some exceptionally large releases, for
the same reason.
Since the enable_fips_mode() now resides inside the isc_tls unit, BIND 9
would fail to compile when FIPS mode was enabled as the DST subsystem
logging functions were missing.
Move the crypto library logging functions from the openssl_link unit to
isc_tls unit and enhance it, so it can now be used from both places
keeping the old dst__openssl_toresult* macros alive.
Merge branch 'ondrej/move-openssl-logging-to-isc_tls-unit' into 'main'
See merge request isc-projects/bind9!9286
Since the enable_fips_mode() now resides inside the isc_tls unit, BIND 9
would fail to compile when FIPS mode was enabled as the DST subsystem
logging functions were missing.
Move the crypto library logging functions from the openssl_link unit to
isc_tls unit and enhance it, so it can now be used from both places
keeping the old dst__openssl_toresult* macros alive.
There were cases in resolver.c when the `max-recursion-queries` quota was ineffective. It was possible to craft zones that would cause a resolver to waste resources by sending excessive queries while attempting to resolve a name. This has been addressed by correcting errors in the implementation of `max-recursion-queries`, and by reducing the default value from 100 to 32.
In addition, a new `max-query-restarts` option has been added which limits the number of times a recursive server will follow CNAME or DNAME records before terminating resolution. This was previously a hard-coded limit of 16, and now defaults to 11.
Closes#4741
Merge branch '4741-reclimit-restarts' into 'main'
Closes#4741
See merge request isc-projects/bind9!9281
implement, document, and test the 'max-query-restarts' option
which specifies the query restart limit - the number of times
we can follow CNAMEs before terminating resolution.
MAX_RESTARTS is no longer hard-coded; ns_server_setmaxrestarts()
and dns_client_setmaxrestarts() can now be used to modify the
max-restarts value at runtime. in both cases, the default is 11.
the number of steps that can be followed in a CNAME chain
before terminating the lookup has been reduced from 16 to 11.
(this is a hard-coded value, but will be made configurable later.)
there were cases in resolver.c when queries for NS records were
started without passing a pointer to the parent fetch's query counter;
as a result, the max-recursion-queries quota for those queries started
counting from zero, instead of sharing the limit for the parent fetch,
making the quota ineffective in some cases.
Use `[[noreturn]]` when compiling with C23 or greater.
The attribute macro name has been capitalized as `NORETURN` as defining it as `noreturn` breaks external headers.
`#define noreturn __attribute__((noreturn))` wasn't used as C11's `stdnoreturn.h`/`_Noreturn` is required to build BIND9 in the first place.
Merge branch 'aydin/noreturn-c23' into 'main'
See merge request isc-projects/bind9!9149
Instead of calling dst_lib_init() and dst_lib_destroy() explicitly by
all the programs, create a separate memory context for the DST subsystem
and use the library constructor and destructor to initialize the DST
internals.
Merge branch 'ondrej/move-dst_lib_init-and-destroy-to-lib-ctor-dtor' into 'main'
See merge request isc-projects/bind9!9254
Instead of calling dst_lib_init() and dst_lib_destroy() explicitly by
all the programs, create a separate memory context for the DST subsystem
and use the library constructor and destructor to initialize the DST
internals.
The new Fedora 40 TSAN images use libuv, urcu and OpenSSL libraries compiled with ThreadSanitizer. This (in theory) should enable better detection of memory races in those (most important) libraries.
Merge branch 'ondrej/test-new-tsan-images' into 'main'
See merge request isc-projects/bind9!9264
When the SSL object was destroyed, it would invalidate all SSL_SESSION
objects including the cached, but not yet used, TLS session objects.
Properly disassociate the SSL object from the SSL_SESSION before we
store it in the TLS session cache, so we can later destroy it without
invalidating the cached TLS sessions.
Closes#4834
Merge branch '4834-detach-SSL-from-cached-SSL_SESSION' into 'main'
Closes#4834
See merge request isc-projects/bind9!9271
When the SSL object was destroyed, it would invalidate all SSL_SESSION
objects including the cached, but not yet used, TLS session objects.
Properly disassociate the SSL object from the SSL_SESSION before we
store it in the TLS session cache, so we can later destroy it without
invalidating the cached TLS sessions.
Co-authored-by: Ondřej Surý <ondrej@isc.org>
Co-authored-by: Artem Boldariev <artem@isc.org>
Co-authored-by: Aram Sargsyan <aram@isc.org>
When TLS connection (TLSstream) connection was accepted, the children
listening socket was not attached to sock->server and thus it could have
been freed before all the accepted connections were actually closed.
In turn, this would cause us to call isc_tls_free() too soon - causing
cascade errors in pending SSL_read_ex() in the accepted connections.
Properly attach and detach the children listening socket when accepting
and closing the server connections.
Closes#4833
Merge branch '4833-tlssock-needs-to-attach-to-child-tlslistener' into 'main'
Closes#4833
See merge request isc-projects/bind9!9270
When TLS connection (TLSstream) connection was accepted, the children
listening socket was not attached to sock->server and thus it could have
been freed before all the accepted connections were actually closed.
In turn, this would cause us to call isc_tls_free() too soon - causing
cascade errors in pending SSL_read_ex() in the accepted connections.
Properly attach and detach the children listening socket when accepting
and closing the server connections.
Missing file util/dtrace.sh prevented builds on system without dtrace utility.
This has been corrected.
Fixes: #4835
Merge branch 'pspacek/gitattribute-fixes' into 'main'
Closes#4835
See merge request isc-projects/bind9!9262
Ensure that system tests can be executed without Python hypothesis
package.
Closes#4831
Merge branch '4831-isctest-make-hypothesis-optional' into 'main'
Closes#4831
See merge request isc-projects/bind9!9265
Query responses should contain the question section with some exceptions. Dig was not reporting this.
Closes#4808
Merge branch '4808-have-dig-report-missing-question-section-in-axfr-response' into 'main'
Closes#4808
See merge request isc-projects/bind9!9233
The question section should be present in the first AXFR/IXFR
response and in other QUERY responses unless no question was sent.
Issue a warning if the question section is not present.
The OpenSSL 1.x Engines support has been deprecated in the OpenSSL 3.x
and is going to be removed from the upstream OpenSSL. Remove the OpenSSL
Engine support from BIND 9 in favor of OpenSSL 3.x Providers.
Closes#4828
Merge branch '4828-remove-OpenSSL-engine-support' into 'main'
Closes#4828
See merge request isc-projects/bind9!9252
OpenSSL_version() first appeared in OpenSSL 1.1.0 and have been
available since LibreSSL 2.7.1 and OpenBSD 6.3, thus we can remove the
compatibility shims with older versions that are not supported anymore.
Since the support for OpenSSL Engines has been removed, we can now also
remove the checks for OPENSSL_API_LEVEL; The OpenSSL 3.x APIs will be
used when compiling with OpenSSL 3.x, and OpenSSL 1.1.xx APIs will be
used only when OpenSSL 1.1.x is used.
The OpenSSL 1.x Engines support has been deprecated in the OpenSSL 3.x
and is going to be removed. Remove the OpenSSL Engine support in favor
of OpenSSL Providers.
The isc_mem_create() in printversion() was created outside of an #ifdef
HAVE_GEOIP, but destroyed inside the #ifdef; move it to the outside of
the #ifdef where it belongs.
This is really a nit as we immediately exit() after printing the
versions, but I found it and it would bug me for the rest of my life.
Merge branch 'ondrej/fix-missing-isc_mem_destroy-in-printversion' into 'main'
See merge request isc-projects/bind9!9255
The isc_mem_create() in printversion() was created outside of an #ifdef
HAVE_GEOIP, but destroyed inside the #ifdef; move it to the outside of
the #ifdef where it belongs.
This is really a nit as we immediately exit() after printing the
versions, but I found it and it would bug me for the rest of my life.
Fix an assertion failure that could happen as a result of data race between free_gluetable() and addglue() on the same headers.
Closes#4691
Merge branch '4691-fix-data-race-between-free_gluetable-and-addglue' into 'main'
Closes#4691
See merge request isc-projects/bind9!9126
when searching the cache for a node so that we can delete an rdataset, it isn't necessary to set the 'create' flag. if the
node doesn't exist yet, we won't be able to delete anything from it anyway.
Merge branch 'each-minor-findnode-refactor' into 'main'
See merge request isc-projects/bind9!9158
When adding glue to the header, we add header to the wait-free stack to
be cleaned up later which sets wfc_node->next to non-NULL value. When
the actual cleaning happens we would only cleanup the .glue_list, but
since the database isn't locked for the time being, the headers could be
reused while cleaning the existing glue entries, which creates a data
race between database versions.
Revert the code back to use per-database-version hashtable where keys
are the node pointers. This allows each database version to have
independent glue cache table that doesn't affect nodes or headers that
could already "belong" to the future database version.
when searching the cache for a node so that we can delete an
rdataset, it is not necessary to set the 'create' flag. if the
node doesn't exist yet, we then we won't be able to delete
anything from it anyway.
dns_difftuple_create() could only return success, so change
its type to void and clean up all the calls to it.
Merge branch 'each-difftuple-create-cannot-fail' into 'main'
See merge request isc-projects/bind9!9151
dns_difftuple_create() could only return success, so change
its type to void and clean up all the calls to it.
other functions that only returned a result value because of it
have been cleaned up in the same way.
When a priming query is complete, it's currently logged at level ISC_LOG_DEBUG(1), regardless of success or failure. We are now raising it to ISC_LOG_NOTICE in the case of failure. [GL #3516]
Closes#3516
Merge branch '3516-log-priming-errors' into 'main'
Closes#3516
See merge request isc-projects/bind9!9121
when a priming query is complete, it's currently logged at
level ISC_LOG_DEBUG(1), regardless of success or failure. we
are now raising it to ISC_LOG_NOTICE in the case of failure.
There isn't a realistic reason to ever use e = 4294967297. Fortunately
its codepath wasn't reachable to users and can be safetly removed.
Keep in mind the `dns_key_generate` header comment was outdated. e = 3
hasn't been used since 2006 so there isn't a reason to panic. The
toggle was the public exponents between 65537 and 4294967297.
Merge branch 'aydin/rsa-cleanup' into 'main'
See merge request isc-projects/bind9!9133
There isn't a realistic reason to ever use e = 4294967297. Fortunately
its codepath wasn't reachable to users and can be safetly removed.
Keep in mind the `dns_key_generate` header comment was outdated. e = 3
hasn't been used since 2006 so there isn't a reason to panic. The
toggle was the public exponents between 65537 and 4294967297.
CRC-64 has been added for map files. Now that the map file format has
been removed, there isn't a reason to keep the implementation.
Merge branch 'aydin/cleanup-crc' into 'main'
See merge request isc-projects/bind9!9135
The previous work in this area was led by the belief that we might be
calling call_rcu() from within call_rcu() callbacks. After carefully
checking all the current callback, it became evident that this is not
the case and the problem isn't enough rcu_barrier() calls, but something
entirely else.
Call the rcu_barrier() just once as that's enough and the multiple
rcu_barrier() calls will not hide the real problem anymore, so we can
find it.
Merge branch '4607-call-a-single-rcu_barrier' into 'main'
See merge request isc-projects/bind9!9134
The previous work in this area was led by the belief that we might be
calling call_rcu() from within call_rcu() callbacks. After carefully
checking all the current callback, it became evident that this is not
the case and the problem isn't enough rcu_barrier() calls, but something
entirely else.
Call the rcu_barrier() just once as that's enough and the multiple
rcu_barrier() calls will not hide the real problem anymore, so we can
find it.
OpenSSL 1.1.1 or better (or equivalent LibreSSL version) is now required to compile BIND 9.
Closes#2806
Merge branch '2806-remove-ax_check_openssl' into 'main'
Closes#2806
See merge request isc-projects/bind9!9110
Since the minimal OpenSSL version is now OpenSSL 1.1.1, remove all kind
of OpenSSL shims and checks for functions that are now always present in
the OpenSSL libraries.
Co-authored-by: Ondřej Surý <ondrej@isc.org>
Co-authored-by: Aydın Mercan <aydin@isc.org>
OpenSSL supports pkg-config method since the 0.9.8 version and we
already require pkg-config for other mandatory libraries. Also
the way the AX_CHECK_OPENSSL macro was integrated into the configure
script was confusing - the macro would be used only if the libcrypto.pc
and libssl.pc file are not usable, so calling ./configure
--with-openssl=/usr/local would have no effect when PKG_CHECK_MODULES
would be successful.
As BIND 9.20 does not support RHEL/CentOS 7 which just reach
end-of-life, we can safely bump the OpenSSL requirements to version
1.1.1, which in turn will allow us to simplify our OpenSSL integration.
Checking the version of `named-checkconf` would end with assertion failure. This has been fixed.
Closes#4827
Merge branch '4827-cleanup-dst-only-if-initialized' into 'main'
Closes#4827
See merge request isc-projects/bind9!9243
The dst_lib_destroy() should be called only if dst_lib_init() was called
before. In named-checkconf, that is guarded by dst_cleanup variable
that was erroneously set to true by default. Set the dst_cleanup to
'false' by default.
An assertion failure triggers when the TSIG has valid cryptographic signature, but the time is invalid. This can happen when the times between the primary and secondary servers are not synchronised.
Closes#4811
Merge branch '4811-fix-isc_buffer_putuint48-buffer-size-requirement' into 'main'
Closes#4811
See merge request isc-projects/bind9!9234
Add a system test that sets TSIG fudge to 0, waits three seconds and
then sends signed message to the server. This tests the path where the
time difference between the client and the server is outside of the TSIG
fudge value.
The tsig unit test was only testing if everything went ok, but it was
not testing whether the error paths work. Add two more unit tests - one
uses the time outside of the TSIG skew, and the second trashes the
signature with random data.
When putting the 48-bit number into a fixed-size buffer that's exactly 6
bytes, the assertion failure would occur as the 48-bit number is
internally represented as 64-bit number and the code was checking if
there is enough space for `sizeof(val)`. This causes assertion failure
when otherwise valid TSIG signature has a bad timing information.
Specify the size of the argument explicitly, so the 48-bit number
doesn't require 8-byte long buffer.
The fcount_incr() was incorrectly skipping the accounting for the
fetches-per-zone if the force argument was set to true. We want to skip
the accounting only when the fetches-per-zone is completely disabled,
but for individual names we need to do the accounting even if we are
forcing the result to be success.
Closes#4786
Merge branch '4786-forced-fcount_incr-should-still-increment-count-and-allowed' into 'main'
Closes#4786
See merge request isc-projects/bind9!9115
The fcount_incr() was incorrectly skipping the accounting for the
fetches-per-zone if the force argument was set to true. We want to skip
the accounting only when the fetches-per-zone is completely disabled,
but for individual names we need to do the accounting even if we are
forcing the result to be success.
The system tests were overriding the local locale by setting LANG to C.
This does not override the locale in case there are individual LC_<*>
variables like LC_CTYPE explicitly set.
Use LC_ALL=C instead which is the proper way of overriding all currently
set locales.
Merge branch 'ondrej/use-LC_ALL-not-LANG' into 'main'
See merge request isc-projects/bind9!9109
The system tests were overriding the local locale by setting LANG to C.
This does not override the locale in case there are individual LC_<*>
variables like LC_CTYPE explicitly set.
Use LC_ALL=C instead which is the proper way of overriding all currently
set locales.
The tlsstream part of the isc_nmsocket_t gets initialized via designater
initializer and doesn't need the extra memset() later; just remove it.
Merge branch 'ondrej/remove-superfluous-memset-in-isc_nmsocket_init' into 'main'
See merge request isc-projects/bind9!9120
The PTHREAD_MUTEX_ADAPTIVE_NP and PTHREAD_MUTEX_ERRORCHECK_NP are
usually not defines, but enum values, so simple preprocessor check
doesn't work.
Check for PTHREAD_MUTEX_ADAPTIVE_NP from the autoconf AS_COMPILE_IFELSE
block and define HAVE_PTHREAD_MUTEX_ADAPTIVE_NP. This should enable
adaptive mutex on Linux and FreeBSD.
As PTHREAD_MUTEX_ERRORCHECK actually comes from POSIX and Linux glibc
does define it when compatibility macros are being set, we can just use
PTHREAD_MUTEX_ERRORCHECK instead of PTHREAD_MUTEX_ERRORCHECK_NP.
Merge branch 'ondrej/fix-adaptive-mutex-use' into 'main'
See merge request isc-projects/bind9!9111
The PTHREAD_MUTEX_ADAPTIVE_NP and PTHREAD_MUTEX_ERRORCHECK_NP are
usually not defines, but enum values, so simple preprocessor check
doesn't work.
Check for PTHREAD_MUTEX_ADAPTIVE_NP from the autoconf AS_COMPILE_IFELSE
block and define HAVE_PTHREAD_MUTEX_ADAPTIVE_NP. This should enable
adaptive mutex on Linux and FreeBSD.
As PTHREAD_MUTEX_ERRORCHECK actually comes from POSIX and Linux glibc
does define it when compatibility macros are being set, we can just use
PTHREAD_MUTEX_ERRORCHECK instead of PTHREAD_MUTEX_ERRORCHECK_NP.
When automatic-interface-scan is disabled, the route socket was still
being opened. Add new API to connect / disconnect from the route socket
only as needed.
Additionally, move the block that disables periodic interface rescans to
a place where it actually have access to the configuration values.
Previously, the values were being checked before the configuration was
loaded.
Closes#4757
Merge branch '4757-dont-open-routing-socket-if-not-needed' into 'main'
Closes#4757
See merge request isc-projects/bind9!9122
When automatic-interface-scan is disabled, the route socket was still
being opened. Add new API to connect / disconnect from the route socket
only as needed.
Additionally, move the block that disables periodic interface rescans to
a place where it actually have access to the configuration values.
Previously, the values were being checked before the configuration was
loaded.
I split this into two commits, one for the actual newline removal, and one for issues I found, ruining the yaml output when some errors were outputted.
Closes: #4772
Merge branch 'yaml-indent' into 'main'
Closes#4772
See merge request isc-projects/bind9!9112
Add INSIST to fail if the multiplication would cause the variables to overflow.
Closes#4798
Merge branch '4798-cid-498025-and-cid-498031-overflowed-constant-integer_overflow' into 'main'
Closes#4798
See merge request isc-projects/bind9!9131
If bufsize overflows we will have an infinite loop. In practice
this will not happen unless we have made a coding error. Add an
INSIST to detect this condition.
181retry:
182 isc_buffer_allocate(mctx, &b, bufsize);
183 result = dns_rdata_totext(rdata, NULL, b);
184 if (result == ISC_R_NOSPACE) {
185 isc_buffer_free(&b);
CID 498031: (#1 of 1): Overflowed constant (INTEGER_OVERFLOW)
overflow_const: Expression bufsize, which is equal to 0, overflows
the type that receives it, an unsigned integer 32 bits wide.
186 bufsize *= 2;
187 goto retry;
188 }
If size overflows we will have an infinite loop. In practice
this will not happen unless we have made a coding error. Add
an INSIST to detect this condition.
181 while (!done) {
182 isc_buffer_allocate(mctx, &b, size);
183 result = dns_rdata_totext(rdata, NULL, b);
184 if (result == ISC_R_SUCCESS) {
185 printf("%.*s\n", (int)isc_buffer_usedlength(b),
186 (char *)isc_buffer_base(b));
187 done = true;
188 } else if (result != ISC_R_NOSPACE) {
189 check_result(result, "dns_rdata_totext");
190 }
191 isc_buffer_free(&b);
CID 498025: (#1 of 1): Overflowed constant (INTEGER_OVERFLOW)
overflow_const: Expression size, which is equal to 0, overflows the type that
receives it, an unsigned integer 32 bits wide.
192 size *= 2;
193 }
Decrementing optlen immediately before calling continue is unneccesary
and inconsistent with the rest of dns_message_pseudosectiontoyaml
and dns_message_pseudosectiontotext. Coverity was also reporting
an impossible false positive overflow of optlen (CID 499061).
4176 } else if (optcode == DNS_OPT_CLIENT_TAG) {
4177 uint16_t id;
4178 ADD_STRING(target, "; CLIENT-TAG:");
4179 if (optlen == 2U) {
4180 id = isc_buffer_getuint16(&optbuf);
4181 snprintf(buf, sizeof(buf), " %u\n", id);
4182 ADD_STRING(target, buf);
CID 499061: (#1 of 1): Overflowed constant (INTEGER_OVERFLOW)
overflow_const: Expression optlen, which is equal to 65534, underflows
the type that receives it, an unsigned integer 16 bits wide.
4183 optlen -= 2;
4184 POST(optlen);
4185 continue;
4186 }
4187 } else if (optcode == DNS_OPT_SERVER_TAG) {
Merge branch 'marka-remove-unnecessary-operations' into 'main'
See merge request isc-projects/bind9!9130
Decrementing optlen immediately before calling continue is unneccesary
and inconsistent with the rest of dns_message_pseudosectiontoyaml
and dns_message_pseudosectiontotext. Coverity was also reporting
an impossible false positive overflow of optlen (CID 499061).
4176 } else if (optcode == DNS_OPT_CLIENT_TAG) {
4177 uint16_t id;
4178 ADD_STRING(target, "; CLIENT-TAG:");
4179 if (optlen == 2U) {
4180 id = isc_buffer_getuint16(&optbuf);
4181 snprintf(buf, sizeof(buf), " %u\n", id);
4182 ADD_STRING(target, buf);
CID 499061: (#1 of 1): Overflowed constant (INTEGER_OVERFLOW)
overflow_const: Expression optlen, which is equal to 65534, underflows
the type that receives it, an unsigned integer 16 bits wide.
4183 optlen -= 2;
4184 POST(optlen);
4185 continue;
4186 }
4187 } else if (optcode == DNS_OPT_SERVER_TAG) {
The minimum allowed value of 'resolver-query-timeout' was lowered to
301 milliseconds instead of the earlier 10000 milliseconds (which is the
default). As earlier, values less than or equal to 300 are converted to
seconds before applying the limit.
Closes#4320
Merge branch '4320-allow-shorter-resolver-query-timeout-configuration' into 'main'
Closes#4320
See merge request isc-projects/bind9!9091
Before there was a gap from 301 to 9999 which would be converted
to 10000 and now there is no such gap.
This settimeout_belowmin test was checking the behavior of a value
in the gap. As there is now no gap left, the minimum is 301 and
anything below that is converted to seconds as before. In order
for this check to still test the "below minimum" behavior, change
the value from 9000 to 300.
Update the settimeout_overmax value test too so it logically aligns
with the minimum value test.
There are use cases for which shorter timeout values make sense.
For example if there is a load balancer which sets RD=1 and
forwards queries to a BIND resolver which is then configured to
talk to backend servers which are not visible in the public NS set.
WIth a shorter timeout value the frontend can give back SERVFAIL
early when backends are not available and the ultimate client will
not penalize the BIND-frontend for non-response.
A new optional argument '-force' has been added to the command channel
command 'rndc retransfer'. When it is specified, named aborts the
ongoing zone transfer (if there is one), and starts a new transfer.
Closes#2299
Merge branch '2299-implement-rndc-force-retransfer' into 'main'
Closes#2299
See merge request isc-projects/bind9!9102
Use a big zone and the slow transfer mode. Initiate a retransfer, wait
several seconds, then initiate a retransfer using a '-force' argument,
which should cancel the previous transfer and start a new one.
The period between the most significant nibble of the encoded IPv4 address and the 2.0.0.2.IP6.ARPA suffix was missing resulting in the wrong name being checked. Add system test for 6to4-self implementation.
Closes#4766
Merge branch '4766-add-system-test-for-6to4-self' into 'main'
Closes#4766
See merge request isc-projects/bind9!9099
The period between the most significant nibble of the IPv4 address
and the 2.0.0.2.IP6.ARPA suffix was missing resulting in the wrong
name being checked.
Remove the false positive "success resolving" log message when QNAME minimisation is in effect and the final result is NXDOMAIN.
Closes#4784
Merge branch '4784-false-qname-minimisation-error-being-reported' into 'main'
Closes#4784
See merge request isc-projects/bind9!9117
In yaml mode we emit a string for each question and record. Certain
names and data could result in invalid yaml being produced. Use single
quote string for all questions and records. This requires that single
quotes get converted to two quotes within the string.
ALPN are defined as 1*255OCTET in RFC 9460. commatxt_fromtext was not
rejecting invalid inputs produces by missing a level of escaping
which where later caught be dns_rdata_fromwire on reception.
These inputs should have been rejected
svcb in svcb 1 1.svcb alpn=\,abc
svcb1 in svcb 1 1.svcb alpn=a\,\,abc
and generated 00 03 61 62 63 and 01 61 00 02 61 62 63 respectively.
The correct inputs to include commas in the alpn requires double
escaping.
svcb in svcb 1 1.svcb alpn=\\,abc
svcb1 in svcb 1 1.svcb alpn=a\\,\\,abc
and generate 04 2C 61 62 63 and 06 61 2C 2C 61 62 63 respectively.
Add a note that 'rndc reload' and 'rndc reconfig' can't change the
querylog option during the runtime of named.
Closes#4801
Merge branch '4801-arm-querylog-clarification' into 'main'
Closes#4801
See merge request isc-projects/bind9!9136
Replace #define DNS_GETDB_ with struct of bools to make
it easier to pretty-print the attributes in a debugger.
Closes#4559
Merge branch '4559-convert-dns_getdb_x-defines-to-1-bit-long-bools' into 'main'
Closes#4559
See merge request isc-projects/bind9!9093
When cutting a long CNAME chain, named was returning NOERROR
instead of SERVFAIL (alongside with a partial answer). This
has been fixed.
Closes#4449
Merge branch '4449-return-servfail-for-a-long-cname-chain' into 'main'
Closes#4449
See merge request isc-projects/bind9!9090
Update the CNAME chain test to correspond to the changed behavior,
because now named returns SERVFAIL when hitting the maximum query
restarts limit (e.g. happening when following a long CNAME chain).
In the current test auth will hit the limit and return partial data
with a SERVFAIL code, while the resolver will return no data with
a SERVFAIL code after auth returns SERVFAIL to it.
Due to the maximum query restart limitation a long CNAME chain
it is cut after 16 queries but named still returns NOERROR.
Return SERVFAIL instead and the partial answer.
Ensure that the selected algorithms remains stable throughout the entire test session. Crypto support detection was rewritten to python and simplified.
Closes#4202Closes#4422
Related #3810
Merge branch '4202-algorithm-detection-pytest' into 'main'
Closes#4202 and #4422
See merge request isc-projects/bind9!8803
Ensure all the variables are initialized when running the main function
of isctest module. This enables proper environment variables during test
script development when only conf.sh is sourced, rather than the script
being executed by the pytest runner.
Run the crypto support checks when initializing the isctest package and
save those results in environment variable. This removes the need to
repeatedly check for crypto operation support, as it's not something
that would change at test runtime.
Instead of invoking get_algorithms.py script repeatedly (which may yield
different results), move the algorithm configuration to an isctest
module. This ensures the variables are consistent across the entire test
run.
Add two mechanisms to close open log files. The first is `rndc closelogs`. The second is `kill -USR1 <pid>`.
They are intended to be used with external log rotation tools.
Closes#4780
Merge branch '4780-add-support-for-external-log-rotation-tools' into 'main'
Closes#4780
See merge request isc-projects/bind9!9113
Some external log file rotation programs use signals to tell programs
to close log files. SIGHUP can be used to do this but it also does
a full reconfiguration. Configure named to accept SIGUSR1 as a
signal to close log files.
On 32 bit machines isc_meminfo_totalphys could return an incorrect value.
Closes#4799
Merge branch '4799-cid-498034-overflowed-return-value-integer_overflow' into 'main'
Closes#4799
See merge request isc-projects/bind9!9132
On a 32 bit machine casting to size_t can still lead to an overflow.
Cast to uint64_t. Also detect all possible negative values for
pages and pagesize to silence warning about possible negative value.
39#if defined(_SC_PHYS_PAGES) && defined(_SC_PAGESIZE)
1. tainted_data_return: Called function sysconf(_SC_PHYS_PAGES),
and a possible return value may be less than zero.
2. assign: Assigning: pages = sysconf(_SC_PHYS_PAGES).
40 long pages = sysconf(_SC_PHYS_PAGES);
41 long pagesize = sysconf(_SC_PAGESIZE);
42
3. Condition pages == -1, taking false branch.
4. Condition pagesize == -1, taking false branch.
43 if (pages == -1 || pagesize == -1) {
44 return (0);
45 }
46
5. overflow: The expression (size_t)pages * pagesize might be negative,
but is used in a context that treats it as unsigned.
CID 498034: (#1 of 1): Overflowed return value (INTEGER_OVERFLOW)
6. return_overflow: (size_t)pages * pagesize, which might have underflowed,
is returned from the function.
47 return ((size_t)pages * pagesize);
48#endif /* if defined(_SC_PHYS_PAGES) && defined(_SC_PAGESIZE) */
Stop updating `find.result_v4` and `find.result_v4` in `clean_finds_at_name`. The values are supposed to be static.
Closes#4118
Merge branch '4118-data-race-lib-dns-adb-c-1537-in-clean_finds_at_name' into 'main'
Closes#4118
See merge request isc-projects/bind9!9108
Don't verify the just signed zone as the RRSIGs could have expired before the signing process completes
Closes#4781#2476
Merge branch '4781-statschannel-setup-can-fail-due-to-short-validity-interval' into 'main'
Closes#4781 and #2476
See merge request isc-projects/bind9!9114
The updatecheck-kskonly.secure zone is being used to test dynamic
updates while the KSK is offline. It ensures that the DNSKEY RRset
will retain the RRSIG record, while the updated data is being signed
with the currently active ZSK.
When walking through ZSK rollovers, ensure that the newest ZSK (ZSK3)
is published before doing the dynamic update, preventing timing
related test failures.
Also fix the test log line ($ZSK_ID3 was not yet created at the time
of logging).
Adjust key state and timing metadata if dnssec-policy key lifetime configuration is updated, so that it also
affects existing keys.
Closes#4677
Merge branch '4677-dnssec-policy-key-lifetime-reconfigure' into 'main'
Closes#4677
See merge request isc-projects/bind9!9118
The key lifetime should no longer be adjusted if the key is being
retired earlier, for example because a manual rollover was started.
This would falsely be seen as a dnssec-policy lifetime reconfiguration,
and would adjust the retire/removed time again.
This also means we should update the status output, and the next
rollover scheduled is now calculated using (retire-active) instead of
key lifetime.
If dnssec-policy is reconfigured and the key lifetime has changed,
update existing keys with the new lifetime and adjust the retire
and removed timing metadata accordingly.
If the key has no lifetime yet, just initialize the lifetime. It
may be that the retire/removed timing metadata has already been set.
Skip keys which goal is not set to omnipresent. These keys are already
in the progress of retiring, or still unused.
Use a single source of truth, the git log, to generate the list of CHANGES. Use the .rst format and include it in the ARM for a quick reference with proper gitlab links to issues and merge requests.
Closes#75
Merge branch 'nicki/add-gitchangelog' into 'main'
Closes#75
See merge request isc-projects/bind9!9152
The files in contrib/ directory shouldn't be subject to our pylint
check. They can come from external sources and we don't subject these to
the same standards as the rest of the BIND9 code / scripts.
The configuration has been crafted to cater for BIND9 needs:
- Define actions that have an equivalent section in existing Release
Notes
- Assume the commits that have the necessary changelog makrup are merge
commits from GitLab and transform them into messages and proper links
- Put the resulting changelog into the proper place in
doc/arm/changelog.rst
- Have a separate configuration for changelog and release notes. Both of
these should be generated from the `git log`, with release notes being
subject to more scrutiny and further editing
The project hasn't seen any new development/changes since 2018 and it
seems unlikely we'd be able to get any changes into the upstream. Since
it's isolated into a single file and its task is fairly straighforward,
pull the code into our own repository and maintain it here as needed.
This also makes it easier to make any changes that are specific to our
project.
This commit ensures that we are not attempting to accept an expired
TCP connection as we are not interested in any data that could have
been accumulated in its internal buffers. Now we just drop them for
good.
When sending fails, the ns__client_request() would not reset the
connection and continue as nothing is happening. This comes from the
model that we don't care about failed UDP sends because datagrams are
unreliable anyway, but it greatly affects TCP connections with
keep-alive.
The worst case scenario is as follows:
1. the 3-way TCP handshake gets completed
2. the libuv calls the "uv_connection_cb" callback
3. the TCP connection gets queue because of the tcp-clients quota
4. the TCP client sends as many DNS messages as the buffers allow
5. the TCP connection gets dropped by the client due to the timeout
6. the TCP connection gets accepted by the server
7. the data already sent by the client gets read
8. all sending fails immediately because the TCP connection is dead
9. we consume all the data in the buffer in a very tight loop
As it doesn't make sense to trying to process more data on the TCP
connection when the sending is failing, drop the connection immediately
on the first sending error.
As ns_query_init() cannot fail now, remove the error paths, especially
in ns__client_setup() where we now don't have to care what to do with
the connection if setting up the client could fail. It couldn't fail
even before, but now it's formal.
Be more aggressive when throttling the reading - when we can't send the
outgoing TCP synchronously with uv_try_write(), we start throttling the
reading immediately instead of waiting for the send buffers to fill up.
This should not affect behaved clients that read the data from the TCP
on the other end.
Instead of outright refusing to add new RR types to the cache, be a bit
smarter:
1. If the new header type is in our priority list, we always add either
positive or negative entry at the beginning of the list.
2. If the new header type is negative entry, and we are over the limit,
we mark it as ancient immediately, so it gets evicted from the cache
as soon as possible.
3. Otherwise add the new header after the priority headers (or at the
head of the list).
4. If we are over the limit, evict the last entry on the normal header
list.
Add HTTPS, SVCB, SRV, PTR, NAPTR, DNSKEY and TXT records to the list of
the priority types that are put at the beginning of the slabheader list
for faster access and to avoid eviction when there are more types than
the max-types-per-name limit.
Due to omission it was possible to un-throttle a TCP connection
previously throttled due to the peer not reading back data we are
sending.
In particular, that affected DoH code, but it could also affect other
transports (the current or future ones) that pause/resume reading
according to its internal state.
Clear qctx->zversion when clearing qctx->zrdataset et al in
lib/ns/query.c:qctx_freedata. The uncleared pointer could lead to
an assertion failure if zone data needed to be re-saved which could
happen with stale data support enabled.
A different solution in the future might be adopted depending
on feedback and other new information, so it makes sense to mark
these options as EXPERIMENTAL until we have more data.
View matching on an incoming query checks the query's signature,
which can be a CPU-heavy task for a SIG(0)-signed message. Implement
an asynchronous mode of the view matching function which uses the
offloaded signature checking facilities, and use it for the incoming
queries.
Add support for using the offload threadpool to perform message
signature verifications. This should allow check SIG(0)-signed
messages without affecting the worker threads.
This is a tiny helper function which is used only once and can be
replaced with two function calls instead. Removing this makes
supporting asynchronous signature checking less complicated.
In order to protect from a malicious DNS client that sends many
queries with a SIG(0)-signed message, add a quota of simultaneously
running SIG(0) checks.
This protection can only help when named is using more than one worker
threads. For example, if named is running with the '-n 4' option, and
'sig0checks-quota 2;' is used, then named will make sure to not use
more than 2 workers for the SIG(0) signature checks in parallel, thus
leaving the other workers to serve the remaining clients which do not
use SIG(0)-signed messages.
That limitation is going to change when SIG(0) signature checks are
offloaded to "slow" threads in a future commit.
The 'sig0checks-quota-exempt' ACL option can be used to exempt certain
clients from the quota requirements using their IP or network addresses.
The 'sig0checks-quota-maxwait-ms' option is used to define a maximum
amount of time for named to wait for a quota to appear. If during that
time no new quota becomes available, named will answer to the client
with DNS_R_REFUSED.
By default we log a rekey failure on debug level. We should probably
change the log level to error. We make an exception for when the zone
is not loaded yet, it often happens at startup that a rekey is
run before the zone is fully loaded.
when signatures were not added because of too many types already
existing at a node, the diff was not being cleaned up; this led to
a memory leak being reported at shutdown.
kasp-max-types-per-name (named2.conf.in):
An unsigned zone with RR type count on a name right below the
configured limit. Then sign the zone using KASP. Adding a RRSIG would
push it over the RR type limit per name. Signing should fail, but
the server should not crash, nor end up in infinite resign-attempt loop.
kasp-max-records-per-type-dnskey (named1.conf.in):
Test with low max-record-per-rrset limit and a DNSSEC policy requiring
more than the limit. Signing should fail.
kasp-max-types-per-name (named1.conf.in):
Each RRSIG(covered type) is counted as an individual RR type. Test the
corner case where a signed zone, which is just below the limit-1,
adds a new type - doing so would trigger signing for the new type and
thus increase the number of "types" by 2, pushing it over the limit
again.
Add two new masterformat tests that use signing. In the case of
'under-limit-kasp', the signing will keep the number of records in the
RRset under the limit. In the case of 'on-limit-kasp', the signing
will push the number of records in the RRset over the limit, because
of the added RRSIG record.
Send a recursive query for a large number of RRsets, which should
fail when using the default max-types-per-name setting of 100, but
succeed when the cap is disabled.
Previously, the number of RR types for a single owner name was limited
only by the maximum number of the types (64k). As the data structure
that holds the RR types for the database node is just a linked list, and
there are places where we just walk through the whole list (again and
again), adding a large number of RR types for a single owner named with
would slow down processing of such name (database node).
Add a configurable limit to cap the number of the RR types for a single
owner. This is enforced at the database (rbtdb, qpzone, qpcache) level
and configured with new max-types-per-name configuration option that
can be configured globally, per-view and per-zone.
Send a recursive query for a large (2500 record) RRset, which should
fail when using the default max-records-per-type setting of 100, but
succeed when the cap is disabled.
Add two new masterformat tests - the 'huge' zone fits within the ns1
limit and loads on the primary ns1 server, but must not transfer to the
ns2 secondary, and the 'uber' zone should not even load on the primary
ns1 server.
Previously, the number of RRs in the RRSets were internally unlimited.
As the data structure that holds the RRs is just a linked list, and
there are places where we just walk through all of the RRs, adding an
RRSet with huge number of RRs inside would slow down processing of said
RRSets.
Add a configurable limit to cap the number of the RRs in a single RRSet.
This is enforced at the database (rbtdb, qpzone, qpcache) level and
configured with new max-records-per-type configuration option that can
be configured globally, per-view and per-zone.
The changes in this MR prevent the memory used for sending the outgoing
TCP requests to spike so much. That strictly remove the extra need for
own memory context, and thus since we generally prefer simplicity,
remove the extra memory context with own jemalloc arenas just for the
outgoing send buffers.
The single TCP read can create as much as 64k divided by the minimum
size of the DNS message. This can clog the processing thread and trash
the memory allocator because we need to do as much as ~20k allocations in
a single UV loop tick.
Limit the number of the DNS messages processed in a single UV loop tick
to just single DNS message and limit the number of the outstanding DNS
messages back to 23. This effectively limits the number of pipelined
DNS messages to that number (this is the limit we already had before).
As a single thread can process only one TCP send at the time, we don't
really need a memory pool for the TCP buffers, but it's enough to have
a single per-loop (client manager) static buffer that's being used to
assemble the DNS message and then it gets copied into own sending
buffer.
In the future, this should get optimized by exposing the uv_try API
from the network manager, and first try to send the message directly
and allocate the sending buffer only if we need to send the data
asynchronously.
Constantly allocating, reallocating and deallocating 64K TCP send
buffers by 'ns_client' instances takes too much CPU time.
There is an existing mechanism to reuse the ns_clent_t structure
associated with the handle using 'isc_nmhandle_getdata/_setdata'
(see ns_client_request()), but it doesn't work with TCP, because
every time ns_client_request() is called it gets a new handle even
for the same TCP connection, see the comments in
streamdns_on_complete_dnsmessage().
To solve the problem, we introduce an array of available (unused)
TCP buffers stored in ns_clientmgr_t structure so that a 'client'
working via TCP can have a chance to reuse one (if there is one)
instead of allocating a new one every time.
When TCP client would not read the DNS message sent to them, the TCP
sends inside named would accumulate and cause degradation of the
service. Throttle the reading from the TCP socket when we accumulate
enough DNS data to be sent. Currently this is limited in a way that a
single largest possible DNS message can fit into the buffer.
DoH: Avoid potential data races in our DoH implementation related to to HTTP/2 session object management and endpoints set object management
See merge request isc-private/bind9!614
This commit ensures that an HTTP endpoints set reference is stored in
a socket object associated with an HTTP/2 stream instead of
referencing the global set stored inside a listener.
This helps to prevent an issue like follows:
1. BIND is configured to serve DoH clients;
2. A client is connected and one or more HTTP/2 stream is
created. Internal pointers are now pointing to the data on the
associated HTTP endpoints set;
3. BIND is reconfigured - the new endpoints set object is created and
promoted to all listeners;
4. The old pointers to the HTTP endpoints set data are now invalid.
Instead referencing a global object that is updated on
re-configurations we now store a local reference which prevents the
endpoints set objects to go out of scope prematurely.
It was reported that HTTP/2 session might get closed or even deleted
before all async. processing has been completed.
This commit addresses that: now we are avoiding using the object when
we do not need it or specifically check if the pointers used are not
'NULL' and by ensuring that there is at least one reference to the
session object while we are doing incoming data processing.
This commit makes the code more resilient to such issues in the
future.
Changed the default value for 'allow-transfer' to 'none'; zone
transfers now require explicit authorization.
Updated all system tests to specify an allow-transfer ACL when needed.
Revised the ARM to specify that the default is 'none'.
Replace the ISC_LIST based deadnodes implementation with isc_queue which
is wait-free and we don't have to acquire neither the tree nor node lock
to append nodes to the queue and the cleaning process can also
copy (splice) the list into a local copy without acquiring the list.
Currently, there's little benefit to this as we need to hold those
locks anyway, but in the future as we move to RCU based implementation,
this will be ready.
To align the cleaning with our event loop based model, remove the
hardcoded count for the node locks and use the number of the event loops
instead. This way, each event loop can have its own cleaning as part of
the process. Use uniform random numbers to spread the nodes evenly
between the buckets (instead of hashing the domain name).
Add an isc_queue implementation that hides the gory details of cds_wfcq
into more neat API. The same caveats as with cds_wfcq.
TODO: Add documentation to the API.
Setting "Affects v9.x" labels on a merge request duplicates information
already present on the GitLab issue associated with that merge request.
For trivial merge requests that are not associated with any GitLab
issue, setting the "Affects v9.x" label(s) is considered unnecessary.
Trigger a failure for every merge request marked with at least one
"Affects v9.x" label.
Merge request titles auto-generated by GitLab are often a source of
confusion regarding the actual contents of a given merge request. Warn
for merge requests containing titles that look like auto-generated ones.
Using "-v9_x" and "-v9.x" version suffixes for branch names is now
deprecated since some automation logic does not handle these. Fail for
any merge request using such old-style version suffixes.
Backports are not expected to have any "Affects v9.x" labels set since
those are only meant to be set for merge requests that should have
backports created for them.
When in -4 mode check that "IPv6 disabled and no IPv4 primaries"
is logged and when in -6 mode check that "IPv4 disabled and no IPv6
primaries" is logged.
When the cache's memory context was in over memory state when the
cache was flushed it resulted in LRU cleaning removing newly entered
data in the new cache straight away until the old cache had been
destroyed enough to take it out of over memory state. When flushing
the cache create a new memory context for the new db to prevent this.
The `axfr_makedb()` didn't set the loop on the newly created database,
effectively killing delayed cleaning on such database. Move the
database creation into dns_zone API that knows all the gory details of
creating new database suitable for the zone.
The times it takes to run tests CI vary significantly enough
that it makes hypothesis test reach their deadlines and fail randomly
marking the tests as flaky.
This commit disables the deadlines when running in CI.
The most important being `dns_names` that generates dns.name.Name
objects based on given paramaters.
No guarantees are given when it comes the uniformity of generated
samples, however it plays nicely with the hypothesis' shrinking
algorithm.
Once we use hypothesis more widely (in at least one more test) this file
should be moved for it to be reused easily.
Check for more rcodes and various properties needed in the wildcard
test. Add a `name` module for various dns.name.Name operations (with
`prepend_label` function only now).
Expose `timeout` as a parameter of `query.tcp`/`query.udp`.
The system test from the BIND 9.19.24 release does not include the
isctest/vars/autoconf.py file from 9.19.25-dev, and therefore the job
will fail before the 9.19.25 release is published. In the meantime,
consider using the conf.sh file.
The rdataslab.c was full of code like this:
length = raw[0] * 256 + raw[1];
and
count2 = *current2++ * 256;
count2 += *current2++;
Refactor code like this into peek_uint16() and get_uint16 macros
to prevent code repetition and possible mistakes when copy and
pasting the same code over and over.
As a side note for an entertainment of a careful reader of the commit
messages: The byte manipulation was changed from multiplication and
addition to shift with or.
The difference in the assembly looks like this:
MUL and ADD:
movzx eax, BYTE PTR [rdi]
movzx edi, BYTE PTR [rdi+1]
sal eax, 8
or edi, eax
SHIFT and OR:
movzx edi, WORD PTR [rdi]
rol di, 8
movzx edi, di
If the result and/or buffer is then being used after the macro call,
there's more differences in favor of the SHIFT+OR solution.
The detach function declaration in `ISC__REFCOUNT_TRACE_DECL` had an
returned an accidental implicit int. While not allowed since C99, it
became an error by default in GCC 14.
`ISC_REFCOUNT_TRACE_IMPL` and `ISC_REFCOUNT_STATIC_TRACE_IMPL` expanded
into the wrong macros, trying to declare it again with the wrong number
of parameters.
- duplicated question
- duplicated answer
- qtype as an answer
- two question types
- question names
- nsec3 bad owner name
- short record
- short question
- mismatching question class
- bad record owner name
- mismatched class in record
- mismatched KEY class
- OPT wrong owner name
- invalid RRSIG "covers" type
- UPDATE malformed delete type
- TSIG wrong class
- TSIG not the last record
there were TSAN error reports because of conflicting uses of
node->dirty and node->nsec, which were in the same qword.
this could be resolved by separating them, but we could also
make them into atomic values and remove some node locking.
The fix_iterator() function had a lot of bugs in it and while fixing
them, the number of corner cases and the complexity of the function
got out of hand. Rewrite the function with the following modifications:
The function now requires that the iterator is pointing to a leaf node.
This removes the cases we have to deal when the iterator was left on a
dead branch.
From the leaf node, pop up the iterator stack until we encounter the
branch where the offset point is before the point where the search key
differs. This will bring us to the right branch, or at the first
unmatched node, in which case we pop up to the parent branch. From
there it is easier to retrieve the predecessor.
Once we are at the right branch, all we have to do is find the right
twig (which is either the twig for the character at the position where
the search key differs, or the previous twig) and walk down from there
to the greatest leaf or, in case there is no good twig, get the
previous twig from the successor and get the greatest leaf from there.
If there is no previous twig to select in this branch, because every
leaf from this branch node is greater than the one we wanted, we need
to pop up the stack again and resume at the parent branch. This is
achieved by calling prevleaf().
Move the fix_iterator out of the loop and only call it when we found
a leaf node. This leaf node may be the wrong leaf node, but fix_iterator
should correct that.
Also, when we don't need to set the iterator, just get any leaf. We
only need to have a leaf for the qpkey_compare and the end result does
not matter if compare was against an ancestor leaf or any leaf below
that point.
An obscured DNSKEY RRset at a delegation was incorrectly added to
the NSEC/NSEC3 type bit map leading to zone verification failures.
This adds such a RRset to the test zone.
DNSSEC only works when DNSKEYs are self signed. This only occurs
when the DNSKEY RRset is at the apex. Cause dnssec-signzone to
fail if it attempts to sign an non-apex DNSKEY RRset.
An assertion failure would be triggered when sending the TCP data ends
after the TCP reading gets closed. Implement proper reference counting
for the isc_httpd object.
When searching for a requested name in dns_qp_lookup(), we may add
a leaf node to the QP chain, then subsequently determine that the
branch we were on was a dead end. When that happens, the chain can be
left holding a pointer to a node that is *not* an ancestor of the
requested name.
We correct for this by unwinding any chain links with an offset
value greater or equal to that of the node we found.
The code below the if/else construction could only be run if the 'if'
code path was taken. Move the code into the 'if' code block so that
it is more easier to read.
The list isn't exactly maintained but it helped with some BIND history
tracking and is basically harmless so it might be worth holding onto it.
I have adapted the name to ASCII so IDN support won't be necessary.
The high-water allows administrators to better tune the recursive
clients limit without having to to poll the statistics channel in high
rates to get this number.
Returning the value allows for better high-water tracking without
running into edge cases like the following:
0. The counter is at value X
1. Increment the value (X+1)
2. The value is decreased multiple times in another threads (X+1-Y)
3. Get the value (X+1-Y)
4. Update-if-greater misses the X+1 value which should have been the
high-water
To avoid any escaping issues or messing with a language-specific format
when the variable has to be parsed, create a dedicated file for each
variable that is obtained from autoconf.
Make sure all initialization takes place in isctest.vars.__init__ and
export the initial env vars there. Remove the no longer needed env
fixture and use os.environ instead.
If OPENSSL_CONF is exported as an empty string, it will cause issues on
rhel9fips. Allow the environment variables to be set and exported, but
make sure to only export them if they have been set by the user.
The openssl config needs to be parsed for some tests that use SoftHSM2.
Rewrite the parsing to python and ensure the required variables are
properly set test-wide.
Provide a single point of access to all the variables used by tests. Use
a custom dict-like structure to access the underlying data without
making a copy. This allows the individual modules to update the contents
at runtime, which is used for some variables.
While this isn't required for pytest operation and execution of the
system test suite, it can be handy to allow test script development and
debugging. Especially setup scripts often source conf.sh and expect
environment variables to be loaded. If these scripts are executed
stand-alone, the environment variables need to be loaded from the python
package.
Remove conf.sh.in and move the environment variables into isctest/vars
python package. This enabled the removal of an ugly pytest hack which
loaded and parsed these variables from the environment.
qp and rbtdb produce stylistically different backup files. This
was causing the xferquota system test to fail. This has been
addressed by making the test independent of the stylistic differences.
The mechanism was published as RFC 8509. I've briefly looked at diff
between versions -08 and the RFC and did not find significant protocol
change. Quick manual check confirms what we seem to comply with the
published protocol.
Draft was eventually published as RFC 9276 but we did not update our
docs. Also add couple mentions in relevant places in the ARM and
dnssec-signzone man page, mainly around "do not touch" places.
Resolve "SVCB/HTTPS: Bind9 and Dig fail to parse packet if zero SvcPriority and existing SvcParams"
Closes#4704
See merge request isc-projects/bind9!9000
When parsing a zonefile named-checkzone (and others) could loop
infinitely if a directory was $INCLUDED. Record the error and treat
as EOF when looking for multiple errors.
This was found by Eric Sesterhenn from X41.
In nibble mode if the value to be converted was negative the parser
would loop forever. Process the value as an unsigned int instead
of as an int to prevent sign extension when shifting.
This was found by Eric Sesterhenn from X41.
This test is outdated because it tested the 'sig-validity-interval'
option that has been replaced by dnssec-policy's 'signatures-validity',
'signatures-refresh', and 'signatures-jitter' options.
Nevertheless, it tests if the jitter is spread correctly.
Update the test to make use of 'signatures-jitter', set the value
to 1 day (meaning resign in 499 days since 'signatures-validity' is
set to 500 days).
Note that this previously changed erroneously the refresh value to
449 days (should have been 499 days, but that is not allowed by
checkconf, since it is above 90% of 'signatures-validity').
After we have changed the maximum allowed iterations to 51 for signing,
the NSEC3 chain has changed and requires one more NSEC to be returned
in the answer (plus corresponding RRSIG). So the expected number or
records in the authority section is now 8.
If the key is offline and the keymgr runs, it will treat it as a missing key,
and generate a new key (according to the policy). Fix the test by putting
back the KSK temporarily when we run 'rndc loadkeys'.
1. When generating keys, don't set timing metadata. Otherwise keys
are considered to be in use and won't be selected when dnssec-policy
starts a new key rollover.
2. Add an extra check to make sure the new ZSK (zsk2) is prepublished.
Also add a check to make sure it has become active.
3. When using dnssec-settime, add -s to also write to key state files.
The config was recently modified to ensure ns4 won't leak any queries to
root servers. However, the test wasn't executed and it turns out the way
this was handled actually broke the test case. Add our custom root hint
to both of the views to ensure the test can still pass without leaking
any queries.
An RPZ response's SOA record TTL is set to 1 instead of the SOA TTL,
a boolean value is passed on to query_addsoa, which is supposed to be
a TTL value. I don't see what value is appropriate to be used for
overriding, so we will pass UINT32_MAX.
Now that this function also creates the DNSKEY record for the KSKs,
as well as other associated records such as CDS and CDNSKEY, rename
the function to something slightly better.
Creating the KSR happens on the "ZSK side". The KSK is offline and while
the public key and state file may be present, draft-icann-dnssec-keymgmt-01.txt
suggest that the KSR only contains ZSKs.
This is also what knot dns does, so it would also be in the spirit of
interoperability.
The final line in a KSR ";; KeySigningRequest generated at ..." was
missing the version number, that has now been fixed.
Thanks Libor Peltan for reporting.
The expireheader() call in the expire_ttl_headers() function
is erroneous as it passes the 'nlocktypep' and 'tlocktypep'
arguments in a wrong order, which then causes an assertion
failure.
Fix the order of the arguments so it corresponds to the function's
prototype.
in QP keys, characters that are not common in DNS names are
encoded as two-octet sequences. this caused a glitch in iterator
positioning when some lookups failed.
consider the case where we're searching for "\009" (represented
in a QP key as {0x03, 0x0c}) and a branch exists for "\000"
(represented as {0x03, 0x03}). we match on the 0x03, and continue
to search down. at the point where we find we have no match,
we need to pop back up to the branch before the 0x03 - which may
be multiple levels up the stack - before we position the iterator.
there were some structure names used in qpcache.c and qpzone.c that
were too similar to each other and could be confusing when debugging.
they have been changed as follows:
in qcache.c:
- changed_t was unused, and has been removed
- search_t -> qpc_search_t
- qpdb_rdatasetiter_t -> qpc_rditer_t
- qpdb_dbiterator_t -> qpc_dbiter_t
in qpzone.c:
- qpdb_changed_t -> qpz_changed_t
- qpdb_changedlist_t -> qpz_changedlist_t
- qpdb_version_t -> qpz_version_t
- qpdb_versionlist_t -> qpz_versionlist_t
- qpdb_search_t -> qpz_search_t
- qpdb_load_t -> qpz_search_t
when calling dns_qp_lookup() from qpcache, instead of passing
'foundname' so that a name would be constructed from the QP key,
we now just use the name field in the node data. this makes
dns_qp_lookup() run faster.
the same optimization has also been added to qpzone.
the documentation for dns_qp_lookup() has been updated to
discuss this performance consideration.
when the cache is over memory, we purge from the LRU list until
we've freed the approximate amount of memory to be added. this
approximation could fail because the memory allocated for nodenames
wasn't being counted.
add a dns_name_size() function so we can look up the size of nodenames,
then add that to the purgesize calculation.
in a cache database, unlike zones, NSEC3 records are stored in
the main tree. it is not necessary to maintain a separate 'nsec3'
tree, nor to have code in the dbiterator implementation to traverse
from one tree to another.
(if we ever implement synth-from-dnssec using NSEC3 records, we'll
need to revert this change. in the meantime, simpler code is better.)
the QP database doesn't support relative names as the RBTDB did, so
there's no need for a 'new_origin' flag or to handle `DNS_R_NEWORIGIN`
result codes.
- remove unneeded struct members and misleading comments.
- remove unused parameters for static functions.
- rename 'find_callback' to 'delegating' for consistency with qpzone;
the find callback mechanism is not used in QP databases.
- change dns_qpdata_t to qpcnode_t (QP cache node), and dns_qpdb_t to
qpcache_t, as these types are only accessed locally.
- also change qpdata_t in qpzone.c to qpznode_t (QP zone node), for
consistency.
- make the refcount declarations for qpcnode_t and qpznode_t static,
using the new ISC_REFCOUNT_STATIC macros.
In qpcache (and rbtdb), there are some functions that acquire
neither the tree lock nor the node lock when calling newref().
In theory, this could lead to a race in which a new reference
is added to a node that was about to be deleted.
We now detect this condition by passing the current tree and node
lock status to newref(). If the node was previously unreferenced
and we don't hold at least one read lock, we will assert.
The FreeBSD autoscaler has been configured to utilize the new "instance"
GitLab Runner executor to spawn "stress" test CI jobs on AWS EC2
dynamically. A shared GitLab Runner named "freebsd-instance-autoscaler"
has been set up in GitLab CI/CD to communicate with EC2, provisioning VM
instances on demand based on a FreeBSD 13 AMI image. This image is the
same as the one previously used for FreeBSD "stress" tests before the
implementation of autoscaling (specifically, the
"freebsd13-amd64-bind9stress.aws.lab.isc.org" GitLab Runner in CI/CD).
Since the BIND 9 QA repository has been made public, adjust the relevant
URLs and paths used in .gitlab-ci.yml so that they work with the public
version of that repository.
Because some tests don't have a legtimate handle, provide a temporary
return early that should be fixed and removed before squashing. This
short circuiting is still correct until DoQ/DoH3 support is introduced.
GCC might fail to compile because it expects a return after UNREACHABLE.
It should ideally just work anyway since UNREACHABLE is either a
noreturn or UB (__builtin_unreachable / C23 unreachable).
Either way, it should be optimized almost always so the fallback is
free or basically free anyway when it isn't optimized out.
the previous commit introduced a possible race in getsigningtime()
where the rdataset header could change between being found on the
heap and being bound.
getsigningtime() now looks at the first element of the heap, gathers the
locknum, locks the respective lock, and retrieves the header from the
heap again. If the locknum has changed, it will rinse and repeat.
Theoretically, this could spin forever, but practically, it almost never
will as the heap changes on the zone are very rare.
we simplify matters further by changing the dns_db_getsigningtime()
API call. instead of passing back a bound rdataset, we pass back the
information the caller actually needed: the resigning time, owner name
and type of the rdataset that was first on the heap.
in RBTDB, the heap was used by zone databases for resigning, and
by the cache for TTL-based cache cleaning. the cache use case required
very frequent updates, so there was a separate heap for each of the
node lock buckets.
qpzone is for zones only, so it doesn't need to support the cache
use case; the heap will only be touched when the zone is updated or
incrementally signed. we can simplify the code by using only a single
heap.
fix_iterator() and related functions are quite difficult to read.
perhaps it would be a little clearer if we didn't assign values
to variables that won't subsequently be used, or unnecessarily
pop the stack and then push the same value back onto it.
also, in dns_qp_lookup() we previously called fix_iterator(),
removed the leaf from the top of the iterator stack, and then
added it back on. this would be clearer if we just push the leaf
onto the stack when we need to, but leave the stack alone when
it's already complete.
under some circumstances it was possible for the iterator to
be set to the first leaf in a set of twigs, when it should have
been set to the last.
a unit test has been added to test this scenario. if there is a
a tree containing the following values: {".", "abb.", "abc."}, and
we query for "acb.", previously the iterator would be positioned at
"abb." instead of "abc.".
the tree structure is:
branch (offset 1, ".")
branch (offset 3, ".ab")
leaf (".abb")
leaf (".abc")
we find the branch with offset 3 (indicating that its twigs differ
from each other in the third position of the label, "abB" vs "abC").
but the search key differs from the found keys at position 2
("aC" vs "aB"). we look up the bit value in position 3 of the
search key ("B"), and incorrectly follow it onto the wrong twig
("abB").
to correct for this, we need to check for the case where the search
key is greater than the found key in a position earlier than the
branch offset. if it is, then we need to pop from the current leaf
to its parent, and get the greatest leaf from there.
a further change is needed to ensure that we don't do this twice;
when we've moved to a new leaf and the point of difference between
it and the search key even earlier than before, then we're definitely
at a predecessor node and there's no need to continue the loop.
The previous value of 30 minutes used to cache the ADB names and entries
was quite long. Change the value to 60 seconds for faster recovery
after cached intermittent failure of the remote nameservers.
The algorithm from the previous commit[1] is now used to calculate all
the expiration values through the code (ncache results, cname/dname
targets).
1. ISC_MIN(cur, ISC_MAX(now + ADB_ENTRY_WINDOW, now + rdataset->ttl))
Correct the logic to set the expiration period of expire_{v4,v6} as
follows:
1. If the trust is ultimate (local entry), immediately set the entry as
expired, so the changes to the local zones have immediate effect.
3. If the expiration is already set and smaller than the new value, then
leave the expiration value as it is.
2. Otherwise pick larger of `now + ADB_ENTRY_WINDOW` and `now + TTL` as
the new expiration value.
When ADB entry was created it was set to never expire. If we never
called any of the functions that adjust the expiration, it could linger
in the ADB forever.
Set the expiration (.expires) to now + ADB_ENTRY_WINDOW when creating
the new ADB entry to ensure the ADB entry will always expire.
In the past, our CI infrastructure was more sensitive to the number of
CI jobs running on it. We tried to limit long-running jobs in merge
request-triggered pipelines, as there are many of them, and spawned them
only in daily scheduled ones. Moving most of the CI infrastructure to
AWS has made it way better to run jobs in parallel, and the existence of
short respdiff jobs has lost its original merit. It can also be harmful
as some problems are detected only by the longer respdiff variant when a
faulty merge request has already been merged. We should run all long
respdiff tests in merge request-triggered pipelines.
Also, move the former respdiff-long job (now just "respdiff") to AWS as
old instance memory constraints (see
f09cf69594) are no longer an issue.
We now have ctx.kskflag, ctx.zskflag, and ctx.revflag, but zskflag is
not quite like the other two, as it doesn't have a special bit in the
DNS packet, and is used as a boolean.
This patch changes so that we use booleans for all three, and
construct the flags based on which ones are set.
patch by @aram
Add test cases for the 'request' command. Reuse the earlier
pregenerated ZSKs. We also need to set up some KSK files, that can
be done with 'dnssec-keygen -k <policy> -fK' now.
The 'check_keys()' function is adjusted such that the expected active
time of the successor key is set to the inactive time of the
predecessor. Some additional information is saved to make 'request'
testing easier.
Add code that can create a Key Signing Request (KSR) given a DNSSEC
policy, a set of keys and an interval.
Multiple keys that match the bundle and kasp parameters are sorted by
keytag, mainly for testing purposes.
Create some helper functions for code that is going to be reused by the
other commands (request, sign), such as setting and checking the context
parameters, and retrieving the dnssec-policy/kasp.
The 'dnssec-keygen' tool now allows the options '-k <dnssec-policy>'
and '-f <flags>' together to create keys from a DNSSEC policy that only
match the given role. Allow setting '-fZ' to only create ZSKs, while
'-fK' will only create KSKs.
Add a system test for testing dnssec-ksr, initally for the keygen
command. This should be able to create or select key files given a
DNSSEC policy and a time window.
Introduce a new DNSSEC tool, dnssec-ksr, for creating signed key
response (SKR) files, given one or more key signing requests (KSRs).
For now it is just a dummy tool, but the future purpose of this utility
is to pregenerate ZSKs and signed RRsets for DNSKEY, CDNSKEY, and CDS
for a given period that a KSK is to be offline.
Previously, only a single controlconf message would be processed from a
single TCP read even if the TCP read buffer contained multiple messages.
Refactor the isccc_ccmsg unit to store the extra buffer in the internal
buffer and use the already read data first before reading from the
network again.
Co-authored-by: Ondřej Surý <ondrej@isc.org>
Co-authored-by: Dominik Thalhammer <dominik@thalhammer.it>
The xmlGetGlobalState allocates per-thread memory that is not properly
cleaned up when the libxml2 is used from offloaded threads. Add the
function the the LeakSanitizer suppression list.
Prepare the statistics channel data in the offloaded worker thread, so
the networking thread is not blocked by the process gathering data from
various data structures. Only the netmgr send is then run on the
networkin thread when all the data is already there.
Protect the access to the trust byte in the ncache data with relaxed
atomic operation to mimick the current behaviour. This will teach
TSAN that the concurrent access is fine.
This prevents TSAN errors with the ncache code where the trust byte
access needs to be protected by a lock. The old code copied the
entire region before determining where the name ended. We now
determine where the name ends then copy just that data and in doing
so avoid reading the trust byte.
This commit makes the dispatch_test use the same timeouts that network
manager tests. We do that because the old values appear to be too
small for our heavy loaded CI machines, leading to spurious failures
on them. The network manager tests are much more stable in this
situation and they use somewhat larger timeout values.
We use a smaller connection timeouts for the tests which are expected
to timeout to not wait for too long.
The source address field of 'newnotify' was not updated from the
default (0.0.0.0) when the destination address was an IPv6 address.
This resulted in the messages failing to be sent. Set the source
address to :: when the destination address is an IPv6 address.
:bulb: **Click [here][checklist_explanations] (internal resource) for general information about the security incident handling process.**
@@ -29,13 +31,13 @@ confidential!
- [ ] [: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 and the CVSS score
- [ ] [: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 and hand it off to Marketing
- [ ] [:link:][step_earliest_send] **(Marketing)** Update "earliest" notification document in SF portal and send bulk email to earliest customers
- [ ] [: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
- [ ] [:link:][step_reproducer_mr] **(SwEng)** Prepare a private merge request containing a system test reproducing the problem
- [ ] [:link:][step_notify_support] **(SwEng)** Notify Support when a reproducer is ready
@@ -46,18 +48,20 @@ 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_changes] **(QA)** (BIND 9 only) Reserve a block of `CHANGES` placeholders once the complete set of vulnerabilities fixed in a given release cycle is determined
- [ ] [:link:][step_coordinate_check] **(SwEng)** Make sure othervendors 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)
### At T-5
- [ ] [:link:][step_asn_documents]**(Marketing)** Update the text on the T-5 (from the Printing Press project) and "earliest" ASN documents in the SF portal
- [ ] [:link:][step_asn_links]**(Marketing)** (BIND 9 only) Update the BIND -S information document in SF with download links to the new versions
- [ ] [:link:][step_asn_send] **(Marketing)** Bulk email eligible customers to check the SF portal
- [ ] [:link:][step_asn_links] **(Marketing)** (BIND 9 only) Update the BIND -S information documentin the support portal with download links to the new versions
- [ ] [:link:][step_asn_send] **(Support)** Notify eligible customers by adding a ticket to the 5 Day queue in RT with the text of the advisory (earliest, and T-5)
- [ ] [:link:][step_preannouncement] **(Marketing)** (BIND 9 only) Send a pre-announcement email to the *bind-announce* mailing list to alert users that the upcoming release will include security fixes
### At T-3
- [ ] [:link:][step_asn_send] **(Support)** Notify eligible customers by adding a ticket to the 3 Day queue in RT with the text of the advisory (T-3)
### At T-1
- [ ] [:link:][step_packager_emails] **(First IM)** Send notifications to OS packagers
@@ -65,15 +69,16 @@ 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_bump_advisory]**(Support)** Bump Document Version for the Security Advisory in Printing Press
- [ ] [:link:][step_publish_advisory] **(Support)** Publish the Security Advisory 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
| `-DOPENSSL_API_COMPAT=10100` | Build using the deprecated OpenSSL APIs so that the `engine` API is available when building with OpenSSL 3.0.0 for PKCS#11 support |
| [security] | Fix for a significant security flaw |
| [experimental] | Used for new features when the syntax or other aspects of the design are still in flux and may change |
| [port] | Portability enhancement |
| [maint] | Updates to built-in data such as root server addresses and keys |
| [tuning] | Changes to built-in configuration defaults and constants to improve performance |
| [performance] | Other changes to improve server performance |
| [protocol] | Updates to the DNS protocol such as new RR types |
| [test] | Changes to the automatic tests, not affecting server functionality |
| [cleanup] | Minor corrections and refactoring |
| [doc] | Documentation |
| [contrib] | Changes to the contributed tools and libraries in the 'contrib' subdirectory |
| [placeholder] | Used in the main development branch to reserve change numbers for use in other branches, e.g., when fixing a bug that only exists in older releases |
In general, [func] and [experimental] tags only appear in new-feature
releases (i.e., those with version numbers ending in zero). Some new
functionality may be backported to older releases on a case-by-case basis.
All other change types may be applied to all currently supported releases.
#### Bug report identifiers
Most notes in the CHANGES file include a reference to a bug report or
Most notes in the ARM Changelog appendix include a reference to a bug report or
issue number. Prior to 2018, these were usually of the form `[RT #NNN]`
and referred to entries in the "bind9-bugs" RT database, which was not open
to the public. More recent entries use the form `[GL #NNN]` or, less often,
``$INCLUDE``. This option cannot be combined with :option:`-O raw <-O>`
or serial-number updating.
..option:: -E engine
This option specifies the hardware to use for cryptographic
operations, such as a secure key store used for signing, when applicable.
When BIND 9 is built with OpenSSL, this needs to be set to the OpenSSL
engine identifier that drives the cryptographic accelerator or
hardware service module (usually ``pkcs11``).
..option:: -F
This options turns on FIPS (US Federal Information Processing Standards)
@@ -89,6 +80,11 @@ Options
where ``digest-type`` is an allowed algorithm such as SHA-256 (2), or SHA-384 (4).
Only works in combination with smart signing (``-S``).
..option:: -J filename
This option tells :program:`dnssec-signzone` to read the journal from the given file
when loading the zone file.
..option:: -K directory
This option specifies the directory to search for DNSSEC keys. If not
@@ -178,6 +174,11 @@ Options
days. Therefore, if any existing RRSIG records are due to expire in
less than 7.5 days, they are replaced.
Note that the calculation of cycle interval is based upon the validity
period of the replacement signatures that would be generated by
``dnssec-signzone``, not on the valid lifetimes of the input RRSIGs being
considered for pre-expiry replacement.
..option:: -I input-format
This option sets the format of the input zone file. Possible formats are
@@ -273,7 +274,7 @@ Options
with cached copies of the old DNSKEY RRset. The :option:`-Q` option forces
:program:`dnssec-signzone` to remove signatures from keys that are no longer
active. This enables ZSK rollover using the procedure described in
:rfc:`4641#4.2.1.1` ("Pre-Publish Key Rollover").
:rfc:`6781#section-4.1.1.1` ("Pre-Publish Zone Signing Key Rollover").
..option:: -q
@@ -290,7 +291,7 @@ Options
This option is similar to :option:`-Q`, except it forces
:program:`dnssec-signzone` to remove signatures from keys that are no longer
published. This enables ZSK rollover using the procedure described in
:rfc:`4641#4.2.1.2` ("Double Signature Zone Signing Key
:rfc:`6781#section-4.1.1.2` ("Double Signature Zone Signing Key
Rollover").
..option:: -S
@@ -374,6 +375,7 @@ Options
..note::
``-3 -`` is the recommended configuration. Adding salt provides no practical benefits.
See :rfc:`9276`.
..option:: -H iterations
@@ -382,6 +384,7 @@ Options
..warning::
Values greater than 0 cause interoperability issues and also increase the risk of CPU-exhausting DoS attacks.
See :rfc:`9276`.
..option:: -A
@@ -390,6 +393,7 @@ Options
..warning::
Do not use this option unless all its implications are fully understood. This option is intended only for extremely large zones (comparable to ``com.``) with sparse secure delegations.
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.
Colin Vidal
Andoni Duarte Pintado
Michal Nowak
Mark Andrews
Arаm Sаrgsyаn
Michał Kępień
Nicki Křížek
Ondřej Surý
Matthijs Mekking
Alessio Podda
Artem Boldariev
Artem Boldariev
Aydın Mercan
Doug FreedandMark Andrews
Petr Špaček
Petr MenšíkandPetr Špaček
JINMEI TatuyaandOndřej Surý
Pavel BřezinaandOndřej Surý
Colin VidalandColin Vidal
Remi GacogneandMark Andrews
Timo EisenmannandMark Andrews
Suzanne GoldlustandNicki Křížek
Vicky RiskandNicki Křížek
James AddisonandPetr Špaček
Yedaya KatsmanandMark Andrews
Tom KrizekandNicki Křížek
Emanuel PetrandNicki Křížek
Štěpán BalážikandPetr Špaček
Dominik ThalhammerandOndřej Surý