Limit the additional processing for large RDATA sets

When answering queries, don't add data to the additional section if
the answer has more than 13 names in the RDATA.  This limits the
number of lookups into the database(s) during a single client query,
reducing query processing load.

Also, don't append any additional data to type=ANY queries. The
answer to ANY is already big enough.

Fix vulnerability test backport

Vulnerability: CVE-2024-11187
Resolves: RHEL-77103
This commit is contained in:
Petr Menšík 2025-02-04 14:32:45 +01:00
parent aec75c660e
commit 2e48ec71f0
3 changed files with 241 additions and 1 deletions

View File

@ -0,0 +1,61 @@
From cc01143082bc688a371a7378ef284c898eedc9df Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ondrej@isc.org>
Date: Tue, 7 Jan 2025 15:22:40 +0100
Subject: [PATCH] Isolate using the -T noaa flag only for part of the resolver
test
Instead of running the whole resolver/ns4 server with -T noaa flag,
use it only for the part where it is actually needed. The -T noaa
could interfere with other parts of the test because the answers don't
have the authoritative-answer bit set, and we could have false
positives (or false negatives) in the test because the authoritative
server doesn't follow the DNS protocol for all the tests in the resolver
system test.
(cherry picked from commit e51d4d3b88af00d6667f2055087ebfc47fb3107c)
---
bin/tests/system/resolver/ns4/named.noaa | 5 -----
bin/tests/system/resolver/tests.sh | 8 ++++++++
2 files changed, 8 insertions(+), 5 deletions(-)
delete mode 100644 bin/tests/system/resolver/ns4/named.noaa
diff --git a/bin/tests/system/resolver/ns4/named.noaa b/bin/tests/system/resolver/ns4/named.noaa
deleted file mode 100644
index 3b121ad..0000000
--- a/bin/tests/system/resolver/ns4/named.noaa
+++ /dev/null
@@ -1,5 +0,0 @@
-Copyright (C) Internet Systems Consortium, Inc. ("ISC")
-
-See COPYRIGHT in the source root or https://isc.org/copyright.html for terms.
-
-Add -T noaa.
diff --git a/bin/tests/system/resolver/tests.sh b/bin/tests/system/resolver/tests.sh
index 711ee05..2eae16f 100755
--- a/bin/tests/system/resolver/tests.sh
+++ b/bin/tests/system/resolver/tests.sh
@@ -289,6 +289,10 @@ done
if [ $ret != 0 ]; then echo_i "failed"; fi
status=`expr $status + $ret`
+stop_server resolver ns4
+touch ns4/named.noaa
+start_server --noclean --restart --port ${PORT} resolver ns4 || ret=1
+
n=`expr $n + 1`
echo_i "RT21594 regression test check setup ($n)"
ret=0
@@ -325,6 +329,10 @@ grep "status: NXDOMAIN" dig.ns5.out.${n} > /dev/null || ret=1
if [ $ret != 0 ]; then echo_i "failed"; fi
status=`expr $status + $ret`
+stop_server resolver ns4
+rm ns4/named.noaa
+start_server --noclean --restart --port ${PORT} resolver ns4 || ret=1
+
n=`expr $n + 1`
echo_i "check that replacement of additional data by a negative cache no data entry clears the additional RRSIGs ($n)"
ret=0
--
2.48.1

View File

