[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv2] Error out when using SPICE TLS with spice_tls=0



On 24.02.2012 11:34, 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 starts QEMU.
> 
> After this patch, an error is reported if a TLS port is set in the XML
> or if secure channels are specified but TLS is disabled in qemu.conf.
> This is the behaviour the oVirt people (where I spotted this issue) said
> they would expect.
> 
> This fixes bug #790436
> ---
>  src/qemu/qemu_command.c |   12 +++++++++++-
>  1 files changed, 11 insertions(+), 1 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 5a34504..4f3e61e 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -5231,7 +5231,12 @@ qemuBuildCommandLine(virConnectPtr conn,
>  
>          virBufferAsprintf(&opt, "port=%u", def->graphics[0]->data.spice.port);
>  
> -        if (driver->spiceTLS && def->graphics[0]->data.spice.tlsPort != -1)
> +        if (def->graphics[0]->data.spice.tlsPort != -1)
> +            if (!driver->spiceTLS) {
> +                qemuReportError(VIR_ERR_XML_ERROR,
> +                                _("spice TLS port set in XML configuration, but TLS is disabled in qemu.conf"));
> +                goto error;
> +            }
>              virBufferAsprintf(&opt, ",tls-port=%u", def->graphics[0]->data.spice.tlsPort);

In fact, this needs to be wrapped with curly braces as the check for
tlsPort != -1 is meant to protect virBufferAsprintf() in the first
place. Sorry for not catching this earlier.

As an act of repentance I'll send patch.

Michal


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]