Matthijs Mekking [Fri, 20 Aug 2021 13:08:29 +0000 (15:08 +0200)]
Add statschannel test case for key removal
Add a statschannel test case to confirm that when keys are removed
(in this case because of a dnssec-policy change), the corresponding
dnssec-sign stats are cleared and are no longer shown in the
statistics.
Matthijs Mekking [Fri, 20 Aug 2021 13:06:13 +0000 (15:06 +0200)]
Clear dnssec-sign stats for removed keys
Clear the key slots for dnssec-sign statistics for keys that are
removed. This way, the number of slots will stabilize to the maximum
key usage in a zone and will not grow every time a key rollover is
triggered.
Matthijs Mekking [Fri, 20 Aug 2021 09:19:28 +0000 (11:19 +0200)]
Add back the statschannel manykeys test case
Add a test case that has more than four keys (the initial number of
key slots that are created for dnssec-sign statistics). We shouldn't
be expecting weird values.
This fixes some errors in the manykeys zone configuration (keys
were created for algorithm RSASHA256, but the policy expected RSASHA1,
and the zone was not allowing dynamic updates).
This also fixes an error in the calls to 'zones-json.pl': The perl
script excepts an index number where the zone can be found, rather
than the zone name.
Matthijs Mekking [Fri, 20 Aug 2021 09:14:49 +0000 (11:14 +0200)]
Grow dnssec-sign statistics instead of rotating
We have introduced dnssec-sign statistics to the zone statistics. This
introduced an operational issue because when using zone-statistics
full, the memory usage was going through the roof. We fixed this by
by allocating just four key slots per zone. If a zone exceeds the
number of keys for example through a key rollover, the keys will be
rotated out on a FIFO basis.
This works for most cases, and fixes the immediate problem of high
memory usage, but if you sign your zone with many, many keys, or are
sign with a ZSK/KSK double algorithm strategy you may experience weird
statistics. A better strategy is to grow the number of key slots per
zone on key rollover events.
That is what this commit is doing: instead of rotating the four slots
to track sign statistics, named now grows the number of key slots
during a key rollover (or via some other method that introduces new
keys).
Matthijs Mekking [Thu, 19 Aug 2021 11:38:51 +0000 (13:38 +0200)]
Add a function isc_stats_resize
Add a new function to resize the number of counters in a statistics
counter structure. This will be needed when we keep track of DNSSEC
sign statistics and new keys are introduced due to a rollover.
Matthijs Mekking [Thu, 19 Aug 2021 10:14:21 +0000 (12:14 +0200)]
Add stats unit test
Add a simple stats unit test that tests the existing library functions
isc_stats_ncounters, isc_stats_increment, isc_stats_decrement,
isc_stats_set, and isc_stats_update_if_greater.
Matthijs Mekking [Wed, 18 Aug 2021 13:57:03 +0000 (15:57 +0200)]
Change "receive_secure_serial: unchanged" log lvl
After a reload, if the zone hasn't changed, this will log a
DNS_R_UNCHANGED error. This should not be at error level because it
happens on every reload.
Matthijs Mekking [Mon, 16 Aug 2021 09:09:25 +0000 (11:09 +0200)]
Test migrating CSK to dnssec-policy
Add a test case for migrating CSK to dnssec-policy. The keymgr has no
way of telling that the key is used as a CSK, but if there is only one
key to migrate it is going to assume it must be a CSK.
Tony Finch [Thu, 16 Jan 2020 18:50:59 +0000 (18:50 +0000)]
Suppress SHA-1 DS records in dnssec-cds
Previously, when dnssec-cds copied CDS records to make DS records,
its -a algorithm option did not have any effect. This means that if
the child zone is signed with older software that generates SHA-1 CDS
records, dnssec-cds would (by default) create SHA-1 DS records in
violation of RFC 8624.
This change makes the dnssec-cds -a option apply to CDS records as
well as CDNSKEY records. In the CDS case, the -a algorithms are the
acceptable subset of possible CDS algorithms. If none of the CDS
records are acceptable, dnssec-cds tries to generate DS records from
CDNSKEY records.
Instead of disabling the fragmentation on the UDP sockets, we now
disable the Path MTU Discovery by setting IP(V6)_MTU_DISCOVER socket
option to IP_PMTUDISC_OMIT on Linux and disabling IP(V6)_DONTFRAG socket
option on FreeBSD. This option sets DF=0 in the IP header and also
ignores the Path MTU Discovery.
As additional mitigation on Linux, we recommend setting
net.ipv4.ip_no_pmtu_disc to Mode 3:
Mode 3 is a hardend pmtu discover mode. The kernel will only accept
fragmentation-needed errors if the underlying protocol can verify
them besides a plain socket lookup. Current protocols for which pmtu
events will be honored are TCP, SCTP and DCCP as they verify
e.g. the sequence number or the association. This mode should not be
enabled globally but is only intended to secure e.g. name servers in
namespaces where TCP path mtu must still work but path MTU
information of other protocols should be discarded. If enabled
globally this mode could break other protocols.
ns_client_error() could assert if rcode was overridden to NOERROR
The client->rcode_override was originally created to force the server
to send SERVFAIL in some cases when it would normally have sent FORMERR.
More recently, it was used in a3ba95116ed04594ea59a8124bf781b30367a7a2
commit (part of GL #2790) to force the sending of a TC=1 NOERROR
response, triggering a retry via TCP, when a UDP packet could not be
sent due to ISC_R_MAXSIZE.
This ran afoul of a pre-existing INSIST in ns_client_error() when
RRL was in use. the INSIST was based on the assumption that
ns_client_error() could never result in a non-error rcode. as
that assumption is no longer valid, the INSIST has been removed.
Mark Andrews [Fri, 5 Jul 2019 06:20:20 +0000 (16:20 +1000)]
Add additional processing to HTTPS and SVBC records
The additional processing method has been expanded to take the
owner name of the record, as HTTPS and SVBC need it to process "."
in service form.
The additional section callback can now return the RRset that was
added. We use this when adding CNAMEs. Previously, the recursion
would stop if it detected that a record you added already exists. With
CNAMEs this rule doesn't work, as you ultimately care about the RRset
at the target of the CNAME and not the presence of the CNAME itself.
Returning the record allows the caller to restart with the target
name. As CNAMEs can form loops, loop protection was added.
As HTTPS and SVBC can produce infinite chains, we prevent this by
tracking recursion depth and stopping if we go too deep.
Add a test case for GL #2845 where a zone is in two views, one base
view and one "in-view" and that zone is using an $INCLUDE. Make sure
that there is a jnl file (have ixfr-from-differences enabled and do a
dynamic update). Then freeze and make updates in the included file
(this requires the test.db file also to be updated because 'rndc freeze'
causes the zone file to be overwritten). Finally reload and ensure that
the edit in the included file has been loaded.
Matthijs Mekking [Wed, 11 Aug 2021 08:31:56 +0000 (10:31 +0200)]
Don't use stale nodes when looking up a zonecut
When looking up a zonecut in cache, we use 'dns_rbt_findnode' to find
the closest matching node. This function however does not take into
account stale nodes. When we do find a stale node and use it, this
has implications for subsequent lookups. For example, this may break
QNAME minimization because we are using a deeper zonecut than we should
have.
Check the header for staleness and if so, and stale entries are not
accepted, look for the deepest zonecut from this node up.
Matthijs Mekking [Tue, 10 Aug 2021 10:18:12 +0000 (12:18 +0200)]
Add extra checks for !ANCIENT(header)
There are some occurrences where we check if a header exists in the
rbtdb. These cases require that the header is also not marked as
ancient (aka ready for cleanup). These cases involve finding certain
data in cache.
Matthijs Mekking [Fri, 13 Aug 2021 07:24:11 +0000 (09:24 +0200)]
Add qmin test cases when RRset has expired
Add test cases for GL #2665: The QNAME minimization (if enabled) should
also occur on the second query, after the RRsets have expired from
cache. BIND will still have the entries in cache, but marked stale.
These stale entries should not prevent the resolver from minimizing
the QNAME. We query for the test domain a.b.stale. in all cases (QNAME
minimization off, strict mode, and relaxed mode) and expect it to
behave the same the second time we have a stale delegation structure in
cache.
Artem Boldariev [Thu, 5 Aug 2021 09:42:40 +0000 (12:42 +0300)]
Fix the doh_recv_send() logic in the doh_test
The commit fixes the doh_recv_send() because occasionally it would
fail because it did not wait for all responses to be sent, making the
check for ssends value to nit pass.
Artem Boldariev [Mon, 2 Aug 2021 14:15:13 +0000 (17:15 +0300)]
Optimise TLS stream for small write size (>= 512 bytes)
This commit changes TLS stream behaviour in such a way, that it is now
optimised for small writes. In the case there is a need to write less
or equal to 512 bytes, we could avoid calling the memory allocator at
the expense of possibly slight increase in memory usage. In case of
larger writes, the behviour remains unchanged.
Artem Boldariev [Mon, 2 Aug 2021 11:43:54 +0000 (14:43 +0300)]
Avoid memory copying during send in TLS stream
At least at this point doing memory copying is not required. Probably
it was a workaround for some problem in the earlier days of DoH, at
this point it appears to be a waste of CPU cycles.
Simplify buffering code logic in http_send_outgoing()
This commit significantly simplifies the code in http_send_outgoing()
as it was unnecessary complicated, because it was dealing with
multiple statically and dynamically allocated buffers, making it
extremely hard to follow, as well as making it to do unnecessary
memory copying in some situations. This commit fixes these issues,
while retaining the high level buffering logic.
When terminating a client session, mark it as closing
When an HTTP/2 client terminates a session it means that it is about
to close the underlying connection. However, we were not doing that.
As a result, with the latest changes to the test suite, which made it
to limit amount of requests per a transport connection, the tests
using quota would hang for quite a while. This commit fixes that.
Limit the number of requests sent per connection in DoH tests
This commit ensures that only a limited number of requests is going to
be sent over a single HTTP/2 connection. Before that change was
introduced, it was possible to complete all of the planned sends via
only one transport connection, which undermines the purpose of the
tests using the quota facility.
Do not call http_do_bio() in isc__nm_http_request()
The function should not be called here because it is, in general,
supposed to be called at the end of the transport level callbacks to
perform I/O, and thus, calling it here is clearly a mistake because it
breaks other code expectations. As a result of the call to
http_do_bio() from within isc__nm_http_request() the unit tests were
running slower than expected in some situations.
In this particular situation http_do_bio() is going to be called at
the end of the transport_connect_cb() (initially), or http_readcb(),
sending all of the scheduled requests at once.
This change affects only the test suite because it is the only place
in the codebase where isc__nm_http_request() is used in order to
ensure that the server is able to handle multiple HTTP/2 streams at
once.
Fix a crash by attach to the transport socket as early as possible
This commit fixes a crash in DoH caused by transport handle to be
detached too early when sending outgoing data.
We need to attach to the session->handle earlier because as an
indirect result of the nghttp2_session_mem_send() the session might
get closed and the handle detached. However, there is still might be
some outgoing data to handle. Besides, even when the underlying socket
was closed via the handle, we still should try to attempt to send
outgoing data via isc_nm_send() to let it call write callback, passed
to the http_send_outgoing().
Use isc_buffer_t to keep track of outgoing response
This commit gets rid of custom code taking care of response buffering
by replacing the custom code with isc_buffer_t. Also, it gets rid of
an unnecessary memory copying when sending a response.
Use isc_buffer_t to keep track of incoming POST data
This commit replaces the ad-hoc 64K buffer for incoming POST data with
isc_buffer_t backed by dynamically allocated buffer sized accordingly
to the value in the "Content-Length" header.