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.
Michal Nowak [Wed, 30 Dec 2020 12:22:46 +0000 (13:22 +0100)]
Merge UNTESTED and SKIPPED system test results
Descriptions of UNTESTED and SKIPPED system test results are very
similar to one another and it may be confusing when to pick one and
when the other. Merging these two system test results removes the
confusion and also makes system test more aligned with Automake,
which does not know about UNTESTED test result.
Michal Nowak [Wed, 30 Dec 2020 12:23:18 +0000 (13:23 +0100)]
Record skipped test as skipped in testsuite summary
When system test execution was ported to Automake, SKIPPED and UNTESTED
system test result were not made to match Automake expectations,
therefore a skipped test is recorded by Automake as "PASS":
$ make check TESTS=cpu V=1
I:cpu:cpu test only runs on Linux, skipping test
I:cpu:Prerequisites missing, skipping test.
R:cpu:SKIPPED
E:cpu:2020-12-16T11:36:58+0000
PASS: cpu
====================================================================
Testsuite summary for BIND 9.17.7
====================================================================
# TOTAL: 1
# PASS: 1
For a test to be recorded by Automake as skipped, the test, or it's test
driver, needs to exit with code 77:
$ make check TESTS=cpu V=1
I:cpu:cpu test only runs on Linux, skipping test
I:cpu:Prerequisites missing, skipping test.
R:cpu:SKIPPED
E:cpu:2020-12-16T11:39:10+0000
SKIP: cpu
====================================================================
Testsuite summary for BIND 9.17.7
====================================================================
# TOTAL: 1
# PASS: 0
# SKIP: 1
Mark Andrews [Thu, 28 Jan 2021 23:12:14 +0000 (10:12 +1100)]
Address theoretical buffer overrun in recent change
The strlcat() call was wrong.
*** CID 316608: Memory - corruptions (OVERRUN)
/lib/dns/resolver.c: 5017 in fctx_create()
5011 * Make fctx->info point to a copy of a formatted string
5012 * "name/type".
5013 */
5014 dns_name_format(name, buf, sizeof(buf));
5015 dns_rdatatype_format(type, typebuf, sizeof(typebuf));
5016 p = strlcat(buf, "/", sizeof(buf));
>>> CID 316608: Memory - corruptions (OVERRUN)
>>> Calling "strlcat" with "buf + p" and "1036UL" is suspicious because "buf" points into a buffer of 1036 bytes and the function call may access "(char *)(buf + p) + 1035UL". [Note: The source code implementation of the function has been overridden by a builtin model.]
5017 strlcat(buf + p, typebuf, sizeof(buf));
5018 fctx->info = isc_mem_strdup(mctx, buf);
5019
5020 FCTXTRACE("create");
5021 dns_name_init(&fctx->name, NULL);
5022 dns_name_dup(name, mctx, &fctx->name);
Michal Nowak [Thu, 14 Jan 2021 11:09:04 +0000 (12:09 +0100)]
Add --enable-option-checking=fatal to ./configure in CI
The --enable-option-checking=fatal option prevents ./configure from
proceeding when an unknown option is used in the ./configure step in CI.
This change will avoid adding unsupported ./configure options or options
with typo or typo in pairwise testing "# [pairwise: ...]" marker.
Michal Nowak [Mon, 7 Dec 2020 17:08:53 +0000 (18:08 +0100)]
Lint manual pages
As we generate manual pages from reStructuredText sources, we don't have
absolute control on manual page output and therefore 'mandoc -Tlint' may
always report warnings we can't eliminate. In light of this some mandoc
warnings need to be ignored.
Michal Nowak [Wed, 3 Feb 2021 12:44:55 +0000 (13:44 +0100)]
Build man pages when "make doc" is run
Man pages are currently only generated from reStructuredText sources
when "make man" is run in the doc/man/ directory. Tweak
doc/man/Makefile.am so that running "make doc" in the top-level
directory also causes man pages to be generated, so that all potential
documentation building problems can be detected by a single make
invocation.
Mark Andrews [Wed, 27 Jan 2021 06:17:36 +0000 (17:17 +1100)]
Silence Insecure data handling (TAINTED_SCALAR)
Coverity assumes that the memory holding any value read using byte
swapping is tainted. As we store the NSEC3PARAM records in wire
form and iterations is byte swapped the memory holding the record
is marked as tainted. nsec3->salt_length is marked as tainted
transitively. To remove the taint the value need to be range checked.
For a correctly formatted record region.length should match
nsec3->salt_length and provides a convenient value to check the field
against.
*** CID 316507: Insecure data handling (TAINTED_SCALAR)
/lib/dns/rdata/generic/nsec3param_51.c: 241 in tostruct_nsec3param()
235 region.length = rdata->length;
236 nsec3param->hash = uint8_consume_fromregion(®ion);
237 nsec3param->flags = uint8_consume_fromregion(®ion);
238 nsec3param->iterations = uint16_consume_fromregion(®ion);
239
240 nsec3param->salt_length = uint8_consume_fromregion(®ion);
>>> CID 316507: Insecure data handling (TAINTED_SCALAR)
>>> Passing tainted expression "nsec3param->salt_length" to "mem_maybedup", which uses it as an offset.
241 nsec3param->salt = mem_maybedup(mctx, region.base,
242 nsec3param->salt_length);
243 if (nsec3param->salt == NULL) {
244 return (ISC_R_NOMEMORY);
245 }
246 isc_region_consume(®ion, nsec3param->salt_length);
Mark Andrews [Wed, 27 Jan 2021 06:11:52 +0000 (17:11 +1100)]
Silence Untrusted value as argument (TAINTED_SCALAR)
Coverity assumes that the memory holding any value read using byte
swapping is tainted. As we store the NSEC3 records in wire form
and iterations is byte swapped the memory holding the record is
marked as tainted. nsec3->salt_length and nsec3->next_length are
marked as tainted transitively. To remove the taint the values need
to be range checked. Valid values for these should never exceed
region.length so that is becomes a reasonable value to check against.
*** CID 316509: (TAINTED_SCALAR)
/lib/dns/rdata/generic/nsec3_50.c: 312 in tostruct_nsec3()
306 if (nsec3->salt == NULL) {
307 return (ISC_R_NOMEMORY);
308 }
309 isc_region_consume(®ion, nsec3->salt_length);
310
311 nsec3->next_length = uint8_consume_fromregion(®ion);
>>> CID 316509: (TAINTED_SCALAR)
>>> Passing tainted expression "nsec3->next_length" to "mem_maybedup", which uses it as an offset.
312 nsec3->next = mem_maybedup(mctx, region.base, nsec3->next_length);
313 if (nsec3->next == NULL) {
314 goto cleanup;
315 }
316 isc_region_consume(®ion, nsec3->next_length);
317
/lib/dns/rdata/generic/nsec3_50.c: 305 in tostruct_nsec3()
299 region.length = rdata->length;
300 nsec3->hash = uint8_consume_fromregion(®ion);
301 nsec3->flags = uint8_consume_fromregion(®ion);
302 nsec3->iterations = uint16_consume_fromregion(®ion);
303
304 nsec3->salt_length = uint8_consume_fromregion(®ion);
>>> CID 316509: (TAINTED_SCALAR)
>>> Passing tainted expression "nsec3->salt_length" to "mem_maybedup", which uses it as an offset.
305 nsec3->salt = mem_maybedup(mctx, region.base, nsec3->salt_length);
306 if (nsec3->salt == NULL) {
307 return (ISC_R_NOMEMORY);
308 }
309 isc_region_consume(®ion, nsec3->salt_length);
310
Mark Andrews [Mon, 1 Feb 2021 00:43:45 +0000 (11:43 +1100)]
Fix wrong length passed to isc_mem_put
If an invalid key name (e.g. "a..b") in a primaries list in named.conf
is specified the wrong size is passed to isc_mem_put resulting in the
returned memory being put on the wrong freed list.
*** CID 316784: Incorrect expression (SIZEOF_MISMATCH)
/bin/named/config.c: 636 in named_config_getname()
630 isc_buffer_constinit(&b, objstr, strlen(objstr));
631 isc_buffer_add(&b, strlen(objstr));
632 dns_fixedname_init(&fname);
633 result = dns_name_fromtext(dns_fixedname_name(&fname), &b, dns_rootname,
634 0, NULL);
635 if (result != ISC_R_SUCCESS) {
CID 316784: Incorrect expression (SIZEOF_MISMATCH)
Passing argument "*namep" of type "dns_name_t *" and argument "8UL /* sizeof (*namep) */" to function "isc__mem_put" is suspicious.
636 isc_mem_put(mctx, *namep, sizeof(*namep));
637 *namep = NULL;
638 return (result);
639 }
640 dns_name_dup(dns_fixedname_name(&fname), mctx, *namep);
641
Test for Ed25519 and Ed448. If both algorithms are not supported, skip
test. If only one algorithm is supported, run test, skip the
unsupported algorithm. If both are supported, run test normally.
Create new ns3. This will test Ed448 specifically, while now ns2 only
tests Ed25519. This moves some files from ns2/ to ns3/.
The number of queries to use in the burst can be reduced, as we have
a very low fetch limit of 1.
The dig command in 'wait_for_fetchlimits()' should time out sooner as
we expect a SERVFAIL to be returned promptly.
Enabling serve-stale can be done before hitting fetch-limits. This
reduces the chance that the resolver queries time out and fetch count
is reset. The chance of that happening is already slim because
'resolver-query-timeout' is 10 seconds, but better to first let the
data become stale rather than doing that while attempting to resolve.
The 'query_usestale()' function was only called when in
'query_gotanswer()' and an unexpected error occurred. This may have
been "quota reached", and thus we were in some cases returning
stale data on fetch-limits (and if serve-stale enabled of course).
But we can also hit fetch-limits when recursing because we are
following a referral (in 'query_notfound()' and
'query_delegation_recurse()'). Here we should also check for using
stale data in case an error occurred.
Specifically don't check for using stale data when refetching a
zero TTL RRset from cache.
Move the setting of DNS_DBFIND_STALESTART into the 'query_usestale()'
function to avoid code duplication.
Mark Andrews [Wed, 3 Feb 2021 06:20:09 +0000 (17:20 +1100)]
Attempt to silence untrusted loop bound
Assign hit_len + key_len to len and test the result
rather than recomputing and letting the compiler simplify.
213 isc_region_consume(®ion, 2); /* hit length + algorithm */
9. tainted_return_value: Function uint16_fromregion returns tainted data. [show details]
10. tainted_data_transitive: Call to function uint16_fromregion with tainted argument *region.base returns tainted data.
11. tainted_return_value: Function uint16_fromregion returns tainted data.
12. tainted_data_transitive: Call to function uint16_fromregion with tainted argument *region.base returns tainted data.
13. var_assign: Assigning: key_len = uint16_fromregion(®ion), which taints key_len.
214 key_len = uint16_fromregion(®ion);
14. lower_bounds: Casting narrower unsigned key_len to wider signed type int effectively tests its lower bound.
15. Condition key_len == 0, taking false branch.
215 if (key_len == 0) {
216 RETERR(DNS_R_FORMERR);
217 }
16. Condition !!(_r->length >= _l), taking true branch.
17. Condition !!(_r->length >= _l), taking true branch.
218 isc_region_consume(®ion, 2);
18. lower_bounds: Casting narrower unsigned key_len to wider signed type int effectively tests its lower bound.
19. Condition region.length < (unsigned int)(hit_len + key_len), taking false branch.
219 if (region.length < (unsigned)(hit_len + key_len)) {
220 RETERR(DNS_R_FORMERR);
221 }
222
20. lower_bounds: Casting narrower unsigned key_len to wider signed type int effectively tests its lower bound.
21. Condition _r != 0, taking false branch.
223 RETERR(mem_tobuffer(target, rr.base, 4 + hit_len + key_len));
22. lower_bounds: Casting narrower unsigned key_len to wider signed type int effectively tests its lower bound.
23. var_assign_var: Compound assignment involving tainted variable 4 + hit_len + key_len to variable source->current taints source->current.
224 isc_buffer_forward(source, 4 + hit_len + key_len);
225
226 dns_decompress_setmethods(dctx, DNS_COMPRESS_NONE);
CID 281461 (#1 of 1): Untrusted loop bound (TAINTED_SCALAR)
24. tainted_data: Using tainted variable source->active - source->current as a loop boundary.
Ensure that tainted values are properly sanitized, by checking that their values are within a permissible range.
227 while (isc_buffer_activelength(source) > 0) {
228 dns_name_init(&name, NULL);
229 RETERR(dns_name_fromwire(&name, source, dctx, options, target));
230 }
projects / thirdparty / bind9.git / log
