So imagine you want to crate new security manager: if (!(mgr = virSecurityManagerNew("selinux", "QEMU", false, true, false, true))); Hard to parse, right? What about this: if (!(mgr = virSecurityManagerNew("selinux", "QEMU", VIR_SECURITY_MANAGER_DEFAULT_CONFINED | VIR_SECURITY_MANAGER_PRIVILEGED))); Now that's better! This is what the commit does. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/lxc/lxc_controller.c | 3 +- src/lxc/lxc_driver.c | 14 +++++--- src/qemu/qemu_driver.c | 28 ++++++++-------- src/security/security_manager.c | 70 +++++++++++++--------------------------- src/security/security_manager.h | 25 ++++++++------ tests/qemuhotplugtest.c | 3 +- tests/seclabeltest.c | 2 +- tests/securityselinuxlabeltest.c | 4 ++- tests/securityselinuxtest.c | 4 ++- 9 files changed, 72 insertions(+), 81 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index b064919..3e5d2b4 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -2645,8 +2645,7 @@ int main(int argc, char *argv[]) ctrl->handshakeFd = handshakeFd; if (!(ctrl->securityManager = virSecurityManagerNew(securityDriver, - LXC_DRIVER_NAME, - false, false, false, false))) + LXC_DRIVER_NAME, 0))) goto cleanup; if (ctrl->def->seclabels) { diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index b408be0..1a9550e 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1556,13 +1556,17 @@ static int lxcCheckNetNsSupport(void) static virSecurityManagerPtr lxcSecurityInit(virLXCDriverConfigPtr cfg) { + unsigned int flags = VIR_SECURITY_MANAGER_PRIVILEGED; + VIR_INFO("lxcSecurityInit %s", cfg->securityDriverName); + + if (cfg->securityDefaultConfined) + flags |= VIR_SECURITY_MANAGER_DEFAULT_CONFINED; + if (cfg->securityRequireConfined) + flags |= VIR_SECURITY_MANAGER_REQUIRE_CONFINED; + virSecurityManagerPtr mgr = virSecurityManagerNew(cfg->securityDriverName, - LXC_DRIVER_NAME, - false, - cfg->securityDefaultConfined, - cfg->securityRequireConfined, - true); + LXC_DRIVER_NAME, flags); if (!mgr) goto error; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index aff1915..598d159 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -389,6 +389,16 @@ qemuSecurityInit(virQEMUDriverPtr driver) virSecurityManagerPtr mgr = NULL; virSecurityManagerPtr stack = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + unsigned int flags = 0; + + if (cfg->allowDiskFormatProbing) + flags |= VIR_SECURITY_MANAGER_ALLOW_DISK_PROBE; + if (cfg->securityDefaultConfined) + flags |= VIR_SECURITY_MANAGER_DEFAULT_CONFINED; + if (cfg->securityRequireConfined) + flags |= VIR_SECURITY_MANAGER_REQUIRE_CONFINED; + if (virQEMUDriverIsPrivileged(driver)) + flags |= VIR_SECURITY_MANAGER_PRIVILEGED; if (cfg->securityDriverNames && cfg->securityDriverNames[0]) { @@ -396,10 +406,7 @@ qemuSecurityInit(virQEMUDriverPtr driver) while (names && *names) { if (!(mgr = virSecurityManagerNew(*names, QEMU_DRIVER_NAME, - cfg->allowDiskFormatProbing, - cfg->securityDefaultConfined, - cfg->securityRequireConfined, - virQEMUDriverIsPrivileged(driver)))) + flags))) goto error; if (!stack) { if (!(stack = virSecurityManagerNewStack(mgr))) @@ -414,10 +421,7 @@ qemuSecurityInit(virQEMUDriverPtr driver) } else { if (!(mgr = virSecurityManagerNew(NULL, QEMU_DRIVER_NAME, - cfg->allowDiskFormatProbing, - cfg->securityDefaultConfined, - cfg->securityRequireConfined, - virQEMUDriverIsPrivileged(driver)))) + flags))) goto error; if (!(stack = virSecurityManagerNewStack(mgr))) goto error; @@ -425,14 +429,12 @@ qemuSecurityInit(virQEMUDriverPtr driver) } if (virQEMUDriverIsPrivileged(driver)) { + if (cfg->dynamicOwnership) + flags |= VIR_SECURITY_MANAGER_DYNAMIC_OWNERSHIP; if (!(mgr = virSecurityManagerNewDAC(QEMU_DRIVER_NAME, cfg->user, cfg->group, - cfg->allowDiskFormatProbing, - cfg->securityDefaultConfined, - cfg->securityRequireConfined, - cfg->dynamicOwnership, - virQEMUDriverIsPrivileged(driver), + flags, qemuSecurityChownCallback))) goto error; if (!stack) { diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 28d7dfd..46b1a25 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -37,10 +37,7 @@ struct _virSecurityManager { virObjectLockable parent; virSecurityDriverPtr drv; - bool allowDiskFormatProbing; - bool defaultConfined; - bool requireConfined; - bool privileged; + unsigned int flags; const char *virtDriver; void *privateData; }; @@ -77,10 +74,7 @@ VIR_ONCE_GLOBAL_INIT(virSecurityManager); static virSecurityManagerPtr virSecurityManagerNewDriver(virSecurityDriverPtr drv, const char *virtDriver, - bool allowDiskFormatProbing, - bool defaultConfined, - bool requireConfined, - bool privileged) + unsigned int flags) { virSecurityManagerPtr mgr; char *privateData; @@ -88,11 +82,10 @@ virSecurityManagerNewDriver(virSecurityDriverPtr drv, if (virSecurityManagerInitialize() < 0) return NULL; - VIR_DEBUG("drv=%p (%s) virtDriver=%s allowDiskFormatProbing=%d " - "defaultConfined=%d requireConfined=%d privileged=%d", - drv, drv->name, virtDriver, - allowDiskFormatProbing, defaultConfined, - requireConfined, privileged); + VIR_DEBUG("drv=%p (%s) virtDriver=%s flags=%x", + drv, drv->name, virtDriver, flags); + + virCheckFlags(VIR_SECURITY_MANAGER_NEW_MASK, NULL); if (VIR_ALLOC_N(privateData, drv->privateDataLen) < 0) return NULL; @@ -103,10 +96,7 @@ virSecurityManagerNewDriver(virSecurityDriverPtr drv, } mgr->drv = drv; - mgr->allowDiskFormatProbing = allowDiskFormatProbing; - mgr->defaultConfined = defaultConfined; - mgr->requireConfined = requireConfined; - mgr->privileged = privileged; + mgr->flags = flags; mgr->virtDriver = virtDriver; mgr->privateData = privateData; @@ -125,10 +115,7 @@ virSecurityManagerNewStack(virSecurityManagerPtr primary) virSecurityManagerPtr mgr = virSecurityManagerNewDriver(&virSecurityDriverStack, virSecurityManagerGetDriver(primary), - virSecurityManagerGetAllowDiskFormatProbing(primary), - virSecurityManagerGetDefaultConfined(primary), - virSecurityManagerGetRequireConfined(primary), - virSecurityManagerGetPrivileged(primary)); + primary->flags); if (!mgr) return NULL; @@ -153,20 +140,13 @@ virSecurityManagerPtr virSecurityManagerNewDAC(const char *virtDriver, uid_t user, gid_t group, - bool allowDiskFormatProbing, - bool defaultConfined, - bool requireConfined, - bool dynamicOwnership, - bool privileged, + unsigned int flags, virSecurityManagerDACChownCallback chownCallback) { virSecurityManagerPtr mgr = virSecurityManagerNewDriver(&virSecurityDriverDAC, virtDriver, - allowDiskFormatProbing, - defaultConfined, - requireConfined, - privileged); + flags & VIR_SECURITY_MANAGER_NEW_MASK); if (!mgr) return NULL; @@ -176,7 +156,7 @@ virSecurityManagerNewDAC(const char *virtDriver, return NULL; } - virSecurityDACSetDynamicOwnership(mgr, dynamicOwnership); + virSecurityDACSetDynamicOwnership(mgr, flags & VIR_SECURITY_MANAGER_DYNAMIC_OWNERSHIP); virSecurityDACSetChownCallback(mgr, chownCallback); return mgr; @@ -186,10 +166,7 @@ virSecurityManagerNewDAC(const char *virtDriver, virSecurityManagerPtr virSecurityManagerNew(const char *name, const char *virtDriver, - bool allowDiskFormatProbing, - bool defaultConfined, - bool requireConfined, - bool privileged) + unsigned int flags) { virSecurityDriverPtr drv = virSecurityDriverLookup(name, virtDriver); if (!drv) @@ -197,13 +174,13 @@ virSecurityManagerNew(const char *name, /* driver "none" needs some special handling of *Confined bools */ if (STREQ(drv->name, "none")) { - if (requireConfined) { + if (flags & VIR_SECURITY_MANAGER_REQUIRE_CONFINED) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Security driver \"none\" cannot create confined guests")); return NULL; } - if (defaultConfined) { + if (flags & VIR_SECURITY_MANAGER_DEFAULT_CONFINED) { if (name != NULL) { VIR_WARN("Configured security driver \"none\" disables default" " policy to create confined guests"); @@ -211,16 +188,13 @@ virSecurityManagerNew(const char *name, VIR_DEBUG("Auto-probed security driver is \"none\";" " confined guests will not be created"); } - defaultConfined = false; + flags &= ~VIR_SECURITY_MANAGER_DEFAULT_CONFINED; } } return virSecurityManagerNewDriver(drv, virtDriver, - allowDiskFormatProbing, - defaultConfined, - requireConfined, - privileged); + flags); } @@ -323,28 +297,28 @@ virSecurityManagerGetBaseLabel(virSecurityManagerPtr mgr, bool virSecurityManagerGetAllowDiskFormatProbing(virSecurityManagerPtr mgr) { - return mgr->allowDiskFormatProbing; + return mgr->flags & VIR_SECURITY_MANAGER_ALLOW_DISK_PROBE; } bool virSecurityManagerGetDefaultConfined(virSecurityManagerPtr mgr) { - return mgr->defaultConfined; + return mgr->flags & VIR_SECURITY_MANAGER_DEFAULT_CONFINED; } bool virSecurityManagerGetRequireConfined(virSecurityManagerPtr mgr) { - return mgr->requireConfined; + return mgr->flags & VIR_SECURITY_MANAGER_REQUIRE_CONFINED; } bool virSecurityManagerGetPrivileged(virSecurityManagerPtr mgr) { - return mgr->privileged; + return mgr->flags & VIR_SECURITY_MANAGER_PRIVILEGED; } @@ -611,7 +585,7 @@ virSecurityManagerGenLabel(virSecurityManagerPtr mgr, } if (seclabel->type == VIR_DOMAIN_SECLABEL_DEFAULT) { - if (sec_managers[i]->defaultConfined) { + if (virSecurityManagerGetDefaultConfined(sec_managers[i])) { seclabel->type = VIR_DOMAIN_SECLABEL_DYNAMIC; } else { seclabel->type = VIR_DOMAIN_SECLABEL_NONE; @@ -620,7 +594,7 @@ virSecurityManagerGenLabel(virSecurityManagerPtr mgr, } if (seclabel->type == VIR_DOMAIN_SECLABEL_NONE) { - if (sec_managers[i]->requireConfined) { + if (virSecurityManagerGetRequireConfined(sec_managers[i])) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Unconfined guests are not allowed on this host")); goto cleanup; diff --git a/src/security/security_manager.h b/src/security/security_manager.h index 53e56f6..e534e31 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -30,12 +30,23 @@ typedef struct _virSecurityManager virSecurityManager; typedef virSecurityManager *virSecurityManagerPtr; +typedef enum { + VIR_SECURITY_MANAGER_ALLOW_DISK_PROBE = 1 << 0, + VIR_SECURITY_MANAGER_DEFAULT_CONFINED = 1 << 1, + VIR_SECURITY_MANAGER_REQUIRE_CONFINED = 1 << 2, + VIR_SECURITY_MANAGER_PRIVILEGED = 1 << 3, + VIR_SECURITY_MANAGER_DYNAMIC_OWNERSHIP = 1 << 4, +} virSecurityManagerNewFlags; + +# define VIR_SECURITY_MANAGER_NEW_MASK \ + (VIR_SECURITY_MANAGER_ALLOW_DISK_PROBE | \ + VIR_SECURITY_MANAGER_DEFAULT_CONFINED | \ + VIR_SECURITY_MANAGER_REQUIRE_CONFINED | \ + VIR_SECURITY_MANAGER_PRIVILEGED) + virSecurityManagerPtr virSecurityManagerNew(const char *name, const char *virtDriver, - bool allowDiskFormatProbing, - bool defaultConfined, - bool requireConfined, - bool privileged); + unsigned int flags); virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary); int virSecurityManagerStackAddNested(virSecurityManagerPtr stack, @@ -59,11 +70,7 @@ typedef int virSecurityManagerPtr virSecurityManagerNewDAC(const char *virtDriver, uid_t user, gid_t group, - bool allowDiskFormatProbing, - bool defaultConfined, - bool requireConfined, - bool dynamicOwnership, - bool privileged, + unsigned int flags, virSecurityManagerDACChownCallback chownCallback); int virSecurityManagerPreFork(virSecurityManagerPtr mgr); diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 2135b5a..2c5c48f 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -364,7 +364,8 @@ mymain(void) if (!driver.lockManager) return EXIT_FAILURE; - if (!(mgr = virSecurityManagerNew("none", "qemu", false, false, false, true))) + if (!(mgr = virSecurityManagerNew("none", "qemu", + VIR_SECURITY_MANAGER_PRIVILEGED))) return EXIT_FAILURE; if (!(driver.securityManager = virSecurityManagerNewStack(mgr))) return EXIT_FAILURE; diff --git a/tests/seclabeltest.c b/tests/seclabeltest.c index 93ddcbb..6b1e789 100644 --- a/tests/seclabeltest.c +++ b/tests/seclabeltest.c @@ -17,7 +17,7 @@ main(int argc ATTRIBUTE_UNUSED, char **argv ATTRIBUTE_UNUSED) if (virThreadInitialize() < 0) return EXIT_FAILURE; - mgr = virSecurityManagerNew(NULL, "QEMU", false, true, false, false); + mgr = virSecurityManagerNew(NULL, "QEMU", VIR_SECURITY_MANAGER_DEFAULT_CONFINED); if (mgr == NULL) { fprintf(stderr, "Failed to start security driver"); return EXIT_FAILURE; diff --git a/tests/securityselinuxlabeltest.c b/tests/securityselinuxlabeltest.c index 4b8fa6f..86660f4 100644 --- a/tests/securityselinuxlabeltest.c +++ b/tests/securityselinuxlabeltest.c @@ -351,7 +351,9 @@ mymain(void) if (!rc) return EXIT_AM_SKIP; - if (!(mgr = virSecurityManagerNew("selinux", "QEMU", false, true, false, true))) { + if (!(mgr = virSecurityManagerNew("selinux", "QEMU", + VIR_SECURITY_MANAGER_DEFAULT_CONFINED | + VIR_SECURITY_MANAGER_PRIVILEGED))) { virErrorPtr err = virGetLastError(); VIR_TEST_VERBOSE("Unable to initialize security driver: %s\n", err->message); diff --git a/tests/securityselinuxtest.c b/tests/securityselinuxtest.c index 3a7862f..49694f3 100644 --- a/tests/securityselinuxtest.c +++ b/tests/securityselinuxtest.c @@ -272,7 +272,9 @@ mymain(void) int ret = 0; virSecurityManagerPtr mgr; - if (!(mgr = virSecurityManagerNew("selinux", "QEMU", false, true, false, true))) { + if (!(mgr = virSecurityManagerNew("selinux", "QEMU", + VIR_SECURITY_MANAGER_DEFAULT_CONFINED | + VIR_SECURITY_MANAGER_PRIVILEGED))) { virErrorPtr err = virGetLastError(); fprintf(stderr, "Unable to initialize security driver: %s\n", err->message); -- 2.4.9