Commit Graph

33850 Commits

Author SHA1 Message Date
Ondřej Surý
2eae7813b6 Run isc__nm_http_stoplistening() synchronously in netmgr
When isc__nm_http_stoplistening() is run from inside the netmgr, we need
to make sure it's run synchronously.  This commit is just a band-aid
though, as the desired behvaior for isc_nm_stoplistening() is not always
the same:

  1. When run from outside user of the interface, the call must be
     synchronous, e.g. the calling code expects the call to really stop
     listening on the interfaces.

  2. But if there's a call from listen<proto> when listening fails,
     that needs to be scheduled to run asynchronously, because
     isc_nm_listen<proto> is being run in a paused (interlocked)
     netmgr thread and we could get stuck.

The proper solution would be to make isc_nm_stoplistening()
behave like uv_close(), i.e., to have a proper callback.
2021-05-07 14:28:32 -07:00
Evan Hunt
5c08f97791 only run tasks as privileged if taskmgr is in privileged mode
all zone loading tasks have the privileged flag, but we only want
them to run as privileged tasks when the server is being initialized;
if we privilege them the rest of the time, the server may hang for a
long time after a reload/reconfig. so now we call isc_taskmgr_setmode()
to turn privileged execution mode on or off in the task manager.

isc_task_privileged() returns true if the task's privilege flag is
set *and* the taskmgr is in privileged execution mode. this is used
to determine in which netmgr event queue the task should be run.
2021-05-07 14:28:30 -07:00
Ondřej Surý
29a208aaf7 Fix crash when allocating UDP socket fails on OpenBSD
When socket() call fails, the UDP connect code would call the connectcb
with empty req->handle.  This has been fixed.
2021-05-07 14:28:30 -07:00
Ondřej Surý
0b491913df Don't clear dig lookup if it was already cleared
This workarounds couple of races where the current_lookup would be
already detached during shutting down the dig, but still processing the
pending reads.
2021-05-07 14:28:30 -07:00
Ondřej Surý
2836bc1854 Fix wrong query accounting in the connect function in dighost.c
The start_udp() function didn't properly attach to the query and thus
a callback with ISC_R_CANCELED would end with wrong accounting on the
query object.

Usually, this doesn't happen because underlying libuv API
uv_udp_connect() is synchronous, but isc_nm_udpconnect() could return
ISC_R_CANCELED in case it's called while the netmgr is shutting down.
2021-05-07 14:28:30 -07:00
Ondřej Surý
dacf586e18 Make the netmgr queue processing quantized
There was a theoretical possibility of clogging up the queue processing
with an endless loop where currently processing netievent would schedule
new netievent that would get processed immediately.  This wasn't such a
problem when only netmgr netievents were processed, but with the
addition of the tasks, there are at least two situation where this could
happen:

 1. In lib/dns/zone.c:setnsec3param() the task would get re-enqueued
    when the zone was not yet fully loaded.

 2. Tasks have internal quantum for maximum number of isc_events to be
    processed, when the task quantum is reached, the task would get
    rescheduled and then immediately processed by the netmgr queue
    processing.

As the isc_queue doesn't have a mechanism to atomically move the queue,
this commit adds a mechanism to quantize the queue, so enqueueing new
netievents will never stop processing other uv_loop_t events.
The default quantum size is 128.

Since the queue used in the network manager allows items to be enqueued
more than once, tasks are now reference-counted around task_ready()
and task_run(). task_ready() now has a public API wrapper,
isc_task_ready(), that the netmgr can use to reschedule processing
of a task if the quantum has been reached.

Incidental changes: Cleaned up some unused fields left in isc_task_t
and isc_taskmgr_t after the last refactoring, and changed atomic
flags to atomic_bools for easier manipulation.
2021-05-07 14:28:30 -07:00
Ondřej Surý
b5bf58b419 Destroy netmgr before destroying taskmgr
With taskmgr running on top of netmgr, the ordering of how the tasks and
netmgr shutdown interacts was wrong as previously isc_taskmgr_destroy()
was waiting until all tasks were properly shutdown and detached.  This
responsibility was moved to netmgr, so we now need to do the following:

  1. shutdown all the tasks - this schedules all shutdown events onto
     the netmgr queue

  2. shutdown the netmgr - this also makes sure all the tasks and
     events are properly executed

  3. Shutdown the taskmgr - this now waits for all the tasks to finish
     running before returning

  4. Shutdown the netmgr - this call waits for all the netmgr netievents
     to finish before returning

