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.

(cherry picked from commit cc38527b63b0164d68ee55c99725a9058def863e)

diff --git a/lib/dns/keymgr.c b/lib/dns/keymgr.c
index fccd3c68f4aa79053d6037c912d0c0dc3dcf6f5b..5db97e917a31d8f556cb3e686451e188bc112416 100644 (file)
--- a/lib/dns/keymgr.c
+++ b/lib/dns/keymgr.c
@@ -557,21 +557,124 @@ keymgr_key_match_state(dst_key_t *key, dst_key_t *subject, int type,
 }
 
 /*
- * Check if a 'k2' is a successor of 'k1'. This is a simplified version of
- * Equation(2) of "Flexible and Robust Key Rollover" which defines a
- * recursive relation.
- *
+ * Key d directly depends on k if d is the direct predecessor of k.
+ */
+static bool
+keymgr_direct_dep(dst_key_t *d, dst_key_t *k) {
+       uint32_t s, p;
+
+       if (dst_key_getnum(d, DST_NUM_SUCCESSOR, &s) != ISC_R_SUCCESS) {
+               return (false);
+       }
+       if (dst_key_getnum(k, DST_NUM_PREDECESSOR, &p) != ISC_R_SUCCESS) {
+               return (false);
+       }
+       return (dst_key_id(d) == p && dst_key_id(k) == s);
+}
+
+/*
+ * Determine which key (if any) has a dependency on k.
+ */
+static bool
+keymgr_dep(dst_key_t *k, dns_dnsseckeylist_t *keyring, uint32_t *dep) {
+       for (dns_dnsseckey_t *d = ISC_LIST_HEAD(*keyring); d != NULL;
+            d = ISC_LIST_NEXT(d, link))
+       {
+               /*
+                * Check if k is a direct successor of d, e.g. d depends on k.
+                */
+               if (keymgr_direct_dep(d->key, k)) {
+                       if (dep != NULL) {
+                               *dep = dst_key_id(d->key);
+                       }
+                       return (true);
+               }
+       }
+       return (false);
+}
+
+/*
+ * Check if a 'z' is a successor of 'x'.
+ * This implements Equation(2) of "Flexible and Robust Key Rollover".
  */
 static bool
-keymgr_key_is_successor(dst_key_t *k1, dst_key_t *k2) {
-       uint32_t suc = 0, pre = 0;
-       if (dst_key_getnum(k1, DST_NUM_SUCCESSOR, &suc) != ISC_R_SUCCESS) {
+keymgr_key_is_successor(dst_key_t *x, dst_key_t *z, dst_key_t *key, int type,
+                       dst_key_state_t next_state,
+                       dns_dnsseckeylist_t *keyring) {
+       uint32_t dep_x;
+       uint32_t dep_z;
+
+       /*
+        * The successor relation requires that the predecessor key must not
+        * have any other keys relying on it. In other words, there must be
+        * nothing depending on x.
+        */
+       if (keymgr_dep(x, keyring, &dep_x)) {
                return (false);
        }
-       if (dst_key_getnum(k2, DST_NUM_PREDECESSOR, &pre) != ISC_R_SUCCESS) {
+
+       /*
+        * If there is no keys relying on key z, then z is not a successor.
+        */
+       if (!keymgr_dep(z, keyring, &dep_z)) {
                return (false);
        }
-       return (dst_key_id(k1) == pre && dst_key_id(k2) == suc);
+
+       /*
+        * x depends on z, thus key z is a direct successor of key x.
+        */
+       if (dst_key_id(x) == dep_z) {
+               return (true);
+       }
+
+       /*
+        * It is possible to roll keys faster than the time required to finish
+        * the rollover procedure. For example, consider the keys x, y, z.
+        * Key x is currently published and is going to be replaced by y. The
+        * DNSKEY for x is removed from the zone and at the same moment the
+        * DNSKEY for y is introduced. Key y is a direct dependency for key x
+        * and is therefore the successor of x. However, before the new DNSKEY
+        * has been propagated, key z will replace key y. The DNSKEY for y is
+        * removed and moves into the same state as key x. Key y now directly
+        * depends on key z, and key z will be a new successor key for x.
+        */
+       dst_key_state_t zst[4] = { NA, NA, NA, NA };
+       for (int i = 0; i < 4; i++) {
+               dst_key_state_t state;
+               if (dst_key_getstate(z, i, &state) != ISC_R_SUCCESS) {
+                       continue;
+               }
+               zst[i] = state;
+       }
+
+       for (dns_dnsseckey_t *y = ISC_LIST_HEAD(*keyring); y != NULL;
+            y = ISC_LIST_NEXT(y, link))
+       {
+               if (dst_key_id(y->key) == dst_key_id(z)) {
+                       continue;
+               }
+
+               if (dst_key_id(y->key) != dep_z) {
+                       continue;
+               }
+               /*
+                * This is another key y, that depends on key z. It may be
+                * part of the successor relation if the key states match
+                * those of key z.
+                */
+
+               if (keymgr_key_match_state(y->key, key, type, next_state, zst))
+               {
+                       /*
+                        * If y is a successor of x, then z is also a
+                        * successor of x.
+                        */
+                       return (keymgr_key_is_successor(x, y->key, key, type,
+                                                       next_state, keyring));
+               }
+       }
+
+       return (false);
 }
 
 /*