From 2733edb2a61da7094f85d6c28d77e2466114cf4f Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Thu, 6 Feb 2020 15:41:47 +0100 Subject: [PATCH] Allow for key lifetime unlimited The keyword 'unlimited' can be used instead of PT0S which means the same but is more comprehensible for users. Also fix some redundant "none" parameters in the kasp test. --- bin/named/named.conf.docbook | 2 +- bin/tests/system/checkconf/good-kasp.conf | 2 +- bin/tests/system/kasp/ns3/named.conf.in | 7 ++ bin/tests/system/kasp/ns3/policies/kasp.conf | 8 ++ bin/tests/system/kasp/ns3/setup.sh | 3 +- bin/tests/system/kasp/tests.sh | 20 +++- doc/arm/Bv9ARM-book.xml | 29 ++++-- doc/arm/dnssec-policy.grammar.xml | 2 +- doc/design/dnssec-policy | 4 +- doc/misc/dnssec-policy.default.conf | 2 +- doc/misc/options | 2 +- doc/misc/options.active | 2 +- lib/isccfg/include/isccfg/grammar.h | 9 ++ lib/isccfg/kaspconf.c | 16 ++- lib/isccfg/namedconf.c | 44 ++++---- lib/isccfg/parser.c | 103 ++++++++++++++++--- lib/isccfg/win32/libisccfg.def | 2 + 17 files changed, 194 insertions(+), 63 deletions(-) diff --git a/bin/named/named.conf.docbook b/bin/named/named.conf.docbook index 87c9254008..9aeeac10cc 100644 --- a/bin/named/named.conf.docbook +++ b/bin/named/named.conf.docbook @@ -115,7 +115,7 @@ dlz string { dnssec-policy string { dnskey-ttl duration; - keys { ( csk | ksk | zsk ) ( key-directory ) lifetime duration + keys { ( csk | ksk | zsk ) ( key-directory ) lifetime ( duration | unlimited ) algorithm integer [ integer ]; ... }; max-zone-ttl duration; parent-ds-ttl duration; diff --git a/bin/tests/system/checkconf/good-kasp.conf b/bin/tests/system/checkconf/good-kasp.conf index b4d3c1e562..19420f2dfd 100644 --- a/bin/tests/system/checkconf/good-kasp.conf +++ b/bin/tests/system/checkconf/good-kasp.conf @@ -19,7 +19,7 @@ dnssec-policy "test" { keys { ksk key-directory lifetime P1Y algorithm 13 256; zsk key-directory lifetime P30D algorithm 13; - csk key-directory lifetime P30D algorithm 8 2048; + csk key-directory lifetime unlimited algorithm 8 2048; }; max-zone-ttl 86400; parent-ds-ttl 7200; diff --git a/bin/tests/system/kasp/ns3/named.conf.in b/bin/tests/system/kasp/ns3/named.conf.in index 2f78aa8146..38a656b0d3 100644 --- a/bin/tests/system/kasp/ns3/named.conf.in +++ b/bin/tests/system/kasp/ns3/named.conf.in @@ -45,6 +45,13 @@ zone "default.kasp" { dnssec-policy "default"; }; +/* Key lifetime unlimited. */ +zone "unlimited.kasp" { + type master; + file "unlimited.kasp.db"; + dnssec-policy "unlimited"; +}; + /* A master zone with dnssec-policy, no keys created. */ zone "rsasha1.kasp" { type master; diff --git a/bin/tests/system/kasp/ns3/policies/kasp.conf b/bin/tests/system/kasp/ns3/policies/kasp.conf index fa60476e65..e0ce931d90 100644 --- a/bin/tests/system/kasp/ns3/policies/kasp.conf +++ b/bin/tests/system/kasp/ns3/policies/kasp.conf @@ -9,6 +9,14 @@ * information regarding copyright ownership. */ +dnssec-policy "unlimited" { + dnskey-ttl 1234; + + keys { + csk key-directory lifetime unlimited algorithm 13; + }; +}; + dnssec-policy "rsasha1" { dnskey-ttl 1234; diff --git a/bin/tests/system/kasp/ns3/setup.sh b/bin/tests/system/kasp/ns3/setup.sh index c39df821af..a1c1806839 100644 --- a/bin/tests/system/kasp/ns3/setup.sh +++ b/bin/tests/system/kasp/ns3/setup.sh @@ -43,7 +43,8 @@ U="UNRETENTIVE" # Set up zones that will be initially signed. # for zn in default rsasha1 dnssec-keygen some-keys legacy-keys pregenerated \ - rumoured rsasha1-nsec3 rsasha256 rsasha512 ecdsa256 ecdsa384 inherit + rumoured rsasha1-nsec3 rsasha256 rsasha512 ecdsa256 ecdsa384 \ + inherit unlimited do setup "${zn}.kasp" cp template.db.in "$zonefile" diff --git a/bin/tests/system/kasp/tests.sh b/bin/tests/system/kasp/tests.sh index 7a7b4d9eb0..47d8e05747 100644 --- a/bin/tests/system/kasp/tests.sh +++ b/bin/tests/system/kasp/tests.sh @@ -1026,6 +1026,22 @@ check_keys check_apex check_subdomain +# +# Zone: unlimited.kasp. +# +zone_properties "ns3" "unlimited.kasp" "unlimited" "1234" "1" "10.53.0.3" +key_properties "KEY1" "csk" "0" "13" "ECDSAP256SHA256" "256" "yes" "yes" +key_clear "KEY2" +key_clear "KEY3" +# The first key is immediately published and activated. +key_timings "KEY1" "published" "active" "none" "none" "none" +# DNSKEY, RRSIG (ksk), RRSIG (zsk) are published. DS needs to wait. +key_states "KEY1" "omnipresent" "rumoured" "rumoured" "rumoured" "hidden" +check_keys +check_apex +check_subdomain +dnssec_verify + # # Zone: inherit.kasp. # @@ -1451,7 +1467,7 @@ check_subdomain # ns5/override.inherit.signed # ns5/inherit.override.signed key_properties "KEY1" "csk" "0" "13" "ECDSAP256SHA256" "256" "yes" "yes" -key_timings "KEY1" "published" "active" "none" "none" "none" "none" +key_timings "KEY1" "published" "active" "none" "none" "none" key_states "KEY1" "omnipresent" "rumoured" "rumoured" "rumoured" "hidden" zone_properties "ns2" "signed.tld" "default" "3600" "1" "10.53.0.2" @@ -1496,7 +1512,7 @@ dnssec_verify # ns5/override.override.unsigned # ns5/override.none.unsigned key_properties "KEY1" "csk" "0" "14" "ECDSAP384SHA384" "384" "yes" "yes" -key_timings "KEY1" "published" "active" "none" "none" "none" "none" +key_timings "KEY1" "published" "active" "none" "none" "none" key_states "KEY1" "omnipresent" "rumoured" "rumoured" "rumoured" "hidden" zone_properties "ns4" "inherit.inherit.signed" "test" "3600" "1" "10.53.0.4" diff --git a/doc/arm/Bv9ARM-book.xml b/doc/arm/Bv9ARM-book.xml index 1fd3e16e63..28f88f89da 100644 --- a/doc/arm/Bv9ARM-book.xml +++ b/doc/arm/Bv9ARM-book.xml @@ -11113,7 +11113,7 @@ example.com CNAME rpz-tcp-only. keys { - ksk key-directory lifetime P5Y algorithm 8 2048; + ksk key-directory lifetime unlimited algorithm 8 2048; zsk key-directory lifetime P30D algorithm 8; csk key-directory lifetime P6MT12H3M15S algorithm 13; }; @@ -11133,16 +11133,27 @@ example.com CNAME rpz-tcp-only. key-directory. - The third token tells how long the key may be used. In the - example the first key has a lifetime of 5 years, the second - key may be used for 30 days and the third key has a rather - peculiar lifetime of 6 months, 12 hours, 3 minutes and 15 - seconds. + The lifetime parameter specifies how + long a key may be used before rolling over. In the + example above, the first key will have an unlimited + lifetime, the second key may be used for 30 days, and the + third key has a rather peculiar lifetime of 6 months, + 12 hours, 3 minutes and 15 seconds. A lifetime of 0 + seconds is the same as unlimited. - The last token(s) are the key's algorithm and algorithm - length. The length may be omitted as shown in the - example for the second and third key. + Note that the lifetime of a key may be extended if + retiring it too soon would cause validation failures: + for example, if the key were configured to roll more + frequently than its TTL, its lifetime would + automatically be extended to account for this. + + + The algorithm parameter(s) are the key's + algorithm, expressed numerically, and its size in bits. The + size may be omitted, as shown in the example for the + second and third keys; in this case an appropriate + default size will be used. diff --git a/doc/arm/dnssec-policy.grammar.xml b/doc/arm/dnssec-policy.grammar.xml index d3e21a4918..08e3890e6b 100644 --- a/doc/arm/dnssec-policy.grammar.xml +++ b/doc/arm/dnssec-policy.grammar.xml @@ -14,7 +14,7 @@ dnssec-policy string { dnskey-ttl duration; - keys { ( csk | ksk | zsk ) key-directory lifetime duration algorithm integer [ integer ] ; ... }; + keys { ( csk | ksk | zsk ) key-directory lifetime ( duration | unlimited ) algorithm integer [ integer ] ; ... }; max-zone-ttl duration; parent-ds-ttl duration; parent-propagation-delay duration; diff --git a/doc/design/dnssec-policy b/doc/design/dnssec-policy index ae16195518..69951814dd 100644 --- a/doc/design/dnssec-policy +++ b/doc/design/dnssec-policy @@ -199,9 +199,9 @@ is referred to as a CSK. Below is an example configuration for the three types of keys: ``` keys { - ksk key-directory lifetime P5Y algorithm ECDSAP256SHA256; + ksk key-directory lifetime unlimited algorithm ECDSAP256SHA256; zsk key-directory lifetime P30D algorithm ECDSAP256SHA256; - csk key-directory lifetime PT0S algorithm 8 2048; + csk key-directory lifetime P5Y algorithm 8 2048; }; ``` diff --git a/doc/misc/dnssec-policy.default.conf b/doc/misc/dnssec-policy.default.conf index 58283f2a0e..1e12aec43e 100644 --- a/doc/misc/dnssec-policy.default.conf +++ b/doc/misc/dnssec-policy.default.conf @@ -2,7 +2,7 @@ dnssec-policy "default" { // Keys keys { - csk key-directory lifetime 0 algorithm 13; + csk key-directory lifetime unlimited algorithm 13; }; // Key timings diff --git a/doc/misc/options b/doc/misc/options index 57b2a4393a..cf66ac3a97 100644 --- a/doc/misc/options +++ b/doc/misc/options @@ -23,7 +23,7 @@ dlz { dnssec-policy { dnskey-ttl ; - keys { ( csk | ksk | zsk ) ( key-directory ) lifetime + keys { ( csk | ksk | zsk ) ( key-directory ) lifetime ( | unlimited ) algorithm [ ]; ... }; max-zone-ttl ; parent-ds-ttl ; diff --git a/doc/misc/options.active b/doc/misc/options.active index 0adfbfa9ec..20fc8d3b37 100644 --- a/doc/misc/options.active +++ b/doc/misc/options.active @@ -23,7 +23,7 @@ dlz { dnssec-policy { dnskey-ttl ; - keys { ( csk | ksk | zsk ) ( key-directory ) lifetime + keys { ( csk | ksk | zsk ) ( key-directory ) lifetime ( | unlimited ) algorithm [ ]; ... }; max-zone-ttl ; parent-ds-ttl ; diff --git a/lib/isccfg/include/isccfg/grammar.h b/lib/isccfg/include/isccfg/grammar.h index ad3dd81d17..1b02820ba0 100644 --- a/lib/isccfg/include/isccfg/grammar.h +++ b/lib/isccfg/include/isccfg/grammar.h @@ -173,6 +173,7 @@ struct cfg_duration { */ uint32_t parts[7]; bool iso8601; + bool unlimited; }; /*% @@ -344,6 +345,7 @@ LIBISCCFG_EXTERNAL_DATA extern cfg_type_t cfg_type_unsupported; LIBISCCFG_EXTERNAL_DATA extern cfg_type_t cfg_type_fixedpoint; LIBISCCFG_EXTERNAL_DATA extern cfg_type_t cfg_type_percentage; LIBISCCFG_EXTERNAL_DATA extern cfg_type_t cfg_type_duration; +LIBISCCFG_EXTERNAL_DATA extern cfg_type_t cfg_type_duration_or_unlimited; /*@}*/ isc_result_t @@ -535,6 +537,13 @@ cfg_parse_duration(cfg_parser_t *pctx, const cfg_type_t *type, void cfg_print_duration(cfg_printer_t *pctx, const cfg_obj_t *obj); +isc_result_t +cfg_parse_duration_or_unlimited(cfg_parser_t *pctx, const cfg_type_t *type, + cfg_obj_t **ret); + +void +cfg_print_duration_or_unlimited(cfg_printer_t *pctx, const cfg_obj_t *obj); + isc_result_t cfg_parse_obj(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret); diff --git a/lib/isccfg/kaspconf.c b/lib/isccfg/kaspconf.c index 036761bdb0..8001c122e7 100644 --- a/lib/isccfg/kaspconf.c +++ b/lib/isccfg/kaspconf.c @@ -78,7 +78,7 @@ cfg_kaspkey_fromconfig(const cfg_obj_t *config, dns_kasp_t* kasp) if (config == NULL) { /* We are creating a key reference for the default kasp. */ key->role |= DNS_KASP_KEY_ROLE_KSK | DNS_KASP_KEY_ROLE_ZSK; - key->lifetime = 0; + key->lifetime = 0; /* unlimited */ key->algorithm = DNS_KEYALG_ECDSA256; key->length = -1; } else { @@ -94,10 +94,16 @@ cfg_kaspkey_fromconfig(const cfg_obj_t *config, dns_kasp_t* kasp) key->role |= DNS_KASP_KEY_ROLE_KSK; key->role |= DNS_KASP_KEY_ROLE_ZSK; } - key->lifetime = cfg_obj_asduration( - cfg_tuple_get(config, "lifetime")); - key->algorithm = cfg_obj_asuint32( - cfg_tuple_get(config, "algorithm")); + + key->lifetime = 0; /* unlimited */ + obj = cfg_tuple_get(config, "lifetime"); + if (cfg_obj_isduration(obj)) { + key->lifetime = cfg_obj_asduration(obj); + } + + obj = cfg_tuple_get(config, "algorithm"); + key->algorithm = cfg_obj_asuint32(obj); + obj = cfg_tuple_get(config, "length"); if (cfg_obj_isuint32(obj)) { key->length = cfg_obj_asuint32(obj); diff --git a/lib/isccfg/namedconf.c b/lib/isccfg/namedconf.c index e8812e9e30..0bfb11a25c 100644 --- a/lib/isccfg/namedconf.c +++ b/lib/isccfg/namedconf.c @@ -96,7 +96,7 @@ static cfg_type_t cfg_type_logseverity; static cfg_type_t cfg_type_logsuffix; static cfg_type_t cfg_type_logversions; static cfg_type_t cfg_type_masterselement; -static cfg_type_t cfg_type_maxttl; +static cfg_type_t cfg_type_maxduration; static cfg_type_t cfg_type_minimal; static cfg_type_t cfg_type_nameportiplist; static cfg_type_t cfg_type_notifytype; @@ -439,6 +439,24 @@ static cfg_type_t cfg_type_category = { &cfg_rep_tuple, category_fields }; +static isc_result_t +parse_maxduration(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { + return (cfg_parse_enum_or_other(pctx, type, &cfg_type_duration, ret)); +} + +static void +doc_maxduration(cfg_printer_t *pctx, const cfg_type_t *type) { + cfg_doc_enum_or_other(pctx, type, &cfg_type_duration); +} + +/*% + * A duration or "unlimited", but not "default". + */ +static const char *maxduration_enums[] = { "unlimited", NULL }; +static cfg_type_t cfg_type_maxduration = { + "maxduration_no_default", parse_maxduration, cfg_print_ustring, + doc_maxduration, &cfg_rep_duration, maxduration_enums +}; /*% * A dnssec key, as used in the "trusted-keys" statement. @@ -509,7 +527,8 @@ static cfg_type_t cfg_type_algorithm = { doc_keyvalue, &cfg_rep_uint32, &algorithm_kw }; -static keyword_type_t lifetime_kw = { "lifetime", &cfg_type_duration }; +static keyword_type_t lifetime_kw = { "lifetime", + &cfg_type_duration_or_unlimited }; static cfg_type_t cfg_type_lifetime = { "lifetime", parse_keyvalue, print_keyvalue, doc_keyvalue, &cfg_rep_duration, &lifetime_kw @@ -2227,7 +2246,7 @@ zone_clauses[] = { { "max-transfer-time-out", &cfg_type_uint32, CFG_ZONE_MASTER | CFG_ZONE_MIRROR | CFG_ZONE_SLAVE }, - { "max-zone-ttl", &cfg_type_maxttl, + { "max-zone-ttl", &cfg_type_maxduration, CFG_ZONE_MASTER | CFG_ZONE_REDIRECT }, { "min-refresh-time", &cfg_type_uint32, @@ -3867,25 +3886,6 @@ static cfg_type_t cfg_type_masterselement = { doc_masterselement, NULL, NULL }; -static isc_result_t -parse_maxttl(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) { - return (cfg_parse_enum_or_other(pctx, type, &cfg_type_duration, ret)); -} - -static void -doc_maxttl(cfg_printer_t *pctx, const cfg_type_t *type) { - cfg_doc_enum_or_other(pctx, type, &cfg_type_duration); -} - -/*% - * A size or "unlimited", but not "default". - */ -static const char *maxttl_enums[] = { "unlimited", NULL }; -static cfg_type_t cfg_type_maxttl = { - "maxttl_no_default", parse_maxttl, cfg_print_ustring, doc_maxttl, - &cfg_rep_string, maxttl_enums -}; - static int cmp_clause(const void *ap, const void *bp) { const cfg_clausedef_t *a = (const cfg_clausedef_t *)ap; const cfg_clausedef_t *b = (const cfg_clausedef_t *)bp; diff --git a/lib/isccfg/parser.c b/lib/isccfg/parser.c index 97647ca58e..5ae6b6bd88 100644 --- a/lib/isccfg/parser.c +++ b/lib/isccfg/parser.c @@ -1088,6 +1088,22 @@ cfg_print_duration(cfg_printer_t *pctx, const cfg_obj_t *obj) { cfg_print_chars(pctx, buf, strlen(buf)); } +void +cfg_print_duration_or_unlimited(cfg_printer_t *pctx, const cfg_obj_t *obj) { + cfg_duration_t duration; + + REQUIRE(pctx != NULL); + REQUIRE(obj != NULL); + + duration = obj->value.duration; + + if (duration.unlimited) { + cfg_print_cstr(pctx, "unlimited"); + } else { + cfg_print_duration(pctx, obj); + } +} + bool cfg_obj_isduration(const cfg_obj_t *obj) { REQUIRE(obj != NULL); @@ -1250,21 +1266,14 @@ duration_fromtext(isc_textregion_t *source, cfg_duration_t *duration) { return (ISC_R_SUCCESS); } -isc_result_t -cfg_parse_duration(cfg_parser_t *pctx, const cfg_type_t *type, - cfg_obj_t **ret) +static isc_result_t +cfg__parse_duration(cfg_parser_t *pctx, cfg_obj_t **ret) { isc_result_t result; cfg_obj_t *obj = NULL; cfg_duration_t duration; - UNUSED(type); - - CHECK(cfg_gettoken(pctx, 0)); - if (pctx->token.type != isc_tokentype_string) { - result = ISC_R_UNEXPECTEDTOKEN; - goto cleanup; - } + duration.unlimited = false; if (TOKEN_STRING(pctx)[0] == 'P') { result = duration_fromtext(&pctx->token.value.as_textregion, @@ -1278,12 +1287,9 @@ cfg_parse_duration(cfg_parser_t *pctx, const cfg_type_t *type, * With dns_ttl_fromtext() the information on optional units. * is lost, and is treated as seconds from now on. */ - duration.parts[0] = 0; - duration.parts[1] = 0; - duration.parts[2] = 0; - duration.parts[3] = 0; - duration.parts[4] = 0; - duration.parts[5] = 0; + for (int i = 0; i < 6; i++) { + duration.parts[i] = 0; + } duration.parts[6] = ttl; duration.iso8601 = false; } @@ -1305,6 +1311,66 @@ cleanup: return (result); } +isc_result_t +cfg_parse_duration(cfg_parser_t *pctx, const cfg_type_t *type, + cfg_obj_t **ret) +{ + isc_result_t result; + + UNUSED(type); + + CHECK(cfg_gettoken(pctx, 0)); + if (pctx->token.type != isc_tokentype_string) { + result = ISC_R_UNEXPECTEDTOKEN; + goto cleanup; + } + + return cfg__parse_duration(pctx, ret); + +cleanup: + cfg_parser_error(pctx, CFG_LOG_NEAR, + "expected ISO 8601 duration or TTL value"); + return (result); +} + +isc_result_t +cfg_parse_duration_or_unlimited(cfg_parser_t *pctx, const cfg_type_t *type, + cfg_obj_t **ret) +{ + isc_result_t result; + cfg_obj_t *obj = NULL; + cfg_duration_t duration; + + UNUSED(type); + + CHECK(cfg_gettoken(pctx, 0)); + if (pctx->token.type != isc_tokentype_string) { + result = ISC_R_UNEXPECTEDTOKEN; + goto cleanup; + } + + if (strcmp(TOKEN_STRING(pctx), "unlimited") == 0) { + for (int i = 0; i < 7; i++) { + duration.parts[i] = 0; + } + duration.iso8601 = false; + duration.unlimited = true; + + CHECK(cfg_create_obj(pctx, &cfg_type_duration, &obj)); + obj->value.duration = duration; + *ret = obj; + return (ISC_R_SUCCESS); + } + + return cfg__parse_duration(pctx, ret); + +cleanup: + cfg_parser_error(pctx, CFG_LOG_NEAR, + "expected ISO 8601 duration, TTL value, or unlimited"); + return (result); +} + + /*% * A duration as defined by ISO 8601 (P[n]Y[n]M[n]DT[n]H[n]M[n]S). * - P is the duration indicator ("period") placed at the start. @@ -1324,6 +1390,11 @@ LIBISCCFG_EXTERNAL_DATA cfg_type_t cfg_type_duration = { "duration", cfg_parse_duration, cfg_print_duration, cfg_doc_terminal, &cfg_rep_duration, NULL }; +LIBISCCFG_EXTERNAL_DATA cfg_type_t cfg_type_duration_or_unlimited = { + "duration_or_unlimited", cfg_parse_duration_or_unlimited, + cfg_print_duration_or_unlimited, cfg_doc_terminal, + &cfg_rep_duration, NULL +}; /* * qstring (quoted string), ustring (unquoted string), astring diff --git a/lib/isccfg/win32/libisccfg.def b/lib/isccfg/win32/libisccfg.def index 1c40b3c34b..eba897ec30 100644 --- a/lib/isccfg/win32/libisccfg.def +++ b/lib/isccfg/win32/libisccfg.def @@ -72,6 +72,7 @@ cfg_parse_bracketed_list cfg_parse_buffer cfg_parse_dscp cfg_parse_duration +cfg_parse_duration_or_unlimited cfg_parse_enum cfg_parse_enum_or_other cfg_parse_file @@ -112,6 +113,7 @@ cfg_print_chars cfg_print_clauseflags cfg_print_cstr cfg_print_duration +cfg_print_duration_or_unlimited cfg_print_fixedpoint cfg_print_grammar cfg_print_indent