From 14c8d7dfb7fa7143936254c02633a4723c8860b0 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Mon, 13 Sep 2021 17:55:34 -0700 Subject: [PATCH 1/3] check port in *-source and *-source-v6 options in named.conf - when transfer-source(-v6), query-source(-v6), notify-source(-v6) or parental-source(-v6) are specified with a port number, issue a warning. - when the port specified is the same as the DNS listener port (i.e., 53, or whatever was specified as "port" in "options"), issue a fatal error. - check that "port" is in range. (previously this was only checked by named, not by named-checkconf.) - added checkconf tests. - incidental fix: removed dead code in check.c:bind9_check_namedconf(). (note: if the DNS port is specified on the command line with "named -p", that is not conveyed to libbind9, so these checks will not take it into account.) --- .../checkconf/bad-notify-source-v6.conf | 20 ++ .../system/checkconf/bad-notify-source.conf | 20 ++ .../checkconf/bad-parental-source-v6.conf | 20 ++ .../system/checkconf/bad-parental-source.conf | 20 ++ bin/tests/system/checkconf/bad-port.conf | 14 ++ .../checkconf/bad-transfer-source-v6.conf | 20 ++ .../system/checkconf/bad-transfer-source.conf | 20 ++ .../checkconf/good-notify-source-v6.conf | 20 ++ .../system/checkconf/good-notify-source.conf | 20 ++ .../checkconf/good-parental-source-v6.conf | 20 ++ .../checkconf/good-parental-source.conf | 20 ++ .../checkconf/good-transfer-source-v6.conf | 20 ++ .../checkconf/good-transfer-source.conf | 20 ++ bin/tests/system/checkconf/tests.sh | 13 ++ .../system/checkconf/warn-notify-source.conf | 20 ++ .../checkconf/warn-parental-source.conf | 20 ++ .../checkconf/warn-transfer-source.conf | 20 ++ lib/bind9/check.c | 220 +++++++++++++----- 18 files changed, 489 insertions(+), 58 deletions(-) create mode 100644 bin/tests/system/checkconf/bad-notify-source-v6.conf create mode 100644 bin/tests/system/checkconf/bad-notify-source.conf create mode 100644 bin/tests/system/checkconf/bad-parental-source-v6.conf create mode 100644 bin/tests/system/checkconf/bad-parental-source.conf create mode 100644 bin/tests/system/checkconf/bad-port.conf create mode 100644 bin/tests/system/checkconf/bad-transfer-source-v6.conf create mode 100644 bin/tests/system/checkconf/bad-transfer-source.conf create mode 100644 bin/tests/system/checkconf/good-notify-source-v6.conf create mode 100644 bin/tests/system/checkconf/good-notify-source.conf create mode 100644 bin/tests/system/checkconf/good-parental-source-v6.conf create mode 100644 bin/tests/system/checkconf/good-parental-source.conf create mode 100644 bin/tests/system/checkconf/good-transfer-source-v6.conf create mode 100644 bin/tests/system/checkconf/good-transfer-source.conf create mode 100644 bin/tests/system/checkconf/warn-notify-source.conf create mode 100644 bin/tests/system/checkconf/warn-parental-source.conf create mode 100644 bin/tests/system/checkconf/warn-transfer-source.conf diff --git a/bin/tests/system/checkconf/bad-notify-source-v6.conf b/bin/tests/system/checkconf/bad-notify-source-v6.conf new file mode 100644 index 0000000000..d25eaf4cd8 --- /dev/null +++ b/bin/tests/system/checkconf/bad-notify-source-v6.conf @@ -0,0 +1,20 @@ +/* + * 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. + */ + +options { + port 5300; +}; + +zone example { + type secondary; + primaries { 1.2.3.4; }; + notify-source-v6 fd92:7065:b8e:ffff::1 port 5300; +}; diff --git a/bin/tests/system/checkconf/bad-notify-source.conf b/bin/tests/system/checkconf/bad-notify-source.conf new file mode 100644 index 0000000000..16d9eece95 --- /dev/null +++ b/bin/tests/system/checkconf/bad-notify-source.conf @@ -0,0 +1,20 @@ +/* + * 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. + */ + +options { + port 5300; +}; + +zone example { + type secondary; + primaries { 1.2.3.4; }; + notify-source 10.53.0.1 port 5300; +}; diff --git a/bin/tests/system/checkconf/bad-parental-source-v6.conf b/bin/tests/system/checkconf/bad-parental-source-v6.conf new file mode 100644 index 0000000000..41128384aa --- /dev/null +++ b/bin/tests/system/checkconf/bad-parental-source-v6.conf @@ -0,0 +1,20 @@ +/* + * 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. + */ + +options { + port 5300; +}; + +zone example { + type secondary; + primaries { 1.2.3.4; }; + parental-source-v6 fd92:7065:b8e:ffff::1 port 5300; +}; diff --git a/bin/tests/system/checkconf/bad-parental-source.conf b/bin/tests/system/checkconf/bad-parental-source.conf new file mode 100644 index 0000000000..4d1b27af70 --- /dev/null +++ b/bin/tests/system/checkconf/bad-parental-source.conf @@ -0,0 +1,20 @@ +/* + * 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. + */ + +options { + port 5300; +}; + +zone example { + type secondary; + primaries { 1.2.3.4; }; + parental-source 10.53.0.1 port 5300; +}; diff --git a/bin/tests/system/checkconf/bad-port.conf b/bin/tests/system/checkconf/bad-port.conf new file mode 100644 index 0000000000..76677df519 --- /dev/null +++ b/bin/tests/system/checkconf/bad-port.conf @@ -0,0 +1,14 @@ +/* + * 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. + */ + +options { + port 99999; +}; diff --git a/bin/tests/system/checkconf/bad-transfer-source-v6.conf b/bin/tests/system/checkconf/bad-transfer-source-v6.conf new file mode 100644 index 0000000000..7464758fa9 --- /dev/null +++ b/bin/tests/system/checkconf/bad-transfer-source-v6.conf @@ -0,0 +1,20 @@ +/* + * 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. + */ + +options { + port 5300; +}; + +zone example { + type secondary; + primaries { 1.2.3.4; }; + transfer-source-v6 fd92:7065:b8e:ffff::1 port 5300; +}; diff --git a/bin/tests/system/checkconf/bad-transfer-source.conf b/bin/tests/system/checkconf/bad-transfer-source.conf new file mode 100644 index 0000000000..54e348f0b0 --- /dev/null +++ b/bin/tests/system/checkconf/bad-transfer-source.conf @@ -0,0 +1,20 @@ +/* + * 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. + */ + +options { + port 5300; +}; + +zone example { + type secondary; + primaries { 1.2.3.4; }; + transfer-source 10.53.0.1 port 5300; +}; diff --git a/bin/tests/system/checkconf/good-notify-source-v6.conf b/bin/tests/system/checkconf/good-notify-source-v6.conf new file mode 100644 index 0000000000..2f9558edd5 --- /dev/null +++ b/bin/tests/system/checkconf/good-notify-source-v6.conf @@ -0,0 +1,20 @@ +/* + * 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. + */ + +options { + port 5300; +}; + +zone example { + type secondary; + primaries { 1.2.3.4; }; + notify-source-v6 fd92:7065:b8e:ffff::1; +}; diff --git a/bin/tests/system/checkconf/good-notify-source.conf b/bin/tests/system/checkconf/good-notify-source.conf new file mode 100644 index 0000000000..36c67c0451 --- /dev/null +++ b/bin/tests/system/checkconf/good-notify-source.conf @@ -0,0 +1,20 @@ +/* + * 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. + */ + +options { + port 5300; +}; + +zone example { + type secondary; + primaries { 1.2.3.4; }; + notify-source 10.53.0.1; +}; diff --git a/bin/tests/system/checkconf/good-parental-source-v6.conf b/bin/tests/system/checkconf/good-parental-source-v6.conf new file mode 100644 index 0000000000..82a0521512 --- /dev/null +++ b/bin/tests/system/checkconf/good-parental-source-v6.conf @@ -0,0 +1,20 @@ +/* + * 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. + */ + +options { + port 5300; +}; + +zone example { + type secondary; + primaries { 1.2.3.4; }; + parental-source-v6 fd92:7065:b8e:ffff::1; +}; diff --git a/bin/tests/system/checkconf/good-parental-source.conf b/bin/tests/system/checkconf/good-parental-source.conf new file mode 100644 index 0000000000..48739750e5 --- /dev/null +++ b/bin/tests/system/checkconf/good-parental-source.conf @@ -0,0 +1,20 @@ +/* + * 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. + */ + +options { + port 5300; +}; + +zone example { + type secondary; + primaries { 1.2.3.4; }; + parental-source 10.53.0.1; +}; diff --git a/bin/tests/system/checkconf/good-transfer-source-v6.conf b/bin/tests/system/checkconf/good-transfer-source-v6.conf new file mode 100644 index 0000000000..2cb133033a --- /dev/null +++ b/bin/tests/system/checkconf/good-transfer-source-v6.conf @@ -0,0 +1,20 @@ +/* + * 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. + */ + +options { + port 5300; +}; + +zone example { + type secondary; + primaries { 1.2.3.4; }; + transfer-source-v6 fd92:7065:b8e:ffff::1; +}; diff --git a/bin/tests/system/checkconf/good-transfer-source.conf b/bin/tests/system/checkconf/good-transfer-source.conf new file mode 100644 index 0000000000..5273457bb3 --- /dev/null +++ b/bin/tests/system/checkconf/good-transfer-source.conf @@ -0,0 +1,20 @@ +/* + * 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. + */ + +options { + port 5300; +}; + +zone example { + type secondary; + primaries { 1.2.3.4; }; + transfer-source 10.53.0.1; +}; diff --git a/bin/tests/system/checkconf/tests.sh b/bin/tests/system/checkconf/tests.sh index afb4a34688..8f0dbadc43 100644 --- a/bin/tests/system/checkconf/tests.sh +++ b/bin/tests/system/checkconf/tests.sh @@ -421,6 +421,7 @@ echo_i "check that named-checkconf -l prints out the zone list ($n)" ret=0 $CHECKCONF -l good.conf | grep -v "is not implemented" | +grep -v "is not recommended" | grep -v "no longer exists" | grep -v "is obsolete" > checkconf.out$n || ret=1 diff good.zonelist checkconf.out$n > diff.out$n || ret=1 @@ -558,6 +559,18 @@ grep "exceeds 100%" < checkconf.out$n > /dev/null || ret=1 if [ $ret != 0 ]; then echo_i "failed"; ret=1; fi status=`expr $status + $ret` +n=`expr $n + 1` +echo_i "check that *-source options with specified port generate warnings ($n)" +ret=0 +$CHECKCONF warn-transfer-source.conf > checkconf.out$n 2>/dev/null || ret=1 +grep "not recommended" < checkconf.out$n > /dev/null || ret=1 +$CHECKCONF warn-notify-source.conf > checkconf.out$n 2>/dev/null || ret=1 +grep "not recommended" < checkconf.out$n > /dev/null || ret=1 +$CHECKCONF warn-parental-source.conf > checkconf.out$n 2>/dev/null || ret=1 +grep "not recommended" < 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" diff --git a/bin/tests/system/checkconf/warn-notify-source.conf b/bin/tests/system/checkconf/warn-notify-source.conf new file mode 100644 index 0000000000..523844d0d9 --- /dev/null +++ b/bin/tests/system/checkconf/warn-notify-source.conf @@ -0,0 +1,20 @@ +/* + * 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. + */ + +options { + port 5300; +}; + +zone example { + type secondary; + primaries { 1.2.3.4; }; + notify-source 10.53.0.1 port 100; +}; diff --git a/bin/tests/system/checkconf/warn-parental-source.conf b/bin/tests/system/checkconf/warn-parental-source.conf new file mode 100644 index 0000000000..1cc73b7689 --- /dev/null +++ b/bin/tests/system/checkconf/warn-parental-source.conf @@ -0,0 +1,20 @@ +/* + * 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. + */ + +options { + port 5300; +}; + +zone example { + type secondary; + primaries { 1.2.3.4; }; + parental-source 10.53.0.1 port 100; +}; diff --git a/bin/tests/system/checkconf/warn-transfer-source.conf b/bin/tests/system/checkconf/warn-transfer-source.conf new file mode 100644 index 0000000000..8def281cf9 --- /dev/null +++ b/bin/tests/system/checkconf/warn-transfer-source.conf @@ -0,0 +1,20 @@ +/* + * 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. + */ + +options { + port 5300; +}; + +zone example { + type secondary; + primaries { 1.2.3.4; }; + transfer-source 10.53.0.1 port 100; +}; diff --git a/lib/bind9/check.c b/lib/bind9/check.c index 156bb6a7b8..eb3d465af5 100644 --- a/lib/bind9/check.c +++ b/lib/bind9/check.c @@ -66,6 +66,8 @@ #include +static in_port_t dnsport = 53; + static isc_result_t fileexist(const cfg_obj_t *obj, isc_symtab_t *symtab, bool writeable, isc_log_t *logctxlogc); @@ -1039,6 +1041,30 @@ check_listeners(const cfg_obj_t *list, const cfg_obj_t *config, return (result); } +static isc_result_t +check_port(const cfg_obj_t *options, isc_log_t *logctx, const char *type, + in_port_t *portp) { + const cfg_obj_t *portobj = NULL; + isc_result_t result; + + result = cfg_map_get(options, type, &portobj); + if (result != ISC_R_SUCCESS) { + return (ISC_R_SUCCESS); + } + + if (cfg_obj_asuint32(portobj) >= UINT16_MAX) { + cfg_obj_log(portobj, logctx, ISC_LOG_ERROR, + "port '%u' out of range", + cfg_obj_asuint32(portobj)); + return (ISC_R_RANGE); + } + + if (portp != NULL) { + *portp = (in_port_t)cfg_obj_asuint32(portobj); + } + return (ISC_R_SUCCESS); +} + static isc_result_t check_options(const cfg_obj_t *options, const cfg_obj_t *config, isc_log_t *logctx, isc_mem_t *mctx, optlevel_t optlevel) { @@ -1055,6 +1081,10 @@ check_options(const cfg_obj_t *options, const cfg_obj_t *config, bool has_dnssecpolicy = false; const char *ccalg = "siphash24"; cfg_aclconfctx_t *actx = NULL; + static const char *sources[] = { + "query-source", + "query-source-v6", + }; /* * { "name", scale, value } @@ -1099,6 +1129,57 @@ check_options(const cfg_obj_t *options, const cfg_obj_t *config, }; #endif /* ifdef HAVE_DNSTAP */ + if (optlevel == optlevel_options) { + /* + * Check port values, and record "port" for later use. + */ + tresult = check_port(options, logctx, "port", &dnsport); + if (tresult != ISC_R_SUCCESS) { + result = tresult; + } + tresult = check_port(options, logctx, "tls-port", NULL); + if (tresult != ISC_R_SUCCESS) { + result = tresult; + } + tresult = check_port(options, logctx, "http-port", NULL); + if (tresult != ISC_R_SUCCESS) { + result = tresult; + } + tresult = check_port(options, logctx, "https-port", NULL); + if (tresult != ISC_R_SUCCESS) { + result = tresult; + } + } + + if (optlevel == optlevel_options || optlevel == optlevel_view) { + /* + * Warn if query-source or query-source-v6 options specify + * a port, and fail if they specify the DNS port. + */ + for (i = 0; i < ARRAY_SIZE(sources); i++) { + obj = NULL; + (void)cfg_map_get(options, sources[i], &obj); + if (obj != NULL) { + const isc_sockaddr_t *sa = + cfg_obj_assockaddr(obj); + in_port_t port = isc_sockaddr_getport(sa); + if (port == dnsport) { + cfg_obj_log(obj, logctx, ISC_LOG_ERROR, + "'%s' cannot specify the " + "DNS listener port (%d)", + sources[i], port); + result = ISC_R_FAILURE; + } else if (port != 0) { + cfg_obj_log(obj, logctx, + ISC_LOG_WARNING, + "'%s': specifying a port " + "is not recommended", + sources[i]); + } + } + } + } + /* * Check that fields specified in units of time other than seconds * have reasonable values. @@ -2568,13 +2649,16 @@ check_zoneconf(const cfg_obj_t *zconfig, const cfg_obj_t *voptions, "allow-update", "allow-update-forwarding", }; - static optionstable dialups[] = { { "notify", CFG_ZONE_PRIMARY | CFG_ZONE_SECONDARY }, { "notify-passive", CFG_ZONE_SECONDARY }, { "passive", CFG_ZONE_SECONDARY | CFG_ZONE_STUB }, { "refresh", CFG_ZONE_SECONDARY | CFG_ZONE_STUB }, }; + static const char *sources[] = { + "transfer-source", "transfer-source-v6", "notify-source", + "notify-source-v6", "parental-source", "parental-source-v6", + }; znamestr = cfg_obj_asstring(cfg_tuple_get(zconfig, "name")); @@ -2852,7 +2936,7 @@ check_zoneconf(const cfg_obj_t *zconfig, const cfg_obj_t *voptions, /* * Check that ACLs expand correctly. */ - for (i = 0; i < (sizeof(acls) / sizeof(acls[0])); i++) { + for (i = 0; i < ARRAY_SIZE(acls); i++) { tresult = checkacl(acls[i], actx, zconfig, voptions, config, logctx, mctx); if (tresult != ISC_R_SUCCESS) { @@ -2871,8 +2955,8 @@ check_zoneconf(const cfg_obj_t *zconfig, const cfg_obj_t *voptions, } /* - * Master, slave, and mirror zones may have an "also-notify" field, but - * shouldn't if notify is disabled. + * Primary, secondary, and mirror zones may have an "also-notify" + * field, but shouldn't if notify is disabled. */ if (ztype == CFG_ZONE_PRIMARY || ztype == CFG_ZONE_SECONDARY || ztype == CFG_ZONE_MIRROR) @@ -2927,9 +3011,10 @@ check_zoneconf(const cfg_obj_t *zconfig, const cfg_obj_t *voptions, } /* - * Slave, mirror, and stub zones must have a "primaries" field, with one - * exception: when mirroring the root zone, a default, built-in master - * server list is used in the absence of one explicitly specified. + * Secondary, mirror, and stub zones must have a "primaries" field, + * with one exception: when mirroring the root zone, a default, + * built-in primary server list is used in the absence of one + * explicitly specified. */ if (ztype == CFG_ZONE_SECONDARY || ztype == CFG_ZONE_STUB || (ztype == CFG_ZONE_MIRROR && zname != NULL && @@ -2975,6 +3060,34 @@ check_zoneconf(const cfg_obj_t *zconfig, const cfg_obj_t *voptions, } } + /* + * Warn if *-source and *-source-v6 options specify a port, + * and fail if they specify the default listener port. + */ + for (i = 0; i < ARRAY_SIZE(sources); i++) { + obj = NULL; + (void)cfg_map_get(zoptions, sources[i], &obj); + if (obj == NULL && goptions != NULL) { + (void)cfg_map_get(goptions, sources[i], &obj); + } + if (obj != NULL) { + in_port_t port = + isc_sockaddr_getport(cfg_obj_assockaddr(obj)); + if (port == dnsport) { + cfg_obj_log(obj, logctx, ISC_LOG_ERROR, + "'%s' cannot specify the " + "DNS listener port (%d)", + sources[i], port); + result = ISC_R_FAILURE; + } else if (port != 0) { + cfg_obj_log(obj, logctx, ISC_LOG_WARNING, + "'%s': specifying a port is " + "not recommended", + sources[i]); + } + } + } + /* * Primary and secondary zones that have a "parental-agents" field, * must have a corresponding "parental-agents" clause. @@ -3016,7 +3129,7 @@ check_zoneconf(const cfg_obj_t *zconfig, const cfg_obj_t *voptions, } /* - * Master zones can't have both "allow-update" and "update-policy". + * Primary zones can't have both "allow-update" and "update-policy". */ if (ztype == CFG_ZONE_PRIMARY || ztype == CFG_ZONE_SECONDARY) { bool signing = false; @@ -3433,8 +3546,8 @@ check_zoneconf(const cfg_obj_t *zconfig, const cfg_obj_t *voptions, } /* - * If the zone type is rbt/rbt64 then master/hint zones require file - * clauses. If inline-signing is used, then slave zones require a + * If the zone type is rbt/rbt64 then primary/hint zones require file + * clauses. If inline-signing is used, then secondary zones require a * file clause as well. */ obj = NULL; @@ -3757,15 +3870,6 @@ check_keylist(const cfg_obj_t *keys, isc_symtab_t *symtab, isc_mem_t *mctx, return (result); } -static struct { - const char *v4; - const char *v6; -} sources[] = { { "transfer-source", "transfer-source-v6" }, - { "notify-source", "notify-source-v6" }, - { "parental-source", "parental-source-v6" }, - { "query-source", "query-source-v6" }, - { NULL, NULL } }; - /* * RNDC keys are not normalised unlike TSIG keys. * @@ -3793,6 +3897,15 @@ rndckey_exists(const cfg_obj_t *keylist, const char *keyname) { return (false); } +static struct { + const char *v4; + const char *v6; +} sources[] = { { "transfer-source", "transfer-source-v6" }, + { "notify-source", "notify-source-v6" }, + { "parental-source", "parental-source-v6" }, + { "query-source", "query-source-v6" }, + { NULL, NULL } }; + static isc_result_t check_servers(const cfg_obj_t *config, const cfg_obj_t *voptions, isc_symtab_t *symtab, isc_log_t *logctx) { @@ -3842,6 +3955,10 @@ check_servers(const cfg_obj_t *config, const cfg_obj_t *voptions, } source = 0; do { + /* + * For a v6 server we can't specify a v4 source, + * and vice versa. + */ obj = NULL; if (n1.family == AF_INET) { xfr = sources[source].v6; @@ -3856,6 +3973,31 @@ check_servers(const cfg_obj_t *config, const cfg_obj_t *voptions, p1, xfr); result = ISC_R_FAILURE; } + + /* + * Check that we aren't using the DNS + * listener port (i.e. 53, or whatever was set + * as "port" in options) as a source port. + */ + obj = NULL; + if (n1.family == AF_INET) { + xfr = sources[source].v4; + } else { + xfr = sources[source].v6; + } + (void)cfg_map_get(v1, xfr, &obj); + if (obj != NULL) { + const isc_sockaddr_t *sa = + cfg_obj_assockaddr(obj); + in_port_t port = isc_sockaddr_getport(sa); + if (port == dnsport) { + cfg_obj_log(obj, logctx, ISC_LOG_ERROR, + "'%s' cannot specify the " + "DNS listener port (%d)", + xfr, port); + result = ISC_R_FAILURE; + } + } } while (sources[++source].v4 != NULL); e2 = e1; while ((e2 = cfg_list_next(e2)) != NULL) { @@ -5351,8 +5493,7 @@ bind9_check_namedconf(const cfg_obj_t *config, bool check_plugins, const cfg_obj_t *options = NULL; const cfg_obj_t *views = NULL; const cfg_obj_t *acls = NULL; - const cfg_obj_t *kals = NULL; - const cfg_obj_t *obj; + const cfg_obj_t *obj = NULL; const cfg_listelt_t *velement; isc_result_t result = ISC_R_SUCCESS; isc_result_t tresult; @@ -5600,43 +5741,6 @@ bind9_check_namedconf(const cfg_obj_t *config, bool check_plugins, } } - tresult = cfg_map_get(config, "kal", &kals); - if (tresult == ISC_R_SUCCESS) { - const cfg_listelt_t *elt; - const cfg_listelt_t *elt2; - const char *aclname; - - for (elt = cfg_list_first(kals); elt != NULL; - elt = cfg_list_next(elt)) { - const cfg_obj_t *acl = cfg_listelt_value(elt); - - aclname = cfg_obj_asstring(cfg_tuple_get(acl, "name")); - - for (elt2 = cfg_list_next(elt); elt2 != NULL; - elt2 = cfg_list_next(elt2)) { - const cfg_obj_t *acl2 = cfg_listelt_value(elt2); - const char *name; - name = cfg_obj_asstring( - cfg_tuple_get(acl2, "name")); - if (strcasecmp(aclname, name) == 0) { - const char *file = cfg_obj_file(acl); - unsigned int line = cfg_obj_line(acl); - - if (file == NULL) { - file = ""; - } - - cfg_obj_log(acl2, logctx, ISC_LOG_ERROR, - "attempt to redefine " - "kal '%s' previous " - "definition: %s:%u", - name, file, line); - result = ISC_R_FAILURE; - } - } - } - } - cleanup: if (symtab != NULL) { isc_symtab_destroy(&symtab); From c9a17c878ac8c136b1313102d8c9b874379266d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 14 Sep 2021 16:48:38 +0200 Subject: [PATCH 2/3] Document caveats related to single source port in the ARM Discourage the single source port on general level and document that the source port cannot be same as the listening port. This applies to query-source, transfer-source, notify-source, parental-source, and their respective IPv6 counterparts. --- doc/arm/reference.rst | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/doc/arm/reference.rst b/doc/arm/reference.rst index e18500d48c..c07a7a031a 100644 --- a/doc/arm/reference.rst +++ b/doc/arm/reference.rst @@ -2643,6 +2643,11 @@ options are: .. note:: Solaris 2.5.1 and earlier does not support setting the source address for TCP sockets. +.. warning:: Specifying a single port is discouraged, as it removes a layer of + protection against spoofing errors. + +.. warning:: The configured ``port`` must not be same as the listening port. + .. note:: See also ``transfer-source``, ``notify-source`` and ``parental-source``. .. _zone_transfers: @@ -2781,6 +2786,11 @@ options apply to zone transfers. .. note:: Solaris 2.5.1 and earlier does not support setting the source address for TCP sockets. + .. warning:: Specifying a single port is discouraged, as it removes a layer of + protection against spoofing errors. + + .. warning:: The configured ``port`` must not be same as the listening port. + ``transfer-source-v6`` This option is the same as ``transfer-source``, except zone transfers are performed using IPv6. @@ -2814,6 +2824,11 @@ options apply to zone transfers. .. note:: Solaris 2.5.1 and earlier does not support setting the source address for TCP sockets. + .. warning:: Specifying a single port is discouraged, as it removes a layer of + protection against spoofing errors. + + .. warning:: The configured ``port`` must not be same as the listening port. + ``notify-source-v6`` This option acts like ``notify-source``, but applies to notify messages sent to IPv6 addresses. @@ -5208,6 +5223,11 @@ The following options apply to DS queries sent to ``parental-agents``: .. note:: Solaris 2.5.1 and earlier does not support setting the source address for TCP sockets. + .. warning:: Specifying a single port is discouraged, as it removes a layer of + protection against spoofing errors. + + .. warning:: The configured ``port`` must not be same as the listening port. + ``parental-source-v6`` This option acts like ``parental-source``, but applies to parental DS queries sent to IPv6 addresses. From bba5e98734c49bdd2c570517f2dc2f0f61c65221 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 14 Sep 2021 15:19:01 +0200 Subject: [PATCH 3/3] Add CHANGES and release notes for [GL #2888] --- CHANGES | 4 ++++ doc/notes/notes-current.rst | 12 ++++++++++++ 2 files changed, 16 insertions(+) diff --git a/CHANGES b/CHANGES index f5cf53ffec..ce8f842977 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5715. [func] Add a check when the *-source(-v6) clashes with the + global listening port. Such a configuration was already + forbidden, but it failed silently. [GL #2888] + 5714. [bug] Remove the "adjust interface" mechanism that set up a listener on interfaces where the *-source(-v6) address and port were the same as the listening diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index efa5c2eb40..ccdfd44b56 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -43,6 +43,18 @@ Feature Changes - SHA-1 CDS records are no longer used by ``dnssec-cds`` to make DS records. Thanks to Tony Finch. :gl:`!2946` +- ``named`` and ``named-checkconf`` now issue a warning when there is a single + configured port in the ``query-source``, ``transfer-source``, + ``notify-source``, and ``parental-source``, and/or in their respective IPv6 counterparts. + :gl:`#2888` + +- ``named`` and ``named-checkconf`` now return an error when the single configured + port in the ``query-source``, ``transfer-source``, ``notify-source``, + ``parental-source``, and/or their respective IPv6 counterparts clashes with the + global listening port. This configuration is no longer supported as of BIND + 9.16.0 but no error was reported, although sending UDP messages + (such as notifies) would fail. :gl:`#2888` + Bug Fixes ~~~~~~~~~