On Fri, Apr 13, 2018 at 10:26:03PM +0300, Nir Soffer wrote:
> When a management application expose images using qemu-nbd, it needs a
> secure way to allow temporary access to the disk. Using a random export
> name can solve this problem:
>
> nbd://server:10809/22965f19-9ab5-4d18-94e1-cbeb321fa433
>
> Assuming that the url is passed to the user in a secure way, and the
> user is using TLS to access the image.
>
> However, since qemu-nbd implements NBD_OPT_LIST, anyone can easily find
> the secret export:
>
> $ nbd-client -l server 10809
> Negotiation: ..
> 22965f19-9ab5-4d18-94e1-cbeb321fa433
>
> Add a new --nolist option, disabling listing, similar the "allowlist"
> nbd-server configuration option.
>
> When used, listing exports will fail like this:
>
> $ nbd-client -l localhost 10809
> Negotiation: ..
>
> E: listing not allowed by server.
> Server said: Listing exports is forbidden
>
> Signed-off-by: Nir Soffer <nir...@gmail.com>
Tested-by: Richard W.M. Jones <rjo...@redhat.com>
The code looks good to me too, so ACK.
Rich.
> blockdev-nbd.c | 2 +-
> include/block/nbd.h | 1 +
> nbd/server.c | 7 +++++++
> qemu-nbd.c | 9 ++++++++-
> qemu-nbd.texi | 2 ++
> 5 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/blockdev-nbd.c b/blockdev-nbd.c
> index 65a84739ed..b9a885dc4b 100644
> --- a/blockdev-nbd.c
> +++ b/blockdev-nbd.c
> @@ -37,7 +37,7 @@ static void nbd_accept(QIONetListener *listener,
> QIOChannelSocket *cioc,
> {
> qio_channel_set_name(QIO_CHANNEL(cioc), "nbd-server");
> nbd_client_new(NULL, cioc,
> - nbd_server->tlscreds, NULL,
> + nbd_server->tlscreds, NULL, true,
> nbd_blockdev_client_closed);
> }
>
> diff --git a/include/block/nbd.h b/include/block/nbd.h
> index fcdcd54502..5c6b6272a0 100644
> --- a/include/block/nbd.h
> +++ b/include/block/nbd.h
> @@ -308,6 +308,7 @@ void nbd_client_new(NBDExport *exp,
> QIOChannelSocket *sioc,
> QCryptoTLSCreds *tlscreds,
> const char *tlsaclname,
> + bool allow_list,
> void (*close_fn)(NBDClient *, bool));
> void nbd_client_get(NBDClient *client);
> void nbd_client_put(NBDClient *client);
> diff --git a/nbd/server.c b/nbd/server.c
> index 9e1f227178..7b91922d1d 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -115,6 +115,7 @@ struct NBDClient {
>
> bool structured_reply;
> NBDExportMetaContexts export_meta;
> + bool allow_list;
>
> uint32_t opt; /* Current option being negotiated */
> uint32_t optlen; /* remaining length of data in ioc for the option being
> @@ -1032,6 +1033,10 @@ static int nbd_negotiate_options(NBDClient *client,
> uint16_t myflags,
> case NBD_OPT_LIST:
> if (length) {
> ret = nbd_reject_length(client, false, errp);
> + } else if (!client->allow_list) {
> + ret = nbd_negotiate_send_rep_err(client,
> + NBD_REP_ERR_POLICY,
> errp,
> + "Listing exports is
> forbidden");
> } else {
> ret = nbd_negotiate_handle_list(client, errp);
> }
> @@ -2141,6 +2146,7 @@ void nbd_client_new(NBDExport *exp,
> QIOChannelSocket *sioc,
> QCryptoTLSCreds *tlscreds,
> const char *tlsaclname,
> + bool allow_list,
> void (*close_fn)(NBDClient *, bool))
> {
> NBDClient *client;
> @@ -2158,6 +2164,7 @@ void nbd_client_new(NBDExport *exp,
> object_ref(OBJECT(client->sioc));
> client->ioc = QIO_CHANNEL(sioc);
> object_ref(OBJECT(client->ioc));
> + client->allow_list = allow_list;
> client->close_fn = close_fn;
>
> co = qemu_coroutine_create(nbd_co_client_start, client);
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index 0af0560ad1..b63d4d9e8b 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -52,6 +52,7 @@
> #define QEMU_NBD_OPT_TLSCREDS 261
> #define QEMU_NBD_OPT_IMAGE_OPTS 262
> #define QEMU_NBD_OPT_FORK 263
> +#define QEMU_NBD_OPT_NOLIST 264
>
> #define MBR_SIZE 512
>
> @@ -66,6 +67,7 @@ static int shared = 1;
> static int nb_fds;
> static QIONetListener *server;
> static QCryptoTLSCreds *tlscreds;
> +static bool allow_list = true;
>
> static void usage(const char *name)
> {
> @@ -86,6 +88,7 @@ static void usage(const char *name)
> " -v, --verbose display extra debugging information\n"
> " -x, --export-name=NAME expose export by name\n"
> " -D, --description=TEXT with -x, also export a human-readable
> description\n"
> +" --nolist do not list export\n"
> "\n"
> "Exposing part of the image:\n"
> " -o, --offset=OFFSET offset into the image\n"
> @@ -355,7 +358,7 @@ static void nbd_accept(QIONetListener *listener,
> QIOChannelSocket *cioc,
> nb_fds++;
> nbd_update_server_watch();
> nbd_client_new(newproto ? NULL : exp, cioc,
> - tlscreds, NULL, nbd_client_closed);
> + tlscreds, NULL, allow_list, nbd_client_closed);
> }
>
> static void nbd_update_server_watch(void)
> @@ -523,6 +526,7 @@ int main(int argc, char **argv)
> { "object", required_argument, NULL, QEMU_NBD_OPT_OBJECT },
> { "export-name", required_argument, NULL, 'x' },
> { "description", required_argument, NULL, 'D' },
> + { "nolist", no_argument, NULL, QEMU_NBD_OPT_NOLIST },
> { "tls-creds", required_argument, NULL, QEMU_NBD_OPT_TLSCREDS },
> { "image-opts", no_argument, NULL, QEMU_NBD_OPT_IMAGE_OPTS },
> { "trace", required_argument, NULL, 'T' },
> @@ -717,6 +721,9 @@ int main(int argc, char **argv)
> case 'D':
> export_description = optarg;
> break;
> + case QEMU_NBD_OPT_NOLIST:
> + allow_list = false;
> + break;
> case 'v':
> verbose = 1;
> break;
> diff --git a/qemu-nbd.texi b/qemu-nbd.texi
> index 9a84e81eed..010b29588f 100644
> --- a/qemu-nbd.texi
> +++ b/qemu-nbd.texi
> @@ -85,6 +85,8 @@ the new style NBD protocol negotiation
> @item -D, --description=@var{description}
> Set the NBD volume export description, as a human-readable
> string. Requires the use of @option{-x}
> +@item --nolist
> +Do not allow the client to fetch a list of exports from this server.
> @item --tls-creds=ID
> Enable mandatory TLS encryption for the server by setting the ID
> of the TLS credentials object previously created with the --object
> --
> 2.14.3
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/
