Previously, the resolver has maintained many tasks that were randomly
used for resolution process. After the last refactoring, this was
reduced to per-thread task. Since the caller always needs to pass
the task for calling the callback, the resolver already have access
to the task object that's evenly distributed among the threads (via
ns_clientmgr), so change the logic to stop using the internal threads,
and use the task that has been passed when creating new fetch context.
Previously, the task could be created unbound or bound to a specific
thread (worker loop). The unbound tasks would be assigned random thread
every time the isc_task_send() would be called. Because there's no
logic that would assigned the task to the least busy worker, this just
creates an unpredictability. Instead of the random assignment, bind all
the previously unbound tasks to worker 0 which must always exists.
Typing from libuv structure to isc_region_t is not possible, because
their sizes differ on 64 bit architectures. Little endian machines seems
to be lucky and still result in test passed. But big endian machine such
as s390x fails the test reliably.
Fix by directly creating the buffer as isc_region_t and skipping the
type conversion. More readable and still more correct.
This commit disables periodic interface re-scans timer on Linux where
a kernel-based dynamic interface mechanisms make it a thing of the
past in most cases.
The recently added TLS client session cache used
SSL_SESSION_is_resumable() to avoid polluting the cache with
non-resumable sessions. However, it turned out that we cannot provide
a shim for this function across the whole range of OpenSSL versions
due to the fact that OpenSSL 1.1.0 does uses opaque pointers for
SSL_SESSION objects.
The commit replaces the shim for SSL_SESSION_is_resumable() with a non
public approximation of it on systems shipped with OpenSSL 1.1.0. It
is not turned into a proper shim because it does not fully emulate the
behaviour of SSL_SESSION_is_resumable(), but in our case it is good
enough, as it still helps to protect the cache from pollution.
For systems shipped with OpenSSL 1.0.X and derivatives (e.g. older
versions of LibreSSL), the provided replacement perfectly mimics the
function it is intended to replace.
Give a little bit more time if we wait on a time out from the
authoritative (aka resolver failure), and give up after one try
(because the second attempt will likely result in a different EDE).
Make sure that the key structure is valid when calling the following
functions:
- dst_key_setexternal
- dst_key_isexternal
- dst_key_setmodified
- dst_key_ismodified
This commit ensures that isc_nm_cancelread() is not called from within
dig code for HTTP sockets, as these lack its implementation.
It does not have much sense to have it due to transactional nature of
HTTP.
Every HTTP request-response pair is represented by a virtual socket,
where read callback is called only when full DNS message is received
or when an error code is being passed there. That is, there is nothing
to cancel at the time of the call.
The commit fixes a corner case in client-side DoH code, when a write
attempt is done on a closing socket (session).
The change ensures that the write call-back will be called with a
proper error code (see failed_send_cb() call in client_httpsend()).
This commit extends TLS context cache with TLS client session cache so
that an associated session cache can be stored alongside the TLS
context within the context cache.
This commit adds an implementation of a client TLS session cache. TLS
client session cache is an object which allows efficient storing and
retrieval of previously saved TLS sessions so that they can be
resumed. This object is supposed to be a foundation for implementing
TLS session resumption - a standard technique to reduce the cost of
re-establishing a connection to the remote server endpoint.
OpenSSL does server-side TLS session caching transparently by
default. However, on the client-side, a TLS session to resume must be
manually specified when establishing the TLS connection. The TLS
client session cache is precisely the foundation for that.
Setting the sock->write_timeout from the TCP, TCPDNS, and TLSDNS send
functions could lead to (harmless) data race when setting the value for
the first time when the isc_nm_send() function would be called from
thread not-matching the socket we are sending to. Move the setting the
sock->write_timeout to the matching async function which is always
called from the matching thread.
Clang added support for the gcc-style fallthrough
attribute (i.e. __attribute__((fallthrough))) in version 10. However,
__has_attribute(fallthrough) will return 1 in C mode in older versions,
even though they only support the C++11 fallthrough attribute. At best,
the unsupported attribute is simply ignored; at worst, it causes errors.
The C2x fallthrough attribute has the advantages of being supported in
the broadest range of clang versions (added in version 9) and being easy
to check for support. Use C2x [[fallthrough]] attribute if possible, and
fall back to not using an attribute for clang versions that don't have
it.
Courtesy of Joshua Root
Since the fctx hash table is now self-resizing, and resolver tasks are
selected to match the thread that created the fetch context, there
shouldn't be any significant advantage to having multiple tasks per CPU;
a single task per thread should be sufficient.
Additionally, the fetch context is always pinned to the calling netmgr
thread to minimize the contention just to coalesced fetches - if two
threads starts the same fetch, it will be pinned to the first one to get
the bucket.
When autoconf was checking for libuv features, the LIBUV_CFLAGS was not
added to CFLAGS and LIBUV_LIBS to LIBS which resulted in false
negatives.
Use AX_SAVE_FLAGS and AX_RESTORE_FLAGS to temporarily add LIBUV_CFLAGS
and LIBUV_LIBS to their respective variables.
When there is no time in a key file, `dnssec-settime` will print
"UNSET", but to unset a time the user must specify "none" or "never".
This change allows "unset" or "UNSET" as well as "none" or "never".
The "UNSET" output remains the same to avoid compatibility problems
with wrapper scripts.
I have also re-synchronized the "Timing Options" sections of the man
pages.
The dns_message_gettempname(), dns_message_gettemprdata(),
dns_message_gettemprdataset(), and dns_message_gettemprdatalist() always
succeeds because the memory allocation cannot fail now. Change the API
to return void and cleanup all the use of aforementioned functions.
7249bad7 introduced the -c option to stat(1) command, but BSD systems
do not know about it. Replace the stat(1) command with a PERL script
that achieves the same.
Why PERL? For consistency purposes, there are more places in the
system test where we use the same method.
Check that the recursing client count is above a reasonable
minimum, as well as below a maximum, so that we can detect
bugs that cause recursion to fail too early or too often.
The ns_statscounter_recursclients counter was previously only
incremented or decremented if client->recursionquota was non-NULL.
This was harmless, because that value should always be non-NULL if
recursion is enabled, but it made the code slightly confusing.
The fetchlimit test depends on a resolver continuing to try UDP
and timing out while the client waits for resolution to succeed.
but since commit bb990030 (flag day 2020), a fetch will always
switch to TCP after two timeouts, unless EDNS was disabled for
the query.
This commit adds "edns no;" to server statements in the fetchlimit
resolver, to restore the behavior expected by the test.
Since commit bad5a523c2, when the fetches-per-server quota
was increased or decreased, instead of the value being set to
the newly calculated quota, it was set to the *minimum* of
the new quota or 1 - which effectively meant it was always set to 1.
it should instead have been the maximum, to prevent the value from
ever dropping to zero.
the ADB depends on the resolver, but previously only accessed it
via the view. as view->resolver may now be detached before the ADB
finishes, a shutdown race was possible. attaching to the resolver
directly prevents this.
the request manager has no direct dependency on the
view, so there's no need for a weak reference. remove the
dns_requestmgr_whenshutdown() mechanism since it is no longer
used.
weakly attaching and detaching when creating and destroying the
resolver obviates the need to have a callback event to do the weak
detach. remove the dns_resolver_whenshutdown() mechanism, as it is
now unused.
weakly attaching and detaching the view when creating or destroying
the ADB obviates the need for a whenshutdown callback event to do
the detaching. remove the dns_adb_whenshutdown() mechanism, since
it is no longer needed.
for better object separation, ADB and resolver statistics counters
are now stored in the ADB and resolver objects themsevles, rather than
in the associated view.
it's not necessary to use view attributes to determine whether
the ADB, resolver and requestmgr need to be shut down; checking
whether they're NULL is sufficient.
dns_view_weakattach() and _weakdetach() are used by objects that
are created by the view and need it to persist until they are
destroyed, but don't need to prevent it from being shut down. the
validator can use normal view references instead of weak references.
- eliminate dns_view_flushanddetach(), which was only called from
one place; instead, we now call a function dns_view_flushonshutdown()
which sets the view up to flush zones when it is detached normally
with dns_view_detach().
- cleaned up code in dns_view_create().
there's no longer any need for a parameter to specify whether the
function is called while holding the bucket lock, because all
unlocked uses have been removed.
Add a new parameter to the dst_key structure, mark a key modified if
dst_key_(un)set[bool,num,state,time] is called. Only write out key
files during a keymgr run if the metadata has changed.
Add a test case that triggers a keymgr run that will not trigger any
metadata changes. Ensure that the last status change of the key files
is unmodified.
When attaching to the trampoline, the isc__trampoline_max was access
unlocked. This would not manifest under normal circumstances because we
initialize 65 trampolines by default and that's enough for most
commodity hardware, but there are ARM machines with 128+ cores where
this would be reported by ThreadSanitizer.
Add locking around the code in isc__trampoline_attach(). This also
requires the lock to leak on exit (along with memory that we already)
because a new thread might be attaching to the trampoline while we are
running the library destructor at the same time.
After removing the isc_task_onshutdown(), the isc_task_shutdown() and
isc_task_destroy() became obsolete.
Remove calls to isc_task_shutdown() and replace the calls to
isc_task_destroy() with isc_task_detach().
Simplify the internal logic to destroy the task when the last reference
is removed.
The isc_task_onshutdown() was used to post event that should be run when
the task is being shutdown. This could happen explicitly in the
isc_test_shutdown() call or implicitly when we detach the last reference
to the task and there are no more events posted on the task.
This whole task onshutdown mechanism just makes things more complicated,
and it's easier to post the "shutdown" events when we are shutting down
explicitly and the existing code already always knows when it should
shutdown the task that's being used to execute the onshutdown events.
Replace the isc_task_onshutdown() calls with explicit calls to execute
the shutdown tasks.
Commit 24961f6068 introduced the concept
of post-merge pipelines. These were used to build documentation after
each merge to any of the maintained branches, to allow up-to-date
documentation to be subsequently published on the bind.isc.org
microsite. As the latter no longer downloads documentation from GitLab,
instead linking to Read the Docs for all currently maintained branches,
post-merge pipelines have become redundant since running the
"autoreconf" and "docs" jobs after each merge is not necessary any more.
Drop the "release_branch_triggering_rules" YAML anchor and revert to
default triggering rules for the "autoconf" and "docs" jobs, effectively
preventing pipelines from being created after each merge to any of the
maintained branches.
Building documentation for Automake-based BIND 9 versions does not
require running "make all" beforehand. Drop ccache setup code from the
definitions of GitLab CI jobs building documentation as it is redundant.
The bind.isc.org microsite no longer downloads documentation from
GitLab, instead linking to Read the Docs for all currently maintained
branches. This makes it redundant to trigger GitLab Pages pipelines
after each merge to any of the maintained branches. Revert changes
introduced by commit 31bde118db: remove
the "push:docs" job along with the "push" stage it is associated with
and revert artifact expiry time for the "docs" job to the default value.
Remove the line "This address must appear in the secondary server’s
parental-agents zone clause". This line is a copy paste error from
notify-source.
Rewrap.
We have had perpetual problem with Sphinx implicitly double-including
files. To avoid that problem all files with name suffix .inc.rst are now
ignored by Sphinx, and writter can conveniently include them without
modifying conf.py for each and every file.
This commit fixes a crash in generic TLS stream code, which could be
reproduced during some runs of the 'sslyze' tool.
The intention of this commit is twofold.
Firstly, it ensures that the TLS socket object cannot be destroyed too
early. Now it is being deleted alongside the underlying TCP socket
object.
Secondly, it ensures that the TLS socket object cannot be destroyed as
a result of calling 'tls_do_bio()' (the primary function which
performs encryption/decryption during the IO) as the code did not
expect that. This code path is fixed now.
Resolve "RPZ rpz-nsip rules seem not to understand stub and static-stub zones and don't handle DNS_R_GLUE result well ..."
Closes#3232
See merge request isc-projects/bind9!6037
RPZ NSIP and NSDNAME checks were failing with "unrecognized NS
rpz_rrset_find() failed: glue" when static or static-stub zones
where used to resolve the query name.
Add tests using stub and static-stub zones that are expected to
be filtered and not-filtered against NSIP and NSDNAME rules.
stub and static-stub queries are expected to be filtered
stub-nomatch and static-stub-nomatch queries are expected to be passed
The named_config_getdefault() was missing void in the function
definition. This broke clang-15 that didn't match the declaration that
had the void in the argument with the definition that hadn't.
Somewhere in the move from netmgr/uv-compat.h to uv.c, the
uv_os_getenv() implementation was lost in the process. Restore the
implementation, so we can support Debian stretch for couple more months.
From the ld man page:
When creating a dynamically linked executable, using the -E option or
the --export-dynamic option causes the linker to add all symbols to
the dynamic symbol table. The dynamic symbol table is the set of
symbols which are visible from dynamic objects at run time.
This should allow the backtrace(3) to fully resolve the symbols when
creating backtrace on an assertion failure.
Add a #if to make it clear that struct xrdata->order is only used
in DNS_RDATASET_FIXED mode.
Re-order some variable declarations to merge two #if blocks into one.
As we are going to use libuv outside of the netmgr, we need the shims to
be readily available for the rest of the codebase.
Move the "netmgr/uv-compat.h" to <isc/uv.h> and netmgr/uv-compat.c to
uv.c, and as a rule of thumb, the users of libuv should include
<isc/uv.h> instead of <uv.h> directly.
Additionally, merge netmgr/uverr2result.c into uv.c and rename the
single function from isc__nm_uverr2result() to isc_uverr2result().
Move the netmgr socket related functions from netmgr/netmgr.c and
netmgr/uv-compat.c to netmgr/socket.c, so they are all present all in
the same place. Adjust the names of couple interal functions
accordingly.
These checks have been redundant since the `rbtdb64` implementation
was removed in 2018 (commit 784087390a). It isn't possible to create
a zone that uses `database "rbt64"` now that the `rbt64` database
implementation has been removed, so the checks will always fail.
Sometimes the compiler is unable to see that the `empty` variable was
initialized by the call to is_empty(), which can cause a build
failure; I encountered this with CFLAGS=-Os. So get rid of it and use
the result from `is_empty()` instead.
The rbtnode->rpz flag was left behind when rbt and rpz were disentangled
by CHANGES #4576. Removing it makes the comment above correct again.
This reduces the flags so they fit in a 32 bit word again. On 64
bit systems there is still padding so it doesn't change the size
of an rbtnode. On 32 bit systems it reduces an rbtnode by 4 bytes.
Default paths were not substituted correctly when Python-only build was
used, i.e. it affected only ReadTheDocs. The incorrect rst_epilog was
overriden by Makefile for all "ordinary" builds.
This error was introduced by 3f78c60539.
Related: !5815
The dig commands appear to be failing unexpectedly on some platforms
when rate limiting kicks in and the response is dropped. Correct
behaviour should be for dig to retry the query. Set +qr and capture
stdout and stderr of each of the dig commands involved.
3034 next = ISC_LIST_NEXT(query, link);
3035 } else {
3036 next = NULL;
3037 }
CID 352554 (#1 of 1): Dereference before null check (REVERSE_INULL)
check_after_deref: Null-checking connectquery suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
3038 if (connectquery != NULL) {
3039 query_detach(&connectquery);
3040 }
In '_check_apex_dnskey' we check for each key (KEY1 to KEY4) if they
are present in the DNSKEY RRset if they should be.
However, we only grep the dig output for the first seven fields (owner,
ttl, class, type, flags, protocol, algorithm). This can be the same
for different keys.
For example, KEY1 may be KSK predecessor and KEY2 a KSK successor,
both DNSKEY records for these keys are the same up to the public key
field. This can cause test failures if KEY1 needs to be present, but
KEY2 not, because when grepping for KEY2 we will falsely detect the
key to be present (because the grep matches KEY1).
Fix the function by grepping looking for the first seven fields in the
corresponding key file and retrieve the public key part. Grep for this
in the dig output.
It might be useful to display built-in configuration with all its
values. It should make it easier to test what default values has changed
in a new release.
Related: #1326
- var_decl: Declaring variable "tbuf" without initializer
- assign: Assigning: "target.base" = "tbuf", which points to
uninitialized data
- assign: Assigning: "r.base" = "target.base", which points to
uninitialized data
I expect it would correctly initialize length always. Add simple
initialization to silent coverity.
Coverity detected issues:
- var_decl: Declaring variable "diff" without initializer.
- uninit_use_in_call: Using uninitialized value "diff.tuples.head" when
calling "dns_diff_clear".
Parser ensures new-zones-directory has qstring parameter before it can
reach this place. dir == NULL then should never happen on any
configuration. Replace silent check with insist.
FALLTHOUGH is a copy of how it is defined in <isc/util.h>
UNREACHABLE follows the model used in MacOS /usr/include/c++/v1/cstdlib
to determine if __builtin_ureachable is available
SIG and RRSIG records for private algorithms are supposed to contain
the name / OID of the algorithm used to generate them at the start
of the signature field.
It was discovered that MariaDB 10 didn't define LIBMARIADB leading
to compilation errors of MySQL DLZ modules on Debian stretch.
Use MARIADB_BASE_VERSION instead which is defined in all tested MariaDB
versions.
The DNS catalog zones draft version 5 document requires that catalog
zones consumers must reset the member zone's internal zone state when
its unique label changes (either within the same catalog zone or
during change of ownership performed using the "coo" property).
BIND already behaves like that, and, in fact, doesn't support keeping
the zone state during change of ownership even if the unique label
has been kept the same, because BIND always removes the member zone
and adds it back during unique label renaming or change of ownership.
Document the described behavior and add a log message to inform when
unique label renaming occurs.
Add a system test case with unique label renaming.
We check the `rdclass` to be of type IN in `dns_catz_update_process()`
function, and all the other static functions where similar checks exist
are called after (and in the result of) that function being called,
so they are effectively redundant.
There is already a check for the missing version property case
(catalog-bad1.example), and this new test should result in the same
outcome, but differs in a way that there exists a version record in the
zone, but it is of a wrong type (A instead of the expected TXT).
According to DNS catalog zones draft version 5 document, the CLASS field
of every RR in a catalog zone MUST be IN.
Add a new check in the catz system test to verify that a non-IN class
catalog zone (in this case CH) fails to load.
BIND does not support having a non-IN class RR in an IN class zone, or
non-IN class zone in an IN class view, so to verify that BIND respects
the mentioned restriction we must try to add a non-IN class catalog
zone and check that it didn't succeed.
The `named` configuration files had to be restructured to put all the
zones inside views, which also resulted in some corresponding changes
in the tests.sh script.
When parsing the configuration file, log a warning message in
configure_view() function when encountering a `catalog-zones`
option in a view with non-IN rdata class.
The DNS catalog zones draft version 5 document describes various
situations when a catalog zones must be considered as "broken" and
not be processed.
Implement those checks in catz.c and add corresponding system tests.
Add DNS extended errors 3 (Stale Answer) and 19 (Stale NXDOMAIN Answer)
to responses. Add extra text with the reason why the stale answer was
returned.
To test, we need to change the configuration such that for the first
set of tests the stale-refresh-time window does not interfer with the
expected extended errors.
In zone.c, the "me" strings were defined for functions that could be
traced with "ENTER" macro.
Use the __func__ that's defined by the compiler and is less prone to
copy&paste errors.
The shutdown test sends 'rdnc status' commands in parallel with
'rndc stop' A new rndc connection arriving will reference the ACL
environment to see whether the client is allowed to connect.
Commit c0995bc380 added a mutex lock to ns_interfacemgr_getaclenv(),
but if the new connection arrives while the interfaces are being
purged during shutdown, that lock is already being held. If the
the connection event slips in ahead of one of the netmgr's "stop
listening" events on a worker thread, a deadlock can occur.
The fix is not to hold the interfacemgr lock while shutting down
interfaces; only while actually traversing the interface list to
identify interfaces needing shutdown.
previously fctx_done() detached the fctx but did not clear the pointer
passed into it from the caller. in some conditions, when rctx_done()
was reached while waiting for a validator to complete, fctx_done()
could be called twice on the same fetch, causing a double detach.
fctx_done() now clears the fctx pointer, to reduce the chances of
such mistakes.
There is a possibility for `udp_recv()` to be called with `eresult`
being `ISC_R_SUCCESS`, but nevertheless with already deactivated `resp`,
which can happen when the request has been canceled in the meantime.
This commit ensures that write callbacks are getting called only after
the data has been sent via the network.
Without this fix, a situation could appear when a write callback could
get called before the actual encrypted data would have been sent to
the network. Instead, it would get called right after it would have
been passed to the OpenSSL (i.e. encrypted).
Most likely, the issue does not reveal itself often because the
callback call was asynchronous, so in most cases it should have been
called after the data has been sent, but that was not guaranteed by
the code logic.
Also, this commit removes one memory allocation (netievent) from a hot
path, as there is no need to call this callback asynchronously
anymore.
There was a query_detach() call missing in dig, which could lead to
dig hanging on TLS context creation errors. This commit fixes.
The error was introduced because the Strict TLS implementation was
initially made over an older version of the code, where this extra
query_detach() call was not needed.
This seems to be most appropriate way to ensure consistency between
release tarballs and public presentation on ReadTheDocs.
Previous attempt with removing docutils constraint, which relied on pip
depedency solver to pick the same packages as in CI was flawed. RTD
installs a bit different set of packages so it was inherently
unreliable.
As a result RTD pulled in sphinx-rtd-theme==0.4.3 while CI
had 1.0.0, and this inconsistency caused Table of Contents in Release
Notes to render incorrectly. Previous solution was to downgrade
docutils to < 0.17, but I think we should rather pin exact versions.
For the long history of messing with versions read also
isc-projects/bind9@2a8eda0084isc-projects/images@d4435b97beisc-projects/bind9@6a2daddf5b
There was an error in AX_PROG_CC_FOR_BUILD macro that cached literal
name of the cache variable `saved_ac_cv_c_compiler_gnu` instead of the
value of said variable breaking the consecutive runs of ./configure
script with caching enabled.
Currently our CI images we use to build docs (which subsequently get
into release tarballs) are using docutils 0.17.1, which is latest version
which fulfills Sphinx 4.5.0 requirement for docutils < 0.18.
The old requirement for docutils < 0.17 was causing discrepancy between
the way we build release artifacts and the docs on ReadTheDocs.org which
uses doc/arm/requirements.txt from our repo.
Remove the limit for RDT with hope that it will pull latest permissible
version of docutils.
For the long history of messing with docutils version read also
isc-projects/images@d4435b97beisc-projects/bind9@6a2daddf5b
In dns_adb_cancelfind(), we need to release the find lock and
then acquire the bucket and find locks in that order, for
consistency with locking hierarchy elsehwere. Previously we
were only acquiring the bucket lock.
Also rewrote the function for better readability.
Man pages for dig/mdig/delv used `.. option:: +[no]bla` to describe two
options at once, and very old Sphinx does not support that [] in option
names.
Solution is to split negative and positive options into `+bla, +nobla`
form. In the end it improves readability because it transforms hard to
read strings with double brackets from
`+[no]subnet=addr[/prefix-length]` to
`+subnet=addr[/prefix-length], +nosubnet`.
As a side-effect it also allows easier linking to dig/mdig/delv options
using their name directly instead of always overriding the link target
to `+[no]bla` form.
Transformation was done using regex:
s/:: +\[no\]\(.*\)/:: +\1, +no\1
... and manual review around occurences matching regex
+no.*=
Fixes: #3301
The connect()ed UDP socket provides feedback on a variety of ICMP
errors (eg port unreachable) which bind can then use to decide what to
do with errors (report them to the client, try again with a different
nameserver etc). However, Linux's implementation does not report what
it considers "transient" conditions, which is defined as Destination
host Unreachable, Destination network unreachable, Source Route Failed
and Message Too Big.
Explicitly enable IP_RECVERR / IPV6_RECVERR (via libuv uv_udp_bind()
flag) to learn about ICMP destination network/host unreachable.
When we compile with libuv that has some capabilities via flags passed
to f.e. uv_udp_listen() or uv_udp_bind(), the call with such flags would
fail with invalid arguments when older libuv version is linked at the
runtime that doesn't understand the flag that was available at the
compile time.
Enforce minimal libuv version when flags have been available at the
compile time, but are not available at the runtime. This check is less
strict than enforcing the runtime libuv version to be same or higher
than compile time libuv version.
The interfacemgr and the .route was being detached while the network
manager had pending read from the socket. Instead of detaching from the
socket, we need to cancel the read which in turn will detach the route
socket and the associated interfacemgr.
Sphinx "standard domain" provides directive types ".. program::" and
".. option::" to create link anchor for a program name + option combination.
These can be referenced using :ref:`program option` syntax.
The problem is that Sphinx 1.8.5 (e.g. in Ubuntu 18.04) generates
conflicting link targets if a page contains two option directives
starting with the same word, e.g.:
.. program:: dnssec-settime
.. option:: -P date
.. option:: -P ds date
The reason is that option directive consumes only first word as "option
name" (-P) and all the rest is considered "option argument" (date, ds
date). Newer versions of Sphinx (e.g. 4.5.0) handle this by creating
numbered link anchors, but older versions warn and BIND build system
turns the warning into a hard error.
To handle that we use method recommended by Sphinx maintainer:
https://github.com/sphinx-doc/sphinx/issues/10218#issuecomment-1059925508
As a bonus it provides more accurate link anchors for sub-options.
Alternatives considered:
- Replacing standard domain definition of .. option - causes more
problems, see BIND issue #3294.
- Removing hyperlinks for options - that would be a step back.
Fixes: #3295
Instead of checking if we need to re-seed for every isc_random call,
seed the random number generator in the libisc global initializer
and the per-thread initializer.
It used to require two 32-bit integer divisions to get a random number
less than some limit. Now we use Daniel Lemire's "nearly-divisionless"
algorithm for unbiased bounded random numbers, which requires one
64-bit integer multiply in the usual case, and one 32-bit integer
division in rare slow cases. Even the slow cases are faster than
before; there are also fewer branches.
I think this algorithm is exceptionally beautiful. It also has more
clever tricks than lines of code, so I have done my best to explain
how it works.
The dns__adb_attach() had an assertion failure that prevented to attach
to dns_adb if the dns_adb was shutting down. There was a race between
checking for .exiting in dns_adb_createfind and creating new_adbfind() -
other thread could have set the .exiting to true between the check.
Remove the assertion failure and allow attaching to dns_adb even while
shutting down. The process of dns_adb shutting down would be noticed
only a moments later when any other callback is called.
The rctx_chaseds() function calls dns_resolver_createfetch(), passing
fctx->task as the target task to run resume_dslookup() from. This
breaks task-based serialization of events as fctx->task is the task that
the dns_resolver_createfetch() caller wants to receive its fetch
completion event in; meanwhile, intermediate fetches started by the
resolver itself (e.g. related to QNAME minimization) must use
res->buckets[bucketnum].task instead. This discrepancy may cause
trouble if the resume_dslookup() callback happens to be run concurrently
with e.g. fctx_doshutdown().
Fix by passing the correct task to dns_resolver_createfetch() in
rctx_chaseds().
BIND 9 plugins are installed using Automake's pkglib_LTLIBRARIES stanza,
which causes the relevant shared objects to be placed in the
$(libdir)/@PACKAGE@/ directory, where @PACKAGE@ is expanded to the
lowercase form of the first argument passed to AC_INIT(), i.e. "bind".
Meanwhile, NAMED_PLUGINDIR - the preprocessor macro that the
ns_plugin_expandpath() function uses for determining the absolute path
to a plugin for which only a filename has been provided (rather than a
path) - is set to $(libdir)/named. This discrepancy breaks loading
plugins using just their filenames. Fix the issue (and also prevent it
from reoccurring) by setting NAMED_PLUGINDIR to $(pkglibdir).
The Debian 11 (bullseye) Docker image, which GitLab CI uses for building
documentation, currently contains the following package versions:
- Sphinx 4.5.0
- sphinx-rtd-theme 1.0.0
- docutils 0.17.1
Regenerate the man pages to match contents produced in a Sphinx
environment using the above package versions. This is necessary to
prevent the "docs" GitLab CI job from failing.
PyLint 2.13.7 reports the following error:
bin/tests/system/doth/conftest.py:34:28: E0601: Using variable 'stderr' before assignment (used-before-assignment)
The reason the current code has not caused problems before is that
invoking gnutls-cli with just the --logfile=/dev/null argument causes it
to always return with a non-zero exit code, either due to the option not
being supported or due to the hostname argument not being provided. In
other words, the 'except' branch has always been taken. PyLint is
obviously right on a syntactical level, though.
Instead of relying on a less than obvious code flow (where the 'except'
branch is always taken), rework the flagged code by employing
subprocess.run(..., check=False) instead of subprocess.check_output(),
making exception handling redundant.
While this issue was investigated, it was also noticed that
subprocess.check_output() was incorrectly used as a context manager:
Popen objects are context managers, but subprocess.check_output() and
subprocess.run() are not. Fix by dropping the relevant 'with'
statement.
Commit 3ec5d2d6ed added a Python-based
name server (bin/tests/system/digdelv/ans8/ans.py) to the "digdelv"
system test, but did not update bin/tests/system/Makefile.am to ensure
Python is present in the test environment before the "digdelv" system
test is run. Update bin/tests/system/Makefile.am to enforce that
requirement.
configure.ac currently requires Python 3.4 for running Python-based
system tests. Meanwhile, there are some features in Python 3.6+ that we
would like to use for making our Python code cleaner (e.g. f-strings).
Update the minimum Python version required for running Python-based
system tests to 3.6, noting that:
- Python 3.4 has reached end-of-life on March 18th, 2019.
- Python 3.5 has reached end-of-life on September 5th, 2020.
Since version 5.0.0, decay-based purging is the only available dirty
page cleanup mechanism in jemalloc. It relies on so-called tickers,
which are simple data structures used for ensuring that certain actions
are taken "once every N times". Ticker data (state) is stored in a
thread-specific data structure called tsd in jemalloc parlance. Ticks
are triggered when extents are allocated and deallocated. Once every
1000 ticks, jemalloc attempts to release some of the dirty pages hanging
around (if any). This allows memory use to be kept in check over time.
This dirty page cleanup mechanism has a quirk. If the first
allocator-related action for a given thread is a free(), a
minimally-initialized tsd is set up which does not include ticker data.
When that thread subsequently calls *alloc(), the tsd transitions to its
nominal state, but due to a certain flag being set during minimal tsd
initialization, ticker data remains unallocated. This prevents
decay-based dirty page purging from working, effectively enabling memory
exhaustion over time. [1]
The quirk described above has been addressed (by moving ticker state to
a different structure) in jemalloc's development branch [2], but not in
any numbered jemalloc version released to date (the latest one being
5.2.1 as of this writing).
Work around the problem by ensuring that every thread spawned by
isc_thread_create() starts with a malloc() call. Avoid immediately
calling free() for the dummy allocation to prevent an optimizing
compiler from stripping away the malloc() + free() pair altogether.
An alternative implementation of this workaround was considered that
used a pair of isc_mem_create() + isc_mem_destroy() calls instead of
malloc() + free(), enabling the change to be fully contained within
isc__trampoline_run() (i.e. to not touch struct isc__trampoline), as the
compiler is not allowed to strip away arbitrary function calls.
However, that solution was eventually dismissed as it triggered
ThreadSanitizer reports when tools like dig, nsupdate, or rndc exited
abruptly without waiting for all worker threads to finish their work.
[1] https://github.com/jemalloc/jemalloc/issues/2251
[2] https://github.com/jemalloc/jemalloc/commit/c259323ab3082324100c708109dbfff660d0f4b8
Resolve "Improve functions parameter validation in lib/dns/message.c to prevent accessing the -1 index of an array"
Closes#2898
See merge request isc-projects/bind9!5824
dns_message_findname and dns_message_sectiontotext incorrectly accepted
DNS_SECTION_ANY. If DNS_SECTION_ANY was passed the section array could
be incorrectly accessed at (-1).
dns_message_pseudosectiontotext and dns_message_pseudosectiontoyaml
incorrectly accepted DNS_PSEUDOSECTION_ANY. These functions are
designed to process a single section.
There were two problems in the notify system test when it waited for
log messages to appear: the shellcheck refactoring introduced a call
to `wait_for_log` with a regex, but `wait_for_log` only supports fixed
strings, so it always ran for the full 45 second timeout; and the new
test to ensure that notify messages time out failed to reset the
nextpart pointer, so if the notify messages timed out before the test
ran, it would fail to see them.
This change adds a `wait_for_log_re` helper that matches a regex, and
uses it where appropriate in the notify system test, which stops the
test from waiting longer than necessary; and it resets the nextpart
pointer so that the notify timeout test works reliably.
Closes#3275
When TASKMGR_TRACE=1 is defined, the task and event objects have
detailed tracing information about function, file, line, and
backtrace (to the extent tracked by gcc) where it was created.
At exit, when there are unfinished tasks, they will be printed along
with the detailed information.
The only place where isc_task_sendto() was used was in dns_resolver
unit, where the "sendto" part was actually no-op, because dns_resolver
uses bound tasks. Remove the isc_task_sendto() and
isc_task_sendtoanddetach() functions in favor of using bound tasks
create with isc_task_create_bound().
Additionally, cache the number of running netmgr threads (nworkers)
locally to reduce the number of function calls.
For some applications, it's useful to not listen on full battery of
threads. Add workers argument to all isc_nm_listen*() functions and
convenience ISC_NM_LISTEN_ONE and ISC_NM_LISTEN_ALL macros.
dns_rdata_fromtext and dns_rdata_fromwire now checks that there is
a valid name or oid at the start of the keydata when the key algorithm
is PRIVATEDNS and PRIVATEOID respectively.
dns_rdata_totext now prints out the oid if the algorithm is PRIVATEOID.
Prime the cache with a negative cache DS entry then make a query for
name beneath that entry. This will cause the DS entry to be retieved
as part of the validation process. Each RRset in the ncache entry
will be validated and the trust level for each will be updated.
dig previously set an exit code of 9 when a TCP connection failed
or when a UDP connection timed out, but when the server address is
localhost it's possible for a UDP query to fail with ISC_R_CONNREFUSED.
that code path didn't update the exit code, causing dig to exit with
status 0. we now set the exit code to 9 in this failure case.
Catalog zones change of ownership is special mechanism to facilitate
controlled migration of a member zone from one catalog to another.
It is implemented using catalog zones property named "coo" and is
documented in DNS catalog zones draft version 5 document.
Implement the feature using a new hash table in the catalog zone
structure, which holds the added "coo" properties for the catalog zone
(containing the target catalog zone's name), and the key for the hash
table being the member zone's name for which the "coo" property is being
created.
Change some log messages to have consistent zone name quoting types.
Update the ARM with change of ownership documentation and usage
examples.
Add tests which check newly the added features.
When there are multiple record datasets in a database node of a catalog
zone, and BIND encounters a soft error during processing of a dataset,
it breaks from the loop and doesn't process the other datasets in the
node.
There are cases when this is not desired. For example, the catalog zones
draft version 5 states that there must be a TXT RRset named
`version.$CATZ` with exactly one RR, but it doesn't set a limitation
on possible non-TXT RRsets named `version.$CATZ` existing alongside
with the TXT one. In case when one exists, we will get a processing
error and will not continue the loop to process the TXT RRset coming
next.
Remove the "break" statement to continue processing all record datasets.
When processing a new or updated catalog zone, the record datasets
from the database are being processed in order. This creates a
problem because we need to know the version of the catalog zone
schema to process some of the records differently, but we do not
know the version until the 'version' record gets processed.
Find the 'version' record and process it first, only then iterate over
the database to process the rest, making sure not to process the
'version' record twice.
According to DNS catalog zones draft version 5 document, catalog
zone custom properties must be placed under the "ext" label.
Make necessary changes to support the new custom properties syntax in
catalog zones with version "2" of the schema.
Change the default catalog zones schema version from "1" to "2" in
ARM to prepare for the new features and changes which come starting
from this commit in order to support the latest DNS catalog zones draft
document.
Make some restructuring in ARM and rename the term catalog zone "option"
to "custom property" to better reflect the terms used in the draft.
Change the version of 'catalog1.zone.' catalog zone in the "catz" system
test to "2", and leave the version of 'catalog2.zone.' catalog zone at
version "1" to test both versions.
Add tests to check that the new syntax works only with the new schema
version, and that the old syntax works only with the legacy schema
version catalog zones.
In `+nssearch` mode `dig` starts the next query of the followup lookup
using `start_udp()` or `start_tcp()` calls without waiting for the
previous query to complete.
In UDP mode that happens in the `send_done()` callback of the previous
query, but in TCP mode that happens in the `start_tcp()` call of the
previous query (recursion) which doesn't work because `start_tcp()`
attaches the `lookup->current_query` to the query it is starting, so a
recursive call will result in an assertion failure.
Make the TCP mode to start the next query in `send_done()`, just like in
the UDP mode. During that time the `lookup->current_query` is already
detached by the `tcp_connected()`/`udp_ready()` callbacks.
Mention in the DNSSEC guide in the "revert to unsigned" recipe that you
can publish CDS and CDNSKEY DELETE records to remove the corresponding
DS records from the parent zone.
Update the function that synchronizes the CDS and CDNSKEY DELETE
records. It now allows for the possibility that the CDS DELETE record
is published and the CDNSKEY DELETE record is not, and vice versa.
Also update the code in zone.c how 'dns_dnssec_syncdelete()' is called.
With KASP, we still maintain the DELETE records our self. Otherwise,
we publish the CDS and CDNSKEY DELETE record only if they are added
to the zone. We do still check if these records can be signed by a KSK.
This change will allow users to add a CDS and/or CDNSKEY DELETE record
manually, without BIND removing them on the next zone sign.
Note that this commit removes the check whether the key is a KSK, this
check is redundant because this check is also made in
'dst_key_is_signing()' when the role is set to DST_BOOL_KSK.
Add a test case for a dynamically added CDS DELETE record and make
sure it is not removed when signing the zone. This happens because
BIND maintains CDS and CDNSKEY publishing and it will only allow
CDS DELETE records if the zone is transitioning to insecure. This is
a state that can be identified when using KASP through 'dnssec-policy',
but not when using 'auto-dnssec'.
Commit bf3fffff67 added a Python-based
name server (bin/tests/system/forward/ans11/ans.py) to the "forward"
system test, but did not update bin/tests/system/Makefile.am to ensure
Python is present in the test environment before the "forward" system
test is run. Update bin/tests/system/Makefile.am to enforce that
requirement.
due to a typo in the code, ADB entries were unlinked from their entry
buckets during shutdown if they had a nonzero reference count. they
were only supposed to be unlinked if the reference count was exactly
one (that being the reference held by the bucket itself).
Implement TCP support in the `ans11` Python-based DNS server.
Implement a control command channel in `ans11` to support an optional
silent mode of operation, which, when enabled, will ignore incoming
queries.
In the added check, make the `ans11` the NS server of
"a.root-servers.nil." for `ns3`, so it uses `ans11` (in silent mode)
for the regular (non-forwarded) name resolutions.
This will trigger the "hung fetch" scenario, which was causing `named`
to crash.
- Check that an NS in an authority section returned from a forwarder
which is above the name in a configured "forward first" or "forward
only" zone (i.e., net/NS in a response from a forwarder configured for
local.net) is not cached.
- Test that a DNAME for a parent domain will not be cached when sent
in a response from a forwarder configured to answer for a child.
- Check that glue is rejected if its name falls below that of zone
configured locally.
- Check that an extra out-of-bailiwick data in the answer section is
not cached (this was already working correctly, but was not explicitly
tested before).
The mctx, zonetask and loadtask pools were being destroyed in the
shutdown function where in theory a dangling zone could be still
attached to it.
Move the isc_mem_put() on the pools to the destroy() function.
There's couple of files that modify behaviour of named when started via
bin/tests/system/start.pl. Add those files as CC-1.0 to .reuse/dep5 as
they are just empty placeholders.
Add a test case to check for lingering TCP sockets stuck in the
CLOSE_WAIT state. This can happen if a client sends some garbage after
its first query.
The system test runs the reproducer script and then sends another TCP
query to the resolver. The resolver is configured to allow one TCP
client only. If BIND has its TCP socket stuck in CLOSE_WAIT, it does
not have the resources available to answer the second query.
Note: A better test would be to check if the named daemon does not
have a TCP socket stuck in CLOSE_WAIT for example with netstat. When
running this test locally you can examine named with netstat manually.
But since netstat is platform specific it is not a good candidate to do
this as a system test.
If you, if you could return, don't let it burn.
Do you have to let it linger?
- Cranberries
This allows Gitlab to show nice summary for individual tests/test
directories and to expose the results in Gitlab API for consumption
elsewhere.
A catch: As of Gitlab 14.7.7, the detailed results are stored
only in artifacts and thus expire. All consumers (including API) need
to be "fast enough" to get the data before they disappear.
This also forces us to always store the artifacts intead of storing them
only on failure.
There are a couple of problems with dns_request_createvia(): a UDP
retry count of zero means unlimited retries (it should mean no
retries), and the overall request timeout is not enforced. The
combination of these bugs means that requests can be retried forever.
This change alters calls to dns_request_createvia() to avoid the
infinite retry bug by providing an explicit retry count. Previously,
the calls specified infinite retries and relied on the limit implied
by the overall request timeout and the UDP timeout (which did not work
because the overall timeout is not enforced). The `udpretries`
argument is also changed to be the number of retries; previously, zero
was interpreted as infinity because of an underflow to UINT_MAX, which
appeared to be a mistake. And `mdig` is updated to match the change in
retry accounting.
The bug could be triggered by zone maintenance queries, including
NOTIFY messages, DS parental checks, refresh SOA queries and stub zone
nameserver lookups. It could also occur with `nsupdate -r 0`.
(But `mdig` had its own code to avoid the bug.)
Implement reference counting for TLS contexts, Resolve#3122 DoT stops working after "rndc reconfigure" when running named as non-root
Closes#3122
See merge request isc-projects/bind9!6087
This commit makes use of isc_nmsocket_set_tlsctx(). Now, instead of
recreating TLS-enabled listeners (including the underlying TCP
listener sockets), only the TLS context in use is replaced.
This commit adds isc_nmsocket_set_tlsctx() - an asynchronous function
that replaces the TLS context within a given TLS-enabled listener
socket object. It is based on the newly added reference counting
functionality.
The intention of adding this function is to add functionality to
replace a TLS context without recreating the whole socket object,
including the underlying TCP listener socket, as a BIND process might
not have enough permissions to re-create it fully on reconfiguration.
The implementation is done on top of the reference counting
functionality found in OpenSSL/LibreSSL, which allows for avoiding
wrapping the object.
Adding this function allows using reference counting for TLS contexts
in BIND 9's codebase.
After some back and forth, it was decidede to match the configuration
option with unbound ("so-reuseport"), PowerDNS ("reuseport") and/or
nginx ("reuseport").
as far as I can determine the order of operations is not important.
*** CID 351372: Concurrent data access violations (ATOMICITY)
/lib/isc/timer.c: 227 in timer_purge()
221 LOCK(&timer->lock);
222 if (!purged) {
223 /*
224 * The event has already been executed, but not
225 * yet destroyed.
226 */
>>> CID 351372: Concurrent data access violations (ATOMICITY)
>>> Using an unreliable value of "event" inside the second locked section. If the data that "event" depends on was changed by another thread, this use might be incorrect.
227 timerevent_unlink(timer, event);
228 }
229 }
230 }
231
232 void
*** CID 351371: Null pointer dereferences (REVERSE_INULL)
/lib/dns/adb.c: 2615 in dns_adb_createfind()
2609 /*
2610 * Copy out error flags from the name structure into the find.
2611 */
2612 find->result_v4 = find_err_map[adbname->fetch_err];
2613 find->result_v6 = find_err_map[adbname->fetch6_err];
2614
>>> CID 351371: Null pointer dereferences (REVERSE_INULL)
>>> Null-checking "find" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
2615 if (find != NULL) {
2616 if (want_event) {
2617 INSIST((find->flags & DNS_ADBFIND_ADDRESSMASK) != 0);
2618 isc_task_attach(task, &(isc_task_t *){ NULL });
2619 find->event.ev_sender = task;
2620 find->event.ev_action = action;
The error code path handling the `ISC_R_CANCELED` code lacks a
`clear_current_lookup()` call, without which dig hangs indefinitely
when handling the error.
Add the missing call to account for all references of the lookup so
it can be destroyed.
In `send_udp()` and `launch_next_query()` functions, when calling
`dighost_printmessage()` to print detailed information about the
sent query, dig always prints the data of the first query in the
lookup's queries list.
The first query in the list can be already finished, having its handles
freed, and accessing this information results in assertion failure.
Print the current query's information instead.
Previously, HAVE_SO_REUSEPORT_LB has been defined only in the private
netmgr-int.h header file, making the configuration of load balanced
sockets inoperable.
Move the missing HAVE_SO_REUSEPORT_LB define the isc/netmgr.h and add
missing isc_nm_getloadbalancesockets() implementation.
Previously, the option to enable kernel load balancing of the sockets
was always enabled when supported by the operating system (SO_REUSEPORT
on Linux and SO_REUSEPORT_LB on FreeBSD).
It was reported that in scenarios where the networking threads are also
responsible for processing long-running tasks (like RPZ processing, CATZ
processing or large zone transfers), this could lead to intermitten
brownouts for some clients, because the thread assigned by the operating
system might be busy. In such scenarious, the overall performance would
be better served by threads competing over the sockets because the idle
threads can pick up the incoming traffic.
Add new configuration option (`load-balance-sockets`) to allow enabling
or disabling the load balancing of the sockets.
Previously, the RPZ updates ran quantized on the main nm_worker loops.
As the quantum was set to 1024, this might lead to service
interruptions when large RPZ update was processed.
Change the RPZ update process to run as the offloaded work. The update
and cleanup loops were refactored to do as little locking of the
maintenance lock as possible for the shortest periods of time and the db
iterator is being paused for every iteration, so we don't hold the rbtdb
tree lock for prolonged periods of time.
Previously dns_rpz_add() were passed dns_rpz_zones_t and index to .zones
array. Because we actually attach to dns_rpz_zone_t, we should be using
the local pointer instead of passing the index and "finding" the
dns_rpz_zone_t again.
Additionally, dns_rpz_add() and dns_rpz_delete() were used only inside
rpz.c, so make them static.
Do a general cleanup of lib/dns/rpz.c style:
* Removed deprecated and unused functions
* Unified dns_rpz_zone_t naming to rpz
* Unified dns_rpz_zones_t naming to rpzs
* Add and use rpz_attach() and rpz_attach_rpzs() functions
* Shuffled variables to be more local (cppcheck cleanup)
Now that the dns_aclenv_t has now properly rwlocked .localhost and
.localnets member, we can remove the task exclusive mode use from the
ns_interfacemgr. Some light related cleanup has been also done.
In order to modify the .localhost and .localnets members of the
dns_aclenv, all other processing on the netmgr loops needed to be
stopped using the task exclusive mode. Add the isc_rwlock to the
dns_aclenv, so any modifications to the .localhost and .localnets can be
done under the write lock.
In recv_done(), when dig decides to start the lookup's next query in
the line using `start_udp()` or `start_tcp()`, and for some reason,
no queries get started, dig doesn't cancel the lookup.
This can occur, for example, when there are two queries in the lookup,
one with a regular IP address, and another with a IPv4 mapped IPv6
address. When the regular IP address fails to serve the query, its
`recv_done()` callback starts the next query in the line (in this
case the one with a mapped IP address), but because `dig` doesn't
connect to such IP addresses, and there are no other queries in the
list, no new queries are being started, and the lookup keeps hanging.
After calling `start_udp()` or `start_tcp()` in `recv_done()`, check
if there are no pending/working queries then cancel the lookup instead
of only detaching from the current query.
The reference counting and isc_timer_attach()/isc_timer_detach()
semantic are actually misleading because it cannot be used under normal
conditions. The usual conditions under which is timer used uses the
object where timer is used as argument to the "timer" itself. This
means that when the caller is using `isc_timer_detach()` it needs the
timer to stop and the isc_timer_detach() does that only if this would be
the last reference. Unfortunately, this also means that if the timer is
attached elsewhere and the timer is fired it will most likely be
use-after-free, because the object used in the timer no longer exists.
Remove the reference counting from the isc_timer unit, remove
isc_timer_attach() function and rename isc_timer_detach() to
isc_timer_destroy() to better reflect how the API needs to be used.
The only caveat is that the already executed event must be destroyed
before the isc_timer_destroy() is called because the timer is no longet
attached to .ev_destroy_arg.
Previously, the task privileged mode has been used only when the named
was starting up and loading the zones from the disk as the "first" thing
to do. The privileged task was setup with quantum == 2, which made the
taskmgr/netmgr spin around the privileged queue processing two events at
the time.
The same effect can be achieved by setting the quantum to UINT_MAX (e.g.
practically unlimited) for the loadzone task, hence the privileged task
mode was removed in favor of just processing all the events on the
loadzone task in a single task_run().
Instead of passing the number of worker to the dns_zonemgr manually,
get the number of nm threads using the new isc_nm_getnworkers() call.
Additionally, remove the isc_pool API and manage the array of memory
context, zonetasks and loadtasks directly in the zonemgr.
After switching to per-thread resources in the zonemgr, the performance
was decreased because the memory context, zonetask and loadtask was
picked from the pool at random.
Pin the zone to single threadid (.tid) and align the memory context,
zonetask and loadtask to be the same, this sets the hard affinity of the
zone to the netmgr thread.
The zone counting in the named was used to properly size the zonemgr
resources (memory contexts, zonetasks and loadtasks). Since this is no
longer the case, remove the whole zone counting from named.
Previously, the zonemgr created 1 task per 100 zones and 1 memory
context per 1000 zones (with minimum 10 tasks and 2 memory contexts) to
reduce the contention between threads.
Instead of reducing the contention by having many resources, create a
per-nm_thread memory context, loadtask and zonetask and spread the zones
between just per-thread resources.
Note: this commit alone does decrease performance when loading the zone
by couple seconds (in case of 1M zone) and thus there's more work in
this whole MR fixing the performance.
Previously, the zone timer was not stopped before detaching the timer.
This could lead to a data race where the timer post_event() could fire
before the timer was detached, but then the event would be executed
after the zone was already destroyed.
This was not noticed before because the timing or the ordering of the
actions were different, but it was causing assertion failures in the
libns tests now.
Properly stop the zone timer before detaching the timer object from the
dns_zone.
When we are loading the zones, set the quantum to UINT_MAX, which makes
task_run process all tasks at once. After the zone loading is finished
the quantum will be dropped to 1 to not block server when we are loading
new zones after reconfiguration.
Add isc_task_setquantum() function that modifies quantum for the future
isc_task_run() invocations.
NOTE: The current isc_task_run() caches the task->quantum into a local
variable and therefore the current event loop is not affected by any
quantum change.
The isc_task_purge() and isc_task_purgerange() were now unused, so sweep
the task.c file. Additionally remove unused ISC_EVENTATTR_NOPURGE event
attribute.
The isc_task_purgerange() was walking through all events on the task to
find a matching task. Instead use the ISC_LINK_LINKED to find whether
the event is active.
Cleanup the related isc_task_unsend() and isc_task_unsendrange()
functions that were not used anywhere.
Adding extra val & 0xffff in the isc_hash_bits32() macros in the hotpath
has significantly reduced the performance. Turn the macro into static
inline function matching the previous hash_32() function used to compute
hashval matching the hashtable->bits.
a test case in the 'resolver' system test was reliant on
logged output that would only be present when query tracing
was enabled, as in developer builds. that test case is now
disabled when query tracing is not available. Thanks to
Anton Castelli.
The `udp_ready()` and `tcp_connected()` functions in dighost.c are
used for similar purposes for UDP and TCP respectively.
Synchronize the `udp_ready()` function entry code to behave like
`tcp_connected()` by adding input validation, debug messages and
early exit code when `cancel_now` is `true`.
When finishing the NSSEARCH task and there is no more followup
lookups to start, dig does not destroy the last lookup, which
causes it to hang indefinitely.
Rename the unused `first_pass` member of `dig_query_t` to `started`
and make it `true` in the first callback after `start_udp()` or
`start_tcp()` of the query to indicate that the query has been
started.
Create a new `check_if_queries_done()` function to check whether
all of the queries inside a lookup have been started and finished,
or canceled.
Use the mentioned function in the TRACE code block in `recv_done()`
to check whether the current query is the last one in the lookup and
cancel the lookup in that case to free the resources.
the line "$GENERATE 19-28/2147483645 $ CNAME x" should generate
a single CNAME with the owner "19.example.com", but prior to the
overflow bug it generated several CNAMEs, half of them with large
negative values.
we now test for the bugfix by using "named-checkzone -D" and
grepping for a single CNAME in the output.
the value of 'i' in generate could overflow when adding 'step' to
it in the 'for' loop. Use an unsigned int for 'i' which will give
an additional bit and prevent the overflow. The inputs are both
less than 2^31 and and the result will be less than 2^32-1.
Ensure the update zone name is mentioned in the NOTAUTH error message
in the server log, so that it is easier to track down problematic
update clients. There are two cases: either the update zone is
unrelated to any of the server's zones (previously no zone was
mentioned); or the update zone is a subdomain of one or more of the
server's zones (previously the name of the irrelevant parent zone was
misleadingly logged).
Closes#3209
The .lock, .exiting and .excl members were not using for anything else
than starting task exclusive mode, setting .exiting to true and ending
exclusive mode.
Remove all the stray members and dead code eliminating the task
exclusive mode use from ns_clientmgr.
The ADB previously used separate reference counters for internal
and external references, plus additional counters for ABD find
and namehook objects, and used all these counters to coordinate
its shutdown process, which was a multi-stage affair involving
a sequence of control events.
It also used a complex interlocking set of static functions for
referencing, deferencing, linking, unlinking, and cleaning up various
internal objects; these functions returned boolean values to their
callers to indicate what additional processing was needed.
The changes in the previous two commits destabilized this fragile
system in a way that was difficult to recover from, so in this commit
we refactor all of it. The dns_adb and dns_adbentry objects now use
conventional attach and detach functions for reference counting, and
the shutdown process is much more straightforward. Instead of
handling shutdown asynchronously, we can just destroy the ADB when
references reach zero
In addition, ADB locking has been simplified. Instead of a
single `find_{name,entry}_and_lock()` function which searches for
a name or entry's hash bucket, locks it, and then searches for the
name or entry in the bucket, we now use one function to find the
bucket (leaving it to the caller to do the locking) and another
find the name or entry. Instead of locking the entire ADB when
modifying hash tables, we now use read-write locks around the
specific hash table. The only remaining need for adb->lock
is when modifying the `whenshutdown` list.
Comments throughout the module have been improved.
Replace adb->{names,entries} and related arrays (indexed by hashed
bucket) with a isc_ht hash tables storing the new struct
adb{name,entry}bucket_t that wraps all the variables that were
originally stored in arrays indexed by "bucket" number stored directly
in the struct dns_adb.
Previously, the task exclusive mode has been used to grow the internal
arrays used to store the named and entries objects. The isc_ht hash
tables are now protected by the isc_rwlock instead and thus the usage of
the task exclusive mode has been removed from the dns_adb.
Co-authored-by: Ondřej Surý <ondrej@isc.org>
the use of "result" as a variable name for a boolean return value
was confusing; all 'result' variables that are not isc_result_t
have been renamed to 'ret'.
The static function print_dns_name() was a duplicate of
dns_name_print(), so it has been replaced with that.
Changed INSIST to REQUIRE where appropriate, and added NULL
initialization for pointer variables.
The use of isc_appctx_t in dns_client was used to wait for
dns_client_startresolve() to finish the processing (the resolve_done()
task callback).
This has been replaced with standard bool+cond+lock combination removing
the need of isc_appctx_t altogether.
Previously, the isc_ht API would always take the key as a literal input
to the hashing function. Change the isc_ht_init() function to take an
'options' argument, in which ISC_HT_CASE_SENSITIVE or _INSENSITIVE can
be specified, to determine whether to use case-sensitive hashing in
isc_hash32() when hashing the key.
Fibonacci hashing was implemented in four separate places (rbt.c,
rbtdb.c, resolver.c, zone.c). This commit combines them into a single
implementation. The hash_32() function is now replaced with
isc_hash_bits32().
In couple places, we have missed INSIST(0) or ISC_UNREACHABLE()
replacement on some branches with UNREACHABLE(). Replace all
ISC_UNREACHABLE() or INSIST(0) calls with UNREACHABLE().
This commit adds points to the CHANGES and the release notes about
supporting remote TLS certificates verification and support for Strict
and Mutual TLS transport connections verification.
Mention that some old cryptographic library versions lack the
functionality to implement ignoring the Subject field (and thus the
Common Name) when establishing DoT connections.
This commit extends the 'doth' system test with a set of Strict/Mutual
TLS related checks.
This commit also makes each doth NS instance use its own TLS
certificate that includes FQDN, IPv4, and IPv6 addresses, issued using
a common Certificate Authority, instead of ad-hoc certs.
Extend servers initialisation timeout to 60 seconds to improve the
tests stability in the CI as certain configurations could fail to
initialise on time under load.
This commit updates the reference manual with short descriptions of
different TLS authentication modes, as mentioned in the RFC 9103,
Section 9.3 (Opportunistic TLS, Strict TLS, Mutual TLS), and mentions
how these authentication modes can be achieved via BIND's
configuration file.
This commit adds support for Strict/Mutual TLS into BIND. It does so
by implementing the backing code for 'hostname' and 'ca-file' options
of the 'tls' statement. The commit also updates the documentation
accordingly.
This commit adds support for Strict/Mutual TLS to dig.
The new command-line options and their behaviour are modelled after
kdig (+tls-ca, +tls-hostname, +tls-certfile, +tls-keyfile) for
compatibility reasons. That is, using +tls-* is sufficient to enable
DoT in dig, implying +tls-ca
If there is no other DNS transport specified via command-line,
specifying any of +tls-* options makes dig use DoT. In this case, its
behaviour is the same as if +tls-ca is specified: that is, the remote
peer's certificate is verified using the platform-specific
intermediate CA certificates store. This behaviour is introduced for
compatibility with kdig.
This commit adds support for ISC_R_TLSBADPEERCERT error code, which is
supposed to be used to signal for TLS peer certificates verification
in dig and other code.
The support for this error code is added to our TLS and TLS DNS
implementations.
This commit also adds isc_nm_verify_tls_peer_result_string() function
which is supposed to be used to get a textual description of the
reason for getting a ISC_R_TLSBADPEERCERT error.
This commit adds support for keeping CA certificates stores associated
with TLS contexts. The intention is to keep one reusable store per a
set of related TLS contexts.
This commit adds a set of functions that can be used to implement
Strict and Mutual TLS:
* isc_tlsctx_load_client_ca_names();
* isc_tlsctx_load_certificate();
* isc_tls_verify_peer_result_string();
* isc_tlsctx_enable_peer_verification().
This commit adds a set of high-level utility functions to manipulate
the certificate stores. The stores are needed to implement TLS
certificates verification efficiently.
There is a possible code path of using the uninitialized `bname`
character array while logging an error message.
Initialize the `bname` buffer earlier in the function.
Also, change the initialization routine to use a helper function.
A successful call to `dns_rdata_tostruct()` expects an accompanying
call to `dns_rdata_freestruct()` to free up any memory that could have
been allocated during the first call.
In catz.c there are several places where `dns_rdata_freestruct()` call
is skipped.
Add the missing cleanup routines.
Because of the "goto" in the "if" body the "else" part is unnecessary
and adds another level of indentation.
Cleanup the code to not have the "else" part.
Catz logs a warning message when it is told to modify a zone which was
not added by the current catalog zone.
When logging a warning, distinguish the two cases when the zone
was not added by a catalog zone at all, and when the zone was
added by a different catalog zone.
The dnssec-settime -p and -up options print times in asctime() and
UNIX time_t formats, respectively. The asctime() format can also be
found inside K*.key public key files. Key files also contain times in
the YYYYMMDDHHMMSS format that can be used in timing parameter
options.
The dnssec-settime -p and -up time formats are now acceptable in
timing parameter options to dnssec-settime and dnssec-keygen, so it is
no longer necessary to parse key files to retrieve times that are
acceptable in timing parameter options.
The way the ns_client_t .shuttingdown member was practically dead code.
The .shuttingdown would be set to true only in ns__client_put() function
meaning that we have detached from all ns_client_t .*handles and the
ns_client_t object being freed:
client->magic = 0;
client->shuttingdown = true;
[...]
isc_mem_put(manager->ctx, client, sizeof(*client))
Meanwhile the ns_client_t object is accessed like this:
isc_nmhandle_detach(&client->fetchhandle);
client->query.attributes &= ~NS_QUERYATTR_RECURSING;
client->state = NS_CLIENTSTATE_WORKING;
qctx_init(client, &devent, 0, &qctx);
client_shuttingdown = ns_client_shuttingdown(client);
if (fetch_canceled || fetch_answered || client_shuttingdown) {
[...]
}
Even if the isc_nmhandle_detach(...) was the last handle detach, it
would mean that immediatelly, after calling the isc_nmhandle_detach(),
we would be causing use-after-free, because the ns_client_t is
immediatelly destroyed after setting .shuttingdown to true.
The similar code in the query_hookresume() already noticed this:
/*
* This event is running under a client task, so it's safe to detach
* the fetch handle. And it should be done before resuming query
* processing below, since that may trigger another recursion or
* asynchronous hook event.
*/
Previously, it was possible to assign a bit of memory space in the
nmhandle to store the client data. This was complicated and prevents
further refactoring of isc_nmhandle_t caching (future work).
Instead of caching the data in the nmhandle, allocate the hot-path
ns_client_t objects from per-thread clientmgr memory context and just
assign it to the isc_nmhandle_t via isc_nmhandle_set().
The ns_client_t is always attached to ns_clientmgr_t which has
associated memory context, server context, task and threadid. Use those
directly from the ns_clientmgr_t instead of attaching it to an extra
copy in ns_client_t to make the ns_client_t more sleek and lean.
Additionally, remove some stray ns_client_t struct members that were not
used anywhere.
Some ancient versions of clang reported uninitialized memory use false
positive (see https://bugs.llvm.org/show_bug.cgi?id=14461). Since clang
4.0.1 has been long obsoleted, just remove the workarounds.
Historically, the inline keyword was a strong suggestion to the compiler
that it should inline the function marked inline. As compilers became
better at optimising, this functionality has receded, and using inline
as a suggestion to inline a function is obsolete. The compiler will
happily ignore it and inline something else entirely if it finds that's
a better optimisation.
Therefore, remove all the occurences of the inline keyword with static
functions inside single compilation unit and leave the decision whether
to inline a function or not entirely on the compiler
NOTE: We keep the usage the inline keyword when the purpose is to change
the linkage behaviour.
C11 has builtin support for _Noreturn function specifier with
convenience noreturn macro defined in <stdnoreturn.h> header.
Replace ISC_NORETURN macro by C11 noreturn with fallback to
__attribute__((noreturn)) if the C11 support is not complete.
Previously, the unreachable code paths would have to be tagged with:
INSIST(0);
ISC_UNREACHABLE();
There was also older parts of the code that used comment annotation:
/* NOTREACHED */
Unify the handling of unreachable code paths to just use:
UNREACHABLE();
The UNREACHABLE() macro now asserts when reached and also uses
__builtin_unreachable(); when such builtin is available in the compiler.
Gcc 7+ and Clang 10+ have implemented __attribute__((fallthrough)) which
is explicit version of the /* FALLTHROUGH */ comment we are currently
using.
Add and apply FALLTHROUGH macro that uses the attribute if available,
but does nothing on older compilers.
In one case (lib/dns/zone.c), using the macro revealed that we were
using the /* FALLTHROUGH */ comment in wrong place, remove that comment.
Instead of passing the "workers" variable back and forth along with
passing the single isc_nm_t instance, add isc_nm_getnworkers() function
that returns the number of netmgr threads are running.
Change the ns_interfacemgr and ns_taskmgr to utilize the newly acquired
knowledge.
From an attacker's point of view, a VLA declaration is essentially a
primitive for performing arbitrary arithmetic on the stack pointer. If
the attacker can control the size of a VLA they have a very powerful
tool for causing memory corruption.
To mitigate this kind of attack, and the more general class of stack
clash vulnerabilities, C compilers insert extra code when allocating a
VLA to probe the growing stack one page at a time. If these probes hit
the stack guard page, the program will crash.
From the point of view of a C programmer, there are a few things to
consider about VLAs:
* If it is important to handle allocation failures in a controlled
manner, don't use VLAs. You can use VLAs if it is OK for
unreasonable inputs to cause an uncontrolled crash.
* If the VLA is known to be smaller than some known fixed size,
use a fixed size array and a run-time check to ensure it is large
enough. This will be more efficient than the compiler's stack
probes that need to cope with arbitrary-size VLAs.
* If the VLA might be large, allocate it on the heap. The heap
allocator can allocate multiple pages in one shot, whereas the
stack clash probes work one page at a time.
Most of the existing uses of VLAs in BIND are in test code where they
are benign, but there was one instance in `named`, in the GSS-TSIG
verification code, which has now been removed.
This commit adjusts the style guide and the C compiler flags to allow
VLAs in test code but not elsewhere.
In the GSS-TSIG verification code there was an alarming
variable-length array whose size came off the network, from the
signature in the request. It turned out to be safe, because the caller
had previously checked that the signature had a reasonable size.
However, the safety checks are in the generic TSIG implementation, and
the risky VLA usage was in the GSS-specific code, and they are
separated by the DST indirection layer, so it wasn't immediately
obvious that the risky VLA was in fact safe.
In fact this risky VLA was completely unnecessary, because the GSS
signature can be verified in place without being copied to the stack,
like the message covered by the signature. The `REGION_TO_GBUFFER()`
macro backwardly assigns the region in its left argument to the GSS
buffer in its right argument; this is just a pointer and length
conversion, without copying any data. The `gss_verify_mic()` call uses
both message and signature GSS buffers in a read-only manner.
Rework the "ans8" server in the "digdelv" system test to support various
modes of operations using a control channel.
The supported modes are:
1. `silent` (do not respond)
2. `close` (UDP: same as `silent`; TCP: also close the connection)
3. `servfail` (always respond with `SERVFAIL`)
4. `unstable` (constantly switch between `silent` and `servfail`)
Add multiple tests to check the handling of both TCP and UDP socket
error scenarios in dig/host.
When encountering a TCP connection error while trying to initiate a
connection to a server, dig erroneously cancels the lookup even when
there are other server(s) to try, which results in an assertion failure.
Cancel the lookup only when there are no more queries left in the
lookup's queries list (i.e. `next` is NULL).
Add a test to check whether dig tries the next query/server after
a connection error.
Add a test to check whether dig tries the next query/server after
a one or more (default is 3) connection/request timeouts.
When timing-out or having other types of socket errors during a query,
dig isn't trying to perform the lookup using other servers which exist
in the lookup's queries list.
After configured amount of timeout retries, or after a socket error,
check if there are other queries/servers in the lookup's queries list,
and start the next one if it exists, instead of unconditionally failing.
This test ensures that `dig` retries with another attempt after a
timed-out request, and that it does not crash when the retried
request returns a SERVFAIL result. See [GL #3020] for the latter
issue.
When a query times out, and `dig` (or `host`) creates a new query
to resend the request, it is being prepended to the lookup's queries
list, which can cause a confusion later, making `dig` (or `host`)
believe that there is another new query in the list, but that is
actually the old one, which was timed out. That mistake will result
in an assertion failure.
That can happen, in particular, when after a timed out request,
the retried request returns a SERVFAIL result, and the recursion
is enabled, and `+nofail` option was used with `dig` (that is the
default behavior in `host`, unless the `-s` option is provided).
Fix the problem by inserting the query just after the current,
timed-out query, instead of prepending to the list.
Before calling start_udp() detach `l->current_query`, like it is
done in another place in the function.
Slightly update a couple of debug messages to make them more
consistent.
After a query results in a SERVFAIL result, and there is another
registered query in the lookup's queries list, `dig` starts the next
query to try another server, but for some reason, reports about that
also when the current query is in the head of the list, even if there
is no other query in the list to try.
Use the same condition for both decisions, and after starting the next
query, jump to the "detach_query" label instead of "next_lookup",
because there is no need to start the next lookup after we just started
a query in the current lookup.
When max-transfer-*-out timeouts were reintroduced, the log message
about starting the timer was errorneously left as ISC_LOG_ERROR.
Change the log level of said message to ISC_LOG_DEBUG(1).
The clang-format-15 has new option InsertBraces that could add missing
branches around single line statements. Use that to our advantage
without switching to not-yet-released LLVM version to add missing braces
in couple of places.
As incremental rehashing has been added to isc_ht implementation, we
need to test whether the rehashing works.
Update the isc_ht unit test to test:
* preinitialized hash table large enough to hold all the elements
* smallest hash table that fully grows to hold all the elements
* partially preinitialized hash table that grows
* iterating while rehashing is in progress
Previously, an incremental hash table resizing was implemented for the
dns_rbt_t hash table implementation. Using that as a base, also
implement the incremental hash table resizing also for isc_ht API
hashtables:
1. During the resize, allocate the new hash table, but keep the old
table unchanged.
2. In each lookup, delete, or iterator operation, check both tables.
3. Perform insertion operations only in the new table.
4. At each insertion also move <r> elements from the old table to
the new table.
5. When all elements are removed from the old table, deallocate it.
To ensure that the old table is completely copied over before the new
table itself needs to be enlarged, it is necessary to increase the
size of the table by a factor of at least (<r> + 1)/<r> during resizing.
In our implementation <r> is equal to 1.
The downside of this approach is that the old table and the new table
could stay in memory for longer when there are no new insertions into
the hash table for prolonged periods of time as the incremental
rehashing happens only during the insertions.
The fetch can be in the shutting down state when resume_dslookup() is
trying to operate on it.
This is also a security issue, because a malicious actor can set up a
name server which delays certain queries in such a way that the fetch
will time out and shut down, which will cause named to crash.
Add a check to see if the fetch has the shutting down attribute set,
and cancel any further operations on it in such case.
A similar bug had been fixed earlier for the resume_qmin() function,
see [GL #966].
This is an optimisation as we can skip a lot of pointless work when we
know there is a DNAME there.
When we have a partial match and a DNAME above the QNAME, the closest
encloser has the same owner as the DNAME, will have the DNAME bit set
in the type map, and we wouldn't use it as we would return the
DNAME + RRSIG(DNAME) instead.
So there is no point in looking for it nor in attempting to check that
it is valid for the QNAME.
When sock->closehandle_cb is set, we need to run nmhandle_detach_cb()
asynchronously to ensure correct order of multiple packets processing in
the isc__nm_process_sock_buffer(). When not run asynchronously, it
would cause:
a) out-of-order processing of the return codes from processbuffer();
b) stack growth because the next TCP DNS message read callback will
be called from within the current TCP DNS message read callback.
The sock->closehandle_cb is set to isc__nm_resume_processing() for TCP
sockets which calls isc__nm_process_sock_buffer(). If the read callback
(called from isc__nm_process_sock_buffer()->processbuffer()) doesn't
attach to the nmhandle (f.e. because it wants to drop the processing or
we send the response directly via uv_try_write()), the
isc__nm_resume_processing() (via .closehandle_cb) would call
isc__nm_process_sock_buffer() recursively.
The below shortened code path shows how the stack can grow:
1: ns__client_request(handle, ...);
2: isc_nm_tcpdns_sequential(handle);
3: ns_query_start(client, handle);
4: query_lookup(qctx);
5: query_send(qctcx->client);
6: isc__nmhandle_detach(&client->reqhandle);
7: nmhandle_detach_cb(&handle);
8: sock->closehandle_cb(sock); // isc__nm_resume_processing
9: isc__nm_process_sock_buffer(sock);
10: processbuffer(sock); // isc__nm_tcpdns_processbuffer
11: isc_nmhandle_attach(req->handle, &handle);
12: isc__nm_readcb(sock, req, ISC_R_SUCCESS);
13: isc__nm_async_readcb(NULL, ...);
14: uvreq->cb.recv(...); // ns__client_request
Instead, if 'sock->closehandle_cb' is set, we need to run detach the
handle asynchroniously in 'isc__nmhandle_detach', so that on line 8 in
the code flow above does not start this recursion. This ensures the
correct order when processing multiple packets in the function
'isc__nm_process_sock_buffer()' and prevents the stack growth.
When not run asynchronously, the out-of-order processing leaves the
first TCP socket open until all requests on the stream have been
processed.
If the pipelining is disabled on the TCP via `keep-response-order`
configuration option, named would keep the first socket in lingering
CLOSE_WAIT state when the client sends an incomplete packet and then
closes the connection from the client side.
'setup_delegation' depends on 'foundname' being the value returned
by 'dns_rbt_findnode' in the cache and 'find_coveringnsec' was
modifying 'foundname' when a covering NSEC was not found.
When caching glue, we need to ensure that there is no closer
source of truth for the name. If the owner name for the glue
record would be answered by a locally configured zone, do not
cache.
When caching additional and glue data *not* from a forwarder, we must
check that there is no "forward only" clause covering the owner name
that would take precedence. Such names would normally be allowed by
baliwick rules, but a "forward only" zone introduces a new baliwick
scope.
If we are using a fowarder, in addition to checking that names to
be cached are subdomains of the forwarded namespace, we must also
check that there are no subsidiary forwarded namespaces which would
take precedence. To be safe, we don't cache any responses if the
forwarding configuration has changed since the query was sent.
Commit 4ca74eee49 update the zone grammar
such that the zone statement is printed with the valid options per
zone type.
This commit is a follow-up, putting back the ZONE heading and adding
a note that these zone statements may also be put inside the view
statement.
It is tricky to actually print the zone statements inside
the view statement, and so we decided that we would add a note to say
that this is possible.
Change the isc_interval_t implementation from separate data type and
separate implementation to be shim implementation on top of isc_time_t.
The distinction between isc_interval_t and isc_time_t has been kept
because they are semantically different - isc_interval_t is relative and
isc_time_t is absolute, but this allows isc_time_t and isc_interval_t to
be freely interchangeable, f.e. this:
isc_time_t *t1;
isc_interval_t *interval;
isc_time_t *t2;
isc_interval_set(interval, isc_time_seconds(t2), isc_time_nanoseconds(t2);;
isc_time_subtract(t1, interval, t2);
isc_interval_set(interval, isc_time_seconds(t2), isc_time_nanoseconds(t2));
to just:
isc_time_t *t1;
isc_interval_t *interval;
isc_time_t *t2;
isc_time_subtract(t1, t2, interval);
without introducing a whole set of new functions.
The isc_timer_reset() now works only with intervals for once timers.
This makes the API almost 1:1 compatible with the libuv timers making
the further refactoring possible.
There were two places where expires argument (absolute isc_time_t value)
was being used. Both places has been converted to use relative interval
argument in preparation of simplification and refactoring of isc_timer
API.
The isc_timer_create() function was a bit conflated. It could have been
used to create a timer and start it at the same time. As there was a
single place where this was done before (see the previous commit for
nta.c), this was cleaned up and the isc_timer_create() function was
changed to only create new timer.
In nta.c, it was the only place where the active timer was created
directly instead of first creating inactive timer and then starting it
with isc_timer_reset().
Change the code to create inactive timer first, so we can refactor the
isc_timer_create() function.
While backporting !5934 I noticed a copy&paste mistake in TSIG
chapter of the ARM.
The incorrect reference was introduced by "Add hyperlinks from
program options to definition in man pages" commit but it is not
worth creating separate MR for that when the backport is not merged
yet.
(cherry picked from commit 4daef4a2a7)
Replace :manpage: with :iscman: to generate internal hyperlinks. That
way reader can use links even when offline, and jumps to man pages
for the same version.
Formerly HTML version of man pages did not have links in See Also
section because :manpage: role in Sphinx can generate only external
hyperlinks - and we do not have that enabled.
Enabling the Sphinx :manpage: linking could reliably create hyperlinks
only to external URLs, but that would take users to another version
of docs.
Generated by:
find bin -name '*.rst' | xargs sed -i -e 's/:manpage:`\([^(]\+\)(\([0-9]\))`/:iscman:`\1(\2) <\1>`/g'
+ hand-edit to revert change for mmencode reference which is
not provided in our source tree.
Use the new role :iscman: to replace all occurences or ``binary``
with :iscman:`binary`, creating a hyperlink to the manual page.
Generated using:
find bin -name *.rst | xargs fgrep --files-with-matches '.. iscman' | xargs -I{} -n1 basename {} .rst > /tmp/progs
for PROG in $(cat /tmp/progs); do find -name '*.rst' | xargs sed -i -e "s/\`\`$PROG\`\`/:iscman:\`$PROG\`/g"; done
Additional hand-edits were done mainly around filter-aaaa and
filter-a which are program names and and option names at the
same time. Couple more edits was neede to fix .rst syntax broken by
automatic replacement.
Sphinx has it's own :program: syntax for refering to program names.
Use it for self-references in manual pages. These self-references are
not clickable and not as eye-cathing as links, which is a good thing.
There is no point in attracting attention to ``dig`` several times on a
single page dedicated to dig itself.
Substituted automatically using:
find bin -name *.rst | xargs fgrep --files-with-matches '.. program' | xargs -n1 bash /tmp/repl.sh
With /tmp/repl.sh being:
BASE=$(basename "$1" .rst)
sed -i -e "s/\`\`$BASE\`\`/:program:\`$BASE\`/g" "$1"
The new directive and role "iscman" allow to tag & reference man pages in
our source tree. Essentially it is just namespacing for ISC man pages,
but it comes with couple benefits.
Differences from .. _man_program label we formerly used:
- Does not expand :ref:`man_program` into full text of the page header.
- Generates index entry with category "manual page".
- Rendering style is closer to ubiquitous to the one produced
by ``named`` syntax.
Differences from Sphinx built-in :manpage: role:
- Supports all builders with support for cross-references.
- Generates internal links (unlike :manpage: which generates external
URLs).
- Checks that target exists withing our source tree.
Side-effect of hyperlinking is that typos in program and option names
are now detected by Sphinx.
Candidate -options were detected using:
find -name *.rst | xargs grep '``-[^`]'
and then modified from ``-o`` to :option:`-o` using regex
s/``\(-[^`]\+\)``/:option:`\1`/
+ manual modifications where necessary.
Non-hyphenated options were detected by looking at context around
program names:
find bin -name *.rst | xargs -I{} -n1 basename {} .rst | sort -u
and grepping for program name with trailing whitespace.
Stand-alone program names like ``named`` are not hyperlinked in this
commit.
The markup allows referencing individual options, and also makes them
more legible (no more thin red text on gray background).
Most of the work was done using regexes:
s/^``-\(.*\)``$/.. option:: -\1\r/
s/^``+\(.*\)``$/.. option:: +\1\r/
on bin/**/*.rst files along with visual inspection and hand-edits,
mostly for positional arguments.
Regex for rndc.rst:
s/^``\(.*\)``/.. option:: \1\r/
+ hand edits to remove extra asterisk and whitespace here and there.
Since pytest itself skips tests using dnspython if the latter is not
available, also using Automake conditionals for silently skipping
pytest-based tests requiring dnspython is redundant and hides
information. Allow all pytest-based tests requiring dnspython to be run
whenever pytest itself is available, in order to ensure test skipping is
done in a uniform manner.
Note that the above reasoning only applies to pytest-based tests, so
similar adjustments were not made for shell-based tests using Python
scripts that require dnspython ("chain", "cookie", "dnssec", "qmin").
The ability to conveniently mark tests which should only be run when the
CI_ENABLE_ALL_TESTS environment variable is set seems to be useful on a
general level and therefore it should not be limited to the "timeouts"
system test, where it is currently used.
pytest documentation [1] suggests to reuse commonly used test markers by
putting them all in a single Python module which then has to be imported
by test files that want to use the markers defined therein. Follow that
advice by creating a new bin/tests/system/pytest_custom_markers.py
Python module containing the relevant marker definitions.
Note that "import pytest_custom_markers" works from a test-specific
subdirectory because pytest modifies sys.path so that it contains the
paths to all parent directories containing a conftest.py file (and
bin/tests/system/ is one). PyLint does not like that, though, so add a
relevant PyLint suppression.
The above changes make bin/tests/system/timeouts/conftest.py redundant,
so remove it.
[1] https://docs.pytest.org/en/7.0.x/how-to/skipping.html#id1
Ensure all "import dns.*" statements are always placed after
pytest.importorskip('dns') calls, in order to allow the latter to
fulfill their purpose. Explicitly import all dnspython modules used by
each dnspython-based test to avoid relying on nested imports. Replace
function-scoped imports with global imports to reduce code duplication.
The intended purpose of the @pytest.mark.dnspython{,2} decorators was to
cause dnspython-based tests to be skipped if dnspython is not available
(or not recent enough). However, a number of system tests employing
those decorators contain global "import dns.resolver" statements which
trigger ImportError exceptions during test initialization if dnspython
is not available. In other words, the @pytest.mark.dnspython{,2}
decorators serve no useful purpose.
Currently, whenever a Python-based test requires dnspython, that
requirement applies to all tests in a given *.py file. Given that,
employ global pytest.importorskip() calls to ensure dnspython-based
parts of various system tests are skipped when dnspython is not
available. Remove all occurrences of the @pytest.mark.dnspython{,2}
decorators (and all associated code) to prevent confusion.
The intended purpose of the @pytest.mark.requests decorator was to cause
Python-based parts of the "statschannel" system test to be skipped if
the requests Python module is not available. However, both
tests-json.py and tests-xml.py contain a global "import requests"
statement which triggers ImportError exceptions during test
initialization if the requests module is not available. In other words,
the @pytest.mark.requests decorator serves no useful purpose.
Since all tests in both tests-json.py and tests-xml.py depend on the
requests Python module, employ pytest.importorskip() to ensure the
Python-based parts of the "statschannel" system test are skipped when
the requests module is not available. Remove all occurrences of the
@pytest.mark.requests decorator (and all associated code) to prevent
confusion.
All tests in bin/tests/system/statschannel/tests-xml.py require libxml2
support to be enabled in BIND 9 at build-time. Instead of applying the
same pytest.mark.skipif() decorator to every test in that file, set the
'pytestmark' global accordingly in order to immediately skip all tests
in tests-xml.py if libxml2 support is not compiled in.
Remove all occurrences of the @pytest.mark.xml decorator (and all
associated code) from the "statschannel" system test as the
xml.etree.ElementTree module is a part of the Python standard library
since Python 2.5 (so checking whether it is available is redundant) and
checking for libxml2 support in the tested BIND 9 build is already
handled by setting the 'pytestmark' global accordingly.
All tests in bin/tests/system/statschannel/tests-json.py require json-c
support to be enabled in BIND 9 at build-time. Instead of applying the
same pytest.mark.skipif() decorator to every test in that file, set the
'pytestmark' global accordingly in order to immediately skip all tests
in tests-json.py if json-c support is not compiled in.
Remove all occurrences of the @pytest.mark.json decorator (and all
associated code) from the "statschannel" system test as the json module
is a part of the Python standard library since Python 2.6 (so checking
whether it is available is redundant) and checking for json-c support in
the tested BIND 9 build is already handled by setting the 'pytestmark'
global accordingly.
Also remove a related excerpt from bin/tests/system/rpzextra/conftest.py
as it is a copy-paste artifact that serves no purpose in the "rpzextra"
system test.
The "statschannel" system test contains two Python helper modules:
- generic.py: test functions directly invoked by both tests-json.py
and test-xml.py,
- helper.py: helper functions invoked by test functions in generic.py.
The above logic for splitting helper functions into Python modules
prevents selective test skipping from working due to unconditional
import statements being present in both helper modules. For example, if
dnspython is not available on the test host, tests-json.py imports
generic.py, which in turn imports helper.py, which in turn attempts to
import various dnspython modules, triggering ImportError exceptions
during test initialization. Various decorators used for some tests
(like @pytest.mark.dnspython) suggest that such a scenario should be
handled gracefully, but that is not the case - modifying the test
collection in conftest.py does not prevent pytest from failing due to
import errors.
Fix by moving helper functions around to achieve a different split:
- generic.py: helper functions only relying on the Python standard
library,
- generic_dnspython.py: helper functions requiring dnspython.
Only two tests in tests-{json,xml}.py need dnspython to work
(test_traffic_json(), test_traffic_xml()). Since all
dnspython-dependent code is now present in generic_dnspython.py, employ
pytest.importorskip() in those two tests to ensure they can be
selectively skipped when dnspython is not available. Adjust other code
to account for the revised Python helper module layout. Remove all
occurrences of the @pytest.mark.dnspython decorator (and all associated
code) from the "statschannel" system test to prevent confusion.
The find invocation used by the bin/tests/system/get_ports.sh script
("find . -maxdepth 1 -mindepth 1 -type d") assumes the list of
directories in bin/tests/system/ remains unchanged throughout the run
time of a single system test suite. With pytest in use and the
conftest.py file now present in bin/tests/system/, that assumption is no
longer true as a __pycache__ directory may be created when the first
pytest-based test is started. Since the list of names returned by the
above find invocation serves as a fixed-size array of "port range
slots", any changes to that list during a system test suite run may lead
to port assignment collisions [1].
Fix by making the find invocation more nuanced, so that it only returns
names of directories containing test code. Squash a grep / cut pipeline
into a single awk invocation.
[1] see commit 31e5ca4bd9
Most Python-based system tests need to know which ports were assigned to
a given test by bin/tests/system/get_ports.sh. This is currently
handled by inspecting the values of various environment variables (set
by bin/tests/system/run.sh) and passing the port numbers to Python
scripts via pytest fixtures. However, this glue code has so far been
copy-pasted into each system test using it, rather than reused.
Since pytest also looks for conftest.py files in parent directories,
move commonly used fixtures to bin/tests/system/conftest.py. Set the
scope of all the moved fixtures to "session" as their return values are
only based on environment variables, so there is no point in recreating
them for every test requesting them. Adjust test code accordingly.
Building BIND 9 with older version of BIND 9 installed would result in
build failure. Fix the last two remaining cases where <prog>_CFLAGS was
being used leading to wrong order of the build flags on the command line.
We now run both docs and docs:tarball jobs at the same time and keeping
artifacts for longer period of time is a waste.
Artifacts for docs job has to be kept for long period of time because
they are used by scripts behind bind.isc.org web site.
The docs:tarball job is deemed to be cheap enough to run all the time
and it catches omissions in dist targets of Makefiles.
MR !5254 was missing changes to dist target in Makefile and broke docs
build from tarball without us noticing during pipeline run on the MR,
and it manifested itself only on scheduled pipelines which include
docs:tarball job.
In httpd.c, the send callback can directly call read callback without
calling isc_nm_resumeread(). When per-send timeout was added, this
could lead to use-after-free when shutting down the named.
Cleanup the way how we attach to .readhandle and .sendhandle, so there's
assurance that .readhandle will be always non-NULL when reading and
.sendhandle will be always non-NULL when sending.
Additionally, it was found that the implementation ignored the
"Connection: close" header and it worked only accidentally by closing
the connection after the first read from the TCP socket. This has been
also fixed.
Previously, the established TCP connections (both client and server)
would be gracefully closed waiting for the write timeout.
Don't wait for TCP connections to gracefully shutdown, but directly
reset them for faster shutdown.
Previously, there was a single per-socket write timer that would get
restarted for every new write. This turned out to be insufficient
because the other side could keep reseting the timer, and never reading
back the responses.
Change the single write timer to per-send timer which would in turn
reset the TCP connection on the first send timeout.
Remove outdated command references from ARM section
3.3.1. Tools for Use With the Name Server Daemon
and replace them with links to man pages.
Fixes: #2799
Both utilities were included as one man page, but this caused a problem:
Sphinx directive .. include was used twice on the same file, which
prevented us from using labels (or anything with unique identifier) in
the man pages. This effectivelly prevented linking to them.
Splitting man pages allows us to solve the linking problems and also
clearly make text easier to follow because it does not mention two tools
at the same time.
This change causes duplication of text, but given the frequecy of changes
to these tools I think it is acceptable. I've considered deduplication
using smaller .rst snippets which get included into both man pages,
but it would require more sed scripting to handle defaults etc. and
I think it would be way too complex solution for this problem.
Related: #2799
Both utilities were included as one man page, but this caused a problem:
Sphinx directive .. include was used twice on the same file, which
prevented us from using labels (or anything with unique identifier) in
the man pages. This effectivelly prevented linking to them.
Splitting man pages allows us to solve the linking problems and also
clearly make text easier to follow because it does not mention two tools
at the same time.
This change causes duplication of text, but given the frequecy of changes
to these tools I think it is acceptable.
Related: #2799
When dig was built without IDN support, it reported an error if the
+noidnin and/or +noidnout options were used. This means the options
were not useful for a script that wants consistent lack of IDN
translation regardless of how BIND is built.
Make dig complain about lack of built-in IDN support only when the
user asks for IDN translation.
Closes#3188
The C17 standard deprecated ATOMIC_VAR_INIT() macro (see [1]). Follow
the suite and remove the ATOMIC_VAR_INIT() usage in favor of simple
assignment of the value as this is what all supported stdatomic.h
implementations do anyway:
* MacOSX.plaform: #define ATOMIC_VAR_INIT(__v) {__v}
* Gcc stdatomic.h: #define ATOMIC_VAR_INIT(VALUE) (VALUE)
1. http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1138r0.pdf
Previously, the function(s) in the commit subject could fail for various
reasons - mostly allocation failures, or other functions returning
different return code than ISC_R_SUCCESS. Now, the aforementioned
function(s) cannot ever fail and they would always return ISC_R_SUCCESS.
Change the function(s) to return void and remove the extra checks in
the code that uses them.
Previously, the function(s) in the commit subject could fail for various
reasons - mostly allocation failures, or other functions returning
different return code than ISC_R_SUCCESS. Now, the aforementioned
function(s) cannot ever fail and they would always return ISC_R_SUCCESS.
Change the function(s) to return void and remove the extra checks in
the code that uses them.
Previously, the function(s) in the commit subject could fail for various
reasons - mostly allocation failures, or other functions returning
different return code than ISC_R_SUCCESS. Now, the aforementioned
function(s) cannot ever fail and they would always return ISC_R_SUCCESS.
Change the function(s) to return void and remove the extra checks in
the code that uses them.
Previously the socket code would set the TCPv6 maximum segment size to
minimum value to prevent IP fragmentation for TCP. This was not yet
implemented for the network manager.
Implement network manager functions to set and use minimum MTU socket
option and set the TCP_MAXSEG socket option for both IPv4 and IPv6 and
use those to clamp the TCP maximum segment size for TCP, TCPDNS and
TLSDNS layers in the network manager to 1220 bytes, that is 1280 (IPv6
minimum link MTU) minus 40 (IPv6 fixed header) minus 20 (TCP fixed
header)
We already rely on a similar value for UDP to prevent IP fragmentation
and it make sense to use the same value for IPv4 and IPv6 because the
modern networks are required to support IPv6 packet sizes. If there's
need for small TCP segment values, the MTU on the interfaces needs to be
properly configured.
The IPV6_USE_MIN_MTU socket option directs the IP layer to limit the
IPv6 packet size to the minimum required supported MTU from the base
IPv6 specification, i.e. 1280 bytes. Many implementations of TCP
running over IPv6 neglect to check the IPV6_USE_MIN_MTU value when
performing MSS negotiation and when constructing a TCP segment despite
MSS being defined to be the MTU less the IP and TCP header sizes (60
bytes for IPv6). This leads to oversized IPv6 packets being sent
resulting in unintended Path Maximum Transport Unit Discovery (PMTUD)
being performed and to fragmented IPv6 packets being sent.
Add and use a function to set socket option to limit the MTU on IPv6
sockets to the minimum MTU (1280) both for UDP and TCP.
For each algorithm there must be a key performing the KSK and
ZSK rolls. After reading the keys from named.conf check that
each algorithm present has both rolls. CSK implicitly has both
rolls.
bad-ksk-without-zsk.conf only has a ksk defined without a
matching zsk for the same algorithm.
bad-zsk-without-ksk.conf only has a zsk defined without a
matching ksk for the same algorithm.
bad-unpaired-keys.conf has two keys of different algorithms
one ksk only and the other zsk only
When get_dispatch() returns an error code, the dns_request_createraw()
function jumps to the `cleanup` label, which will leave a previous
attachment to the `request` pointer unattached.
Fix the issue by jumping to the `detach` label instead.
The AX_PROG_CC_FOR_BUILD implementation to find a native CC compiler is
slightly better because it uses AC_PROG_CC and AC_PROG_CPP to find the
native compiler instead of just defaulting to `gcc` as AX_CC_FOR_BUILD
does.
AX_PROG_CC_FOR_BUILD also sets BUILD_EXEEXT that we already use in the
Makefile.am for `lib/dns/gen` while AX_CC_FOR_BUILD uses
EXEEXT_FOR_BUILD.
Formerly, the gen.h header contained a compatibility layer between Win32
and POSIX platforms. Since we have already dropped the Win32 build, we
can merged gen.h into gen.c as the header file is not used elsewhere.
The current implementation of isc_queue uses Michael-Scott lock-free
queue that in turn uses hazard pointers. It was discovered that the way
we use the isc_queue, such complicated mechanism isn't really needed,
because most of the time, we either execute the work directly when on
nmthread (in case of UDP) or schedule the work from the matching
nmthreads.
Replace the current implementation of the isc_queue with a simple locked
ISC_LIST. There's a slight improvement - since copying the whole list
is very lightweight - we move the queue into a new list before we start
the processing and locking just for moving the queue and not for every
single item on the list.
NOTE: There's a room for future improvements - since we don't guarantee
the order in which the netievents are processed, we could have two lists
- one unlocked that would be used when scheduling the work from the
matching thread and one locked that would be used from non-matching
thread.
If the dns_request send callback is delayed, the dst API would get
deinitialized and then the detach from the tsig key would cause an
assertion failure.
Shutdown the isc_managers early, and only then dereference the dst
objects when cleaning up the resources used by nsupdate.
The order in which the netievents are processed on the network manager
loop is not guaranteed. Therefore the recv/read callback can come
earlier than the send/write callback.
The dns_request API wasn't ready for this reordering and it was
destroying the dns_request_t object before the send callback has been
called.
Add additional attach/detach in the req_send()/req_senddone() functions
to make sure we don't destroy the dns_request_t while it's still being
references by asynchronous call.
For the reference, the _cancel_lookup() function iterates through
the lookup's queries list and detaches them. In the ideal scenario,
that should be the last reference and the query will be destroyed
after that, but it is also possible that we are still expecting a
callback, which also holds a reference (for example, _cancel_lookup()
could have been called from recv_done(), when send_done() was still
not executed).
The start_udp() and start_tcp() functions are currently designed in
slightly different ways: start_udp() creates a new query attachment
`connectquery`, to be called in the callback function, while
start_tcp() does not, which is a bug, but is hidden by the fact
that when the query is being erroneously destroyed prematurely (before
_cancel_lookup() is called) in the result of that, it also gets
de-listed from the lookup's queries' list, so _cancel_lookup() doesn't
even try to detach it.
For better understanding, here's an illustration of the query's
references count changes, and from where it was changed:
UDP
---
1. _new_query() -> refcount = 1 (initial)
2. start_udp() -> refcount = 2 (lookup->current_query)
3. start_udp() -> refcount = 3 (connectquery)
4. udp_ready() -> refcount = 4 (readquery)
5. udp_ready() -> refcount = 5 (sendquery)
6. udp_ready() -> refcount = 4 (lookup->current_query)
7. udp_ready() -> refcount = 3 (connectquery)
8. send_done() -> refcount = 2 (sendquery)
9. recv_done() -> refcount = 1 (readquery)
10. _cancel_lookup() -> refcount = 0 (initial)
11. the query gets destroyed and removed from `lookup->q`
TCP, fortunate scenario
-----------------------
1. _new_query() -> refcount = 1 (initial)
2. start_tcp() -> refcount = 2 (lookup->current_query)
3. launch_next_query() -> refcount = 3 (readquery)
4. launch_next_query() -> refcount = 4 (sendquery)
5. tcp_connected() -> refcount = 3 (lookup->current_query)
6. tcp_connected() -> refcount = 2 (bug, there was no connectquery)
7. send_done() -> refcount = 1 (sendquery)
8. recv_done() -> refcount = 0 (readquery)
9. the query gets prematurely destroyed and removed from `lookup->q`
10. _cancel_lookup() -> the query is not in `lookup->q`
TCP, unfortunate scenario, revealing the bug
--------------------------------------------
1. _new_query() -> refcount = 1 (initial)
2. start_tcp() -> refcount = 2 (lookup->current_query)
3. launch_next_query() -> refcount = 3 (readquery)
4. launch_next_query() -> refcount = 4 (sendquery)
5. tcp_connected() -> refcount = 3 (lookup->current_query)
6. tcp_connected() -> refcount = 2 (bug, there was no connectquery)
7. recv_done() -> refcount = 1 (readquery)
8. _cancel_lookup() -> refcount = 0 (the query was in `lookup->q`)
9. we hit an assertion here when trying to destroy the query, because
sendhandle is not detached (which is done by send_done()).
10. send_done() -> this never happens
This commit does the following:
1. Add a `connectquery` attachment in start_tcp(), like done in
start_udp().
2. Add missing _cancel_lookup() calls for error scenarios, which
were possibly missing because before fixing the bug, calling
_cancel_lookup() and then calling query_detach() would cause
an assertion.
3. Log a debug message and call isc_nm_cancelread(query->readhandle)
for every query in the lookup from inside the _cancel_lookup()
function, like it is done in _cancel_all().
4. Add a `canceled` property for the query which becomes `true` when
the lookup (and subsequently, its queries) are canceled.
5. Use the `canceled` property in the network manager callbacks to
know that the query was canceled, and act like `eresult` was equal
to `ISC_R_CANCELED`.
There was a missing UNLOCK_LOOKUP in the recv_done() callback when
the operation had been canceled. That omission could result in a
deadlock situation.
BIND unconditionally uses shims for BN_GENCB_new(), BN_GENCB_free(),
and BN_GENCB_get_arg() for all LibreSSL versions and, correctly, for
OpenSSL <1.1.0 versions.
This breaks LibreSSL compilation starting with LibreSSL 3.5.0.
Use autoconf check instead to check whether the family of the functions
are available.
LibreSSL 3.5.0 fails to compile with these shims. We could have just
removed the LibreSSL check from the pre-processor condition, but it
seems that these shims are no longer needed because all the supported
versions of OpenSSL and LibreSSL have those functions.
According to EVP_ENCRYPTINIT(3) manual page in LibreSSL,
EVP_CIPHER_CTX_new() and EVP_CIPHER_CTX_free() first appeared in
OpenSSL 0.9.8b, and have been available since OpenBSD 4.5.
the "zone" clause can be documented using, for instance,
`cfg_test --zonegrammar primary", which prints only
options that are valid in primary zones. this was not
the method being used when generating the named.conf
man page; instead, "zone" was documented with all possible
options, and no zone types at all.
this commit removes "zone" from the generic documentation
and adds include statements in named.conf.rst so that
correct zone grammars will be included in the man page.
when parsing key pairs, if the '=' character fell at max_token
a protective INSIST preventing buffer overrun could be triggered.
Attempt to grow the buffer immediately before the INSIST.
Also removed an unnecessary INSIST on the opening double quote
of key buffer pair.
Resolve "Issue 45110 by ClusterFuzz-External: bind9:dns_master_load_fuzzer: Undefined-shift in soa_get"
Closes#3176
See merge request isc-projects/bind9!5909
By default C promotes short unsigned values to signed int which
leads to undefined behaviour when the value is shifted by too much.
Force unsigned arithmetic to be perform by explicitly casting to a
unsigned type.
The isc__nmsocket_reset() was missing a case for raw TCP sockets (used
by RNDC and DoH) which would case a assertion failure when write timeout
would be triggered.
TCP sockets are now also properly handled in isc__nmsocket_reset().
mem_maybedup() calls isc_mem_allocate() if an mctx is supplied,
but that can no longer fail, so now the only way mem_maybedup()
could return NULL is if it was given a NULL source address by the
caller. this commit adds a REQUIRE to prevent that scenario, and
cleans up all the calling code that previously checked for NULL
return values.
this function is mostly used in rdata tostruct() implementations, so
the documentation for dns_rdata_tostruct() has been updated to
remove 'ISC_R_NOMEMORY' as a possible return value.
"masters" and "default-masters" are now flagged so they will
not be included in the named.conf man page, despite being
accepted as valid options by the parser for backward
compatibiility.
... along with dns_rdataclass_fromtext and dns_rdatatype_fromtext
Most of the test binary is modified named-rrchecker. Main differences:
- reads single RR and exists
- does not refuse meta classes and rr types
We actually do have some fromtext code for meta-things so erroring out
in named-rrchecker would prevent us from testing this code.
Corpus has examples of all currently supported RR types. I did not do
any minimization.
In future use command
diff -U0 \
<(sed -n -e 's/^.*fromtext_\(.*\)(.*$/\1/p' lib/dns/code.h | \
sort) \
<(ls fuzz/dns_rdata_fromtext.in/)
to check for missing RR types.
When isc__nm_uvreq_t gets deactivated, it could be just put onto array
stack to be reused later to save some initialization time.
Unfortunately, this might hide some use-after-free errors.
Disable the inactive uvreqs caching when compiled with Address or
Thread Sanitizer.
When isc_nmhandle_t gets deactivated, it could be just put onto array
stack to be reused later to safe some initialization time.
Unfortunately, this might hide some use-after-free errors.
Disable the inactive handles caching when compiled with Address or
Thread Sanitizer.
The isc__nmsocket_t has locked array of isc_nmhandle_t that's not used
for anything. The isc__nmhandle_get() adds the isc_nmhandle_t to the
locked array (and resized if necessary) and removed when
isc_nmhandle_put() finally destroys the handle. That's all it does, so
it serves no useful purpose.
Remove the .ah_handles, .ah_size, and .ah_frees members of the
isc__nmsocket_t and .ah_pos member of the isc_nmhandle_t struct.
When the TCP, TCPDNS or TLSDNS connection times out, the isc__nm_uvreq_t
would be pushed into sock->inactivereqs before the uv_tcp_connect()
callback finishes. Because the isc__nmsocket_t keeps the list of
inactive isc__nm_uvreq_t, this would cause use-after-free only when the
sock->inactivereqs is full (which could never happen because the failure
happens in connection timeout callback) or when the sock->inactivereqs
mechanism is completely removed (f.e. when running under Address or
Thread Sanitizer).
Delay isc__nm_uvreq_t deallocation to the connection callback and only
signal the connection callback should be called by shutting down the
libuv socket from the connection timeout callback.
Commit aab691d512 did not fix all possible
scenarios in which the ns_statscounter_recursclients counter underflows.
The solution implemented therein can be ineffective e.g. when CNAME
chaining happens with prefetching enabled.
Here is an example recursive resolution scenario in which the
ns_statscounter_recursclients counter can underflow with the current
logic in effect:
1. Query processing starts, the answer is not found in the cache, so
recursion is started. The NS_CLIENTATTR_RECURSING attribute is set.
ns_statscounter_recursclients is incremented (Δ = +1).
2. Recursion completes, returning a CNAME. client->recursionquota is
non-NULL, so the NS_CLIENTATTR_RECURSING attribute remains set.
ns_statscounter_recursclients is decremented (Δ = 0).
3. Query processing restarts.
4. The current QNAME (the target of the CNAME from step 2) is found in
the cache, with a TTL low enough to trigger a prefetch.
5. query_prefetch() attaches to client->recursionquota.
ns_statscounter_recursclients is not incremented because
query_prefetch() does not do that (Δ = 0).
6. Query processing restarts.
7. The current QNAME (the target of the CNAME from step 4) is not found
in the cache, so recursion is started. client->recursionquota is
already attached to (since step 5) and the NS_CLIENTATTR_RECURSING
attribute is set (since step 1), so ns_statscounter_recursclients is
not incremented (Δ = 0).
8. The prefetch from step 5 completes. client->recursionquota is
detached from in prefetch_done(). ns_statscounter_recursclients is
not decremented because prefetch_done() does not do that (Δ = 0).
9. Recursion for the current QNAME completes. client->recursionquota
is already detached from, i.e. set to NULL (since step 8), and the
NS_CLIENTATTR_RECURSING attribute is set (since step 1), so
ns_statscounter_recursclients is decremented (Δ = -1).
Another possible scenario is that after step 7, recursion for the target
of the CNAME from step 4 completes before the prefetch for the CNAME
itself. fetch_callback() then notices that client->recursionquota is
non-NULL and decrements ns_statscounter_recursclients, even though
client->recursionquota was attached to by query_prefetch() and therefore
not accompanied by an incrementation of ns_statscounter_recursclients.
The net result is also an underflow.
Instead of trying to properly handle all possible orderings of events
set into motion by normal recursion and prefetch-triggered recursion,
adjust ns_statscounter_recursclients whenever the recursive clients
quota is successfully attached to or detached from. Remove the
NS_CLIENTATTR_RECURSING attribute altogether as its only purpose is made
obsolete by this change.
Commit b6d40b3c4e removed most uses of the
'fctx' variable from the rctx_dispfail() function: it is now only needed
by the FCTXTRACE3() macro. However, when --enable-querytrace is not in
effect, that macro evaluates to a list of UNUSED() macros that does not
include "UNUSED(fctx);". This triggers the following compilation
warning when building without --enable-querytrace:
resolver.c: In function 'rctx_dispfail':
resolver.c:7888:21: warning: unused variable 'fctx' [-Wunused-variable]
7888 | fetchctx_t *fctx = rctx->fctx;
| ^~~~
Fix by adding "UNUSED(fctx);" lines to all FCTXTRACE*() macros. This is
safe to do because all of those macros use the 'fctx' local variable, so
there is no danger of introducing new errors caused by use of undeclared
identifiers.
Due to a bug in gcc-11, the build fails when AddressSanitizer is
enabled. Downgrading the -Wstringop-overread to just a warning in the
gcc:asan build allows the code to compile.
The keep-response-order option has been obsoleted, and in this commit,
remove the keep-response-order ACL map rendering the option no-op, the
call the isc_nm_sequential() and the now unused isc_nm_sequential()
function itself.
The keep-response-order option has been introduced when TCP pipelining
has been introduced to BIND 9 as a failsafe for possibly non-compliant
clients.
Declare the keep-response-order obsolete as all DNS clients should
either support out-of-order processing or don't send more DNS queries
until the DNS response for the previous one has been received.
There was an artificial limit of 23 on the number of simultaneous
pipelined queries in the single TCP connection. The new network
managers is capable of handling "unlimited" (limited only by the TCP
read buffer size ) queries similar to "unlimited" handling of the DNS
queries receive over UDP.
Don't limit the number of TCP queries that we can process within a
single TCP read callback.
Extend the timeouts system test to ensure that the maximum outgoing
transfer time (max-transfer-time-out) and maximum outgoing transfer idle
time (max-transfer-idle-out) works as expected. This is done by
lowering the limits to 5/1 minutes and testing that the connection has
been dropped while sleeping between the individual XFR messages.
While refactoring the libns to use the new network manager, the
max-transfer-*-out options were not implemented and they were turned
non-operational.
Reimplement the max-transfer-idle-out functionality using the write
timer and max-transfer-time-out using the new isc_nm_timer API.
While refactoring the lib/ns/xfrout.c, it was discovered that .shutdown
and .shutdown_arg members of ns_client_t structure are unused.
Remove the unused members and associated code that was using in it in
the ns_xfrout.
bin/tests/system/makejournal needs to ignore DNS_R_SEENINCLUDE
when calling dns_db_load(), otherwise it cannot generate a journal
for a zone file with a $INCLUDE statement.
When invalid DNS message is received, there was a handling mechanism for
DoH that would be called to return proper HTTP response.
Reuse this mechanism and reset the TCP connection when the client is
blackholed, DNS message is completely bogus or the ns_client receives
response instead of query.
- certain TCP result codes, including ISC_R_EOF and
ISC_R_CONNECTIONRESET, were being mapped to ISC_R_SHUTTINGDOWN
before calling the response handler in tcp_recv_cancelall().
the result codes should be passed through to the response handler
without being changed.
- the response handlers, resquery_response() and req_response(), had
code to return immediately if encountering ISC_R_EOF, but this is
not the correct behavior; that should only happen in the case of
ISC_R_CANCELED when it was the caller that canceled the operation
- ISC_R_CONNECTIONRESET was not being caught in rctx_dispfail().
- removed code in rctx_dispfail() to retry queries without EDNS
when receiving ISC_R_EOF; this is now treated the same as any
other connection failure.
Use the isc_nmhandle_setwritetimeout() function in the netmgr unit test
to allow more time for writing and reading the responses because some of
the intervals that are used in the unit tests are really small leaving a
little room for any delays.
In some situations (unit test and forthcoming XFR timeouts MR), we need
to modify the write timeout independently of the read timeout. Add a
isc_nmhandle_setwritetimeout() function that could be called before
isc_nm_send() to specify a custom write timeout interval.
Extend the timeouts system test that bursts the queries for large TXT
record and never read any responses back filling up the server TCP write
buffer. The test should work with the default wmem_max value on
Linux (208k).
When the outgoing TCP write buffers are full because the other party is
not reading the data, the uv_write() could wait indefinitely on the
uv_loop and never calling the callback. Add a new write timer that uses
the `tcp-idle-timeout` value to interrupt the TCP connection when we are
not able to send data for defined period of time.
The uv_tcp_close_reset() function was added in libuv 1.32.0 and since we
support older libuv releases, we have to add a shim uv_tcp_close_reset()
implementation loosely based on libuv.
Before adding the write timer, we have to remove the generic sock->timer
to sock->read_timer. We don't touch the function names to limit the
impact of the refactoring.
There was a bug in the checking of the "blackhole" ACL in
dns_request_create*(), causing an address to be treated as included
in the ACL if it was explicitly *excluded*. Thus, leaving "blackhole"
unset had no effect, but setting it to "none" would cause any
destination addresses to be rejected for dns_request purposes. This
would cause zone transfer requests and SOA queries to fail, among
other things.
The bug has been fixed, and "blackhole { none; };" was added to the
xfer system test as a regression test.
When a resolver priming attempt completes, the following message is
currently logged:
resolver priming query complete
This message is identical for both successful and failed priming
attempts. Consider the following log excerpts:
- successful priming attempt:
10-Feb-2022 11:33:11.272 all zones loaded
10-Feb-2022 11:33:11.272 running
10-Feb-2022 11:33:19.722 resolver priming query complete
- failed priming attempt:
10-Feb-2022 11:33:29.978 all zones loaded
10-Feb-2022 11:33:29.978 running
10-Feb-2022 11:33:38.432 timed out resolving '_.org/A/IN': 2001:500:9f::42#53
10-Feb-2022 11:33:38.522 timed out resolving './NS/IN': 2001:500:9f::42#53
10-Feb-2022 11:33:42.132 timed out resolving '_.org/A/IN': 2001:500:12::d0d#53
10-Feb-2022 11:33:42.285 timed out resolving './NS/IN': 2001:500:12::d0d#53
10-Feb-2022 11:33:44.685 resolver priming query complete
Include the result of each priming attempt in the relevant log message
to give the administrator better insight into named's resolver priming
process.
The UV_RUNTIME_CHECK() macro requires to keep the function name in sync
like this:
r = func(...);
UV_RUNTIME_CHECK(func, r);
Add semantic patch to keep the function name and return variable in sync
with the previous line.
When libuv functions fail, they return correct return value that could
be useful for more detailed debugging. Currently, we usually just check
whether the return value is 0 and invoke assertion error if it doesn't
throwing away the details why the call has failed. Unfortunately, this
often happen on more exotic platforms.
Add a UV_RUNTIME_CHECK() macro that can be used to print more detailed
error message (via uv_strerror() before ending the execution of the
program abruptly with the assertion.
Add a note to the DNSSEC guide and to the ARM reference that A ZSK/KSK
pair used for signing your zone should have the same algorithm.
This commit also updates the 'dnssec-policy/keys' example to use the
slightly more modern 'rsasha256' algorithm.
For users it's not really important if a RFC is Internet Standard,
Proposed Standard, or Experimental. RFCs are now regrouped by
"Protocol", Best Current Practice, and "catch all" category FYI.
After the build system refactoring, we no longer call AM_PROG_CC_C_O
because it is obsolescent macro. According to the automake manual the
`AC_PROG_CC` has been rewritten in automake 1.14 to not required the
call, thus we need to require at least automake version 1.14.
In autoconf, the AC_INIT() accepts bugreport address for reporting
issues (f.e. when the test suite fails). Instead of providing generic
emails address, change this to the address where to report with the
default Bug template applied.
The task exclusive mode stops all processing (tasks and networking IO)
except the designated exclusive task events. This has impact on the
operation of the server. Add log messages indicating when we start the
exclusive mode, and when we end exclusive task mode.
There are reported occurences where the statitic counters underflows and
starts reporting non-sense.
Add a check for the underflow, when ``named`` is compiled in the
developer mode.
Replace the hard-coded paths for various BIND 9 files (configuration,
pid, etc.) in the man pages and ARM with compile-time values using the
sphinx-build replace system.
This is more complicated, because the restructured text specification
doesn't allow |substitions| inside ``code-blocks``, so for each specific
file we had to create own substition which is sub-optimal, but it is
only way how to do this without adding Sphinx extension.
The isc_thread_setaffinity call was removed in !5265 and we are not
going to restore it because it was proven that the performance is better
without it. Additionally, remove the already disabled cpu system test.
The isc_thread_setconcurrency function is unused and also calling
pthread_setconcurrency() on Linux has no meaning, formerly it was
added because of Solaris in 2001 and it was removed when taskmgr was
refactored to run on top of netmgr in !4918.
The "suppr-lsan.txt" file needs to be referenced with GitLab-specific
variable, otherwise AddressSanitizer won't find it outside the
"isc-projects" project group.
This has been introduced in 8a4f098dee.
When there are more than one tokens initialized in SoftHSMv2,
care must be taken to correctly identify them.
Use a SoftHSMv2 token label which will uniquely identify the
token used for this test.
Use the "--token-label" parameter for the `pkcs11-tool` program
to make sure that it finds and uses the correct token.
The 'id' variable is either keyfromlabel-ksk or keyfromlabel-zsk and is
set in the 'keygen' and 'keyfromlabel' functions. It should not be used
outside these functions.
This test was originally in the pkcs11 system test. While this crash
happened in the native pkcs11 of BIND 9, and that code has been
removed in 9.17, there is no need for this test. Nevertheless, it
doesn't hurt having the test case persist.
Add a system test for engine_pkcs11 interactions that replaces the
tests that are done in the native PKCS#11 system test.
The native PKCS#11 code was removed in 9.17 but without copying the
pkcs11 system test.
The "directory" configuration options affects the configuration listed
after the directive but not before which may affect ``include``
directive with relative file paths.
When isc_quota_attach_cb() API returns ISC_R_QUOTA (meaning hard quota
was reached) the accept_connection() would return without logging a
message about quota reached.
Change the connection callback to log the quota reached message.
Formerly parental-agents grammar was an exception and it did not
auto-generate itself from source code. From now on it is generated using
the same mechanism as other grammars.
For consistency with rest of the system, I've also renamed the grammar
file and the link anchors from "parentals" to "parental-agents".
Technically this is fixup for commit
0311705d4b.
Related: !5234
The missing `::` in the .rst files caused grammar section in docs to
render empty.
The `::` was accidentally removed in an unrelated commit
58bd26b6cf which was supposed to update
only copyright headers.
Fixes: #3120
DLZ modules no longer support being built without threads,
so the "#if PTHREADS" conditionals were no longer necessary,
and were also causing errors in some of the modules due to
PTHREADS no longer being defined in dlz_pthread.h.
Apparently we forgot about DLZ when updating DNS_CLIENTINFO_VERSION
constant for ECS, which is at value "3" since ECS was introduced.
The code in example drivers and tests now hardcodes version numbers
2 (without ECS) and 3 (with ECS) depending on what a given code path
requires.
this brings DNS_CLIENTINFO_VERSION into line with the subscription
branch so that fixes applied to clientinfo processing can also be
applied to the main branch without diverging.
System test used to have sequential system tests, which can't run in
parallel with the rest of system tests. As there are no such tests
anymore the underlying infrastructure can be dropped.
"parallel.sh" script was used on Windows to run system tests in
parallel. Since Windows support was removed from BIND 9, the script is
not needed anymore.
testsummary.sh was not updated after build system rewrite to Autotools,
and needs to be fixed to produce test summary and core dump, assertion
failures, and ThreadSanitizer reports.
Given that all of this is provided by Autotools and run.sh already,
there's little use to testsummary.sh script and should be dropped.
IBM power architecture has L1 cache line size equal to 128. Take
advantage of that on that architecture, do not force more common value
of 64. When it is possible to detect higher value, use that value
instead. Keep the default to be 64.
In the RPZ documentation, there's a mistake where it states that the
default behavior will be disabled by setting `qname-wait-recurse yes;`
while in fact it's opposite `qname-wait-recurse no;`.
This affects only the RST documentation.
runall.sh was mainly used on Windows and as it's support was removed
from the "main" branch the script is not needed anymore.
Also, remove bin/tests/system/README text on running multiple system
test suites simultaneously with runall.sh as that support was not
present in the script anyway.
bin/tests/system/setup.sh just executes setup.sh script of a particular
system test in the directory of the system test. This does not seems to
be useful enough to maintain it.
stopall.sh script takes almost 2 minutes to go thru all test
subdirectories (due to a sleep in stop.pl) and does not seems to be
efficient way to stop manually started tests.
The keyfromlabel system ECDSA tests sometimes fail. When this happens
the ZSK and KSK key id values differ by 1, which is an indication that
the same key is used for both DNSKEY records.
When the private key is retrieved with 'ENGINE_load_private_key()', the
public key is already set. But sometimes that key differs from the key
which was retrieved with 'ENGINE_load_public_key()'.
The libp11 source code uses id to find the key and without IDs all the
keys are "equal", so it is returning the first key in the array of the
enumerated keys instead of the matching key. In our test we didn't use
'--id', just '--label'. With this change, the system test should no
longer fail intermittently.
Note this is only an issue for ECDSA keys, not RSA keys.
These memory leaks are a known issue in libp11: From Timo Teras:
The relevant code is:
https://github.com/OpenSC/libp11/blob/master/src/eng_front.c#L114-L123
The authors of libp11 did not get the locking right and decided
that having intentional memory leaks is better than risking a deadlock.
The leak logs indicate that it is the cached structures that should
have been freed.
These are not a run-time leaks, so suppressing these leaks is probably
okay.
Add missing system test for dnssec-keyfromlabel. Test for various
algorithms that we can generate key files from a key that is stored in a
HSM, and that those keys can be used for signing with dnssec-signzone.
GitLab CI needs to know about some environment variables that will
tell where OpenSSL and SoftHSM2 is installed. This is done in the
image, making the prepare-softhsm2.sh script obsolete.
The SoftHSM2 module location is system specific.
- Removed all code that only runs under CYGWIN, and made all
code that doesn't run under CYGWIN non-optional.
- Removed the $TP variable which was used to add optional
trailing dots to filenames; they're no longer optional.
- Removed references to pssuspend and dos2unix.
- No need to use environment variables for diff and kill.
- Removed uses of "tr -d '\r'"; this was a workaround for
a cygwin regex bug that is no longer needed.
Commit c787a539d2 fixed a certain class of
intermittent system test failures caused by named instances unable to
restart. The root cause was bin/tests/system/stop.pl returning without
waiting for a named instance to remove its lock file.
Later on, it turned out that the above change causes other issues on
Windows due to the way named handles signals on that platform. Commit
761ba4514f intended to address those
issues by making the server_lock_file() subroutine in
bin/tests/system/stop.pl return an empty value on Windows, in order to
prevent the script for waiting for lock file cleanup on that platform.
Note, however, that Windows detection in that subroutine is limited to
checking whether the CYGWIN environment variable is set.
While that environment variable was not set on Unix-like systems before
commit 761ba4514f, another commit
(a33237f070, merged a few weeks later)
changed that by setting the CYGWIN environment variable to an empty
value on Unix-like systems. This made the defined($ENV{'CYGWIN'}) check
in server_lock_file() return true, inadvertently preventing
bin/tests/system/stop.pl from waiting for lock file removal before
exiting on Unix-like systems and therefore reintroducing the original
issue.
Fix by making server_lock_file() only return an empty value when the
CYGWIN environment variable is set to a non-empty value (which is what
bin/tests/system/conf.sh.win32 does). Adjust a similar check in the
pid_file_exists() subroutine in the same way for consistency.
The echo_*() and cat_*() functions in bin/tests/system/conf.sh.common
call the "read" builtin command without specifying the field separator
to use. This results in leading whitespace getting stripped from each
line of the texts passed to those functions, which mangles e.g. pytest
output, hindering test failure troubleshooting.
Address by setting IFS to an empty value for the "read" calls used in
the aforementioned helper functions.
The bin/tests/system/start.pl script truncates the named.run file for a
given named instance unless it is invoked with the --restart
command-line option. Ever since Python-based tests were introduced,
bin/tests/system/run.sh may start named instances used by a given system
test multiple times within a single run, causing the
bin/tests/system/start.pl script to truncate some of the log files
written during the test. This makes troubleshooting certain test
failures hard or even impossible.
Fix by calling bin/tests/system/start.pl with the --restart command-line
option for every start_servers() invocation except the first one.
TLS clients can have their clock a short time in the past which will
result in not being able to validate the certificate.
Setting the "not before" property 5 minutes in the past will
accommodate with some possible clock skew across systems.
dns_dlzcreate() fails to free the memory allocated for dlzname
when an error occurs.
Free dlzname's memory (acquired earlier with isc_mem_strdup())
by calling isc_mem_free() before returning an error code.
When failure is expected, the `rndc` command in the catz system test
is being called directly instead of using a function, i.e.:
$RNDC -c ../common/rndc.conf -s 10.53.0.2 -p 9953 reconfig \
> /dev/null 2>&1 && ret=1
... instead of:
rndccmd 10.53.0.2 reconfig && ret=1
This is done to suppress messages like "lt-rndc: 'reconfig' failed:
failure" appearing in the message log of the test, because failure
is actually expected, and the appearance of that message can be
confusing.
The port value used in this case is not correct, making the
`rndc reload` command to fail. This error was not detected earlier
only because the failure of the command is actually expected, but
the failure happens for a "wrong" reason, and the test still passes.
Fix the error by using the existing variable instead of the fixed
number.
Test the view reverting code by introducing a faulty dlz configuration
in named.conf and using `rndc reconfig` to check if named handles the
situation correctly.
We use "dlz" because the dlz processing code is located in an ideal
place in the view configuration function for the test to cover the
view reverting code.
This test is specifically added to the catz system test to additionally
cover the catz reconfiguration during the mentioned failed
reconfiguration attempt.
When a zone is being configured with a new view, the catalog zones
structure will also be linked to that view. Later on, in case of some
error, should the zone be reverted to the previous view, the link
between the catalog zones structure and the view won't be reverted.
Change the dns_zone_setviewrevert() function so it calls
dns_zone_catz_enable() during a zone revert, which will reset the
link between `catzs` and view.
Separate the locked parts of dns_zone_catz_enable() and
dns_zone_catz_disable() functions into static functions. This will
let us perform those tasks from the other parts of the module while
the zone is locked, avoiding one pair of additional unlocking and
locking operations.
If a view configuration error occurs during a named reconfiguration
procedure, BIND can end up having twin views (old and new), with some
zones and internal structures attached to the old one, and others
attached to the new one, which essentially creates chaos.
Implement some additional view reverting mechanisms to avoid the
situation described above:
1. Revert rpz configuration.
2. Revert catz configuration.
3. Revert zones to view attachments.
There were three RFCs listed in list of "RFCs we implement" but missing
in the ARM.
Command to compare lists in the two documents:
diff <(grep -o '^ RFC[0-9]\+' doc/misc/rfc-compliance | sed -e 's/[^0-9]//g' | sort -n) <(grep '^:rfc:`' doc/arm/general.rst | sed -e 's/^.*`\([0-9]*\)`.*$/\1/' | sort -n)
Supported Platforms section is now really only about platforms and not
libraries. Libraries were moved to the Building BIND section.
We now have section for required libraries, and second with optional
features. Wordy explanations were taken verbatim from the original
README.md.
Converted using pandoc 2.14.2-9 on Arch Linux:
$ pandoc --shift-heading-level-by=-1 -f markdown -t rst README.md > doc/arm/build.rst
Plus hand-edit to remove sections other than Building BIND 9, remove
misindentation in section headers, and add a standard copyright header.
Converted using pandoc 2.14.2-9 on Arch Linux:
$ pandoc -f markdown -t rst PLATFORMS.md > PLATFORMS.rst
The pandoc-generated copyright header was subsequently replaced with
usual one for .rst files.
For reproducible builds, we use last modification time of the CHANGES
file. This works pretty well, unless the builds are made in different
timezones.
Use UTC option to date command to make the builds reproducible.
2022-01-24 16:09:48 +01:00
955 changed files with 36235 additions and 29777 deletions
- [ ]***(Support)*** Update tickets in case of waiting support customers.
- [ ]***(QA)*** Build and test any outstanding private packages.
- [ ]***(QA)*** Build public RPMs.
- [ ]***(SwEng)*** Build Debian/Ubuntu packages.
- [ ]***(SwEng)*** Update Docker images.
- [ ]***(SwEng)*** Build Debian/Ubuntu packages.
- [ ]***(SwEng)*** Update Docker images.
- [ ]***(QA)*** Inform Marketing of the release.
- [ ]***(QA)*** Update the internal [BIND release dates wiki page](https://wiki.isc.org/bin/view/Main/BindReleaseDates) when public announcement has been made.
- [ ]***(Marketing)*** Post short note to Twitter.
- [ ]***(Marketing)*** Update [Wikipedia entry for BIND](https://en.wikipedia.org/wiki/BIND).
- [ ]***(Marketing)*** Write blog article (if a major release).
- [ ]***(QA)*** Ensure all new tags are annotated and signed.
- [ ]***(QA)*** Push tags for the published releases to the public repository.
- [ ]***(QA)*** Merge the automatically prepared `prep 9.x.y` commit which updates `version` and documentation on the release branch into the relevant maintenance branch (`v9_x`).
- [ ]***(QA)*** For each maintained branch, update the `BIND_BASELINE_VERSION` variable for the `abi-check` job in `.gitlab-ci.yml` to the latest published BIND version tag for a given branch.
- [ ]***(QA)*** Prepare empty release notes for the next set of releases.
- [ ]***(QA)*** Merge published release tags (non-linearly) back into the their relevant development/maintenance branches.
- [ ]***(QA)*** Sanitize confidential issues which are assigned to the current release milestone and do not describe a security vulnerability, then make them public.
- [ ]***(QA)*** Sanitize confidential issues which are assigned to older release milestones and describe security vulnerabilities, then make them public if appropriate[^2].
- [ ]***(QA)*** Update QA tools used in GitLab CI (e.g. Flake8, PyLint) by modifying the relevant `Dockerfile`.
softhsm2-util --init-token --free --pin 1234 --so-pin 1234 --label "softhsm2"| awk '/^The token has been initialized and is reassigned to slot/ { print $NF }'
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Ondřej Surý
Petr Menšík
Artem Boldariev
Artem Boldariev
Matthijs Mekking
Michal Nowak
Evan Hunt
Mark Andrews
Tony Finch
Michał Kępień
Petr Špaček
Suzanne GoldlustandPetr Špaček
Ron AitchisonandPetr Špaček
Arаm Sаrgsyаn
Ondřej Surý
Tony FinchandOndřej Surý