After the split to dns_rdataslab and dns_rdatavec, the
dns_rdataslab_merge() function was unused and it suffered from the same
data race as fixed in the previous commit. Instead of fixing it, just
remove the function and bunch of other unused functions from the
dns_rdataslab unit.
List 'rndc dnssec' arguments in alphabetic order.
The `-step` argument was erroneously omitted from the usage output.
Closes#5731
Merge branch '5731-rndc-documentation-corrections' into 'main'
See merge request isc-projects/bind9!11529
Replace the custom DNS server used in the "pipelined" system test with
new code based on the isctest.asyncserver module.
Merge branch 'michal/pipelined-asyncserver' into 'main'
See merge request isc-projects/bind9!11516
Add a new response handler, ForwarderHandler, which enables forwarding
all queries to another DNS server. To simplify implementation, always
forward queries to the target server via UDP, even if they are
originally received using a different transport protocol.
Extend AsyncDnsServer._log_query() and AsyncDnsServer._log_response() so
that they also log the <address, port> tuple for the socket on which a
given query was received on. Minimize the signatures of those methods
by taking advantage of all the information contained in the QueryContext
instances passed to them.
Extend the QueryContext class with a field holding the <address, port>
tuple for the socket on which a given query was received. This will
enable query handlers to act upon that information in arbitrary ways.
Followup on !11339, which removes further unnecessary copies in the lookups in qpzone.c. The performance impact seems minor though.
Merge branch 'alessio/qpzone-fewer-name-copies' into 'main'
See merge request isc-projects/bind9!11418
Fixes a case where an in-domain NS with an expired glue would fail to resolve.
Let's consider the following parent-side delegation (both for `foo.example.` and `dnshost.example.`
```
foo.example. 3600 NS ns.dnshost.example.
dnshost.example. 3600 NS ns.dnshost.example.
ns.dnshost.example. 3600 A 1.2.3.4
```
Then the child-side of `dnshost.example.`:
```
dnshost.example. 300 NS ns.dnshost.example.
ns.dnshost.example. 300 A 1.2.3.4
```
And then the child-side of `foo.example.`:
```
foo.example 3600 NS ns.dnshost.example.
a.foo.example 300 A 5.6.7.8
```
While there is a zone misconfiguration (the TTL of the delegation and glue doesn't match in the parent and the child), it is possible to resolve `a.foo.example` on a cold-cache resolver. However, after the `ns.dnshost.example.` glue expires, the resolution would have failed with a "fetch loop detected" error. This is now fixed.
Closes#5588
Merge branch '5588-loopfetches' into 'main'
See merge request isc-projects/bind9!11535
The fetch loop detection occured in two places: when
`dns_resolver_createfetch()` is invoked (looking up through the parent
fetches chain and stops the fetch if a parent fetch is the same qname and
qtype) and right after calling `dns_adb_findname()` in the resolver
(stops the fetch if the current fetch is the same name from the ADB
lookup, and ADB lookup needs to fetch it).
Regarding fetch loop detection at the `dns_resulver_createfetch()`
entry, there are case where both qname and qtype are similar but the
zonecut is different. This will then query different name servers and
get different responses. For instance, the following delegation
parent-side (both for `foo.example.` and `dnshost.example.`):
foo.example. 3600 NS ns.dnshost.example.
dnshost.example. 3600 NS ns.dnshost.example.
ns.dnshost.example. 3600 A 1.2.3.4
Then the child-side of `dnshost.example.`:
dnshost.example. 300 NS ns.dnshost.example.
ns.dnshost.example. 300 A 1.2.3.4
Then the child-side of `foo.example.`:
foo.example 3600 NS ns.dnshost.example.
a.foo.example 300 A 5.6.7.8
Obviously, there is a misconfiguration between the parent-side and the
child-side of `dnshost.example` (the mismatch of the TTL), but, this
happens...
Because the resolver is currently child-centric, the parent-side
delegation's glue of `dnshost.example.` will be overriden by the
child-side of the delegation. Once both A records will expires, the
resolver will attempt to find out the A RRs but will start from the
`foo.example.` zonecut, as the delegation itself is still valid.
Then the resolver will attempt to resolve `ns.dnshost.example.`, still
using the `foo.example.` zonecut, which will immediately trigger another
attempt to resolve `ns.foo.example.` (because the A RR is expired). This
is, however _not_ a loop, because the second attempt will have
`dnshost.example.` zonecut. And this changes everything, because the
resolver detects the A name is in-domain, and pass a flag to ADB so
`dns_view_find()` won't use the cache. As a result, the zonecut will be
`.`, and the hints (root servers) will be queried instead.
From that point, they'll return the parent-side delegation, which
includes the glue for `ns.dnshost.example/A`, and the resolution can
continue. Previously, this wouldn't be possible because a loop would be
detected from the second attempt to looking `ns.foo.example/A` and would
result in a SERVFAIL.
Now, the loop detection is relaxed as the loop is detected if the qname,
qtype _and_ zonecut are equals.
This commit also changes the way the loop detection post
`dns_adb_createfind()` works. From the same example above, there would
be two ADB fetches with the same name, but with two different ADB flags
(the first one without DNS_ADB_STARTATZONE, the second one with that
flag). It means that there will be two fetches out of those two ADB
lookups, both legit, and not a loop (i.e. it won't be stuck). To
differenciate between a find which has a pending fetch (which could be
from another find the current find has been attached to), a new find
option `DNS_ADBFIND_STARTEDFETCH` is introduced, which tells that the
current has did started a fetch.
That way, if a find doesn't have `DNS_ADBFIND_STARTEDFETCH` option but
has pending fetches, we know this is a find attached to a similar find
so this is a loop. Otherwise, with `DNS_ADBFIND_STARTEDFETCH`, we know
that even if there is a pending fetch, this is not a loop as the fetch
has just been started
ADB entry window and ADB min cache time can be tweaked using `named -T
adbentrywindow=<unsigned int>` and `named -T adbmincache=<unsigned
int>`.
While those values doesn't needs to be exposed to the operator, this can
be needed to be able to system test ADB behaviors without having to wait
as long as those values are by default.
Instead of cloning fetch responses immediately after inserting them at the head of the `fetch_response` list, defer cloning until the events are actually sent.
This enables to:
- Remove the `fctx->cloned` state;
- Simplify the code by eliminating explicit calls to `clone_result()`;
- Remove the logic that enforced having a fetch response with a `sigrdataset` at the head of the list;
- Remove (just a bit of) locking in some places.
The fetch result is stored directly in new `fctx` properties, but there is no memory increase as those are grouped in an anonymous struct used in a union besides another (bigger) anonymous struct wrapping properties used by qmin fetch only (and, in the case of qmin fetch, those fetch result properties are not needed).
Merge branch 'colin/resolver-cloneresults' into 'main'
See merge request isc-projects/bind9!11458
It's potentially confusing to use "resp_rdataset" for QNAME
minimization, but we can make it a union and have resp.rdataset
and qmin.rdataset using the same memory.
We can save even more space by using the same union to combine
qminname and resp_foundname and access them as qmin.name and
resp.foundname.
Two rdataset property `qminrrset` and `qminsigrrset` are removed from
the fetch context. They only are used as temporary storage for the query
result of the qmin query, and are immediately detached from
`resume_qmin` once the query is over.
As an alternative, use `resp_rdataset` and `resp_sigrdataset`
instead; those are not needed for storing the response data until
after qmin_resume() is over.
Instead of first copying query response data into each fetch response
and then iterating again to send the response to the caller, perform
both operations in one go.
Also removed some duplicate code.
There is no longer a need to decide whether a fetch response should be
prepended or appended to the fetch response list. As query response data
is stored directly in the fetch context object, responses containing a
sigrdataset no longer need to be ordered first. Remove the code
implementing this logic.
Additionally, the distinction between `fetchstate_done` and
`fetchstate_sendevents` is no longer needed. New clients
`dns_fetchresponse_t` can be attached any time to the fetch context
until `fctx__done()` is called, since there is no dependency on the
first fetch response in the list. This simplifies the code and reduces
(just a bit) locking usage.
Query answers are now stored in dedicated fetch context properties,
instead of using `ISC_LIST_HEAD(fctx->resps)`.
This reduces lock critical section usage in some places, and enables
further simplifications. (In particular, it removes the need for special
logic to prepend a fetch response to the list when it contains a
sigrdataset.)
Instead of cloning fetch responses immediately after writing to the
head of the fetch response list, defer cloning until the events are
actually sent.
This removes the need for the `fctx->cloned` state. However, a new
fetch state value, fetchstate_sentevents, is introduced and occurs
after fetchstate_done. To prevent new fetch responses from being
prepended after the head is written but before cloning occurs,
fetchstate_done is now set at all call sites that previously invoked
`clone_results()`.
When RRSIG(rdtype) was independently cached before the RDATA for the
rdtype itself, named would crash on the subsequent query for the RDATA
itself. This has been fixed.
ISC would like to thank Vitaly Simonovich for bringing this
vulnerability to our attention.
Closes#5738
Merge branch '5738-null-pointer-dereference-in-qpcache-add' into 'main'
See merge request isc-projects/bind9!11539
When RRSIG(rdtype) was independently cached before the RDATA for the
rdtype itself, named would crash on the subsequent query for the RDATA
itself. This has been fixed.
ISC would like to thank Vitaly Simonovich for bringing this
vulnerability to our attention.
In dst_gssapi_acceptctx(), the gnamebuf could leak a little bit of
memory if dns_name_fromtext() would theoretically fail. This would
require a Kerberos principal with invalid DNS name.
Closes#5737
Merge branch '5737-memory-leak-in-dst_gssapi_acceptctx-on-dns_name_fromtext-failure' into 'main'
See merge request isc-projects/bind9!11536
In dst_gssapi_acceptctx(), the gnamebuf could leak a little bit of
memory if dns_name_fromtext() would theoretically fail. This would
require a Kerberos principal with invalid DNS name.
Not all DNS responses had the query time set in their corresponding
dnstap messages. This has been fixed.
Closes#3695
Merge branch '3695-record-query-time-for-all-dnstap-responses' into 'main'
See merge request isc-projects/bind9!11527
The description in the protobuf specification is not a list of request
types to process but rather a list of examples to qualify the
description of whether the time indicates when the message is received
or sent.
Previously, the issue when the kasp.test_kasp_case[secondary.kasp] fails
due to a timeout has been only ocassionally observed on FreeBSD 13
in our CI. It seems to have come back on FreeBSD 15.
A lingering `sizeof` from the prototype era of !11094 caused the
key-wipe in `isc_hmac_key_destroy` to use `sizeof(key->len)` instead of
`key->len` for the length argument of `isc_safe_memwipe`.
This results in a buffer overflow of zero bytes in HMAC keys that are
less than 4 bytes. As such, the overflow can only be visibile in keys
that are less than 32-bits, which is beyond broken and creating such
keys are only possible in testing.
Therefore, this change is *not* a security fix since the conditions are
never reachable in any imaginable deployment scenario.
Builds that use OpenSSL >=3.0 are unaffected as the `sizeof` was only
remaining in pre-3.0 builds.
Closes#5732
Merge branch '5732-invalid-params-to-isc_safe_memwipe' into 'main'
See merge request isc-projects/bind9!11520
