From 91dca0f8da192e8b3439e80717bad3bfc139118f Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 19 Mar 2019 10:14:44 -0700 Subject: [PATCH 1/2] don't fail when allow-update{,-forwarding} is used globally --- bin/named/server.c | 5 ++- ...=> good-allow-update-forwarding-view.conf} | 2 +- ...conf => good-allow-update-forwarding.conf} | 0 ...-view.conf => good-allow-update-view.conf} | 2 +- ...low-update.conf => good-allow-update.conf} | 0 doc/arm/Bv9ARM-book.xml | 24 ++++++++--- lib/bind9/check.c | 42 ------------------- 7 files changed, 23 insertions(+), 52 deletions(-) rename bin/tests/system/checkconf/{bad-allow-update-forwarding-view.conf => good-allow-update-forwarding-view.conf} (97%) rename bin/tests/system/checkconf/{bad-allow-update-forwarding.conf => good-allow-update-forwarding.conf} (100%) rename bin/tests/system/checkconf/{bad-allow-update-view.conf => good-allow-update-view.conf} (97%) rename bin/tests/system/checkconf/{bad-allow-update.conf => good-allow-update.conf} (100%) diff --git a/bin/named/server.c b/bin/named/server.c index 461cf1d9cd..5653358aba 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -5074,8 +5074,9 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, /* * Configure default allow-update and allow-update-forwarding ACLs, - * so they can be inherited by zones. (Note these cannot be set at - * options/view level.) + * so they can be inherited by zones. (XXX: These are not + * read from the options/view level here. However, they may be + * read from there in zoneconf.c:configure_zone_acl() later.) */ if (view->updateacl == NULL) { CHECK(configure_view_acl(NULL, NULL, named_g_config, diff --git a/bin/tests/system/checkconf/bad-allow-update-forwarding-view.conf b/bin/tests/system/checkconf/good-allow-update-forwarding-view.conf similarity index 97% rename from bin/tests/system/checkconf/bad-allow-update-forwarding-view.conf rename to bin/tests/system/checkconf/good-allow-update-forwarding-view.conf index 47f34950ad..0c2aeb8ddb 100644 --- a/bin/tests/system/checkconf/bad-allow-update-forwarding-view.conf +++ b/bin/tests/system/checkconf/good-allow-update-forwarding-view.conf @@ -9,6 +9,6 @@ * information regarding copyright ownership. */ -view { +view one { allow-update-forwarding { any; }; }; diff --git a/bin/tests/system/checkconf/bad-allow-update-forwarding.conf b/bin/tests/system/checkconf/good-allow-update-forwarding.conf similarity index 100% rename from bin/tests/system/checkconf/bad-allow-update-forwarding.conf rename to bin/tests/system/checkconf/good-allow-update-forwarding.conf diff --git a/bin/tests/system/checkconf/bad-allow-update-view.conf b/bin/tests/system/checkconf/good-allow-update-view.conf similarity index 97% rename from bin/tests/system/checkconf/bad-allow-update-view.conf rename to bin/tests/system/checkconf/good-allow-update-view.conf index 22c4361e1e..a893b9a2c5 100644 --- a/bin/tests/system/checkconf/bad-allow-update-view.conf +++ b/bin/tests/system/checkconf/good-allow-update-view.conf @@ -9,6 +9,6 @@ * information regarding copyright ownership. */ -view { +view one { allow-update { any; }; }; diff --git a/bin/tests/system/checkconf/bad-allow-update.conf b/bin/tests/system/checkconf/good-allow-update.conf similarity index 100% rename from bin/tests/system/checkconf/bad-allow-update.conf rename to bin/tests/system/checkconf/good-allow-update.conf diff --git a/doc/arm/Bv9ARM-book.xml b/doc/arm/Bv9ARM-book.xml index a97a4d3acf..b313840324 100644 --- a/doc/arm/Bv9ARM-book.xml +++ b/doc/arm/Bv9ARM-book.xml @@ -7025,15 +7025,21 @@ options { When set in the zone statement for a master zone, specifies which hosts are allowed to submit Dynamic DNS updates to that zone. The default - is to deny updates from all hosts. This can only - be set at the zone level, not in - options or view. + is to deny updates from all hosts. Note that allowing updates based on the requestor's IP address is insecure; see for details. + + In general this option should only be set at the + zone level. While a default + value can be set at the options or + view level and inherited by zones, + this could lead to some zones unintentionally allowing + updates. + @@ -7046,9 +7052,7 @@ options { submit Dynamic DNS updates and have them be forwarded to the master. The default is { none; }, which means that no - update forwarding will be performed. This can only be - set at the zone level, not in - options or view. + update forwarding will be performed. To enable update forwarding, specify @@ -7066,6 +7070,14 @@ options { on insecure IP-address-based access control; see for more details. + + In general this option should only be set at the + zone level. While a default + value can be set at the options or + view level and inherited by zones, + this can lead to some zones unintentionally forwarding + updates. + diff --git a/lib/bind9/check.c b/lib/bind9/check.c index 813cb7930b..1d7a3d4a3b 100644 --- a/lib/bind9/check.c +++ b/lib/bind9/check.c @@ -482,43 +482,6 @@ check_viewacls(cfg_aclconfctx_t *actx, const cfg_obj_t *voptions, return (result); } -static isc_result_t -check_non_viewacls(const cfg_obj_t *voptions, const cfg_obj_t *config, - isc_log_t *logctx) -{ - const cfg_obj_t *aclobj = NULL; - const cfg_obj_t *options; - const char *where = NULL; - int i; - - static const char *acls[] = { - "allow-update", "allow-update-forwarding", NULL - }; - - for (i = 0; acls[i] != NULL; i++) { - if (voptions != NULL && aclobj == NULL) { - cfg_map_get(voptions, acls[i], &aclobj); - where = "view"; - } - if (config != NULL && aclobj == NULL) { - options = NULL; - cfg_map_get(config, "options", &options); - if (options != NULL) { - cfg_map_get(options, acls[i], &aclobj); - where = "options"; - } - } - if (aclobj != NULL) { - cfg_obj_log(aclobj, logctx, ISC_LOG_ERROR, - "'%s' can only be set per-zone, " - "not in '%s'", acls[i], where); - return (ISC_R_FAILURE); - } - } - - return (ISC_R_SUCCESS); -} - static const unsigned char zeros[16]; static isc_result_t @@ -3702,11 +3665,6 @@ check_viewconf(const cfg_obj_t *config, const cfg_obj_t *voptions, if (tresult != ISC_R_SUCCESS) result = tresult; - tresult = check_non_viewacls(voptions, config, logctx); - if (tresult != ISC_R_SUCCESS) { - result = tresult; - } - tresult = check_recursionacls(actx, voptions, viewname, config, logctx, mctx); if (tresult != ISC_R_SUCCESS) From 55a7961cf370a5140e92e345aba468f87d681871 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 19 Mar 2019 15:18:16 -0700 Subject: [PATCH 2/2] CHANGES, release notes --- CHANGES | 4 ++++ doc/arm/notes.xml | 9 ++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/CHANGES b/CHANGES index 50e7f621cb..d1611d7a92 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5195. [bug] "allow-update" and "allow-update-forwarding" were + treated as configuration errors if used at the + options or view level. [GL #913] + 5194. [bug] Enforce non empty ZOMEMD hash. [GL #899] 5193. [bug] EID and NIMLOC failed to do multi-line output diff --git a/doc/arm/notes.xml b/doc/arm/notes.xml index 9b02ca64ac..4ddca00572 100644 --- a/doc/arm/notes.xml +++ b/doc/arm/notes.xml @@ -132,9 +132,12 @@ - The presence of certain record types in an otherwise empty - node of a zone could trigger an assertion failure while processing - a query of type ANY. [GL #901] + The allow-update and + allow-update-forwarding options were + inadvertently treated as configuration errors when used at the + options or view level. + This has now been corrected. + [GL #913]