From fa7b7973e36056440dd688c7f312c89600d4f8cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= 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) --- bin/tests/system/additional/tests.sh | 2 +- lib/dns/include/dns/rdataset.h | 10 +++++++++- lib/dns/rbtdb.c | 2 +- lib/dns/rdataset.c | 7 ++++++- lib/dns/resolver.c | 19 ++++++++++++------- lib/ns/query.c | 12 ++++++++---- 6 files changed, 37 insertions(+), 15 deletions(-) diff --git a/bin/tests/system/additional/tests.sh b/bin/tests/system/additional/tests.sh index 193c9f9270..e1b0cfb823 100644 --- a/bin/tests/system/additional/tests.sh +++ b/bin/tests/system/additional/tests.sh @@ -279,7 +279,7 @@ n=$((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)) diff --git a/lib/dns/include/dns/rdataset.h b/lib/dns/include/dns/rdataset.h index 41e7a72f0a..3a85226836 100644 --- a/lib/dns/include/dns/rdataset.h +++ b/lib/dns/include/dns/rdataset.h @@ -54,6 +54,8 @@ #include #include +#define DNS_RDATASET_MAXADDITIONAL 13 + ISC_LANG_BEGINDECLS typedef enum { @@ -454,7 +456,8 @@ dns_rdataset_towirepartial(dns_rdataset_t *rdataset, isc_result_t dns_rdataset_additionaldata(dns_rdataset_t *rdataset, const dns_name_t *owner_name, - dns_additionaldatafunc_t add, void *arg); + dns_additionaldatafunc_t add, void *arg, + size_t limit); /*%< * For each rdata in rdataset, call 'add' for each name and type in the * rdata which is subject to additional section processing. @@ -473,10 +476,15 @@ 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. */ diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index 3e6c031733..a341b007c7 100644 --- a/lib/dns/rbtdb.c +++ b/lib/dns/rbtdb.c @@ -10332,7 +10332,7 @@ no_glue: idx = hash_32(hash, rbtversion->glue_table_bits); (void)dns_rdataset_additionaldata(rdataset, dns_rootname, - glue_nsdname_cb, &ctx); + 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 40f04a0675..532e49ac00 100644 --- a/lib/dns/rdataset.c +++ b/lib/dns/rdataset.c @@ -577,7 +577,8 @@ dns_rdataset_towire(dns_rdataset_t *rdataset, const dns_name_t *owner_name, isc_result_t dns_rdataset_additionaldata(dns_rdataset_t *rdataset, const dns_name_t *owner_name, - dns_additionaldatafunc_t add, void *arg) { + dns_additionaldatafunc_t add, void *arg, + size_t limit) { dns_rdata_t rdata = DNS_RDATA_INIT; isc_result_t result; @@ -589,6 +590,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/dns/resolver.c b/lib/dns/resolver.c index bc2d0bd519..a0c8d42565 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -8961,7 +8961,7 @@ rctx_answer_any(respctx_t *rctx) { rdataset->trust = rctx->trust; (void)dns_rdataset_additionaldata(rdataset, rctx->aname, - check_related, rctx); + check_related, rctx, 0); } return ISC_R_SUCCESS; @@ -9009,7 +9009,7 @@ rctx_answer_match(respctx_t *rctx) { rctx->ardataset->attributes |= DNS_RDATASETATTR_CACHE; rctx->ardataset->trust = rctx->trust; (void)dns_rdataset_additionaldata(rctx->ardataset, rctx->aname, - check_related, rctx); + check_related, rctx, 0); for (sigrdataset = ISC_LIST_HEAD(rctx->aname->list); sigrdataset != NULL; @@ -9216,7 +9216,7 @@ rctx_authority_positive(respctx_t *rctx) { */ (void)dns_rdataset_additionaldata( rdataset, name, check_related, - rctx); + rctx, 0); done = true; } } @@ -9723,8 +9723,12 @@ rctx_referral(respctx_t *rctx) { */ INSIST(rctx->ns_rdataset != NULL); FCTX_ATTR_SET(fctx, FCTX_ATTR_GLUING); + + /* + * Mark the glue records in the additional section to be cached. + */ (void)dns_rdataset_additionaldata(rctx->ns_rdataset, rctx->ns_name, - check_related, rctx); + check_related, rctx, 0); #if CHECK_FOR_GLUE_IN_ANSWER /* * Look in the answer section for "glue" that is incorrectly @@ -9736,8 +9740,9 @@ rctx_referral(respctx_t *rctx) { if (rctx->glue_in_answer && (fctx->type == dns_rdatatype_aaaa || fctx->type == dns_rdatatype_a)) { - (void)dns_rdataset_additionaldata( - rctx->ns_rdataset, rctx->ns_name, check_answer, fctx); + (void)dns_rdataset_additionaldata(rctx->ns_rdataset, + rctx->ns_name, check_answer, + fctx, 0); } #endif /* if CHECK_FOR_GLUE_IN_ANSWER */ FCTX_ATTR_CLR(fctx, FCTX_ATTR_GLUING); @@ -9839,7 +9844,7 @@ again: if (CHASE(rdataset)) { rdataset->attributes &= ~DNS_RDATASETATTR_CHASE; (void)dns_rdataset_additionaldata( - rdataset, name, check_related, rctx); + rdataset, name, check_related, rctx, 0); rescan = true; } } diff --git a/lib/ns/query.c b/lib/ns/query.c index fd25dfb61e..11d2520c61 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -2098,7 +2098,8 @@ addname: if (trdataset != NULL && dns_rdatatype_followadditional(type)) { if (client->additionaldepth++ < client->view->max_restarts) { eresult = dns_rdataset_additionaldata( - trdataset, fname, query_additional_cb, qctx); + trdataset, fname, query_additional_cb, qctx, + DNS_RDATASET_MAXADDITIONAL); } client->additionaldepth--; } @@ -2198,7 +2199,7 @@ regular: * We don't care if dns_rdataset_additionaldata() fails. */ (void)dns_rdataset_additionaldata(rdataset, name, query_additional_cb, - qctx); + qctx, DNS_RDATASET_MAXADDITIONAL); CTRACE(ISC_LOG_DEBUG(3), "query_additional: done"); } @@ -2224,7 +2225,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 @@ -2279,7 +2281,9 @@ query_addrrset(query_ctx_t *qctx, dns_name_t **namep, */ query_addtoname(mname, rdataset); query_setorder(qctx, mname, rdataset); - query_additional(qctx, mname, rdataset); + if (qctx->qtype != dns_rdatatype_any) { + query_additional(qctx, mname, rdataset); + } /* * Note: we only add SIGs if we've added the type they cover, so