mirror of
https://github.com/isc-projects/bind9.git
synced 2026-06-08 23:52:05 -04:00
Stop treating SIG and NXT records specially
RFC 3755 retired SIG and NXT in favour of RRSIG and NSEC. BIND still warned about them at zone load, refused them in dynamic updates, parsed SIG with a non-zero "type covered" field as a signature on an RRset, and tracked them via dns_rdatatype_issig(). Those carve-outs were the sole path that made the GL#5818 crash class reachable. Treat both types as ordinary unknown rdata: they load, transfer, sign and answer like any other record, and dynamic updates carry them through the generic path. SIG(0) is unaffected; its message-parsing carve-out is preserved.
This commit is contained in:
Ondřej Surý
parent
81b78d6c10
commit
2de202a6b7
10 changed files with 67 additions and 119 deletions
|
|
@ -10,14 +10,13 @@
|
|||
# information regarding copyright ownership.
|
||||
|
||||
"""
|
||||
GL#5818 Finding 1 regression support — AsyncDnsServer primary.
|
||||
AsyncDnsServer primary serving "sigaxfr.nil.".
|
||||
|
||||
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.
|
||||
The AXFR carries two SIG (type 24) rdatas at the same owner with
|
||||
different "covered type" body fields (A and MX) so the secondary can
|
||||
verify it stores both under a single opaque rdataset. Per RFC 3755
|
||||
SIG has no covered-type semantics on BIND any more; the two rdatas
|
||||
share the rdataset TTL.
|
||||
"""
|
||||
|
||||
from collections.abc import AsyncGenerator
|
||||
|
|
@ -75,7 +74,7 @@ class SigAxfrServer(DomainHandler):
|
|||
return
|
||||
|
||||
# AXFR: opening SOA, NS, NS's A, two SIG RRs at the same owner
|
||||
# with distinct covered types and TTLs, closing SOA.
|
||||
# with distinct "covered type" body fields, closing SOA.
|
||||
resp = qctx.response
|
||||
resp.answer.append(soa_rrset)
|
||||
|
||||
|
|
@ -89,17 +88,16 @@ class SigAxfrServer(DomainHandler):
|
|||
)
|
||||
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_rrset = dns.rrset.RRset(HOST, dns.rdataclass.IN, dns.rdatatype.SIG)
|
||||
sig_rrset.add(
|
||||
_make_sig_rdata("A 6 2 600 20260331170000 20260318160000 21831 . 0000"),
|
||||
ttl=600,
|
||||
)
|
||||
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)
|
||||
sig_rrset.add(
|
||||
_make_sig_rdata("MX 6 2 600 20260331170000 20260318160000 21831 . 0000"),
|
||||
ttl=600,
|
||||
)
|
||||
resp.answer.append(sig_rrset)
|
||||
|
||||
# Closing SOA terminates the AXFR.
|
||||
resp.answer.append(soa_rrset)
|
||||
|
|
|
|||
|
|
@ -9,8 +9,7 @@
|
|||
# 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.
|
||||
"""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
|
||||
|
|
@ -24,11 +23,9 @@ exposed two bugs in sequence:
|
|||
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.
|
||||
The adopted defence is to treat the legacy SIG and NXT records as normal RR
|
||||
records without any special processing.
|
||||
|
||||
"""
|
||||
|
||||
from re import compile as Re
|
||||
|
|
@ -92,18 +89,17 @@ def _make_nxt_rdata():
|
|||
|
||||
|
||||
def test_tcp_self_sig_record(ns6):
|
||||
"""SIG (type 24) updates must be refused at the front door.
|
||||
"""SIG (type 24) updates are accepted and stored as opaque rdata.
|
||||
|
||||
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.
|
||||
Per RFC 3755 SIG is obsolete (superseded by RRSIG). BIND treats
|
||||
incoming SIG records as a generic unknown type with no covered-type
|
||||
semantics: dynamic updates carrying SIG are accepted and the record
|
||||
becomes queryable. A PTR add first ensures the node exists.
|
||||
"""
|
||||
owner = "1.0.53.10.in-addr.arpa."
|
||||
|
||||
ptr_update = dns.update.UpdateMessage("in-addr.arpa.")
|
||||
ptr_update.add("1.0.53.10.in-addr.arpa.", 600, "PTR", "localhost.")
|
||||
ptr_update.add(owner, 600, "PTR", "localhost.")
|
||||
response = isctest.query.tcp(
|
||||
ptr_update, ns6.ip, port=ns6.ports.dns, source="10.53.0.1"
|
||||
)
|
||||
|
|
@ -114,34 +110,27 @@ def test_tcp_self_sig_record(ns6):
|
|||
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)
|
||||
sig_update.add(owner, 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
|
||||
response = isctest.query.tcp(
|
||||
sig_update, ns6.ip, port=ns6.ports.dns, source="10.53.0.1"
|
||||
)
|
||||
assert response.rcode() == dns.rcode.NOERROR
|
||||
|
||||
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")
|
||||
# Confirm the SIG record was stored.
|
||||
msg = isctest.query.create(owner, "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"
|
||||
assert stored, "SIG record was not stored despite NOERROR response"
|
||||
|
||||
|
||||
def test_tcp_self_nxt_record(ns6):
|
||||
"""NXT (type 30) updates must be refused at the front door.
|
||||
"""NXT (type 30) updates are accepted and stored as opaque rdata.
|
||||
|
||||
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.
|
||||
RFC 3755 replaced it with NSEC. BIND treats it as a generic
|
||||
unknown rdata type.
|
||||
"""
|
||||
# A second owner under a source that also matches tcp-self.
|
||||
source = "10.53.0.2"
|
||||
owner = "2.0.53.10.in-addr.arpa."
|
||||
|
||||
|
|
@ -157,28 +146,23 @@ def test_tcp_self_nxt_record(ns6):
|
|||
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
|
||||
response = isctest.query.tcp(nxt_update, ns6.ip, port=ns6.ports.dns, source=source)
|
||||
assert response.rcode() == dns.rcode.NOERROR
|
||||
|
||||
watcher.wait_for_line(
|
||||
"updating zone 'in-addr.arpa/IN': update failed: NXT updates are not allowed (REFUSED)"
|
||||
)
|
||||
# Confirm the NXT record was stored.
|
||||
msg = isctest.query.create(owner, "NXT")
|
||||
res = isctest.query.tcp(msg, ns6.ip, port=ns6.ports.dns)
|
||||
stored = any(rrset.rdtype == dns.rdatatype.NXT for rrset in res.answer)
|
||||
assert stored, "NXT record was not stored despite NOERROR response"
|
||||
|
||||
|
||||
def test_sig_covers_preserved_via_axfr(ns6):
|
||||
"""Regression test for GL#5818 Finding 1, reached via AXFR.
|
||||
def test_sig_axfr_stored_opaque(ns6):
|
||||
"""SIG records received via AXFR are stored as opaque rdata.
|
||||
|
||||
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.
|
||||
the same owner with different "covered type" body fields (A, MX).
|
||||
Per RFC 3755 SIG has no covered-type semantics; both rdatas land in
|
||||
a single opaque rdataset and both survive in the zone DB.
|
||||
|
||||
rndc dumpdb is used to inspect the secondary's stored state
|
||||
directly; the wire-level response can merge same-(owner,type,class)
|
||||
|
|
@ -211,12 +195,11 @@ def test_sig_covers_preserved_via_axfr(ns6):
|
|||
else:
|
||||
raise AssertionError(f"{dump_path} never contained {deadline_marker!r}")
|
||||
|
||||
# Collect every SIG line for the owner from the dump. Format is:
|
||||
# <owner>. <ttl> IN SIG <covered> <alg> <labels> ...
|
||||
# Collect every SIG line for the owner from the dump.
|
||||
sig_lines = []
|
||||
for line in text.splitlines():
|
||||
fields = line.split()
|
||||
if len(fields) < 6:
|
||||
if len(fields) < 4:
|
||||
continue
|
||||
if not fields[0].lower().startswith("host.sigaxfr.nil"):
|
||||
continue
|
||||
|
|
@ -226,14 +209,10 @@ def test_sig_covers_preserved_via_axfr(ns6):
|
|||
|
||||
assert (
|
||||
len(sig_lines) == 2
|
||||
), f"expected 2 SIG records at {owner}, got {len(sig_lines)}: {sig_lines}"
|
||||
), f"expected 2 SIG rdatas 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)."
|
||||
)
|
||||
ttls = {int(fields[1]) for fields in sig_lines}
|
||||
assert ttls == {600}, f"SIG rdataset should share a single TTL, got {ttls}"
|
||||
|
||||
|
||||
def parse_named_conf_keys(conf_text):
|
||||
|
|
|
|||
|
|
@ -720,20 +720,19 @@ dns_rdatatype_isknown(dns_rdatatype_t type) {
|
|||
|
||||
/*%
|
||||
* Return true iff a query for the rdata type can have multiple
|
||||
* unrelated answers in a response: ANY, RRSIG, or SIG.
|
||||
* unrelated answers in a response: ANY, or RRSIG.
|
||||
*/
|
||||
static inline bool
|
||||
dns_rdatatype_ismulti(dns_rdatatype_t type) {
|
||||
return type == dns_rdatatype_any || type == dns_rdatatype_rrsig ||
|
||||
type == dns_rdatatype_sig;
|
||||
return type == dns_rdatatype_any || type == dns_rdatatype_rrsig;
|
||||
}
|
||||
|
||||
/*%
|
||||
* Return true iff the rdata type is a signature: either RRSIG or SIG.
|
||||
* Return true iff the rdata type is RRSIG.
|
||||
*/
|
||||
static inline bool
|
||||
dns_rdatatype_issig(dns_rdatatype_t type) {
|
||||
return type == dns_rdatatype_rrsig || type == dns_rdatatype_sig;
|
||||
return type == dns_rdatatype_rrsig;
|
||||
}
|
||||
|
||||
/*%
|
||||
|
|
|
|||
|
|
@ -494,7 +494,6 @@ loadctx_create(dns_masterformat_t format, isc_mem_t *mctx, unsigned int options,
|
|||
.ttl_known = ((options & DNS_MASTER_NOTTL) != 0),
|
||||
.default_ttl_known = ((options & DNS_MASTER_NOTTL) != 0),
|
||||
.warn_1035 = true,
|
||||
.warn_tcr = true,
|
||||
.warn_sigexpired = true,
|
||||
.options = options,
|
||||
.zclass = zclass,
|
||||
|
|
@ -1947,16 +1946,6 @@ load_text(dns_loadctx_t *lctx) {
|
|||
}
|
||||
}
|
||||
|
||||
if ((type == dns_rdatatype_sig || type == dns_rdatatype_nxt) &&
|
||||
lctx->warn_tcr && dns_master_isprimary(lctx))
|
||||
{
|
||||
(*callbacks->warn)(callbacks,
|
||||
"%s:%lu: old style DNSSEC "
|
||||
" zone detected",
|
||||
source, line);
|
||||
lctx->warn_tcr = false;
|
||||
}
|
||||
|
||||
if ((lctx->options & DNS_MASTER_AGETTL) != 0) {
|
||||
/*
|
||||
* Adjust the TTL for $DATE. If the RR has
|
||||
|
|
|
|||
|
|
@ -1285,12 +1285,7 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx,
|
|||
issigzero = true;
|
||||
}
|
||||
} else {
|
||||
if (msg->rdclass != dns_rdataclass_any &&
|
||||
msg->rdclass != rdclass)
|
||||
{
|
||||
/* XXX test coverage */
|
||||
DO_ERROR(DNS_R_FORMERR);
|
||||
}
|
||||
covers = dns_rdatatype_none;
|
||||
}
|
||||
} else {
|
||||
covers = dns_rdatatype_none;
|
||||
|
|
|
|||
|
|
@ -380,8 +380,8 @@ dns_nsec_noexistnodata(dns_rdatatype_t type, const dns_name_t *name,
|
|||
(*logit)(arg, ISC_LOG_DEBUG(3), "ignoring child nsec");
|
||||
return ISC_R_IGNORE;
|
||||
}
|
||||
if (type == dns_rdatatype_cname || type == dns_rdatatype_nxt ||
|
||||
type == dns_rdatatype_nsec || type == dns_rdatatype_key ||
|
||||
if (type == dns_rdatatype_cname || type == dns_rdatatype_nsec ||
|
||||
type == dns_rdatatype_key ||
|
||||
!dns_nsec_typepresent(&rdata, dns_rdatatype_cname))
|
||||
{
|
||||
*exists = true;
|
||||
|
|
|
|||
|
|
@ -1940,7 +1940,6 @@ dns_nsec3_noexistnodata(dns_rdatatype_t type, const dns_name_t *name,
|
|||
return ISC_R_IGNORE;
|
||||
}
|
||||
if (type == dns_rdatatype_cname ||
|
||||
type == dns_rdatatype_nxt ||
|
||||
type == dns_rdatatype_nsec ||
|
||||
type == dns_rdatatype_key ||
|
||||
!dns_nsec3_typepresent(&rdata, dns_rdatatype_cname))
|
||||
|
|
|
|||
|
|
@ -1829,7 +1829,6 @@ cname_and_other(qpznode_t *node, uint32_t serial) {
|
|||
cname = true;
|
||||
}
|
||||
} else if (rdtype != dns_rdatatype_key &&
|
||||
rdtype != dns_rdatatype_sig &&
|
||||
rdtype != dns_rdatatype_nsec &&
|
||||
rdtype != dns_rdatatype_rrsig)
|
||||
{
|
||||
|
|
|
|||
|
|
@ -936,7 +936,7 @@ typedef struct respctx {
|
|||
* fctx_query() when resending */
|
||||
|
||||
dns_rdatatype_t type; /* type being sought (set to
|
||||
* ANY if qtype was SIG or RRSIG) */
|
||||
* ANY if qtype was RRSIG) */
|
||||
bool aa; /* authoritative answer? */
|
||||
dns_trust_t trust; /* answer trust level */
|
||||
bool chaining; /* CNAME/DNAME processing? */
|
||||
|
|
@ -4574,7 +4574,6 @@ resume_qmin(void *arg) {
|
|||
fctx->type != dns_rdatatype_key &&
|
||||
fctx->type != dns_rdatatype_nsec &&
|
||||
fctx->type != dns_rdatatype_any &&
|
||||
fctx->type != dns_rdatatype_sig &&
|
||||
fctx->type != dns_rdatatype_rrsig)
|
||||
{
|
||||
pull_from_resp(resp, fctx);
|
||||
|
|
@ -5577,12 +5576,10 @@ evict_cname_other(fetchctx_t *fctx, dns_name_t *name) {
|
|||
dns_typepair_t typepair = DNS_TYPEPAIR_VALUE(rdataset.type,
|
||||
rdataset.covers);
|
||||
switch (typepair) {
|
||||
/* KEY, NSEC and NXT records are allowed */
|
||||
/* KEY and NSEC records are allowed */
|
||||
case DNS_TYPEPAIR(dns_rdatatype_nsec):
|
||||
case DNS_TYPEPAIR(dns_rdatatype_nxt):
|
||||
case DNS_TYPEPAIR(dns_rdatatype_key):
|
||||
case DNS_SIGTYPEPAIR(dns_rdatatype_nsec):
|
||||
case DNS_SIGTYPEPAIR(dns_rdatatype_nxt):
|
||||
case DNS_SIGTYPEPAIR(dns_rdatatype_key):
|
||||
/* Keep the CNAME and its signature */
|
||||
case DNS_TYPEPAIR(dns_rdatatype_cname):
|
||||
|
|
@ -5664,8 +5661,7 @@ cache_rrset(fetchctx_t *fctx, isc_stdtime_t now, dns_name_t *name,
|
|||
*/
|
||||
if (!dns_rdataset_matchestype(rdataset, dns_rdatatype_cname) &&
|
||||
!dns_rdataset_matchestype(rdataset, dns_rdatatype_key) &&
|
||||
!dns_rdataset_matchestype(rdataset, dns_rdatatype_nsec) &&
|
||||
!dns_rdataset_matchestype(rdataset, dns_rdatatype_nxt))
|
||||
!dns_rdataset_matchestype(rdataset, dns_rdatatype_nsec))
|
||||
{
|
||||
delete_rrset(fctx, name, dns_rdatatype_cname);
|
||||
}
|
||||
|
|
@ -8132,7 +8128,7 @@ rctx_answer_init(respctx_t *rctx) {
|
|||
}
|
||||
|
||||
/*
|
||||
* There can be multiple RRSIG and SIG records at a name so
|
||||
* There can be multiple RRSIG records at a name so
|
||||
* we treat these types as a subset of ANY.
|
||||
*/
|
||||
rctx->type = fctx->type;
|
||||
|
|
|
|||
|
|
@ -1687,12 +1687,6 @@ 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))
|
||||
{
|
||||
|
|
|
|||
Loading…
Reference in a new issue