Merge branch '2918-deprecate-max-zone-ttl-v9_18' into 'v9_18'

dnssec-policy max-zone-ttl was not fully effective

See merge request isc-projects/bind9!6599

CHANGES

@@ -11,6 +11,15 @@
 			option (or use the default values of 2 or 3
 			respectively). [GL #3407]
 
+5929.	[bug]		The "max-zone-ttl" option in "dnssec-policy" was
+			not fully effective; it was used for timing key
+			rollovers but did not actually place an upper limit
+			on TTLs when loading a zone. This has been
+			corrected, and the documentation has been clarified
+			to indicate that the old "max-zone-ttl" zone option
+			is now ignored when "dnssec-policy" is in use.
+			[GL #2918]
+
 5927.	[bug]		A race was possible in dns_dispatch_connect()
 			that could trigger an assertion failure if two
 			threads called it near-simultaneously. [GL #3456]

bin/named/zoneconf.c

@@ -907,6 +907,7 @@ named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig,
 	dns_stats_t *dnssecsignstats;
 	dns_zonestat_level_t statlevel = dns_zonestat_none;
 	int seconds;
+	dns_ttl_t maxttl = 0; /* unlimited */
 	dns_zone_t *mayberaw = (raw != NULL) ? raw : zone;
 	isc_dscp_t dscp;
 
@@ -1065,20 +1066,6 @@ named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig,
 		}
 	}
 
-	obj = NULL;
-	result = named_config_get(maps, "max-zone-ttl", &obj);
-	if (result == ISC_R_SUCCESS) {
-		dns_ttl_t maxttl = 0; /* unlimited */
-
-		if (cfg_obj_isduration(obj)) {
-			maxttl = cfg_obj_asduration(obj);
-		}
-		dns_zone_setmaxttl(zone, maxttl);
-		if (raw != NULL) {
-			dns_zone_setmaxttl(raw, maxttl);
-		}
-	}
-
 	obj = NULL;
 	result = named_config_get(maps, "max-records", &obj);
 	INSIST(result == ISC_R_SUCCESS && obj != NULL);
@@ -1531,6 +1518,22 @@ named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig,
 		dns_zone_setjournalsize(zone, journal_size);
 	}
 
+	if (use_kasp) {
+		maxttl = dns_kasp_zonemaxttl(dns_zone_getkasp(zone));
+	} else {
+		obj = NULL;
+		result = named_config_get(maps, "max-zone-ttl", &obj);
+		if (result == ISC_R_SUCCESS) {
+			if (cfg_obj_isduration(obj)) {
+				maxttl = cfg_obj_asduration(obj);
+			}
+		}
+	}
+	dns_zone_setmaxttl(zone, maxttl);
+	if (raw != NULL) {
+		dns_zone_setmaxttl(raw, maxttl);
+	}
+
 	/*
 	 * Configure update-related options.  These apply to
 	 * primary servers only.

bin/tests/system/checkconf/tests.sh

@@ -607,6 +607,14 @@ grep "not recommended" < 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 using both max-zone-ttl and dnssec-policy generates a warning ($n)"
+ret=0
+$CHECKCONF warn-kasp-max-zone-ttl.conf > checkconf.out$n 2>/dev/null || ret=1
+grep "option 'max-zone-ttl' is ignored when used together with 'dnssec-policy'" < 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"

bin/tests/system/checkconf/warn-kasp-max-zone-ttl.conf

@@ -0,0 +1,26 @@
+/*
+ * 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.
+ */
+
+/*
+ * The dnssec-policy is not defined. Should also be caught if it is inherited.
+ */
+
+options {
+	dnssec-policy default;
+};
+
+zone "example.net" {
+	type primary;
+	file "example.db";
+        max-zone-ttl 600;
+};

bin/tests/system/kasp/ns3/named.conf.in

@@ -223,6 +223,15 @@ zone "ecdsa384.kasp" {
 	dnssec-policy "ecdsa384";
 };
 
+/*
+ * Zone with too high TTL.
+ */
+zone "max-zone-ttl.kasp" {
+	type primary;
+	file "max-zone-ttl.kasp.db";
+	dnssec-policy "ttl";
+};
+
 /*
  * Zones in different signing states.
  */

