You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
135 lines
5.3 KiB
135 lines
5.3 KiB
From 97012ea86a4a0a28fef68e43b989d858c8392e2a Mon Sep 17 00:00:00 2001
|
|
From: Eric Blake <eblake@redhat.com>
|
|
Date: Thu, 8 Aug 2024 16:05:08 -0500
|
|
Subject: [PATCH 4/5] nbd/server: CVE-2024-7409: Drop non-negotiating clients
|
|
MIME-Version: 1.0
|
|
Content-Type: text/plain; charset=UTF-8
|
|
Content-Transfer-Encoding: 8bit
|
|
|
|
RH-Author: Eric Blake <eblake@redhat.com>
|
|
RH-MergeRequest: 263: nbd/server: fix CVE-2024-7409 (qemu crash on nbd-server-stop) [RHEL 10.0]
|
|
RH-Jira: RHEL-52599
|
|
RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>
|
|
RH-Commit: [3/4] c3dad94d423d2f431d1e605c412099b9fe0bd76e (redhat/centos-stream/src/qemu-kvm)
|
|
|
|
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 = 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é <berrange@redhat.com>
|
|
Signed-off-by: Eric Blake <eblake@redhat.com>
|
|
Message-ID: <20240807174943.771624-13-eblake@redhat.com>
|
|
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
|
|
[eblake: rebase to changes earlier in series, reduce scope of timer]
|
|
Signed-off-by: Eric Blake <eblake@redhat.com>
|
|
|
|
(cherry picked from commit b9b72cb3ce15b693148bd09cef7e50110566d8a0)
|
|
Jira: https://issues.redhat.com/browse/RHEL-52599
|
|
Signed-off-by: Eric Blake <eblake@redhat.com>
|
|
---
|
|
nbd/server.c | 28 +++++++++++++++++++++++++++-
|
|
nbd/trace-events | 1 +
|
|
2 files changed, 28 insertions(+), 1 deletion(-)
|
|
|
|
diff --git a/nbd/server.c b/nbd/server.c
|
|
index e50012499f..39285cc971 100644
|
|
--- a/nbd/server.c
|
|
+++ b/nbd/server.c
|
|
@@ -3186,22 +3186,48 @@ static void nbd_client_receive_next_request(NBDClient *client)
|
|
}
|
|
}
|
|
|
|
+static void nbd_handshake_timer_cb(void *opaque)
|
|
+{
|
|
+ QIOChannel *ioc = 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 = opaque;
|
|
Error *local_err = NULL;
|
|
+ QEMUTimer *handshake_timer = NULL;
|
|
|
|
qemu_co_mutex_init(&client->send_lock);
|
|
|
|
- /* 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 = 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;
|
|
}
|
|
|
|
+ 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=0x%" PRIx64 ", len=0x%" PRIx64
|
|
nbd_co_receive_align_compliance(const char *op, uint64_t from, uint64_t len, uint32_t align) "client sent non-compliant unaligned %s request: from=0x%" PRIx64 ", len=0x%" PRIx64 ", align=0x%" PRIx32
|
|
nbd_trip(void) "Reading request"
|
|
+nbd_handshake_timer_cb(void) "client took too long to negotiate"
|
|
|
|
# client-connection.c
|
|
nbd_connect_thread_sleep(uint64_t timeout) "timeout %" PRIu64
|
|
--
|
|
2.39.3
|
|
|