2
0
mirror of https://gitlab.isc.org/isc-projects/bind9 synced 2025-08-22 18:19:42 +00:00

61 Commits

Author SHA1 Message Date
Matthijs Mekking
dc6dafdad1 Ignore max-zone-ttl on dnssec-policy insecure
Allow larger TTL values in zones that go insecure. This is necessary
because otherwise the zone will not be loaded due to the max-zone-ttl
of P1D that is part of the current insecure policy.

In the keymgr.c code, default back to P1D if the max-zone-ttl is set
to zero.
2023-08-01 08:56:52 +02:00
Matthijs Mekking
e752656a38 Add key state init debugging
When debugging an issue it can be useful to see what BIND initially
set the key states to.
2023-04-17 10:56:08 +02:00
Tony Finch
6927a30926 Remove do-nothing header <isc/print.h>
This one really truly did nothing. No lines added!
2023-02-15 16:44:47 +00:00
Matthijs Mekking
ee42f66fbe Force set DS state after 'rndc dnssec -checkds'
Set the DS state after issuing 'rndc dnssec -checkds'. If the DS
was published, it should go in RUMOURED state, regardless whether it
is already safe to do so according to the state machine.

Leaving it in HIDDEN (or if it was magically already in OMNIPRESENT or
UNRETENTIVE) would allow for easy shoot in the foot situations.

Similar, if the DS was withdrawn, the state should be set to
UNRETENTIVE. Leaving it in OMNIPRESENT (or RUMOURED/HIDDEN)
would also allow for easy shoot in the foot situations.
2023-01-27 15:07:26 +00:00
Michal Nowak
afdb41a5aa
Update sources to Clang 15 formatting 2022-11-29 08:54:34 +01:00
Matthijs Mekking
5d6f0de84b Nit changes in keymgr and kasp
Use the ISC_MAX define instead of "x = a > b ? a : b" paradigm.

Remove an unneeded include.
2022-06-28 11:56:31 +02:00
Matthijs Mekking
955a69109e Only log "new successor in ..." if prepub != 0
If 'prepub' is 0, this has the special meaning that no rollover is
scheduled. If so, don't log "new successor in x seconds".
2022-05-31 15:45:14 +02:00
Matthijs Mekking
1da91b3ab4 Check if key metadata is modified before writing
Add a new parameter to the dst_key structure, mark a key modified if
dst_key_(un)set[bool,num,state,time] is called. Only write out key
files during a keymgr run if the metadata has changed.
2022-05-13 13:31:17 +02:00
Ondřej Surý
584f0d7a7e Simplify way we tag unreachable code with only ISC_UNREACHABLE()
Previously, the unreachable code paths would have to be tagged with:

    INSIST(0);
    ISC_UNREACHABLE();

There was also older parts of the code that used comment annotation:

    /* NOTREACHED */

Unify the handling of unreachable code paths to just use:

    UNREACHABLE();

The UNREACHABLE() macro now asserts when reached and also uses
__builtin_unreachable(); when such builtin is available in the compiler.
2022-03-25 08:33:43 +01:00
Evan Hunt
e8ac7cf6ec remove error handling code around dns_dnsseckey_create()
this function can no longer fail, so error checking is not necessary.
2022-01-31 10:39:04 -08:00
Ondřej Surý
58bd26b6cf Update the copyright information in all files in the repository
This commit converts the license handling to adhere to the REUSE
specification.  It specifically:

1. Adds used licnses to LICENSES/ directory

2. Add "isc" template for adding the copyright boilerplate

3. Changes all source files to include copyright and SPDX license
   header, this includes all the C sources, documentation, zone files,
   configuration files.  There are notes in the doc/dev/copyrights file
   on how to add correct headers to the new files.

4. Handle the rest that can't be modified via .reuse/dep5 file.  The
   binary (or otherwise unmodifiable) files could have license places
   next to them in <foo>.license file, but this would lead to cluttered
   repository and most of the files handled in the .reuse/dep5 file are
   system test files.
