After refactoring of `validated()`, the `maybe_destroy()` function is
no longer expected to actually destroy the fetch context when it is
being called, so effectively it only ensures that the validators are
canceled when the context has no more queries and pending events, but
that is redundant, because `maybe_destroy()` `REQUIRE`s that the context
should be in the shutting down state, and the function which sets that
state is already canceling the validators in its own turn.
As a failsafe, to make sure that no validators will be created after
`fctx_doshutdown()` is called, add an early return from `valcreate()` if
the context is in the shutting down state.
The `resolver.c:validated()` function unlinks the current validator from
the fetch's validators list, which can leave it empty, then unlocks
the bucket lock. If, by a chance, the fetch was timed out just before
the `validated()` call, the final timeout callback running in parallel
with `validated()` can find the fetch context with no active fetches
and with an empty validators list and destroy it, which is unexpected
for the `validated()` function and can lead to a crash.
Increase the fetch context's reference count in the beginning of
`validated()` and decrease it when it finishes its work to avoid the
unexpected destruction of the fetch context.
When there are multiple record datasets in a database node of a catalog
zone, and BIND encounters a soft error during processing of a dataset,
it breaks from the loop and doesn't process the other datasets in the
node.
There are cases when this is not desired. For example, the catalog zones
draft version 5 states that there must be a TXT RRset named
`version.$CATZ` with exactly one RR, but it doesn't set a limitation
on possible non-TXT RRsets named `version.$CATZ` existing alongside
with the TXT one. In case when one exists, we will get a processing
error and will not continue the loop to process the TXT RRset coming
next.
Remove the "break" statement to continue processing all record datasets.
(cherry picked from commit 0b2d5490cd)
When processing a catalog zone update, skip processing records with
DNSSEC-related and ZONEMD types, because we are not interested in them
in the context of a catalog zone, and processing them will fail and
produce an unnecessary warning message.
(cherry picked from commit 73d6643137)
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
This commit is adapted because 9.16 has a different approach
of deconsting the variable.
(cherry picked from commit 888ec4e0d4)
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.
(cherry picked from commit 1da91b3ab4)
Since commit bad5a523c2, 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.
(cherry picked from commit 694bc50273)
inet_ntop result should always protect against empty string accepted
without an error. Make additional check to satisfy coverity scans.
(cherry picked from commit 656a0f076f)
Coverity detected issues:
- var_decl: Declaring variable "diff" without initializer.
- uninit_use_in_call: Using uninitialized value "diff.tuples.head" when
calling "dns_diff_clear".
(cherry picked from commit 67e773c93c)
Update the function that synchronizes the CDS and CDNSKEY DELETE
records. It now allows for the possibility that the CDS DELETE record
is published and the CDNSKEY DELETE record is not, and vice versa.
Also update the code in zone.c how 'dns_dnssec_syncdelete()' is called.
With KASP, we still maintain the DELETE records our self. Otherwise,
we publish the CDS and CDNSKEY DELETE record only if they are added
to the zone. We do still check if these records can be signed by a KSK.
This change will allow users to add a CDS and/or CDNSKEY DELETE record
manually, without BIND removing them on the next zone sign.
Note that this commit removes the check whether the key is a KSK, this
check is redundant because this check is also made in
'dst_key_is_signing()' when the role is set to DST_BOOL_KSK.
(cherry picked from commit 3d05c99abb)
Previously, the RPZ updates ran quantized on the main nm_worker loops.
As the quantum was set to 1024, this might lead to service
interruptions when large RPZ update was processed.
Change the RPZ update process to run as the offloaded work. The update
and cleanup loops were refactored to do as little locking of the
maintenance lock as possible for the shortest periods of time and the db
iterator is being paused for every iteration, so we don't hold the rbtdb
tree lock for prolonged periods of time.
(cherry picked from commit f106d0ed2b)
(cherry picked from commit e128b6a951)
Previously dns_rpz_add() were passed dns_rpz_zones_t and index to .zones
array. Because we actually attach to dns_rpz_zone_t, we should be using
the local pointer instead of passing the index and "finding" the
dns_rpz_zone_t again.
Additionally, dns_rpz_add() and dns_rpz_delete() were used only inside
rpz.c, so make them static.
(cherry picked from commit b6e885c97f)
(cherry picked from commit f4cba0784e)
Do a general cleanup of lib/dns/rpz.c style:
* Removed deprecated and unused functions
* Unified dns_rpz_zone_t naming to rpz
* Unified dns_rpz_zones_t naming to rpzs
* Add and use rpz_attach() and rpz_attach_rpzs() functions
* Shuffled variables to be more local (cppcheck cleanup)
(cherry picked from commit 840179a247)
(cherry picked from commit bfee462403)
the value of 'i' in generate could overflow when adding 'step' to
it in the 'for' loop. Use an unsigned int for 'i' which will give
an additional bit and prevent the overflow. The inputs are both
less than 2^31 and and the result will be less than 2^32-1.
(cherry picked from commit 5abdee9004)
In couple places, we have missed INSIST(0) or ISC_UNREACHABLE()
replacement on some branches with UNREACHABLE(). Replace all
ISC_UNREACHABLE() or INSIST(0) calls with UNREACHABLE().
Some ancient versions of clang reported uninitialized memory use false
positive (see https://bugs.llvm.org/show_bug.cgi?id=14461). Since clang
4.0.1 has been long obsoleted, just remove the workarounds.
(cherry picked from commit ae508c17bc)
Historically, the inline keyword was a strong suggestion to the compiler
that it should inline the function marked inline. As compilers became
better at optimising, this functionality has receded, and using inline
as a suggestion to inline a function is obsolete. The compiler will
happily ignore it and inline something else entirely if it finds that's
a better optimisation.
Therefore, remove all the occurences of the inline keyword with static
functions inside single compilation unit and leave the decision whether
to inline a function or not entirely on the compiler
NOTE: We keep the usage the inline keyword when the purpose is to change
the linkage behaviour.
(cherry picked from commit 20f0936cf2)
Previously, the unreachable code paths would have to be tagged with:
INSIST(0);
ISC_UNREACHABLE();
There was also older parts of the code that used comment annotation:
/* NOTREACHED */
Unify the handling of unreachable code paths to just use:
UNREACHABLE();
The UNREACHABLE() macro now asserts when reached and also uses
__builtin_unreachable(); when such builtin is available in the compiler.
(cherry picked from commit 584f0d7a7e)
Gcc 7+ and Clang 10+ have implemented __attribute__((fallthrough)) which
is explicit version of the /* FALLTHROUGH */ comment we are currently
using.
Add and apply FALLTHROUGH macro that uses the attribute if available,
but does nothing on older compilers.
In one case (lib/dns/zone.c), using the macro revealed that we were
using the /* FALLTHROUGH */ comment in wrong place, remove that comment.
(cherry picked from commit fe7ce629f4)
In the GSS-TSIG verification code there was an alarming
variable-length array whose size came off the network, from the
signature in the request. It turned out to be safe, because the caller
had previously checked that the signature had a reasonable size.
However, the safety checks are in the generic TSIG implementation, and
the risky VLA usage was in the GSS-specific code, and they are
separated by the DST indirection layer, so it wasn't immediately
obvious that the risky VLA was in fact safe.
In fact this risky VLA was completely unnecessary, because the GSS
signature can be verified in place without being copied to the stack,
like the message covered by the signature. The `REGION_TO_GBUFFER()`
macro backwardly assigns the region in its left argument to the GSS
buffer in its right argument; this is just a pointer and length
conversion, without copying any data. The `gss_verify_mic()` call uses
both message and signature GSS buffers in a read-only manner.
Upcoming LLVM/Clang 15 has marked the ATOMIC_VAR_INIT() as deprecated
breaking the build. In the previous commit, we have removed the use of
ATOMIC_VAR_INIT(), but as that was a prerequisite to using the
--enable-mutexatomic debugging mode, we have to remove the debugging
mode.
The C17 standard deprecated ATOMIC_VAR_INIT() macro (see [1]). Follow
the suite and remove the ATOMIC_VAR_INIT() usage in favor of simple
assignment of the value as this is what all supported stdatomic.h
implementations do anyway:
* MacOSX.plaform: #define ATOMIC_VAR_INIT(__v) {__v}
* Gcc stdatomic.h: #define ATOMIC_VAR_INIT(VALUE) (VALUE)
1. http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1138r0.pdf
(cherry picked from commit f251d69eba)
The clang-format-15 has new option InsertBraces that could add missing
branches around single line statements. Use that to our advantage
without switching to not-yet-released LLVM version to add missing braces
in couple of places.
When caching glue, we need to ensure that there is no closer
source of truth for the name. If the owner name for the glue
record would be answered by a locally configured zone, do not
cache.
When caching additional and glue data *not* from a forwarder, we must
check that there is no "forward only" clause covering the owner name
that would take precedence. Such names would normally be allowed by
baliwick rules, but a "forward only" zone introduces a new baliwick
scope.
If we are using a fowarder, in addition to checking that names to
be cached are subdomains of the forwarded namespace, we must also
check that there are no subsidiary forwarded namespaces which would
take precedence. To be safe, we don't cache any responses if the
forwarding configuration has changed since the query was sent.
Previously, the function(s) in the commit subject could fail for various
reasons - mostly allocation failures, or other functions returning
different return code than ISC_R_SUCCESS. Now, the aforementioned
function(s) cannot ever fail and they would always return ISC_R_SUCCESS.
Change the function(s) to return void and remove the extra checks in
the code that uses them.
(cherry picked from commit bbb4cdb92d)
Previously, the function(s) in the commit subject could fail for various
reasons - mostly allocation failures, or other functions returning
different return code than ISC_R_SUCCESS. Now, the aforementioned
function(s) cannot ever fail and they would always return ISC_R_SUCCESS.
Change the function(s) to return void and remove the extra checks in
the code that uses them.
(cherry picked from commit d128656d2e)
Previously, the function(s) in the commit subject could fail for various
reasons - mostly allocation failures, or other functions returning
different return code than ISC_R_SUCCESS. Now, the aforementioned
function(s) cannot ever fail and they would always return ISC_R_SUCCESS.
Change the function(s) to return void and remove the extra checks in
the code that uses them.
(cherry picked from commit 8fa27365ec)
BIND unconditionally uses shims for BN_GENCB_new(), BN_GENCB_free(),
and BN_GENCB_get_arg() for all LibreSSL versions and, correctly, for
OpenSSL <1.1.0 versions.
This breaks LibreSSL compilation starting with LibreSSL 3.5.0.
Use autoconf check instead to check whether the family of the functions
are available.
(cherry picked from commit 749973f3259b7638a6af02b7da2f40ae28bdd402)
By default C promotes short unsigned values to signed int which
leads to undefined behaviour when the value is shifted by too much.
Force unsigned arithmetic to be perform by explicitly casting to a
unsigned type.
(cherry picked from commit b8b99603f1)
Apparently we forgot about DLZ when updating DNS_CLIENTINFO_VERSION
constant for ECS, which is at value "3" since ECS was introduced.
The code in example drivers and tests now hardcodes version numbers
2 (without ECS) and 3 (with ECS) depending on what a given code path
requires.
(cherry picked from commit f81debe1c8)
this brings DNS_CLIENTINFO_VERSION into line with the subscription
branch so that fixes applied to clientinfo processing can also be
applied to the main branch without diverging.
(cherry picked from commit 737e658602)
dns_dlzcreate() fails to free the memory allocated for dlzname
when an error occurs.
Free dlzname's memory (acquired earlier with isc_mem_strdup())
by calling isc_mem_free() before returning an error code.
(cherry picked from commit 4a6c66288f)
When a zone is being configured with a new view, the catalog zones
structure will also be linked to that view. Later on, in case of some
error, should the zone be reverted to the previous view, the link
between the catalog zones structure and the view won't be reverted.
Change the dns_zone_setviewrevert() function so it calls
dns_zone_catz_enable() during a zone revert, which will reset the
link between `catzs` and view.
(cherry picked from commit 2fd967136a)
Separate the locked parts of dns_zone_catz_enable() and
dns_zone_catz_disable() functions into static functions. This will
let us perform those tasks from the other parts of the module while
the zone is locked, avoiding one pair of additional unlocking and
locking operations.
(cherry picked from commit 6b937ed5f6)
This commit converts the license handling to adhere to the REUSE
specification. It specifically:
1. Adds used licnses to LICENSES/ directory
2. Add "isc" template for adding the copyright boilerplate
3. Changes all source files to include copyright and SPDX license
header, this includes all the C sources, documentation, zone files,
configuration files. There are notes in the doc/dev/copyrights file
on how to add correct headers to the new files.
4. Handle the rest that can't be modified via .reuse/dep5 file. The
binary (or otherwise unmodifiable) files could have license places
next to them in <foo>.license file, but this would lead to cluttered
repository and most of the files handled in the .reuse/dep5 file are
system test files.
(cherry picked from commit 58bd26b6cf)