diff --git a/bin/tests/system/ssumaxtype/ns1/example.db.in b/bin/tests/system/ssumaxtype/ns1/example.db.in new file mode 100644 index 0000000000..dd200dc9bc --- /dev/null +++ b/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 diff --git a/bin/tests/system/ssumaxtype/ns1/maxrrperset.db.in b/bin/tests/system/ssumaxtype/ns1/maxrrperset.db.in new file mode 100644 index 0000000000..90f9a5190a --- /dev/null +++ b/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 diff --git a/bin/tests/system/ssumaxtype/ns1/named.conf.j2 b/bin/tests/system/ssumaxtype/ns1/named.conf.j2 new file mode 100644 index 0000000000..4af979e900 --- /dev/null +++ b/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; +}; diff --git a/bin/tests/system/ssumaxtype/setup.sh b/bin/tests/system/ssumaxtype/setup.sh new file mode 100644 index 0000000000..0001ca8c7a --- /dev/null +++ b/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 diff --git a/bin/tests/system/ssumaxtype/tests_ssumaxtype.py b/bin/tests/system/ssumaxtype/tests_ssumaxtype.py new file mode 100644 index 0000000000..ef1edee72b --- /dev/null +++ b/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 diff --git a/lib/ns/update.c b/lib/ns/update.c index 8dd26a8ad6..0f959d1817 100644 --- a/lib/ns/update.c +++ b/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; } /*