[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH for-2.6] nbd: Don't kill server on
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH for-2.6] nbd: Don't kill server on client that doesn't request TLS |
Date: |
Thu, 14 Apr 2016 09:25:31 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.1 |
[adding qemu-block in cc, since Paolo can't send pull request]
On 04/07/2016 02:29 PM, Eric Blake wrote:
> Upstream NBD is documenting that servers MAY choose to operate
> in a conditional mode, where it is up to the client whether to
> use TLS. For qemu's case, we want to always be in FORCEDTLS
> mode, because of the risk of man-in-the-middle attacks, and since
> we never export more than one device; likewise, the qemu client
> will ALWAYS send NBD_OPT_STARTTLS as its first option. But now
> that SELECTIVETLS servers exist, it is feasible to encounter a
> (non-qemu) client that does not do NBD_OPT_STARTTLS first, but
> rather wants to take advantage of the conditional modes it might
> find elsewhere.
>
> Since we require TLS, we are within our rights to drop connections
> on any client that doesn't negotiate it right away, or which
> attempts to negotiate it incorrectly, without violating the intent
> of the NBD Protocol. However, it's better to allow the client to
> continue trying, on the grounds that maybe the client will get the
> hint to send NBD_OPT_STARTTLS.
>
> Signed-off-by: Eric Blake <address@hidden>
> ---
>
> My earlier patch was arguably incomplete:
> https://lists.gnu.org/archive/html/qemu-devel/2016-04/msg01265.html
>
> But as it is already in a pull request, and as this one is
> a bit more controversial, it's best to keep it as a separate patch.
>
> nbd/server.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/nbd/server.c b/nbd/server.c
> index 7843584..2b727f0 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -450,9 +450,12 @@ static int nbd_negotiate_options(NBDClient *client)
>
> default:
> TRACE("Option 0x%x not permitted before TLS", clientflags);
> + if (nbd_negotiate_drop_sync(client->ioc, length) != length) {
> + return -EIO;
> + }
> nbd_negotiate_send_rep(client->ioc, NBD_REP_ERR_TLS_REQD,
> clientflags);
> - return -EINVAL;
> + break;
> }
> } else if (fixedNewstyle) {
> switch (clientflags) {
> @@ -470,6 +473,9 @@ static int nbd_negotiate_options(NBDClient *client)
> return nbd_negotiate_handle_export_name(client, length);
>
> case NBD_OPT_STARTTLS:
> + if (nbd_negotiate_drop_sync(client->ioc, length) != length) {
> + return -EIO;
> + }
> if (client->tlscreds) {
> TRACE("TLS already enabled");
> nbd_negotiate_send_rep(client->ioc, NBD_REP_ERR_INVALID,
> @@ -479,7 +485,7 @@ static int nbd_negotiate_options(NBDClient *client)
> nbd_negotiate_send_rep(client->ioc, NBD_REP_ERR_POLICY,
> clientflags);
> }
> - return -EINVAL;
> + break;
> default:
> TRACE("Unsupported option 0x%x", clientflags);
> if (nbd_negotiate_drop_sync(client->ioc, length) != length) {
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-block] [Qemu-devel] [PATCH for-2.6] nbd: Don't kill server on client that doesn't request TLS,
Eric Blake <=