[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
- Re: [Qemu-devel] [PATCH 03/18] nbd: More debug typo fixes, use correct formats, (continued)
- [Qemu-devel] [PATCH 09/18] nbd: Share common reply-sending code in server, Eric Blake, 2016/04/08
- [Qemu-devel] [PATCH 02/18] nbd: Don't fail handshake on NBD_OPT_LIST descriptions, Eric Blake, 2016/04/08
- [Qemu-devel] [PATCH 14/18] nbd: Implement NBD_OPT_GO on client, Eric Blake, 2016/04/08
- [Qemu-devel] [PATCH 10/18] nbd: Share common option-sending code in client, Eric Blake, 2016/04/08
- [Qemu-devel] [PATCH 01/18] nbd: Don't kill server on client that doesn't request TLS, Eric Blake, 2016/04/08
- Re: [Qemu-devel] [PATCH 01/18] nbd: Don't kill server on client that doesn't request TLS,
Alex Bligh <=
- [Qemu-devel] [PATCH 08/18] nbd: Limit nbdflags to 16 bits, Eric Blake, 2016/04/08
- [Qemu-devel] [PATCH 12/18] nbd: Less allocation during NBD_OPT_LIST, Eric Blake, 2016/04/08
- [Qemu-devel] [PATCH 15/18] nbd: Implement NBD_OPT_GO on server, Eric Blake, 2016/04/08
- [Qemu-devel] [PATCH 13/18] nbd: Support shorter handshake, Eric Blake, 2016/04/08