Re: [libvirt] [PATCH v10 2/4] domain: Add optional 'tls' attribute for TCP chardev

2016-10-21 Thread John Ferlan 


On 10/21/2016 07:57 AM, Pavel Hrdina wrote:
> On Fri, Oct 21, 2016 at 07:15:55AM -0400, John Ferlan wrote:
>>
>>
>> On 10/21/2016 02:29 AM, Pavel Hrdina wrote:
>>> On Thu, Oct 20, 2016 at 03:48:30PM -0400, John Ferlan wrote:
 [...]

>>
>> Since I assume you have these changes in your local branch - let's go
>> with the two patches and move on.
>>
>> It would be nice while it's still fresh to update the documentation, but
>> that's a separate patch.
>>
>> So "officially" it's an ACK of your changes on top of and in addition to
>> my changes.
>>
>> John
> 
> We should probably figure out also rng, smartcard and redirdevs before 
> pushing.
> They also use the source as you've pointed out and currently they can be
> configured to use TLS with the chardev_tls config option.  I'll send a new
> version of patch series to cover all users of TCP source type.
> 

The rng, smartcard, and redirdevs for hotplug is a different issue
though.  They come more into play when adding the passphrase.

As painful as it is (or will be to review) - I'm trying to move the
privateData from virDomainChrDef to virDomainChrSourceDef. That will
then magically do the right thing for rng, smartcard, and redirdevs.
Right now for command line processing they work fine since adding the
tls-creds is based on virDomainChrSourceDef


John

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

Re: [libvirt] [PATCH v10 2/4] domain: Add optional 'tls' attribute for TCP chardev

2016-10-21 Thread Pavel Hrdina 
On Fri, Oct 21, 2016 at 07:15:55AM -0400, John Ferlan wrote:
> 
> 
> On 10/21/2016 02:29 AM, Pavel Hrdina wrote:
> > On Thu, Oct 20, 2016 at 03:48:30PM -0400, John Ferlan wrote:
> >> [...]
> >>
> 
> Since I assume you have these changes in your local branch - let's go
> with the two patches and move on.
> 
> It would be nice while it's still fresh to update the documentation, but
> that's a separate patch.
> 
> So "officially" it's an ACK of your changes on top of and in addition to
> my changes.
> 
> John

We should probably figure out also rng, smartcard and redirdevs before pushing.
They also use the source as you've pointed out and currently they can be
configured to use TLS with the chardev_tls config option.  I'll send a new
version of patch series to cover all users of TCP source type.

Pavel

> 
> [...]
> 
> > Now I see why we are not able to agree on this change.  The modification 
> > done
> > in qemuProcessPrepareDomain() are only to live definition, the config
> > definition remains untouched, which means the *tls* attribute (if it's 
> > based on
> > qemu.conf) will appear only in live XML.  The config XML will remain the 
> > same
> > after the guest is stopped.
> > 
> >> I think it should be strictly optional and that's where we differ. I see
> >> no reason to change the domain xml unless as a consumer that's what you
> >> want to do - be able to control which domains will have the setting.
> >> What else would be the purpose of a host wide setting to go with a
> >> domain optional setting?
> >>
> >> Finally, if your idea is accepted, that means for any configuration with
> >> chardev_tls=0 (either because it's commented or set that way), every
> >> domain that starts will be updated to have this new attribute
> >> "tls='no'". Then one day, I read up on this wonderful new feature and
> > 
> > Not the domain, only the live XML which is not saved as config XML ...
> > 
> >> modify my qemu.conf file to set chardev_tls=1 and set up the TLS
> >> environment properly. I go to start my domain, but wait it's not using
> > 
> > And after you start the domain there will be "tls='yes'" because the config 
> > XML
> > doesn't contain any *tls* attribute.
> > 
> > I've tested all of those cases before proposing this patch:
> > 
> > prerequisite: prepare certificate files to be used for chardev devices
> > 
> > for running domain:
> > live XML - virsh dumpxml $domain
> > config XML - virsh dumpxml $domain --config
> > migratable XML - virsh dumpxml $domain --migratable
> > 
> > 1. set chardev_tls = 1
> > a) start domain where there is no *tls* attribute in config XML
> > - the domain is started and TLS is properly configured
> > - in the live XML there is "tls='yes'"
> > - in the config XML there is no *tls* attribute
> > - in the migratable XML there is no *tls* attribure
> > 
> > b) start domain where there is "tls='no'" in config XML
> > - the domain is started and TLS is not configured
> > - in the live XML there is "tls='no'"
> > - in the config XML there is "tls='no'"
> > - in the migratable XML there is "tls='no'"
> > 
> > c) start domain where there is "tls='yes'" in config XML
> > - the domain is started and TLS is properly configured
> > - in the live XML there is "tls='yes'"
> > - in the config XML there is "tls='yes'"
> > - in the migratable XML there is "tls='yes'"
> > 
> > 2. set chardev_tls = 0
> > a) start domain where there is no *tls* attribute in config XML
> > - the domain is started and TLS is not configured
> > - in the live XML there is "tls='no'"
> > - in the config XML there is no *tls* attribute
> > - in the migratable XML there is no *tls* attribure
> > 
> > b) start domain where there is "tls='no'" in config XML
> > - the domain is started and TLS is not configured
> > - in the live XML there is "tls='no'"
> > - in the config XML there is "tls='no'"
> > - in the migratable XML there is "tls='no'"
> > 
> > c) start domain where there is "tls='yes'" in config XML
> > - the domain is started and TLS is properly configured
> > - in the live XML there is "tls='yes'"
> > - in the config XML there is "tls='yes'"
> > - in the migratable XML there is "tls='yes'"
> > 
> > Pavel
> > 
> >> it. Closer inspection finds, someone put "tls='no'" into my domain... To
> >> me that's not right.  And I won't necessarily know unless I know to look
> >> at the cmdline of the started domain to find that 'tls-creds' or I in
> >> some way "track" when TLS is being used.
> >>
> >>
> >> John
> >>
> >> --
> >> libvir-list mailing list
> >> libvir-list@redhat.com
> >> https://www.redhat.com/mailman/listinfo/libvir-list
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list