bin/tests/system/kasp/ns3/policies/kasp.conf.in

@@ -132,3 +132,7 @@ dnssec-policy "checkds-csk" {
 		csk key-directory lifetime unlimited algorithm @DEFAULT_ALGORITHM@;
 	};
 };
+
+dnssec-policy "ttl" {
+	max-zone-ttl 299;
+};

bin/tests/system/kasp/ns3/setup.sh

@@ -64,20 +64,16 @@ if [ -f ../ed448-supported.file ]; then
 	cat ed448.conf >> named.conf
 fi
 
-# Set up zone that stays unsigned.
-zone="unsigned.kasp"
-echo_i "setting up zone: $zone"
-zonefile="${zone}.db"
-infile="${zone}.db.infile"
-cp template.db.in $infile
-cp template.db.in $zonefile
-
-# Set up zone that stays unsigned.
-zone="insecure.kasp"
-echo_i "setting up zone: $zone"
-zonefile="${zone}.db"
-infile="${zone}.db.infile"
-cp template.db.in $zonefile
+# Set up zones that stay unsigned.
+for zn in unsigned insecure max-zone-ttl
+do
+	zone="${zn}.kasp"
+	echo_i "setting up zone: $zone"
+	zonefile="${zone}.db"
+	infile="${zone}.db.infile"
+	cp template.db.in $infile
+	cp template.db.in $zonefile
+done
 
 # Some of these zones already have keys.
 zone="dnssec-keygen.kasp"

bin/tests/system/kasp/tests.sh

@@ -252,6 +252,15 @@ status=$((status+ret))
 
 next_key_event_threshold=$((next_key_event_threshold+i))
 
+# Test max-zone-ttl rejects zones with too high TTL.
+n=$((n+1))
+echo_i "check that max-zone-ttl rejects zones with too high TTL ($n)"
+ret=0
+set_zone "max-zone-ttl.kasp"
+grep "loading from master file ${ZONE}.db failed: out of range" "ns3/named.run" > /dev/null || ret=1
+test "$ret" -eq 0 || echo_i "failed"
+status=$((status+ret))
+
 #
 # Zone: default.kasp.
 #

doc/arm/reference.rst

@@ -1796,26 +1796,26 @@ default is used.
    The default is five minutes. It cannot be longer than :any:`nta-lifetime`, which
    cannot be longer than a week.
 
-.. namedconf:statement:: max-zone-ttl
+:any:`max-zone-ttl`
 
-   This specifies a maximum permissible TTL value in seconds. For
-   convenience, TTL-style time-unit suffixes may be used to specify the
-   maximum value. When loading a zone file using a :any:`masterfile-format`
-   of ``text`` or ``raw``, any record encountered with a TTL higher than
-   :any:`max-zone-ttl` causes the zone to be rejected.
+   This should now be configured as part of :namedconf:ref:`dnssec-policy`.
+   Use of this option in :namedconf:ref:`options`, :namedconf:ref:`view`
+   and :namedconf:ref:`zone` blocks has no effect on any zone for which
+   a :namedconf:ref:`dnssec-policy` has also been configured.
+
+   :any:`max-zone-ttl` specifies a maximum permissible TTL value in seconds.
+   For convenience, TTL-style time-unit suffixes may be used to specify the
+   maximum value. When a zone file is loaded, any record encountered with a
+   TTL higher than :any:`max-zone-ttl` causes the zone to be rejected.
 
    This is needed in DNSSEC-maintained zones because when rolling to a new
    DNSKEY, the old key needs to remain available until RRSIG records
    have expired from caches. The :any:`max-zone-ttl` option guarantees that
    the largest TTL in the zone is no higher than the set value.
 