2022-01-11 09:05:02 +01:00
Ondřej Surý
2e3a2eecfe Make isc_result a static enum
Remove the dynamic registration of result codes.  Convert isc_result_t
from unsigned + #defines into 32-bit enum type in grand unified
<isc/result.h> header.  Keep the existing values of the result codes
even at the expense of the description and identifier tables being
unnecessary large.

Additionally, add couple of:

    switch (result) {
    [...]
    default:
        break;
    }

statements where compiler now complains about missing enum values in the
switch statement.
2021-10-06 11:22:20 +02:00
Matthijs Mekking
3ea953512a Migrate a single key to CSK with dnssec-policy
When migrating keys to dnssec-policy, if a zone has only one key,
assume it is going to be a CSK.
2021-08-23 09:53:51 +02:00
Matthijs Mekking
1a50554963 Add checkds log notice
When the checkds published/withdrawn is activated, log a notice. Can
be used for testing, but also operationally useful.
2021-06-30 17:28:48 +02:00
Matthijs Mekking
19395fd168 Fix coverity issue 331478
Move the "cannot start rollover" warning into code block that checks
if 'active_key' is not NULL.
2021-05-19 00:45:54 +00: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
668301f138 Check for keyid conflicts between new keys
When the keymgr needs to create new keys, it is possible it needs to
create multiple keys. The keymgr checks for keyid conflicts with
already existing keys, but it should also check against that it just
created.
2021-04-26 10:42:46 +02:00
Matthijs Mekking
27e7d5f698 Fix keymgr key init bug
The 'keymgr_key_init()' function initializes key states if they have
not been set previously. It looks at the key timing metadata and
determines using the given times whether a state should be set to
RUMOURED or OMNIPRESENT.

However, the DNSKEY and ZRRSIG states were mixed up: When looking
at the Activate timing metadata we should set the ZRRSIG state, and
when looking at the Published timing metadata we should set the
DNSKEY state.
2021-03-22 09:50:05 +01:00
Matthijs Mekking
8c526cb67f Purge keys implementation
On each keymgr run, we now also check if key files can be removed.
The 'purge-keys' interval determines how long keys should be retained
after they have become completely hidden.

Key files should not be removed if it has a state that is set to
something else then HIDDEN, if purge-keys is 0 (disabled), if
the key goal is set to OMNIPRESENT, or if the key is unused (a key is
unused if no timing metadata set, and no states are set or if set,
they are set to HIDDEN).

If the last changed timing metadata plus the purge-keys interval is
in the past, the key files may be removed.

Add a dst_key_t variable 'purge' to signal that the key file should
not be written to file again.
2021-02-23 09:16:48 +01:00
Matthijs Mekking
98ace6d97d Use NUM_KEYSTATES constant where appropriate
We use the number 4 a lot when working on key states. Better to use
the NUM_KEYSTATES constant instead.
2021-02-03 15:35:06 +01:00
Matthijs Mekking
189d9a2d21 Cleanup keymgr.c
Three small cleanups:

1. Remove an unused keystr/dst_key_format.
2. Initialize a dst_key_state_t state with NA.
3. Update false comment about local policy (local policy only adds
   barrier on transitions to the RUMOURED state, not the UNRETENTIVE
   state).
2021-02-03 15:35:06 +01:00
Matthijs Mekking
291bcc3721 Fix DS/DNSKEY hidden or chained functions
There was a bug in function 'keymgr_ds_hidden_or_chained()'.

The funcion 'keymgr_ds_hidden_or_chained()' implements (3e) of rule2
as defined in the "Flexible and Robust Key Rollover" paper. The rules
says: All DS records need to be in the HIDDEN state, or if it is not
there must be a key with its DNSKEY and KRRSIG in OMNIPRESENT, and
its DS in the same state as the key in question. In human langauge,
if all keys have their DS in HIDDEN state you can do what you want,
but if a DS record is available to some validators, there must be
a chain of trust for it.

Note that the barriers on transitions first check if the current
state is valid, and then if the next state is valid too. But
here we falsely updated the 'dnskey_omnipresent' (now 'dnskey_chained')
with the next state. The next state applies to 'key' not to the state
to be checked. Updating the state here leads to (true) always, because
the key that will move its state will match the falsely updated
expected state. This could lead to the assumption that Key 2 would be
a valid chain of trust for Key 1, while clearly the presence of any
DS is uncertain.

