Improve RBT overmem cache cleaning (CVE-2023-2828)

This commit is contained in:
Andrew Lukoshko 2023-07-17 21:27:38 +00:00
parent fd05d77e87
commit adb0054341
2 changed files with 198 additions and 1 deletions

View File

@ -0,0 +1,190 @@
From f1d9e9ee3859976f403914d20ad2a10855343702 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ondrej@isc.org>
Date: Tue, 30 May 2023 08:46:17 +0200
Subject: [PATCH] Improve RBT overmem cache cleaning
When cache memory usage is over the configured cache size (overmem) and
we are cleaning unused entries, it might not be enough to clean just two
entries if the entries to be expired are smaller than the newly added
rdata. This could be abused by an attacker to cause a remote Denial of
Service by possibly running out of the operating system memory.
Currently, the addrdataset() tries to do a single TTL-based cleaning
considering the serve-stale TTL and then optionally moves to overmem
cleaning if we are in that condition. Then the overmem_purge() tries to
do another single TTL based cleaning from the TTL heap and then continue
with LRU-based cleaning up to 2 entries cleaned.
Squash the TTL-cleaning mechanism into single call from addrdataset(),
but ignore the serve-stale TTL if we are currently overmem.
Then instead of having a fixed number of entries to clean, pass the size
of newly added rdatasetheader to the overmem_purge() function and
cleanup at least the size of the newly added data. This prevents the
cache going over the configured memory limit (`max-cache-size`).
Additionally, refactor the overmem_purge() function to reduce for-loop
nesting for readability.
---
lib/dns/rbtdb.c | 105 ++++++++++++++++++++++++++++++------------------
1 file changed, 65 insertions(+), 40 deletions(-)
diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c
index c32db44cad7..ee06c51e925 100644
--- a/lib/dns/rbtdb.c
+++ b/lib/dns/rbtdb.c
@@ -605,7 +605,7 @@ static void
expire_header(dns_rbtdb_t *rbtdb, rdatasetheader_t *header, bool tree_locked,
expire_t reason);
static void
-overmem_purge(dns_rbtdb_t *rbtdb, unsigned int locknum_start, isc_stdtime_t now,
+overmem_purge(dns_rbtdb_t *rbtdb, unsigned int locknum_start, size_t purgesize,
bool tree_locked);
static isc_result_t
resign_insert(dns_rbtdb_t *rbtdb, int idx, rdatasetheader_t *newheader);
@@ -6823,6 +6823,16 @@ cleanup:
static dns_dbmethods_t zone_methods;
+static size_t
+rdataset_size(rdatasetheader_t *header) {
+ if (!NONEXISTENT(header)) {
+ return (dns_rdataslab_size((unsigned char *)header,
+ sizeof(*header)));
+ }
+
+ return (sizeof(*header));
+}
+
static isc_result_t
addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
isc_stdtime_t now, dns_rdataset_t *rdataset, unsigned int options,
@@ -6987,7 +6997,8 @@ addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
}
if (cache_is_overmem) {
- overmem_purge(rbtdb, rbtnode->locknum, now, tree_locked);
+ overmem_purge(rbtdb, rbtnode->locknum, rdataset_size(newheader),
+ tree_locked);
}
NODE_LOCK(&rbtdb->node_locks[rbtnode->locknum].lock,
@@ -7006,10 +7017,18 @@ addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
}
header = isc_heap_element(rbtdb->heaps[rbtnode->locknum], 1);
- if (header != NULL && header->rdh_ttl + rbtdb->serve_stale_ttl <
- now - RBTDB_VIRTUAL)
- {
- expire_header(rbtdb, header, tree_locked, expire_ttl);
+ if (header != NULL) {
+ dns_ttl_t rdh_ttl = header->rdh_ttl;
+
+ /* Only account for stale TTL if cache is not overmem */
+ if (!cache_is_overmem) {
+ rdh_ttl += rbtdb->serve_stale_ttl;
+ }
+
+ if (rdh_ttl < now - RBTDB_VIRTUAL) {
+ expire_header(rbtdb, header, tree_locked,
+ expire_ttl);
+ }
}
/*
@@ -10547,52 +10566,58 @@ update_header(dns_rbtdb_t *rbtdb, rdatasetheader_t *header, isc_stdtime_t now) {
ISC_LIST_PREPEND(rbtdb->rdatasets[header->node->locknum], header, link);
}
+static size_t
+expire_lru_headers(dns_rbtdb_t *rbtdb, unsigned int locknum, size_t purgesize,
+ bool tree_locked) {
+ rdatasetheader_t *header, *header_prev;
+ size_t purged = 0;
+
+ for (header = ISC_LIST_TAIL(rbtdb->rdatasets[locknum]);
+ header != NULL && purged <= purgesize; header = header_prev)
+ {
+ header_prev = ISC_LIST_PREV(header, link);
+ /*
+ * Unlink the entry at this point to avoid checking it
+ * again even if it's currently used someone else and
+ * cannot be purged at this moment. This entry won't be
+ * referenced any more (so unlinking is safe) since the
+ * TTL was reset to 0.
+ */
+ ISC_LIST_UNLINK(rbtdb->rdatasets[locknum], header, link);
+ size_t header_size = rdataset_size(header);
+ expire_header(rbtdb, header, tree_locked, expire_lru);
+ purged += header_size;
+ }
+
+ return (purged);
+}
+
/*%
- * Purge some expired and/or stale (i.e. unused for some period) cache entries
- * under an overmem condition. To recover from this condition quickly, up to
- * 2 entries will be purged. This process is triggered while adding a new
- * entry, and we specifically avoid purging entries in the same LRU bucket as
- * the one to which the new entry will belong. Otherwise, we might purge
- * entries of the same name of different RR types while adding RRsets from a
- * single response (consider the case where we're adding A and AAAA glue records
- * of the same NS name).
+ * Purge some stale (i.e. unused for some period - LRU based cleaning) cache
+ * entries under the overmem condition. To recover from this condition quickly,
+ * we cleanup entries up to the size of newly added rdata (passed as purgesize).
+ *
+ * This process is triggered while adding a new entry, and we specifically avoid
+ * purging entries in the same LRU bucket as the one to which the new entry will
+ * belong. Otherwise, we might purge entries of the same name of different RR
+ * types while adding RRsets from a single response (consider the case where
+ * we're adding A and AAAA glue records of the same NS name).
*/
static void
-overmem_purge(dns_rbtdb_t *rbtdb, unsigned int locknum_start, isc_stdtime_t now,
+overmem_purge(dns_rbtdb_t *rbtdb, unsigned int locknum_start, size_t purgesize,
bool tree_locked) {
- rdatasetheader_t *header, *header_prev;
unsigned int locknum;
- int purgecount = 2;
+ size_t purged = 0;
for (locknum = (locknum_start + 1) % rbtdb->node_lock_count;
- locknum != locknum_start && purgecount > 0;
+ locknum != locknum_start && purged <= purgesize;
locknum = (locknum + 1) % rbtdb->node_lock_count)
{
NODE_LOCK(&rbtdb->node_locks[locknum].lock,
isc_rwlocktype_write);
- header = isc_heap_element(rbtdb->heaps[locknum], 1);
- if (header && header->rdh_ttl < now - RBTDB_VIRTUAL) {
- expire_header(rbtdb, header, tree_locked, expire_ttl);
- purgecount--;
- }
-
- for (header = ISC_LIST_TAIL(rbtdb->rdatasets[locknum]);
- header != NULL && purgecount > 0; header = header_prev)
- {
- header_prev = ISC_LIST_PREV(header, link);
- /*
- * Unlink the entry at this point to avoid checking it
- * again even if it's currently used someone else and
- * cannot be purged at this moment. This entry won't be
- * referenced any more (so unlinking is safe) since the
- * TTL was reset to 0.
- */
- ISC_LIST_UNLINK(rbtdb->rdatasets[locknum], header,
- link);
- expire_header(rbtdb, header, tree_locked, expire_lru);
- purgecount--;
- }
+ purged += expire_lru_headers(rbtdb, locknum, purgesize - purged,
+ tree_locked);
NODE_UNLOCK(&rbtdb->node_locks[locknum].lock,
isc_rwlocktype_write);
--
GitLab

View File

@ -57,7 +57,7 @@ Summary: The Berkeley Internet Name Domain (BIND) DNS (Domain Name System) serv
Name: bind9.16 Name: bind9.16
License: MPLv2.0 License: MPLv2.0
Version: 9.16.23 Version: 9.16.23
Release: 0.14%{?dist} Release: 0.14%{?dist}.1.alma
Epoch: 32 Epoch: 32
Url: https://www.isc.org/downloads/bind/ Url: https://www.isc.org/downloads/bind/
# #
@ -126,6 +126,9 @@ Patch185: bind-9.16-CVE-2022-3094-test.patch
Patch186: bind-9.16-CVE-2022-3736.patch Patch186: bind-9.16-CVE-2022-3736.patch
# https://gitlab.isc.org/isc-projects/bind9/commit/b4a65aaea19762a3712932aa2270e8a833fbde22 # https://gitlab.isc.org/isc-projects/bind9/commit/b4a65aaea19762a3712932aa2270e8a833fbde22
Patch187: bind-9.16-CVE-2022-3924.patch Patch187: bind-9.16-CVE-2022-3924.patch
# Patch is taken from bind upstream gitlab and updated to apply correctly:
# https://gitlab.isc.org/isc-projects/bind9/commit/f1d9e9ee3859976f403914d20ad2a10855343702
Patch188: bind-9.16-CVE-2023-2828.patch
%{?systemd_ordering} %{?systemd_ordering}
Requires: coreutils Requires: coreutils
@ -437,6 +440,7 @@ in HTML and PDF format.
%patch185 -p1 -b .CVE-2022-3094-test %patch185 -p1 -b .CVE-2022-3094-test
%patch186 -p1 -b .CVE-2022-3736 %patch186 -p1 -b .CVE-2022-3736
%patch187 -p1 -b .CVE-2022-3924 %patch187 -p1 -b .CVE-2022-3924
%patch188 -p1 -b .CVE-2023-2828
%if %{with PKCS11} %if %{with PKCS11}
%patch135 -p1 -b .config-pkcs11 %patch135 -p1 -b .config-pkcs11
@ -1156,6 +1160,9 @@ fi;
%endif %endif
%changelog %changelog
* Mon Jul 17 2023 Andrew Lukoshko <alukoshko@almalinux.org> - 32:9.16.23-0.14.1.alma
- Improve RBT overmem cache cleaning (CVE-2023-2828)
* Sat Feb 25 2023 Petr Menšík <pemensik@redhat.com> - 32:9.16.23-0.14 * Sat Feb 25 2023 Petr Menšík <pemensik@redhat.com> - 32:9.16.23-0.14
- Handle subtle difference between upstream and rhel (CVE-2022-3094) - Handle subtle difference between upstream and rhel (CVE-2022-3094)