-   In the :namedconf:ref:`options` and :namedconf:ref:`zone` blocks,
-   the default value is ``unlimited``. A :any:`max-zone-ttl` of zero is
-   treated as ``unlimited``.
-
-   In the :namedconf:ref:`dnssec-policy` block,
-   the default value is ``PT24H`` (24 hours).  A :any:`max-zone-ttl` of
-   zero is treated as if the default value were in use.
+   When used in :namedconf:ref:`options`, :namedconf:ref:`view` and
+   :namedconf:ref:`zone` blocks, setting :any:`max-zone-ttl` to zero
+   is equivalent to "unlimited".
 
 .. namedconf:statement:: stale-answer-ttl
 
@@ -4225,9 +4225,9 @@ Tuning
    Note that when a zone file in a format other than ``text`` is loaded,
    :iscman:`named` may omit some of the checks which are performed for a file in
    ``text`` format. For example, :any:`check-names` only applies when loading
-   zones in ``text`` format, and :any:`max-zone-ttl` only applies to ``text``
-   and ``raw``.  Zone files in binary formats should be generated with the
-   same check level as that specified in the :iscman:`named` configuration file.
+   zones in ``text`` format. Zone files in ``raw`` format should be generated
+   with the same check level as that specified in the :iscman:`named`
+   configuration file.
 
    When configured in :namedconf:ref:`options`, this statement sets the
    :any:`masterfile-format` for all zones, but it can be overridden on a
@@ -5985,10 +5985,20 @@ The following options can be specified in a :any:`dnssec-policy` statement:
     This is similar to :any:`signatures-validity`, but for DNSKEY records.
     The default is ``P2W`` (2 weeks).
 
-:any:`max-zone-ttl`
+.. namedconf:statement:: max-zone-ttl
+
+   This specifies the maximum permissible TTL value for the zone.  When
+   a zone file is loaded, any record encountered with a TTL higher than
+   :any:`max-zone-ttl` causes the zone to be rejected.
+
+   This ensures that when rolling to a new DNSKEY, the old key will remain
+   available until RRSIG records have expired from caches. The
+   :any:`max-zone-ttl` option guarantees that the largest TTL in the
+   zone is no higher than a known and predictable value.
+
+   The default value ``PT24H`` (24 hours).  A value of zero is treated
+   as if the default value were in use.
 
-   Like the :namedconf:ref:`max-zone-ttl` zone option, this specifies the maximum
-   permissible TTL value, in seconds, for the zone.
 
 .. namedconf:statement:: nsec3param
 

doc/notes/notes-current.rst

@@ -49,3 +49,11 @@ Bug Fixes
 - Non-dynamic zones that inherit dnssec-policy from the view or
   options level were not marked as inline-signed, and thus were never
   scheduled to be re-signed. This is now fixed. :gl:`#3438`
+
+- The old ``max-zone-ttl`` zone option was meant to be superseded by
+  the ``max-zone-ttl`` option in ``dnssec-policy``; however, the latter
+  option was not fully effective. This has been corrected: zones will
+  not load if they contain TTLs greater than the limit configured in
+  ``dnssec-policy``. In zones with both the old ``max-zone-ttl``
+  option and ``dnssec-policy`` configured, the old option will be
+  ignored, and a warning will be generated. :gl:`#2918`

lib/bind9/check.c

@@ -3144,6 +3144,27 @@ check_zoneconf(const cfg_obj_t *zconfig, const cfg_obj_t *voptions,
 		}
 	}
 
+	/*
+	 * Warn about zones with both dnssec-policy and max-zone-ttl
+	 */
+	if (has_dnssecpolicy) {
+		obj = NULL;
+		(void)cfg_map_get(zoptions, "max-zone-ttl", &obj);
+		if (obj == NULL && voptions != NULL) {
+			(void)cfg_map_get(voptions, "max-zone-ttl", &obj);
+		}
+		if (obj == NULL && goptions != NULL) {
+			(void)cfg_map_get(goptions, "max-zone-ttl", &obj);
+		}
+		if (obj != NULL) {
+			cfg_obj_log(obj, logctx, ISC_LOG_WARNING,
+				    "zone '%s': option 'max-zone-ttl' "
+				    "is ignored when used together with "
+				    "'dnssec-policy'",
+				    znamestr);
+		}
+	}
+
 	/*
 	 * Check validity of the zone options.
 	 */