[libvirt] [PATCH 0/4] qemu: vnc: switch to tls-creds-x509
Ján Tomko (4): tests: qemu: Remove disk from graphics-vnc-tls FIXDOWN: copy graphics-vnc-tls args to versioned paths tests: qemu: test more versions for graphics-vnc-tls qemu: vnc: switch to tls-creds-x509 src/qemu/qemu_command.c | 26 +++++++++++++---- tests/qemuxml2argvdata/graphics-vnc-tls.args | 2 -- .../graphics-vnc-tls.x86_64-2.4.0.args | 28 ++++++++++++++++++ .../graphics-vnc-tls.x86_64-latest.args | 33 ++++++++++++++++++++++ tests/qemuxml2argvdata/graphics-vnc-tls.xml | 6 ---- tests/qemuxml2argvtest.c | 2 ++ tests/qemuxml2xmloutdata/graphics-vnc-tls.xml | 6 ---- 7 files changed, 83 insertions(+), 20 deletions(-) create mode 100644 tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-2.4.0.args create mode 100644 tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-latest.args -- 2.16.1
The disk command line is tested elsewhere. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/qemuxml2argvdata/graphics-vnc-tls.args | 2 -- tests/qemuxml2argvdata/graphics-vnc-tls.xml | 6 ------ tests/qemuxml2xmloutdata/graphics-vnc-tls.xml | 6 ------ 3 files changed, 14 deletions(-) diff --git a/tests/qemuxml2argvdata/graphics-vnc-tls.args b/tests/qemuxml2argvdata/graphics-vnc-tls.args index b87c2116a3..3668fa889d 100644 --- a/tests/qemuxml2argvdata/graphics-vnc-tls.args +++ b/tests/qemuxml2argvdata/graphics-vnc-tls.args @@ -22,7 +22,5 @@ server,nowait \ -no-acpi \ -boot c \ -usb \ --drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ --device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ -vnc 127.0.0.1:3,tls,x509verify=/etc/pki/libvirt-vnc,sasl \ -vga cirrus diff --git a/tests/qemuxml2argvdata/graphics-vnc-tls.xml b/tests/qemuxml2argvdata/graphics-vnc-tls.xml index d0729e830d..079f6241c4 100644 --- a/tests/qemuxml2argvdata/graphics-vnc-tls.xml +++ b/tests/qemuxml2argvdata/graphics-vnc-tls.xml @@ -14,12 +14,6 @@ <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu-system-i686</emulator> - <disk type='block' device='disk'> - <driver name='qemu' type='raw'/> - <source dev='/dev/HostVG/QEMUGuest1'/> - <target dev='hda' bus='ide'/> - <address type='drive' controller='0' bus='0' target='0' unit='0'/> - </disk> <controller type='usb' index='0'/> <controller type='ide' index='0'/> <controller type='pci' index='0' model='pci-root'/> diff --git a/tests/qemuxml2xmloutdata/graphics-vnc-tls.xml b/tests/qemuxml2xmloutdata/graphics-vnc-tls.xml index 7b53c2ae1f..dcbc4bcf1d 100644 --- a/tests/qemuxml2xmloutdata/graphics-vnc-tls.xml +++ b/tests/qemuxml2xmloutdata/graphics-vnc-tls.xml @@ -14,12 +14,6 @@ <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu-system-i686</emulator> - <disk type='block' device='disk'> - <driver name='qemu' type='raw'/> - <source dev='/dev/HostVG/QEMUGuest1'/> - <target dev='hda' bus='ide'/> - <address type='drive' controller='0' bus='0' target='0' unit='0'/> - </disk> <controller type='usb' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> -- 2.16.1
On Tue, Jul 17, 2018 at 17:15:53 +0200, Ján Tomko wrote:
The disk command line is tested elsewhere.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/qemuxml2argvdata/graphics-vnc-tls.args | 2 -- tests/qemuxml2argvdata/graphics-vnc-tls.xml | 6 ------ tests/qemuxml2xmloutdata/graphics-vnc-tls.xml | 6 ------ 3 files changed, 14 deletions(-)
ACK
For easier review of how these tests are different from the one with capabilities listed. This commit will be absorbed by the next one. --- .../graphics-vnc-tls.x86_64-2.4.0.args | 26 ++++++++++++++++++++++ .../graphics-vnc-tls.x86_64-latest.args | 26 ++++++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-2.4.0.args create mode 100644 tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-latest.args diff --git a/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-2.4.0.args b/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-2.4.0.args new file mode 100644 index 0000000000..3668fa889d --- /dev/null +++ b/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-2.4.0.args @@ -0,0 +1,26 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +SASL_CONF_PATH=/root/.sasl2 \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name QEMUGuest1 \ +-S \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot c \ +-usb \ +-vnc 127.0.0.1:3,tls,x509verify=/etc/pki/libvirt-vnc,sasl \ +-vga cirrus diff --git a/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-latest.args b/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-latest.args new file mode 100644 index 0000000000..3668fa889d --- /dev/null +++ b/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-latest.args @@ -0,0 +1,26 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +SASL_CONF_PATH=/root/.sasl2 \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name QEMUGuest1 \ +-S \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot c \ +-usb \ +-vnc 127.0.0.1:3,tls,x509verify=/etc/pki/libvirt-vnc,sasl \ +-vga cirrus -- 2.16.1
Add a test with QEMU 2.4.0 capabilites, as well as the latest caps. The code paths for formatting TLS options will be altered and 2.4.0 is the newest version where QEMU_CAPS_OBJECT_TLS_CREDS_X509 is not supported. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- .../qemuxml2argvdata/graphics-vnc-tls.x86_64-2.4.0.args | 12 +++++++----- .../graphics-vnc-tls.x86_64-latest.args | 17 +++++++++++------ tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-2.4.0.args b/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-2.4.0.args index 3668fa889d..7b8e3a4aa6 100644 --- a/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-2.4.0.args +++ b/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-2.4.0.args @@ -6,10 +6,11 @@ LOGNAME=test \ SASL_CONF_PATH=/root/.sasl2 \ QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ --name QEMUGuest1 \ +-name guest=QEMUGuest1,debug-threads=on \ -S \ --machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-machine pc-i440fx-2.4,accel=tcg,usb=off,dump-guest-core=off \ -m 214 \ +-realtime mlock=off \ -smp 1,sockets=1,cores=1,threads=1 \ -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ -no-user-config \ @@ -20,7 +21,8 @@ server,nowait \ -rtc base=utc \ -no-shutdown \ -no-acpi \ --boot c \ --usb \ +-boot strict=on \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ -vnc 127.0.0.1:3,tls,x509verify=/etc/pki/libvirt-vnc,sasl \ --vga cirrus +-device cirrus-vga,id=video0,bus=pci.0,addr=0x2 \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-latest.args b/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-latest.args index 3668fa889d..01743eff2a 100644 --- a/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-latest.args +++ b/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-latest.args @@ -6,21 +6,26 @@ LOGNAME=test \ SASL_CONF_PATH=/root/.sasl2 \ QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ --name QEMUGuest1 \ +-name guest=QEMUGuest1,debug-threads=on \ -S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -machine pc,accel=tcg,usb=off,dump-guest-core=off \ -m 214 \ +-realtime mlock=off \ -smp 1,sockets=1,cores=1,threads=1 \ -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ -no-user-config \ -nodefaults \ --chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ -server,nowait \ +-chardev socket,id=charmonitor,fd=1729,server,nowait \ -mon chardev=charmonitor,id=monitor,mode=control \ -rtc base=utc \ -no-shutdown \ -no-acpi \ --boot c \ --usb \ +-boot strict=on \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ -vnc 127.0.0.1:3,tls,x509verify=/etc/pki/libvirt-vnc,sasl \ --vga cirrus +-device cirrus-vga,id=video0,bus=pci.0,addr=0x2 \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ +resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index a929e4314e..ca7b315567 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1194,6 +1194,8 @@ mymain(void) driver.config->vncTLS = 1; driver.config->vncTLSx509verify = 1; DO_TEST("graphics-vnc-tls", QEMU_CAPS_VNC, QEMU_CAPS_DEVICE_CIRRUS_VGA); + DO_TEST_CAPS_VER("graphics-vnc-tls", "2.4.0"); + DO_TEST_CAPS_LATEST("graphics-vnc-tls"); driver.config->vncSASL = driver.config->vncTLSx509verify = driver.config->vncTLS = 0; VIR_FREE(driver.config->vncSASLdir); VIR_FREE(driver.config->vncTLSx509certdir); -- 2.16.1
On Tue, Jul 17, 2018 at 17:15:55 +0200, Ján Tomko wrote:
Add a test with QEMU 2.4.0 capabilites, as well as the latest caps.
The code paths for formatting TLS options will be altered and 2.4.0 is the newest version where QEMU_CAPS_OBJECT_TLS_CREDS_X509 is not supported.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- .../qemuxml2argvdata/graphics-vnc-tls.x86_64-2.4.0.args | 12 +++++++----- .../graphics-vnc-tls.x86_64-latest.args | 17 +++++++++++------ tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 20 insertions(+), 11 deletions(-)
ACK
The tls, x509 and x509verify options were deprecated in QEMU v2.5.0: commit 3e305e4a4752f70c0b5c3cf5b43ec957881714f7 Author: Daniel P. Berrange <berrange@redhat.com> ui: convert VNC server to use QCryptoTLSSession Use the tls-creds-x509 object when available. https://bugzilla.redhat.com/show_bug.cgi?id=1598167 Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 26 +++++++++++++++++----- .../graphics-vnc-tls.x86_64-latest.args | 4 +++- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 44ae8dcef7..9326abbe63 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7917,13 +7917,27 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, virBufferAddLit(&opt, ",password"); if (cfg->vncTLS) { - virBufferAddLit(&opt, ",tls"); - if (cfg->vncTLSx509verify) { - virBufferAddLit(&opt, ",x509verify="); - virQEMUBuildBufferEscapeComma(&opt, cfg->vncTLSx509certdir); + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_TLS_CREDS_X509)) { + const char *alias = "vnc-tls-creds0"; + if (qemuBuildTLSx509CommandLine(cmd, + cfg->vncTLSx509certdir, + true, + cfg->vncTLSx509verify, + NULL, + alias, + qemuCaps) < 0) + goto error; + + virBufferAsprintf(&opt, ",tls-creds=%s", alias); } else { - virBufferAddLit(&opt, ",x509="); - virQEMUBuildBufferEscapeComma(&opt, cfg->vncTLSx509certdir); + virBufferAddLit(&opt, ",tls"); + if (cfg->vncTLSx509verify) { + virBufferAddLit(&opt, ",x509verify="); + virQEMUBuildBufferEscapeComma(&opt, cfg->vncTLSx509certdir); + } else { + virBufferAddLit(&opt, ",x509="); + virQEMUBuildBufferEscapeComma(&opt, cfg->vncTLSx509certdir); + } } } diff --git a/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-latest.args b/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-latest.args index 01743eff2a..97775fad42 100644 --- a/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-latest.args +++ b/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-latest.args @@ -24,7 +24,9 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -no-acpi \ -boot strict=on \ -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ --vnc 127.0.0.1:3,tls,x509verify=/etc/pki/libvirt-vnc,sasl \ +-object tls-creds-x509,id=vnc-tls-creds0,dir=/etc/pki/libvirt-vnc,\ +endpoint=server,verify-peer=yes \ +-vnc 127.0.0.1:3,tls-creds=vnc-tls-creds0,sasl \ -device cirrus-vga,id=video0,bus=pci.0,addr=0x2 \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ resourcecontrol=deny \ -- 2.16.1
On Tue, Jul 17, 2018 at 05:15:56PM +0200, Ján Tomko wrote:
The tls, x509 and x509verify options were deprecated in QEMU v2.5.0:
commit 3e305e4a4752f70c0b5c3cf5b43ec957881714f7 Author: Daniel P. Berrange <berrange@redhat.com>
ui: convert VNC server to use QCryptoTLSSession
Use the tls-creds-x509 object when available.
https://bugzilla.redhat.com/show_bug.cgi?id=1598167
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 26 +++++++++++++++++----- .../graphics-vnc-tls.x86_64-latest.args | 4 +++- 2 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 44ae8dcef7..9326abbe63 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7917,13 +7917,27 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, virBufferAddLit(&opt, ",password");
if (cfg->vncTLS) { - virBufferAddLit(&opt, ",tls"); - if (cfg->vncTLSx509verify) { - virBufferAddLit(&opt, ",x509verify="); - virQEMUBuildBufferEscapeComma(&opt, cfg->vncTLSx509certdir); + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_TLS_CREDS_X509)) { + const char *alias = "vnc-tls-creds0"; + if (qemuBuildTLSx509CommandLine(cmd, + cfg->vncTLSx509certdir, + true, + cfg->vncTLSx509verify, + NULL, + alias, + qemuCaps) < 0) + goto error; + + virBufferAsprintf(&opt, ",tls-creds=%s", alias); } else { - virBufferAddLit(&opt, ",x509="); - virQEMUBuildBufferEscapeComma(&opt, cfg->vncTLSx509certdir); + virBufferAddLit(&opt, ",tls"); + if (cfg->vncTLSx509verify) { + virBufferAddLit(&opt, ",x509verify="); + virQEMUBuildBufferEscapeComma(&opt, cfg->vncTLSx509certdir); + } else { + virBufferAddLit(&opt, ",x509="); + virQEMUBuildBufferEscapeComma(&opt, cfg->vncTLSx509certdir); + } } }
So this is better than what we have today, but it is still not comparable with what John did for the other TLS features. Specifically it is missing support for encrypted keys, so we're still broken if the user has editted qemu.conf to set a "default_tls_x509_secret_uuid". We should also have a new "vnc_tls_x509_secret_uuid" to match what's done for chardev and migration. We need to use more of the helpers that John created for dealing with TLS creds to handle this correctly. 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 Wed, Jul 18, 2018 at 08:32:44AM +0100, Daniel P. Berrangé wrote:
On Tue, Jul 17, 2018 at 05:15:56PM +0200, Ján Tomko wrote:
The tls, x509 and x509verify options were deprecated in QEMU v2.5.0:
commit 3e305e4a4752f70c0b5c3cf5b43ec957881714f7 Author: Daniel P. Berrange <berrange@redhat.com>
ui: convert VNC server to use QCryptoTLSSession
Use the tls-creds-x509 object when available.
https://bugzilla.redhat.com/show_bug.cgi?id=1598167
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 26 +++++++++++++++++----- .../graphics-vnc-tls.x86_64-latest.args | 4 +++- 2 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 44ae8dcef7..9326abbe63 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7917,13 +7917,27 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, virBufferAddLit(&opt, ",password");
if (cfg->vncTLS) { - virBufferAddLit(&opt, ",tls"); - if (cfg->vncTLSx509verify) { - virBufferAddLit(&opt, ",x509verify="); - virQEMUBuildBufferEscapeComma(&opt, cfg->vncTLSx509certdir); + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_TLS_CREDS_X509)) { + const char *alias = "vnc-tls-creds0"; + if (qemuBuildTLSx509CommandLine(cmd, + cfg->vncTLSx509certdir, + true, + cfg->vncTLSx509verify, + NULL, + alias, + qemuCaps) < 0) + goto error; + + virBufferAsprintf(&opt, ",tls-creds=%s", alias); } else { - virBufferAddLit(&opt, ",x509="); - virQEMUBuildBufferEscapeComma(&opt, cfg->vncTLSx509certdir); + virBufferAddLit(&opt, ",tls"); + if (cfg->vncTLSx509verify) { + virBufferAddLit(&opt, ",x509verify="); + virQEMUBuildBufferEscapeComma(&opt, cfg->vncTLSx509certdir); + } else { + virBufferAddLit(&opt, ",x509="); + virQEMUBuildBufferEscapeComma(&opt, cfg->vncTLSx509certdir); + } } }
So this is better than what we have today, but it is still not comparable with what John did for the other TLS features. Specifically it is missing support for encrypted keys, so we're still broken if the user has editted qemu.conf to set a "default_tls_x509_secret_uuid". We should also have a new "vnc_tls_x509_secret_uuid" to match what's done for chardev and migration.
I'm aware of that, as I said in the bugzilla comment: https://bugzilla.redhat.com/show_bug.cgi?id=1598167#c1 Do you consider the lack of this feature a blocker for this patch aiming to prevent a regression when QEMU stops accepting the old syntax? Jano
We need to use more of the helpers that John created for dealing with TLS creds to handle this correctly.
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 Wed, Jul 18, 2018 at 09:55:47AM +0200, Ján Tomko wrote:
On Wed, Jul 18, 2018 at 08:32:44AM +0100, Daniel P. Berrangé wrote:
On Tue, Jul 17, 2018 at 05:15:56PM +0200, Ján Tomko wrote:
The tls, x509 and x509verify options were deprecated in QEMU v2.5.0:
commit 3e305e4a4752f70c0b5c3cf5b43ec957881714f7 Author: Daniel P. Berrange <berrange@redhat.com>
ui: convert VNC server to use QCryptoTLSSession
Use the tls-creds-x509 object when available.
https://bugzilla.redhat.com/show_bug.cgi?id=1598167
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 26 +++++++++++++++++----- .../graphics-vnc-tls.x86_64-latest.args | 4 +++- 2 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 44ae8dcef7..9326abbe63 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7917,13 +7917,27 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, virBufferAddLit(&opt, ",password");
if (cfg->vncTLS) { - virBufferAddLit(&opt, ",tls"); - if (cfg->vncTLSx509verify) { - virBufferAddLit(&opt, ",x509verify="); - virQEMUBuildBufferEscapeComma(&opt, cfg->vncTLSx509certdir); + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_TLS_CREDS_X509)) { + const char *alias = "vnc-tls-creds0"; + if (qemuBuildTLSx509CommandLine(cmd, + cfg->vncTLSx509certdir, + true, + cfg->vncTLSx509verify, + NULL, + alias, + qemuCaps) < 0) + goto error; + + virBufferAsprintf(&opt, ",tls-creds=%s", alias); } else { - virBufferAddLit(&opt, ",x509="); - virQEMUBuildBufferEscapeComma(&opt, cfg->vncTLSx509certdir); + virBufferAddLit(&opt, ",tls"); + if (cfg->vncTLSx509verify) { + virBufferAddLit(&opt, ",x509verify="); + virQEMUBuildBufferEscapeComma(&opt, cfg->vncTLSx509certdir); + } else { + virBufferAddLit(&opt, ",x509="); + virQEMUBuildBufferEscapeComma(&opt, cfg->vncTLSx509certdir); + } } }
So this is better than what we have today, but it is still not comparable with what John did for the other TLS features. Specifically it is missing support for encrypted keys, so we're still broken if the user has editted qemu.conf to set a "default_tls_x509_secret_uuid". We should also have a new "vnc_tls_x509_secret_uuid" to match what's done for chardev and migration.
I'm aware of that, as I said in the bugzilla comment: https://bugzilla.redhat.com/show_bug.cgi?id=1598167#c1
Do you consider the lack of this feature a blocker for this patch aiming to prevent a regression when QEMU stops accepting the old syntax?
Even today if you use an encrypted key for default_tls_x509_dir the VNC server config will be broken, so it isn't a regression, and thus not a blocker. I should have made it clearer in the BZ though, that the intention was to get parity for TLS config across all servers, so that we both avoid the deprecated feature & fix the existing bug wrt encrypted TLS keys. Perhaps better file a separate BZ for the latter now. 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 Tue, Jul 17, 2018 at 17:15:56 +0200, Ján Tomko wrote:
The tls, x509 and x509verify options were deprecated in QEMU v2.5.0:
commit 3e305e4a4752f70c0b5c3cf5b43ec957881714f7 Author: Daniel P. Berrange <berrange@redhat.com>
ui: convert VNC server to use QCryptoTLSSession
Use the tls-creds-x509 object when available.
https://bugzilla.redhat.com/show_bug.cgi?id=1598167
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 26 +++++++++++++++++----- .../graphics-vnc-tls.x86_64-latest.args | 4 +++- 2 files changed, 23 insertions(+), 7 deletions(-)
ACK, as we've established that the we'll need to deal with the encrypted key part later.
participants (3)
-
Daniel P. Berrangé -
Ján Tomko -
Peter Krempa