On 06/29/2017 07:49 AM, Daniel P. Berrange wrote: > On Thu, Jun 29, 2017 at 07:45:06AM -0400, John Ferlan wrote: >> >> >> On 06/29/2017 06:40 AM, Daniel P. Berrange wrote: >>> On Thu, Jun 29, 2017 at 09:24:30AM +0200, Jiri Denemark wrote: >>>> On Wed, Jun 28, 2017 at 15:30:28 -0400, John Ferlan wrote: >>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1458630 >>>>> >>>>> Introduce virQEMUDriverConfigSetCertDir which will handle reading the >>>>> qemu.conf config file specific setting for default, vnc, spice, chardev, >>>>> and migrate. Then if a setting was provided, validating the existence of >>>>> the directory and overwriting the default set by virQEMUDriverConfigNew. >>>>> >>>>> Also update the qemu.conf description for default to indicate the consequences >>>>> if the default directory does not exist. >>>>> >>>>> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> >>>>> --- >>>>> src/qemu/qemu.conf | 9 ++++++++- >>>>> src/qemu/qemu_conf.c | 42 ++++++++++++++++++++++++++++++++++-------- >>>>> 2 files changed, 42 insertions(+), 9 deletions(-) >>>>> >>>>> diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf >>>>> index e6c0832..737fa46 100644 >>>>> --- a/src/qemu/qemu.conf >>>>> +++ b/src/qemu/qemu.conf >>>>> @@ -3,7 +3,7 @@ >>>>> # defaults are used. >>>>> >>>>> # Use of TLS requires that x509 certificates be issued. The default is >>>>> -# to keep them in /etc/pki/qemu. This directory must contain >>>>> +# to keep them in /etc/pki/qemu. This directory must exist and contain: >>>>> # >>>>> # ca-cert.pem - the CA master certificate >>>>> # server-cert.pem - the server certificate signed with ca-cert.pem >>>>> @@ -13,6 +13,13 @@ >>>>> # >>>>> # dh-params.pem - the DH params configuration file >>>>> # >>>>> +# If the directory does not exist or does not contain the necessary files, >>>>> +# QEMU domains will fail to start if they are configured to use TLS. >>>>> +# >>>>> +# In order to overwrite the default directory alter the following. If the >>>>> +# provided directory does not exist, then the setting reverts back to the >>>>> +# default /etc/pki/qemu. >>>>> +# >>>> >>>> I don't think this is a good idea. We should use the directory a user >>>> specified in qemu.conf. If it doesn't exist, well things won't work. >>>> Sure, we can complain about it in the logs, but we should not fallback >>>> to any magic default in that case. Anyone setting a custom directory for >>>> TLS certificates does this because they want to use it. If the directory >>>> does not exist, it's either because they forgot to create it or they >>>> made a typo somewhere. It's very unlikely someone actually wants to use >>>> a default directory even though they set a custom one. >>>> >>>> NACK >>> >>> Agreed, I think we need to distinguish between the default dirs for each >>> settings, vs user specified dir for each setting. >>> >>> ie, if the user has *not* set 'chardev_tls_x509_cert_dir' then its default >>> value is '/etc/pki/libvirt-chardev'. If that directory does not exist, >>> then falling back to "default_tls_x509_cert_dir" is good. >> >> This essentially what happens in virQEMUDriverConfigNew. The caveat here >> is that the default value for default_tls_x509_cert_dir (e.g. >> /etc/pki/qemu) is not checked for existence, rather it's assumed. > > As long as we get an error message it is ok, but it might be wise to > explicitly check /etc/pki/qemu if it would give a better error > message to the user. > Currently we'd get an error "eventually" when someone tries to start a guest: error: internal error: process exited while connecting to monitor: 2017-06-28T19:24:18.810938Z qemu-system-x86_64: -object tls-creds-x509,id=objcharserial1_tls0,dir=/etc/pki/qemu,endpoint=client,verify-peer=no: Unable to access credentials /etc/pki/qemu/ca-cert.pem: No such file or directory The problem with checking and generating an error at startup is that we don't create the /etc/pki/qemu directory during install (at least as far as I can tell). If we do, then something's broken because I have a couple of environments without it. John >>> If the user *has* set 'chardev_tls_x509_cert_dir' and it does nto exist, >>> then we should report an hard error, preferrably at startup so the admin >>> sees their mistake immediately. >> >> OK and that answers most of the questions I had... If the user changes >> "default_*" (as processed in virQEMUDriverConfigLoadFile) and it does >> not exist, then should startup fail? > > Yep > > Regards, > Daniel > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
