From: Matthijs Mekking <matthijs@isc.org>
Date: Tue, 4 May 2021 13:35:39 +0000 (+0200)
Subject: Check key-directory duplicates for kasp zones
X-Git-Tag: v9.17.14~46^2~2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=494e8b2cbd362af682e9699a9c4712cf79e27de8;p=thirdparty%2Fbind9.git

Check key-directory duplicates for kasp zones

Don't allow the same zone with different dnssec-policies in separate
views have the same key-directory.

Track zones plus key-directory in a symtab and if there is a match,
check the offending zone's dnssec-policy name. If the name is "none"
(there is no kasp for the offending zone), or if the name is the same
(the zone shares keys), it is fine, otherwise it is an error (zones
in views using different policies cannot share the same key-directory).
---

diff --git a/lib/bind9/check.c b/lib/bind9/check.c
index a7837e6caf1..693b55ab8b9 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);
 }