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@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? 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
