Merge branch '3071-signed-version-of-an-inline-signed-zone-may-be-dumped-without-unsigned-serial-number-fix' into 'main'

Check unsigned serial number in signed zone files

Closes #3071

See merge request isc-projects/bind9!5692

CHANGES

@@ -6,7 +6,7 @@
 			activated at zone level. [GL #3023]
 
 5786.	[bug]		Defer detaching from zone->raw in zone_shutdown() if
-			the zone is in the process of being dumped to disk to
+			the zone is in the process of being dumped to disk, to
 			ensure that the unsigned serial number information is
 			always written in the raw-format header of the signed
 			version on an inline-signed zone. [GL #3071]

bin/tests/system/inline/ns8/named.conf.in

@@ -151,3 +151,10 @@ zone example {
 	auto-dnssec maintain;
 	file "example.db";
 };
+
+zone "unsigned-serial-test" {
+	type primary;
+	inline-signing yes;
+	auto-dnssec maintain;
+	file "unsigned-serial-test.db";
+};

bin/tests/system/inline/ns8/sign.sh

@@ -21,12 +21,13 @@ do
   keyname=`$KEYGEN -q -a $DEFAULT_ALGORITHM -b $DEFAULT_BITS -n zone $zone`
   keyname=`$KEYGEN -q -a $DEFAULT_ALGORITHM -b $DEFAULT_BITS -n zone -f KSK $zone`
   cp example.com.db.in ${zone}.db
-  $SIGNER -S -T 3600 -O raw -o ${zone} ${zone}.db > /dev/null 2>&1
+  $SIGNER -S -T 3600 -O raw -L 2000042407 -o ${zone} ${zone}.db > /dev/null 2>&1
 done
 
-zone=example
-rm -f K${zone}.+*+*.key
-rm -f K${zone}.+*+*.private
-keyname=`$KEYGEN -q -a $DEFAULT_ALGORITHM -b $DEFAULT_BITS -n zone $zone`
-keyname=`$KEYGEN -q -a $DEFAULT_ALGORITHM -b $DEFAULT_BITS -n zone -f KSK $zone`
-cp ${zone}.db.in ${zone}.db
+for zone in example unsigned-serial-test; do
+  rm -f K${zone}.+*+*.key
+  rm -f K${zone}.+*+*.private
+  keyname=`$KEYGEN -q -a $DEFAULT_ALGORITHM -b $DEFAULT_BITS -n zone $zone`
+  keyname=`$KEYGEN -q -a $DEFAULT_ALGORITHM -b $DEFAULT_BITS -n zone -f KSK $zone`
+  cp example.db.in ${zone}.db
+done

bin/tests/system/inline/tests_signed_zone_files.py

@@ -0,0 +1,68 @@
+############################################################################
+# 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 https://mozilla.org/MPL/2.0/.
+#
+# See the COPYRIGHT file distributed with this work for additional
+# information regarding copyright ownership.
+############################################################################
+
+import glob
+import struct
+
+
+class RawFormatHeader(dict):
+    '''
+    A dictionary of raw-format header fields read from a zone file.
+    '''
+
+    fields = [
+        'format',
+        'version',
+        'dumptime',
+        'flags',
+        'sourceserial',
+        'lastxfrin',
+    ]
+
+    def __init__(self, file_name):
+        header = struct.Struct('>IIIIII')
+        with open(file_name, 'rb') as data:
+            header_data = data.read(header.size)
+        super().__init__(zip(self.fields, header.unpack_from(header_data)))
+
+
+def test_unsigned_serial_number():
+
+    '''
+    Check whether all signed zone files in the "ns8" subdirectory contain the
+    serial number of the unsigned version of the zone in the raw-format header.
+    The test assumes that all "*.signed" files in the "ns8" subdirectory are in
+    raw format.
+
+    Notes:
+
+      - The actual zone signing and dumping happens while the tests.sh phase of
+        the "inline" system test is set up and run.  This check only verifies
+        the outcome of those events; it does not initiate any signing or
+        dumping itself.
+
+      - example[0-9][0-9].com.db.signed files are initially signed by
+        dnssec-signzone while the others - by named.
+    '''
+
+    zones_with_unsigned_serial_missing = []
+
+    for signed_zone in sorted(glob.glob('ns8/*.signed')):
+        raw_header = RawFormatHeader(signed_zone)
+        # Ensure the unsigned serial number is placed where it is expected.
+        assert raw_header['format'] == 2
+        assert raw_header['version'] == 1
+        # Check whether the header flags indicate that the unsigned serial
+        # number is set and that the latter is indeed set.
+        if raw_header['flags'] & 0x02 == 0 or raw_header['sourceserial'] == 0:
+            zones_with_unsigned_serial_missing.append(signed_zone)
+
+    assert not zones_with_unsigned_serial_missing

doc/notes/notes-current.rst

@@ -44,8 +44,8 @@ Bug Fixes
 - On FreeBSD, a TCP connection would leak a small amount of heap memory leading
   to out-of-memory problem in a long run. This has been fixed. :gl:`#3051`
 
-- Under certain circumstances, the signed version of an inline-signed zone could
-  be dumped to disk without the serial number of the unsigned version of the
-  zone being saved. This could prevent resynchronization of zone contents after
-  ``named`` restarted, if the unsigned zone file had been modified while
-  ``named`` was not running. This has been fixed. :gl:`#3071`
+- Under certain circumstances, the signed version of an inline-signed
+  zone could be dumped to disk without the serial number of the unsigned
+  version of the zone, preventing resynchronization of zone contents
+  after ``named`` restart in case the unsigned zone file gets modified
+  while ``named`` is not running. This has been fixed. :gl:`#3071`

lib/dns/zone.c

@@ -11925,7 +11925,22 @@ dump_done(void *arg, isc_result_t result) {
 	if (compact) {
 		DNS_ZONE_SETFLAG(zone, DNS_ZONEFLG_NEEDCOMPACT);
 	}
-	if (result != ISC_R_SUCCESS && result != ISC_R_CANCELED) {
+	if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_SHUTDOWN)) {
+		/*
+		 * If DNS_ZONEFLG_SHUTDOWN is set, all external references to
+		 * the zone are gone, which means it is in the process of being
+		 * cleaned up, so do not reschedule dumping.
+		 *
+		 * Detach from the raw version of the zone in case this
+		 * operation has been deferred in zone_shutdown().
+		 */
+		if (zone->raw != NULL) {
+			dns_zone_detach(&zone->raw);
+		}
+		if (result == ISC_R_SUCCESS) {
+			DNS_ZONE_CLRFLAG(zone, DNS_ZONEFLG_FLUSH);
+		}
+	} else if (result != ISC_R_SUCCESS && result != ISC_R_CANCELED) {
 		/*
 		 * Try again in a short while.
 		 */
@@ -11947,9 +11962,6 @@ dump_done(void *arg, isc_result_t result) {
 		dns_dumpctx_detach(&zone->dctx);
 	}
 	zonemgr_putio(&zone->writeio);
-	if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_SHUTDOWN) && zone->raw != NULL) {
-		dns_zone_detach(&zone->raw);
-	}
 	UNLOCK_ZONE(zone);
 	if (again) {
 		(void)zone_dump(zone, false);
@@ -15031,6 +15043,12 @@ zone_shutdown(isc_task_t *task, isc_event_t *event) {
 	 */
 	DNS_ZONE_SETFLAG(zone, DNS_ZONEFLG_SHUTDOWN);
 	free_needed = exit_check(zone);
+	/*
+	 * If a dump is in progress for the secure zone, defer detaching from
+	 * the raw zone as it may prevent the unsigned serial number from being
+	 * stored in the raw-format dump of the secure zone.  In this scenario,
+	 * dump_done() takes care of cleaning up the zone->raw reference.
+	 */
 	if (inline_secure(zone) && !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_DUMPING)) {
 		raw = zone->raw;
 		zone->raw = NULL;

util/copyrights

@@ -426,6 +426,7 @@
 ./bin/tests/system/inline/ns8/sign.sh		SH	2020,2021,2022
 ./bin/tests/system/inline/setup.sh		SH	2011,2012,2013,2014,2016,2017,2018,2019,2020,2021,2022
 ./bin/tests/system/inline/tests.sh		SH	2011,2012,2013,2014,2016,2017,2018,2019,2020,2021,2022
+./bin/tests/system/inline/tests_signed_zone_files.py	PYTHON	2022
 ./bin/tests/system/integrity/clean.sh		SH	2017,2018,2019,2020,2021,2022
 ./bin/tests/system/integrity/setup.sh		SH	2018,2019,2020,2021,2022
 ./bin/tests/system/integrity/tests.sh		SH	2017,2018,2019,2020,2021,2022