From nobody Fri Dec 19 06:56:59 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1509101127779742.6462110246744; Fri, 27 Oct 2017 03:45:27 -0700 (PDT) Received: from localhost ([::1]:56560 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1e828i-0004lu-Vq for importer@patchew.org; Fri, 27 Oct 2017 06:45:25 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57287) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1e824Q-0001G3-QZ for qemu-devel@nongnu.org; Fri, 27 Oct 2017 06:40:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1e824P-0005cK-Rf for qemu-devel@nongnu.org; Fri, 27 Oct 2017 06:40:58 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42746) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1e824M-0005Zv-Nv; Fri, 27 Oct 2017 06:40:54 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B6217C04AC42; Fri, 27 Oct 2017 10:40:53 +0000 (UTC) Received: from red.redhat.com (ovpn-120-166.rdu2.redhat.com [10.10.120.166]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4212D5C881; Fri, 27 Oct 2017 10:40:52 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com B6217C04AC42 Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=eblake@redhat.com From: Eric Blake To: qemu-devel@nongnu.org Date: Fri, 27 Oct 2017 12:40:31 +0200 Message-Id: <20171027104037.8319-7-eblake@redhat.com> In-Reply-To: <20171027104037.8319-1-eblake@redhat.com> References: <20171027104037.8319-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Fri, 27 Oct 2017 10:40:53 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v6 06/12] nbd/server: Refactor zero-length option check X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: pbonzini@redhat.com, vsementsov@virtuozzo.com, qemu-block@nongnu.org Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Consolidate the response for a non-zero-length option payload into a new function, nbd_reject_length(). This check will also be used when introducing support for structured replies. Note that STARTTLS response differs based on time: if the connection is still unencrypted, we set fatal to true (a client that can't request TLS correctly may still think that we are ready to start the TLS handshake, so we must disconnect); while if the connection is already encrypted, the client is sending a bogus request but is no longer at risk of being confused by continuing the connection. Signed-off-by: Eric Blake Reviewed-by: Vladimir Sementsov-Ogievskiy --- v6: split, rework logic to avoid subtle regression on starttls [Vladimir] v5: new patch --- nbd/server.c | 74 +++++++++++++++++++++++++++++++++++++-------------------= ---- 1 file changed, 46 insertions(+), 28 deletions(-) diff --git a/nbd/server.c b/nbd/server.c index 6af708662d..a98f5622c9 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -253,21 +253,10 @@ static int nbd_negotiate_send_rep_list(QIOChannel *io= c, NBDExport *exp, /* Process the NBD_OPT_LIST command, with a potential series of replies. * Return -errno on error, 0 on success. */ -static int nbd_negotiate_handle_list(NBDClient *client, uint32_t length, - Error **errp) +static int nbd_negotiate_handle_list(NBDClient *client, Error **errp) { NBDExport *exp; - if (length) { - if (nbd_drop(client->ioc, length, errp) < 0) { - return -EIO; - } - return nbd_negotiate_send_rep_err(client->ioc, - NBD_REP_ERR_INVALID, NBD_OPT_LIS= T, - errp, - "OPT_LIST should not have length= "); - } - /* For each export, send a NBD_REP_SERVER reply. */ QTAILQ_FOREACH(exp, &exports, next) { if (nbd_negotiate_send_rep_list(client->ioc, exp, errp)) { @@ -531,7 +520,6 @@ static int nbd_negotiate_handle_info(NBDClient *client,= uint32_t length, /* Handle NBD_OPT_STARTTLS. Return NULL to drop connection, or else the * new channel for all further (now-encrypted) communication. */ static QIOChannel *nbd_negotiate_handle_starttls(NBDClient *client, - uint32_t length, Error **errp) { QIOChannel *ioc; @@ -540,15 +528,6 @@ static QIOChannel *nbd_negotiate_handle_starttls(NBDCl= ient *client, trace_nbd_negotiate_handle_starttls(); ioc =3D client->ioc; - if (length) { - if (nbd_drop(ioc, length, errp) < 0) { - return NULL; - } - nbd_negotiate_send_rep_err(ioc, NBD_REP_ERR_INVALID, NBD_OPT_START= TLS, - errp, - "OPT_STARTTLS should not have length"); - return NULL; - } if (nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, NBD_OPT_STARTTLS, errp) < 0) { @@ -584,6 +563,34 @@ static QIOChannel *nbd_negotiate_handle_starttls(NBDCl= ient *client, return QIO_CHANNEL(tioc); } +/* nbd_reject_length: Handle any unexpected payload. + * @fatal requests that we quit talking to the client, even if we are able + * to successfully send an error to the guest. + * Return: + * -errno transmission error occurred or @fatal was requested, errp is set + * 0 error message successfully sent to client, errp is not set + */ +static int nbd_reject_length(NBDClient *client, uint32_t length, + uint32_t option, bool fatal, Error **errp) +{ + int ret; + + assert(length); + if (nbd_drop(client->ioc, length, errp) < 0) { + return -EIO; + } + ret =3D nbd_negotiate_send_rep_err(client->ioc, NBD_REP_ERR_INVALID, + option, errp, + "option '%s' should have zero length", + nbd_opt_lookup(option)); + if (fatal && !ret) { + error_setg(errp, "option '%s' should have zero length", + nbd_opt_lookup(option)); + return -EINVAL; + } + return ret; +} + /* nbd_negotiate_options * Process all NBD_OPT_* client option commands, during fixed newstyle * negotiation. @@ -674,7 +681,13 @@ static int nbd_negotiate_options(NBDClient *client, ui= nt16_t myflags, } switch (option) { case NBD_OPT_STARTTLS: - tioc =3D nbd_negotiate_handle_starttls(client, length, err= p); + if (length) { + /* Unconditionally drop the connection if the client + * can't start a TLS negotiation correctly */ + nbd_reject_length(client, length, option, true, errp); + return -EINVAL; + } + tioc =3D nbd_negotiate_handle_starttls(client, errp); if (!tioc) { return -EIO; } @@ -709,7 +722,12 @@ static int nbd_negotiate_options(NBDClient *client, ui= nt16_t myflags, } else if (fixedNewstyle) { switch (option) { case NBD_OPT_LIST: - ret =3D nbd_negotiate_handle_list(client, length, errp); + if (length) { + ret =3D nbd_reject_length(client, length, option, fals= e, + errp); + } else { + ret =3D nbd_negotiate_handle_list(client, errp); + } break; case NBD_OPT_ABORT: @@ -735,10 +753,10 @@ static int nbd_negotiate_options(NBDClient *client, u= int16_t myflags, break; case NBD_OPT_STARTTLS: - if (nbd_drop(client->ioc, length, errp) < 0) { - return -EIO; - } - if (client->tlscreds) { + if (length) { + ret =3D nbd_reject_length(client, length, option, fals= e, + errp); + } else if (client->tlscreds) { ret =3D nbd_negotiate_send_rep_err(client->ioc, NBD_REP_ERR_INVALID, option, errp, --=20 2.13.6