qemu-nbd: Disallow listing exports

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 <[email protected]>

 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(-)

blockdev-nbd.c +1 -1

index 65a84739ed..b9a885dc4b 100644
@@ -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);
 }
 

include/block/nbd.h +1 -0

index fcdcd54502..5c6b6272a0 100644
@@ -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);

nbd/server.c +7 -0

index 9e1f227178..7b91922d1d 100644
@@ -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);

qemu-nbd.c +8 -1

index 0af0560ad1..b63d4d9e8b 100644
@@ -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;

qemu-nbd.texi +2 -0

index 9a84e81eed..010b29588f 100644
@@ -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

Signature · 1 line

2.14.3

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

Essentially this is abusing the export name as a crude authentication token. There are NBD servers that expect NBD_OPT_LIST to always succeeed

when they detect that the new style protocol is available. I really hate the idea of making it possible to break the NBD_OPT_LIST functionality via a command line arg like this.

Furthermore, applications are *not* considering the export names to be security sensitive data, so will not be taking any precautions to ensure they remain secret, as they would do with authentication credentials. Again I really hate the idea of using NBD exports an an auth credential.

So I don't think we should be suggesting that security through obscurity of the export name is a supported approach to securing NBD.

I understand the desire to be able to secure NBD exports though, so think we need to come up with some kind of supportable solution for this. There are two approaches we should take

- Add support for TLS client certification whitelisting. eg every client has a unique identity based on the distinguished name (dname) in the x509 cert they were issued. The NBD server can be told which of these dnames should be a permitted to connect. This is supported in VNC for years, and I've had patches pending to support this in a QEMU for chardevs NBD and migration for a while. These were stalled on way to convert -object ... syntax into nested QOM objects.

- Define a mapping of the SASL protocol ontop NBD. SASL is a generic pluggable authentication mechanism for network protocols. It is already used in libvirt, VNC and SPICE, and would easily fit in with NBD from a conceptual POV. When used in combination with TLS, this offers a wide range of auth mechanisms from simple username+password, to full integration with Kerberos.

If this need is urgent, I think we could partially unblock the TLS x509 whitelisting support without much difficulty. We haven't been pushing hard to unblock it simply because no one was urgently blocked by its absence so far. This provides a strong solution, but the difficulty is that the server may not know the x509 dname of the permitted client, which makes it hard to use in practice.

SASL with a simple username+password scheme
is thus still very compelling to implement, but will obviously take longer due to the amount of code/spec work required.

Quoted 2 lines · Daniel P. Berrangé

Essentially this is abusing the export name as a crude authentication token. There are NBD servers that expect NBD_OPT_LIST to always succeeed

I guess you mean "NBD clients" ...

Quoted 3 lines

when they detect that the new style protocol is available. I really hate the idea of making it possible to break the NBD_OPT_LIST functionality via a command line arg like this.

The specific use case I have in mind is virt-v2v forked an instance of ‘qemu-img convert’ which connects to the NBD server.

Of course this does also reveal a flaw in the plan because ...

Quoted 4 lines

Furthermore, applications are *not* considering the export names to be security sensitive data, so will not be taking any precautions to ensure they remain secret, as they would do with authentication credentials. Again I really hate the idea of using NBD exports an an auth credential.

‘ps ax’ on the conversion server will reveal the export name/ticket from the qemu-img command line.

Quoted 16 lines

So I don't think we should be suggesting that security through obscurity of the export name is a supported approach to securing NBD.

I understand the desire to be able to secure NBD exports though, so think we need to come up with some kind of supportable solution for this. There are two approaches we should take

- Add support for TLS client certification whitelisting. eg every client has a unique identity based on the distinguished name (dname) in the x509 cert they were issued. The NBD server can be told which of these dnames should be a permitted to connect. This is supported in VNC for years, and I've had patches pending to support this in a QEMU for chardevs NBD and migration for a while. These were stalled on way to convert -object ... syntax into nested QOM objects.

Quoted 6 lines

- Define a mapping of the SASL protocol ontop NBD. SASL is a generic pluggable authentication mechanism for network protocols. It is already used in libvirt, VNC and SPICE, and would easily fit in with NBD from a conceptual POV. When used in combination with TLS, this offers a wide range of auth mechanisms from simple username+password, to full integration with Kerberos.

The first one sounds heavyweight but at least implementable from the virt-v2v point of view. The second one sounds like it would be impossible for mere humans to set it up.

Quoted 6 lines

If this need is urgent, I think we could partially unblock the TLS x509 whitelisting support without much difficulty. We haven't been pushing hard to unblock it simply because no one was urgently blocked by its absence so far. This provides a strong solution, but the difficulty is that the server may not know the x509 dname of the permitted client, which makes it hard to use in practice.

Can you clarify what you mean by the last sentence above? Can't we just create a client certificate in virt-v2v and pass that to qemu-img, and at the same time pass the server a list of permitted names? (likely only a single name in practice)

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.

Quoted 5 lines · Richard W.M. Jones

Quoted 2 lines · Daniel P. Berrangé

Essentially this is abusing the export name as a crude authentication token. There are NBD servers that expect NBD_OPT_LIST to always succeeed

I guess you mean "NBD clients" ...

Sigh, yes, of course.

Quoted 4 lines

Quoted 4 lines

when they detect that the new style protocol is available. I really hate the idea of making it possible to break the NBD_OPT_LIST functionality via a command line arg like this.

tests/qemu-iotests/214 line 29

Quoted 9 lines · Nir Soffer

Add new test module for tesing the --nolist option.

Signed-off-by Nir Soffer <[email protected]>

---

tests/qemu-iotests/214 new file +5 -0

+iotests.log('Check that listing exports is allowed by default')
+disk, nbd_sock = iotests.file_path('disk1', 'nbd-sock1')
+iotests.qemu_img_create('-f', iotests.imgfmt, disk, '1m')
+iotests.qemu_nbd('-k', nbd_sock, '-f', iotests.imgfmt, '-x', 'export', disk)
+out = iotests.run('nbd-client', '-l', '--unix', nbd_sock)

Should we really be relying on the third-party nbd-client to be installed? Would it not be better to teach our own NBD client to learn how to do interesting things over NBD? Your use case of listing the output of NBD_OPT_LIST is one, but another that readily comes to mind is listing the possibilities of NBD_OPT_LIST_META_CONTEXT that just went into 2.12. Maybe making 'qemu-img info' give more details when connecting to an NBD server, compared to what is normally needed just for connecting to an export on that server?

Additionally, once we merge in Vladimir's work to expose persistent dirty bitmaps via NBD_OPT_SET_META_CONTEXT/NBD_CMD_BLOCK_STATUS, it would be nice to have an in-tree tool for reading out the context information of an NBD export, perhaps extending what 'qemu-img map' can already do (Vladimir already mentioned that he only implemented the server side, and left the client side for an out-of-tree solution [1], although I'm wondering if that is still the wisest course of action).

[1]

tests/qemu-iotests/214 line 39

Quoted 11 lines

tests/qemu-iotests/214 new file +10 -0

+
+assert 'export' in out.splitlines(), 'Export not in %r' % out
+
+iotests.log('Check that listing exports is forbidden with --nolist')
+disk, nbd_sock = iotests.file_path('disk2', 'nbd-sock2')
+iotests.qemu_img_create('-f', iotests.imgfmt, disk, '1m')
+iotests.qemu_nbd('-k', nbd_sock, '-f', iotests.imgfmt, '-x', 'secret',
+                 '--nolist', disk)
+
+# nbd-client fails when listing is not allowed, but lets not depend on 3rd

s/lets/let's/