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 1a90e7b90e562e7d6bd7b027271823996283b463..574497338c6d1a5f821b2ef01f263265bb90a144 100644 (file)
--- 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 dd8173c79c197b5c670dd03c7f29da18fe7d9541..93240ca250c2108d0815de17bd7b8ec51b837889 100644 (file)
--- 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))
                {