On Thu, Aug 30, 2012 at 01:37:01AM +0100, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> If no 'security_driver' config option was set, then the code
> just loaded the 'dac' security driver. This is a regression
> on previous behaviour, where we would probe for a possible
> security driver. ie default to SELinux if available.
>
> This changes things so that it 'security_driver' is not set,
> we once again do probing. For simplicity we also always
> create the stack driver, even if there is only one driver
> active.
>
> The desired semantics are:
>
> - security_driver not set
> -> probe for selinux/apparmour/nop
> -> auto-add DAC driver
> - security_driver set to a string
> -> add that one driver
> -> auto-add DAC driver
> - security_driver set to a list
> -> add all drivers in list
> -> auto-add DAC driver
>
> It is not allowed, or possible to specify 'dac' in the
> security_driver config param, since that is always
> enabled.
>
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
> src/qemu/qemu_driver.c | 135 +++++++++++++---------------------------
> src/security/security_manager.c | 8 ++-
> src/security/security_stack.c | 38 ++++-------
> src/security/security_stack.h | 8 ---
> 4 files changed, 61 insertions(+), 128 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 9a25253..374349a 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -249,119 +249,70 @@ static int
> qemuSecurityInit(struct qemud_driver *driver)
> {
> char **names;
> - char *primary = NULL;
> virSecurityManagerPtr mgr = NULL;
> - virSecurityManagerPtr nested = NULL;
> virSecurityManagerPtr stack = NULL;
> bool hasDAC = false;
>
> - /* set the name of the primary security driver */
> - if (driver->securityDriverNames)
> - primary = driver->securityDriverNames[0];
> -
> - /* add primary security driver */
> - if ((primary == NULL && driver->privileged) ||
> - STREQ_NULLABLE(primary, "dac")) {
> - if (!driver->privileged) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("DAC security driver usable only when "
> - "running privileged (as root)"));
> - goto error;
> - }
> -
> - mgr = virSecurityManagerNewDAC(QEMU_DRIVER_NAME,
> - driver->user,
> - driver->group,
> - driver->allowDiskFormatProbing,
> - driver->securityDefaultConfined,
> - driver->securityRequireConfined,
> - driver->dynamicOwnership);
> - hasDAC = true;
> - } else {
> - mgr = virSecurityManagerNew(primary,
> - QEMU_DRIVER_NAME,
> - driver->allowDiskFormatProbing,
> - driver->securityDefaultConfined,
> - driver->securityRequireConfined);
> - }
> -
> - if (!mgr)
> - goto error;
> -
> - /* We need a stack to group the security drivers if:
> - * - additional drivers are provived in configuration
> - * - the primary driver isn't DAC and we are running privileged
> - */
> - if ((driver->privileged && !hasDAC) ||
> - (driver->securityDriverNames && driver->securityDriverNames[1])) {
> - if (!(stack = virSecurityManagerNewStack(mgr)))
> - goto error;
> - mgr = stack;
> - }
> -
> - /* Loop through additional driver names and add them as nested */
> if (driver->securityDriverNames) {
> - names = driver->securityDriverNames + 1;
> + names = driver->securityDriverNames;
> while (names && *names) {
> - if (STREQ("dac", *names)) {
> - /* A DAC driver has specific parameters */
> - if (!driver->privileged) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("DAC security driver usable only when "
> - "running privileged (as root)"));
> - goto error;
> - }
> -
> - nested = virSecurityManagerNewDAC(QEMU_DRIVER_NAME,
> - driver->user,
> - driver->group,
> - driver->allowDiskFormatProbing,
> - driver->securityDefaultConfined,
> - driver->securityRequireConfined,
> - driver->dynamicOwnership);
> + if (STREQ("dac", *names))
> hasDAC = true;
> - } else {
> - nested = virSecurityManagerNew(*names,
> - QEMU_DRIVER_NAME,
> - driver->allowDiskFormatProbing,
> - driver->securityDefaultConfined,
> - driver->securityRequireConfined);
> - }
> -
> - if (!nested)
> - goto error;
>
> - if (virSecurityManagerStackAddNested(stack, nested))
> + if (!(mgr = virSecurityManagerNew(*names,
> + QEMU_DRIVER_NAME,
> + driver->allowDiskFormatProbing,
> + driver->securityDefaultConfined,
> + driver->securityRequireConfined)))
> goto error;
> -
> - nested = NULL;
> + if (!stack) {
> + if (!(stack = virSecurityManagerNewStack(mgr)))
> + goto error;
> + } else {
> + if (virSecurityManagerStackAddNested(stack, mgr) < 0)
> + goto error;
> + }
> + mgr = NULL;
> names++;
> }
> - }
> -
> - if (driver->privileged && !hasDAC) {
> - if (!(nested = virSecurityManagerNewDAC(QEMU_DRIVER_NAME,
> - driver->user,
> - driver->group,
> - driver->allowDiskFormatProbing,
> - driver->securityDefaultConfined,
> - driver->securityRequireConfined,
> - driver->dynamicOwnership)))
> + } else {
> + if (!(mgr = virSecurityManagerNew(NULL,
> + QEMU_DRIVER_NAME,
> + driver->allowDiskFormatProbing,
> + driver->securityDefaultConfined,
> + driver->securityRequireConfined)))
> goto error;
> -
> - if (virSecurityManagerStackAddNested(stack, nested))
> + if (!(stack = virSecurityManagerNewStack(mgr)))
> goto error;
> + mgr = NULL;
> + }
>
> - nested = NULL;
> + if (!hasDAC && driver->privileged) {
> + if (!(mgr = virSecurityManagerNewDAC(QEMU_DRIVER_NAME,
> + driver->user,
> + driver->group,
> + driver->allowDiskFormatProbing,
> + driver->securityDefaultConfined,
> + driver->securityRequireConfined,
> + driver->dynamicOwnership)))
> + goto error;
> + if (!stack) {
> + if (!(stack = virSecurityManagerNewStack(mgr)))
> + goto error;
> + } else {
> + if (virSecurityManagerStackAddNested(stack, mgr) < 0)
> + goto error;
> + }
> + mgr = NULL;
> }
>
> - driver->securityManager = mgr;
> + driver->securityManager = stack;
> return 0;
>
> error:
> VIR_ERROR(_("Failed to initialize security drivers"));
> + virSecurityManagerFree(stack);
> virSecurityManagerFree(mgr);
> - virSecurityManagerFree(nested);
> return -1;
> }
>
> diff --git a/src/security/security_manager.c b/src/security/security_manager.c
> index 0e106d5..367f7ad 100644
> --- a/src/security/security_manager.c
> +++ b/src/security/security_manager.c
> @@ -49,6 +49,12 @@ static virSecurityManagerPtr virSecurityManagerNewDriver(virSecurityDriverPtr dr
> {
> virSecurityManagerPtr mgr;
>
> + VIR_DEBUG("drv=%p (%s) virtDriver=%s allowDiskFormatProbing=%d "
> + "defaultConfined=%d requireConfined=%d",
> + drv, drv->name, virtDriver,
> + allowDiskFormatProbing, defaultConfined,
> + requireConfined);
> +
> if (VIR_ALLOC_VAR(mgr, char, drv->privateDataLen) < 0) {
> virReportOOMError();
> return NULL;
> @@ -80,7 +86,7 @@ virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary)
> if (!mgr)
> return NULL;
>
> - virSecurityStackAddPrimary(mgr, primary);
> + virSecurityStackAddNested(mgr, primary);
>
> return mgr;
> }
> diff --git a/src/security/security_stack.c b/src/security/security_stack.c
> index 7dcd626..0eb7e76 100644
> --- a/src/security/security_stack.c
> +++ b/src/security/security_stack.c
> @@ -38,35 +38,31 @@ struct _virSecurityStackItem {
> };
>
> struct _virSecurityStackData {
> - virSecurityManagerPtr primary;
> virSecurityStackItemPtr itemsHead;
> };
>
> int
> -virSecurityStackAddPrimary(virSecurityManagerPtr mgr,
> - virSecurityManagerPtr primary)
> -{
> - virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr);
> - if (virSecurityStackAddNested(mgr, primary) < 0)
> - return -1;
> - priv->primary = primary;
> - return 0;
> -}
> -
> -int
> virSecurityStackAddNested(virSecurityManagerPtr mgr,
> virSecurityManagerPtr nested)
> {
> virSecurityStackItemPtr item = NULL;
> virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr);
> + virSecurityStackItemPtr tmp;
> +
> + tmp = priv->itemsHead;
> + while (tmp && tmp->next)
> + tmp = tmp->next;
>
> if (VIR_ALLOC(item) < 0) {
> virReportOOMError();
> return -1;
> }
> item->securityManager = nested;
> - item->next = priv->itemsHead;
> - priv->itemsHead = item;
> + if (tmp)
> + tmp->next = item;
> + else
> + priv->itemsHead = item;
> +
> return 0;
> }
>
> @@ -74,19 +70,7 @@ virSecurityManagerPtr
> virSecurityStackGetPrimary(virSecurityManagerPtr mgr)
> {
> virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr);
> - return (priv->primary) ? priv->primary : priv->itemsHead->securityManager;
> -}
> -
> -void virSecurityStackSetPrimary(virSecurityManagerPtr mgr,
> - virSecurityManagerPtr primary)
> -{
> - virSecurityStackAddPrimary(mgr, primary);
> -}
> -
> -void virSecurityStackSetSecondary(virSecurityManagerPtr mgr,
> - virSecurityManagerPtr secondary)
> -{
> - virSecurityStackAddNested(mgr, secondary);
> + return priv->itemsHead->securityManager;
> }
>
> static virSecurityDriverStatus
> diff --git a/src/security/security_stack.h b/src/security/security_stack.h
> index 6898c03..5bb3be7 100644
> --- a/src/security/security_stack.h
> +++ b/src/security/security_stack.h
> @@ -27,19 +27,11 @@ extern virSecurityDriver virSecurityDriverStack;
>
>
> int
> -virSecurityStackAddPrimary(virSecurityManagerPtr mgr,
> - virSecurityManagerPtr primary);
> -int
> virSecurityStackAddNested(virSecurityManagerPtr mgr,
> virSecurityManagerPtr nested);
> virSecurityManagerPtr
> virSecurityStackGetPrimary(virSecurityManagerPtr mgr);
>
> -void virSecurityStackSetPrimary(virSecurityManagerPtr mgr,
> - virSecurityManagerPtr primary);
> -void virSecurityStackSetSecondary(virSecurityManagerPtr mgr,
> - virSecurityManagerPtr secondary);
> -
> virSecurityManagerPtr*
> virSecurityStackGetNested(virSecurityManagerPtr mgr);
ACK, tested, works for me, so pushed !
thanks !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list