From 021aeeed38b563d97394f7639ce89ef7c6dff1ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= Date: Fri, 25 Mar 2022 20:49:49 +0100 Subject: [PATCH] [CVE-2022-0396] Resolve #3112 TCP sockets stuck in CLOSE_WAIT 5818. [security] A synchronous call to closehandle_cb() caused isc__nm_process_sock_buffer() to be called recursively, which in turn left TCP connections hanging in the CLOSE_WAIT state blocking indefinitely when out-of-order processing was disabled. (CVE-2022-0396) [GL #3112] Resolves: CVE-2022-0396 --- bind-9.16-CVE-2022-0396.patch | 81 +++++++++++++++++++++++++++++++++++ bind.spec | 9 +++- 2 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 bind-9.16-CVE-2022-0396.patch diff --git a/bind-9.16-CVE-2022-0396.patch b/bind-9.16-CVE-2022-0396.patch new file mode 100644 index 0000000..5a374f1 --- /dev/null +++ b/bind-9.16-CVE-2022-0396.patch @@ -0,0 +1,81 @@ +From 33064cd077cf6fa386f0a5a840c2161868da7b3a Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= +Date: Tue, 8 Feb 2022 12:42:34 +0100 +Subject: [PATCH] Run .closehandle_cb asynchrounosly in nmhandle_detach_cb() + +When sock->closehandle_cb is set, we need to run nmhandle_detach_cb() +asynchronously to ensure correct order of multiple packets processing in +the isc__nm_process_sock_buffer(). When not run asynchronously, it +would cause: + + a) out-of-order processing of the return codes from processbuffer(); + + b) stack growth because the next TCP DNS message read callback will + be called from within the current TCP DNS message read callback. + +The sock->closehandle_cb is set to isc__nm_resume_processing() for TCP +sockets which calls isc__nm_process_sock_buffer(). If the read callback +(called from isc__nm_process_sock_buffer()->processbuffer()) doesn't +attach to the nmhandle (f.e. because it wants to drop the processing or +we send the response directly via uv_try_write()), the +isc__nm_resume_processing() (via .closehandle_cb) would call +isc__nm_process_sock_buffer() recursively. + +The below shortened code path shows how the stack can grow: + + 1: ns__client_request(handle, ...); + 2: isc_nm_tcpdns_sequential(handle); + 3: ns_query_start(client, handle); + 4: query_lookup(qctx); + 5: query_send(qctcx->client); + 6: isc__nmhandle_detach(&client->reqhandle); + 7: nmhandle_detach_cb(&handle); + 8: sock->closehandle_cb(sock); // isc__nm_resume_processing + 9: isc__nm_process_sock_buffer(sock); +10: processbuffer(sock); // isc__nm_tcpdns_processbuffer +11: isc_nmhandle_attach(req->handle, &handle); +12: isc__nm_readcb(sock, req, ISC_R_SUCCESS); +13: isc__nm_async_readcb(NULL, ...); +14: uvreq->cb.recv(...); // ns__client_request + +Instead, if 'sock->closehandle_cb' is set, we need to run detach the +handle asynchroniously in 'isc__nmhandle_detach', so that on line 8 in +the code flow above does not start this recursion. This ensures the +correct order when processing multiple packets in the function +'isc__nm_process_sock_buffer()' and prevents the stack growth. + +When not run asynchronously, the out-of-order processing leaves the +first TCP socket open until all requests on the stream have been +processed. + +If the pipelining is disabled on the TCP via `keep-response-order` +configuration option, named would keep the first socket in lingering +CLOSE_WAIT state when the client sends an incomplete packet and then +closes the connection from the client side. + +(cherry picked from commit afee2b5a7bc933a2d987907fc327a9f118fdbd17) +--- + lib/isc/netmgr/netmgr.c | 6 +++++- + 1 file changed, 5 insertions(+), 1 deletion(-) + +diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c +index 3283eb6e4f..0ed3182fb6 100644 +--- a/lib/isc/netmgr/netmgr.c ++++ b/lib/isc/netmgr/netmgr.c +@@ -1746,8 +1746,12 @@ isc__nmhandle_detach(isc_nmhandle_t **handlep FLARG) { + handle = *handlep; + *handlep = NULL; + ++ /* ++ * If the closehandle_cb is set, it needs to run asynchronously to ++ * ensure correct ordering of the isc__nm_process_sock_buffer(). ++ */ + sock = handle->sock; +- if (sock->tid == isc_nm_tid()) { ++ if (sock->tid == isc_nm_tid() && sock->closehandle_cb == NULL) { + nmhandle_detach_cb(&handle FLARG_PASS); + } else { + isc__netievent_detach_t *event = +-- +2.34.1 + diff --git a/bind.spec b/bind.spec index 6e30f0a..5773514 100644 --- a/bind.spec +++ b/bind.spec @@ -51,7 +51,7 @@ Summary: The Berkeley Internet Name Domain (BIND) DNS (Domain Name System) serv Name: bind License: MPLv2.0 Version: 9.16.23 -Release: 1%{?dist} +Release: 2%{?dist} Epoch: 32 Url: https://www.isc.org/downloads/bind/ # @@ -101,6 +101,8 @@ Patch157:bind-9.11-fips-tests.patch Patch164:bind-9.11-rh1666814.patch Patch170:bind-9.11-feature-test-named.patch Patch171:bind-9.11-tests-variants.patch +# https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/5987 +Patch172:bind-9.16-CVE-2022-0396.patch %{?systemd_ordering} Requires: coreutils @@ -401,6 +403,7 @@ in HTML and PDF format. %patch164 -p1 -b .rh1666814 %patch170 -p1 -b .featuretest-named %patch171 -p1 -b .test-variant +%patch172 -p1 -b .CVE-2022-0396 %if %{with PKCS11} %patch135 -p1 -b .config-pkcs11 @@ -1123,6 +1126,10 @@ fi; %endif %changelog +* Fri Mar 25 2022 Petr Menšík - 32:9.16.23-2 +- TCP connections with 'keep-response-order' are properly close in all cases + (CVE-2022-0396) + * Fri Nov 19 2021 Petr Menšík - 32:9.16.23-1 - Update to 9.16.23 (#2024210)