signature.asc
Description: Digital signature
--

Re: [libvirt] [PATCH v10 2/4] domain: Add optional 'tls' attribute for TCP chardev

2016-10-21 Thread John Ferlan 


On 10/21/2016 02:29 AM, Pavel Hrdina wrote:
> On Thu, Oct 20, 2016 at 03:48:30PM -0400, John Ferlan wrote:
>> [...]
>>

Since I assume you have these changes in your local branch - let's go
with the two patches and move on.

It would be nice while it's still fresh to update the documentation, but
that's a separate patch.

So "officially" it's an ACK of your changes on top of and in addition to
my changes.

John

[...]

> Now I see why we are not able to agree on this change.  The modification done
> in qemuProcessPrepareDomain() are only to live definition, the config
> definition remains untouched, which means the *tls* attribute (if it's based 
> on
> qemu.conf) will appear only in live XML.  The config XML will remain the same
> after the guest is stopped.
> 
>> I think it should be strictly optional and that's where we differ. I see
>> no reason to change the domain xml unless as a consumer that's what you
>> want to do - be able to control which domains will have the setting.
>> What else would be the purpose of a host wide setting to go with a
>> domain optional setting?
>>
>> Finally, if your idea is accepted, that means for any configuration with
>> chardev_tls=0 (either because it's commented or set that way), every
>> domain that starts will be updated to have this new attribute
>> "tls='no'". Then one day, I read up on this wonderful new feature and
> 
> Not the domain, only the live XML which is not saved as config XML ...
> 
>> modify my qemu.conf file to set chardev_tls=1 and set up the TLS
>> environment properly. I go to start my domain, but wait it's not using
> 
> And after you start the domain there will be "tls='yes'" because the config 
> XML
> doesn't contain any *tls* attribute.
> 
> I've tested all of those cases before proposing this patch:
> 
> prerequisite: prepare certificate files to be used for chardev devices
> 
> for running domain:
> live XML - virsh dumpxml $domain
> config XML - virsh dumpxml $domain --config
> migratable XML - virsh dumpxml $domain --migratable
> 
> 1. set chardev_tls = 1
> a) start domain where there is no *tls* attribute in config XML
> - the domain is started and TLS is properly configured
> - in the live XML there is "tls='yes'"
> - in the config XML there is no *tls* attribute
> - in the migratable XML there is no *tls* attribure
> 
> b) start domain where there is "tls='no'" in config XML
> - the domain is started and TLS is not configured
> - in the live XML there is "tls='no'"
> - in the config XML there is "tls='no'"
> - in the migratable XML there is "tls='no'"
> 
> c) start domain where there is "tls='yes'" in config XML
> - the domain is started and TLS is properly configured
> - in the live XML there is "tls='yes'"
> - in the config XML there is "tls='yes'"
> - in the migratable XML there is "tls='yes'"
> 
> 2. set chardev_tls = 0
> a) start domain where there is no *tls* attribute in config XML
> - the domain is started and TLS is not configured
> - in the live XML there is "tls='no'"
> - in the config XML there is no *tls* attribute
> - in the migratable XML there is no *tls* attribure
> 
> b) start domain where there is "tls='no'" in config XML
> - the domain is started and TLS is not configured
> - in the live XML there is "tls='no'"
> - in the config XML there is "tls='no'"
> - in the migratable XML there is "tls='no'"
> 
> c) start domain where there is "tls='yes'" in config XML
> - the domain is started and TLS is properly configured
> - in the live XML there is "tls='yes'"
> - in the config XML there is "tls='yes'"
> - in the migratable XML there is "tls='yes'"
> 
> Pavel
> 
>> it. Closer inspection finds, someone put "tls='no'" into my domain... To
>> me that's not right.  And I won't necessarily know unless I know to look
>> at the cmdline of the started domain to find that 'tls-creds' or I in
>> some way "track" when TLS is being used.
>>
>>
>> John
>>
>> --
>> libvir-list mailing list
>> libvir-list@redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list

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

Re: [libvirt] [PATCH v10 2/4] domain: Add optional 'tls' attribute for TCP chardev

