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 <jfer...@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

[...]

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to