chg: dev: Unify parsing of query-source and other X-source options

The query-source option currently allows the address to be specified in two ways, either as every other X-source option, or as an
"address" key-value pair.
This merge request extends the `parse_sockaddrsub` config parsing function so that it can parse the query-source option. It also removes the separate config parsing function for `query-source`.

Closes #4961

Merge branch '4961-query-source-parsing-refactor' into 'main'

See merge request isc-projects/bind9!9551

bin/tests/system/checkconf/bad-forwarders-dot.conf

@@ -0,0 +1,29 @@
+/*
+ * 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.
+ */
+
+tls test-tls {
+	protocols { TLSv1.2; };
+	ciphers "HIGH:!kRSA:!aNULL:!eNULL:!RC4:!3DES:!MD5:!EXP:!PSK:!SRP:!DSS:!SHA1:!SHA256:!SHA384";
+	prefer-server-ciphers yes;
+};
+
+tls another-tls {
+	protocols { TLSv1.2; };
+	session-tickets no;
+};
+
+zone "example" {
+	type forward;
+	forward only;
+	forwarders port 5300 tls test-tls { 10.53.0.1; 10.53.0.2 port 5301 tls another-tls tls third-tls tls "fourth-tls"; };
+};

bin/tests/system/checkconf/bad-query-source-address-v4-double.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.
+ */
+
+server 1.2.3.4 {
+	query-source 10.10.10.10 address 10.10.10.11;
+};

bin/tests/system/checkconf/bad-query-source-address-v6-double.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.
+ */
+
+server fd92:7065:b8e:ffff::1 {
+	query-source-v6 fd92:7065:b8e:ffff::2 address fd92:7065:b8e:ffff::3;
+};

bin/tests/system/checkconf/good-query-source-address-v4-1.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.
+ */
+
+server 1.2.3.4 {
+	query-source address 10.10.10.10;
+};

bin/tests/system/checkconf/good-query-source-address-v4-2.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.
+ */
+
+server 1.2.3.4 {
+	query-source 10.10.10.10;
+};

bin/tests/system/checkconf/good-query-source-address-v6-1.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.
+ */
+
+server fd92:7065:b8e:ffff::1 {
+	query-source-v6 address fd92:7065:b8e:ffff::2;
+};

bin/tests/system/checkconf/good-query-source-address-v6-2.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.
+ */
+
+server fd92:7065:b8e:ffff::1 {
+	query-source-v6 fd92:7065:b8e:ffff::1;
+};

lib/isccfg/include/isccfg/grammar.h

