this improves the performance of looking for NSEC and RRSIG(NSEC)
records in the cache by skipping lots of nodes in the main trees
in the cache without these records present. This is a simplified
version of previous_closest_nsec() which uses the same underlying
mechanism to look for NSEC and RRSIG(NSEC) records in authorative
zones.
The auxilary NSEC tree was already being maintained as a side effect
of looking for the covering NSEC in large zones where there can be
lots of glue records that needed to be skipped. Nodes are added
to the tree whenever a NSEC record is added to the primary tree.
They are removed when the corresponding node is removed from the
primary tree.
Having nodes in the NSEC tree w/o NSEC records in the primary tree
should not impact on synth-from-dnssec efficiency as that node would
have held the NSEC we would have been needed to synthesise the
response. Removing the node when the NSEC RRset expires would only
cause rbtdb to return a NSEC which would be rejected at a higher
level.
Previously, when TCP accept failed, we have logged a message with
ISC_LOG_ERROR level. One common case, how this could happen is that the
client hits TCP client quota and is put on hold and when resumed, the
client has already given up and closed the TCP connection. In such
case, the named would log:
TCP connection failed: socket is not connected
This message was quite confusing because it actually doesn't say that
it's related to the accepting the TCP connection and also it logs
everything on the ISC_LOG_ERROR level.
Change the log message to "Accepting TCP connection failed" and for
specific error states lower the severity of the log message to
ISC_LOG_INFO.
A TCP connection may be held open past its proper timeout if it's
receiving a stream of DNS responses that don't match any queries.
In this case, we now check whether the oldest query should have timed
out.
When the outgoing TCP dispatch times-out active response, we might still
receive the answer during the lifetime of the connection. Previously,
we would just ignore any non-matching DNS answers, which would allow the
server to feed us with otherwise valid DNS answer and keep the
connection open.
Add a counter for timed-out DNS queries over TCP and tear down the whole
TCP connection if we receive unexpected number of DNS answers.
Previously, when invalid DNS message is received over TCP we throw the
garbage DNS message away and continued looking for valid DNS message
that would match our outgoing queries. This logic makes sense for UDP,
because anyone can send DNS message over UDP.
Change the logic that the TCP connection is closed when we receive
garbage, because the other side is acting malicious.
When outgoing TCP connection was prematurely terminated (f.e. with
connection reset), the dispatch code would not cleanup the resources
used by such connection leading to dangling dns_dispentry_t entries.
There was a logical bug when setting a list of enabled TLS protocols,
which may lead to a crash (an abort()) on systems with ancient OpenSSL
versions.
The problem was due to the fact that we were INSIST()ing on supporting
all of the TLS versions, while checking only for mentioned in the
configuration was implied.
This commit ensure that the 'tls' name specified in the 'primaries'
clause of a 'zone' statement is a valid one.
Prior to that such a name would be silently accepted, leading to
silent XFRs-via-TLS failures.
This commit adds support for client-side TLS parameters to XoT.
Prior to this commit all client-side TLS contexts were using default
parameters only, ignoring the options from the BIND's configuration
file.
Currently, the following 'tls' parameters are supported:
- protocols;
- ciphers;
- prefer-server-ciphers.
This commit completes the integration of the new, extended ACL syntax
featuring 'port' and 'transport' options.
The runtime presentation and ACL loading code are extended to allow
the syntax to be used beyond the 'allow-transfer' option (e.g. in
'acl' definitions and other 'allow-*' options) and can be used to
ultimately extend the ACL support with transport-only
ACLs (e.g. 'transport-acl tls-acl port 853 transport tls'). But, due
to fundamental nature of such a change, it has not been completed as a
part of 9.17.X release series due to it being close to 9.18 stable
release status. That means that we do not have enough time to fully
test it.
The complete integration is planned as a part of 9.19.X release
series.
The code was manually verified to work as expected by temporarily
enabling the extended syntax for 'acl' statements and 'allow-query'
options, including ACL merging, negated ACLs.
This commit extends ACL syntax handling code with 'port' and
'transport' options. Currently, the extended syntax is available only
for allow-transfer options.
This commit adds an isc_nm_socket_type() function which can be used to
obtain a handle's socket type.
This change obsoletes isc_nm_is_tlsdns_handle() and
isc_nm_is_http_handle(). However, it was decided to keep the latter as
we eventually might end up supporting multiple HTTP versions.
This commit disables the unused 'tls' clause options. For these some
backing code exists, but their values are not really used anywhere,
nor there are sufficient syntax tests for them.
These options are only disabled temporarily, until TLS certificate
verification gets implemented.
This commit makes the TLS stream code to not issue mostly useless
debug log message on error during TLS I/O. This message was cluttering
logs a lot, as it can be generated on (almost) any non-clean TLS
connection termination, even in the cases when the actual query
completed successfully. Nor does it provide much value for end-users,
yet it can occasionally be seen when using dig and quite often when
running BIND over a publicly available network interface.
This commit removes unneeded isc__nmsocket_prep_destroy() call on ALPN
negotiation failure, which was eventually causing the TLS handle to
leak.
This call is not needed, as not attaching to the transport (TLS)
handle should be enough. At this point it seems like a kludge from
earlier days of the TLS code.
This prevents a direct leak in OPENSSL_init_crypto (called from
OPENSSL_init_ssl).
Add shim version of OPENSSL_cleanup because it is missing in LibreSSL on
OpenBSD.
The parsing loop needs to process ISC_R_NOSPACE to properly
size the buffer. If result is still ISC_R_NOSPACE at the end
of the parsing loop set result to DNS_R_SERVFAIL.
In file included from rdata.c:602:
In file included from ./code.h:88:
./rdata/in_1/svcb_64.c:259:9: warning: array subscript is of type 'char' [-Wchar-subscripts]
if (!isdigit(*region->base)) {
^~~~~~~~~~~~~~~~~~~~~~
/usr/include/sys/ctype_inline.h:51:44: note: expanded from macro 'isdigit'
#define isdigit(c) ((int)((_ctype_tab_ + 1)[(c)] & _CTYPE_D))
^~~~
This commit fixes a peculiar corner case in the client-side DoT code
because of which a crash could occur during a zone transfer. A junk
DNS message should be sent at the end of a zone transfer via TLS to
trigger the crash (abort).
This commit, hopefully, fixes that.
Also, this commit adds similar changes to the TCP DNS code, as it
shares the same origin and most of the logic.
When a UDP dispatch receives a mismatched response, it checks whether
there is still enough time to wait for the correct one to arrive before
the timeout fires. If there is not, the result code is set to
ISC_R_TIMEDOUT, but it is not subsequently used anywhere as 'response'
is set to NULL a few lines earlier. This results in the higher-level
read callback (resquery_response() in case of resolver code) not being
called. However, shortly afterwards, a few levels up the call chain,
isc__nm_udp_read_cb() calls isc__nmsocket_timer_stop() on the dispatch
socket, effectively disabling read timeout handling for that socket.
Combined with the fact that reading is not restarted in such a case
(e.g. by calling dispatch_getnext() from udp_recv()), this leads to the
higher-level query structure remaining referenced indefinitely because
the dispatch socket it uses will neither be read from nor closed due to
a timeout. This in turn causes fetch contexts to linger around
indefinitely, which in turn i.a. prevents certain cache nodes (those
containing rdatasets used by fetch contexts, like fctx->nameservers)
from being cleaned.
Fix by making sure the higher-level callback does get invoked with the
ISC_R_TIMEDOUT result code when udp_recv() determines there is no more
time left to receive the correct UDP response before the timeout fires.
This allows the higher-level callback to clean things up, preventing the
reference leak described above.
The following scenario triggers a "named" crash:
1. Configure a catalog zone.
2. Start "named".
3. Comment out the "catalog-zone" clause.
4. Run `rndc reconfig`.
5. Uncomment the "catalog-zone" clause.
6. Run `rndc reconfig` again.
Implement the required cleanup of the in-memory catalog zone during
the first `rndc reconfig`, so that the second `rndc reconfig` could
find it in an expected state.
opensslecdsa_fromdns() already rejects too short ECDSA public keys.
Make it also reject too long ones. Remove an assignment made redundant
by this change.
raw_key_to_ossl() assumes fixed ECDSA private key sizes (32 bytes for
ECDSAP256SHA256, 48 bytes for ECDSAP384SHA384). Meanwhile, in rare
cases, ECDSAP256SHA256 private keys are representable in 31 bytes or
less (similarly for ECDSAP384SHA384) and that is how they are then
stored in the "PrivateKey" field of the key file. Nevertheless,
raw_key_to_ossl() always calls BN_bin2bn() with a fixed length argument,
which in the cases mentioned above leads to erroneously interpreting
uninitialized memory as a part of the private key. This results in the
latter being malformed and broken signatures being generated. Address
by using the key length provided by the caller rather than a fixed one.
Apply the same change to public key parsing code for consistency, adding
an INSIST() to prevent buffer overruns.
when processing a mismatched response, we call dns_dispatch_getnext().
If that fails, for example because of a timeout, fctx_done() is called,
which cancels all queries. This triggers a crash afterward when
fctx_cancelquery() is called, and is unnecessary since fctx_done()
would have been called later anyway.
When dns_adb is shutting down, first the adb->shutting_down flag is set
and then task is created that runs shutdown_stage2() that sets the
shutdown flag on names and entries. However, when dns_adb_createfind()
is called, only the individual shutdown flags are being checked, and the
global adb->shutting_down flag was not checked. Because of that it was
possible for a different thread to slip in and create new find between
the dns_adb_shutdown() and dns_adb_detach(), but before the
shutdown_stage2() task is complete. This was detected by
ThreadSanitizer as data race because the zonetable might have been
already detached by dns_view shutdown process and simultaneously
accessed by dns_adb_createfind().
This commit converts the adb->shutting_down to atomic_bool to prevent
the global adb lock when creating the find.
Add a new parameter to 'ns_client_t' to store potential extended DNS
error. Reset when the client request ends, or is put back.
Add defines for all well-known info-codes.
Update the number of DNS_EDNSOPTIONS that we are willing to set.
Create a new function to set the extended error for a client reply.
Change 5756 (GL #2854) introduced build errors when using
'configure --disable-doh'. To fix this, isc_nm_is_http_handle() is
now defined in all builds, not just builds that have DoH enabled.
Missing code comments were added both for that function and for
isc_nm_is_tlsdns_handle().
This commit makes BIND set the "max-age" value of the "Cache-Control"
HTTP header to the minimal TTL from the Answer section for positive
answers, as RFC 8484 advises in section 5.1.
We calculate the minimal TTL as a side effect of rendering the
response DNS message, so it does not change the code flow much, nor
should it have any measurable negative impact on the performance.
For negative answers, the "max-age" value is set using the TTL and
SOA-minimum values from an SOA record in the Authority section.
This commit adds an isc_nm_set_min_answer_ttl() function which is
intended to to be used to give a hint to the underlying transport
regarding the answer TTL.
The interface is intentionally kept generic because over time more
transports might benefit from this functionality, but currently it is
intended for DoH to set "max-age" value within "Cache-Control" HTTP
header (as recommended in the RFC8484, section 5.1 "Cache
Interaction").
It is no-op for other DNS transports for the time being.
Check to see whether there are outstanding requests in the
httpd receive buffer after sending the response, and if so,
process them.
Test that pipelined requests are handled by sending multiple
minimal HTTP/1.1 using netcat (nc) and checking that we get
back the same number of responses.
Remember the amount of space consumed by the HTTP headers, then
move any trailing data to the start of the httpd->recvbuf once
we have finished processing the request.
if an incoming HTTP request is incomplete, but nothing else is clearly
wrong with it, the stats channel continues reading to see if there's
more coming. the buffer length was not being processed correctly in
this case. also, the server state was not reset correctly when the
request was complete, so that subsequent requests could be appended to
the first buffer instead of being treated as new.
in addition fixing the above problems, this commit also increases the
size of the httpd request buffer from 1024 to 4096, because some
browsers send a lot of headers.
1) if 'key->external' is set we just need to call
dst__privstruct_writefile
2) the cleanup of 'bufs' was incorrect as 'i' doesn't reflect the
the current index into 'bufs'. Use a simple for loop.
This review was triggered by Coverity reporting a buffer overrun
on 'bufs'.
'dh' was being assigned to key->keydata.dh too soon which could
result in a memory leak on error. Moved the assignement of
key->keydata.dh until after dh was correct.
Coverity was reporting dead code on the error path cleaning up 'dh'
which triggered this review.
'make dist' omits lib/dns/tests/comparekeys/ (added in
7101afa23c) from release tarball it
creates which makes the unit:gcc:tarball CI job permanently fail in the
dst unit test.
In the 9.17.19 release "tls" statements verification code was
added. The code was too strict and assumed that every such a statement
should have both "cert-file" and "key-file" specified. This turned out
to be a regression, as in some cases we plan to use the "tls"
statement to specify TLS connection parameters.
This commit fixes this behaviour; now a "tls" statement should either
have both "cert-file" and "key-file" specified, or both should be
omitted.
Previously, when lame cache would be disabled by setting lame-ttl to 0,
it would also disable lame answer detection. In this commit, we enable
the lame response detection even when the lame cache is disabled. This
enables stopping answer processing early rather than going through the
whole answer processing flow.
Unless being configured with the `no-deprecated` option, OpenSSL 3.0.0
still has the deprecated APIs present and will throw warnings during
compilation, when using them.
Make sure that the old APIs are being used only with the older versions
of OpenSSL.
OpenSSL 3 deprecates most of the DH* family and associated APIs.
Reimplement the existing functionality using a newer set of APIs
which will be used when compiling/linking with OpenSSL 3.0.0 or newer
versions.
OpenSSL 3 deprecates most of the RSA* family and associated APIs.
Reimplement the existing functionality using a newer set of APIs
which will be used when compiling/linking with OpenSSL 3.0.0 or newer
versions.
