From 35a58d30c9ddc0ce5c1b77b84f1708b277d5f29d Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 28 Apr 2020 15:37:19 +1000 Subject: [PATCH] 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] = {