Evan Hunt [Fri, 3 Jun 2022 23:22:01 +0000 (16:22 -0700)]
don't keep stale NXDOMAIN cache entries
when serve-stale is enabled, NXDOMAIN cache entries are no longer
preserved after the normal negative cache TTL, in order to reduce
unnecessary cache memory consumption.
query.c:9394:26: warning: Dereference of null pointer [core.NullDereference]
if (!qctx->nxrewrite || qctx->rpz_st->m.rpz->addsoa) {
^~~~~~~~~~~~~~~~~~~
1 warning generated.
The warning above is for qctx->rpz_st potentially being a NULL pointer
when query_nxdomain() is called from query_resume(). This is a false
positive because none of the database lookup result codes currently
causing query_nxdomain() to be called (DNS_R_EMPTYWILD, DNS_R_NXDOMAIN)
can be returned by a database lookup following a recursive resolution
attempt. Add a NULL check nevertheless in order to future-proof the
code and silence Clang Static Analyzer.
Michał Kępień [Fri, 10 Jun 2022 12:30:23 +0000 (14:30 +0200)]
Remove NULL checks for ns_client_getnamebuf()
ns_client_getnamebuf() cannot fail (i.e. return NULL) since commit e31cc1eeb436095490c7caa120de148df82ecd6c. Remove redundant NULL checks
performed on the pointer returned by ns_client_getnamebuf().
Petr Špaček [Thu, 12 May 2022 07:20:46 +0000 (09:20 +0200)]
Refactor and unite internal data structures for iscconf Sphinx extension
It turns out it is easier to regenerate Sphinx-mandated structure in
get_objects than to maintain two separate data structures. I should have
realized that before.
Petr Špaček [Tue, 10 May 2022 12:50:34 +0000 (14:50 +0200)]
Add table generator into Sphinx config extension
New directive .. statementlist:: generates table of statements in a
the given domain (named.conf or rndc.conf). The table contains link to
definition, short description, and also list of tags.
Short description and tags have to be provided by user using optional
parameters. E.g.:
.. statement:: max-cache-size
:tags: resolver, cache
:short: Short description
.. statementlist:: is currently not parametrized.
This modification is based on Sphinx "tutorial" extension "TODO".
The main trick is to use placeholder node for .. statementlist:: and
replace it with table at later stage, when all source files were
processed and all cross-references can be resolved.
Beware, some details in Sphinx docs are not up-to-date, it's better
to read Sphinx and docutil sources.
Petr Špaček [Tue, 10 May 2022 13:00:06 +0000 (15:00 +0200)]
Add Sphinx extension to help with ARM maintenance and cross-linking
The extension provides a "Sphinx domain factory". Each new Sphinx domain
defines a namespace for configuration statements so named.conf and
rndc.conf do not clash. Currently the Sphinx domains are instantiated
twice and resuling domains are named "namedconf" and "rndcconf".
This commit adds a single new directive:
.. statement:: max-cache-size
It is namespaced like this:
.. namedconf:statement:: max-cache-size
This directive generates a new anchor for configuration statement and it
can be referenced like :any:`max-cache-size` (if the identifier is
unique), or more specific :namedconf:ref:`max-cache-size`.
It is based on Sphinx "tutorial" extension "recipe".
Beware, some details in Sphinx docs are not up-to-date, it's better
to read Sphinx and docutil sources.
Aram Sargsyan [Wed, 4 May 2022 09:43:49 +0000 (09:43 +0000)]
Cleanup dns_fwdtable_delete()
The conversion of `DNS_R_PARTIALMATCH` into `DNS_R_NOTFOUND` is done
in the `dns_rbt_deletename()` function so there is no need to do that
in `dns_fwdtable_delete()`.
Add a possible return value of `ISC_R_NOSPACE` into the header file's
function description comment.
Aram Sargsyan [Tue, 3 May 2022 22:28:45 +0000 (22:28 +0000)]
Convert some catz error messages from ISC_LOG_INFO to ISC_LOG_WARNING
There is no reason for these two messages to be `ISC_LOG_INFO` while all
the other similar messages in `catz_addmodzone_taskaction()` and
`catz_delzone_taskaction()` functions are logged as `ISC_LOG_WARNING`.
Aram Sargsyan [Tue, 3 May 2022 22:24:32 +0000 (22:24 +0000)]
Check that catz member zone is not a configured forward zone
When processing a catalog zone member zone make sure that there is no
configured pre-existing forward zone with that name.
Refactor the `dns_fwdtable_find()` function to not alter the
`DNS_R_PARTIALMATCH` result (coming from `dns_rbt_findname()`) into
`DNS_R_SUCCESS`, so that now the caller can differentiate partial
and exact matches. Patch the calling sites to expect and process
the new return value.
Tom Krizek [Tue, 7 Jun 2022 15:27:25 +0000 (17:27 +0200)]
Move pylint CI job to precheck stage
Historically, some *.py files were generated, so Python checks required
running ./configure beforehand. This is no longer the case since v9_18,
so let's run the job ASAP without the unnecessary extra dependency on
autoconf job.
Tom Krizek [Tue, 7 Jun 2022 14:29:52 +0000 (16:29 +0200)]
Remove flake8 linter for Python from CI
Python codestyle is now handled by black and other issues are checked by
pylint. Flake8 checking has been made redundant and is thus removed as
obsolete.
Aram Sargsyan [Mon, 6 Jun 2022 15:12:46 +0000 (15:12 +0000)]
Remove unneded NULL-checking
Fix an issue reported by Coverity by removing the unneded check.
*** CID 352554: Null pointer dereferences (REVERSE_INULL)
/bin/dig/dighost.c: 3056 in start_tcp()
3050
3051 if (ISC_LINK_LINKED(query, link)) {
3052 next = ISC_LIST_NEXT(query, link);
3053 } else {
3054 next = NULL;
3055 }
>>> CID 352554: Null pointer dereferences (REVERSE_INULL)
>>> Null-checking "connectquery" suggests that it may be null, but it
has already been dereferenced on all paths leading to the check.
3056 if (connectquery != NULL) {
3057 query_detach(&connectquery);
3058 }
3059 query_detach(&query);
3060 if (next == NULL) {
3061 clear_current_lookup();
In the cases where we test SOA serial updates and TTL updates, we check
if for "all zones loaded" to ensure the new zone content is loaded. But
this is the unsigned zone, the signed zone still needs to be produced.
There is thus a timing issue where the dig request comes in before
the signing process has finished.
Petr Špaček [Tue, 15 Mar 2022 10:55:36 +0000 (11:55 +0100)]
Flag new user-visible log messages for review
Messages with log levels INFO or higher are flagged for manual review.
Purpose of this check is to prevent debug logs to being released with
too-high log level.
Petr Špaček [Fri, 6 May 2022 16:44:15 +0000 (18:44 +0200)]
ARM style change: render literals in black color
After enormous amount of bikesheding about colors we decided to override
ReadTheDocs default style for literals (``literal`` in the RST markup).
Justification:
- The default RTD "light red literal on white background" is hard to
read. https://webaim.org/resources/contrastchecker/ reports that text
colored as rgb(231, 76, 60) on white background has insufficient
contrast.
- The ARM has enormous amount of literals all over the place and thus
one sentence can contain several black/red/black color changes. This
is distracting. As a consequence, the ARM looks like a Geronimo
Stilton book.
What we experimented with as replacements for red:
- Green - way too distracting
- Blue - too similar to "usual clickable link"
- Violet - too Geronimo Stilton style
- Brown - better but still distracting
After all the bikesheding we settled on black, i.e. the same as all
"normal" text. I.e. the color is now the same and literals are denoted
by monospaced font and a box around the literal. This has best contrast
and is way less distracting than it used to be.
This lead to a new problem: Internal references to "term definitions"
defined using directives like .. option:: were rendered almost the same
as literals:
- References: monospaced + box + bold + clickable
- Literals: monospaced + box To distinguish these two we added black
dotted underline to clickable references.
Petr Špaček [Tue, 10 May 2022 14:53:40 +0000 (16:53 +0200)]
Allow wrapping for ARM table content
RTD style default never wraps <th> and <td> elements and that just does
not work for real sentences or any other long lines.
We can reconsider styling some tables separately, but at the moment we
do not have use for tables with long but unwrappable lines so it's
easier to allow wrapping globally.
Aram Sargsyan [Wed, 1 Jun 2022 08:51:55 +0000 (08:51 +0000)]
Don't process DNSSEC-related and ZONEMD records in catz
When processing a catalog zone update, skip processing records with
DNSSEC-related and ZONEMD types, because we are not interested in them
in the context of a catalog zone, and processing them will fail and
produce an unnecessary warning message.
Ondřej Surý [Wed, 1 Jun 2022 11:10:37 +0000 (13:10 +0200)]
Properly adjust the srcdir vs builddir paths
Affected unit tests load testdata from the srcdir. Previously, there
was a kludge that chdir()ed to the tests srcdir, but that get removed
during refactoring. Instead of introducing the kludge again, the paths
were fixed to be properly prefixed with TESTS_DIR as needed.
Ondřej Surý [Wed, 1 Jun 2022 07:02:10 +0000 (09:02 +0200)]
Don't list libtest.la headers in HEADERS variable
The libtest.la headers were installed in very weird place, in fact, we
don't need to list them in the HEADERS variable, listing them in SOURCES
is enough for autotools to figure out how to compile the convenience
library.
Artem Boldariev [Wed, 25 May 2022 11:49:32 +0000 (14:49 +0300)]
Increase server start timeout for system tests
This commit increases server start timeout from 60 to 90 seconds in
order to avoid system test failures on some platforms due to inability
to initialise TLS contexts in time.
Tony Finch [Fri, 6 May 2022 07:19:54 +0000 (08:19 +0100)]
CHANGES note for [GL !6270]
[cleanup] Simplify BIND's internal DNS name compression API. As
RFC 6891 explains, it isn't practical to deploy new
label types or compression methods, so it isn't
necessary to have an API designed to support them.
Remove compression terminology that refers to Internet
Drafts that expired in the 1990s.
Tony Finch [Thu, 5 May 2022 17:36:48 +0000 (18:36 +0100)]
Clean up remaining references to global compression
It is simply called "compression" now, without any qualifiers. Also,
improve some variable names in dns_name_towire2() so they are not two
letter abbreviations for global something.
Tony Finch [Thu, 5 May 2022 15:36:52 +0000 (16:36 +0100)]
Shrink decompression contexts
It's wasteful to use 20 bytes and a pointer indirection to represent
two bits of information, so turn the struct into an enum. And change
the names of the enumeration constants to make the intent more clear.
This change introduces some inline functions into another header,
which confuses `gcovr` when it is trying to collect code coverage
statistics. So, in the CI job, copy more header files into a directory
where `gcovr` looks for them.
Tony Finch [Wed, 4 May 2022 16:35:39 +0000 (17:35 +0100)]
DNS name compression does not depend on the EDNS version
There was a proposal in the late 1990s that it might, but it turned
out to be unworkable. See RFC 6891, Extension Mechanisms for
DNS (EDNS(0)), section 5, Extended Label Types.
The remnants of the code that supported this in BIND are redundant.
Tony Finch [Wed, 4 May 2022 08:38:54 +0000 (09:38 +0100)]
Remove obsolete notes on name compression
These notes describe the initial compression design for BIND 9 in
1998/1999, when the IETF had some over-optimistic plans for using EDNS
to change the wire format of domain names. (Another example was
bitstring labels for IPv6 reverse DNS.) By the end of 2000 the EDNS
name compression schemes had been abandoned, and BIND 9's compression
code was rewritten to use a hash table.
There is nothing left of the implementation described here, and the
API functions are better described in `compress.h`, so these notes are
more misleading than helpful. Those who are interested in the past can
look at the version control history.
Mark Andrews [Wed, 11 May 2022 04:32:11 +0000 (14:32 +1000)]
Make modifications to keyless.example deterministic
The perl modifation code for keyless.example was not deterministic
(/NXT/ matched part of signature) resulting in different error
strings being returned. Replaced /NXT/ with /A RRSIG NSEC/ and
updated expected error string,
Evan Hunt [Sat, 14 May 2022 03:55:23 +0000 (20:55 -0700)]
don't create managed-keys zone unless dnssec-validation is "auto"
previously, a managed-keys zone was created for every view
regardless of whether rfc5011 was in use; when it was not in
use, the zone would be left empty. this made for some confusing
log messages.
we now only set up the managed-keys zone if dnssec-validation is
set to the default value of "auto".
certain system test servers have had their dnssec-validation settings
changed to auto because the tests depended on the existence of the
zone.
The key lifetime should not be shorter than the time it costs to
introduce the successor key, otherwise keys will be created faster than
they are removed, resulting in a large key set.
The time it takes to replace a key is determined by the publication
interval (Ipub) of the successor key and the retire interval of the
predecessor key (Iret).
For the ZSK, Ipub is the sum of the DNSKEY TTL and zone propagation
delay (and publish safety). Iret is the sum of Dsgn, the maximum zone
TTL and zone propagation delay (and retire safety). The sign delay is
the signature validity period minus the refresh interval: The time to
ensure that all existing RRsets have been re-signed with the new key.
The ZSK lifetime should be larger than both values.
For the KSK, Ipub is the sum of the DNSKEY TTL and zone propagation
delay (and publish safety). Iret is the sum of the DS TTL and parent
zone propagation delay (and retire safety). The KSK lifetime should be
larger than both values.
The signatures-refresh should not near the signatures-validity value,
to prevent operational instability. Same is true when checking against
signatures-validity-dnskey.
Evan Hunt [Wed, 4 May 2022 02:43:23 +0000 (19:43 -0700)]
Stop the unit tests from running twice
Move the libtest code into a 'libtest' subdirectory and make it
one of the SUBDIRS in the tests Makefile. having it at the top level
required having "." as one of the subdirs, and that caused the
unit tests to be executed twice.
Ondřej Surý [Tue, 3 May 2022 09:37:31 +0000 (11:37 +0200)]
Move all the unit tests to /tests/<libname>/
The unit tests are now using a common base, which means that
lib/dns/tests/ code now has to include lib/isc/include/isc/test.h and
link with lib/isc/test.c and lib/ns/tests has to include both libisc and
libdns parts.
Instead of cross-linking code between the directories, move the
/lib/<foo>/test.c to /tests/<foo>.c and /lib/<foo>/include/<foo>test.h
to /tests/include/tests/<foo>.h and create a single libtest.la
convenience library in /tests/.
At the same time, move the /lib/<foo>/tests/ to /tests/<foo>/ (but keep
it symlinked to the old location) and adjust paths accordingly. In few
places, we are now using absolute paths instead of relative paths,
because the directory level has changed. By moving the directories
under the /tests/ directory, the test-related code is kept in a single
place and we can avoid referencing files between libns->libdns->libisc
which is unhealthy because they live in a separate Makefile-space.
In the future, the /bin/tests/ should be merged to /tests/ and symlink
kept, and the /fuzz/ directory moved to /tests/fuzz/.
Ondřej Surý [Mon, 2 May 2022 08:56:42 +0000 (10:56 +0200)]
Give the unit tests a big overhaul
The unit tests contain a lot of duplicated code and here's an attempt
to reduce code duplication.
This commit does several things:
1. Remove #ifdef HAVE_CMOCKA - we already solve this with automake
conditionals.
2. Create a set of ISC_TEST_* and ISC_*_TEST_ macros to wrap the test
implementations, test lists, and the main test routine, so we don't
have to repeat this all over again. The macros were modeled after
libuv test suite but adapted to cmocka as the test driver.
ISC_TEST_MAIN (Discussion: Should this be ISC_TEST_RUN ?)
For more complicated examples including group setup and teardown
functions, and per-test setup and teardown functions.
3. The macros prefix the test functions and cmocka entries, so the name
of the test can now match the tested function name, and we don't have
to append `_test` because `run_test_` is automatically prepended to
the main test function, and `setup_test_` and `teardown_test_` is
prepended to setup and teardown function.
4. Update all the unit tests to use the new syntax and fix a few bits
here and there.
5. In the future, we can separate the test declarations and test
implementations which are going to greatly help with uncluttering the
bigger unit tests like doh_test and netmgr_test, because the test
implementations are not declared static (see `ISC_RUN_TEST_DECLARE`
and `ISC_RUN_TEST_IMPL` for more details.
NOTE: This heavily relies on preprocessor macros, but the result greatly
outweighs all the negatives of using the macros. There's less
duplicated code, the tests are more uniform and the implementation can
be more flexible.
Ondřej Surý [Thu, 19 May 2022 09:20:21 +0000 (11:20 +0200)]
Make all tasks to be bound to a thread
Previously, tasks could be created either unbound or bound to a specific
thread (worker loop). The unbound tasks would be assigned to a random
thread every time isc_task_send() was called. Because there's no logic
that would assign the task to the least busy worker, this just creates
unpredictability. Instead of random assignment, bind all the previously
unbound tasks to worker 0, which is guaranteed to exist.