diff --git a/CHANGES b/CHANGES index b0c5f899b8..ad32dc08f6 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5642. [bug] Check "key-directory" conflicts in "named.conf" for + zones in multiple views with different "dnssec-policy". + [GL #2463]. + 5641. [bug] Address potential memory leak in dst_key_fromnamedfile. [GL #2689] diff --git a/bin/tests/system/checkconf/bad-kasp-keydir1.conf b/bin/tests/system/checkconf/bad-kasp-keydir1.conf new file mode 100644 index 0000000000..5be13a1feb --- /dev/null +++ b/bin/tests/system/checkconf/bad-kasp-keydir1.conf @@ -0,0 +1,42 @@ +/* + * Copyright (C) Internet Systems Consortium, Inc. ("ISC") + * + * 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 http://mozilla.org/MPL/2.0/. + * + * See the COPYRIGHT file distributed with this work for additional + * information regarding copyright ownership. + */ + +key "keyforview1" { + algorithm "hmac-sha1"; + secret "YPfMoAk6h+3iN8MDRQC004iSNHY="; +}; + +key "keyforview2" { + algorithm "hmac-sha1"; + secret "4xILSZQnuO1UKubXHkYUsvBRPu8="; +}; + +view "example1" { + match-clients { key "keyforview1"; }; + + zone "example.net" { + type primary; + dnssec-policy "default"; + key-directory "."; + file "example1.db"; + }; +}; + +view "example2" { + match-clients { key "keyforview2"; }; + + zone "example.net" { + type primary; + dnssec-policy "insecure"; + key-directory "."; + file "example2.db"; + }; +}; diff --git a/bin/tests/system/checkconf/bad-kasp-keydir2.conf b/bin/tests/system/checkconf/bad-kasp-keydir2.conf new file mode 100644 index 0000000000..67161a8436 --- /dev/null +++ b/bin/tests/system/checkconf/bad-kasp-keydir2.conf @@ -0,0 +1,40 @@ +/* + * Copyright (C) Internet Systems Consortium, Inc. ("ISC") + * + * 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 http://mozilla.org/MPL/2.0/. + * + * See the COPYRIGHT file distributed with this work for additional + * information regarding copyright ownership. + */ + +key "keyforview1" { + algorithm "hmac-sha1"; + secret "YPfMoAk6h+3iN8MDRQC004iSNHY="; +}; + +key "keyforview2" { + algorithm "hmac-sha1"; + secret "4xILSZQnuO1UKubXHkYUsvBRPu8="; +}; + +view "example1" { + match-clients { key "keyforview1"; }; + + zone "example.net" { + type primary; + dnssec-policy "default"; + file "example1.db"; + }; +}; + +view "example2" { + match-clients { key "keyforview2"; }; + + zone "example.net" { + type primary; + dnssec-policy "insecure"; + file "example2.db"; + }; +}; diff --git a/bin/tests/system/checkconf/good.conf b/bin/tests/system/checkconf/good.conf index e09b9e802b..6b950996cc 100644 --- a/bin/tests/system/checkconf/good.conf +++ b/bin/tests/system/checkconf/good.conf @@ -192,7 +192,53 @@ view "fourth" { file "dnssec-none.db"; dnssec-policy "none"; }; + zone "dnssec-view1" { + type master; + file "dnssec-view41.db"; + dnssec-policy "test"; + }; + zone "dnssec-view2" { + type master; + file "dnssec-view42.db"; + }; + zone "dnssec-view3" { + type master; + file "dnssec-view43.db"; + dnssec-policy "none"; + key-directory "keys"; + }; + zone "dnssec-view4" { + type master; + file "dnssec-view44.db"; + dnssec-policy "none"; + }; dnssec-policy "default"; + key-directory "."; +}; +view "fifth" { + zone "dnssec-view1" { + type master; + file "dnssec-view51.db"; + dnssec-policy "test"; + }; + zone "dnssec-view2" { + type master; + file "dnssec-view52.db"; + dnssec-policy "test"; + key-directory "keys"; + }; + zone "dnssec-view3" { + type master; + file "dnssec-view53.db"; + dnssec-policy "default"; + key-directory "keys"; + }; + zone "dnssec-view4" { + type master; + file "dnssec-view54.db"; + dnssec-policy "none"; + }; + key-directory "."; }; view "chaos" chaos { zone "hostname.bind" chaos { diff --git a/bin/tests/system/checkconf/good.zonelist b/bin/tests/system/checkconf/good.zonelist index b33d2fc239..08a5665afd 100644 --- a/bin/tests/system/checkconf/good.zonelist +++ b/bin/tests/system/checkconf/good.zonelist @@ -13,4 +13,12 @@ dnssec-test IN fourth master dnssec-default IN fourth master dnssec-inherit IN fourth master dnssec-none IN fourth master +dnssec-view1 IN fourth master +dnssec-view2 IN fourth master +dnssec-view3 IN fourth master +dnssec-view4 IN fourth master +dnssec-view1 IN fifth master +dnssec-view2 IN fifth master +dnssec-view3 IN fifth master +dnssec-view4 IN fifth master hostname.bind chaos chaos master diff --git a/bin/tests/system/checkconf/tests.sh b/bin/tests/system/checkconf/tests.sh index e6b2e16e40..23d2c430f9 100644 --- a/bin/tests/system/checkconf/tests.sh +++ b/bin/tests/system/checkconf/tests.sh @@ -12,6 +12,8 @@ status=0 n=0 +mkdir keys + n=`expr $n + 1` echo_i "checking that named-checkconf handles a known good config ($n)" ret=0 @@ -549,5 +551,7 @@ grep "exceeds 100%" < checkconf.out$n > /dev/null || ret=1 if [ $ret != 0 ]; then echo_i "failed"; ret=1; fi status=`expr $status + $ret` +rmdir keys + echo_i "exit status: $status" [ $status -eq 0 ] || exit 1 diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index c4da0ed151..a1272fb24a 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -98,3 +98,7 @@ Bug Fixes - ``named-checkconf`` now complains if zones with ``dnssec-policy`` reference the same zone file more than once. :gl:`#2603` + +- Check ``key-directory`` conflicts in ``named.conf`` for zones in multiple + views with different ``dnssec-policy``. Using the same ``key-directory`` for + such zones is not allowed. :gl:`#2463` diff --git a/lib/bind9/check.c b/lib/bind9/check.c index a7837e6caf..693b55ab8b 100644 --- a/lib/bind9/check.c +++ b/lib/bind9/check.c @@ -72,6 +72,9 @@ static isc_result_t fileexist(const cfg_obj_t *obj, isc_symtab_t *symtab, bool writeable, isc_log_t *logctxlogc); +static isc_result_t +keydirexist(const cfg_obj_t *zcgf, const char *dir, const char *kaspnamestr, + isc_symtab_t *symtab, isc_log_t *logctx, isc_mem_t *mctx); static void freekey(char *key, unsigned int type, isc_symvalue_t value, void *userarg) { UNUSED(type); @@ -2377,9 +2380,9 @@ cleanup: static isc_result_t check_zoneconf(const cfg_obj_t *zconfig, const cfg_obj_t *voptions, const cfg_obj_t *config, isc_symtab_t *symtab, - isc_symtab_t *files, isc_symtab_t *inview, const char *viewname, - dns_rdataclass_t defclass, cfg_aclconfctx_t *actx, - isc_log_t *logctx, isc_mem_t *mctx) { + isc_symtab_t *files, isc_symtab_t *keydirs, isc_symtab_t *inview, + const char *viewname, dns_rdataclass_t defclass, + cfg_aclconfctx_t *actx, isc_log_t *logctx, isc_mem_t *mctx) { const char *znamestr; const char *typestr = NULL; const char *target = NULL; @@ -2404,6 +2407,8 @@ check_zoneconf(const cfg_obj_t *zconfig, const cfg_obj_t *voptions, bool has_dnssecpolicy = false; const void *clauses = NULL; const char *option = NULL; + const char *kaspname = NULL; + const char *dir = NULL; static const char *acls[] = { "allow-notify", "allow-transfer", @@ -2633,8 +2638,8 @@ check_zoneconf(const cfg_obj_t *zconfig, const cfg_obj_t *voptions, (void)cfg_map_get(zoptions, "dnssec-policy", &obj); if (obj != NULL) { const cfg_obj_t *kasps = NULL; - const char *kaspname = cfg_obj_asstring(obj); + kaspname = cfg_obj_asstring(obj); if (strcmp(kaspname, "default") == 0) { has_dnssecpolicy = true; } else if (strcmp(kaspname, "insecure") == 0) { @@ -3188,7 +3193,8 @@ check_zoneconf(const cfg_obj_t *zconfig, const cfg_obj_t *voptions, obj = NULL; tresult = cfg_map_get(zoptions, "key-directory", &obj); if (tresult == ISC_R_SUCCESS) { - const char *dir = cfg_obj_asstring(obj); + dir = cfg_obj_asstring(obj); + tresult = isc_file_isdirectory(dir); switch (tresult) { case ISC_R_SUCCESS: @@ -3210,6 +3216,25 @@ check_zoneconf(const cfg_obj_t *zconfig, const cfg_obj_t *voptions, } } + /* + * Make sure there is no other zone with the same + * key-directory and a different dnssec-policy. + */ + if (zname != NULL) { + char keydirbuf[DNS_NAME_FORMATSIZE + 128]; + char *tmp = keydirbuf; + size_t len = sizeof(keydirbuf); + dns_name_format(zname, keydirbuf, sizeof(keydirbuf)); + tmp += strlen(tmp); + len -= strlen(tmp); + (void)snprintf(tmp, len, "/%s", (dir == NULL) ? "(null)" : dir); + tresult = keydirexist(zconfig, (const char *)keydirbuf, + kaspname, keydirs, logctx, mctx); + if (tresult != ISC_R_SUCCESS) { + result = tresult; + } + } + /* * Check various options. */ @@ -3420,6 +3445,56 @@ fileexist(const cfg_obj_t *obj, isc_symtab_t *symtab, bool writeable, return (result); } +static isc_result_t +keydirexist(const cfg_obj_t *zcfg, const char *keydir, const char *kaspnamestr, + isc_symtab_t *symtab, isc_log_t *logctx, isc_mem_t *mctx) { + isc_result_t result; + isc_symvalue_t symvalue; + char *symkey; + + if (kaspnamestr == NULL || strcmp(kaspnamestr, "none") == 0) { + return (ISC_R_SUCCESS); + } + + result = isc_symtab_lookup(symtab, keydir, 0, &symvalue); + if (result == ISC_R_SUCCESS) { + const cfg_obj_t *kasp = NULL; + const cfg_obj_t *exist = symvalue.as_cpointer; + const char *file = cfg_obj_file(exist); + unsigned int line = cfg_obj_line(exist); + + /* + * Having the same key-directory for the same zone is fine + * iff the zone is using the same policy, or has no policy. + */ + (void)cfg_map_get(cfg_tuple_get(exist, "options"), + "dnssec-policy", &kasp); + if (kasp == NULL || + strcmp(cfg_obj_asstring(kasp), "none") == 0 || + strcmp(cfg_obj_asstring(kasp), kaspnamestr) == 0) + { + return (ISC_R_SUCCESS); + } + + cfg_obj_log(zcfg, logctx, ISC_LOG_ERROR, + "key-directory '%s' already in use by zone %s with " + "policy %s: %s:%u", + keydir, + cfg_obj_asstring(cfg_tuple_get(exist, "name")), + cfg_obj_asstring(kasp), file, line); + return (ISC_R_EXISTS); + } + + /* + * Add the new zone plus key-directory. + */ + symkey = isc_mem_strdup(mctx, keydir); + symvalue.as_cpointer = zcfg; + result = isc_symtab_define(symtab, symkey, 2, symvalue, + isc_symexists_reject); + return (result); +} + /* * Check key list for duplicates key names and that the key names * are valid domain names as these keys are used for TSIG. @@ -4379,8 +4454,8 @@ check_dnstap(const cfg_obj_t *voptions, const cfg_obj_t *config, static isc_result_t check_viewconf(const cfg_obj_t *config, const cfg_obj_t *voptions, const char *viewname, dns_rdataclass_t vclass, - isc_symtab_t *files, bool check_plugins, isc_symtab_t *inview, - isc_log_t *logctx, isc_mem_t *mctx) { + isc_symtab_t *files, isc_symtab_t *keydirs, bool check_plugins, + isc_symtab_t *inview, isc_log_t *logctx, isc_mem_t *mctx) { const cfg_obj_t *zones = NULL; const cfg_obj_t *view_tkeys = NULL, *global_tkeys = NULL; const cfg_obj_t *view_mkeys = NULL, *global_mkeys = NULL; @@ -4437,8 +4512,8 @@ check_viewconf(const cfg_obj_t *config, const cfg_obj_t *voptions, const cfg_obj_t *zone = cfg_listelt_value(element); tresult = check_zoneconf(zone, voptions, config, symtab, files, - inview, viewname, vclass, actx, logctx, - mctx); + keydirs, inview, viewname, vclass, + actx, logctx, mctx); if (tresult != ISC_R_SUCCESS) { result = ISC_R_FAILURE; } @@ -5035,6 +5110,7 @@ bind9_check_namedconf(const cfg_obj_t *config, bool check_plugins, isc_result_t tresult; isc_symtab_t *symtab = NULL; isc_symtab_t *files = NULL; + isc_symtab_t *keydirs = NULL; isc_symtab_t *inview = NULL; static const char *builtin[] = { "localhost", "localnets", "any", @@ -5086,6 +5162,12 @@ bind9_check_namedconf(const cfg_obj_t *config, bool check_plugins, goto cleanup; } + tresult = isc_symtab_create(mctx, 100, freekey, mctx, false, &keydirs); + if (tresult != ISC_R_SUCCESS) { + result = tresult; + goto cleanup; + } + tresult = isc_symtab_create(mctx, 100, freekey, mctx, true, &inview); if (tresult != ISC_R_SUCCESS) { result = tresult; @@ -5094,8 +5176,8 @@ bind9_check_namedconf(const cfg_obj_t *config, bool check_plugins, if (views == NULL) { tresult = check_viewconf(config, NULL, NULL, dns_rdataclass_in, - files, check_plugins, inview, logctx, - mctx); + files, keydirs, check_plugins, inview, + logctx, mctx); if (result == ISC_R_SUCCESS && tresult != ISC_R_SUCCESS) { result = ISC_R_FAILURE; } @@ -5186,8 +5268,8 @@ bind9_check_namedconf(const cfg_obj_t *config, bool check_plugins, } if (tresult == ISC_R_SUCCESS) { tresult = check_viewconf(config, voptions, key, vclass, - files, check_plugins, inview, - logctx, mctx); + files, keydirs, check_plugins, + inview, logctx, mctx); } if (tresult != ISC_R_SUCCESS) { result = ISC_R_FAILURE; @@ -5306,6 +5388,9 @@ cleanup: if (files != NULL) { isc_symtab_destroy(&files); } + if (keydirs != NULL) { + isc_symtab_destroy(&keydirs); + } return (result); }