401 lines
12 KiB
Diff
401 lines
12 KiB
Diff
From 0a64c105173a39592d3dc1739d0e53ae02302012 Mon Sep 17 00:00:00 2001
|
|
From: Colin Vidal <colin@isc.org>
|
|
Date: Thu, 5 Feb 2026 09:46:01 +0100
|
|
Subject: [PATCH 1/2] Limit the number of addresses returned per ADB find
|
|
|
|
Add a hard limit on the number of addresses that ADB returns from a
|
|
single NS lookup (dns_adbfind_t). This mitigates a flood attack
|
|
where an attacker controls a zone with many addresses for a
|
|
nameserver, each returning an invalid response. The global
|
|
max-query count (default 50) also limits this, but significant harm
|
|
can be done before that limit is reached.
|
|
|
|
The default limit is now 6 (v4 and/or v6) addresses for an ADB find (so,
|
|
ADB looking up for A/AAAA addresses of a name server name).
|
|
|
|
(cherry picked from commit 9b61b194c58c02d4fbbbe439eceeec8fd2a2a54c)
|
|
---
|
|
lib/dns/adb.c | 21 +++++++++++++++++++++
|
|
1 file changed, 21 insertions(+)
|
|
|
|
diff --git a/lib/dns/adb.c b/lib/dns/adb.c
|
|
index a972395..fefec56 100644
|
|
--- a/lib/dns/adb.c
|
|
+++ b/lib/dns/adb.c
|
|
@@ -86,6 +86,12 @@
|
|
|
|
#define DNS_ADB_MINADBSIZE (1024U * 1024U) /*%< 1 Megabyte */
|
|
|
|
+/*
|
|
+ * Default for the per-find address limit, the sum of the number of A and AAAA
|
|
+ * RR from an ADB NS name resolution
|
|
+ */
|
|
+#define DEFAULT_ADDRSLIMIT 6
|
|
+
|
|
typedef ISC_LIST(dns_adbname_t) dns_adbnamelist_t;
|
|
typedef struct dns_adbnamehook dns_adbnamehook_t;
|
|
typedef ISC_LIST(dns_adbnamehook_t) dns_adbnamehooklist_t;
|
|
@@ -2228,6 +2234,7 @@ copy_namehook_lists(dns_adb_t *adb, dns_adbfind_t *find,
|
|
dns_adbaddrinfo_t *addrinfo;
|
|
dns_adbentry_t *entry;
|
|
int bucket;
|
|
+ size_t count = 0;
|
|
|
|
bucket = DNS_ADB_INVALIDBUCKET;
|
|
|
|
@@ -2262,6 +2269,13 @@ copy_namehook_lists(dns_adb_t *adb, dns_adbfind_t *find,
|
|
inc_entry_refcnt(adb, entry, false);
|
|
ISC_LIST_APPEND(find->list, addrinfo, publink);
|
|
addrinfo = NULL;
|
|
+
|
|
+ if (++count >= DEFAULT_ADDRSLIMIT) {
|
|
+ DP(ISC_LOG_DEBUG(3), "skipping addresses");
|
|
+ UNLOCK(&adb->entrylocks[bucket]);
|
|
+ return;
|
|
+ }
|
|
+
|
|
nextv4:
|
|
UNLOCK(&adb->entrylocks[bucket]);
|
|
bucket = DNS_ADB_INVALIDBUCKET;
|
|
@@ -2300,6 +2314,13 @@ copy_namehook_lists(dns_adb_t *adb, dns_adbfind_t *find,
|
|
inc_entry_refcnt(adb, entry, false);
|
|
ISC_LIST_APPEND(find->list, addrinfo, publink);
|
|
addrinfo = NULL;
|
|
+
|
|
+ if (++count >= DEFAULT_ADDRSLIMIT) {
|
|
+ DP(ISC_LOG_DEBUG(3), "skipping addresses");
|
|
+ UNLOCK(&adb->entrylocks[bucket]);
|
|
+ return;
|
|
+ }
|
|
+
|
|
nextv6:
|
|
UNLOCK(&adb->entrylocks[bucket]);
|
|
bucket = DNS_ADB_INVALIDBUCKET;
|
|
--
|
|
2.55.0
|
|
|
|
|
|
From bb7800197b4b2881428c69172c5abf15c12a40e2 Mon Sep 17 00:00:00 2001
|
|
From: Colin Vidal <colin@isc.org>
|
|
Date: Wed, 4 Feb 2026 10:18:42 +0100
|
|
Subject: [PATCH 2/2] Remove duplicate addresses from the resolver SLIST
|
|
|
|
The SLIST (essentially `fctx->finds`, forwarders and dual-stack
|
|
alternatives aside) can have duplicate server addresses when multiple
|
|
in-domain nameservers share the same IP addresses:
|
|
|
|
sub.example. NS ns1.sub.example.
|
|
sub.example. NS ns2.sub.example.
|
|
ns1.sub.example. A 1.2.3.4
|
|
ns1.sub.example. A 5.6.7.8
|
|
ns2.sub.example. A 1.2.3.4
|
|
ns2.sub.example. A 5.6.7.8
|
|
|
|
If both 1.2.3.4 and 5.6.7.8 fail to return a valid answer, the resolver
|
|
would query each address twice.
|
|
|
|
The problem is fixed by replacing the two-phase server selection (sort
|
|
each find list by SRTT, sort finds by head SRTT) with a single linear
|
|
scan in nextaddress() that finds the lowest-SRTT unmarked, non-duplicate
|
|
address across all find lists.
|
|
|
|
The old approach had a correctness bug: after sorting, the resolver
|
|
picked the next address from the "current" find list rather than
|
|
globally. For example, with find lists [1, 15, 26] and [3, 4, 5], the
|
|
second pick would be SRTT 15 instead of the correct SRTT 3.
|
|
|
|
The new approach is both simpler and correct: each call to nextaddress()
|
|
walks all addresses, skips marked and duplicate entries, and returns the
|
|
one with the lowest SRTT. While this walk is repeated for each server
|
|
attempt, it operates on a small bounded list and is negligible compared
|
|
to the network I/O of querying the server.
|
|
|
|
(cherry picked from commit ced6b66fafa1badfd044d569a6d3cfbdb600f5a7)
|
|
---
|
|
lib/dns/resolver.c | 220 ++++++++++++++++++---------------------------
|
|
1 file changed, 89 insertions(+), 131 deletions(-)
|
|
|
|
diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c
|
|
index 0807341..b413b89 100644
|
|
--- a/lib/dns/resolver.c
|
|
+++ b/lib/dns/resolver.c
|
|
@@ -319,7 +319,16 @@ struct fetchctx {
|
|
dns_message_t *qmessage;
|
|
ISC_LIST(resquery_t) queries;
|
|
dns_adbfindlist_t finds;
|
|
- dns_adbfind_t *find;
|
|
+ /*
|
|
+ * This is a state to keep track of the latest upstream server which is
|
|
+ * being queried. See `nextaddress()`.
|
|
+ *
|
|
+ * `addrinfo` is basically a copy of `foundaddrinfo` but came from the
|
|
+ * response of the query, so fields like the SRTT/timing might have been
|
|
+ * altered. So it might be possible (?) to wrap those two in an union
|
|
+ * for clarity (and memory saving).
|
|
+ */
|
|
+ dns_adbaddrinfo_t *foundaddrinfo;
|
|
/*
|
|
* altfinds are names and/or addresses of dual stack servers that
|
|
* should be used when iterative resolution to a server is not
|
|
@@ -1544,7 +1553,7 @@ fctx_cleanupfinds(fetchctx_t *fctx) {
|
|
ISC_LIST_UNLINK(fctx->finds, find, publink);
|
|
dns_adb_destroyfind(&find);
|
|
}
|
|
- fctx->find = NULL;
|
|
+ fctx->foundaddrinfo = NULL;
|
|
}
|
|
|
|
static void
|
|
@@ -3509,88 +3518,6 @@ add_bad(fetchctx_t *fctx, dns_message_t *rmessage, dns_adbaddrinfo_t *addrinfo,
|
|
dns_result_totext(reason), namebuf, typebuf, classbuf, addrbuf);
|
|
}
|
|
|
|
-/*
|
|
- * Sort addrinfo list by RTT.
|
|
- */
|
|
-static void
|
|
-sort_adbfind(dns_adbfind_t *find, unsigned int bias) {
|
|
- dns_adbaddrinfo_t *best, *curr;
|
|
- dns_adbaddrinfolist_t sorted;
|
|
- unsigned int best_srtt, curr_srtt;
|
|
-
|
|
- /* Lame N^2 bubble sort. */
|
|
- ISC_LIST_INIT(sorted);
|
|
- while (!ISC_LIST_EMPTY(find->list)) {
|
|
- best = ISC_LIST_HEAD(find->list);
|
|
- best_srtt = best->srtt;
|
|
- if (isc_sockaddr_pf(&best->sockaddr) != AF_INET6) {
|
|
- best_srtt += bias;
|
|
- }
|
|
- curr = ISC_LIST_NEXT(best, publink);
|
|
- while (curr != NULL) {
|
|
- curr_srtt = curr->srtt;
|
|
- if (isc_sockaddr_pf(&curr->sockaddr) != AF_INET6) {
|
|
- curr_srtt += bias;
|
|
- }
|
|
- if (curr_srtt < best_srtt) {
|
|
- best = curr;
|
|
- best_srtt = curr_srtt;
|
|
- }
|
|
- curr = ISC_LIST_NEXT(curr, publink);
|
|
- }
|
|
- ISC_LIST_UNLINK(find->list, best, publink);
|
|
- ISC_LIST_APPEND(sorted, best, publink);
|
|
- }
|
|
- find->list = sorted;
|
|
-}
|
|
-
|
|
-/*
|
|
- * Sort a list of finds by server RTT.
|
|
- */
|
|
-static void
|
|
-sort_finds(dns_adbfindlist_t *findlist, unsigned int bias) {
|
|
- dns_adbfind_t *best, *curr;
|
|
- dns_adbfindlist_t sorted;
|
|
- dns_adbaddrinfo_t *addrinfo, *bestaddrinfo;
|
|
- unsigned int best_srtt, curr_srtt;
|
|
-
|
|
- /* Sort each find's addrinfo list by SRTT. */
|
|
- for (curr = ISC_LIST_HEAD(*findlist); curr != NULL;
|
|
- curr = ISC_LIST_NEXT(curr, publink))
|
|
- {
|
|
- sort_adbfind(curr, bias);
|
|
- }
|
|
-
|
|
- /* Lame N^2 bubble sort. */
|
|
- ISC_LIST_INIT(sorted);
|
|
- while (!ISC_LIST_EMPTY(*findlist)) {
|
|
- best = ISC_LIST_HEAD(*findlist);
|
|
- bestaddrinfo = ISC_LIST_HEAD(best->list);
|
|
- INSIST(bestaddrinfo != NULL);
|
|
- best_srtt = bestaddrinfo->srtt;
|
|
- if (isc_sockaddr_pf(&bestaddrinfo->sockaddr) != AF_INET6) {
|
|
- best_srtt += bias;
|
|
- }
|
|
- curr = ISC_LIST_NEXT(best, publink);
|
|
- while (curr != NULL) {
|
|
- addrinfo = ISC_LIST_HEAD(curr->list);
|
|
- INSIST(addrinfo != NULL);
|
|
- curr_srtt = addrinfo->srtt;
|
|
- if (isc_sockaddr_pf(&addrinfo->sockaddr) != AF_INET6) {
|
|
- curr_srtt += bias;
|
|
- }
|
|
- if (curr_srtt < best_srtt) {
|
|
- best = curr;
|
|
- best_srtt = curr_srtt;
|
|
- }
|
|
- curr = ISC_LIST_NEXT(curr, publink);
|
|
- }
|
|
- ISC_LIST_UNLINK(*findlist, best, publink);
|
|
- ISC_LIST_APPEND(sorted, best, publink);
|
|
- }
|
|
- *findlist = sorted;
|
|
-}
|
|
-
|
|
static void
|
|
findname(fetchctx_t *fctx, const dns_name_t *name, in_port_t port,
|
|
unsigned int options, unsigned int flags, isc_stdtime_t now,
|
|
@@ -4052,8 +3979,6 @@ out:
|
|
* We've found some addresses. We might still be looking
|
|
* for more addresses.
|
|
*/
|
|
- sort_finds(&fctx->finds, res->view->v6bias);
|
|
- sort_finds(&fctx->altfinds, 0);
|
|
result = ISC_R_SUCCESS;
|
|
}
|
|
|
|
@@ -4128,6 +4053,80 @@ possibly_mark(fetchctx_t *fctx, dns_adbaddrinfo_t *addr) {
|
|
}
|
|
}
|
|
|
|
+static dns_adbaddrinfo_t *
|
|
+nextaddress(fetchctx_t *fctx) {
|
|
+ dns_adbaddrinfo_t *prevai = fctx->foundaddrinfo, *lowestsrttai = NULL;
|
|
+ unsigned int v6bias = fctx->res->view->v6bias, lowestsrtt = 0;
|
|
+
|
|
+ /*
|
|
+ * Let's walk through the list of dns_adbaddrinfo_t to find the best
|
|
+ * next server address to query. This is linear on the number of
|
|
+ * dns_adbaddrinfo_t which are grouped in find list (for each ADB find).
|
|
+ */
|
|
+ for (dns_adbfind_t *find = ISC_LIST_HEAD(fctx->finds); find != NULL;
|
|
+ find = ISC_LIST_NEXT(find, publink))
|
|
+ {
|
|
+ for (dns_adbaddrinfo_t *ai = ISC_LIST_HEAD(find->list);
|
|
+ ai != NULL; ai = ISC_LIST_NEXT(ai, publink))
|
|
+ {
|
|
+ /*
|
|
+ * This address has been marked already, skip it.
|
|
+ */
|
|
+ if (!UNMARKED(ai)) {
|
|
+ continue;
|
|
+ }
|
|
+
|
|
+ /*
|
|
+ * This address is the same as the previously used
|
|
+ * address, it's a duplicate, mark it and skip it!
|
|
+ */
|
|
+ if (prevai != NULL) {
|
|
+ if (prevai->entry == ai->entry) {
|
|
+ ai->flags |= FCTX_ADDRINFO_MARK;
|
|
+ continue;
|
|
+ }
|
|
+ }
|
|
+
|
|
+ /*
|
|
+ * Mark and skip this address if incompatible (i.e. IPv6
|
|
+ * address on a v4 only server, or for ACL reason, etc.)
|
|
+ */
|
|
+ possibly_mark(fctx, ai);
|
|
+ if (!UNMARKED(ai)) {
|
|
+ continue;
|
|
+ }
|
|
+
|
|
+ /*
|
|
+ * This address hasn't been tried yet and is a
|
|
+ * good candidate. Let's keep track of it if it
|
|
+ * has the lowest SRTT so far (or if there is no
|
|
+ * address with lowest SRTT found yet).
|
|
+ */
|
|
+ unsigned int aisrtt = ai->srtt;
|
|
+
|
|
+ if (isc_sockaddr_pf(&ai->sockaddr) != AF_INET6) {
|
|
+ aisrtt += v6bias;
|
|
+ }
|
|
+
|
|
+ if (lowestsrttai == NULL || aisrtt < lowestsrtt) {
|
|
+ lowestsrttai = ai;
|
|
+ lowestsrtt = aisrtt;
|
|
+ continue;
|
|
+ }
|
|
+ }
|
|
+ }
|
|
+
|
|
+ /*
|
|
+ * This is the next address to query. If this is NULL, we're done.
|
|
+ */
|
|
+ if (lowestsrttai != NULL) {
|
|
+ lowestsrttai->flags |= FCTX_ADDRINFO_MARK;
|
|
+ }
|
|
+ fctx->foundaddrinfo = lowestsrttai;
|
|
+
|
|
+ return lowestsrttai;
|
|
+}
|
|
+
|
|
static dns_adbaddrinfo_t *
|
|
fctx_nextaddress(fetchctx_t *fctx) {
|
|
dns_adbfind_t *find, *start;
|
|
@@ -4150,7 +4149,6 @@ fctx_nextaddress(fetchctx_t *fctx) {
|
|
possibly_mark(fctx, addrinfo);
|
|
if (UNMARKED(addrinfo)) {
|
|
addrinfo->flags |= FCTX_ADDRINFO_MARK;
|
|
- fctx->find = NULL;
|
|
fctx->forwarding = true;
|
|
|
|
/*
|
|
@@ -4171,49 +4169,9 @@ fctx_nextaddress(fetchctx_t *fctx) {
|
|
fctx->forwarding = false;
|
|
FCTX_ATTR_SET(fctx, FCTX_ATTR_TRIEDFIND);
|
|
|
|
- find = fctx->find;
|
|
- if (find == NULL) {
|
|
- find = ISC_LIST_HEAD(fctx->finds);
|
|
- } else {
|
|
- find = ISC_LIST_NEXT(find, publink);
|
|
- if (find == NULL) {
|
|
- find = ISC_LIST_HEAD(fctx->finds);
|
|
- }
|
|
- }
|
|
-
|
|
- /*
|
|
- * Find the first unmarked addrinfo.
|
|
- */
|
|
- addrinfo = NULL;
|
|
- if (find != NULL) {
|
|
- start = find;
|
|
- do {
|
|
- for (addrinfo = ISC_LIST_HEAD(find->list);
|
|
- addrinfo != NULL;
|
|
- addrinfo = ISC_LIST_NEXT(addrinfo, publink))
|
|
- {
|
|
- if (!UNMARKED(addrinfo)) {
|
|
- continue;
|
|
- }
|
|
- possibly_mark(fctx, addrinfo);
|
|
- if (UNMARKED(addrinfo)) {
|
|
- addrinfo->flags |= FCTX_ADDRINFO_MARK;
|
|
- break;
|
|
- }
|
|
- }
|
|
- if (addrinfo != NULL) {
|
|
- break;
|
|
- }
|
|
- find = ISC_LIST_NEXT(find, publink);
|
|
- if (find == NULL) {
|
|
- find = ISC_LIST_HEAD(fctx->finds);
|
|
- }
|
|
- } while (find != start);
|
|
- }
|
|
-
|
|
- fctx->find = find;
|
|
- if (addrinfo != NULL) {
|
|
- return (addrinfo);
|
|
+ faddrinfo = nextaddress(fctx);
|
|
+ if (faddrinfo != NULL) {
|
|
+ return faddrinfo;
|
|
}
|
|
|
|
/*
|
|
@@ -5239,7 +5197,7 @@ fctx_create(dns_resolver_t *res, const dns_name_t *name, dns_rdatatype_t type,
|
|
ISC_LIST_INIT(fctx->bad_edns);
|
|
ISC_LIST_INIT(fctx->validators);
|
|
fctx->validator = NULL;
|
|
- fctx->find = NULL;
|
|
+ fctx->foundaddrinfo = NULL;
|
|
fctx->altfind = NULL;
|
|
fctx->pending = 0;
|
|
fctx->restarts = 0;
|
|
--
|
|
2.55.0
|
|
|