Michał Kępień [Mon, 18 Jul 2022 12:39:02 +0000 (14:39 +0200)]
Run a short respdiff test for all merge requests
Running a respdiff test for every merge request would be useful for
catching protocol-breaking changes before they are applied to the source
code. However, the existing respdiff-based tests take a while to
complete (about half an hour with our current CI infrastructure), which
does not make them a good fit for this purpose. Add a new GitLab CI
job, "respdiff-short", which uses a smaller query set that gets
processed within a couple of minutes on our current CI infrastructure.
Rename the existing respdiff-based jobs to make distinguishing them
easier.
Michał Kępień [Mon, 18 Jul 2022 12:39:02 +0000 (14:39 +0200)]
Extract respdiff job definition to a YAML anchor
Ensure the common parts of all jobs using respdiff are available in the
form of a reusable YAML anchor, to reduce code duplication and to
simplify adding more respdiff-based jobs to GitLab CI.
Michał Kępień [Mon, 18 Jul 2022 12:39:02 +0000 (14:39 +0200)]
Use a pre-built executable as the reference named
The "respdiff" GitLab CI job compares DNS responses produced by the
current version of named with those produced by a reference version.
The latter is built from source in each "respdiff" job, despite the fact
that the reference version changes very rarely. Use a pre-built named
executable as the reference version instead, assuming it is available in
the OS image used for "respdiff" tests.
The BUFSIZ value varies between platforms, it could be 8K on Linux and
512 bytes on mingw. Make sure the buffers are always big enough for the
output data to prevent truncation of the output by appropriately
enlarging or sizing the buffers.
Michał Kępień [Fri, 15 Jul 2022 08:23:03 +0000 (10:23 +0200)]
Make "named -h" output match option-handling code
The usage instructions printed by "named -h" are missing the "external"
and "internal" flags that can be passed to the -M command-line option.
Add the missing flags to "named -h" output.
Make the style of the relevant paragraph more in line with the next one
and split its contents up into an unordered list of options for improved
readability.
Michał Kępień [Fri, 15 Jul 2022 08:23:03 +0000 (10:23 +0200)]
Handle ISC_MEM_DEFAULTFILL consistently
Contrary to what the documentation states, memory filling is only
enabled by --enable-developer (or by setting -DISC_MEM_DEFAULTFILL=1) if
the internal memory allocator is used. However, the internal memory
allocator is disabled by default, so just using the --enable-developer
build-time option does not enable memory filling (passing "-M fill" on
the named command line is necessary to actually enable it). As memory
filling is a useful tool for troubleshooting certain types of bugs, it
should also be enabled by --enable-developer when the system allocator
is used.
Furthermore, memory-related preprocessor macros are handled in two
distinct locations: lib/isc/include/isc/mem.h and bin/named/main.c.
This makes the logic hard to follow.
Move all code handling the ISC_MEM_DEFAULTFILL preprocessor macro to
lib/isc/include/isc/mem.h, ensuring memory filling is enabled by the
--enable-developer build-time switch, no matter which memory allocator
is used.
Michał Kępień [Fri, 15 Jul 2022 08:23:03 +0000 (10:23 +0200)]
Fix mempool stats bug in the internal allocator
Commit c96b6eb5ece1d44fdfbce45da2364e3764956822 changed the way mempool
code handles freed allocations that cannot be retained for later use as
"free list" items: it no longer uses different logic depending on
whether the internal allocator is used or the system one. However, that
commit did not update a relevant piece of code in isc_mempool_destroy(),
causing memory context statistics to always be off upon shutdown when
BIND 9 is built with -DISC_MEM_USE_INTERNAL_MALLOC=1. This causes
assertion failures. Update isc_mempool_destroy() accordingly in order
to prevent this issue from being triggered.
Mark Andrews [Tue, 21 Dec 2021 01:44:17 +0000 (12:44 +1100)]
disassociate rdatasets when cleaning up
free_namelist could be passed names with associated rdatasets
when handling errors. These need to be disassociated before
calling dns_message_puttemprdataset.
Mark Andrews [Thu, 23 Jun 2022 04:22:10 +0000 (14:22 +1000)]
Make "checking revoked key with duplicate key ID" work
There should be 2 keys with the same key id after the numerically
lower one is revoked (serial space arithmetic). The DS points
at the non-revoked key so validation should still succeed.
log the reason for falling back to AXFR from IXFR at level info
messages indicating the reason for a fallback to AXFR (i.e, because
the requested serial number is not present in the journal, or because
the size of the IXFR response would exceeed "max-ixfr-ratio") are now
logged at level info instead of debug(4).
When dnssec-policy is used, and the zone is not dynamic, BIND will
assume that the zone is inline-signed. But the function responsible
for this did not inherit the dnssec-policy option from the view or
options level, and thus never enabled inline-signing, while the zone
should have been.
Michał Kępień [Fri, 8 Jul 2022 09:26:34 +0000 (11:26 +0200)]
Fix fetch context use-after-free bugs
fctx_decreference() may call fctx_destroy(), which in turn may free the
fetch context by calling isc_mem_putanddetach(). This means that
whenever fctx_decreference() is called, the fetch context pointer should
be assumed to point to garbage after that call. Meanwhile, the
following pattern is used in several places in lib/dns/resolver.c:
Given that 'fctx' may be freed by the fctx_decreference() call, there is
no guarantee that the value of fctx->bucketnum will be the same before
and after the fctx_decreference() call. This can cause all kinds of
locking issues as LOCK() calls no longer match up with their UNLOCK()
counterparts.
Fix by always using a helper variable to hold the bucket number when the
pattern above is used.
Note that fctx_try() still uses 'fctx' after calling fctx_decreference()
(it calls fctx_done()). This is safe to do because the reference count
for 'fctx' is increased a few lines earlier and it also cannot be zero
right before that increase happens, so the fctx_decreference() call in
that particular location never invokes fctx_destroy(). Nevertheless,
use a helper variable for that call site as well, to retain consistency
and to prevent copy-pasted code from causing similar problems in the
future.
Petr Špaček [Thu, 16 Jun 2022 12:03:45 +0000 (14:03 +0200)]
Deduplicate Manual Signing between DNSSEC chapter and DNSSEC Guide
The two procedures were essentially the same, but each instance was
missing some details from the other. They are now combined into one text
in the DNSSEC Guide and linked from DNSSEC chapter.
Petr Špaček [Thu, 9 Jun 2022 09:53:13 +0000 (11:53 +0200)]
Rewrite DNSSEC Validation subchapter in the ARM
Mostly deduplicating and linking information across the ARM.
Generally people should not touch it unless they what they are doing, so
let's try to discourage them a bit.
There were structural changes to the ARM in the main branch, and
replacing the whole file with a new version is an order of magniture
easier than attempting to cherry-pick individual changes which should, in
the end, produce the same file under a different name.
File names in the main branch and v9_16 are now in sync (for the DNSSEC
chapter).
Mark Andrews [Thu, 9 Jun 2022 08:13:35 +0000 (18:13 +1000)]
update ifconfig.sh
* make it harder to get the interface numbers wrong by using 'max'
to specify the upper bound of the sequence of interfaces and use 'max'
when calculating the interface number
* extract the platform specific instruction into 'up' and 'down'
and call them from the inner loop so that the interface number is
calculated in one place.
* calculate the A and AAAA address in a single place rather than
in each command
* use /sbin/ipadm on Solaris 2.11 and greater
Mark Andrews [Fri, 1 Jul 2022 01:13:51 +0000 (11:13 +1000)]
Tighten $GENERATE directive parsing
The original sscanf processing allowed for a number of syntax errors
to be accepted. This included missing the closing brace in
${modifiers}
Look for both comma and right brace as intermediate seperators as
well as consuming the final right brace in the sscanf processing
for ${modifiers}. Check when we got right brace to determine if
the sscanf consumed more input than expected and if so behave as
if it had stopped at the first right brace.
Mark Andrews [Fri, 1 Jul 2022 01:40:37 +0000 (11:40 +1000)]
Check for overflow in $GENERATE computations
$GENERATE uses 'int' for its computations and some constructions
can overflow values that can be represented by an 'int' resulting
in undefined behaviour. Detect these conditions and return a
range error.
Mark Andrews [Tue, 5 Jul 2022 03:05:58 +0000 (13:05 +1000)]
Increase the amount of time allowed for signing to occur in
On slow systems we have seen this take 9 seconds. Increased the
allowance from 3 seconds to 10 seconds to reduce the probabilty of
a false negative from the system test.
Mark Andrews [Tue, 5 Jul 2022 03:01:11 +0000 (13:01 +1000)]
Only report not matching stderr content when we look for it
The previous test code could emit "D:cds:stderr did not match ''" rather
that just showing the contents of stderr. Moved the debug line inside
the if/else block.
Replaced backquotes with $() and $(()) as approriate.
Mark Andrews [Mon, 4 Jul 2022 03:32:01 +0000 (13:32 +1000)]
Fix for GitLab 15.0: cobertura replaced by coverage_report
From Gitlab 15.0 release notes:
artifacts:reports:cobertura keyword
As of GitLab 15.0, the artifacts:reports:cobertura keyword has
been replaced by artifacts:reports:coverage_report. Cobertura
is the only supported report file, but this is the first step
towards GitLab supporting other report types.
Aram Sargsyan [Wed, 15 Jun 2022 10:27:41 +0000 (10:27 +0000)]
Remove resolver.c:maybe_destroy()
After refactoring of `validated()`, the `maybe_destroy()` function is
no longer expected to actually destroy the fetch context when it is
being called, so effectively it only ensures that the validators are
canceled when the context has no more queries and pending events, but
that is redundant, because `maybe_destroy()` `REQUIRE`s that the context
should be in the shutting down state, and the function which sets that
state is already canceling the validators in its own turn.
As a failsafe, to make sure that no validators will be created after
`fctx_doshutdown()` is called, add an early return from `valcreate()` if
the context is in the shutting down state.
Aram Sargsyan [Fri, 10 Jun 2022 14:44:52 +0000 (14:44 +0000)]
Fix a race between resolver query timeout and validation
The `resolver.c:validated()` function unlinks the current validator from
the fetch's validators list, which can leave it empty, then unlocks
the bucket lock. If, by a chance, the fetch was timed out just before
the `validated()` call, the final timeout callback running in parallel
with `validated()` can find the fetch context with no active fetches
and with an empty validators list and destroy it, which is unexpected
for the `validated()` function and can lead to a crash.
Increase the fetch context's reference count in the beginning of
`validated()` and decrease it when it finishes its work to avoid the
unexpected destruction of the fetch context.
Michał Kępień [Wed, 22 Jun 2022 11:45:46 +0000 (13:45 +0200)]
Fix destination port extraction for client queries
The current logic for determining the address of the socket to which a
client sent its query is:
1. Get the address:port tuple from the netmgr handle using
isc_nmhandle_localaddr() or from the ns_interface_t structure.
2. Convert the address:port tuple from step 1 into an isc_netaddr_t
using isc_netaddr_fromsockaddr().
3. Convert the address from step 2 back into a socket address with the
port set to 0 using isc_sockaddr_fromnetaddr().
Note that the port number (readily available in the netmgr handle or in
the ns_interface_t structure) is needlessly lost in the process,
preventing it from being recorded in dnstap captures of client traffic
produced by named.
Fix by first storing the address:port tuple in client->destsockaddr and
then creating an isc_netaddr_t from that structure. This allows the
port number to be retained in client->destsockaddr, which is what
subsequently gets passed to dns_dt_send().
Michal Nowak [Wed, 15 Jun 2022 14:06:48 +0000 (16:06 +0200)]
Do not run Ubuntu 18.04 jobs in MR-triggered pipelines
With the addition of Ubuntu 22.04 three more CI jobs were added. To
compensate for that, move Ubuntu 18.04 jobs out of MR-triggered
pipelines to schedule-triggered ones.
Also, move --disable-geoip ./configure options from Ubuntu 18.04 to
Ubuntu 20.04 jobs to keep these options in the more frequent
MR-triggered pipelines.
Michal Nowak [Thu, 16 Jun 2022 09:25:43 +0000 (11:25 +0200)]
Fix implicit string concatenation in tests-checkds.py
pylint 2.14.2 reports the following warnings:
bin/tests/system/checkds/tests-checkds.py:265:0: W1404: Implicit string concatenation found in call (implicit-str-concat)
bin/tests/system/checkds/tests-checkds.py:273:0: W1404: Implicit string concatenation found in call (implicit-str-concat)
projects / thirdparty / bind9.git / log
