Previously, taskmgr was using ISC_LIST for keeping the enqueued tasks
and the ISC_LIST have an inherent property that the task would not get
enqueued twice. When taskmgr switched to using netmgr, we are using
isc_queue_t which can enqueue same item multiple times, and this was
causing problem when the previous isc_task_run() would find the task to
be empty and shutting down. It would directly destroy the task and
leave the following netievents with already freed task.
This commit adds references counting around task_ready(++) and
task_run(--) and adds isc_task_ready() public API wrapper around
task_ready, so we can call it from netmgr when the event quantum would
be reached.
... 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.
This workarounds couple of races where the current_lookup would be
already detached during shutting down the dig, but still processing the
pending reads.
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 mechanism to quantize the queue, so enqueueing new
netievents will never stop processing other uv_loop_t events.
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.
After the last refactoring, the struct isc_task and struct isc_taskmgr
had couple of unused fields. In this commit, we remove unused fields
from the both structs and convert the isc_task_t atomic flags into two
atomic_bools for easier manipulation.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
