qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 01/18] nbd: Don't kill server on client that doe


From: Alex Bligh
Subject: Re: [Qemu-devel] [PATCH 01/18] nbd: Don't kill server on client that doesn't request TLS
Date: Sat, 9 Apr 2016 11:28:41 +0100

On 8 Apr 2016, at 23:05, Eric Blake <address@hidden> 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>

Reviewed-by: Alex Bligh <address@hidden>

> ---
> nbd/server.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/nbd/server.c b/nbd/server.c
> index 2a4dd10..e7e4881 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -451,9 +451,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) {
> @@ -471,6 +474,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,
> @@ -480,7 +486,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) {
> -- 
> 2.5.5
> 
> 

-- 
Alex Bligh







reply via email to

[Prev in Thread] Current Thread [Next in Thread]