From: Matthijs Mekking <matthijs@isc.org>
Date: Tue, 13 Oct 2020 12:48:04 +0000 (+0200)
Subject: Fix a reconfig bug wrt inline-signing
X-Git-Tag: v9.17.8~27^2~11
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ba8128ea0038298bd379662ea6f438c650f68340;p=thirdparty%2Fbind9.git

Fix a reconfig bug wrt inline-signing

When doing 'rndc reconfig', named may complain about a zone not being
reusable because it has a raw version of the zone, and the new
configuration has not set 'inline-signing'. However, 'inline-signing'
may be implicitly true if a 'dnssec-policy' is used for the zone, and
the zone is not dynamic.

Improve the check in 'named_zone_reusable'.  Create a new function for
checking 'inline-signing' configuration that matches existing code in
'bin/named/server.c'.
---

diff --git a/bin/named/include/named/zoneconf.h b/bin/named/include/named/zoneconf.h
index a6e372056df..962d5a229af 100644
--- a/bin/named/include/named/zoneconf.h
+++ b/bin/named/include/named/zoneconf.h
@@ -43,7 +43,8 @@ named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig,
  */
 
 bool
-named_zone_reusable(dns_zone_t *zone, const cfg_obj_t *zconfig);
+named_zone_reusable(dns_zone_t *zone, const cfg_obj_t *zconfig,
+		    const cfg_obj_t *vconfig, const cfg_obj_t *config);
 /*%<
  * If 'zone' can be safely reconfigured according to the configuration
  * data in 'zconfig', return true.  If the configuration data is so
@@ -51,6 +52,14 @@ named_zone_reusable(dns_zone_t *zone, const cfg_obj_t *zconfig);
  * and recreated, return false.
  */
 
+bool
+named_zone_inlinesigning(dns_zone_t *zone, const cfg_obj_t *zconfig,
+			 const cfg_obj_t *vconfig, const cfg_obj_t *config);
+/*%<
+ * Determine if zone uses inline-signing. This is true if inline-signing
+ * is set to yes, or if there is a dnssec-policy on a non-dynamic zone.
+ */
+
 isc_result_t
 named_zone_configure_writeable_dlz(dns_dlzdb_t *dlzdatabase, dns_zone_t *zone,
 				   dns_rdataclass_t rdclass, dns_name_t *name);
