Ondřej Surý [Thu, 31 Oct 2019 11:50:58 +0000 (06:50 -0500)]
libdns: add missing checks for return values in dnstap unit test
Related scan-build report:
dnstap_test.c:169:2: warning: Value stored to 'result' is never read
result = dns_test_makeview("test", &view);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
dnstap_test.c:193:2: warning: Value stored to 'result' is never read
result = dns_compress_init(&cctx, -1, dt_mctx);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 warnings generated.
Ondřej Surý [Thu, 31 Oct 2019 11:46:32 +0000 (06:46 -0500)]
named: remove named_g_defaultdnstap global variable
The named_g_defaultdnstap was never used as the dnstap requires
explicit configuration of the output file.
Related scan-build report:
./server.c:3476:14: warning: Value stored to 'dpath' during its initialization is never read
const char *dpath = named_g_defaultdnstap;
^~~~~ ~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
Tony Finch [Tue, 15 Oct 2019 14:12:29 +0000 (15:12 +0100)]
Do not flush the cache for `rndc validation status`
And add a note to the man page that `rndc validation` flushes the
cache when the validation state is changed. (It is necessary to flush
the cache when turning on validation, to avoid continuing to use
cryptographically invalid data. It is probably wise to flush the cache
when turning off validation to recover from lameness problems.)
Tony Finch [Tue, 15 Oct 2019 14:06:01 +0000 (15:06 +0100)]
Include all views in output of `rndc validation status`
The implementation of `rndc validation status` iterates over all the
views to print their validation status. It takes care to print newlines
in between, but it also used put a nul byte at the end of the first view
which truncated the output.
After this change, the nul byte is added at the end so that it prints
the validation status in all views. The `_bind` view is skipped
because its validation status is irrelevant.
Michał Kępień [Thu, 31 Oct 2019 07:48:35 +0000 (08:48 +0100)]
Prevent query loops for misbehaving servers
If a TCP connection fails while attempting to send a query to a server,
the fetch context will be restarted without marking the target server as
a bad one. If this happens for a server which:
- was already marked with the DNS_FETCHOPT_EDNS512 flag,
- responds to EDNS queries with the UDP payload size set to 512 bytes,
- does not send response packets larger than 512 bytes,
and the response for the query being sent is larger than 512 byes, then
named will pointlessly alternate between sending UDP queries with EDNS
UDP payload size set to 512 bytes (which are responded to with truncated
answers) and TCP connections until the fetch context retry limit is
reached. Prevent such query loops by marking the server as bad for a
given fetch context if the advertised EDNS UDP payload size for that
server gets reduced to 512 bytes and it is impossible to reach it using
TCP.
Tony Finch [Tue, 22 Oct 2019 14:37:38 +0000 (15:37 +0100)]
Fix hang in `named-compilezone | head`
I was truncating zone files for experimental purposes when I found
that `named-compilezone | head` got stuck. The full command line that
exhibited the problem was:
This requires a large enough zone to exhibit the problem, more than
about 70000 bytes of plain text output from named-compilezone.
I was running the command on Debian Stretch amd64.
This was puzzling since it looked like something was suppressing the
SIGPIPE. I used `strace` to examine what was happening at the hang.
The program was just calling write() a lot to print the zone file, and
the last write() hanged until I sent it a SIGINT.
During some discussion with friends, Ian Jackson guessed that opening
/dev/stdout O_RDRW might be the problem, and after some tests we found
that this does in fact suppress SIGPIPE.
Since `named-compilezone` only needs to write to its output file, the
fix is to omit the stdio "+" update flag.
Ondřej Surý [Mon, 28 Oct 2019 20:04:38 +0000 (15:04 -0500)]
Disable NSEC Aggressive Cache (synth-from-dnssec) by default
It was found that NSEC Aggressive Caching has a significant performance impact
on BIND 9 when used as recursor. This commit disables the synth-from-dnssec
configuration option by default to provide immediate remedy for people running
BIND 9.12+. The NSEC Aggressive Cache will be enabled again after a proper fix
will be prepared.
Michał Kępień [Tue, 29 Oct 2019 08:26:41 +0000 (09:26 +0100)]
Revamp the release checklist
Make the release checklist match the current release process better by
adding missing steps, rearranging existing ones, reassigning
responsibilities, and dividing the list into sections (by due date).
Michał Kępień [Wed, 16 Oct 2019 20:06:00 +0000 (22:06 +0200)]
Fix cppcheck 1.89 warnings
cppcheck 1.89 enabled certain value flow analysis mechanisms [1] which
trigger null pointer dereference false positives in lib/dns/rpz.c:
lib/dns/rpz.c:582:7: warning: Possible null pointer dereference: tgt_ip [nullPointer]
if (KEY_IS_IPV4(tgt_prefix, tgt_ip)) {
^
lib/dns/rpz.c:1419:44: note: Calling function 'adj_trigger_cnt', 4th argument 'NULL' value is 0
adj_trigger_cnt(rpzs, rpz_num, rpz_type, NULL, 0, true);
^
lib/dns/rpz.c:582:7: note: Null pointer dereference
if (KEY_IS_IPV4(tgt_prefix, tgt_ip)) {
^
lib/dns/rpz.c:596:7: warning: Possible null pointer dereference: tgt_ip [nullPointer]
if (KEY_IS_IPV4(tgt_prefix, tgt_ip)) {
^
lib/dns/rpz.c:1419:44: note: Calling function 'adj_trigger_cnt', 4th argument 'NULL' value is 0
adj_trigger_cnt(rpzs, rpz_num, rpz_type, NULL, 0, true);
^
lib/dns/rpz.c:596:7: note: Null pointer dereference
if (KEY_IS_IPV4(tgt_prefix, tgt_ip)) {
^
lib/dns/rpz.c:610:7: warning: Possible null pointer dereference: tgt_ip [nullPointer]
if (KEY_IS_IPV4(tgt_prefix, tgt_ip)) {
^
lib/dns/rpz.c:1419:44: note: Calling function 'adj_trigger_cnt', 4th argument 'NULL' value is 0
adj_trigger_cnt(rpzs, rpz_num, rpz_type, NULL, 0, true);
^
lib/dns/rpz.c:610:7: note: Null pointer dereference
if (KEY_IS_IPV4(tgt_prefix, tgt_ip)) {
^
It seems that cppcheck no longer treats at least some REQUIRE()
assertion failures as fatal, so add extra assertion macro definitions to
lib/isc/include/isc/util.h that are only used when the CPPCHECK
preprocessor macro is defined; these definitions make cppcheck 1.89
behave as expected.
There is an important requirement for these custom definitions to work:
cppcheck must properly treat abort() as a function which does not
return. In order for that to happen, the __GNUC__ macro must be set to
a high enough number (because system include directories are used and
system headers compile attributes away if __GNUC__ is not high enough).
__GNUC__ is thus set to the major version number of the GCC compiler
used, which is what that latter does itself during compilation.
Michał Kępień [Tue, 15 Oct 2019 19:57:58 +0000 (21:57 +0200)]
Remove remnants of the --with-cc-alg option
Commit afa81ee4e4e863fa646177947c55e8c6b1475f47 omitted some spots in
the source tree which are still referencing the removed --with-cc-alg
"configure" option. Make sure the latter is removed completely.
Michał Kępień [Tue, 15 Oct 2019 19:35:18 +0000 (21:35 +0200)]
Limit triggers for OpenBSD system test jobs
When a GitLab CI runner is not under load, a single OpenBSD system test
job completes in about 12 minutes, which is considered decent. However,
such jobs are usually multiplexed with other system test jobs on the
same host, which causes each of them to take even 40 minutes to
complete. Taking retries into account, this is completely unacceptable
for everyday use, so only start OpenBSD system test jobs for pipelines
created through GitLab's web interface and for pipelines created for Git
tags.
Michał Kępień [Tue, 15 Oct 2019 18:49:08 +0000 (20:49 +0200)]
Tweak dependencies for the Windows build job
Since the Windows build job does not use the files created as a result
of running "autoreconf -fi" in the "autoreconf:sid:amd64" job, set its
dependencies to an empty list.
Since it is currently not possible to use "needs: []" for jobs which do
not belong to the first stage of a pipeline, set the "needs" key for the
Windows build job to the "autoreconf:sid:amd64" job so that all build
jobs are started at the same time (without this change, the Windows
build job does not start until all jobs in the "precheck" stage are
finished).
As a side note, these changes also attempt to eliminate intermittent,
bogus GitLab error messages ("There has been a missing dependency
failure").
Michał Kępień [Tue, 15 Oct 2019 18:49:08 +0000 (20:49 +0200)]
Fix artifacts created by the "autoreconf" CI job
The intended purpose of the "autoreconf:sid:amd64" GitLab CI job is to
run "autoreconf -fi" and then pass the updated files on to subsequent
non-Windows build jobs. However, the artifacts currently created by
that job only include files which are not tracked by Git. Since we
currently do track e.g. "configure" with Git, the aforementioned job is
essentially a no-op. Fix by manually specifying the files generated by
the "autoreconf:sid:amd64" job that should be passed on to subsequent
build jobs.
Michał Kępień [Tue, 15 Oct 2019 14:38:04 +0000 (16:38 +0200)]
Add OpenBSD to GitLab CI
Ensure BIND can be tested on OpenBSD in GitLab CI to more quickly catch
build and test errors on that operating system.
Some notes:
- While GCC is packaged for OpenBSD, only old versions (4.2.1, 4.9.4)
are readily available and none of them is the default system
compiler, so we are only doing Clang builds in GitLab CI.
- Unit tests are currently not run on OpenBSD because it ships with an
old version of kyua which does not handle skipped tests properly.
These jobs will be added when we move away from using kyua in the
future as the test code itself works fine.
- All OpenBSD jobs are run inside QEMU virtual machines, using GitLab
Runner Custom executor.
However, if the .NOTPARALLEL pseudo-target is added to this Makefile,
"make -k -j6 foo" will return 0 as well.
Since bin/tests/Makefile contains the .NOTPARALLEL pseudo-target,
running "make -k -j6 test" from bin/tests/ on OpenBSD prevents any
errors from being reported through that command's exit code.
Work around the issue by running "make -k -j6 test" in the
bin/tests/system/ directory instead as bin/tests/system/Makefile does
not contain the .NOTPARALLEL pseudo-target and thus things work as
expected there.
Mark Andrews [Sun, 13 Oct 2019 14:27:38 +0000 (10:27 -0400)]
Merge branch '1143-a-minor-documentation-issue-consideration-of-parsing-inconsistencies-in-ipv4s-in-address-match-lists-and-in-a-controls-inet-statement' into 'master'
Resolve "A minor documentation issue & consideration of parsing inconsistencies in IPv4s in address match lists and in a controls/inet statement"
Tony Finch [Wed, 2 Oct 2019 18:43:09 +0000 (19:43 +0100)]
cleanup: more consistent abbreviated DS digest type mnemonics
BIND supports the non-standard DNSKEY algorithm mnemonic ECDSA256
everywhere ECDSAP256SHA256 is allowed, and allows algorithm numbers
interchangeably with mnemonics. This is all done in one place by the
dns_secalg_fromtext() function.
DS digest types were less consistent: the rdata parser does not allow
abbreviations like SHA1, but the dnssec-* command line tools do; and
the command line tools do not alow numeric types though that is the
norm in rdata.
The command line tools now use the dns_dsdigest_fromtext() function
instead of rolling their own variant, and dns_dsdigest_fromtext() now
knows about abbreviated digest type mnemonics.