GitLab

Fix Coverity keymgr reports

Closes #2006

See merge request !3808

Fix Coverity CHECKED_RETURN reports for dst_key_getbool().  In most
cases we do not really care about its return value, but it is prudent
to check it.

In one case, where a dst_key_getbool() error should be treated
identically as success, cast the return value to void and add a relevant
comment.

Use "image" key in QEMU-based CI job templates

See merge request !3855

Our GitLab Runner Custom executor scripts now use the "image" key
instead of the job name for determining the QCOW2 image to use for a
given CI job.  Update .gitlab-ci.yml to reflect that change.

Fix re-signing when `sig-validity-interval` has two arguments

See merge request !3735

Check that resign interval is actually in days rather than hours
by checking that RRSIGs are all within the allowed day range.

Since October 2019 I have had complaints from `dnssec-cds` reporting
that the signatures on some of my test zones had expired. These were
zones signed by BIND 9.15 or 9.17, with a DNSKEY TTL of 24h and
`sig-validity-interval 10 8`.

This is the same setup we have used for our production zones since
2015, which is intended to re-sign the zones every 2 days, keeping
at least 8 days signature validity. The SOA expire interval is 7
days, so even in the presence of zone transfer problems, no-one
should ever see expired signatures. (These timers are a bit too
tight to be completely correct, because I should have increased
the expiry timers when I increased the DNSKEY TTLs from 1h to 24h.
But that should only matter when zone transfers are broken, which
was not the case for the error reports that led to this patch.)

For example, this morning my test zone contained:

        dev.dns.cam.ac.uk. 86400 IN RRSIG DNSKEY 13 5 86400 (
                                20200701221418 20200621213022 ...)

But one of my resolvers had cached:

        dev.dns.cam.ac.uk. 21424 IN RRSIG DNSKEY 13 5 86400 (
                                20200622063022 20200612061136 ...)

This TTL was captured at 20200622105807 so the resolver cached the
RRset 64976 seconds previously (18h02m56s), at 20200621165511
only about 12h before expiry.

The other symptom of this error was incorrect `resign` times in
the output from `rndc zonestatus`.

For example, I have configured a test zone

        zone fast.dotat.at {
                file "../u/z/fast.dotat.at";
                type primary;
                auto-dnssec maintain;
                sig-validity-interval 500 499;
        };

The zone is reset to a minimal zone containing only SOA and NS
records, and when `named` starts it loads and signs the zone. After
that, `rndc zonestatus` reports:

        next resign node: fast.dotat.at/NS
        next resign time: Fri, 28 May 2021 12:48:47 GMT

The resign time should be within the next 24h, but instead it is
near the signature expiry time, which the RRSIG(NS) says is
20210618074847. (Note 499 hours is a bit more than 20 days.)
May/June 2021 is less than 500 days from now because expiry time
jitter is applied to the NS records.

Using this test I bisected this bug to 09990672 which contained a
mistake leading to the resigning interval always being calculated in
hours, when days are expected.

This bug only occurs for configurations that use the two-argument form
of `sig-validity-interval`.

Merge branch '1994-netscope-c-23-50-error-unused-parameter-addr-when-have_if_nametoindex-undefined-on-illumos' into 'main'

Resolve "netscope.c:23:50: error: unused parameter 'addr' when HAVE_IF_NAMETOINDEX undefined on illumos"

Closes #1994

See merge request !3829

Also 'zone' should be initialised to zero.

Merge branch '1995-gssapictx-c-681-10-error-implicit-declaration-of-function-gsskrb5_register_acceptor_identity' into 'main'

Resolve "gssapictx.c:681:10: error: implicit declaration of function 'gsskrb5_register_acceptor_identity' on illumos"

Closes #1995

See merge request !3830

Merge branch '1993-check-c-1576-37-error-expected-identifier-before-numeric-constant-on-illumos' into 'main'

Resolve "check.c:1576:37: error: expected identifier before numeric constant on illumos"

Closes #1993

See merge request !3828