2016-10-21 Thread Pavel Hrdina 
On Thu, Oct 20, 2016 at 03:48:30PM -0400, John Ferlan wrote:
> [...]
> 
>  +
>  +  Since 2.4.0, the optional attribute
>  +  tls can be used to control whether a serial chardev
>  +  TCP communication channel would utilize a hypervisor configured
>  +  TLS X.509 certificate environment in order to encrypt the data
>  +  channel. For the QEMU hypervisor, usage of a TLS envronment can
>  +  be controlled on the host by the chardev_tls and
>  +  chardev_tls_x509_cert_dir or
>  +  default_tls_x509_cert_dir settings in the file
>  +  /etc/libvirt/qemu.conf. If chardev_tls is enabled,
>  +  then unless the tls attribute is set to "no", libvirt
>  +  will use the host configured TLS environment.
>  +  If chardev_tls is disabled, but the tls
>  +  attribute is set to "yes", then libvirt will attempt to use the
>  +  host TLS environment if either the 
>  chardev_tls_x509_cert_dir
>  +  or default_tls_x509_cert_dir TLS directory structure 
>  exists.
>  +
> >>>
> >>> Nice, this is a good description how to use the *tls* attribute.
> >>>
> >>
> >> BTW (regarding your followup reply):
> >>
> >> The 4 "consumers" of virDomainChrSourceDefParseXML (where this would be
> >> parsed) refer to this as a "serial chardev"
> > 
> > This is a generic function that parses source for a lot of different device
> > types/
> > 
> 
> Shortcutting in my mind to  service='%s'/> which is the VIR_DOMAIN_DEVICE_CHR, smartcard, rng, and
> redirdev. And yes, VIR_DOMAIN_DEVICE_CHR has paths for parallels,
> serials, consoles, and channels that are defined using <%s type='tcp'>.
> 
> >> The 'virDomainChrDefParseXML' comments have a list of " >> types. The location of the above description is describing a  >> type="tcp"> definition.
> > 
> > Well, the comment have that list but is used to parse all character devices,
> > not only serial char device.  TLS encryption can be used also for those 
> > types:
> > parallel, channel, and console.
> > 
> 
> My continual battle against under documentation.  The code is not self
> documenting in all cases...

I agree, we should be better in documenting things.

> >> The 'smartcard' discussion for a 'passthrough' device that would use
> >> this code says "Rather than having the hypervisor directly communicate
> >> with the host, it is possible to tunnel all requests through a secondary
> >> character device to a third-party provider (which may in turn be talking
> >> to a smartcard or using three certificate files). In this mode of
> >> operation, an additional attribute type is required, matching one of the
> >> supported serial device types, to describe the host side of the tunnel;..."
> > 
> > This comment is wrong, it should be "supported character device types".  
> > This
> > attribute tells what interface is presented to the host.  Check this part of
> > documentation for all character devices:
> > 
> >   
> > 
> > there is this sentence:
> > 
> >   "The interface presented to the host is given in the type attribute of the
> >   top-level element. The host interface is configured by the source 
> > element."
> > 
> > So it refers to all host interfaces:
> > 
> >   
> > 
> >> The 'rng' discussion for backend that would use this code says "This
> >> backend connects to a source using the EGD protocol. The source is
> >> specified as a character device. Refer to character device host
> >> interface for more information. ..."
> > 
> > This is correct, there is no reference to serial character device.
> > 
> >> Redevdir says "An additional attribute type is required, matching one of
> >> the supported serial device types, to describe the host side of the
> >> tunnel; type='tcp' or type='spicevmc' ..."
> > 
> > This is the same case as smartcard device, this is wrong.
> > 
> >> So the long and short of it is, IMO it's a serial chardev device.
> >> Semantically it could be claimed otherwise, but the parsing proves
> >> otherwise as does the existing documentation of "Host interface"
> >> character devices.
> >>
> >> I prefer to keep it described as is. It's only ever used, parsed, etc.
> >> when ... ...  >>
> >> If anything, the description should become more restrictive to indicate
> >> that the option shouldn't be used for smartcards, rngs, and redirdevs,
> >> but I'll save that discussion for patch 3.
> > 
> > Based on the documentation it may appear that it should be a serial chardev,
> > but that's misleading and it should refer only to the "host interfaces of
> > character devices".
> > 
> 
> I cannot begin to describe how many times I've scrolled up and down
> through that discussion and thought how does anyone get this stuff
> correct... Trial and error I suppose.
> 
> In any case, it seems of the 3 the rng is the most correct and the

Re: [libvirt] [PATCH v10 2/4] domain: Add optional 'tls' attribute for TCP chardev

2016-10-20 Thread John Ferlan 
[...]

 +
 +  Since 2.4.0, the optional attribute
 +  tls can be used to control whether a serial chardev
 +  TCP communication channel would utilize a hypervisor configured
 +  TLS X.509 certificate environment in order to encrypt the data
 +  channel. For the QEMU hypervisor, usage of a TLS envronment can
 +  be controlled on the host by the chardev_tls and
 +  chardev_tls_x509_cert_dir or
 +  default_tls_x509_cert_dir settings in the file
 +  /etc/libvirt/qemu.conf. If chardev_tls is enabled,
 +  then unless the tls attribute is set to "no", libvirt
 +  will use the host configured TLS environment.
 +  If chardev_tls is disabled, but the tls
 +  attribute is set to "yes", then libvirt will attempt to use the
 +  host TLS environment if either the 
 chardev_tls_x509_cert_dir
 +  or default_tls_x509_cert_dir TLS directory structure 
 exists.
 +
