From nobody Sun Nov 24 06:46:56 2024 Delivered-To: importer@patchew.org Authentication-Results: mx.zohomail.com; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 172560638104091.84700190852698; Fri, 6 Sep 2024 00:06:21 -0700 (PDT) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1smSvu-0002ZD-6M; Fri, 06 Sep 2024 02:59:02 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1smSuu-0005Um-2h; Fri, 06 Sep 2024 02:58:01 -0400 Received: from isrv.corpit.ru ([86.62.121.231]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1smSuq-0003ar-TG; Fri, 06 Sep 2024 02:57:59 -0400 Received: from tsrv.corpit.ru (tsrv.tls.msk.ru [192.168.177.2]) by isrv.corpit.ru (Postfix) with ESMTP id 3A7DD8C25C; Fri, 6 Sep 2024 09:53:14 +0300 (MSK) Received: from tls.msk.ru (mjt.wg.tls.msk.ru [192.168.177.130]) by tsrv.corpit.ru (Postfix) with SMTP id 02E0C133414; Fri, 6 Sep 2024 09:54:32 +0300 (MSK) Received: (nullmailer pid 43636 invoked by uid 1000); Fri, 06 Sep 2024 06:54:30 -0000 From: Michael Tokarev To: qemu-devel@nongnu.org Cc: qemu-stable@nongnu.org, Eric Blake , =?UTF-8?q?Daniel=20P=20=2E=20Berrang=C3=A9?= , Michael Tokarev Subject: [Stable-8.2.7 39/53] nbd/server: CVE-2024-7409: Drop non-negotiating clients Date: Fri, 6 Sep 2024 09:54:09 +0300 Message-Id: <20240906065429.42415-39-mjt@tls.msk.ru> X-Mailer: git-send-email 2.39.2 In-Reply-To: References: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Received-SPF: pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Received-SPF: pass client-ip=86.62.121.231; envelope-from=mjt@tls.msk.ru; helo=isrv.corpit.ru X-Spam_score_int: -68 X-Spam_score: -6.9 X-Spam_bar: ------ X-Spam_report: (-6.9 / 5.0 requ) BAYES_00=-1.9, RCVD_IN_DNSWL_HI=-5, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: qemu-devel-bounces+importer=patchew.org@nongnu.org X-ZM-MESSAGEID: 1725606381954116600 From: Eric Blake A client that opens a socket but does not negotiate is merely hogging qemu's resources (an open fd and a small amount of memory); and a malicious client that can access the port where NBD is listening can attempt a denial of service attack by intentionally opening and abandoning lots of unfinished connections. The previous patch put a default bound on the number of such ongoing connections, but once that limit is hit, no more clients can connect (including legitimate ones). The solution is to insist that clients complete handshake within a reasonable time limit, defaulting to 10 seconds. A client that has not successfully completed NBD_OPT_GO by then (including the case of where the client didn't know TLS credentials to even reach the point of NBD_OPT_GO) is wasting our time and does not deserve to stay connected. Later patches will allow fine-tuning the limit away from the default value (including disabling it for doing integration testing of the handshake process itself). Note that this patch in isolation actually makes it more likely to see qemu SEGV after nbd-server-stop, as any client socket still connected when the server shuts down will now be closed after 10 seconds rather than at the client's whims. That will be addressed in the next patch. For a demo of this patch in action: $ qemu-nbd -f raw -r -t -e 10 file & $ nbdsh --opt-mode -c ' H =3D list() for i in range(20): print(i) H.insert(i, nbd.NBD()) H[i].set_opt_mode(True) H[i].connect_uri("nbd://localhost") ' $ kill $! where later connections get to start progressing once earlier ones are forcefully dropped for taking too long, rather than hanging. Suggested-by: Daniel P. Berrang=C3=A9 Signed-off-by: Eric Blake Message-ID: <20240807174943.771624-13-eblake@redhat.com> Reviewed-by: Daniel P. Berrang=C3=A9 [eblake: rebase to changes earlier in series, reduce scope of timer] Signed-off-by: Eric Blake (cherry picked from commit b9b72cb3ce15b693148bd09cef7e50110566d8a0) Signed-off-by: Michael Tokarev diff --git a/nbd/server.c b/nbd/server.c index 91131883ee..3ed8b1df9c 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -3189,22 +3189,48 @@ static void nbd_client_receive_next_request(NBDClie= nt *client) } } =20 +static void nbd_handshake_timer_cb(void *opaque) +{ + QIOChannel *ioc =3D opaque; + + trace_nbd_handshake_timer_cb(); + qio_channel_shutdown(ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL); +} + static coroutine_fn void nbd_co_client_start(void *opaque) { NBDClient *client =3D opaque; Error *local_err =3D NULL; + QEMUTimer *handshake_timer =3D NULL; =20 qemu_co_mutex_init(&client->send_lock); =20 - /* TODO - utilize client->handshake_max_secs */ + /* + * Create a timer to bound the time spent in negotiation. If the + * timer expires, it is likely nbd_negotiate will fail because the + * socket was shutdown. + */ + if (client->handshake_max_secs > 0) { + handshake_timer =3D aio_timer_new(qemu_get_aio_context(), + QEMU_CLOCK_REALTIME, + SCALE_NS, + nbd_handshake_timer_cb, + client->sioc); + timer_mod(handshake_timer, + qemu_clock_get_ns(QEMU_CLOCK_REALTIME) + + client->handshake_max_secs * NANOSECONDS_PER_SECOND); + } + if (nbd_negotiate(client, &local_err)) { if (local_err) { error_report_err(local_err); } + timer_free(handshake_timer); client_close(client, false); return; } =20 + timer_free(handshake_timer); WITH_QEMU_LOCK_GUARD(&client->lock) { nbd_client_receive_next_request(client); } diff --git a/nbd/trace-events b/nbd/trace-events index 00ae3216a1..cbd0a4ab7e 100644 --- a/nbd/trace-events +++ b/nbd/trace-events @@ -76,6 +76,7 @@ nbd_co_receive_request_payload_received(uint64_t cookie, = uint64_t len) "Payload nbd_co_receive_ext_payload_compliance(uint64_t from, uint64_t len) "client= sent non-compliant write without payload flag: from=3D0x%" PRIx64 ", len= =3D0x%" PRIx64 nbd_co_receive_align_compliance(const char *op, uint64_t from, uint64_t le= n, uint32_t align) "client sent non-compliant unaligned %s request: from=3D= 0x%" PRIx64 ", len=3D0x%" PRIx64 ", align=3D0x%" PRIx32 nbd_trip(void) "Reading request" +nbd_handshake_timer_cb(void) "client took too long to negotiate" =20 # client-connection.c nbd_connect_thread_sleep(uint64_t timeout) "timeout %" PRIu64 --=20 2.39.2