Libvirt uses the path you told it to use. default_tls_x509_cert_dir is only used if the more specific option is not set.

Oh actually I see, the comment is quite misleading... Let's use this bz to fix the comment.

Not entirely a qemu.conf doc bug, but more a bug of using a defined value. Prior to reading the qemu.conf file, the various settings are initialized to /etc/pki/qemu (e.g. the default).  When overwriting, the existence was not checked. I've sent a couple of patches to handle this, see cover letter:

https://www.redhat.com/archives/libvir-list/2017-June/msg01278.html

Ironically, the default /etc/pki/qemu is not checked for existence and it cannot be checked as that would cause failure to start libvirtd even if no domain used TLS. Failure to have /etc/pki/qemu and failure to have the valid contents of that directory would still be handled by QEMU failure. That's been added to qemu.conf as part of these patches.

$ git describe dc4c2f75ab233561f47c8804667ed129dad05d69
v3.6.0-33-gdc4c2f7
$ git show
commit dc4c2f75ab233561f47c8804667ed129dad05d69
Author: John Ferlan <jferlan@redhat.com>
Date:   Thu Jun 29 08:27:55 2017 -0400

    qemu: Check for existence of provided *_tls_x509_cert_dir
    
...
    
    Introduce virQEMUDriverConfigTLSDirResetDefaults in order to check
    if the defaultTLSx509certdir was changed, then change the default
    for any other *TLSx509certdir that was not set to the default default.
    
    Introduce virQEMUDriverConfigValidate to validate the existence of
    any of the *_tls_x509_cert_dir values that were uncommented/set,
    incuding the default.
    
    Update the qemu.conf description for default to describe the consequences
    if the default directory path does not exist.
    
    Signed-off-by: John Ferlan <jferlan@redhat.com>

Hi John

I tried to verify this bug with libvirt-3.8.0-1.virtcov.el7.x86_64.

The current behaviour is that if *_tls_x509_cert_dir is set to a path that doesn't exist, libvirtd will fail to start.

But the description in qemu.conf for {spice, vnc, migrate, chardev, vxhs}_tls_x509_cert_dir is not updated, it still says:

# In order to override the default TLS certificate location for
# spice certificates, supply a valid path to the certificate directory.
# If the provided path does not exist then the default_tls_x509_cert_dir
# path will be used.

And the description for default_tls_x509_cert_dir is not correct, it says:

# If the directory does not exist or contain the necessary files, QEMU
# domains will fail to start if they are configured to use TLS.

The following excerpts are taken from source - tell me exactly what's wrong with the wording, provide an example of better wording, and I'll work to get it in.  If your system doesn't have the updated wording, then you may want to check if a libvirtd.conf.rpmnew file exists.

Tks -

John

...
# In order to overwrite the default path alter the following. This path
# definition will be used as the default path for other *_tls_x509_cert_dir
# configuration settings if their default path does not exist or is not
# specifically set.
#
#default_tls_x509_cert_dir = "/etc/pki/qemu"
...
# In order to override the default TLS certificate location for
# vnc certificates, supply a valid path to the certificate directory.
# If the provided path does not exist then the default_tls_x509_cert_dir
# path will be used.
#
#vnc_tls_x509_cert_dir = "/etc/pki/libvirt-vnc"
...
# In order to override the default TLS certificate location for
# spice certificates, supply a valid path to the certificate directory.
# If the provided path does not exist then the default_tls_x509_cert_dir
# path will be used.
#
#spice_tls_x509_cert_dir = "/etc/pki/libvirt-spice"
...
# In order to override the default TLS certificate location for character
# device TCP certificates, supply a valid path to the certificate directory.
# If the provided path does not exist then the default_tls_x509_cert_dir
# path will be used.
#
#chardev_tls_x509_cert_dir = "/etc/pki/libvirt-chardev"
...
#
# If the provided path does not exist then the default_tls_x509_cert_dir
# path will be used.
#
# VxHS block device clients expect the client certificate and key to be
# present in the certificate directory along with the CA master certificate.
# If using the default environment, default_tls_x509_verify must be configured.
# Since this is only a client the server-key.pem certificate is not needed.
# Thus a VxHS directory must contain the following:
#
#  ca-cert.pem - the CA master certificate
#  client-cert.pem - the client certificate signed with the ca-cert.pem
#  client-key.pem - the client private key
#
#vxhs_tls_x509_cert_dir = "/etc/pki/libvirt-vxhs"
...
# In order to override the default TLS certificate location for migration
# certificates, supply a valid path to the certificate directory. If the
# provided path does not exist then the default_tls_x509_cert_dir path
# will be used. Once/if a default certificate is enabled/defined, migration
# will then be able to use the certificate via migration API flags.
#
#migrate_tls_x509_cert_dir = "/etc/pki/libvirt-migrate"