>>>
>>> Nice, this is a good description how to use the *tls* attribute.
>>>
>>
>> BTW (regarding your followup reply):
>>
>> The 4 "consumers" of virDomainChrSourceDefParseXML (where this would be
>> parsed) refer to this as a "serial chardev"
> 
> This is a generic function that parses source for a lot of different device
> types/
> 

Shortcutting in my mind to  which is the VIR_DOMAIN_DEVICE_CHR, smartcard, rng, and
redirdev. And yes, VIR_DOMAIN_DEVICE_CHR has paths for parallels,
serials, consoles, and channels that are defined using <%s type='tcp'>.

>> The 'virDomainChrDefParseXML' comments have a list of "> types. The location of the above description is describing a > type="tcp"> definition.
> 
> Well, the comment have that list but is used to parse all character devices,
> not only serial char device.  TLS encryption can be used also for those types:
> parallel, channel, and console.
> 

My continual battle against under documentation.  The code is not self
documenting in all cases...

>> The 'smartcard' discussion for a 'passthrough' device that would use
>> this code says "Rather than having the hypervisor directly communicate
>> with the host, it is possible to tunnel all requests through a secondary
>> character device to a third-party provider (which may in turn be talking
>> to a smartcard or using three certificate files). In this mode of
>> operation, an additional attribute type is required, matching one of the
>> supported serial device types, to describe the host side of the tunnel;..."
> 
> This comment is wrong, it should be "supported character device types".  This
> attribute tells what interface is presented to the host.  Check this part of
> documentation for all character devices:
> 
>   
> 
> there is this sentence:
> 
>   "The interface presented to the host is given in the type attribute of the
>   top-level element. The host interface is configured by the source element."
> 
> So it refers to all host interfaces:
> 
>   
> 
>> The 'rng' discussion for backend that would use this code says "This
>> backend connects to a source using the EGD protocol. The source is
>> specified as a character device. Refer to character device host
>> interface for more information. ..."
> 
> This is correct, there is no reference to serial character device.
> 
>> Redevdir says "An additional attribute type is required, matching one of
>> the supported serial device types, to describe the host side of the
>> tunnel; type='tcp' or type='spicevmc' ..."
> 
> This is the same case as smartcard device, this is wrong.
> 
>> So the long and short of it is, IMO it's a serial chardev device.
>> Semantically it could be claimed otherwise, but the parsing proves
>> otherwise as does the existing documentation of "Host interface"
>> character devices.
>>
>> I prefer to keep it described as is. It's only ever used, parsed, etc.
>> when ... ... >
>> If anything, the description should become more restrictive to indicate
>> that the option shouldn't be used for smartcards, rngs, and redirdevs,
>> but I'll save that discussion for patch 3.
> 
> Based on the documentation it may appear that it should be a serial chardev,
> but that's misleading and it should refer only to the "host interfaces of
> character devices".
> 

I cannot begin to describe how many times I've scrolled up and down
through that discussion and thought how does anyone get this stuff
correct... Trial and error I suppose.

In any case, it seems of the 3 the rng is the most correct and the other
two should get patches in order to be more correct. Not sure I can do it
justice. It would seem to me that smartcard and redirdev should use the
pointer to the elementsCharHostInterface.

Still for the purposes of supported 'elementsCharHostInterface' when
being used for specific smartcard, rng, and redirdev entries that are
using "type='tcp'", only the

Re: [libvirt] [PATCH v10 2/4] domain: Add optional 'tls' attribute for TCP chardev

