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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list