The fix here is to check if the DNSKEY and KRRSIG are in OMNIPRESENT
state for the key that does not have its DS in the HIDDEN state, and
only if that is not the case, ensure that there is a key with the same
algorithm, that provides a valid chain of trust, that is, has its
DNSKEY, KRRSIG, and DS in OMNIPRESENT state.

The changes in 'keymgr_dnskey_hidden_or_chained()' are only cosmetical,
renaming 'rrsig_omnipresent' to 'rrsig_chained' and removing the
redundant initialization of the DST_KEY_DNSKEY expected state to NA.
2021-02-03 15:34:36 +01:00
Matthijs Mekking
600915d1b2 Update keymgr_key_is_successor() calls
The previous commit changed the function definition of
'keymgr_key_is_successor()', this commit updates the code where
this function is called.

In 'keymgr_key_exists_with_state()' the logic is also updated slightly
to become more readable. First handle the easy cases:
- If the key does not match the state, continue with the next key.
- If we found a key with matching state, and there is no need to
  check the successor relationship, return (true).
- Otherwise check the successor relationship.

In 'keymgr_key_has_successor()' it is enough to check if a key has
a direct successor, so instead of calling 'keymgr_key_is_successor()',
we can just check 'keymgr_direct_dep()'.

In 'dns_keymgr_run()', we want to make sure that there is no
dependency on the keys before retiring excess keys, so replace
'keymgr_key_is_successor()' with 'keymgr_dep()'.
2021-02-03 15:34:36 +01:00
Matthijs Mekking
cc38527b63 Implement Equation(2) of "Flexible Key Rollover"
So far the key manager could only deal with two keys in a rollover,
because it used a simplified version of the successor relationship
equation from "Flexible and Robust Key Rollover" paper. The simplified
version assumes only two keys take part in the key rollover and it
for that it is enough to check the direct relationship between two
keys (is key x the direct predecessor of key z and is key z the direct
successor of key x?).

But when a third key (or more keys) comes into the equation, the key
manager would assume that one key (or more) is redundant and removed
it from the zone prematurely.

Fix by implementing Equation(2) correctly, where we check for
dependencies on keys:

z ->T x: Dep(x, T) = ∅ ∧
         (x ∈ Dep(z, T) ∨
          ∃ y ∈ Dep(z, T)(y != z ∧ y ->T x ∧ DyKyRySy = DzKzRzSz))

This says: key z is a successor of key x if:
- key x depends on key z if z is a direct successor of x,
- or if there is another key y that depends on key z that has identical
  key states as key z and key y is a successor of key x.
- Also, key x may not have any other keys depending on it.

This is still a simplified version of Equation(2) (but at least much
better), because the paper allows for a set of keys to depend on a
key. This is defined as the set Dep(x, T). Keys in the set Dep(x, T)
have a dependency on key x for record type T. The BIND implementation
can only have one key in the set Dep(x, T). The function
'keymgr_dep()' stores this key in 'uint32_t *dep' if there is a
dependency.

There are two scenarios where multiple keys can depend on a single key:

1. Rolling keys is faster than the time required to finish the
   rollover procedure. This scenario is covered by the recursive
   implementation, and checking for a chain of direct dependencies
   will suffice.

2. Changing the policy, when a zone is requested to be signed with
   a different key length for example. BIND 9 will not mark successor
   relationships in this case, but tries to move towards the new
   policy. Since there is no successor relationship, the rules are
   even more strict, and the DNSSEC reconfiguration is actually slower
   than required.

Note: this commit breaks the build, because the function definition
of 'keymgr_key_is_successor' changed. This will be fixed in the
following commit.
2021-02-03 15:34:36 +01:00
Matthijs Mekking
82632fa6d9 Remove initialize goal code
Since keys now have their goals initialized in 'keymgr_key_init()',
remove this redundant piece of code in 'keymgr_key_run()'.
2021-02-03 08:36:14 +01:00
Matthijs Mekking
76cf72e65a Correctly initialize old key with state file
The 'key_init()' function is used to initialize a state file for keys
that don't have one yet. This can happen if you are migrating from a
'auto-dnssec' or 'inline-signing' to a 'dnssec-policy' configuration.