diff --git a/bin/named/server.c b/bin/named/server.c
index 9ee19aa733c..6ffc8da3e2d 100644
--- a/bin/named/server.c
+++ b/bin/named/server.c
@@ -6082,14 +6082,12 @@ configure_zone(const cfg_obj_t *config, const cfg_obj_t *zconfig,
 	dns_zone_t *raw = NULL;	  /* New or reused raw zone */
 	dns_zone_t *dupzone = NULL;
 	const cfg_obj_t *options = NULL;
-	const cfg_obj_t *voptions = NULL;
 	const cfg_obj_t *zoptions = NULL;
 	const cfg_obj_t *typeobj = NULL;
 	const cfg_obj_t *forwarders = NULL;
 	const cfg_obj_t *forwardtype = NULL;
 	const cfg_obj_t *ixfrfromdiffs = NULL;
 	const cfg_obj_t *only = NULL;
-	const cfg_obj_t *signing = NULL;
 	const cfg_obj_t *viewobj = NULL;
 	isc_result_t result;
 	isc_result_t tresult;
@@ -6108,9 +6106,6 @@ configure_zone(const cfg_obj_t *config, const cfg_obj_t *zconfig,
 	(void)cfg_map_get(config, "options", &options);
 
 	zoptions = cfg_tuple_get(zconfig, "options");
-	if (vconfig != NULL) {
-		voptions = cfg_tuple_get(vconfig, "options");
-	}
 
 	/*
 	 * Get the zone origin as a dns_name_t.
@@ -6372,7 +6367,8 @@ configure_zone(const cfg_obj_t *config, const cfg_obj_t *zconfig,
 		goto cleanup;
 	}
 
-	if (zone != NULL && !named_zone_reusable(zone, zconfig)) {
+	if (zone != NULL &&
+	    !named_zone_reusable(zone, zconfig, vconfig, config)) {
 		dns_zone_detach(&zone);
 	}
 
@@ -6451,62 +6447,10 @@ configure_zone(const cfg_obj_t *config, const cfg_obj_t *zconfig,
 			      strcasecmp(ztypestr, "secondary") == 0 ||
 			      strcasecmp(ztypestr, "slave") == 0));
 
-	signing = NULL;
-	inline_signing = (zone_maybe_inline &&
-			  ((cfg_map_get(zoptions, "inline-signing", &signing) ==
-				    ISC_R_SUCCESS &&
-			    cfg_obj_asboolean(signing))));
-
-	/*
-	 * If inline-signing is not set, perhaps implictly through a
-	 * dnssec-policy.  Since automated DNSSEC maintenance requires
-	 * a dynamic zone, or inline-siging to be enabled, check if
-	 * the zone with dnssec-policy allows updates.  If not, enable
-	 * inline-signing.
-	 */
-	signing = NULL;
-	if (zone_maybe_inline && !inline_signing &&
-	    cfg_map_get(zoptions, "dnssec-policy", &signing) == ISC_R_SUCCESS &&
-	    signing != NULL && strcmp(cfg_obj_asstring(signing), "none") != 0)
-	{
-		isc_result_t res;
-		bool zone_is_dynamic = false;
-		const cfg_obj_t *au = NULL;
-		const cfg_obj_t *up = NULL;
-
-		if (cfg_map_get(zoptions, "update-policy", &up) ==
-		    ISC_R_SUCCESS) {
-			zone_is_dynamic = true;
-		} else {
-			res = cfg_map_get(zoptions, "allow-update", &au);
-			if (res != ISC_R_SUCCESS && voptions != NULL) {
-				res = cfg_map_get(voptions, "allow-update",
-						  &au);
-			}
-			if (res != ISC_R_SUCCESS && options != NULL) {
-				res = cfg_map_get(options, "allow-update", &au);
-			}
-			if (res == ISC_R_SUCCESS) {
-				dns_acl_t *acl = NULL;
-				cfg_aclconfctx_t *actx = NULL;
-				res = cfg_acl_fromconfig(au, config,
-							 named_g_lctx, actx,
-							 mctx, 0, &acl);
-				if (res == ISC_R_SUCCESS && acl != NULL &&
-				    !dns_acl_isnone(acl)) {
-					zone_is_dynamic = true;
-				}
-				if (acl != NULL) {
-					dns_acl_detach(&acl);
-				}
-			}
-		}
-
-		if (!zone_is_dynamic) {
-			inline_signing = true;
-		}
+	if (zone_maybe_inline) {
+		inline_signing = named_zone_inlinesigning(zone, zconfig,
+							  vconfig, config);
 	}
-
 	if (inline_signing) {
 		dns_zone_getraw(zone, &raw);
 		if (raw == NULL) {
diff --git a/bin/named/zoneconf.c b/bin/named/zoneconf.c
index 6c8ffceade6..1f1dbaabed5 100644
--- a/bin/named/zoneconf.c
+++ b/bin/named/zoneconf.c
@@ -2065,13 +2065,14 @@ named_zone_configure_writeable_dlz(dns_dlzdb_t *dlzdatabase, dns_zone_t *zone,
 }
 
 bool
-named_zone_reusable(dns_zone_t *zone, const cfg_obj_t *zconfig) {
+named_zone_reusable(dns_zone_t *zone, const cfg_obj_t *zconfig,
+		    const cfg_obj_t *vconfig, const cfg_obj_t *config) {
 	const cfg_obj_t *zoptions = NULL;
 	const cfg_obj_t *obj = NULL;
 	const char *cfilename;
 	const char *zfilename;
 	dns_zone_t *raw = NULL;
-	bool has_raw;
+	bool has_raw, inline_signing;
 	dns_zonetype_t ztype;
 
 	zoptions = cfg_tuple_get(zconfig, "options");
@@ -2099,13 +2100,13 @@ named_zone_reusable(dns_zone_t *zone, const cfg_obj_t *zconfig) {
 		has_raw = false;
 	}
 
-	obj = NULL;
-	(void)cfg_map_get(zoptions, "inline-signing", &obj);
-	if ((obj == NULL || !cfg_obj_asboolean(obj)) && has_raw) {
+	inline_signing = named_zone_inlinesigning(zone, zconfig, vconfig,
+						  config);
+	if (!inline_signing && has_raw) {
 		dns_zone_log(zone, ISC_LOG_DEBUG(1),
 			     "not reusable: old zone was inline-signing");
 		return (false);
-	} else if ((obj != NULL && cfg_obj_asboolean(obj)) && !has_raw) {
+	} else if (inline_signing && !has_raw) {
 		dns_zone_log(zone, ISC_LOG_DEBUG(1),
 			     "not reusable: old zone was not inline-signing");
 		return (false);
@@ -2135,3 +2136,82 @@ named_zone_reusable(dns_zone_t *zone, const cfg_obj_t *zconfig) {
 
 	return (true);
 }
+
+bool
+named_zone_inlinesigning(dns_zone_t *zone, const cfg_obj_t *zconfig,
+			 const cfg_obj_t *vconfig, const cfg_obj_t *config) {
+	const cfg_obj_t *zoptions = NULL;
+	const cfg_obj_t *voptions = NULL;
+	const cfg_obj_t *options = NULL;
+	const cfg_obj_t *signing = NULL;
+	bool inline_signing;
+
+	(void)cfg_map_get(config, "options", &options);
+
+	zoptions = cfg_tuple_get(zconfig, "options");
+	if (vconfig != NULL) {
+		voptions = cfg_tuple_get(vconfig, "options");
+	}
+
+	inline_signing = (cfg_map_get(zoptions, "inline-signing", &signing) ==
+				  ISC_R_SUCCESS &&
+			  cfg_obj_asboolean(signing));
+
+	/*
+	 * If inline-signing is not set, perhaps implictly through a
+	 * dnssec-policy.  Since automated DNSSEC maintenance requires
+	 * a dynamic zone, or inline-siging to be enabled, check if
+	 * the zone with dnssec-policy allows updates.  If not, enable
+	 * inline-signing.
+	 */
+	signing = NULL;
+	if (!inline_signing &&
+	    cfg_map_get(zoptions, "dnssec-policy", &signing) == ISC_R_SUCCESS &&
+	    signing != NULL && strcmp(cfg_obj_asstring(signing), "none") != 0)
+	{
+		{
+			isc_result_t res;
+			bool zone_is_dynamic = false;
+			const cfg_obj_t *au = NULL;
+			const cfg_obj_t *up = NULL;
+
+			if (cfg_map_get(zoptions, "update-policy", &up) ==
+			    ISC_R_SUCCESS) {
+				zone_is_dynamic = true;
+			} else {
+				res = cfg_map_get(zoptions, "allow-update",
+						  &au);
+				if (res != ISC_R_SUCCESS && voptions != NULL) {
+					res = cfg_map_get(voptions,
+							  "allow-update", &au);
+				}
+				if (res != ISC_R_SUCCESS && options != NULL) {
+					res = cfg_map_get(options,
+							  "allow-update", &au);
+				}
+				if (res == ISC_R_SUCCESS) {
+					dns_acl_t *acl = NULL;
+					cfg_aclconfctx_t *actx = NULL;
+					res = cfg_acl_fromconfig(
+						au, config, named_g_lctx, actx,
+						dns_zone_getmctx(zone), 0,
+						&acl);
+					if (res == ISC_R_SUCCESS &&
+					    acl != NULL && !dns_acl_isnone(acl))
+					{
+						zone_is_dynamic = true;
+					}
+					if (acl != NULL) {
+						dns_acl_detach(&acl);
+					}
+				}
+			}
+
+			if (!zone_is_dynamic) {
+				inline_signing = true;
+			}
+		}
+	}
+
+	return (inline_signing);
+}