2016-10-20 Thread Pavel Hrdina 
On Thu, Oct 20, 2016 at 10:28:04AM -0400, John Ferlan wrote:
> 
> 
> On 10/20/2016 02:51 AM, Pavel Hrdina wrote:
> > On Wed, Oct 19, 2016 at 04:53:54PM -0400, John Ferlan wrote:
> >> Add an optional "tls='yes|no'" attribute for a TCP chardev.
> >>
> >> For QEMU, this will allow for disabling the host config setting of the
> >> 'chardev_tls' for a domain chardev channel by setting the value to "no" or
> >> to attempt to use a host TLS environment when setting the value to "yes"
> >> when the host config 'chardev_tls' setting is disabled, but a TLS 
> >> environment
> >> is configured via either the host config 'chardev_tls_x509_cert_dir' or
> >> 'default_tls_x509_cert_dir'
> >>
> >> Alter qemuDomainSupportTLSChardevTCP to augment the decision points for
> >> choosing whether to try to use TLS.
> >>
> >> Signed-off-by: John Ferlan 
> >> ---
> >>  docs/formatdomain.html.in  | 28 
> >>  docs/schemas/domaincommon.rng  |  5 +++
> >>  src/conf/domain_conf.c | 22 +-
> >>  src/conf/domain_conf.h |  1 +
> >>  src/qemu/qemu_command.c|  2 +-
> >>  src/qemu/qemu_domain.c | 20 +++--
> >>  src/qemu/qemu_domain.h |  3 +-
> >>  src/qemu/qemu_hotplug.c|  4 +-
> >>  ...uxml2argv-serial-tcp-tlsx509-chardev-notls.args | 30 +
> >>  ...muxml2argv-serial-tcp-tlsx509-chardev-notls.xml | 50 
> >> ++
> >>  tests/qemuxml2argvtest.c   |  3 ++
> >>  ...xml2xmlout-serial-tcp-tlsx509-chardev-notls.xml |  1 +
> >>  tests/qemuxml2xmltest.c|  1 +
> >>  13 files changed, 162 insertions(+), 8 deletions(-)
> >>  create mode 100644 
> >> tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.args
> >>  create mode 100644 
> >> tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml
> >>  create mode 12 
> >> tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-notls.xml
> >>
> >> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> >> index 9051178..da6be67 100644
> >> --- a/docs/formatdomain.html.in
> >> +++ b/docs/formatdomain.html.in
> >> @@ -6204,6 +6204,34 @@ qemu-kvm -net nic,model=? /dev/null
> >>/devices
> >>...
> >>  
> >> +
> >> +  Since 2.4.0, the optional attribute
> >> +  tls can be used to control whether a serial chardev
> >> +  TCP communication channel would utilize a hypervisor configured
> >> +  TLS X.509 certificate environment in order to encrypt the data
> >> +  channel. For the QEMU hypervisor, usage of a TLS envronment can
> >> +  be controlled on the host by the chardev_tls and
> >> +  chardev_tls_x509_cert_dir or
> >> +  default_tls_x509_cert_dir settings in the file
> >> +  /etc/libvirt/qemu.conf. If chardev_tls is enabled,
> >> +  then unless the tls attribute is set to "no", libvirt
> >> +  will use the host configured TLS environment.
> >> +  If chardev_tls is disabled, but the tls
> >> +  attribute is set to "yes", then libvirt will attempt to use the
> >> +  host TLS environment if either the 
> >> chardev_tls_x509_cert_dir
> >> +  or default_tls_x509_cert_dir TLS directory structure 
> >> exists.
> >> +
> > 
> > Nice, this is a good description how to use the *tls* attribute.
> > 
> 
> BTW (regarding your followup reply):
> 
> The 4 "consumers" of virDomainChrSourceDefParseXML (where this would be
> parsed) refer to this as a "serial chardev"

This is a generic function that parses source for a lot of different device
types/

> The 'virDomainChrDefParseXML' comments have a list of " types. The location of the above description is describing a  type="tcp"> definition.

Well, the comment have that list but is used to parse all character devices,
not only serial char device.  TLS encryption can be used also for those types:
parallel, channel, and console.

> The 'smartcard' discussion for a 'passthrough' device that would use
> this code says "Rather than having the hypervisor directly communicate
> with the host, it is possible to tunnel all requests through a secondary
> character device to a third-party provider (which may in turn be talking
> to a smartcard or using three certificate files). In this mode of
> operation, an additional attribute type is required, matching one of the
> supported serial device types, to describe the host side of the tunnel;..."

This comment is wrong, it should be "supported character device types".  This
attribute tells what interface is presented to the host.  Check this part of
documentation for all character devices:

  

there is this sentence:

  "The interface presented to the host is given in the type attribute of the
  top-level element. The host

Re: [libvirt] [PATCH v10 2/4] domain: Add optional 'tls' attribute for TCP chardev

2016-10-20 Thread John Ferlan 


On 10/20/2016 02:51 AM, Pavel Hrdina wrote:
> On Wed, Oct 19, 2016 at 04:53:54PM -0400, John Ferlan wrote:
>> Add an optional "tls='yes|no'" attribute for a TCP chardev.
>>
>> For QEMU, this will allow for disabling the host config setting of the
>> 'chardev_tls' for a domain chardev channel by setting the value to "no" or
>> to attempt to use a host TLS environment when setting the value to "yes"
>> when the host config 'chardev_tls' setting is disabled, but a TLS environment
>> is configured via either the host config 'chardev_tls_x509_cert_dir' or
>> 'default_tls_x509_cert_dir'
>>
>> Alter qemuDomainSupportTLSChardevTCP to augment the decision points for
>> choosing whether to try to use TLS.
>>
>> Signed-off-by: John Ferlan 
>> ---
>>  docs/formatdomain.html.in  | 28 
>>  docs/schemas/domaincommon.rng  |  5 +++
>>  src/conf/domain_conf.c | 22 +-
>>  src/conf/domain_conf.h |  1 +
>>  src/qemu/qemu_command.c|  2 +-
>>  src/qemu/qemu_domain.c | 20 +++--
>>  src/qemu/qemu_domain.h |  3 +-
>>  src/qemu/qemu_hotplug.c|  4 +-
>>  ...uxml2argv-serial-tcp-tlsx509-chardev-notls.args | 30 +
>>  ...muxml2argv-serial-tcp-tlsx509-chardev-notls.xml | 50 
>> ++
>>  tests/qemuxml2argvtest.c   |  3 ++
>>  ...xml2xmlout-serial-tcp-tlsx509-chardev-notls.xml |  1 +
>>  tests/qemuxml2xmltest.c|  1 +
>>  13 files changed, 162 insertions(+), 8 deletions(-)
>>  create mode 100644 
>> tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.args
>>  create mode 100644 
>> tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml
>>  create mode 12 
>> tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-notls.xml
>>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index 9051178..da6be67 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -6204,6 +6204,34 @@ qemu-kvm -net nic,model=? /dev/null
>>/devices
>>...
>>  
>> +
>> +  Since 2.4.0, the optional attribute
>> +  tls can be used to control whether a serial chardev
>> +  TCP communication channel would utilize a hypervisor configured
>> +  TLS X.509 certificate environment in order to encrypt the data
>> +  channel. For the QEMU hypervisor, usage of a TLS envronment can
>> +  be controlled on the host by the chardev_tls and
>> +  chardev_tls_x509_cert_dir or
>> +  default_tls_x509_cert_dir settings in the file
>> +  /etc/libvirt/qemu.conf. If chardev_tls is enabled,
>> +  then unless the tls attribute is set to "no", libvirt
>> +  will use the host configured TLS environment.
>> +  If chardev_tls is disabled, but the tls
>> +  attribute is set to "yes", then libvirt will attempt to use the
>> +  host TLS environment if either the 
>> chardev_tls_x509_cert_dir
>> +  or default_tls_x509_cert_dir TLS directory structure 
>> exists.
>> +
> 
> Nice, this is a good description how to use the *tls* attribute.
> 

