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_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_update_sig.py b/bin/tests/system/nsupdate/tests_update_sig.py new file mode 100644 index 0000000000..261b47efa2 --- /dev/null +++ b/bin/tests/system/nsupdate/tests_update_sig.py @@ -0,0 +1,234 @@ +# 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 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 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. + + 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 _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)) + + +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. + """ + ptr_update = dns.update.UpdateMessage("in-addr.arpa.") + 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="10.53.0.1" + ) + assert response.rcode() == dns.rcode.NOERROR + + 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="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.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. + + 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 = 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. + 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)." + ) 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) && 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 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)) {