Add a test for freezing, manually updating, and then thawing a dynamic
zone with "dnssec-policy". In the kasp system test we add parameters
to the "update_is_signed" check to signal the indicated IP addresses
for the labels "a" and "d". If set to '-', the test is skipped.
After nsupdating the dynamic.kasp zone, we revert the update (with
nsupdate) and update the zone again, but now with the freeze/thaw
approach.
Dynamic zones with dnssec-policy could not be thawed because KASP
zones were considered always dynamic. But a dynamic KASP zone should
also check whether updates are disabled.
Artem Boldariev [Tue, 9 Mar 2021 12:45:03 +0000 (14:45 +0200)]
Load full certificate chain from a certificate chain file
This commit fixes loading the certificate chain files so that the full
chain could be sent to the clients which require that for
verification. Before that fix only the top most certificate would be
loaded from the chain and sent to clients preventing some of them to
perform certificate validation (e.g. Windows 10 DoH client).
Matthijs Mekking [Mon, 15 Mar 2021 14:08:04 +0000 (15:08 +0100)]
Fix a XoT crash
The transport should also be detached when we skip a master, otherwise
named will crash when sending a SOA query to the next master over TLS,
because the transport must be NULL when we enter
'dns_view_gettransport'.
Matthijs Mekking [Wed, 10 Mar 2021 14:14:56 +0000 (15:14 +0100)]
Fix servestale fetchlimits crash
When we query the resolver for a domain name that is in the same zone
for which is already one or more fetches outstanding, we could
potentially hit the fetch limits. If so, recursion fails immediately
for the incoming query and if serve-stale is enabled, we may try to
return a stale answer.
If the resolver is also is authoritative for the parent zone (for
example the root zone), first a delegation is found, but we first
check the cache for a better response.
Nothing is found in the cache, so we try to recurse to find the
answer to the query.
Because of fetch-limits 'dns_resolver_createfetch()' returns an error,
which 'ns_query_recurse()' propagates to the caller,
'query_delegation_recurse()'.
Because serve-stale is enabled, 'query_usestale()' is called,
setting 'qctx->db' to the cache db, but leaving 'qctx->version'
untouched. Now 'query_lookup()' is called to search for stale data
in the cache database with a non-NULL 'qctx->version'
(which is set to a zone db version), and thus we hit an assertion
in rbtdb.
Michał Kępień [Mon, 8 Mar 2021 11:27:39 +0000 (12:27 +0100)]
Fix documentation for the "max-ixfr-ratio" option
Commit 9fb6d11abbdb10ded128f0ee5c004f24b4030ede (which converted BIND 9
documentation from DocBook to Sphinx) inadvertently removed a paragraph
from the description of the "max-ixfr-ratio" option. Add the missing
paragraph back.
Ondřej Surý [Mon, 8 Mar 2021 11:09:14 +0000 (12:09 +0100)]
Add GitHub Action that immediately closes issue / PRs
Unfortunately, it's not possible to disable Pull Requests on the
mirrored repository on the GitHub, so this commit adds external action
that closes any new open Issue or Pull Requests instead letting them rot
unnoticed.
Evan Hunt [Fri, 5 Mar 2021 22:28:33 +0000 (14:28 -0800)]
fix automatic journal upgrade on windows
- use a value less than 2^32 for DNS_ZONEFLG_FIXJOURNAL; a larger value
could cause problems in some build environments. the zone flag
DNS_ZONEFLG_DIFFONRELOAD, which was no longer in use, has now been
deleted and its value reused for _FIXJOURNAL.
Mark Andrews [Sun, 7 Mar 2021 20:06:57 +0000 (07:06 +1100)]
Silence CID 329157: Dereference before null check in lib/dns/journal.c
*** CID 329157: Null pointer dereferences (REVERSE_INULL)
/lib/dns/journal.c: 754 in journal_open()
748 j->header.index_size * sizeof(journal_rawpos_t));
749 }
750 if (j->index != NULL) {
751 isc_mem_put(j->mctx, j->index,
752 j->header.index_size * sizeof(journal_pos_t));
753 }
CID 329157: Null pointer dereferences (REVERSE_INULL)
Null-checking "j->filename" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
754 if (j->filename != NULL) {
755 isc_mem_free(j->mctx, j->filename);
756 }
757 if (j->fp != NULL) {
758 (void)isc_stdio_close(j->fp);
759 }
Evan Hunt [Fri, 12 Feb 2021 02:42:00 +0000 (18:42 -0800)]
add basic DoH system tests
- rename dot to doth, as it now covers both dot and doh.
- merge xot into doth as it's closely related.
- added long-lived key and cert files (expiring 2121).
- add tests with https-get, https-post, http-plain, alternate
endpoints, and both static and ephemeral TLS configuration.
- incidentally fixed a memory leak in dig that occurred if +https
was specified more than once.
Artem Boldariev [Sun, 28 Feb 2021 17:33:16 +0000 (19:33 +0200)]
Disable Nagle's algorithm for HTTP/2 connections
It is advisable to disable Nagle's algorithm for HTTP/2 connections
because multiple HTTP/2 streams could be multiplexed over one
transport connection. Thus, delays when delivering small packets could
bring down performance for the whole session. HTTP/2 is meant to be
used this way.
Artem Boldariev [Thu, 11 Feb 2021 13:03:44 +0000 (15:03 +0200)]
Fix deadlock in isc_nm_tlsconnect()
when called from within the context of a network thread,
isc_nm_tlsconnect() hangs. it is waiting for the socket's
result code to be updated, but that update is supposed to happen
asynchronously in the network thread, and if we're already blocking
in the network thread, it can never occur.
we can kluge around this by setting the socket result code
early; this works for most clients (including "dig"), but it causes
inconsistent behaviors that manifest as test failures in the DoH unit
test.
so we kluged around it even more by setting the socket result code
early *only when running in the network thread*. we need a better
solution for this problem, but this will do for now.
Artem Boldariev [Tue, 16 Feb 2021 14:54:51 +0000 (16:54 +0200)]
Put sane limitations in place to handle bad requests gracefully
This commit makes the server-side code polite.
It fixes the error handling code on the server side and fixes
returning error code in responses (there was a nasty bug which could
potentially crash the server).
Also, in this commit we limit max size POST request data to 96K, max
processed data size in headers to 128K (should be enough to handle any
GET requests).
If these limits are surpassed, server will terminate the request with
RST_STREAM without responding with error code. Otherwise it politely
responds with error code.
This commit also limits number of concurrent HTTP/2 streams per
transport connection on server to 100 (as nghttp2 advises by default).
Ideally, these parameters should be configurable both globally and per
every HTTP endpoint description in the configuration file, but for now
putting sane limits should be enough.
Evan Hunt [Thu, 4 Feb 2021 00:59:49 +0000 (16:59 -0800)]
refactor outgoing HTTP connection support
- style, cleanup, and removal of unnecessary code.
- combined isc_nm_http_add_endpoint() and isc_nm_http_add_doh_endpoint()
into one function, renamed isc_http_endpoint().
- moved isc_nm_http_connect_send_request() into doh_test.c as a helper
function; remove it from the public API.
- renamed isc_http2 and isc_nm_http2 types and functions to just isc_http
and isc_nm_http, for consistency with other existing names.
- shortened a number of long names.
- the caller is now responsible for determining the peer address.
in isc_nm_httpconnect(); this eliminates the need to parse the URI
and the dependency on an external resolver.
- the caller is also now responsible for creating the SSL client context,
for consistency with isc_nm_tlsdnsconnect().
- added setter functions for HTTP/2 ALPN. instead of setting up ALPN in
isc_tlsctx_createclient(), we now have a function
isc_tlsctx_enable_http2client_alpn() that can be run from
isc_nm_httpconnect().
- refactored isc_nm_httprequest() into separate read and send functions.
isc_nm_send() or isc_nm_read() is called on an http socket, it will
be stored until a corresponding isc_nm_read() or _send() arrives; when
we have both halves of the pair the HTTP request will be initiated.
- isc_nm_httprequest() is renamed isc__nm_http_request() for use as an
internal helper function by the DoH unit test. (eventually doh_test
should be rewritten to use read and send, and this function should
be removed.)
- added implementations of isc__nm_tls_settimeout() and
isc__nm_http_settimeout().
- increased NGHTTP2 header block length for client connections to 128K.
- use isc_mem_t for internal memory allocations inside nghttp2, to
help track memory leaks.
- send "Cache-Control" header in requests and responses. (note:
currently we try to bypass HTTP caching proxies, but ideally we should
interact with them: https://tools.ietf.org/html/rfc8484#section-5.1)
Mark Andrews [Thu, 4 Mar 2021 06:22:01 +0000 (17:22 +1100)]
Move cleanup of queries to later in the shutdown sequence
to avoid TSAN report
WARNING: ThreadSanitizer: data race
Write of size 8 at 0x000000000001 by main thread:
#0 free <null>
#1 default_memfree lib/isc/mem.c:440
#2 mem_put lib/isc/mem.c:363
#3 isc__mem_free lib/isc/mem.c:1012
#4 main bin/tools/mdig.c:2231
Previous read of size 1 at 0x000000000005 by thread T1:
#0 dns_name_fromtext lib/dns/name.c:1121
#1 sendquery bin/tools/mdig.c:596
#2 sendqueries bin/tools/mdig.c:779
#3 dispatch lib/isc/task.c:1153
#4 run lib/isc/task.c:1345
#5 isc__trampoline_run lib/isc/trampoline.c:184
#6 <null> <null>
Thread T1 (running) created by main thread at:
#0 pthread_create <null>
#1 isc_thread_create pthreads/thread.c:79
#2 isc_taskmgr_create lib/isc/task.c:1435
#3 main bin/tools/mdig.c:2148
SUMMARY: ThreadSanitizer: data race in __interceptor_free
Ondřej Surý [Thu, 4 Mar 2021 09:47:56 +0000 (10:47 +0100)]
Fix comparison between signed and unsigned integer expressions
Simple typecast to size_t should be enough to silence the warning on
ARMv7, even though the code is in fact correct, because the readlen is
checked for being < 0 in the block before the warning.
Ondřej Surý [Thu, 4 Mar 2021 09:43:00 +0000 (10:43 +0100)]
Use int type to store result from isc_commandline_parse()
The C standard actually doesn't define char as signed or unsigned, and
it could be either according to underlying architecture. It turns out
that while it's usually signed type, it isn't on arm64 where it's
unsigned.
isc_commandline_parse() return int, just use that instead of the char.
Evan Hunt [Mon, 22 Feb 2021 23:17:54 +0000 (15:17 -0800)]
create 'journal' system test
tests that version 1 journal files containing version 1 transaction
headers are rolled forward correctly on server startup, then updated
into version 2 journals. also checks journal file consistency and
'max-journal-size' behavior.
Evan Hunt [Wed, 3 Mar 2021 22:59:30 +0000 (14:59 -0800)]
print journal index data and test for consistency
'named-journalprint -x' now prints the journal's index table and
the offset of each transaction in the journal, so that index consistency
can be confirmed.
Evan Hunt [Fri, 19 Feb 2021 23:04:50 +0000 (15:04 -0800)]
allow dns_journal_rollforward() to read old journal files
when the 'max-ixfr-ratio' option was added, journal transaction
headers were revised to include a count of RR's in each transaction.
this made it impossible to read old journal files after an upgrade.
this branch restores the ability to read version 1 transaction
headers. when rolling forward, printing journal contents, if
the wrong transaction header format is found, we can switch.
when dns_journal_rollforward() detects a version 1 transaction
header, it returns DNS_R_RECOVERABLE. this triggers zone_postload()
to force a rewrite of the journal file in the new format, and
also to schedule a dump of the zone database with minimal delay.
journal repair is done by dns_journal_compact(), which rewrites
the entire journal, ignoring 'max-journal-size'. journal size is
corrected later.
newly created journal files now have "BIND LOG V9.2" in their headers
instead of "BIND LOG V9". files with the new version string cannot be
read using the old transaction header format. note that this means
newly created journal files will be rejected by older versions of named.
named-journalprint now takes a "-x" option, causing it to print
transaction header information before each delta, including its
format version.
Ondřej Surý [Mon, 1 Mar 2021 13:21:05 +0000 (14:21 +0100)]
Call isc__initialize()/isc__shutdown() from win32 DllMain
Call the libisc isc__initialize() constructor and isc__shutdown()
destructor from DllMain instead of having duplicate code between
those and DllMain() code.
Ondřej Surý [Thu, 25 Feb 2021 10:08:34 +0000 (11:08 +0100)]
Add mempool get/put tracking with AddressSanitizer
When AddressSanitizer is in use, disable the internal mempool
implementation and redirect the isc_mempool_get to isc_mem_get
(and similarly for isc_mempool_put). This is the method recommended
by the AddressSanitizer authors for tracking allocations and
deallocations instead of custom poison/unpoison code (see
https://github.com/google/sanitizers/wiki/AddressSanitizerManualPoisoning).
Ondřej Surý [Tue, 16 Feb 2021 18:54:04 +0000 (19:54 +0100)]
Change the isc_thread_self() return type to uintptr_t
The pthread_self(), thrd_current() or GetCurrentThreadId() could
actually be a pointer, so we should rather convert the value into
uintptr_t instead of unsigned long.
Ondřej Surý [Tue, 16 Feb 2021 15:02:51 +0000 (16:02 +0100)]
Use globally assigned thread_id in the isc_hp API
Convert the isc_hp API to use the globally available isc_tid_v instead
of locally defined tid_v. This should solve most of the problems on
machines with many number of cores / CPUs.
Ondřej Surý [Tue, 16 Feb 2021 14:57:39 +0000 (15:57 +0100)]
Add isc_trampoline API to have simple accounting around threads
The current isc_hp API uses internal tid_v variable that gets
incremented for each new thread using hazard pointers. This tid_v
variable is then used as a index to global shared table with hazard
pointers state. Since the tid_v is only incremented and never
decremented the table could overflow very quickly if we create set of
threads for short period of time, they finish the work and cease to
exist. Then we create identical set of threads and so on and so on.
This is not a problem for a normal `named` operation as the set of
threads is stable, but the problematic place are the unit tests where we
test network manager or other APIs (task, timer) that create threads.
This commits adds a thin wrapper around any function called from
isc_thread_create() that adds unique-but-reusable small digit thread id
that can be used as index to f.e. hazard pointer tables. The trampoline
wrapper ensures that the thread ids will be reused, so the highest
thread_id number doesn't grow indefinitely when threads are created and
destroyed and then created again. This fixes the hazard pointer table
overflow on machines with many cores. [GL #2396]
Matthijs Mekking [Mon, 22 Feb 2021 11:08:49 +0000 (12:08 +0100)]
Don't servfail on staleonly lookups
When a staleonly lookup doesn't find a satisfying answer, it should
not try to respond to the client.
This is not true when the initial lookup is staleonly (that is when
'stale-answer-client-timeout' is set to 0), because no resolver fetch
has been created at this point. In this case continue with the lookup
normally.
Matthijs Mekking [Thu, 18 Feb 2021 15:09:41 +0000 (16:09 +0100)]
Don't allow recursion on staleonly lookups
Fix a crash that can happen in the following scenario:
A client request is received. There is no data for it in the cache,
(not even stale data). A resolver fetch is created as part of
recursion.
Some time later, the fetch still hasn't completed, and
stale-answer-client-timeout is triggered. A staleonly lookup is
started. It will also find no data in the cache.
So 'query_lookup()' will call 'query_gotanswer()' with ISC_R_NOTFOUND,
so this will call 'query_notfound()' and this will start recursion.
We will eventually end up in 'ns_query_recurse()' and that requires
the client query fetch to be NULL:
REQUIRE(client->query.fetch == NULL);
If the previously started fetch is still running this assertion
fails.
The crash is easily prevented by not requiring recursion for
staleonly lookups.
Also remove a redundant setting of the staleonly flag at the end of
'query_lookup_staleonly()' before destroying the query context.
Matthijs Mekking [Wed, 24 Feb 2021 08:35:06 +0000 (09:35 +0100)]
Fix dnssec-policy NSEC3 on dynamic zones
When applying dnssec-policy on a dynamic zone (e.g. that allows Dynamic
Updates), the NSEC3 parameters were put on the queue, but they were
not being processed (until a reload of the zone or reconfiguration).
Process the NSEC3PARAM queue on zone postload when handling a
dynamic zone.
Ondřej Surý [Wed, 24 Feb 2021 05:55:36 +0000 (06:55 +0100)]
Disable safe-guard assertion in DLL_THREAD_ATTACH/DLL_THREAD_DETACH
The BIND 9 libraries on Windows define DllMain() optional entry point
into a dynamic-link library (DLL). When the system starts or terminates
a process or thread, it calls the entry-point function for each loaded
DLL using the first thread of the process.
When the DLL is being loaded into the virtual address space of the
current process as a result of the process starting up, we make a call
to DisableThreadLibraryCalls() which should disable the
DLL_THREAD_ATTACH and DLL_THREAD_DETACH notifications for the specified
dynamic-link library (DLL).
This seems not be the case because we never check the return value of
the DisableThreadLibraryCalls() call, and it could in fact fail. The
DisableThreadLibraryCalls() function fails if the DLL specified by
hModule has active static thread local storage, or if hModule is an
invalid module handle.
In this commit, we remove the safe-guard assertion put in place for the
DLL_THREAD_ATTACH and DLL_THREAD_DETACH events and we just ignore them.
BIND 9 doesn't create/destroy enough threads for it actually to make any
difference, and in fact we do use static thread local storage in the
code.
Michal Nowak [Mon, 22 Feb 2021 12:50:11 +0000 (13:50 +0100)]
Initialize checknames field in dns_view_create()
The 'checknames' field wasn't initialized in dns_view_create(), but it
should otherwise AddressSanitizer identifies the following runtime error
in query_test.c.
runtime error: load of value 190, which is not a valid value for type '_Bool'
projects / thirdparty / bind9.git / log
