This commit makes the 'doth' system test skip HTTP headers check when
curl version is new enough but was compiled without HTTP/2 support.
This should fix the 'doth' system test for macOS systems using
macports.
This commit fixes a peculiar corner case in the client-side DoT code
because of which a crash could occur during a zone transfer. A junk
DNS message should be sent at the end of a zone transfer via TLS to
trigger the crash (abort).
This commit, hopefully, fixes that.
Also, this commit adds similar changes to the TCP DNS code, as it
shares the same origin and most of the logic.
When a UDP dispatch receives a mismatched response, it checks whether
there is still enough time to wait for the correct one to arrive before
the timeout fires. If there is not, the result code is set to
ISC_R_TIMEDOUT, but it is not subsequently used anywhere as 'response'
is set to NULL a few lines earlier. This results in the higher-level
read callback (resquery_response() in case of resolver code) not being
called. However, shortly afterwards, a few levels up the call chain,
isc__nm_udp_read_cb() calls isc__nmsocket_timer_stop() on the dispatch
socket, effectively disabling read timeout handling for that socket.
Combined with the fact that reading is not restarted in such a case
(e.g. by calling dispatch_getnext() from udp_recv()), this leads to the
higher-level query structure remaining referenced indefinitely because
the dispatch socket it uses will neither be read from nor closed due to
a timeout. This in turn causes fetch contexts to linger around
indefinitely, which in turn i.a. prevents certain cache nodes (those
containing rdatasets used by fetch contexts, like fctx->nameservers)
from being cleaned.
Fix by making sure the higher-level callback does get invoked with the
ISC_R_TIMEDOUT result code when udp_recv() determines there is no more
time left to receive the correct UDP response before the timeout fires.
This allows the higher-level callback to clean things up, preventing the
reference leak described above.
The following scenario triggers a "named" crash:
1. Configure a catalog zone.
2. Start "named".
3. Comment out the "catalog-zone" clause.
4. Run `rndc reconfig`.
5. Uncomment the "catalog-zone" clause.
6. Run `rndc reconfig` again.
Implement the required cleanup of the in-memory catalog zone during
the first `rndc reconfig`, so that the second `rndc reconfig` could
find it in an expected state.
the resolver test checks that the correct number of fetches have
been sent NS rrsets of a given size, but it formerly did so by
counting queries received by the authoritative server, which could
result in an off-by-one count if one of the queries had been resent
due to a timeout or a port number collision.
this commit changes the test to count fetches initiated by the
resolver, which should prevent the intermittent test failure, and
is the actual datum we were interested in anyway.
opensslecdsa_fromdns() already rejects too short ECDSA public keys.
Make it also reject too long ones. Remove an assignment made redundant
by this change.
raw_key_to_ossl() assumes fixed ECDSA private key sizes (32 bytes for
ECDSAP256SHA256, 48 bytes for ECDSAP384SHA384). Meanwhile, in rare
cases, ECDSAP256SHA256 private keys are representable in 31 bytes or
less (similarly for ECDSAP384SHA384) and that is how they are then
stored in the "PrivateKey" field of the key file. Nevertheless,
raw_key_to_ossl() always calls BN_bin2bn() with a fixed length argument,
which in the cases mentioned above leads to erroneously interpreting
uninitialized memory as a part of the private key. This results in the
latter being malformed and broken signatures being generated. Address
by using the key length provided by the caller rather than a fixed one.
Apply the same change to public key parsing code for consistency, adding
an INSIST() to prevent buffer overruns.
Most of the test zones in the dnssec system test can be verified.
Use -z when only a single key is being used so that the verifier
knows that only a single key is in use.
The method used to generate a test zone with multiple NSEC and
NSEC3 chains was incorrect. Multiple calls to dnssec-signzone
with multiple parameters is not additive. Extract the chain on
each run then add them to the final signed zone instance.
when processing a mismatched response, we call dns_dispatch_getnext().
If that fails, for example because of a timeout, fctx_done() is called,
which cancels all queries. This triggers a crash afterward when
fctx_cancelquery() is called, and is unnecessary since fctx_done()
would have been called later anyway.
When dns_adb is shutting down, first the adb->shutting_down flag is set
and then task is created that runs shutdown_stage2() that sets the
shutdown flag on names and entries. However, when dns_adb_createfind()
is called, only the individual shutdown flags are being checked, and the
global adb->shutting_down flag was not checked. Because of that it was
possible for a different thread to slip in and create new find between
the dns_adb_shutdown() and dns_adb_detach(), but before the
shutdown_stage2() task is complete. This was detected by
ThreadSanitizer as data race because the zonetable might have been
already detached by dns_view shutdown process and simultaneously
accessed by dns_adb_createfind().
This commit converts the adb->shutting_down to atomic_bool to prevent
the global adb lock when creating the find.
Add a new parameter to 'ns_client_t' to store potential extended DNS
error. Reset when the client request ends, or is put back.
Add defines for all well-known info-codes.
Update the number of DNS_EDNSOPTIONS that we are willing to set.
Create a new function to set the extended error for a client reply.
The documentation was inconsistent with the code. The new description
for cookie-algorithm now reflects the current behavior.
The following two commits are the relevant code changes to this
section of docs: afa81ee4a912f313
Change 5756 (GL #2854) introduced build errors when using
'configure --disable-doh'. To fix this, isc_nm_is_http_handle() is
now defined in all builds, not just builds that have DoH enabled.
Missing code comments were added both for that function and for
isc_nm_is_tlsdns_handle().
Gitlab feature
https://docs.gitlab.com/ee/ci/pipelines/settings.html#auto-cancel-redundant-pipelines
can automatically cancel jobs which operate on an outdated code, i.e. on
branches which received new commits while jobs with an older set of
commits are still running. For this feature to work jobs have to be
configured with boolean interruptible: true.
I think practically all of our current CI jobs can be cancelled,
so the option is now on by default for all jobs.
This is almost minimal prototype to show how to use python-hypothesis
library in a system test. It does not fully replace existing shell-based
system test for wildcards.
Resolve#2854: DoH: Assign HTTP responses freshness lifetime according to the smallest TTL found in the Answer section
Closes#2854
See merge request isc-projects/bind9!5493
This commit makes BIND set the "max-age" value of the "Cache-Control"
HTTP header to the minimal TTL from the Answer section for positive
answers, as RFC 8484 advises in section 5.1.
We calculate the minimal TTL as a side effect of rendering the
response DNS message, so it does not change the code flow much, nor
should it have any measurable negative impact on the performance.
For negative answers, the "max-age" value is set using the TTL and
SOA-minimum values from an SOA record in the Authority section.
This commit adds an isc_nm_set_min_answer_ttl() function which is
intended to to be used to give a hint to the underlying transport
regarding the answer TTL.
The interface is intentionally kept generic because over time more
transports might benefit from this functionality, but currently it is
intended for DoH to set "max-age" value within "Cache-Control" HTTP
header (as recommended in the RFC8484, section 5.1 "Cache
Interaction").
It is no-op for other DNS transports for the time being.
The version number for the XML statistics channel was not incremented
correctly after removal of isc_socket code in
a55589f881, and the JSON version number
was not incremented at all.
Check to see whether there are outstanding requests in the
httpd receive buffer after sending the response, and if so,
process them.
Test that pipelined requests are handled by sending multiple
minimal HTTP/1.1 using netcat (nc) and checking that we get
back the same number of responses.
Remember the amount of space consumed by the HTTP headers, then
move any trailing data to the start of the httpd->recvbuf once
we have finished processing the request.
if an incoming HTTP request is incomplete, but nothing else is clearly
wrong with it, the stats channel continues reading to see if there's
more coming. the buffer length was not being processed correctly in
this case. also, the server state was not reset correctly when the
request was complete, so that subsequent requests could be appended to
the first buffer instead of being treated as new.
in addition fixing the above problems, this commit also increases the
size of the httpd request buffer from 1024 to 4096, because some
browsers send a lot of headers.
A typo introduced in f3f1cab05e prevents
execution of the dns_name_copy-with-result.spatch. The replacement
should end with semicolon not a colon:
plus: parse error:
File "cocci/dns_name_copy-with-result.spatch", line 28, column 23, charpos = 421
around = ':',
whole content = + dns_name_copy(E1, E2):
1) if 'key->external' is set we just need to call
dst__privstruct_writefile
2) the cleanup of 'bufs' was incorrect as 'i' doesn't reflect the
the current index into 'bufs'. Use a simple for loop.
This review was triggered by Coverity reporting a buffer overrun
on 'bufs'.
'dh' was being assigned to key->keydata.dh too soon which could
result in a memory leak on error. Moved the assignement of
key->keydata.dh until after dh was correct.
Coverity was reporting dead code on the error path cleaning up 'dh'
which triggered this review.
'make dist' omits lib/dns/tests/comparekeys/ (added in
7101afa23c) from release tarball it
creates which makes the unit:gcc:tarball CI job permanently fail in the
dst unit test.
Be less strict regarding "tls" statements in the configuration file by allowing both "key-file" and "cert-file" be omitted
See merge request isc-projects/bind9!5546
In the 9.17.19 release "tls" statements verification code was
added. The code was too strict and assumed that every such a statement
should have both "cert-file" and "key-file" specified. This turned out
to be a regression, as in some cases we plan to use the "tls"
statement to specify TLS connection parameters.
This commit fixes this behaviour; now a "tls" statement should either
have both "cert-file" and "key-file" specified, or both should be
omitted.
It was used only as guard against unused variable declaration, but the
surrounding code depends on strtok_r being defined unconditionally, so
there is no point in guarding a variable.
Glibc documentation suggests it is obsolete anyway and e.g. Meson build
system decided to ignore it. It seems to be required only by old
Solaris compiler and OpenIndiana uses gcc.
It's major PITA trying to guess what exactly clang-format has changed,
so how CI stores patch file with changes which can be applied locally if
needed.
PyLint 2.11 reports a new warning, C0209 (consider-using-f-string).
Since f-strings are only available in Python 3.6+, existing scripts
cannot be updated to use this feature just yet because they would stop
working with older Python versions. Instead, disable PyLint warning
C0209 for the time being. Sort all disabled warnings in .pylintrc.
GL #2308 was originally referenced by CHANGES entry 5727. However, the
corresponding code change turned out to be flawed and had to be reverted
in BIND 9.17.19, causing CHANGES entry 5727 to be turned into a
placeholder on the release branch.
Commit 63145fb1d3 subsequently addressed
the flaw, so the fix for GL #2308 will be included in BIND 9.17.20.
Move the relevant CHANGES entry to reflect that.
Previously, when lame cache would be disabled by setting lame-ttl to 0,
it would also disable lame answer detection. In this commit, we enable
the lame response detection even when the lame cache is disabled. This
enables stopping answer processing early rather than going through the
whole answer processing flow.
The lame-ttl cache is implemented in ADB as per-server locked
linked-list "indexed" with <qname,qtype>. This list has to be walked
every time there's a new query or new record added into the lame cache.
Determined attacker can use this to degrade performance of the resolver.
Resolver testing has shown that disabling the lame cache has little
impact on the resolver performance and it's a minimal viable defense
against this kind of attack.
Unless being configured with the `no-deprecated` option, OpenSSL 3.0.0
still has the deprecated APIs present and will throw warnings during
compilation, when using them.
Make sure that the old APIs are being used only with the older versions
of OpenSSL.
OpenSSL 3 deprecates most of the DH* family and associated APIs.
Reimplement the existing functionality using a newer set of APIs
which will be used when compiling/linking with OpenSSL 3.0.0 or newer
versions.
OpenSSL 3 deprecates most of the RSA* family and associated APIs.
Reimplement the existing functionality using a newer set of APIs
which will be used when compiling/linking with OpenSSL 3.0.0 or newer
versions.
OpenSSL 3 deprecates most of the EC* family and associated APIs.
Reimplement the existing functionality using a newer set of APIs
which will be used when compiling/linking with OpenSSL 3.0.0 or newer
versions.
EVP_PKEY_eq() is the replacement with a smaller result range (0, 1)
instead of (-1, 0, 1). EVP_PKEY_cmp() is mapped to EVP_PKEY_eq() when
building with older versions of OpenSSL.
The EVP_MD_CTX_new() and EVP_MD_CTX_free() functions are renamed APIs
which were previously available as EVP_MD_CTX_create() and
EVP_MD_CTX_destroy() respectively, which means that we can use them
instead of providing our own shim functions.
OpenSSL 3.0.0 deprecates the ERR_get_error_line_data() function.
Use ERR_get_error_all() instead of ERR_get_error_line_data() and create
a shim to use the old variant for the older OpenSSL versions which don't
have the newer ERR_get_error_all().
OpenSSL 3.0.0 deprecates the EVP_MD_CTX_md() function.
Use EVP_MD_CTX_md() instead of EVP_MD_CTX_get0_md() and create a shim
to use the old variant for the older OpenSSL versions which don't have
the newer EVP_MD_CTX_get0_md().
OpenSSL 3.0.0 deprecates many low level API functions.
In preparation for the future support of linking BIND with OpenSSL 3.0.0
without the deprecated API functions, change the configure.ac script to
use functions which are available on all supported versions of OpenSSL
and LibreSSL.
The dst_key_pubcompare() and dst_key_compare() didn't have a unit test,
add the unit tests which test comparing the same keys, different keys,
and, where possible, similar keys with a manually altered parameter.
dst_key_pubcompare() internally uses the *_todns() functions of the
lib/dns/openssl*_link.c modules.
dst_key_compare() internally uses the *_compare() functions of the
lib/dns/openssl*_link.c modules.
Duplicate catalog zone entries caused an assertion failure
in named during configuration. This is now a soft error
that is detected earlier by named and also by named-checkconf.
Update the nsec3 system tests to use the new default values. Change
the policy for "nsec3-other" so that we still have a test case for
non-zero salt length.
When using 'nsec3param' in 'dnssec-policy' and no specific parameters
are provided, default to zero additional iterations and no salt, as
recommended by draft-ietf-dnsop-nsec3-guidance.
For the sake of running ASAN and TSAN jobs with the latest stable GCC,
replace "base image" (Debian Buster with GCC 8.3.0) with Fedora 34 image
with GCC 11.
Depending upon when the directory is sampled there may be 2
(oldest version removed and rename / reopen is in progresss) or
3 old versions of the log file.
It was found, that the original commit adding the setmodtime() was
incompletely squashed and there was double check for
DNS_ZONEFLG_NEEDDUMP instead of check for DNS_ZONEFLG_NEEDDUMP and
DNS_ZONEFLG_DUMPING.
Change the duplicate check to DNS_ZONEFLG_DUMPING.
Add a lame delegation to lame.example.org with only an A record
in the additional section; on failure, this will trigger a retry
with AAAA, which will loop. Test that dig returns SERVFAIL, in
addition to confirming that named doesn't hang on shutdown.
If an ADB find is started on behalf of a resolver fetch, and fails to
find any addresses but has a pending resolver fetch associated with it,
then we need to check whether the fetch it's waiting on is the one
that created it. If so, it can never finish and needs to be terminated.
The NAME_FETCH_A and NAME_FETCH_AAAA macros were meant to be
boolean, indicating whether the pointers were set or not, while
the NAME_FETCH_V4 and NAME_FETCH_V6 macros were meant to return
the pointer values. The latter were only used as booleans, so
they've been removed in favor of the former.
Also did some style cleanup and removed an unreachable code block.
there was a race possible in which a dispatch was put into
the 'connected' state before it had a TCP handle attached,
which could cause an assertion failure in dns_dispatch_gettcp().
The isc_time_add() and isc_time_subtract() didn't have a unit test, add
the unit test with couple of edge case vectors to check whether overflow
and underflow is correctly handled.
Use the __builtin_uadd_overflow() and __builtin_usub_overflow() for
overflow checks in isc_time_add() and isc_time_subtract(). This
generates more efficient and safe code.
The isc_time_add() could overflow when t.seconds + i.seconds == UINT_MAX
and t.nanoseconds + i.nanoseconds >= NS_PER_S.
Fix the overflow in isc_time_add(), and simplify the ISC_R_RANGE checks
both in isc_time_add() and isc_time_subtract() functions.
The qmin system test was printing spurious output. On investigation,
the test case turned out to be both broken and ineffective: its
expectations were wrong, and it was printing the output because its
wrong expectations were not met, and those failed expectations were
not causing a test failure. All of this has been corrected.
The ReferenceRole class is only available in Sphinx >= 2.0.0, which
makes building BIND 9 documentation impossible with older Sphinx
versions:
Running Sphinx v1.7.6
Configuration error:
There is a programable error in your configuration file:
Traceback (most recent call last):
File "/usr/lib/python3.6/site-packages/sphinx/config.py", line 161, in __init__
execfile_(filename, config)
File "/usr/lib/python3.6/site-packages/sphinx/util/pycompat.py", line 150, in execfile_
exec_(code, _globals)
File "conf.py", line 21, in <module>
from sphinx.util.docutils import ReferenceRole
ImportError: cannot import name 'ReferenceRole'
Work around the problem by defining a stub version of the ReferenceRole
class if the latter cannot be imported. This allows documentation
(without GitLab hyperlinks in release notes) to be built with older
Sphinx versions.
The fctxbucket_t properly attaches to the fetchctx_t, so it can safely
use its memory context. Save a little bit of memory by removing own
memory context from fctxbucket_t.
Using proper attach/detach functions for the fetch context
instead of fctx_increference() and _decreference() makes
it easier to debug reference counting errors in the resolver.
Fixed several such errors that were found as a result.
it is possible for udp_recv_cb() to fire after the socket
is already shutting down and statichandle is NULL; we need to
create a temporary handle in this case.
it was possible for the route socket's udp_recv() callback to fire
after the interfacemgr was detached, causing an assertion failure.
this has now been fixed by referencing the interfacemgr when setting up
the route socket, and dereferencing it when shutting it down.
The statistics system test sometimes needs a pause to wait for the
expected stats to be reported.
Also, the test for priming queries was ineffective; the result of
the grep was not being checked.
The catz system test included a test case that was looking for a single
answer record after an update, when it should have been looking for two.
The test usually passed because of timing - the first dig usually got a
response before the update was completed - but occasionally the update
processed fast enough for the test to fail. On investigation, it turned
out to be the test that was wrong.
The digdelv system test has a test case in which stderr was
included in the dig output. When trace logging was in use,
this confused the grep and caused a spurious test failure.
The autoconf script prints used compiler version at the end of the
configure script. Solaris native compiler doesn't support --version,
and -V has to be used which in turn isn't supported by Gcc/Clang.
Detect which version flag has to be used and call $CC with it.
Some of the libns unit tests override the isc_nmhandle_attach() and
_detach() functions. This causes a failure in ns_interface_create()
if a route socket is being used, so we add a parameter to disable it.
route/netlink sockets don't have stats counters associated with them,
so it's now necessary to check whether socket stats exist before
incrementing or decrementing them. rather than relying on the caller
for this, we now just pass the socket and an index, and the correct
stats counter will be updated if it exists.
isc_nm_routeconnect() opens a route/netlink socket, then calls a
connect callback, much like isc_nm_udpconnect(), with a handle that
can then be monitored for network changes.
Internally the socket is treated as a UDP socket, since route/netlink
sockets follow the datagram contract.
After support for route/netlink sockets is merged, not all sockets
will have stats counters associated with them, so it's now necessary
to check whether socket stats exist before incrementing or decrementing
them. rather than relying on the caller for this, we now just pass the
socket and an index, and the correct stats counter will be updated if
it exists.
Update the 'catz' system test by adding tests that update an
catalog zone (catalog1.example) while preserving existing entries
(increase SOA serial) then check that catalog zone has transferred
and that the existing entries have not accidentally been removed
as a consequence (can return updated zone content).
After receiving a new version of a catalog zone it is required
to merge it with the old version.
The algorithm walks through the new version's hash table and applies
the following logic:
1. If an entry from the new version does not exist in the old
version, then it's a new entry, add the entry to the `toadd` hash
table.
2. If the zone does not exist in the set of configured zones, because
it was deleted via rndc delzone or it was removed from another
catalog zone instance, then add into to the `toadd` hash table to
be reinstantiated.
3. If an entry from the new version also exists in the old version,
but is modified, then add the entry to the `tomod` hash table, then
remove it from the old version's hash table.
4. If an entry from the new version also exists in the old version and
is the same (unmodified) then just remove it from the old version's
hash table.
The algorithm then deletes all the remaining zones which still exist
in the old version's hash table (because only the ones that don't
exist in the new version should now remain there), then adds the ones
that were added to the `toadd`, and modifies the ones that were added
to the `tomod`, completing the merge.
During a recent refactoring, the part when the entry should be
removed from the old version's hash table on condition (4.) above
was accidentally omitted, so the unmodified zones were remaining
in the old version's hash table and consequently being deleted.
The new rules compare the target name in PTR and SRV records against
the machine name embedded in the kerberos principal. This can be
used to further restrict what PTR and SRV records can be added or
deleted via dynamic updates if desired.
The librpz.h defined LIRPZ_LIKELY() and LIBRPZ_UNLIKELY() macros that
were actually unused in the code. Remove the macros and the autoconf
check for __builtin_expect().
The __builtin_expect() can be used to provide the compiler with branch
prediction information. The Gcc manual says[1] on the subject:
In general, you should prefer to use actual profile feedback for
this (-fprofile-arcs), as programmers are notoriously bad at
predicting how their programs actually perform.
Stop using __builtin_expect() and ISC_LIKELY() and ISC_UNLIKELY() macros
to provide the branch prediction information as the performance testing
shows that named performs better when the __builtin_expect() is not
being used.
1. https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fexpect
pytest was failing because it was testing features that had
not been configured. test to see if those features have been
configured before running the tests.
due to comparing logfile suffixes as 32 bit rather than 64 bit
integers, logfiles with timestamp suffixes that should have been
removed when rolling could be left in place. this has been fixed.
the logfileconfig system test did not conform to the style of
other tests, and was difficult to read and maintain. it has
been cleaned up and simplifeid in several ways:
- named.args used when appropriate so that named can be started with
specified command line arguments, instead of having it launched
directly from tests.sh
- unused root zone removed from named configuration
- an existing directory used instead of using 'mkdir' to create one
- dnssec-validation disabled to stop the server sending unnecessary queries
incidental fix: removed leftover debugging printfs from logconf.c.
This commit removes a superfluous call to isc_tlsctx_free() which was
leading to double free() error in a case of a TLS listener creation
failure.
The call is superfluous because the TLS context object is supposed to
be destroyed in ns_listenelt_destroy() only.
Unify the header guard style and replace the inconsistent include guards
with #pragma once.
The #pragma once is widely and very well supported in all compilers that
BIND 9 supports, and #pragma once was already in use in several new or
refactored headers.
Using simpler method will also allow us to automate header guard checks
as this is simpler to programatically check.
For reference, here are the reasons for the change taken from
Wikipedia[1]:
> In the C and C++ programming languages, #pragma once is a non-standard
> but widely supported preprocessor directive designed to cause the
> current source file to be included only once in a single compilation.
>
> Thus, #pragma once serves the same purpose as include guards, but with
> several advantages, including: less code, avoidance of name clashes,
> and sometimes improvement in compilation speed. On the other hand,
> #pragma once is not necessarily available in all compilers and its
> implementation is tricky and might not always be reliable.
1. https://en.wikipedia.org/wiki/Pragma_once
With isc_mem_get() and dns_name_dup() no longer being able to fail, some
functions can now only return ISC_R_SUCCESS. Change the return type to
void for the following function(s):
* dns_zone_setprimaries()
* dns_zone_setparentals()
* dns_zone_setparentals()
* dns_zone_setalsonotify()
With isc_mem_get() and dns_name_dup() no longer being able to fail, some
functions can now only return ISC_R_SUCCESS. Change the return type to
void for the following function(s):
* dns_view_adddelegationonly()
* dns_view_excludedelegationonly()
With isc_mem_get() and dns_name_dup() no longer being able to fail, some
functions can now only return ISC_R_SUCCESS. Change the return type to
void for the following function(s):
* dns_ssutable_addrule()
* dns_ssutable_create()
* dns_ssutable_createdlz()
With isc_mem_get() and dns_name_dup() no longer being able to fail, some
functions can now only return ISC_R_SUCCESS. Change the return type to
void for the following function(s):
* dns_resolver_addalternate()
With isc_mem_get() and dns_name_dup() no longer being able to fail, some
functions can now only return ISC_R_SUCCESS. Change the return type to
void for the following function(s):
* name_duporclone()
With isc_mem_get() and dns_name_dup() no longer being able to fail, some
functions can now only return ISC_R_SUCCESS. Change the return type to
void for the following function(s):
* build_event()
With isc_mem_get() and dns_name_dup() no longer being able to fail, some
functions can now only return ISC_R_SUCCESS. Change the return type to
void for the following function(s):
* dns_catz_options_copy()
* dns_catz_options_setdefault()
* dns_catz_entry_new()
* dns_catz_entry_copy()
POSIX.1-2008 changed the st_atim, st_mtim, and st_ctime members of the
struct stat from time_t to struct timespec and because not all operating
systems already implemented this version of the standard or historically
deviated to include own nanosecond precision in the structure.
The autoconf script used to include <sys/fcntl.h> which contradicts
POSIX.1 as it mandates <sys/stat.h> inclusion. Change the autoconf
check to include <sys/stat.h>.
Also fix the missing AC_MSG_RESULT([yes/no]) in the check.
Replace some "master/slave" terminology in the code with the preferred
"primary/secondary" keywords. This also changes user output such as
log messages, and fixes a typo ("seconary") in cfg_test.c.
There are still some references to "master" and "slave" for various
reasons:
- The old syntax can still be used as a synonym.
- The master syntax is kept when it refers to master files and formats.
- This commit replaces mainly keywords that are local. If "master" or
"slave" is used in for example a structure that is all over the
place, it is considered out of scope for the moment.
Replace most "master/slave" terminology in tests with the preferred
"primary/secondary", with the following exceptions:
- When testing the old syntax
- When master is used in master file and master file format terms
- When master is used in hostmaster or postmaster terms
- When master used in legacy domain names (for example in dig.batch)
- When there is no replacement (for example default-masters)
Originally, the hash table used in RBT database would be resized when it
reached certain number of elements (defined by overcommit). This was
causing resolution brownouts for busy resolvers, because the rehashing
could take several seconds to complete. This was mitigated by
pre-allocating the hash table in the RBT database used for caching to be
large-enough as determined by max-cache-size. The downside of this
solution was that the pre-allocated hash table could take a significant
chunk of the memory even when the resolver cache would be otherwise
empty because the default value for max-cache-size is 90% of available
memory.
Implement incremental resizing[1] to perform the rehashing gradually:
1. During the resize, allocate the new hash table, but keep the old
table unchanged.
2. In each lookup or delete operation, check both tables.
3. Perform insertion operations only in the new table.
4. At each insertion also move r elements from the old table to the new
table.
5. When all elements are removed from the old table, deallocate it.
To ensure that the old table is completely copied over before the new
table itself needs to be enlarged, it is necessary to increase the
size of the table by a factor of at least (r + 1)/r during resizing.
In our implementation r is equal to 1.
The downside of this approach is that the old table and the new table
could stay in memory for longer when there are no new insertions into
the hash table for prolonged periods of time as the incremental
rehashing happens only during the insertions.
The upside of this approach is that it's no longer necessary to
pre-allocate large hash table, because the RBT hash table rehashing
doesn't cause resolution brownouts anymore and thus we can use the
memory as needed.
1. https://en.m.wikipedia.org/wiki/Hash_table#Dynamic_resizing
The documentation and feature-test were using '--with-idn' but the
configure script doesn't recognize this option. The correct option to
enable IDN support is '--with-libidn2'.
Add test to encode unicode sequence that encodes differently with
UseSTD3ASCIIRules=false which is default with idn2 >= 2.0.3 and
UseSTD3ASCIIRules=true which is what should be used to encode hostnames
and domains.
libidn2 defaults to UseSTD3ASCIIRules=false. That allows arbitrary ASCII
characters to show up in the toASCII output, including space and
underscore. Enable IDN2_USE_STD3_ASCII_RULES to the libidn2 conversion
to disallow additional characters from the conversion (see Validity
Criteria[1]).
The AX_CHECK_JEMALLOC() m4 macro sets the JEMALLOC_CFLAGS variable, not
JEMALLOC_CPPFLAGS. Furthermore, the JEMALLOC_CFLAGS and JEMALLOC_LIBS
variables should only be included in the build flags if jemalloc was
successfully configured. Tweak lib/isc/Makefile.am accordingly.
Remove the dynamic registration of result codes. Convert isc_result_t
from unsigned + #defines into 32-bit enum type in grand unified
<isc/result.h> header. Keep the existing values of the result codes
even at the expense of the description and identifier tables being
unnecessary large.
Additionally, add couple of:
switch (result) {
[...]
default:
break;
}
statements where compiler now complains about missing enum values in the
switch statement.
Previously, when using compiler without support for static assertions,
the STATIC_ASSERT() macro would be replaced with runtime assertion.
Change the STATIC_ASSERT() macro to a version that's compile time
assertion even when using pre-C11 compilers.
Courtesy of Joseph Quinsey: https://godbolt.org/z/K9RvWS
Both <isccc/util.h> and <isc/util.h> defined DE_CONST() macro. As
<isccc/util.h> header includes <isc/util.h>, remove the macro from
<isccc/util.h> header.
There's value mismatch between the return type of dns_rrl() that's
dns_rrl_result_t and ISC_R_SUCCESS which belongs to isc_result_t. This
works incidentally, because DNS_RRL_RESULT_OK == ISC_R_SUCCESS.
This would break when we change isc_result_t to be static enum in
consecutive commit. Change the value to match the type.
Renamed some functions for clarity and readability:
- dns_dispatch_addresponse() -> dns_dispatch_add()
- dns_dispatch_removeresponse() -> dns_dispatch_done()
The dns_dispatch_cancel() function now calls dns_dispatch_done()
directly, so it is no longer ever necessary to call both functions.
dns_dispatch_cancel() is used to terminate dispatch connections
that are still pending, while dns_dispatch_done() is used when they
are complete.
The build would fail if the OpenSSL libraries were not in default
include path because we include <openssl/opensslv.h> header in
lib/bind9/check.c. Add $(OPENSSL_CFLAGS) to lib/bind9/Makefile.am.
This commit fixes a crash in DoT code when it was attempting to call a
read callback on the later stages of the connection when it is not
available.
It also fixes [GL #2884] (back-trace provided in the bug report is
exactly the same as was seen when fixing this problem).
This commit makes dig fail with error in case a zone transfer is
attempted over a connections where ALPN was not negotiated. All other
request types will work fine.
This commit make the code handling incoming zone transfers to verify
if they are allowed to be done over the underlying connections. As a
result the check ensures that the "dot" ALPN token has been negotiated
over the underlying connection.
This commit makes BIND verify that zone transfers are allowed to be
done over the underlying connection. Currently, it makes sense only
for DoT, but the code is deliberately made to be protocol-agnostic.
The intention of having this function is to have a predicate to check
if a zone transfer could be performed over the given handle. In most
cases we can assume that we can do zone transfers over any stream
transport except DoH, but this assumption will not work for zone
transfers over DoT (XoT), as the RFC9103 requires ALPN to happen,
which might not be the case for all deployments of DoT.
The 'listenlist_test', 'notify_test', and 'query_test' tests failed
when the descriptor limit was 256 on MacOS 11.6 with 8 cpus. On the
test platform the limit needed to be increased to ~400. Increase
the limit to at least 1024 to give some head room.
as libdns is no longer exported, it's not necessary to have
init and shutdown functions. the only purpose they served
was to create a private mctx and run dst_lib_init(), which
can be called directly instead.
- serve-stale: dig wasn't always running in background when it should.
some of the serve-stale test cases are based on groups of dig calls
running simultaneously in the background: the test pauses and resumes
running after 'wait'. in some cases the final call to dig in a group
wasn't in the background, and this sometimes caused delays that
affected later test results. in another case, a test was simplified
and made more reliable by running dig in the foreground removing a
sleep.
- serve-stale: The extension of the dig timeout period from 10 to 11
seconds in commit 5307bf64ce was left undone in a few places and has
now been completed.
- serve-stale: Resolver-query-timeout was set incorrectly. a comment
above a test case in serve-stale/tests.sh says: "We configured a long
value of 30 seconds for resolver-query-timeout," but
resolver-query-timeout was actually set to 10, not 30. this is now
fixed.
- rpz: Force retransfer of the fast-expire zone, to ensure it's fully
loaded in ns3; previously it could have been left unloaded if ns5
wasn't up yet when ns3 attempted the zone transfer.
- statistics: The TCP4SendErr counter is incremented when a TCP dispatch
is canceled while sending. depending on test timing, this may have
happened by the time the statistics are dumped. worked around by
ignoring that stat couunter when checking for errors.
- hooks: Add a prereq.sh script to prevent running under TSAN.
- zero: Disabled the servfail cache so that SERVFAIL is reported only
when there actually is a failure, not repeatedly every time the same
query is sent.
- Prevent shutdown races: attach/detach to dns_resolver in dns_fetch_t
and fctx_t; delay destruction of fctx when finds are still active;
reference the fctx while canceling; reverse the order of
fctx_destroy() and empty_bucket().
- Don't resend queries if fetches have been canceled.
- It's possible for fctx_doshutdown() to run before a TCP connection has
completed. if the query is not on the queries list, then it is not
canceled, but the adbaddrinfo is freed. when tcp_connected() runs
later, the query is in an inconstent state. to fix this, we add the
query to queries before running dns_dispatch_connect(), instead of in
the connect callback.
- Combined the five fctx_cleanup* functions into a single one.
- Added comments and changed some names to make this code easier to
understand.
udp_recv() will call dispatch_getnext() if the message received is
invalid or doesn't match; we need to reduce the timeout each time this
happens so we can't be starved forever by someone sending garbage
packets.
- disp_connected() has been split into two functions,
udp_connected() (which takes 'resp' as an argument) and
tcp_connected() (which takes 'disp', and calls the connect callbacks
for all pending resps).
- In dns_dispatch_connect(), if a connection is already open, we need to
detach the dispentry immediately because we won't be running
tcp_connected().
- dns_disptach_cancel() also now calls the connect callbacks for pending
TCP responses, and the response callbacks for open TCP connections
waiting on read.
- If udp_connected() runs after dns_dispatch_cancel() has been called,
ensure that the caller's connect callback is run.
- If a UDP connection fails with EADDRINUSE, we try again up to five
times with a different local port number before giving up.
- If a TCP connection is canceled while still pending connection, the
connect timeout may still fire. we attach the dispatch before
connecting to ensure that it won't be detached too soon in this case.
- The dispentry is no longer removed from the pending list when
deactivating, so that the connect callback can still be run if
dns_dispatch_removeresponse() was run while the connecting was
pending.
- Rewrote dns_dispatch_gettcp() to avoid a data race.
- startrecv() and dispatch_getnext() can be called with a NULL resp when
using TCP.
- Refactored udp_recv() and tcp_recv() and added result logging.
- EOF is now treated the same as CANCELED in response callbacks.
- ISC_R_SHUTTINGDOWN is sent to the reponse callbacks for all resps if
tcp_recv() is triggered by a netmgr shutdown. (response callbacks
are *not* sent by udp_recv() in this case.)
- startrecv() and getnext() have been rewritten.
- Don't set TCP flag when connecting a UDP dispatch.
- Prevent TCP connections from trying to connect twice.
- dns_dispatch_gettcp() can now find a matching TCP dispatch that has
not yet fully connected, and attach to it. when the connection is
completed, the connect callbacks are run for all of the pending
entries.
- An atomic 'state' variable is now used for connection state instead of
attributes.
- When dns_dispatch_cancel() is called on a TCP dispatch entry, only
that one entry is canceled. the dispatch itself should not be shut
down until there are no dispatch entries left associated with it.
- Other incidental cleanup, including removing DNS_DISPATCHATTR_IPV4 and
_IPV6 (they were being set in the dispatch attributes but never used),
cleaning up dns_requestmgr_create(), and renaming dns_dispatch_read()
to the more descriptive dns_dispatch_resume().
- It is no longer necessary to pass a 'timeout' callback to
dns_dispatch_addresponse(); timeouts are handled directly by the
'response' callback instead.
- The netmgr handle is no longer passed to dispatch callbacks, since
they don't (and can't) use it. instead, dispatch_cb_t now takes a
result code, region, and argument.
- Cleaned up timeout-related tests in dispatch_test.c
- Responses received by the dispatch are no longer sent to the caller
via a task event, but via a netmgr-style recv callback. the 'action'
parameter to dns_dispatch_addresponse() is now called 'response' and
is called directly from udp_recv() or tcp_recv() when a valid response
has been received.
- All references to isc_task and isc_taskmgr have been removed from
dispatch functions.
- All references to dns_dispatchevent_t have been removed and the type
has been deleted.
- Added a task to the resolver response context, to be used for fctx
events.
- When the caller cancels an operation, the response handler will be
called with ISC_R_CANCELED; it can abort immediately since the caller
will presumably have taken care of cleanup already.
- Cleaned up attach/detach in resquery and request.
Remove the debugging printfs. (leaving this as a separate commit rather
than squashing it so we can restore it in the future if we ever need it
again.)
Since every dispsock was associated with a dispentry anyway (though not
always vice versa), the members of dispsock have been combined into
dispentry, which is now reference-counted. dispentry objects are now
attached before connecting and detached afterward to prevent races
between the connect callback and dns_dispatch_removeresponse().
Dispatch and dispatchmgr objects are now reference counted as well, and
the shutdown process has been simplified. reference counting of
resquery and request objects has also been cleaned up significantly.
dns_dispatch_cancel() now flags a dispentry as having been canceled, so
that if the connect callback runs after cancellation, it will not
initiate a read.
The isblackholed() function has been simplified.
- The `timeout_action` parameter to dns_dispatch_addresponse() been
replaced with a netmgr callback that is called when a dispatch read
times out. this callback may optionally reset the read timer and
resume reading.
- Added a function to convert isc_interval to milliseconds; this is used
to translate fctx->interval into a value that can be passed to
dns_dispatch_addresponse() as the timeout.
- Note that netmgr timeouts are accurate to the millisecond, so code to
check whether a timeout has been reached cannot rely on microsecond
accuracy.
- If serve-stale is configured, then a timeout received by the resolver
may trigger it to return stale data, and then resume waiting for the
read timeout. this is no longer based on a separate stale timer.
- The code for canceling requests in request.c has been altered so that
it can run asynchronously.
- TCP timeout events apply to the dispatch, which may be shared by
multiple queries. since in the event of a timeout we have no query ID
to use to identify the resp we wanted, we now just send the timeout to
the oldest query that was pending.
- There was some additional refactoring in the resolver: combining
fctx_join() and fctx_try_events() into one function to reduce code
duplication, and using fixednames in fetchctx and fetchevent.
- Incidental fix: new_adbaddrinfo() can't return NULL anymore, so the
code can be simplified.
The flow of operations in dispatch is changing and will now be similar
for both UDP and TCP queries:
1) Call dns_dispatch_addresponse() to assign a query ID and register
that we'll be listening for a response with that ID soon. the
parameters for this function include callback functions to inform the
caller when the socket is connected and when the message has been
sent, as well as a task action that will be sent when the response
arrives. (later this could become a netmgr callback, but at this
stage to minimize disruption to the calling code, we continue to use
isc_task for the response event.) on successful completion of this
function, a dispatch entry object will be instantiated.
2) Call dns_dispatch_connect() on the dispatch entry. this runs
isc_nm_udpconnect() or isc_nm_tcpdnsconnect(), as needed, and begins
listening for responses. the caller is informed via a callback
function when the connection is established.
3) Call dns_dispatch_send() on the dispatch entry. this runs
isc_nm_send() to send a request.
4) Call dns_dispatch_removeresponse() to terminate listening and close
the connection.
Implementation comments below:
- As we will be using netmgr buffers now. code to send the length in
TCP queries has also been removed as that is handled by the netmgr.
- TCP dispatches can be used by multiple simultaneous queries, so
dns_dispatch_connect() now checks whether the dispatch is already
connected before calling isc_nm_tcpdnsconnect() again.
- Running dns_dispatch_getnext() from a non-network thread caused a
crash due to assertions in the netmgr read functions that appear to be
unnecessary now. the assertions have been removed.
- fctx->nqueries was formerly incremented when the connection was
successful, but is now incremented when the query is started and
decremented if the connection fails.
- It's no longer necessary for each dispatch to have a pool of tasks, so
there's now a single task per dispatch.
- Dispatch code to avoid UDP ports already in use has been removed.
- dns_resolver and dns_request have been modified to use netmgr callback
functions instead of task events. some additional changes were needed
to handle shutdown processing correctly.
- Timeout processing is not yet fully converted to use netmgr timeouts.
- Fixed a lock order cycle reported by TSAN (view -> zone-> adb -> view)
by by calling dns_zt functions without holding the view lock.
- The read timer must always be stopped when reading stops.
- Read callbacks can now call isc_nm_read() again in TCP, TCPDNS and
TLSDNS; previously this caused an assertion.
- The wrong failure code could be sent after a UDP recv failure because
the if statements were in the wrong order. the check for a NULL
address needs to be after the check for an error code, otherwise the
result will always be set to ISC_R_EOF.
- When aborting a read or connect because the netmgr is shutting down,
use ISC_R_SHUTTINGDOWN. (ISC_R_CANCELED is now reserved for when the
read has been canceled by the caller.)
- A new function isc_nmhandle_timer_running() has been added enabling a
callback to check whether the timer has been reset after processing a
timeout.
- Incidental netmgr fix: always use isc__nm_closing() instead of
referencing sock->mgr->closing directly
- Corrected a few comments that used outdated function names.
Previously isc_nm_read() required references on the handle to be at
least 2, under the assumption that it would only ever be called from a
connect or accept callback. however, it can also be called from a read
callback, in which case the reference count might be only 1.
We now use dns_dispatch_cancel() for this purpose. NOTE: The caller
still has to track whether there are pending send or connect events in
the dispatch or dispatch entry; later this should be moved into the
dispatch module as well.
Also removed some public dns_dispatch_*() API calls that are no longer
used outside dispatch itself.
dns_dispatch_connect() connects a dispatch socket (for TCP) or a
dispatch entry socket (for UDP). This is the next step in moving all
uses of the isc_socket code into the dispatch module.
This API is temporary; it needs to be cleaned up further so that it can
be called the same way for both TCP and UDP.
Continuing the effort to move all uses of the isc_socket API into
dispatch.c, this commit removes the dns_tcpmsg module entirely, as
dispatch was its only caller, and moves the parts of its functionality
that were being used into the dispatch module.
This code will be removed when we switch to using netmgr TCPDNS.
Previously, creation of TCP dispatches differed from UDP in that a TCP
dispatch was created to attach to an existing socket, whereas a UDP
dispatch would be created in a vacuum and sockets would be opened on
demand when a transaction was initiated.
We are moving as much socket code as possible into the dispatch module,
so that it can be replaced with a netmgr version as easily as
possible. (This will also have the side effect of making TCP and UDP
dispatches more similar.)
As a step in that direction, this commit changes
dns_dispatch_createtcp() so that it creates the TCP socket.
- Many dispatch attributes can be set implicitly instead of being passed
in. we can infer whether to set DNS_DISPATCHATTR_TCP or _UDP from
whether we're calling dns_dispatch_createtcp() or _createudp(). we
can also infer DNS_DISPATCHATTR_IPV4 or _IPV6 from the addresses or
the socket that were passed in.
- We no longer use dup'd sockets in UDP dispatches, so the 'dup_socket'
parameter has been removed from dns_dispatch_createudp(), along with
the code implementing it. also removed isc_socket_dup() since it no
longer has any callers.
- The 'buffersize' parameter was ignored and has now been removed;
buffersize is now fixed at 4096.
- Maxbuffers and maxrequests don't need to be passed in on every call to
dns_dispatch_createtcp() and _createudp().
In all current uses, the value for mgr->maxbuffers will either be
raised once from its default of 20000 to 32768, or else left
alone. (passing in a value lower than 20000 does not lower it.) there
isn't enough difference between these values for there to be any need
to configure this.
The value for disp->maxrequests controls both the quota of concurrent
requests for a dispatch and also the size of the dispatch socket
memory pool. it's not clear that this quota is necessary at all. the
memory pool size currently starts at 32768, but is sometimes lowered
to 4096, which is definitely unnecessary.
This commit sets both values permanently to 32768.
- Previously TCP dispatches allocated their own separate QID table,
which didn't incorporate a port table. this commit removes
per-dispatch QID tables and shares the same table between all
dispatches. since dispatches are created for each TCP socket, this may
speed up the dispatch allocation process. there may be a slight
increase in lock contention since all dispatches are sharing a single
QID table, but since TCP sockets are used less often than UDP
sockets (which were already sharing a QID table), it should not be a
substantial change.
- The dispatch port table was being used to determine whether a port was
already in use; if so, then a UDP socket would be bound with
REUSEADDR. this commit removes the port table, and always binds UDP
sockets that way.
Currently the netmgr doesn't support unconnected, shared UDP sockets, so
there's no reason to retain that functionality in the dispatcher prior
to porting to the netmgr.
In this commit, the DNS_DISPATCHATTR_EXCLUSIVE attribute has been
removed as it is now non-optional; UDP dispatches are alwasy exclusive.
Code implementing non-exclusive UDP dispatches has been removed.
dns_dispatch_getentrysocket() now always returns the dispsocket for UDP
dispatches and the dispatch socket for TCP dispatches.
There is no longer any need to search for existing dispatches from
dns_dispatch_getudp(), so the 'mask' option has been removed, and the
function renamed to the more descriptive dns_dispatch_createudp().
- style cleanup
- removed NULL checks in places where they are not currently needed
- use isc_refcount for dispatch reference counting
- revised code flow for readability
- remove some #ifdefs that are no longer relevant
- remove unused struct members
- removed unnecessary function parameters
- use C99 struct initialization
The DNS_REQUESTOPT_SHARE flag was added when client-side pipelining of
TCP queries was implemented. there was no need to make it optional;
forcing it to be in effect for all requests simplfiies the code.
- UDP buffersize is now established when creating dispatch manager
and is always set to 4096.
- Set up the default port range in dispatchmgr before setting the magic
number.
- Magic is not set until dispatchmgr is fully created.
- DNS_DISPATCHATTR_CANREUSE was never set. the code that implements it
has been removed.
- DNS_DISPATCHOPT_FIXEDID and DNS_DISPATCHATTR_FIXEDID were both
defined, but only the DISPATCHOPT was ever set; it appears the
DISPATCHATTR was added accidentally.
- DNS_DISPATCHATTR_NOLISTEN was set but never used.
Resolve#2795, #2796: implement TLS configuration options to make it possible to specify supported TLS versions and implement perfect forward secrecy for DoH and DoT
Closes#2796 and #2795
See merge request isc-projects/bind9!5444
We have to mention that every option within a "tls" clause has
defaults out of our control as some platforms have means for defining
encryption policies globally for any application on the system.
In order to comply with these policies, we have not to modify TLS
contexts settings, unless we have to do so according to the options
specified within "tls" clauses.
This commit adds the ability to enable or disable stateless TLS
session resumption tickets (see RFC5077). Having this ability is
twofold.
Firstly, these tickets are encrypted by the server, and the algorithm
might be weaker than the algorithm negotiated during the TLS session
establishment (it is in general the case for TLSv1.2, but the generic
principle applies to TLSv1.3 as well, despite it having better ciphers
for session tickets). Thus, they might compromise Perfect Forward
Secrecy.
Secondly, disabling it might be necessary if the same TLS key/cert
pair is supposed to be used by multiple servers to achieve, e.g., load
balancing because the session ticket by default gets generated in
runtime, while to achieve successful session resumption ability, in
this case, would have required using a shared key.
The proper alternative to having the ability to disable stateless TLS
session resumption tickets is to implement a proper session tickets
key rollover mechanism so that key rotation might be performed
often (e.g. once an hour) to not compromise forward secrecy while
retaining the associated performance benefits. That is much more work,
though. On the other hand, having the ability to disable session
tickets allows having a deployable configuration right now in the
cases when either forward secrecy is wanted or sharing the TLS
key/cert pair between multiple servers is needed (or both).
This commit adds support for enforcing the preference of server
ciphers over the client ones. This way, the server attains control
over the ciphers priority and, thus, can choose more strong cyphers
when a client prioritises less strong ciphers over the more strong
ones, which is beneficial when trying to achieve Perfect Forward
Secrecy.
This commit adds support for setting TLS cipher list string in the
format specified in the OpenSSL
documentation (https://www.openssl.org/docs/man1.1.1/man1/ciphers.html).
The syntax of the cipher list is verified so that specifying the wrong
string will prevent the configuration from being loaded.
This commit adds support for loading DH-parameters (Diffie-Hellman
parameters) via the new "dhparam-file" option within "tls" clause. In
particular, Diffie-Hellman parameters are needed to enable the range
of forward-secrecy enabled cyphers for TLSv1.2, which are getting
silently disabled otherwise.
This commit adds the ability to specify allowed TLS protocols versions
within the "tls" clause. If an unsupported TLS protocol version is
specified in a file, the configuration file will not pass
verification.
Also, this commit adds strict checks for "tls" clauses verification,
in particular:
- it ensures that loading configuration files containing duplicated
"tls" clauses is not allowed;
- it ensures that loading configuration files containing "tls" clauses
missing "cert-file" or "key-file" is not allowed;
- it ensures that loading configuration files containing "tls" clauses
named as "ephemeral" or "none" is not allowed.
Previously a missing/deleted zone which was referenced by a catalog
zone was causing a crash when doing a reload.
This commit will make `named` to ignore the fact that the zone is
missing, and make sure to restore it later on.
It was discovered that named could crash due to a segmentation fault
when jemalloc was in use and memory allocation failed. This was not
intended to happen as jemalloc's "xmalloc" option was set to "true" in
the "malloc_conf" configuration variable. However, that variable was
only set after jemalloc was already done with parsing it, which
effectively caused setting that variable to have no effect.
While investigating this issue, it was also discovered that enabling the
"xmalloc" option makes jemalloc use a slow processing path, decreasing
its performance by about 25%. [1]
Additionally, further testing (carried out after fixing the way
"malloc_conf" was set) revealed that the non-default configuration
options do not have any measurable effect on either authoritative or
recursive DNS server performance.
Replace code setting various jemalloc options to non-default values with
assertion checks of mallocx()/rallocx() return values.
[1] https://github.com/jemalloc/jemalloc/pull/523
This commit fixes heap use after free when checking BIND's
configuration files for errors with http clauses. The old code
was unnecessarially copying the http element name and freeing
it to early. The name is now used directly.
zone.c:integrity_checks() acquires a read lock while iterating the
zone database, and calls zone_check_mx() which acquires another
read lock. If another thread tries to acquire a write lock in the
meantime, it can deadlock. Calling dns_dbiterator_pause() to release
the first read lock prevents this.
check for type "master" / "slave" at the same time as checking
for "primary" / "secondary" as we step through the maps.
Checking "primary" then "master" or "master" then "primary" does
not work as the synomym is not checked for to stop the search.
Similarly with "secondary" and "slave".
On TCPDNS/TLSDNS read callback, the socket buffer could be reallocated
if the received contents would be larger than the buffer. The existing
code would not preserve the contents of the existing buffer which lead
to the loss of the already received data.
This commit changes the isc_mem_put()+isc_mem_get() with isc_mem_reget()
to preserve the existing contents of the socket buffer.
The netmgr, has an internal cache for freed active handles. This cache
was allocated using isc_mem_allocate()/isc_mem_free() API because it was
simpler to reallocate the cache when we needed to grow it. The new
isc_mem_reget() function could be used here reducing the need to use
isc_mem_allocate() API which is tad bit slower than isc_mem_get() API.
Previously, we cannot use isc_mem_reallocate() for growing the buffer
dynamically, because the memory was allocated using the
isc_mem_get()/isc_mem_put() API. With the introduction of the
isc_mem_reget() function, we can use grow/shrink the memory directly
without always moving the memory around as the allocator might have
reserved some extra space after the initial allocation.
Previously, the zero-sized allocations would return NULL pointer and the
caller had to make sure to not dereference such pointer. The C standard
defines the zero-sized calls to malloc() as implementation specific and
jemalloc mallocx() with zero size would be undefined behaviour. This
complicated the code as it had to handle such cases in a special manner
in all allocator and deallocator functions.
Now, for realloc(), the situation is even more complicated. In C
standard up to C11, the behavior would be implementation defined, and
actually some implementation would free to orig ptr and some would not.
Since C17 (via DR400) would deprecate such usage and since C23, the
behaviour would be undefined.
This commits changes helper mem_get(), mem_put() and mem_realloc()
functions to grow the zero-allocation from 0 to sizeof(void *).
This way we get a predicable behaviour that all the allocations will
always return valid pointer.
The isc_mem_get() and isc_mem_put() functions are leaving the memory
allocation size tracking to the users of the API, while
isc_mem_allocate() and isc_mem_free() would track the sizes internally.
This allowed to have isc_mem_rellocate() to manipulate the memory
allocations by the later set, but not the former set of the functions.
This commit introduces isc_mem_reget(ctx, old_ptr, old_size, new_size)
function that operates on the memory allocations with external size
tracking completing the API.
Previously, the Makefiles for mysql and mysqldyn DLZ modules were
generated from autoconf to get CFLAGS and LIBS for MariaDB or MySQL
libraries. The static Makefiles uses a simpler method by calling
`mysql_config` directly from the Makefile.
The old-style DLZ drivers were already marked as no longer actively
maintained and expected to be removed eventually. With the new automake
build system, the old-style DLZ drivers were not updated, and instead of
putting an effort into something that's not being maintained, let's
rather remove the unmaintained code.
Closes: #2814
The map masterfile-format is very fragile and it needs API bump every
time a RBTDB data structures changes. Also while testing it, we found
out that files larger than 2GB weren't loading and nobody noticed, and
loading many map files were also failing (subject to kernel limits).
Thus we are marking the masterfile-format type 'map' as deprecated and
to be removed in the next stable BIND 9 release.
The Debian 10 (buster) Docker image, which GitLab CI uses for building
documentation, currently contains the following package versions:
- Sphinx 4.2.0
- sphinx-rtd-theme 1.0.0
- docutils 0.17.1
Regenerate the man pages to match contents produced in a Sphinx
environment using the above package versions. This is necessary to
prevent the "docs" GitLab CI job from failing.
"cache-file" was already documented as intended for testing
purposes only and not to be used, so we can remove it without
waiting. this commit marks the option as "ancient", and
removes all the documentation and implementing code, including
dns_cache_setfilename() and dns_cache_dump().
it also removes the documentation for the '-x cachefile`
parameter to named, which had already been removed, but the man
page was not updated at the time.
Address the following warnings reported by PyLint 2.10.2:
************* Module conf
doc/arm/conf.py:90:10: W1406: The u prefix for strings is no longer necessary in Python >=3.0 (redundant-u-string-prefix)
doc/arm/conf.py:92:12: W1406: The u prefix for strings is no longer necessary in Python >=3.0 (redundant-u-string-prefix)
doc/arm/conf.py:93:9: W1406: The u prefix for strings is no longer necessary in Python >=3.0 (redundant-u-string-prefix)
doc/arm/conf.py:143:31: W1406: The u prefix for strings is no longer necessary in Python >=3.0 (redundant-u-string-prefix)
doc/man/conf.py:33:10: W1406: The u prefix for strings is no longer necessary in Python >=3.0 (redundant-u-string-prefix)
doc/man/conf.py:38:12: W1406: The u prefix for strings is no longer necessary in Python >=3.0 (redundant-u-string-prefix)
doc/man/conf.py:39:9: W1406: The u prefix for strings is no longer necessary in Python >=3.0 (redundant-u-string-prefix)
Address the following warnings reported by PyLint 2.10.2:
************* Module tests-checkds
bin/tests/system/checkds/tests-checkds.py:70:9: W1514: Using open without explicitly specifying an encoding (unspecified-encoding)
bin/tests/system/checkds/tests-checkds.py:120:13: W1514: Using open without explicitly specifying an encoding (unspecified-encoding)
bin/tests/system/checkds/tests-checkds.py:206:17: W1514: Using open without explicitly specifying an encoding (unspecified-encoding)
************* Module yamlget
bin/tests/system/digdelv/yamlget.py:22:5: W1514: Using open without explicitly specifying an encoding (unspecified-encoding)
************* Module stress_http_quota
bin/tests/system/doth/stress_http_quota.py:131:13: W1514: Using open without explicitly specifying an encoding (unspecified-encoding)
************* Module tests-rpz-passthru-logging
bin/tests/system/rpzextra/tests-rpz-passthru-logging.py:40:9: W1514: Using open without explicitly specifying an encoding (unspecified-encoding)
bin/tests/system/rpzextra/tests-rpz-passthru-logging.py:44:9: W1514: Using open without explicitly specifying an encoding (unspecified-encoding)
Add an item to the release checklist to make sure regression tests
reproducing publicly disclosed security issues are eventually merged
into each maintained branch.
Discourage the single source port on general level and document that the
source port cannot be same as the listening port. This applies to
query-source, transfer-source, notify-source, parental-source, and their
respective IPv6 counterparts.
- when transfer-source(-v6), query-source(-v6), notify-source(-v6)
or parental-source(-v6) are specified with a port number, issue a
warning.
- when the port specified is the same as the DNS listener port (i.e.,
53, or whatever was specified as "port" in "options"), issue a fatal
error.
- check that "port" is in range. (previously this was only checked
by named, not by named-checkconf.)
- added checkconf tests.
- incidental fix: removed dead code in check.c:bind9_check_namedconf().
(note: if the DNS port is specified on the command line with "named -p",
that is not conveyed to libbind9, so these checks will not take it into
account.)
The ns3->ns2 forwarding is now done using the IPv6 addresses, so we also
test that the query-source-v6 address is still operational after removal
of interface adjustment.
Previously, named would run with a configuration
where *-source-v6 (notify-source-v6, transfer-source-v6 and
query-source-v6) address and port could be simultaneously used for
listening. This is no longer true for BIND 9.16+ and the code that
would do interface adjustments would unexpectedly disable listening on
TCP for such interfaces.
This commit removes the code that would adjust listening interfaces
for addresses/ports configured in *-source-v6 option.
Until we have a system test that would directly test the engine_pkcs11
integration, we need to disable the system tests that enabled native
PKCS#11 in the CI because it's currently broken.
The native PKCS#11 support has been removed in favour of better
maintained, more performance and easier to use OpenSSL PKCS#11 engine
from the OpenSC project.
when "checking lame server clients are dropped below the hard limit",
periodically a query is sent for a name for which the server is
authoritative, to verify that legitimate queries can still be
processed while the server is dealing with a flood of lame delegation
queries. those queries used the same dig options as elsewhere in the
fetchlimit test, including "+tries=1 +timeout=1". on slow systems, a
1-second timeout may be insufficient to get an answer even if the server
is behaving well. this commit increases the timeout for the check
queries to 2 seconds in hopes that will be enough to eliminate test
failures in CI.
Document that the interval on new RRSIG records is randomally
chosen between the limits specified by sig-validity-interval.
document the operatations when this occurs.
- fixed a size comparison using "signed int" that failed if the file
size was more than 2GB, since that was treated as a negative number.
- incidentally renamed deserialize32() to just deserialize(). we no
longer have separate 32 and 64 bit rbtdb implementations.
Dependencies regression: Re-enable some common TLS-related options for non-DoH builds, making DoT usable in them
See merge request isc-projects/bind9!5377
This commit fixes a regression introduced at
ea80bcc41c. Some options, which are
common to both DoH and DoT were mistakenly disabled for non-DoH
builds. That is a mistake, because DoH does not imply DoT and vice
versa. Not fixing this would make DoT functionality not accessible
without DoH.
This commit modifies the MTU of the loopback interface on
Linux systems to 1500, so that oversized UDP packets can
trigger EMSGSIZE errors, and tests that named handles
such errors correctly.
Note that the loopback MTU size has not yet been modified
for other platforms.
This commit ensures that DoH (and DoT) functionality works well via
IPv6 as well.
The changes were made because it turned out that dig could not make
DoH queries against an IPv6 IP address. These tests ensure that such a
bug will not remain unnoticed.
The commit also increases the servers' startup timeout to 25 seconds
because the initial timeout of 14 seconds was too short to generate
(!) eight 4096 bit ephemeral RSA certificates on a heavily loaded CI
runner in some pipeline runs.
This commit replaces ad-hoc code for DoH connect URI construction with
isc_nm_http_makeuri(), making it handle IPv6 adresses properly (among
other things).
This commit adds new function isc_nm_http_makeuri() which is supposed
to unify DoH URI construction throughout the codebase.
It handles IPv6 addresses, hostnames, and IPv6 addresses given as
hostnames properly, and replaces similar ad-hoc code in the codebase.
- removed unused functions
- changed some public functions to static that are never called
from outside client.c
- removed unused types and function prototypes
- renamed dns_client_destroy() to dns_client_detach()
The previous versions of BIND 9 exported its internal libraries so that
they can be used by third-party applications more easily. Certain
library functions were altered from specific BIND-only behavior to more
generic behavior when used by other applications.
This commit removes the function isc_lib_register() that was used by
external applications to enable the functionality.
bump the map zonefile version number to avoid an assertion
failure when loading map files from versions of BIND prior to
the most recent change to the in-memory structure of zone
databases.
test server now has tcp-idle-timeout set to 5 seconds and
tcp-keepalive-timeout set to 7, so queries that follow a 6-second sleep
should either succeed or fail depending on whether the keepalive option
was sent.
this commit removes isc__nm_tcpdns_keepalive() and
isc__nm_tlsdns_keepalive(); keepalive for these protocols and
for TCP will now be set directly from isc_nmhandle_keepalive().
protocols that have an underlying TCP socket (i.e., TLS stream
and HTTP), now have protocol-specific routines, called by
isc_nmhandle_keeaplive(), to set the keepalive value on the
underlying socket.
previously, receiving a keepalive option had no effect on how
long named would keep the connection open; there was a place to
configure the keepalive timeout but it was never used. this commit
corrects that.
this also fixes an error in isc__nm_{tcp,tls}dns_keepalive()
in which the sense of a REQUIRE test was reversed; previously this
error had not been noticed because the functions were not being
used.
- fix some duplicated and out-of-order prototypes declared in
netmgr-int.h
- rename isc_nm_tcpdns_keepalive to isc__nm_tcpdns_keepalive as
it's for internal use
The removed function 'newchain(a, b)' was almost the same as calling
!chain_equal(a, b), varying only in the amount of data compared
in the non-fixed-length data portion of given chain nodes.
A third argument 'data_size' has been introduced into 'chain_equal'
function in order to allow it to know how many bytes to compare in the
variable-length data portion of the chain nodes.
A helper function 'chain_length(e)' has been introduced to allow
easy calculation of the total length of the non-fixed-length data part
of chain nodes.
Check the thread below for more details:
https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/291#note_12184
This commit changes the DoH code in such a way that it makes no
assumptions regarding which headers are expected to be processed
first. In particular, the code expected the :method: pseudo-header to
be processed early, which might not be true.
Add a statschannel test case to confirm that when keys are removed
(in this case because of a dnssec-policy change), the corresponding
dnssec-sign stats are cleared and are no longer shown in the
statistics.
Clear the key slots for dnssec-sign statistics for keys that are
removed. This way, the number of slots will stabilize to the maximum
key usage in a zone and will not grow every time a key rollover is
triggered.
Add a test case that has more than four keys (the initial number of
key slots that are created for dnssec-sign statistics). We shouldn't
be expecting weird values.
This fixes some errors in the manykeys zone configuration (keys
were created for algorithm RSASHA256, but the policy expected RSASHA1,
and the zone was not allowing dynamic updates).
This also fixes an error in the calls to 'zones-json.pl': The perl
script excepts an index number where the zone can be found, rather
than the zone name.
We have introduced dnssec-sign statistics to the zone statistics. This
introduced an operational issue because when using zone-statistics
full, the memory usage was going through the roof. We fixed this by
by allocating just four key slots per zone. If a zone exceeds the
number of keys for example through a key rollover, the keys will be
rotated out on a FIFO basis.
This works for most cases, and fixes the immediate problem of high
memory usage, but if you sign your zone with many, many keys, or are
sign with a ZSK/KSK double algorithm strategy you may experience weird
statistics. A better strategy is to grow the number of key slots per
zone on key rollover events.
That is what this commit is doing: instead of rotating the four slots
to track sign statistics, named now grows the number of key slots
during a key rollover (or via some other method that introduces new
keys).
Add a new function to resize the number of counters in a statistics
counter structure. This will be needed when we keep track of DNSSEC
sign statistics and new keys are introduced due to a rollover.
Add a simple stats unit test that tests the existing library functions
isc_stats_ncounters, isc_stats_increment, isc_stats_decrement,
isc_stats_set, and isc_stats_update_if_greater.
After a reload, if the zone hasn't changed, this will log a
DNS_R_UNCHANGED error. This should not be at error level because it
happens on every reload.
Add a test case for migrating CSK to dnssec-policy. The keymgr has no
way of telling that the key is used as a CSK, but if there is only one
key to migrate it is going to assume it must be a CSK.
Previously, when dnssec-cds copied CDS records to make DS records,
its -a algorithm option did not have any effect. This means that if
the child zone is signed with older software that generates SHA-1 CDS
records, dnssec-cds would (by default) create SHA-1 DS records in
violation of RFC 8624.
This change makes the dnssec-cds -a option apply to CDS records as
well as CDNSKEY records. In the CDS case, the -a algorithms are the
acceptable subset of possible CDS algorithms. If none of the CDS
records are acceptable, dnssec-cds tries to generate DS records from
CDNSKEY records.
Instead of disabling the fragmentation on the UDP sockets, we now
disable the Path MTU Discovery by setting IP(V6)_MTU_DISCOVER socket
option to IP_PMTUDISC_OMIT on Linux and disabling IP(V6)_DONTFRAG socket
option on FreeBSD. This option sets DF=0 in the IP header and also
ignores the Path MTU Discovery.
As additional mitigation on Linux, we recommend setting
net.ipv4.ip_no_pmtu_disc to Mode 3:
Mode 3 is a hardend pmtu discover mode. The kernel will only accept
fragmentation-needed errors if the underlying protocol can verify
them besides a plain socket lookup. Current protocols for which pmtu
events will be honored are TCP, SCTP and DCCP as they verify
e.g. the sequence number or the association. This mode should not be
enabled globally but is only intended to secure e.g. name servers in
namespaces where TCP path mtu must still work but path MTU
information of other protocols should be discarded. If enabled
globally this mode could break other protocols.
The client->rcode_override was originally created to force the server
to send SERVFAIL in some cases when it would normally have sent FORMERR.
More recently, it was used in a3ba95116e
commit (part of GL #2790) to force the sending of a TC=1 NOERROR
response, triggering a retry via TCP, when a UDP packet could not be
sent due to ISC_R_MAXSIZE.
This ran afoul of a pre-existing INSIST in ns_client_error() when
RRL was in use. the INSIST was based on the assumption that
ns_client_error() could never result in a non-error rcode. as
that assumption is no longer valid, the INSIST has been removed.
The additional processing method has been expanded to take the
owner name of the record, as HTTPS and SVBC need it to process "."
in service form.
The additional section callback can now return the RRset that was
added. We use this when adding CNAMEs. Previously, the recursion
would stop if it detected that a record you added already exists. With
CNAMEs this rule doesn't work, as you ultimately care about the RRset
at the target of the CNAME and not the presence of the CNAME itself.
Returning the record allows the caller to restart with the target
name. As CNAMEs can form loops, loop protection was added.
As HTTPS and SVBC can produce infinite chains, we prevent this by
tracking recursion depth and stopping if we go too deep.
Add a test case for GL #2845 where a zone is in two views, one base
view and one "in-view" and that zone is using an $INCLUDE. Make sure
that there is a jnl file (have ixfr-from-differences enabled and do a
dynamic update). Then freeze and make updates in the included file
(this requires the test.db file also to be updated because 'rndc freeze'
causes the zone file to be overwritten). Finally reload and ensure that
the edit in the included file has been loaded.
string.endswith("label.sequence") doesn't check for the implict
period before "label.sequence" when matching longer strings.
"foo.label.sequence" should match but "foolabel.sequence shouldn't".
When looking up a zonecut in cache, we use 'dns_rbt_findnode' to find
the closest matching node. This function however does not take into
account stale nodes. When we do find a stale node and use it, this
has implications for subsequent lookups. For example, this may break
QNAME minimization because we are using a deeper zonecut than we should
have.
Check the header for staleness and if so, and stale entries are not
accepted, look for the deepest zonecut from this node up.
There are some occurrences where we check if a header exists in the
rbtdb. These cases require that the header is also not marked as
ancient (aka ready for cleanup). These cases involve finding certain
data in cache.
Add test cases for GL #2665: The QNAME minimization (if enabled) should
also occur on the second query, after the RRsets have expired from
cache. BIND will still have the entries in cache, but marked stale.
These stale entries should not prevent the resolver from minimizing
the QNAME. We query for the test domain a.b.stale. in all cases (QNAME
minimization off, strict mode, and relaxed mode) and expect it to
behave the same the second time we have a stale delegation structure in
cache.
The commit fixes the doh_recv_send() because occasionally it would
fail because it did not wait for all responses to be sent, making the
check for ssends value to nit pass.
This commit changes TLS stream behaviour in such a way, that it is now
optimised for small writes. In the case there is a need to write less
or equal to 512 bytes, we could avoid calling the memory allocator at
the expense of possibly slight increase in memory usage. In case of
larger writes, the behviour remains unchanged.
At least at this point doing memory copying is not required. Probably
it was a workaround for some problem in the earlier days of DoH, at
this point it appears to be a waste of CPU cycles.
This commit significantly simplifies the code in http_send_outgoing()
as it was unnecessary complicated, because it was dealing with
multiple statically and dynamically allocated buffers, making it
extremely hard to follow, as well as making it to do unnecessary
memory copying in some situations. This commit fixes these issues,
while retaining the high level buffering logic.
When an HTTP/2 client terminates a session it means that it is about
to close the underlying connection. However, we were not doing that.
As a result, with the latest changes to the test suite, which made it
to limit amount of requests per a transport connection, the tests
using quota would hang for quite a while. This commit fixes that.
This commit ensures that only a limited number of requests is going to
be sent over a single HTTP/2 connection. Before that change was
introduced, it was possible to complete all of the planned sends via
only one transport connection, which undermines the purpose of the
tests using the quota facility.
The function should not be called here because it is, in general,
supposed to be called at the end of the transport level callbacks to
perform I/O, and thus, calling it here is clearly a mistake because it
breaks other code expectations. As a result of the call to
http_do_bio() from within isc__nm_http_request() the unit tests were
running slower than expected in some situations.
In this particular situation http_do_bio() is going to be called at
the end of the transport_connect_cb() (initially), or http_readcb(),
sending all of the scheduled requests at once.
This change affects only the test suite because it is the only place
in the codebase where isc__nm_http_request() is used in order to
ensure that the server is able to handle multiple HTTP/2 streams at
once.
This commit fixes a crash in DoH caused by transport handle to be
detached too early when sending outgoing data.
We need to attach to the session->handle earlier because as an
indirect result of the nghttp2_session_mem_send() the session might
get closed and the handle detached. However, there is still might be
some outgoing data to handle. Besides, even when the underlying socket
was closed via the handle, we still should try to attempt to send
outgoing data via isc_nm_send() to let it call write callback, passed
to the http_send_outgoing().
This commit gets rid of custom code taking care of response buffering
by replacing the custom code with isc_buffer_t. Also, it gets rid of
an unnecessary memory copying when sending a response.
This commit replaces the ad-hoc 64K buffer for incoming POST data with
isc_buffer_t backed by dynamically allocated buffer sized accordingly
to the value in the "Content-Length" header.
The commit replaces an ad-hoc incoming DNS-message buffer in the
client-side DoH code with isc_buffer_t.
The commit also fixes a timing issue in the unit tests revealed by the
change.
This commit replaces a static ad-hoc HTTP/2 session's temporary buffer
with a realloc-able isc_buffer_t object, which is being allocated on
as needed basis, lowering the memory consumption somewhat. The buffer
is needed in very rare cases, so allocating it prematurely is not
wise.
Also, it fixes a bug in http_readcb() where the ad-hoc buffer appeared
to be improperly used, leading to a situation when the processed data
from the receiving regions can be processed twice, while unprocessed
data will never be processed.
When copying metadata from one dst_key to another, when the source
dst_key has a boolean metadata unset, the destination dst_key will
have a numeric metadata unset instead.
This means that if a key has KSK or ZSK unset, we may be clearing the
Predecessor or Successor metadata in the destination dst_key.
Add a test case to the dnssec system test to check that:
- a zone with a prepublished key is only signed with the active key.
- a zone with an inactive key but valid signatures retains those
signatures and does not add signatures from successor key.
- signatures are swapped in a zone when signatures of predecessor
inactive key are within the refresh interval.
When signing with a ZSK, check if it has a predecessor. If so, and if
the predecessor key is sane (same algorithm, key id matches predecessor
value, is zsk), check if the RRset is signed with this key. If so, skip
signing with this successor key. Otherwise, do sign with the successor
key.
This change means we also need to apply the interval to keys that are
not actively signing. In other words, 'expired' is always
'isc_serial_gt(now + cycle, rrsig.timeexpire)'.
Fix a print style issue ("removing signature by ..." was untabbed).
In the "Migrating from NSEC to NSEC3" section, it says:
dnssec-policy "standard" {
nsec3param iterations optout no salt-length 16;
};
There should be an integer after "iterations". Based on the following
text, the number of iterations should be 10.
This commit gets rid of RW locks in a hot path of the DoH code. In the
original design, it was implied that we add new endpoints after the
HTTP listener was created. Such a design implies some locking. We do
not need such flexibility, though. Instead, we could build a set of
endpoints before the HTTP listener gets created. Such a design does
not need RW locks at all.
This commit increases the idle TCP timeout to let the DoH quota system
test pass on some platforms (namely FreeBSD 11). It turned out to run
slow enough on the CI under load for the idle TCP timeout to kick in.
This commit refactors the DoH quota system test to make it more
reliable.
The test tries to establish dummy TCP connections to stress the quota
one by one instead of in bulk until the BIND instance cannot answer
queries anymore. This design is better because the test itself does
not need to be aware of the actual quota size.
respdiff needs to be run regularly to identify problems with query
responses discrepancies sooner than after tagging a release.
MAX_DISAGREEMENTS_PERCENTAGE variable is set to 0.5 on the main branch
to make room for a greater number of response disagreements between a
relatively old baseline version and the Development Version.
On the isc_mem water change the old water_t structure could be used
after free. Instead of introducing reference counting on the hot-path
we are going to introduce additional constraints on the
isc_mem_setwater. Once it's set for the first time, the additional
calls have to be made with the same water and water_arg arguments.
Increasing the nodelock count had major impact on the memory footprint
in scenarios where multiple rbtdb structure would be created like
hosting many zones in a single server.
This reverts commit 0344684385 and sets
the nodelock count to previously used values.
Since the forced removal of gcc:sid:i386 in 0aacabc6, we lacked a 32-bit
environment to build and test BIND 9 in the CI. gcc:buster:amd64cross32
adds an environment to cross-compile BIND 9 to 32-bits on Debian Buster
amd64 image with 32-bit BIND 9 dependencies. Commit also adds sanity
checks to ensure that compiled objects are not of the build platform
triplet type.
The support for stat.pl's --restart option was incomplete in run.sh.
This change makes sure it's handled properly and that named.run file is
not being removed by clean.sh when the --restart option is used.
When named failed to start and produced core dump, the core file wasn't
processed by GDB because of run.sh script exiting immediately. This
remedies the limitation, simplifies the surrounding code, and makes the
script shellcheck clean.
Anchor lets the user see the full command logged in GitLab CI:
${CONFIGURE} --disable-maintainer-mode --enable-developer ...
Instead of a folded multi-line when literal block is used:
${CONFIGURE} \ # collapsed multi-line command
Make DoH-quota separate and configurable, make it possible to limit the number of HTTP/2 streams per connection
See merge request isc-projects/bind9!5036
The system tests stress out the DoH quota by opening many TCP
connections and then running dig instances against the "overloaded"
server to perform some queries. The processes cannot make any
resolutions because the quota is exceeded. Then the opened connections
are getting closed in random order allowing the queries to proceed.
This commit makes number of concurrent HTTP/2 streams per connection
configurable as a mean to fight DDoS attacks. As soon as the limit is
reached, BIND terminates the whole session.
The commit adds a global configuration
option (http-streams-per-connection) which can be overridden in an
http <name> {...} statement like follows:
http local-http-server {
...
streams-per-connection 100;
...
};
For now the default value is 100, which should be enough (e.g. NGINX
uses 128, but it is a full-featured WEB-server). When using lower
numbers (e.g. ~70), it is possible to hit the limit with
e.g. flamethrower.
This commit adds support for http-listener-clients global options as
well as ability to override the default in an HTTP server description,
like:
http local-http-server {
...
listener-clients 100;
...
};
This way we have ability to specify per-listener active connections
quota globally and then override it when required. This is exactly
what AT&T requested us: they wanted a functionality to specify quota
globally and then override it for specific IPs. This change
functionality makes such a configuration possible.
It makes sense: for example, one could have different quotas for
internal and external clients. Or, for example, one could use BIND's
internal ability to serve encrypted DoH with some sane quota value for
internal clients, while having un-encrypted DoH listener without quota
to put BIND behind a load balancer doing TLS offloading for external
clients.
Moreover, the code no more shares the quota with TCP, which makes
little sense anyway (see tcp-clients option), because of the nature of
interaction of DoH clients: they tend to keep idle opened connections
for longer periods of time, preventing the TCP and TLS client from
being served. Thus, the need to have a separate, generally larger,
quota for them.
Also, the change makes any option within "http <name> { ... };"
statement optional, making it easier to override only required default
options.
By default, the DoH connections are limited to 300 per listener. I
hope that it is a good initial guesstimate.
This commit adds the code (and some tests) which allows verifying
validity of HTTP paths both in incoming HTTP requests and in BIND's
configuration file.
Extend the "chain" system test with AUTHORITY section checks for signed,
secure delegations. This complements the checks for signed, insecure
delegations added by commit 26ec4b9a89.
Extend the existing AUTHORITY section checks for signed, insecure
delegations to ensure nonexistence of DS RRsets in such responses.
Adjust comments accordingly.
Ensure dig failures cause the "chain" system test to fail.
It has been noticed that commit 7a87bf468b
did not only fix NSEC record handling in signed, insecure delegations
prepared using both wildcard expansion and CNAME chaining - it also
inadvertently fixed DS record handling in signed, secure delegations
of that flavor. This is because the 'rdataset' variable in the relevant
location in query_addds() can be either a DS RRset or an NSEC RRset.
Update a code comment in query_addds() to avoid confusion.
Update the comments describing the purpose of query_addds() so that they
also mention NSEC(3) records.
If we have a CDS or CDNSKEY we at least need to have a DNSKEY with the
same algorithm published and signing the CDS RRset. Same for CDNSKEY
of course.
This relaxes the zone_cdscheck function, because before the CDS or
CDNSKEY had to match a DNSKEY, now only the algorithm has to match.
This allows a provider in a multisigner model to update the CDS/CDNSKEY
RRset in the zone that is served by the other provider.
Add tests to the nsupdate system test to make sure that CDS and/or
CDNSKEY that match an algorithm in the DNSKEY RRset are allowed. Also
add tests that updates are rejected if the algorithm does not match.
Remove the now redundant test cases from the dnssec system test.
Update the checkzone system test: Change the algorithm of the CDS and
CDNSKEY records so that the zone is still rejected.
An unhandled code path left GET query string data uninitialised (equal
to NULL) and led to a crash during the requests' base64 data
decoding. This commit fixes that.
As we don't set the thread affinity, the cpu test would consistently
fail. Disable it, but don't remove it as we might restore setting the
affinity in the future versions of BIND 9.
It was discovered that setting the thread affinity on both the netmgr
and netthread threads lead to inconsistent recursive performance because
sometimes the netmgr and netthread threads would compete over single
resource and sometimes not.
Removing setting the affinity causes a slight dip in the authoritative
performance around 5% (the measured range was from 3.8% to 7.8%), but
the recursive performance is now consistently good.
On OpenBSD and more generally on platforms without either jemalloc or
malloc_(usable_)size, we need to increase the alignment for the memory
to sizeof(max_align_t) as with plain sizeof(void *), the compiled code
would be crashing when accessing the returned memory.
It was discovered that on some platforms (f.e. Alpine Linux with MUSL)
the result of isc_os_ncpus() call differ when called before and after we
drop privileges. This commit changes the isc_os_ncpus() call to cache
the result from the first call and thus always return the same value
during the runtime of the named. The first call to isc_os_ncpus() is
made as soon as possible on the library initalization.
The isc_mem_get(), isc_mem_allocate() and isc_mem_reallocate() can
return NULL ptr in case where the allocation size is NULL. Remove the
nonnull attribute from the functions' declarations.
This stems from the following definition in the C11 standard:
> If the size of the space requested is zero, the behavior is
> implementation-defined: either a null pointer is returned, or the
> behavior is as if the size were some nonzero value, except that the
> returned pointer shall not be used to access an object.
In this case, we return NULL as it's easier to detect errors when
accessing pointer from zero-sized allocation which should obviously
never happen.
In the rallocx() shim for OpenBSD (that's the only platform that doesn't
have malloc_size() or malloc_usable_size() equivalent), the newly
allocated size was missing the extra size_t member for storing the
allocation size leading to size_t sized overflow at the end of the
reallocated memory chunk.
2607
43. tainted_argument: Calling function journal_read_xhdr taints argument xhdr.size. [show details]
2608 result = journal_read_xhdr(j1, &xhdr);
44. Condition rewrite, taking true branch.
45. Condition result == 29, taking false branch.
2609 if (rewrite && result == ISC_R_NOMORE) {
2610 break;
2611 }
46. Condition result != 0, taking false branch.
2612 CHECK(result);
2613
47. var_assign_var: Assigning: size = xhdr.size. Both are now tainted.
2614 size = xhdr.size;
CID 331088 (#3 of 3): Untrusted allocation size (TAINTED_SCALAR)
48. tainted_data: Passing tainted expression size to isc__mem_get, which uses it as an allocation size. [show details]
Ensure that tainted values are properly sanitized, by checking that their values are within a permissible range.
2615 buf = isc_mem_get(mctx, size);
In the jemalloc merge request, we missed the fact that ah_frees and ah_handles
are reallocated which is not compatible with using isc_mem_get() for allocation
and isc_mem_put() for deallocation. This commit reverts that part and restores
use of isc_mem_allocate() and isc_mem_free().
The proper way how to disable the water limit in the isc_mem context is
to call:
isc_mem_setwater(ctx, NULL, NULL, 0, 0);
this ensures that the old water callback is called with ISC_MEM_LOWATER
if the callback was called with ISC_MEM_HIWATER before.
Historically, there were some places where the limits were disabled by
calling:
isc_mem_setwater(ctx, water, water_arg, 0, 0);
which would also call the old callback, but it also causes the water_t
to be allocated and extra check to be executed because water callback is
not NULL.
This commits unifies the calls to disable water to the preferred form.
The Address and Thread Sanitizers both intercept the malloc calls and
using the extended jemalloc API interferes with that. This commit
disables the use of jemalloc for both ASAN and TSAN enabled builds to
eliminate both false positives and false negatives.
Previously, the isc_mem_allocate() and isc_mem_free() would be used for
isc_mem_total test, but since we now use the real allocation
size (sallocx, malloc_size, malloc_usable_size) to track the allocation
size, it's impossible to get the test value right. Changing the test to
use isc_mem_get() and isc_mem_put() will use the exact size provided, so
the test would work again on all the platforms even when jemalloc is not
being used.
It was discovered that softhsm2.4 has a bug that causes invalid free()
call to be called when unloading libsofthsm.so.2 library. The native
PKCS#11 API is scheduled to removed in the 9.17+ release, we could
safely just disable jemalloc for this particular build.
This commit refactors the water mechanism in the isc_mem API to use
single pointer to a water_t structure that can be swapped with
atomic_exchange operation instead of having four different
values (water, water_arg, hi_water, lo_water) in the flat namespace.
This reduces the need for locking and prevents a race when water and
water_arg could be desynchronized.
Calls to jemalloc extended API with size == 0 ends up in undefined
behaviour. This commit makes the isc_mem_get() and friends calls
more POSIX aligned:
If size is 0, either a null pointer or a unique pointer that can be
successfully passed to free() shall be returned.
We picked the easier route (which have been already supported in the old
code) and return NULL on calls to the API where size == 0.
This commit adds support for systems where the jemalloc library is not
available as a package, here's the quick summary:
* On Linux - the jemalloc is usually available as a package, if
configured --without-jemalloc, the shim would be used around
malloc(), free(), realloc() and malloc_usable_size()
* On macOS - the jemalloc is available from homebrew or macports, if
configured --without-jemalloc, the shim would be used around
malloc(), free(), realloc() and malloc_size()
* On FreeBSD - the jemalloc is *the* system allocator, we just need
to check for <malloc_np.h> header to get access to non-standard API
* On NetBSD - the jemalloc is *the* system allocator, we just need to
check for <jemalloc/jemalloc.h> header to get access to non-standard
API
* On a system hostile to users and developers (read OpenBSD) - the
jemalloc API is emulated by using ((size_t *)ptr)[-1] field to hold
the size information. The OpenBSD developers care only for
themselves, so why should we care about speed on OpenBSD?
- isc_mempool_get() can no longer fail; when there are no more objects
in the pool, more are always allocated. checking for NULL return is
no longer necessary.
- the isc_mempool_setmaxalloc() and isc_mempool_getmaxalloc() functions
are no longer used and have been removed.
Current mempools are kind of hybrid structures - they serve two
purposes:
1. mempool with a lock is basically static sized allocator with
pre-allocated free items
2. mempool without a lock is a doubly-linked list of preallocated items
The first kind of usage could be easily replaced with jemalloc small
sized arena objects and thread-local caches.
The second usage not-so-much and we need to keep this (in
libdns:message.c) for performance reasons.
Previously, we only had capability to trace the mempool gets and puts,
but for debugging, it's sometimes also important to keep track how many
and where do the memory pools get created and destroyed. This commit
adds such tracking capability.
The isc_mem_allocate() comes with additional cost because of the memory
tracking. In this commit, we replace the usage with isc_mem_get()
because we track the allocated sizes anyway, so it's possible to also
replace isc_mem_free() with isc_mem_put().
The jemalloc non-standard API fits nicely with our memory contexts, so
just rewrite the memory context internals to use the non-public API.
There's just one caveat - since we no longer track the size of the
allocation for isc_mem_allocate/isc_mem_free combination, we need to use
sallocx() to get real allocation size in both allocator and deallocator
because otherwise the sizes would not match.
The ISC_MEM_DEBUGSIZE and ISC_MEM_DEBUGCTX did sanity checks on matching
size and memory context on the memory returned to the allocator. Those
will no longer needed when most of the allocator will be replaced with
jemalloc.
There's global variable called `malloc_conf` that can be used to
configure jemalloc behaviour at the program startup. We use following
configuration:
* xmalloc:true - abort-on-out-of-memory enabled.
* background_thread:true - Enable internal background worker threads
to handle purging asynchronously.
* metadata_thp:auto - allow jemalloc to use transparent huge page
(THP) for internal metadata initially, but may begin to do so when
metadata usage reaches certain level.
* dirty_decay_ms:30000 - Approximate time in milliseconds from the
creation of a set of unused dirty pages until an equivalent set of
unused dirty pages is purged and/or reused.
* muzzy_decay_ms:30000 - Approximate time in milliseconds from the
creation of a set of unused muzzy pages until an equivalent set of
unused muzzy pages is purged and/or reused.
More information about the specific meaning can be found in the jemalloc
manpage or online at http://jemalloc.net/jemalloc.3.html
The jemalloc allocator is scalable high performance allocator, this is
the first in the series of commits that will add jemalloc as a memory
allocator for BIND 9.
This commit adds configure.ac check and Makefile modifications to use
jemalloc as BIND 9 allocator.
Previously, we only had capability to trace the memory gets and puts,
but for debugging, it's sometimes also important to keep track how many
and where do the memory contexts get created and destroyed. This commit
adds such tracking capability.
This commit makes BIND return HTTP status codes for malformed or too
small requests.
DNS request processing code would ignore such requests. Such an
approach works well for other DNS transport but does not make much
sense for HTTP, not allowing it to complete the request/response
sequence.
Suppose execution has reached the point where DNS message handling
code has been called. In that case, it means that the HTTP request has
been successfully processed, and, thus, we are expected to respond to
it either with a message containing some DNS payload or at least to
return an error status code. This commit ensures that BIND behaves
this way.
This error code fits better than the more generic "Internal Server
Error" (500) which implies that the problem is on the server.
Also, do not end the whole HTTP/2 session on a bad request.
We were too strict regarding the value and presence of "Accept" HTTP
header, slightly breaking compatibility with the specification.
According to RFC8484 client SHOULD add "Accept" header to the requests
but MUST be able to handle "application/dns-message" media type
regardless of the value of the header. That basically suggests we
ignore its value.
Besides, verifying the value of the "Accept" header is a bit tricky
because it could contain multiple media types, thus requiring proper
parsing. That is doable but does not provide us with any benefits.
Among other things, not verifying the value also fixes compatibility
with clients, which could advertise multiple media types as supported,
which we should accept. For example, it is possible for a perfectly
valid request to contain "application/dns-message", "application/*",
and "*/*" in the "Accept" header value. Still, we would treat such a
request as invalid.
The commit fixes BIND hanging when browsers end HTTP/2 streams
prematurely (for example, by sending RST_STREAM). It ensures that
isc__nmsocket_prep_destroy() will be called for an HTTP/2 stream,
allowing it to be properly disposed.
The problem was impossible to reproduce using dig or DoH benchmarking
software (e.g. flamethrower) because these do not tend to end HTTP/2
streams prematurely.
This commit adds two new autoconf options `--enable-doh` (enabled by
default) and `--with-libnghttp2` (mandatory when DoH is enabled).
When DoH support is disabled the library is not linked-in and support
for http(s) protocol is disabled in the netmgr, named and dig.
if a control channel listener was configured with more than one
key algorithm, message verification would be attempted with each
algorithm in turn. if the first key failed due to the wrong
signature length, the entire verification process was aborted,
rather than continuing on to try with another key.
The isc/platform.h header was left empty which things either already
moved to config.h or to appropriate headers. This is just the final
cleanup commit.
The last remaining defines needed for platforms without NAME_MAX and
PATH_MAX (I'm looking at you, GNU Hurd) were moved to isc/dir.h where
it's prevalently used.
The ISC_STRERRORSIZE was defined in isc/platform.h header as the
value was different between Windows and POSIX platforms. Now that
Windows is gone, move the define to where it belongs.
The function 'private_type_record()' is now used in multiple system
setup scripts and should be moved to the common configuration script
conf.sh.common.
The old approach where each zone structure has its own mutex that
a thread needs to obtain multiple locks to do safe keyfile I/O
operations lead to a race condition ending in a possible deadlock.
Consider a zone in two views. Each such zone is stored in a separate
zone structure. A thread that needs to read or write the key files for
this zone needs to obtain both mutexes in seperate structures. If
another thread is working on the same zone in a different view, they
race to get the locks. It would be possible that thread1 grabs the
lock of the zone in view1, while thread2 wins the race for the lock
of the zone in view2. Now both threads try to get the other lock, both
of them are already locked.
Ideally, when a thread wants to do key file operations, it only needs
to lock a single mutex. This commit introduces a key management hash
table, stored in the zonemgr structure. Each time a zone is being
managed, an object is added to the hash table (and removed when the
zone is being released). This object is identified by the zone name
and contains a mutex that needs to be locked prior to reading or
writing key files.
(cherry-picked from commit ef4619366d49efd46f9fae5f75c4a67c246ba2e6)
Similar to notify, add code to send and keep track of checkds requests.
On every zone_rekey event, we will check the DS at parental agents
(but we will only actually query parental agents if theree is a DS
scheduled to be published/withdrawn).
On a zone_rekey event, we will first clear the ongoing checkds requests.
Reset the counter, to avoid continuing KSK rollover premature.
This has the risk that if zone_rekey events happen too soon after each
other, there are redundant DS queries to the parental agents. But
if TTLs and the configured durations in the dnssec-policy are sane (as
in not ridiculous short) the chance of this happening is low.
This code gathers DNSSEC keys from key files and from the DNSKEY RRset.
It is used for the 'rndc dnssec -status' command, but will also be
needed for "checkds". Turn it into a function.
Change the static function 'get_ksk_zsk' to a library function that
can be used to determine the role of a dst_key. Add checks if the
boolean parameters to store the role are not NULL. Rename to
'dst_key_role'.
Add a Pytest based system test for the 'checkds' feature. There is
one nameserver (ns9, because it should be started the latest) that
has configured several zones with dnssec-policy. The zones are set
in such a state that they are waiting for DS publication or DS
withdrawal.
Then several other name servers act as parent servers that either have
the DS for these published, or not. Also one server in the mix is
to test a badly configured parental-agent.
There are tests for DS publication, DS publication error handling,
DS withdrawal and DS withdrawal error handling.
The tests ensures that the zone is DNSSEC valid, and that the
DSPublish/DSRemoved key metadata is set (or not in case of the error
handling).
It does not test if the rollover continues, this is already tested in
the kasp system test (that uses 'rndc -dnssec checkds' to set the
DSPublish/DSRemoved key metadata).
Add checks for "parental-agents" configuration, checking for the option
being at wrong type of zone (only allowed for primaries and
secondaries), duplicate definitions, duplicate references, and
undefined parental clauses (the name referenced in the zone clause
does not have a matching "parental-agent" clause).
When performing the 'setnsec3param' task, zones that are not loaded will have
their task rescheduled. We should do this only if the zone load is still
pending, this prevents zones that failed to load get stuck in a busy wait and
causing a hang on shutdown.
Add a zone to the configuration file that uses NSEC3 with dnssec-policy
and fails to load. This will cause setnsec3param to go into a busy wait
and will cause a hang on shutdown.
The Makefile.tests was modifying global AM_CFLAGS and LDADD and could
accidentally pull /usr/include to be listed before the internal
libraries, which is known to cause problems if the headers from the
previous version of BIND 9 has been installed on the build machine.
We should drop the HISTORY file because it's confusing and the same
information is covered by the release notes for .0 releases (or at
least they should be).
Remove references to the HISTORY file, update the README to tell
people go look somewhere else.
This was written down in the outdated doc/dev/release documentation.
Since the rest of that file can go, add these steps to a separate file
and update it to current standards (e.g. use git commands).
The util/, doc/design/, and doc/dev/ directories included couple of
tools or documents there were completely outdated because they either
refered the the VCS we no longer use (cvs) or described processes that
have been redesigned and they are documented elsewhere.
When backporting the Don't Fragment UDP socket option, it was noticed
that the edns-udp-size probing uses 1432 as one of the values to be
probed and the documentation would be recommending 1400 as the safe
value. As the safe value can be from the 1400-1500 interval, the
documentation has been changed to match the probed value, so we do not
skip it.
- add an 'nsupdate -C' option to override resolv.conf file for nsupdate
- set resolv.conf to use two test servers, the first one of which will
return REFUSED for a query for 'example'.
when nsupdate sends an SOA query to a resolver, if it fails
with REFUSED, nsupdate will now try the next server rather than
aborting the update completely.
In DNS Flag Day 2020, we started setting the DF (Don't Fragment socket
option on the UDP sockets. It turned out, that this code was incomplete
leading to dropping the outgoing UDP packets.
This has been now remedied, so it is possible to disable the
fragmentation on the UDP sockets again as the sending error is now
handled by sending back an empty response with TC (truncated) bit set.
This reverts commit 66eefac78c.
When the fragmentation is disabled on UDP sockets, the uv_udp_send()
call can fail with UV_EMSGSIZE for messages larger than path MTU.
Previously, this error would end with just discarding the response. In
this commit, a proper handling of such case is added and on such error,
a new DNS response with truncated bit set is generated and sent to the
client.
This change allows us to disable the fragmentation on the UDP
sockets again.
Add three more test cases that detect a configuration error if the
key-directory is inherited but has the same value for a zone in a
different view with a deviating DNSSEC policy.
This commit adds a unittest that tests private rdataset_getownercase()
and rdataset_setownercase() methods from rbtdb.c. The test setups
minimal mock dns_rbtdb_t and dns_rbtdbnode_t data structures.
As the rbtdb methods are generally hidden behind layers and layers, we
include the "rbtdb.c" directly from rbtdb_test.c, and thus we can use
the private methods and data structures directly. This also opens up
opportunity to add more unittest for the rbtdb private functions without
going through all the layers.
This check intermittently failed:
I:serve-stale:check not in cache longttl.example times out...
I:serve-stale:failed
This corresponds to this query in the test:
$DIG -p ${PORT} +tries=1 +timeout=3 @10.53.0.3 longttl.example TXT
Looking at the dig output for a failed test, the query actually got a
response from the authoritative server (in one specific example the
query time was 2991 msec, close to 3 seconds).
After doing the query for the test, we enable the authoritative
server after a sleep of three seconds. If we bump this sleep to 4
seconds, the race will be more in favor of the query timing out,
making it unlikely that this test will fail intermittently.
Bump the subsequent wait_for_log checks also with one second.
In the code that rdataset_setownercase() and rdataset_getownercase() we
now use tolower()/toupper()/isupper() functions appropriately instead of
rolling our own code.
Previously, we would set the locale on a global level and that could
possibly lead to different behaviour in underlying functions. In this
commit, we change to code to use the system locale only when calling the
libidn2 functions and reset the locale back to "POSIX" when exiting the
libidn2 code.
Expand the description of mirror zones in the ARM by adding a brief
discussion of how the validation process works for AXFR and IXFR. Move
the paragraph mentioning the "file" option higher up. Apply minor
stylistic and whitespace-related tweaks to the relevant section of the
ARM.
Apply minor stylistical and whitespace-related tweaks to the
descriptions of the "tcp-receive-buffer", "udp-receive-buffer",
"tcp-send-buffer", and "udp-send-buffer" options in the ARM.
The ARM contains typos in the names of the following two options:
- "tcp-receive-buffer"
- "udp-receive-buffer"
Fix the ARM so that it contains proper option names.
Improve the description of the "max-cache-size" option in the ARM by
focusing on its meaning for multiple views and default values.
Add mention of a hash table preallocation.
This commit adds a set of tests to verify that BIND will not crash
when some opcodes are sent over DoT or DoH, leading to marking network
handle in question as sequential.
Previously, each protocol (TCPDNS, TLSDNS) has specified own function to
disable pipelining on the connection. An oversight would lead to
assertion failure when opcode is not query over non-TCPDNS protocol
because the isc_nm_tcpdns_sequential() function would be called over
non-TCPDNS socket. This commit removes the per-protocol functions and
refactors the code to have and use common isc_nm_sequential() function
that would either disable the pipelining on the socket or would handle
the request in per specific manner. Currently it ignores the call for
HTTP sockets and causes assertion failure for protocols where it doesn't
make sense to call the function at all.
The built-in "_bind" view does not allow recursion and therefore does
not need a large cache database. However, as "max-cache-size" is not
explicitly set for that view in the default configuration, it inherits
that setting from global options. Set "max-cache-size" for the built-in
"_bind" view to a fixed value (2 MB, i.e. the smallest allowed value) to
prevent needlessly preallocating memory for its cache RBT hash table.
Currently the implicit default for the "max-cache-size" option is "90%".
As this option is inherited by all configured views, using multiple
views can lead to memory exhaustion over time due to overcommitment.
The "max-cache-size 90%;" default also causes cache RBT hash tables to
be preallocated for every configured view, which does not really make
sense for views which do not allow recursion.
To limit this problem's potential for causing operational issues, use a
minimal-sized cache for views which do not allow recursion and do not
have "max-cache-size" explicitly set (either in global configuration or
in view configuration).
For configurations which include multiple views allowing recursion,
adjusting "max-cache-size" appropriately is still left to the operator.
When locking key files for a zone, we iterate over all the views and
lock a mutex inside the zone structure. However, if we envounter an
in-view zone, we will try to lock the key files twice, one time for
the home view and one time for the in-view view. This will lead to
a deadlock because one thread is trying to get the same lock twice.
When "max-cache-size" is changed to "unlimited" (or "0") for a running
named instance (using "rndc reconfig"), the hash table size limit for
each affected cache DB is not reset to the maximum possible value,
preventing those hash tables from being allowed to grow as a result of
new nodes being added.
Extend dns_rbt_adjusthashsize() to interpret "size" set to 0 as a signal
to remove any previously imposed limits on the hash table size. Adjust
API documentation for dns_db_adjusthashsize() accordingly. Move the
call to dns_db_adjusthashsize() from dns_cache_setcachesize() so that it
also happens when "size" is set to 0.
Upon creation, each dns_rbt_t structure has its "maxhashbits" field
initialized to the value of the RBT_HASH_MAX_BITS preprocessor macro,
i.e. 32. When the dns_rbt_adjusthashsize() function is called for the
first time for a given RBT (for cache RBTs, this happens when they are
first created, i.e. upon named startup), it lowers the value of the
"maxhashbits" field to the number of bits required to index the
requested number of hash table slots. When a larger hash table size is
subsequently requested, the value of the "maxhashbits" field should be
increased accordingly, up to RBT_HASH_MAX_BITS. However, the loop in
the rehash_bits() function currently ensures that the number of bits
necessary to index the resized hash table will not be larger than
rbt->maxhashbits instead of RBT_HASH_MAX_BITS, preventing the hash table
from being grown once the "maxhashbits" field of a given dns_rbt_t
structure is set to any value lower than RBT_HASH_MAX_BITS.
Fix by tweaking the loop guard condition in the rehash_bits() function
so that it compares the new number of bits used for indexing the hash
table against RBT_HASH_MAX_BITS rather than rbt->maxhashbits.
The timeout originally picked for "rndc status" invocations (2 seconds)
in the test attempting to reproduce a deadlock caused by running
multiple "rndc addzone", "rndc modzone", and "rndc delzone" commands
concurrently causes intermittent failures of the "addzone" system test
in GitLab CI. Increase the timeout to 10 seconds to make such failures
less probable. Adjust code comments accordingly.
The requirements for BIND 9.17+ now requires C11 support from the
compiler, so we can safely drop most of the stdatomic.h shims from
lib/isc/unix/include/stdatomic.h.
This commit removes support for clang atomic builtins (clang >= 3.6.0
includes stdatomic.h header) and for Gcc __sync builtins.
The only compatibility shim that remains is support for __atomic
builtins for Gcc >= 4.7.0 since CentOS 7 still includes only Gcc 4.8.1
and the proper stdatomic.h header was only introduced in Gcc >= 4.9.
The warning was produced by an ASAN build:
runtime error: null pointer passed as argument 2, which is declared to
never be null
This commit fixes it by checking if nghttp2_session_mem_send() has
actually returned anything.
Resolve "ThreadSanitizer: data race lib/isc/task.c:435 in task_send (unprotected access to `task->threadid`)"
Closes#2739
See merge request isc-projects/bind9!5149
This change sets the mentioned fields properly and gets rid of klusges
added in the times when we were keeping pointers to isc_sockaddr_t
instead of copies. Among other things it helps to avoid a situation
when garbage instead of an address appears in dig output.
We cannot use DoH for zone transfers. According to RFC8484 a DoH
request contains exactly one DNS message (see Section 6: Definition of
the "application/dns-message" Media Type,
https://datatracker.ietf.org/doc/html/rfc8484#section-6). This makes
DoH unsuitable for zone transfers as often (and usually!) these need
more than one DNS message, especially for larger zones.
As zone transfers over DoH are not (yet) standardised, nor discussed
in RFC8484, the best thing we can do is to return "not implemented."
Technically DoH can be used to transfer small zones which fit in one
message, but that is not enough for the generic case.
Also, this commit makes the server-side DoH code ensure that no
multiple responses could be attempted to be sent over one HTTP/2
stream. In HTTP/2 one stream is mapped to one request/response
transaction. Now the write callback will be called with failure error
code in such a case.
Support a situation in header processing callback when client side
code could receive a belated response or part of it. That could
happen when the HTTP/2 session was already closed, but there were some
response data from server in flight. Other client-side nghttp2
callbacks code already handled this case.
The bug became apparent after HTTP/2 write buffering was supported,
leading to rare unit test failures.
This commit ensures that sock->h2.connect.cstream gets nullified when
the object in question is deleted. This fixes a nasty crash in dig
exposed when receiving large responses leading to double free()ing.
Also, it refactors how the client-side code keeps track of client
streams (hopefully) preventing from similar errors appearing in the
future.
This commit makes NM code to report HTTP as a stream protocol. This
makes it possible to handle large responses properly. Like:
dig +https @127.0.0.1 A cmts1-dhcp.longlines.com
When answering a query requires wildcard expansion, the AUTHORITY
section of the response needs to include NSEC(3) record(s) proving that
the QNAME does not exist.
When a response to a query is an insecure delegation, the AUTHORITY
section needs to include an NSEC(3) proof that no DS record exists at
the parent side of the zone cut.
These two conditions combined trip up the NSEC part of the logic
contained in query_addds(), which expects the NS RRset to be owned by
the first name found in the AUTHORITY section of a delegation response.
This may not always be true, for example if wildcard expansion causes an
NSEC record proving QNAME nonexistence to be added to the AUTHORITY
section before the delegation is added to the response. In such a case,
named incorrectly omits the NSEC record proving nonexistence of QNAME
from the AUTHORITY section.
The same block of code is affected by another flaw: if the same NSEC
record proves nonexistence of both the QNAME and the DS record at the
parent side of the zone cut, this NSEC record will be added to the
AUTHORITY section twice.
Fix by looking for the NS RRset in the entire AUTHORITY section and
adding the NSEC record to the delegation using query_addrrset() (which
handles duplicate RRset detection).
Add a set of system tests which check the contents of the AUTHORITY
section for signed, insecure delegation responses constructed from CNAME
records and wildcards, both for zones using NSEC and NSEC3.
Instead of checking the value of the variable modified two lines earlier
(the number of SOA records present at the apex of the old version of the
zone), one of the RUNTIME_CHECK() assertions in zone_postload() checks
the number of SOA records present at the apex of the new version of the
zone, which is already checked before. Fix the assertion by making it
check the correct variable.
The Windows support has been completely removed from the source tree
and BIND 9 now no longer supports native compilation on Windows.
We might consider reviewing mingw-w64 port if contributed by external
party, but no development efforts will be put into making BIND 9 compile
and run on Windows again.
When named restarts, it will examine signed zones and checks if the
current denial of existence strategy matches the dnssec-policy. If not,
it will schedule to create a new NSEC(3) chain.
However, on startup the zone database may not be read yet, fooling
BIND that the denial of existence chain needs to be created. This
results in a replacement of the previous NSEC(3) chain.
Change the code such that if the NSEC3PARAM lookup failed (the result
did not return in ISC_R_SUCCESS or ISC_R_NOTFOUND), we will try
again later. The nsec3param structure has additional variables to
signal if the lookup is postponed. We also need to save the signal
if an explicit resalt was requested.
In addition to the two added boolean variables, we add a variable to
store the NSEC3PARAM rdata. This may have a yet to be determined salt
value. We can't create the private data yet because there may be a
mismatch in salt length and the NULL salt value.
Add a test case where 'named' is restarted and ensure that an already
signed zone does not change its NSEC3 parameters.
The test case first tests the current zone and saves the used salt
value. Then after restart it checks if the salt (and other parameters)
are the same as before the restart.
This test case changes 'set_nsec3param'. This will now reset the salt
value, and when checking for NSEC3PARAM we will store the salt and
use it when testing the NXDOMAIN response. This does mean that for
every test case we now have to call 'set_nsec3param' explicitly (and
can not omit it because it is the same as the previous zone).
Finally, slightly changed some echo output to make debugging friendlier.
When we rewrote the zone dumping to use the separate threadpool, the
dumping would acquire the read lock for the whole time the zone dumping
process is dumping the zone.
When combined with incoming IXFR that tries to acquire the write lock on
the same rwlock, we would end up blocking all the other readers.
In this commit, we pause the dbiterator every time we get next record
and before start dumping it to the disk.
Make sure an incoming IXFR containing an SOA record which is not placed
at the apex of the transferred zone does not result in a broken version
of the zone being served by named and/or a subsequent crash.
While cleaning up the usage of HAVE_UV_<func> macros, we forgot to
cleanup the HAVE_UV_UDP_CONNECT in the actual code and
HAVE_UV_TRANSLATE_SYS_ERROR and this was causing Windows build to fail
on uv_udp_send() because the socket was already connected and we were
falsely assuming that it was not.
The platforms with autoconf support were not affected, because we were
still checking for the functions from the configure.
This commit adds the ability to consolidate HTTP/2 write requests if
there is already one in flight. If it is the case, the code will
consolidate multiple subsequent write request into a larger one
allowing to utilise the network in a more efficient way by creating
larger TCP packets as well as by reducing TLS records overhead (by
creating large TLS records instead of multiple small ones).
This optimisation is especially efficient for clients, creating many
concurrent HTTP/2 streams over a transport connection at once. This
way, the code might create a small amount of multi-kilobyte requests
instead of many 50-120 byte ones.
In fact, it turned out to work so well that I had to add a work-around
to the code to ensure compatibility with the flamethrower, which, at
the time of writing, does not support TLS records larger than two
kilobytes. Now the code tries to flush the write buffer after 1.5
kilobyte, which is still pretty adequate for our use case.
Essentially, this commit implements a recommendation given by nghttp2
library:
https://nghttp2.org/documentation/nghttp2_session_mem_send.html
2021-06-01 21:07:45 +03:00
1758 changed files with 37007 additions and 95307 deletions
- [ ]***(Support)*** Send eligible customers updated links to the Subscription Edition (update the -S edition delivery tickets, even if those links were provided earlier via an ASN ticket).
- [ ]***(Support)*** Update tickets in case of waiting support customers.
- [ ]***(QA)*** Build and test any outstanding private packages.
- [ ]***(QA)*** Build public packages (`*.deb`, RPMs).
- [ ]***(QA)*** Build public RPMs.
- [ ]***(SwEng) *** Build Debian/Ubuntu packages.
- [ ]***(SwEng) *** Update Docker images.
- [ ]***(QA)*** Inform Marketing of the release.
- [ ]***(QA)*** Update the internal [BIND release dates wiki page](https://wiki.isc.org/bin/view/Main/BindReleaseDates) when public announcement has been made.
- [ ]***(Marketing)*** Post short note to Twitter.
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Artem Boldariev
Artem Boldariev
Michał Kępień
Arаm Sаrgsyаn
Evan HuntandAram Sargsyan
Mark Andrews
Ondřej Surý
Matthijs Mekking
Dan TheisenandMatthijs Mekking
Petr Špaček
Michal Nowak
Petr MensikandMichał Kępień
Vicky Risk
Diego FronzaandEvan Hunt
Tony FinchandEvan Hunt
Ondrej SuryandOndřej Surý