| Message ID | 1460153158-21612-2-git-send-email-eblake@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 8 Apr 2016, at 23:05, Eric Blake <eblake@redhat.com> 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 <eblake@redhat.com> Reviewed-by: Alex Bligh <alex@alex.org.uk> > --- > 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 > >
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) {
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 <eblake@redhat.com> --- nbd/server.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)