When looking for changes in a catalog zone member zone we need to
also check if the TSIG key name associated with a primary server
has be added, removed or changed.
(cherry picked from commit 9172bd9b5a)
dns_rdata_tostruct doesn't need a mctx passed to it for SIG (the signer
is already expanded at this point). About the only time when mctx is
needed is when the structure is to be used after the rdata has been
destroyed.
(cherry picked from commit d6ad56bd9e)
Impact should be visible only in tests or tools because named never
uses view == NULL, which is a necessary condition to trigger this leak.
(cherry picked from commit 69256b3553)
Limit the amount of database lookups that can be triggered in
fctx_getaddresses() (i.e. when determining the name server addresses to
query next) by setting a hard limit on the number of NS RRs processed
for any delegation encountered. Without any limit in place, named can
be forced to perform large amounts of database lookups per each query
received, which severely impacts resolver performance.
The limit used (20) is an arbitrary value that is considered to be big
enough for any sane DNS delegation.
(cherry picked from commit 3a44097fd6)
It is possible to bypass Response Rate Limiting (RRL)
`responses-per-second` limitation using specially crafted wildcard
names, because the current implementation, when encountering a found
DNS name generated from a wildcard record, just strips the leftmost
label of the name before making a key for the bucket.
While that technique helps with limiting random requests like
<random>.example.com (because all those requests will be accounted
as belonging to a bucket constructed from "example.com" name), it does
not help with random names like subdomain.<random>.example.com.
The best solution would have been to strip not just the leftmost
label, but as many labels as necessary until reaching the suffix part
of the wildcard record from which the found name is generated, however,
we do not have that information readily available in the context of RRL
processing code.
Fix the issue by interpreting all valid wildcard domain names as
the zone's origin name concatenated to the "*" name, so they all will
be put into the same bucket.
(cherry picked from commit baa9698c9d)
The dnstap query_message field was in some cases being filled in
with response messages, along with the response_message field.
The query_message field should only be used when logging requests,
and the response_message field only when logging responses.
There is one case in 'dns_nsec3_activex()' where it returns but forgets
to detach the db node. Add the missing 'dns_db_detachnode()' call.
This case only triggers if 'sig-signing-type' (privatetype) is set to 0
(which by default is not), or if the function is called with 'complete'
is set to 'true' (which at this moment do not exist).
(cherry picked from commit 0cf6c18ccb2205a1fc81431f908c8310f6136bbb)
When dumping an ADB address entry associated with a name,
the name bucket lock was held, but the entry bucket lock was
not; this could cause data races when other threads were updating
address entry info. (These races are probably not operationally
harmful, but they triggered TSAN error reports.)
(cherry picked from commit f841f545b7)
When initially hitting the `fetches-per-zone` value, a log message
is being generated for the event of dropping the first fetch, then
any further log events occur only when another fetch is being dropped
and 60 seconds have been passed since the last logged message.
That logic isn't ideal because when the counter of the outstanding
fetches reaches zero, the structure holding the counters' values will
get deleted, and the information about the dropped fetches accumulated
during the last minute will not be logged.
Improve the fcount_logspill() function to makie sure that the final
values are getting logged before the counter object gets destroyed.
(cherry picked from commit 039871ceb767088205563965f7aae622a3f77082)
The command 'rndc dumpdb -expired' will include expired RRsets in the
output, but only for the RBTDB_VIRTUAL time (of 5 minutes). This means
that if there is a cache cleaning problem and contents are not cleaned
up, the rndc command has little diagnostic value. Fix this by including
all RRsets in the dumpdb output if the '-expired' flag is set.
(cherry picked from commit 930ba2c914a0abc07fd087d663a7bfb57850d4ca)
Fedora 33 doesn't support RSASHA1 in future mode. There is no easy
check for this other than by attempting to perform a verification
using known good signatures. We don't attempt to sign with RSASHA1
as that would not work in FIPS mode. RSASHA1 is verify only.
The test vectors were generated using OpenSSL 3.0 and
util/gen-rsa-sha-vectors.c. Rerunning will generate a new set of
test vectors as the private key is not preserved.
e.g.
cc util/gen-rsa-sha-vectors.c -I /opt/local/include \
-L /opt/local/lib -lcrypto
(cherry picked from commit cd3f00874f63a50954cebb78edac8f580a27c0de)
The BUFSIZ value varies between platforms, it could be 8K on Linux and
512 bytes on mingw. Make sure the buffers are always big enough for the
output data to prevent truncation of the output by appropriately
enlarging or sizing the buffers.
(cherry picked from commit b19d932262e84608174cb89eeed32ae0212f8a87)
free_namelist could be passed names with associated rdatasets
when handling errors. These need to be disassociated before
calling dns_message_puttemprdataset.
(cherry picked from commit 745d5edc3a8ca6f232b2d700ae076c2caee2bfc5)
fctx_decreference() may call fctx_destroy(), which in turn may free the
fetch context by calling isc_mem_putanddetach(). This means that
whenever fctx_decreference() is called, the fetch context pointer should
be assumed to point to garbage after that call. Meanwhile, the
following pattern is used in several places in lib/dns/resolver.c:
LOCK(&res->buckets[fctx->bucketnum].lock);
bucket_empty = fctx_decreference(fctx);
UNLOCK(&res->buckets[fctx->bucketnum].lock);
Given that 'fctx' may be freed by the fctx_decreference() call, there is
no guarantee that the value of fctx->bucketnum will be the same before
and after the fctx_decreference() call. This can cause all kinds of
locking issues as LOCK() calls no longer match up with their UNLOCK()
counterparts.
Fix by always using a helper variable to hold the bucket number when the
pattern above is used.
Note that fctx_try() still uses 'fctx' after calling fctx_decreference()
(it calls fctx_done()). This is safe to do because the reference count
for 'fctx' is increased a few lines earlier and it also cannot be zero
right before that increase happens, so the fctx_decreference() call in
that particular location never invokes fctx_destroy(). Nevertheless,
use a helper variable for that call site as well, to retain consistency
and to prevent copy-pasted code from causing similar problems in the
future.
The original sscanf processing allowed for a number of syntax errors
to be accepted. This included missing the closing brace in
${modifiers}
Look for both comma and right brace as intermediate seperators as
well as consuming the final right brace in the sscanf processing
for ${modifiers}. Check when we got right brace to determine if
the sscanf consumed more input than expected and if so behave as
if it had stopped at the first right brace.
(cherry picked from commit 7be64c0e94)
$GENERATE uses 'int' for its computations and some constructions
can overflow values that can be represented by an 'int' resulting
in undefined behaviour. Detect these conditions and return a
range error.
(cherry picked from commit 5327b9708f)
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().
