From 2adaa53619bbc3ad9732e788f2a674ba99060217 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 8 Sep 2022 18:24:57 +0200 Subject: [PATCH 1/3] Handle canceled read during sending data over stats channel An assertion failure would be triggered when the TCP connection is canceled during sending the data back to the client. Don't require the state to be `RECV` on non successful read to gracefully handle canceled TCP connection during the SEND state of the HTTPD channel. (cherry picked from commit 6562227cc8a0732002c01cfc045129bf6080b94c) --- lib/isc/httpd.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/isc/httpd.c b/lib/isc/httpd.c index a701fb2a84..134806bea9 100644 --- a/lib/isc/httpd.c +++ b/lib/isc/httpd.c @@ -904,13 +904,14 @@ httpd_request(isc_nmhandle_t *handle, isc_result_t eresult, httpd = isc_nmhandle_getdata(handle); - REQUIRE(httpd->state == RECV); REQUIRE(httpd->handle == handle); if (eresult != ISC_R_SUCCESS) { goto cleanup_readhandle; } + REQUIRE(httpd->state == RECV); + result = process_request( httpd, region == NULL ? &(isc_region_t){ NULL, 0 } : region, &buflen); @@ -1195,7 +1196,6 @@ httpd_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { isc_httpd_t *httpd = (isc_httpd_t *)arg; REQUIRE(VALID_HTTPD(httpd)); - REQUIRE(httpd->state == SEND); REQUIRE(httpd->handle == handle); isc_buffer_free(&httpd->sendbuffer); @@ -1222,6 +1222,8 @@ httpd_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { goto cleanup_readhandle; } + REQUIRE(httpd->state == SEND); + httpd->state = RECV; httpd->sendhandle = NULL; From 474676a38c6f4033756cf29f943316ee9deb0aff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 14 Sep 2022 14:18:32 +0200 Subject: [PATCH 2/3] Provide stronger wording about the security of statistics channel Add more text about the importance of properly securing the statistics channel and what is and what is not considered a security vulnerability. (cherry picked from commit 6869c98d369270e4efbc3ffa0cd21526b32907de) --- doc/arm/reference.rst | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/doc/arm/reference.rst b/doc/arm/reference.rst index 651c39d058..ef5e72228e 100644 --- a/doc/arm/reference.rst +++ b/doc/arm/reference.rst @@ -5842,9 +5842,21 @@ If no port is specified, port 80 is used for HTTP channels. The asterisk Attempts to open a statistics channel are restricted by the optional ``allow`` clause. Connections to the statistics channel are permitted based on the :term:`address_match_list`. If no ``allow`` clause is -present, :iscman:`named` accepts connection attempts from any address; since -the statistics may contain sensitive internal information, it is highly -recommended to restrict the source of connection requests appropriately. +present, :iscman:`named` accepts connection attempts from any address. Since +the statistics may contain sensitive internal information, the source of +connection requests must be restricted appropriately so that only +trusted parties can access the statistics channel. + +Gathering data exposed by the statistics channel locks various subsystems in +:iscman:`named`, which could slow down query processing if statistics data is +requested too often. + +An issue in the statistics channel would be considered a security issue +only if it could be exploited by unprivileged users circumventing the access +control list. In other words, any issue in the statistics channel that could be +used to access information unavailable otherwise, or to crash :iscman:`named`, is +not considered a security issue if it can be avoided through the +use of a secure configuration. If no :any:`statistics-channels` statement is present, :iscman:`named` does not open any communication channels. From c9c4de0626d7300e63476ec8391616685809d7a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Fri, 9 Sep 2022 10:48:13 +0200 Subject: [PATCH 3/3] Add CHANGES and release note for [GL #3542] (cherry picked from commit e29563173bf931830d0a2eba0d0aaebfadbfad8e) --- CHANGES | 4 ++++ doc/notes/notes-current.rst | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index c944deda4a..ddc05088fa 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5972. [bug] Gracefully handle when the statschannel HTTP connection + gets cancelled during sending data back to the client. + [GL #3542] + 5970. [func] Log the reason why a query was refused. [GL !6669] 5967. [cleanup] Flagged the "random-device" option (which was diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 1c26f5934f..3ab1293370 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -40,4 +40,5 @@ Feature Changes Bug Fixes ~~~~~~~~~ -- None. +- An assertion failure was fixed in ``named`` that was caused by aborting the statistics + channel connection while sending statistics data to the client. :gl:`#3542`