Since the removal of NS_QUERY_QCTX_DESTROYED hook, there is no need for
the `qctx->detach_client` object anymore, as this was designed to tell
the plugin whether the client object is about to be, or is already,
freed from memory. This is not needed anymore, as NS_QUERY_RESET is
called _always_ when the client object is about to be freed from memory.
Remove `detach_client` and tidy up the code a bit by including the
freeing of the qctx object (when allocated) inside the qctx_destroy
function instead of requiring extra calls.
Update hooks-related unit tests since the removal of
NS_QUERY_QCTX_DESTROY and the introduction of NS_QUERY_RESET hook. This
also simplifies (a bit) the plugin usage as NS_QUERY_RESET is _always_
called when the client plugin is about to be freed from memory.
The hook NS_QUERY_QCTX_DESTROY is problematic with zone plugins because
it can be called in some contexts where `qctx->client` is invalid (the
pointer is dangling); which would lead to a use-after-free (spotted by
TSAN build) as `qctx->client` is used to get the zone hooktable, to find
out whether there is an authoritive zone which would have
NS_QUERY_QCTX_DESTROY registered.
This can't easily be fixed, because there is no easy way to know from
query.c code if `client` is still a valid object: `client->reqhandle`,
representing the request from a client, is refcounted, and the `client`
object is freed from memory once its refcounter gets to 0. While
`reqhandle` is attached from query.c code, it can be attached more than
once from asynchronous code and there is no clear path where detaching
it would lead to a client free. Hence, there is no way to know for sure
when to set `qctx->client = NULL` (this is why the pointer remains
dangling).
Back to the original problem; this is why the NS_QUERY_QCTX_DESTROY hook
is incompatible with zone plugins. `qctx->detach_client`, which is used
to tell a plugin that the `client` object is either free or about to be
free can't be use either, because in some cases the client is still
there, and should be used.
Code issue aside, the `qctx` object is really just an aggregate of
various data to pass easily in the various functions and callbacks,
initially stored on the stack, but allocated in some cases (for some
asynchronous flow, when recursion is needed), so the point it gets
created/"destroyed" is really just an implementation "detail", and
providing a higher level hook for the plugin would be beneficial. Hence,
NS_QUERY_RESET and NS_QUERY_INIT are removed, and instead, the existing
NS_QUERY_SETUP can be used as well as the newly introduced
NS_QUERY_RESET (which replaces NS_QUERY_QCTX_DESTROY). The advanage is
that NS_QUERY_RESET is called _only_ when the client object is _always_
about to be freed, which avoids usage of the extra `qctx->detach_client`
usage from the plugin.
The way NS_QUERY_RESET works is that when the `client` is freed, a
callback (from `query.c`) is called. This callback creates a transient
qctx object on the stack with a pointer to the view, and uses that
to call the hook.
Update the existing test's syncplugin plugin with a new parameter
indicating whether the plugin should be loaded in a view or a zone.
If it doesn't match where the plugin is actually loaded, it fails the
initialization. This covers the correctless of the `source` parameter of
`plugin_register` API.
The plugin `plugin_register` API has a new parameter `source` indicating
whether the plugin is loaded from a view or a zone.
This extra parameter enables the plugin to fail early during
initialization if a plugin written to be used in a zone exclusively
is loaded at a view level, or vice versa.
Update the filter-aaaa system test so the two authoritative zones
in ns4 both configure filter-aaaa as a zone plugin.
In order to work in both contexts, the plugin must register both
the `NS_QUERY_QCTX_INITIALIZED` and `NS_QUERY_AUTHZONE_ATTACHED`
hooks.
When the plugin is configured at the zone level in an authoritative
server, `NS_QUERY_QCTX_INITIALIZED` is skipped, because no zone will
have been looked up by the time it is called. When the zone is
found, calling `NS_QUERY_AUTHZONE_ATTACHED` will allow the same
initialization to occur.
Add a new query hook called `NS_QUERY_AUTHZONE_ATTACHED`. This hook is
called whenever an authoritative zone is found and attached during a
query answer.
From code level, this hook is called when `qctx->client->query->authzone`
is attached during a query. This enables zone-specific plugins to
initialize specific states whenever a local zone is found that can
answer a query.
Attempt to add zone plugin specificities into the hook developer
documentation. In particular about the hook call order and hookpoint
which can't be called on a zone plugin.
The zone plugin loading code now also looks into the zone template
configuration property of a zone. If it exists, it checks whether there
is a plugin sub-tree defined in the template and, if that exists, loads
the plugin definition from the template.
A new query unit test covers the logic where zone hooks must be called
first, then view ones, and finally the default hooks. It also ensures
that if any hook returns NS_HOOK_RETURN the chain immediately stops.
The rrset-order random doesn't offer uniform distribution of all
permutations and it isn't superior to cyclic order in any way. Make the
random ordering an alias to the cyclic ordering.
Closes: #5513
Merge branch 'ondrej/remove-rrset-order-random' into 'main'
See merge request isc-projects/bind9!10912
Mimic the Unbound behaviour where the cyclic offset is taken from query
ID, and remove recording of the current state. As the incoming query ID
should have random distribution, the cyclic ordering should also have
uniform distribution of the starting record.
With random ordering removed, the cyclic ordering can be rewritten in a
that it uses thread_local static array to keep the cyclic order.
This could be further improved by keeping the current position inside
the slabheader and adding a function to start directly there instead at
dns_rdataset_first().
The rrset-order random doesn't offer uniform distribution of all
permutations and it isn't superior to cyclic order in any way. Make the
random ordering an alias to the cyclic ordering.
Extend the `$name`, `$view` and `$type` tokens (expanding into the zone
name, zone's view name and type); the new following tokens are now also
accepted:
- `$name` or `%s` is replaced with the zone name in lower case;
- `$type` or `%t` is replaced with the zone type -- i.e., primary,
secondary, etc);
- `$view` or `%v` is replaced with the view name;
- `$char1` or `%1` is replaced with the first character of the zone name;
- `$char2` or `%2` is replaced with the second character of the zone name
(or a dot if there is no second character);
- `$char3` or `%3` is replaced with the third character of the zone name (or
a dot if there is no third character);
- `$label1` or `%z` is replaced with the toplevel domain of the zone (or a
dot if it is the root zone);
- `$label2` or `%y` is replaced with the next label under the toplevel
domain (or a dot if there is no next label);
- `$label3` or `%x` is replaced with the next-next label under the toplevel
domain (or a dot if there is no next-next label).
Closes#85
Merge branch '85-file-template' into 'main'
See merge request isc-projects/bind9!10779
Extend the `$name`, `$view` and `$type` tokens (expanding into the zone
name, zone's view name and type); the new following tokens are now also
accepted:
- $name or %s is replaced with the zone name in lower case;
- $type or %t is replaced with the zone type -- i.e., primary,
secondary, etc);
- $view or %v is replaced with the view name;
- $char1 or %1 is replaced with the first character of the zone name;
- $char2 or %2 is replaced with the second character of the zone name
(or a dot if there is no second character);
- $char3 or %3 is replaced with the third character of the zone name (or
a dot if there is no third character);
- $label1 or %z is replaced with the toplevel domain of the zone (or a
dot if it is the root zone);
- $label2 or %y is replaced with the next label under the toplevel
domain (or a dot if there is no next label);
- $label3 or %x is replaced with the next-next label under the toplevel
domain (or a dot if there is no next-next label).
Update to REUSE Specification 3.2+ that uses REUSE.toml instead of DEP5
based specification.
Merge branch 'ondrej/cover-reuse-dep5-to-toml' into 'main'
See merge request isc-projects/bind9!10945
In order to not pollute the SERVFAIL cache with the configured
SERVFAIL answers while RPZ is loading, set the NS_CLIENTATTR_NOSETFC
attribute for the client.
Merge branch 'aram/rpz-servfail-until-ready-tunings' into 'main'
See merge request isc-projects/bind9!10904
Since the log level has been raised, busy servers can "explode" from
the amount of log messages. Use the usual practice of logging "every
once in a while".
The "RPZ not ready yet" message is logged at debug 3 level. Use the
info level instead for better visibility.
After raising the log level, the rpz_log_fail_helper() function starts
appending " failed: " the the message. Change the log message so it
makes more sense.
In order to not pollute the SERVFAIL cache with the configured
SERVFAIL answers while RPZ is loading, set the NS_CLIENTATTR_NOSETFC
attribute for the client.
The randomized order of the records in the rrset is not uniform across
all permutations. Clarify this in the documentation.
Closes#5485
Merge branch 'ondrej/clarify-rrset-order-random' into 'main'
See merge request isc-projects/bind9!10909
When an IPv6 rpz prefix entry is entered incorrectly the log
message was just displaying the prefix rather than the full
entry. This has been corrected.
Closes#5491
Merge branch '5491-rpz-canonical-warning-displays-zone-entry-incorrectly' into 'main'
See merge request isc-projects/bind9!10890
Both qname and qtype arguments to dns_adb_createfind() were unused.
Remove both these arguments from the function prototype.
Merge branch 'ondrej/minor-dns_adb-cleanup' into 'main'
See merge request isc-projects/bind9!10918
Waiting for "keymgr: done" logs caused some manual-mode tests to fail intermittently. Waiting for "rekey done" logs should be more reliable.
Closes#5493
Merge branch '5493-algoroll-csk-step3-manual-unstable-test' into 'main'
See merge request isc-projects/bind9!10923
The counter in ns_client_t is used to track the maximum number of
recursions in the resolver, but it is created unconditionally when
starting the client and deallocated when resetting it.
This commit defers the allocation of the counter till recursion needs to
actually happen, speeding up authoritative workloads in perflab by
1.5~2%.
Merge branch 'alessio/lazy-fetch-counter-alloc' into 'main'
See merge request isc-projects/bind9!10917
The counter in ns_client_t is used to track the maximum number of
recursions in the resolver, but it is created unconditionally when
starting the client and deallocated when resetting it.
This commit defers the allocation of the counter till recursion needs to
actually happen, speeding up authoritative workloads in perflab by
1.5~2%.
Add a test to check serve-stale with the 'stale-answer-client-timeout 0'
configuration option and with a delegation which is a CNAME to a auth
zone.
Closes#5372
Merge branch '5372-serve-stale-crash-on-insist-unreachable-test' into 'main'
See merge request isc-projects/bind9!10920
Mark the ``tkey-domain`` statement as obsolete, since it has not had any
effect on server behavior since support for TKEY Mode 2 (Diffie-Hellman)
was removed (in BIND 9.20.0).
See #4204
Merge branch '4204-obsolete-tkey-domain' into 'main'
See merge request isc-projects/bind9!10798
The "tkey-domain" statement has effectively been a no-op since commit
bd4576b3ce, which removed the only bit of
code using it: the logic implementing TKEY Mode 2 (Diffie-Hellman).
A subsequent cleanup commit, 885c132f4a,
also missed the opportunity to remove the "tkey-domain" statement
altogether.
Mark the "tkey-domain" statement as obsolete and remove all code and
documentation related to it.
