From: Libor Peltan Date: Thu, 12 Aug 2021 09:50:50 +0000 (+0200) Subject: dnssec: preserve deleted keys for one more lifetime X-Git-Tag: v3.1.2~5 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6b5d9774f9072b88feba6d77c73164a938b115e6;p=thirdparty%2Fknot-dns.git dnssec: preserve deleted keys for one more lifetime --- diff --git a/src/knot/dnssec/key-events.c b/src/knot/dnssec/key-events.c index 95b3abc36f..932e54310c 100644 --- a/src/knot/dnssec/key-events.c +++ b/src/knot/dnssec/key-events.c @@ -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; diff --git a/src/knot/dnssec/zone-keys.c b/src/knot/dnssec/zone-keys.c index 1b1cb4240e..76a1f63f12 100644 --- a/src/knot/dnssec/zone-keys.c +++ b/src/knot/dnssec/zone-keys.c @@ -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" !