Resolve "Coverity is reporting double unlock."

Closes #2005

See merge request !3807

RNDC using netmgr

See merge request !3724

- make isccc message receiving code clearer by merging recv_nonce and
  recv_message into a single recv_data function and adding a boolean
  state field.

by having these functions act on netmgr handles instead of socket
objects, they can be used in callback functions outside the netgmr.

When we're shutting the system down via "rndc stop" or "rndc halt",
or reconfiguring the control channel, there are potential shutdown
races between the server task and network manager.  These are adressed by:

- purging any pending command tasks when shutting down the control channel
- adding an extra handle reference before the command handler to
  ensure the handle can't be deleted out from under us before calling
  command_respond()

- using an isc_task to execute all rndc functions makes it relatively
  simple for them to acquire task exclusive mode when needed
- control_recvmessage() has been separated into two functions,
  control_recvmessage() and control_respond(). the respond function
  can be called immediately from control_recvmessage() when processing
  a nonce, or it can be called after returning from the task event
  that ran the rndc command function.

- updated libisccc to use netmgr events
- updated rndc to use isc_nm_tcpconnect() to establish connections
- updated control channel to use isc_nm_listentcp()

open issues:

- the control channel timeout was previously 60 seconds, but it is now
  overridden by the TCP idle timeout setting, which defaults to 30
  seconds. we should add a function that sets the timeout value for
  a specific listener socket, instead of always using the global value
  set in the netmgr. (for the moment, since 30 seconds is a reasonable
  timeout for the control channel, I'm not prioritizing this.)
- the netmgr currently has no support for UNIX-domain sockets; until
  this is addressed, it will not be possible to configure rndc to use
  them. we will need to either fix this or document the change in
  behavior.

"showzone" and "tsig-list" both used exclusive mode unnecessarily;
changing this will simplify future refactoring a bit.

clean up style in rndc and the control channel in preparation for
changing them to use the new network manager.

ensure new_zone_lock is released after count_newzones()

See merge request !3833

it was possible for the count_newzones() function to try to
unlock view->new_zone_lock on return before locking it, which
caused a crash on shutdown.

Resolve "Observed stats underflow in multiple stats"

Closes #1719

See merge request !3818

The basic scenario for the problem was that in the process of
resolving a query, if any rrset was eligible for prefetching, then it
would trigger a call to query_prefetch(), this call would run in
parallel to the normal query processing.

The problem arises due to the fact that both query_prefetch(), and,
in the original thread, a call to ns_query_recurse(), try to attach
to the recursionquota, but recursing client stats counter is only
incremented if ns_query_recurse() attachs to it first.

Conversely, if fetch_callback() is called before prefetch_done(),
it would not only detach from recursionquota, but also decrement
the stats counter, if query_prefetch() attached to te quota first
that would result in a decrement not matched by an increment, as
expected.

To solve this issue an atomic bool was added, it is set once in
ns_query_recurse(), allowing fetch_callback() to check for it
and decrement stats accordingly.

For a more compreensive explanation check the thread comment below:
#1719 (comment 145857)

Fallback to built in trust-anchors, managed-keys, or trusted-keys

Closes #1235

See merge request !3827

if the bind.keys file cannot be parsed.

Merge branch '1989-rndc-dnstap-roll-with-too-big-a-argument-128-can-cause-a-buffer-overflow' into 'main'

Resolve "'rndc dnstap --roll' with too big a argument (>128) can cause a buffer overflow."

Closes #1989

See merge request !3788

If too many versions of log / dnstap files to be saved where requests
the memory after to_keep could be overwritten.  Force the number of
versions to be saved to a save level.  Additionally the memmove length
was incorrect.

Merge branch '2012-add-assertion-check-to-silence-dereference-before-null-check-in-tsig_test-c-v9_11' into 'main'

Resolve "Add assertion check to silence dereference before NULL check in tsig_test.c"

Closes #2012

See merge request !3814

Resolve "Statschannel system test failed at setup stage."

Closes #2014

See merge request !3823