@ -0,0 +1,172 @@
From b095854ee885dd5960cb54e7fbefb962498e9fdb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ondrej@isc.org>
Date: Thu, 14 Nov 2024 10:37:29 +0100
Subject: [PATCH] Limit the additional processing for large RDATA sets
When answering queries, don't add data to the additional section if
the answer has more than 13 names in the RDATA. This limits the
number of lookups into the database(s) during a single client query,
reducing query processing load.
Also, don't append any additional data to type=ANY queries. The
answer to ANY is already big enough.
(cherry picked from commit a1982cf1bb95c818aa7b58988b5611dec80f2408)
PatchNumber: 47
---
bin/tests/system/additional/tests.sh | 2 +-
lib/dns/include/dns/rdataset.h | 12 ++++++++++++
lib/dns/rbtdb.c | 2 +-
lib/dns/rdataset.c | 11 +++++++++++
lib/ns/query.c | 15 ++++++++++-----
5 files changed, 35 insertions(+), 7 deletions(-)
diff --git a/bin/tests/system/additional/tests.sh b/bin/tests/system/additional/tests.sh
index 025f11f..846c800 100644
--- a/bin/tests/system/additional/tests.sh
+++ b/bin/tests/system/additional/tests.sh
@@ -260,7 +260,7 @@ n=`expr $n + 1`
echo_i "testing with 'minimal-any no;' ($n)"
ret=0
$DIG $DIGOPTS -t ANY www.rt.example @10.53.0.1 > dig.out.$n || ret=1
-grep "ANSWER: 3, AUTHORITY: 2, ADDITIONAL: 2" dig.out.$n > /dev/null || ret=1
+grep "ANSWER: 3, AUTHORITY: 2, ADDITIONAL: 1" dig.out.$n > /dev/null || ret=1
if [ $ret -eq 1 ] ; then
echo_i "failed"; status=$((status+1))
fi
diff --git a/lib/dns/include/dns/rdataset.h b/lib/dns/include/dns/rdataset.h
index f2585ef..54ad7b2 100644
--- a/lib/dns/include/dns/rdataset.h
+++ b/lib/dns/include/dns/rdataset.h
@@ -53,6 +53,8 @@
#include <dns/rdatastruct.h>
#include <dns/types.h>
+#define DNS_RDATASET_MAXADDITIONAL 13
+
ISC_LANG_BEGINDECLS
typedef enum {
@@ -458,13 +460,23 @@ dns_rdataset_additionaldata(dns_rdataset_t *rdataset,
*\li If a call to dns_rdata_additionaldata() is not successful, the
* result returned will be the result of dns_rdataset_additionaldata().
*
+ *\li If 'limit' is non-zero and the number of the rdatasets is larger
+ * than 'limit', no additional data will be processed.
+ *
* Returns:
*
*\li #ISC_R_SUCCESS
*
+ *\li #DNS_R_TOOMANYRECORDS in case rdataset count is larger than 'limit'
+ *
*\li Any error that dns_rdata_additionaldata() can return.
*/
+isc_result_t
+dns_rdataset_additionaldata2(dns_rdataset_t *rdataset,
+ dns_additionaldatafunc_t add, void *arg,
+ size_t limit);
+
isc_result_t
dns_rdataset_getnoqname(dns_rdataset_t *rdataset, dns_name_t *name,
dns_rdataset_t *neg, dns_rdataset_t *negsig);
diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c
index ed5015c..7586e2a 100644
--- a/lib/dns/rbtdb.c
+++ b/lib/dns/rbtdb.c
@@ -10626,7 +10626,7 @@ no_glue:
maybe_rehash_gluetable(rbtversion);
idx = hash_32(hash, rbtversion->glue_table_bits);
- (void)dns_rdataset_additionaldata(rdataset, glue_nsdname_cb, &ctx);
+ (void)dns_rdataset_additionaldata2(rdataset, glue_nsdname_cb, &ctx, 0);
cur = isc_mem_get(rbtdb->common.mctx, sizeof(*cur));
diff --git a/lib/dns/rdataset.c b/lib/dns/rdataset.c
index bf9e7af..ffe6163 100644
--- a/lib/dns/rdataset.c
+++ b/lib/dns/rdataset.c
@@ -27,6 +27,7 @@
#include <dns/ncache.h>
#include <dns/rdata.h>
#include <dns/rdataset.h>
+#include <dns/result.h>
static const char *trustnames[] = {
"none", "pending-additional",
@@ -577,6 +578,12 @@ dns_rdataset_towire(dns_rdataset_t *rdataset, const dns_name_t *owner_name,
isc_result_t
dns_rdataset_additionaldata(dns_rdataset_t *rdataset,
dns_additionaldatafunc_t add, void *arg) {
+ return dns_rdataset_additionaldata2(rdataset, add, arg, 0);
+}
+
+isc_result_t
+dns_rdataset_additionaldata2(dns_rdataset_t *rdataset,
+ dns_additionaldatafunc_t add, void *arg, size_t limit) {
dns_rdata_t rdata = DNS_RDATA_INIT;
isc_result_t result;
@@ -588,6 +595,10 @@ dns_rdataset_additionaldata(dns_rdataset_t *rdataset,
REQUIRE(DNS_RDATASET_VALID(rdataset));
REQUIRE((rdataset->attributes & DNS_RDATASETATTR_QUESTION) == 0);
+ if (limit != 0 && dns_rdataset_count(rdataset) > limit) {
+ return DNS_R_TOOMANYRECORDS;
+ }
+
result = dns_rdataset_first(rdataset);
if (result != ISC_R_SUCCESS) {
return (result);
diff --git a/lib/ns/query.c b/lib/ns/query.c
index f7b3f78..5bc2908 100644
--- a/lib/ns/query.c
+++ b/lib/ns/query.c
@@ -2030,8 +2030,9 @@ addname:
* This cannot go more than MAX_RESTARTS levels deep.
*/
if (trdataset != NULL && dns_rdatatype_followadditional(type)) {
- eresult = dns_rdataset_additionaldata(
- trdataset, query_additional_cb, qctx);
+ eresult = dns_rdataset_additionaldata2(
+ trdataset, query_additional_cb, qctx,
+ DNS_RDATASET_MAXADDITIONAL);
}
cleanup:
@@ -2122,7 +2123,8 @@ regular:
* Add other additional data if needed.
* We don't care if dns_rdataset_additionaldata() fails.
*/
- (void)dns_rdataset_additionaldata(rdataset, query_additional_cb, qctx);
+ (void)dns_rdataset_additionaldata2(rdataset, query_additional_cb,
+ qctx, DNS_RDATASET_MAXADDITIONAL);
CTRACE(ISC_LOG_DEBUG(3), "query_additional: done");
}
@@ -2148,7 +2150,8 @@ query_addrrset(query_ctx_t *qctx, dns_name_t **namep,
* To the current response for 'client', add the answer RRset
* '*rdatasetp' and an optional signature set '*sigrdatasetp', with
* owner name '*namep', to section 'section', unless they are
- * already there. Also add any pertinent additional data.
+ * already there. Also add any pertinent additional data, unless
+ * the query was for type ANY.
*
* If 'dbuf' is not NULL, then '*namep' is the name whose data is
* stored in 'dbuf'. In this case, query_addrrset() guarantees that
@@ -2203,7 +2206,9 @@ query_addrrset(query_ctx_t *qctx, dns_name_t **namep,
*/
query_addtoname(mname, rdataset);
query_setorder(qctx, mname, rdataset);
- query_additional(qctx, rdataset);
+ if (qctx->qtype != dns_rdatatype_any) {
+ query_additional(qctx, rdataset);
+ }
/*
* Note: we only add SIGs if we've added the type they cover, so
--
2.48.1

View File

@ -60,7 +60,7 @@ Summary: The Berkeley Internet Name Domain (BIND) DNS (Domain Name System) serv
Name: bind9.16
License: MPLv2.0
Version: 9.16.23
Release: 0.22%{?dist}
Release: 0.22%{?dist}.1
Epoch: 32
Url: https://www.isc.org/downloads/bind/
#
@ -167,6 +167,9 @@ Patch211: bind-9.16-CVE-2024-1737-types.patch
Patch212: bind-9.16-CVE-2024-1737-types-test.patch
# backport issue fix
Patch213: bind-9.16-CVE-2024-1737-records-test2.patch
# https://gitlab.isc.org/isc-projects/bind9/-/commit/c6e6a7af8ac6b575dd3657b0f5cf4248d734c2b0
Patch215: bind-9.18-CVE-2024-11187-pre-test.patch
Patch216: bind-9.18-CVE-2024-11187.patch
%{?systemd_ordering}
Requires: coreutils
@ -1224,6 +1227,10 @@ fi;
%endif
%changelog
* Wed Feb 05 2025 Petr Menšík <pemensik@redhat.com> - 32:9.16.23-0.22.1
- Limit additional section records CPU processing (CVE-2024-11187)
- Fix test backport changes
* Fri Aug 09 2024 Petr Menšík <pemensik@redhat.com> - 32:9.16.23-0.22
- Minor fix of reclimit test backport (CVE-2024-1737)