[PATCH 00/12] nbd: enable use of TLS on non-TCP transports and other TLS improvements

[PATCH 00/12] nbd: enable use of TLS on non-TCP transports and other TLS improvements

[Daniel P. Berrangé]

This series was principally motivated by a desire to enabl use of TLS
on non-TCP transports. For x509 certificates this means we need a way
to set the hostname to use for validation. This also lets us override
the hostname when connecting on a TCP transport that is tunnelled or
port-forwarded. It also unlocks the ability to use PSK (pre-shared
keys) with UNIX sockets which would always have worked, had it not
been blocked by explicit checks in NBD code.

NB, the first patch in this series is common with my corresponding
migration series for TLS

  https://lists.gnu.org/archive/html/qemu-devel/2022-03/msg00556.html

Daniel P. Berrangé (12):
  crypto: mandate a hostname when checking x509 creds on a client
  block: pass desired TLS hostname through from block driver client
  block/nbd: support override of hostname for TLS certificate validation
  qemu-nbd: add --tls-hostname option for TLS certificate validation
  block/nbd: don't restrict TLS usage to IP sockets
  tests/qemu-iotests: add QEMU_IOTESTS_REGEN=1 to update reference file
  tests/qemu-iotests: expand _filter_nbd rules
  tests/qemu-iotests: introduce filter for qemu-nbd export list
  tests/qemu-iotests: convert NBD TLS test to use standard filters
  tests/qemu-iotests: validate NBD TLS with hostname mismatch
  tests/qemu-iotests: validate NBD TLS with UNIX sockets
  tests/qemu-iotests: validate NBD TLS with UNIX sockets and PSK

 block/nbd.c                      | 25 +++++---
 blockdev-nbd.c                   |  6 --
 crypto/tlssession.c              |  6 ++
 docs/tools/qemu-nbd.rst          | 14 +++++
 include/block/nbd.h              |  3 +-
 nbd/client-connection.c          | 12 +++-
 qapi/block-core.json             |  3 +
 qemu-nbd.c                       | 25 ++++++--
 tests/qemu-iotests/233           | 99 +++++++++++++++++++++++++++-----
 tests/qemu-iotests/233.out       | 56 +++++++++++++++---
 tests/qemu-iotests/241           |  6 +-
 tests/qemu-iotests/241.out       |  3 +
 tests/qemu-iotests/common.filter |  9 +++
 tests/qemu-iotests/common.tls    | 31 +++++++++-
 tests/qemu-iotests/testrunner.py |  6 ++
 15 files changed, 252 insertions(+), 52 deletions(-)

-- 
2.34.1

[PATCH 01/12] crypto: mandate a hostname when checking x509 creds on a client

[Daniel P. Berrangé]

Currently the TLS session object assumes that the caller will always
provide a hostname when using x509 creds on a client endpoint. This
relies on the caller to detect and report an error if the user has
configured QEMU with x509 credentials on a UNIX socket. The migration
code has such a check, but it is too broad, reporting an error when
the user has configured QEMU with PSK credentials on a UNIX socket,
where hostnames are irrelevant.

Putting the check into the TLS session object credentials validation
code ensures we report errors in only the scenario that matters.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 crypto/tlssession.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/crypto/tlssession.c b/crypto/tlssession.c
index a8db8c76d1..b302d835d2 100644
--- a/crypto/tlssession.c
+++ b/crypto/tlssession.c
@@ -373,6 +373,12 @@ qcrypto_tls_session_check_certificate(QCryptoTLSSession *session,
                                session->hostname);
                     goto error;
                 }
+            } else {
+                if (session->creds->endpoint ==
+                    QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT) {
+                    error_setg(errp, "No hostname for certificate validation");
+                    goto error;
+                }
             }
         }
 
-- 
2.34.1

[PATCH 02/12] block: pass desired TLS hostname through from block driver client

[Daniel P. Berrangé]

In

  commit a71d597b989fd701b923f09b3c20ac4fcaa55e81
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   Thu Jun 10 13:08:00 2021 +0300

    block/nbd: reuse nbd_co_do_establish_connection() in nbd_open()

the use of the 'hostname' field from the BDRVNBDState struct was
lost, and 'nbd_connect' just hardcoded it to match the IP socket
address. This was a harmless bug at the time since we block use
with anything other than IP sockets.

Shortly though, We want to allow the caller to override the hostname
used in the TLS certificate checks. This is to allow for TLS
when doing port forwarding or tunneling. Thus we need to reinstate
the passing along of the 'hostname'.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 block/nbd.c             |  7 ++++---
 include/block/nbd.h     |  3 ++-
 nbd/client-connection.c | 12 +++++++++---
 3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 5853d85d60..dd43929207 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -92,7 +92,7 @@ typedef struct BDRVNBDState {
     SocketAddress *saddr;
     char *export, *tlscredsid;
     QCryptoTLSCreds *tlscreds;
-    const char *hostname;
+    const char *tlshostname;
     char *x_dirty_bitmap;
     bool alloc_depth;
 
@@ -1835,7 +1835,7 @@ static int nbd_process_options(BlockDriverState *bs, QDict *options,
             error_setg(errp, "TLS only supported over IP sockets");
             goto error;
         }
-        s->hostname = s->saddr->u.inet.host;
+        s->tlshostname = s->saddr->u.inet.host;
     }
 
     s->x_dirty_bitmap = g_strdup(qemu_opt_get(opts, "x-dirty-bitmap"));
@@ -1875,7 +1875,8 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     s->conn = nbd_client_connection_new(s->saddr, true, s->export,
-                                        s->x_dirty_bitmap, s->tlscreds);
+                                        s->x_dirty_bitmap, s->tlscreds,
+                                        s->tlshostname);
 
     if (s->open_timeout) {
         nbd_client_connection_enable_retry(s->conn);
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 78d101b774..a98eb665da 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -415,7 +415,8 @@ NBDClientConnection *nbd_client_connection_new(const SocketAddress *saddr,
                                                bool do_negotiation,
                                                const char *export_name,
                                                const char *x_dirty_bitmap,
-                                               QCryptoTLSCreds *tlscreds);
+                                               QCryptoTLSCreds *tlscreds,
+                                               const char *tlshostname);
 void nbd_client_connection_release(NBDClientConnection *conn);
 
 QIOChannel *coroutine_fn
diff --git a/nbd/client-connection.c b/nbd/client-connection.c
index 2bda42641d..2a632931c3 100644
--- a/nbd/client-connection.c
+++ b/nbd/client-connection.c
@@ -33,6 +33,7 @@ struct NBDClientConnection {
     /* Initialization constants, never change */
     SocketAddress *saddr; /* address to connect to */
     QCryptoTLSCreds *tlscreds;
+    char *tlshostname;
     NBDExportInfo initial_info;
     bool do_negotiation;
     bool do_retry;
@@ -77,7 +78,8 @@ NBDClientConnection *nbd_client_connection_new(const SocketAddress *saddr,
                                                bool do_negotiation,
                                                const char *export_name,
                                                const char *x_dirty_bitmap,
-                                               QCryptoTLSCreds *tlscreds)
+                                               QCryptoTLSCreds *tlscreds,
+                                               const char *tlshostname)
 {
     NBDClientConnection *conn = g_new(NBDClientConnection, 1);
 
@@ -85,6 +87,7 @@ NBDClientConnection *nbd_client_connection_new(const SocketAddress *saddr,
     *conn = (NBDClientConnection) {
         .saddr = QAPI_CLONE(SocketAddress, saddr),
         .tlscreds = tlscreds,
+        .tlshostname = g_strdup(tlshostname),
         .do_negotiation = do_negotiation,
 
         .initial_info.request_sizes = true,
@@ -107,6 +110,7 @@ static void nbd_client_connection_do_free(NBDClientConnection *conn)
     }
     error_free(conn->err);
     qapi_free_SocketAddress(conn->saddr);
+    g_free(conn->tlshostname);
     object_unref(OBJECT(conn->tlscreds));
     g_free(conn->initial_info.x_dirty_bitmap);
     g_free(conn->initial_info.name);
@@ -120,6 +124,7 @@ static void nbd_client_connection_do_free(NBDClientConnection *conn)
  */
 static int nbd_connect(QIOChannelSocket *sioc, SocketAddress *addr,
                        NBDExportInfo *info, QCryptoTLSCreds *tlscreds,
+                       const char *tlshostname,
                        QIOChannel **outioc, Error **errp)
 {
     int ret;
@@ -140,7 +145,7 @@ static int nbd_connect(QIOChannelSocket *sioc, SocketAddress *addr,
     }
 
     ret = nbd_receive_negotiate(NULL, QIO_CHANNEL(sioc), tlscreds,
-                                tlscreds ? addr->u.inet.host : NULL,
+                                tlshostname,
                                 outioc, info, errp);
     if (ret < 0) {
         /*
@@ -183,7 +188,8 @@ static void *connect_thread_func(void *opaque)
 
         ret = nbd_connect(conn->sioc, conn->saddr,
                           conn->do_negotiation ? &conn->updated_info : NULL,
-                          conn->tlscreds, &conn->ioc, &local_err);
+                          conn->tlscreds, conn->tlshostname,
+                          &conn->ioc, &local_err);
 
         /*
          * conn->updated_info will finally be returned to the user. Clear the
-- 
2.34.1

[PATCH 03/12] block/nbd: support override of hostname for TLS certificate validation

[Daniel P. Berrangé]

When connecting to an NBD server with TLS and x509 credentials,
the client must validate the hostname it uses for the connection,
against that published in the server's certificate. If the client
is tunnelling its connection over some other channel, however, the
hostname it uses may not match the info reported in the server's
certificate. In such a case, the user needs to explicitly set an
override for the hostname to use for certificate validation.

This is achieved by adding a 'tls-hostname' property to the NBD
block driver.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 block/nbd.c          | 18 +++++++++++++++---
 qapi/block-core.json |  3 +++
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index dd43929207..113aa5d3af 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -90,9 +90,10 @@ typedef struct BDRVNBDState {
     uint32_t reconnect_delay;
     uint32_t open_timeout;
     SocketAddress *saddr;
-    char *export, *tlscredsid;
+    char *export;
+    char *tlscredsid;
     QCryptoTLSCreds *tlscreds;
-    const char *tlshostname;
+    char *tlshostname;
     char *x_dirty_bitmap;
     bool alloc_depth;
 
@@ -121,6 +122,8 @@ static void nbd_clear_bdrvstate(BlockDriverState *bs)
     s->export = NULL;
     g_free(s->tlscredsid);
     s->tlscredsid = NULL;
+    g_free(s->tlshostname);
+    s->tlshostname = NULL;
     g_free(s->x_dirty_bitmap);
     s->x_dirty_bitmap = NULL;
 }
@@ -1764,6 +1767,11 @@ static QemuOptsList nbd_runtime_opts = {
             .type = QEMU_OPT_STRING,
             .help = "ID of the TLS credentials to use",
         },
+        {
+            .name = "tls-hostname",
+            .type = QEMU_OPT_STRING,
+            .help = "Override hostname for validating TLS x509 certificate",
+        },
         {
             .name = "x-dirty-bitmap",
             .type = QEMU_OPT_STRING,
@@ -1835,7 +1843,10 @@ static int nbd_process_options(BlockDriverState *bs, QDict *options,
             error_setg(errp, "TLS only supported over IP sockets");
             goto error;
         }
-        s->tlshostname = s->saddr->u.inet.host;
+        s->tlshostname = g_strdup(qemu_opt_get(opts, "tls-hostname"));
+        if (!s->tlshostname) {
+            s->tlshostname = g_strdup(s->saddr->u.inet.host);
+        }
     }
 
     s->x_dirty_bitmap = g_strdup(qemu_opt_get(opts, "x-dirty-bitmap"));
@@ -2037,6 +2048,7 @@ static const char *const nbd_strong_runtime_opts[] = {
     "port",
     "export",
     "tls-creds",
+    "tls-hostname",
     "server.",
 
     NULL
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 9a5a3641d0..c1b0435f57 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4078,6 +4078,8 @@
 #
 # @tls-creds: TLS credentials ID
 #
+# @tls-hostname: TLS hostname override for certificate validation
+#
 # @x-dirty-bitmap: A metadata context name such as "qemu:dirty-bitmap:NAME"
 #                  or "qemu:allocation-depth" to query in place of the
 #                  traditional "base:allocation" block status (see
@@ -4108,6 +4110,7 @@
   'data': { 'server': 'SocketAddress',
             '*export': 'str',
             '*tls-creds': 'str',
+            '*tls-hostname': 'str',
             '*x-dirty-bitmap': { 'type': 'str', 'features': [ 'unstable' ] },
             '*reconnect-delay': 'uint32',
             '*open-timeout': 'uint32' } }
-- 
2.34.1

[PATCH 04/12] qemu-nbd: add --tls-hostname option for TLS certificate validation

[Daniel P. Berrangé]

When using the --list option, qemu-nbd acts as an NBD client rather
than a server. As such when using TLS, it has a need to validate
the server certificate. This adds a --tls-hostname option which can
be used to override the default hostname used for certificate
validation.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 docs/tools/qemu-nbd.rst | 14 ++++++++++++++
 qemu-nbd.c              | 17 ++++++++++++++++-
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/docs/tools/qemu-nbd.rst b/docs/tools/qemu-nbd.rst
index 6031f96893..acce54a39d 100644
--- a/docs/tools/qemu-nbd.rst
+++ b/docs/tools/qemu-nbd.rst
@@ -169,6 +169,20 @@ driver options if ``--image-opts`` is specified.
   option; or provide the credentials needed for connecting as a client
   in list mode.
 
+.. option:: --tls-hostname=hostname
+
+  When validating an x509 certificate received over a TLS connection,
+  the hostname that the NBD client used to connect will be checked
+  against information in the server provided certificate. Sometimes
+  it might be required to override the hostname used to perform this
+  check. For example if the NBD client is using a tunnel from localhost
+  to connect to the remote server. In this case the `--tls-hostname`
+  option should be used to set the officially expected hostname of
+  the remote NBD server. This can also be used if accessing NBD over
+  a UNIX socket where there is no inherant hostname available. This
+  only is only permitted when acting as a NBD client with the `--list`
+  option.
+
 .. option:: --fork
 
   Fork off the server process and exit the parent once the server is running.
diff --git a/qemu-nbd.c b/qemu-nbd.c
index c6c20df68a..be8043fb00 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -69,6 +69,7 @@
 #define QEMU_NBD_OPT_TLSAUTHZ      264
 #define QEMU_NBD_OPT_PID_FILE      265
 #define QEMU_NBD_OPT_SELINUX_LABEL 266
+#define QEMU_NBD_OPT_TLSHOSTNAME   267
 
 #define MBR_SIZE 512
 
@@ -542,6 +543,7 @@ int main(int argc, char **argv)
         { "export-name", required_argument, NULL, 'x' },
         { "description", required_argument, NULL, 'D' },
         { "tls-creds", required_argument, NULL, QEMU_NBD_OPT_TLSCREDS },
+        { "tls-hostname", required_argument, NULL, QEMU_NBD_OPT_TLSHOSTNAME },
         { "tls-authz", required_argument, NULL, QEMU_NBD_OPT_TLSAUTHZ },
         { "image-opts", no_argument, NULL, QEMU_NBD_OPT_IMAGE_OPTS },
         { "trace", required_argument, NULL, 'T' },
@@ -568,6 +570,7 @@ int main(int argc, char **argv)
     strList *bitmaps = NULL;
     bool alloc_depth = false;
     const char *tlscredsid = NULL;
+    const char *tlshostname = NULL;
     bool imageOpts = false;
     bool writethrough = false; /* Client will flush as needed. */
     bool fork_process = false;
@@ -747,6 +750,9 @@ int main(int argc, char **argv)
         case QEMU_NBD_OPT_TLSCREDS:
             tlscredsid = optarg;
             break;
+        case QEMU_NBD_OPT_TLSHOSTNAME:
+            tlshostname = optarg;
+            break;
         case QEMU_NBD_OPT_IMAGE_OPTS:
             imageOpts = true;
             break;
@@ -835,6 +841,10 @@ int main(int argc, char **argv)
             error_report("TLS authorization is incompatible with export list");
             exit(EXIT_FAILURE);
         }
+        if (tlshostname && !list) {
+            error_report("TLS hostname is only required with export list");
+            exit(EXIT_FAILURE);
+        }
         tlscreds = nbd_get_tls_creds(tlscredsid, list, &local_err);
         if (local_err) {
             error_reportf_err(local_err, "Failed to get TLS creds: ");
@@ -845,6 +855,10 @@ int main(int argc, char **argv)
             error_report("--tls-authz is not permitted without --tls-creds");
             exit(EXIT_FAILURE);
         }
+        if (tlshostname) {
+            error_report("--tls-hostname is not permitted without --tls-creds");
+            exit(EXIT_FAILURE);
+        }
     }
 
     if (selinux_label) {
@@ -861,7 +875,8 @@ int main(int argc, char **argv)
 
     if (list) {
         saddr = nbd_build_socket_address(sockpath, bindto, port);
-        return qemu_nbd_client_list(saddr, tlscreds, bindto);
+        return qemu_nbd_client_list(saddr, tlscreds,
+                                    tlshostname ? tlshostname : bindto);
     }
 
 #if !HAVE_NBD_DEVICE
-- 
2.34.1

[PATCH 05/12] block/nbd: don't restrict TLS usage to IP sockets

[Daniel P. Berrangé]

The TLS usage for NBD was restricted to IP sockets because validating
x509 certificates requires knowledge of the hostname that the client
is connecting to.

TLS does not have to use x509 certificates though, as PSK (pre-shared
keys) provide an alternative credential option. These have no
requirement for a hostname and can thus be trivially used for UNIX
sockets.

Furthermore, with the ability to overide the default hostname for
TLS validation in the previous patch, it is now also valid to want
to use x509 certificates with FD passing and UNIX sockets.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 block/nbd.c    | 8 ++------
 blockdev-nbd.c | 6 ------
 qemu-nbd.c     | 8 +++-----
 3 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 113aa5d3af..3ede47dec9 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -1838,13 +1838,9 @@ static int nbd_process_options(BlockDriverState *bs, QDict *options,
             goto error;
         }
 
-        /* TODO SOCKET_ADDRESS_KIND_FD where fd has AF_INET or AF_INET6 */
-        if (s->saddr->type != SOCKET_ADDRESS_TYPE_INET) {
-            error_setg(errp, "TLS only supported over IP sockets");
-            goto error;
-        }
         s->tlshostname = g_strdup(qemu_opt_get(opts, "tls-hostname"));
-        if (!s->tlshostname) {
+        if (!s->tlshostname &&
+            s->saddr->type == SOCKET_ADDRESS_TYPE_INET) {
             s->tlshostname = g_strdup(s->saddr->u.inet.host);
         }
     }
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index bdfa7ed3a5..9840d25a82 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -148,12 +148,6 @@ void nbd_server_start(SocketAddress *addr, const char *tls_creds,
         if (!nbd_server->tlscreds) {
             goto error;
         }
-
-        /* TODO SOCKET_ADDRESS_TYPE_FD where fd has AF_INET or AF_INET6 */
-        if (addr->type != SOCKET_ADDRESS_TYPE_INET) {
-            error_setg(errp, "TLS is only supported with IPv4/IPv6");
-            goto error;
-        }
     }
 
     nbd_server->tlsauthz = g_strdup(tls_authz);
diff --git a/qemu-nbd.c b/qemu-nbd.c
index be8043fb00..f4c5b247de 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -808,7 +808,9 @@ int main(int argc, char **argv)
 
     socket_activation = check_socket_activation();
     if (socket_activation == 0) {
-        setup_address_and_port(&bindto, &port);
+        if (!sockpath) {
+            setup_address_and_port(&bindto, &port);
+        }
     } else {
         /* Using socket activation - check user didn't use -p etc. */
         const char *err_msg = socket_activation_validate_opts(device, sockpath,
@@ -829,10 +831,6 @@ int main(int argc, char **argv)
     }
 
     if (tlscredsid) {
-        if (sockpath) {
-            error_report("TLS is only supported with IPv4/IPv6");
-            exit(EXIT_FAILURE);
-        }
         if (device) {
             error_report("TLS is not supported with a host device");
             exit(EXIT_FAILURE);
-- 
2.34.1

[PATCH 06/12] tests/qemu-iotests: add QEMU_IOTESTS_REGEN=1 to update reference file

[Daniel P. Berrangé]

When developing an I/O test it is typical to add some logic to the
test script, run it to view the output diff, and then apply the
output diff to the reference file. This can be drastically simplified
by letting the test runner update the reference file in place.

By setting 'QEMU_IOTESTS_REGEN=1', the test runner will report the
failure and show the diff, but at the same time update the reference
file. So next time the I/O test is run it will succeed.

Continuing to display the diff when updating the reference gives the
developer a chance to review what was changed.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/qemu-iotests/testrunner.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
index 9a94273975..8a82696a6b 100644
--- a/tests/qemu-iotests/testrunner.py
+++ b/tests/qemu-iotests/testrunner.py
@@ -25,6 +25,7 @@
 import contextlib
 import json
 import termios
+import shutil
 import sys
 from multiprocessing import Pool
 from contextlib import contextmanager
@@ -320,6 +321,11 @@ def do_run_test(self, test: str, mp: bool) -> TestResult:
 
         diff = file_diff(str(f_reference), str(f_bad))
         if diff:
+            if os.environ.get("QEMU_IOTESTS_REGEN", None) is not None:
+                shutil.copyfile(str(f_bad), str(f_reference))
+                print("########################################")
+                print("#####    REFERENCE FILE UPDATED    #####")
+                print("########################################")
             return TestResult(status='fail', elapsed=elapsed,
                               description=f'output mismatch (see {f_bad})',
                               diff=diff, casenotrun=casenotrun)
-- 
2.34.1

[PATCH 07/12] tests/qemu-iotests: expand _filter_nbd rules

[Daniel P. Berrangé]

Some tests will want to use 'localhost' instead of '127.0.0.1', and
some will use the image options syntax rather than the classic URI
syntax.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/qemu-iotests/common.filter | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index 75cc241580..25d1d22929 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -300,6 +300,10 @@ _filter_nbd()
     # Filter out the TCP port number since this changes between runs.
     $SED -e '/nbd\/.*\.c:/d' \
         -e 's#127\.0\.0\.1:[0-9]*#127.0.0.1:PORT#g' \
+        -e 's#localhost:[0-9]*#localhost:PORT#g' \
+        -e 's#host=127\.0\.0\.1,port=[0-9]*#host=127.0.0.1,port=PORT#g' \
+        -e 's#host=localhost,port=[0-9]*#host=localhost,port=PORT#g' \
+        -e "s#path=$SOCK_DIR#path=SOCK_DIR#g" \
         -e "s#?socket=$SOCK_DIR#?socket=SOCK_DIR#g" \
         -e 's#\(foo\|PORT/\?\|.sock\): Failed to .*$#\1#'
 }
-- 
2.34.1

[PATCH 08/12] tests/qemu-iotests: introduce filter for qemu-nbd export list

[Daniel P. Berrangé]

Introduce a filter for the output of qemu-nbd export list so it can be
reused in multiple tests.

The filter is a bit more permissive that what test 241 currently uses,
as its allows printing of the export count, along with any possible
error messages that might be emitted.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/qemu-iotests/241           | 6 +++---
 tests/qemu-iotests/241.out       | 3 +++
 tests/qemu-iotests/common.filter | 5 +++++
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/241 b/tests/qemu-iotests/241
index c962c8b607..f196650afa 100755
--- a/tests/qemu-iotests/241
+++ b/tests/qemu-iotests/241
@@ -58,7 +58,7 @@ echo
 
 nbd_server_start_unix_socket -f $IMGFMT "$TEST_IMG_FILE"
 
-$QEMU_NBD_PROG --list -k $nbd_unix_socket | grep '\(size\|min\)'
+$QEMU_NBD_PROG --list -k $nbd_unix_socket | _filter_qemu_nbd_exports
 $QEMU_IMG map -f raw --output=json "$TEST_IMG" | _filter_qemu_img_map
 $QEMU_IO -f raw -c map "$TEST_IMG"
 nbd_server_stop
@@ -71,7 +71,7 @@ echo
 # sector alignment, here at the server.
 nbd_server_start_unix_socket "$TEST_IMG_FILE" 2> "$TEST_DIR/server.log"
 
-$QEMU_NBD_PROG --list -k $nbd_unix_socket | grep '\(size\|min\)'
+$QEMU_NBD_PROG --list -k $nbd_unix_socket | _filter_qemu_nbd_exports
 $QEMU_IMG map -f raw --output=json "$TEST_IMG" | _filter_qemu_img_map
 $QEMU_IO -f raw -c map "$TEST_IMG"
 nbd_server_stop
@@ -84,7 +84,7 @@ echo
 # Now force sector alignment at the client.
 nbd_server_start_unix_socket -f $IMGFMT "$TEST_IMG_FILE"
 
-$QEMU_NBD_PROG --list -k $nbd_unix_socket | grep '\(size\|min\)'
+$QEMU_NBD_PROG --list -k $nbd_unix_socket | _filter_qemu_nbd_exports
 $QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
 $QEMU_IO -c map "$TEST_IMG"
 nbd_server_stop
diff --git a/tests/qemu-iotests/241.out b/tests/qemu-iotests/241.out
index 56e95b599a..db2d71ab9d 100644
--- a/tests/qemu-iotests/241.out
+++ b/tests/qemu-iotests/241.out
@@ -2,6 +2,7 @@ QA output created by 241
 
 === Exporting unaligned raw image, natural alignment ===
 
+exports available: 1
   size:  1024
   min block: 1
 [{ "start": 0, "length": 1000, "depth": 0, "present": true, "zero": false, "data": true, "offset": OFFSET},
@@ -10,6 +11,7 @@ QA output created by 241
 
 === Exporting unaligned raw image, forced server sector alignment ===
 
+exports available: 1
   size:  1024
   min block: 512
 [{ "start": 0, "length": 1024, "depth": 0, "present": true, "zero": false, "data": true, "offset": OFFSET}]
@@ -20,6 +22,7 @@ WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed
 
 === Exporting unaligned raw image, forced client sector alignment ===
 
+exports available: 1
   size:  1024
   min block: 1
 [{ "start": 0, "length": 1000, "depth": 0, "present": true, "zero": false, "data": true, "offset": OFFSET},
diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index 25d1d22929..940c9884bd 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -308,6 +308,11 @@ _filter_nbd()
         -e 's#\(foo\|PORT/\?\|.sock\): Failed to .*$#\1#'
 }
 
+_filter_qemu_nbd_exports()
+{
+    grep '\(exports available\|size\|min block\|qemu-nbd\):'
+}
+
 _filter_qmp_empty_return()
 {
     grep -v '{"return": {}}'
-- 
2.34.1

[PATCH 09/12] tests/qemu-iotests: convert NBD TLS test to use standard filters

[Daniel P. Berrangé]

Using standard filters is more future proof than rolling our own.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/qemu-iotests/233     | 29 ++++++++++++++++-------------
 tests/qemu-iotests/233.out |  9 ---------
 2 files changed, 16 insertions(+), 22 deletions(-)

diff --git a/tests/qemu-iotests/233 b/tests/qemu-iotests/233
index 9ca7b68f42..050267298d 100755
--- a/tests/qemu-iotests/233
+++ b/tests/qemu-iotests/233
@@ -65,7 +65,7 @@ tls_x509_create_client "ca1" "client3"
 echo
 echo "== preparing image =="
 _make_test_img 64M
-$QEMU_IO -c 'w -P 0x11 1m 1m' "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c 'w -P 0x11 1m 1m' "$TEST_IMG" 2>&1 | _filter_qemu_io
 
 echo
 echo "== check TLS client to plain server fails =="
@@ -74,9 +74,9 @@ nbd_server_start_tcp_socket -f $IMGFMT "$TEST_IMG" 2> "$TEST_DIR/server.log"
 obj=tls-creds-x509,dir=${tls_dir}/client1,endpoint=client,id=tls0
 $QEMU_IMG info --image-opts --object $obj \
     driver=nbd,host=$nbd_tcp_addr,port=$nbd_tcp_port,tls-creds=tls0 \
-    2>&1 | sed "s/$nbd_tcp_port/PORT/g"
+    2>&1 | _filter_nbd
 $QEMU_NBD_PROG -L -b $nbd_tcp_addr -p $nbd_tcp_port --object $obj \
-    --tls-creds=tls0
+    --tls-creds=tls0 2>&1 | _filter_qemu_nbd_exports
 
 nbd_server_stop
 
@@ -88,8 +88,10 @@ nbd_server_start_tcp_socket \
     --tls-creds tls0 \
     -f $IMGFMT "$TEST_IMG" 2>> "$TEST_DIR/server.log"
 
-$QEMU_IMG info nbd://localhost:$nbd_tcp_port 2>&1 | sed "s/$nbd_tcp_port/PORT/g"
-$QEMU_NBD_PROG -L -b $nbd_tcp_addr -p $nbd_tcp_port
+$QEMU_IMG info nbd://localhost:$nbd_tcp_port \
+    2>&1 | _filter_nbd
+$QEMU_NBD_PROG -L -b $nbd_tcp_addr -p $nbd_tcp_port \
+    2>&1 | _filter_qemu_nbd_exports
 
 echo
 echo "== check TLS works =="
@@ -97,21 +99,21 @@ obj1=tls-creds-x509,dir=${tls_dir}/client1,endpoint=client,id=tls0
 obj2=tls-creds-x509,dir=${tls_dir}/client3,endpoint=client,id=tls0
 $QEMU_IMG info --image-opts --object $obj1 \
     driver=nbd,host=$nbd_tcp_addr,port=$nbd_tcp_port,tls-creds=tls0 \
-    2>&1 | sed "s/$nbd_tcp_port/PORT/g"
+    2>&1 | _filter_nbd
 $QEMU_IMG info --image-opts --object $obj2 \
     driver=nbd,host=$nbd_tcp_addr,port=$nbd_tcp_port,tls-creds=tls0 \
-    2>&1 | sed "s/$nbd_tcp_port/PORT/g"
+    2>&1 | _filter_nbd
 $QEMU_NBD_PROG -L -b $nbd_tcp_addr -p $nbd_tcp_port --object $obj1 \
-    --tls-creds=tls0
+    --tls-creds=tls0 2>&1 | _filter_qemu_nbd_exports
 
 echo
 echo "== check TLS with different CA fails =="
 obj=tls-creds-x509,dir=${tls_dir}/client2,endpoint=client,id=tls0
 $QEMU_IMG info --image-opts --object $obj \
     driver=nbd,host=$nbd_tcp_addr,port=$nbd_tcp_port,tls-creds=tls0 \
-    2>&1 | sed "s/$nbd_tcp_port/PORT/g"
+    2>&1 | _filter_nbd
 $QEMU_NBD_PROG -L -b $nbd_tcp_addr -p $nbd_tcp_port --object $obj \
-    --tls-creds=tls0
+    --tls-creds=tls0 2>&1 | _filter_qemu_nbd_exports
 
 echo
 echo "== perform I/O over TLS =="
@@ -121,7 +123,8 @@ $QEMU_IO -c 'r -P 0x11 1m 1m' -c 'w -P 0x22 1m 1m' --image-opts \
     driver=nbd,host=$nbd_tcp_addr,port=$nbd_tcp_port,tls-creds=tls0 \
     2>&1 | _filter_qemu_io
 
-$QEMU_IO -f $IMGFMT -r -U -c 'r -P 0x22 1m 1m' "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -f $IMGFMT -r -U -c 'r -P 0x22 1m 1m' "$TEST_IMG" \
+    2>&1 | _filter_qemu_io
 
 echo
 echo "== check TLS with authorization =="
@@ -139,12 +142,12 @@ nbd_server_start_tcp_socket \
 $QEMU_IMG info --image-opts \
     --object tls-creds-x509,dir=${tls_dir}/client1,endpoint=client,id=tls0 \
     driver=nbd,host=$nbd_tcp_addr,port=$nbd_tcp_port,tls-creds=tls0 \
-    2>&1 | sed "s/$nbd_tcp_port/PORT/g"
+    2>&1 | _filter_nbd
 
 $QEMU_IMG info --image-opts \
     --object tls-creds-x509,dir=${tls_dir}/client3,endpoint=client,id=tls0 \
     driver=nbd,host=$nbd_tcp_addr,port=$nbd_tcp_port,tls-creds=tls0 \
-    2>&1 | sed "s/$nbd_tcp_port/PORT/g"
+    2>&1 | _filter_nbd
 
 echo
 echo "== final server log =="
diff --git a/tests/qemu-iotests/233.out b/tests/qemu-iotests/233.out
index 4b1f6a0e15..a1e45765b8 100644
--- a/tests/qemu-iotests/233.out
+++ b/tests/qemu-iotests/233.out
@@ -17,15 +17,12 @@ wrote 1048576/1048576 bytes at offset 1048576
 qemu-img: Could not open 'driver=nbd,host=127.0.0.1,port=PORT,tls-creds=tls0': Denied by server for option 5 (starttls)
 server reported: TLS not configured
 qemu-nbd: Denied by server for option 5 (starttls)
-server reported: TLS not configured
 
 == check plain client to TLS server fails ==
 qemu-img: Could not open 'nbd://localhost:PORT': TLS negotiation required before option 7 (go)
 Did you forget a valid tls-creds?
 server reported: Option 0x7 not permitted before TLS
 qemu-nbd: TLS negotiation required before option 3 (list)
-Did you forget a valid tls-creds?
-server reported: Option 0x3 not permitted before TLS
 
 == check TLS works ==
 image: nbd://127.0.0.1:PORT
@@ -37,14 +34,8 @@ file format: nbd
 virtual size: 64 MiB (67108864 bytes)
 disk size: unavailable
 exports available: 1
- export: ''
   size:  67108864
-  flags: 0xced ( flush fua trim zeroes df cache fast-zero )
   min block: 1
-  opt block: 4096
-  max block: 33554432
-  available meta contexts: 1
-   base:allocation
 
 == check TLS with different CA fails ==
 qemu-img: Could not open 'driver=nbd,host=127.0.0.1,port=PORT,tls-creds=tls0': The certificate hasn't got a known issuer
-- 
2.34.1

[PATCH 10/12] tests/qemu-iotests: validate NBD TLS with hostname mismatch

[Daniel P. Berrangé]

This validates that connections to an NBD server where the certificate
hostname does not match will fail. It further validates that using the
new 'tls-hostname' override option can solve the failure.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/qemu-iotests/233        | 18 ++++++++++++++++++
 tests/qemu-iotests/233.out    | 15 +++++++++++++++
 tests/qemu-iotests/common.tls |  7 ++++---
 3 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/233 b/tests/qemu-iotests/233
index 050267298d..09cfb7039b 100755
--- a/tests/qemu-iotests/233
+++ b/tests/qemu-iotests/233
@@ -106,6 +106,24 @@ $QEMU_IMG info --image-opts --object $obj2 \
 $QEMU_NBD_PROG -L -b $nbd_tcp_addr -p $nbd_tcp_port --object $obj1 \
     --tls-creds=tls0 2>&1 | _filter_qemu_nbd_exports
 
+echo
+echo "== check TLS fail over TCP with mismatch hostname =="
+obj1=tls-creds-x509,dir=${tls_dir}/client1,endpoint=client,id=tls0
+$QEMU_IMG info --image-opts --object $obj1 \
+    driver=nbd,host=localhost,port=$nbd_tcp_port,tls-creds=tls0 \
+    2>&1 | _filter_nbd
+$QEMU_NBD_PROG -L -b localhost -p $nbd_tcp_port --object $obj1 \
+    --tls-creds=tls0 | _filter_qemu_nbd_exports
+
+echo
+echo "== check TLS works over TCP with mismatch hostname and override =="
+obj1=tls-creds-x509,dir=${tls_dir}/client1,endpoint=client,id=tls0
+$QEMU_IMG info --image-opts --object $obj1 \
+    driver=nbd,host=localhost,port=$nbd_tcp_port,tls-creds=tls0,tls-hostname=127.0.0.1 \
+    2>&1 | _filter_nbd
+$QEMU_NBD_PROG -L -b localhost -p $nbd_tcp_port --object $obj1 \
+    --tls-creds=tls0 --tls-hostname=127.0.0.1 | _filter_qemu_nbd_exports
+
 echo
 echo "== check TLS with different CA fails =="
 obj=tls-creds-x509,dir=${tls_dir}/client2,endpoint=client,id=tls0
diff --git a/tests/qemu-iotests/233.out b/tests/qemu-iotests/233.out
index a1e45765b8..05abf470ac 100644
--- a/tests/qemu-iotests/233.out
+++ b/tests/qemu-iotests/233.out
@@ -37,6 +37,19 @@ exports available: 1
   size:  67108864
   min block: 1
 
+== check TLS fail over TCP with mismatch hostname ==
+qemu-img: Could not open 'driver=nbd,host=localhost,port=PORT,tls-creds=tls0': Certificate does not match the hostname localhost
+qemu-nbd: Certificate does not match the hostname localhost
+
+== check TLS works over TCP with mismatch hostname and override ==
+image: nbd://localhost:PORT
+file format: nbd
+virtual size: 64 MiB (67108864 bytes)
+disk size: unavailable
+exports available: 1
+  size:  67108864
+  min block: 1
+
 == check TLS with different CA fails ==
 qemu-img: Could not open 'driver=nbd,host=127.0.0.1,port=PORT,tls-creds=tls0': The certificate hasn't got a known issuer
 qemu-nbd: The certificate hasn't got a known issuer
@@ -54,6 +67,8 @@ qemu-img: Could not open 'driver=nbd,host=127.0.0.1,port=PORT,tls-creds=tls0': F
 qemu-img: Could not open 'driver=nbd,host=127.0.0.1,port=PORT,tls-creds=tls0': Failed to read option reply: Cannot read from TLS channel: Software caused connection abort
 
 == final server log ==
+qemu-nbd: option negotiation failed: Failed to read opts magic: Cannot read from TLS channel: Software caused connection abort
+qemu-nbd: option negotiation failed: Failed to read opts magic: Cannot read from TLS channel: Software caused connection abort
 qemu-nbd: option negotiation failed: Verify failed: No certificate was found.
 qemu-nbd: option negotiation failed: Verify failed: No certificate was found.
 qemu-nbd: option negotiation failed: TLS x509 authz check for DISTINGUISHED-NAME is denied
diff --git a/tests/qemu-iotests/common.tls b/tests/qemu-iotests/common.tls
index 6ba28a78d3..4a5760949d 100644
--- a/tests/qemu-iotests/common.tls
+++ b/tests/qemu-iotests/common.tls
@@ -118,12 +118,13 @@ tls_x509_create_server()
     caname=$1
     name=$2
 
+    # We don't include 'localhost' in the cert, as
+    # we want to keep it unlisted to let tests
+    # validate hostname override
     mkdir -p "${tls_dir}/$name"
     cat > "${tls_dir}/cert.info" <<EOF
 organization = Cthulhu Dark Lord Enterprises $name
-cn = localhost
-dns_name = localhost
-dns_name = localhost.localdomain
+cn = iotests.qemu.org
 ip_address = 127.0.0.1
 ip_address = ::1
 tls_www_server
-- 
2.34.1

[PATCH 11/12] tests/qemu-iotests: validate NBD TLS with UNIX sockets

[Daniel P. Berrangé]

This validates that connections to an NBD server running on a UNIX
socket can use TLS, and require a TLS hostname override to pass
certificate validation.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/qemu-iotests/233     | 24 ++++++++++++++++++++++++
 tests/qemu-iotests/233.out | 15 +++++++++++++++
 2 files changed, 39 insertions(+)

diff --git a/tests/qemu-iotests/233 b/tests/qemu-iotests/233
index 09cfb7039b..27b0a123d3 100755
--- a/tests/qemu-iotests/233
+++ b/tests/qemu-iotests/233
@@ -167,6 +167,30 @@ $QEMU_IMG info --image-opts \
     driver=nbd,host=$nbd_tcp_addr,port=$nbd_tcp_port,tls-creds=tls0 \
     2>&1 | _filter_nbd
 
+nbd_server_stop
+
+nbd_server_start_unix_socket \
+    --object tls-creds-x509,dir=${tls_dir}/server1,endpoint=server,id=tls0,verify-peer=on \
+    --tls-creds tls0 \
+    -f $IMGFMT "$TEST_IMG" 2>> "$TEST_DIR/server.log"
+
+echo
+echo "== check TLS fail over UNIX with no hostname =="
+obj1=tls-creds-x509,dir=${tls_dir}/client1,endpoint=client,id=tls0
+$QEMU_IMG info --image-opts --object $obj1 \
+    driver=nbd,path=$nbd_unix_socket,tls-creds=tls0 2>&1 | _filter_nbd
+$QEMU_NBD_PROG -L -k $nbd_unix_socket --object $obj1 --tls-creds=tls0 \
+    2>&1 | _filter_qemu_nbd_exports
+
+echo
+echo "== check TLS works over UNIX with hostname override =="
+obj1=tls-creds-x509,dir=${tls_dir}/client1,endpoint=client,id=tls0
+$QEMU_IMG info --image-opts --object $obj1 \
+    driver=nbd,path=$nbd_unix_socket,tls-creds=tls0,tls-hostname=127.0.0.1 \
+    2>&1 | _filter_nbd
+$QEMU_NBD_PROG -L -k $nbd_unix_socket --object $obj1 \
+    --tls-creds=tls0 --tls-hostname=127.0.0.1  2>&1 | _filter_qemu_nbd_exports
+
 echo
 echo "== final server log =="
 cat "$TEST_DIR/server.log" | _filter_authz_check_tls
diff --git a/tests/qemu-iotests/233.out b/tests/qemu-iotests/233.out
index 05abf470ac..a00e4c5b08 100644
--- a/tests/qemu-iotests/233.out
+++ b/tests/qemu-iotests/233.out
@@ -66,6 +66,19 @@ read 1048576/1048576 bytes at offset 1048576
 qemu-img: Could not open 'driver=nbd,host=127.0.0.1,port=PORT,tls-creds=tls0': Failed to read option reply: Cannot read from TLS channel: Software caused connection abort
 qemu-img: Could not open 'driver=nbd,host=127.0.0.1,port=PORT,tls-creds=tls0': Failed to read option reply: Cannot read from TLS channel: Software caused connection abort
 
+== check TLS fail over UNIX with no hostname ==
+qemu-img: Could not open 'driver=nbd,path=SOCK_DIR/qemu-nbd.sock,tls-creds=tls0': No hostname for certificate validation
+qemu-nbd: No hostname for certificate validation
+
+== check TLS works over UNIX with hostname override ==
+image: nbd+unix://?socket=SOCK_DIR/qemu-nbd.sock
+file format: nbd
+virtual size: 64 MiB (67108864 bytes)
+disk size: unavailable
+exports available: 1
+  size:  67108864
+  min block: 1
+
 == final server log ==
 qemu-nbd: option negotiation failed: Failed to read opts magic: Cannot read from TLS channel: Software caused connection abort
 qemu-nbd: option negotiation failed: Failed to read opts magic: Cannot read from TLS channel: Software caused connection abort
@@ -73,4 +86,6 @@ qemu-nbd: option negotiation failed: Verify failed: No certificate was found.
 qemu-nbd: option negotiation failed: Verify failed: No certificate was found.
 qemu-nbd: option negotiation failed: TLS x509 authz check for DISTINGUISHED-NAME is denied
 qemu-nbd: option negotiation failed: TLS x509 authz check for DISTINGUISHED-NAME is denied
+qemu-nbd: option negotiation failed: Failed to read opts magic: Cannot read from TLS channel: Software caused connection abort
+qemu-nbd: option negotiation failed: Failed to read opts magic: Cannot read from TLS channel: Software caused connection abort
 *** done
-- 
2.34.1

[PATCH 12/12] tests/qemu-iotests: validate NBD TLS with UNIX sockets and PSK

[Daniel P. Berrangé]

This validates that connections to an NBD server running on a UNIX
socket can use TLS with pre-shared keys (PSK).

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/qemu-iotests/233        | 28 ++++++++++++++++++++++++++++
 tests/qemu-iotests/233.out    | 17 +++++++++++++++++
 tests/qemu-iotests/common.tls | 24 ++++++++++++++++++++++++
 3 files changed, 69 insertions(+)

diff --git a/tests/qemu-iotests/233 b/tests/qemu-iotests/233
index 27b0a123d3..0488f3bbef 100755
--- a/tests/qemu-iotests/233
+++ b/tests/qemu-iotests/233
@@ -61,6 +61,8 @@ tls_x509_create_server "ca1" "server1"
 tls_x509_create_client "ca1" "client1"
 tls_x509_create_client "ca2" "client2"
 tls_x509_create_client "ca1" "client3"
+tls_psk_create_creds "psk1"
+tls_psk_create_creds "psk2"
 
 echo
 echo "== preparing image =="
@@ -191,6 +193,32 @@ $QEMU_IMG info --image-opts --object $obj1 \
 $QEMU_NBD_PROG -L -k $nbd_unix_socket --object $obj1 \
     --tls-creds=tls0 --tls-hostname=127.0.0.1  2>&1 | _filter_qemu_nbd_exports
 
+
+echo
+echo "== check TLS works over UNIX with PSK =="
+nbd_server_stop
+
+nbd_server_start_unix_socket \
+    --object tls-creds-psk,dir=${tls_dir}/psk1,endpoint=server,id=tls0,verify-peer=on \
+    --tls-creds tls0 \
+    -f $IMGFMT "$TEST_IMG" 2>> "$TEST_DIR/server.log"
+
+obj1=tls-creds-psk,dir=${tls_dir}/psk1,username=psk1,endpoint=client,id=tls0
+$QEMU_IMG info --image-opts --object $obj1 \
+    driver=nbd,path=$nbd_unix_socket,tls-creds=tls0 \
+    2>&1 | _filter_nbd
+$QEMU_NBD_PROG -L -k $nbd_unix_socket --object $obj1 \
+    --tls-creds=tls0 2>&1 | _filter_qemu_nbd_exports
+
+echo
+echo "== check TLS fails over UNIX with mismatch PSK =="
+obj1=tls-creds-psk,dir=${tls_dir}/psk2,username=psk2,endpoint=client,id=tls0
+$QEMU_IMG info --image-opts --object $obj1 \
+    driver=nbd,path=$nbd_unix_socket,tls-creds=tls0 \
+    2>&1 | _filter_nbd
+$QEMU_NBD_PROG -L -k $nbd_unix_socket --object $obj1 \
+    --tls-creds=tls0 2>&1 | _filter_qemu_nbd_exports
+
 echo
 echo "== final server log =="
 cat "$TEST_DIR/server.log" | _filter_authz_check_tls
diff --git a/tests/qemu-iotests/233.out b/tests/qemu-iotests/233.out
index a00e4c5b08..ecb36a2f97 100644
--- a/tests/qemu-iotests/233.out
+++ b/tests/qemu-iotests/233.out
@@ -7,6 +7,8 @@ Generating a signed certificate...
 Generating a signed certificate...
 Generating a signed certificate...
 Generating a signed certificate...
+Generating a random key for user 'psk1'
+Generating a random key for user 'psk2'
 
 == preparing image ==
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
@@ -79,6 +81,19 @@ exports available: 1
   size:  67108864
   min block: 1
 
+== check TLS works over UNIX with PSK ==
+image: nbd+unix://?socket=SOCK_DIR/qemu-nbd.sock
+file format: nbd
+virtual size: 64 MiB (67108864 bytes)
+disk size: unavailable
+exports available: 1
+  size:  67108864
+  min block: 1
+
+== check TLS fails over UNIX with mismatch PSK ==
+qemu-img: Could not open 'driver=nbd,path=SOCK_DIR/qemu-nbd.sock,tls-creds=tls0': TLS handshake failed: The TLS connection was non-properly terminated.
+qemu-nbd: TLS handshake failed: The TLS connection was non-properly terminated.
+
 == final server log ==
 qemu-nbd: option negotiation failed: Failed to read opts magic: Cannot read from TLS channel: Software caused connection abort
 qemu-nbd: option negotiation failed: Failed to read opts magic: Cannot read from TLS channel: Software caused connection abort
@@ -88,4 +103,6 @@ qemu-nbd: option negotiation failed: TLS x509 authz check for DISTINGUISHED-NAME
 qemu-nbd: option negotiation failed: TLS x509 authz check for DISTINGUISHED-NAME is denied
 qemu-nbd: option negotiation failed: Failed to read opts magic: Cannot read from TLS channel: Software caused connection abort
 qemu-nbd: option negotiation failed: Failed to read opts magic: Cannot read from TLS channel: Software caused connection abort
+qemu-nbd: option negotiation failed: TLS handshake failed: An illegal parameter has been received.
+qemu-nbd: option negotiation failed: TLS handshake failed: An illegal parameter has been received.
 *** done
diff --git a/tests/qemu-iotests/common.tls b/tests/qemu-iotests/common.tls
index 4a5760949d..b9c5462986 100644
--- a/tests/qemu-iotests/common.tls
+++ b/tests/qemu-iotests/common.tls
@@ -24,6 +24,7 @@ tls_x509_cleanup()
 {
     rm -f "${tls_dir}"/*.pem
     rm -f "${tls_dir}"/*/*.pem
+    rm -f "${tls_dir}"/*/*.psk
     rmdir "${tls_dir}"/*
     rmdir "${tls_dir}"
 }
@@ -40,6 +41,18 @@ tls_certtool()
     rm -f "${tls_dir}"/certtool.log
 }
 
+tls_psktool()
+{
+    psktool "$@" 1>"${tls_dir}"/psktool.log 2>&1
+    if test "$?" = 0; then
+      head -1 "${tls_dir}"/psktool.log
+    else
+      cat "${tls_dir}"/psktool.log
+    fi
+    rm -f "${tls_dir}"/psktool.log
+}
+
+
 tls_x509_init()
 {
     (certtool --help) >/dev/null 2>&1 || \
@@ -176,3 +189,14 @@ EOF
 
     rm -f "${tls_dir}/cert.info"
 }
+
+tls_psk_create_creds()
+{
+    name=$1
+
+    mkdir -p "${tls_dir}/$name"
+
+    tls_psktool \
+	--pskfile "${tls_dir}/$name/keys.psk" \
+	--username "$name"
+}
-- 
2.34.1

Re: [PATCH 01/12] crypto: mandate a hostname when checking x509 creds on a client

[Eric Blake]

On Thu, Mar 03, 2022 at 04:03:19PM +0000, Daniel P. Berrangé wrote:
> Currently the TLS session object assumes that the caller will always
> provide a hostname when using x509 creds on a client endpoint. This
> relies on the caller to detect and report an error if the user has
> configured QEMU with x509 credentials on a UNIX socket. The migration
> code has such a check, but it is too broad, reporting an error when
> the user has configured QEMU with PSK credentials on a UNIX socket,
> where hostnames are irrelevant.
> 
> Putting the check into the TLS session object credentials validation
> code ensures we report errors in only the scenario that matters.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  crypto/tlssession.c | 6 ++++++
>  1 file changed, 6 insertions(+)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 02/12] block: pass desired TLS hostname through from block driver client

[Eric Blake]

On Thu, Mar 03, 2022 at 04:03:20PM +0000, Daniel P. Berrangé wrote:
> In
> 
>   commit a71d597b989fd701b923f09b3c20ac4fcaa55e81
>   Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>   Date:   Thu Jun 10 13:08:00 2021 +0300
> 
>     block/nbd: reuse nbd_co_do_establish_connection() in nbd_open()
> 
> the use of the 'hostname' field from the BDRVNBDState struct was
> lost, and 'nbd_connect' just hardcoded it to match the IP socket
> address. This was a harmless bug at the time since we block use
> with anything other than IP sockets.
> 
> Shortly though, We want to allow the caller to override the hostname

s/We/we/

> used in the TLS certificate checks. This is to allow for TLS
> when doing port forwarding or tunneling. Thus we need to reinstate
> the passing along of the 'hostname'.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  block/nbd.c             |  7 ++++---
>  include/block/nbd.h     |  3 ++-
>  nbd/client-connection.c | 12 +++++++++---
>  3 files changed, 15 insertions(+), 7 deletions(-)

Nice - this a great step towards fixing a longstanding annoyance of
mine that libnbd and nbdkit support TLS over Unix sockets, but qemu
didn't.

> @@ -1875,7 +1875,8 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
>      }
>  
>      s->conn = nbd_client_connection_new(s->saddr, true, s->export,
> -                                        s->x_dirty_bitmap, s->tlscreds);
> +                                        s->x_dirty_bitmap, s->tlscreds,
> +                                        s->tlshostname);
>  
>      if (s->open_timeout) {
>          nbd_client_connection_enable_retry(s->conn);
> diff --git a/include/block/nbd.h b/include/block/nbd.h
> index 78d101b774..a98eb665da 100644
> --- a/include/block/nbd.h
> +++ b/include/block/nbd.h
> @@ -415,7 +415,8 @@ NBDClientConnection *nbd_client_connection_new(const SocketAddress *saddr,
>                                                 bool do_negotiation,
>                                                 const char *export_name,
>                                                 const char *x_dirty_bitmap,
> -                                               QCryptoTLSCreds *tlscreds);
> +                                               QCryptoTLSCreds *tlscreds,
> +                                               const char *tlshostname);

We already have a lot of parameters; does it make sense to bundle
tlshostname into the QCryptoTLSCreds struct at all?  But that would
change the QAPI (or maybe you do it later in the series), it is not a
show-stopper to this patch.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 03/12] block/nbd: support override of hostname for TLS certificate validation

[Eric Blake]

On Thu, Mar 03, 2022 at 04:03:21PM +0000, Daniel P. Berrangé wrote:
> When connecting to an NBD server with TLS and x509 credentials,
> the client must validate the hostname it uses for the connection,
> against that published in the server's certificate. If the client
> is tunnelling its connection over some other channel, however, the
> hostname it uses may not match the info reported in the server's
> certificate. In such a case, the user needs to explicitly set an
> override for the hostname to use for certificate validation.
> 
> This is achieved by adding a 'tls-hostname' property to the NBD
> block driver.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  block/nbd.c          | 18 +++++++++++++++---
>  qapi/block-core.json |  3 +++
>  2 files changed, 18 insertions(+), 3 deletions(-)
> 
> +++ b/qapi/block-core.json
> @@ -4078,6 +4078,8 @@
>  #
>  # @tls-creds: TLS credentials ID
>  #
> +# @tls-hostname: TLS hostname override for certificate validation

Add the tag '(since 7.0)' (in the interest of soft freeze deadlines, I
can do that as part of queuing through my NBD tree if nothing else
major turns up in the series), and you can have:

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 04/12] qemu-nbd: add --tls-hostname option for TLS certificate validation

[Eric Blake]

On Thu, Mar 03, 2022 at 04:03:22PM +0000, Daniel P. Berrangé wrote:
> When using the --list option, qemu-nbd acts as an NBD client rather
> than a server. As such when using TLS, it has a need to validate
> the server certificate. This adds a --tls-hostname option which can
> be used to override the default hostname used for certificate
> validation.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  docs/tools/qemu-nbd.rst | 14 ++++++++++++++
>  qemu-nbd.c              | 17 ++++++++++++++++-
>  2 files changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/tools/qemu-nbd.rst b/docs/tools/qemu-nbd.rst
> index 6031f96893..acce54a39d 100644
> --- a/docs/tools/qemu-nbd.rst
> +++ b/docs/tools/qemu-nbd.rst
> @@ -169,6 +169,20 @@ driver options if ``--image-opts`` is specified.
>    option; or provide the credentials needed for connecting as a client
>    in list mode.
>  
> +.. option:: --tls-hostname=hostname
> +
> +  When validating an x509 certificate received over a TLS connection,
> +  the hostname that the NBD client used to connect will be checked
> +  against information in the server provided certificate. Sometimes
> +  it might be required to override the hostname used to perform this
> +  check. For example if the NBD client is using a tunnel from localhost
> +  to connect to the remote server. In this case the `--tls-hostname`

For example, if the ... to the remote server, the `--tls-hostname`


> +  option should be used to set the officially expected hostname of
> +  the remote NBD server. This can also be used if accessing NBD over
> +  a UNIX socket where there is no inherant hostname available. This

inherent

> +  only is only permitted when acting as a NBD client with the `--list`

s/only is/is/

> +  option.
> +
>  .. option:: --fork
>  

> @@ -835,6 +841,10 @@ int main(int argc, char **argv)
>              error_report("TLS authorization is incompatible with export list");
>              exit(EXIT_FAILURE);
>          }
> +        if (tlshostname && !list) {
> +            error_report("TLS hostname is only required with export list");

maybe s/required/supported/

> +            exit(EXIT_FAILURE);
> +        }
>          tlscreds = nbd_get_tls_creds(tlscredsid, list, &local_err);
>          if (local_err) {
>              error_reportf_err(local_err, "Failed to get TLS creds: ");
> @@ -845,6 +855,10 @@ int main(int argc, char **argv)
>              error_report("--tls-authz is not permitted without --tls-creds");
>              exit(EXIT_FAILURE);
>          }
> +        if (tlshostname) {
> +            error_report("--tls-hostname is not permitted without --tls-creds");
> +            exit(EXIT_FAILURE);
> +        }
>      }
>  
>      if (selinux_label) {
> @@ -861,7 +875,8 @@ int main(int argc, char **argv)
>  
>      if (list) {
>          saddr = nbd_build_socket_address(sockpath, bindto, port);
> -        return qemu_nbd_client_list(saddr, tlscreds, bindto);
> +        return qemu_nbd_client_list(saddr, tlscreds,
> +                                    tlshostname ? tlshostname : bindto);

With the grammar fixes,

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 05/12] block/nbd: don't restrict TLS usage to IP sockets

[Eric Blake]

On Thu, Mar 03, 2022 at 04:03:23PM +0000, Daniel P. Berrangé wrote:
> The TLS usage for NBD was restricted to IP sockets because validating
> x509 certificates requires knowledge of the hostname that the client
> is connecting to.
> 
> TLS does not have to use x509 certificates though, as PSK (pre-shared
> keys) provide an alternative credential option. These have no
> requirement for a hostname and can thus be trivially used for UNIX
> sockets.
> 
> Furthermore, with the ability to overide the default hostname for
> TLS validation in the previous patch, it is now also valid to want
> to use x509 certificates with FD passing and UNIX sockets.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  block/nbd.c    | 8 ++------
>  blockdev-nbd.c | 6 ------
>  qemu-nbd.c     | 8 +++-----
>  3 files changed, 5 insertions(+), 17 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 06/12] tests/qemu-iotests: add QEMU_IOTESTS_REGEN=1 to update reference file

[Eric Blake]

On Thu, Mar 03, 2022 at 04:03:24PM +0000, Daniel P. Berrangé wrote:
> When developing an I/O test it is typical to add some logic to the
> test script, run it to view the output diff, and then apply the
> output diff to the reference file. This can be drastically simplified
> by letting the test runner update the reference file in place.
> 
> By setting 'QEMU_IOTESTS_REGEN=1', the test runner will report the
> failure and show the diff, but at the same time update the reference
> file. So next time the I/O test is run it will succeed.
> 
> Continuing to display the diff when updating the reference gives the
> developer a chance to review what was changed.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  tests/qemu-iotests/testrunner.py | 6 ++++++
>  1 file changed, 6 insertions(+)

I like it - I've wished for similar when writing a test.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 07/12] tests/qemu-iotests: expand _filter_nbd rules

[Eric Blake]

On Thu, Mar 03, 2022 at 04:03:25PM +0000, Daniel P. Berrangé wrote:
> Some tests will want to use 'localhost' instead of '127.0.0.1', and
> some will use the image options syntax rather than the classic URI
> syntax.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  tests/qemu-iotests/common.filter | 4 ++++
>  1 file changed, 4 insertions(+)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 08/12] tests/qemu-iotests: introduce filter for qemu-nbd export list

[Eric Blake]

On Thu, Mar 03, 2022 at 04:03:26PM +0000, Daniel P. Berrangé wrote:
> Introduce a filter for the output of qemu-nbd export list so it can be
> reused in multiple tests.
> 
> The filter is a bit more permissive that what test 241 currently uses,
> as its allows printing of the export count, along with any possible
> error messages that might be emitted.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  tests/qemu-iotests/241           | 6 +++---
>  tests/qemu-iotests/241.out       | 3 +++
>  tests/qemu-iotests/common.filter | 5 +++++
>  3 files changed, 11 insertions(+), 3 deletions(-)
>

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 09/12] tests/qemu-iotests: convert NBD TLS test to use standard filters

[Eric Blake]

On Thu, Mar 03, 2022 at 04:03:27PM +0000, Daniel P. Berrangé wrote:
> Using standard filters is more future proof than rolling our own.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  tests/qemu-iotests/233     | 29 ++++++++++++++++-------------
>  tests/qemu-iotests/233.out |  9 ---------
>  2 files changed, 16 insertions(+), 22 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 10/12] tests/qemu-iotests: validate NBD TLS with hostname mismatch

[Eric Blake]

On Thu, Mar 03, 2022 at 04:03:28PM +0000, Daniel P. Berrangé wrote:
> This validates that connections to an NBD server where the certificate
> hostname does not match will fail. It further validates that using the
> new 'tls-hostname' override option can solve the failure.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  tests/qemu-iotests/233        | 18 ++++++++++++++++++
>  tests/qemu-iotests/233.out    | 15 +++++++++++++++
>  tests/qemu-iotests/common.tls |  7 ++++---
>  3 files changed, 37 insertions(+), 3 deletions(-)

> 
> diff --git a/tests/qemu-iotests/233 b/tests/qemu-iotests/233
> index 050267298d..09cfb7039b 100755
> --- a/tests/qemu-iotests/233
> +++ b/tests/qemu-iotests/233
> @@ -106,6 +106,24 @@ $QEMU_IMG info --image-opts --object $obj2 \
>  $QEMU_NBD_PROG -L -b $nbd_tcp_addr -p $nbd_tcp_port --object $obj1 \
>      --tls-creds=tls0 2>&1 | _filter_qemu_nbd_exports
>  
> +echo
> +echo "== check TLS fail over TCP with mismatch hostname =="

Maybe s/mismatch/mismatched/, with corresponding .out tweaks.

> +obj1=tls-creds-x509,dir=${tls_dir}/client1,endpoint=client,id=tls0
> +$QEMU_IMG info --image-opts --object $obj1 \
> +    driver=nbd,host=localhost,port=$nbd_tcp_port,tls-creds=tls0 \
> +    2>&1 | _filter_nbd
> +$QEMU_NBD_PROG -L -b localhost -p $nbd_tcp_port --object $obj1 \
> +    --tls-creds=tls0 | _filter_qemu_nbd_exports
> +
> +echo
> +echo "== check TLS works over TCP with mismatch hostname and override =="
> +obj1=tls-creds-x509,dir=${tls_dir}/client1,endpoint=client,id=tls0
> +$QEMU_IMG info --image-opts --object $obj1 \
> +    driver=nbd,host=localhost,port=$nbd_tcp_port,tls-creds=tls0,tls-hostname=127.0.0.1 \

And hence your need to filter both 127.0.0.1 and localhost.  Cute way
to force hostname mismatch without needing two hosts.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 11/12] tests/qemu-iotests: validate NBD TLS with UNIX sockets

[Eric Blake]

On Thu, Mar 03, 2022 at 04:03:29PM +0000, Daniel P. Berrangé wrote:
> This validates that connections to an NBD server running on a UNIX
> socket can use TLS, and require a TLS hostname override to pass
> certificate validation.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  tests/qemu-iotests/233     | 24 ++++++++++++++++++++++++
>  tests/qemu-iotests/233.out | 15 +++++++++++++++
>  2 files changed, 39 insertions(+)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 12/12] tests/qemu-iotests: validate NBD TLS with UNIX sockets and PSK

[Eric Blake]

On Thu, Mar 03, 2022 at 04:03:30PM +0000, Daniel P. Berrangé wrote:
> This validates that connections to an NBD server running on a UNIX
> socket can use TLS with pre-shared keys (PSK).
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  tests/qemu-iotests/233        | 28 ++++++++++++++++++++++++++++
>  tests/qemu-iotests/233.out    | 17 +++++++++++++++++
>  tests/qemu-iotests/common.tls | 24 ++++++++++++++++++++++++
>  3 files changed, 69 insertions(+)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 02/12] block: pass desired TLS hostname through from block driver client

[Daniel P. Berrangé]

On Thu, Mar 03, 2022 at 02:14:34PM -0600, Eric Blake wrote:
> On Thu, Mar 03, 2022 at 04:03:20PM +0000, Daniel P. Berrangé wrote:
> > In
> > 
> >   commit a71d597b989fd701b923f09b3c20ac4fcaa55e81
> >   Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> >   Date:   Thu Jun 10 13:08:00 2021 +0300
> > 
> >     block/nbd: reuse nbd_co_do_establish_connection() in nbd_open()
> > 
> > the use of the 'hostname' field from the BDRVNBDState struct was
> > lost, and 'nbd_connect' just hardcoded it to match the IP socket
> > address. This was a harmless bug at the time since we block use
> > with anything other than IP sockets.
> > 
> > Shortly though, We want to allow the caller to override the hostname
> 
> s/We/we/
> 
> > used in the TLS certificate checks. This is to allow for TLS
> > when doing port forwarding or tunneling. Thus we need to reinstate
> > the passing along of the 'hostname'.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  block/nbd.c             |  7 ++++---
> >  include/block/nbd.h     |  3 ++-
> >  nbd/client-connection.c | 12 +++++++++---
> >  3 files changed, 15 insertions(+), 7 deletions(-)
> 
> Nice - this a great step towards fixing a longstanding annoyance of
> mine that libnbd and nbdkit support TLS over Unix sockets, but qemu
> didn't.


> > diff --git a/include/block/nbd.h b/include/block/nbd.h
> > index 78d101b774..a98eb665da 100644
> > --- a/include/block/nbd.h
> > +++ b/include/block/nbd.h
> > @@ -415,7 +415,8 @@ NBDClientConnection *nbd_client_connection_new(const SocketAddress *saddr,
> >                                                 bool do_negotiation,
> >                                                 const char *export_name,
> >                                                 const char *x_dirty_bitmap,
> > -                                               QCryptoTLSCreds *tlscreds);
> > +                                               QCryptoTLSCreds *tlscreds,
> > +                                               const char *tlshostname);
> 
> We already have a lot of parameters; does it make sense to bundle
> tlshostname into the QCryptoTLSCreds struct at all?  But that would
> change the QAPI (or maybe you do it later in the series), it is not a
> show-stopper to this patch.

The credentials object is something that can be used for multiple
connections. The TLS hostname override meanwhile is specific to
a single connection. Thus it would not be appropriate to store the
TLS hostname in the credentials struct.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 08/12] tests/qemu-iotests: introduce filter for qemu-nbd export list

[Daniel P. Berrangé]

On Fri, Mar 04, 2022 at 10:43:45AM -0600, Eric Blake wrote:
> On Thu, Mar 03, 2022 at 04:03:26PM +0000, Daniel P. Berrangé wrote:
> > Introduce a filter for the output of qemu-nbd export list so it can be
> > reused in multiple tests.
> > 
> > The filter is a bit more permissive that what test 241 currently uses,
> > as its allows printing of the export count, along with any possible
> > error messages that might be emitted.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  tests/qemu-iotests/241           | 6 +++---
> >  tests/qemu-iotests/241.out       | 3 +++
> >  tests/qemu-iotests/common.filter | 5 +++++
> >  3 files changed, 11 insertions(+), 3 deletions(-)
> >
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>

I'm going to post a v2 with a slight tweak to expose one more
interesting piece of (stable) info in the output:

diff --git a/tests/qemu-iotests/241.out b/tests/qemu-iotests/241.out
index db2d71ab9d..88e8cfcd7e 100644
--- a/tests/qemu-iotests/241.out
+++ b/tests/qemu-iotests/241.out
@@ -3,6 +3,7 @@ QA output created by 241
 === Exporting unaligned raw image, natural alignment ===
 
 exports available: 1
+ export: ''
   size:  1024
   min block: 1
 [{ "start": 0, "length": 1000, "depth": 0, "present": true, "zero": false, "data": true, "offset": OFFSET},
@@ -12,6 +13,7 @@ exports available: 1
 === Exporting unaligned raw image, forced server sector alignment ===
 
 exports available: 1
+ export: ''
   size:  1024
   min block: 512
 [{ "start": 0, "length": 1024, "depth": 0, "present": true, "zero": false, "data": true, "offset": OFFSET}]
@@ -23,6 +25,7 @@ WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed
 === Exporting unaligned raw image, forced client sector alignment ===
 
 exports available: 1
+ export: ''
   size:  1024
   min block: 1
 [{ "start": 0, "length": 1000, "depth": 0, "present": true, "zero": false, "data": true, "offset": OFFSET},
diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index 940c9884bd..14b6f80dcb 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -310,7 +310,7 @@ _filter_nbd()
 
 _filter_qemu_nbd_exports()
 {
-    grep '\(exports available\|size\|min block\|qemu-nbd\):'
+    grep '\(exports available\|export\|size\|min block\|qemu-nbd\):'
 }
 
 _filter_qmp_empty_return()


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|