From 4fb2ecd444260fc58a77d3f471c5623c699a659d Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Tue, 21 Jun 2022 12:22:36 +0200 Subject: [PATCH 1/8] Move duration structure to libisccfg/duration Having the duration structure and parsing code here, it becomes more accessible to be used in other places. (cherry picked from commit a28d91950393640441191e8f4c80df7737344100) --- lib/isccfg/Makefile.am | 2 + lib/isccfg/duration.c | 155 ++++++++++++++++++++++++++ lib/isccfg/include/isccfg/duration.h | 58 ++++++++++ lib/isccfg/include/isccfg/grammar.h | 53 +++------ lib/isccfg/parser.c | 156 +++------------------------ 5 files changed, 248 insertions(+), 176 deletions(-) create mode 100644 lib/isccfg/duration.c create mode 100644 lib/isccfg/include/isccfg/duration.h diff --git a/lib/isccfg/Makefile.am b/lib/isccfg/Makefile.am index 32e905aa96..0c95c4f0d4 100644 --- a/lib/isccfg/Makefile.am +++ b/lib/isccfg/Makefile.am @@ -6,6 +6,7 @@ libisccfg_ladir = $(includedir)/isccfg libisccfg_la_HEADERS = \ include/isccfg/aclconf.h \ include/isccfg/cfg.h \ + include/isccfg/duration.h \ include/isccfg/grammar.h \ include/isccfg/kaspconf.h \ include/isccfg/log.h \ @@ -15,6 +16,7 @@ libisccfg_la_SOURCES = \ $(libisccfg_la_HEADERS) \ aclconf.c \ dnsconf.c \ + duration.c \ kaspconf.c \ log.c \ namedconf.c \ diff --git a/lib/isccfg/duration.c b/lib/isccfg/duration.c new file mode 100644 index 0000000000..30d110ab0a --- /dev/null +++ b/lib/isccfg/duration.c @@ -0,0 +1,155 @@ +/* + * Copyright (C) Internet Systems Consortium, Inc. ("ISC") + * + * SPDX-License-Identifier: MPL-2.0 + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * See the COPYRIGHT file distributed with this work for additional + * information regarding copyright ownership. + */ + +/*! \file */ + +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +#include + +/* + * isccfg_duration_fromtext initially taken from OpenDNSSEC code base. + * Modified to fit the BIND 9 code. + */ +isc_result_t +isccfg_duration_fromtext(isc_textregion_t *source, + isccfg_duration_t *duration) { + char buf[DURATION_MAXLEN]; + char *P, *X, *T, *W, *str; + bool not_weeks = false; + int i; + + /* + * Copy the buffer as it may not be NULL terminated. + * Anyone having a duration longer than 63 characters is crazy. + */ + if (source->length > sizeof(buf) - 1) { + return (ISC_R_BADNUMBER); + } + /* Copy source->length bytes and NULL terminate. */ + snprintf(buf, sizeof(buf), "%.*s", (int)source->length, source->base); + str = buf; + + /* Clear out duration. */ + for (i = 0; i < 7; i++) { + duration->parts[i] = 0; + } + duration->iso8601 = false; + duration->unlimited = false; + + /* Every duration starts with 'P' */ + P = strpbrk(str, "Pp"); + if (P == NULL) { + return (ISC_R_BADNUMBER); + } + + /* Record the time indicator. */ + T = strpbrk(str, "Tt"); + + /* Record years. */ + X = strpbrk(str, "Yy"); + if (X != NULL) { + duration->parts[0] = atoi(str + 1); + str = X; + not_weeks = true; + } + + /* Record months. */ + X = strpbrk(str, "Mm"); + + /* + * M could be months or minutes. This is months if there is no time + * part, or this M indicator is before the time indicator. + */ + if (X != NULL && (T == NULL || (size_t)(X - P) < (size_t)(T - P))) { + duration->parts[1] = atoi(str + 1); + str = X; + not_weeks = true; + } + + /* Record days. */ + X = strpbrk(str, "Dd"); + if (X != NULL) { + duration->parts[3] = atoi(str + 1); + str = X; + not_weeks = true; + } + + /* Time part? */ + if (T != NULL) { + str = T; + not_weeks = true; + } + + /* Record hours. */ + X = strpbrk(str, "Hh"); + if (X != NULL && T != NULL) { + duration->parts[4] = atoi(str + 1); + str = X; + not_weeks = true; + } + + /* Record minutes. */ + X = strpbrk(str, "Mm"); + + /* + * M could be months or minutes. This is minutes if there is a time + * part and the M indicator is behind the time indicator. + */ + if (X != NULL && T != NULL && (size_t)(X - P) > (size_t)(T - P)) { + duration->parts[5] = atoi(str + 1); + str = X; + not_weeks = true; + } + + /* Record seconds. */ + X = strpbrk(str, "Ss"); + if (X != NULL && T != NULL) { + duration->parts[6] = atoi(str + 1); + str = X; + not_weeks = true; + } + + /* Or is the duration configured in weeks? */ + W = strpbrk(buf, "Ww"); + if (W != NULL) { + if (not_weeks) { + /* Mix of weeks and other indicators is not allowed */ + return (ISC_R_BADNUMBER); + } else { + duration->parts[2] = atoi(str + 1); + str = W; + } + } + + /* Deal with trailing garbage. */ + if (str[1] != '\0') { + return (ISC_R_BADNUMBER); + } + + duration->iso8601 = true; + return (ISC_R_SUCCESS); +} diff --git a/lib/isccfg/include/isccfg/duration.h b/lib/isccfg/include/isccfg/duration.h new file mode 100644 index 0000000000..32b23e4342 --- /dev/null +++ b/lib/isccfg/include/isccfg/duration.h @@ -0,0 +1,58 @@ + +/* + * Copyright (C) Internet Systems Consortium, Inc. ("ISC") + * + * SPDX-License-Identifier: MPL-2.0 + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * See the COPYRIGHT file distributed with this work for additional + * information regarding copyright ownership. + */ + +#pragma once + +/*! \file */ + +#include +#include + +#include +#include + +ISC_LANG_BEGINDECLS + +#define DURATION_MAXLEN 64 + +/*% + * A configuration object to store ISO 8601 durations. + */ +typedef struct isccfg_duration { + /* + * The duration is stored in multiple parts: + * [0] Years + * [1] Months + * [2] Weeks + * [3] Days + * [4] Hours + * [5] Minutes + * [6] Seconds + */ + uint32_t parts[7]; + bool iso8601; + bool unlimited; +} isccfg_duration_t; + +isc_result_t +isccfg_duration_fromtext(isc_textregion_t *source, isccfg_duration_t *duration); +/*%< + * Converts an ISO 8601 duration style value. + * + * Returns: + *\li ISC_R_SUCCESS + *\li DNS_R_BADNUMBER + */ + +ISC_LANG_ENDDECLS diff --git a/lib/isccfg/include/isccfg/grammar.h b/lib/isccfg/include/isccfg/grammar.h index 2f1b54673d..9c5e5dfa26 100644 --- a/lib/isccfg/include/isccfg/grammar.h +++ b/lib/isccfg/include/isccfg/grammar.h @@ -25,6 +25,7 @@ #include #include +#include /* * Definitions shared between the configuration parser @@ -77,11 +78,8 @@ typedef struct cfg_clausedef cfg_clausedef_t; typedef struct cfg_tuplefielddef cfg_tuplefielddef_t; typedef struct cfg_printer cfg_printer_t; typedef ISC_LIST(cfg_listelt_t) cfg_list_t; -typedef struct cfg_map cfg_map_t; -typedef struct cfg_rep cfg_rep_t; -typedef struct cfg_duration cfg_duration_t; - -#define CFG_DURATION_MAXLEN 64 +typedef struct cfg_map cfg_map_t; +typedef struct cfg_rep cfg_rep_t; /* * Function types for configuration object methods @@ -125,17 +123,17 @@ struct cfg_tuplefielddef { /*% A configuration object type definition. */ struct cfg_type { - const char *name; /*%< For debugging purposes only */ + const char *name; /*%< For debugging purposes only */ cfg_parsefunc_t parse; cfg_printfunc_t print; cfg_docfunc_t doc; /*%< Print grammar description */ - cfg_rep_t *rep; /*%< Data representation */ - const void *of; /*%< Additional data for meta-types */ + cfg_rep_t *rep; /*%< Data representation */ + const void *of; /*%< Additional data for meta-types */ }; /*% A keyword-type definition, for things like "port ". */ typedef struct { - const char *name; + const char *name; const cfg_type_t *type; } keyword_type_t; @@ -155,30 +153,11 @@ struct cfg_netprefix { unsigned int prefixlen; }; -/*% - * A configuration object to store ISO 8601 durations. - */ -struct cfg_duration { - /* - * The duration is stored in multiple parts: - * [0] Years - * [1] Months - * [2] Weeks - * [3] Days - * [4] Hours - * [5] Minutes - * [6] Seconds - */ - uint32_t parts[7]; - bool iso8601; - bool unlimited; -}; - /*% * A configuration data representation. */ struct cfg_rep { - const char *name; /*%< For debugging only */ + const char *name; /*%< For debugging only */ cfg_freefunc_t free; /*%< How to free this kind of data. */ }; @@ -196,17 +175,17 @@ struct cfg_obj { bool boolean; cfg_map_t map; cfg_list_t list; - cfg_obj_t **tuple; + cfg_obj_t **tuple; isc_sockaddr_t sockaddr; struct { isc_sockaddr_t sockaddr; isc_dscp_t dscp; } sockaddrdscp; - cfg_netprefix_t netprefix; - cfg_duration_t duration; + cfg_netprefix_t netprefix; + isccfg_duration_t duration; } value; isc_refcount_t references; /*%< reference counter */ - const char *file; + const char *file; unsigned int line; cfg_parser_t *pctx; }; @@ -219,9 +198,9 @@ struct cfg_listelt { /*% The parser object. */ struct cfg_parser { - isc_mem_t *mctx; - isc_log_t *lctx; - isc_lex_t *lexer; + isc_mem_t *mctx; + isc_log_t *lctx; + isc_lex_t *lexer; unsigned int errors; unsigned int warnings; isc_token_t token; @@ -275,7 +254,7 @@ struct cfg_parser { isc_refcount_t references; cfg_parsecallback_t callback; - void *callbackarg; + void *callbackarg; }; /* Parser context flags */ diff --git a/lib/isccfg/parser.c b/lib/isccfg/parser.c index eeea6e7790..db2a328c2a 100644 --- a/lib/isccfg/parser.c +++ b/lib/isccfg/parser.c @@ -12,9 +12,6 @@ */ /* - * duration_fromtext initially taken from OpenDNSSEC code base. - * Modified to fit the BIND 9 code. - * * Copyright (c) 2009-2018 NLNet Labs. * All rights reserved. * @@ -1033,12 +1030,12 @@ numlen(uint32_t num) { */ void cfg_print_duration(cfg_printer_t *pctx, const cfg_obj_t *obj) { - char buf[CFG_DURATION_MAXLEN]; + char buf[DURATION_MAXLEN]; char *str; const char *indicators = "YMWDHMS"; int count, i; int durationlen[7]; - cfg_duration_t duration; + isccfg_duration_t duration; /* * D ? The duration has a date part. * T ? The duration has a time part. @@ -1090,7 +1087,7 @@ cfg_print_duration(cfg_printer_t *pctx, const cfg_obj_t *obj) { if (T) { count++; } - INSIST(count < CFG_DURATION_MAXLEN); + INSIST(count < DURATION_MAXLEN); /* Now print the duration. */ for (i = 0; i < 6; i++) { @@ -1119,7 +1116,7 @@ cfg_print_duration(cfg_printer_t *pctx, const cfg_obj_t *obj) { void cfg_print_duration_or_unlimited(cfg_printer_t *pctx, const cfg_obj_t *obj) { - cfg_duration_t duration; + isccfg_duration_t duration; REQUIRE(pctx != NULL); REQUIRE(obj != NULL); @@ -1157,149 +1154,30 @@ cfg_obj_asduration(const cfg_obj_t *obj) { return (duration); } -static isc_result_t -duration_fromtext(isc_textregion_t *source, cfg_duration_t *duration) { - char buf[CFG_DURATION_MAXLEN]; - char *P, *X, *T, *W, *str; - bool not_weeks = false; - int i; - - /* - * Copy the buffer as it may not be NULL terminated. - * Anyone having a duration longer than 63 characters is crazy. - */ - if (source->length > sizeof(buf) - 1) { - return (ISC_R_BADNUMBER); - } - /* Copy source->length bytes and NULL terminate. */ - snprintf(buf, sizeof(buf), "%.*s", (int)source->length, source->base); - str = buf; - - /* Clear out duration. */ - for (i = 0; i < 7; i++) { - duration->parts[i] = 0; - } - - /* Every duration starts with 'P' */ - P = strpbrk(str, "Pp"); - if (P == NULL) { - return (ISC_R_BADNUMBER); - } - - /* Record the time indicator. */ - T = strpbrk(str, "Tt"); - - /* Record years. */ - X = strpbrk(str, "Yy"); - if (X != NULL) { - duration->parts[0] = atoi(str + 1); - str = X; - not_weeks = true; - } - - /* Record months. */ - X = strpbrk(str, "Mm"); - - /* - * M could be months or minutes. This is months if there is no time - * part, or this M indicator is before the time indicator. - */ - if (X != NULL && (T == NULL || (size_t)(X - P) < (size_t)(T - P))) { - duration->parts[1] = atoi(str + 1); - str = X; - not_weeks = true; - } - - /* Record days. */ - X = strpbrk(str, "Dd"); - if (X != NULL) { - duration->parts[3] = atoi(str + 1); - str = X; - not_weeks = true; - } - - /* Time part? */ - if (T != NULL) { - str = T; - not_weeks = true; - } - - /* Record hours. */ - X = strpbrk(str, "Hh"); - if (X != NULL && T != NULL) { - duration->parts[4] = atoi(str + 1); - str = X; - not_weeks = true; - } - - /* Record minutes. */ - X = strpbrk(str, "Mm"); - - /* - * M could be months or minutes. This is minutes if there is a time - * part and the M indicator is behind the time indicator. - */ - if (X != NULL && T != NULL && (size_t)(X - P) > (size_t)(T - P)) { - duration->parts[5] = atoi(str + 1); - str = X; - not_weeks = true; - } - - /* Record seconds. */ - X = strpbrk(str, "Ss"); - if (X != NULL && T != NULL) { - duration->parts[6] = atoi(str + 1); - str = X; - not_weeks = true; - } - - /* Or is the duration configured in weeks? */ - W = strpbrk(buf, "Ww"); - if (W != NULL) { - if (not_weeks) { - /* Mix of weeks and other indicators is not allowed */ - return (ISC_R_BADNUMBER); - } else { - duration->parts[2] = atoi(str + 1); - str = W; - } - } - - /* Deal with trailing garbage. */ - if (str[1] != '\0') { - return (ISC_R_BADNUMBER); - } - - return (ISC_R_SUCCESS); -} - static isc_result_t parse_duration(cfg_parser_t *pctx, cfg_obj_t **ret) { isc_result_t result; cfg_obj_t *obj = NULL; - cfg_duration_t duration; + isccfg_duration_t duration; duration.unlimited = false; - if (toupper((unsigned char)TOKEN_STRING(pctx)[0]) == 'P') { - result = duration_fromtext(&pctx->token.value.as_textregion, - &duration); - duration.iso8601 = true; - } else { + result = isccfg_duration_fromtext(&pctx->token.value.as_textregion, + &duration); + if (result == ISC_R_BADNUMBER) { + /* Fallback to dns_ttl_fromtext. */ uint32_t ttl; result = dns_ttl_fromtext(&pctx->token.value.as_textregion, &ttl); - /* - * With dns_ttl_fromtext() the information on optional units. - * is lost, and is treated as seconds from now on. - */ - for (int i = 0; i < 6; i++) { - duration.parts[i] = 0; + if (result == ISC_R_SUCCESS) { + /* + * With dns_ttl_fromtext() the information on optional + * units is lost, and is treated as seconds from now on. + */ + duration.iso8601 = false; + duration.parts[6] = ttl; } - duration.parts[6] = ttl; - duration.iso8601 = false; } - if (result == ISC_R_RANGE) { cfg_parser_error(pctx, CFG_LOG_NEAR, "duration or TTL out of range"); @@ -1346,7 +1224,7 @@ 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; + isccfg_duration_t duration; UNUSED(type); From 03c0c72aeb795dc6f52af021bce679b3a076ad04 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Tue, 21 Jun 2022 12:31:05 +0200 Subject: [PATCH 2/8] Store built-in dnssec-policies in defaultconf Update the defaultconf with the built-in policies. These will now be printed with "named -C". Change the defines in kasp.h to be strings, so they can be concatenated in the defaultconf. This means when creating a kasp structure, we no longer initialize the defaults (this is fine because only kaspconf.c uses dns_kasp_create() and it inherits from the default policy). In kaspconf.c, the default values now need to be parsed from string. Introduce some variables so we don't need to do get_duration multiple times on the same configuration option. Finally, clang-format-14 decided to do some random formatting changes. (cherry picked from commit 5ff414e986fad453b0fd65c7ee14af277b1b73c2) --- bin/named/config.c | 39 +++++++++ lib/dns/include/dns/kasp.h | 22 ++--- lib/dns/kasp.c | 28 ++----- lib/isccfg/include/isccfg/grammar.h | 22 ++--- lib/isccfg/kaspconf.c | 121 +++++++++++++++++----------- 5 files changed, 142 insertions(+), 90 deletions(-) diff --git a/bin/named/config.c b/bin/named/config.c index 1471b7bfb6..8fd96b91a5 100644 --- a/bin/named/config.c +++ b/bin/named/config.c @@ -29,6 +29,7 @@ #include #include +#include #include #include #include @@ -294,6 +295,44 @@ view \"_bind\" chaos {\n\ database \"_builtin id\";\n\ };\n\ };\n\ +" + "#\n\ +# Built-in DNSSEC key and signing policies.\n\ +#\n\ +dnssec-policy \"default\" {\n\ + keys {\n\ + csk key-directory lifetime unlimited algorithm 13;\n\ + };\n\ +\n\ + dnskey-ttl " DNS_KASP_KEY_TTL ";\n\ + publish-safety " DNS_KASP_PUBLISH_SAFETY "; \n\ + retire-safety " DNS_KASP_RETIRE_SAFETY "; \n\ + purge-keys " DNS_KASP_PURGE_KEYS "; \n\ + signatures-refresh " DNS_KASP_SIG_REFRESH "; \n\ + signatures-validity " DNS_KASP_SIG_VALIDITY "; \n\ + signatures-validity-dnskey " DNS_KASP_SIG_VALIDITY_DNSKEY "; \n\ + max-zone-ttl " DNS_KASP_ZONE_MAXTTL "; \n\ + zone-propagation-delay " DNS_KASP_ZONE_PROPDELAY "; \n\ + parent-ds-ttl " DNS_KASP_DS_TTL "; \n\ + parent-propagation-delay " DNS_KASP_PARENT_PROPDELAY "; \n\ +};\n\ +\n\ +dnssec-policy \"insecure\" {\n\ + keys { };\n\ +\n\ + dnskey-ttl " DNS_KASP_KEY_TTL "; \n\ + publish-safety " DNS_KASP_PUBLISH_SAFETY "; \n\ + retire-safety " DNS_KASP_RETIRE_SAFETY "; \n\ + purge-keys " DNS_KASP_PURGE_KEYS "; \n\ + signatures-refresh " DNS_KASP_SIG_REFRESH "; \n\ + signatures-validity " DNS_KASP_SIG_VALIDITY "; \n\ + signatures-validity-dnskey " DNS_KASP_SIG_VALIDITY_DNSKEY "; \n\ + max-zone-ttl " DNS_KASP_ZONE_MAXTTL "; \n\ + zone-propagation-delay " DNS_KASP_ZONE_PROPDELAY "; \n\ + parent-ds-ttl " DNS_KASP_DS_TTL "; \n\ + parent-propagation-delay " DNS_KASP_PARENT_PROPDELAY "; \n\ +};\n\ +\n\ " "#\n\ # Default trusted key(s), used if \n\ diff --git a/lib/dns/include/dns/kasp.h b/lib/dns/include/dns/kasp.h index ca7dfce6f0..b429494ca3 100644 --- a/lib/dns/include/dns/kasp.h +++ b/lib/dns/include/dns/kasp.h @@ -105,17 +105,17 @@ struct dns_kasp { #define DNS_KASP_VALID(kasp) ISC_MAGIC_VALID(kasp, DNS_KASP_MAGIC) /* Defaults */ -#define DNS_KASP_SIG_REFRESH (86400 * 5) -#define DNS_KASP_SIG_VALIDITY (86400 * 14) -#define DNS_KASP_SIG_VALIDITY_DNSKEY (86400 * 14) -#define DNS_KASP_KEY_TTL (3600) -#define DNS_KASP_DS_TTL (86400) -#define DNS_KASP_PUBLISH_SAFETY (3600) -#define DNS_KASP_PURGE_KEYS (86400 * 90) -#define DNS_KASP_RETIRE_SAFETY (3600) -#define DNS_KASP_ZONE_MAXTTL (86400) -#define DNS_KASP_ZONE_PROPDELAY (300) -#define DNS_KASP_PARENT_PROPDELAY (3600) +#define DNS_KASP_SIG_REFRESH "P5D" +#define DNS_KASP_SIG_VALIDITY "P14D" +#define DNS_KASP_SIG_VALIDITY_DNSKEY "P14D" +#define DNS_KASP_KEY_TTL "3600" +#define DNS_KASP_DS_TTL "86400" +#define DNS_KASP_PUBLISH_SAFETY "3600" +#define DNS_KASP_PURGE_KEYS "P90D" +#define DNS_KASP_RETIRE_SAFETY "3600" +#define DNS_KASP_ZONE_MAXTTL "86400" +#define DNS_KASP_ZONE_PROPDELAY "300" +#define DNS_KASP_PARENT_PROPDELAY "3600" /* Key roles */ #define DNS_KASP_KEY_ROLE_KSK 0x01 diff --git a/lib/dns/kasp.c b/lib/dns/kasp.c index 09c6810958..cdc70fd2d9 100644 --- a/lib/dns/kasp.c +++ b/lib/dns/kasp.c @@ -30,44 +30,26 @@ isc_result_t dns_kasp_create(isc_mem_t *mctx, const char *name, dns_kasp_t **kaspp) { dns_kasp_t *kasp; + dns_kasp_t k = { + .magic = DNS_KASP_MAGIC, + }; REQUIRE(name != NULL); REQUIRE(kaspp != NULL && *kaspp == NULL); kasp = isc_mem_get(mctx, sizeof(*kasp)); + *kasp = k; + kasp->mctx = NULL; isc_mem_attach(mctx, &kasp->mctx); - kasp->name = isc_mem_strdup(mctx, name); isc_mutex_init(&kasp->lock); - kasp->frozen = false; - isc_refcount_init(&kasp->references, 1); ISC_LINK_INIT(kasp, link); - - kasp->signatures_refresh = DNS_KASP_SIG_REFRESH; - kasp->signatures_validity = DNS_KASP_SIG_VALIDITY; - kasp->signatures_validity_dnskey = DNS_KASP_SIG_VALIDITY_DNSKEY; - ISC_LIST_INIT(kasp->keys); - kasp->dnskey_ttl = DNS_KASP_KEY_TTL; - kasp->publish_safety = DNS_KASP_PUBLISH_SAFETY; - kasp->retire_safety = DNS_KASP_RETIRE_SAFETY; - kasp->purge_keys = DNS_KASP_PURGE_KEYS; - - kasp->zone_max_ttl = DNS_KASP_ZONE_MAXTTL; - kasp->zone_propagation_delay = DNS_KASP_ZONE_PROPDELAY; - - kasp->parent_ds_ttl = DNS_KASP_DS_TTL; - kasp->parent_propagation_delay = DNS_KASP_PARENT_PROPDELAY; - - kasp->nsec3 = false; - - kasp->magic = DNS_KASP_MAGIC; *kaspp = kasp; - return (ISC_R_SUCCESS); } diff --git a/lib/isccfg/include/isccfg/grammar.h b/lib/isccfg/include/isccfg/grammar.h index 9c5e5dfa26..01d31fc3f1 100644 --- a/lib/isccfg/include/isccfg/grammar.h +++ b/lib/isccfg/include/isccfg/grammar.h @@ -123,17 +123,17 @@ struct cfg_tuplefielddef { /*% A configuration object type definition. */ struct cfg_type { - const char *name; /*%< For debugging purposes only */ + const char *name; /*%< For debugging purposes only */ cfg_parsefunc_t parse; cfg_printfunc_t print; cfg_docfunc_t doc; /*%< Print grammar description */ - cfg_rep_t *rep; /*%< Data representation */ - const void *of; /*%< Additional data for meta-types */ + cfg_rep_t *rep; /*%< Data representation */ + const void *of; /*%< Additional data for meta-types */ }; /*% A keyword-type definition, for things like "port ". */ typedef struct { - const char *name; + const char *name; const cfg_type_t *type; } keyword_type_t; @@ -157,7 +157,7 @@ struct cfg_netprefix { * A configuration data representation. */ struct cfg_rep { - const char *name; /*%< For debugging only */ + const char *name; /*%< For debugging only */ cfg_freefunc_t free; /*%< How to free this kind of data. */ }; @@ -175,7 +175,7 @@ struct cfg_obj { bool boolean; cfg_map_t map; cfg_list_t list; - cfg_obj_t **tuple; + cfg_obj_t **tuple; isc_sockaddr_t sockaddr; struct { isc_sockaddr_t sockaddr; @@ -185,7 +185,7 @@ struct cfg_obj { isccfg_duration_t duration; } value; isc_refcount_t references; /*%< reference counter */ - const char *file; + const char *file; unsigned int line; cfg_parser_t *pctx; }; @@ -198,9 +198,9 @@ struct cfg_listelt { /*% The parser object. */ struct cfg_parser { - isc_mem_t *mctx; - isc_log_t *lctx; - isc_lex_t *lexer; + isc_mem_t *mctx; + isc_log_t *lctx; + isc_lex_t *lexer; unsigned int errors; unsigned int warnings; isc_token_t token; @@ -254,7 +254,7 @@ struct cfg_parser { isc_refcount_t references; cfg_parsecallback_t callback; - void *callbackarg; + void *callbackarg; }; /* Parser context flags */ diff --git a/lib/isccfg/kaspconf.c b/lib/isccfg/kaspconf.c index d7a01ccd04..db99ebbae3 100644 --- a/lib/isccfg/kaspconf.c +++ b/lib/isccfg/kaspconf.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -27,8 +28,10 @@ #include #include #include +#include #include +#include #include #include @@ -50,18 +53,48 @@ confget(cfg_obj_t const *const *maps, const char *name, const cfg_obj_t **obj) { } } +/* + * Utility function for parsing durations from string. + */ +static uint32_t +parse_duration(const char *str) { + uint32_t time = 0; + isccfg_duration_t duration; + isc_result_t result; + isc_textregion_t tr; + + DE_CONST(str, tr.base); + tr.length = strlen(tr.base); + result = isccfg_duration_fromtext(&tr, &duration); + if (result == ISC_R_BADNUMBER) { + /* Fallback to dns_ttl_fromtext. */ + (void)dns_ttl_fromtext(&tr, &time); + return (time); + } + if (result == ISC_R_SUCCESS) { + time += duration.parts[6]; /* Seconds */ + time += duration.parts[5] * 60; /* Minutes */ + time += duration.parts[4] * 3600; /* Hours */ + time += duration.parts[3] * 86400; /* Days */ + time += duration.parts[2] * 86400 * 7; /* Weaks */ + time += duration.parts[1] * 86400 * 31; /* Months */ + time += duration.parts[0] * 86400 * 365; /* Years */ + } + return (time); +} + /* * Utility function for configuring durations. */ static uint32_t -get_duration(const cfg_obj_t **maps, const char *option, uint32_t dfl) { +get_duration(const cfg_obj_t **maps, const char *option, const char *dfl) { const cfg_obj_t *obj; isc_result_t result; obj = NULL; result = confget(maps, option, &obj); if (result == ISC_R_NOTFOUND) { - return (dfl); + return (parse_duration(dfl)); } INSIST(result == ISC_R_SUCCESS); return (cfg_obj_asduration(obj)); @@ -291,14 +324,16 @@ cfg_kasp_fromconfig(const cfg_obj_t *config, const char *name, isc_mem_t *mctx, dns_kasp_t *kasp = NULL; size_t i = 0; uint32_t sigrefresh = 0, sigvalidity = 0; + uint32_t dnskeyttl = 0, dsttl = 0, maxttl = 0; + uint32_t publishsafety = 0, retiresafety = 0; + uint32_t zonepropdelay = 0, parentpropdelay = 0; uint32_t ipub = 0, iret = 0; uint32_t ksk_min_lifetime = 0, zsk_min_lifetime = 0; + REQUIRE(config != NULL); REQUIRE(kaspp != NULL && *kaspp == NULL); - kaspname = (name == NULL) - ? cfg_obj_asstring(cfg_tuple_get(config, "name")) - : name; + kaspname = cfg_obj_asstring(cfg_tuple_get(config, "name")); INSIST(kaspname != NULL); result = dns_kasplist_find(kasplist, kaspname, &kasp); @@ -352,10 +387,11 @@ cfg_kasp_fromconfig(const cfg_obj_t *config, const char *name, isc_mem_t *mctx, sigvalidity = get_duration(maps, "signatures-validity", DNS_KASP_SIG_VALIDITY); if (sigrefresh >= (sigvalidity * 0.9)) { - cfg_obj_log(config, logctx, ISC_LOG_ERROR, - "dnssec-policy: policy '%s' signatures-refresh " - "must be at most 90%% of the signatures-validity", - kaspname); + cfg_obj_log( + config, logctx, ISC_LOG_ERROR, + "dnssec-policy: policy '%s' signatures-refresh must be " + "at most 90%% of the signatures-validity", + kaspname); result = ISC_R_FAILURE; } dns_kasp_setsigvalidity(kasp, sigvalidity); @@ -364,34 +400,43 @@ cfg_kasp_fromconfig(const cfg_obj_t *config, const char *name, isc_mem_t *mctx, goto cleanup; } + /* Configuration: Zone settings */ + maxttl = get_duration(maps, "max-zone-ttl", DNS_KASP_ZONE_MAXTTL); + dns_kasp_setzonemaxttl(kasp, maxttl); + + zonepropdelay = get_duration(maps, "zone-propagation-delay", + DNS_KASP_ZONE_PROPDELAY); + dns_kasp_setzonepropagationdelay(kasp, zonepropdelay); + + /* Configuration: Parent settings */ + dsttl = get_duration(maps, "parent-ds-ttl", DNS_KASP_DS_TTL); + dns_kasp_setdsttl(kasp, dsttl); + + parentpropdelay = get_duration(maps, "parent-propagation-delay", + DNS_KASP_PARENT_PROPDELAY); + dns_kasp_setparentpropagationdelay(kasp, parentpropdelay); + /* Configuration: Keys */ - dns_kasp_setdnskeyttl( - kasp, get_duration(maps, "dnskey-ttl", DNS_KASP_KEY_TTL)); - dns_kasp_setpublishsafety(kasp, get_duration(maps, "publish-safety", - DNS_KASP_PUBLISH_SAFETY)); - dns_kasp_setretiresafety(kasp, get_duration(maps, "retire-safety", - DNS_KASP_RETIRE_SAFETY)); + dnskeyttl = get_duration(maps, "dnskey-ttl", DNS_KASP_KEY_TTL); + dns_kasp_setdnskeyttl(kasp, dnskeyttl); + + publishsafety = get_duration(maps, "publish-safety", + DNS_KASP_PUBLISH_SAFETY); + dns_kasp_setpublishsafety(kasp, publishsafety); + + retiresafety = get_duration(maps, "retire-safety", + DNS_KASP_RETIRE_SAFETY); + dns_kasp_setretiresafety(kasp, retiresafety); + dns_kasp_setpurgekeys( kasp, get_duration(maps, "purge-keys", DNS_KASP_PURGE_KEYS)); - ipub = get_duration(maps, "dnskey-ttl", DNS_KASP_KEY_TTL) + - get_duration(maps, "publish-safety", DNS_KASP_PUBLISH_SAFETY) + - get_duration(maps, "zone-propagation-delay", - DNS_KASP_ZONE_PROPDELAY); - - iret = get_duration(maps, "parent-ds-ttl", DNS_KASP_DS_TTL) + - get_duration(maps, "retire-safety", DNS_KASP_RETIRE_SAFETY) + - get_duration(maps, "parent-propagation-delay", - DNS_KASP_PARENT_PROPDELAY); - + ipub = dnskeyttl + publishsafety + zonepropdelay; + iret = dsttl + retiresafety + parentpropdelay; ksk_min_lifetime = ISC_MAX(ipub, iret); - iret = (sigvalidity - sigrefresh) + - get_duration(maps, "max-zone-ttl", DNS_KASP_ZONE_MAXTTL) + - get_duration(maps, "retire-safety", DNS_KASP_RETIRE_SAFETY) + - get_duration(maps, "zone-propagation-delay", - DNS_KASP_ZONE_PROPDELAY); - + iret = (sigvalidity - sigrefresh) + maxttl + retiresafety + + zonepropdelay; zsk_min_lifetime = ISC_MAX(ipub, iret); (void)confget(maps, "keys", &keys); @@ -489,20 +534,6 @@ cfg_kasp_fromconfig(const cfg_obj_t *config, const char *name, isc_mem_t *mctx, } } - /* Configuration: Zone settings */ - dns_kasp_setzonemaxttl( - kasp, get_duration(maps, "max-zone-ttl", DNS_KASP_ZONE_MAXTTL)); - dns_kasp_setzonepropagationdelay( - kasp, get_duration(maps, "zone-propagation-delay", - DNS_KASP_ZONE_PROPDELAY)); - - /* Configuration: Parent settings */ - dns_kasp_setdsttl(kasp, - get_duration(maps, "parent-ds-ttl", DNS_KASP_DS_TTL)); - dns_kasp_setparentpropagationdelay( - kasp, get_duration(maps, "parent-propagation-delay", - DNS_KASP_PARENT_PROPDELAY)); - /* Append it to the list for future lookups. */ ISC_LIST_APPEND(*kasplist, kasp, link); INSIST(!(ISC_LIST_EMPTY(*kasplist))); From e16cfce91d00db501c1973a6204ff0f8b4b76c2d Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Tue, 21 Jun 2022 12:40:12 +0200 Subject: [PATCH 3/8] When loading dnssec-policies, inherit from default Most of the settings (durations) are already inheriting from the default because they use the constants from lib/dns/kasp.h. We need them as constants so we can use them in named-checkconf to verify the policy parameters. The NSEC(3) parameters and keys should come from the actual default policy. Change the call to cfg_kasp_fromconfig() to include the default kasp. We also no longer need to corner case where config is NULL we load the built-in policy: the built-in policies are now loaded when config is set to named_g_config. Finally, add a debug log (it is useful to see which policies are being loaded). (cherry picked from commit 20acb8d3a385f62fcabcc8b0279ca74ba05e3e1d) --- bin/named/server.c | 39 ++++++++++------- lib/isccfg/include/isccfg/kaspconf.h | 15 +++---- lib/isccfg/kaspconf.c | 63 +++++++++++++++++++++------- 3 files changed, 80 insertions(+), 37 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 842277d46a..4995e47fef 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -8479,6 +8479,7 @@ load_configuration(const char *filename, named_server_t *server, const cfg_obj_t *kasps; dns_kasp_t *kasp = NULL; dns_kasp_t *kasp_next = NULL; + dns_kasp_t *default_kasp = NULL; dns_kasplist_t tmpkasplist, kasplist; const cfg_obj_t *views; dns_view_t *view = NULL; @@ -9205,10 +9206,10 @@ load_configuration(const char *filename, named_server_t *server, (void)configure_session_key(maps, server, named_g_mctx, first_time); /* - * Create the DNSSEC key and signing policies (KASP). + * Create the built-in kasp policies ("default", "insecure"). */ kasps = NULL; - (void)cfg_map_get(config, "dnssec-policy", &kasps); + (void)cfg_map_get(named_g_config, "dnssec-policy", &kasps); for (element = cfg_list_first(kasps); element != NULL; element = cfg_list_next(element)) { @@ -9218,25 +9219,31 @@ load_configuration(const char *filename, named_server_t *server, named_g_lctx, &kasplist, &kasp)); INSIST(kasp != NULL); dns_kasp_freeze(kasp); + if (strcmp(dns_kasp_getname(kasp), "default") == 0) { + dns_kasp_attach(kasp, &default_kasp); + } dns_kasp_detach(&kasp); } + INSIST(default_kasp != NULL); + /* - * Create the built-in kasp policies ("default", "insecure"). + * Create the DNSSEC key and signing policies (KASP). */ - kasp = NULL; - CHECK(cfg_kasp_fromconfig(NULL, "default", named_g_mctx, named_g_lctx, - &kasplist, &kasp)); - INSIST(kasp != NULL); - dns_kasp_freeze(kasp); - dns_kasp_detach(&kasp); - - kasp = NULL; - CHECK(cfg_kasp_fromconfig(NULL, "insecure", named_g_mctx, named_g_lctx, - &kasplist, &kasp)); - INSIST(kasp != NULL); - dns_kasp_freeze(kasp); - dns_kasp_detach(&kasp); + kasps = NULL; + (void)cfg_map_get(config, "dnssec-policy", &kasps); + for (element = cfg_list_first(kasps); element != NULL; + element = cfg_list_next(element)) + { + cfg_obj_t *kconfig = cfg_listelt_value(element); + kasp = NULL; + CHECK(cfg_kasp_fromconfig(kconfig, default_kasp, named_g_mctx, + named_g_lctx, &kasplist, &kasp)); + INSIST(kasp != NULL); + dns_kasp_freeze(kasp); + dns_kasp_detach(&kasp); + } + dns_kasp_detach(&default_kasp); tmpkasplist = server->kasplist; server->kasplist = kasplist; kasplist = tmpkasplist; diff --git a/lib/isccfg/include/isccfg/kaspconf.h b/lib/isccfg/include/isccfg/kaspconf.h index f7df0fbf5f..65e73e0137 100644 --- a/lib/isccfg/include/isccfg/kaspconf.h +++ b/lib/isccfg/include/isccfg/kaspconf.h @@ -26,14 +26,15 @@ ISC_LANG_BEGINDECLS isc_result_t -cfg_kasp_fromconfig(const cfg_obj_t *config, const char *name, isc_mem_t *mctx, - isc_log_t *logctx, dns_kasplist_t *kasplist, - dns_kasp_t **kaspp); +cfg_kasp_fromconfig(const cfg_obj_t *config, dns_kasp_t *default_kasp, + isc_mem_t *mctx, isc_log_t *logctx, + dns_kasplist_t *kasplist, dns_kasp_t **kaspp); /*%< - * Create and configure a KASP. If 'config' is NULL, a built-in configuration - * is used, referred to by 'name'. If a 'kasplist' is provided, a lookup - * happens and if a KASP already exists with the same name, no new KASP is - * created, and no attach to 'kaspp' happens. + * Create and configure a KASP. If 'default_kasp' is not NULL, the built-in + * default configuration is used to set values that are not explicitly set in + * the policy. If a 'kasplist' is provided, a lookup happens and if a KASP + * already exists with the same name, no new KASP is created, and no attach to + * 'kaspp' happens. * * Requires: * diff --git a/lib/isccfg/kaspconf.c b/lib/isccfg/kaspconf.c index db99ebbae3..932466aad7 100644 --- a/lib/isccfg/kaspconf.c +++ b/lib/isccfg/kaspconf.c @@ -311,9 +311,9 @@ cfg_nsec3param_fromconfig(const cfg_obj_t *config, dns_kasp_t *kasp, } isc_result_t -cfg_kasp_fromconfig(const cfg_obj_t *config, const char *name, isc_mem_t *mctx, - isc_log_t *logctx, dns_kasplist_t *kasplist, - dns_kasp_t **kaspp) { +cfg_kasp_fromconfig(const cfg_obj_t *config, dns_kasp_t *default_kasp, + isc_mem_t *mctx, isc_log_t *logctx, + dns_kasplist_t *kasplist, dns_kasp_t **kaspp) { isc_result_t result; const cfg_obj_t *maps[2]; const cfg_obj_t *koptions = NULL; @@ -336,6 +336,9 @@ cfg_kasp_fromconfig(const cfg_obj_t *config, const char *name, isc_mem_t *mctx, kaspname = cfg_obj_asstring(cfg_tuple_get(config, "name")); INSIST(kaspname != NULL); + cfg_obj_log(config, logctx, ISC_LOG_DEBUG(1), + "dnssec-policy: load policy '%s'", kaspname); + result = dns_kasplist_find(kasplist, kaspname, &kasp); if (result == ISC_R_SUCCESS) { @@ -456,7 +459,6 @@ cfg_kasp_fromconfig(const cfg_obj_t *config, const char *name, isc_mem_t *mctx, goto cleanup; } } - INSIST(!(dns_kasp_keylist_empty(kasp))); dns_kasp_freeze(kasp); for (kkey = ISC_LIST_HEAD(dns_kasp_keys(kasp)); kkey != NULL; kkey = ISC_LIST_NEXT(kkey, link)) @@ -509,23 +511,56 @@ cfg_kasp_fromconfig(const cfg_obj_t *config, const char *name, isc_mem_t *mctx, if (result != ISC_R_SUCCESS) { goto cleanup; } - } else if (strcmp(kaspname, "insecure") == 0) { - /* "dnssec-policy insecure": key list must be empty */ - INSIST(strcmp(kaspname, "insecure") == 0); - INSIST(dns_kasp_keylist_empty(kasp)); - } else { - /* No keys clause configured, use the "default". */ - result = cfg_kaspkey_fromconfig(NULL, kasp, logctx, 0, 0); - if (result != ISC_R_SUCCESS) { - goto cleanup; + } else if (default_kasp && strcmp(kaspname, "insecure") != 0) { + dns_kasp_key_t *key, *new_key; + + /* + * If there are no specific keys configured in the policy, + * inherit from the default policy (except for the built-in + * "insecure" policy). + */ + for (key = ISC_LIST_HEAD(dns_kasp_keys(default_kasp)); + key != NULL; key = ISC_LIST_NEXT(key, link)) + { + /* Create a new key reference. */ + new_key = NULL; + result = dns_kasp_key_create(kasp, &new_key); + if (result != ISC_R_SUCCESS) { + goto cleanup; + } + + if (dns_kasp_key_ksk(key)) { + new_key->role |= DNS_KASP_KEY_ROLE_KSK; + } + if (dns_kasp_key_zsk(key)) { + new_key->role |= DNS_KASP_KEY_ROLE_ZSK; + } + new_key->lifetime = dns_kasp_key_lifetime(key); + new_key->algorithm = dns_kasp_key_algorithm(key); + new_key->length = dns_kasp_key_size(key); + dns_kasp_addkey(kasp, new_key); } + } + + if (strcmp(kaspname, "insecure") == 0) { + /* "dnssec-policy insecure": key list must be empty */ + INSIST(dns_kasp_keylist_empty(kasp)); + } else if (default_kasp != NULL) { + /* There must be keys configured. */ INSIST(!(dns_kasp_keylist_empty(kasp))); } /* Configuration: NSEC3 */ (void)confget(maps, "nsec3param", &nsec3); if (nsec3 == NULL) { - dns_kasp_setnsec3(kasp, false); + if (default_kasp != NULL && dns_kasp_nsec3(default_kasp)) { + dns_kasp_setnsec3param( + kasp, dns_kasp_nsec3iter(default_kasp), + (dns_kasp_nsec3flags(default_kasp) == 0x01), + dns_kasp_nsec3saltlen(default_kasp)); + } else { + dns_kasp_setnsec3(kasp, false); + } } else { dns_kasp_setnsec3(kasp, true); result = cfg_nsec3param_fromconfig(nsec3, kasp, logctx); From fd34ea8523d10f2091d45d1a0d574a4d76d96522 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Tue, 21 Jun 2022 12:45:54 +0200 Subject: [PATCH 4/8] Nit changes in keymgr and kasp Use the ISC_MAX define instead of "x = a > b ? a : b" paradigm. Remove an unneeded include. (cherry picked from commit 5d6f0de84bc1a899f1174e92dcaf2a1acf3603cf) --- lib/dns/keymgr.c | 6 +++--- lib/isccfg/include/isccfg/kaspconf.h | 2 -- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/dns/keymgr.c b/lib/dns/keymgr.c index 990c442ca1..024107a893 100644 --- a/lib/dns/keymgr.c +++ b/lib/dns/keymgr.c @@ -131,12 +131,12 @@ keymgr_settime_remove(dns_dnsseckey_t *key, dns_kasp_t *kasp) { dns_kasp_retiresafety(kasp); } - remove = ksk_remove > zsk_remove ? ksk_remove : zsk_remove; + remove = ISC_MAX(ksk_remove, zsk_remove); dst_key_settime(key->key, DST_TIME_DELETE, remove); } /* - * Set the SyncPublish time (when the DS may be submitted to the parent) + * Set the SyncPublish time (when the DS may be submitted to the parent). * */ static void @@ -250,7 +250,7 @@ keymgr_prepublication_time(dns_dnsseckey_t *key, dns_kasp_t *kasp, dns_kasp_zonepropagationdelay(kasp); } - syncpub = syncpub1 > syncpub2 ? syncpub1 : syncpub2; + syncpub = ISC_MAX(syncpub1, syncpub2); dst_key_settime(key->key, DST_TIME_SYNCPUBLISH, syncpub); } diff --git a/lib/isccfg/include/isccfg/kaspconf.h b/lib/isccfg/include/isccfg/kaspconf.h index 65e73e0137..7b1e075fef 100644 --- a/lib/isccfg/include/isccfg/kaspconf.h +++ b/lib/isccfg/include/isccfg/kaspconf.h @@ -15,8 +15,6 @@ #include -#include - #include /*** From bd15b7c3c61f0fee787c62376fcd5dd70438aa43 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Tue, 21 Jun 2022 14:46:44 +0200 Subject: [PATCH 5/8] Add change entry for dnssec-policy in defaultconf (cherry picked from commit 80b55f9cfa441b0c516b774323fcf348198fe706) --- CHANGES | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES b/CHANGES index 3997f9d452..c663b53fec 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +5910. [cleanup] Move built-in dnssec-policies into the defaultconf. + These are now printed with 'named -C'. [GL !6467] + 5909. [bug] The server-side destination port was missing from dnstap captures of client traffic. [GL #3309] From 8af88d41115072c1d311474addff9aec75f64748 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Fri, 24 Jun 2022 09:22:38 +0200 Subject: [PATCH 6/8] Also inherit from "default" for "insecure" policy Remove the duplication from the defaultconf and inherit the values not set in the "insecure" policy from the "default" policy. Therefore, we must insist that the first read built-in policy is the default one. (cherry picked from commit c2a7950417024cf6db52a9dc3f3135c473d22072) --- bin/named/config.c | 12 ------------ bin/named/server.c | 9 +++++++-- lib/isccfg/kaspconf.c | 3 +-- 3 files changed, 8 insertions(+), 16 deletions(-) diff --git a/bin/named/config.c b/bin/named/config.c index 8fd96b91a5..7743645eb3 100644 --- a/bin/named/config.c +++ b/bin/named/config.c @@ -319,18 +319,6 @@ dnssec-policy \"default\" {\n\ \n\ dnssec-policy \"insecure\" {\n\ keys { };\n\ -\n\ - dnskey-ttl " DNS_KASP_KEY_TTL "; \n\ - publish-safety " DNS_KASP_PUBLISH_SAFETY "; \n\ - retire-safety " DNS_KASP_RETIRE_SAFETY "; \n\ - purge-keys " DNS_KASP_PURGE_KEYS "; \n\ - signatures-refresh " DNS_KASP_SIG_REFRESH "; \n\ - signatures-validity " DNS_KASP_SIG_VALIDITY "; \n\ - signatures-validity-dnskey " DNS_KASP_SIG_VALIDITY_DNSKEY "; \n\ - max-zone-ttl " DNS_KASP_ZONE_MAXTTL "; \n\ - zone-propagation-delay " DNS_KASP_ZONE_PROPDELAY "; \n\ - parent-ds-ttl " DNS_KASP_DS_TTL "; \n\ - parent-propagation-delay " DNS_KASP_PARENT_PROPDELAY "; \n\ };\n\ \n\ " diff --git a/bin/named/server.c b/bin/named/server.c index 4995e47fef..6f63a85365 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -9214,14 +9214,19 @@ load_configuration(const char *filename, named_server_t *server, element = cfg_list_next(element)) { cfg_obj_t *kconfig = cfg_listelt_value(element); + kasp = NULL; - CHECK(cfg_kasp_fromconfig(kconfig, NULL, named_g_mctx, + CHECK(cfg_kasp_fromconfig(kconfig, default_kasp, named_g_mctx, named_g_lctx, &kasplist, &kasp)); INSIST(kasp != NULL); dns_kasp_freeze(kasp); - if (strcmp(dns_kasp_getname(kasp), "default") == 0) { + + /* Insist that the first built-in policy is the default one. */ + if (default_kasp == NULL) { + INSIST(strcmp(dns_kasp_getname(kasp), "default") == 0); dns_kasp_attach(kasp, &default_kasp); } + dns_kasp_detach(&kasp); } INSIST(default_kasp != NULL); diff --git a/lib/isccfg/kaspconf.c b/lib/isccfg/kaspconf.c index 932466aad7..7c476b1a68 100644 --- a/lib/isccfg/kaspconf.c +++ b/lib/isccfg/kaspconf.c @@ -511,9 +511,8 @@ cfg_kasp_fromconfig(const cfg_obj_t *config, dns_kasp_t *default_kasp, if (result != ISC_R_SUCCESS) { goto cleanup; } - } else if (default_kasp && strcmp(kaspname, "insecure") != 0) { + } else if (default_kasp) { dns_kasp_key_t *key, *new_key; - /* * If there are no specific keys configured in the policy, * inherit from the default policy (except for the built-in From feaf3950fd02c0e76e268252d35dfa61f27751d0 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Fri, 24 Jun 2022 09:58:40 +0200 Subject: [PATCH 7/8] Fix a bug in the duration_fromtext function The function actually did not enforce that the duration string starts with a P (or p), just that there is a P (or p) in the string. (cherry picked from commit 8e18fa5874226600198138c098a437275cd31557) --- bin/tests/system/checkconf/bad-duration.conf | 16 ++++++++++++++++ lib/isccfg/duration.c | 4 ++-- 2 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 bin/tests/system/checkconf/bad-duration.conf diff --git a/bin/tests/system/checkconf/bad-duration.conf b/bin/tests/system/checkconf/bad-duration.conf new file mode 100644 index 0000000000..1fbecc7229 --- /dev/null +++ b/bin/tests/system/checkconf/bad-duration.conf @@ -0,0 +1,16 @@ +/* + * Copyright (C) Internet Systems Consortium, Inc. ("ISC") + * + * SPDX-License-Identifier: MPL-2.0 + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * See the COPYRIGHT file distributed with this work for additional + * information regarding copyright ownership. + */ + +dnssec-policy "test" { + dnskey-ttl xPT1H; +}; diff --git a/lib/isccfg/duration.c b/lib/isccfg/duration.c index 30d110ab0a..0a7501b022 100644 --- a/lib/isccfg/duration.c +++ b/lib/isccfg/duration.c @@ -61,10 +61,10 @@ isccfg_duration_fromtext(isc_textregion_t *source, duration->unlimited = false; /* Every duration starts with 'P' */ - P = strpbrk(str, "Pp"); - if (P == NULL) { + if (toupper(str[0]) != 'P') { return (ISC_R_BADNUMBER); } + P = str; /* Record the time indicator. */ T = strpbrk(str, "Tt"); From 35f6cabab43342425cc46df57b8848e5a1783e82 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Mon, 27 Jun 2022 16:31:43 +0200 Subject: [PATCH 8/8] Add isccfg duration utility functions Add function isccfg_duration_toseconds and isccfg_parse_duration to get rid of code duplication. (cherry picked from commit d8dae61832460e8e7b865083ec1acff8687ad340) --- lib/isccfg/duration.c | 47 ++++++++++++++++++++++++++++ lib/isccfg/include/isccfg/duration.h | 25 +++++++++++++++ lib/isccfg/kaspconf.c | 15 ++------- lib/isccfg/parser.c | 33 ++----------------- 4 files changed, 77 insertions(+), 43 deletions(-) diff --git a/lib/isccfg/duration.c b/lib/isccfg/duration.c index 0a7501b022..27dddca1cc 100644 --- a/lib/isccfg/duration.c +++ b/lib/isccfg/duration.c @@ -28,6 +28,8 @@ #include #include +#include + #include /* @@ -153,3 +155,48 @@ isccfg_duration_fromtext(isc_textregion_t *source, duration->iso8601 = true; return (ISC_R_SUCCESS); } + +isc_result_t +isccfg_parse_duration(isc_textregion_t *source, isccfg_duration_t *duration) { + isc_result_t result; + + REQUIRE(duration != NULL); + + duration->unlimited = false; + result = isccfg_duration_fromtext(source, duration); + if (result == ISC_R_BADNUMBER) { + /* Fallback to dns_ttl_fromtext. */ + uint32_t ttl; + result = dns_ttl_fromtext(source, &ttl); + if (result == ISC_R_SUCCESS) { + /* + * With dns_ttl_fromtext() the information on optional + * units is lost, and is treated as seconds from now on. + */ + duration->iso8601 = false; + duration->parts[6] = ttl; + } + } + + return (result); +} + +uint32_t +isccfg_duration_toseconds(const isccfg_duration_t *duration) { + uint32_t seconds = 0; + + REQUIRE(duration != NULL); + + seconds += duration->parts[6]; /* Seconds */ + seconds += duration->parts[5] * 60; /* Minutes */ + seconds += duration->parts[4] * 3600; /* Hours */ + seconds += duration->parts[3] * 86400; /* Days */ + seconds += duration->parts[2] * 86400 * 7; /* Weeks */ + /* + * The below additions are not entirely correct + * because days may very per month and per year. + */ + seconds += duration->parts[1] * 86400 * 31; /* Months */ + seconds += duration->parts[0] * 86400 * 365; /* Years */ + return (seconds); +} diff --git a/lib/isccfg/include/isccfg/duration.h b/lib/isccfg/include/isccfg/duration.h index 32b23e4342..b64bea520f 100644 --- a/lib/isccfg/include/isccfg/duration.h +++ b/lib/isccfg/include/isccfg/duration.h @@ -55,4 +55,29 @@ isccfg_duration_fromtext(isc_textregion_t *source, isccfg_duration_t *duration); *\li DNS_R_BADNUMBER */ +isc_result_t +isccfg_parse_duration(isc_textregion_t *source, isccfg_duration_t *duration); +/*%< + * Converts a duration string to a ISO 8601 duration. + * If the string does not start with a P (or p), fall back to TTL-style value. + * In that case the duration will be treated in seconds only. + * + * Returns: + *\li ISC_R_SUCCESS + *\li DNS_R_BADNUMBER + *\li DNS_R_BADTTL + */ + +uint32_t +isccfg_duration_toseconds(const isccfg_duration_t *duration); +/*%< + * Converts an ISO 8601 duration to seconds. + * The conversion is approximate: + * - Months will be treated as 31 days. + * - Years will be treated as 365 days. + * + * Returns: + *\li The duration in seconds. + */ + ISC_LANG_ENDDECLS diff --git a/lib/isccfg/kaspconf.c b/lib/isccfg/kaspconf.c index 7c476b1a68..a0460ff968 100644 --- a/lib/isccfg/kaspconf.c +++ b/lib/isccfg/kaspconf.c @@ -65,20 +65,9 @@ parse_duration(const char *str) { DE_CONST(str, tr.base); tr.length = strlen(tr.base); - result = isccfg_duration_fromtext(&tr, &duration); - if (result == ISC_R_BADNUMBER) { - /* Fallback to dns_ttl_fromtext. */ - (void)dns_ttl_fromtext(&tr, &time); - return (time); - } + result = isccfg_parse_duration(&tr, &duration); if (result == ISC_R_SUCCESS) { - time += duration.parts[6]; /* Seconds */ - time += duration.parts[5] * 60; /* Minutes */ - time += duration.parts[4] * 3600; /* Hours */ - time += duration.parts[3] * 86400; /* Days */ - time += duration.parts[2] * 86400 * 7; /* Weaks */ - time += duration.parts[1] * 86400 * 31; /* Months */ - time += duration.parts[0] * 86400 * 365; /* Years */ + time = isccfg_duration_toseconds(&duration); } return (time); } diff --git a/lib/isccfg/parser.c b/lib/isccfg/parser.c index db2a328c2a..4ea849e64f 100644 --- a/lib/isccfg/parser.c +++ b/lib/isccfg/parser.c @@ -1139,19 +1139,7 @@ cfg_obj_isduration(const cfg_obj_t *obj) { uint32_t cfg_obj_asduration(const cfg_obj_t *obj) { REQUIRE(obj != NULL && obj->type->rep == &cfg_rep_duration); - uint32_t duration = 0; - duration += obj->value.duration.parts[6]; /* Seconds */ - duration += obj->value.duration.parts[5] * 60; /* Minutes */ - duration += obj->value.duration.parts[4] * 3600; /* Hours */ - duration += obj->value.duration.parts[3] * 86400; /* Days */ - duration += obj->value.duration.parts[2] * 86400 * 7; /* Weaks */ - /* - * The below additions are not entirely correct - * because days may very per month and per year. - */ - duration += obj->value.duration.parts[1] * 86400 * 31; /* Months */ - duration += obj->value.duration.parts[0] * 86400 * 365; /* Years */ - return (duration); + return isccfg_duration_toseconds(&(obj->value.duration)); } static isc_result_t @@ -1160,24 +1148,9 @@ parse_duration(cfg_parser_t *pctx, cfg_obj_t **ret) { cfg_obj_t *obj = NULL; isccfg_duration_t duration; - duration.unlimited = false; - - result = isccfg_duration_fromtext(&pctx->token.value.as_textregion, + result = isccfg_parse_duration(&pctx->token.value.as_textregion, &duration); - if (result == ISC_R_BADNUMBER) { - /* Fallback to dns_ttl_fromtext. */ - uint32_t ttl; - result = dns_ttl_fromtext(&pctx->token.value.as_textregion, - &ttl); - if (result == ISC_R_SUCCESS) { - /* - * With dns_ttl_fromtext() the information on optional - * units is lost, and is treated as seconds from now on. - */ - duration.iso8601 = false; - duration.parts[6] = ttl; - } - } + if (result == ISC_R_RANGE) { cfg_parser_error(pctx, CFG_LOG_NEAR, "duration or TTL out of range");