@@ -273,6 +273,7 @@ struct cfg_parser {
 #define CFG_ADDR_WILDOK	    0x00000008
 #define CFG_ADDR_PORTOK	    0x00000010
 #define CFG_ADDR_TLSOK	    0x00000020
+#define CFG_ADDR_TRAILINGOK 0x00000040
 #define CFG_ADDR_MASK	    (CFG_ADDR_V6OK | CFG_ADDR_V4OK)
 /*@}*/
 
@@ -378,6 +379,9 @@ cfg_lookingat_netaddr(cfg_parser_t *pctx, unsigned int flags);
 isc_result_t
 cfg_parse_rawport(cfg_parser_t *pctx, unsigned int flags, in_port_t *port);
 
+isc_result_t
+cfg_parse_sockaddr_generic(cfg_parser_t *pctx, cfg_type_t *klass,
+			   const cfg_type_t *type, cfg_obj_t **ret);
 isc_result_t
 cfg_parse_sockaddr(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret);
 

lib/isccfg/namedconf.c

@@ -3218,77 +3218,14 @@ static cfg_type_t cfg_type_optional_class = { "optional_class",
 
 static isc_result_t
 parse_querysource(cfg_parser_t *pctx, const cfg_type_t *type, cfg_obj_t **ret) {
-	isc_result_t result;
-	cfg_obj_t *obj = NULL;
-	isc_netaddr_t netaddr;
-	in_port_t port = 0;
-	unsigned int have_address = 0;
-	unsigned int have_port = 0;
-	unsigned int have_tls = 0;
-	const unsigned int *flagp = type->of;
+	REQUIRE(type != NULL);
 
-	if ((*flagp & CFG_ADDR_V4OK) != 0) {
-		isc_netaddr_any(&netaddr);
-	} else if ((*flagp & CFG_ADDR_V6OK) != 0) {
-		isc_netaddr_any6(&netaddr);
-	} else {
-		UNREACHABLE();
+	isc_result_t result = cfg_parse_sockaddr_generic(
+		pctx, &cfg_type_querysource, type, ret);
+	/* Preserve legacy query-source logging. */
+	if (result != ISC_R_SUCCESS) {
+		cfg_parser_error(pctx, CFG_LOG_NEAR, "invalid query source");
 	}
-
-	for (;;) {
-		CHECK(cfg_peektoken(pctx, 0));
-		if (pctx->token.type == isc_tokentype_string) {
-			if (strcasecmp(TOKEN_STRING(pctx), "address") == 0) {
-				/* read "address" */
-				CHECK(cfg_gettoken(pctx, 0));
-				CHECK(cfg_parse_rawaddr(pctx, *flagp,
-							&netaddr));
-				have_address++;
-			} else if (strcasecmp(TOKEN_STRING(pctx), "port") == 0)
-			{
-				/* Port has been removed */
-				++have_port;
-			} else if (strcasecmp(TOKEN_STRING(pctx), "tls") == 0) {
-				/* We do not expect TLS here, not parsing. */
-				++have_tls;
-			} else if (have_port == 0 && have_tls == 0 &&
-				   have_address == 0)
-			{
-				return (cfg_parse_sockaddr(pctx, type, ret));
-			} else {
-				cfg_parser_error(pctx, CFG_LOG_NEAR,
-						 "expected 'address' "
-						 "or 'port'");
-				return (ISC_R_UNEXPECTEDTOKEN);
-			}
-		} else {
-			break;
-		}
-	}
-
-	if (have_address != 1) {
-		cfg_parser_error(pctx, 0, "expected exactly one address");
-		return (ISC_R_UNEXPECTEDTOKEN);
-	}
-
-	if (have_tls > 0) {
-		cfg_parser_error(pctx, 0, "unexpected tls");
-		return (ISC_R_UNEXPECTEDTOKEN);
-	}
-
-	if (have_port > 0) {
-		cfg_parser_error(pctx, 0, "subconfig 'port' no longer exists");
-		return (ISC_R_UNEXPECTEDTOKEN);
-	}
-
-	CHECK(cfg_create_obj(pctx, &cfg_type_querysource, &obj));
-	isc_sockaddr_fromnetaddr(&obj->value.sockaddr, &netaddr, port);
-	*ret = obj;
-	return (ISC_R_SUCCESS);
-
-cleanup:
-	cfg_parser_error(pctx, CFG_LOG_NEAR, "invalid query source");
-	CLEANUP_OBJ(obj);
 	return (result);
 }
 
@@ -3318,14 +3255,19 @@ doc_querysource(cfg_printer_t *pctx, const cfg_type_t *type) {
 static unsigned int sockaddr4wild_flags = CFG_ADDR_WILDOK | CFG_ADDR_V4OK;
 static unsigned int sockaddr6wild_flags = CFG_ADDR_WILDOK | CFG_ADDR_V6OK;
 
+static unsigned int querysource4wild_flags = CFG_ADDR_WILDOK | CFG_ADDR_V4OK |
+					     CFG_ADDR_TRAILINGOK;
+static unsigned int querysource6wild_flags = CFG_ADDR_WILDOK | CFG_ADDR_V6OK |
+					     CFG_ADDR_TRAILINGOK;
+
 static cfg_type_t cfg_type_querysource4 = {
-	"querysource4", parse_querysource,   NULL, doc_querysource,
-	NULL,		&sockaddr4wild_flags
+	"querysource4", parse_querysource,	NULL, doc_querysource,
+	NULL,		&querysource4wild_flags
 };
 
 static cfg_type_t cfg_type_querysource6 = {
-	"querysource6", parse_querysource,   NULL, doc_querysource,
-	NULL,		&sockaddr6wild_flags
+	"querysource6", parse_querysource,	NULL, doc_querysource,
+	NULL,		&querysource6wild_flags
 };
 
 static cfg_type_t cfg_type_querysource = { "querysource",     NULL,

lib/isccfg/parser.c

@@ -75,6 +75,7 @@
 #define MAP_SYM 1 /* Unique type for isc_symtab */
 
 #define TOKEN_STRING(pctx) (pctx->token.value.as_textregion.base)
+#define TOKEN_REGION(pctx) (pctx->token.value.as_textregion)
 
 /* Check a return value. */
 #define CHECK(op)                            \
@@ -117,9 +118,6 @@ create_string(cfg_parser_t *pctx, const char *contents, const cfg_type_t *type,
 static void
 free_string(cfg_parser_t *pctx, cfg_obj_t *obj);
 
-static void
-copy_string(cfg_parser_t *pctx, const cfg_obj_t *obj, isc_textregion_t *dst);
-
 static void
 free_sockaddrtls(cfg_parser_t *pctx, cfg_obj_t *obj);
 
@@ -1552,18 +1550,6 @@ free_string(cfg_parser_t *pctx, cfg_obj_t *obj) {
 		    obj->value.string.length + 1);
 }
 
-static void
-copy_string(cfg_parser_t *pctx, const cfg_obj_t *obj, isc_textregion_t *dst) {
-	if (dst->base != NULL) {
-		INSIST(dst->length != 0);
-		isc_mem_put(pctx->mctx, dst->base, dst->length + 1);
-	}
-	dst->length = obj->value.string.length;
-	dst->base = isc_mem_get(pctx->mctx, dst->length + 1);
-	memmove(dst->base, obj->value.string.base, dst->length);
-	dst->base[dst->length] = '\0';
-}
-
 static void
 free_sockaddrtls(cfg_parser_t *pctx, cfg_obj_t *obj) {
 	if (obj->value.sockaddrtls.tls.base != NULL) {
@@ -3263,6 +3249,16 @@ cfg_type_t cfg_type_netprefix = { "netprefix",	      cfg_parse_netprefix,
 				  print_netprefix,    cfg_doc_terminal,
 				  &cfg_rep_netprefix, NULL };
 
+static void
+copy_textregion(isc_mem_t *mctx, isc_textregion_t *dest, isc_textregion_t src) {
+	size_t dest_mem_length = (dest->base != NULL) ? dest->length + 1 : 0;
+	dest->base = isc_mem_creget(mctx, dest->base, dest_mem_length,
+				    src.length + 1, sizeof(char));
+	dest->length = src.length;
+	memmove(dest->base, src.base, src.length);
+	dest->base[dest->length] = '\0';
+}
+
 static isc_result_t
 parse_sockaddrsub(cfg_parser_t *pctx, const cfg_type_t *type, int flags,
 		  cfg_obj_t **ret) {
@@ -3270,31 +3266,45 @@ parse_sockaddrsub(cfg_parser_t *pctx, const cfg_type_t *type, int flags,
 	isc_netaddr_t netaddr;
 	in_port_t port = 0;
 	cfg_obj_t *obj = NULL;
+	int have_address = 0;
 	int have_port = 0;
 	int have_tls = 0;
 	int is_port_ok = (flags & CFG_ADDR_PORTOK) != 0;
 	int is_tls_ok = (flags & CFG_ADDR_TLSOK) != 0;
+	int is_address_ok = (flags & CFG_ADDR_TRAILINGOK) != 0;
 
-	CHECK(cfg_create_obj(pctx, type, &obj));
-	CHECK(cfg_parse_rawaddr(pctx, flags, &netaddr));
+	isc_textregion_t tls = { .base = NULL, .length = 0 };
+
+	CHECK(cfg_peektoken(pctx, 0));
+	if (cfg_lookingat_netaddr(pctx, flags)) {
+		CHECK(cfg_parse_rawaddr(pctx, flags, &netaddr));
+		++have_address;
+	}
 
 	for (;;) {
 		CHECK(cfg_peektoken(pctx, 0));
 		if (pctx->token.type == isc_tokentype_string) {
-			if (strcasecmp(TOKEN_STRING(pctx), "port") == 0) {
+			if (is_address_ok &&
+			    strcasecmp(TOKEN_STRING(pctx), "address") == 0)
+			{
+				/* read "address" */
+				CHECK(cfg_gettoken(pctx, 0));
+				CHECK(cfg_parse_rawaddr(pctx, flags, &netaddr));
+				++have_address;
+			} else if (strcasecmp(TOKEN_STRING(pctx), "port") == 0)
+			{
 				CHECK(cfg_gettoken(pctx, 0)); /* read "port" */
 				CHECK(cfg_parse_rawport(pctx, flags, &port));
 				++have_port;
 			} else if (is_tls_ok &&
 				   strcasecmp(TOKEN_STRING(pctx), "tls") == 0)
 			{
-				cfg_obj_t *tls = NULL;
-
 				CHECK(cfg_gettoken(pctx, 0)); /* read "tls" */
-				CHECK(cfg_parse_astring(pctx, NULL, &tls));
-				copy_string(pctx, tls,
-					    &obj->value.sockaddrtls.tls);
-				CLEANUP_OBJ(tls);
+				CHECK(cfg_getstringtoken(pctx));
+
+				isc_textregion_t tok = TOKEN_REGION(pctx);
+				copy_textregion(pctx->mctx, &tls, tok);
+
 				++have_tls;
 			} else {
 				break;
@@ -3304,6 +3314,12 @@ parse_sockaddrsub(cfg_parser_t *pctx, const cfg_type_t *type, int flags,
 		}
 	}
 
+	if (have_address != 1) {
+		cfg_parser_error(pctx, 0, "expected exactly one address");
+		result = ISC_R_UNEXPECTEDTOKEN;
+		goto cleanup;
+	}
+
 	if (!is_port_ok && have_port > 0) {
 		cfg_parser_error(pctx, 0, "subconfig 'port' no longer exists");
 		result = ISC_R_UNEXPECTEDTOKEN;
@@ -3314,22 +3330,30 @@ parse_sockaddrsub(cfg_parser_t *pctx, const cfg_type_t *type, int flags,
 		result = ISC_R_UNEXPECTEDTOKEN;
 		goto cleanup;
 	}
+
 	if (have_tls > 1) {
 		cfg_parser_error(pctx, 0, "expected at most one tls");
 		result = ISC_R_UNEXPECTEDTOKEN;
 		goto cleanup;
 	}
 
+	CHECK(cfg_create_obj(pctx, type, &obj));
+	if (have_tls == 1) {
+		obj->value.sockaddrtls.tls = tls;
+	}
 	isc_sockaddr_fromnetaddr(&obj->value.sockaddr, &netaddr, port);
 	*ret = obj;
 	return (ISC_R_SUCCESS);
 
 cleanup:
+	if (tls.base != NULL) {
+		isc_mem_put(pctx->mctx, tls.base, tls.length + 1);
+	}
 	CLEANUP_OBJ(obj);
 	return (result);
 }
 
-static isc_result_t
+isc_result_t
 cfg_parse_sockaddr_generic(cfg_parser_t *pctx, cfg_type_t *klass,
 			   const cfg_type_t *type, cfg_obj_t **ret) {
 	const unsigned int *flagp;