From 9f1ebd25f6af155073156be4a426793a6d1d4cad Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 1 Sep 2022 16:05:04 -0700 Subject: [PATCH 01/19] add an update quota limit the number of simultaneous DNS UPDATE events that can be processed by adding a quota for update and update forwarding. this quota currently, arbitrarily, defaults to 100. also add a statistics counter to record when the update quota has been exceeded. (cherry picked from commit 7c47254a140c3e9cf383cda73c7b6a55c4782826) --- bin/named/bind9.xsl | 2 +- bin/named/statschannel.c | 5 +++-- doc/arm/reference.rst | 5 +++++ lib/ns/include/ns/server.h | 1 + lib/ns/include/ns/stats.h | 4 +++- lib/ns/server.c | 2 ++ lib/ns/update.c | 38 +++++++++++++++++++++++++++++++++++++- 7 files changed, 52 insertions(+), 5 deletions(-) diff --git a/bin/named/bind9.xsl b/bin/named/bind9.xsl index 27161860d0..9dda61deea 100644 --- a/bin/named/bind9.xsl +++ b/bin/named/bind9.xsl @@ -15,7 +15,7 @@ - + diff --git a/bin/named/statschannel.c b/bin/named/statschannel.c index cd56029245..f6ce425445 100644 --- a/bin/named/statschannel.c +++ b/bin/named/statschannel.c @@ -55,11 +55,11 @@ #include "xsl_p.h" #define STATS_XML_VERSION_MAJOR "3" -#define STATS_XML_VERSION_MINOR "12" +#define STATS_XML_VERSION_MINOR "13" #define STATS_XML_VERSION STATS_XML_VERSION_MAJOR "." STATS_XML_VERSION_MINOR #define STATS_JSON_VERSION_MAJOR "1" -#define STATS_JSON_VERSION_MINOR "6" +#define STATS_JSON_VERSION_MINOR "7" #define STATS_JSON_VERSION STATS_JSON_VERSION_MAJOR "." STATS_JSON_VERSION_MINOR #define CHECK(m) \ @@ -351,6 +351,7 @@ init_desc(void) { SET_NSSTATDESC(reclimitdropped, "queries dropped due to recursive client limit", "RecLimitDropped"); + SET_NSSTATDESC(updatequota, "Update quota exceeded", "UpdateQuota"); INSIST(i == ns_statscounter_max); diff --git a/doc/arm/reference.rst b/doc/arm/reference.rst index c9c2cefb43..aec0fb6368 100644 --- a/doc/arm/reference.rst +++ b/doc/arm/reference.rst @@ -7902,6 +7902,11 @@ Name Server Statistics Counters ``UpdateBadPrereq`` This indicates the number of dynamic updates rejected due to a prerequisite failure. +``UpdateQuota`` + This indicates the number of times a dynamic update or update + forwarding request was rejected because the number of pending + requests exceeded :any:`update-quota`. + ``RateDropped`` This indicates the number of responses dropped due to rate limits. diff --git a/lib/ns/include/ns/server.h b/lib/ns/include/ns/server.h index 0f11612d1e..4393e9798c 100644 --- a/lib/ns/include/ns/server.h +++ b/lib/ns/include/ns/server.h @@ -84,6 +84,7 @@ struct ns_server { isc_quota_t recursionquota; isc_quota_t tcpquota; isc_quota_t xfroutquota; + isc_quota_t updquota; ISC_LIST(isc_quota_t) http_quotas; isc_mutex_t http_quotas_lock; diff --git a/lib/ns/include/ns/stats.h b/lib/ns/include/ns/stats.h index 2de556423b..c235384f48 100644 --- a/lib/ns/include/ns/stats.h +++ b/lib/ns/include/ns/stats.h @@ -107,7 +107,9 @@ enum { ns_statscounter_reclimitdropped = 66, - ns_statscounter_max = 67, + ns_statscounter_updatequota = 67, + + ns_statscounter_max = 68, }; void diff --git a/lib/ns/server.c b/lib/ns/server.c index 7baf2efc37..ce46aad2be 100644 --- a/lib/ns/server.c +++ b/lib/ns/server.c @@ -54,6 +54,7 @@ ns_server_create(isc_mem_t *mctx, ns_matchview_t matchingview, isc_quota_init(&sctx->xfroutquota, 10); isc_quota_init(&sctx->tcpquota, 10); isc_quota_init(&sctx->recursionquota, 100); + isc_quota_init(&sctx->updquota, 100); ISC_LIST_INIT(sctx->http_quotas); isc_mutex_init(&sctx->http_quotas_lock); @@ -136,6 +137,7 @@ ns_server_detach(ns_server_t **sctxp) { isc_mem_put(sctx->mctx, altsecret, sizeof(*altsecret)); } + isc_quota_destroy(&sctx->updquota); isc_quota_destroy(&sctx->recursionquota); isc_quota_destroy(&sctx->tcpquota); isc_quota_destroy(&sctx->xfroutquota); diff --git a/lib/ns/update.c b/lib/ns/update.c index 186eef1d1d..1f296488c7 100644 --- a/lib/ns/update.c +++ b/lib/ns/update.c @@ -1645,6 +1645,19 @@ send_update_event(ns_client_t *client, dns_zone_t *zone) { update_event_t *event = NULL; isc_task_t *zonetask = NULL; + result = isc_quota_attach(&client->manager->sctx->updquota, + &(isc_quota_t *){ NULL }); + if (result != ISC_R_SUCCESS) { + update_log(client, zone, LOGLEVEL_PROTOCOL, + "update failed: too many DNS UPDATEs queued (%s)", + isc_result_totext(result)); + ns_stats_increment(client->manager->sctx->nsstats, + ns_statscounter_updatequota); + ns_client_drop(client, result); + isc_nmhandle_detach(&client->reqhandle); + return (DNS_R_DROP); + } + event = (update_event_t *)isc_event_allocate( client->mctx, client, DNS_EVENT_UPDATE, update_action, NULL, sizeof(*event)); @@ -1783,12 +1796,19 @@ failure: dns_zone_gettype(zone) == dns_zone_mirror); inc_stats(client, zone, ns_statscounter_updaterej); } + /* * We failed without having sent an update event to the zone. * We are still in the client task context, so we can * simply give an error response without switching tasks. */ - respond(client, result); + if (result == DNS_R_DROP) { + ns_client_drop(client, result); + isc_nmhandle_detach(&client->reqhandle); + } else { + respond(client, result); + } + if (zone != NULL) { dns_zone_detach(&zone); } @@ -3669,6 +3689,7 @@ updatedone_action(isc_task_t *task, isc_event_t *event) { respond(client, uev->result); + isc_quota_detach(&(isc_quota_t *){ &client->manager->sctx->updquota }); isc_event_free(&event); isc_nmhandle_detach(&client->updatehandle); } @@ -3685,6 +3706,8 @@ forward_fail(isc_task_t *task, isc_event_t *event) { INSIST(client->nupdates > 0); client->nupdates--; respond(client, DNS_R_SERVFAIL); + + isc_quota_detach(&(isc_quota_t *){ &client->manager->sctx->updquota }); isc_event_free(&event); isc_nmhandle_detach(&client->updatehandle); } @@ -3722,6 +3745,8 @@ forward_done(isc_task_t *task, isc_event_t *event) { client->nupdates--; ns_client_sendraw(client, uev->answer); dns_message_detach(&uev->answer); + + isc_quota_detach(&(isc_quota_t *){ &client->manager->sctx->updquota }); isc_event_free(&event); isc_nmhandle_detach(&client->reqhandle); isc_nmhandle_detach(&client->updatehandle); @@ -3757,6 +3782,17 @@ send_forward_event(ns_client_t *client, dns_zone_t *zone) { update_event_t *event = NULL; isc_task_t *zonetask = NULL; + result = isc_quota_attach(&client->manager->sctx->updquota, + &(isc_quota_t *){ NULL }); + if (result != ISC_R_SUCCESS) { + update_log(client, zone, LOGLEVEL_PROTOCOL, + "update failed: too many DNS UPDATEs queued (%s)", + isc_result_totext(result)); + ns_stats_increment(client->manager->sctx->nsstats, + ns_statscounter_updatequota); + return (DNS_R_DROP); + } + event = (update_event_t *)isc_event_allocate( client->mctx, client, DNS_EVENT_UPDATE, forward_action, NULL, sizeof(*event)); From 3d2033bb89bf2ecc3168987c0cd364992bb2b0ce Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 1 Sep 2022 16:22:46 -0700 Subject: [PATCH 02/19] add a configuration option for the update quota add an "update-quota" option to configure the update quota. (cherry picked from commit f57758a7303ad0034ff2ff08eaaf2ef899630f19) --- bin/named/config.c | 1 + bin/named/server.c | 1 + bin/tests/system/checkconf/good.conf | 1 + doc/arm/reference.rst | 8 ++++++++ doc/man/named.conf.5in | 1 + doc/misc/options | 1 + lib/isccfg/namedconf.c | 1 + 7 files changed, 14 insertions(+) diff --git a/bin/named/config.c b/bin/named/config.c index 86bade216e..b2b802806b 100644 --- a/bin/named/config.c +++ b/bin/named/config.c @@ -137,6 +137,7 @@ options {\n\ trust-anchor-telemetry yes;\n\ udp-receive-buffer 0;\n\ udp-send-buffer 0;\n\ + update-quota 100;\n\ \n\ /* view */\n\ allow-new-zones no;\n\ diff --git a/bin/named/server.c b/bin/named/server.c index d7b5273d9c..3d63ee239b 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -8685,6 +8685,7 @@ load_configuration(const char *filename, named_server_t *server, configure_server_quota(maps, "tcp-clients", &server->sctx->tcpquota); configure_server_quota(maps, "recursive-clients", &server->sctx->recursionquota); + configure_server_quota(maps, "update-quota", &server->sctx->updquota); max = isc_quota_getmax(&server->sctx->recursionquota); if (max > 1000) { diff --git a/bin/tests/system/checkconf/good.conf b/bin/tests/system/checkconf/good.conf index 93939ff3c8..f8d04089f0 100644 --- a/bin/tests/system/checkconf/good.conf +++ b/bin/tests/system/checkconf/good.conf @@ -72,6 +72,7 @@ options { recursive-clients 3000; serial-query-rate 100; server-id none; + update-quota 200; check-names primary warn; check-names secondary ignore; max-cache-size 20000000000000; diff --git a/doc/arm/reference.rst b/doc/arm/reference.rst index aec0fb6368..ad05f0cef2 100644 --- a/doc/arm/reference.rst +++ b/doc/arm/reference.rst @@ -3979,6 +3979,14 @@ system. value as :any:`tcp-keepalive-timeout`. This value can be updated at runtime by using :option:`rndc tcp-timeouts`. +.. namedconf:statement:: update-quota + :tags: server + :short: Specifies the maximum number of concurrent DNS UPDATE messages that can be processed by the server. + + This is the maximum number of simultaneous DNS UPDATE messages that + the server will accept for updating local authoritiative zones or + forwarding to a primary server. The default is ``100``. + .. _intervals: Periodic Task Intervals diff --git a/doc/man/named.conf.5in b/doc/man/named.conf.5in index 97f970acdf..e04143d8ba 100644 --- a/doc/man/named.conf.5in +++ b/doc/man/named.conf.5in @@ -367,6 +367,7 @@ options { udp\-receive\-buffer ; udp\-send\-buffer ; update\-check\-ksk ; + update\-quota ; use\-alt\-transfer\-source ; // deprecated use\-v4\-udp\-ports { ; ... }; use\-v6\-udp\-ports { ; ... }; diff --git a/doc/misc/options b/doc/misc/options index a15ba67a69..7ca815617f 100644 --- a/doc/misc/options +++ b/doc/misc/options @@ -310,6 +310,7 @@ options { udp-receive-buffer ; udp-send-buffer ; update-check-ksk ; + update-quota ; use-alt-transfer-source ; // deprecated use-v4-udp-ports { ; ... }; use-v6-udp-ports { ; ... }; diff --git a/lib/isccfg/namedconf.c b/lib/isccfg/namedconf.c index 5409a4ac49..9b37671230 100644 --- a/lib/isccfg/namedconf.c +++ b/lib/isccfg/namedconf.c @@ -1342,6 +1342,7 @@ static cfg_clausedef_t options_clauses[] = { { "treat-cr-as-space", NULL, CFG_CLAUSEFLAG_ANCIENT }, { "udp-receive-buffer", &cfg_type_uint32, 0 }, { "udp-send-buffer", &cfg_type_uint32, 0 }, + { "update-quota", &cfg_type_uint32, 0 }, { "use-id-pool", NULL, CFG_CLAUSEFLAG_ANCIENT }, { "use-ixfr", NULL, CFG_CLAUSEFLAG_ANCIENT }, { "use-v4-udp-ports", &cfg_type_bracketed_portlist, 0 }, From 65d70ebd205824a4a941d5d3a0d6a6941203a33b Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 8 Nov 2022 17:32:41 -0800 Subject: [PATCH 03/19] move update ACL and update-policy checks before quota check allow-update, update-policy, and allow-update-forwarding before consuming quota slots, so that unauthorized clients can't fill the quota. (this moves the access check before the prerequisite check, which violates the precise wording of RFC 2136. however, RFC co-author Paul Vixie has stated that the RFC is mistaken on this point; it should have said that access checking must happen *no later than* the completion of prerequisite checks, not that it must happen exactly then.) (cherry picked from commit 964f559edb5036880b8e463b8f190b9007ee055d) --- lib/ns/update.c | 491 ++++++++++++++++++++++++++---------------------- 1 file changed, 264 insertions(+), 227 deletions(-) diff --git a/lib/ns/update.c b/lib/ns/update.c index 1f296488c7..46e74df8ea 100644 --- a/lib/ns/update.c +++ b/lib/ns/update.c @@ -231,6 +231,8 @@ struct update_event { dns_zone_t *zone; isc_result_t result; dns_message_t *answer; + const dns_ssurule_t **rules; + size_t ruleslen; }; /*% @@ -270,6 +272,9 @@ static void forward_done(isc_task_t *task, isc_event_t *event); static isc_result_t add_rr_prepare_action(void *data, rr_t *rr); +static isc_result_t +rr_exists(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name, + const dns_rdata_t *rdata, bool *flag); /**************************************************************************/ @@ -342,25 +347,26 @@ inc_stats(ns_client_t *client, dns_zone_t *zone, isc_statscounter_t counter) { static isc_result_t checkqueryacl(ns_client_t *client, dns_acl_t *queryacl, dns_name_t *zonename, dns_acl_t *updateacl, dns_ssutable_t *ssutable) { + isc_result_t result; char namebuf[DNS_NAME_FORMATSIZE]; char classbuf[DNS_RDATACLASS_FORMATSIZE]; - int level; - isc_result_t result; + bool update_possible = + ((updateacl != NULL && !dns_acl_isnone(updateacl)) || + ssutable != NULL); result = ns_client_checkaclsilent(client, NULL, queryacl, true); if (result != ISC_R_SUCCESS) { + int level = update_possible ? ISC_LOG_ERROR : ISC_LOG_INFO; + dns_name_format(zonename, namebuf, sizeof(namebuf)); dns_rdataclass_format(client->view->rdclass, classbuf, sizeof(classbuf)); - level = (updateacl == NULL && ssutable == NULL) ? ISC_LOG_INFO - : ISC_LOG_ERROR; - ns_client_log(client, NS_LOGCATEGORY_UPDATE_SECURITY, NS_LOGMODULE_UPDATE, level, "update '%s/%s' denied due to allow-query", namebuf, classbuf); - } else if (updateacl == NULL && ssutable == NULL) { + } else if (!update_possible) { dns_name_format(zonename, namebuf, sizeof(namebuf)); dns_rdataclass_format(client->view->rdclass, classbuf, sizeof(classbuf)); @@ -1644,6 +1650,227 @@ send_update_event(ns_client_t *client, dns_zone_t *zone) { isc_result_t result = ISC_R_SUCCESS; update_event_t *event = NULL; isc_task_t *zonetask = NULL; + dns_ssutable_t *ssutable = NULL; + dns_message_t *request = client->message; + isc_mem_t *mctx = client->manager->mctx; + dns_aclenv_t *env = client->manager->aclenv; + dns_rdataclass_t zoneclass; + dns_rdatatype_t covers; + dns_name_t *zonename = NULL; + const dns_ssurule_t **rules = NULL; + size_t rule = 0, ruleslen = 0; + dns_db_t *db = NULL; + dns_dbversion_t *ver = NULL; + + CHECK(dns_zone_getdb(zone, &db)); + zonename = dns_db_origin(db); + zoneclass = dns_db_class(db); + dns_zone_getssutable(zone, &ssutable); + dns_db_currentversion(db, &ver); + + /* + * Update message processing can leak record existence information + * so check that we are allowed to query this zone. Additionally, + * if we would refuse all updates for this zone, we bail out here. + */ + CHECK(checkqueryacl(client, dns_zone_getqueryacl(zone), + dns_zone_getorigin(zone), + dns_zone_getupdateacl(zone), ssutable)); + + /* + * Check requestor's permissions. + */ + if (ssutable == NULL) { + CHECK(checkupdateacl(client, dns_zone_getupdateacl(zone), + "update", dns_zone_getorigin(zone), false, + false)); + } else if (client->signer == NULL && !TCPCLIENT(client)) { + CHECK(checkupdateacl(client, NULL, "update", + dns_zone_getorigin(zone), false, true)); + } + + if (dns_zone_getupdatedisabled(zone)) { + FAILC(DNS_R_REFUSED, "dynamic update temporarily disabled " + "because the zone is frozen. Use " + "'rndc thaw' to re-enable updates."); + } + + /* + * Prescan the update section, checking for updates that + * are illegal or violate policy. + */ + if (ssutable != NULL) { + ruleslen = request->counts[DNS_SECTION_UPDATE]; + rules = isc_mem_get(mctx, sizeof(*rules) * ruleslen); + memset(rules, 0, sizeof(*rules) * ruleslen); + } + + for (rule = 0, + result = dns_message_firstname(request, DNS_SECTION_UPDATE); + result == ISC_R_SUCCESS; + rule++, result = dns_message_nextname(request, DNS_SECTION_UPDATE)) + { + dns_name_t *name = NULL; + dns_rdata_t rdata = DNS_RDATA_INIT; + dns_ttl_t ttl; + dns_rdataclass_t update_class; + + INSIST(ssutable == NULL || rule < ruleslen); + + get_current_rr(request, DNS_SECTION_UPDATE, zoneclass, &name, + &rdata, &covers, &ttl, &update_class); + + if (!dns_name_issubdomain(name, zonename)) { + FAILC(DNS_R_NOTZONE, "update RR is outside zone"); + } + if (update_class == zoneclass) { + /* + * Check for meta-RRs. The RFC2136 pseudocode says + * check for ANY|AXFR|MAILA|MAILB, but the text adds + * "or any other QUERY metatype" + */ + if (dns_rdatatype_ismeta(rdata.type)) { + FAILC(DNS_R_FORMERR, "meta-RR in update"); + } + result = dns_zone_checknames(zone, name, &rdata); + if (result != ISC_R_SUCCESS) { + FAIL(DNS_R_REFUSED); + } + } else if (update_class == dns_rdataclass_any) { + if (ttl != 0 || rdata.length != 0 || + (dns_rdatatype_ismeta(rdata.type) && + rdata.type != dns_rdatatype_any)) + { + FAILC(DNS_R_FORMERR, "meta-RR in update"); + } + } else if (update_class == dns_rdataclass_none) { + if (ttl != 0 || dns_rdatatype_ismeta(rdata.type)) { + FAILC(DNS_R_FORMERR, "meta-RR in update"); + } + } else { + update_log(client, zone, ISC_LOG_WARNING, + "update RR has incorrect class %d", + update_class); + FAIL(DNS_R_FORMERR); + } + + /* + * draft-ietf-dnsind-simple-secure-update-01 says + * "Unlike traditional dynamic update, the client + * is forbidden from updating NSEC records." + */ + if (rdata.type == dns_rdatatype_nsec3) { + FAILC(DNS_R_REFUSED, "explicit NSEC3 updates are not " + "allowed " + "in secure zones"); + } else if (rdata.type == dns_rdatatype_nsec) { + FAILC(DNS_R_REFUSED, "explicit NSEC updates are not " + "allowed " + "in secure zones"); + } else if (rdata.type == dns_rdatatype_rrsig && + !dns_name_equal(name, zonename)) + { + FAILC(DNS_R_REFUSED, "explicit RRSIG updates are " + "currently " + "not supported in secure zones " + "except " + "at the apex"); + } + + if (ssutable != NULL) { + isc_netaddr_t netaddr; + dns_name_t *target = NULL; + dst_key_t *tsigkey = NULL; + dns_rdata_ptr_t ptr; + dns_rdata_in_srv_t srv; + + isc_netaddr_fromsockaddr(&netaddr, &client->peeraddr); + + if (client->message->tsigkey != NULL) { + tsigkey = client->message->tsigkey->key; + } + + if ((update_class == dns_rdataclass_in || + update_class == dns_rdataclass_none) && + rdata.type == dns_rdatatype_ptr) + { + result = dns_rdata_tostruct(&rdata, &ptr, NULL); + RUNTIME_CHECK(result == ISC_R_SUCCESS); + target = &ptr.ptr; + } + + if ((update_class == dns_rdataclass_in || + update_class == dns_rdataclass_none) && + rdata.type == dns_rdatatype_srv) + { + result = dns_rdata_tostruct(&rdata, &srv, NULL); + RUNTIME_CHECK(result == ISC_R_SUCCESS); + target = &srv.target; + } + + if (update_class == dns_rdataclass_any && + zoneclass == dns_rdataclass_in && + (rdata.type == dns_rdatatype_ptr || + rdata.type == dns_rdatatype_srv)) + { + ssu_check_t ssuinfo; + + ssuinfo.name = name; + ssuinfo.table = ssutable; + ssuinfo.signer = client->signer; + ssuinfo.addr = &netaddr; + ssuinfo.aclenv = env; + ssuinfo.tcp = TCPCLIENT(client); + ssuinfo.key = tsigkey; + + result = foreach_rr(db, ver, name, rdata.type, + dns_rdatatype_none, + ssu_checkrr, &ssuinfo); + if (result != ISC_R_SUCCESS) { + FAILC(DNS_R_REFUSED, + "rejected by secure update"); + } + } else if (target != NULL && + update_class == dns_rdataclass_none) + { + bool flag; + CHECK(rr_exists(db, ver, name, &rdata, &flag)); + if (flag && + !dns_ssutable_checkrules( + ssutable, client->signer, name, + &netaddr, TCPCLIENT(client), env, + rdata.type, target, tsigkey, + &rules[rule])) + { + FAILC(DNS_R_REFUSED, + "rejected by secure update"); + } + } else if (rdata.type != dns_rdatatype_any) { + if (!dns_ssutable_checkrules( + ssutable, client->signer, name, + &netaddr, TCPCLIENT(client), env, + rdata.type, target, tsigkey, + &rules[rule])) + { + FAILC(DNS_R_REFUSED, "rejected by " + "secure update"); + } + } else { + if (!ssu_checkall(db, ver, name, ssutable, + client->signer, &netaddr, env, + TCPCLIENT(client), tsigkey)) + { + FAILC(DNS_R_REFUSED, "rejected by " + "secure update"); + } + } + } + } + if (result != ISC_R_NOMORE) { + FAIL(result); + } + + update_log(client, zone, LOGLEVEL_DEBUG, "update section prescan OK"); result = isc_quota_attach(&client->manager->sctx->updquota, &(isc_quota_t *){ NULL }); @@ -1653,9 +1880,7 @@ send_update_event(ns_client_t *client, dns_zone_t *zone) { isc_result_totext(result)); ns_stats_increment(client->manager->sctx->nsstats, ns_statscounter_updatequota); - ns_client_drop(client, result); - isc_nmhandle_detach(&client->reqhandle); - return (DNS_R_DROP); + CHECK(DNS_R_DROP); } event = (update_event_t *)isc_event_allocate( @@ -1663,6 +1888,9 @@ send_update_event(ns_client_t *client, dns_zone_t *zone) { sizeof(*event)); event->zone = zone; event->result = ISC_R_SUCCESS; + event->rules = rules; + event->ruleslen = ruleslen; + rules = NULL; INSIST(client->nupdates == 0); client->nupdates++; @@ -1672,6 +1900,20 @@ send_update_event(ns_client_t *client, dns_zone_t *zone) { dns_zone_gettask(zone, &zonetask); isc_task_send(zonetask, ISC_EVENT_PTR(&event)); +failure: + if (db != NULL) { + dns_db_closeversion(db, &ver, false); + dns_db_detach(&db); + } + + if (rules != NULL) { + isc_mem_put(mctx, rules, sizeof(*rules) * ruleslen); + } + + if (ssutable != NULL) { + dns_ssutable_detach(&ssutable); + } + return (result); } @@ -1779,9 +2021,6 @@ ns_update_start(ns_client_t *client, isc_nmhandle_t *handle, break; case dns_zone_secondary: case dns_zone_mirror: - CHECK(checkupdateacl(client, dns_zone_getforwardacl(zone), - "update forwarding", zonename, true, - false)); dns_message_clonebuffer(client->message); CHECK(send_forward_event(client, zone)); break; @@ -1792,8 +2031,6 @@ ns_update_start(ns_client_t *client, isc_nmhandle_t *handle, failure: if (result == DNS_R_REFUSED) { - INSIST(dns_zone_gettype(zone) == dns_zone_secondary || - dns_zone_gettype(zone) == dns_zone_mirror); inc_stats(client, zone, ns_statscounter_updaterej); } @@ -2642,6 +2879,8 @@ update_action(isc_task_t *task, isc_event_t *event) { update_event_t *uev = (update_event_t *)event; dns_zone_t *zone = uev->zone; ns_client_t *client = (ns_client_t *)event->ev_arg; + const dns_ssurule_t **rules = uev->rules; + size_t rule = 0, ruleslen = uev->ruleslen; isc_result_t result; dns_db_t *db = NULL; dns_dbversion_t *oldver = NULL; @@ -2653,7 +2892,7 @@ update_action(isc_task_t *task, isc_event_t *event) { dns_rdatatype_t covers; dns_message_t *request = client->message; dns_rdataclass_t zoneclass; - dns_name_t *zonename; + dns_name_t *zonename = NULL; dns_ssutable_t *ssutable = NULL; dns_fixedname_t tmpnamefixed; dns_name_t *tmpname = NULL; @@ -2665,10 +2904,6 @@ update_action(isc_task_t *task, isc_event_t *event) { dns_ttl_t maxttl = 0; uint32_t maxrecords; uint64_t records; - dns_aclenv_t *env = client->manager->aclenv; - size_t ruleslen = 0; - size_t rule; - const dns_ssurule_t **rules = NULL; INSIST(event->ev_type == DNS_EVENT_UPDATE); @@ -2679,14 +2914,7 @@ update_action(isc_task_t *task, isc_event_t *event) { zonename = dns_db_origin(db); zoneclass = dns_db_class(db); dns_zone_getssutable(zone, &ssutable); - - /* - * Update message processing can leak record existence information - * so check that we are allowed to query this zone. Additionally - * if we would refuse all updates for this zone we bail out here. - */ - CHECK(checkqueryacl(client, dns_zone_getqueryacl(zone), zonename, - dns_zone_getupdateacl(zone), ssutable)); + options = dns_zone_getoptions(zone); /* * Get old and new versions now that queryacl has been checked. @@ -2821,205 +3049,10 @@ update_action(isc_task_t *task, isc_event_t *event) { update_log(client, zone, LOGLEVEL_DEBUG, "prerequisites are OK"); - /* - * Check Requestor's Permissions. It seems a bit silly to do this - * only after prerequisite testing, but that is what RFC2136 says. - */ - if (ssutable == NULL) { - CHECK(checkupdateacl(client, dns_zone_getupdateacl(zone), - "update", zonename, false, false)); - } else if (client->signer == NULL && !TCPCLIENT(client)) { - CHECK(checkupdateacl(client, NULL, "update", zonename, false, - true)); - } - - if (dns_zone_getupdatedisabled(zone)) { - FAILC(DNS_R_REFUSED, "dynamic update temporarily disabled " - "because the zone is frozen. Use " - "'rndc thaw' to re-enable updates."); - } - - /* - * Perform the Update Section Prescan. - */ - if (ssutable != NULL) { - ruleslen = request->counts[DNS_SECTION_UPDATE]; - rules = isc_mem_get(mctx, sizeof(*rules) * ruleslen); - memset(rules, 0, sizeof(*rules) * ruleslen); - } - - for (rule = 0, - result = dns_message_firstname(request, DNS_SECTION_UPDATE); - result == ISC_R_SUCCESS; - rule++, result = dns_message_nextname(request, DNS_SECTION_UPDATE)) - { - dns_name_t *name = NULL; - dns_rdata_t rdata = DNS_RDATA_INIT; - dns_ttl_t ttl; - dns_rdataclass_t update_class; - - INSIST(ssutable == NULL || rule < ruleslen); - - get_current_rr(request, DNS_SECTION_UPDATE, zoneclass, &name, - &rdata, &covers, &ttl, &update_class); - - if (!dns_name_issubdomain(name, zonename)) { - FAILC(DNS_R_NOTZONE, "update RR is outside zone"); - } - if (update_class == zoneclass) { - /* - * Check for meta-RRs. The RFC2136 pseudocode says - * check for ANY|AXFR|MAILA|MAILB, but the text adds - * "or any other QUERY metatype" - */ - if (dns_rdatatype_ismeta(rdata.type)) { - FAILC(DNS_R_FORMERR, "meta-RR in update"); - } - result = dns_zone_checknames(zone, name, &rdata); - if (result != ISC_R_SUCCESS) { - FAIL(DNS_R_REFUSED); - } - } else if (update_class == dns_rdataclass_any) { - if (ttl != 0 || rdata.length != 0 || - (dns_rdatatype_ismeta(rdata.type) && - rdata.type != dns_rdatatype_any)) - { - FAILC(DNS_R_FORMERR, "meta-RR in update"); - } - } else if (update_class == dns_rdataclass_none) { - if (ttl != 0 || dns_rdatatype_ismeta(rdata.type)) { - FAILC(DNS_R_FORMERR, "meta-RR in update"); - } - } else { - update_log(client, zone, ISC_LOG_WARNING, - "update RR has incorrect class %d", - update_class); - FAIL(DNS_R_FORMERR); - } - - /* - * draft-ietf-dnsind-simple-secure-update-01 says - * "Unlike traditional dynamic update, the client - * is forbidden from updating NSEC records." - */ - if (rdata.type == dns_rdatatype_nsec3) { - FAILC(DNS_R_REFUSED, "explicit NSEC3 updates are not " - "allowed " - "in secure zones"); - } else if (rdata.type == dns_rdatatype_nsec) { - FAILC(DNS_R_REFUSED, "explicit NSEC updates are not " - "allowed " - "in secure zones"); - } else if (rdata.type == dns_rdatatype_rrsig && - !dns_name_equal(name, zonename)) - { - FAILC(DNS_R_REFUSED, "explicit RRSIG updates are " - "currently " - "not supported in secure zones " - "except " - "at the apex"); - } - - if (ssutable != NULL) { - isc_netaddr_t netaddr; - dns_name_t *target = NULL; - dst_key_t *tsigkey = NULL; - dns_rdata_ptr_t ptr; - dns_rdata_in_srv_t srv; - - isc_netaddr_fromsockaddr(&netaddr, &client->peeraddr); - - if (client->message->tsigkey != NULL) { - tsigkey = client->message->tsigkey->key; - } - - if ((update_class == dns_rdataclass_in || - update_class == dns_rdataclass_none) && - rdata.type == dns_rdatatype_ptr) - { - result = dns_rdata_tostruct(&rdata, &ptr, NULL); - RUNTIME_CHECK(result == ISC_R_SUCCESS); - target = &ptr.ptr; - } - - if ((update_class == dns_rdataclass_in || - update_class == dns_rdataclass_none) && - rdata.type == dns_rdatatype_srv) - { - result = dns_rdata_tostruct(&rdata, &srv, NULL); - RUNTIME_CHECK(result == ISC_R_SUCCESS); - target = &srv.target; - } - - if (update_class == dns_rdataclass_any && - zoneclass == dns_rdataclass_in && - (rdata.type == dns_rdatatype_ptr || - rdata.type == dns_rdatatype_srv)) - { - ssu_check_t ssuinfo; - - ssuinfo.name = name; - ssuinfo.table = ssutable; - ssuinfo.signer = client->signer; - ssuinfo.addr = &netaddr; - ssuinfo.aclenv = env; - ssuinfo.tcp = TCPCLIENT(client); - ssuinfo.key = tsigkey; - - result = foreach_rr(db, ver, name, rdata.type, - dns_rdatatype_none, - ssu_checkrr, &ssuinfo); - if (result != ISC_R_SUCCESS) { - FAILC(DNS_R_REFUSED, - "rejected by secure update"); - } - } else if (target != NULL && - update_class == dns_rdataclass_none) - { - bool flag; - CHECK(rr_exists(db, ver, name, &rdata, &flag)); - if (flag && - !dns_ssutable_checkrules( - ssutable, client->signer, name, - &netaddr, TCPCLIENT(client), env, - rdata.type, target, tsigkey, - &rules[rule])) - { - FAILC(DNS_R_REFUSED, - "rejected by secure update"); - } - } else if (rdata.type != dns_rdatatype_any) { - if (!dns_ssutable_checkrules( - ssutable, client->signer, name, - &netaddr, TCPCLIENT(client), env, - rdata.type, target, tsigkey, - &rules[rule])) - { - FAILC(DNS_R_REFUSED, "rejected by " - "secure update"); - } - } else { - if (!ssu_checkall(db, ver, name, ssutable, - client->signer, &netaddr, env, - TCPCLIENT(client), tsigkey)) - { - FAILC(DNS_R_REFUSED, "rejected by " - "secure update"); - } - } - } - } - if (result != ISC_R_NOMORE) { - FAIL(result); - } - - update_log(client, zone, LOGLEVEL_DEBUG, "update section prescan OK"); - /* * Process the Update Section. */ - - options = dns_zone_getoptions(zone); + INSIST(ssutable == NULL || rules != NULL); for (rule = 0, result = dns_message_firstname(request, DNS_SECTION_UPDATE); result == ISC_R_SUCCESS; @@ -3481,10 +3514,7 @@ update_action(isc_task_t *task, isc_event_t *event) { if (result == ISC_R_SUCCESS && records > maxrecords) { update_log(client, zone, ISC_LOG_ERROR, "records in zone (%" PRIu64 ") " - "exceeds" - " max-" - "records" - " (%u)", + "exceeds max-records (%u)", records, maxrecords); result = DNS_R_TOOMANYRECORDS; goto failure; @@ -3782,6 +3812,13 @@ send_forward_event(ns_client_t *client, dns_zone_t *zone) { update_event_t *event = NULL; isc_task_t *zonetask = NULL; + result = checkupdateacl(client, dns_zone_getforwardacl(zone), + "update forwarding", dns_zone_getorigin(zone), + true, false); + if (result != ISC_R_SUCCESS) { + return (result); + } + result = isc_quota_attach(&client->manager->sctx->updquota, &(isc_quota_t *){ NULL }); if (result != ISC_R_SUCCESS) { From 24a684db945cec2bd9acff891c7ddfb1bcd4e5c5 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 9 Nov 2022 21:56:16 -0800 Subject: [PATCH 04/19] test failure conditions verify that updates are refused when the client is disallowed by allow-query, and update forwarding is refused when the client is is disallowed by update-forwarding. verify that "too many DNS UPDATEs" appears in the log file when too many simultaneous updates are processing. (cherry picked from commit b91339b80e5b82a56622c93cc1e3cca2d0c11bc0) --- bin/tests/system/nsupdate/ns1/named.conf.in | 2 + bin/tests/system/nsupdate/tests.sh | 28 ++++++++++++ bin/tests/system/upforwd/clean.sh | 2 + .../ns3/{named.conf.in => named1.conf.in} | 17 +++++--- bin/tests/system/upforwd/ns3/named2.conf.in | 43 +++++++++++++++++++ bin/tests/system/upforwd/setup.sh | 2 +- bin/tests/system/upforwd/tests.sh | 39 +++++++++++++++++ 7 files changed, 127 insertions(+), 6 deletions(-) rename bin/tests/system/upforwd/ns3/{named.conf.in => named1.conf.in} (75%) create mode 100644 bin/tests/system/upforwd/ns3/named2.conf.in diff --git a/bin/tests/system/nsupdate/ns1/named.conf.in b/bin/tests/system/nsupdate/ns1/named.conf.in index fef5beb1f0..b502ea7a1e 100644 --- a/bin/tests/system/nsupdate/ns1/named.conf.in +++ b/bin/tests/system/nsupdate/ns1/named.conf.in @@ -23,6 +23,7 @@ options { recursion no; notify yes; minimal-responses no; + update-quota 1; }; acl named-acl { @@ -83,6 +84,7 @@ zone "other.nil" { check-integrity no; check-mx warn; update-policy local; + allow-query { !10.53.0.2; any; }; allow-query-on { 10.53.0.1; 127.0.0.1; }; allow-transfer { any; }; }; diff --git a/bin/tests/system/nsupdate/tests.sh b/bin/tests/system/nsupdate/tests.sh index d612a226d0..9b80dd464c 100755 --- a/bin/tests/system/nsupdate/tests.sh +++ b/bin/tests/system/nsupdate/tests.sh @@ -1353,6 +1353,34 @@ grep '10.53.0.1.*REFUSED' nsupdate.out-$n > /dev/null || ret=1 grep 'Reply from SOA query' nsupdate.out-$n > /dev/null || ret=1 [ $ret = 0 ] || { echo_i "failed"; status=1; } +n=$((n + 1)) +ret=0 +echo_i "check that update is rejected if query is not allowed ($n)" +{ + $NSUPDATE -d < nsupdate.out.test$n 2>&1 +grep 'failed: REFUSED' nsupdate.out.test$n > /dev/null || ret=1 +[ $ret = 0 ] || { echo_i "failed"; status=1; } + +n=$((n + 1)) +ret=0 +echo_i "check that update is rejected if quota is exceeded ($n)" +for loop in 1 2 3 4 5 6 7 8 9 10; do +{ + $NSUPDATE -4 -l -p ${PORT} -k ns1/session.key > /dev/null 2>&1 < nsupdate.out.$n 2>&1 +grep REFUSED nsupdate.out.$n > /dev/null || ret=1 +if [ $ret != 0 ] ; then echo_i "failed"; status=`expr $status + $ret`; fi +n=`expr $n + 1` + +n=$((n + 1)) +ret=0 +echo_i "attempting updates that should exceed quota ($n)" +# lower the update quota to 1. +copy_setports ns3/named2.conf.in ns3/named.conf +rndc_reconfig ns3 10.53.0.3 +nextpart ns3/named.run > /dev/null +for loop in 1 2 3 4 5 6 7 8 9 10; do +{ + $NSUPDATE -- - > /dev/null 2>&1 < Date: Thu, 1 Sep 2022 16:34:21 -0700 Subject: [PATCH 05/19] CHANGES and release notes for [GL #3523] (cherry picked from commit 991de0aa7612cca50eae26b92b764cd5e37a3179) --- CHANGES | 9 +++++++++ doc/notes/notes-current.rst | 17 +++++++++++++++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/CHANGES b/CHANGES index c6227baf10..95a7fc82bd 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,12 @@ +6064. [security] An UPDATE message flood could cause named to exhaust all + available memory. This flaw was addressed by adding a + new "update-quota" statement that controls the number of + simultaneous UPDATE messages that can be processed or + forwarded. The default is 100. A stats counter has been + added to record events when the update quota is + exceeded, and the XML and JSON statistics version + numbers have been updated. (CVE-2022-3094) [GL #3523] + 6063. [bug] Revert a change that limited to honour single read for TLSDNS as it broke XoT. [GL #3772] diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index ef72732ede..305cea5933 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -15,12 +15,25 @@ Notes for BIND 9.18.11 Security Fixes ~~~~~~~~~~~~~~ -- None. +- An UPDATE message flood could cause :iscman:`named` to exhaust all + available memory. This flaw was addressed by adding a new + :any:`update-quota` option that controls the maximum number of + outstanding DNS UPDATE messages that :iscman:`named` can hold in a + queue at any given time (default: 100). (CVE-2022-3094) + + ISC would like to thank Rob Schulhof from Infoblox for bringing this + vulnerability to our attention. :gl:`#3523` New Features ~~~~~~~~~~~~ -- None. +- The new :any:`update-quota` option can be used to control the number + of simultaneous DNS UPDATE messages that can be processed to update an + authoritative zone on a primary server, or forwarded to the primary + server by a secondary server. The default is 100. A new statistics + counter has also been added to record events when this quota is + exceeded, and the version numbers for the XML and JSON statistics + schemas have been updated. :gl:`#3523` Removed Features ~~~~~~~~~~~~~~~~ From 38323f3b9f6e9a41a78f92d21905548c64bba3a5 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 27 Oct 2022 13:22:11 +1100 Subject: [PATCH 06/19] Move the mapping of SIG and RRSIG to ANY dns_db_findext() asserts if RRSIG is passed to it and query_lookup_stale() failed to map RRSIG to ANY to prevent this. To avoid cases like this in the future, move the mapping of SIG and RRSIG to ANY for qctx->type to qctx_init(). (cherry picked from commit 56eae064183488bcf7ff08c3edf59f2e1742c1b6) --- lib/ns/query.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/ns/query.c b/lib/ns/query.c index 36f3c40dc1..0ad4f0f13e 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -5237,6 +5237,15 @@ qctx_init(ns_client_t *client, dns_fetchevent_t **eventp, dns_rdatatype_t qtype, qctx->result = ISC_R_SUCCESS; qctx->findcoveringnsec = qctx->view->synthfromdnssec; + /* + * If it's an RRSIG or SIG query, we'll iterate the node. + */ + if (qctx->qtype == dns_rdatatype_rrsig || + qctx->qtype == dns_rdatatype_sig) + { + qctx->type = dns_rdatatype_any; + } + CALL_HOOK_NORETURN(NS_QUERY_QCTX_INITIALIZED, qctx); } @@ -5424,15 +5433,6 @@ query_setup(ns_client_t *client, dns_rdatatype_t qtype) { CALL_HOOK(NS_QUERY_SETUP, &qctx); - /* - * If it's a SIG query, we'll iterate the node. - */ - if (qctx.qtype == dns_rdatatype_rrsig || - qctx.qtype == dns_rdatatype_sig) - { - qctx.type = dns_rdatatype_any; - } - /* * Check SERVFAIL cache */ From a4b760d8f60592e9f6ebf76e912b0dc80290b3f7 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Fri, 28 Oct 2022 11:26:59 +1100 Subject: [PATCH 07/19] Add CHANGES note for [GL #3622] (cherry picked from commit 8ca018b5ec2c25bbfc4b951aa9826a46d01fba51) --- CHANGES | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES b/CHANGES index 95a7fc82bd..bdfbf45a2d 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +6066. [security] Handle RRSIG lookups when serve-stale is active. + (CVE-2022-3736) [GL #3622] + 6064. [security] An UPDATE message flood could cause named to exhaust all available memory. This flaw was addressed by adding a new "update-quota" statement that controls the number of From 645dd3fdf1522d66fabe403e0ca6ec2ebbea2e96 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Fri, 28 Oct 2022 11:31:19 +1100 Subject: [PATCH 08/19] Add release note for [GL #3622] (cherry picked from commit 42c42be9a997a30dcf83c8a77a2f57811757a72d) --- doc/notes/notes-current.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 305cea5933..a5b1df2982 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -24,6 +24,14 @@ Security Fixes ISC would like to thank Rob Schulhof from Infoblox for bringing this vulnerability to our attention. :gl:`#3523` +- :iscman:`named` could crash with an assertion failure when an RRSIG + query was received and :any:`stale-answer-client-timeout` was set to a + non-zero value. This has been fixed. (CVE-2022-3736) + + ISC would like to thank Borja Marcos from Sarenet (with assistance by + Iratxe Niño from Fundación Sarenet) for bringing this vulnerability to + our attention. :gl:`#3622` + New Features ~~~~~~~~~~~~ From a4fc5e51584c311de2c848fa08d22aad7378ebe2 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Mon, 14 Nov 2022 12:18:06 +0000 Subject: [PATCH 09/19] Cancel all fetch events in dns_resolver_cancelfetch() Although 'dns_fetch_t' fetch can have two associated events, one for each of 'DNS_EVENT_FETCHDONE' and 'DNS_EVENT_TRYSTALE' types, the dns_resolver_cancelfetch() function is designed in a way that it expects only one existing event, which it must cancel, and when it happens so that 'stale-answer-client-timeout' is enabled and there are two events, only one of them is canceled, and it results in an assertion in dns_resolver_destroyfetch(), when it finds a dangling event. Change the logic of dns_resolver_cancelfetch() function so that it cancels both the events (if they exist), and in the right order. (cherry picked from commit ec2098ca35039e4f81fd0aa7c525eb960b8f47bf) --- lib/dns/resolver.c | 45 +++++++++++++++++++++++++++++++++++++-------- lib/ns/query.c | 4 +++- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 880d33b4d3..ca9d7449a7 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -10976,6 +10976,8 @@ dns_resolver_cancelfetch(dns_fetch_t *fetch) { fetchctx_t *fctx = NULL; dns_resolver_t *res = NULL; dns_fetchevent_t *event = NULL; + dns_fetchevent_t *event_trystale = NULL; + dns_fetchevent_t *event_fetchdone = NULL; REQUIRE(DNS_FETCH_VALID(fetch)); fctx = fetch->private; @@ -10987,9 +10989,9 @@ dns_resolver_cancelfetch(dns_fetch_t *fetch) { LOCK(&res->buckets[fctx->bucketnum].lock); /* - * Find the completion event for this fetch (as opposed + * Find the events for this fetch (as opposed * to those for other fetches that have joined the same - * fctx) and send it with result = ISC_R_CANCELED. + * fctx) and send them with result = ISC_R_CANCELED. */ if (fctx->state != fetchstate_done) { dns_fetchevent_t *next_event = NULL; @@ -10999,15 +11001,42 @@ dns_resolver_cancelfetch(dns_fetch_t *fetch) { next_event = ISC_LIST_NEXT(event, ev_link); if (event->fetch == fetch) { ISC_LIST_UNLINK(fctx->events, event, ev_link); - break; + switch (event->ev_type) { + case DNS_EVENT_TRYSTALE: + INSIST(event_trystale == NULL); + event_trystale = event; + break; + case DNS_EVENT_FETCHDONE: + INSIST(event_fetchdone == NULL); + event_fetchdone = event; + break; + default: + UNREACHABLE(); + } + if (event_trystale != NULL && + event_fetchdone != NULL) + { + break; + } } } } - if (event != NULL) { - isc_task_t *etask = event->ev_sender; - event->ev_sender = fctx; - event->result = ISC_R_CANCELED; - isc_task_sendanddetach(&etask, ISC_EVENT_PTR(&event)); + /* + * The "trystale" event must be sent before the "fetchdone" event, + * because the latter clears the "recursing" query attribute, which is + * required by both events (handled by the same callback function). + */ + if (event_trystale != NULL) { + isc_task_t *etask = event_trystale->ev_sender; + event_trystale->ev_sender = fctx; + event_trystale->result = ISC_R_CANCELED; + isc_task_sendanddetach(&etask, ISC_EVENT_PTR(&event_trystale)); + } + if (event_fetchdone != NULL) { + isc_task_t *etask = event_fetchdone->ev_sender; + event_fetchdone->ev_sender = fctx; + event_fetchdone->result = ISC_R_CANCELED; + isc_task_sendanddetach(&etask, ISC_EVENT_PTR(&event_fetchdone)); } /* diff --git a/lib/ns/query.c b/lib/ns/query.c index 0ad4f0f13e..0d2ba6bc0b 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -6236,7 +6236,9 @@ fetch_callback(isc_task_t *task, isc_event_t *event) { CTRACE(ISC_LOG_DEBUG(3), "fetch_callback"); if (event->ev_type == DNS_EVENT_TRYSTALE) { - query_lookup_stale(client); + if (devent->result != ISC_R_CANCELED) { + query_lookup_stale(client); + } isc_event_free(ISC_EVENT_PTR(&event)); return; } From 601066e854aba7dc0ad5a614ca4226ab503f032a Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Mon, 14 Nov 2022 12:30:49 +0000 Subject: [PATCH 10/19] Add CHANGES and release notes for [GL #3619] (cherry picked from commit d08a478b4219163bcba3f31641f8f1d4e77681ff) --- CHANGES | 3 +++ doc/notes/notes-current.rst | 9 +++++++++ 2 files changed, 12 insertions(+) diff --git a/CHANGES b/CHANGES index bdfbf45a2d..420cd05e7f 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +6067. [security] Fix serve-stale crash when recursive clients soft quota + is reached. (CVE-2022-3924) [GL #3619] + 6066. [security] Handle RRSIG lookups when serve-stale is active. (CVE-2022-3736) [GL #3622] diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index a5b1df2982..e58b5ec8d8 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -32,6 +32,15 @@ Security Fixes Iratxe Niño from Fundación Sarenet) for bringing this vulnerability to our attention. :gl:`#3622` +- :iscman:`named` running as a resolver with the + :any:`stale-answer-client-timeout` option set to any value greater + than ``0`` could crash with an assertion failure, when the + :any:`recursive-clients` soft quota was reached. This has been fixed. + (CVE-2022-3924) + + ISC would like to thank Maksym Odinintsev from AWS for bringing this + vulnerability to our attention. :gl:`#3619` + New Features ~~~~~~~~~~~~ From 04b41cd54e0e8865529f8dfc84ac1610633493d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Thu, 12 Jan 2023 22:11:14 +0100 Subject: [PATCH 11/19] Fix a typo in the DNSSEC Guide --- doc/dnssec-guide/introduction.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/dnssec-guide/introduction.rst b/doc/dnssec-guide/introduction.rst index ad0a04f8aa..87fb45b876 100644 --- a/doc/dnssec-guide/introduction.rst +++ b/doc/dnssec-guide/introduction.rst @@ -250,7 +250,7 @@ at a very high level, looking up the name ``www.isc.org`` : Let's take a quick break here and look at what we've got so far... how can our server trust this answer? If a clever attacker had taken over - the ``isc.org`` name server(s), or course she would send matching + the ``isc.org`` name server(s), of course she would send matching keys and signatures. We need to ask someone else to have confidence that we are really talking to the real ``isc.org`` name server. This is a critical part of DNSSEC: at some point, the DNS administrators From 1bec7e09a3134f5bbcdbeefd1be5e2d2d1294410 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Thu, 12 Jan 2023 22:11:14 +0100 Subject: [PATCH 12/19] Update documentation for GL #3212 --- CHANGES | 4 +++- doc/notes/notes-current.rst | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGES b/CHANGES index 420cd05e7f..995638ee04 100644 --- a/CHANGES +++ b/CHANGES @@ -74,7 +74,9 @@ [GL !7206] 5830. [func] Implement incremental resizing of isc_ht hash tables to - perform the rehashing gradually. [GL #3212] + perform the rehashing gradually. The catalog zone + implementation has been optimized to work with hundreds + of thousands of member zones. [GL #3212] [GL #3744] --- 9.18.10 released --- diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index e58b5ec8d8..c5892c0d93 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -64,7 +64,8 @@ Removed Features Feature Changes ~~~~~~~~~~~~~~~ -- None. +- The catalog zone implementation has been optimized to work with + hundreds of thousands of member zones. :gl:`#3212` :gl:`#3744` Bug Fixes ~~~~~~~~~ From 166523fd619487b16f143621063d5d591e7dcf34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Thu, 12 Jan 2023 22:11:14 +0100 Subject: [PATCH 13/19] Prepare release notes for BIND 9.18.11 --- doc/arm/notes.rst | 2 +- doc/notes/{notes-current.rst => notes-9.18.11.rst} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename doc/notes/{notes-current.rst => notes-9.18.11.rst} (100%) diff --git a/doc/arm/notes.rst b/doc/arm/notes.rst index fe53af064a..3f578888b7 100644 --- a/doc/arm/notes.rst +++ b/doc/arm/notes.rst @@ -35,7 +35,7 @@ information about each release, and source code. .. include:: ../notes/notes-known-issues.rst -.. include:: ../notes/notes-current.rst +.. include:: ../notes/notes-9.18.11.rst .. include:: ../notes/notes-9.18.10.rst .. include:: ../notes/notes-9.18.9.rst .. include:: ../notes/notes-9.18.8.rst diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-9.18.11.rst similarity index 100% rename from doc/notes/notes-current.rst rename to doc/notes/notes-9.18.11.rst From 828d5d51d0296686849136c4aa721565b9b3184a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Thu, 12 Jan 2023 22:11:14 +0100 Subject: [PATCH 14/19] Tweak and reword release notes --- doc/notes/notes-9.18.11.rst | 52 +++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/doc/notes/notes-9.18.11.rst b/doc/notes/notes-9.18.11.rst index c5892c0d93..16a0ffd994 100644 --- a/doc/notes/notes-9.18.11.rst +++ b/doc/notes/notes-9.18.11.rst @@ -55,11 +55,11 @@ New Features Removed Features ~~~~~~~~~~~~~~~~ -- The Differentiated Services Code Point (DSCP) feature in BIND - has been non-operational since the new Network Manager was introduced - in BIND 9.16. It is now marked as obsolete, and vestigial code - implementing it has been removed. Configuring DSCP values in - ``named.conf`` will cause a warning to be logged. :gl:`#3773` +- The Differentiated Services Code Point (DSCP) feature in BIND has been + non-operational since the new Network Manager was introduced in BIND + 9.16. It is now marked as obsolete, and vestigial code implementing it + has been removed. Configuring DSCP values in ``named.conf`` now causes + a warning to be logged. :gl:`#3773` Feature Changes ~~~~~~~~~~~~~~~ @@ -70,32 +70,34 @@ Feature Changes Bug Fixes ~~~~~~~~~ -- TLS session resumption might lead to handshake failures when client - certificates are used for authentication (Mutual TLS). This has - been fixed. :gl:`#3725` +- Previously, TLS session resumption could have led to handshake + failures when client certificates were used for authentication (Mutual + TLS). This has been fixed. :gl:`#3725` -- When an outgoing request timed out, the ``named`` would retry up to three - times with the same server instead of trying a next available name server. - This has been fixed. :gl:`#3637` +- When an outgoing request timed out, :iscman:`named` would retry up to + three times with the same server instead of trying the next available + name server. This has been fixed. :gl:`#3637` -- Recently used ADB names and ADB entries (IP addresses) could get cleaned when - ADB would be under memory pressure. To mitigate this, count only actual ADB - names and ADB entries into the overmem memory limit (exclude internal memory - structures used for "housekeeping") and exclude recently used (<= 10 seconds) - ADB names and entries from the overmem memory cleaner. :gl:`#3739` +- Recently used ADB names and ADB entries (IP addresses) could get + cleaned when ADB was under memory pressure. To mitigate this, only + actual ADB names and ADB entries are now counted (excluding internal + memory structures used for "housekeeping") and recently used (<= 10 + seconds) ADB names and entries are excluded from the overmem memory + cleaner. :gl:`#3739` -- Fix a rare assertion failure in the outgoing TCP DNS connection handling. - :gl:`#3178` :gl:`#3636` +- A rare assertion failure was fixed in outgoing TCP DNS connection + handling. :gl:`#3178` :gl:`#3636` -- In addition to a previously fixed bug, another similar issue was discovered - where quotas could be erroneously reached for servers, including any - configured forwarders, resulting in SERVFAIL answers being sent to clients. - This has been fixed. :gl:`#3752` +- In addition to a previously fixed bug, another similar issue was + discovered where quotas could be erroneously reached for servers, + including any configured forwarders, resulting in SERVFAIL answers + being sent to clients. This has been fixed. :gl:`#3752` -- Clients may see an unexpected "Prohibited" extended DNS error when ``named`` - is configured with :any:`allow-recursion`). :gl:`#3743` +- The "Prohibited" Extended DNS Error was inadvertently set in some + NOERROR responses. This has been fixed. :gl:`#3743` -- Fix a TLS error that occured with large transfers over XoT. :gl:`#3772` +- Large zone transfers over TLS (XoT) could fail. This has been fixed. + :gl:`#3772` Known Issues ~~~~~~~~~~~~ From 3fcc0212941c26904d3f222dfa2e6ac820e6aed7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Thu, 12 Jan 2023 22:11:14 +0100 Subject: [PATCH 15/19] Reorder release notes --- doc/notes/notes-9.18.11.rst | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/doc/notes/notes-9.18.11.rst b/doc/notes/notes-9.18.11.rst index 16a0ffd994..d0cc062f4f 100644 --- a/doc/notes/notes-9.18.11.rst +++ b/doc/notes/notes-9.18.11.rst @@ -70,9 +70,16 @@ Feature Changes Bug Fixes ~~~~~~~~~ -- Previously, TLS session resumption could have led to handshake - failures when client certificates were used for authentication (Mutual - TLS). This has been fixed. :gl:`#3725` +- A rare assertion failure was fixed in outgoing TCP DNS connection + handling. :gl:`#3178` :gl:`#3636` + +- Large zone transfers over TLS (XoT) could fail. This has been fixed. + :gl:`#3772` + +- In addition to a previously fixed bug, another similar issue was + discovered where quotas could be erroneously reached for servers, + including any configured forwarders, resulting in SERVFAIL answers + being sent to clients. This has been fixed. :gl:`#3752` - When an outgoing request timed out, :iscman:`named` would retry up to three times with the same server instead of trying the next available @@ -85,19 +92,12 @@ Bug Fixes seconds) ADB names and entries are excluded from the overmem memory cleaner. :gl:`#3739` -- A rare assertion failure was fixed in outgoing TCP DNS connection - handling. :gl:`#3178` :gl:`#3636` - -- In addition to a previously fixed bug, another similar issue was - discovered where quotas could be erroneously reached for servers, - including any configured forwarders, resulting in SERVFAIL answers - being sent to clients. This has been fixed. :gl:`#3752` - - The "Prohibited" Extended DNS Error was inadvertently set in some NOERROR responses. This has been fixed. :gl:`#3743` -- Large zone transfers over TLS (XoT) could fail. This has been fixed. - :gl:`#3772` +- Previously, TLS session resumption could have led to handshake + failures when client certificates were used for authentication (Mutual + TLS). This has been fixed. :gl:`#3725` Known Issues ~~~~~~~~~~~~ From c9012706701b0e01ab454cc4ce1b45851e4920ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Thu, 12 Jan 2023 22:11:14 +0100 Subject: [PATCH 16/19] Add release note for GL #3678 --- doc/notes/notes-9.18.11.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/doc/notes/notes-9.18.11.rst b/doc/notes/notes-9.18.11.rst index d0cc062f4f..3e44dc2d69 100644 --- a/doc/notes/notes-9.18.11.rst +++ b/doc/notes/notes-9.18.11.rst @@ -81,6 +81,11 @@ Bug Fixes including any configured forwarders, resulting in SERVFAIL answers being sent to clients. This has been fixed. :gl:`#3752` +- In certain query resolution scenarios (e.g. when following CNAME + records), :iscman:`named` configured to answer from stale cache could + return a SERVFAIL response despite a usable, non-stale answer being + present in the cache. This has been fixed. :gl:`#3678` + - When an outgoing request timed out, :iscman:`named` would retry up to three times with the same server instead of trying the next available name server. This has been fixed. :gl:`#3637` From 3b4a34ccb9458eebd41fc868a3d1894b1dc15a9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Thu, 12 Jan 2023 22:11:14 +0100 Subject: [PATCH 17/19] Remove reused CHANGES entry Changes entry 6063 was added to the v9_18 branch (by commit cb3990001fb2ce978a44cc9d03b679cf626d03b1) without an associated placeholder in the main branch. The same entry number was subsequently reused for a different change in the main branch (by commit 41870dccbae3872154d5a6a76d70b5cb40d82c05). To prevent confusion, remove the entry from the v9_18 branch as the original code change whose reversal is mentioned in entry 6063 was not accompanied by its own CHANGES entry. --- CHANGES | 3 --- 1 file changed, 3 deletions(-) diff --git a/CHANGES b/CHANGES index 995638ee04..102387f4ac 100644 --- a/CHANGES +++ b/CHANGES @@ -13,9 +13,6 @@ exceeded, and the XML and JSON statistics version numbers have been updated. (CVE-2022-3094) [GL #3523] -6063. [bug] Revert a change that limited to honour single - read for TLSDNS as it broke XoT. [GL #3772] - 6062. [func] The DSCP implementation, which has been nonfunctional for some time, is now marked as obsolete and the implementation has been removed. From 703fefc88c9d5a8034d480e0cc86a3f4ae9d2620 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Thu, 12 Jan 2023 23:20:04 +0100 Subject: [PATCH 18/19] Add a CHANGES marker --- CHANGES | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES b/CHANGES index 102387f4ac..ca2f3a3ba3 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,5 @@ + --- 9.18.11 released --- + 6067. [security] Fix serve-stale crash when recursive clients soft quota is reached. (CVE-2022-3924) [GL #3619] From 1f554acee8df440c2fc211f976b196b31b9c9284 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Thu, 12 Jan 2023 23:20:04 +0100 Subject: [PATCH 19/19] Update BIND version for release --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index d087762da8..a143329498 100644 --- a/configure.ac +++ b/configure.ac @@ -17,7 +17,7 @@ m4_define([bind_VERSION_MAJOR], 9)dnl m4_define([bind_VERSION_MINOR], 18)dnl m4_define([bind_VERSION_PATCH], 11)dnl -m4_define([bind_VERSION_EXTRA], -dev)dnl +m4_define([bind_VERSION_EXTRA], )dnl m4_define([bind_DESCRIPTION], [(Stable Release)])dnl m4_define([bind_SRCID], [m4_esyscmd_s([git rev-parse --short HEAD | cut -b1-7])])dnl m4_define([bind_PKG_VERSION], [[bind_VERSION_MAJOR.bind_VERSION_MINOR.bind_VERSION_PATCH]bind_VERSION_EXTRA])dnl