BTW (regarding your followup reply):

The 4 "consumers" of virDomainChrSourceDefParseXML (where this would be
parsed) refer to this as a "serial chardev"

The 'virDomainChrDefParseXML' comments have a list of " definition.

The 'smartcard' discussion for a 'passthrough' device that would use
this code says "Rather than having the hypervisor directly communicate
with the host, it is possible to tunnel all requests through a secondary
character device to a third-party provider (which may in turn be talking
to a smartcard or using three certificate files). In this mode of
operation, an additional attribute type is required, matching one of the
supported serial device types, to describe the host side of the tunnel;..."

The 'rng' discussion for backend that would use this code says "This
backend connects to a source using the EGD protocol. The source is
specified as a character device. Refer to character device host
interface for more information. ..."

Redevdir says "An additional attribute type is required, matching one of
the supported serial device types, to describe the host side of the
tunnel; type='tcp' or type='spicevmc' ..."

So the long and short of it is, IMO it's a serial chardev device.
Semantically it could be claimed otherwise, but the parsing proves
otherwise as does the existing documentation of "Host interface"
character devices.

I prefer to keep it described as is. It's only ever used, parsed, etc.
when ... ... > +
>> +  ...
>> +  devices
>> +serial type="tcp"
>> +  source mode='connect' host="127.0.0.1" service="" 
>> tls="yes"/
>> +  protocol type="raw"/
>> +  target port="0"/
>> +/serial
>> +  /devices
>> +  ...
>> +
>>

Re: [libvirt] [PATCH v10 2/4] domain: Add optional 'tls' attribute for TCP chardev

2016-10-20 Thread Pavel Hrdina 
On Thu, Oct 20, 2016 at 08:51:45AM +0200, Pavel Hrdina wrote:
> On Wed, Oct 19, 2016 at 04:53:54PM -0400, John Ferlan wrote:
> > Add an optional "tls='yes|no'" attribute for a TCP chardev.
> > 
> > For QEMU, this will allow for disabling the host config setting of the
> > 'chardev_tls' for a domain chardev channel by setting the value to "no" or
> > to attempt to use a host TLS environment when setting the value to "yes"
> > when the host config 'chardev_tls' setting is disabled, but a TLS 
> > environment
> > is configured via either the host config 'chardev_tls_x509_cert_dir' or
> > 'default_tls_x509_cert_dir'
> > 
> > Alter qemuDomainSupportTLSChardevTCP to augment the decision points for
> > choosing whether to try to use TLS.
> > 
> > Signed-off-by: John Ferlan 
> > ---
> >  docs/formatdomain.html.in  | 28 
> >  docs/schemas/domaincommon.rng  |  5 +++
> >  src/conf/domain_conf.c | 22 +-
> >  src/conf/domain_conf.h |  1 +
> >  src/qemu/qemu_command.c|  2 +-
> >  src/qemu/qemu_domain.c | 20 +++--
> >  src/qemu/qemu_domain.h |  3 +-
> >  src/qemu/qemu_hotplug.c|  4 +-
> >  ...uxml2argv-serial-tcp-tlsx509-chardev-notls.args | 30 +
> >  ...muxml2argv-serial-tcp-tlsx509-chardev-notls.xml | 50 
> > ++
> >  tests/qemuxml2argvtest.c   |  3 ++
> >  ...xml2xmlout-serial-tcp-tlsx509-chardev-notls.xml |  1 +
> >  tests/qemuxml2xmltest.c|  1 +
> >  13 files changed, 162 insertions(+), 8 deletions(-)
> >  create mode 100644 
> > tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.args
> >  create mode 100644 
> > tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml
> >  create mode 12 
> > tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-notls.xml
> > 
> > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> > index 9051178..da6be67 100644
> > --- a/docs/formatdomain.html.in
> > +++ b/docs/formatdomain.html.in
> > @@ -6204,6 +6204,34 @@ qemu-kvm -net nic,model=? /dev/null
> >/devices
> >...
> >  
> > +
> > +  Since 2.4.0, the optional attribute
> > +  tls can be used to control whether a serial chardev

