Commit Graph

104 Commits

Author SHA1 Message Date
Ondřej Surý
301e4145de Fix the isc_nm_closedown() to actually close the pending connections
1. The isc__nm_tcp_send() and isc__nm_tcp_read() was not checking
   whether the socket was still alive and scheduling reads/sends on
   closed socket.

2. The isc_nm_read(), isc_nm_send() and isc_nm_resumeread() have been
   changed to always return the error conditions via the callbacks, so
   they always succeed.  This applies to all protocols (UDP, TCP and
   TCPDNS).

(cherry picked from commit f7c82e406e)
2020-10-22 15:00:00 -07:00
Evan Hunt
ba2e9dfb99 change from isc_nmhandle_ref/unref to isc_nmhandle attach/detach
Attaching and detaching handle pointers will make it easier to
determine where and why reference counting errors have occurred.

A handle needs to be referenced more than once when multiple
asynchronous operations are in flight, so callers must now maintain
multiple handle pointers for each pending operation. For example,
ns_client objects now contain:

        - reqhandle:    held while waiting for a request callback (query,
                        notify, update)
        - sendhandle:   held while waiting for a send callback
        - fetchhandle:  held while waiting for a recursive fetch to
                        complete
        - updatehandle: held while waiting for an update-forwarding
                        task to complete

(cherry picked from commit 57b4dde974)
2020-10-01 18:09:35 +02:00
Evan Hunt
b14cb9e2f1 restore "blackhole" functionality
the blackhole ACL was accidentally disabled with respect to client
queries during the netmgr conversion.

in order to make this work for TCP, it was necessary to add a return
code to the accept callback functions passed to isc_nm_listentcp() and
isc_nm_listentcpdns().

(cherry picked from commit 23c7373d68)
2020-10-01 16:44:43 +02:00
Evan Hunt
f64a881a30 change the signature of recv callbacks to include a result code
this will allow recv event handlers to distinguish between cases
in which the region is NULL because of error, shutdown, or cancelation.

(cherry picked from commit 75c985c07f)
2020-10-01 16:44:43 +02:00
Ondřej Surý
826ddb246e Revert the tree to allow cherry-picking netmgr changes from main
The following reverted changes will be picked again as part of the
netmgr sync with main branch.

Revert "Merge branch '1996-confidential-issue-v9_16' into 'security-v9_16'"

This reverts commit e160b1509f, reversing
changes made to c01e643715.

Revert "Merge branch '2038-use-freebind-when-bind-fails-v9_16' into 'v9_16'"

This reverts commit 5f8ecfb918, reversing
changes made to 23021385d5.

Revert "Merge branch '1936-blackhole-fix-v9_16' into 'v9_16'"

This reverts commit f20bc90a72, reversing
changes made to 490016ebf1.

Revert "Merge branch '1938-fix-udp-race' into 'v9_16'"

This reverts commit 0a6c7ab2a9, reversing
changes made to 4ea84740e6.

Revert "Merge branch '1947-fix-tcpdns-race' into 'v9_16'"

This reverts commit 4ea84740e6, reversing
changes made to d761cd576b.
2020-10-01 16:44:43 +02:00
Ondřej Surý
f0989bdf03 The dns_message_create() cannot fail, change the return to void
The dns_message_create() function cannot soft fail (as all memory
allocations either succeed or cause abort), so we change the function to
return void and cleanup the calls.

(cherry picked from commit 33eefe9f85)
2020-09-30 14:26:26 +02:00
Diego Fronza
da84f8d1fd Refactored dns_message_t for using attach/detach semantics
This commit will be used as a base for the next code updates in
order to have a better control of dns_message_t objects' lifetime.

(cherry picked from commit 12d6d13100)
2020-09-30 11:34:42 +10:00
Evan Hunt
df698d73f4 update all copyright headers to eliminate the typo 2020-09-14 16:50:58 -07:00
Mark Andrews
2dc26ebdb6 Map DNS_R_BADTSIG to FORMERR
Now that the log message has been printed set the result code to
DNS_R_FORMERR.  We don't do this via dns_result_torcode() as we
don't want upstream errors to produce FORMERR if that processing
end with DNS_R_BADTSIG.

(cherry picked from commit 20488d6ad3)
2020-08-04 23:04:34 +10:00
Evan Hunt
952461b6af restore "blackhole" functionality
the blackhole ACL was accidentally disabled with respect to client
queries during the netmgr conversion.

in order to make this work for TCP, it was necessary to add a return
code to the accept callback functions passed to isc_nm_listentcp() and
isc_nm_listentcpdns().

(cherry picked from commit 23c7373d68)
2020-06-30 21:10:31 -07:00
Mark Andrews
ee6f60349b Adjust NS_CLIENT_TCP_BUFFER_SIZE and cleanup client_allocsendbuf
NS_CLIENT_TCP_BUFFER_SIZE was 2 byte too large following the
move to netmgr add associated changes to lib/ns/client.c and
as a result an INSIST could be trigger if the DNS message being
constructed had a checkpoint stage that fell in those two extra
bytes.  Adjusted NS_CLIENT_TCP_BUFFER_SIZE and cleaned up
client_allocsendbuf now that the previously reserved 2 bytes
are no longer used.

(cherry picked from commit 5a92af19b7dce684b0e6670ae6ec1c4c58613263)
2020-06-18 10:18:41 +02:00
Evan Hunt
c19a593d3e change "expr == false" to "!expr" in conditionals
(cherry picked from commit 57e54c46e4)
2020-05-25 17:04:04 -07:00
Evan Hunt
00c816778d change 'expr == true' to 'expr' in conditionals
(cherry picked from commit 68a1c9d679)
2020-05-25 17:03:59 -07:00
Ondřej Surý
cacaa94350 Fix the statistic counter underflow in ns_client_t
In case of normal fetch, the .recursionquota is attached and
ns_statscounter_recursclients is incremented when the fetch is created.  Then
the .recursionquota is detached and the counter decremented in the
fetch_callback().

In case of prefetch or rpzfetch, the quota is attached, but the counter is not
incremented.  When we reach the soft-quota, the function returns early but don't
detach from the quota, and it gets destroyed during the ns_client_endrequest(),
so no memory was leaked.

But because the ns_statscounter_recursclients is only incremented during the
normal fetch the counter would be incorrectly decremented on two occassions:

1) When we reached the softquota, because the quota was not properly detached
2) When the prefetch or rpzfetch was cancelled mid-flight and the callback
   function was never called.

(cherry picked from commit 78886d4bed)
2020-04-03 20:22:56 +02:00
Witold Kręcicki
aec3dd28d6 Destroy query in killoldestquery under a lock.
Fixes a race between ns_client_killoldestquery and ns_client_endrequest -
killoldestquery takes a client from `recursing` list while endrequest
destroys client object, then killoldestquery works on a destroyed client
object. Prevent it by holding reclist lock while cancelling query.

(cherry picked from commit df3dbdff81)
2020-03-05 23:55:42 +00:00
Evan Hunt
d794d85ce1 comments
(cherry picked from commit 0b76d8a490)
2020-02-28 10:05:25 +01:00
Witold Kręcicki
e11a8e832e Don't define NS_CLIENT_TRACE by default
(cherry picked from commit 4b6a064972)
2020-02-28 10:05:25 +01:00
Witold Kręcicki
0edf2e290b Remove some stale fields from ns_client_t; make sendbuf allocated on heap
(cherry picked from commit 8c6c07286f)
2020-02-28 10:05:25 +01:00
Witold Kręcicki
d0b8c6a378 Don't check if the client is on recursing list (requires locking) if it's not RECURSING
(cherry picked from commit 938b61405b)
2020-02-28 10:05:25 +01:00
Witold Kręcicki
c8dd7e07cd Don't issue ns_client_endrequest on a NS_CLIENTSTATE_READY client.
Fix a potential assertion failure on shutdown in ns__client_endrequest.
Scenario:
1. We are shutting down, interface->clientmgr is gone.
2. We receive a packet, it gets through ns__client_request
3. mgr == NULL, return
4. isc_nmhandle_detach calls ns_client_reset_cb
5. ns_client_reset_cb calls ns_client_endrequest
6. INSIST(client->state == NS_CLIENTSTATE_WORKING ||
          client->state == NS_CLIENTSTATE_RECURSING) is not met
   - we haven't started processing this packet so
   client->state == NS_CLIENTSTATE_READY.
