Fix update-policy per-type max quota bypass via counter desynchronization

The prescan and main update loops in DNS UPDATE processing both used the
same counter to index the maxbytype[] quota array.  The prescan loop
always incremented the counter, but the main loop had 14 continue paths
that skipped the increment.  This allowed an authenticated DDNS client to
craft an UPDATE message with padding records (e.g. CNAME+A pairs that
trigger CNAME-conflict skips) to shift the counter and read wrong quota
entries, bypassing per-type record limits entirely.

Fix by incrementing the counter unconditionally at the start of each
iteration in the main loop.

bin/tests/system/ssumaxtype/ns1/example.db.in

@@ -0,0 +1,21 @@
+; 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.
+
+$TTL 300
+@	IN SOA	ns.example. admin.example. (
+			1	; serial
+			3600	; refresh
+			900	; retry
+			604800	; expire
+			300	; minimum
+		)
+@	IN NS	ns.example.
+ns	IN A	10.53.0.1

bin/tests/system/ssumaxtype/ns1/maxrrperset.db.in

@@ -0,0 +1,21 @@
+; 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.
+
+$TTL 300
+@	IN SOA	ns.maxrrperset. admin.maxrrperset. (
+			1	; serial
+			3600	; refresh
+			900	; retry
+			604800	; expire
+			300	; minimum
+		)
+@	IN NS	ns.maxrrperset.
+ns	IN A	10.53.0.1

bin/tests/system/ssumaxtype/ns1/named.conf.j2

@@ -0,0 +1,53 @@
+/*
+ * 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.
+ */
+
+options {
+	query-source address 10.53.0.1;
+	notify-source 10.53.0.1;
+	transfer-source 10.53.0.1;
+	port @PORT@;
+	pid-file "named.pid";
+	listen-on { 10.53.0.1; };
+	listen-on-v6 { none; };
+	recursion no;
+	dnssec-validation no;
+};
+
+key rndc_key {
+	secret "1234abcd8765";
+	algorithm @DEFAULT_HMAC@;
+};
+
+controls {
+	inet 10.53.0.1 port @CONTROLPORT@ allow { any; } keys { rndc_key; };
+};
+
+key "ddns-key" {
+	algorithm @DEFAULT_HMAC@;
+	secret "c2VjcmV0";
+};
+
+zone "example" {
+	type primary;
+	file "example.db";
+	update-policy {
+		grant ddns-key subdomain example. CNAME A TXT(3);
+	};
+};
+
+zone "maxrrperset" {
+	type primary;
+	file "maxrrperset.db";
+	allow-update { any; };
+	max-records-per-type 10;
+};

bin/tests/system/ssumaxtype/setup.sh

@@ -0,0 +1,18 @@
+#!/bin/sh -e
+
+# 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.
+
+# shellcheck source=conf.sh
+. ../conf.sh
+
+cp ns1/example.db.in ns1/example.db
+cp ns1/maxrrperset.db.in ns1/maxrrperset.db

bin/tests/system/ssumaxtype/tests_ssumaxtype.py

