Unit test for isc_netaddr_masktoprefixlen were missing IPv6 mask cases.
Add those and few other IPv4 cases. Also, the test is refactored in
order to make it easy to add new cases.
(cherry picked from commit ad2c3fa4f7)
The string literal initialalising compressed was too big for the
array as it has an unwanted NUL terminator. This is allowed for
in C for historical reasons but produces a warning with some
compilers. Adjust the declaration to include the NUL and adjust
the users to pass in an adjusted size which excludes the NUL rather
than sizeof(compressed).
Closes#5258
Backport of MR !10326
Merge branch 'backport-5258-avoid-warning-initialising-compresss-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10331
The string literal initialalising compressed was too big for the
array as it has an unwanted NUL terminator. This is allowed for
in C for historical reasons but produces a warning with some
compilers. Adjust the declaration to include the NUL and adjust
the users to pass in an adjusted size which excludes the NUL rather
than sizeof(compressed).
(cherry picked from commit 6a6b6be824)
Previously, in some cases, the resolver could return rdatasets of type CNAME or DNAME without the result code being set to `DNS_R_CNAME` or `DNS_R_DNAME`. This could trigger an assertion failure in the ADB. The resolver error has been fixed.
Closes#5201
Backport of MR !10172
Backport of MR !10178
Merge branch 'backport-5201-adb-cname-error-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10329
the fix in commit 1edbbc32b4 was incomplete; the wrong
event result could also be set in cache_name() and validated().
(cherry picked from commit 9ebeb60174)
when the caching of a negative record failed because of the
presence of a positive one, ncache_adderesult() could override
this to ISC_R_SUCCESS. this could cause CNAME and DNAME responses
to be handled incorrectly. ncache_adderesult() now sets the result
code correctly in such cases.
(cherry picked from commit 1edbbc32b4)
Otherwise curl 8.13 rejects the line with:
I:Check HTTP/1.1 keep-alive with truncated stream (21)
curl: option --header: error encountered when reading a file
curl: try 'curl --help' or 'curl --manual' for more information
Also, see https://github.com/curl/curl/pull/16572.
Closes#5249
Backport of MR !10319
Merge branch 'backport-5249-statschannel-limit-http-header-size-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10321
Otherwise curl 8.13 rejects the line with:
I:Check HTTP/1.1 keep-alive with truncated stream (21)
curl: option --header: error encountered when reading a file
curl: try 'curl --help' or 'curl --manual' for more information
Also, see https://github.com/curl/curl/pull/16572.
(cherry picked from commit bcb931f4a8)
Previously, when named-checkzone loaded the zone to the QP database, the delayed
memory reclamation could cause an assertion check on exit. This has
been fixed.
Closes#5220
Merge branch '5220-add-rcu-barrier-to-named-checkconf-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10307
When named-checkzone loads the zone to the QP database, the delayed
memory reclamation could cause an assertion check on exit. Add RCU
barrier to wait for the memory reclamation to complete.
The isc_queue_t was missing in the calculation of the required
padding size inside the qpcache bucket structure.
Backport of MR !10306
Merge branch 'backport-ondrej/qpcache-fix-invalid-padding-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10317
The isc_queue_t was missing in the calculation of the required
padding size inside the qpcache bucket structure.
(cherry picked from commit 3ef9b09620)
All DNSKEY keys are able to authenticate. The `DNS_KEYTYPE_NOAUTH` (and `DNS_KEYTYPE_NOCONF`) flags were defined for the KEY rdata type, and are not applicable to DNSKEY. Previously, however, because the DNSKEY implementation was built on top of KEY, the `_NOAUTH` flag prevented authentication in DNSKEYs as well. This has been corrected.
Closes#5240
Backport of MR !10261
Merge branch 'backport-5240-ignore-noauth-flag-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10315
All DNSKEY keys are able to authenticate. The DNS_KEYTYPE_NOAUTH
(and DNS_KEYTYPE_NOCONF) flags were defined for the KEY rdata type,
and are not applicable to DNSKEY.
Previously, because the DNSKEY implementation was built on top of
KEY, the NOAUTH flag prevented authentication in DNSKEYs as well.
This has been corrected.
(cherry picked from commit 5c21576f82)
Use enums for DNS_KEYFLAG_, DNS_KEYTYPE_, DNS_KEYOWNER_, DNS_KEYALG_,
and DNS_KEYPROTO_ values.
Remove values that are never used.
Eliminate the obsolete DNS_KEYFLAG_SIGNATORYMASK. Instead, add three
more RESERVED bits for the key flag values that it covered but which
were never used.
(cherry picked from commit fee1ba40df)
This function was removed in 6217e434b5 but not from the header file.
Backport of MR !10308
Merge branch 'backport-matthijs-remove-unused-qpmulti-lockedread-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10314
Replace the custom DNS server used in the "upforwd" system test with new
code based on the isctest.asyncserver module. The ans4 server currently
used in that test is a copy of bin/tests/system/ans.pl modified to
receive queries over UDP and TCP without ever responding to any of them.
Closes#5012
Backport of MR !10283
Merge branch 'backport-5012-upforwd-asyncserver-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10312
Replace the custom DNS server used in the "upforwd" system test with new
code based on the isctest.asyncserver module. The ans4 server currently
used in that test is a copy of bin/tests/system/ans.pl modified to
receive queries over UDP and TCP without ever responding to any of them.
(cherry picked from commit a8878cf35d)
Dropping all incoming queries is a typical use case for a custom server
used in BIND 9 system tests. Add a response handler implementing that
behavior so that it can be reused.
(cherry picked from commit f24a534ff1)
Instead of requiring each class inheriting from ResponseHandler to
define its match() method, make the latter non-abstract and default to
returning True for all queries. This will reduce the amount of
boilerplate code in custom servers.
(cherry picked from commit 75567f86ca)
Call `dns_adbname_ref` before calling `dns_resolver_createfetch` to
ensure `adbname->name` remains stable for the life of the fetch.
Closes#5239
Backport of MR !10290
Merge branch 'backport-5239-fix-adb-reference-counting-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10303
Call dns_adbname_ref before calling dns_resolver_createfetch to
ensure adbname->name remains stable for the life of the fetch.
(cherry picked from commit 8e7229f641)
The following small issues related to `dnssec-policy` have been fixed:
- In some cases the key manager inside BIND 9 could run every hour, while it could have run less often.
- While `CDS` and `CDNSKEY` records will be removed correctly from the zone when the corresponding `DS` record needs to be updated, the expected timing metadata when this will happen was never set.
- There were a couple of cases where the safety intervals are added inappropriately, delaying key rollovers longer than necessary.
- If you have identical `keys` in your `dnssec-policy`, they may be retired inappropriately. Note that having keys with identical properties is discouraged in all cases.
Closes#5242
Backport of MR !10251
Merge branch 'backport-5242-several-keymgr-issues-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10301
If we are updating the lifetime, and it was not set before, also
set/update the Retired and Removed timing metadata.
(cherry picked from commit 3e836a87e6)
The dnssec-keygen command for the ZSK generation for the zone
multisigner-model2.kasp was wrong (no ZSK was generated in the setup
script, but when 'named' is started, the missing ZSK was created
anyway by 'dnssec-policy'.
(cherry picked from commit b93cb2e80e)
Only set the next time the keymgr should run if the value is non zero.
Otherwise we default back to one hour. This may happen if there is one
or more key with an unlimited lifetime.
(cherry picked from commit 6c6b8796d3)
The keymgr never set the expected timing metadata when CDS/CDNSKEY
records for the corresponding key will be removed from the zone. This
is not troublesome, as key states dictate when this happens, but with
the new pytest we use the timing metadata to determine if the CDS and/or
CDNSKEY for the given key needs to be published.
(cherry picked from commit 8c9d2eb2bf)
There are a couple of cases where the safety intervals are added
inappropriately:
1. When setting the PublishCDS/SyncPublish timing metadata, we don't
need to add the publish-safety value if we are calculating the time
when the zone is completely signed for the first time. This value
is for when the DNSKEY has been published and we add a safety
interval before considering the DNSKEY omnipresent.
2. The retire-safety value should only be added to ZSK rollovers if
there is an actual rollover happening, similar to adding the sign
delay.
3. The retire-safety value should only be added to KSK rollovers if
there is an actual rollover happening. We consider the new DS
omnipresent a bit later, so that we are forced to keep the old DS
a bit longer.
(cherry picked from commit 63edc4435f)
While converting the kasp system test to pytest, I encountered a small
bug in the keymgr code. We retire keys when there is more than one
key matching a 'keys' line from the dnssec-policy. But if there are
multiple identical 'keys' lines, as is the case for the test zone
'checkds-doubleksk.kasp', we retire one of the two keys that have the
same properties.
Fix this by checking if there are double matches. This is not fool proof
because there may be many keys for a few identical 'keys' lines, but it
is good enough for now. In practice it makes no sense to have a policy
that dictates multiple keys with identical properties.
(cherry picked from commit ef671919d5)
If the `max-clients-per-query` option is set to a lower value than `clients-per-query`, the value is adjusted to match `clients-per-query`.
Closes#5224
Backport of MR !10241
Merge branch 'backport-5224-raise-max-clients-per-query-to-be-at-least-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10244
In the case where 'clients-per-query' is larger than
'max-clients-per-query', raise 'max-clients-per-query' so that
'clients-per-query' equals 'max-clients-per-query' and log a warning
that this is what happened.
(cherry picked from commit f6f9645ed1)
The new intended behavior is that 'max-clients-per-query' value is
raised to equal 'clients-per-query' if it is lower.
(cherry picked from commit f50753f303)
Raw integer pointers were being used for the validator's nvalidations
and nfails values but the memory holding them could be freed before
they ceased to be used. Use reference counted counters instead.
Closes#5239
Backport of MR !10248
Merge branch 'backport-5239-use-counter-for-nvalidations-and-nfailss-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10300
The fetch context that held these values could be freed while there
were still active pointers to the memory. Using a reference counted
pointer avoids this.
(cherry picked from commit bfbaacc9a0)
When query responses timed out, the resolver could incorrectly increase the regular responses counters, even if no response was received. This has been fixed.
Closes#5193
Backport of MR !10227
Merge branch 'backport-5193-resolver-statistics-counters-fix-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10287
When a response times out the fctx_cancelquery() function
incorrectly calculates it in the 'dns_resstatscounter_queryrtt5'
counter (i.e. >=1600 ms). To avoid this, the rctx_timedout()
function should make sure that 'rctx->finish' is NULL. And in order
to adjust the RTT values for the timed out server, 'rctx->no_response'
should be true. Update the rctx_timedout() function to make those
changes.
(cherry picked from commit 830e548111)
The resquery_response() function increases the response counter without
checking if the response was successful. Increase the counter only when
the result indicates success.
(cherry picked from commit 12e7dfa397)
This branch started off as `michal/upforwd-asyncserver`. It quickly
turned out that the critical `asyncserver.py` change that was needed for
the `upforwd` system test was for the server to be able to read multiple
TCP queries on a single connection. As currently present in `main`,
`asyncserver.py` closes every client connection after servicing a single
query. Retaining that behavior would cause the `upforwd` system test to
fail and, in general, capturing all data sent by a client seems more
useful in tests than just closing connections quickly. `asyncserver.py`
can always be extended in the future (e.g. by adding a new
`ResponseAction` that the networking code would react to) to reinstate
the original behavior, if it turns out to be necessary.
While working on changing that particular `asyncserver.py` behavior, I
noticed a couple of other deficiencies in the TCP connection handling
code, so I started addressing them. One thing led to another and before
I noticed, enough changes were applied to be worth doing a separate
merge request, particularly given that the actual rewrite of
`upforwd/ans4/ans.pl` using `asyncserver.py` is trivial once the
required changes to `asyncserver.py` itself are applied.
Backport of MR !10276
Merge branch 'backport-michal/asyncserver-tcp-improvements-9.20' into 'bind-9.20'
See merge request isc-projects/bind9!10284
Instead of closing every incoming TCP connection after handling a single
query, continue receiving queries on each TCP connection until the
client disconnects itself. When coupled with response dropping, this
enables silently receiving all incoming data, simulating an unresponsive
server.
(cherry picked from commit 575a874582)
A TCP DNS client may send its queries in chunks, causing
StreamReader.read() to return less data than previously declared by the
client as the DNS message length; even the two-octet DNS message length
itself may be split up into two single-octet transmissions. Sending
data in chunks is valid client behavior that should not be treated as an
error. Add a new helper method for reading TCP data in a loop, properly
distinguishing between chunked queries and client disconnections. Use
the new method for reading all TCP data from clients.
(cherry picked from commit 68fe9a5df5)
Emit more log messages from TCP connection handling code and extend
existing ones to improve debuggability of servers using asyncserver.py.
(cherry picked from commit 8c3f673f37)
A TCP peer may reset the connection at any point, but asyncserver.py
currently only handles connection resets when it is sending data to the
client. Handle connection resets during reading in the same way.
(cherry picked from commit 748ed4259b)
Prevent premature client disconnections during reading from triggering
unhandled exceptions in TCP connection handling code.
(cherry picked from commit e4c3186a7c)
Add a helper class, Peer, which holds the <host, port> tuple of a
connection endpoint and gets pretty-printed when formatted as a string.
This enables passing instances of this new class directly to logging
functions, eliminating the need for the AsyncDnsServer._format_peer()
helper method.
(cherry picked from commit 5764a9d660)
