diff options
| author | Hitendra Prajapati <hprajapati@mvista.com> | 2024-09-03 13:38:34 +0530 |
|---|---|---|
| committer | Steve Sakoman <steve@sakoman.com> | 2024-09-07 05:38:17 -0700 |
| commit | a952dec3e479f50a645faaf7258f5322c1767e42 (patch) | |
| tree | 6f88d48d55d8a2121ab23f22cd1cce38ab93f711 | |
| parent | 0ae3b2bd492d32ba401a447f09679ebaf10ddad4 (diff) | |
| download | poky-a952dec3e479f50a645faaf7258f5322c1767e42.tar.gz | |
qemu: fix CVE-2024-7409
A flaw was found in the QEMU NBD Server. This vulnerability allows a denial of service (DoS) attack
via improper synchronization during socket closure when a client keeps a socket open as the server
is taken offline.
Reference:
https://nvd.nist.gov/vuln/detail/CVE-2024-7409
Upstream Patches:
https://github.com/qemu/qemu/commit/fb1c2aaa981e0a2fa6362c9985f1296b74f055ac
https://github.com/qemu/qemu/commit/c8a76dbd90c2f48df89b75bef74917f90a59b623
https://gitlab.com/qemu-project/qemu/-/commit/b9b72cb3ce15b693148bd09cef7e50110566d8a0
https://gitlab.com/qemu-project/qemu/-/commit/3e7ef738c8462c45043a1d39f702a0990406a3b3
(From OE-Core rev: d84ab04dc66cb83638f96fcd2f4c67e67489c410)
Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com>
Signed-off-by: Steve Sakoman <steve@sakoman.com>
| -rw-r--r-- | meta/recipes-devtools/qemu/qemu.inc | 4 | ||||
| -rw-r--r-- | meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0001.patch | 162 | ||||
| -rw-r--r-- | meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0002.patch | 174 | ||||
| -rw-r--r-- | meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0003.patch | 122 | ||||
| -rw-r--r-- | meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0004.patch | 163 |
5 files changed, 625 insertions, 0 deletions
diff --git a/meta/recipes-devtools/qemu/qemu.inc b/meta/recipes-devtools/qemu/qemu.inc index 4747310ae4..4684e44524 100644 --- a/meta/recipes-devtools/qemu/qemu.inc +++ b/meta/recipes-devtools/qemu/qemu.inc | |||
| @@ -109,6 +109,10 @@ SRC_URI = "https://download.qemu.org/${BPN}-${PV}.tar.xz \ | |||
| 109 | file://scsi-disk-ensure-block-size-is-non-zero-and-changes-limited-to-bits-8-15.patch \ | 109 | file://scsi-disk-ensure-block-size-is-non-zero-and-changes-limited-to-bits-8-15.patch \ |
| 110 | file://CVE-2023-42467.patch \ | 110 | file://CVE-2023-42467.patch \ |
| 111 | file://CVE-2023-6683.patch \ | 111 | file://CVE-2023-6683.patch \ |
| 112 | file://CVE-2024-7409-0001.patch \ | ||
| 113 | file://CVE-2024-7409-0002.patch \ | ||
| 114 | file://CVE-2024-7409-0003.patch \ | ||
| 115 | file://CVE-2024-7409-0004.patch \ | ||
| 112 | " | 116 | " |
| 113 | UPSTREAM_CHECK_REGEX = "qemu-(?P<pver>\d+(\.\d+)+)\.tar" | 117 | UPSTREAM_CHECK_REGEX = "qemu-(?P<pver>\d+(\.\d+)+)\.tar" |
| 114 | 118 | ||
diff --git a/meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0001.patch b/meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0001.patch new file mode 100644 index 0000000000..f4dad65097 --- /dev/null +++ b/meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0001.patch | |||
| @@ -0,0 +1,162 @@ | |||
| 1 | From fb1c2aaa981e0a2fa6362c9985f1296b74f055ac Mon Sep 17 00:00:00 2001 | ||
| 2 | From: Eric Blake <eblake@redhat.com> | ||
| 3 | Date: Wed, 7 Aug 2024 08:50:01 -0500 | ||
| 4 | Subject: [PATCH] nbd/server: Plumb in new args to nbd_client_add() | ||
| 5 | |||
| 6 | Upcoming patches to fix a CVE need to track an opaque pointer passed | ||
| 7 | in by the owner of a client object, as well as request for a time | ||
| 8 | limit on how fast negotiation must complete. Prepare for that by | ||
| 9 | changing the signature of nbd_client_new() and adding an accessor to | ||
| 10 | get at the opaque pointer, although for now the two servers | ||
| 11 | (qemu-nbd.c and blockdev-nbd.c) do not change behavior even though | ||
| 12 | they pass in a new default timeout value. | ||
| 13 | |||
| 14 | Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> | ||
| 15 | Signed-off-by: Eric Blake <eblake@redhat.com> | ||
| 16 | Message-ID: <20240807174943.771624-11-eblake@redhat.com> | ||
| 17 | Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> | ||
| 18 | [eblake: s/LIMIT/MAX_SECS/ as suggested by Dan] | ||
| 19 | Signed-off-by: Eric Blake <eblake@redhat.com> | ||
| 20 | |||
| 21 | CVE: CVE-2024-7409 | ||
| 22 | Upstream-Status: Backport [https://github.com/qemu/qemu/commit/fb1c2aaa981e0a2fa6362c9985f1296b74f055ac] | ||
| 23 | Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com> | ||
| 24 | --- | ||
| 25 | blockdev-nbd.c | 6 ++++-- | ||
| 26 | include/block/nbd.h | 11 ++++++++++- | ||
| 27 | nbd/server.c | 20 +++++++++++++++++--- | ||
| 28 | qemu-nbd.c | 4 +++- | ||
| 29 | 4 files changed, 34 insertions(+), 7 deletions(-) | ||
| 30 | |||
| 31 | diff --git a/blockdev-nbd.c b/blockdev-nbd.c | ||
| 32 | index bdfa7ed3a..b9e8dc78f 100644 | ||
| 33 | --- a/blockdev-nbd.c | ||
| 34 | +++ b/blockdev-nbd.c | ||
| 35 | @@ -59,8 +59,10 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc, | ||
| 36 | nbd_update_server_watch(nbd_server); | ||
| 37 | |||
| 38 | qio_channel_set_name(QIO_CHANNEL(cioc), "nbd-server"); | ||
| 39 | - nbd_client_new(cioc, nbd_server->tlscreds, nbd_server->tlsauthz, | ||
| 40 | - nbd_blockdev_client_closed); | ||
| 41 | + /* TODO - expose handshake timeout as QMP option */ | ||
| 42 | + nbd_client_new(cioc, NBD_DEFAULT_HANDSHAKE_MAX_SECS, | ||
| 43 | + nbd_server->tlscreds, nbd_server->tlsauthz, | ||
| 44 | + nbd_blockdev_client_closed, NULL); | ||
| 45 | } | ||
| 46 | |||
| 47 | static void nbd_update_server_watch(NBDServerData *s) | ||
| 48 | diff --git a/include/block/nbd.h b/include/block/nbd.h | ||
| 49 | index 78d101b77..b71a29724 100644 | ||
| 50 | --- a/include/block/nbd.h | ||
| 51 | +++ b/include/block/nbd.h | ||
| 52 | @@ -27,6 +27,12 @@ | ||
| 53 | |||
| 54 | extern const BlockExportDriver blk_exp_nbd; | ||
| 55 | |||
| 56 | +/* | ||
| 57 | + * NBD_DEFAULT_HANDSHAKE_MAX_SECS: Number of seconds in which client must | ||
| 58 | + * succeed at NBD_OPT_GO before being forcefully dropped as too slow. | ||
| 59 | + */ | ||
| 60 | +#define NBD_DEFAULT_HANDSHAKE_MAX_SECS 10 | ||
| 61 | + | ||
| 62 | /* Handshake phase structs - this struct is passed on the wire */ | ||
| 63 | |||
| 64 | struct NBDOption { | ||
| 65 | @@ -338,9 +344,12 @@ AioContext *nbd_export_aio_context(NBDExport *exp); | ||
| 66 | NBDExport *nbd_export_find(const char *name); | ||
| 67 | |||
| 68 | void nbd_client_new(QIOChannelSocket *sioc, | ||
| 69 | + uint32_t handshake_max_secs, | ||
| 70 | QCryptoTLSCreds *tlscreds, | ||
| 71 | const char *tlsauthz, | ||
| 72 | - void (*close_fn)(NBDClient *, bool)); | ||
| 73 | + void (*close_fn)(NBDClient *, bool), | ||
| 74 | + void *owner); | ||
| 75 | +void *nbd_client_owner(NBDClient *client); | ||
| 76 | void nbd_client_get(NBDClient *client); | ||
| 77 | void nbd_client_put(NBDClient *client); | ||
| 78 | |||
| 79 | diff --git a/nbd/server.c b/nbd/server.c | ||
| 80 | index 4630dd732..12680c8dc 100644 | ||
| 81 | --- a/nbd/server.c | ||
| 82 | +++ b/nbd/server.c | ||
| 83 | @@ -121,9 +121,11 @@ struct NBDClient { | ||
| 84 | int refcount; | ||
| 85 | void (*close_fn)(NBDClient *client, bool negotiated); | ||
| 86 | |||
| 87 | + void *owner; | ||
| 88 | NBDExport *exp; | ||
| 89 | QCryptoTLSCreds *tlscreds; | ||
| 90 | char *tlsauthz; | ||
| 91 | + uint32_t handshake_max_secs; | ||
| 92 | QIOChannelSocket *sioc; /* The underlying data channel */ | ||
| 93 | QIOChannel *ioc; /* The current I/O channel which may differ (eg TLS) */ | ||
| 94 | |||
| 95 | @@ -2703,6 +2705,7 @@ static coroutine_fn void nbd_co_client_start(void *opaque) | ||
| 96 | |||
| 97 | qemu_co_mutex_init(&client->send_lock); | ||
| 98 | |||
| 99 | + /* TODO - utilize client->handshake_max_secs */ | ||
| 100 | if (nbd_negotiate(client, &local_err)) { | ||
| 101 | if (local_err) { | ||
| 102 | error_report_err(local_err); | ||
| 103 | @@ -2715,14 +2718,17 @@ static coroutine_fn void nbd_co_client_start(void *opaque) | ||
| 104 | } | ||
| 105 | |||
| 106 | /* | ||
| 107 | - * Create a new client listener using the given channel @sioc. | ||
| 108 | + * Create a new client listener using the given channel @sioc and @owner. | ||
| 109 | * Begin servicing it in a coroutine. When the connection closes, call | ||
| 110 | - * @close_fn with an indication of whether the client completed negotiation. | ||
| 111 | + * @close_fn with an indication of whether the client completed negotiation | ||
| 112 | + * within @handshake_max_secs seconds (0 for unbounded). | ||
| 113 | */ | ||
| 114 | void nbd_client_new(QIOChannelSocket *sioc, | ||
| 115 | + uint32_t handshake_max_secs, | ||
| 116 | QCryptoTLSCreds *tlscreds, | ||
| 117 | const char *tlsauthz, | ||
| 118 | - void (*close_fn)(NBDClient *, bool)) | ||
| 119 | + void (*close_fn)(NBDClient *, bool), | ||
| 120 | + void *owner) | ||
| 121 | { | ||
| 122 | NBDClient *client; | ||
| 123 | Coroutine *co; | ||
| 124 | @@ -2734,12 +2740,20 @@ void nbd_client_new(QIOChannelSocket *sioc, | ||
| 125 | object_ref(OBJECT(client->tlscreds)); | ||
| 126 | } | ||
| 127 | client->tlsauthz = g_strdup(tlsauthz); | ||
| 128 | + client->handshake_max_secs = handshake_max_secs; | ||
| 129 | client->sioc = sioc; | ||
| 130 | object_ref(OBJECT(client->sioc)); | ||
| 131 | client->ioc = QIO_CHANNEL(sioc); | ||
| 132 | object_ref(OBJECT(client->ioc)); | ||
| 133 | client->close_fn = close_fn; | ||
| 134 | + client->owner = owner; | ||
| 135 | |||
| 136 | co = qemu_coroutine_create(nbd_co_client_start, client); | ||
| 137 | qemu_coroutine_enter(co); | ||
| 138 | } | ||
| 139 | + | ||
| 140 | +void * | ||
| 141 | +nbd_client_owner(NBDClient *client) | ||
| 142 | +{ | ||
| 143 | + return client->owner; | ||
| 144 | +} | ||
| 145 | diff --git a/qemu-nbd.c b/qemu-nbd.c | ||
| 146 | index c6c20df68..f48abf379 100644 | ||
| 147 | --- a/qemu-nbd.c | ||
| 148 | +++ b/qemu-nbd.c | ||
| 149 | @@ -363,7 +363,9 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc, | ||
| 150 | |||
| 151 | nb_fds++; | ||
| 152 | nbd_update_server_watch(); | ||
| 153 | - nbd_client_new(cioc, tlscreds, tlsauthz, nbd_client_closed); | ||
| 154 | + /* TODO - expose handshake timeout as command line option */ | ||
| 155 | + nbd_client_new(cioc, NBD_DEFAULT_HANDSHAKE_MAX_SECS, | ||
| 156 | + tlscreds, tlsauthz, nbd_client_closed, NULL); | ||
| 157 | } | ||
| 158 | |||
| 159 | static void nbd_update_server_watch(void) | ||
| 160 | -- | ||
| 161 | 2.25.1 | ||
| 162 | |||
diff --git a/meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0002.patch b/meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0002.patch new file mode 100644 index 0000000000..ccef8b36c5 --- /dev/null +++ b/meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0002.patch | |||
| @@ -0,0 +1,174 @@ | |||
| 1 | From c8a76dbd90c2f48df89b75bef74917f90a59b623 Mon Sep 17 00:00:00 2001 | ||
| 2 | From: Eric Blake <eblake@redhat.com> | ||
| 3 | Date: Tue, 6 Aug 2024 13:53:00 -0500 | ||
| 4 | Subject: [PATCH] nbd/server: CVE-2024-7409: Cap default max-connections to 100 | ||
| 5 | |||
| 6 | Allowing an unlimited number of clients to any web service is a recipe | ||
| 7 | for a rudimentary denial of service attack: the client merely needs to | ||
| 8 | open lots of sockets without closing them, until qemu no longer has | ||
| 9 | any more fds available to allocate. | ||
| 10 | |||
| 11 | For qemu-nbd, we default to allowing only 1 connection unless more are | ||
| 12 | explicitly asked for (-e or --shared); this was historically picked as | ||
| 13 | a nice default (without an explicit -t, a non-persistent qemu-nbd goes | ||
| 14 | away after a client disconnects, without needing any additional | ||
| 15 | follow-up commands), and we are not going to change that interface now | ||
| 16 | (besides, someday we want to point people towards qemu-storage-daemon | ||
| 17 | instead of qemu-nbd). | ||
| 18 | |||
| 19 | But for qemu proper, and the newer qemu-storage-daemon, the QMP | ||
| 20 | nbd-server-start command has historically had a default of unlimited | ||
| 21 | number of connections, in part because unlike qemu-nbd it is | ||
| 22 | inherently persistent until nbd-server-stop. Allowing multiple client | ||
| 23 | sockets is particularly useful for clients that can take advantage of | ||
| 24 | MULTI_CONN (creating parallel sockets to increase throughput), | ||
| 25 | although known clients that do so (such as libnbd's nbdcopy) typically | ||
| 26 | use only 8 or 16 connections (the benefits of scaling diminish once | ||
| 27 | more sockets are competing for kernel attention). Picking a number | ||
| 28 | large enough for typical use cases, but not unlimited, makes it | ||
| 29 | slightly harder for a malicious client to perform a denial of service | ||
| 30 | merely by opening lots of connections withot progressing through the | ||
| 31 | handshake. | ||
| 32 | |||
| 33 | This change does not eliminate CVE-2024-7409 on its own, but reduces | ||
| 34 | the chance for fd exhaustion or unlimited memory usage as an attack | ||
| 35 | surface. On the other hand, by itself, it makes it more obvious that | ||
| 36 | with a finite limit, we have the problem of an unauthenticated client | ||
| 37 | holding 100 fds opened as a way to block out a legitimate client from | ||
| 38 | being able to connect; thus, later patches will further add timeouts | ||
| 39 | to reject clients that are not making progress. | ||
| 40 | |||
| 41 | This is an INTENTIONAL change in behavior, and will break any client | ||
| 42 | of nbd-server-start that was not passing an explicit max-connections | ||
| 43 | parameter, yet expects more than 100 simultaneous connections. We are | ||
| 44 | not aware of any such client (as stated above, most clients aware of | ||
| 45 | MULTI_CONN get by just fine on 8 or 16 connections, and probably cope | ||
| 46 | with later connections failing by relying on the earlier connections; | ||
| 47 | libvirt has not yet been passing max-connections, but generally | ||
| 48 | creates NBD servers with the intent for a single client for the sake | ||
| 49 | of live storage migration; meanwhile, the KubeSAN project anticipates | ||
| 50 | a large cluster sharing multiple clients [up to 8 per node, and up to | ||
| 51 | 100 nodes in a cluster], but it currently uses qemu-nbd with an | ||
| 52 | explicit --shared=0 rather than qemu-storage-daemon with | ||
| 53 | nbd-server-start). | ||
| 54 | |||
| 55 | We considered using a deprecation period (declare that omitting | ||
| 56 | max-parameters is deprecated, and make it mandatory in 3 releases - | ||
| 57 | then we don't need to pick an arbitrary default); that has zero risk | ||
| 58 | of breaking any apps that accidentally depended on more than 100 | ||
| 59 | connections, and where such breakage might not be noticed under unit | ||
| 60 | testing but only under the larger loads of production usage. But it | ||
| 61 | does not close the denial-of-service hole until far into the future, | ||
| 62 | and requires all apps to change to add the parameter even if 100 was | ||
| 63 | good enough. It also has a drawback that any app (like libvirt) that | ||
| 64 | is accidentally relying on an unlimited default should seriously | ||
| 65 | consider their own CVE now, at which point they are going to change to | ||
| 66 | pass explicit max-connections sooner than waiting for 3 qemu releases. | ||
| 67 | Finally, if our changed default breaks an app, that app can always | ||
| 68 | pass in an explicit max-parameters with a larger value. | ||
| 69 | |||
| 70 | It is also intentional that the HMP interface to nbd-server-start is | ||
| 71 | not changed to expose max-connections (any client needing to fine-tune | ||
| 72 | things should be using QMP). | ||
| 73 | |||
| 74 | Suggested-by: Daniel P. Berrangé <berrange@redhat.com> | ||
| 75 | Signed-off-by: Eric Blake <eblake@redhat.com> | ||
| 76 | Message-ID: <20240807174943.771624-12-eblake@redhat.com> | ||
| 77 | Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> | ||
| 78 | [ericb: Expand commit message to summarize Dan's argument for why we | ||
| 79 | break corner-case back-compat behavior without a deprecation period] | ||
| 80 | Signed-off-by: Eric Blake <eblake@redhat.com> | ||
| 81 | |||
| 82 | CVE: CVE-2024-7409 | ||
| 83 | Upstream-Status: Backport [https://github.com/qemu/qemu/commit/c8a76dbd90c2f48df89b75bef74917f90a59b623] | ||
| 84 | Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com> | ||
| 85 | --- | ||
| 86 | block/monitor/block-hmp-cmds.c | 3 ++- | ||
| 87 | blockdev-nbd.c | 8 ++++++++ | ||
| 88 | include/block/nbd.h | 7 +++++++ | ||
| 89 | qapi/block-export.json | 4 ++-- | ||
| 90 | 4 files changed, 19 insertions(+), 3 deletions(-) | ||
| 91 | |||
| 92 | diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c | ||
| 93 | index 2ac4aedff..32a666b5d 100644 | ||
| 94 | --- a/block/monitor/block-hmp-cmds.c | ||
| 95 | +++ b/block/monitor/block-hmp-cmds.c | ||
| 96 | @@ -411,7 +411,8 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict) | ||
| 97 | goto exit; | ||
| 98 | } | ||
| 99 | |||
| 100 | - nbd_server_start(addr, NULL, NULL, 0, &local_err); | ||
| 101 | + nbd_server_start(addr, NULL, NULL, NBD_DEFAULT_MAX_CONNECTIONS, | ||
| 102 | + &local_err); | ||
| 103 | qapi_free_SocketAddress(addr); | ||
| 104 | if (local_err != NULL) { | ||
| 105 | goto exit; | ||
| 106 | diff --git a/blockdev-nbd.c b/blockdev-nbd.c | ||
| 107 | index b9e8dc78f..4bd90bac1 100644 | ||
| 108 | --- a/blockdev-nbd.c | ||
| 109 | +++ b/blockdev-nbd.c | ||
| 110 | @@ -171,6 +171,10 @@ void nbd_server_start(SocketAddress *addr, const char *tls_creds, | ||
| 111 | |||
| 112 | void nbd_server_start_options(NbdServerOptions *arg, Error **errp) | ||
| 113 | { | ||
| 114 | + if (!arg->has_max_connections) { | ||
| 115 | + arg->max_connections = NBD_DEFAULT_MAX_CONNECTIONS; | ||
| 116 | + } | ||
| 117 | + | ||
| 118 | nbd_server_start(arg->addr, arg->tls_creds, arg->tls_authz, | ||
| 119 | arg->max_connections, errp); | ||
| 120 | } | ||
| 121 | @@ -183,6 +187,10 @@ void qmp_nbd_server_start(SocketAddressLegacy *addr, | ||
| 122 | { | ||
| 123 | SocketAddress *addr_flat = socket_address_flatten(addr); | ||
| 124 | |||
| 125 | + if (!has_max_connections) { | ||
| 126 | + max_connections = NBD_DEFAULT_MAX_CONNECTIONS; | ||
| 127 | + } | ||
| 128 | + | ||
| 129 | nbd_server_start(addr_flat, tls_creds, tls_authz, max_connections, errp); | ||
| 130 | qapi_free_SocketAddress(addr_flat); | ||
| 131 | } | ||
| 132 | diff --git a/include/block/nbd.h b/include/block/nbd.h | ||
| 133 | index b71a29724..a31c34a8a 100644 | ||
| 134 | --- a/include/block/nbd.h | ||
| 135 | +++ b/include/block/nbd.h | ||
| 136 | @@ -33,6 +33,13 @@ extern const BlockExportDriver blk_exp_nbd; | ||
| 137 | */ | ||
| 138 | #define NBD_DEFAULT_HANDSHAKE_MAX_SECS 10 | ||
| 139 | |||
| 140 | +/* | ||
| 141 | + * NBD_DEFAULT_MAX_CONNECTIONS: Number of client sockets to allow at | ||
| 142 | + * once; must be large enough to allow a MULTI_CONN-aware client like | ||
| 143 | + * nbdcopy to create its typical number of 8-16 sockets. | ||
| 144 | + */ | ||
| 145 | +#define NBD_DEFAULT_MAX_CONNECTIONS 100 | ||
| 146 | + | ||
| 147 | /* Handshake phase structs - this struct is passed on the wire */ | ||
| 148 | |||
| 149 | struct NBDOption { | ||
| 150 | diff --git a/qapi/block-export.json b/qapi/block-export.json | ||
| 151 | index c1b92ce1c..181d7238f 100644 | ||
| 152 | --- a/qapi/block-export.json | ||
| 153 | +++ b/qapi/block-export.json | ||
| 154 | @@ -21,7 +21,7 @@ | ||
| 155 | # recreated on the fly while the NBD server is active. | ||
| 156 | # If missing, it will default to denying access (since 4.0). | ||
| 157 | # @max-connections: The maximum number of connections to allow at the same | ||
| 158 | -# time, 0 for unlimited. (since 5.2; default: 0) | ||
| 159 | +# time, 0 for unlimited. (since 5.2; default: 100) | ||
| 160 | # | ||
| 161 | # Since: 4.2 | ||
| 162 | ## | ||
| 163 | @@ -50,7 +50,7 @@ | ||
| 164 | # recreated on the fly while the NBD server is active. | ||
| 165 | # If missing, it will default to denying access (since 4.0). | ||
| 166 | # @max-connections: The maximum number of connections to allow at the same | ||
| 167 | -# time, 0 for unlimited. (since 5.2; default: 0) | ||
| 168 | +# time, 0 for unlimited. (since 5.2; default: 100) | ||
| 169 | # | ||
| 170 | # Returns: error if the server is already running. | ||
| 171 | # | ||
| 172 | -- | ||
| 173 | 2.25.1 | ||
| 174 | |||
diff --git a/meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0003.patch b/meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0003.patch new file mode 100644 index 0000000000..1d27f4712c --- /dev/null +++ b/meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0003.patch | |||
| @@ -0,0 +1,122 @@ | |||
| 1 | From b9b72cb3ce15b693148bd09cef7e50110566d8a0 Mon Sep 17 00:00:00 2001 | ||
| 2 | From: Eric Blake <eblake@redhat.com> | ||
| 3 | Date: Thu, 8 Aug 2024 16:05:08 -0500 | ||
| 4 | Subject: [PATCH] nbd/server: CVE-2024-7409: Drop non-negotiating clients | ||
| 5 | |||
| 6 | A client that opens a socket but does not negotiate is merely hogging | ||
| 7 | qemu's resources (an open fd and a small amount of memory); and a | ||
| 8 | malicious client that can access the port where NBD is listening can | ||
| 9 | attempt a denial of service attack by intentionally opening and | ||
| 10 | abandoning lots of unfinished connections. The previous patch put a | ||
| 11 | default bound on the number of such ongoing connections, but once that | ||
| 12 | limit is hit, no more clients can connect (including legitimate ones). | ||
| 13 | The solution is to insist that clients complete handshake within a | ||
| 14 | reasonable time limit, defaulting to 10 seconds. A client that has | ||
| 15 | not successfully completed NBD_OPT_GO by then (including the case of | ||
| 16 | where the client didn't know TLS credentials to even reach the point | ||
| 17 | of NBD_OPT_GO) is wasting our time and does not deserve to stay | ||
| 18 | connected. Later patches will allow fine-tuning the limit away from | ||
| 19 | the default value (including disabling it for doing integration | ||
| 20 | testing of the handshake process itself). | ||
| 21 | |||
| 22 | Note that this patch in isolation actually makes it more likely to see | ||
| 23 | qemu SEGV after nbd-server-stop, as any client socket still connected | ||
| 24 | when the server shuts down will now be closed after 10 seconds rather | ||
| 25 | than at the client's whims. That will be addressed in the next patch. | ||
| 26 | |||
| 27 | For a demo of this patch in action: | ||
| 28 | $ qemu-nbd -f raw -r -t -e 10 file & | ||
| 29 | $ nbdsh --opt-mode -c ' | ||
| 30 | H = list() | ||
| 31 | for i in range(20): | ||
| 32 | print(i) | ||
| 33 | H.insert(i, nbd.NBD()) | ||
| 34 | H[i].set_opt_mode(True) | ||
| 35 | H[i].connect_uri("nbd://localhost") | ||
| 36 | ' | ||
| 37 | $ kill $! | ||
| 38 | |||
| 39 | where later connections get to start progressing once earlier ones are | ||
| 40 | forcefully dropped for taking too long, rather than hanging. | ||
| 41 | |||
| 42 | Suggested-by: Daniel P. Berrangé <berrange@redhat.com> | ||
| 43 | Signed-off-by: Eric Blake <eblake@redhat.com> | ||
| 44 | Message-ID: <20240807174943.771624-13-eblake@redhat.com> | ||
| 45 | Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> | ||
| 46 | [eblake: rebase to changes earlier in series, reduce scope of timer] | ||
| 47 | Signed-off-by: Eric Blake <eblake@redhat.com> | ||
| 48 | |||
| 49 | CVE: CVE-2024-7409 | ||
| 50 | Upstream-Status: Backport [https://gitlab.com/qemu-project/qemu/-/commit/b9b72cb3ce15b693148bd09cef7e50110566d8a0] | ||
| 51 | Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com> | ||
| 52 | --- | ||
| 53 | nbd/server.c | 28 +++++++++++++++++++++++++++- | ||
| 54 | nbd/trace-events | 1 + | ||
| 55 | 2 files changed, 28 insertions(+), 1 deletion(-) | ||
| 56 | |||
| 57 | diff --git a/nbd/server.c b/nbd/server.c | ||
| 58 | index 12680c8dc..1bb253726 100644 | ||
| 59 | --- a/nbd/server.c | ||
| 60 | +++ b/nbd/server.c | ||
| 61 | @@ -2698,22 +2698,48 @@ static void nbd_client_receive_next_request(NBDClient *client) | ||
| 62 | } | ||
| 63 | } | ||
| 64 | |||
| 65 | +static void nbd_handshake_timer_cb(void *opaque) | ||
| 66 | +{ | ||
| 67 | + QIOChannel *ioc = opaque; | ||
| 68 | + | ||
| 69 | + trace_nbd_handshake_timer_cb(); | ||
| 70 | + qio_channel_shutdown(ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL); | ||
| 71 | +} | ||
| 72 | + | ||
| 73 | static coroutine_fn void nbd_co_client_start(void *opaque) | ||
| 74 | { | ||
| 75 | NBDClient *client = opaque; | ||
| 76 | Error *local_err = NULL; | ||
| 77 | + QEMUTimer *handshake_timer = NULL; | ||
| 78 | |||
| 79 | qemu_co_mutex_init(&client->send_lock); | ||
| 80 | |||
| 81 | - /* TODO - utilize client->handshake_max_secs */ | ||
| 82 | + /* | ||
| 83 | + * Create a timer to bound the time spent in negotiation. If the | ||
| 84 | + * timer expires, it is likely nbd_negotiate will fail because the | ||
| 85 | + * socket was shutdown. | ||
| 86 | + */ | ||
| 87 | + if (client->handshake_max_secs > 0) { | ||
| 88 | + handshake_timer = aio_timer_new(qemu_get_aio_context(), | ||
| 89 | + QEMU_CLOCK_REALTIME, | ||
| 90 | + SCALE_NS, | ||
| 91 | + nbd_handshake_timer_cb, | ||
| 92 | + client->sioc); | ||
| 93 | + timer_mod(handshake_timer, | ||
| 94 | + qemu_clock_get_ns(QEMU_CLOCK_REALTIME) + | ||
| 95 | + client->handshake_max_secs * NANOSECONDS_PER_SECOND); | ||
| 96 | + } | ||
| 97 | + | ||
| 98 | if (nbd_negotiate(client, &local_err)) { | ||
| 99 | if (local_err) { | ||
| 100 | error_report_err(local_err); | ||
| 101 | } | ||
| 102 | + timer_free(handshake_timer); | ||
| 103 | client_close(client, false); | ||
| 104 | return; | ||
| 105 | } | ||
| 106 | |||
| 107 | + timer_free(handshake_timer); | ||
| 108 | nbd_client_receive_next_request(client); | ||
| 109 | } | ||
| 110 | |||
| 111 | diff --git a/nbd/trace-events b/nbd/trace-events | ||
| 112 | index c4919a2dd..553546f1f 100644 | ||
| 113 | --- a/nbd/trace-events | ||
| 114 | +++ b/nbd/trace-events | ||
| 115 | @@ -73,3 +73,4 @@ nbd_co_receive_request_decode_type(uint64_t handle, uint16_t type, const char *n | ||
| 116 | nbd_co_receive_request_payload_received(uint64_t handle, uint32_t len) "Payload received: handle = %" PRIu64 ", len = %" PRIu32 | ||
| 117 | nbd_co_receive_align_compliance(const char *op, uint64_t from, uint32_t len, uint32_t align) "client sent non-compliant unaligned %s request: from=0x%" PRIx64 ", len=0x%" PRIx32 ", align=0x%" PRIx32 | ||
| 118 | nbd_trip(void) "Reading request" | ||
| 119 | +nbd_handshake_timer_cb(void) "client took too long to negotiate" | ||
| 120 | -- | ||
| 121 | 2.25.1 | ||
| 122 | |||
diff --git a/meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0004.patch b/meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0004.patch new file mode 100644 index 0000000000..ffdb1b0d94 --- /dev/null +++ b/meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0004.patch | |||
| @@ -0,0 +1,163 @@ | |||
| 1 | From 3e7ef738c8462c45043a1d39f702a0990406a3b3 Mon Sep 17 00:00:00 2001 | ||
| 2 | From: Eric Blake <eblake@redhat.com> | ||
| 3 | Date: Wed, 7 Aug 2024 12:23:13 -0500 | ||
| 4 | Subject: [PATCH] nbd/server: CVE-2024-7409: Close stray clients at server-stop | ||
| 5 | |||
| 6 | A malicious client can attempt to connect to an NBD server, and then | ||
| 7 | intentionally delay progress in the handshake, including if it does | ||
| 8 | not know the TLS secrets. Although the previous two patches reduce | ||
| 9 | this behavior by capping the default max-connections parameter and | ||
| 10 | killing slow clients, they did not eliminate the possibility of a | ||
| 11 | client waiting to close the socket until after the QMP nbd-server-stop | ||
| 12 | command is executed, at which point qemu would SEGV when trying to | ||
| 13 | dereference the NULL nbd_server global which is no longer present. | ||
| 14 | This amounts to a denial of service attack. Worse, if another NBD | ||
| 15 | server is started before the malicious client disconnects, I cannot | ||
| 16 | rule out additional adverse effects when the old client interferes | ||
| 17 | with the connection count of the new server (although the most likely | ||
| 18 | is a crash due to an assertion failure when checking | ||
| 19 | nbd_server->connections > 0). | ||
| 20 | |||
| 21 | For environments without this patch, the CVE can be mitigated by | ||
| 22 | ensuring (such as via a firewall) that only trusted clients can | ||
| 23 | connect to an NBD server. Note that using frameworks like libvirt | ||
| 24 | that ensure that TLS is used and that nbd-server-stop is not executed | ||
| 25 | while any trusted clients are still connected will only help if there | ||
| 26 | is also no possibility for an untrusted client to open a connection | ||
| 27 | but then stall on the NBD handshake. | ||
| 28 | |||
| 29 | Given the previous patches, it would be possible to guarantee that no | ||
| 30 | clients remain connected by having nbd-server-stop sleep for longer | ||
| 31 | than the default handshake deadline before finally freeing the global | ||
| 32 | nbd_server object, but that could make QMP non-responsive for a long | ||
| 33 | time. So intead, this patch fixes the problem by tracking all client | ||
| 34 | sockets opened while the server is running, and forcefully closing any | ||
| 35 | such sockets remaining without a completed handshake at the time of | ||
| 36 | nbd-server-stop, then waiting until the coroutines servicing those | ||
| 37 | sockets notice the state change. nbd-server-stop now has a second | ||
| 38 | AIO_WAIT_WHILE_UNLOCKED (the first is indirectly through the | ||
| 39 | blk_exp_close_all_type() that disconnects all clients that completed | ||
| 40 | handshakes), but forced socket shutdown is enough to progress the | ||
| 41 | coroutines and quickly tear down all clients before the server is | ||
| 42 | freed, thus finally fixing the CVE. | ||
| 43 | |||
| 44 | This patch relies heavily on the fact that nbd/server.c guarantees | ||
| 45 | that it only calls nbd_blockdev_client_closed() from the main loop | ||
| 46 | (see the assertion in nbd_client_put() and the hoops used in | ||
| 47 | nbd_client_put_nonzero() to achieve that); if we did not have that | ||
| 48 | guarantee, we would also need a mutex protecting our accesses of the | ||
| 49 | list of connections to survive re-entrancy from independent iothreads. | ||
| 50 | |||
| 51 | Although I did not actually try to test old builds, it looks like this | ||
| 52 | problem has existed since at least commit 862172f45c (v2.12.0, 2017) - | ||
| 53 | even back when that patch started using a QIONetListener to handle | ||
| 54 | listening on multiple sockets, nbd_server_free() was already unaware | ||
| 55 | that the nbd_blockdev_client_closed callback can be reached later by a | ||
| 56 | client thread that has not completed handshakes (and therefore the | ||
| 57 | client's socket never got added to the list closed in | ||
| 58 | nbd_export_close_all), despite that patch intentionally tearing down | ||
| 59 | the QIONetListener to prevent new clients. | ||
| 60 | |||
| 61 | Reported-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> | ||
| 62 | Fixes: CVE-2024-7409 | ||
| 63 | CC: qemu-stable@nongnu.org | ||
| 64 | Signed-off-by: Eric Blake <eblake@redhat.com> | ||
| 65 | Message-ID: <20240807174943.771624-14-eblake@redhat.com> | ||
| 66 | Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> | ||
| 67 | |||
| 68 | CVE: CVE-2024-7409 | ||
| 69 | Upstream-Status: Backport [https://gitlab.com/qemu-project/qemu/-/commit/3e7ef738c8462c45043a1d39f702a0990406a3b3] | ||
| 70 | Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com> | ||
| 71 | --- | ||
| 72 | blockdev-nbd.c | 35 ++++++++++++++++++++++++++++++++++- | ||
| 73 | 1 file changed, 34 insertions(+), 1 deletion(-) | ||
| 74 | |||
| 75 | diff --git a/blockdev-nbd.c b/blockdev-nbd.c | ||
| 76 | index 4bd90bac1..c71ca38d2 100644 | ||
| 77 | --- a/blockdev-nbd.c | ||
| 78 | +++ b/blockdev-nbd.c | ||
| 79 | @@ -21,12 +21,18 @@ | ||
| 80 | #include "io/channel-socket.h" | ||
| 81 | #include "io/net-listener.h" | ||
| 82 | |||
| 83 | +typedef struct NBDConn { | ||
| 84 | + QIOChannelSocket *cioc; | ||
| 85 | + QLIST_ENTRY(NBDConn) next; | ||
| 86 | +} NBDConn; | ||
| 87 | + | ||
| 88 | typedef struct NBDServerData { | ||
| 89 | QIONetListener *listener; | ||
| 90 | QCryptoTLSCreds *tlscreds; | ||
| 91 | char *tlsauthz; | ||
| 92 | uint32_t max_connections; | ||
| 93 | uint32_t connections; | ||
| 94 | + QLIST_HEAD(, NBDConn) conns; | ||
| 95 | } NBDServerData; | ||
| 96 | |||
| 97 | static NBDServerData *nbd_server; | ||
| 98 | @@ -46,6 +52,14 @@ bool nbd_server_is_running(void) | ||
| 99 | |||
| 100 | static void nbd_blockdev_client_closed(NBDClient *client, bool ignored) | ||
| 101 | { | ||
| 102 | + NBDConn *conn = nbd_client_owner(client); | ||
| 103 | + | ||
| 104 | + assert(qemu_mutex_iothread_locked() && nbd_server); | ||
| 105 | + | ||
| 106 | + object_unref(OBJECT(conn->cioc)); | ||
| 107 | + QLIST_REMOVE(conn, next); | ||
| 108 | + g_free(conn); | ||
| 109 | + | ||
| 110 | nbd_client_put(client); | ||
| 111 | assert(nbd_server->connections > 0); | ||
| 112 | nbd_server->connections--; | ||
| 113 | @@ -55,14 +69,20 @@ static void nbd_blockdev_client_closed(NBDClient *client, bool ignored) | ||
| 114 | static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc, | ||
| 115 | gpointer opaque) | ||
| 116 | { | ||
| 117 | + NBDConn *conn = g_new0(NBDConn, 1); | ||
| 118 | + | ||
| 119 | + assert(qemu_mutex_iothread_locked() && nbd_server); | ||
| 120 | nbd_server->connections++; | ||
| 121 | + object_ref(OBJECT(cioc)); | ||
| 122 | + conn->cioc = cioc; | ||
| 123 | + QLIST_INSERT_HEAD(&nbd_server->conns, conn, next); | ||
| 124 | nbd_update_server_watch(nbd_server); | ||
| 125 | |||
| 126 | qio_channel_set_name(QIO_CHANNEL(cioc), "nbd-server"); | ||
| 127 | /* TODO - expose handshake timeout as QMP option */ | ||
| 128 | nbd_client_new(cioc, NBD_DEFAULT_HANDSHAKE_MAX_SECS, | ||
| 129 | nbd_server->tlscreds, nbd_server->tlsauthz, | ||
| 130 | - nbd_blockdev_client_closed, NULL); | ||
| 131 | + nbd_blockdev_client_closed, conn); | ||
| 132 | } | ||
| 133 | |||
| 134 | static void nbd_update_server_watch(NBDServerData *s) | ||
| 135 | @@ -76,12 +96,25 @@ static void nbd_update_server_watch(NBDServerData *s) | ||
| 136 | |||
| 137 | static void nbd_server_free(NBDServerData *server) | ||
| 138 | { | ||
| 139 | + NBDConn *conn, *tmp; | ||
| 140 | + | ||
| 141 | if (!server) { | ||
| 142 | return; | ||
| 143 | } | ||
| 144 | |||
| 145 | + /* | ||
| 146 | + * Forcefully close the listener socket, and any clients that have | ||
| 147 | + * not yet disconnected on their own. | ||
| 148 | + */ | ||
| 149 | qio_net_listener_disconnect(server->listener); | ||
| 150 | object_unref(OBJECT(server->listener)); | ||
| 151 | + QLIST_FOREACH_SAFE(conn, &server->conns, next, tmp) { | ||
| 152 | + qio_channel_shutdown(QIO_CHANNEL(conn->cioc), QIO_CHANNEL_SHUTDOWN_BOTH, | ||
| 153 | + NULL); | ||
| 154 | + } | ||
| 155 | + | ||
| 156 | + AIO_WAIT_WHILE(NULL, server->connections > 0); | ||
| 157 | + | ||
| 158 | if (server->tlscreds) { | ||
| 159 | object_unref(OBJECT(server->tlscreds)); | ||
| 160 | } | ||
| 161 | -- | ||
| 162 | 2.25.1 | ||
| 163 | |||
