]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Add regression test for SIG covers being dropped in dns_diff_apply
authorOndřej Surý <ondrej@isc.org>
Thu, 16 Apr 2026 09:16:40 +0000 (11:16 +0200)
committerOndřej Surý <ondrej@isc.org>
Fri, 17 Apr 2026 14:09:39 +0000 (16:09 +0200)
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.

bin/tests/system/nsupdate/tests_tcp_self_sig.py

index 3378097e30ff40a82af53dd26905b1e3ce3e1841..1a90e7b90e562e7d6bd7b027271823996283b463 100644 (file)
 # 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}"