On 10/14/2016 10:53 AM, Pavel Hrdina wrote:
On Fri, Oct 07, 2016 at 07:00:27AM -0400, John Ferlan wrote:
> Add a new qemu.conf variables to store the UUID for the secret that could
> be used to present credentials to access the TLS chardev. Since this will
> be a server level and it's possible to use some sort of default, introduce
> both the default and chardev logic at the same time making the setting of
> the chardev check for it's own value, then if not present checking whether
> the default value had been set.
>
> The chardevTLSx509haveUUID bool will be used as the marker for whether
> the chardevTLSx509secretUUID was successfully read. In the future this
> is how it'd determined whether to add the secret object for a TLS object.
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/qemu/libvirtd_qemu.aug | 2 ++
> src/qemu/qemu.conf | 24 ++++++++++++++++++++++++
> src/qemu/qemu_conf.c | 22 ++++++++++++++++++++++
> src/qemu/qemu_conf.h | 3 +++
> src/qemu/test_libvirtd_qemu.aug.in | 2 ++
> 5 files changed, 53 insertions(+)
>
> diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
> index 988201e..73ebeda 100644
> --- a/src/qemu/libvirtd_qemu.aug
> +++ b/src/qemu/libvirtd_qemu.aug
> @@ -29,6 +29,7 @@ module Libvirtd_qemu =
> (* Config entry grouped by function - same order as example config *)
> let default_tls_entry = str_entry "default_tls_x509_cert_dir"
> | bool_entry "default_tls_x509_verify"
> + | str_entry "default_tls_x509_secret_uuid"
>
> let vnc_entry = str_entry "vnc_listen"
> | bool_entry "vnc_auto_unix_socket"
> @@ -51,6 +52,7 @@ module Libvirtd_qemu =
> let chardev_entry = bool_entry "chardev_tls"
> | str_entry "chardev_tls_x509_cert_dir"
> | bool_entry "chardev_tls_x509_verify"
> + | str_entry "chardev_tls_x509_secret_uuid"
>
> let nogfx_entry = bool_entry "nographics_allow_host_audio"
>
> diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
> index e4c2aae..f136fa9 100644
> --- a/src/qemu/qemu.conf
> +++ b/src/qemu/qemu.conf
> @@ -28,6 +28,20 @@
> #
> #default_tls_x509_verify = 1
>
> +#
> +# Libvirt assumes the server-key.pem file is unencrypted by default.
> +# To use an encrypted server-key.pem file, the password to decrypt the
s/ the//
OK
> +# the PEM file is requird. This can be provided by creating a
secret
Fixed required too..
> +# object in libvirt and then uncommenting this setting to set
the UUID
changed uncommenting to "to uncomment"
> +# UUID of the secret.
> +#
> +# NB This default all-zeros UUID will not work. Replace it with the
> +# output from the UUID for the TLS secret from a 'virsh secret-list'
> +# command and then uncomment the entry
> +#
> +#default_tls_x509_secret_uuid = "00000000-0000-0000-0000-000000000000"
> +
> +
> # VNC is configured to listen on 127.0.0.1 by default.
> # To make it listen on all public interfaces, uncomment
> # this next option.
> @@ -214,6 +228,16 @@
> #chardev_tls_x509_verify = 1
>
>
> +# Uncomment and use the following option to override the default secret
> +# uuid provided in the default_tls_x509_secret_uuid parameter.
> +#
> +# NB This default all-zeros UUID will not work. Replace it with the
> +# output from the UUID for the TLS secret from a 'virsh secret-list'
> +# command and then uncomment the entry
> +#
> +#chardev_tls_x509_secret_uuid = "00000000-0000-0000-0000-000000000000"
> +
> +
> # By default, if no graphical front end is configured, libvirt will disable
> # QEMU audio output since directly talking to alsa/pulseaudio may not work
> # with various security settings. If you know what you're doing, enable
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 635fa27..c650961 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -365,6 +365,7 @@ static void virQEMUDriverConfigDispose(void *obj)
> VIR_FREE(cfg->nvramDir);
>
> VIR_FREE(cfg->defaultTLSx509certdir);
> + VIR_FREE(cfg->defaultTLSx509secretUUID);
>
> VIR_FREE(cfg->vncTLSx509certdir);
> VIR_FREE(cfg->vncListen);
> @@ -377,6 +378,7 @@ static void virQEMUDriverConfigDispose(void *obj)
> VIR_FREE(cfg->spiceSASLdir);
>
> VIR_FREE(cfg->chardevTLSx509certdir);
> + VIR_FREE(cfg->chardevTLSx509secretUUID);
>
> while (cfg->nhugetlbfs) {
> cfg->nhugetlbfs--;
> @@ -424,6 +426,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
> int ret = -1;
> int rv;
> size_t i, j;
> + bool defaultTLSx509haveUUID;
> char *stdioHandler = NULL;
> char *user = NULL, *group = NULL;
> char **controllers = NULL;
> @@ -446,6 +449,12 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
> goto cleanup;
> if (virConfGetValueBool(conf, "default_tls_x509_verify",
&cfg->defaultTLSx509verify) < 0)
> goto cleanup;
> + if ((rv = virConfGetValueString(conf, "default_tls_x509_secret_uuid",
> + &cfg->defaultTLSx509secretUUID)) < 0)
> + goto cleanup;
> + if (rv == 1)
> + defaultTLSx509haveUUID = true;
I don't see any reason to use the extra bool variable. Function
virConfGetValueString() has this logic:
-1 - we error out
0 - string was not found, value is set to NULL
1 - string was found, value is set
IOW you can just use defaultTLSx509secretUUID to detect whether there is
something configured or not. The same applies to chardevTLSx509haveUUID.
Well it's been so long I have no idea what I was thinking, but yeah the
booleans are unnecessary. I'll remove them.
> +
> if (virConfGetValueBool(conf, "vnc_auto_unix_socket",
&cfg->vncAutoUnixSocket) < 0)
> goto cleanup;
> if (virConfGetValueBool(conf, "vnc_tls", &cfg->vncTLS) < 0)
> @@ -513,6 +522,19 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
> goto cleanup;
> if (rv == 0)
> cfg->chardevTLSx509verify = cfg->defaultTLSx509verify;
> + if ((rv = virConfGetValueString(conf, "chardev_tls_x509_secret_uuid",
> + &cfg->chardevTLSx509secretUUID)) < 0)
> + goto cleanup;
> + if (rv == 1) {
> + cfg->chardevTLSx509haveUUID = true;
> + } else {
> + if (defaultTLSx509haveUUID) {
> + if (VIR_STRDUP(cfg->chardevTLSx509secretUUID,
> + cfg->defaultTLSx509secretUUID) < 0)
> + goto cleanup;
> + cfg->chardevTLSx509haveUUID = true;
> + }
Copying the defaultTLSx509secretUUID should be done only in case rv == 0,
otherwise we need to call goto cleanup;
This is changed to:
if (!cfg->chardevTLSx509secretUUID && cfg->defaultTLSx509secretUUID) {
IOW: We didn't find a chardev specific UUID, but do have a default one,
so copy it.
The setting affects patch 4 as well (since chardevTLSx509haveUUID would
be removed).
Tks -
John
[...]