From 5b2798e01346cd77741873091babf6c4a3128449 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 19 Jan 2022 17:38:18 +1100 Subject: [PATCH] Add additional name checks when using a forwarder When using a forwarder, check that the owner name of response records are within the bailiwick of the forwarded name space. (cherry picked from commit 24155213be59faad17f0215ecf73ea49ab781e5b) Check that the forward declaration is unchanged and not overridden If we are using a fowarder, in addition to checking that names to be cached are subdomains of the forwarded namespace, we must also check that there are no subsidiary forwarded namespaces which would take precedence. To be safe, we don't cache any responses if the forwarding configuration has changed since the query was sent. (cherry picked from commit 3fc7accd88cd0890f8f57bb13765876774298ba3) Check cached names for possible "forward only" clause When caching additional and glue data *not* from a forwarder, we must check that there is no "forward only" clause covering the owner name that would take precedence. Such names would normally be allowed by baliwick rules, but a "forward only" zone introduces a new baliwick scope. (cherry picked from commit ea06552a3d1fed56f7d3a13710e084ec79797b78) Look for zones deeper than the current domain or forward name When caching glue, we need to ensure that there is no closer source of truth for the name. If the owner name for the glue record would be answered by a locally configured zone, do not cache. (cherry picked from commit 71b24210542730355149130770deea3e58d8527a) --- lib/dns/resolver.c | 128 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 123 insertions(+), 5 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index a7bc661bb7..7603a07b7b 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -63,6 +63,8 @@ #include #include #include +#include + #ifdef WANT_QUERYTRACE #define RTRACE(m) \ isc_log_write(dns_lctx, DNS_LOGCATEGORY_RESOLVER, \ @@ -337,6 +339,8 @@ struct fetchctx { dns_fetch_t *qminfetch; dns_rdataset_t qminrrset; dns_name_t qmindcname; + dns_fixedname_t fwdfname; + dns_name_t *fwdname; /*% * The number of events we're waiting for. @@ -3764,6 +3768,7 @@ fctx_getaddresses(fetchctx_t *fctx, bool badcache) { if (result == ISC_R_SUCCESS) { fwd = ISC_LIST_HEAD(forwarders->fwdrs); fctx->fwdpolicy = forwarders->fwdpolicy; + dns_name_copynf(domain, fctx->fwdname); if (fctx->fwdpolicy == dns_fwdpolicy_only && isstrictsubdomain(domain, &fctx->domain)) { @@ -5153,6 +5158,9 @@ fctx_create(dns_resolver_t *res, const dns_name_t *name, dns_rdatatype_t type, fctx->restarts = 0; fctx->querysent = 0; fctx->referrals = 0; + + fctx->fwdname = dns_fixedname_initname(&fctx->fwdfname); + TIME_NOW(&fctx->start); fctx->timeouts = 0; fctx->lamecount = 0; @@ -5215,6 +5223,7 @@ fctx_create(dns_resolver_t *res, const dns_name_t *name, dns_rdatatype_t type, fname, &forwarders); if (result == ISC_R_SUCCESS) { fctx->fwdpolicy = forwarders->fwdpolicy; + dns_name_copynf(fname, fctx->fwdname); } if (fctx->fwdpolicy != dns_fwdpolicy_only) { @@ -7118,6 +7127,107 @@ mark_related(dns_name_t *name, dns_rdataset_t *rdataset, bool external, } } +/* + * Returns true if 'name' is external to the namespace for which + * the server being queried can answer, either because it's not a + * subdomain or because it's below a forward declaration or a + * locally served zone. + */ +static inline bool +name_external(const dns_name_t *name, dns_rdatatype_t type, fetchctx_t *fctx) { + isc_result_t result; + dns_forwarders_t *forwarders = NULL; + dns_fixedname_t fixed, zfixed; + dns_name_t *fname = dns_fixedname_initname(&fixed); + dns_name_t *zfname = dns_fixedname_initname(&zfixed); + dns_name_t *apex = NULL; + dns_name_t suffix; + dns_zone_t *zone = NULL; + unsigned int labels; + dns_namereln_t rel; + + apex = ISFORWARDER(fctx->addrinfo) ? fctx->fwdname : &fctx->domain; + + /* + * The name is outside the queried namespace. + */ + rel = dns_name_fullcompare(name, apex, &(int){ 0 }, + &(unsigned int){ 0U }); + if (rel != dns_namereln_subdomain && rel != dns_namereln_equal) { + return (true); + } + + /* + * If the record lives in the parent zone, adjust the name so we + * look for the correct zone or forward clause. + */ + labels = dns_name_countlabels(name); + if (dns_rdatatype_atparent(type) && labels > 1U) { + dns_name_init(&suffix, NULL); + dns_name_getlabelsequence(name, 1, labels - 1, &suffix); + name = &suffix; + } else if (rel == dns_namereln_equal) { + /* If 'name' is 'apex', no further checking is needed. */ + return (false); + } + + /* + * If there is a locally served zone between 'apex' and 'name' + * then don't cache. + */ + LOCK(&fctx->res->view->lock); + if (fctx->res->view->zonetable != NULL) { + unsigned int options = DNS_ZTFIND_NOEXACT | DNS_ZTFIND_MIRROR; + result = dns_zt_find(fctx->res->view->zonetable, name, options, + zfname, &zone); + if (zone != NULL) { + dns_zone_detach(&zone); + } + if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) { + if (dns_name_fullcompare(zfname, apex, &(int){ 0 }, + &(unsigned int){ 0U }) == + dns_namereln_subdomain) + { + UNLOCK(&fctx->res->view->lock); + return (true); + } + } + } + UNLOCK(&fctx->res->view->lock); + + /* + * Look for a forward declaration below 'name'. + */ + result = dns_fwdtable_find(fctx->res->view->fwdtable, name, fname, + &forwarders); + + if (ISFORWARDER(fctx->addrinfo)) { + /* + * See if the forwarder declaration is better. + */ + if (result == ISC_R_SUCCESS) { + return (!dns_name_equal(fname, fctx->fwdname)); + } + + /* + * If the lookup failed, the configuration must have + * changed: play it safe and don't cache. + */ + return (true); + } else if (result == ISC_R_SUCCESS && + forwarders->fwdpolicy == dns_fwdpolicy_only && + !ISC_LIST_EMPTY(forwarders->fwdrs)) + { + /* + * If 'name' is covered by a 'forward only' clause then we + * can't cache this repsonse. + */ + return (true); + } + + return (false); +} + static isc_result_t check_section(void *arg, const dns_name_t *addname, dns_rdatatype_t type, dns_section_t section) { @@ -7144,7 +7254,7 @@ check_section(void *arg, const dns_name_t *addname, dns_rdatatype_t type, result = dns_message_findname(rctx->query->rmessage, section, addname, dns_rdatatype_any, 0, &name, NULL); if (result == ISC_R_SUCCESS) { - external = !dns_name_issubdomain(name, &fctx->domain); + external = name_external(name, type, fctx); if (type == dns_rdatatype_a) { for (rdataset = ISC_LIST_HEAD(name->list); rdataset != NULL; @@ -8768,6 +8878,13 @@ rctx_answer_scan(respctx_t *rctx) { break; case dns_namereln_subdomain: + /* + * Don't accept DNAME from parent namespace. + */ + if (name_external(name, dns_rdatatype_dname, fctx)) { + continue; + } + /* * In-scope DNAME records must have at least * as many labels as the domain being queried. @@ -9081,13 +9198,11 @@ rctx_authority_positive(respctx_t *rctx) { DNS_SECTION_AUTHORITY); while (!done && result == ISC_R_SUCCESS) { dns_name_t *name = NULL; - bool external; dns_message_currentname(rctx->query->rmessage, DNS_SECTION_AUTHORITY, &name); - external = !dns_name_issubdomain(name, &fctx->domain); - if (!external) { + if (!name_external(name, dns_rdatatype_ns, fctx)) { dns_rdataset_t *rdataset = NULL; /* @@ -9474,7 +9589,10 @@ rctx_authority_dnssec(respctx_t *rctx) { } if (!dns_name_issubdomain(name, &fctx->domain)) { - /* Invalid name found; preserve it for logging later */ + /* + * Invalid name found; preserve it for logging + * later. + */ rctx->found_name = name; rctx->found_type = ISC_LIST_HEAD(name->list)->type; continue; -- 2.34.1