@@ -0,0 +1,154 @@
+# 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.
+
+"""
+Regression test for GL#5799: counter desynchronization in update-policy
+max-records-per-type enforcement.
+
+The prescan and main update loops used the same counter to index the
+maxbytype[] array, but the main loop had continue paths that skipped the
+increment, causing subsequent records to be checked against the wrong
+quota values.
+
+An attacker could craft an UPDATE message with CNAME-conflict padding
+records (which are silently skipped in the main loop) to shift the
+counter and bypass the per-type quota.
+"""
+
+import dns.exception
+import dns.name
+import dns.rcode
+import dns.rdatatype
+import dns.tsig
+import dns.tsigkeyring
+import dns.update
+import pytest
+
+import isctest
+
+pytestmark = pytest.mark.extra_artifacts(
+    [
+        "*/*.db",
+        "*/*.db.jnl",
+    ]
+)
+
+KEYRING = dns.tsigkeyring.from_text({"ddns-key.": "c2VjcmV0"})
+KEYNAME = dns.name.from_text("ddns-key.")
+KEYALGO = dns.tsig.HMAC_SHA256
+
+
+def make_update():
+    """Create a TSIG-signed UpdateMessage for the example zone."""
+    return dns.update.UpdateMessage(
+        "example.",
+        keyring=KEYRING,
+        keyname=KEYNAME,
+        keyalgorithm=KEYALGO,
+    )
+
+
+def count_txt(ns1, name):
+    """Query for TXT records at name and return the count."""
+    msg = isctest.query.create(name, "TXT")
+    try:
+        res = isctest.query.udp(msg, ns1.ip, port=ns1.ports.dns, attempts=3)
+    except dns.exception.Timeout:
+        return 0
+    for rrset in res.answer:
+        if rrset.rdtype == dns.rdatatype.TXT:
+            return len(rrset)
+    return 0
+
+
+def test_ssu_max_basic(ns1):
+    """Verify that update-policy max limit is enforced for normal updates."""
+    # Add 4 TXT records; policy allows max 3
+    up = make_update()
+    up.add("basic.example.", 300, "TXT", "record1")
+    up.add("basic.example.", 300, "TXT", "record2")
+    up.add("basic.example.", 300, "TXT", "record3")
+    up.add("basic.example.", 300, "TXT", "record4")
+    ns1.nsupdate(up)
+
+    assert count_txt(ns1, "basic.example.") == 3
+
+
+def test_ssu_max_across_updates(ns1):
+    """Quota is enforced across multiple UPDATE messages."""
+    # Fill up to the limit
+    up = make_update()
+    up.add("multi.example.", 300, "TXT", "first")
+    up.add("multi.example.", 300, "TXT", "second")
+    up.add("multi.example.", 300, "TXT", "third")
+    ns1.nsupdate(up)
+    assert count_txt(ns1, "multi.example.") == 3
+
+    # Try to add one more in a separate update
+    up = make_update()
+    up.add("multi.example.", 300, "TXT", "fourth")
+    ns1.nsupdate(up)
+    assert count_txt(ns1, "multi.example.") == 3
+
+
+def test_ssu_max_cname_padding_bypass(ns1):
+    """CNAME-conflict padding must not shift the maxbytype counter."""
+    # 4 CNAME+A padding pairs: the A records will be silently skipped
+    # because a CNAME already exists at each pad name.  Without the fix,
+    # this shifts the maxbytype counter by 4, causing the subsequent TXT
+    # records to read unlimited (0) quota entries instead of 3.
+    up = make_update()
+    up.add("pad1.example.", 300, "CNAME", "x.example.")
+    up.add("pad1.example.", 300, "A", "198.51.100.1")
+    up.add("pad2.example.", 300, "CNAME", "x.example.")
+    up.add("pad2.example.", 300, "A", "198.51.100.2")
+    up.add("pad3.example.", 300, "CNAME", "x.example.")
+    up.add("pad3.example.", 300, "A", "198.51.100.3")
+    up.add("pad4.example.", 300, "CNAME", "x.example.")
+    up.add("pad4.example.", 300, "A", "198.51.100.4")
+    up.add("target.example.", 300, "TXT", "data1")
+    up.add("target.example.", 300, "TXT", "data2")
+    up.add("target.example.", 300, "TXT", "data3")
+    up.add("target.example.", 300, "TXT", "data4")
+    ns1.nsupdate(up)
+
+    # With the fix: only 3 TXT records are added (4th rejected by quota)
+    # Without the fix: all 4 are added (quota bypassed via counter shift)
+    assert count_txt(ns1, "target.example.") == 3
+
+
+def count_a(ns1, name):
+    """Query for A records at name and return the count."""
+    msg = isctest.query.create(name, "A")
+    try:
+        res = isctest.query.udp(msg, ns1.ip, port=ns1.ports.dns, attempts=3)
+    except dns.exception.Timeout:
+        return 0
+    for rrset in res.answer:
+        if rrset.rdtype == dns.rdatatype.A:
+            return len(rrset)
+    return 0
+
+
+def test_max_records_per_type(ns1):
+    """Zone option max-records-per-type rejects updates that exceed the limit."""
+    # Add 10 A records; zone allows max 10 per type
+    up = dns.update.UpdateMessage("maxrrperset.")
+    for i in range(1, 11):
+        up.add("a.maxrrperset.", 300, "A", f"192.0.2.{i}")
+    ns1.nsupdate(up)
+    assert count_a(ns1, "a.maxrrperset.") == 10
+
+    # Adding an 11th must fail (SERVFAIL — entire update is rolled back)
+    up = dns.update.UpdateMessage("maxrrperset.")
+    up.add("a.maxrrperset.", 300, "A", "192.0.2.11")
+    ns1.nsupdate(up, expected_rcode=dns.rcode.SERVFAIL)
+    assert count_a(ns1, "a.maxrrperset.") == 10

lib/ns/update.c

@@ -2773,8 +2773,9 @@ update_action(void *arg) {
 		dns_ttl_t ttl;
 		dns_rdataclass_t update_class;
 		bool flag;
+		size_t maxidx = update++;
 
-		INSIST(ssutable == NULL || update < maxbytypelen);
+		INSIST(ssutable == NULL || maxidx < maxbytypelen);
 
 		get_current_rr(zoneclass, name, &rdata, &covers, &ttl,
 			       &update_class);
@@ -2918,17 +2919,17 @@ update_action(void *arg) {
 				}
 			}
 
-			if (maxbytype != NULL && maxbytype[update] != 0) {
+			if (maxbytype != NULL && maxbytype[maxidx] != 0) {
 				unsigned int count = 0;
 				CHECK(foreach_rr(db, ver, name, rdata.type,
 						 covers, count_action, &count));
-				if (count >= maxbytype[update]) {
+				if (count >= maxbytype[maxidx]) {
 					update_log(client, zone,
 						   LOGLEVEL_PROTOCOL,
 						   "attempt to add more "
 						   "records than permitted by "
 						   "policy max=%u",
-						   maxbytype[update]);
+						   maxbytype[maxidx]);
 					continue;
 				}
 			}
@@ -3123,8 +3124,6 @@ update_action(void *arg) {
 			CHECK(delete_if(rr_equal_p, db, ver, name, rdata.type,
 					covers, &rdata, &diff));
 		}
-
-		++update;
 	}
 
 	/*