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 dd8173c79c..93240ca250 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)) {