Mark Andrews [Sun, 21 Feb 2021 22:28:37 +0000 (09:28 +1100)]
Silence CID 320481: Null pointer dereferences
*** CID 320481: Null pointer dereferences (REVERSE_INULL)
/bin/tests/wire_test.c: 261 in main()
255 process_message(input);
256 }
257 } else {
258 process_message(input);
259 }
260
CID 320481: Null pointer dereferences (REVERSE_INULL)
Null-checking "input" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
261 if (input != NULL) {
262 isc_buffer_free(&input);
263 }
264
265 if (printmemstats) {
266 isc_mem_stats(mctx, stdout);
Mark Andrews [Tue, 16 Feb 2021 05:15:25 +0000 (16:15 +1100)]
Silence CID 281450: Dereference before null check
remove redundant 'inst != NULL' test
162cleanup:
CID 281450 (#1 of 1): Dereference before null check (REVERSE_INULL)
check_after_deref: Null-checking inst suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
163 if (result != ISC_R_SUCCESS && inst != NULL) {
164 plugin_destroy((void **)&inst);
165 }
Mark Andrews [Tue, 16 Feb 2021 05:05:56 +0000 (16:05 +1100)]
Silence CID 304936 Dereference before null check
Removed redundant 'listener != NULL' check.
1191cleanup:
CID 304936 (#1 of 1): Dereference before null check (REVERSE_INULL)
check_after_deref: Null-checking listener suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
1192 if (listener != NULL) {
1193 isc_refcount_decrement(&listener->refs);
1194 listener->exiting = true;
1195 free_listener(listener);
1196 }
1. A wrong comment in ns3/setup.sh (we are subtracting 2 hours, not
adding them).
2. 'get_keyids' used bad parameters "$1" "$2" when 'check_numkeys'
failed. Also, 'check_numkeys' can use $DIR, $ZONE, and $NUMKEYS
directly, no need to pass them.
Add some more zones to the kasp system test to test the 'purge-keys'
option. Three zones test that the predecessor key files are removed
after the purge keys interval, one test checks that the key files
are retained if 'purge-keys' is disabled. For that, we change the
times to 90 days in the past (the default value for 'purge-keys').
On each keymgr run, we now also check if key files can be removed.
The 'purge-keys' interval determines how long keys should be retained
after they have become completely hidden.
Key files should not be removed if it has a state that is set to
something else then HIDDEN, if purge-keys is 0 (disabled), if
the key goal is set to OMNIPRESENT, or if the key is unused (a key is
unused if no timing metadata set, and no states are set or if set,
they are set to HIDDEN).
If the last changed timing metadata plus the purge-keys interval is
in the past, the key files may be removed.
Add a dst_key_t variable 'purge' to signal that the key file should
not be written to file again.
Add a new option 'purge-keys' to 'dnssec-policy' that will purge key
files for deleted keys. The option determines how long key files
should be retained prior to removing the corresponding files from
disk.
If set to 0, the option is disabled and 'named' will not remove key
files from disk.
Ondřej Surý [Fri, 19 Feb 2021 11:53:36 +0000 (12:53 +0100)]
Include lib/isc/tls_p.h in release tarballs
The addition of lib/isc/tls_p.h to the source tree was not accounted for
in the relevant variable in lib/isc/Makefile.am and thus the former file
is not being included in release tarballs prepared using "make dist".
Fix by tweaking the libisc_la_SOURCES list in lib/isc/Makefile.am
accordingly.
Michał Kępień [Fri, 19 Feb 2021 10:52:56 +0000 (11:52 +0100)]
Do not require libtool in PATH at build time
The build-time requirement for libtool was introduced inadvertently:
1. Commit 1628f5865acb2d472ce4adf71fc78ac99094fa1c added a check to
configure.ac which claims to test whether the libtool script is
available. There are two problems with that check:
- it is effectively a no-op as the AC_PROG_LIBTOOL() macro always
sets the LIBTOOL variable [1],
- this check was intended to be performed before autoreconf is
run, not when ./configure is run; the libtool script is supposed
to be dynamically generated by ./configure on the build host and
thus there is no need for a standalone libtool script to be
installed system-wide on every host attempting to build BIND 9
e.g. from a tarball produced by "make dist".
2. Commit a7982d14dddb864420deb49e735f782022d1fa07 was based on the
incorrect assumption that the AC_PROG_LIBTOOL() macro looks for the
libtool binary in PATH and sets the LIBTOOL variable accordingly,
which is what other AC_PROG_*() macros do. Meanwhile, the
AC_PROG_LIBTOOL() macro only initializes libtool for use with
Automake. It is not necessary for a standalone libtool script to be
available in PATH on the build host when ./configure is run.
Do not look for libtool in PATH at build time as it prevents hosts
without a libtool script available system-wide from building BIND 9 from
source tarballs prepared using "make dist". Note that libtool m4
macros, utilities, etc. still need to be present on a given host if
autoreconf is to be run on it.
Ondřej Surý [Tue, 9 Feb 2021 16:44:40 +0000 (17:44 +0100)]
Use library constructor/destructor to initialize OpenSSL
Instead of calling isc_tls_initialize()/isc_tls_destroy() explicitly use
gcc/clang attributes on POSIX and DLLMain on Windows to initialize and
shutdown OpenSSL library.
This resolves the issue when isc_nm_create() / isc_nm_destroy() was
called multiple times and it would call OpenSSL library destructors from
isc_nm_destroy().
At the same time, since we now have introduced the ctor/dtor for libisc,
this commit moves the isc_mem API initialization (the list of the
contexts) and changes the isc_mem_checkdestroyed() to schedule the
checking of memory context on library unload instead of executing the
code immediately.
Disable calling DllMain() on thread creation/destruction
Disables the DLL_THREAD_ATTACH and DLL_THREAD_DETACH notifications for
the specified dynamic-link library (DLL). This can reduce the size of
the working set for some applications.
Ondřej Surý [Tue, 9 Feb 2021 12:25:52 +0000 (13:25 +0100)]
Fix the invalid condition variable
Although harmless, the memmove() in tlsdns and tcpdns was guarded by a
current message length variable that was always bigger than 0 instead of
correct current buffer length remainder variable.
Ondřej Surý [Tue, 9 Feb 2021 12:25:46 +0000 (13:25 +0100)]
Move most of the OpenSSL initialization to isc_tls
Since we now require both libcrypto and libssl to be initialized for
netmgr, we move all the OpenSSL initialization code except the engine
initialization to isc_tls API.
The isc_tls_initialize() and isc_tls_destroy() has been made idempotent,
so they could be called multiple time. However when isc_tls_destroy()
has been called, the isc_tls_initialize() could not be called again.
Ondřej Surý [Fri, 5 Feb 2021 16:18:28 +0000 (17:18 +0100)]
Remove overrun checking code from memory allocator
The ISC_MEM_CHECKOVERRUN would add canary byte at the end of every
allocations and check whether the canary byte hasn't been changed at the
free time. The AddressSanitizer and valgrind memory checks surpases
simple checks like this, so there's no need to actually keep the code
inside the allocator.
Ondřej Surý [Fri, 5 Feb 2021 09:25:07 +0000 (10:25 +0100)]
Modify the way we benchmark mem_{get,put}
Previously, the mem_{get,put} benchmark would pass the allocation size
as thread_create argument. This has been now changed, so the allocation
size is stored and decremented (divided) in atomic variable and the
thread create routing is given a memory context. This will allow to
write tests where each thread is given different memory context and do
the same for mempool benchmarking.
Ondřej Surý [Fri, 5 Feb 2021 09:25:07 +0000 (10:25 +0100)]
Disable memory debugging features in non-developer build
The two memory debugging features: ISC_MEM_DEFAULTFILL
(ISC_MEMFLAG_FILL) and ISC_MEM_TRACKLINES were always enabled in all
builds and the former was only disabled in `named`.
This commits disables those two features in non-developer build to make
the memory allocator significantly faster.
Ondřej Surý [Thu, 4 Feb 2021 20:56:49 +0000 (21:56 +0100)]
Make the memory and mempool counters to be stdatomic types
This is yet another step into unlocking some parts of the memory
contexts. All the regularly updated variables has been turned into
atomic types, so we can later remove the locks when updating various
counters.
Also unlock as much code as possible without breaking anything.
Bump the maximum number of hazard pointers in tests
On 24-core machine, the tests would crash because we would run out of
the hazard pointers. We now adjust the number of hazard pointers to be
in the <128,256> interval based on the number of available cores.
Note: This is just a band-aid and needs a proper fix.
Ondřej Surý [Thu, 4 Feb 2021 19:19:09 +0000 (20:19 +0100)]
Remove the extra level of indirection via isc_memmethods_t
Previously, the applications using libisc would be able to override the
internal memory methods with own implementation. This was no longer
possible, but the extra level of indirection was not removed. This
commit removes the extra level of indirection for the memory methods and
the default_memalloc() and default_memfree().
Ondřej Surý [Thu, 4 Feb 2021 19:11:20 +0000 (20:11 +0100)]
Remove the internal memory allocator
The internal memory allocator had an extra code to keep a list of blocks
for small size allocation. This would help to reduce the interactions
with the system malloc as the memory would be already allocated from the
system, but there's an extra cost associated with that - all the
allocations/deallocations must be locked, effectively eliminating any
optimizations in the system allocator targeted at multi-threaded
applications. While the isc_mem API is still using locks pretty heavily,
this is a first step into reducing the memory allocation/deallocation
contention.
Michal Nowak [Tue, 16 Feb 2021 10:33:58 +0000 (11:33 +0100)]
Prevent Git to expand $systest
CentOS 8 "git status" unexpectedly expands search directory "tsig" to
also search in the "tsiggss" directory, thus incorrectly identifying
files as "not removed" in the "tsig" directory:
$ git status -su --ignored tsig
$ touch tsiggss/ns1/{named.run,named.memstats}
$ git status -su --ignored tsig
!! tsiggss/ns1/named.memstats
!! tsiggss/ns1/named.run
Michal Nowak [Mon, 15 Feb 2021 10:59:33 +0000 (11:59 +0100)]
Check for "file not removed" in system test output
Run this check only when in Git repository, because the run.sh function
which produces the "file not removed" warning is run only when build
directory is the same as the source directory, that is only for in-tree
builds.
Ondřej Surý [Thu, 11 Feb 2021 07:37:52 +0000 (08:37 +0100)]
Rollback setting IP_DONTFRAG option on the UDP sockets
In DNS Flag Day 2020, the development branch started setting the
IP_DONTFRAG option on the UDP sockets. It turned out, that this
code was incomplete leading to dropping the outgoing UDP packets.
Henceforth this commit rolls back this setting until we have a
proper fix that would send back empty response with TC flag set.
Evan Hunt [Wed, 3 Feb 2021 19:36:33 +0000 (11:36 -0800)]
enable listen-on parameters to be specified in any order
updated the parser to allow the "port", "tls" and "http"
paramters to "listen-on" and "listen-on-v6" to be specified in any
order. previously the parser would throw an error if any other order
was used than port, tls, http.
Evan Hunt [Wed, 3 Feb 2021 18:58:46 +0000 (10:58 -0800)]
require "tls none" for unencrypted HTTP listeners
unencrypted DoH connections may be used in some operational
environments where encryption is handled by a reverse proxy,
but it's going to be relatively rare, so we shouldn't make it
easy to do by mistake. this commit changes the syntax for
listen-on and listen-on-v6 so that if "http" is specified, "tls"
must also be specified; for unencrypted listeners, "tls none"
can be used.
Mark Andrews [Fri, 22 Jan 2021 04:59:03 +0000 (15:59 +1100)]
Fix linking order for OpenSSL libraries
As libssl depends on libcrypto, -lssl needs to precede -lcrypto in
linker invocations or else the build will fail with static OpenSSL
libraries. Adjust m4/ax_check_openssl.m4 to prevent this issue from
getting triggered when pkg-config files for OpenSSL are not available.
Michal Nowak [Mon, 15 Feb 2021 16:20:50 +0000 (17:20 +0100)]
Use SKIPPED_TEST_EXIT_CODE consistently
Commit fa505bfb0e7623d7cfc94ae15a0246ae71000904 omitted two unit tests
while introducing the SKIP_TEST_EXIT_CODE preprocessor macro. Fix the
outliers to make use of SKIP_TEST_EXIT_CODE consistent across all unit
tests. Also make sure lib/dns/tests/dnstap_test returns an exit code
that indicates a skipped test when dnstap is not enabled.
Ondřej Surý [Thu, 11 Feb 2021 13:40:59 +0000 (14:40 +0100)]
Stop including gssapi.h from dst/gssapi.h header
The only reason for including the gssapi.h from the dst/gssapi.h header
was to get the typedefs of gss_cred_id_t and gss_ctx_id_t. Instead of
using those types directly this commit introduces dns_gss_cred_id_t and
dns_gss_ctx_id_t types that are being used in the public API and
privately retyped to their counterparts when we actually call the gss
api.
This also conceals the gssapi headers, so users of the libdns library
doesn't have to add GSSAPI_CFLAGS to the Makefile when including libdns
dst API.
Ondřej Surý [Thu, 11 Feb 2021 13:15:38 +0000 (14:15 +0100)]
Stop including lmdb.h from <dns/view.h>
The lmdb.h doesn't have to be included from the dns/view.h header as it
is separately included where used. This stops exposing the inclusion of
lmdb.h from the libdns headers.
Ondřej Surý [Thu, 11 Feb 2021 13:13:27 +0000 (14:13 +0100)]
Move the <isc/readline.h> header to bin/dig/readline.h
The <isc/readline.h> header provided a compatibility shim to use when
other non-GNU readline libraries are in use. The two places where
readline library is being used is nslookup and nsupdate, so the header
file has been moved to bin/dig directory and it's directly included from
bin/nsupdate.
This also conceals any readline headers exposed from the libisc headers.
Ondřej Surý [Thu, 11 Feb 2021 13:07:26 +0000 (14:07 +0100)]
Remove the extra CFLAGS from libisc_CFLAGS and libdns_CFLAGS
The extra library CFLAGS were causing the headers to be included in
wrong order possibly pulling header files from previously installed
BIND 9 version.
This commit cleans up the extra <foo>_CFLAGS from the includes in favor
of not exposing 3rd party headers in our own header files.
Diego Fronza [Thu, 11 Feb 2021 14:32:20 +0000 (11:32 -0300)]
Fix dangling references to outdated views after reconfig
This commit fix a leak which was happening every time an inline-signed
zone was added to the configuration, followed by a rndc reconfig.
During the reconfig process, the secure version of every inline-signed
zone was "moved" to a new view upon a reconfig and it "took the raw
version along", but only once the secure version was freed (at shutdown)
was prev_view for the raw version detached from, causing the old view to
be released as well.
This caused dangling references to be kept for the previous view, thus
keeping all resources used by that view in memory.