From patchwork Wed Aug 7 17:43:27 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 1970184 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=RzWrNfks; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=patchwork.ozlabs.org) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4WfHmD1wjvz1ydt for ; Thu, 8 Aug 2024 03:51:24 +1000 (AEST) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sbknU-0007Fj-MG; Wed, 07 Aug 2024 13:50:04 -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 1sbknR-00077C-QC for qemu-devel@nongnu.org; Wed, 07 Aug 2024 13:50:02 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sbknP-0000Rl-V4 for qemu-devel@nongnu.org; Wed, 07 Aug 2024 13:50:01 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1723052998; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=nuIGRKa6aa188HkbqJ/CgjfL046BU5hWeK0P66oLYGY=; b=RzWrNfks2mEW/HNkO5ylH7x4aUX1WH++7/GPL2dLFS/Vh7K2HnBRXXBsECVfkTgM2XqXVf QVp1pu7KyqNCTD59LLrUz7IgLp8f4IEQyUFEhQyMi5ExSbEc5bim+0upnGtkXmEB2h9HcD 7lwbNvNzMtzM5wvr0DFlUcteQ2tMG08= Received: from mx-prod-mc-04.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-3-Cr0tuU8FNUiEQrzRML_8kA-1; Wed, 07 Aug 2024 13:49:54 -0400 X-MC-Unique: Cr0tuU8FNUiEQrzRML_8kA-1 Received: from mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 714961955D44; Wed, 7 Aug 2024 17:49:52 +0000 (UTC) Received: from green.redhat.com (unknown [10.2.16.114]) by mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 15B57300018D; Wed, 7 Aug 2024 17:49:48 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Cc: kwolf@redhat.com, hreitz@redhat.com, berrange@redhat.com, qemu-block@nongnu.org, den@virtuozzo.com, andrey.drobyshev@virtuozzo.com, alexander.ivanov@virtuozzo.com, vsementsov@yandex-team.ru Subject: [PATCH v4 1/7] nbd: Minor style fixes Date: Wed, 7 Aug 2024 12:43:27 -0500 Message-ID: <20240807174943.771624-10-eblake@redhat.com> In-Reply-To: <20240807174943.771624-9-eblake@redhat.com> References: <20240807174943.771624-9-eblake@redhat.com> MIME-Version: 1.0 Content-type: text/plain X-Scanned-By: MIMEDefang 3.4.1 on 10.30.177.4 Received-SPF: pass client-ip=170.10.129.124; envelope-from=eblake@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -21 X-Spam_score: -2.2 X-Spam_bar: -- X-Spam_report: (-2.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.144, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 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+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Touch up a comment with the wrong type name, and an over-long line, both noticed while working on upcoming patches. Signed-off-by: Eric Blake Reviewed-by: Daniel P. Berrangé --- nbd/server.c | 2 +- qemu-nbd.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/nbd/server.c b/nbd/server.c index 892797bb111..ecd9366ba64 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -1972,7 +1972,7 @@ static void nbd_export_request_shutdown(BlockExport *blk_exp) blk_exp_ref(&exp->common); /* - * TODO: Should we expand QMP NbdServerRemoveNode enum to allow a + * TODO: Should we expand QMP BlockExportRemoveMode enum to allow a * close mode that stops advertising the export to new clients but * still permits existing clients to run to completion? Because of * that possibility, nbd_export_close() can be called more than diff --git a/qemu-nbd.c b/qemu-nbd.c index d7b3ccab21c..8e104ef22c3 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -588,7 +588,8 @@ int main(int argc, char **argv) pthread_t client_thread; const char *fmt = NULL; Error *local_err = NULL; - BlockdevDetectZeroesOptions detect_zeroes = BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF; + BlockdevDetectZeroesOptions detect_zeroes = + BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF; QDict *options = NULL; const char *export_name = NULL; /* defaults to "" later for server mode */ const char *export_description = NULL; From patchwork Wed Aug 7 17:43:28 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 1970180 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=eg38c6MA; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=patchwork.ozlabs.org) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4WfHlT34tJz1ydt for ; Thu, 8 Aug 2024 03:50:45 +1000 (AEST) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sbknR-00076s-NG; Wed, 07 Aug 2024 13:50:01 -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 1sbknP-000724-R7 for qemu-devel@nongnu.org; Wed, 07 Aug 2024 13:49:59 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sbknO-0000RV-0M for qemu-devel@nongnu.org; Wed, 07 Aug 2024 13:49:59 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1723052997; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=8xrc1r3NaGmUxA1XHAispy7DXme7ap+ScyWyMJR1bS8=; b=eg38c6MANajuhv1oQ93VqZDzQcKeaUdfEqCXrl3Z1AhtAlE0o1axYt+BRsBRT6SJfTwR5i YKDCiXBh7FC8VWpWjg+uc0ww3Kp5HZr8l0nhmM7bo0LbRlOL/T6g0+KTLtRNSMp6l09WxW 8mVogx6zuCi/oQf2LeTKLGxlD7W5/Dk= Received: from mx-prod-mc-03.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-284-UCfuqWf9MRSlIyAqQChiKA-1; Wed, 07 Aug 2024 13:49:56 -0400 X-MC-Unique: UCfuqWf9MRSlIyAqQChiKA-1 Received: from mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id CE06C1955F38; Wed, 7 Aug 2024 17:49:54 +0000 (UTC) Received: from green.redhat.com (unknown [10.2.16.114]) by mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id D2AD2300018D; Wed, 7 Aug 2024 17:49:52 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Cc: kwolf@redhat.com, hreitz@redhat.com, berrange@redhat.com, qemu-block@nongnu.org, den@virtuozzo.com, andrey.drobyshev@virtuozzo.com, alexander.ivanov@virtuozzo.com, vsementsov@yandex-team.ru Subject: [PATCH v4 2/7] nbd/server: Plumb in new args to nbd_client_add() Date: Wed, 7 Aug 2024 12:43:28 -0500 Message-ID: <20240807174943.771624-11-eblake@redhat.com> In-Reply-To: <20240807174943.771624-9-eblake@redhat.com> References: <20240807174943.771624-9-eblake@redhat.com> MIME-Version: 1.0 Content-type: text/plain X-Scanned-By: MIMEDefang 3.4.1 on 10.30.177.4 Received-SPF: pass client-ip=170.10.133.124; envelope-from=eblake@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -21 X-Spam_score: -2.2 X-Spam_bar: -- X-Spam_report: (-2.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.144, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 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+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Upcoming patches to fix a CVE need to track an opaque pointer passed in by the owner of a client object, as well as reequest for a time limit on how fast negotiation must complete. Prepare for that by changing the signature of nbd_client_new() and adding an accessor to get at the opaque pointer, although for now the two servers (qemu-nbd.c and blockdev-nbd.c) do not change behavior. Suggested-by: Vladimir Sementsov-Ogievskiy Signed-off-by: Eric Blake Reviewed-by: Daniel P. Berrangé --- include/block/nbd.h | 11 ++++++++++- blockdev-nbd.c | 6 ++++-- nbd/server.c | 20 +++++++++++++++++--- qemu-nbd.c | 4 +++- 4 files changed, 34 insertions(+), 7 deletions(-) diff --git a/include/block/nbd.h b/include/block/nbd.h index 4e7bd6342f9..5fe14786414 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -33,6 +33,12 @@ typedef struct NBDMetaContexts NBDMetaContexts; extern const BlockExportDriver blk_exp_nbd; +/* + * NBD_DEFAULT_HANDSHAKE_LIMIT: Number of seconds in which client must + * succeed at NBD_OPT_GO before being forcefully dropped as too slow. + */ +#define NBD_DEFAULT_HANDSHAKE_LIMIT 10 + /* Handshake phase structs - this struct is passed on the wire */ typedef struct NBDOption { @@ -403,9 +409,12 @@ AioContext *nbd_export_aio_context(NBDExport *exp); NBDExport *nbd_export_find(const char *name); void nbd_client_new(QIOChannelSocket *sioc, + uint32_t handshake_limit, QCryptoTLSCreds *tlscreds, const char *tlsauthz, - void (*close_fn)(NBDClient *, bool)); + void (*close_fn)(NBDClient *, bool), + void *owner); +void *nbd_client_owner(NBDClient *client); void nbd_client_get(NBDClient *client); void nbd_client_put(NBDClient *client); diff --git a/blockdev-nbd.c b/blockdev-nbd.c index 213012435f4..11f878b6db3 100644 --- a/blockdev-nbd.c +++ b/blockdev-nbd.c @@ -64,8 +64,10 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc, nbd_update_server_watch(nbd_server); qio_channel_set_name(QIO_CHANNEL(cioc), "nbd-server"); - nbd_client_new(cioc, nbd_server->tlscreds, nbd_server->tlsauthz, - nbd_blockdev_client_closed); + /* TODO - expose handshake limit as QMP option */ + nbd_client_new(cioc, NBD_DEFAULT_HANDSHAKE_LIMIT, + nbd_server->tlscreds, nbd_server->tlsauthz, + nbd_blockdev_client_closed, NULL); } static void nbd_update_server_watch(NBDServerData *s) diff --git a/nbd/server.c b/nbd/server.c index ecd9366ba64..31b77bf0d4f 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -124,12 +124,14 @@ struct NBDMetaContexts { struct NBDClient { int refcount; /* atomic */ void (*close_fn)(NBDClient *client, bool negotiated); + void *owner; QemuMutex lock; NBDExport *exp; QCryptoTLSCreds *tlscreds; char *tlsauthz; + uint32_t handshake_limit; QIOChannelSocket *sioc; /* The underlying data channel */ QIOChannel *ioc; /* The current I/O channel which may differ (eg TLS) */ @@ -3191,6 +3193,7 @@ static coroutine_fn void nbd_co_client_start(void *opaque) qemu_co_mutex_init(&client->send_lock); + /* TODO - utilize client->handshake_limit */ if (nbd_negotiate(client, &local_err)) { if (local_err) { error_report_err(local_err); @@ -3205,14 +3208,17 @@ static coroutine_fn void nbd_co_client_start(void *opaque) } /* - * Create a new client listener using the given channel @sioc. + * Create a new client listener using the given channel @sioc and @owner. * Begin servicing it in a coroutine. When the connection closes, call - * @close_fn with an indication of whether the client completed negotiation. + * @close_fn with an indication of whether the client completed negotiation + * within @handshake_limit seconds (0 for unbounded). */ void nbd_client_new(QIOChannelSocket *sioc, + uint32_t handshake_limit, QCryptoTLSCreds *tlscreds, const char *tlsauthz, - void (*close_fn)(NBDClient *, bool)) + void (*close_fn)(NBDClient *, bool), + void *owner) { NBDClient *client; Coroutine *co; @@ -3225,13 +3231,21 @@ void nbd_client_new(QIOChannelSocket *sioc, object_ref(OBJECT(client->tlscreds)); } client->tlsauthz = g_strdup(tlsauthz); + client->handshake_limit = handshake_limit; client->sioc = sioc; qio_channel_set_delay(QIO_CHANNEL(sioc), false); object_ref(OBJECT(client->sioc)); client->ioc = QIO_CHANNEL(sioc); object_ref(OBJECT(client->ioc)); client->close_fn = close_fn; + client->owner = owner; co = qemu_coroutine_create(nbd_co_client_start, client); qemu_coroutine_enter(co); } + +void * +nbd_client_owner(NBDClient *client) +{ + return client->owner; +} diff --git a/qemu-nbd.c b/qemu-nbd.c index 8e104ef22c3..7bf86a6566b 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -390,7 +390,9 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc, nb_fds++; nbd_update_server_watch(); - nbd_client_new(cioc, tlscreds, tlsauthz, nbd_client_closed); + /* TODO - expose handshake limit as command line option */ + nbd_client_new(cioc, NBD_DEFAULT_HANDSHAKE_LIMIT, + tlscreds, tlsauthz, nbd_client_closed, NULL); } static void nbd_update_server_watch(void) From patchwork Wed Aug 7 17:43:29 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 1970182 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=FQrnnV5I; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=patchwork.ozlabs.org) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4WfHll2DJLz1ydt for ; Thu, 8 Aug 2024 03:50:59 +1000 (AEST) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sbkna-0007a1-7f; Wed, 07 Aug 2024 13:50:10 -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 1sbknX-0007Po-4k for qemu-devel@nongnu.org; Wed, 07 Aug 2024 13:50:07 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sbknT-0000Sf-TV for qemu-devel@nongnu.org; Wed, 07 Aug 2024 13:50:06 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1723053002; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=WQK01Bifiek87mMyVcAtTTYs9fgskkmzBNhnR63Cn9w=; b=FQrnnV5I6c/wjQID06won+f3GUvkUBCKlAUuscfIIup39YMTTb1M+6eyhxpYKb8qhL/B4B h/e6g/a6yRo3bLsKsTYfSOCwIcYhuKp6avrNRIt0UdwBN+26sRLlDBJL/JfnvBm+fkE5zv 3xhPb4XanVFxtkBcEi00bShXg69JKc4= Received: from mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-475-XX7cXEvIOci2zXH9V0i9Nw-1; Wed, 07 Aug 2024 13:49:58 -0400 X-MC-Unique: XX7cXEvIOci2zXH9V0i9Nw-1 Received: from mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 82AAE19560A3; Wed, 7 Aug 2024 17:49:57 +0000 (UTC) Received: from green.redhat.com (unknown [10.2.16.114]) by mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 3E7E4300018D; Wed, 7 Aug 2024 17:49:55 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Cc: kwolf@redhat.com, hreitz@redhat.com, berrange@redhat.com, qemu-block@nongnu.org, den@virtuozzo.com, andrey.drobyshev@virtuozzo.com, alexander.ivanov@virtuozzo.com, vsementsov@yandex-team.ru, Markus Armbruster Subject: [PATCH v4 3/7] nbd/server: CVE-2024-7409: Change default max-connections to 100 Date: Wed, 7 Aug 2024 12:43:29 -0500 Message-ID: <20240807174943.771624-12-eblake@redhat.com> In-Reply-To: <20240807174943.771624-9-eblake@redhat.com> References: <20240807174943.771624-9-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.30.177.4 Received-SPF: pass client-ip=170.10.129.124; envelope-from=eblake@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -21 X-Spam_score: -2.2 X-Spam_bar: -- X-Spam_report: (-2.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.144, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 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+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Allowing an unlimited number of clients to any web service is a recipe for a rudimentary denial of service attack: the client merely needs to open lots of sockets without closing them, until qemu no longer has any more fds available to allocate. For qemu-nbd, we default to allowing only 1 connection unless more are explicitly asked for (-e or --shared); this was historically picked as a nice default (without an explicit -t, a non-persistent qemu-nbd goes away after a client disconnects, without needing any additional follow-up commands), and we are not going to change that interface now (besides, someday we want to point people towards qemu-storage-daemon instead of qemu-nbd). But for qemu proper, the QMP nbd-server-start command has historically had a default of unlimited number of connections, in part because unlike qemu-nbd it is inherently persistent. Allowing multiple client sockets is particularly useful for clients that can take advantage of MULTI_CONN (creating parallel sockets to increase throughput), although known clients that do so (such as libnbd's nbdcopy) typically use only 8 or 16 connections (the benefits of scaling diminish once more sockets are competing for kernel attention). Picking a number large enough for typical use cases, but not unlimited, makes it slightly harder for a malicious client to perform a denial of service merely by opening lots of connections withot progressing through the handshake. This change does not eliminate CVE-2024-7409 on its own, but reduces the chance for fd exhaustion or unlimited memory usage as an attack surface. On the other hand, by itself, it makes it more obvious that with a finite limit, we have the problem of an unauthenticated client holding 100 fds opened as a way to block out a legitimate client from being able to connect; thus, later patches will further add timeouts to reject clients that are not making progress. Suggested-by: Daniel P. Berrangé Signed-off-by: Eric Blake Reviewed-by: Daniel P. Berrangé --- qapi/block-export.json | 4 ++-- include/block/nbd.h | 7 +++++++ block/monitor/block-hmp-cmds.c | 3 ++- blockdev-nbd.c | 8 ++++++++ 4 files changed, 19 insertions(+), 3 deletions(-) diff --git a/qapi/block-export.json b/qapi/block-export.json index 665d5fd0262..ce33fe378df 100644 --- a/qapi/block-export.json +++ b/qapi/block-export.json @@ -28,7 +28,7 @@ # @max-connections: The maximum number of connections to allow at the # same time, 0 for unlimited. Setting this to 1 also stops the # server from advertising multiple client support (since 5.2; -# default: 0) +# default: 100) # # Since: 4.2 ## @@ -63,7 +63,7 @@ # @max-connections: The maximum number of connections to allow at the # same time, 0 for unlimited. Setting this to 1 also stops the # server from advertising multiple client support (since 5.2; -# default: 0). +# default: 100). # # Errors: # - if the server is already running diff --git a/include/block/nbd.h b/include/block/nbd.h index 5fe14786414..fd5044359dc 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -39,6 +39,13 @@ extern const BlockExportDriver blk_exp_nbd; */ #define NBD_DEFAULT_HANDSHAKE_LIMIT 10 +/* + * NBD_DEFAULT_MAX_CONNECTIONS: Number of client sockets to allow at + * once; must be large enough to allow a MULTI_CONN-aware client like + * nbdcopy to create its typical number of 8-16 sockets. + */ +#define NBD_DEFAULT_MAX_CONNECTIONS 100 + /* Handshake phase structs - this struct is passed on the wire */ typedef struct NBDOption { diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c index d954bec6f1e..bdf2eb50b68 100644 --- a/block/monitor/block-hmp-cmds.c +++ b/block/monitor/block-hmp-cmds.c @@ -402,7 +402,8 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict) goto exit; } - nbd_server_start(addr, NULL, NULL, 0, &local_err); + nbd_server_start(addr, NULL, NULL, NBD_DEFAULT_MAX_CONNECTIONS, + &local_err); qapi_free_SocketAddress(addr); if (local_err != NULL) { goto exit; diff --git a/blockdev-nbd.c b/blockdev-nbd.c index 11f878b6db3..19c57897819 100644 --- a/blockdev-nbd.c +++ b/blockdev-nbd.c @@ -170,6 +170,10 @@ void nbd_server_start(SocketAddress *addr, const char *tls_creds, void nbd_server_start_options(NbdServerOptions *arg, Error **errp) { + if (!arg->has_max_connections) { + arg->max_connections = NBD_DEFAULT_MAX_CONNECTIONS; + } + nbd_server_start(arg->addr, arg->tls_creds, arg->tls_authz, arg->max_connections, errp); } @@ -182,6 +186,10 @@ void qmp_nbd_server_start(SocketAddressLegacy *addr, { SocketAddress *addr_flat = socket_address_flatten(addr); + if (!has_max_connections) { + max_connections = NBD_DEFAULT_MAX_CONNECTIONS; + } + nbd_server_start(addr_flat, tls_creds, tls_authz, max_connections, errp); qapi_free_SocketAddress(addr_flat); } From patchwork Wed Aug 7 17:43:30 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 1970187 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=hiAzxwtT; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=patchwork.ozlabs.org) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4WfHn80bMyz1ydt for ; Thu, 8 Aug 2024 03:52:12 +1000 (AEST) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sbknd-0007iP-3X; Wed, 07 Aug 2024 13:50:13 -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 1sbknb-0007cb-Ev for qemu-devel@nongnu.org; Wed, 07 Aug 2024 13:50:11 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sbknY-0000ec-W6 for qemu-devel@nongnu.org; Wed, 07 Aug 2024 13:50:11 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1723053006; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=mSsShYzPlJI57BAyKKLWDzA6RZk1TkPdg3ohuGwM5oU=; b=hiAzxwtT2bnubVoCa3CqkerokrGwFVRmFhMWs9jYvecqHG8tf3X8gyqP2eRC3CpjwNYsXP oEzA9DuWaoFww2NkyXawi1d5K91jgm9L33FP+qYyHcK1E5W5ooghO/2tw30lQNlAlt8+kt 76gzthz87+PSoXoEuvriEzlq5RA5/pg= Received: from mx-prod-mc-03.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-624-ynPJrK0RPsiEhxrKOeFMcw-1; Wed, 07 Aug 2024 13:50:02 -0400 X-MC-Unique: ynPJrK0RPsiEhxrKOeFMcw-1 Received: from mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 6781E19560B6; Wed, 7 Aug 2024 17:50:01 +0000 (UTC) Received: from green.redhat.com (unknown [10.2.16.114]) by mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 4F0A7300018D; Wed, 7 Aug 2024 17:49:58 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Cc: kwolf@redhat.com, hreitz@redhat.com, berrange@redhat.com, qemu-block@nongnu.org, den@virtuozzo.com, andrey.drobyshev@virtuozzo.com, alexander.ivanov@virtuozzo.com, vsementsov@yandex-team.ru Subject: [PATCH v4 4/7] nbd/server: CVE-2024-7409: Drop non-negotiating clients Date: Wed, 7 Aug 2024 12:43:30 -0500 Message-ID: <20240807174943.771624-13-eblake@redhat.com> In-Reply-To: <20240807174943.771624-9-eblake@redhat.com> References: <20240807174943.771624-9-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.30.177.4 Received-SPF: pass client-ip=170.10.133.124; envelope-from=eblake@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -21 X-Spam_score: -2.2 X-Spam_bar: -- X-Spam_report: (-2.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.144, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 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+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org 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") ' 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é Signed-off-by: Eric Blake Reviewed-by: Daniel P. Berrangé --- nbd/server.c | 31 ++++++++++++++++++++++++++++++- nbd/trace-events | 1 + 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/nbd/server.c b/nbd/server.c index 31b77bf0d4f..a470052d957 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -132,6 +132,7 @@ struct NBDClient { QCryptoTLSCreds *tlscreds; char *tlsauthz; uint32_t handshake_limit; + QEMUTimer *handshake_timer; QIOChannelSocket *sioc; /* The underlying data channel */ QIOChannel *ioc; /* The current I/O channel which may differ (eg TLS) */ @@ -3186,6 +3187,14 @@ 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; @@ -3193,15 +3202,35 @@ static coroutine_fn void nbd_co_client_start(void *opaque) qemu_co_mutex_init(&client->send_lock); - /* TODO - utilize client->handshake_limit */ + /* + * 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. + */ + client->handshake_timer = aio_timer_new(qemu_get_aio_context(), + QEMU_CLOCK_REALTIME, + SCALE_NS, + nbd_handshake_timer_cb, + client->sioc); + if (client->handshake_limit > 0) { + timer_mod(client->handshake_timer, + qemu_clock_get_ns(QEMU_CLOCK_REALTIME) + + client->handshake_limit * NANOSECONDS_PER_SECOND); + } + if (nbd_negotiate(client, &local_err)) { if (local_err) { error_report_err(local_err); } + timer_free(client->handshake_timer); + client->handshake_timer = NULL; client_close(client, false); return; } + timer_free(client->handshake_timer); + client->handshake_timer = NULL; + WITH_QEMU_LOCK_GUARD(&client->lock) { nbd_client_receive_next_request(client); } diff --git a/nbd/trace-events b/nbd/trace-events index 00ae3216a11..cbd0a4ab7e4 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 From patchwork Wed Aug 7 17:43:31 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 1970183 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=EqI4lIPc; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=patchwork.ozlabs.org) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4WfHm81VRrz1ydt for ; Thu, 8 Aug 2024 03:51:20 +1000 (AEST) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sbknf-0007sJ-Cv; Wed, 07 Aug 2024 13:50:15 -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 1sbknd-0007l2-OU for qemu-devel@nongnu.org; Wed, 07 Aug 2024 13:50:13 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sbknb-0000ex-PQ for qemu-devel@nongnu.org; Wed, 07 Aug 2024 13:50:13 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1723053008; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=TpEGtHu3K56nxX32v7quHRWHOdQOFyRF1XKS4TQ5TIQ=; b=EqI4lIPcNNlJfDNlNdX0H86KgSwfBtux8aejtVesf79+Eq/OS0FpUcmMoQHoEGHf+BGF+7 7DXOB3EvWyxuxlgBtgLxIpBcR3RVB2iT7Y8t5DOAV5Z/DRinHbajJaYSEqEkBabId9l2jY U+RRQ8hPC2333TWQNsBTHnIlELxiBeE= Received: from mx-prod-mc-02.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-77-jzofFtsFOBG97PaRYCzlrA-1; Wed, 07 Aug 2024 13:50:04 -0400 X-MC-Unique: jzofFtsFOBG97PaRYCzlrA-1 Received: from mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 71E451955F33; Wed, 7 Aug 2024 17:50:03 +0000 (UTC) Received: from green.redhat.com (unknown [10.2.16.114]) by mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 8AEED300018D; Wed, 7 Aug 2024 17:50:01 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Cc: kwolf@redhat.com, hreitz@redhat.com, berrange@redhat.com, qemu-block@nongnu.org, den@virtuozzo.com, andrey.drobyshev@virtuozzo.com, alexander.ivanov@virtuozzo.com, vsementsov@yandex-team.ru Subject: [PATCH v4 5/7] nbd/server: CVE-2024-7409: Close stray client sockets at shutdown Date: Wed, 7 Aug 2024 12:43:31 -0500 Message-ID: <20240807174943.771624-14-eblake@redhat.com> In-Reply-To: <20240807174943.771624-9-eblake@redhat.com> References: <20240807174943.771624-9-eblake@redhat.com> MIME-Version: 1.0 Content-type: text/plain X-Scanned-By: MIMEDefang 3.4.1 on 10.30.177.4 Received-SPF: pass client-ip=170.10.129.124; envelope-from=eblake@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -21 X-Spam_score: -2.2 X-Spam_bar: -- X-Spam_report: (-2.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.144, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 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+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org A malicious client can attempt to connect to an NBD server, and then intentionally delay progress in the handshake, including if it does not know the TLS secrets. Although this behavior can be bounded by the max-connections parameter, the QMP nbd-server-start currently defaults to unlimited incoming client connections. Worse, if the client waits to close the socket until after the QMP nbd-server-stop command is executed, qemu will then SEGV when trying to dereference the NULL nbd_server global which is no longer present, which amounts to a denial of service attack. If another NBD server is started before the malicious client disconnects, I cannot rule out additional adverse effects when the old client interferes with the connection count of the new server. For environments without this patch, the CVE can be mitigated by ensuring (such as via a firewall) that only trusted clients can connect to an NBD server. Note that using frameworks like libvirt that ensure that TLS is used and that nbd-server-stop is not executed while any trusted clients are still connected will only help if there is also no possibility for an untrusted client to open a connection but then stall on the NBD handshake. Given the previous patches, it would be possible to guarantee that no clients remain connected by having nbd-server-stop sleep for longer than the default handshake deadline before finally freeing the global nbd_server object, but that could make QMP non-responsive for a long time. So intead, this patch fixes the problem by tracking all client sockets opened while the server is running, and forcefully closing any such sockets remaining without a completed handshake at the time of nbd-server-stop, then waiting until the coroutines servicing those sockets notice the state change. nbd-server-stop now has a second AIO_WAIT_WHILE_UNLOCKED (the first is indirectly through the blk_exp_close_all_type() that disconnects all clients that completed handshakes), but forced socket shutdown is enough to progress the coroutines and quickly tear down all clients before the server is freed, thus finally fixing the CVE. This patch relies heavily on the fact that nbd/server.c guarantees that it only calls nbd_blockdev_client_closed() from the main loop (see the assertion in nbd_client_put() and the hoops used in nbd_client_put_nonzero() to achieve that); if we did not have that guarantee, we would also need a mutex protecting our accesses of the list of connections to survive re-entrancy from independent iothreads. Although I did not actually try to test old builds, it looks like this problem has existed since at least commit 862172f45c (v2.12.0, 2017) - even back when that patch started using a QIONetListener to handle listening on multiple sockets, nbd_server_free() was already unaware that the nbd_blockdev_client_closed callback can be reached later by a client thread that has not completed handshakes (and therefore the client's socket never got added to the list closed in nbd_export_close_all), despite that patch intentionally tearing down the QIONetListener to prevent new clients. Reported-by: Alexander Ivanov Fixes: CVE-2024-7409 Signed-off-by: Eric Blake Reviewed-by: Daniel P. Berrangé --- blockdev-nbd.c | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/blockdev-nbd.c b/blockdev-nbd.c index 19c57897819..4e38ff46747 100644 --- a/blockdev-nbd.c +++ b/blockdev-nbd.c @@ -21,12 +21,18 @@ #include "io/channel-socket.h" #include "io/net-listener.h" +typedef struct NBDConn { + QIOChannelSocket *cioc; + QLIST_ENTRY(NBDConn) next; +} NBDConn; + typedef struct NBDServerData { QIONetListener *listener; QCryptoTLSCreds *tlscreds; char *tlsauthz; uint32_t max_connections; uint32_t connections; + QLIST_HEAD(, NBDConn) conns; } NBDServerData; static NBDServerData *nbd_server; @@ -51,6 +57,14 @@ int nbd_server_max_connections(void) static void nbd_blockdev_client_closed(NBDClient *client, bool ignored) { + NBDConn *conn = nbd_client_owner(client); + + assert(qemu_in_main_thread() && nbd_server); + + object_unref(OBJECT(conn->cioc)); + QLIST_REMOVE(conn, next); + g_free(conn); + nbd_client_put(client); assert(nbd_server->connections > 0); nbd_server->connections--; @@ -60,14 +74,20 @@ static void nbd_blockdev_client_closed(NBDClient *client, bool ignored) static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc, gpointer opaque) { + NBDConn *conn = g_new0(NBDConn, 1); + + assert(qemu_in_main_thread() && nbd_server); nbd_server->connections++; + object_ref(OBJECT(cioc)); + conn->cioc = cioc; + QLIST_INSERT_HEAD(&nbd_server->conns, conn, next); nbd_update_server_watch(nbd_server); qio_channel_set_name(QIO_CHANNEL(cioc), "nbd-server"); /* TODO - expose handshake limit as QMP option */ nbd_client_new(cioc, NBD_DEFAULT_HANDSHAKE_LIMIT, nbd_server->tlscreds, nbd_server->tlsauthz, - nbd_blockdev_client_closed, NULL); + nbd_blockdev_client_closed, conn); } static void nbd_update_server_watch(NBDServerData *s) @@ -81,12 +101,25 @@ static void nbd_update_server_watch(NBDServerData *s) static void nbd_server_free(NBDServerData *server) { + NBDConn *conn, *tmp; + if (!server) { return; } + /* + * Forcefully close the listener socket, and any clients that have + * not yet disconnected on their own. + */ qio_net_listener_disconnect(server->listener); object_unref(OBJECT(server->listener)); + QLIST_FOREACH_SAFE(conn, &server->conns, next, tmp) { + qio_channel_shutdown(QIO_CHANNEL(conn->cioc), QIO_CHANNEL_SHUTDOWN_BOTH, + NULL); + } + + AIO_WAIT_WHILE_UNLOCKED(NULL, server->connections > 0); + if (server->tlscreds) { object_unref(OBJECT(server->tlscreds)); } From patchwork Wed Aug 7 17:43:32 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 1970181 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=QMIcBhTP; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=patchwork.ozlabs.org) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4WfHlk095Fz1ydt for ; Thu, 8 Aug 2024 03:50:58 +1000 (AEST) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sbkni-00085H-Dc; Wed, 07 Aug 2024 13:50:18 -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 1sbkne-0007qT-QI for qemu-devel@nongnu.org; Wed, 07 Aug 2024 13:50:14 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sbknc-0000fe-Jy for qemu-devel@nongnu.org; Wed, 07 Aug 2024 13:50:14 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1723053011; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=LwxbBGHFmsZsfH9DS9IhpgS16mhEtSGLb/yQJe19xw4=; b=QMIcBhTPeo49BXQESFfDep5dEzckALytjD4Cm28EU8j9KEWARZ0jRB1C5NT85jjscR0CHl IdtTTErXODGOF0lXHK7wVCGVlE11M8H4IUCT8x4yaKIg2XhqDkTKrVl5ZyAzLihoWbcvyA 4QwNoSDuz7iCZnVlYjxrv/6U5H4RaAw= Received: from mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-615-_45jkO4eOj2CoAN0QRlKog-1; Wed, 07 Aug 2024 13:50:07 -0400 X-MC-Unique: _45jkO4eOj2CoAN0QRlKog-1 Received: from mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 32F951955F49; Wed, 7 Aug 2024 17:50:06 +0000 (UTC) Received: from green.redhat.com (unknown [10.2.16.114]) by mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id B14AD3000197; Wed, 7 Aug 2024 17:50:03 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Cc: kwolf@redhat.com, hreitz@redhat.com, berrange@redhat.com, qemu-block@nongnu.org, den@virtuozzo.com, andrey.drobyshev@virtuozzo.com, alexander.ivanov@virtuozzo.com, vsementsov@yandex-team.ru Subject: [PATCH v4 6/7] qemu-nbd: Allow users to adjust handshake limit Date: Wed, 7 Aug 2024 12:43:32 -0500 Message-ID: <20240807174943.771624-15-eblake@redhat.com> In-Reply-To: <20240807174943.771624-9-eblake@redhat.com> References: <20240807174943.771624-9-eblake@redhat.com> MIME-Version: 1.0 Content-type: text/plain X-Scanned-By: MIMEDefang 3.4.1 on 10.30.177.4 Received-SPF: pass client-ip=170.10.129.124; envelope-from=eblake@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -21 X-Spam_score: -2.2 X-Spam_bar: -- X-Spam_report: (-2.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.144, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 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+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Although defaulting the handshake limit to 10 seconds was a nice QoI change to weed out intentionally slow clients, it can interfere with integration testing done with manual NBD_OPT commands over 'nbdsh --opt-mode'. Expose a command line option to allow the user to alter the timeout away from the default. This option is unlikely to be used in enough scenarios to warrant a short option letter. Signed-off-by: Eric Blake --- I'm not sure if this is 9.1 material. It is a new feature (user-visible command line option) implemented after soft freeze; on the other hand, it allows one to recover the behavior that existed prior to plugging the CVE which may be useful in integration testing. --- docs/tools/qemu-nbd.rst | 5 +++++ qemu-nbd.c | 41 ++++++++++++++++++++++++++--------------- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/docs/tools/qemu-nbd.rst b/docs/tools/qemu-nbd.rst index 329f44d9895..f55c10eb39c 100644 --- a/docs/tools/qemu-nbd.rst +++ b/docs/tools/qemu-nbd.rst @@ -154,6 +154,11 @@ driver options if :option:`--image-opts` is specified. Set the NBD volume export description, as a human-readable string. +.. option:: --handshake-limit=N + + Set the timeout for a client to successfully complete its handshake + to N seconds (default 10), or 0 for no limit. + .. option:: -L, --list Connect as a client and list all details about the exports exposed by diff --git a/qemu-nbd.c b/qemu-nbd.c index 7bf86a6566b..4287a595d92 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -57,19 +57,20 @@ #define HAVE_NBD_DEVICE 0 #endif -#define SOCKET_PATH "/var/lock/qemu-nbd-%s" -#define QEMU_NBD_OPT_CACHE 256 -#define QEMU_NBD_OPT_AIO 257 -#define QEMU_NBD_OPT_DISCARD 258 -#define QEMU_NBD_OPT_DETECT_ZEROES 259 -#define QEMU_NBD_OPT_OBJECT 260 -#define QEMU_NBD_OPT_TLSCREDS 261 -#define QEMU_NBD_OPT_IMAGE_OPTS 262 -#define QEMU_NBD_OPT_FORK 263 -#define QEMU_NBD_OPT_TLSAUTHZ 264 -#define QEMU_NBD_OPT_PID_FILE 265 -#define QEMU_NBD_OPT_SELINUX_LABEL 266 -#define QEMU_NBD_OPT_TLSHOSTNAME 267 +#define SOCKET_PATH "/var/lock/qemu-nbd-%s" +#define QEMU_NBD_OPT_CACHE 256 +#define QEMU_NBD_OPT_AIO 257 +#define QEMU_NBD_OPT_DISCARD 258 +#define QEMU_NBD_OPT_DETECT_ZEROES 259 +#define QEMU_NBD_OPT_OBJECT 260 +#define QEMU_NBD_OPT_TLSCREDS 261 +#define QEMU_NBD_OPT_IMAGE_OPTS 262 +#define QEMU_NBD_OPT_FORK 263 +#define QEMU_NBD_OPT_TLSAUTHZ 264 +#define QEMU_NBD_OPT_PID_FILE 265 +#define QEMU_NBD_OPT_SELINUX_LABEL 266 +#define QEMU_NBD_OPT_TLSHOSTNAME 267 +#define QEMU_NBD_OPT_HANDSHAKE_LIMIT 268 #define MBR_SIZE 512 @@ -80,6 +81,7 @@ static int nb_fds; static QIONetListener *server; static QCryptoTLSCreds *tlscreds; static const char *tlsauthz; +static int handshake_limit = NBD_DEFAULT_HANDSHAKE_LIMIT; static void usage(const char *name) { @@ -101,6 +103,7 @@ static void usage(const char *name) " -v, --verbose display extra debugging information\n" " -x, --export-name=NAME expose export by name (default is empty string)\n" " -D, --description=TEXT export a human-readable description\n" +" --handshake-limit=N limit client's handshake to N seconds (default 10)\n" "\n" "Exposing part of the image:\n" " -o, --offset=OFFSET offset into the image\n" @@ -390,8 +393,7 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc, nb_fds++; nbd_update_server_watch(); - /* TODO - expose handshake limit as command line option */ - nbd_client_new(cioc, NBD_DEFAULT_HANDSHAKE_LIMIT, + nbd_client_new(cioc, handshake_limit, tlscreds, tlsauthz, nbd_client_closed, NULL); } @@ -569,6 +571,8 @@ int main(int argc, char **argv) { "object", required_argument, NULL, QEMU_NBD_OPT_OBJECT }, { "export-name", required_argument, NULL, 'x' }, { "description", required_argument, NULL, 'D' }, + { "handshake-limit", required_argument, NULL, + QEMU_NBD_OPT_HANDSHAKE_LIMIT }, { "tls-creds", required_argument, NULL, QEMU_NBD_OPT_TLSCREDS }, { "tls-hostname", required_argument, NULL, QEMU_NBD_OPT_TLSHOSTNAME }, { "tls-authz", required_argument, NULL, QEMU_NBD_OPT_TLSAUTHZ }, @@ -815,6 +819,13 @@ int main(int argc, char **argv) case QEMU_NBD_OPT_SELINUX_LABEL: selinux_label = optarg; break; + case QEMU_NBD_OPT_HANDSHAKE_LIMIT: + if (qemu_strtoi(optarg, NULL, 0, &handshake_limit) < 0 || + handshake_limit < 0) { + error_report("Invalid handshake limit '%s'", optarg); + exit(EXIT_FAILURE); + } + break; } } From patchwork Wed Aug 7 17:43:33 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 1970185 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=jGz3I8mC; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=patchwork.ozlabs.org) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4WfHmQ49Hfz1ydt for ; Thu, 8 Aug 2024 03:51:34 +1000 (AEST) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sbknj-000899-4Q; Wed, 07 Aug 2024 13:50:19 -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 1sbknf-0007ui-QV for qemu-devel@nongnu.org; Wed, 07 Aug 2024 13:50:15 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sbknd-0000g5-Tf for qemu-devel@nongnu.org; Wed, 07 Aug 2024 13:50:15 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1723053013; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=vztq1CNCWY30LcnqqakMkAq58CmWNubW9CXcUKXabaQ=; b=jGz3I8mCVYUb8QFHps9i+TD1x35EUGneJgr2uv4YlRRNzW4tND4OtMld4nySOel9L9kYsn otANQj0YLc60j59+V1n8yBGOGyw4mwoi2zm2lL2fe4TIcaY/00cJqbxQ8rL2yrBHDUd5U0 JtHHrnEtiiVbbvtgzy6Bo8PNnxK3OMk= Received: from mx-prod-mc-02.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-412-FYQOZm2IO3Sp0HSXB_kAvA-1; Wed, 07 Aug 2024 13:50:10 -0400 X-MC-Unique: FYQOZm2IO3Sp0HSXB_kAvA-1 Received: from mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id A82221955F3D; Wed, 7 Aug 2024 17:50:08 +0000 (UTC) Received: from green.redhat.com (unknown [10.2.16.114]) by mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 8DA40300018D; Wed, 7 Aug 2024 17:50:06 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Cc: kwolf@redhat.com, hreitz@redhat.com, berrange@redhat.com, qemu-block@nongnu.org, den@virtuozzo.com, andrey.drobyshev@virtuozzo.com, alexander.ivanov@virtuozzo.com, vsementsov@yandex-team.ru, Markus Armbruster Subject: [PATCH v4 7/7] nbd/server: Allow users to adjust handshake limit in QMP Date: Wed, 7 Aug 2024 12:43:33 -0500 Message-ID: <20240807174943.771624-16-eblake@redhat.com> In-Reply-To: <20240807174943.771624-9-eblake@redhat.com> References: <20240807174943.771624-9-eblake@redhat.com> MIME-Version: 1.0 Content-type: text/plain X-Scanned-By: MIMEDefang 3.4.1 on 10.30.177.4 Received-SPF: pass client-ip=170.10.133.124; envelope-from=eblake@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -21 X-Spam_score: -2.2 X-Spam_bar: -- X-Spam_report: (-2.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.144, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 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+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Although defaulting the handshake limit to 10 seconds was a nice QoI change to weed out intentionally slow clients, it can interfere with integration testing done with manual NBD_OPT commands over 'nbdsh --opt-mode'. Expose a QMP knob to allow the user to alter the timeout away from the default. Signed-off-by: Eric Blake --- I'm not sure if this is 9.1 material. It is a new feature (user-visible QMP addition) implemented after soft freeze; on the other hand, it allows one to recover the behavior that existed prior to plugging the CVE which may be useful in integration testing. --- qapi/block-export.json | 14 ++++++++++++-- include/block/nbd.h | 2 +- block/monitor/block-hmp-cmds.c | 2 +- blockdev-nbd.c | 19 ++++++++++++++----- 4 files changed, 28 insertions(+), 9 deletions(-) diff --git a/qapi/block-export.json b/qapi/block-export.json index ce33fe378df..b485b380f1a 100644 --- a/qapi/block-export.json +++ b/qapi/block-export.json @@ -30,13 +30,18 @@ # server from advertising multiple client support (since 5.2; # default: 100) # +# @handshake-limit: Time limit, in seconds, at which a client that +# has not completed the negotiation handshake will be disconnected, +# or 0 for no limit (since 9.1; default: 10). +# # Since: 4.2 ## { 'struct': 'NbdServerOptions', 'data': { 'addr': 'SocketAddress', '*tls-creds': 'str', '*tls-authz': 'str', - '*max-connections': 'uint32' } } + '*max-connections': 'uint32', + '*handshake-limit': 'uint32' } } ## # @nbd-server-start: @@ -65,6 +70,10 @@ # server from advertising multiple client support (since 5.2; # default: 100). # +# @handshake-limit: Time limit, in seconds, at which a client that +# has not completed the negotiation handshake will be disconnected, +# or 0 for no limit (since 9.1; default: 10). +# # Errors: # - if the server is already running # @@ -74,7 +83,8 @@ 'data': { 'addr': 'SocketAddressLegacy', '*tls-creds': 'str', '*tls-authz': 'str', - '*max-connections': 'uint32' }, + '*max-connections': 'uint32', + '*handshake-limit': 'uint32' }, 'allow-preconfig': true } ## diff --git a/include/block/nbd.h b/include/block/nbd.h index fd5044359dc..22071aea797 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -430,7 +430,7 @@ bool nbd_server_is_running(void); int nbd_server_max_connections(void); void nbd_server_start(SocketAddress *addr, const char *tls_creds, const char *tls_authz, uint32_t max_connections, - Error **errp); + uint32_t handshake_limit, Error **errp); void nbd_server_start_options(NbdServerOptions *arg, Error **errp); /* nbd_read diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c index bdf2eb50b68..a258e029f78 100644 --- a/block/monitor/block-hmp-cmds.c +++ b/block/monitor/block-hmp-cmds.c @@ -403,7 +403,7 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict) } nbd_server_start(addr, NULL, NULL, NBD_DEFAULT_MAX_CONNECTIONS, - &local_err); + NBD_DEFAULT_HANDSHAKE_LIMIT, &local_err); qapi_free_SocketAddress(addr); if (local_err != NULL) { goto exit; diff --git a/blockdev-nbd.c b/blockdev-nbd.c index 4e38ff46747..ad21bd1412d 100644 --- a/blockdev-nbd.c +++ b/blockdev-nbd.c @@ -30,6 +30,7 @@ typedef struct NBDServerData { QIONetListener *listener; QCryptoTLSCreds *tlscreds; char *tlsauthz; + uint32_t handshake_limit; uint32_t max_connections; uint32_t connections; QLIST_HEAD(, NBDConn) conns; @@ -84,8 +85,7 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc, nbd_update_server_watch(nbd_server); qio_channel_set_name(QIO_CHANNEL(cioc), "nbd-server"); - /* TODO - expose handshake limit as QMP option */ - nbd_client_new(cioc, NBD_DEFAULT_HANDSHAKE_LIMIT, + nbd_client_new(cioc, nbd_server->handshake_limit, nbd_server->tlscreds, nbd_server->tlsauthz, nbd_blockdev_client_closed, conn); } @@ -160,7 +160,7 @@ static QCryptoTLSCreds *nbd_get_tls_creds(const char *id, Error **errp) void nbd_server_start(SocketAddress *addr, const char *tls_creds, const char *tls_authz, uint32_t max_connections, - Error **errp) + uint32_t handshake_limit, Error **errp) { if (nbd_server) { error_setg(errp, "NBD server already running"); @@ -169,6 +169,7 @@ void nbd_server_start(SocketAddress *addr, const char *tls_creds, nbd_server = g_new0(NBDServerData, 1); nbd_server->max_connections = max_connections; + nbd_server->handshake_limit = handshake_limit; nbd_server->listener = qio_net_listener_new(); qio_net_listener_set_name(nbd_server->listener, @@ -206,15 +207,19 @@ void nbd_server_start_options(NbdServerOptions *arg, Error **errp) if (!arg->has_max_connections) { arg->max_connections = NBD_DEFAULT_MAX_CONNECTIONS; } + if (!arg->has_handshake_limit) { + arg->handshake_limit = NBD_DEFAULT_HANDSHAKE_LIMIT; + } nbd_server_start(arg->addr, arg->tls_creds, arg->tls_authz, - arg->max_connections, errp); + arg->max_connections, arg->handshake_limit, errp); } void qmp_nbd_server_start(SocketAddressLegacy *addr, const char *tls_creds, const char *tls_authz, bool has_max_connections, uint32_t max_connections, + bool has_handshake_limit, uint32_t handshake_limit, Error **errp) { SocketAddress *addr_flat = socket_address_flatten(addr); @@ -222,8 +227,12 @@ void qmp_nbd_server_start(SocketAddressLegacy *addr, if (!has_max_connections) { max_connections = NBD_DEFAULT_MAX_CONNECTIONS; } + if (!has_handshake_limit) { + handshake_limit = NBD_DEFAULT_HANDSHAKE_LIMIT; + } - nbd_server_start(addr_flat, tls_creds, tls_authz, max_connections, errp); + nbd_server_start(addr_flat, tls_creds, tls_authz, max_connections, + handshake_limit, errp); qapi_free_SocketAddress(addr_flat); }