This solves the race when the taskmgr object would be destroyed before
all the tasks were finished running in the netmgr loops.
2021-05-07 14:28:30 -07:00
Ondřej Surý
a011d42211 Add new isc_managers API to simplify <*>mgr create/destroy
Previously, netmgr, taskmgr, timermgr and socketmgr all had their own
isc_<*>mgr_create() and isc_<*>mgr_destroy() functions.  The new
isc_managers_create() and isc_managers_destroy() fold all four into a
single function and makes sure the objects are created and destroy in
correct order.

Especially now, when taskmgr runs on top of netmgr, the correct order is
important and when the code was duplicated at many places it's easy to
make mistake.

The former isc_<*>mgr_create() and isc_<*>mgr_destroy() functions were
made private and a single call to isc_managers_create() and
isc_managers_destroy() is required at the program startup / shutdown.
2021-05-07 10:19:05 -07:00
Artem Boldariev
f23afce683 Merge branch 'artem/doh-tests-fix' into 'main'
Fix flawed DoH unit tests logic and some corner cases in the DoH code. Fix doh_test failure on FreeBSD 13.0

Closes #2632

See merge request isc-projects/bind9!5005
2021-05-07 13:25:56 +00:00
Artem Boldariev
8c0ea01f34 DoH: close active server streams when finishing session
Under some circumstances a situation might occur when server-side
session gets finished while there are still active HTTP/2
streams. This would lead to isc_nm_httpsocket object leaks.

This commit fixes this behaviour as well as refactors failed_read_cb()
to allow better code reuse.
2021-05-07 15:47:24 +03:00
Artem Boldariev
a9e97f28b7 Fix crash in client side DoH code
This commit fixes a situation when a cstream object could get unlinked
from the list as a result of a cstream->read_cb call. Thus, unlinking
it after the call could crash the program.
2021-05-07 15:47:24 +03:00
Artem Boldariev
cd178043d9 Make some TLS tests actually use quota
A directive to check quota was missing from some of the TLS tests
which were supposed to test TLS code with quotas.
2021-05-07 15:47:24 +03:00
Artem Boldariev
22376fc69a TLS: cancel reading on the underlying TCP socket after (see below)
... the last handle has been detached after calling write
callback. That makes it possible to detach from the underlying socket
and not to keep the socket object alive for too long. This issue was
causing TLS tests with quota to fail because quota might not have been
detached on time (because it was still referenced by the underlying
TCP socket).

One could say that this commit is an ideological continuation of:

513cdb52ec.
2021-05-07 15:47:24 +03:00
Artem Boldariev
3bf331c453 Fix crashes in TLS when handling TLS shutdown messages
This commit fixes some situations which could appear in TLS code when
dealing with shutdown messages and lead to crashes.
2021-05-07 15:47:24 +03:00
Artem Boldariev
0d3f503dc9 Avoid creating connect netievents during low level failures in HTTP
This way we create less netievent objects, not bombarding NM with the
messages in case of numerous low-level errors (like too many open
files) in e.g. unit tests.
2021-05-07 15:47:24 +03:00
Artem Boldariev
0e8ac61d6e Avoid creating httpclose netievents in case of low level failures
This way we create less load on NM workers by avoiding netievent
creation.
2021-05-07 15:47:24 +03:00
Artem Boldariev
8510c5cd59 Always call TCP connect callback from within a worker context
This change ensures that a TCP connect callback is called from within
the context of a worker thread in case of a low-level error when
descriptors cannot be created (e.g. when there are too many open file
descriptors).
2021-05-07 15:47:24 +03:00
Artem Boldariev
1349142333 Got rid of tlsconnect event and corresponding code
We do not need it since we decided to not return values from connect
functions.
2021-05-07 15:47:24 +03:00
Artem Boldariev
39448c1581 Finish HTTP session on write failure
Not doing so caused client-side code to not free file descriptors as
soon as possible, that was causing unit tests to fail.
2021-05-07 15:47:24 +03:00
Artem Boldariev
4c5b36780b Fix flawed DoH unit tests logic
This commit fixes some logical mistakes in DoH unit tests logic,
causing them either to fail or not to do what they are intended to do.
2021-05-07 15:47:24 +03:00
Michal Nowak
b31b29d166 Merge branch 'mnowak/add-placeholder-for-issue-2671' into 'main'
Add placeholder for #2671

