]> git.ipfire.org Git - thirdparty/knot-dns.git/commitdiff
dnssec: preserve deleted keys for one more lifetime
authorLibor Peltan <libor.peltan@nic.cz>
Thu, 12 Aug 2021 09:50:50 +0000 (11:50 +0200)
committerDaniel Salzman <daniel.salzman@nic.cz>
Tue, 7 Sep 2021 13:39:59 +0000 (15:39 +0200)
src/knot/dnssec/key-events.c
src/knot/dnssec/zone-keys.c

index 95b3abc36f85b45f649d0e5fedd9e757c9f71a03..932e54310c5de7b3e3f314e4677e1d62ed6a7f5c 100644 (file)
@@ -29,7 +29,8 @@ static bool key_present(const kdnssec_ctx_t *ctx, bool ksk, bool zsk)
        assert(ctx->zone);
        for (size_t i = 0; i < ctx->zone->num_keys; i++) {
                const knot_kasp_key_t *key = &ctx->zone->keys[i];
-               if (key->is_ksk == ksk && key->is_zsk == zsk && !key->is_pub_only) {
+               if (key->is_ksk == ksk && key->is_zsk == zsk && !key->is_pub_only &&
+                   get_key_state(key, ctx->now) != DNSSEC_KEY_STATE_REMOVED) {
                        return true;
                }
        }
@@ -43,7 +44,8 @@ static bool key_id_present(const kdnssec_ctx_t *ctx, const char *keyid, bool wan
        for (size_t i = 0; i < ctx->zone->num_keys; i++) {
                const knot_kasp_key_t *key = &ctx->zone->keys[i];
                if (strcmp(keyid, key->id) == 0 &&
-                   key->is_ksk == want_ksk) {
+                   key->is_ksk == want_ksk &&
+                   get_key_state(key, ctx->now) != DNSSEC_KEY_STATE_REMOVED) {
                        return true;
                }
        }
@@ -59,6 +61,7 @@ static unsigned algorithm_present(const kdnssec_ctx_t *ctx, uint8_t alg)
                const knot_kasp_key_t *key = &ctx->zone->keys[i];
                knot_time_t activated = knot_time_min(key->timing.pre_active, key->timing.ready);
                if (knot_time_cmp(knot_time_min(activated, key->timing.active), ctx->now) <= 0 &&
+                   get_key_state(key, ctx->now) != DNSSEC_KEY_STATE_REMOVED &&
                    dnssec_key_get_algorithm(key->key) == alg && !key->is_pub_only) {
                        ret++;
                }
@@ -251,6 +254,7 @@ typedef enum {
        REPLACE,
        RETIRE,
        REMOVE,
+       REALLY_REMOVE,
 } roll_action_type_t;
 
 typedef struct {
@@ -345,6 +349,22 @@ static knot_time_t ksk_remove_time(knot_time_t retire_time, bool is_csk, const k
        return knot_time_add(retire_time, ctx->policy->propagation_delay + use_ttl);
 }
 
+static knot_time_t ksk_really_remove_time(knot_time_t remove_time, const kdnssec_ctx_t *ctx)
+{
+       if (ctx->keep_deleted_keys) {
+               return 0;
+       }
+       return knot_time_add(remove_time, ctx->policy->ksk_lifetime);
+}
+
+static knot_time_t zsk_really_remove_time(knot_time_t remove_time, const kdnssec_ctx_t *ctx)
+{
+       if (ctx->keep_deleted_keys) {
+               return 0;
+       }
+       return knot_time_add(remove_time, ctx->policy->zsk_lifetime);
+}
+
 // algorithm rollover related timers must be the same for KSK and ZSK
 
 static knot_time_t alg_publish_time(knot_time_t pre_active_time, const kdnssec_ctx_t *ctx)
@@ -416,8 +436,8 @@ static roll_action_t next_action(kdnssec_ctx_t *ctx, zone_sign_roll_flags_t flag
                                restype = REMOVE;
                                break;
                        case DNSSEC_KEY_STATE_REMOVED:
-                               keytime = ctx->now;
-                               restype = REMOVE;
+                               keytime = ksk_really_remove_time(key->timing.remove, ctx);
+                               restype = REALLY_REMOVE;
                                break;
                        default:
                                continue;
@@ -446,18 +466,15 @@ static roll_action_t next_action(kdnssec_ctx_t *ctx, zone_sign_roll_flags_t flag
                                keytime = alg_remove_time(key->timing.post_active, ctx);
                                restype = REMOVE;
                                break;
-                       case DNSSEC_KEY_STATE_REMOVED:
-                               // ad REMOVED state: normally this wouldn't happen
-                               // (key in removed state is instantly deleted)
-                               // but if imported keys, they can be in this state
-                               if (ctx->keep_deleted_keys) {
-                                       break;
-                               } // else FALLTHROUGH
                        case DNSSEC_KEY_STATE_RETIRED:
                                keytime = knot_time_min(key->timing.retire, key->timing.remove);
                                keytime = zsk_remove_time(keytime, ctx);
                                restype = REMOVE;
                                break;
+                       case DNSSEC_KEY_STATE_REMOVED:
+                               keytime = zsk_really_remove_time(key->timing.remove, ctx);
+                               restype = REALLY_REMOVE;
+                               break;
                        case DNSSEC_KEY_STATE_READY:
                        default:
                                continue;
@@ -575,12 +592,14 @@ static int exec_remove_old_key(kdnssec_ctx_t *ctx, knot_kasp_key_t *key)
               get_key_state(key, ctx->now) == DNSSEC_KEY_STATE_POST_ACTIVE ||
               get_key_state(key, ctx->now) == DNSSEC_KEY_STATE_REMOVED);
        key->timing.remove = ctx->now;
+       return KNOT_EOK;
+}
 
-       if (ctx->keep_deleted_keys) {
-               return KNOT_EOK;
-       } else {
-               return kdnssec_delete_key(ctx, key);
-       }
+static int exec_really_remove(kdnssec_ctx_t *ctx, knot_kasp_key_t *key)
+{
+       assert(get_key_state(key, ctx->now) == DNSSEC_KEY_STATE_REMOVED);
+       assert(!ctx->keep_deleted_keys);
+       return kdnssec_delete_key(ctx, key);
 }
 
 int knot_dnssec_key_rollover(kdnssec_ctx_t *ctx, zone_sign_roll_flags_t flags,
@@ -714,6 +733,9 @@ int knot_dnssec_key_rollover(kdnssec_ctx_t *ctx, zone_sign_roll_flags_t flags,
                case REMOVE:
                        ret = exec_remove_old_key(ctx, next.key);
                        break;
+               case REALLY_REMOVE:
+                       ret = exec_really_remove(ctx, next.key);
+                       break;
                default:
                        log_keytag = false;
                        ret = KNOT_EINVAL;
index 1b1cb4240ee5c91de8c9f59a3252c975afaadd57..76a1f63f12f1cdfded4d086bb5205d593006370f 100644 (file)
@@ -323,7 +323,8 @@ static void set_key(knot_kasp_key_t *kasp_key, knot_time_t now,
        zone_key->is_zsk_active_plus = zone_key->is_ready && !same_alg_act_zsk;
        if (knot_time_cmp(timing->pre_active, now) <= 0 &&
            knot_time_cmp(timing->ready, now) > 0 &&
-           knot_time_cmp(timing->active, now) > 0) {
+           knot_time_cmp(timing->active, now) > 0 &&
+           knot_time_cmp(timing->remove, now) > 0) {
                zone_key->is_zsk_active_plus = zone_key->is_zsk;
                // zone_key->is_ksk_active_plus = (knot_time_cmp(timing->publish, now) <= 0 && zone_key->is_ksk); // redundant, but helps understand
        }
@@ -333,7 +334,8 @@ static void set_key(knot_kasp_key_t *kasp_key, knot_time_t now,
                zone_key->is_public = zone_key->is_zsk;
        }
        if (knot_time_cmp(timing->retire_active, now) <= 0 &&
-           knot_time_cmp(timing->retire, now) > 0) {
+           knot_time_cmp(timing->retire, now) > 0 &&
+           knot_time_cmp(timing->remove, now) > 0) {
                zone_key->is_ksk_active_plus = zone_key->is_ksk;
                zone_key->is_zsk_active_plus = !same_alg_act_zsk;
        } // not "else" !