[libvirt] [PATCH v9 1/5] domain: Add optional 'tls' attribute for TCP chardev

[John Ferlan]

On 10/18/2016 07:21 AM, Daniel P. Berrange wrote:
> On Tue, Oct 18, 2016 at 06:59:57AM -0400, John Ferlan wrote:
>>
>>
>> On 10/18/2016 02:27 AM, Pavel Hrdina wrote:
>> [...]
>>
>>>>
>>>> "As default behaviour I think it is desirable that we can turn TLS on
>>>> for every VM at once - I tend to view it as a host network integration
>>>> task, rather than a VM configuration task. Same rationale that we use
>>>> for TLS wth VNC/SPICE."
>>>
>>> Don't forget this part of the same review:
>>>
>>> "There's no reason we can't have a tri-state TLS flag against the chardev
>>> in the XML too, to override the default behaviour of cfg->chardevTLS"
>>>
>>> That also means to override chardev_tls = "0" by "tls='yes'".
>>
>> If the default cfg behaviour is "1", then that tells us "someone" has
>> set up the TLS environment and thus the domain/chardev override would be
>> "no".
>>
>> If the default cfg behaviour is "0", then that means we cannot guarantee
>> the environment necessary has been set up and we cannot allow the
>> domain/chardev setting to enable TLS.
> 
> We have two separate tasks at the host level
> 
>  - Setup of TLS certificates (ie put the PEM files in the right places)
>  - Global default for use of TLS by chardevs
> 
> We only have a config option in qemu.conf for the latter. ie if
> chardev_tls=1, this is implicitly saying that TLS certs are deployed
> in right place.  IIUC, you're saying that if chardev_tls=0, then we
> should interpret that to meant TLS certs are *not* deployed.
> 

If someone has gone through the trouble of setting up their environment
because they found in qemu.conf support was added, why would they set
chardev_tls=0 afterwards? IOW: Are we assuming something or are we over
engineering things?

Why would someone set up and define chardevTLSx509certdir and then have
chardevTLS=0? Maybe, they believe this will disable TLS for every
domain. After all, why would disabling a global setting allow some
localized setting override that.

Just because there's a "default" environment, why should we assume that
because they added "tls='yes'" into their domain that that is the
environment they want?

Maybe they do have a chardev specific environment, but also decided to
comment it out when they commented out chardev_tls=1? So we're assuming
that by finding that "tls='yes'" in the domain that the domain should
use that environment? How can we be so sure that's what's desired?

This is the problem with two triggers - which one do you pull and which
one has a bullet in the chamber? We're having a hard enough time
agreeing on the implementation. Now the customer has to figure out all
the permutations and possibilities.

Personally, if I disable a global variable/value, then I wouldn't expect
some other definition to override that. For example, if I set my SELinux
to be "permissive" or "disabled", then I'm probably not very happy if
something ignores that and decides to be "enforcing".

> Pavel is saying that if chardev_tls=0 in qemu.conf, and tls=1 in the
> XML, then we should assume that TLS certs *are* deployed on the host
> in the right place. I think this is not unreasonable - we can easily
> check to see if the certs exist on disk in this case.

FWIW: Remember how qemu_conf.c sets things up...

During qemuStateInitialize, we call virQEMUDriverConfigNew which will
set a default directory path for "cfg->defaultTLSx509certdir" and then
will use that as a default value for vnc, spice, and chardev. There is
no check "if" that directory structure exists.

Once the defaults are set up, then if "chardev_tls_x509_cert_dir" is
defined, it will override the default (similar for others).

So chardevTLSx509certdir always has some default value, but is that the
certificate environment we want to use for chardev's? How do we know?

> 
> IOW, I agree that we should have a tri-state at the XML level
> 
>  - no tls attribute in XML - honour  chardev_tls from qemu.conf
>  - tls=1 in XML, then turn on TLS
>  - tls=0 in XML, then don't use TLS
> 

So if we get this far, then a qemu_domain.c function, such as

bool
qemuDomainSupportTLSChardevTCP(virQEMUDriverConfigPtr cfg,
                               const virDomainChrSourceDef *dev)
{
    if (cfg->chardevTLS && dev->data.tcp.haveTLS != VIR_TRISTATE_BOOL_NO)
        return true;
    if (!cfg->chardevTLS && dev->data.tcp.haveTLS ==
VIR_TRISTATE_BOOL_YES &&
        virFileExists(cfg->chardevTLSx509certdir))
        return true;
    return false;
}

where I suppose we could also compare chardevTLSx509certdir and
defaultTLSx509certdir... Still how do we know that by merely having the
environment there and "tls='yes'" in the domain chardev config that this
is what's desired.  It might well be, but it may also be that the
clearing of the global chardev_tls was meant to disable regardless of
what the domain setting was.

John