From ad489c44dff449ea1ef01286fa32d6a0e2326420 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Thu, 9 May 2024 17:06:14 +0000 Subject: [PATCH] Remove sig0checks-quota-maxwait-ms support Waiting for a quota to appear complicates things and wastes rosources on timer management. Just answer with REFUSE if there is no quota. --- bin/named/config.c | 1 - bin/named/server.c | 6 -- doc/arm/reference.rst | 24 +------ doc/misc/options | 1 - lib/isccfg/namedconf.c | 1 - lib/ns/client.c | 125 ++++++++++--------------------------- lib/ns/include/ns/client.h | 2 - lib/ns/include/ns/server.h | 1 - 8 files changed, 36 insertions(+), 125 deletions(-) diff --git a/bin/named/config.c b/bin/named/config.c index 62c05a8314..3ad700a887 100644 --- a/bin/named/config.c +++ b/bin/named/config.c @@ -110,7 +110,6 @@ options {\n\ session-keyname local-ddns;\n\ startup-notify-rate 20;\n\ sig0checks-quota 1;\n\ - sig0checks-quota-maxwait-ms 1500;\n\ statistics-file \"named.stats\";\n\ tcp-advertised-timeout 300;\n\ tcp-clients 150;\n\ diff --git a/bin/named/server.c b/bin/named/server.c index a4c3269432..dc16659341 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -8463,12 +8463,6 @@ load_configuration(const char *filename, named_server_t *server, INSIST(result == ISC_R_SUCCESS); } - obj = NULL; - result = named_config_get(maps, "sig0checks-quota-maxwait-ms", &obj); - if (result == ISC_R_SUCCESS) { - server->sctx->sig0checksquota_maxwaitms = cfg_obj_asuint32(obj); - } - /* * Set "blackhole". Only legal at options level; there is * no default. diff --git a/doc/arm/reference.rst b/doc/arm/reference.rst index b0c593e4b4..1b782b976d 100644 --- a/doc/arm/reference.rst +++ b/doc/arm/reference.rst @@ -4005,27 +4005,9 @@ system. :short: Specifies the maximum number of concurrent SIG(0) signature checks that can be processed by the server. This is the maximum number of simultaneous SIG(0)-signed messages that - the server will accept. If the quota is reached, then :iscman:`named` waits - for the maximum of :any:`sig0checks-quota-maxwait-ms` time for a quota to - appear or to answer with a status code of REFUSED. The value of ``0`` - disables the quota. The default is ``1``. - - .. note:: - - :any:`sig0checks-quota` protection does not work when there is only one - worker thread available, or when the option is set to a value that is - greater or equal to the worker threads available. See the ``-n #cpus`` - option of :iscman:`named` for more information about the worker threads. - -.. namedconf:statement:: sig0checks-quota-maxwait-ms - :tags: server - :short: Specifies the maximum number of milliseconds to wait for a SIG(0) signature checking quota to appear. - - When :any:`sig0checks-quota` is effective and a client reaches the quota, - then :iscman:`named` waits for the maximum of - :any:`sig0checks-quota-maxwait-ms` time (in milliseconds) for a quota to - appear. If no quota becomes available, then an answer with a status code of - REFUSED is sent. The default is ``1500``. + the server accepts. If the quota is reached, then :iscman:`named` answers + with a status code of REFUSED. The value of ``0`` disables the quota. The + default is ``1``. .. namedconf:statement:: sig0checks-quota-exempt :tags: server diff --git a/doc/misc/options b/doc/misc/options index d003e9e741..0aec7c5dea 100644 --- a/doc/misc/options +++ b/doc/misc/options @@ -279,7 +279,6 @@ options { sig-validity-interval [ ]; // obsolete sig0checks-quota ; sig0checks-quota-exempt { ; ... }; - sig0checks-quota-maxwait-ms ; sortlist { ; ... }; // deprecated stale-answer-client-timeout ( disabled | off | ); stale-answer-enable ; diff --git a/lib/isccfg/namedconf.c b/lib/isccfg/namedconf.c index 3a27432f7e..b9f8500d7a 100644 --- a/lib/isccfg/namedconf.c +++ b/lib/isccfg/namedconf.c @@ -1362,7 +1362,6 @@ static cfg_clausedef_t options_clauses[] = { { "session-keyfile", &cfg_type_qstringornone, 0 }, { "session-keyname", &cfg_type_astring, 0 }, { "sig0checks-quota", &cfg_type_uint32, 0 }, - { "sig0checks-quota-maxwait-ms", &cfg_type_uint32, 0 }, { "sig0checks-quota-exempt", &cfg_type_bracketed_aml, 0 }, { "sit-secret", NULL, CFG_CLAUSEFLAG_ANCIENT }, { "stacksize", &cfg_type_size, CFG_CLAUSEFLAG_ANCIENT }, diff --git a/lib/ns/client.c b/lib/ns/client.c index 1e2229b95e..8d91ad07d0 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -1709,8 +1709,6 @@ ns__client_reset_cb(void *client0) { client->keytag_len = 0; } - isc_timer_stop(client->quotatimer); - if (client->async) { client->async = false; if (client->handle != NULL) { @@ -1751,8 +1749,6 @@ ns__client_put_cb(void *client0) { dns_message_puttemprdataset(client->message, &client->opt); } - isc_timer_async_destroy(&client->quotatimer); - if (client->async) { client->async = false; if (client->handle != NULL) { @@ -2139,7 +2135,6 @@ ns_client_request(isc_nmhandle_t *handle, isc_result_t eresult, static void ns_client_request_continue(void *arg) { ns_client_t *client = arg; - isc_netaddr_t netaddr; const dns_name_t *signame = NULL; bool ra; /* Recursion available. */ isc_result_t result = ISC_R_UNSET; @@ -2168,72 +2163,16 @@ ns_client_request_continue(void *arg) { INSIST(client->viewmatchresult != ISC_R_UNSET); - isc_netaddr_fromsockaddr(&netaddr, &client->peeraddr); - /* - * This function could be running asynchronously when validating a - * SIG(0) signature or waiting for a quota for it. In that case we - * need to update the current 'now', and check that named doesn't wait - * for too long. + * This function could be running asynchronously, in which case update + * the current 'now' for correct timekeeping. */ if (client->async) { client->tnow = isc_time_now(); client->now = isc_time_seconds(&client->tnow); } - if (client->async && client->viewmatchresult == ISC_R_QUOTA) { - uint64_t wait_us, maxwait_us; - - /* Waiting for a quota. */ - wait_us = isc_time_microdiff(&client->tnow, - &client->requesttime); - maxwait_us = US_PER_MS * - client->manager->sctx->sig0checksquota_maxwaitms; - if (wait_us > maxwait_us) { - isc_buffer_t b; - isc_region_t *r; - - ns_client_log(client, NS_LOGCATEGORY_CLIENT, - NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(5), - "client reached max wait time for quota"); - - /* - * Do a dummy TSIG verification attempt so that the - * response will have a TSIG if the query did, as - * required by RFC2845. - */ - dns_message_resetsig(client->message); - r = dns_message_getrawmessage(client->message); - isc_buffer_init(&b, r->base, r->length); - isc_buffer_add(&b, r->length); - (void)dns_tsig_verify(&b, client->message, NULL, NULL); - - ns_client_extendederror(client, DNS_EDE_PROHIBITED, - NULL); - ns_client_error(client, DNS_R_REFUSED); - goto cleanup; - } - - if (can_log_sigchecks_quota()) { - ns_client_log(client, NS_LOGCATEGORY_CLIENT, - NS_LOGMODULE_CLIENT, ISC_LOG_INFO, - "SIG(0) checks quota reached"); - ns_client_dumpmessage(client, - "SIG(0) checks quota reached"); - } - - /* Retry after 10 milliseconds. */ - isc_interval_t interval; - isc_interval_set(&interval, 0, 10 * NS_PER_MS); - isc_nmhandle_ref(client->handle); - isc_timer_start(client->quotatimer, isc_timertype_once, - &interval); - - result = DNS_R_WAIT; - goto cleanup; - } if (client->viewmatchresult != ISC_R_SUCCESS) { - char classname[DNS_RDATACLASS_FORMATSIZE]; isc_buffer_t b; isc_region_t *r; @@ -2248,12 +2187,31 @@ ns_client_request_continue(void *arg) { isc_buffer_add(&b, r->length); (void)dns_tsig_verify(&b, client->message, NULL, NULL); - dns_rdataclass_format(client->message->rdclass, classname, - sizeof(classname)); - ns_client_log(client, NS_LOGCATEGORY_CLIENT, - NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(1), - "no matching view in class '%s'", classname); - ns_client_dumpmessage(client, "no matching view in class"); + if (client->viewmatchresult == ISC_R_QUOTA) { + ns_client_log(client, NS_LOGCATEGORY_CLIENT, + NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(5), + "SIG(0) checks quota reached"); + + if (can_log_sigchecks_quota()) { + ns_client_log(client, NS_LOGCATEGORY_CLIENT, + NS_LOGMODULE_CLIENT, ISC_LOG_INFO, + "SIG(0) checks quota reached"); + ns_client_dumpmessage( + client, "SIG(0) checks quota reached"); + } + } else { + char classname[DNS_RDATACLASS_FORMATSIZE]; + + dns_rdataclass_format(client->message->rdclass, + classname, sizeof(classname)); + + ns_client_log(client, NS_LOGCATEGORY_CLIENT, + NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(1), + "no matching view in class '%s'", + classname); + ns_client_dumpmessage(client, + "no matching view in class"); + } ns_client_extendederror(client, DNS_EDE_PROHIBITED, NULL); ns_client_error(client, DNS_R_REFUSED); @@ -2460,6 +2418,9 @@ ns_client_request_continue(void *arg) { if (client->udpsize > 512) { dns_peer_t *peer = NULL; uint16_t udpsize = client->view->maxudp; + isc_netaddr_t netaddr; + + isc_netaddr_fromsockaddr(&netaddr, &client->peeraddr); (void)dns_peerlist_peerbyaddr(client->view->peers, &netaddr, &peer); if (peer != NULL) { @@ -2521,17 +2482,14 @@ ns_client_request_continue(void *arg) { cleanup: /* - * If we are running async then 'unref' the handle, and also if there - * are no more async calls scheduled the reset the async flag, so that - * the destructor doesn't try to 'unref' the handle too. + * If we are running async then 'unref' the handle and reset the async + * flag, so that the destructor doesn't try to 'unref' the handle too. */ if (client->async) { + client->async = false; if (client->handle != NULL) { isc_nmhandle_unref(client->handle); } - if (result != DNS_R_WAIT) { - client->async = false; - } } } @@ -2569,18 +2527,6 @@ ns__client_tcpconn(isc_nmhandle_t *handle, isc_result_t result, void *arg) { return (ISC_R_SUCCESS); } -static void -client_quotatimer_event(void *arg) { - ns_client_t *client = arg; - isc_netaddr_t netaddr; - isc_result_t result; - - isc_netaddr_fromsockaddr(&netaddr, &client->peeraddr); - result = ns_client_setup_view(client, &netaddr); - INSIST(result == DNS_R_WAIT); /* We are in async mode. */ - isc_nmhandle_unref(client->handle); -} - isc_result_t ns__client_setup(ns_client_t *client, ns_clientmgr_t *mgr, bool new) { isc_result_t result; @@ -2604,9 +2550,6 @@ ns__client_setup(ns_client_t *client, ns_clientmgr_t *mgr, bool new) { client->manager->rdspool, DNS_MESSAGE_INTENTPARSE, &client->message); - isc_timer_create(client->manager->loop, client_quotatimer_event, - client, &client->quotatimer); - /* * Set magic earlier than usual because ns_query_init() * and the functions it calls will require it. @@ -2628,7 +2571,6 @@ ns__client_setup(ns_client_t *client, ns_clientmgr_t *mgr, bool new) { .magic = 0, .manager = client->manager, .message = client->message, - .quotatimer = client->quotatimer, .query = client->query, }; } @@ -2652,7 +2594,6 @@ ns__client_setup(ns_client_t *client, ns_clientmgr_t *mgr, bool new) { return (ISC_R_SUCCESS); cleanup: - isc_timer_destroy(&client->quotatimer); dns_message_detach(&client->message); ns_clientmgr_detach(&client->manager); diff --git a/lib/ns/include/ns/client.h b/lib/ns/include/ns/client.h index 14acb4297c..1213c6ec3b 100644 --- a/lib/ns/include/ns/client.h +++ b/lib/ns/include/ns/client.h @@ -203,8 +203,6 @@ struct ns_client { isc_netaddr_t destaddr; isc_sockaddr_t destsockaddr; - isc_timer_t *quotatimer; - dns_ecs_t ecs; /*%< EDNS client subnet sent by client */ /*% diff --git a/lib/ns/include/ns/server.h b/lib/ns/include/ns/server.h index 9acf83a966..fc01ee2c62 100644 --- a/lib/ns/include/ns/server.h +++ b/lib/ns/include/ns/server.h @@ -91,7 +91,6 @@ struct ns_server { isc_quota_t xfroutquota; isc_quota_t updquota; isc_quota_t sig0checksquota; - uint32_t sig0checksquota_maxwaitms; dns_acl_t *sig0checksquota_exempt; ISC_LIST(isc_quota_t) http_quotas; isc_mutex_t http_quotas_lock;