bind9.16/bind-9.16-CVE-2024-1737.patch
Petr Menšík 1f78a2c237 Resolve CVE-2024-1737
6400.	[security]	Excessively large rdatasets can slow down database
			query processing, so a limit has been placed on the
			number of records that can be stored per rdataset
			in a cache or zone database. This is configured
			with the new "max-records-per-type" option, and
			defaults to 100. (CVE-2024-1737)
			[GL #497] [GL #3405]

6401.	[security]	An excessively large number of rrtypes per owner can
			slow down database query processing, so a limit has been
			placed on the number of rrtypes that can be stored per
			owner (node) in a cache or zone database. This is
			configured with the new "max-rrtypes-per-name" option,
			and defaults to 100. (CVE-2024-1737)
			[GL #3403] [GL #4548]

Does not change db methods like 9.18 fix. It makes limits set at build
time and fixed numbers, but does not need adjusting db interface to set
new limits.

Resolves: RHEL-50591
2024-08-08 00:50:22 +02:00

365 lines
13 KiB
Diff

From c5357835c98b7b028f8a041b6976bb335c9a4056 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ondrej@isc.org>
Date: Fri, 1 Mar 2024 08:26:07 +0100
Subject: [PATCH] Add a limit to the number of RRs in RRSets
Previously, the number of RRs in the RRSets were internally unlimited.
As the data structure that holds the RRs is just a linked list, and
there are places where we just walk through all of the RRs, adding an
RRSet with huge number of RRs inside would slow down processing of said
RRSets.
The fix for end-of-life branches make the limit compile-time only for
simplicity and the limit can be changed at the compile time by adding
following define to CFLAGS:
-DDNS_RDATASET_MAX_RECORDS=<limit>
(cherry picked from commit c5c4d00c38530390c9e1ae4c98b65fbbadfe9e5e)
(cherry picked from commit fdabf4b9570a60688f9f7d1e88d885f7a3718bca)
Add a limit to the number of RR types for single name
Previously, the number of RR types for a single owner name was limited
only by the maximum number of the types (64k). As the data structure
that holds the RR types for the database node is just a linked list, and
there are places where we just walk through the whole list (again and
again), adding a large number of RR types for a single owner named with
would slow down processing of such name (database node).
Add a hard-coded limit (100) to cap the number of the RR types for a single
owner. The limit can be changed at the compile time by adding following
define to CFLAGS:
-DDNS_RBTDB_MAX_RTYPES=<limit>
(cherry picked from commit dfcadc2085c8844b5836aff2b5ea51fb60c34868)
Optimize the slabheader placement for certain RRTypes
Mark the infrastructure RRTypes as "priority" types and place them at
the beginning of the rdataslab header data graph. The non-priority
types either go right after the priority types (if any).
(cherry picked from commit 3ac482be7fd058d284e89873021339579fad0615)
(cherry picked from commit 8ef414a7f38a04cfc11df44adaedaf3126fa3878)
Expand the list of the priority types
Add HTTPS, SVCB, SRV, PTR, NAPTR, DNSKEY and TXT records to the list of
the priority types that are put at the beginning of the slabheader list
for faster access and to avoid eviction when there are more types than
the max-types-per-name limit.
(cherry picked from commit b27c6bcce894786a8e082eafd59eccbf6f2731cb)
(cherry picked from commit d56d2a32b861e81c2aaaabd309c4c58b629ede32)
Make the resolver qtype ANY test order agnostic
Instead of relying on a specific order of the RR types in the databases
pick the first RR type as returned from the cache.
(cherry picked from commit 58f660cf2b800963fa649bc9823a626009db3a7e)
(cherry picked from commit c5ebda6deb0997dc520b26fa0639891459de5cb6)
Be smarter about refusing to add many RR types to the database
Instead of outright refusing to add new RR types to the cache, be a bit
smarter:
1. If the new header type is in our priority list, we always add either
positive or negative entry at the beginning of the list.
2. If the new header type is negative entry, and we are over the limit,
we mark it as ancient immediately, so it gets evicted from the cache
as soon as possible.
3. Otherwise add the new header after the priority headers (or at the
head of the list).
4. If we are over the limit, evict the last entry on the normal header
list.
(cherry picked from commit 57cd34441a1b4ecc9874a4a106c2c95b8d7a3120)
(cherry picked from commit 26c9da5f2857b72077c17e06ac79f068c63782cc)
---
bin/tests/system/resolver/tests.sh | 9 ++-
configure | 2 +-
configure.ac | 2 +-
lib/dns/rbtdb.c | 125 ++++++++++++++++++++++++++++-
lib/dns/rdataslab.c | 12 +++
5 files changed, 144 insertions(+), 6 deletions(-)
diff --git a/bin/tests/system/resolver/tests.sh b/bin/tests/system/resolver/tests.sh
index 6c69c1104e..bd997a61a4 100755
--- a/bin/tests/system/resolver/tests.sh
+++ b/bin/tests/system/resolver/tests.sh
@@ -553,15 +553,20 @@ n=`expr $n + 1`
echo_i "check prefetch qtype * (${n})"
ret=0
$DIG $DIGOPTS @10.53.0.5 fetchall.tld any > dig.out.1.${n} || ret=1
-ttl1=`awk '/"A" "short" "ttl"/ { print $2 - 3 }' dig.out.1.${n}`
+ttl1=$(awk '/^fetchall.tld/ { print $2 - 3; exit }' dig.out.1.${n})
# sleep so we are in prefetch range
sleep ${ttl1:-0}
# trigger prefetch
$DIG $DIGOPTS @10.53.0.5 fetchall.tld any > dig.out.2.${n} || ret=1
-ttl2=`awk '/"A" "short" "ttl"/ { print $2 }' dig.out.2.${n}`
+ttl2=$(awk '/^fetchall.tld/ { print $2; exit }' dig.out.2.${n})
sleep 1
# check that the nameserver is still alive
$DIG $DIGOPTS @10.53.0.5 fetchall.tld any > dig.out.3.${n} || ret=1
+# note that only the first record is prefetched,
+# because of the order of the records in the cache
+$DIG $DIGOPTS @10.53.0.5 fetchall.tld any >dig.out.3.${n} || ret=1
+ttl3=$(awk '/^fetchall.tld/ { print $2; exit }' dig.out.3.${n})
+test "${ttl3:-0}" -gt "${ttl2:-1}" || ret=1
if [ $ret != 0 ]; then echo_i "failed"; fi
status=`expr $status + $ret`
diff --git a/configure b/configure
index ed2d4869e5..be0f60eaba 100755
--- a/configure
+++ b/configure
@@ -12295,7 +12295,7 @@ fi
XTARGETS=
if test "$enable_developer" = "yes"; then :
- STD_CDEFINES="$STD_CDEFINES -DISC_MEM_DEFAULTFILL=1 -DISC_LIST_CHECKINIT=1"
+ STD_CDEFINES="$STD_CDEFINES -DISC_MEM_DEFAULTFILL=1 -DISC_LIST_CHECKINIT=1 -DDNS_RDATASET_MAX_RECORDS=5000 -DDNS_RBTDB_MAX_RTYPES=5000"
test "${enable_fixed_rrset+set}" = set || enable_fixed_rrset=yes
test "${enable_querytrace+set}" = set || enable_querytrace=yes
test "${with_cmocka+set}" = set || with_cmocka=yes
diff --git a/configure.ac b/configure.ac
index 287de41369..3ff4bdd135 100644
--- a/configure.ac
+++ b/configure.ac
@@ -94,7 +94,7 @@ AC_ARG_ENABLE([developer],
XTARGETS=
AS_IF([test "$enable_developer" = "yes"],
- [STD_CDEFINES="$STD_CDEFINES -DISC_MEM_DEFAULTFILL=1 -DISC_LIST_CHECKINIT=1"
+ [STD_CDEFINES="$STD_CDEFINES -DISC_MEM_DEFAULTFILL=1 -DISC_LIST_CHECKINIT=1 -DDNS_RDATASET_MAX_RECORDS=5000 -DDNS_RBTDB_MAX_RTYPES=5000"
test "${enable_fixed_rrset+set}" = set || enable_fixed_rrset=yes
test "${enable_querytrace+set}" = set || enable_querytrace=yes
test "${with_cmocka+set}" = set || with_cmocka=yes
diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c
index 2707507bd7..e840c0665d 100644
--- a/lib/dns/rbtdb.c
+++ b/lib/dns/rbtdb.c
@@ -967,6 +967,48 @@ set_ttl(dns_rbtdb_t *rbtdb, rdatasetheader_t *header, dns_ttl_t newttl) {
}
}
+static bool
+prio_type(rbtdb_rdatatype_t type) {
+ switch (type) {
+ case dns_rdatatype_soa:
+ case RBTDB_RDATATYPE_VALUE(dns_rdatatype_rrsig, dns_rdatatype_soa):
+ case dns_rdatatype_a:
+ case RBTDB_RDATATYPE_VALUE(dns_rdatatype_rrsig, dns_rdatatype_a):
+ case dns_rdatatype_mx:
+ case RBTDB_RDATATYPE_VALUE(dns_rdatatype_rrsig, dns_rdatatype_mx):
+ case dns_rdatatype_aaaa:
+ case RBTDB_RDATATYPE_VALUE(dns_rdatatype_rrsig, dns_rdatatype_aaaa):
+ case dns_rdatatype_nsec:
+ case RBTDB_RDATATYPE_VALUE(dns_rdatatype_rrsig, dns_rdatatype_nsec):
+ case dns_rdatatype_nsec3:
+ case RBTDB_RDATATYPE_VALUE(dns_rdatatype_rrsig, dns_rdatatype_nsec3):
+ case dns_rdatatype_ns:
+ case RBTDB_RDATATYPE_VALUE(dns_rdatatype_rrsig, dns_rdatatype_ns):
+ case dns_rdatatype_ds:
+ case RBTDB_RDATATYPE_VALUE(dns_rdatatype_rrsig, dns_rdatatype_ds):
+ case dns_rdatatype_cname:
+ case RBTDB_RDATATYPE_VALUE(dns_rdatatype_rrsig, dns_rdatatype_cname):
+ case dns_rdatatype_dname:
+ case RBTDB_RDATATYPE_VALUE(dns_rdatatype_rrsig, dns_rdatatype_dname):
+ case dns_rdatatype_svcb:
+ case RBTDB_RDATATYPE_VALUE(dns_rdatatype_rrsig, dns_rdatatype_svcb):
+ case dns_rdatatype_https:
+ case RBTDB_RDATATYPE_VALUE(dns_rdatatype_rrsig, dns_rdatatype_https):
+ case dns_rdatatype_dnskey:
+ case RBTDB_RDATATYPE_VALUE(dns_rdatatype_rrsig, dns_rdatatype_dnskey):
+ case dns_rdatatype_srv:
+ case RBTDB_RDATATYPE_VALUE(dns_rdatatype_rrsig, dns_rdatatype_srv):
+ case dns_rdatatype_txt:
+ case RBTDB_RDATATYPE_VALUE(dns_rdatatype_rrsig, dns_rdatatype_txt):
+ case dns_rdatatype_ptr:
+ case RBTDB_RDATATYPE_VALUE(dns_rdatatype_rrsig, dns_rdatatype_ptr):
+ case dns_rdatatype_naptr:
+ case RBTDB_RDATATYPE_VALUE(dns_rdatatype_rrsig, dns_rdatatype_naptr):
+ return (true);
+ }
+ return (false);
+}
+
/*%
* These functions allow the heap code to rank the priority of each
* element. It returns true if v1 happens "sooner" than v2.
@@ -6179,6 +6221,30 @@ update_recordsandxfrsize(bool add, rbtdb_version_t *rbtversion,
RWUNLOCK(&rbtversion->rwlock, isc_rwlocktype_write);
}
+#ifndef DNS_RBTDB_MAX_RTYPES
+#define DNS_RBTDB_MAX_RTYPES 100
+#endif /* DNS_RBTDB_MAX_RTYPES */
+
+static bool
+overmaxtype(dns_rbtdb_t *rbtdb, uint32_t ntypes) {
+ UNUSED(rbtdb);
+
+ if (DNS_RBTDB_MAX_RTYPES == 0) {
+ return (false);
+ }
+
+ return (ntypes >= DNS_RBTDB_MAX_RTYPES);
+}
+
+static bool
+prio_header(rdatasetheader_t *header) {
+ if (NEGATIVE(header) && prio_type(RBTDB_RDATATYPE_EXT(header->type))) {
+ return (true);
+ }
+
+ return (prio_type(header->type));
+}
+
/*
* write lock on rbtnode must be held.
*/
@@ -6190,6 +6256,7 @@ add32(dns_rbtdb_t *rbtdb, dns_rbtnode_t *rbtnode, const dns_name_t *nodename,
rbtdb_changed_t *changed = NULL;
rdatasetheader_t *topheader = NULL, *topheader_prev = NULL;
rdatasetheader_t *header = NULL, *sigheader = NULL;
+ rdatasetheader_t *prioheader = NULL, *expireheader = NULL;
unsigned char *merged = NULL;
isc_result_t result;
bool header_nx;
@@ -6199,6 +6266,7 @@ add32(dns_rbtdb_t *rbtdb, dns_rbtnode_t *rbtnode, const dns_name_t *nodename,
rbtdb_rdatatype_t negtype, sigtype;
dns_trust_t trust;
int idx;
+ uint32_t ntypes = 0;
/*
* Add an rdatasetheader_t to a node.
@@ -6272,6 +6340,7 @@ add32(dns_rbtdb_t *rbtdb, dns_rbtnode_t *rbtnode, const dns_name_t *nodename,
topheader = topheader->next) {
if (topheader->type == sigtype) {
sigheader = topheader;
+ break;
}
}
negtype = RBTDB_RDATATYPE_VALUE(covers, 0);
@@ -6331,6 +6400,15 @@ add32(dns_rbtdb_t *rbtdb, dns_rbtnode_t *rbtnode, const dns_name_t *nodename,
for (topheader = rbtnode->data; topheader != NULL;
topheader = topheader->next) {
+ if (IS_CACHE(rbtdb) && ACTIVE(topheader, now)) {
+ ++ntypes;
+ expireheader = topheader;
+ } else if (!IS_CACHE(rbtdb)) {
+ ++ntypes;
+ }
+ if (prio_header(topheader)) {
+ prioheader = topheader;
+ }
if (topheader->type == newheader->type ||
topheader->type == negtype) {
break;
@@ -6712,9 +6790,52 @@ find_header:
/*
* No rdatasets of the given type exist at the node.
*/
- newheader->next = rbtnode->data;
+ if (!IS_CACHE(rbtdb) && overmaxtype(rbtdb, ntypes)) {
+ free_rdataset(rbtdb, rbtdb->common.mctx,
+ newheader);
+ return (ISC_R_QUOTA);
+ }
+
newheader->down = NULL;
- rbtnode->data = newheader;
+
+ if (prio_header(newheader)) {
+ /* This is a priority type, prepend it */
+ newheader->next = rbtnode->data;
+ rbtnode->data = newheader;
+ } else if (prioheader != NULL) {
+ /* Append after the priority headers */
+ newheader->next = prioheader->next;
+ prioheader->next = newheader;
+ } else {
+ /* There were no priority headers */
+ newheader->next = rbtnode->data;
+ rbtnode->data = newheader;
+ }
+
+ if (IS_CACHE(rbtdb) && overmaxtype(rbtdb, ntypes)) {
+ if (expireheader == NULL) {
+ expireheader = newheader;
+ }
+ if (NEGATIVE(newheader) &&
+ !prio_header(newheader))
+ {
+ /*
+ * Add the new non-priority negative
+ * header to the database only
+ * temporarily.
+ */
+ expireheader = newheader;
+ }
+
+ set_ttl(rbtdb, expireheader, 0);
+ mark_header_ancient(rbtdb, expireheader);
+ /*
+ * FIXME: In theory, we should mark the RRSIG
+ * and the header at the same time, but there is
+ * no direct link between those two header, so
+ * we would have to check the whole list again.
+ */
+ }
}
}
diff --git a/lib/dns/rdataslab.c b/lib/dns/rdataslab.c
index 1d5e88f745..dda903819a 100644
--- a/lib/dns/rdataslab.c
+++ b/lib/dns/rdataslab.c
@@ -110,6 +110,10 @@ fillin_offsets(unsigned char *offsetbase, unsigned int *offsettable,
}
#endif /* if DNS_RDATASET_FIXED */
+#ifndef DNS_RDATASET_MAX_RECORDS
+#define DNS_RDATASET_MAX_RECORDS 100
+#endif /* DNS_RDATASET_MAX_RECORDS */
+
isc_result_t
dns_rdataslab_fromrdataset(dns_rdataset_t *rdataset, isc_mem_t *mctx,
isc_region_t *region, unsigned int reservelen) {
@@ -154,6 +158,10 @@ dns_rdataslab_fromrdataset(dns_rdataset_t *rdataset, isc_mem_t *mctx,
return (ISC_R_SUCCESS);
}
+ if (nitems > DNS_RDATASET_MAX_RECORDS) {
+ return (DNS_R_TOOMANYRECORDS);
+ }
+
if (nitems > 0xffff) {
return (ISC_R_NOSPACE);
}
@@ -520,6 +528,10 @@ dns_rdataslab_merge(unsigned char *oslab, unsigned char *nslab,
#endif /* if DNS_RDATASET_FIXED */
INSIST(ocount > 0 && ncount > 0);
+ if (ocount + ncount > DNS_RDATASET_MAX_RECORDS) {
+ return (DNS_R_TOOMANYRECORDS);
+ }
+
#if DNS_RDATASET_FIXED
oncount = ncount;
#endif /* if DNS_RDATASET_FIXED */
--
2.45.2