From: Ondřej Surý Date: Thu, 16 Apr 2026 10:23:34 +0000 (+0200) Subject: Refuse SIG and NXT records in dynamic updates X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3a44a13232991ba054aa157a1c3ffb8376359b42;p=thirdparty%2Fbind9.git 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. --- diff --git a/bin/tests/system/nsupdate/tests_tcp_self_sig.py b/bin/tests/system/nsupdate/tests_tcp_self_sig.py index 1a90e7b90e5..574497338c6 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. - -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. +Regression tests for GL#5818: legacy DNSSEC types on the dynamic-update path. + +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: + + * 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. + + 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)) + - 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. +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 - - 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 + response = isctest.query.tcp( + sig_update, ns6.ip, port=ns6.ports.dns, source="127.0.0.1" + ) + assert response.rcode() == dns.rcode.REFUSED + + # 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. - 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. +def test_tcp_self_nxt_record(ns6): + """NXT (type 30) updates must be refused at the front door. - 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." - - # 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 + # A second owner under a source that also matches tcp-self. + source = "127.0.0.2" + owner = "2.0.0.127.in-addr.arpa." - # 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) + 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 - # 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" - ) + 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) - # 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 dd8173c79c1..93240ca250c 100644 --- a/lib/ns/update.c +++ b/lib/ns/update.c @@ -1682,6 +1682,12 @@ send_update(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)) {