Remove reference to "serial" because this is valid for all chardevs.

Pavel

> > +  TCP communication channel would utilize a hypervisor configured
> > +  TLS X.509 certificate environment in order to encrypt the data
> > +  channel. For the QEMU hypervisor, usage of a TLS envronment can
> > +  be controlled on the host by the chardev_tls and
> > +  chardev_tls_x509_cert_dir or
> > +  default_tls_x509_cert_dir settings in the file
> > +  /etc/libvirt/qemu.conf. If chardev_tls is enabled,
> > +  then unless the tls attribute is set to "no", libvirt
> > +  will use the host configured TLS environment.
> > +  If chardev_tls is disabled, but the tls
> > +  attribute is set to "yes", then libvirt will attempt to use the
> > +  host TLS environment if either the 
> > chardev_tls_x509_cert_dir
> > +  or default_tls_x509_cert_dir TLS directory structure 
> > exists.
> > +

[...]


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

Re: [libvirt] [PATCH v10 2/4] domain: Add optional 'tls' attribute for TCP chardev

2016-10-20 Thread Pavel Hrdina 
On Wed, Oct 19, 2016 at 04:53:54PM -0400, John Ferlan wrote:
> Add an optional "tls='yes|no'" attribute for a TCP chardev.
> 
> For QEMU, this will allow for disabling the host config setting of the
> 'chardev_tls' for a domain chardev channel by setting the value to "no" or
> to attempt to use a host TLS environment when setting the value to "yes"
> when the host config 'chardev_tls' setting is disabled, but a TLS environment
> is configured via either the host config 'chardev_tls_x509_cert_dir' or
> 'default_tls_x509_cert_dir'
> 
> Alter qemuDomainSupportTLSChardevTCP to augment the decision points for
> choosing whether to try to use TLS.
> 
> Signed-off-by: John Ferlan 
> ---
>  docs/formatdomain.html.in  | 28 
>  docs/schemas/domaincommon.rng  |  5 +++
>  src/conf/domain_conf.c | 22 +-
>  src/conf/domain_conf.h |  1 +
>  src/qemu/qemu_command.c|  2 +-
>  src/qemu/qemu_domain.c | 20 +++--
>  src/qemu/qemu_domain.h |  3 +-
>  src/qemu/qemu_hotplug.c|  4 +-
>  ...uxml2argv-serial-tcp-tlsx509-chardev-notls.args | 30 +
>  ...muxml2argv-serial-tcp-tlsx509-chardev-notls.xml | 50 
> ++
>  tests/qemuxml2argvtest.c   |  3 ++
>  ...xml2xmlout-serial-tcp-tlsx509-chardev-notls.xml |  1 +
>  tests/qemuxml2xmltest.c|  1 +
>  13 files changed, 162 insertions(+), 8 deletions(-)
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.args
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml
>  create mode 12 
> tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-notls.xml
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 9051178..da6be67 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -6204,6 +6204,34 @@ qemu-kvm -net nic,model=? /dev/null
>/devices
>...
>  
> +
> +  Since 2.4.0, the optional attribute
> +  tls can be used to control whether a serial chardev
> +  TCP communication channel would utilize a hypervisor configured
> +  TLS X.509 certificate environment in order to encrypt the data
> +  channel. For the QEMU hypervisor, usage of a TLS envronment can
> +  be controlled on the host by the chardev_tls and
> +  chardev_tls_x509_cert_dir or
> +  default_tls_x509_cert_dir settings in the file
> +  /etc/libvirt/qemu.conf. If chardev_tls is enabled,
> +  then unless the tls attribute is set to "no", libvirt
> +  will use the host configured TLS environment.
> +  If chardev_tls is disabled, but the tls
> +  attribute is set to "yes", then libvirt will attempt to use the
> +  host TLS environment if either the 
> chardev_tls_x509_cert_dir
> +  or default_tls_x509_cert_dir TLS directory structure 
> exists.
> +

Nice, this is a good description how to use the *tls* attribute.

