Re: [libvirt] [PATCH 4/4] qemu: vnc: switch to tls-creds-x509

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(a)redhat.com&gt; > > 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(a)redhat.com&gt; > --- > 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 :|