Artem Boldariev [Tue, 24 May 2022 08:39:26 +0000 (11:39 +0300)]
CID 352848: split xfrin_start() and remove dead code
This commit separates TLS context creation code from xfrin_start() as
it has become too large and hard to follow into a new
function (similarly how it is done in dighost.c)
The dead code has been removed from the cleanup section of the TLS
creation code:
* there is no way 'tlsctx' can equal 'found';
* there is no way 'sess_cache' can be non-NULL in the cleanup section.
Also, it fixes a bug in the older version of the code, where TLS
client session context fetched from the cache would not get passed to
isc_nm_tlsdnsconnect().
Artem Boldariev [Tue, 24 May 2022 08:25:30 +0000 (11:25 +0300)]
CID 352849: refactor get_create_tls_context() within dighost.c
This commit removes dead code from cleanup handling part of the
get_create_tls_context().
In particular, currently:
* there is no way 'found_ctx' might equal 'ctx';
* there is no way 'session_cache' might equal a non-NULL value while
cleaning up after a TLS initialisation error.
Petr Menšík [Tue, 24 May 2022 17:42:41 +0000 (19:42 +0200)]
Fix failures in isc netmgr_test on big endian machines
Typing from libuv structure to isc_region_t is not possible, because
their sizes differ on 64 bit architectures. Little endian machines seems
to be lucky and still result in test passed. But big endian machine such
as s390x fails the test reliably.
Fix by directly creating the buffer as isc_region_t and skipping the
type conversion. More readable and still more correct.
Disable periodic interface re-scans on modern platforms
This commit disables periodic interface re-scans timer on Linux where
a kernel-based dynamic interface mechanisms make it a thing of the
past in most cases.
Artem Boldariev [Mon, 23 May 2022 10:41:06 +0000 (13:41 +0300)]
Do not provide a shim for SSL_SESSION_is_resumable()
The recently added TLS client session cache used
SSL_SESSION_is_resumable() to avoid polluting the cache with
non-resumable sessions. However, it turned out that we cannot provide
a shim for this function across the whole range of OpenSSL versions
due to the fact that OpenSSL 1.1.0 does uses opaque pointers for
SSL_SESSION objects.
The commit replaces the shim for SSL_SESSION_is_resumable() with a non
public approximation of it on systems shipped with OpenSSL 1.1.0. It
is not turned into a proper shim because it does not fully emulate the
behaviour of SSL_SESSION_is_resumable(), but in our case it is good
enough, as it still helps to protect the cache from pollution.
For systems shipped with OpenSSL 1.0.X and derivatives (e.g. older
versions of LibreSSL), the provided replacement perfectly mimics the
function it is intended to replace.
Matthijs Mekking [Tue, 10 May 2022 13:09:29 +0000 (15:09 +0200)]
Tweak timings in serve-stale system test
Give a little bit more time if we wait on a time out from the
authoritative (aka resolver failure), and give up after one try
(because the second attempt will likely result in a different EDE).
Matthijs Mekking [Tue, 17 May 2022 10:02:43 +0000 (12:02 +0200)]
Require valid key for dst_key functions
Make sure that the key structure is valid when calling the following
functions:
- dst_key_setexternal
- dst_key_isexternal
- dst_key_setmodified
- dst_key_ismodified
Artem Boldariev [Tue, 10 May 2022 20:09:59 +0000 (23:09 +0300)]
Dig: Do not call isc_nm_cancelread() for HTTP sockets
This commit ensures that isc_nm_cancelread() is not called from within
dig code for HTTP sockets, as these lack its implementation.
It does not have much sense to have it due to transactional nature of
HTTP.
Every HTTP request-response pair is represented by a virtual socket,
where read callback is called only when full DNS message is received
or when an error code is being passed there. That is, there is nothing
to cancel at the time of the call.
Extend TLS context cache with TLS client session cache
This commit extends TLS context cache with TLS client session cache so
that an associated session cache can be stored alongside the TLS
context within the context cache.
This commit adds an implementation of a client TLS session cache. TLS
client session cache is an object which allows efficient storing and
retrieval of previously saved TLS sessions so that they can be
resumed. This object is supposed to be a foundation for implementing
TLS session resumption - a standard technique to reduce the cost of
re-establishing a connection to the remote server endpoint.
OpenSSL does server-side TLS session caching transparently by
default. However, on the client-side, a TLS session to resume must be
manually specified when establishing the TLS connection. The TLS
client session cache is precisely the foundation for that.
Ondřej Surý [Tue, 17 May 2022 19:31:37 +0000 (21:31 +0200)]
Move setting the sock->write_timeout to the async_*send
Setting the sock->write_timeout from the TCP, TCPDNS, and TLSDNS send
functions could lead to (harmless) data race when setting the value for
the first time when the isc_nm_send() function would be called from
thread not-matching the socket we are sending to. Move the setting the
sock->write_timeout to the matching async function which is always
called from the matching thread.
Ondřej Surý [Thu, 19 May 2022 19:40:24 +0000 (21:40 +0200)]
Use C2x [[fallthrough]] when supported by LLVM/clang
Clang added support for the gcc-style fallthrough
attribute (i.e. __attribute__((fallthrough))) in version 10. However,
__has_attribute(fallthrough) will return 1 in C mode in older versions,
even though they only support the C++11 fallthrough attribute. At best,
the unsupported attribute is simply ignored; at worst, it causes errors.
The C2x fallthrough attribute has the advantages of being supported in
the broadest range of clang versions (added in version 9) and being easy
to check for support. Use C2x [[fallthrough]] attribute if possible, and
fall back to not using an attribute for clang versions that don't have
it.
Evan Hunt [Sun, 8 May 2022 03:58:19 +0000 (20:58 -0700)]
Always use the number of CPUS for resolver->ntasks
Since the fctx hash table is now self-resizing, and resolver tasks are
selected to match the thread that created the fetch context, there
shouldn't be any significant advantage to having multiple tasks per CPU;
a single task per thread should be sufficient.
Additionally, the fetch context is always pinned to the calling netmgr
thread to minimize the contention just to coalesced fetches - if two
threads starts the same fetch, it will be pinned to the first one to get
the bucket.
Tony Finch [Tue, 17 May 2022 12:13:57 +0000 (14:13 +0200)]
Teach dnssec-settime to read unset times that it writes
When there is no time in a key file, `dnssec-settime` will print
"UNSET", but to unset a time the user must specify "none" or "never".
This change allows "unset" or "UNSET" as well as "none" or "never".
The "UNSET" output remains the same to avoid compatibility problems
with wrapper scripts.
I have also re-synchronized the "Timing Options" sections of the man
pages.
Ondřej Surý [Mon, 16 May 2022 11:28:13 +0000 (13:28 +0200)]
Cleanup dns_message_gettemp*() functions - they cannot fail
The dns_message_gettempname(), dns_message_gettemprdata(),
dns_message_gettemprdataset(), and dns_message_gettemprdatalist() always
succeeds because the memory allocation cannot fail now. Change the API
to return void and cleanup all the use of aforementioned functions.
Matthijs Mekking [Mon, 16 May 2022 09:23:15 +0000 (11:23 +0200)]
Replace stat with PERL stat in kasp system test
7249bad7 introduced the -c option to stat(1) command, but BSD systems
do not know about it. Replace the stat(1) command with a PERL script
that achieves the same.
Why PERL? For consistency purposes, there are more places in the
system test where we use the same method.
Evan Hunt [Mon, 9 May 2022 00:17:29 +0000 (17:17 -0700)]
Add lower bound checks to fetchlimit test
Check that the recursing client count is above a reasonable
minimum, as well as below a maximum, so that we can detect
bugs that cause recursion to fail too early or too often.
The ns_statscounter_recursclients counter was previously only
incremented or decremented if client->recursionquota was non-NULL.
This was harmless, because that value should always be non-NULL if
recursion is enabled, but it made the code slightly confusing.
Evan Hunt [Thu, 5 May 2022 21:52:15 +0000 (14:52 -0700)]
Disable EDNS for the fetchlimit test server
The fetchlimit test depends on a resolver continuing to try UDP
and timing out while the client waits for resolution to succeed.
but since commit bb990030 (flag day 2020), a fetch will always
switch to TCP after two timeouts, unless EDNS was disabled for
the query.
This commit adds "edns no;" to server statements in the fetchlimit
resolver, to restore the behavior expected by the test.
Evan Hunt [Thu, 5 May 2022 00:27:56 +0000 (17:27 -0700)]
Fix the fetches-per-server quota calculation
Since commit bad5a523c2e, when the fetches-per-server quota
was increased or decreased, instead of the value being set to
the newly calculated quota, it was set to the *minimum* of
the new quota or 1 - which effectively meant it was always set to 1.
it should instead have been the maximum, to prevent the value from
ever dropping to zero.
Evan Hunt [Thu, 12 May 2022 22:51:10 +0000 (15:51 -0700)]
attach the resolver to the ADB
the ADB depends on the resolver, but previously only accessed it
via the view. as view->resolver may now be detached before the ADB
finishes, a shutdown race was possible. attaching to the resolver
directly prevents this.
Evan Hunt [Wed, 11 May 2022 20:55:01 +0000 (13:55 -0700)]
remove requestmgr whenshutdown events
the request manager has no direct dependency on the
view, so there's no need for a weak reference. remove the
dns_requestmgr_whenshutdown() mechanism since it is no longer
used.
Evan Hunt [Wed, 11 May 2022 20:55:01 +0000 (13:55 -0700)]
remove resolver whenshutdown events
weakly attaching and detaching when creating and destroying the
resolver obviates the need to have a callback event to do the weak
detach. remove the dns_resolver_whenshutdown() mechanism, as it is
now unused.
Evan Hunt [Wed, 11 May 2022 20:55:01 +0000 (13:55 -0700)]
remove ADB whenshutdown events
weakly attaching and detaching the view when creating or destroying
the ADB obviates the need for a whenshutdown callback event to do
the detaching. remove the dns_adb_whenshutdown() mechanism, since
it is no longer needed.
Evan Hunt [Wed, 11 May 2022 22:38:54 +0000 (15:38 -0700)]
move ADB and resolver stats out of the view object
for better object separation, ADB and resolver statistics counters
are now stored in the ADB and resolver objects themsevles, rather than
in the associated view.
Evan Hunt [Wed, 11 May 2022 21:18:45 +0000 (14:18 -0700)]
remove view->attributes field
it's not necessary to use view attributes to determine whether
the ADB, resolver and requestmgr need to be shut down; checking
whether they're NULL is sufficient.
Evan Hunt [Wed, 11 May 2022 19:15:46 +0000 (12:15 -0700)]
the validator can attach to the view normally
dns_view_weakattach() and _weakdetach() are used by objects that
are created by the view and need it to persist until they are
destroyed, but don't need to prevent it from being shut down. the
validator can use normal view references instead of weak references.
Evan Hunt [Tue, 10 May 2022 19:48:31 +0000 (12:48 -0700)]
minor view refactoring
- eliminate dns_view_flushanddetach(), which was only called from
one place; instead, we now call a function dns_view_flushonshutdown()
which sets the view up to flush zones when it is detached normally
with dns_view_detach().
- cleaned up code in dns_view_create().
Evan Hunt [Mon, 9 May 2022 19:55:07 +0000 (12:55 -0700)]
maybe_cancel_validators() is always called locked
there's no longer any need for a parameter to specify whether the
function is called while holding the bucket lock, because all
unlocked uses have been removed.
Add a new parameter to the dst_key structure, mark a key modified if
dst_key_(un)set[bool,num,state,time] is called. Only write out key
files during a keymgr run if the metadata has changed.
Add a test case that triggers a keymgr run that will not trigger any
metadata changes. Ensure that the last status change of the key files
is unmodified.
Ondřej Surý [Wed, 4 May 2022 07:26:34 +0000 (09:26 +0200)]
Lock the trampoline when attaching
When attaching to the trampoline, the isc__trampoline_max was access
unlocked. This would not manifest under normal circumstances because we
initialize 65 trampolines by default and that's enough for most
commodity hardware, but there are ARM machines with 128+ cores where
this would be reported by ThreadSanitizer.
Add locking around the code in isc__trampoline_attach(). This also
requires the lock to leak on exit (along with memory that we already)
because a new thread might be attaching to the trampoline while we are
running the library destructor at the same time.
Ondřej Surý [Mon, 9 May 2022 09:33:09 +0000 (11:33 +0200)]
Remove isc_task_onshutdown()
The isc_task_onshutdown() was used to post event that should be run when
the task is being shutdown. This could happen explicitly in the
isc_test_shutdown() call or implicitly when we detach the last reference
to the task and there are no more events posted on the task.
This whole task onshutdown mechanism just makes things more complicated,
and it's easier to post the "shutdown" events when we are shutting down
explicitly and the existing code already always knows when it should
shutdown the task that's being used to execute the onshutdown events.
Replace the isc_task_onshutdown() calls with explicit calls to execute
the shutdown tasks.
Michał Kępień [Wed, 11 May 2022 12:59:47 +0000 (14:59 +0200)]
Drop post-merge pipelines
Commit 24961f606882939b62f461f843933da9c2d44ea8 introduced the concept
of post-merge pipelines. These were used to build documentation after
each merge to any of the maintained branches, to allow up-to-date
documentation to be subsequently published on the bind.isc.org
microsite. As the latter no longer downloads documentation from GitLab,
instead linking to Read the Docs for all currently maintained branches,
post-merge pipelines have become redundant since running the
"autoreconf" and "docs" jobs after each merge is not necessary any more.
Drop the "release_branch_triggering_rules" YAML anchor and revert to
default triggering rules for the "autoconf" and "docs" jobs, effectively
preventing pipelines from being created after each merge to any of the
maintained branches.
Michał Kępień [Wed, 11 May 2022 12:10:17 +0000 (14:10 +0200)]
Do not set up ccache in CI jobs building docs
Building documentation for Automake-based BIND 9 versions does not
require running "make all" beforehand. Drop ccache setup code from the
definitions of GitLab CI jobs building documentation as it is redundant.
Michał Kępień [Wed, 11 May 2022 12:10:17 +0000 (14:10 +0200)]
Do not trigger GitLab Pages pipelines any more
The bind.isc.org microsite no longer downloads documentation from
GitLab, instead linking to Read the Docs for all currently maintained
branches. This makes it redundant to trigger GitLab Pages pipelines
after each merge to any of the maintained branches. Revert changes
introduced by commit 31bde118db8cae4a0f5a9484e3beead66fcc24d0: remove
the "push:docs" job along with the "push" stage it is associated with
and revert artifact expiry time for the "docs" job to the default value.