[libvirt] [PATCH v4 4/5] Support for multiple default security drivers in QEMU config

[Michal Privoznik]

On 16.08.2012 00:10, Marcelo Cerri wrote:
> This patch updates the key "security_driver" in QEMU config to suport
> both a sigle default driver or a list of default drivers. This ensures
> that it will remain compatible with older versions of the config file.
> 
> Signed-off-by: Marcelo Cerri <mhcerri at linux.vnet.ibm.com>
> ---
>  src/qemu/qemu.conf     |    6 ++-
>  src/qemu/qemu_conf.c   |   38 +++++++++++++++++-
>  src/qemu/qemu_conf.h   |    2 +-
>  src/qemu/qemu_driver.c |   99 +++++++++++++++++++++++++++++++++++++-----------
>  4 files changed, 118 insertions(+), 27 deletions(-)
> 
> diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
> index ed4683c..257a477 100644
> --- a/src/qemu/qemu.conf
> +++ b/src/qemu/qemu.conf
> @@ -144,7 +144,11 @@
>  # on the host, then the security driver will automatically disable
>  # itself. If you wish to disable QEMU SELinux security driver while
>  # leaving SELinux enabled for the host in general, then set this
> -# to 'none' instead.
> +# to 'none' instead. It's also possible to use more than one security
> +# driver at the same time, for this use a list of names separated by
> +# comma and delimited by square brackets. For example:
> +#
> +#       security_driver = [ "selinux", "dac" ]
>  #
>  #security_driver = "selinux"
>  
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index b7db277..ed6d832 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -193,13 +193,45 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
>      }
>  
>      p = virConfGetValue (conf, "security_driver");
> -    CHECK_TYPE ("security_driver", VIR_CONF_STRING);
> -    if (p && p->str) {
> -        if (!(driver->securityDriverName = strdup(p->str))) {
> +    if (p && p->type == VIR_CONF_LIST) {
> +        size_t len;
> +        virConfValuePtr pp;
> +
> +        /* Calc lenght and check items */
> +        for (len = 0, pp = p->list; pp; len++, pp = pp->next) {
> +            if (pp->type != VIR_CONF_STRING) {
> +                VIR_ERROR(_("security_driver be a list of strings"));
> +                virConfFree(conf);
> +                return -1;
> +            }
> +        }
> +
> +        if (VIR_ALLOC_N(driver->securityDriverNames, len + 1) < 0) {
>              virReportOOMError();
>              virConfFree(conf);
>              return -1;
>          }
> +
> +        for (i = 0, pp = p->list; pp; i++, pp = pp->next) {
> +            driver->securityDriverNames[i] = strdup(pp->str);
> +            if (driver->securityDriverNames == NULL) {
> +                virReportOOMError();
> +                virConfFree(conf);
> +                return -1;
> +            }
> +        }
> +        driver->securityDriverNames[len] = NULL;
> +    } else {
> +        CHECK_TYPE ("security_driver", VIR_CONF_STRING);
> +        if (p && p->str) {
> +            if (VIR_ALLOC_N(driver->securityDriverNames, 2) < 0 ||
> +                !(driver->securityDriverNames[0] = strdup(p->str))) {
> +                virReportOOMError();
> +                virConfFree(conf);
> +                return -1;
> +            }
> +            driver->securityDriverNames[1] = NULL;
> +        }
>      }
>  
>      p = virConfGetValue (conf, "security_default_confined");
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index 92e4968..8a51471 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -116,7 +116,7 @@ struct qemud_driver {
>  
>      virDomainEventStatePtr domainEventState;
>  
> -    char *securityDriverName;
> +    char **securityDriverNames;
>      bool securityDefaultConfined;
>      bool securityRequireConfined;
>      virSecurityManagerPtr securityManager;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 48376c3..0c88310 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -248,36 +248,91 @@ qemuAutostartDomains(struct qemud_driver *driver)
>  static int
>  qemuSecurityInit(struct qemud_driver *driver)
>  {
> -    virSecurityManagerPtr mgr = virSecurityManagerNew(driver->securityDriverName,
> -                                                      QEMU_DRIVER_NAME,
> -                                                      driver->allowDiskFormatProbing,
> -                                                      driver->securityDefaultConfined,
> -                                                      driver->securityRequireConfined);
> +    char **names;
> +    char *primary;
> +    virSecurityManagerPtr mgr, nested, stack;

Again, gcc thinks 'stack' may be used uninitialized ...

>  
> +    if (driver->securityDriverNames == NULL)
> +        primary = NULL;
> +    else
> +        primary = driver->securityDriverNames[0];
> +
> +    /* Create primary driver */
> +    mgr = virSecurityManagerNew(primary,
> +                                QEMU_DRIVER_NAME,
> +                                driver->allowDiskFormatProbing,
> +                                driver->securityDefaultConfined,
> +                                driver->securityRequireConfined);
>      if (!mgr)
>          goto error;
>  
> +    /* If a DAC driver is required or additional drivers are provived, a stack
> +     * driver should be create to group them all */
> +    if (driver->privileged ||
> +        (driver->securityDriverNames && driver->securityDriverNames[1])) {
> +        stack = virSecurityManagerNewStack(mgr);
> +        if (!stack)
> +            goto error;
> +        mgr = stack;
> +    }
> +
> +    /* Loop through additional driver names and add a secudary driver to each
> +     * one */
> +    if (driver->securityDriverNames) {
> +        names = driver->securityDriverNames + 1;
> +        while (names && *names) {
> +            if (STREQ("dac", *names)) {
> +                /* A DAC driver has specific parameters */
> +                nested = virSecurityManagerNewDAC(QEMU_DRIVER_NAME,
> +                                                  driver->user,
> +                                                  driver->group,
> +                                                  driver->allowDiskFormatProbing,
> +                                                  driver->securityDefaultConfined,
> +                                                  driver->securityRequireConfined,
> +                                                  driver->dynamicOwnership);
> +            } else {
> +                nested = virSecurityManagerNew(*names,
> +                                               QEMU_DRIVER_NAME,
> +                                               driver->allowDiskFormatProbing,
> +                                               driver->securityDefaultConfined,
> +                                               driver->securityRequireConfined);
> +            }
> +            if (nested == NULL)
> +                goto error;
> +            if (virSecurityManagerStackAddNested(stack, nested))

... here.

ACK with this squashed in:
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a70ca92..116d447 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -250,7 +250,7 @@ qemuSecurityInit(struct qemud_driver *driver)
 {
     char **names;
     char *primary;
-    virSecurityManagerPtr mgr, nested, stack;
+    virSecurityManagerPtr mgr, nested, stack = NULL;

     if (driver->securityDriverNames == NULL)
         primary = NULL;