Hi John


1) For default_tls_x509_cert_dir, this sentence is not correct:

"# If the directory does not exist or contain the necessary files, QEMU
# domains will fail to start if they are configured to use TLS."


I think it should be like: 

"# If the directory does not exist, libvirtd will fail to start.
# If the directory doesn't contain the necessary files, QEMU
# domains will fail to start if they are configured to use TLS."


2) For other *_tls_x509_cert_dir, this sentence is not correct:

"# If the provided path does not exist then the default_tls_x509_cert_dir
# path will be used."


I think it should be like:

"# If the provided path does not exist, libvirtd will fail to start.
# If the default path for vnc_tls_cert_dir does not exist, then the 
# default_tls_x509_cert_dir path will be used."

No need for the needsinfo since you've provided the information I asked for!

In any case, for the non default tls path cases, it's more like:

# If the path is not provided, but vnc_tls = 1, then the
# default_tls_x509_cert_dir path will be used.

where "vnc" is also spice, chardev, vxhs, 


In any case, I've posted a patch - let's see what upstream says:

https://www.redhat.com/archives/libvir-list/2017-October/msg01276.html

Patch was accepted and pushed upstream:


commit c9f7a04e283ef43a0b358b2a337e884e20b86b3c
Author: John Ferlan <jferlan@redhat.com>
Date:   Fri Oct 27 05:23:25 2017 -0400

    qemu.conf: Clarify the various _tls_x509_cert_dir descriptions
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1458630
    
    Apparantly commit id 'dc4c2f75a' wasn't specific enough, so here's
    a few more clarifications.

$ git describe c9f7a04e283ef43a0b358b2a337e884e20b86b3c
CVE-2017-1000256-149-gc9f7a04e28
$

Hi John

I still have other opinions about the description for *_tls_x509_cert_dir. Please have a look. Thank you!

Version: libvirt-3.9.0-1.el7.x86_64

1) For default_tls_x509_cert_dir, I think it will be more accurate if change the "or" to "and" in this sentence:

# In order to overwrite the default path alter the following. This path
# definition will be used as the default path for other *_tls_x509_cert_dir
# configuration settings if their default path does not exist or is not
                                                              ^^
# specifically set.
#
#default_tls_x509_cert_dir = "/etc/pki/qemu"


2) For other *_tls_x509_cert_dir, the following sentence is not correct. Because the actual behavior is that if the path is not provided, the default path /etc/pki/libvirt/vnc will used. If the default path doesn't exist, default_tls_x509_cert will be used.


# If the path is not provided, but vnc_tls = 1, then the
# default_tls_x509_cert_dir path will be used.
#
#vnc_tls_x509_cert_dir = "/etc/pki/libvirt-vnc"

1) I think "or" is fine there

For the non default TLS dirs if what's shown as the default directory exists, then that's saved in the cfg->*TLSx509certdir field at cfg initialization; however, if that directory doesn't exist, then whatever was used for the defaultTLSx509certdir is duplicated. So it's either the "provided" path for each other tls dir or it will be whatever the default is set to.

Although it's perhaps semantics, the "is not specifically set" implies uncommenting the field. If someone (for whatever reason) changed the commented field to something else and expected that to be used, they'd be mistaken.

Using "and" to me could imply that if someone supplied a path and it didn't exist, then the default would be used. That is not the case, if someone provides an uncommented path and it doesn't exist, then an error 

For example, if the "/etc/pki/libvirt-vnc" directory exists, then internally cfg->vncTLSx509certdir will be "/etc/pki/libvirt-vnc" regardless of whether vnc_tls = 1 is uncommented.  If the "/etc/pki/libvirt-vnc" doesn't exist, then 
cfg->vncTLSx509certdir will be whatever is found in cfg->defaultTLSx509certdir.


2) I disagree

While I see your point, I think if someone reads the default_tls_x509_cert_dir description they could easily infer that the path used will be either what's shown in the commented value for the *_tls_x509_cert_dir or whatever the default_tls_x509_cert_dir value was set to in the event that commented path doesn't exist. It's very difficult to describe though in "precise wording" that won't be too verbose.

The best this could be changed to would be:

"If the path is not provided or the default shown commented path does not exist, but vnc_tls = 1, then the default_tls_x509_cert_dir path will be used."

But to me that's really overkill since the default_tls_x509_cert_dir path indicates "...This path definition will be used as the default path for other *_tls_x509_cert_dir configuration settings if their default path does not exist or is not specifically set."

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHEA-2018:0704