It did not look at the "Inactive" and "Delete" timing metadata and so
old keys left behind in the key directory would also be considered as
a possible active key. This commit fixes this and now explicitly sets
the key goal to OMNIPRESENT for keys that have their "Active/Publish"
timing metadata in the past, but their "Inactive/Delete" timing
metadata in the future. If the "Inactive/Delete" timing metadata is
also in the past, the key goal is set to HIDDEN.

If the "Inactive/Delete" timing metadata is in the past, also the
key states are adjusted to either UNRETENTIVE or HIDDEN, depending on
how far in the past the metadata is set.
2021-02-03 08:36:01 +01:00
Matthijs Mekking
9134100069 Update keymgr to allow transition to insecure mode
The keymgr prevented zones from going to insecure mode. If we
have a policy with an empty key list this is a signal that the zone
wants to go back to insecure mode. In this case allow one extra state
transition to be valid when checking for DNSSEC safety.
2020-12-23 09:02:11 +01:00
Matthijs Mekking
70d1ec432f Use explicit result codes for 'rndc dnssec' cmd
It is better to add new result codes than to overload existing codes.
2020-10-05 10:53:46 +02:00
Matthijs Mekking
edc53fc416 Various rndc dnssec -checkds fixes
While working on 'rndc dnssec -rollover' I noticed the following
(small) issues:

- The key files where updated with hints set to "-when" and that
  should always be "now.
- The kasp system test did not properly update the test number when
  calling 'rndc dnssec -checkds' (and ensuring that works).
- There was a missing ']' in the rndc.c help output.
2020-10-05 10:53:46 +02:00
Matthijs Mekking
fcd34abb9e Test rndc rollover inactive key
When users (accidentally) try to roll an inactive key, throw an error.
2020-10-05 10:53:46 +02:00
Matthijs Mekking
df8276aef0 Add manual key rollover logic
Add to the keymgr a function that will schedule a rollover. This
basically means setting the time when the key needs to retire,
and updating the key lifetime, then update the state file. The next
time that named runs the keymgr the new lifetime will be taken into
account.
2020-10-05 10:52:19 +02:00
Evan Hunt
dcee985b7f update all copyright headers to eliminate the typo 2020-09-14 16:20:40 -07:00
Matthijs Mekking
c8205bfa0e Fix CDS (non-)publication
The CDS/CDNSKEY record will be published when the DS is in the
rumoured state. However, with the introduction of the rndc '-checkds'
command, the logic in the keymgr was changed to prevent the DS
state to go in RUMOURED unless the specific command was given. Hence,
the CDS was never published before it was seen in the parent.

Initially I thought this was a policy approval rule, however it is
actually a DNSSEC timing rule. Remove the restriction from
'keymgr_policy_approval' and update the 'keymgr_transition_time'
function. When looking to move the DS state to OMNIPRESENT it will
no longer calculate the state from its last change, but from when
the DS was seen in the parent, "DS Publish". If the time was not set,
default to next key event of an hour.

Similarly for moving the DS state to HIDDEN, the time to wait will
be derived from the "DS Delete" time, not from when the DS state
last changed.
2020-09-02 12:00:14 +02:00
Matthijs Mekking
46fcd927e7 rndc dnssec -checkds set algorithm
In the rare case that you have multiple keys acting as KSK and that
have the same keytag, you can now set the algorithm when calling
'-checkds'.
2020-08-07 11:26:09 +02:00
Matthijs Mekking
04d8fc0143 Implement 'rndc dnssec -checkds'
Add a new 'rndc' command 'dnssec -checkds' that allows the user to
signal named that a new DS record has been seen published in the
parent, or that an existing DS record has been withdrawn from the
parent.

Upon the 'checkds' request, 'named' will write out the new state for
the key, updating the 'DSPublish' or 'DSRemoved' timing metadata.

This replaces the "parent-registration-delay" configuration option,
this was unreliable because it was purely time based (if the user
did not actually submit the new DS to the parent for example, this
could result in an invalid DNSSEC state).