As a solution - don't do anything in ns_client_reset_cb if the client
is still in READY state.

(cherry picked from commit b0888ff039)
2020-02-27 12:08:02 +00:00
Witold Kręcicki
1b9df6779d Use thread-friendly mctxpool and taskpool in ns_client.
Make ns_client mctxpool more thread-friendly by sharding it by
netmgr threadid, use task pool also sharded by thread id to avoid
lock contention.
2020-02-18 11:57:46 +01:00
Ondřej Surý
829b461c54 Merge branch '46-enforce-clang-format-rules' into 'master'
Start enforcing the clang-format rules on changed files

Closes #46

See merge request isc-projects/bind9!3063

(cherry picked from commit a04cdde45d)

d2b5853b Start enforcing the clang-format rules on changed files
618947c6 Switch AlwaysBreakAfterReturnType from TopLevelDefinitions to All
654927c8 Add separate .clang-format files for headers
5777c44a Reformat using the new rules
60d29f69 Don't enforce copyrights on .clang-format
2020-02-14 08:45:59 +00:00
Ondřej Surý
cdef20bb66 Merge branch 'each-style-tweak' into 'master'
adjust clang-format options to get closer to ISC style

See merge request isc-projects/bind9!3061

(cherry picked from commit d3b49b6675)

0255a974 revise .clang-format and add a C formatting script in util
e851ed0b apply the modified style
2020-02-14 05:35:29 +00:00
Ondřej Surý
2e55baddd8 Merge branch '46-add-curly-braces' into 'master'
Add curly braces using uncrustify and then reformat with clang-format back

Closes #46

See merge request isc-projects/bind9!3057

(cherry picked from commit 67b68e06ad)

36c6105e Use coccinelle to add braces to nested single line statement
d14bb713 Add copy of run-clang-tidy that can fixup the filepaths
056e133c Use clang-tidy to add curly braces around one-line statements
2020-02-13 21:28:35 +00:00
Ondřej Surý
c931d8e417 Merge branch '46-just-use-clang-format-to-reformat-sources' into 'master'
Reformat source code with clang-format

Closes #46

See merge request isc-projects/bind9!2156

(cherry picked from commit 7099e79a9b)

