Mark Andrews [Tue, 4 Apr 2023 01:01:36 +0000 (11:01 +1000)]
Remove 'inst != NULL' from cleanup check in plugin_register
'inst' is guarenteed to be non NULL at this point.
358 *instp = inst;
359
360cleanup:
CID 281450 (#2 of 2): Dereference before null check (REVERSE_INULL)
check_after_deref: Null-checking inst suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
361 if (result != ISC_R_SUCCESS && inst != NULL) {
362 plugin_destroy((void **)&inst);
363 }
364
365 return (result);
Bump the requirement in the shutdown test to dnspython 2.0.0
The dnspython.Resolve.resolve() requires at least dnspython >= 2.0.0,
this wasn't enforced in the shutdown system test leading to infinite
loop waiting for the server start due to failing resolve() call.
Ondřej Surý [Wed, 22 Mar 2023 14:11:17 +0000 (15:11 +0100)]
Add test for RPZ in multiple views
This adds rudimentary test for response-policy zones in multiple
views. Different combinations are tested:
- two views with response-policy inherited from options {};
- two views view explicit response-policy using same RPZ zone name
- two views view explicit response-policy using secondary RPZ zone
Ondřej Surý [Thu, 30 Mar 2023 19:19:17 +0000 (21:19 +0200)]
Change dns_adbentry_overquota() to dns_adb_overquota()
The dns_adbentry_overquota() was violating the layers accessing the
adbentry struct members directly. Change it to dns_adb_overquota() to
match the dns_adb API.
Attach catzs to catz instead of doing this explicitly
Instead of explicitly adding a reference to catzs (catalog zones) when
calling the update callback, attach the catzs to the catz (catalog zone)
object to keep it referenced for the whole time the catz exists.
As we are now using dispatch instead of netmgr for XFR TCP connection,
the xfrin_recv_done() will be called when cancelling the dispatch with
ISC_R_CANCELED. This could lead to double detach from the dns_xfrin_t,
one in the xfrin_recv_done() and one in the dns_xfrin_shutdown().
Remove the extra detach from the dns_xfrin_shutdown() and rely on the
dispatch read callback to be always called.
use ISC_REFCOUNT_IMPL for external dns_zone references
use the ISC_REFCOUNT implementation for dns_zone_attach() and
_detach(). (this applies only to external zone references, not
to dns_zone_iattach() and dns_zone_idetach().)
use dns_zone_ref() where previously a dummy zone object had been
used to increment the reference count.
Mark Andrews [Thu, 24 Nov 2022 03:18:20 +0000 (14:18 +1100)]
Reduce the number of verifiations required
In selfsigned_dnskey only call dns_dnssec_verify if the signature's
key id matches a revoked key, the trust is pending and the key
matches a trust anchor. Previously named was calling dns_dnssec_verify
unconditionally resulted in busy work.
Aram Sargsyan [Wed, 28 Dec 2022 10:20:37 +0000 (10:20 +0000)]
nsupdate: use the configurable timeout and retry values for all queries
The 'nsupdate' tool, when sending SOA queries, uses a hard-coded value
3 UDP retries and of 5 seconds of timeout for UDP queries, and 100
seconds of timeout for TCP queries.
Use the timeout and retry values which can be configured using the
-t, -u, -r command line options, and which are already used for
sending the update query.
Aram Sargsyan [Mon, 12 Dec 2022 12:12:13 +0000 (12:12 +0000)]
Do not resend TCP requests
The req_response() function is using 'udpcount' variable to resend
the request 'udpcount' times on timeout even for TCP requests,
which does not make sense, as it would use the same connection.
Add a condition to use the resend logic only for UDP requests.
Aram Sargsyan [Mon, 12 Dec 2022 11:55:09 +0000 (11:55 +0000)]
Synchronize dns_request_createraw() and dns_request_create() UDP timeout
The dns_request_createraw() function, unlike dns_request_create(), when
calculating the UDP timeout value, doesn't check that 'udpretries' is
not zero, and that is the more logical behavior, because the calculation
formula uses division to 'udpretries + 1', where '1' is the first try.
Change the dns_request_create() function to remove the 'udpretries != 0'
condition.
Add a 'REQUIRE(udpretries != UINT_MAX)' check to protect from a division
by zero.
Make the 'request->udpcount' field to represent the number of tries,
instead of the number of retries.
Aram Sargsyan [Wed, 28 Dec 2022 15:55:43 +0000 (15:55 +0000)]
Add nsupdate timeout tests
* nsupdate should take 12 seconds (one try and three retries with
3 second timeout for each), UDP mode
* nsupdate -u 4 -r 1 should take 8 seconds (one try and one retry with
4 second timeout for each), UDP mode
* nsupdate -u 0 -t 8 -r 1 should also take 8 seconds, UDP mode
* nsupdate -u 4 -t 30 -r 1 should also take 8 seconds, as -u takes
precedence over -t, UDP mode
* nsupdate -t 8 -v should also take 8 seconds, TCP mode
Tony Finch [Mon, 3 Apr 2023 13:20:20 +0000 (14:20 +0100)]
More dns_qpkey_t safety checks
My original idea had been that the core qp-trie code would be mostly
independent of the storage for keys, so I did not make it check at run
time that key lengths are sensible. However, the qp-trie search
routines need to get keys out of leaf objects, for which they provide
storage on the stack, which is particularly dangerous for unchecked
buffer overflows. So this change checks that key lengths are in bounds
at the API boundary between the qp-trie code and the rest of BIND, and
there is no more pretence that keys might be longer.
Matthijs Mekking [Wed, 29 Mar 2023 09:26:22 +0000 (11:26 +0200)]
Make checkds system test more resilient
The checkds system test could fail if some parent secondary servers did
not yet loaded all the zones before ns9 started sending DS queries. This
leads to SERVFAIL responses, while the test case expects good DS
responses. In order to mitigate against this issue, call 'rndc loadkeys'
to quickly restart the checkds procedure again.
Also refactor the checkds system test, to get rid of the many zone
name duplications. Update the functions 'zone_check' and
'keystate_check' to make the zone name an FQDN so we can just pass
the 'zone' variable into the function.
Matthijs Mekking [Tue, 28 Mar 2023 14:57:58 +0000 (16:57 +0200)]
Determine checkds default from config
If the 'checkds' option is not explicitly set, check if there are
'parental-agents' for the zone configured. If so, default to "explicit",
otherwise default to "yes".
Matthijs Mekking [Tue, 28 Mar 2023 14:47:16 +0000 (16:47 +0200)]
Add two checkds test servers
Add two new checkds test servers, that are hidden secondaries (hidden
as in not published in the NS RRset), that can be used specifically
for testing explicitly configured parental-agents.
Matthijs Mekking [Tue, 28 Mar 2023 13:55:51 +0000 (15:55 +0200)]
Implement auto parental-agents (checkds yes)
Implement the new feature, automatic parental-agents. This is enabled
with 'checkds yes'.
When set to 'yes', instead of querying the explicit configured
parental agents, look up the parental agents by resolving the parent
NS records. The found parent NS RRset is considered to be the list
of parental agents that should be queried during a KSK rollover,
looking up the DS RRset corresponding to the key signing keys.
For each NS record, look up the addresses in the ADB. These addresses
will be used to send the DS requests. Count the number of servers and
keep track of how many good DS responses were seen.
Matthijs Mekking [Tue, 28 Mar 2023 12:35:57 +0000 (14:35 +0200)]
Add test cases for 'checkds no'
Add test cases for when checkds is disabled. Copy the test cases that
would have resulted in a DSPublish or DSRemoved and make sure that
with 'checkds no' the metadata is not set.
Matthijs Mekking [Tue, 28 Mar 2023 10:00:56 +0000 (12:00 +0200)]
Add test cases for 'checkds yes'
Add the test cases for automatic parental-agents, i.e. when 'checkds'
is set to 'yes'. Split out the special cases that use a reference
or a resolver as parental-agent so that the common use cases can be
tested with the same function.
Matthijs Mekking [Fri, 24 Mar 2023 16:22:24 +0000 (17:22 +0100)]
Update checkds system test
Make the checkds system test more structured with the many more test
cases to come. Add a README for clarity.
Update the 'has_signed_apex_nsec' helper function so it can take any
domain name regardless of the number of labels.
Change the DNS tree structure such that we have different TLD names
for the various test scenarios, because we need servers that respond
differently to DS queries. Note that this isn't applicable to the
existing "checkds explicit" test cases, but is preparation work for
testing "checkds yes" (automatic parental agents).
Add a trust-anchor to the server that will be querying for parent
NS records.
Mark Andrews [Wed, 8 Mar 2023 05:05:03 +0000 (16:05 +1100)]
Silence NULL pointer dereferene false positive
Only attempt to digest 'in' if it is non NULL. This will prevent
false positives about NULL pointer dereferences against 'in' and
should also speed up the processing.
Artem Boldariev [Mon, 26 Dec 2022 15:42:49 +0000 (17:42 +0200)]
Stream DNS: avoid memory copying/buffer resizing when reading data
This commit optimises isc_dnsstream_assembler_t in such a way that
memory copying and reallocation are avoided when receiving one or more
complete DNS messages at once. We try to handle the data from the
messages directly, without storing them in an intermediate memory
buffer.
In write_public_key() and write_key_state(), there were left-over checks
for result, that were effectively dead code after the last refactoring.
Remove those.
Tony Finch [Thu, 16 Mar 2023 14:31:46 +0000 (14:31 +0000)]
Use isc_histo for the message size statistics
This should have no functional effects.
The message size stats are specified by RSSAC002 so it's best not
to mess around with how they appear in the statschannel. But it's
worth changing the implementation to use general-purpose histograms,
to reduce code size and benefit from sharded counters.
Tony Finch [Mon, 20 Mar 2023 15:05:36 +0000 (15:05 +0000)]
Simplify histogram quantiles
The `isc_histosummary_t` functions were written in the early days of
`hg64` and carried over when I brought `hg64` into BIND. They were
intended to be useful for graphing cumulative frequency distributions
and the like, but in practice whatever draws charts is better off with
a raw histogram export. Especially because of the poor performance of
the old functions.
The replacement `isc_histo_quantiles()` function is intended for
providing a few quantile values in BIND's stats channel, when the user
does not want the full histogram. Unlike the old functions, the caller
provides all the query fractions up-front, so that the values can be
found in a single scan instead of a scan per value. The scan is from
larger values to smaller, since larger quantiles are usually more
interesting, so the scan can bail out early.
Tony Finch [Thu, 16 Mar 2023 09:46:15 +0000 (09:46 +0000)]
Add per-thread sharded histograms for heavy loads
Although an `isc_histo_t` is thread-safe, it can suffer
from cache contention under heavy load. To avoid this,
an `isc_histomulti_t` contains a histogram per thread,
so updates are local and low-contention.
Tony Finch [Wed, 8 Mar 2023 09:55:42 +0000 (09:55 +0000)]
Add isc_histo for histogram statistics
This is an adaptation of my `hg64` experiments for use in BIND.
As well as renaming everything according to ISC style, I have
written some more extensive tests that ensure the edge cases are
correct and the fenceposts are in the right places.
I have added utility functions for working with precision in terms of
decimal significant figures as well as this code's native binary.
Remove the reference to setting the DF-flag as we don't do that right
now. Rephrase the paragraph that the default value should not be
causing fragmentation.
Ondřej Surý [Thu, 30 Mar 2023 15:35:00 +0000 (17:35 +0200)]
Cleanup the last Windows / MSC ifdefs and comments
Cleanup the remnants of MS Compiler bits from <isc/refcount.h>, printing
the information in named/main.c, and cleanup some comments about Windows
that no longer apply.
The bits in picohttpparser.{h,c} were left out, because it's not our
code.
Tom Krizek [Mon, 13 Mar 2023 12:36:24 +0000 (13:36 +0100)]
Find errors in dig output in system tests
Facilitate faster system test failure identification and debugging by
checking any dig outputs for errors, which are typically indicative of
CI runner network / load issues.
Mark Andrews [Tue, 21 Mar 2023 03:29:46 +0000 (14:29 +1100)]
Don't check for OPENSSL_cleanup failures by default
OPENSSL_cleanup is supposed to free all remaining memory in use
provided the application has cleaned up properly. This is not the
case on some operating systems. Silently ignore memory that is
freed after OPENSSL_cleanup has been called.
Mark Andrews [Fri, 27 Jan 2023 05:52:59 +0000 (16:52 +1100)]
Handle fatal and FIPS provider interactions
When fatal is called we may be holding memory allocated by OpenSSL.
This may result in the reference count for the FIPS provider not
going to zero and the shared library not being unloaded during
OPENSSL_cleanup. When the shared library is ultimately unloaded,
when all remaining dynamically loaded libraries are freed, we have
already destroyed the memory context we where using to track memory
leaks / late frees resulting in INSIST being called.
Disable triggering the INSIST when fatal has being called.