From e2aed3e1885bbc6d94d8845edbd9a8dfb869eb67 Mon Sep 17 00:00:00 2001 From: Petr Mensik Date: Fri, 15 May 2020 14:55:26 +0200 Subject: [PATCH] CVE-2020-8616 5395. [security] Further limit the number of queries that can be triggered from a request. Root and TLD servers are no longer exempt from max-recursion-queries. Fetches for missing name server address records are limited to 4 for any domain. (CVE-2020-8616) [GL #1388] --- lib/dns/adb.c | 18 ++++++++-------- lib/dns/include/dns/adb.h | 4 ++++ lib/dns/resolver.c | 45 ++++++++++++++++++++++++++------------- 3 files changed, 43 insertions(+), 24 deletions(-) diff --git a/lib/dns/adb.c b/lib/dns/adb.c index 1eb00c2..ea06a95 100644 --- a/lib/dns/adb.c +++ b/lib/dns/adb.c @@ -402,14 +402,13 @@ static void log_quota(dns_adbentry_t *entry, const char *fmt, ...) */ #define FIND_WANTEVENT(fn) (((fn)->options & DNS_ADBFIND_WANTEVENT) != 0) #define FIND_WANTEMPTYEVENT(fn) (((fn)->options & DNS_ADBFIND_EMPTYEVENT) != 0) -#define FIND_AVOIDFETCHES(fn) (((fn)->options & DNS_ADBFIND_AVOIDFETCHES) \ - != 0) -#define FIND_STARTATZONE(fn) (((fn)->options & DNS_ADBFIND_STARTATZONE) \ - != 0) -#define FIND_HINTOK(fn) (((fn)->options & DNS_ADBFIND_HINTOK) != 0) -#define FIND_GLUEOK(fn) (((fn)->options & DNS_ADBFIND_GLUEOK) != 0) -#define FIND_HAS_ADDRS(fn) (!ISC_LIST_EMPTY((fn)->list)) -#define FIND_RETURNLAME(fn) (((fn)->options & DNS_ADBFIND_RETURNLAME) != 0) +#define FIND_AVOIDFETCHES(fn) (((fn)->options & DNS_ADBFIND_AVOIDFETCHES) != 0) +#define FIND_STARTATZONE(fn) (((fn)->options & DNS_ADBFIND_STARTATZONE) != 0) +#define FIND_HINTOK(fn) (((fn)->options & DNS_ADBFIND_HINTOK) != 0) +#define FIND_GLUEOK(fn) (((fn)->options & DNS_ADBFIND_GLUEOK) != 0) +#define FIND_HAS_ADDRS(fn) (!ISC_LIST_EMPTY((fn)->list)) +#define FIND_RETURNLAME(fn) (((fn)->options & DNS_ADBFIND_RETURNLAME) != 0) +#define FIND_NOFETCH(fn) (((fn)->options & DNS_ADBFIND_NOFETCH) != 0) /* * These are currently used on simple unsigned ints, so they are @@ -3167,7 +3166,8 @@ dns_adb_createfind2(dns_adb_t *adb, isc_task_t *task, isc_taskaction_t action, else have_address = ISC_FALSE; if (wanted_fetches != 0 && - ! (FIND_AVOIDFETCHES(find) && have_address)) { + ! (FIND_AVOIDFETCHES(find) && have_address) && + ! FIND_NOFETCH(find)) { /* * We're missing at least one address family. Either the * caller hasn't instructed us to avoid fetches, or we don't diff --git a/lib/dns/include/dns/adb.h b/lib/dns/include/dns/adb.h index bfc8e43..0efaf89 100644 --- a/lib/dns/include/dns/adb.h +++ b/lib/dns/include/dns/adb.h @@ -204,6 +204,10 @@ struct dns_adbfind { * lame for this query. */ #define DNS_ADBFIND_OVERQUOTA 0x00000400 +/*% + * Don't perform a fetch even if there are no address records available. + */ +#define DNS_ADBFIND_NOFETCH 0x00000800 /*% * The answers to queries come back as a list of these. diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 9df33c7..e13d684 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -175,6 +175,14 @@ #define DEFAULT_MAX_QUERIES 75 #endif +/* + * After NS_FAIL_LIMIT attempts to fetch a name server address, + * if the number of addresses in the NS RRset exceeds NS_RR_LIMIT, + * stop trying to fetch, in order to avoid wasting resources. + */ +#define NS_FAIL_LIMIT 4 +#define NS_RR_LIMIT 5 + /* Number of hash buckets for zone counters */ #ifndef RES_DOMAIN_BUCKETS #define RES_DOMAIN_BUCKETS 523 @@ -3086,8 +3094,8 @@ sort_finds(dns_adbfindlist_t *findlist, unsigned int bias) { static void findname(fetchctx_t *fctx, dns_name_t *name, in_port_t port, unsigned int options, unsigned int flags, isc_stdtime_t now, - isc_boolean_t *overquota, isc_boolean_t *need_alternate) -{ + isc_boolean_t *overquota, isc_boolean_t *need_alternate, + unsigned int *no_addresses) { dns_adbaddrinfo_t *ai; dns_adbfind_t *find; dns_resolver_t *res; @@ -3176,6 +3184,9 @@ findname(fetchctx_t *fctx, dns_name_t *name, in_port_t port, (res->dispatches6 == NULL && find->result_v4 != DNS_R_NXDOMAIN))) *need_alternate = ISC_TRUE; + if (no_addresses != NULL) { + (*no_addresses)++; + } } else { if ((find->options & DNS_ADBFIND_OVERQUOTA) != 0) { if (overquota != NULL) @@ -3226,6 +3237,7 @@ fctx_getaddresses(fetchctx_t *fctx, isc_boolean_t badcache) { dns_rdata_ns_t ns; isc_boolean_t need_alternate = ISC_FALSE; isc_boolean_t all_spilled = ISC_TRUE; + unsigned int no_addresses = 0; FCTXTRACE5("getaddresses", "fctx->depth=", fctx->depth); @@ -3384,8 +3396,13 @@ fctx_getaddresses(fetchctx_t *fctx, isc_boolean_t badcache) { if (result != ISC_R_SUCCESS) continue; - findname(fctx, &ns.name, 0, stdoptions, 0, now, - &overquota, &need_alternate); + if (no_addresses > NS_FAIL_LIMIT && + dns_rdataset_count(&fctx->nameservers) > NS_RR_LIMIT) + { + stdoptions |= DNS_ADBFIND_NOFETCH; + } + findname(fctx, &ns.name, 0, stdoptions, 0, now, &overquota, + &need_alternate, &no_addresses); if (!overquota) all_spilled = ISC_FALSE; @@ -3409,7 +3426,7 @@ fctx_getaddresses(fetchctx_t *fctx, isc_boolean_t badcache) { if (!a->isaddress) { findname(fctx, &a->_u._n.name, a->_u._n.port, stdoptions, FCTX_ADDRINFO_FORWARDER, - now, NULL, NULL); + now, NULL, NULL, NULL); continue; } if (isc_sockaddr_pf(&a->_u.addr) != family) @@ -3771,16 +3788,14 @@ fctx_try(fetchctx_t *fctx, isc_boolean_t retrying, isc_boolean_t badcache) { } } - if (dns_name_countlabels(&fctx->domain) > 2) { - result = isc_counter_increment(fctx->qc); - if (result != ISC_R_SUCCESS) { - isc_log_write(dns_lctx, DNS_LOGCATEGORY_RESOLVER, - DNS_LOGMODULE_RESOLVER, ISC_LOG_DEBUG(3), - "exceeded max queries resolving '%s'", - fctx->info); - fctx_done(fctx, DNS_R_SERVFAIL, __LINE__); - return; - } + result = isc_counter_increment(fctx->qc); + if (result != ISC_R_SUCCESS) { + isc_log_write(dns_lctx, DNS_LOGCATEGORY_RESOLVER, + DNS_LOGMODULE_RESOLVER, ISC_LOG_DEBUG(3), + "exceeded max queries resolving '%s'", + fctx->info); + fctx_done(fctx, DNS_R_SERVFAIL, __LINE__); + return; } bucketnum = fctx->bucketnum; -- 2.21.1