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

2012-02-29 Thread Christophe Fergeau 
On Tue, Feb 28, 2012 at 05:04:57PM +0100, Michal Privoznik wrote:
 On 24.02.2012 11:34, Christophe Fergeau wrote:
   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.

Ugh, sorry about that silly bug, thanks for catching it.

Christophe


pgpIk6d6ad8Dj.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2012-02-28 Thread Michal Privoznik 
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'
   nameauto-tls-port/name
   memory65536/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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2012-02-24 Thread Eric Blake 
On 02/24/2012 03:41 AM, Daniel P. Berrange wrote:
 On Fri, Feb 24, 2012 at 11:34:45AM +0100, 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.



 This fixes bug #790436

Thanks for chasing this down.

 ACK, if we  s/XML_ERROR/CONFIG_UNSUPPORTED/  in the those two error
 messages

Changed and pushed.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2012-02-24 Thread Christophe Fergeau 
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'
  nameauto-tls-port/name
  memory65536/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);
 
 switch (virDomainGraphicsListenGetType(def-graphics[0], 0)) {
@@ -5287,6 +5292,11 @@ qemuBuildCommandLine(virConnectPtr conn,
 int mode = def-graphics[0]-data.spice.channels[i];
 switch (mode) {
 case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE:
+if (!driver-spiceTLS) {
+qemuReportError(VIR_ERR_XML_ERROR,
+_(spice secure channels set in XML 
configuration, but TLS is disabled in qemu.conf));
+goto error;
+}
 virBufferAsprintf(opt, ,tls-channel=%s,
   
virDomainGraphicsSpiceChannelNameTypeToString(i));
 break;
-- 
1.7.7.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2012-02-24 Thread Daniel P. Berrange 
On Fri, Feb 24, 2012 at 11:34:45AM +0100, 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'
   nameauto-tls-port/name
   memory65536/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);
  
  switch (virDomainGraphicsListenGetType(def-graphics[0], 0)) {
 @@ -5287,6 +5292,11 @@ qemuBuildCommandLine(virConnectPtr conn,
  int mode = def-graphics[0]-data.spice.channels[i];
  switch (mode) {
  case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE:
 +if (!driver-spiceTLS) {
 +qemuReportError(VIR_ERR_XML_ERROR,
 +_(spice secure channels set in XML 
 configuration, but TLS is disabled in qemu.conf));
 +goto error;
 +}
  virBufferAsprintf(opt, ,tls-channel=%s,

 virDomainGraphicsSpiceChannelNameTypeToString(i));
  break;

ACK, if we  s/XML_ERROR/CONFIG_UNSUPPORTED/  in the those two error
messages

Regards,
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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list