[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Stable-8.2.7 39/53] nbd/server: CVE-2024-7409: Drop non-negotiating cli
From: |
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 |
From: Eric Blake <eblake@redhat.com>
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)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
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(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.2
- [Stable-8.2.7 32/53] vvfat: Fix bug in writing to middle of file, (continued)
- [Stable-8.2.7 32/53] vvfat: Fix bug in writing to middle of file, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 34/53] vvfat: Fix wrong checks for cluster mappings invariant, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 30/53] target/i386: Fix VSIB decode, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 33/53] vvfat: Fix usage of `info.file.offset`, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 37/53] nbd/server: Plumb in new args to nbd_client_add(), Michael Tokarev, 2024/09/06
- [Stable-8.2.7 36/53] iotests: Add `vvfat` tests, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 38/53] nbd/server: CVE-2024-7409: Cap default max-connections to 100, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 35/53] vvfat: Fix reading files with non-continuous clusters, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 44/53] target/i386: Do not apply REX to MMX operands, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 40/53] nbd/server: CVE-2024-7409: Close stray clients at server-stop, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 39/53] nbd/server: CVE-2024-7409: Drop non-negotiating clients,
Michael Tokarev <=
- [Stable-8.2.7 43/53] block/blkio: use FUA flag on write zeroes only if supported, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 42/53] hw/core/ptimer: fix timer zero period condition for freq > 1GHz, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 41/53] nbd/server: CVE-2024-7409: Avoid use-after-free when closing server, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 46/53] module: Prevent crash by resetting local_err in module_load_qom_all(), Michael Tokarev, 2024/09/06
- [Stable-8.2.7 47/53] target/hexagon: don't look for static glib, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 45/53] target/arm: Clear high SVE elements in handle_vec_simd_wshli, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 48/53] target/sparc: Restrict STQF to sparcv9, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 49/53] crypto/tlscredspsk: Free username on finalize, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 53/53] target/hppa: Fix PSW V-bit packaging in cpu_hppa_get for hppa64, Michael Tokarev, 2024/09/06
- [Stable-8.2.7 50/53] hw/nvme: fix leak of uninitialized memory in io_mgmt_recv, Michael Tokarev, 2024/09/06