While QEMU accepts and interprets an empty string in the tls-hostname
field in migration parametes as if it's unset, the same does not apply
for the 'tls-hostname' field when 'blockdev-add'-ing a NBD backend for
non-shared storage migration.

When libvirt sets up migation with TLS in 'qemuMigrationParamsEnableTLS'
the QEMU_MIGRATION_PARAM_TLS_HOSTNAME migration parameter will be set to
empty string in case when the 'hostname' argument is passed as NULL.

Later on when setting up the NBD connections for non-shared storage
migration 'qemuMigrationParamsGetTLSHostname', which fetches the value
of the aforementioned TLS parameter.

This bug was mostly latent until recently as libvirt used
MIGRATION_DEST_CONNECT_HOST mode in most cases which required the
hostname to be passed, thus the parameter was set properly.

This changed with 8d693d79c40 for post-copy migration, where libvirt now
instructs qemu to connect and thus passes NULL hostname to
qemuMigrationParamsEnableTLS, which in turn causes libvirt to try to
add NBD connection with empty string as tls-hostname resulting in:

  error: internal error: unable to execute QEMU command 'blockdev-add': Certificate does not match the hostname

To address this modify 'qemuMigrationParamsGetTLSHostname' to undo the
weird semantics the migration code uses to handle TLS hostname and make
it return NULL if the hostname is an empty string.

Fixes: e8fa09d66bc
Resolves: https://issues.redhat.com/browse/RHEL-32880
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_migration_params.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c
index e955822f68..48f8657f71 100644
--- a/src/qemu/qemu_migration_params.c
+++ b/src/qemu/qemu_migration_params.c
@@ -1158,6 +1158,7 @@ qemuMigrationParamsEnableTLS(virQEMUDriver *driver,
                                      *tlsAlias) < 0)
         return -1;

+    /* QEMU interprets an empty string for hostname as if it is not populated */
     if (!migParams->params[QEMU_MIGRATION_PARAM_TLS_HOSTNAME].set &&
         qemuMigrationParamsSetString(migParams,
                                      QEMU_MIGRATION_PARAM_TLS_HOSTNAME,
@@ -1659,13 +1660,23 @@ qemuMigrationCapsGet(virDomainObj *vm,
  * @migParams: Migration params object
  *
  * Fetches the value of the QEMU_MIGRATION_PARAM_TLS_HOSTNAME parameter which is
- * passed from the user as VIR_MIGRATE_PARAM_TLS_DESTINATION
+ * passed from the user as VIR_MIGRATE_PARAM_TLS_DESTINATION.
+ *
+ * In contrast with the migration parameter semantics, where an empty string
+ * is considered as if the hostname was not provided, this function will return
+ * NULL instead of an empty string as other parts of QEMU expect that the
+ * hostname is not provided at all.
  */
 const char *
 qemuMigrationParamsGetTLSHostname(qemuMigrationParams *migParams)
 {
+    const char *hostname = migParams->params[QEMU_MIGRATION_PARAM_TLS_HOSTNAME].value.s;
+
     if (!migParams->params[QEMU_MIGRATION_PARAM_TLS_HOSTNAME].set)
         return NULL;

-    return migParams->params[QEMU_MIGRATION_PARAM_TLS_HOSTNAME].value.s;
+    if (STREQ(hostname, ""))
+        return NULL;
+
+    return hostname;
 }
-- 
2.44.0
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org

On Fri, Apr 19, 2024 at 16:29:47 +0200, Peter Krempa wrote:
> While QEMU accepts and interprets an empty string in the tls-hostname
> field in migration parametes as if it's unset, the same does not apply
> for the 'tls-hostname' field when 'blockdev-add'-ing a NBD backend for
> non-shared storage migration.
> 
> When libvirt sets up migation with TLS in 'qemuMigrationParamsEnableTLS'
> the QEMU_MIGRATION_PARAM_TLS_HOSTNAME migration parameter will be set to
> empty string in case when the 'hostname' argument is passed as NULL.
> 
> Later on when setting up the NBD connections for non-shared storage
> migration 'qemuMigrationParamsGetTLSHostname', which fetches the value
> of the aforementioned TLS parameter.
> 
> This bug was mostly latent until recently as libvirt used
> MIGRATION_DEST_CONNECT_HOST mode in most cases which required the
> hostname to be passed, thus the parameter was set properly.
> 
> This changed with 8d693d79c40 for post-copy migration, where libvirt now
> instructs qemu to connect and thus passes NULL hostname to
> qemuMigrationParamsEnableTLS, which in turn causes libvirt to try to
> add NBD connection with empty string as tls-hostname resulting in:
> 
>   error: internal error: unable to execute QEMU command 'blockdev-add': Certificate does not match the hostname
> 
> To address this modify 'qemuMigrationParamsGetTLSHostname' to undo the
> weird semantics the migration code uses to handle TLS hostname and make
> it return NULL if the hostname is an empty string.
> 
> Fixes: e8fa09d66bc
> Resolves: https://issues.redhat.com/browse/RHEL-32880
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  src/qemu/qemu_migration_params.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)

Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org