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.

diff --git a/bin/named/config.c b/bin/named/config.c
index 62c05a8314180e8d792afe4d029f8e25a0da6def..3ad700a887374a330ffdeedefe642e9e0b38a25f 100644 (file)
--- 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 a4c326943202d3aff221b169cec753d5ccd2053c..dc16659341f10355e031a6eeedf07efdcfece6fe 100644 (file)
--- 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 b0c593e4b4904de6d12230b703c9f75f2aa4c698..1b782b976d094a435f5f671a5207ed8cfe11cc81 100644 (file)
--- 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 d003e9e74104824b6361a278d05ecb9a0b4b42cf..0aec7c5deab7ea83aa391c8ddbd5da98c94dfc62 100644 (file)
--- a/doc/misc/options
+++ b/doc/misc/options
@@ -279,7 +279,6 @@ options {
        sig-validity-interval <integer> [ <integer> ]; // obsolete
        sig0checks-quota <integer>;
        sig0checks-quota-exempt { <address_match_element>; ... };
-       sig0checks-quota-maxwait-ms <integer>;
        sortlist { <address_match_element>; ... }; // deprecated
        stale-answer-client-timeout ( disabled | off | <integer> );
        stale-answer-enable <boolean>;

diff --git a/lib/isccfg/namedconf.c b/lib/isccfg/namedconf.c
index 3a27432f7ec48a7bf05f3861624bacb10cbb5ffc..b9f8500d7a484b783791227bd8e7d461618be02c 100644 (file)
--- 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 1e2229b95ec9c710df006fb2799dc9d20e95ec0c..8d91ad07d04a711011114aa19a7ca1a6b70bcb91 100644 (file)
--- 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 14acb4297c399ee572607379e406a35a6bc937ae..1213c6ec3b83fff2897ae8d18da4e6a353b5233b 100644 (file)
--- 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 9acf83a9660edbad4697253473408ab827c53099..fc01ee2c62d9d32c8e3b7f3ab57de792325fc897 100644 (file)
--- 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;