Tom Krizek [Mon, 7 Nov 2022 15:38:49 +0000 (16:38 +0100)]
Ensure test interpreters are defined before common config
Nothing from conf.sh.common is required to set these values. On the
contrary, a Python interpreter needs to be set in order to randomize the
algorithm set (which happens in conf.sh.common).
Tom Krizek [Mon, 7 Nov 2022 14:58:40 +0000 (15:58 +0100)]
Force quiet mode when using testcrypto.sh directly
When testcrypto.sh is used as a standalone script, always use quiet mode
to avoid using undefined commands (such as echo_i) which require
inclusion of the entire conf.sh machinery.
Artem Boldariev [Wed, 7 Dec 2022 12:32:20 +0000 (14:32 +0200)]
StreamDNS: Use isc__nm_senddns() to send DNS messages
This commit modifies the Stream DNS message so that it uses the
optimised code path (isc__nm_senddns()) for sending DNS messages over
the underlying transport. This way we avoid allocating any
intermediate memory buffers needed to render a DNS message with its
length pre-pended ahead of the contents (TCP DNS message format).
Artem Boldariev [Wed, 7 Dec 2022 11:33:52 +0000 (13:33 +0200)]
Add isc__nm_senddns()
The new internal function works in the same way as isc_nm_send()
except that it sends a DNS message size ahead of the DNS message
data (the format used in DNS over TCP).
The intention is to provide a fast path for sending DNS messages over
streams protocols - that is, without allocating any intermediate
memory buffers.
Artem Boldariev [Mon, 5 Dec 2022 18:19:03 +0000 (20:19 +0200)]
TLS: try to avoid allocating send request objects
This commit optimises TLS send request object allocation to enable
send request object reuse, somewhat reducing pressure on the memory
manager. It is especially helpful in the case when Stream DNS uses the
TLS implementation as the transport.
Artem Boldariev [Tue, 23 Aug 2022 19:15:54 +0000 (22:15 +0300)]
Make isc_nm_tlssocket unit tests available in non DoH-enabled builds
This commit ensures that generic TLS unit tests are available in non
DoH-enabled builds, as isc_nm_tlssocket is not tied exclusively to the
DoH implementation anymore.
Artem Boldariev [Tue, 18 Oct 2022 12:36:00 +0000 (15:36 +0300)]
Make isc_nm_tlssocket non-optional
This commit unties generic TLS code (isc_nm_tlssocket) from DoH, so
that it will be available regardless of the fact if BIND was built
with DNS over HTTP support or not.
This commit ensures that Stream DNS code attempts to disable Nagle's
algorithm regardless of underlying stream transport (TCP or TLS), as
we are not interested in trading latency for throughout when dealing
with DNS messages.
Artem Boldariev [Thu, 25 Aug 2022 19:37:26 +0000 (22:37 +0300)]
Disable Nagle's algorithm for TLS connections by default
This commit ensures that Nagle's algorithm is disabled by default for
TLS connections on best effort basis, just like other networking
software (e.g. NGINX) does, as, in the case of TLS, we are not
interested in trading latency for throughput, rather vice versa.
We attempt to disable it as early as we can, right after TCP
connections establishment, as an attempt to speed up handshake
handling.
Artem Boldariev [Thu, 25 Aug 2022 19:18:15 +0000 (22:18 +0300)]
TCP: make it possible to set Nagle's algorithms state via handle
This commit adds ability to turn the Nagle's algorithm on or off via
connections handle. It adds the isc_nmhandle_set_tcp_nodelay()
function as the public interface for this functionality.
Artem Boldariev [Tue, 8 Nov 2022 14:43:01 +0000 (16:43 +0200)]
Re-purpose TLS DNS and TCP DNS unit tests for Stream DNS
This commit modifies the existing unit tests for TLS DNS and TCP DNS
in such a way that the new Stream DNS transport is used as it is
intended to be a drop-in replacement for these two transports.
Artem Boldariev [Mon, 20 Jun 2022 17:30:12 +0000 (20:30 +0300)]
Add isc_nm_streamdnssocket (aka Stream DNS)
This commit adds an initial implementation of isc_nm_streamdnssocket
transport: a unified transport for DNS over stream protocols messages,
which is capable of replacing both TCP DNS and TLS DNS
transports. Currently, the interface it provides is a unified set of
interfaces provided by both of the transports it attempts to replace.
The transport is built around "isc_dnsbuffer_t" and
"isc_dnsstream_assembler_t" objects and attempts to minimise both the
number of memory allocations during network transfers as well as
memory usage.
Artem Boldariev [Mon, 13 Jun 2022 16:20:08 +0000 (19:20 +0300)]
Add isc_dnsstream_assembler_t implementation
This commit adds the implementation for an "isc_dnsstream_assembler_t"
object. The object is built on top of "isc_dnsbuffer_t" and is
intended to encapsulate the state machine used for handling DNS
messages received in the format used for messages transmitted over
TCP.
The idea is that the object accepts the input data received from a
socket, tries to assemble DNS messages from the incoming data and
calls the callback which contains the status of the incoming data as
well as a pointer to the memory region referencing the data of the
assembled message. It is capable of assembling DNS messages no matter
how torn apart they are when sent over network.
The following statuses might be passed to the callback:
* ISC_R_SUCCESS - a message has been successfully assembled;
* ISC_R_NOMORE - not enough data has been processed to assemble a
message;
* ISC_R_RANGE - there was an attempt to process a zero-sized DNS
message (someone attempts to send us junk data).
One could say that the object replaces the implementation of
"isc__nm_*_processbuffer()" functions used by the old TCP DNS and TLS
DNS transports with a better defined state machine completely
decoupled from the networking code itself.
Such a design makes it trivial to write unit tests for it, leading to
better verification of its correctness.
Another important difference is directly related to the fact that it
is built on top of "isc_dnsbuffer_t", which tries to manage memory in
a smart way. In particular:
* It tries to use a static buffer for smaller messages, reducing
pressure on the memory manager (hot path);
* When allocating dynamic memory for larger messages, it tries to
allocate memory conservatively (generic path).
These characteristics is a significant upgrade over the older logic
where a 64KB(+2 bytes) buffer was allocated from dynamic memory
regardless of the fact if we need a buffer this large or not. That is,
lesser memory usage is expected in a generic case for DNS transports
built on top of "isc_dnsstream_assembler_t."
Artem Boldariev [Tue, 7 Jun 2022 10:14:29 +0000 (13:14 +0300)]
Add isc_dnsbuffer_t implementation
This commit adds "isc_dnsbuffer_t" object implementation, a thin
wrapper on top of "isc_buffer_t" which has the following
characteristics:
* provides interface specifically atuned for handling/generating DNS
messages, especially in the format used for DNS messages over TCP;
* avoids allocating dynamic memory when handling small DNS messages,
while transparently switching to using dynamic memory when handling
larger messages. This approach significantly reduces pressure on the
memory allocator, as most of the DNS messages are small.
Artem Boldariev [Tue, 18 Oct 2022 12:21:10 +0000 (15:21 +0300)]
TCP: add manual read timer control mode
This commit adds a manual read timer control mode to the TCP
code (adding isc__nmhandle_set_manual_timer() as the interface to it).
Manual read timer control mode suppresses read timer restarting the
read timer when receiving any amount of data. This way the read timer
can be controlled manually using:
TLS: expose the ability to (re)start and stop underlying read timer
This commit adds implementation of isc__nmsocket_timer_restart() and
isc__nmsocket_timer_stop() for generic TLS code in order to make its
interface more compatible with that of TCP.
TLS: isc_nm_bad_request() and isc__nmsocket_reset() support
This commit adds implementations of isc_nm_bad_request() and
isc__nmsocket_reset() to the generic TLS stream code in order to make
it more compatible with TCP code.
Artem Boldariev [Tue, 20 Dec 2022 18:56:22 +0000 (20:56 +0200)]
Use 'restrict' and 'const' for 'isc_buffer_t'
The purpose of this commit is to aid compiler in generating better
code when working with `isc_buffer_t` objects by using restricted
pointers (and, to a lesser extent, 'const' modifier for read-only
arguments).
This way we, basically, instruct the compiler that the members of
structured passed by pointers into the functions can be treated as
local variables in the scope of a function. That should reduce the
number of load/store operations emitted by compilers when accessing
objects (e.g. 'isc_buffer_t') via pointers.
Ondřej Surý [Fri, 16 Dec 2022 10:43:20 +0000 (11:43 +0100)]
Add isc_buffer_setmctx() and isc_buffer_clearmctx() function
Add two extra functions needed by StreamDNS:
1. isc_buffer_setmctx() sets the buffer internal memory context, so we
can use isc_buffer_reserve() on the buffer. For this, we also need
to track whether the .base was dynamically allocated or not. This
needs to be called after isc_buffer_init() and before first
isc_buffer_reserve() call.
2. isc_buffer_clearmctx() clears the buffer internal memory context, and
frees any dynamically allocated buffer. This needs to be called
after the last isc_buffer_reserve() call and before calling the
isc_buffer_invalidate()
Ondřej Surý [Fri, 16 Dec 2022 10:11:59 +0000 (11:11 +0100)]
Cleanup the isc_buffer_reserve() usage in put*() helpers
There are couple places where we use putstr(), putmem(), ... helpers
that tries to reserve space and only if successful puts the data onto
the buffer. Cleanup the double reference as it's not needed there.
Ondřej Surý [Thu, 15 Dec 2022 21:51:52 +0000 (22:51 +0100)]
Add a static pre-allocated buffer to isc_buffer_t
When the buffer is allocated via isc_buffer_allocate() and the size is
smaller or equal ISC_BUFFER_STATIC_SIZE (currently 512 bytes), the
buffer will be allocated as a flexible array member in the buffer
structure itself instead of allocating it on the heap. This should help
when the buffer is used on the hot-path with small allocations.
Ondřej Surý [Thu, 15 Dec 2022 21:27:12 +0000 (22:27 +0100)]
Enable auto-reallocation for all isc_buffer_allocate() buffers
When isc_buffer_t buffer is created with isc_buffer_allocate() assume
that we want it to always auto-reallocate instead of having an extra
call to enable auto-reallocation.
Ondřej Surý [Thu, 15 Dec 2022 21:15:37 +0000 (22:15 +0100)]
Remove single use isc_buffer_putdecint() function
The isc_buffer_putdecint() could be easily replaced with
isc_buffer_printf() with just a small overhead of calling vsnprintf()
twice instead once. This is not on a hot-path (dns_catz unit), so we
can ignore the overhead and instead have less single-use code in favor
of using reusable more generic function.
Ondřej Surý [Thu, 15 Dec 2022 11:38:31 +0000 (12:38 +0100)]
Refactor the isc_buffer_{get,put}uintN, add isc_buffer_peekuintN
The Stream DNS implementation needs a peek methods that read the value
from the buffer, but it doesn't advance the current position. Add
isc_buffer_peekuintX methods, refactor the isc_buffer_{get,put}uintN
methods to modern integer types, and move the isc_buffer_getuintN to the
header as static inline functions.
Ondřej Surý [Thu, 15 Dec 2022 10:54:51 +0000 (11:54 +0100)]
Change the isc_buffer_reserve() to take just buffer pointer
The isc_buffer_reserve() would be passed a reference to the buffer
pointer, which was unnecessary as the pointer would never be changed
in the current implementation. Remove the extra dereference.
Ondřej Surý [Wed, 30 Nov 2022 16:58:35 +0000 (17:58 +0100)]
Fix the thread safety in the dns_dispatch unit
The dispatches are not thread-bound, and used freely between various
threads (see the dns_resolver and dns_request units for details).
This refactoring make sure that all non-const dns_dispatch_t and
dns_dispentry_t members are accessed under a lock, and both object now
track their internal state (NONE, CONNECTING, CONNECTED, CANCELED)
instead of guessing the state from the state of various struct members.
During the refactoring, the artificial limit DNS_DISPATCH_SOCKSQUOTA on
UDP sockets per dispatch was removed as the limiting needs to happen and
happens on in dns_resolver and limiting the number of UDP sockets
artificially in dispatch could lead to unpredictable behaviour in case
one dispatch has the limit exhausted by others are idle.
The TCP artificial limit of DNS_DISPATCH_MAXREQUESTS makes even less
sense as the TCP connections are only reused in the dns_request API
that's not a heavy user of the outgoing connections.
As a side note, the fact that UDP and TCP dispatch pretends to be same
thing, but in fact the connected UDP is handled from dns_dispentry_t and
dns_dispatch_t acts as a broker, but connected TCP is handled from
dns_dispatch_t and dns_dispatchmgr_t acts as a broker doesn't really
help the clarity of this unit.
This refactoring kept to API almost same - only dns_dispatch_cancel()
and dns_dispatch_done() were merged into dns_dispatch_done() as we need
to cancel active netmgr handles in any case to not leave dangling
connections around. The functions handling UDP and TCP have been mostly
split to their matching counterparts and the dns_dispatch_<function>
functions are now thing wrappers that call <udp|tcp>_dispatch_<function>
based on the socket type.
More debugging-level logging was added to the unit to accomodate for
this fact.
Ondřej Surý [Fri, 16 Dec 2022 20:46:50 +0000 (21:46 +0100)]
Fix reference counting in get_attached_entry (again)
When get_attached_entry() encounters entry that would be expired, it
needs to get reference to the entry before calling maybe_expire_entry(),
so the ADB entry doesn't get destroyed inside the its own lock.
This creeped into the code base again during review, so I am adding
an extra comment to prevent this.
Tom Krizek [Thu, 15 Dec 2022 16:52:52 +0000 (17:52 +0100)]
danger: check backport commits for original commit IDs
A full backport must have all the commit from the original MR and the
original commit IDs must be referenced in the backport commit messages.
If the criteria above is not met, the MR should be marked as a partial
backport. In that case, any discrepencies are only logged as informative
messages rather than failures.
Tom Krizek [Thu, 15 Dec 2022 16:51:24 +0000 (17:51 +0100)]
danger: check that original MR has been merged
When checking a backport MR, ensure that the original MR has been merged
already. This is vital for followup checks that verify commit IDs from
original commits are present in backport commit messages.
Tom Krizek [Thu, 15 Dec 2022 16:48:34 +0000 (17:48 +0100)]
danger: check backport links to the original MR
When doing archeology, it is much easier to find stuff if it's properly
linked. This check ensures that backport MR are linked to their original
MR via a "Backport of !XXXX" message.
The regular expression is fairly broad and has been tested to accept the
following variants of the message:
Backport of MR !XXXX
Backport of: !XXXX
backport of mr !XXXX
Backport of !XXXX
Backport of https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/XXXX
Ondřej Surý [Tue, 13 Dec 2022 13:32:19 +0000 (14:32 +0100)]
Don't expire fresh ADB names and entries
The overmem cleaning in ADB could become overzealous and clean fresh ADB
names and entries. Add a safety check to not clean any ADB names and
entries that are below ADB_CACHE_MINIMUM threshold.
Ondřej Surý [Tue, 13 Dec 2022 13:14:21 +0000 (14:14 +0100)]
Exclude the ADB hashmaps from ADB overmem accounting
The ADB overmem accounting would include the memory used by hashtables
thus vastly reducing the space that can be used for ADB names and
entries when the hashtables would grow. Create own memory context for
the ADB names and entries hash tables.
Ondřej Surý [Tue, 13 Dec 2022 12:48:55 +0000 (13:48 +0100)]
Lock the adbname and adbentry prior to unlocking hash locks
There was a datarace that could expire a freshly created ADB names and
entries between the return from get_attached_{name,entry} and locking it
again. Lock the ADB name and ADB entry inside the hash table lock, so
they won't get expired until the full initialization has been complete.
Aram Sargsyan [Wed, 14 Dec 2022 14:40:31 +0000 (14:40 +0000)]
Fix logging a uint32_t SOA serial value in dns_catz_update_from_db()
The dns_catz_update_from_db() function prints serial number as a signed
number (with "%d" in the format string), but the `vers` variable's type
is 'uint32_t'. This breaks serials bigger than 2^31.
Ondřej Surý [Wed, 7 Dec 2022 08:45:34 +0000 (09:45 +0100)]
Add internal logging functions to the netmgr
Add internal logging functions isc__netmgr_log, isc__nmsocket_log(), and
isc__nmhandle_log() that can be used to add logging messages to the
netmgr, and change all direct use of isc_log_write() to use those
logging functions to properly prefix them with netmgr, nmsocket and
nmsocket+nmhandle.
Ondřej Surý [Tue, 13 Dec 2022 14:20:10 +0000 (15:20 +0100)]
Revert the statistics system test change after we fixed the resolver
When the resolver was refactored, the statistics system test had to be
adjusted in c6b4d8255775a24a12b832a90a78cbf86e9faa8d. Unfortunately,
this change had to be done because of an error in the resolver
refactoring where timeout would not retry next server, but keep trying
the same server. As we have now fixed this bug, revert the change to
the test back to the previous state.
Ondřej Surý [Thu, 8 Dec 2022 09:46:09 +0000 (10:46 +0100)]
Allow zero length keys in isc_hashmap
In case, we are trying to hash the empty key into the hashmap, the key
is going to have zero length. This might happen in the unit test.
Allow this and add a unit test to ensure the empty zero-length key
doesn't hash to slot 0 as SipHash 2-4 (our hash function of choice) has
no problem with zero-length inputs.
Artem Boldariev [Fri, 9 Dec 2022 16:44:01 +0000 (18:44 +0200)]
Fix TLS session resumption via IDs when Mutual TLS is used
This commit fixes TLS session resumption via session IDs when
client certificates are used. To do so it makes sure that session ID
contexts are set within server TLS contexts. See OpenSSL documentation
for 'SSL_CTX_set_session_id_context()', the "Warnings" section.
Ondřej Surý [Tue, 13 Dec 2022 10:02:47 +0000 (11:02 +0100)]
Fix intermittent memory leak in dns_resolver unit
A rdataset could have been left unassociated on the error path in the
resume_dslookup() in the dns_resolver unit. Clone the rdataset after
the error check, so it's not cloned before we check whether we can make
further progress chasing DS records.
Mark Andrews [Tue, 13 Dec 2022 01:03:49 +0000 (12:03 +1100)]
Properly initialise local_ndata in isdotlocal in dig
Remove the trailing '\0' so that the length field of the dns_name_t
structure is correct. The old data just happens to work with
dns_name_issubdomain but would fail with dns_name_equal.