Make isc_thread_join() assert internally on failure
Previously isc_thread_join() would return ISC_R_UNEXPECTED on a failure to
create new thread. All such occurences were caught and wrapped into assert
function at higher level. The function was simplified to assert directly in the
isc_thread_join() function and all caller level assertions were removed.
Make isc_thread_create() assert internally on failure
Previously isc_thread_create() would return ISC_R_UNEXPECTED on a failure to
create new thread. All such occurences were caught and wrapped into assert
function at higher level. The function was simplified to assert directly in the
isc_thread_create() function and all caller level assertions were removed.
Michał Kępień [Wed, 31 Jul 2019 09:06:34 +0000 (11:06 +0200)]
Make root hints consistent with authoritative data
Multiple resolvers in the "wildcard" system test are configured with a
single root hint: "ns.root-servers.nil", pointing to 10.53.0.1, which is
inconsistent with authoritative data served by ns1. This may cause
intermittent resolution failures, triggering false positives for the
"wildcard" system test. Prevent this from happening by making ns2, ns3,
and ns5 use root hints corresponding to the contents of ns1/root.db.in.
Replace usage of isc_mem_put+isc_mem_detach with isc_mem_putanddetach
Using isc_mem_put(mctx, ...) + isc_mem_detach(mctx) required juggling with the
local variables when mctx was part of the freed object. The isc_mem_putanddetach
function can handle this case internally, but it wasn't used everywhere. This
commit apply the semantic patching plus bit of manual work to replace all such
occurrences with proper usage of isc_mem_putanddetach().
Add -q (quiet) option to dnssec-signzone and dnssec-verify tool
With the move of the normal output to stdout, we need a way how to silence the
extra output, so the signed file name can be captured in a simple way. This
commit adds `-q` command line option that will silence all the normal output
that get's printed from both tools.
Change the zoneverify.c to print the information to user supplied function
The lib/dns/zoneverify.c output was hardwired to stderr, which was inconsistent
with lib/dns/dnssec.c. This commit changes zoneverify.c to print the normal run
information to caller supplied function - same model as in the lib/dns/dnssec.c.
Previously, the default output from the libdns library went to stderr by
default. This was inconsistent with the rest of the output. This commit
changes the default logging to go to stdout, with notable exception - when the
output of the signing process goes to stdout, the messages are printed to the
stderr. This is consistent with other functions that output information about
the signing process - e.g. print_stats().
Michał Kępień [Wed, 31 Jul 2019 07:18:02 +0000 (09:18 +0200)]
Make root hints consistent with authoritative data
The ns2 named instance in the "staticstub" system test is configured
with a single root hint commonly used in BIND system tests
(a.root-servers.nil with an address of 10.53.0.1), which is inconsistent
with authoritative data served by ns1. This may cause intermittent
resolution failures, triggering false positives for the "staticstub"
system test. Prevent this from happening by making ns1 serve data
corresponding to the contents of bin/tests/system/common/root.hint.
Michał Kępień [Tue, 30 Jul 2019 19:08:40 +0000 (21:08 +0200)]
Add Alpine Linux to GitLab CI
Ensure BIND is continuously tested on Alpine Linux as it is commonly
used as a base for Docker containers and employs a less popular libc
implementation, musl libc.
Michał Kępień [Tue, 30 Jul 2019 19:08:40 +0000 (21:08 +0200)]
Do not use legacy time zone names
"PST8PDT" is a legacy time zone name whose use in modern code is
discouraged. It so happens that using this time zone with musl libc
time functions results in different output than for other libc
implementations, which breaks the lib/isc/tests/time_test unit test.
Use the "America/Los_Angeles" time zone instead in order to get
consistent output across all tested libc implementations.
Michał Kępień [Tue, 30 Jul 2019 19:08:40 +0000 (21:08 +0200)]
Fix awk invocation in the "verify" system test
Appending output of a command to the same file as the one that command
is reading from is a dangerous practice. It seems to have accidentally
worked with all the awk implementations we have tested against so far,
but for BusyBox awk, doing this may result in the input/output file
being written to in an infinite loop. Prevent this from happening by
redirect awk output to a temporary file and appending its contents to
the original file in a separate shell pipeline.
Michał Kępień [Tue, 30 Jul 2019 19:08:40 +0000 (21:08 +0200)]
Extend prerequisites for the "xfer" system test
The Net::DNS Perl module needs the Digest::HMAC module to support TSIG.
However, since the latter is not a hard requirement for the former, some
packagers do not make Net::DNS depend on Digest::HMAC. If Net::DNS is
installed on a host but Digest::HMAC is not, the "xfer" system test
breaks in a very hard-to-debug way (ans5 returns TSIG RRs with empty
RDATA, which prevents TSIG-signed SOA queries and transfers from
working). Prevent this from happening by making the "xfer" system test
explicitly require Digest::HMAC apart from Net::DNS.
Michał Kępień [Tue, 30 Jul 2019 19:08:40 +0000 (21:08 +0200)]
Make "digdelv" system test work with BusyBox sed
The BusyBox version of sed treats leading '\+' in a regular expression
to be matched as a syntax error ("Repetition not preceded by valid
expression"), which triggers false positives for the "digdelv" system
test. Make the relevant sed invocations work portably across all sed
implementations by removing the leading backslash.
Michał Kępień [Tue, 30 Jul 2019 19:08:40 +0000 (21:08 +0200)]
Make "autosign" system test work with BusyBox awk
The BusyBox version of awk treats some variables which other awk
implementations consider to be decimal values as octal values. This
intermittently breaks key event interval calculations in the "autosign"
system test, trigger false positives for it. Prevent the problem from
happening by stripping leading zeros from the affected awk variables.
Michał Kępień [Tue, 30 Jul 2019 19:08:40 +0000 (21:08 +0200)]
Tweak buffer sizes to prevent compilation warnings
For some libc implementations, BUFSIZ is small enough (e.g. 1024 for
musl libc) to trigger compilation warnings about insufficient size of
certain buffers. Since the relevant buffers are used for printing DNS
names, increase their size to '(n + 1) * DNS_NAME_FORMATSIZE', where 'n'
is the number of DNS names which are printed to a given buffer. This
results in somewhat arbitrary, albeit nicely-aligned and large enough
buffer sizes.
Michał Kępień [Tue, 30 Jul 2019 19:08:40 +0000 (21:08 +0200)]
Always include <errno.h> instead of <sys/errno.h>
Including <sys/errno.h> instead of <errno.h> raises a compiler warning
when building against musl libc. Always include <errno.h> instead of
<sys/errno.h> to prevent that compilation warning from being triggered
and to achieve consistency in this regard across the entire source tree.
Michał Kępień [Tue, 30 Jul 2019 19:08:40 +0000 (21:08 +0200)]
Unify header ordering in unit tests
Make sure all unit tests include headers in a similar order:
1. Three headers which must be included before <cmocka.h>.
2. System headers.
3. UNIT_TESTING definition, followed by the <cmocka.h> header.
4. libisc headers.
5. Headers from other BIND libraries.
6. Local headers.
Also make sure header file names are sorted alphabetically within each
block of #include directives.
Michał Kępień [Tue, 30 Jul 2019 19:08:40 +0000 (21:08 +0200)]
Include <sched.h> where necessary for musl libc
All unit tests define the UNIT_TESTING macro, which causes <cmocka.h> to
replace malloc(), calloc(), realloc(), and free() with its own functions
tracking memory allocations. In order for this not to break
compilation, the system header declaring the prototypes for these
standard functions must be included before <cmocka.h>.
Normally, these prototypes are only present in <stdlib.h>, so we make
sure it is included before <cmocka.h>. However, musl libc also defines
the prototypes for calloc() and free() in <sched.h>, which is included
by <pthread.h>, which is included e.g. by <isc/mutex.h>. Thus, unit
tests including "dnstest.h" (which includes <isc/mem.h>, which includes
<isc/mutex.h>) after <cmocka.h> will not compile with musl libc as for
these programs, <sched.h> will be included after <cmocka.h>.
Always including <cmocka.h> after all other header files is not a
feasible solution as that causes the mock assertion macros defined in
<isc/util.h> to mangle the contents of <cmocka.h>, thus breaking
compilation. We cannot really use the __noreturn__ or analyzer_noreturn
attributes with cmocka assertion functions because they do return if the
tested condition is true. The problem is that what BIND unit tests do
is incompatible with Clang Static Analyzer's assumptions: since we use
cmocka, our custom assertion handlers are present in a shared library
(i.e. it is the cmocka library that checks the assertion condition, not
a macro in unit test code). Redefining cmocka's assertion macros in
<isc/util.h> is an ugly hack to overcome that problem - unfortunately,
this is the only way we can think of to make Clang Static Analyzer
properly process unit test code. Giving up on Clang Static Analyzer
being able to properly process unit test code is not a satisfactory
solution.
Undefining _GNU_SOURCE for unit test code could work around the problem
(musl libc's <sched.h> only defines the prototypes for calloc() and
free() when _GNU_SOURCE is defined), but doing that could introduce
discrepancies for unit tests including entire *.c files, so it is also
not a good solution.
All in all, including <sched.h> before <cmocka.h> for all affected unit
tests seems to be the most benign way of working around this musl libc
quirk. While quite an ugly solution, it achieves our goals here, which
are to keep the benefit of proper static analysis of unit test code and
to fix compilation against musl libc.
Michał Kępień [Mon, 29 Jul 2019 19:44:31 +0000 (21:44 +0200)]
Make root hints consistent with authoritative data
Resolvers in the "filter-aaaa" system test are configured with a single
root hint: "ns.rootservers.net", pointing to 10.53.0.1. However,
querying ns1 for "ns.rootservers.net" results in NXDOMAIN answers.
Since the TTL for the root hint is set to 0, it may happen that a
resolver's ADB will be asked to return any known addresses for
"ns.rootservers.net", but it will only have access to a cached NXDOMAIN
answer for that name and an expired root hint, which will result in a
resolution failure, triggering a false positive for the "filter-aaaa"
system test. Prevent this from happening by making all the root hints
consistent with authoritative data served by ns1.
- removed some dead code
- dns_zone_setdbtype is now void as it could no longer return
anything but ISC_R_SUCCESS; calls to it no longer check for a result
- controlkeylist_fromconfig() is also now void
- fixed a whitespace error
Add semantic patch to remove checking for isc_mem_get() return value
The isc_mem_get() cannot fail gracefully now, it either gets memory of
assert()s. The added semantic patch cleans all the blocks checking whether
the return value of isc_mem_get() was NULL.
The coccinelle and util/update_copyright script have different
idea about how the whitespace should look like. Revert the script
to the previous version, so it doesn't mangle the files in place,
and deal with just whitespace changes.
Michał Kępień [Tue, 23 Jul 2019 09:43:46 +0000 (11:43 +0200)]
Restore locking in resume_dslookup()
Commit 9da902a201b6d0e1bdbac0af067a59bb0a489c9c removed locking around
the fctx_decreference() call inside resume_dslookup(). This allows
fctx_unlink() to be called without the bucket lock being held, which
must never happen. Ensure the bucket lock is held by resume_dslookup()
before it calls fctx_decreference().
Michał Kępień [Fri, 19 Jul 2019 09:44:40 +0000 (11:44 +0200)]
Add dnstap builds to CI
Ensure BIND with dnstap support enabled is being continuously tested by
adding --enable-dnstap to the ./configure invocation used for CentOS 7
and Debian sid builds in GitLab CI.
Restore locking in dns_resolver_shutdown and dns_resolver_attach
Although the struct dns_resolver.exiting member is protected by stdatomics, we
actually need to wait for whole dns_resolver_shutdown() to finish before
destroying the resolver object. Otherwise, there would be a data race and some
fctx objects might not be destroyed yet at the time we tear down the
dns_resolver object.
projects / thirdparty / bind9.git / log
