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@xxxxxxxxxx> 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@xxxxxxxxxx> --- 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 :|
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
