[PATCH 0/7] qemu: migration: TLS enforcement and non-shared-storage error improvement
A collection of fixes for migration. Peter Krempa (7): docs: migration: Fix example for unix socket migration qemu: migration: Remove TODO about implementing NBD for TUNNELLED migration qemu: migration: Aggregate logic depending on tunnelled migration qemu: migration: Forbid tunnelled non-shared storage migration with -blockdev docs: migration: Mention that features may not work with tunnelled migration qemu: conf: Introduce "migrate_tls_force" qemu.conf option docs: migration: Add a mention of VIR_MIGRATE_TLS and it's enforcement for qemu docs/migration.html.in | 21 ++++++++++- src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 8 ++++ src/qemu/qemu_conf.c | 2 + src/qemu/qemu_conf.h | 1 + src/qemu/qemu_migration.c | 60 ++++++++++++++++++++++-------- src/qemu/test_libvirtd_qemu.aug.in | 1 + 7 files changed, 78 insertions(+), 16 deletions(-) -- 2.28.0
Fix the following issues: 1) the very long line is overflowing the code box 2) '--migrateuri' was missing for the qemu data stream 3) '--desturi' was not used making it non-obvious what the argument corresponds to Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/migration.html.in | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/migration.html.in b/docs/migration.html.in index 77731eeb37..e84e5f5452 100644 --- a/docs/migration.html.in +++ b/docs/migration.html.in @@ -655,7 +655,10 @@ virsh migrate --p2p --tunnelled web1 qemu+ssh://desthost/system qemu+ssh://10.0. software): </p> <pre> -virsh migrate web1 [--p2p] --copy-storage-all 'qemu+unix:///system?socket=/tmp/migdir/test-sock-driver' 'unix:///tmp/migdir/test-sock-qemu' --disks-uri unix:///tmp/migdir/test-sock-nbd +virsh migrate --domain web1 [--p2p] --copy-storage-all + --desturi 'qemu+unix:///system?socket=/tmp/migdir/test-sock-driver' + --migrateuri 'unix:///tmp/migdir/test-sock-qemu' + --disks-uri unix:///tmp/migdir/test-sock-nbd </pre> <p> -- 2.28.0
Our streams are not the best transport for migration data and we support TLS for security now. It's unlikely that there will be enough motivation to add a new migration protocol to tunnell NBD too. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index fef0be63a1..85f3c4ccee 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2244,10 +2244,7 @@ qemuMigrationSrcBeginPhase(virQEMUDriverPtr driver, } } - /* TODO support NBD for TUNNELLED migration */ - if (flags & VIR_MIGRATE_TUNNELLED) { - VIR_WARN("NBD in tunnelled migration is currently not supported"); - } else { + if (!(flags & VIR_MIGRATE_TUNNELLED)) { cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; priv->nbdPort = 0; } -- 2.28.0
On a Tuesday in 2020, Peter Krempa wrote:
Our streams are not the best transport for migration data and we support TLS for security now. It's unlikely that there will be enough motivation to add a new migration protocol to tunnell NBD too.
s/tunnell/tunnel/ Jano
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
Move and aggregate all the logic which is switched based on whether the migration is tunnelled or not before other checks. Further checks will be added later. While the code is being moved the error message is put on a single line per new coding style. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 85f3c4ccee..13d73638f4 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2219,6 +2219,17 @@ qemuMigrationSrcBeginPhase(virQEMUDriverPtr driver, } if (flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC)) { + if (flags & VIR_MIGRATE_TUNNELLED) { + if (nmigrate_disks) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Selecting disks to migrate is not implemented for tunnelled migration")); + return NULL; + } + } else { + cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; + priv->nbdPort = 0; + } + if (nmigrate_disks) { size_t i, j; /* Check user requested only known disk targets. */ @@ -2235,18 +2246,6 @@ qemuMigrationSrcBeginPhase(virQEMUDriverPtr driver, return NULL; } } - - if (flags & VIR_MIGRATE_TUNNELLED) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("Selecting disks to migrate is not " - "implemented for tunnelled migration")); - return NULL; - } - } - - if (!(flags & VIR_MIGRATE_TUNNELLED)) { - cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; - priv->nbdPort = 0; } } -- 2.28.0
qemu's internals were not prepared for switching to -blockdev for the legacy storage migration. Add a proper error message since qemu is unlikely to attempt fixing the old protocol. Resolves: https://gitlab.com/libvirt/libvirt/-/issues/65 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 13d73638f4..2be0fc29ae 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2220,6 +2220,12 @@ qemuMigrationSrcBeginPhase(virQEMUDriverPtr driver, if (flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC)) { if (flags & VIR_MIGRATE_TUNNELLED) { + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("migration of non-shared storage is not supported with tunnelled migration and this QEMU")); + return NULL; + } + if (nmigrate_disks) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("Selecting disks to migrate is not implemented for tunnelled migration")); -- 2.28.0
Enumerate some features which are incompatible with tunnelled migration. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/migration.html.in | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/migration.html.in b/docs/migration.html.in index e84e5f5452..c3c64fb51f 100644 --- a/docs/migration.html.in +++ b/docs/migration.html.in @@ -49,6 +49,14 @@ migration operations. </p> + <p> + <em>Note:</em> Certain features such as migration of non-shared storage + (<code>VIR_MIGRATE_NON_SHARED_DISK</code>), the multi-connection migration + (<code>VIR_MIGRATE_PARALLEL</code>), or post-copy migration + (<code>VIR_MIGRATE_POSTCOPY</code>) may not be available when using + libvirt's tunnelling. + </p> + <p> <img class="diagram" src="migration-tunnel.png" alt="Migration tunnel path"/> </p> -- 2.28.0
Forgetting to use the VIR_MIGRATE_TLS flag with migration can lead to leak of sensitive information. Add an administrative knob to force use of the flag. Note that without VIR_MIGRATE_PEER2PEER, the migration is driven by an instance of the client library which doesn't necessarily run on either of the hosts so the flag can't be used to assume VIR_MIGRATE_TLS even if it wasn't provided by the user instead of rejecting if it's not. Resolves: https://gitlab.com/libvirt/libvirt/-/issues/67 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 8 ++++++++ src/qemu/qemu_conf.c | 2 ++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_migration.c | 28 ++++++++++++++++++++++++++++ src/qemu/test_libvirtd_qemu.aug.in | 1 + 6 files changed, 41 insertions(+) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index abbac549f2..3c1045858b 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -58,6 +58,7 @@ module Libvirtd_qemu = let migrate_entry = str_entry "migrate_tls_x509_cert_dir" | bool_entry "migrate_tls_x509_verify" | str_entry "migrate_tls_x509_secret_uuid" + | bool_entry "migrate_tls_force" let backup_entry = str_entry "backup_tls_x509_cert_dir" | bool_entry "backup_tls_x509_verify" diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index a7b864f594..0c1054f198 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -401,6 +401,14 @@ #migrate_tls_x509_secret_uuid = "00000000-0000-0000-0000-000000000000" +# By default TLS is requested using the VIR_MIGRATE_TLS flag, thus not requested +# automatically. Setting 'migate_tls_force' to "1" will prevent any migration +# which is not using VIR_MIGRATE_TLS to ensure higher level of security in +# deployments with TLS. +# +#migrate_tls_force = 0 + + # In order to override the default TLS certificate location for backup NBD # server certificates, supply a valid path to the certificate directory. If the # provided path does not exist, libvirtd will fail to start. If the path is diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 83de26ab56..d6615ca0dd 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -494,6 +494,8 @@ virQEMUDriverConfigLoadSpecificTLSEntry(virQEMUDriverConfigPtr cfg, return -1; if (virConfGetValueBool(conf, "chardev_tls", &cfg->chardevTLS) < 0) return -1; + if (virConfGetValueBool(conf, "migrate_tls_force", &cfg->migrateTLSForce) < 0) + return -1; #define GET_CONFIG_TLS_CERTINFO_COMMON(val) \ do { \ diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 8748212a82..411d08db36 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -140,6 +140,7 @@ struct _virQEMUDriverConfig { bool migrateTLSx509verify; bool migrateTLSx509verifyPresent; char *migrateTLSx509secretUUID; + bool migrateTLSForce; char *backupTLSx509certdir; bool backupTLSx509verify; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 2be0fc29ae..122481dea1 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2332,9 +2332,18 @@ qemuMigrationSrcBegin(virConnectPtr conn, unsigned long flags) { virQEMUDriverPtr driver = conn->privateData; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); char *xml = NULL; qemuDomainAsyncJob asyncJob; + if (cfg->migrateTLSForce && + !(flags & VIR_MIGRATE_TUNNELLED) && + !(flags & VIR_MIGRATE_TLS)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("this libvirtd instance allows migration only with VIR_MIGRATE_TLS flag")); + goto cleanup; + } + if ((flags & VIR_MIGRATE_CHANGE_PROTECTION)) { if (qemuMigrationJobStart(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT, flags) < 0) @@ -2501,6 +2510,7 @@ qemuMigrationDstPrepareAny(virQEMUDriverPtr driver, qemuMigrationParamsPtr migParams, unsigned long flags) { + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); virDomainObjPtr vm = NULL; virObjectEventPtr event = NULL; virErrorPtr origErr; @@ -2563,6 +2573,14 @@ qemuMigrationDstPrepareAny(virQEMUDriverPtr driver, goto cleanup; } + if (cfg->migrateTLSForce && + !(flags & VIR_MIGRATE_TUNNELLED) && + !(flags & VIR_MIGRATE_TLS)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("this libvirtd instance allows migration only with VIR_MIGRATE_TLS flag")); + goto cleanup; + } + if (!qemuMigrationSrcIsAllowedHostdev(*def)) goto cleanup; @@ -5013,6 +5031,8 @@ qemuMigrationSrcPerform(virQEMUDriverPtr driver, unsigned long resource, bool v3proto) { + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + VIR_DEBUG("driver=%p, conn=%p, vm=%p, xmlin=%s, dconnuri=%s, " "uri=%s, graphicsuri=%s, listenAddress=%s, " "nmigrate_disks=%zu, migrate_disks=%p, nbdPort=%d, " @@ -5025,6 +5045,14 @@ qemuMigrationSrcPerform(virQEMUDriverPtr driver, NULLSTR(cookiein), cookieinlen, cookieout, cookieoutlen, flags, NULLSTR(dname), resource, v3proto); + if (cfg->migrateTLSForce && + !(flags & VIR_MIGRATE_TUNNELLED) && + !(flags & VIR_MIGRATE_TLS)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("this libvirtd instance allows migration only with VIR_MIGRATE_TLS flag")); + return -1; + } + if ((flags & (VIR_MIGRATE_TUNNELLED | VIR_MIGRATE_PEER2PEER))) { if (cookieinlen) { virReportError(VIR_ERR_OPERATION_INVALID, diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index 6a54e2322a..9310dcec1c 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -35,6 +35,7 @@ module Test_libvirtd_qemu = { "migrate_tls_x509_cert_dir" = "/etc/pki/libvirt-migrate" } { "migrate_tls_x509_verify" = "1" } { "migrate_tls_x509_secret_uuid" = "00000000-0000-0000-0000-000000000000" } +{ "migrate_tls_force" = "0" } { "backup_tls_x509_cert_dir" = "/etc/pki/libvirt-backup" } { "backup_tls_x509_verify" = "1" } { "backup_tls_x509_secret_uuid" = "00000000-0000-0000-0000-000000000000" } -- 2.28.0
Mention the flag to enable TLS and also the knob to enforce it in the qmemu hypervisor driver. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/migration.html.in | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/migration.html.in b/docs/migration.html.in index c3c64fb51f..b080e3a7f5 100644 --- a/docs/migration.html.in +++ b/docs/migration.html.in @@ -31,6 +31,14 @@ of ports on the firewall to allow multiple concurrent migration operations. </p> + <p> + Modern hypervisors support TLS for encryption and authentication of the + migration connections which can be enabled using the + <code>VIR_MIGRATE_TLS</code> flag. The <em>qemu</em> hypervisor driver + allows users to force use of TLS via the <code>migrate_tls_force</code> + knob configured in <code>/etc/libvirt/qemu.conf</code>. + </p> + <p> <img class="diagram" src="migration-native.png" alt="Migration native path"/> </p> -- 2.28.0
In the commit summary: s/it's/its/ On a Tuesday in 2020, Peter Krempa wrote:
Mention the flag to enable TLS and also the knob to enforce it in the qmemu hypervisor driver.
s/qmemu/qemu/
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/migration.html.in | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/docs/migration.html.in b/docs/migration.html.in index c3c64fb51f..b080e3a7f5 100644 --- a/docs/migration.html.in +++ b/docs/migration.html.in @@ -31,6 +31,14 @@ of ports on the firewall to allow multiple concurrent migration operations. </p>
+ <p> + Modern hypervisors support TLS for encryption and authentication of the + migration connections which can be enabled using the + <code>VIR_MIGRATE_TLS</code> flag. The <em>qemu</em> hypervisor driver + allows users to force use of TLS via the <code>migrate_tls_force</code> + knob configured in <code>/etc/libvirt/qemu.conf</code>. + </p> + <p> <img class="diagram" src="migration-native.png" alt="Migration native path"/> </p> -- 2.28.0
On a Tuesday in 2020, Peter Krempa wrote:
A collection of fixes for migration.
Peter Krempa (7): docs: migration: Fix example for unix socket migration qemu: migration: Remove TODO about implementing NBD for TUNNELLED migration qemu: migration: Aggregate logic depending on tunnelled migration qemu: migration: Forbid tunnelled non-shared storage migration with -blockdev docs: migration: Mention that features may not work with tunnelled migration qemu: conf: Introduce "migrate_tls_force" qemu.conf option docs: migration: Add a mention of VIR_MIGRATE_TLS and it's enforcement for qemu
docs/migration.html.in | 21 ++++++++++- src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 8 ++++ src/qemu/qemu_conf.c | 2 + src/qemu/qemu_conf.h | 1 + src/qemu/qemu_migration.c | 60 ++++++++++++++++++++++-------- src/qemu/test_libvirtd_qemu.aug.in | 1 + 7 files changed, 78 insertions(+), 16 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa