[libvirt] [PATCH] Don't add SPICE TLS channels when TLS is disabled
It's possible to disable SPICE TLS in qemu.conf. When this happens, libvirt ignores any SPICE TLS port or x509 directory that may have been set when it builds the qemu command line to use. However, it's not ignoring the secure channels that may have been set and adds tls-channel arguments to qemu command line. Current qemu versions don't report an error when this happens, and try to use TLS for the specified channels. Before this patch <domain type='kvm'> <name>auto-tls-port</name> <memory>65536</memory> <os> <type arch='x86_64' machine='pc'>hvm</type> </os> <devices> <graphics type='spice' port='5900' tlsPort='-1' autoport='yes' listen='0' ke <listen type='address' address='0'/> <channel name='main' mode='secure'/> <channel name='inputs' mode='secure'/> </graphics> </devices> </domain> generates -spice port=5900,addr=0,disable-ticketing,tls-channel=main,tls-channel=inputs and after this patch we get -spice port=5900,addr=0,disable-ticketing This fixes bug #790436 --- src/qemu/qemu_command.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 99d7129..30fb3b1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5286,8 +5286,9 @@ qemuBuildCommandLine(virConnectPtr conn, int mode = def->graphics[0]->data.spice.channels[i]; switch (mode) { case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: - virBufferAsprintf(&opt, ",tls-channel=%s", - virDomainGraphicsSpiceChannelNameTypeToString(i)); + if (driver->spiceTLS) + virBufferAsprintf(&opt, ",tls-channel=%s", + virDomainGraphicsSpiceChannelNameTypeToString(i)); break; case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: virBufferAsprintf(&opt, ",plaintext-channel=%s", -- 1.7.7.6
On 02/14/2012 11:04 AM, Christophe Fergeau wrote:
It's possible to disable SPICE TLS in qemu.conf. When this happens, libvirt ignores any SPICE TLS port or x509 directory that may have been set when it builds the qemu command line to use. However, it's not ignoring the secure channels that may have been set and adds tls-channel arguments to qemu command line. Current qemu versions don't report an error when this happens, and try to use TLS for the specified channels.
Before this patch
<domain type='kvm'> <name>auto-tls-port</name> <memory>65536</memory> <os> <type arch='x86_64' machine='pc'>hvm</type> </os> <devices> <graphics type='spice' port='5900' tlsPort='-1' autoport='yes' listen='0' ke <listen type='address' address='0'/> <channel name='main' mode='secure'/> <channel name='inputs' mode='secure'/> </graphics> </devices> </domain>
generates
-spice port=5900,addr=0,disable-ticketing,tls-channel=main,tls-channel=inputs
and after this patch we get
-spice port=5900,addr=0,disable-ticketing
Meta-question - if the XML requests secure, but TLS is disabled, should we instead be failing to start the domain with a complaint that we can't honor the XML? If the answer is that we should start the domain anyways, and there is just no security, because whoever disabled tls in qemu.conf knows what they are doing, then ACK. If the answer is that we should error out on an impossible configuration, then
+++ b/src/qemu/qemu_command.c @@ -5286,8 +5286,9 @@ qemuBuildCommandLine(virConnectPtr conn, int mode = def->graphics[0]->data.spice.channels[i]; switch (mode) { case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: - virBufferAsprintf(&opt, ",tls-channel=%s", - virDomainGraphicsSpiceChannelNameTypeToString(i)); + if (driver->spiceTLS) + virBufferAsprintf(&opt, ",tls-channel=%s", + virDomainGraphicsSpiceChannelNameTypeToString(i));
this needs an else clause that errors out. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Tue, Feb 14, 2012 at 02:10:37PM -0700, Eric Blake wrote:
Meta-question - if the XML requests secure, but TLS is disabled, should we instead be failing to start the domain with a complaint that we can't honor the XML?
Meta-non-answer, when a TLS port is set but TLS is disabled in the config file, it's silently ignored: if (driver->spiceTLS && def->graphics[0]->data.spice.tlsPort != -1) virBufferAsprintf(&opt, ",tls-port=%u", def->graphics[0]->data.spice.tlsPort); so I just did the same for the secure channels. If we decide an error should be returned, this can be done in a follow-up patch changing both locations.
If the answer is that we should start the domain anyways, and there is just no security, because whoever disabled tls in qemu.conf knows what they are doing, then ACK. If the answer is that we should error out on an impossible configuration,
My initial feeling was that we should error out. However, I don't think it's realistic at this point: * oVirt is already disabling TLS in qemu.conf if the admin says he does not want it during engine-setup, but oVirt still adds the TLS port/secure channels to its libvirt VMs even when TLS is disabled. They probably won't be that happy if libvirt starts erroring out here. Though we can also argue that it's a bug on their side, and that the VMs they start this way (with TLS XML bits but TLS disabled in the conf file) are not usable anyway because of these extra tls-channel options * this will also be annoying if someone has setup a bunch of VMs with TLS parameters and then decides to change the TLS setting in qemu.conf, these VMs will suddenly fail to start until their conf is modified Because of that, I think we should unfortunately just ignore these TLS bits when TLS is disabled. We can log a warning though when we detect this. Once again, regardless of what we decide to do, it's better done in a separate patch. Christophe
On Wed, Feb 15, 2012 at 10:08:24AM +0100, Christophe Fergeau wrote:
On Tue, Feb 14, 2012 at 02:10:37PM -0700, Eric Blake wrote:
Meta-question - if the XML requests secure, but TLS is disabled, should we instead be failing to start the domain with a complaint that we can't honor the XML?
Meta-non-answer, when a TLS port is set but TLS is disabled in the config file, it's silently ignored:
What value does allowing TLS configuration in qemu.conf add? That seems wrong to me because it creates the possibility of the kind of ambiguity discovered here. Shouldn't the domain XML be the only required statement of the user's intent? Dave
if (driver->spiceTLS && def->graphics[0]->data.spice.tlsPort != -1) virBufferAsprintf(&opt, ",tls-port=%u", def->graphics[0]->data.spice.tlsPort);
so I just did the same for the secure channels. If we decide an error should be returned, this can be done in a follow-up patch changing both locations.
If the answer is that we should start the domain anyways, and there is just no security, because whoever disabled tls in qemu.conf knows what they are doing, then ACK. If the answer is that we should error out on an impossible configuration,
My initial feeling was that we should error out. However, I don't think it's realistic at this point: * oVirt is already disabling TLS in qemu.conf if the admin says he does not want it during engine-setup, but oVirt still adds the TLS port/secure channels to its libvirt VMs even when TLS is disabled. They probably won't be that happy if libvirt starts erroring out here. Though we can also argue that it's a bug on their side, and that the VMs they start this way (with TLS XML bits but TLS disabled in the conf file) are not usable anyway because of these extra tls-channel options * this will also be annoying if someone has setup a bunch of VMs with TLS parameters and then decides to change the TLS setting in qemu.conf, these VMs will suddenly fail to start until their conf is modified
Because of that, I think we should unfortunately just ignore these TLS bits when TLS is disabled. We can log a warning though when we detect this. Once again, regardless of what we decide to do, it's better done in a separate patch.
Christophe
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Feb 15, 2012 at 09:59:57AM -0500, Dave Allan wrote:
On Wed, Feb 15, 2012 at 10:08:24AM +0100, Christophe Fergeau wrote:
On Tue, Feb 14, 2012 at 02:10:37PM -0700, Eric Blake wrote:
Meta-question - if the XML requests secure, but TLS is disabled, should we instead be failing to start the domain with a complaint that we can't honor the XML?
Meta-non-answer, when a TLS port is set but TLS is disabled in the config file, it's silently ignored:
What value does allowing TLS configuration in qemu.conf add? That seems wrong to me because it creates the possibility of the kind of ambiguity discovered here. Shouldn't the domain XML be the only required statement of the user's intent?
It enables you to turn on TLS for all guests, regardless of the domain XML configuration, which is a desirable policy control knob for a host level administrator to have. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Wed, Feb 15, 2012 at 03:10:47PM +0000, Daniel P. Berrange wrote:
It enables you to turn on TLS for all guests, regardless of the domain XML configuration, which is a desirable policy control knob for a host level administrator to have.
I'm under the impression that it's doing the opposite in the SPICE case, but maybe I haven't been looking in the right place (qemu_command.c) Christophe
On 02/15/2012 09:36 AM, Christophe Fergeau wrote:
On Wed, Feb 15, 2012 at 03:10:47PM +0000, Daniel P. Berrange wrote:
It enables you to turn on TLS for all guests, regardless of the domain XML configuration, which is a desirable policy control knob for a host level administrator to have.
I'm under the impression that it's doing the opposite in the SPICE case, but maybe I haven't been looking in the right place (qemu_command.c)
Is this a case where adding a third mode in libvirt XML might make sense? mode='insecure' => don't bother with security, regardless of qemu.conf mode='secure' => use security if tls is available in qemu.conf, but silently fall back to insecure mode='mandatory-secure' => use security, and error out if qemu.conf disabled tls Or am I confusing things? -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Copying my comments (https://bugzilla.redhat.com/show_bug.cgi?id=790436#c8) here as requested: =============================================================================== (In reply to comment #6)
mode='insecure' - don't bother with security
By this, you mean plaintext-only setting, or provide tls if certs/keys are available?
mode='secure' - use security if tls is available, but fall back to insecure
What does falling back mean here? Provide both and let client choose or provide plaintext channel only if tls-port or x509 stuff is not set?
mode='mandatory-secure'
^^^ FWIW, this is what RHEV means by mode='secure'. They _do want_ to enforce main and input channels to TLS. oVirt's feature of hosts without mandatory TLS is quite new and clearly not yet stabilized. Another consideration is that sometimes, you could want some channels to be mandatory insecure, like display + playback on hosts that do not support aes-ni. Given the facts above, I'd propose going with these four modes: * mode='insecure' - provide plaintext-only (-spice plaintext-channel=...) * mode='...' - provide both if BOTH x509 stuff is set and TLS port will get alocated, else provide plaintext-only * mode='...' - provide both tls and plaintext, error out if x509 stuff is not set * mode='secure' - provide TLS channel only, error out if x509 stuff is not set The secure mode name is chosen with respect to compatibility with RHEV usage and I didn't manage to name the middle ones sanely. When spice_tls is == 0, second mode should be default, when spice_tls == 1, third mode should be default. minor correction: (In reply to comment #8)
and I didn't manage to name the middle ones sanely. When spice_tls is == 0, second mode should be default, when spice_tls == 1, third mode should be default.
with spice_tls set to 1, main and inputs channels should default to fourth mode, the rest to third. =============================================================================== Keep me in CC please as I'm not subscribed. David -- David Jaša, RHCE SPICE QE based in Brno GPG Key: 22C33E24 Fingerprint: 513A 060B D1B4 2A72 7F0D 0278 B125 CD00 22C3 3E24
participants (5)
-
Christophe Fergeau -
Daniel P. Berrange -
Dave Allan -
David Jaša -
Eric Blake