> +
> +  ...
> +  devices
> +serial type="tcp"
> +  source mode='connect' host="127.0.0.1" service="" 
> tls="yes"/
> +  protocol type="raw"/
> +  target port="0"/
> +/serial
> +  /devices
> +  ...
> +
>  UDP network console
>  
>  
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 3106510..e6741bb 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -3453,6 +3453,11 @@
>  
>
>  
> +
> +  
> +
> +  
> +
>  
>
>  
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 89473db..e4fa9ad 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1997,6 +1997,8 @@ virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest,
>  
>  if (VIR_STRDUP(dest->data.tcp.service, src->data.tcp.service) < 0)
>  return -1;
> +
> +dest->data.tcp.haveTLS = src->data.tcp.haveTLS;
>  break;
>  
>  case VIR_DOMAIN_CHR_TYPE_UNIX:
> @@ -10040,6 +10042,7 @@ 
> virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
>  char *master = NULL;
>  char *slave = NULL;
>  char *append = NULL;
> +char *haveTLS = NULL;
>  int remaining = 0;
>  
>  while (cur != NULL) {
> @@ -10047,6 +10050,8 @@ 
> virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
>  if (xmlStrEqual(cur->name, BAD_CAST "source")) {
>  if (!mode)
>  mode = virXMLPropString(cur, "mode");
> +if (!haveTLS)
> +haveTLS = virXMLPropString(cur, "tls");
>  
>  switch ((virDomainChrType) def->type) {
>  case VIR_DOMAIN_CHR_TYPE_FILE:
>

[libvirt] [PATCH v10 2/4] domain: Add optional 'tls' attribute for TCP chardev

2016-10-19 Thread John Ferlan 
Add an optional "tls='yes|no'" attribute for a TCP chardev.

For QEMU, this will allow for disabling the host config setting of the
'chardev_tls' for a domain chardev channel by setting the value to "no" or
to attempt to use a host TLS environment when setting the value to "yes"
when the host config 'chardev_tls' setting is disabled, but a TLS environment
is configured via either the host config 'chardev_tls_x509_cert_dir' or
'default_tls_x509_cert_dir'

Alter qemuDomainSupportTLSChardevTCP to augment the decision points for
choosing whether to try to use TLS.

Signed-off-by: John Ferlan 
---
 docs/formatdomain.html.in  | 28 
 docs/schemas/domaincommon.rng  |  5 +++
 src/conf/domain_conf.c | 22 +-
 src/conf/domain_conf.h |  1 +
 src/qemu/qemu_command.c|  2 +-
 src/qemu/qemu_domain.c | 20 +++--
 src/qemu/qemu_domain.h |  3 +-
 src/qemu/qemu_hotplug.c|  4 +-
 ...uxml2argv-serial-tcp-tlsx509-chardev-notls.args | 30 +
 ...muxml2argv-serial-tcp-tlsx509-chardev-notls.xml | 50 ++
 tests/qemuxml2argvtest.c   |  3 ++
 ...xml2xmlout-serial-tcp-tlsx509-chardev-notls.xml |  1 +
 tests/qemuxml2xmltest.c|  1 +
 13 files changed, 162 insertions(+), 8 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml
 create mode 12 
tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-notls.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 9051178..da6be67 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -6204,6 +6204,34 @@ qemu-kvm -net nic,model=? /dev/null
   /devices
   ...
 
+
+  Since 2.4.0, the optional attribute
+  tls can be used to control whether a serial chardev
+  TCP communication channel would utilize a hypervisor configured
+  TLS X.509 certificate environment in order to encrypt the data
+  channel. For the QEMU hypervisor, usage of a TLS envronment can
+  be controlled on the host by the chardev_tls and
+  chardev_tls_x509_cert_dir or
+  default_tls_x509_cert_dir settings in the file
+  /etc/libvirt/qemu.conf. If chardev_tls is enabled,
+  then unless the tls attribute is set to "no", libvirt
+  will use the host configured TLS environment.
+  If chardev_tls is disabled, but the tls
+  attribute is set to "yes", then libvirt will attempt to use the
+  host TLS environment if either the chardev_tls_x509_cert_dir
+  or default_tls_x509_cert_dir TLS directory structure exists.
+
+
+  ...
+  devices
+serial type="tcp"
+  source mode='connect' host="127.0.0.1" service="" tls="yes"/
+  protocol type="raw"/
+  target port="0"/
+/serial
+  /devices
+  ...
+
 UDP network console
 
 
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 3106510..e6741bb 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3453,6 +3453,11 @@
 
   
 
+
+  
+
+  
+
 
   
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 89473db..e4fa9ad 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1997,6 +1997,8 @@ virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest,
 
 if (VIR_STRDUP(dest->data.tcp.service, src->data.tcp.service) < 0)
 return -1;
+
+dest->data.tcp.haveTLS = src->data.tcp.haveTLS;
 break;
 
 case VIR_DOMAIN_CHR_TYPE_UNIX:
@@ -10040,6 +10042,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr 
def,
 char *master = NULL;
 char *slave = NULL;
 char *append = NULL;
+char *haveTLS = NULL;
 int remaining = 0;
 
 while (cur != NULL) {
@@ -10047,6 +10050,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr 
def,
 if (xmlStrEqual(cur->name, BAD_CAST "source")) {
 if (!mode)
 mode = virXMLPropString(cur, "mode");
+if (!haveTLS)
+haveTLS = virXMLPropString(cur, "tls");
 
 switch ((virDomainChrType) def->type) {
 case VIR_DOMAIN_CHR_TYPE_FILE:
@@ -10223,6 +10228,15 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr 
def,
 def->data.tcp.listen = true;
 }
 
+if (haveTLS &&
+(def->data.tcp.haveTLS =
+ virTristateBoolTypeFromString(haveTLS)) <= 0) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("unknown chardev 'tls' setting '%s'"),
+