4c3b063e Import Linux kernel .clang-format with small modifications
f50b1e06 Use clang-format to reformat the source files
11341c76 Update the definition files for Windows
df6c1f76 Remove tkey_test (which is no-op anyway)
2020-02-12 14:51:18 +00:00
Ondřej Surý
bc1d4c9cb4 Clear the pointer to destroyed object early using the semantic patch
Also disable the semantic patch as the code needs tweaks here and there because
some destroy functions might not destroy the object and return early if the
object is still in use.
2020-02-09 18:00:17 -08:00
Ondřej Surý
41fe9b7a14 Formatting issues found by local coccinelle run 2020-02-08 03:12:09 -08:00
Ondřej Surý
c73e5866c4 Refactor the isc_buffer_allocate() usage using the semantic patch
The isc_buffer_allocate() function now cannot fail with ISC_R_MEMORY.
This commit removes all the checks on the return code using the semantic
patch from previous commit, as isc_buffer_allocate() now returns void.
2020-02-03 08:29:00 +01:00
Witold Kręcicki
8d6dc8613a clean up some handle/client reference counting errors in error cases.
We weren't consistent about who should unreference the handle in
case of network error. Make it consistent so that it's always the
client code responsibility to unreference the handle - either
in the callback or right away if send function failed and the callback
will never be called.
2020-01-20 22:28:36 +01:00
Witold Kręcicki
dcc0835a3a cleanup properly if we fail to initialize ns_client structure
If taskmgr is shutting down ns_client_setup will fail to create
a task for the newly created client, we weren't cleaning up already
created/attached things (memory context, server, clientmgr).
2020-01-20 22:28:36 +01:00
Witold Kręcicki
eda4300bbb netmgr: have a single source of truth for tcpdns callback
We pass interface as an opaque argument to tcpdns listening socket.
If we stop listening on an interface but still have in-flight connections
the opaque 'interface' is not properly reference counted, and we might
hit a dead memory. We put just a single source of truth in a listening
socket and make the child sockets use that instead of copying the
value from listening socket. We clean the callback when we stop listening.
2020-01-15 17:22:13 +01:00
Ondřej Surý
7c3e342935 Use isc_refcount_increment0() where appropriate 2020-01-14 13:12:13 +01:00
Ondřej Surý
fbf9856f43 Add isc_refcount_destroy() as appropriate 2020-01-14 13:12:13 +01:00
Diego Fronza
ed9853e739 Fix tcp-highwater stats updating
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).
2019-12-12 11:23:10 -08:00
Evan Hunt
715afa9c57 add a stats counter for clients dropped due to recursive-clients limit 2019-11-26 17:55:06 +00:00
Evan Hunt
199bd6b623 netmgr: make TCP timeouts configurable
- restore support for tcp-initial-timeout, tcp-idle-timeout,
  tcp-keepalive-timeout and tcp-advertised-timeout configuration
  options, which were ineffective previously.
2019-11-22 16:46:31 -08:00
Evan Hunt
123ee350dc place a limit on pipelined queries that can be processed simultaneously
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.
2019-11-17 18:59:39 -08:00
Ondřej Surý
e95af30b23 Make lib/ns Thread Sanitizer clean 2019-11-17 17:42:41 -08:00
Evan Hunt
b9a5508e52 remove ISC_QUEUE as it is no longer used 2019-11-07 11:55:37 -08:00
Evan Hunt
53f0b6c34d convert ns_client and related objects to use netmgr
- 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.
2019-11-07 11:55:37 -08:00
Evan Hunt
64e1a4a398 temporarily move ISC_QUEUE to list.h
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.
2019-11-07 11:55:37 -08:00
Diego Fronza
66fe8627de Added TCP high-water statistics variable
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.
2019-11-06 09:18:27 +01:00
Ondřej Surý
b4a42a286f lib/ns/client.c: Fix invalid order of DbC checks that could cause dereference before NULL check 2019-10-03 09:04:27 +02:00
Mark Andrews
b59fe46e76 address or suppress cppcheck warnings 2019-09-12 17:59:28 +10:00
Ondřej Surý
4957255d13 Use the semantic patch to change the usage isc_mem_create() to new API 2019-09-12 09:26:09 +02:00
Ondřej Surý
ae83801e2b Remove blocks checking whether isc_mem_get() failed using the coccinelle 2019-07-23 15:32:35 -04:00
Ondřej Surý
a912f31398 Add new default siphash24 cookie algorithm, but keep AES as legacy
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
2019-07-21 15:16:28 -04:00
Witold Kręcicki
afa81ee4e4 Remove all cookie algorithms but AES, which was used as a default, for legacy purposes. 2019-07-21 10:08:14 -04:00
Witold Kręcicki
de73904d03 lib/ns/client: use refcount_t for reference counting 2019-07-09 16:09:36 +02:00
Ondřej Surý
8965a0ba98 Replace atomic operations in bin/named/client.c with isc_refcount reference counting
(cherry picked from commit ef49780d30)
(cherry picked from commit e203d4d65a)
2019-04-26 22:14:26 +02:00