qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Nir Soffer <nirsof@gmail.com>
To: qemu-devel@nongnu.org
Cc: eblake@redhat.com, pbonzini@redhat.com, kwolf@redhat.com,
	mreitz@redhat.com, rjones@redhat.com, qemu-block@nongnu.org,
	Nir Soffer <nirsof@gmail.com>
Subject: [Qemu-devel] [PATCH 1/3] nbd: Add option to disallow listing exports
Date: Fri, 13 Apr 2018 22:26:03 +0300	[thread overview]
Message-ID: <20180413192605.2145-2-nirsof@gmail.com> (raw)
In-Reply-To: <20180413192605.2145-1-nirsof@gmail.com>

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 <nirsof@gmail.com>
---
 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

  reply	other threads:[~2018-04-13 19:26 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-13 19:26 [Qemu-devel] [PATCH 0/3] qemu-nbd: Disallow listing exports Nir Soffer
2018-04-13 19:26 ` Nir Soffer [this message]
2018-04-13 21:07   ` [Qemu-devel] [PATCH 1/3] nbd: Add option to disallow " Richard W.M. Jones
2018-04-16 10:31   ` Daniel P. Berrangé
2018-04-16 10:53     ` Richard W.M. Jones
2018-04-16 11:00       ` Daniel P. Berrangé
2018-04-17 19:47         ` Eric Blake
2018-04-17 19:41   ` Eric Blake
2018-04-13 19:26 ` [Qemu-devel] [PATCH 2/3] iotests.py: Add helper for running commands Nir Soffer
2018-04-13 19:26 ` [Qemu-devel] [PATCH 3/3] qemu-iotests: Test new qemu-nbd --nolist option Nir Soffer
2018-04-17 19:56   ` Eric Blake
2018-04-18  9:43     ` Vladimir Sementsov-Ogievskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180413192605.2145-2-nirsof@gmail.com \
    --to=nirsof@gmail.com \
    --cc=eblake@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=rjones@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).