nbd: Don't kill server on client that doesn't request TLS
Upstream NBD documents (as of commit 4feebc95) 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 is programmed to talk to such a server, and does not do NBD_OPT_STARTTLS first, but rather wants to probe if it can use a non-encrypted export. The NBD protocol documents that we should let such a client continue trying, on the grounds that maybe the client will get the hint to send NBD_OPT_STARTTLS, rather than immediately dropping the connection. Note that NBD_OPT_EXPORT_NAME is a special case: since it is the only option request that can't have an error return, we have to (continue to) drop the connection on that one; rather, what we are fixing here is that all other replies prior to TLS initiation tell the client NBD_REP_ERR_TLS_REQD, but keep the connection alive. Signed-off-by: Eric Blake <eblake@redhat.com> Message-id: 1460671343-18485-1-git-send-email-eblake@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
This commit is contained in:
parent
23994a5f52
commit
d1129a8ad9
15
nbd/server.c
15
nbd/server.c
@ -449,11 +449,19 @@ static int nbd_negotiate_options(NBDClient *client)
|
||||
client->ioc = QIO_CHANNEL(tioc);
|
||||
break;
|
||||
|
||||
case NBD_OPT_EXPORT_NAME:
|
||||
/* No way to return an error to client, so drop connection */
|
||||
TRACE("Option 0x%x not permitted before TLS", clientflags);
|
||||
return -EINVAL;
|
||||
|
||||
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 +479,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 +491,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) {
|
||||
|
Loading…
Reference in New Issue
Block a user