See merge request isc-projects/bind9!5004
2021-05-07 11:15:42 +00:00
Michal Nowak
8cc6725280 Add placeholder for #2671
Issue #2671 was merged to 9.16 but a respective placeholder was not
added to "main".
2021-05-07 12:08:40 +02:00
Matthijs Mekking
cf99c1dc85 Merge branch '2603-test-dnssec-policy-none' into 'main'
Check zonefile is untouched if dnssec-policy none

Closes #2603

See merge request isc-projects/bind9!4888
2021-05-05 17:15:58 +00:00
Matthijs Mekking
66f2cd228d Use isdigit instead of checking character range
When looking for key files, we could use isdigit rather than checking
if the character is within the range [0-9].

Use (unsigned char) cast to ensure the value is representable in the
unsigned char type (as suggested by the isdigit manpage).

Change " & 0xff" occurrences to the recommended (unsigned char) type
cast.
2021-05-05 19:15:33 +02:00
Matthijs Mekking
0c09867e96 Changes and release notes for [#2603] 2021-05-05 19:15:32 +02:00
Matthijs Mekking
511bc1b882 Check for filename clashes /w dnssec-policy zones
Just like with dynamic and/or inline-signing zones, check if no two
or more zone configurations set the same filename. In these cases,
the zone files are not read-only and named-checkconf should catch
a configuration where multiple zone statements write to the same file.

Add some bad configuration tests where KASP zones reference the same
zone file.

Update the good-kasp test to allow for two zones configure the same
file name, dnssec-policy none.
2021-05-05 19:13:55 +02:00
Matthijs Mekking
2d1b3a9899 Check zonefile is untouched if dnssec-policy none
Make sure no DNSSEC contents are added to the zonefile if dnssec-policy
is set to "none" (and no .state files exist for the zone).
2021-05-05 19:13:55 +02:00
Mark Andrews
d8cc16a659 Merge branch '2670-always-set-rewrite-when-compacting-a-version-1-journal' into 'main'
Resolve "Always set 'rewrite' when compacting a version 1 journal."

Closes #2670

See merge request isc-projects/bind9!4985
2021-05-05 13:33:55 +00:00
Mark Andrews
a60b54e1df Add release note for [GL #2670] 2021-05-05 23:13:55 +10:00
Mark Andrews
79da175a76 Add CHANGES note for [GL #2670] 2021-05-05 23:13:09 +10:00
Mark Andrews
ae1ae07b03 Check journal compaction 2021-05-05 23:12:37 +10:00
Mark Andrews
4a8e33b9f0 Always perform a re-write when processing a version 1 journal
version 1 journals may have a mix of type 1 and type 2 transaction
headers so always use the recovery code.
2021-05-05 23:12:37 +10:00
Mark Andrews
71df4fb84c Allow named-journalprint to compact journals at a given serial 2021-05-05 23:12:37 +10:00
Matthijs Mekking
cecc905b46 Merge branch 'matthijs-follow-up-2596' into 'main'
Remove double "insecure" check

See merge request isc-projects/bind9!4997
2021-05-05 10:48:38 +00:00
Matthijs Mekking
22243ac804 Remove double "insecure" check
This was a conflict between dba13d280a
and 636ff1e15c.
2021-05-05 12:45:47 +02:00
Matthijs Mekking
09ed248f60 Merge branch '2596-dnssec-policy-keys-inaccessible' into 'main'
Don't roll keys when the private key file is offline

Closes #2596

See merge request isc-projects/bind9!4885
2021-05-05 10:33:46 +00:00
Matthijs Mekking
636ff1e15c No longer need to strcmp for "none"
When we introduced "dnssec-policy insecure" we could have removed the
'strcmp' check for "none", because if it was set to "none", the 'kasp'
variable would have been set to NULL.
2021-05-05 11:23:53 +02:00
Matthijs Mekking
366ed047dd Changes and release notes for [#2596] 2021-05-05 11:14:35 +02:00
Matthijs Mekking
4a8ad0a77f Add kasp tests for offline keys
Add a test for default.kasp that if we remove the private key file,
no successor key is created for it. We need to update the kasp script
to deal with a missing private key. If this is the case, skip checks
for private key files.

Add a test with a zone for which the private key of the ZSK is missing.

Add a test with a zone for which the private key of the KSK is missing.
2021-05-05 11:14:02 +02:00
Matthijs Mekking
6a60bf637d Update smart signing when key is offline
BIND 9 is smart about when to sign with what key. If a key is offline,
BIND will delete the old signature anyway if there is another key to
sign the RRset with.

With KASP we don't want to fallback to the KSK if the ZSK is missing,
only for the SOA RRset. If the KSK is missing, but we do have a ZSK,
deleting the signature is fine. Otherwise it depends on if we use KASP
or not. Update the 'delsig_ok' function to reflect that.
2021-05-05 11:13:19 +02:00
Matthijs Mekking
3e6fc49c16 Don't roll offline keys
When checking the current DNSSEC state against the policy, consider
offline keys. If we didn't found an active key, check if the key is
offline by checking the public key list. If there is a match in the
public key list (the key data is retrieved from the .key and the
.state files), treat the key as offline and don't create a successor
key for it.
2021-05-05 11:13:19 +02:00
Matthijs Mekking
b3a5859a9b rndc dnssec -status should include offline keys
The rndc command 'dnssec -status' only considered keys from
'dns_dnssec_findmatchingkeys' which only includes keys with accessible
private keys. Change it so that offline keys are also listed in the
status.
2021-05-05 11:13:19 +02:00
Matthijs Mekking
7ed089576f Try to read state when reading keylist from rdata
The function 'dns_dnssec_keylistfromrdataset()' creates a keylist from
the DNSKEY RRset. If we attempt to read the private key, we also store
the key state. However, if the private key is offline, the key state
will not be stored. To fix this, first attempt to read the public key
file. If then reading the private key file fails, and we do have a
public key, add that to the keylist, with appropriate state. If we
also failed to read the public key file, add the DNSKEY to the keylist,
as we did before.
2021-05-05 11:13:19 +02:00
Matthijs Mekking
fa05c1b8da When reading public key from file, also read state
The 'dst_key_fromnamedfile()' function did not read and store the
key state from the .state file when reading a public key file.
2021-05-05 11:13:19 +02:00
Matthijs Mekking
cf17698f87 Fix a kasp lock issue
The kasp lock would stay locked if 'dns_keymgr_run' failed.
2021-05-05 11:13:19 +02:00
Mark Andrews
2be319b759 Merge branch '2678-named-checkconf-doesn-t-catch-redefinition-of-dnssec-policy-insecure' into 'main'
Resolve "named-checkconf doesn't catch redefinition of dnssec-policy insecure"

Closes #2678

See merge request isc-projects/bind9!4994
2021-05-05 07:03:12 +00:00
Mark Andrews
dba13d280a named-checkconf now detects redefinition of dnssec-policy 'insecure' 2021-05-05 16:23:19 +10:00
Mark Andrews
0f53872542 Merge branch '2536-inline-signing-documentation-doesn-t-match-reality' into 'main'
Resolve "inline-signing documentation doesn't match reality"

Closes #2536

See merge request isc-projects/bind9!4751
2021-05-05 00:17:40 +00:00
Matthijs Mekking
a548a450b3 checkconf tests for inline-signing at options/view 2021-05-04 23:35:59 +00:00
Mark Andrews
03978a7881 Add Release note for [GL #2536] 2021-05-04 23:35:59 +00:00