From 89c363b555c764f476c24c6cf2986aa115e8b05f Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 7 Apr 2026 16:39:57 +0200 Subject: [PATCH 1/7] Fix assertion failure in dns_db_findrdataset() for SIG records dns__db_findrdataset() had a REQUIRE() that only accepted dns_rdatatype_rrsig when the covers parameter was set. A dynamic update containing a SIG record (type 24) would trigger this assertion, crashing named. Use dns_rdatatype_issig() to accept both SIG and RRSIG. (cherry picked from commit 03edeccaa16991e05c101ae82a03568ae1b2dbd0) --- lib/dns/db.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/dns/db.c b/lib/dns/db.c index 358ec710d8..7b575faaf2 100644 --- a/lib/dns/db.c +++ b/lib/dns/db.c @@ -668,7 +668,8 @@ dns_db_findrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, REQUIRE(node != NULL); REQUIRE(DNS_RDATASET_VALID(rdataset)); REQUIRE(!dns_rdataset_isassociated(rdataset)); - REQUIRE(covers == 0 || type == dns_rdatatype_rrsig); + REQUIRE(covers == dns_rdatatype_none || type == dns_rdatatype_rrsig || + type == dns_rdatatype_sig); REQUIRE(type != dns_rdatatype_any); REQUIRE(sigrdataset == NULL || (DNS_RDATASET_VALID(sigrdataset) && From 88a87068fa18f1bfd8bdc6012bef6a5c007cb7ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 7 Apr 2026 16:40:36 +0200 Subject: [PATCH 2/7] Add system test for SIG record handling in update-policy tcp-self Verify that a SIG record sent via TCP dynamic update is accepted by the tcp-self update-policy and correctly stored in the zone. (cherry picked from commit ecddeab6962fe7aab70ccaff56fab8f496316a9b) --- .../system/nsupdate/tests_tcp_self_sig.py | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 bin/tests/system/nsupdate/tests_tcp_self_sig.py diff --git a/bin/tests/system/nsupdate/tests_tcp_self_sig.py b/bin/tests/system/nsupdate/tests_tcp_self_sig.py new file mode 100644 index 0000000000..3378097e30 --- /dev/null +++ b/bin/tests/system/nsupdate/tests_tcp_self_sig.py @@ -0,0 +1,88 @@ +# 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#5818: update-policy tcp-self must handle SIG records. + +The dns_db_findrdataset() REQUIRE check only accepted dns_rdatatype_rrsig +for the covers parameter, causing named to abort when processing a SIG +record (type 24) via dynamic update with tcp-self policy. +""" + +import dns.rcode +import dns.rdata +import dns.rdataclass +import dns.rdataset +import dns.rdatatype +import dns.update + +import isctest + + +def _make_sig_rdata(text): + """Create a SIG rdata from text. + + dnspython has no native text parser for the legacy SIG type (24), + but the wire format is identical to RRSIG (46). Parse as RRSIG, + then re-wrap as SIG via the wire representation. + """ + rrsig = dns.rdata.from_text(dns.rdataclass.IN, dns.rdatatype.RRSIG, text) + wire = rrsig.to_digestable() + return dns.rdata.from_wire(dns.rdataclass.IN, dns.rdatatype.SIG, wire, 0, len(wire)) + + +def test_tcp_self_sig_record(ns6): + """Verify that update-policy tcp-self accepts a SIG record via TCP. + + The node must already exist (have at least one RR) so that + dns_db_findrdataset() is called during the update — that is the + function whose REQUIRE was too strict. We therefore add a PTR + record first. + """ + # First, create the node by adding a PTR record (allowed by tcp-self). + ptr_update = dns.update.UpdateMessage("in-addr.arpa.") + ptr_update.add("1.0.0.127.in-addr.arpa.", 600, "PTR", "localhost.") + response = isctest.query.tcp( + ptr_update, ns6.ip, port=ns6.ports.dns, source="127.0.0.1" + ) + assert response.rcode() == dns.rcode.NOERROR + + # Now add a SIG record at the same node — this triggers the + # dns_db_findrdataset() call with type=SIG and covers=A. + sig = _make_sig_rdata("A 6 0 86400 20260331170000 20260318160000 21831 . 0000") + rds = dns.rdataset.Rdataset(dns.rdataclass.IN, dns.rdatatype.SIG) + rds.update_ttl(600) + rds.add(sig) + + sig_update = dns.update.UpdateMessage("in-addr.arpa.") + sig_update.add("1.0.0.127.in-addr.arpa.", rds) + + with ns6.watch_log_from_here() as watcher: + response = isctest.query.tcp( + sig_update, ns6.ip, port=ns6.ports.dns, source="127.0.0.1" + ) + assert response.rcode() == dns.rcode.NOERROR + + watcher.wait_for_sequence( + [ + "update-policy: using: signer= name=1.0.0.127.in-addr.arpa" + " addr=127.0.0.1 tcp=1 type=SIG target=", + "update-policy: trying: grant * tcp-self . PTR(1) ANY(2) A", + "update-policy: tcp-self=1.0.0.127.IN-ADDR.ARPA", + "update-policy: matched: grant * tcp-self . PTR(1) ANY(2) A", + ] + ) + + # Verify the SIG record was actually stored + msg = isctest.query.create("1.0.0.127.in-addr.arpa.", "SIG") + res = isctest.query.tcp(msg, ns6.ip, port=ns6.ports.dns) + found = any(rrset.rdtype == dns.rdatatype.SIG for rrset in res.answer) + assert found, "SIG record not found in answer section" From b575dbfd9e976ebe06f16b677b92062cc8bb5fef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 16 Apr 2026 11:21:48 +0200 Subject: [PATCH 3/7] Fix dropped covers field for SIG records in dns_diff_apply rdata_covers() in lib/dns/diff.c discriminated only on dns_rdatatype_rrsig (46) and returned 0 for the legacy SIG (24), so the covered-type field was silently discarded on the dynamic-update and IXFR paths. Every SIG rdataset was then filed in the zone DB under typepair (SIG, 0) instead of (SIG, covered_type); a second SIG add with a different covers but a different TTL collided at that bucket, tripped DNS_DBADD_EXACTTTL in qpzone, returned DNS_R_NOTEXACT, and came back to the client as SERVFAIL. Use dns_rdatatype_issig() here so both SIG and RRSIG carry their covers through the diff, matching the helper pattern already used in lib/dns/master.c, lib/ns/xfrout.c, lib/dns/qpcache.c, and the dns__db_findrdataset() REQUIRE that the surrounding merge request just relaxed. (cherry picked from commit 0a5ba57116857779680d76fe4b20e125f3bc3b71) --- lib/dns/diff.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/dns/diff.c b/lib/dns/diff.c index a6a78f5d16..3529224558 100644 --- a/lib/dns/diff.c +++ b/lib/dns/diff.c @@ -40,7 +40,13 @@ static dns_rdatatype_t rdata_covers(dns_rdata_t *rdata) { - return rdata->type == dns_rdatatype_rrsig ? dns_rdata_covers(rdata) : 0; + if (rdata->type == dns_rdatatype_rrsig || + rdata->type == dns_rdatatype_sig) + { + return dns_rdata_covers(rdata); + } + + return 0; } isc_result_t From c3ccf8b287778bf9f9ddc29584c1d9ccc7f42370 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 16 Apr 2026 11:16:40 +0200 Subject: [PATCH 4/7] Add regression test for SIG covers being dropped in dns_diff_apply rdata_covers() in lib/dns/diff.c tests `type == dns_rdatatype_rrsig` instead of dns_rdatatype_issig(), so for a legacy SIG (24) rdata it returns 0 and the covered type is discarded on the dynamic-update / IXFR path. The zone DB then files every SIG rdataset under typepair (SIG, 0) instead of (SIG, covered_type), and a follow-up add with a different covers field but a different TTL collides at that bucket, trips DNS_DBADD_EXACTTTL in qpzone, returns DNS_R_NOTEXACT, and comes back to the client as SERVFAIL. The new test adds a PTR to establish the node (tcp-self requires the client IP's reverse form to equal the owner), then two SIG updates with different covers and different TTLs; on a buggy build the second update is SERVFAIL and named logs `dns_diff_apply: .../SIG/IN: add not exact`. The test is expected to pass once rdata_covers() is switched to dns_rdatatype_issig(), matching the fix already adopted for dns__db_findrdataset() on this branch and the helper pattern used in master.c, xfrout.c, and qpcache.c. (cherry picked from commit b9fc0e595b5f907654bc241efee5f942d57b7903) --- .../system/nsupdate/tests_tcp_self_sig.py | 79 ++++++++++++++++++- 1 file changed, 75 insertions(+), 4 deletions(-) diff --git a/bin/tests/system/nsupdate/tests_tcp_self_sig.py b/bin/tests/system/nsupdate/tests_tcp_self_sig.py index 3378097e30..1a90e7b90e 100644 --- a/bin/tests/system/nsupdate/tests_tcp_self_sig.py +++ b/bin/tests/system/nsupdate/tests_tcp_self_sig.py @@ -10,11 +10,20 @@ # information regarding copyright ownership. """ -Regression test for GL#5818: update-policy tcp-self must handle SIG records. +Regression tests for GL#5818: SIG (type 24) records on the dynamic-update path. -The dns_db_findrdataset() REQUIRE check only accepted dns_rdatatype_rrsig -for the covers parameter, causing named to abort when processing a SIG -record (type 24) via dynamic update with tcp-self policy. +1. test_tcp_self_sig_record: + The dns_db_findrdataset() REQUIRE check only accepted + dns_rdatatype_rrsig for the covers parameter, causing named to abort + when processing a SIG record via dynamic update with tcp-self policy. + +2. test_sig_covers_preserved_in_diff: + The rdata_covers() helper in lib/dns/diff.c only recognised RRSIG (46), + so it dropped the covered-type field for legacy SIG (24) records. The + zone DB then filed every SIG rdataset under typepair (SIG, 0) instead + of (SIG, covered_type). A second SIG add with a different covers and + a different TTL collided at that bucket, tripped DNS_DBADD_EXACTTTL + in qpzone, and came back as SERVFAIL. """ import dns.rcode @@ -86,3 +95,65 @@ def test_tcp_self_sig_record(ns6): res = isctest.query.tcp(msg, ns6.ip, port=ns6.ports.dns) found = any(rrset.rdtype == dns.rdatatype.SIG for rrset in res.answer) assert found, "SIG record not found in answer section" + + +def test_sig_covers_preserved_in_diff(ns6): + """Regression test for GL#5818 Finding 1. + + lib/dns/diff.c rdata_covers() only recognised RRSIG and returned 0 + for SIG (24), so the zone DB stored every SIG rdataset under + typepair (SIG, 0) instead of (SIG, covered_type). The second add + at the same name with a different covers field but a different TTL + then targeted the same bucket, hit DNS_DBADD_EXACTTTL in qpzone's + add(), and returned DNS_R_NOTEXACT -- which dns_diff_apply + propagates as SERVFAIL. + + With the fix (rdata_covers using dns_rdatatype_issig), the two + records land in separate typepairs and both updates succeed. + """ + # tcp-self requires the client IP in reverse form to equal the + # update's owner name. Use a distinct (source, owner) pair so + # this test does not interact with test_tcp_self_sig_record. + source = "127.0.0.5" + owner = "5.0.0.127.in-addr.arpa." + + # Create the node with a PTR (allowed by tcp-self). + ptr = dns.update.UpdateMessage("in-addr.arpa.") + ptr.add(owner, 600, "PTR", "localhost.") + response = isctest.query.tcp(ptr, ns6.ip, port=ns6.ports.dns, source=source) + assert response.rcode() == dns.rcode.NOERROR + + # First SIG: covers=A, TTL=600. + sig_a = _make_sig_rdata("A 6 0 600 20260331170000 20260318160000 21831 . 0000") + rds_a = dns.rdataset.Rdataset(dns.rdataclass.IN, dns.rdatatype.SIG) + rds_a.update_ttl(600) + rds_a.add(sig_a) + upd_a = dns.update.UpdateMessage("in-addr.arpa.") + upd_a.add(owner, rds_a) + response = isctest.query.tcp(upd_a, ns6.ip, port=ns6.ports.dns, source=source) + assert response.rcode() == dns.rcode.NOERROR + + # Second SIG: different covers (MX) and different TTL (1200). With + # the fix this lands in typepair (SIG, MX) and succeeds. Without + # the fix it collides with the first record at typepair (SIG, 0), + # the TTL mismatch trips DNS_DBADD_EXACTTTL in qpzone, and + # dns_diff_apply returns DNS_R_NOTEXACT -> SERVFAIL. + sig_mx = _make_sig_rdata("MX 6 0 1200 20260331170000 20260318160000 21831 . 0000") + rds_mx = dns.rdataset.Rdataset(dns.rdataclass.IN, dns.rdatatype.SIG) + rds_mx.update_ttl(1200) + rds_mx.add(sig_mx) + upd_mx = dns.update.UpdateMessage("in-addr.arpa.") + upd_mx.add(owner, rds_mx) + response = isctest.query.tcp(upd_mx, ns6.ip, port=ns6.ports.dns, source=source) + assert response.rcode() == dns.rcode.NOERROR, ( + f"second SIG add returned {dns.rcode.to_text(response.rcode())}; Finding 1 (rdata_covers dropping " + "covers for SIG) is likely still present" + ) + + # Both SIG rdatas must be retrievable. + q = isctest.query.create(owner, "SIG") + res = isctest.query.tcp(q, ns6.ip, port=ns6.ports.dns) + sig_count = sum( + 1 for rrset in res.answer if rrset.rdtype == dns.rdatatype.SIG for _ in rrset + ) + assert sig_count == 2, f"expected 2 SIG rdatas, got {sig_count}" From a61f9a09d70f5b71f6e44b16f6f9e29c38d3ed7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 16 Apr 2026 12:23:34 +0200 Subject: [PATCH 5/7] Refuse SIG and NXT records in dynamic updates SIG (24) and NXT (30) are obsolete DNSSEC record types, superseded by RRSIG and NSEC in RFC 3755. Allowing them through dynamic update exposes two distinct bugs that the surrounding GL#5818 work already fixes as defense-in-depth: - dns__db_findrdataset() used to REQUIRE that (covers == 0 || type == RRSIG), which aborts named when a SIG update reaches the prescan foreach_rr() call. Fixed to accept dns_rdatatype_issig(). - diff.c rdata_covers() used to test only RRSIG, dropping the covered-type field for SIG rdatas; the zone DB then filed every SIG rdataset under typepair (SIG, 0) instead of (SIG, covered_type) and follow-up adds collided at that bucket. Fixed to use dns_rdatatype_issig(). Both underlying bugs are still reachable via inbound zone transfer (diff.c rdata_covers() runs from both dns_diff_apply on the IXFR path and dns_diff_load on the AXFR path), so the type-helper fixes above remain necessary. For the dynamic-update path, the simplest and safest posture is to refuse SIG and NXT outright at the front door in ns/update.c, alongside the existing NSEC/NSEC3/non-apex-RRSIG refusals. KEY remains permitted because it is still used to carry public keys for SIG(0) transaction authentication. The existing tcp-self SIG regression test is repointed to assert REFUSED on the SIG add, a symmetric NXT test is added, and the SIG-via-dyn-update covers-bucket test is removed because it is no longer reachable through this entry point; AXFR-based coverage of diff.c rdata_covers() follows in a separate commit. (cherry picked from commit 3a44a13232991ba054aa157a1c3ffb8376359b42) --- .../system/nsupdate/tests_tcp_self_sig.py | 159 +++++++----------- lib/ns/update.c | 6 + 2 files changed, 71 insertions(+), 94 deletions(-) diff --git a/bin/tests/system/nsupdate/tests_tcp_self_sig.py b/bin/tests/system/nsupdate/tests_tcp_self_sig.py index 1a90e7b90e..574497338c 100644 --- a/bin/tests/system/nsupdate/tests_tcp_self_sig.py +++ b/bin/tests/system/nsupdate/tests_tcp_self_sig.py @@ -10,20 +10,25 @@ # information regarding copyright ownership. """ -Regression tests for GL#5818: SIG (type 24) records on the dynamic-update path. +Regression tests for GL#5818: legacy DNSSEC types on the dynamic-update path. -1. test_tcp_self_sig_record: - The dns_db_findrdataset() REQUIRE check only accepted - dns_rdatatype_rrsig for the covers parameter, causing named to abort - when processing a SIG record via dynamic update with tcp-self policy. +SIG (24) and NXT (30) are obsolete DNSSEC record types, superseded by RRSIG +and NSEC in RFC 3755. Allowing a client to inject them via dynamic update +exposed two bugs in sequence: -2. test_sig_covers_preserved_in_diff: - The rdata_covers() helper in lib/dns/diff.c only recognised RRSIG (46), - so it dropped the covered-type field for legacy SIG (24) records. The - zone DB then filed every SIG rdataset under typepair (SIG, 0) instead - of (SIG, covered_type). A second SIG add with a different covers and - a different TTL collided at that bucket, tripped DNS_DBADD_EXACTTTL - in qpzone, and came back as SERVFAIL. + * dns__db_findrdataset() asserted `covers == 0 || type == RRSIG`, which + crashed named when a SIG update reached the prescan foreach_rr() call. + * diff.c rdata_covers() dropped the covered type for SIG rdatas, so the + zone DB stored every SIG rdataset under typepair (SIG, 0) instead of + (SIG, covered_type); a second SIG add with a different covers and + different TTL then tripped DNS_DBADD_EXACTTTL in qpzone and came back + as SERVFAIL. + +The adopted defence is to outright refuse SIG and NXT updates at the front +door (ns/update.c), keeping KEY updates permitted for SIG(0) transaction +signatures. These tests verify the refusal. The reachability of the +diff.c:rdata_covers() bug via inbound zone transfer is covered separately +by the AXFR-based regression test in this file. """ import dns.rcode @@ -48,15 +53,29 @@ def _make_sig_rdata(text): return dns.rdata.from_wire(dns.rdataclass.IN, dns.rdatatype.SIG, wire, 0, len(wire)) -def test_tcp_self_sig_record(ns6): - """Verify that update-policy tcp-self accepts a SIG record via TCP. +def _make_nxt_rdata(): + """Create a minimal NXT rdata. - The node must already exist (have at least one RR) so that - dns_db_findrdataset() is called during the update — that is the - function whose REQUIRE was too strict. We therefore add a PTR - record first. + NXT wire format (RFC 2535) is: next-name + type-bitmap. The exact + content does not matter for the refusal test; we just need a + syntactically valid NXT rdata. + """ + # next-name = root (\x00), type bitmap covering type A only. + wire = b"\x00\x00\x00\x00\x40" + return dns.rdata.from_wire(dns.rdataclass.IN, dns.rdatatype.NXT, wire, 0, len(wire)) + + +def test_tcp_self_sig_record(ns6): + """SIG (type 24) updates must be refused at the front door. + + Prior to the fix in dns__db_findrdataset(), a SIG update here + crashed named. Prior to the fix in diff.c rdata_covers(), the + record was silently misfiled under typepair (SIG, 0). The + adopted policy outright refuses SIG (obsolete; use RRSIG) so the + buggy dynamic-update paths are no longer reachable. A PTR add + first ensures the node exists, which is the original + crash-reproducing precondition. """ - # First, create the node by adding a PTR record (allowed by tcp-self). ptr_update = dns.update.UpdateMessage("in-addr.arpa.") ptr_update.add("1.0.0.127.in-addr.arpa.", 600, "PTR", "localhost.") response = isctest.query.tcp( @@ -64,96 +83,48 @@ def test_tcp_self_sig_record(ns6): ) assert response.rcode() == dns.rcode.NOERROR - # Now add a SIG record at the same node — this triggers the - # dns_db_findrdataset() call with type=SIG and covers=A. sig = _make_sig_rdata("A 6 0 86400 20260331170000 20260318160000 21831 . 0000") rds = dns.rdataset.Rdataset(dns.rdataclass.IN, dns.rdatatype.SIG) rds.update_ttl(600) rds.add(sig) - sig_update = dns.update.UpdateMessage("in-addr.arpa.") sig_update.add("1.0.0.127.in-addr.arpa.", rds) - with ns6.watch_log_from_here() as watcher: - response = isctest.query.tcp( - sig_update, ns6.ip, port=ns6.ports.dns, source="127.0.0.1" - ) - assert response.rcode() == dns.rcode.NOERROR + response = isctest.query.tcp( + sig_update, ns6.ip, port=ns6.ports.dns, source="127.0.0.1" + ) + assert response.rcode() == dns.rcode.REFUSED - watcher.wait_for_sequence( - [ - "update-policy: using: signer= name=1.0.0.127.in-addr.arpa" - " addr=127.0.0.1 tcp=1 type=SIG target=", - "update-policy: trying: grant * tcp-self . PTR(1) ANY(2) A", - "update-policy: tcp-self=1.0.0.127.IN-ADDR.ARPA", - "update-policy: matched: grant * tcp-self . PTR(1) ANY(2) A", - ] - ) - - # Verify the SIG record was actually stored + # Confirm nothing of type SIG was stored. msg = isctest.query.create("1.0.0.127.in-addr.arpa.", "SIG") res = isctest.query.tcp(msg, ns6.ip, port=ns6.ports.dns) - found = any(rrset.rdtype == dns.rdatatype.SIG for rrset in res.answer) - assert found, "SIG record not found in answer section" + stored = any(rrset.rdtype == dns.rdatatype.SIG for rrset in res.answer) + assert not stored, "SIG record was stored despite REFUSED response" -def test_sig_covers_preserved_in_diff(ns6): - """Regression test for GL#5818 Finding 1. +def test_tcp_self_nxt_record(ns6): + """NXT (type 30) updates must be refused at the front door. - lib/dns/diff.c rdata_covers() only recognised RRSIG and returned 0 - for SIG (24), so the zone DB stored every SIG rdataset under - typepair (SIG, 0) instead of (SIG, covered_type). The second add - at the same name with a different covers field but a different TTL - then targeted the same bucket, hit DNS_DBADD_EXACTTTL in qpzone's - add(), and returned DNS_R_NOTEXACT -- which dns_diff_apply - propagates as SERVFAIL. - - With the fix (rdata_covers using dns_rdatatype_issig), the two - records land in separate typepairs and both updates succeed. + NXT is the legacy DNSSEC denial-of-existence type, obsolete since + RFC 3755 replaced it with NSEC. Accepting it via dynamic update + would let an authorised updater inject records that the signing + and cut-point logic has no provision for. """ - # tcp-self requires the client IP in reverse form to equal the - # update's owner name. Use a distinct (source, owner) pair so - # this test does not interact with test_tcp_self_sig_record. - source = "127.0.0.5" - owner = "5.0.0.127.in-addr.arpa." + # A second owner under a source that also matches tcp-self. + source = "127.0.0.2" + owner = "2.0.0.127.in-addr.arpa." - # Create the node with a PTR (allowed by tcp-self). - ptr = dns.update.UpdateMessage("in-addr.arpa.") - ptr.add(owner, 600, "PTR", "localhost.") - response = isctest.query.tcp(ptr, ns6.ip, port=ns6.ports.dns, source=source) + ptr_update = dns.update.UpdateMessage("in-addr.arpa.") + ptr_update.add(owner, 600, "PTR", "localhost.") + response = isctest.query.tcp(ptr_update, ns6.ip, port=ns6.ports.dns, source=source) assert response.rcode() == dns.rcode.NOERROR - # First SIG: covers=A, TTL=600. - sig_a = _make_sig_rdata("A 6 0 600 20260331170000 20260318160000 21831 . 0000") - rds_a = dns.rdataset.Rdataset(dns.rdataclass.IN, dns.rdatatype.SIG) - rds_a.update_ttl(600) - rds_a.add(sig_a) - upd_a = dns.update.UpdateMessage("in-addr.arpa.") - upd_a.add(owner, rds_a) - response = isctest.query.tcp(upd_a, ns6.ip, port=ns6.ports.dns, source=source) - assert response.rcode() == dns.rcode.NOERROR + nxt = _make_nxt_rdata() + rds = dns.rdataset.Rdataset(dns.rdataclass.IN, dns.rdatatype.NXT) + rds.update_ttl(600) + rds.add(nxt) + nxt_update = dns.update.UpdateMessage("in-addr.arpa.") + nxt_update.add(owner, rds) - # Second SIG: different covers (MX) and different TTL (1200). With - # the fix this lands in typepair (SIG, MX) and succeeds. Without - # the fix it collides with the first record at typepair (SIG, 0), - # the TTL mismatch trips DNS_DBADD_EXACTTTL in qpzone, and - # dns_diff_apply returns DNS_R_NOTEXACT -> SERVFAIL. - sig_mx = _make_sig_rdata("MX 6 0 1200 20260331170000 20260318160000 21831 . 0000") - rds_mx = dns.rdataset.Rdataset(dns.rdataclass.IN, dns.rdatatype.SIG) - rds_mx.update_ttl(1200) - rds_mx.add(sig_mx) - upd_mx = dns.update.UpdateMessage("in-addr.arpa.") - upd_mx.add(owner, rds_mx) - response = isctest.query.tcp(upd_mx, ns6.ip, port=ns6.ports.dns, source=source) - assert response.rcode() == dns.rcode.NOERROR, ( - f"second SIG add returned {dns.rcode.to_text(response.rcode())}; Finding 1 (rdata_covers dropping " - "covers for SIG) is likely still present" - ) - - # Both SIG rdatas must be retrievable. - q = isctest.query.create(owner, "SIG") - res = isctest.query.tcp(q, ns6.ip, port=ns6.ports.dns) - sig_count = sum( - 1 for rrset in res.answer if rrset.rdtype == dns.rdatatype.SIG for _ in rrset - ) - assert sig_count == 2, f"expected 2 SIG rdatas, got {sig_count}" + response = isctest.query.tcp(nxt_update, ns6.ip, port=ns6.ports.dns, source=source) + assert response.rcode() == dns.rcode.REFUSED diff --git a/lib/ns/update.c b/lib/ns/update.c index c02535130f..415836fd13 100644 --- a/lib/ns/update.c +++ b/lib/ns/update.c @@ -1728,6 +1728,12 @@ send_update_event(ns_client_t *client, dns_zone_t *zone) { } else if (rdata.type == dns_rdatatype_nsec) { FAILC(DNS_R_REFUSED, "explicit NSEC updates are not " "allowed in secure zones"); + } else if (rdata.type == dns_rdatatype_sig) { + FAILC(DNS_R_REFUSED, "SIG updates are not " + "allowed"); + } else if (rdata.type == dns_rdatatype_nxt) { + FAILC(DNS_R_REFUSED, "NXT updates are not " + "allowed"); } else if (rdata.type == dns_rdatatype_rrsig && !dns_name_equal(name, zonename)) { From a625a797e3a69f98877832e42e11f519ce813d37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 16 Apr 2026 13:25:03 +0200 Subject: [PATCH 6/7] Add AXFR regression test for SIG covers preservation diff.c rdata_covers() runs on both dns_diff_apply (IXFR, ns/update.c dynamic updates) and dns_diff_load (AXFR). After the previous commit refused SIG and NXT in dynamic updates, the AXFR path remains the most natural way to drive legacy SIG records into a secondary's zone DB and regression-gate the rdata_covers() fix. The test adds ans11 as an AsyncDnsServer primary for a small zone whose AXFR carries two SIG rdatas at the same owner with different covered types (A, MX) and different TTLs (600, 1200), and declares ns6 a secondary of that zone. With the bug present, dns_diff_load groups both tuples at typepair (SIG, 0) and the MX-covering record inherits the first-seen TTL (600); the fix keeps them at (SIG, A) and (SIG, MX) with their original TTLs. rndc dumpdb -zones on the secondary is used to inspect stored state directly, because the wire-level SIG query response merges same-(owner,type,class) RRs and masks the per-rdataset TTLs. (cherry picked from commit e9f880c78ff3c2f799763628060eca0f0d64e70f) --- bin/tests/system/nsupdate/ans11/ans.py | 117 ++++++++++++++++++ bin/tests/system/nsupdate/ns6/named.conf.in | 7 ++ .../system/nsupdate/tests_tcp_self_sig.py | 70 +++++++++++ 3 files changed, 194 insertions(+) create mode 100644 bin/tests/system/nsupdate/ans11/ans.py diff --git a/bin/tests/system/nsupdate/ans11/ans.py b/bin/tests/system/nsupdate/ans11/ans.py new file mode 100644 index 0000000000..177a889e3c --- /dev/null +++ b/bin/tests/system/nsupdate/ans11/ans.py @@ -0,0 +1,117 @@ +# 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. + +""" +GL#5818 Finding 1 regression support — AsyncDnsServer primary. + +Serves a minimal zone "sigaxfr.nil." whose AXFR carries two SIG records +at the same owner with different covered types (A and MX) and different +TTLs (600 and 1200). A buggy secondary running dns_diff_load() with +rdata_covers() that only recognises RRSIG will file both rdatas under +typepair (SIG, 0) with the first tuple's TTL; a fixed secondary keeps +them under (SIG, A) and (SIG, MX) with their distinct TTLs. +""" + +from collections.abc import AsyncGenerator + +import dns.name +import dns.rcode +import dns.rdata +import dns.rdataclass +import dns.rdatatype +import dns.rrset + +from isctest.asyncserver import ( + AsyncDnsServer, + DnsResponseSend, + DomainHandler, + QueryContext, + ResponseAction, +) + +ZONE = dns.name.from_text("sigaxfr.nil.") +NS_NAME = dns.name.from_text("ns.sigaxfr.nil.") +HOST = dns.name.from_text("host.sigaxfr.nil.") + +SOA_TEXT = "ns.sigaxfr.nil. hostmaster.sigaxfr.nil. 1 3600 1200 604800 3600" + + +def _make_sig_rdata(covered_text): + """Produce a legacy SIG (24) rdata via RRSIG (46) round-trip.""" + rrsig = dns.rdata.from_text(dns.rdataclass.IN, dns.rdatatype.RRSIG, covered_text) + wire = rrsig.to_digestable() + return dns.rdata.from_wire(dns.rdataclass.IN, dns.rdatatype.SIG, wire, 0, len(wire)) + + +class SigAxfrServer(DomainHandler): + """Serve SOA and AXFR for sigaxfr.nil.; other qtypes get NOERROR/NODATA.""" + + domains = ["sigaxfr.nil."] + + async def get_responses( + self, qctx: QueryContext + ) -> AsyncGenerator[ResponseAction, None]: + soa_rrset = dns.rrset.from_text( + ZONE, 3600, dns.rdataclass.IN, dns.rdatatype.SOA, SOA_TEXT + ) + + if qctx.qtype == dns.rdatatype.SOA: + resp = qctx.response + resp.answer.append(soa_rrset) + yield DnsResponseSend(resp) + return + + if qctx.qtype != dns.rdatatype.AXFR: + # Other types: empty NOERROR response. + yield DnsResponseSend(qctx.response) + return + + # AXFR: opening SOA, NS, NS's A, two SIG RRs at the same owner + # with distinct covered types and TTLs, closing SOA. + resp = qctx.response + resp.answer.append(soa_rrset) + + ns_rrset = dns.rrset.from_text( + ZONE, 3600, dns.rdataclass.IN, dns.rdatatype.NS, str(NS_NAME) + ) + resp.answer.append(ns_rrset) + + a_rrset = dns.rrset.from_text( + NS_NAME, 3600, dns.rdataclass.IN, dns.rdatatype.A, "10.53.0.11" + ) + resp.answer.append(a_rrset) + + sig_a = _make_sig_rdata("A 6 2 600 20260331170000 20260318160000 21831 . 0000") + sig_a_rrset = dns.rrset.RRset(HOST, dns.rdataclass.IN, dns.rdatatype.SIG) + sig_a_rrset.add(sig_a, ttl=600) + resp.answer.append(sig_a_rrset) + + sig_mx = _make_sig_rdata( + "MX 6 2 1200 20260331170000 20260318160000 21831 . 0000" + ) + sig_mx_rrset = dns.rrset.RRset(HOST, dns.rdataclass.IN, dns.rdatatype.SIG) + sig_mx_rrset.add(sig_mx, ttl=1200) + resp.answer.append(sig_mx_rrset) + + # Closing SOA terminates the AXFR. + resp.answer.append(soa_rrset) + + yield DnsResponseSend(resp) + + +def main() -> None: + server = AsyncDnsServer(default_aa=True, default_rcode=dns.rcode.NOERROR) + server.install_response_handler(SigAxfrServer()) + server.run() + + +if __name__ == "__main__": + main() diff --git a/bin/tests/system/nsupdate/ns6/named.conf.in b/bin/tests/system/nsupdate/ns6/named.conf.in index e2950aa3b7..54f9c8ff86 100644 --- a/bin/tests/system/nsupdate/ns6/named.conf.in +++ b/bin/tests/system/nsupdate/ns6/named.conf.in @@ -49,3 +49,10 @@ zone "2.0.0.2.ip6.arpa" { file "2.0.0.2.ip6.addr.db"; update-policy { grant * 6to4-self . NS(10) DS(4); }; }; + +zone "sigaxfr.nil" { + type secondary; + primaries { 10.53.0.11; }; + file "sigaxfr.bk"; + request-ixfr no; # ans11 serves AXFR only +}; diff --git a/bin/tests/system/nsupdate/tests_tcp_self_sig.py b/bin/tests/system/nsupdate/tests_tcp_self_sig.py index 574497338c..6e83aded29 100644 --- a/bin/tests/system/nsupdate/tests_tcp_self_sig.py +++ b/bin/tests/system/nsupdate/tests_tcp_self_sig.py @@ -31,6 +31,8 @@ diff.c:rdata_covers() bug via inbound zone transfer is covered separately by the AXFR-based regression test in this file. """ +import time + import dns.rcode import dns.rdata import dns.rdataclass @@ -102,6 +104,74 @@ def test_tcp_self_sig_record(ns6): assert not stored, "SIG record was stored despite REFUSED response" +def test_sig_covers_preserved_via_axfr(ns6): + """Regression test for GL#5818 Finding 1, reached via AXFR. + + ans11 serves an AXFR for sigaxfr.nil. containing two SIG rdatas at + the same owner with different covered types (A, MX) and different + TTLs (600, 1200). ns6 pulls the zone via dns_diff_load(), which + calls diff.c rdata_covers(); before the fix that helper returned 0 + for SIG, so both tuples were grouped and filed under typepair + (SIG, 0) with the first TTL (600) — the MX-covering record's TTL + (1200) was silently dropped. With the fix the records land in + distinct typepairs and both TTLs survive. + + rndc dumpdb is used to inspect the secondary's stored state + directly; the wire-level response can merge same-(owner,type,class) + RRs and mask the difference. + """ + zone = "sigaxfr.nil" + owner = f"host.{zone}." + dump_path = ns6.directory / "named_dump.db" + + # ns6 may have tried to SOA-poll ans11 before it was listening; force + # a fresh refresh attempt and wait for the transfer to complete. + with ns6.watch_log_from_here() as watcher: + ns6.rndc(f"refresh {zone}") + watcher.wait_for_line(f"zone {zone}/IN: transferred serial 1") + + # Remove any stale dump and ask named for a fresh one. + if dump_path.exists(): + dump_path.unlink() + ns6.rndc("dumpdb -zones") + + # rndc dumpdb is asynchronous; wait for the file and for its + # trailing "Dump complete" marker. + deadline_marker = "; Dump complete" + for _ in range(50): + if dump_path.exists(): + text = dump_path.read_text() + if deadline_marker in text: + break + time.sleep(0.1) + else: + raise AssertionError(f"{dump_path} never contained {deadline_marker!r}") + + # Collect every SIG line for the owner from the dump. Format is: + # . IN SIG ... + sig_lines = [] + for line in text.splitlines(): + fields = line.split() + if len(fields) < 6: + continue + if not fields[0].lower().startswith("host.sigaxfr.nil"): + continue + if fields[2] != "IN" or fields[3] != "SIG": + continue + sig_lines.append(fields) + + assert ( + len(sig_lines) == 2 + ), f"expected 2 SIG records at {owner}, got {len(sig_lines)}: {sig_lines}" + + ttl_by_covers = {fields[4]: int(fields[1]) for fields in sig_lines} + assert ttl_by_covers == {"A": 600, "MX": 1200}, ( + f"SIG records lost their covers/TTL binding: {ttl_by_covers}. With " + "the Finding 1 bug both records are filed under typepair (SIG, 0) " + "and share the first-seen TTL (600)." + ) + + def test_tcp_self_nxt_record(ns6): """NXT (type 30) updates must be refused at the front door. From ab16e86a5035347df8a23feb78e135020a1403e5 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Thu, 16 Apr 2026 16:17:34 +0200 Subject: [PATCH 7/7] Add test for SIG in prequisites of dynamic update Make sure the nameserver correctly handles SIG records in the prerequisites of the dynamic update. The first check is to ensure that the prerequisites are not examined prior to checking the credentials. The second test case checks that the SIG present prerequisite is examined and therefore refuses the update. Also this should not trigger an assertion failure in dns__db_findrdataset() (due to the REQUIRE() only accepted dns_rdatatype_rrsig when the covers parameter was set). (cherry picked from commit 51f27fda46870acea69835f82835ca857c000bd0) --- .../system/nsupdate/tests_sh_nsupdate.py | 1 + ...ts_tcp_self_sig.py => tests_update_sig.py} | 106 ++++++++++++------ 2 files changed, 71 insertions(+), 36 deletions(-) rename bin/tests/system/nsupdate/{tests_tcp_self_sig.py => tests_update_sig.py} (83%) diff --git a/bin/tests/system/nsupdate/tests_sh_nsupdate.py b/bin/tests/system/nsupdate/tests_sh_nsupdate.py index f3ddfe2c01..8239d668dd 100644 --- a/bin/tests/system/nsupdate/tests_sh_nsupdate.py +++ b/bin/tests/system/nsupdate/tests_sh_nsupdate.py @@ -58,6 +58,7 @@ pytestmark = pytest.mark.extra_artifacts( "ns5/local.db", "ns6/2.0.0.2.ip6.addr.db", "ns6/in-addr.db", + "ns6/sigaxfr.bk", "ns7/_default.tsigkeys", "ns7/example.com.db", "ns7/in-addr.db", diff --git a/bin/tests/system/nsupdate/tests_tcp_self_sig.py b/bin/tests/system/nsupdate/tests_update_sig.py similarity index 83% rename from bin/tests/system/nsupdate/tests_tcp_self_sig.py rename to bin/tests/system/nsupdate/tests_update_sig.py index 6e83aded29..261b47efa2 100644 --- a/bin/tests/system/nsupdate/tests_tcp_self_sig.py +++ b/bin/tests/system/nsupdate/tests_update_sig.py @@ -32,16 +32,38 @@ by the AXFR-based regression test in this file. """ import time +from pathlib import Path import dns.rcode import dns.rdata import dns.rdataclass import dns.rdataset import dns.rdatatype +import dns.tsigkeyring import dns.update +import pytest import isctest +pytestmark = pytest.mark.extra_artifacts( + [ + "ans*/ans.run", + "ns*/*.bk", + "ns*/*.conf", + "ns*/*.db", + "ns*/*.db.jnl", + "ns*/*.db.signed", + "ns*/*.jnl", + "ns*/*.key", + "ns*/K*.key", + "ns*/K*.private", + "ns*/K*.state", + "ns*/dsset-*.", + "ns6/sigaxfr.bk", + "verylarge", + ] +) + def _make_sig_rdata(text): """Create a SIG rdata from text. @@ -79,9 +101,9 @@ def test_tcp_self_sig_record(ns6): crash-reproducing precondition. """ ptr_update = dns.update.UpdateMessage("in-addr.arpa.") - ptr_update.add("1.0.0.127.in-addr.arpa.", 600, "PTR", "localhost.") + ptr_update.add("1.0.53.10.in-addr.arpa.", 600, "PTR", "localhost.") response = isctest.query.tcp( - ptr_update, ns6.ip, port=ns6.ports.dns, source="127.0.0.1" + ptr_update, ns6.ip, port=ns6.ports.dns, source="10.53.0.1" ) assert response.rcode() == dns.rcode.NOERROR @@ -92,18 +114,58 @@ def test_tcp_self_sig_record(ns6): sig_update = dns.update.UpdateMessage("in-addr.arpa.") sig_update.add("1.0.0.127.in-addr.arpa.", rds) - response = isctest.query.tcp( - sig_update, ns6.ip, port=ns6.ports.dns, source="127.0.0.1" - ) - assert response.rcode() == dns.rcode.REFUSED + with ns6.watch_log_from_here() as watcher: + response = isctest.query.tcp( + sig_update, ns6.ip, port=ns6.ports.dns, source="10.53.0.1" + ) + assert response.rcode() == dns.rcode.REFUSED + + watcher.wait_for_line( + "updating zone 'in-addr.arpa/IN': update failed: SIG updates are not allowed (REFUSED)" + ) # Confirm nothing of type SIG was stored. - msg = isctest.query.create("1.0.0.127.in-addr.arpa.", "SIG") + msg = isctest.query.create("1.0.53.10.in-addr.arpa.", "SIG") res = isctest.query.tcp(msg, ns6.ip, port=ns6.ports.dns) stored = any(rrset.rdtype == dns.rdatatype.SIG for rrset in res.answer) assert not stored, "SIG record was stored despite REFUSED response" +def test_tcp_self_nxt_record(ns6): + """NXT (type 30) updates must be refused at the front door. + + NXT is the legacy DNSSEC denial-of-existence type, obsolete since + RFC 3755 replaced it with NSEC. Accepting it via dynamic update + would let an authorised updater inject records that the signing + and cut-point logic has no provision for. + """ + # A second owner under a source that also matches tcp-self. + source = "10.53.0.2" + owner = "2.0.53.10.in-addr.arpa." + + ptr_update = dns.update.UpdateMessage("in-addr.arpa.") + ptr_update.add(owner, 600, "PTR", "localhost.") + response = isctest.query.tcp(ptr_update, ns6.ip, port=ns6.ports.dns, source=source) + assert response.rcode() == dns.rcode.NOERROR + + nxt = _make_nxt_rdata() + rds = dns.rdataset.Rdataset(dns.rdataclass.IN, dns.rdatatype.NXT) + rds.update_ttl(600) + rds.add(nxt) + nxt_update = dns.update.UpdateMessage("in-addr.arpa.") + nxt_update.add(owner, rds) + + with ns6.watch_log_from_here() as watcher: + response = isctest.query.tcp( + nxt_update, ns6.ip, port=ns6.ports.dns, source="10.53.0.1" + ) + assert response.rcode() == dns.rcode.REFUSED + + watcher.wait_for_line( + "updating zone 'in-addr.arpa/IN': update failed: NXT updates are not allowed (REFUSED)" + ) + + def test_sig_covers_preserved_via_axfr(ns6): """Regression test for GL#5818 Finding 1, reached via AXFR. @@ -122,7 +184,7 @@ def test_sig_covers_preserved_via_axfr(ns6): """ zone = "sigaxfr.nil" owner = f"host.{zone}." - dump_path = ns6.directory / "named_dump.db" + dump_path = Path("ns6") / "named_dump.db" # ns6 may have tried to SOA-poll ans11 before it was listening; force # a fresh refresh attempt and wait for the transfer to complete. @@ -170,31 +232,3 @@ def test_sig_covers_preserved_via_axfr(ns6): "the Finding 1 bug both records are filed under typepair (SIG, 0) " "and share the first-seen TTL (600)." ) - - -def test_tcp_self_nxt_record(ns6): - """NXT (type 30) updates must be refused at the front door. - - NXT is the legacy DNSSEC denial-of-existence type, obsolete since - RFC 3755 replaced it with NSEC. Accepting it via dynamic update - would let an authorised updater inject records that the signing - and cut-point logic has no provision for. - """ - # A second owner under a source that also matches tcp-self. - source = "127.0.0.2" - owner = "2.0.0.127.in-addr.arpa." - - ptr_update = dns.update.UpdateMessage("in-addr.arpa.") - ptr_update.add(owner, 600, "PTR", "localhost.") - response = isctest.query.tcp(ptr_update, ns6.ip, port=ns6.ports.dns, source=source) - assert response.rcode() == dns.rcode.NOERROR - - nxt = _make_nxt_rdata() - rds = dns.rdataset.Rdataset(dns.rdataclass.IN, dns.rdatatype.NXT) - rds.update_ttl(600) - rds.add(nxt) - nxt_update = dns.update.UpdateMessage("in-addr.arpa.") - nxt_update.add(owner, rds) - - response = isctest.query.tcp(nxt_update, ns6.ip, port=ns6.ports.dns, source=source) - assert response.rcode() == dns.rcode.REFUSED