Because we cannot rely on the parent registration delay for state
transition, we need to replace it with a different guard. Instead,
if a key wants its DS state to be moved to RUMOURED, the "DSPublish"
time must be set and must not be in the future. If a key wants its
DS state to be moved to UNRETENTIVE, the "DSRemoved" time must be set
and must not be in the future.

By default, with '-checkds' you set the time that the DS has been
published or withdrawn to now, but you can set a different time with
'-when'. If there is only one KSK for the zone, that key has its
DS state moved to RUMOURED. If there are multiple keys for the zone,
specify the right key with '-key'.
2020-08-07 11:26:09 +02:00
Matthijs Mekking
e645d2ef1e Check return value of dst_key_getbool()
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.
2020-07-14 12:53:54 +00:00
Matthijs Mekking
19ce9ec1d4 Output rndc dnssec -status
Implement the 'rndc dnssec -status' command that will output
some information about the key states, such as which policy is
used for the zone, what keys are in use, and when rollover is
scheduled.

Add loose testing in the kasp system test, the actual times are
already tested via key file inspection.
2020-06-30 09:51:04 +02:00
Matthijs Mekking
e71d60299f Retire predecessor when creating successor
When creating the successor, the current active key (predecessor)
should change its goal state to HIDDEN.

Also add two useful debug logs in the keymgr_key_rollover function.
2020-06-02 10:01:28 +02:00
Matthijs Mekking
c08d0f7dd6 If prepub > retire, prepub now
Catch a case where if the prepublication time of the successor key
is later than the retire time of the predecessor. If that is the
case we should prepublish as soon as possible, a.k.a. now.
2020-06-02 10:00:53 +02:00
Matthijs Mekking
bcf8192438 Put new key rollover logic in separate function
The `dns_keymgr_run()` function became quite long, put the logic
that looks if a new key needs to be created (start a key rollover)
in a separate function.
2020-06-02 10:00:53 +02:00
Matthijs Mekking
0d578097ef Fix bug in keymgr_key_has_successor
The logic in `keymgr_key_has_successor(key, keyring)` is flawed, it
returns true if there is any key in the keyring that has a successor,
while what we really want here is to make sure that the given key
has a successor in the given keyring.

Rather than relying on `keymgr_key_exists_with_state`, walk the
list of keys in the keyring and check if the key is a successor of
the given predecessor key.
2020-06-02 10:00:51 +02:00
Matthijs Mekking
e233433772 Test keytimes on CSK rollover
This improves keytime testing on CSK rollover.  It now
tests for specific times, and also tests for SyncPublish and
Removed keytimes.

Since an "active key" for ZSK and KSK means something
different, this makes it tricky to decide when a CSK is
active. An "active key" intuitively means the key is signing
so we say a CSK is active when it is creating zone signatures.

This change means a lot of timings for the CSK rollover tests
need to be adjusted.

The keymgr code needs a slight change on calculating the
prepublication time: For a KSK we need to include the parent
registration delay, but for CSK we look at the zone signing
property and stick with the ZSK prepublication calculation.
2020-06-02 09:14:18 +02:00
Matthijs Mekking
50bbbb76a8 kasp: registration delay adjustments
Registration delay is not part of the Iret retire interval, thus
removed from the calculation when setting the Delete time metadata.

Include the registration delay in prepublication time, because
we need to prepublish the key sooner than just the Ipub
publication interval.
2020-06-02 09:14:15 +02:00
Matthijs Mekking
30cb5c97c2 Set SyncPublish on keys
Set the SyncPublish metadata on keys that don't have them yet.
2020-06-02 09:14:09 +02:00
Matthijs Mekking
18dc27afd3 Set keytimes appropriately when using kasp
While kasp relies on key states to determine when a key needs to
be published or be used for signing, the keytimes are used by
operators to get some expectation of key publication and usage.

Update the code such that these keytimes are set appropriately.
That means:
- Print "PublishCDS" and "DeleteCDS" times in the state files.
- The keymgr sets the "Removed" and "PublishCDS" times and derives
  those from the dnssec-policy.
- Tweak setting of the "Retired" time, when retiring keys, only
  update the time to now when the retire time is not yet set, or is
  in the future.

