After the network manager rewrite, tcp-higwater stats was only being
updated when a valid DNS query was received over tcp.
It turns out tcp-quota is updated right after a tcp connection is
accepted, before any data is read, so in the event that some client
connect but don't send a valid query, it wouldn't be taken into
account to update tcp-highwater stats, that is wrong.
This commit fix tcp-highwater to update its stats whenever a tcp connection
is established, independent of what happens after (timeout/invalid
request, etc).
- restore support for tcp-initial-timeout, tcp-idle-timeout,
tcp-keepalive-timeout and tcp-advertised-timeout configuration
options, which were ineffective previously.
when the TCPDNS_CLIENTS_PER_CONN limit has been exceeded for a TCP
DNS connection, switch to sequential mode to ensure that memory cannot
be exhausted by too many simultaneous queries.
- ns__client_request() is now called by netmgr with an isc_nmhandle_t
parameter. The handle can then be permanently associated with an
ns_client object.
- The task manager is paused so that isc_task events that may be
triggred during client processing will not fire until after the netmgr is
finished with it. Before any asynchronous event, the client MUST
call isc_nmhandle_ref(client->handle), to prevent the client from
being reset and reused while waiting for an event to process. When
the asynchronous event is complete, isc_nmhandle_unref(client->handle)
must be called to ensure the handle can be reused later.
- reference counting of client objects is now handled in the nmhandle
object. when the handle references drop to zero, the client's "reset"
callback is used to free temporary resources and reiniialize it,
whereupon the handle (and associated client) is placed in the
"inactive handles" queue. when the sysstem is shutdown and the
handles are cleaned up, the client's "put" callback is called to free
all remaining resources.
- because client allocation is no longer handled in the same way,
the '-T clienttest' option has now been removed and is no longer
used by any system tests.
- the unit tests require wrapping the isc_nmhandle_unref() function;
when LD_WRAP is supported, that is used. otherwise we link a
libwrap.so interposer library and use that.
The double-locked queue implementation is still currently in use
in ns_client, but will be replaced by a fetch-and-add array queue.
This commit moves it from queue.h to list.h so that queue.h can be
used for the new data structure, and clean up dependencies between
list.h and types.h. Later, when the ISC_QUEUE is no longer is use,
it will be removed completely.
This variable will report the maximum number of simultaneous tcp clients
that BIND has served while running.
It can be verified by running rndc status, then inspect "tcp high-water:
count", or by generating statistics file, rndc stats, then inspect the
line with "TCP connection high-water" text.
The tcp-highwater variable is atomically updated based on an existing
tcp-quota system handled in ns/client.c.
This commit changes the BIND cookie algorithms to match
draft-sury-toorop-dnsop-server-cookies-00. Namely, it changes the Client Cookie
algorithm to use SipHash 2-4, adds the new Server Cookie algorithm using SipHash
2-4, and changes the default for the Server Cookie algorithm to be siphash24.
Add siphash24 cookie algorithm, and make it keep legacy aes as
in the "refactor tcpquota and pipeline refs" commit, the counting
of active interfaces was tightened in such a way that named could
fail to listen on an interface if there were more interfaces than
tcp-clients. when checking the quota to start accepting on an
interface, if the number of active clients was above zero, then
it was presumed that some other client was able to handle accepting
new connections. this, however, ignored the fact that the current client
could be included in that count, so if the quota was already exceeded
before all the interfaces were listening, some interfaces would never
listen.
we now check whether the current client has been marked active; if so,
then the number of active clients on the interface must be greater
than 1, not 0.
(cherry picked from commit 02365b87ea0b1ea5ea8b17376f6734c811c95e61)
(cherry picked from commit cae79e1bab677ed1c2ce3adc5d54163a78f0d30b)
- if the TCP quota has been exceeded but there are no clients listening
for new connections on the interface, we can now force attachment to the
quota using isc_quota_force(), instead of carrying on with the quota not
attached.
- the TCP client quota is now referenced via a reference-counted
'ns_tcpconn' object, one of which is created whenever a client begins
listening for new connections, and attached to by members of that
client's pipeline group. when the last reference to the tcpconn
object is detached, it is freed and the TCP quota slot is released.
- reduce code duplication by adding mark_tcp_active() function
- convert counters to stdatomic
(cherry picked from commit a8dd133d270873b736c1be9bf50ebaa074f5b38f)
(cherry picked from commit 4a8fc979c49104534cf6be5d81dc54da5b6836c9)
- ensure that tcpactive is cleaned up correctly when accept() fails.
- set 'client->tcpattached' when the client is attached to the tcpquota.
carry this value on to new clients sharing the same pipeline group.
don't call isc_quota_detach() on the tcpquota unless tcpattached is
set. this way clients that were allowed to accept TCP connections
despite being over quota (and therefore, were never attached to the
quota) will not inadvertently detach from it and mess up the
accounting.
- simplify the code for tcpquota disconnection by using a new function
tcpquota_disconnect().
- before deciding whether to reject a new connection due to quota
exhaustion, check to see whether there are at least two active
clients. previously, this was "at least one", but that could be
insufficient if there was one other client in READING state (waiting
for messages on an open connection) but none in READY (listening
for new connections).
- before deciding whether a TCP client object can to go inactive, we
must ensure there are enough other clients to maintain service
afterward -- both accepting new connections and reading/processing new
queries. A TCP client can't shut down unless at least one
client is accepting new connections and (in the case of pipelined
clients) at least one additional client is waiting to read.
(cherry picked from commit 427a2fb4d17bc04ca3262f58a9dcf5c93fc6d33e)
(cherry picked from commit 08968412726d680777de6e596c836c6be07819a1)
Track pipeline groups using a shared reference counter
instead of a linked list.
(cherry picked from commit 31f392db20207a1b05d6286c3c56f76c8d69e574)
(cherry picked from commit 2211120222b5f008a96145474b7f6749d4307028)
the TCP client quota could still be ineffective under some
circumstances. this change:
- improves quota accounting to ensure that TCP clients are
properly limited, while still guaranteeing that at least one client
is always available to serve TCP connections on each interface.
- uses more descriptive names and removes one (ntcptarget) that
was no longer needed
- adds comments
(cherry picked from commit 9e74969f85329fe26df2fad390468715215e2edd)
(cherry picked from commit d7e84cee0bd7957a0707b86d47c29de4b798d350)
tcp-clients settings could be exceeded in some cases by
creating more and more active TCP clients that are over
the set quota limit, which in the end could lead to a
DoS attack by e.g. exhaustion of file descriptors.
If TCP client we're closing went over the quota (so it's
not attached to a quota) mark it as mortal - so that it
will be destroyed and not set up to listen for new
connections - unless it's the last client for a specific
interface.
(cherry picked from commit eafcff07c25bdbe038ae1e4b6660602a080b9395)
(cherry picked from commit 9e7617cc84f465769be1a3f426f30cd516220902)
- named could return FORMERR if parsing iterative responses
ended with a result code such as DNS_R_OPTERR. instead of
computing a response code based on the result, in this case
we now just force the response to be SERVFAIL.
- eliminate qctx->hookdata and client->hookflags.
- use a memory pool to allocate data blobs in the filter-aaaa module,
and associate them with the client address in a hash table
- instead of detaching the client in query_done(), mark it for deletion
and then call ns_client_detach() from qctx_destroy(); this ensures
that it will still exist when the QCTX_DESTROYED hook point is
reached.
- added a 'hookdata' array to qctx to store pointers to up to
16 blobs of data which are allocated by modules as needed.
each module is assigned an ID number as it's loaded, and this
is the index into the hook data array. this is to be used for
holding persistent state between calls to a hook module for a
specific query.
- instead of using qctx->filter_aaaa, we now use qctx->hookdata.
(this was the last piece of filter-aaaa specific code outside the
module.)
- added hook points for qctx initialization and destruction. we get
a filter-aaaa data pointer from the mempool when initializing and
store it in the qctx->hookdata table; return to to the mempool
when destroying the qctx.
- link the view to the qctx so that detaching the client doesn't cause
hooks to fail
- added a qctx_destroy() function which must be called after qctx_init;
this calls the QCTX_DESTROY hook and detaches the view
- general cleanup and comments
- these formerly static helper functions have been moved into client.c
and made external so that they can be used in hook modules as well as
internally in libns: query_newrdataset, query_putrdataset,
query_newnamebuf, query_newname, query_getnamebuf, query_keepname,
query_releasename, query_newdbversion, query_findversion
- made query_recurse() and query_done() into public functions
ns_query_recurse() and ns_query_done() so they can be called from
modules.
- the goal of this change is for AAAA filtering to be fully contained
in the query logic, and implemented at discrete points that can be
replaced with hook callouts later on.
- the new code may be slightly less efficient than the old filter-aaaa
implementation, but maximum efficiency was never a priority for AAAA
filtering anyway.
- we now use the rdataset RENDERED attribute to indicate that an AAAA
rdataset should not be included when rendering the message. (this
flag was originally meant to indicate that an rdataset has already
been rendered and should not be repeated, but it can also be used to
prevent rendering in the first place.)
- the DNS_MESSAGERENDER_FILTER_AAAA, NS_CLIENTATTR_FILTER_AAAA,
and DNS_RDATASETGLUE_FILTERAAAA flags are all now unnecessary and
have been removed.
- the purpose of this change is allow for more well-defined hook points
to be available in the query processing logic. some functions that
formerly didn't have access to 'qctx' do now; this is needed because
'qctx' is what gets passed when calling a hook function.
- query_addrdataset() has been broken up into three separate functions
since it used to do three unrelated things, and what was formerly
query_addadditional() has been renamed query_additional_cb() for
clarity.
- client->filter_aaaa is now qctx->filter_aaaa. (later, it will be moved
into opaque storage in the qctx, for use by the filter-aaaa module.)
- cleaned up style and braces
If we know that we'll have a task pool doing specific thing it's better
to use this knowledge and bind tasks to task queues, this behaves better
than randomly choosing the task queue.
- use bound resolver tasks - we have a pool of tasks doing resolutions,
we can spread the load evenly using isc_task_create_bound
- quantum set universally to 25
While implementing the new unit testing framework cmocka, it was found that the
BIND 9 code doesn't compile when assertions are disabled or replaced with any
function (such as mock_assert() from cmocka unit testing framework) that's not
directly recognized as assertion by the compiler.
This made the compiler to complain about blocks of code that was recognized as
unreachable before, but now it isn't.
The changes in this commit include:
* assigns default values to couple of local variables,
* moves some return statements around INSIST assertions,
* adds __builtin_unreachable(); annotations after some INSIST assertions,
* fixes one broken assertion (= instead of ==)
Rationale: the nonce here is only used to make sure there is a low
probability of duplication, according to section B.2 of RFC7873.
It is only 32-bit, and even if an attacker knows the algorithm used
to generate nonces it won't, in any way, give him any platform to
attack the server as long as server secret used to sign the
(nonce, time) pair with HMAC-SHA1 is secure.
On the other hand, currently, each packet sent requires (unnecessarily)
a CS pseudo-random number which is ineffective.
