From 35a58d30c9ddc0ce5c1b77b84f1708b277d5f29d Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 28 Apr 2020 15:37:19 +1000 Subject: [PATCH 1/4] Reject primary zones with an DS record at the zone apex. DS records only belong at delegation points and if present at the zone apex are invariably the result of administrative errors. Additionally they can't be queried for with modern resolvers as the parent servers will be queried. --- bin/tests/system/checkzone/zones/bad-ds.db | 4 ++ lib/dns/include/dns/result.h | 3 +- lib/dns/master.c | 61 ++++++++++++++-------- lib/dns/result.c | 2 + 4 files changed, 47 insertions(+), 23 deletions(-) create mode 100644 bin/tests/system/checkzone/zones/bad-ds.db diff --git a/bin/tests/system/checkzone/zones/bad-ds.db b/bin/tests/system/checkzone/zones/bad-ds.db new file mode 100644 index 0000000000..723843a995 --- /dev/null +++ b/bin/tests/system/checkzone/zones/bad-ds.db @@ -0,0 +1,4 @@ +example. 0 SOA . . 0 0 0 0 0 +example. 0 NS . +example. 0 DNSKEY 257 3 10 AwEAAbqjg7xdvnU2Q/gtLw5LOfr5cDeTRjYuEbkzGrUiVSOSoxcTxuao WS/AFPQHuD8OSLiE/CeZ087JowREXl058rRfae8KMrveY17V0wmKs9N1 F1wf/hRDpXiThlRHWlskp8eSEEIqYrrHgWTesy/xDGIEOFM1gwRo0w8j KdRRJeL2hseTMa+m3rTzrYudUsI0BHLW8PiDUCbG5xgdee8/5YR4847i AAqHIiPJ1Z/IT53OIjMmtv5BUykZ8RYjlJxxX+C+dpRKiK73SQaR3hCB XAYOL9WsDp2/fpmEZpewavkMkdC+j2CX+z27MCS3ASO0AeKK0lcNXwND kgreE+Kr7gc= +example. 0 DS 14364 10 2 FD03B2312C8F0FE72C1751EFA1007D743C94EC91594FF0047C23C37CE119BA0C diff --git a/lib/dns/include/dns/result.h b/lib/dns/include/dns/result.h index d0df43ac14..a67257a214 100644 --- a/lib/dns/include/dns/result.h +++ b/lib/dns/include/dns/result.h @@ -154,8 +154,9 @@ #define DNS_R_BADSIG0 (ISC_RESULTCLASS_DNS + 116) #define DNS_R_TOOMANYRECORDS (ISC_RESULTCLASS_DNS + 117) #define DNS_R_VERIFYFAILURE (ISC_RESULTCLASS_DNS + 118) +#define DNS_R_ATZONETOP (ISC_RESULTCLASS_DNS + 119) -#define DNS_R_NRESULTS 119 /*%< Number of results */ +#define DNS_R_NRESULTS 120 /*%< Number of results */ /* * DNS wire format rcodes. diff --git a/lib/dns/master.c b/lib/dns/master.c index 59a134611e..8537b239e6 100644 --- a/lib/dns/master.c +++ b/lib/dns/master.c @@ -339,6 +339,13 @@ static unsigned char ip6_arpa_offsets[] = { 0, 4, 9 }; static dns_name_t const ip6_arpa = DNS_NAME_INITABSOLUTE(ip6_arpa_data, ip6_arpa_offsets); +static inline bool +dns_master_isprimary(dns_loadctx_t *lctx) { + return ((lctx->options & DNS_MASTER_ZONE) != 0 && + (lctx->options & DNS_MASTER_SLAVE) == 0 && + (lctx->options & DNS_MASTER_KEY) == 0); +} + static inline isc_result_t gettoken(isc_lex_t *lex, unsigned int options, isc_token_t *token, bool eol, dns_rdatacallbacks_t *callbacks) { @@ -840,10 +847,7 @@ generate(dns_loadctx_t *lctx, char *range, char *lhs, char *gtype, char *rhs, * RFC2930: TKEY and TSIG are not allowed to be loaded * from master files. */ - if ((lctx->options & DNS_MASTER_ZONE) != 0 && - (lctx->options & DNS_MASTER_SLAVE) == 0 && - dns_rdatatype_ismeta(type)) - { + if (dns_master_isprimary(lctx) && dns_rdatatype_ismeta(type)) { (*callbacks->error)(callbacks, "%s: %s:%lu: meta RR type '%s'", "$GENERATE", source, line, gtype); result = DNS_R_METATYPE; @@ -869,11 +873,8 @@ generate(dns_loadctx_t *lctx, char *range, char *lhs, char *gtype, char *rhs, goto error_cleanup; } - if ((lctx->options & DNS_MASTER_ZONE) != 0 && - (lctx->options & DNS_MASTER_SLAVE) == 0 && - (lctx->options & DNS_MASTER_KEY) == 0 && - !dns_name_issubdomain(owner, lctx->top)) - { + if (dns_master_isprimary(lctx) && + !dns_name_issubdomain(owner, lctx->top)) { char namebuf[DNS_NAME_FORMATSIZE]; dns_name_format(owner, namebuf, sizeof(namebuf)); /* @@ -1541,11 +1542,8 @@ load_text(dns_loadctx_t *lctx) { callbacks); } } - if ((lctx->options & DNS_MASTER_ZONE) != 0 && - (lctx->options & DNS_MASTER_SLAVE) == 0 && - (lctx->options & DNS_MASTER_KEY) == 0 && - !dns_name_issubdomain(new_name, lctx->top)) - { + if (dns_master_isprimary(lctx) && + !dns_name_issubdomain(new_name, lctx->top)) { char namebuf[DNS_NAME_FORMATSIZE]; dns_name_format(new_name, namebuf, sizeof(namebuf)); @@ -1740,8 +1738,7 @@ load_text(dns_loadctx_t *lctx) { * RFC1123: MD and MF are not allowed to be loaded from * master files. */ - if ((lctx->options & DNS_MASTER_ZONE) != 0 && - (lctx->options & DNS_MASTER_SLAVE) == 0 && + if (dns_master_isprimary(lctx) && (type == dns_rdatatype_md || type == dns_rdatatype_mf)) { char typebuf[DNS_RDATATYPE_FORMATSIZE]; @@ -1763,10 +1760,7 @@ load_text(dns_loadctx_t *lctx) { * RFC2930: TKEY and TSIG are not allowed to be loaded * from master files. */ - if ((lctx->options & DNS_MASTER_ZONE) != 0 && - (lctx->options & DNS_MASTER_SLAVE) == 0 && - dns_rdatatype_ismeta(type)) - { + if (dns_master_isprimary(lctx) && dns_rdatatype_ismeta(type)) { char typebuf[DNS_RDATATYPE_FORMATSIZE]; result = DNS_R_METATYPE; @@ -1902,6 +1896,30 @@ load_text(dns_loadctx_t *lctx) { } } + if (dns_rdatatype_atparent(type) && + dns_master_isprimary(lctx) && + dns_name_equal(ictx->current, lctx->top)) + { + char namebuf[DNS_NAME_FORMATSIZE]; + char typebuf[DNS_RDATATYPE_FORMATSIZE]; + + dns_name_format(ictx->current, namebuf, + sizeof(namebuf)); + dns_rdatatype_format(type, typebuf, sizeof(typebuf)); + (*callbacks->error)( + callbacks, + "%s:%lu: %s record at top of zone (%s)", source, + line, typebuf, namebuf); + result = DNS_R_ATZONETOP; + if (MANYERRS(lctx, result)) { + SETRESULT(lctx, result); + target = target_ft; + continue; + } else { + goto insist_and_cleanup; + } + } + if (type == dns_rdatatype_rrsig || type == dns_rdatatype_sig) { covers = dns_rdata_covers(&rdata[rdcount]); } else { @@ -1963,8 +1981,7 @@ load_text(dns_loadctx_t *lctx) { } if ((type == dns_rdatatype_sig || type == dns_rdatatype_nxt) && - lctx->warn_tcr && (lctx->options & DNS_MASTER_ZONE) != 0 && - (lctx->options & DNS_MASTER_SLAVE) == 0) + lctx->warn_tcr && dns_master_isprimary(lctx)) { (*callbacks->warn)(callbacks, "%s:%lu: old style DNSSEC " diff --git a/lib/dns/result.c b/lib/dns/result.c index d1457ef43a..69445ad4c1 100644 --- a/lib/dns/result.c +++ b/lib/dns/result.c @@ -164,6 +164,7 @@ static const char *text[DNS_R_NRESULTS] = { "SIG(0) in wrong location", /*%< 116 DNS_R_BADSIG0 */ "too many records", /*%< 117 DNS_R_TOOMANYRECORDS */ "verify failure", /*%< 118 DNS_R_VERIFYFAILURE */ + "at top of zone", /*%< 119 DNS_R_ATZONETOP */ }; static const char *ids[DNS_R_NRESULTS] = { @@ -290,6 +291,7 @@ static const char *ids[DNS_R_NRESULTS] = { "DNS_R_BADSIG0", "DNS_R_TOOMANYRECORDS", "DNS_R_VERIFYFAILURE", + "DNS_R_ATZONETOP", }; static const char *rcode_text[DNS_R_NRCODERESULTS] = { From ae55fbbe9cacb1a2c275bab48a611a0e174410d9 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 7 May 2020 09:36:50 +1000 Subject: [PATCH 2/4] Ignore attempts to add DS records at zone apex DS records belong in the parent zone at a zone cut and are not retrievable with modern recursive servers. --- bin/tests/system/nsupdate/tests.sh | 21 +++++++++++++++++++++ lib/ns/update.c | 13 +++++++++++++ 2 files changed, 34 insertions(+) diff --git a/bin/tests/system/nsupdate/tests.sh b/bin/tests/system/nsupdate/tests.sh index f190f7f410..055161b4b7 100755 --- a/bin/tests/system/nsupdate/tests.sh +++ b/bin/tests/system/nsupdate/tests.sh @@ -32,6 +32,8 @@ RNDCCMD="$RNDC -c $SYSTEMTESTTOP/common/rndc.conf -p ${CONTROLPORT} -s" status=0 n=0 +nextpartreset ns3/named.run + # wait for zone transfer to complete tries=0 while true; do @@ -1087,6 +1089,25 @@ then echo_i "failed"; status=1 fi +echo_i "check that DS to the zone apex is ignored ($n)" +$DIG $DIGOPTS +tcp +norec example DS @10.53.0.3 > dig.out.pre.test$n || ret=1 +grep "status: NOERROR" dig.out.pre.test$n > /dev/null || ret=1 +grep "ANSWER: 0," dig.out.pre.test$n > /dev/null || ret=1 +nextpart ns3/named.run > /dev/null +# specify zone to override the default of adding to parent zone +$NSUPDATE -d < nsupdate.out-$n 2>&1 || ret=1 +server 10.53.0.3 ${PORT} +zone example +update add example 0 in DS 14364 10 2 FD03B2312C8F0FE72C1751EFA1007D743C94EC91594FF0047C23C37CE119BA0C +send +END +msg=": attempt to add a DS record at zone apex ignored" +nextpart ns3/named.run | grep "$msg" > /dev/null || ret=1 +$DIG $DIGOPTS +tcp +norec example DS @10.53.0.3 > dig.out.post.test$n || ret=1 +grep "status: NOERROR" dig.out.post.test$n > /dev/null || ret=1 +grep "ANSWER: 0," dig.out.post.test$n > /dev/null || ret=1 +[ $ret = 0 ] || { echo_i "failed"; status=1; } + if $FEATURETEST --gssapi ; then n=`expr $n + 1` ret=0 diff --git a/lib/ns/update.c b/lib/ns/update.c index 790b10eb6e..9b780a62b9 100644 --- a/lib/ns/update.c +++ b/lib/ns/update.c @@ -2969,6 +2969,19 @@ update_action(isc_task_t *task, isc_event_t *event) { soa_serial_changed = true; } + if (dns_rdatatype_atparent(rdata.type) && + dns_name_equal(name, zonename)) { + char typebuf[DNS_RDATATYPE_FORMATSIZE]; + + dns_rdatatype_format(rdata.type, typebuf, + sizeof(typebuf)); + update_log(client, zone, LOGLEVEL_PROTOCOL, + "attempt to add a %s record at " + "zone apex ignored", + typebuf); + continue; + } + if (rdata.type == privatetype) { update_log(client, zone, LOGLEVEL_PROTOCOL, "attempt to add a private type " From 06e714df0d972bf8762204d53874b2eb2a648e8a Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 28 Apr 2020 16:02:00 +1000 Subject: [PATCH 3/4] Add CHANGES entry for #1798 --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index 173e8dbd15..40d25780c8 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5431. [func] Reject DS records at the zone apex when loading + master files. Log but otherwise ignore attempts to + add DS records at the zone apex via UPDATE. [GL #1798] + 5430. [doc] Update docs - with netmgr we're creating separate socket for each IPv6 interface, just as with IPv4. [GL #1782] From 8b05e6f7103bde492cf7e2cf8a0d98dbce43edb8 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 28 Apr 2020 16:03:41 +1000 Subject: [PATCH 4/4] Add release note for #1798 --- doc/notes/notes-current.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 1409833692..c2f46975a5 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -71,6 +71,11 @@ New Features statements, to limit the number of records of a particular type that can be added to a domain name via dynamic update. [GL #1657] +- ``named`` and ``named-checkzone`` now reject master zones that + have a DS RRset at the zone apex. Attempts to add DS records + at the zone apex via UPDATE will be logged but otherwise ignored. + DS records belong in the parent zone, not at the zone apex. [GL #1798] + Feature Changes ~~~~~~~~~~~~~~~