This also fixes a bug in "keymgr_transition_time" where we may wait
too long before zone signatrues become omnipresent or hidden. Not
only can we skip waiting the sign delay Dsgn if there is no
predecessor, we can also skip it if there is no successor.

Finally, this commit moves setting the lifetime, reducing two calls
to one.
2020-06-02 09:12:47 +02:00
Evan Hunt
68a1c9d679 change 'expr == true' to 'expr' in conditionals 2020-05-25 16:09:57 -07:00
Matthijs Mekking
564f9dca35 Address Coverity warnings in keymgr.c
Coverity showed that the return value of `dst_key_gettime` was
unchecked in INITIALIZE_STATE. If DST_TIME_CREATED was not set we
would set the state to be initialized to a weird last changed time.

This would normally not happen because DST_TIME_CREATED is always
set. However, we would rather set the time to now (as the comment
also indicates) not match the creation time.

The comment on INITIALIZE_STATE also needs updating as we no
longer always initialize to HIDDEN.
2020-04-20 09:21:40 +02:00
Matthijs Mekking
2389fcb4dc Only initialize goal on active keys
If we initialize goals on all keys, superfluous keys that match
the policy all desire to be active.  For example, there are six
keys available for a policy that needs just two, we only want to
set the goal state to OMNIPRESENT on two keys, not six.
2020-04-03 08:29:22 +02:00
Matthijs Mekking
7f43520893 Test migration to dnssec-policy, retire old keys
Migrating from 'auto-dnssec maintain;' to dnssec-policy did not
work properly, mainly because the legacy keys were initialized
badly.  Earlier commit deals with migration where existing keys
match the policy.  This commit deals with migration where existing
keys do not match the policy.  In that case, named must not
immediately delete the existing keys, but gracefully roll to the
dnssec-policy.

However, named did remove the existing keys immediately.  This is
because the legacy key states were initialized badly.  Because
those keys had their states initialized to HIDDEN or RUMOURED, the
keymgr decides that they can be removed (because only when the key
has its states in OMNIPRESENT it can be used safely).

The original thought to initialize key states to HIDDEN (and
RUMOURED to deal with existing keys) was to ensure that those keys
will go through the required propagation time before the keymgr
decides they can be used safely.  However, those keys are already
in the zone for a long time and making the key states represent
otherwise is dangerous: keys may be pulled out of the zone while
in fact they are required to establish the chain of trust.

Fix initializing key states for existing keys by looking more closely
at the time metadata.  Add TTL and propagation delays to the time
metadata and see if the DNSSEC records have been propagated.
Initialize the state to OMNIPRESENT if so, otherwise initialize to
RUMOURED.  If the time metadata is in the future, or does not exist,
keep initializing the state to HIDDEN.

The added test makes sure that new keys matching the policy are
introduced, but existing keys are kept in the zone until the new
keys have been propagated.
2020-04-03 08:29:22 +02:00
Matthijs Mekking
6801899134 Fix and test migration to dnssec-policy
Migrating from 'auto-dnssec maintain;' to dnssec-policy did not
work properly, mainly because the legacy keys were initialized
badly. Several adjustments in the keymgr are required to get it right:

- Set published time on keys when we calculate prepublication time.
  This is not strictly necessary, but it is weird to have an active
  key without the published time set.

- Initalize key states also before matching keys. Determine the
  target state by looking at existing time metadata: If the time
  data is set and is in the past, it is a hint that the key and
  its corresponding records have been published in the zone already,
  and the state is initialized to RUMOURED. Otherwise, initialize it
  as HIDDEN. This fixes migration to dnssec-policy from existing
  keys.

- Initialize key goal on keys that match key policy to OMNIPRESENT.
  These may be existing legacy keys that are being migrated.

- A key that has its goal to OMNIPRESENT *or* an active key can
  match a kasp key.  The code was changed with CHANGE 5354 that
  was a bugfix to prevent creating new KSK keys for zones in the
  initial stage of signing.  However, this caused problems for
  restarts when rollovers are in progress, because an outroducing
  key can still be an active key.

The test for this introduces a new KEY property 'legacy'.  This is
used to skip tests related to .state files.
2020-04-03 08:29:22 +02:00