[libvirt] [RFC PATCH 0/4] qemu: Forbid NBD migration with TLS
Currently we can't use TLS for NBD so allowing it if TLS is requested creates a security problem. Reject it by refusing to migrate disks and setup TLS on destination since that is easy enough. Note: That I've did not test this yet since my TLS setup was broken. I'll fix it later today and reprot the findings. Peter Krempa (4): qemu: caps: Add capability for TLS transport in the NBD server qemu: monitor: Add 'tls-creds' parameter to 'nbd-server-start' command qemu: migration: Use TLS environment for NBD server if requested qemu: migration: Forbid 'nbd' migration of non-shared storage if TLS is requested src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_migration.c | 28 +++++++++++++++++++--- src/qemu/qemu_monitor.c | 7 +++--- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 4 +++- src/qemu/qemu_monitor_json.h | 3 ++- tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + tests/qemumonitorjsontest.c | 2 +- 27 files changed, 59 insertions(+), 10 deletions(-) -- 2.16.2
The NBD server in qemu supports TLS transport. Detect this capability. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + 21 files changed, 22 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 833c75514c..aa8d350f51 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -473,6 +473,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, /* 290 */ "query-cpus-fast", "disk-write-cache", + "nbd-tls", ); @@ -1247,6 +1248,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsQMPSchemaQueries[] = { { "blockdev-add/arg-type/+vxhs", QEMU_CAPS_VXHS}, { "blockdev-add/arg-type/+iscsi/password-secret", QEMU_CAPS_ISCSI_PASSWORD_SECRET }, { "blockdev-add/arg-type/+qcow2/encrypt/+luks/key-secret", QEMU_CAPS_QCOW2_LUKS }, + { "nbd-server-start/arg-type/tls-creds", QEMU_CAPS_NBD_TLS }, }; typedef struct _virQEMUCapsObjectTypeProps virQEMUCapsObjectTypeProps; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index c55cada7b8..2afe7ef580 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -457,6 +457,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ /* 290 */ QEMU_CAPS_QUERY_CPUS_FAST, /* query-cpus-fast command */ QEMU_CAPS_DISK_WRITE_CACHE, /* qemu block frontends support write-cache param */ + QEMU_CAPS_NBD_TLS, /* NBD server supports TLS transport */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml index 527b425dee..a7f8ddd661 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml @@ -153,6 +153,7 @@ <flag name='dump-completed'/> <flag name='qcow2-luks'/> <flag name='disk-write-cache'/> + <flag name='nbd-tls'/> <version>2010000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>303541</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml index 3ac6be2d88..41d07ffb2b 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml @@ -152,6 +152,7 @@ <flag name='dump-completed'/> <flag name='qcow2-luks'/> <flag name='disk-write-cache'/> + <flag name='nbd-tls'/> <version>2010000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>382824</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml index b682d36c93..098326ecc9 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml @@ -113,6 +113,7 @@ <flag name='dump-completed'/> <flag name='qcow2-luks'/> <flag name='disk-write-cache'/> + <flag name='nbd-tls'/> <version>2010000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>303326</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml index 8dd30ccbcf..dd794092a1 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml @@ -196,6 +196,7 @@ <flag name='dump-completed'/> <flag name='qcow2-luks'/> <flag name='disk-write-cache'/> + <flag name='nbd-tls'/> <version>2010000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>344938</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml index 6dd392502e..64bd554541 100644 --- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml @@ -118,6 +118,7 @@ <flag name='qcow2-luks'/> <flag name='seccomp-blacklist'/> <flag name='disk-write-cache'/> + <flag name='nbd-tls'/> <version>2011000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>342058</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml index 31c5d0dd23..197060abb5 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml @@ -157,6 +157,7 @@ <flag name='seccomp-blacklist'/> <flag name='query-cpus-fast'/> <flag name='disk-write-cache'/> + <flag name='nbd-tls'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>342346</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml index 7dead4a1f4..b0eb0552da 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml @@ -154,6 +154,7 @@ <flag name='seccomp-blacklist'/> <flag name='query-cpus-fast'/> <flag name='disk-write-cache'/> + <flag name='nbd-tls'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>419215</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml index 70ae8f91c7..80f3ec6ec7 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml @@ -119,6 +119,7 @@ <flag name='seccomp-blacklist'/> <flag name='query-cpus-fast'/> <flag name='disk-write-cache'/> + <flag name='nbd-tls'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml index d809a78380..0b18dd00f9 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml @@ -195,6 +195,7 @@ <flag name='seccomp-blacklist'/> <flag name='query-cpus-fast'/> <flag name='disk-write-cache'/> + <flag name='nbd-tls'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>390060</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.6.0.aarch64.xml index b1bb3e7bd2..d80036cc6b 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.aarch64.xml @@ -140,6 +140,7 @@ <flag name='isa-serial'/> <flag name='pl011'/> <flag name='dump-completed'/> + <flag name='nbd-tls'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>228241</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml index f3611e1922..60463930e0 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml @@ -135,6 +135,7 @@ <flag name='spapr-vty'/> <flag name='isa-serial'/> <flag name='dump-completed'/> + <flag name='nbd-tls'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>263005</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml index 0e48180275..3e2399ef20 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml @@ -171,6 +171,7 @@ <flag name='virtio-gpu.max_outputs'/> <flag name='isa-serial'/> <flag name='dump-completed'/> + <flag name='nbd-tls'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>227332</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml index 34eed23694..266287344d 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml @@ -103,6 +103,7 @@ <flag name='sclplmconsole'/> <flag name='dump-completed'/> <flag name='disk-write-cache'/> + <flag name='nbd-tls'/> <version>2007000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>216732</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml index ee4b25d7fd..711e4c935a 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml @@ -176,6 +176,7 @@ <flag name='isa-serial'/> <flag name='dump-completed'/> <flag name='disk-write-cache'/> + <flag name='nbd-tls'/> <version>2007000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>239029</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml index 21046c0546..3449ca5ff1 100644 --- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml @@ -105,6 +105,7 @@ <flag name='sclplmconsole'/> <flag name='dump-completed'/> <flag name='disk-write-cache'/> + <flag name='nbd-tls'/> <version>2007093</version> <kvmVersion>0</kvmVersion> <microcodeVersion>241633</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml index c4801ac23a..62e104f3a6 100644 --- a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml @@ -178,6 +178,7 @@ <flag name='isa-serial'/> <flag name='dump-completed'/> <flag name='disk-write-cache'/> + <flag name='nbd-tls'/> <version>2008000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>255684</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml index 73f42a7392..0e679d1ce3 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml @@ -144,6 +144,7 @@ <flag name='isa-serial'/> <flag name='dump-completed'/> <flag name='disk-write-cache'/> + <flag name='nbd-tls'/> <version>2009000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>346538</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml index 30fce6ecf1..2122a5126e 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml @@ -108,6 +108,7 @@ <flag name='iscsi.password-secret'/> <flag name='dump-completed'/> <flag name='disk-write-cache'/> + <flag name='nbd-tls'/> <version>2009000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>265051</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml index e1dc257a75..a359e2e75f 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml @@ -191,6 +191,7 @@ <flag name='isa-serial'/> <flag name='dump-completed'/> <flag name='disk-write-cache'/> + <flag name='nbd-tls'/> <version>2009000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>320947</microcodeVersion> -- 2.16.2
On Thu, Apr 26, 2018 at 04:51:46PM +0200, Peter Krempa wrote:
The NBD server in qemu supports TLS transport. Detect this capability.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + 21 files changed, 22 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> 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 :|
To allow encryption of the non-shared storage migration NBD connection we will need to instantiated the NBD server with the TLS env. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 2 +- src/qemu/qemu_monitor.c | 7 ++++--- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 4 +++- src/qemu/qemu_monitor_json.h | 3 ++- tests/qemumonitorjsontest.c | 2 +- 6 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 88b8253fa9..743ae77dbb 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -411,7 +411,7 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver, else if (virPortAllocatorAcquire(driver->migrationPorts, &port) < 0) goto exit_monitor; - if (qemuMonitorNBDServerStart(priv->mon, listenAddr, port) < 0) + if (qemuMonitorNBDServerStart(priv->mon, listenAddr, port, NULL) < 0) goto exit_monitor; } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index f642d9a51a..8c26ee66b3 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3999,13 +3999,14 @@ qemuMonitorGetGICCapabilities(qemuMonitorPtr mon, int qemuMonitorNBDServerStart(qemuMonitorPtr mon, const char *host, - unsigned int port) + unsigned int port, + const char *tls_alias) { - VIR_DEBUG("host=%s port=%u", host, port); + VIR_DEBUG("host=%s port=%u tls_alias=%s", host, port, NULLSTR(tls_alias)); QEMU_CHECK_MONITOR_JSON(mon); - return qemuMonitorJSONNBDServerStart(mon, host, port); + return qemuMonitorJSONNBDServerStart(mon, host, port, tls_alias); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index d6b68b44ca..a93844f77b 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1052,7 +1052,8 @@ char *qemuMonitorGetTargetArch(qemuMonitorPtr mon); int qemuMonitorNBDServerStart(qemuMonitorPtr mon, const char *host, - unsigned int port); + unsigned int port, + const char *tls_alias); int qemuMonitorNBDServerAdd(qemuMonitorPtr mon, const char *deviceID, bool writable); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 24d37eb41d..05ac8d0a3c 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6396,7 +6396,8 @@ qemuMonitorJSONBuildUnixSocketAddress(const char *path) int qemuMonitorJSONNBDServerStart(qemuMonitorPtr mon, const char *host, - unsigned int port) + unsigned int port, + const char *tls_alias) { int ret = -1; virJSONValuePtr cmd = NULL; @@ -6412,6 +6413,7 @@ qemuMonitorJSONNBDServerStart(qemuMonitorPtr mon, if (!(cmd = qemuMonitorJSONMakeCommand("nbd-server-start", "a:addr", &addr, + "S:tls-creds", tls_alias, NULL))) goto cleanup; diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 665da27d6d..ed5f29b20a 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -449,7 +449,8 @@ char *qemuMonitorJSONGetTargetArch(qemuMonitorPtr mon); int qemuMonitorJSONNBDServerStart(qemuMonitorPtr mon, const char *host, - unsigned int port); + unsigned int port, + const char *tls_alias); int qemuMonitorJSONNBDServerAdd(qemuMonitorPtr mon, const char *deviceID, bool writable); diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index f8d39c35ab..9a83c9a608 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1350,7 +1350,7 @@ GEN_TEST_FUNC(qemuMonitorJSONBlockCommit, "vdb", "/foo/bar1", "/foo/bar2", NULL, GEN_TEST_FUNC(qemuMonitorJSONDrivePivot, "vdb") GEN_TEST_FUNC(qemuMonitorJSONScreendump, "/foo/bar") GEN_TEST_FUNC(qemuMonitorJSONOpenGraphics, "spice", "spicefd", false) -GEN_TEST_FUNC(qemuMonitorJSONNBDServerStart, "localhost", 12345) +GEN_TEST_FUNC(qemuMonitorJSONNBDServerStart, "localhost", 12345, "test-alias") GEN_TEST_FUNC(qemuMonitorJSONNBDServerAdd, "vda", true) GEN_TEST_FUNC(qemuMonitorJSONDetachCharDev, "serial1") -- 2.16.2
On Thu, Apr 26, 2018 at 04:51:47PM +0200, Peter Krempa wrote:
To allow encryption of the non-shared storage migration NBD connection we will need to instantiated the NBD server with the TLS env.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 2 +- src/qemu/qemu_monitor.c | 7 ++++--- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 4 +++- src/qemu/qemu_monitor_json.h | 3 ++- tests/qemumonitorjsontest.c | 2 +- 6 files changed, 13 insertions(+), 8 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> 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 :|
Use the TLS env for migration when starting the NBD server if TLS is enabled for migration. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 743ae77dbb..3b5ba4f0a1 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -369,7 +369,8 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver, const char *listenAddr, size_t nmigrate_disks, const char **migrate_disks, - int nbdPort) + int nbdPort, + const char *tls_alias) { int ret = -1; qemuDomainObjPrivatePtr priv = vm->privateData; @@ -411,7 +412,7 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver, else if (virPortAllocatorAcquire(driver->migrationPorts, &port) < 0) goto exit_monitor; - if (qemuMonitorNBDServerStart(priv->mon, listenAddr, port, NULL) < 0) + if (qemuMonitorNBDServerStart(priv->mon, listenAddr, port, tls_alias) < 0) goto exit_monitor; } @@ -2401,9 +2402,21 @@ qemuMigrationDstPrepareAny(virQEMUDriverPtr driver, if (mig->nbd && flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NBD_SERVER)) { + const char *nbdTLSAlias = NULL; + + if (flags & VIR_MIGRATE_TLS) { + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NBD_TLS)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("QEMU NBD server does not support TLS transport")); + goto stopjob; + } + + nbdTLSAlias = tlsAlias; + } + if (qemuMigrationDstStartNBDServer(driver, vm, incoming->address, nmigrate_disks, migrate_disks, - nbdPort) < 0) { + nbdPort, nbdTLSAlias) < 0) { goto stopjob; } cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; -- 2.16.2
On Thu, Apr 26, 2018 at 04:51:48PM +0200, Peter Krempa wrote:
Use the TLS env for migration when starting the NBD server if TLS is enabled for migration.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> 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 :|
Since libvirt is currently not able to setup the NBD migration stream secured by TLS we should not allow such migration since data would be transferred unencrypted. This will break compatibility of TLS migration if non-shared storage is requested but the security implications are more severe. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 3b5ba4f0a1..24ef819738 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3352,6 +3352,15 @@ qemuMigrationSrcRun(virQEMUDriverPtr driver, if (migrate_flags & (QEMU_MONITOR_MIGRATE_NON_SHARED_DISK | QEMU_MONITOR_MIGRATE_NON_SHARED_INC)) { if (mig->nbd) { + /* Currently libvirt does not support setting up of the NBD + * non-shared storage migration with TLS. As we need to honour the + * VIR_MIGRATE_TLS flag, we need to reject such migration. */ + if (flags & VIR_MIGRATE_TLS) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("NBD migration with TLS is not supported")); + goto error; + } + /* This will update migrate_flags on success */ if (qemuMigrationSrcDriveMirror(driver, vm, mig, spec->dest.host.name, -- 2.16.2
On Thu, Apr 26, 2018 at 04:51:49PM +0200, Peter Krempa wrote:
Since libvirt is currently not able to setup the NBD migration stream secured by TLS we should not allow such migration since data would be transferred unencrypted.
This will break compatibility of TLS migration if non-shared storage is requested but the security implications are more severe.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 3b5ba4f0a1..24ef819738 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3352,6 +3352,15 @@ qemuMigrationSrcRun(virQEMUDriverPtr driver, if (migrate_flags & (QEMU_MONITOR_MIGRATE_NON_SHARED_DISK | QEMU_MONITOR_MIGRATE_NON_SHARED_INC)) { if (mig->nbd) { + /* Currently libvirt does not support setting up of the NBD + * non-shared storage migration with TLS. As we need to honour the + * VIR_MIGRATE_TLS flag, we need to reject such migration. */
You might want to reword the last sentence to be explicitly clear that: "... reject such migration until TLS for NBD streams is implemented." Or something like that. Your choice.
From what I understand, what you are saying is -- today if one sets VIR_MIGRATE_TLS flag, then libvirt will use TLS for the migration stream but not for the NBD stream via which non-shared disks will be migrated. You are fixing that inconsistency.
+ if (flags & VIR_MIGRATE_TLS) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("NBD migration with TLS is not supported")); + goto error; + } + /* This will update migrate_flags on success */ if (qemuMigrationSrcDriveMirror(driver, vm, mig, spec->dest.host.name, -- 2.16.2
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- /kashyap
On Thu, Apr 26, 2018 at 04:51:49PM +0200, Peter Krempa wrote:
Since libvirt is currently not able to setup the NBD migration stream secured by TLS we should not allow such migration since data would be transferred unencrypted.
This will break compatibility of TLS migration if non-shared storage is requested but the security implications are more severe.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 9 +++++++++ 1 file changed, 9 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> IIUC, this doesn't actually require the 3 previous patches and can be pushed on its own - we should push for this immediate release.
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 3b5ba4f0a1..24ef819738 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3352,6 +3352,15 @@ qemuMigrationSrcRun(virQEMUDriverPtr driver, if (migrate_flags & (QEMU_MONITOR_MIGRATE_NON_SHARED_DISK | QEMU_MONITOR_MIGRATE_NON_SHARED_INC)) { if (mig->nbd) { + /* Currently libvirt does not support setting up of the NBD + * non-shared storage migration with TLS. As we need to honour the + * VIR_MIGRATE_TLS flag, we need to reject such migration. */ + if (flags & VIR_MIGRATE_TLS) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("NBD migration with TLS is not supported")); + goto error; + } + /* This will update migrate_flags on success */ if (qemuMigrationSrcDriveMirror(driver, vm, mig, spec->dest.host.name, -- 2.16.2
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 :|
On Fri, Apr 27, 2018 at 10:55:56 +0100, Daniel Berrange wrote:
On Thu, Apr 26, 2018 at 04:51:49PM +0200, Peter Krempa wrote:
Since libvirt is currently not able to setup the NBD migration stream secured by TLS we should not allow such migration since data would be transferred unencrypted.
This will break compatibility of TLS migration if non-shared storage is requested but the security implications are more severe.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 9 +++++++++ 1 file changed, 9 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Pushed now, thanks.
IIUC, this doesn't actually require the 3 previous patches and can be pushed on its own - we should push for this immediate release.
The idea behind the other 3 patches was to actually implement the destination side, so that we have both sides covered. If you enable TLS for the NBD server it will not connect unless TLS is used. By using this patch only, an older source libvirtd will be able to migrate even with newer destination libvirtd, since that will not require TLS until those 3 patches will be pushed.
On Mon, Apr 30, 2018 at 10:42:24AM +0200, Peter Krempa wrote:
On Fri, Apr 27, 2018 at 10:55:56 +0100, Daniel Berrange wrote:
On Thu, Apr 26, 2018 at 04:51:49PM +0200, Peter Krempa wrote:
Since libvirt is currently not able to setup the NBD migration stream secured by TLS we should not allow such migration since data would be transferred unencrypted.
This will break compatibility of TLS migration if non-shared storage is requested but the security implications are more severe.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 9 +++++++++ 1 file changed, 9 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Pushed now, thanks.
IIUC, this doesn't actually require the 3 previous patches and can be pushed on its own - we should push for this immediate release.
The idea behind the other 3 patches was to actually implement the destination side, so that we have both sides covered. If you enable TLS for the NBD server it will not connect unless TLS is used. By using this patch only, an older source libvirtd will be able to migrate even with newer destination libvirtd, since that will not require TLS until those 3 patches will be pushed.
Oh i see, nice trick. 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 :|
On Mon, Apr 30, 2018 at 10:08:05 +0100, Daniel Berrange wrote:
On Mon, Apr 30, 2018 at 10:42:24AM +0200, Peter Krempa wrote:
On Fri, Apr 27, 2018 at 10:55:56 +0100, Daniel Berrange wrote:
On Thu, Apr 26, 2018 at 04:51:49PM +0200, Peter Krempa wrote:
Since libvirt is currently not able to setup the NBD migration stream secured by TLS we should not allow such migration since data would be transferred unencrypted.
This will break compatibility of TLS migration if non-shared storage is requested but the security implications are more severe.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 9 +++++++++ 1 file changed, 9 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Pushed now, thanks.
IIUC, this doesn't actually require the 3 previous patches and can be pushed on its own - we should push for this immediate release.
The idea behind the other 3 patches was to actually implement the destination side, so that we have both sides covered. If you enable TLS for the NBD server it will not connect unless TLS is used. By using this patch only, an older source libvirtd will be able to migrate even with newer destination libvirtd, since that will not require TLS until those 3 patches will be pushed.
Oh i see, nice trick.
I've verified that everything works fine without TLS and with TLS if we implement the transport properly and pushed these. This means that TLS migration should for-now behave sanely.
participants (3)
-
Daniel P. Berrangé -
Kashyap Chamarthy -
Peter Krempa