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

Daniel P. Berrangé posted 12 patches 3 years, 11 months ago
Maintainers: Eric Blake <eblake@redhat.com>, Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Markus Armbruster <armbru@redhat.com>
There is a newer version of this series
[PATCH 03/12] block/nbd: support override of hostname for TLS certificate validation
Posted by Daniel P. Berrangé 3 years, 11 months ago
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


Re: [PATCH 03/12] block/nbd: support override of hostname for TLS certificate validation
Posted by Eric Blake 3 years, 11 months ago
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