cfg.mk | 5 ++++ src/qemu/qemu_command.c | 7 +++--- src/qemu/qemu_conf.c | 9 ++++--- src/qemu/qemu_domain.c | 17 ++++++------- src/qemu/qemu_driver.c | 63 ++++++++++++++++++++++------------------------- src/qemu/qemu_hotplug.c | 4 +-- src/qemu/qemu_migration.c | 13 +++++----- src/qemu/qemu_process.c | 61 ++++++++++++++++++++++----------------------- src/qemu/qemu_security.h | 32 ++++++++++++++++++++++++ 9 files changed, 122 insertions(+), 89 deletions(-)
Now that we have some qemuSecurity wrappers over
virSecurityManager APIs, lets make sure everybody sticks with
them. We have them for a reason and calling virSecurityManager
API directly instead of wrapper may lead into accidentally
labelling a file on the host instead of namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
This is an alternative approach to:
https://www.redhat.com/archives/libvir-list/2017-February/msg00271.html
cfg.mk | 5 ++++
src/qemu/qemu_command.c | 7 +++---
src/qemu/qemu_conf.c | 9 ++++---
src/qemu/qemu_domain.c | 17 ++++++-------
src/qemu/qemu_driver.c | 63 ++++++++++++++++++++++-------------------------
src/qemu/qemu_hotplug.c | 4 +--
src/qemu/qemu_migration.c | 13 +++++-----
src/qemu/qemu_process.c | 61 ++++++++++++++++++++++-----------------------
src/qemu/qemu_security.h | 32 ++++++++++++++++++++++++
9 files changed, 122 insertions(+), 89 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index 69e3f3a1a..489fda8ea 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -983,6 +983,11 @@ sc_prohibit_sysconf_pagesize:
halt='use virGetSystemPageSize[KB] instead of sysconf(_SC_PAGESIZE)' \
$(_sc_search_regexp)
+sc_prohibit_virSecurity:
+ @grep -P 'virSecurityManager(?!Ptr)' $$($(VC_LIST_EXCEPT) | grep '^src/qemu/' | \
+ grep -v '^src/qemu/qemu_security') && \
+ { echo '$(ME): prefer qemuSecurity wrappers' 1>&2; exit 1; } || :
+
sc_prohibit_pthread_create:
@prohibit='\bpthread_create\b' \
exclude='sc_prohibit_pthread_create' \
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c00a47a91..110540ba7 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -28,6 +28,7 @@
#include "qemu_capabilities.h"
#include "qemu_interface.h"
#include "qemu_alias.h"
+#include "qemu_security.h"
#include "cpu/cpu.h"
#include "dirname.h"
#include "viralloc.h"
@@ -8321,8 +8322,8 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
}
for (i = 0; i < tapfdSize; i++) {
- if (virSecurityManagerSetTapFDLabel(driver->securityManager,
- def, tapfd[i]) < 0)
+ if (qemuSecuritySetTapFDLabel(driver->securityManager,
+ def, tapfd[i]) < 0)
goto cleanup;
virCommandPassFD(cmd, tapfd[i],
VIR_COMMAND_PASS_FD_CLOSE_PARENT);
@@ -8403,7 +8404,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
/* NOTE: Not using const virDomainDef here since eventually a call is made
- * into virSecurityManagerSetTapFDLabel which calls it's driver
+ * into qemuSecuritySetTapFDLabel which calls it's driver
* API domainSetSecurityTapFDLabel that doesn't use the const format.
*/
static int
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 0223a95d2..4fc0dee39 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -38,6 +38,7 @@
#include "qemu_conf.h"
#include "qemu_capabilities.h"
#include "qemu_domain.h"
+#include "qemu_security.h"
#include "viruuid.h"
#include "virbuffer.h"
#include "virconf.h"
@@ -904,7 +905,7 @@ virCapsPtr virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver)
}
/* access sec drivers and create a sec model for each one */
- if (!(sec_managers = virSecurityManagerGetNested(driver->securityManager)))
+ if (!(sec_managers = qemuSecurityGetNested(driver->securityManager)))
goto error;
/* calculate length */
@@ -917,14 +918,14 @@ virCapsPtr virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver)
for (i = 0; sec_managers[i]; i++) {
virCapsHostSecModelPtr sm = &caps->host.secModels[i];
- doi = virSecurityManagerGetDOI(sec_managers[i]);
- model = virSecurityManagerGetModel(sec_managers[i]);
+ doi = qemuSecurityGetDOI(sec_managers[i]);
+ model = qemuSecurityGetModel(sec_managers[i]);
if (VIR_STRDUP(sm->model, model) < 0 ||
VIR_STRDUP(sm->doi, doi) < 0)
goto error;
for (j = 0; j < ARRAY_CARDINALITY(virtTypes); j++) {
- lbl = virSecurityManagerGetBaseLabel(sec_managers[i], virtTypes[j]);
+ lbl = qemuSecurityGetBaseLabel(sec_managers[i], virtTypes[j]);
type = virDomainVirtTypeToString(virtTypes[j]);
if (lbl &&
virCapabilitiesHostSecModelAddBaseLabel(sm, type, lbl) < 0)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f62bf8f1d..2c827ea2c 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -588,8 +588,8 @@ qemuDomainWriteMasterKeyFile(virQEMUDriverPtr driver,
goto cleanup;
}
- if (virSecurityManagerDomainSetPathLabel(driver->securityManager,
- vm->def, path) < 0)
+ if (qemuSecurityDomainSetPathLabel(driver->securityManager,
+ vm->def, path) < 0)
goto cleanup;
ret = 0;
@@ -2688,7 +2688,7 @@ qemuDomainDefPostParse(virDomainDefPtr def,
if (qemuDomainRecheckInternalPaths(def, cfg, parseFlags) < 0)
goto cleanup;
- if (virSecurityManagerVerify(driver->securityManager, def) < 0)
+ if (qemuSecurityVerify(driver->securityManager, def) < 0)
goto cleanup;
if (qemuDomainDefVcpusPostParse(def) < 0)
@@ -7257,8 +7257,7 @@ qemuDomainSetupDev(virQEMUDriverPtr driver,
VIR_DEBUG("Setting up /dev/ for domain %s", vm->def->name);
- mount_options = virSecurityManagerGetMountOptions(driver->securityManager,
- vm->def);
+ mount_options = qemuSecurityGetMountOptions(driver->securityManager, vm->def);
if (!mount_options &&
VIR_STRDUP(mount_options, "") < 0)
@@ -7679,7 +7678,7 @@ qemuDomainAttachDeviceMknodHelper(pid_t pid ATTRIBUTE_UNUSED,
bool delDevice = false;
bool isLink = S_ISLNK(data->sb.st_mode);
- virSecurityManagerPostFork(data->driver->securityManager);
+ qemuSecurityPostFork(data->driver->securityManager);
if (virFileMakeParentPath(data->file) < 0) {
virReportSystemError(errno,
@@ -7841,16 +7840,16 @@ qemuDomainAttachDeviceMknodRecursive(virQEMUDriverPtr driver,
#endif
if (STRPREFIX(file, DEVPREFIX)) {
- if (virSecurityManagerPreFork(driver->securityManager) < 0)
+ if (qemuSecurityPreFork(driver->securityManager) < 0)
goto cleanup;
if (virProcessRunInMountNamespace(vm->pid,
qemuDomainAttachDeviceMknodHelper,
&data) < 0) {
- virSecurityManagerPostFork(driver->securityManager);
+ qemuSecurityPostFork(driver->securityManager);
goto cleanup;
}
- virSecurityManagerPostFork(driver->securityManager);
+ qemuSecurityPostFork(driver->securityManager);
}
if (isLink &&
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 89bc833de..096fe36fe 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -405,26 +405,26 @@ qemuSecurityInit(virQEMUDriverPtr driver)
cfg->securityDriverNames[0]) {
names = cfg->securityDriverNames;
while (names && *names) {
- if (!(mgr = virSecurityManagerNew(*names,
- QEMU_DRIVER_NAME,
- flags)))
+ if (!(mgr = qemuSecurityNew(*names,
+ QEMU_DRIVER_NAME,
+ flags)))
goto error;
if (!stack) {
- if (!(stack = virSecurityManagerNewStack(mgr)))
+ if (!(stack = qemuSecurityNewStack(mgr)))
goto error;
} else {
- if (virSecurityManagerStackAddNested(stack, mgr) < 0)
+ if (qemuSecurityStackAddNested(stack, mgr) < 0)
goto error;
}
mgr = NULL;
names++;
}
} else {
- if (!(mgr = virSecurityManagerNew(NULL,
- QEMU_DRIVER_NAME,
- flags)))
+ if (!(mgr = qemuSecurityNew(NULL,
+ QEMU_DRIVER_NAME,
+ flags)))
goto error;
- if (!(stack = virSecurityManagerNewStack(mgr)))
+ if (!(stack = qemuSecurityNewStack(mgr)))
goto error;
mgr = NULL;
}
@@ -432,17 +432,17 @@ 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,
- flags,
- qemuSecurityChownCallback)))
+ if (!(mgr = qemuSecurityNewDAC(QEMU_DRIVER_NAME,
+ cfg->user,
+ cfg->group,
+ flags,
+ qemuSecurityChownCallback)))
goto error;
if (!stack) {
- if (!(stack = virSecurityManagerNewStack(mgr)))
+ if (!(stack = qemuSecurityNewStack(mgr)))
goto error;
} else {
- if (virSecurityManagerStackAddNested(stack, mgr) < 0)
+ if (qemuSecurityStackAddNested(stack, mgr) < 0)
goto error;
}
mgr = NULL;
@@ -3088,7 +3088,7 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver,
if (fd < 0)
goto cleanup;
- if (virSecurityManagerSetImageFDLabel(driver->securityManager, vm->def, fd) < 0)
+ if (qemuSecuritySetImageFDLabel(driver->securityManager, vm->def, fd) < 0)
goto cleanup;
if (!(wrapperFd = virFileWrapperFdNew(&fd, path, wrapperFlags)))
@@ -3553,8 +3553,7 @@ static int qemuDumpToFd(virQEMUDriverPtr driver, virDomainObjPtr vm,
return -1;
}
- if (virSecurityManagerSetImageFDLabel(driver->securityManager, vm->def,
- fd) < 0)
+ if (qemuSecuritySetImageFDLabel(driver->securityManager, vm->def, fd) < 0)
return -1;
VIR_FREE(priv->job.current);
@@ -3846,7 +3845,7 @@ qemuDomainScreenshot(virDomainPtr dom,
}
unlink_tmp = true;
- virSecurityManagerSetSavedStateLabel(driver->securityManager, vm->def, tmp);
+ qemuSecuritySetSavedStateLabel(driver->securityManager, vm->def, tmp);
qemuDomainObjEnterMonitor(driver, vm);
if (qemuMonitorScreendump(priv->mon, tmp) < 0) {
@@ -5928,8 +5927,8 @@ static int qemuDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr secl
* QEMU monitor hasn't seen SIGHUP/ERR on poll().
*/
if (virDomainObjIsActive(vm)) {
- if (virSecurityManagerGetProcessLabel(driver->securityManager,
- vm->def, vm->pid, seclabel) < 0) {
+ if (qemuSecurityGetProcessLabel(driver->securityManager,
+ vm->def, vm->pid, seclabel) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("Failed to get security label"));
goto cleanup;
@@ -5973,8 +5972,7 @@ static int qemuDomainGetSecurityLabelList(virDomainPtr dom,
ret = 0;
} else {
int len = 0;
- virSecurityManagerPtr* mgrs = virSecurityManagerGetNested(
- driver->securityManager);
+ virSecurityManagerPtr* mgrs = qemuSecurityGetNested(driver->securityManager);
if (!mgrs)
goto cleanup;
@@ -5990,8 +5988,8 @@ static int qemuDomainGetSecurityLabelList(virDomainPtr dom,
/* Fill the array */
for (i = 0; i < len; i++) {
- if (virSecurityManagerGetProcessLabel(mgrs[i], vm->def, vm->pid,
- &(*seclabels)[i]) < 0) {
+ if (qemuSecurityGetProcessLabel(mgrs[i], vm->def, vm->pid,
+ &(*seclabels)[i]) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("Failed to get security label"));
VIR_FREE(mgrs);
@@ -6369,8 +6367,8 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
cleanup:
virCommandFree(cmd);
VIR_FREE(errbuf);
- if (virSecurityManagerRestoreSavedStateLabel(driver->securityManager,
- vm->def, path) < 0)
+ if (qemuSecurityRestoreSavedStateLabel(driver->securityManager,
+ vm->def, path) < 0)
VIR_WARN("failed to restore save state label on %s", path);
virObjectUnref(cfg);
return ret;
@@ -11196,7 +11194,7 @@ qemuDomainMemoryPeek(virDomainPtr dom,
goto endjob;
}
- virSecurityManagerSetSavedStateLabel(driver->securityManager, vm->def, tmp);
+ qemuSecuritySetSavedStateLabel(driver->securityManager, vm->def, tmp);
priv = vm->privateData;
qemuDomainObjEnterMonitor(driver, vm);
@@ -17064,8 +17062,7 @@ qemuDomainOpenGraphics(virDomainPtr dom,
goto endjob;
}
- if (virSecurityManagerSetImageFDLabel(driver->securityManager, vm->def,
- fd) < 0)
+ if (qemuSecuritySetImageFDLabel(driver->securityManager, vm->def, fd) < 0)
goto endjob;
qemuDomainObjEnterMonitor(driver, vm);
@@ -17129,13 +17126,13 @@ qemuDomainOpenGraphicsFD(virDomainPtr dom,
goto cleanup;
}
- if (virSecurityManagerSetSocketLabel(driver->securityManager, vm->def) < 0)
+ if (qemuSecuritySetSocketLabel(driver->securityManager, vm->def) < 0)
goto cleanup;
if (socketpair(PF_UNIX, SOCK_STREAM, 0, pair) < 0)
goto cleanup;
- if (virSecurityManagerClearSocketLabel(driver->securityManager, vm->def) < 0)
+ if (qemuSecurityClearSocketLabel(driver->securityManager, vm->def) < 0)
goto cleanup;
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 2f209f12b..b99b0e9fb 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1134,8 +1134,8 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
}
for (i = 0; i < tapfdSize; i++) {
- if (virSecurityManagerSetTapFDLabel(driver->securityManager,
- vm->def, tapfd[i]) < 0)
+ if (qemuSecuritySetTapFDLabel(driver->securityManager,
+ vm->def, tapfd[i]) < 0)
goto cleanup;
}
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 0f4a6cf21..c40cb1391 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -40,6 +40,7 @@
#include "qemu_cgroup.h"
#include "qemu_hotplug.h"
#include "qemu_blockjob.h"
+#include "qemu_security.h"
#include "domain_audit.h"
#include "virlog.h"
@@ -4597,7 +4598,7 @@ qemuMigrationConnect(virQEMUDriverPtr driver,
spec->destType = MIGRATION_DEST_FD;
spec->dest.fd.qemu = -1;
- if (virSecurityManagerSetSocketLabel(driver->securityManager, vm->def) < 0)
+ if (qemuSecuritySetSocketLabel(driver->securityManager, vm->def) < 0)
goto cleanup;
if (virNetSocketNewConnectTCP(host, port,
AF_UNSPEC,
@@ -4605,7 +4606,7 @@ qemuMigrationConnect(virQEMUDriverPtr driver,
spec->dest.fd.qemu = virNetSocketDupFD(sock, true);
virObjectUnref(sock);
}
- if (virSecurityManagerClearSocketLabel(driver->securityManager, vm->def) < 0 ||
+ if (qemuSecurityClearSocketLabel(driver->securityManager, vm->def) < 0 ||
spec->dest.fd.qemu == -1)
goto cleanup;
@@ -5076,8 +5077,8 @@ static int doTunnelMigrate(virQEMUDriverPtr driver,
spec.dest.fd.local = fds[0];
}
if (spec.dest.fd.qemu == -1 ||
- virSecurityManagerSetImageFDLabel(driver->securityManager, vm->def,
- spec.dest.fd.qemu) < 0) {
+ qemuSecuritySetImageFDLabel(driver->securityManager, vm->def,
+ spec.dest.fd.qemu) < 0) {
virReportSystemError(errno, "%s",
_("cannot create pipe for tunnelled migration"));
goto cleanup;
@@ -6463,8 +6464,8 @@ qemuMigrationToFile(virQEMUDriverPtr driver, virDomainObjPtr vm,
* doesn't have to open() the file, so while we still have to
* grant SELinux access, we can do it on fd and avoid cleanup
* later, as well as skip futzing with cgroup. */
- if (virSecurityManagerSetImageFDLabel(driver->securityManager, vm->def,
- compressor ? pipeFD[1] : fd) < 0)
+ if (qemuSecuritySetImageFDLabel(driver->securityManager, vm->def,
+ compressor ? pipeFD[1] : fd) < 0)
goto cleanup;
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 92fa69b3c..5c44e565b 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -221,8 +221,7 @@ qemuConnectAgent(virQEMUDriverPtr driver, virDomainObjPtr vm)
return 0;
}
- if (virSecurityManagerSetDaemonSocketLabel(driver->securityManager,
- vm->def) < 0) {
+ if (qemuSecuritySetDaemonSocketLabel(driver->securityManager, vm->def) < 0) {
VIR_ERROR(_("Failed to set security context for agent for %s"),
vm->def->name);
goto cleanup;
@@ -250,8 +249,7 @@ qemuConnectAgent(virQEMUDriverPtr driver, virDomainObjPtr vm)
return -1;
}
- if (virSecurityManagerClearSocketLabel(driver->securityManager,
- vm->def) < 0) {
+ if (qemuSecurityClearSocketLabel(driver->securityManager, vm->def) < 0) {
VIR_ERROR(_("Failed to clear security context for agent for %s"),
vm->def->name);
qemuAgentClose(agent);
@@ -1657,8 +1655,7 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob,
int ret = -1;
qemuMonitorPtr mon = NULL;
- if (virSecurityManagerSetDaemonSocketLabel(driver->securityManager,
- vm->def) < 0) {
+ if (qemuSecuritySetDaemonSocketLabel(driver->securityManager, vm->def) < 0) {
VIR_ERROR(_("Failed to set security context for monitor for %s"),
vm->def->name);
return -1;
@@ -1695,7 +1692,7 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob,
}
priv->mon = mon;
- if (virSecurityManagerClearSocketLabel(driver->securityManager, vm->def) < 0) {
+ if (qemuSecurityClearSocketLabel(driver->securityManager, vm->def) < 0) {
VIR_ERROR(_("Failed to clear security context for monitor for %s"),
vm->def->name);
return -1;
@@ -2638,7 +2635,7 @@ static int qemuProcessHook(void *data)
* protected across fork()
*/
- virSecurityManagerPostFork(h->driver->securityManager);
+ qemuSecurityPostFork(h->driver->securityManager);
/* Some later calls want pid present */
h->vm->pid = getpid();
@@ -2651,7 +2648,7 @@ static int qemuProcessHook(void *data)
* sockets the lock driver opens that we don't want
* labelled. So far we're ok though.
*/
- if (virSecurityManagerSetSocketLabel(h->driver->securityManager, h->vm->def) < 0)
+ if (qemuSecuritySetSocketLabel(h->driver->securityManager, h->vm->def) < 0)
goto cleanup;
if (virDomainLockProcessStart(h->driver->lockManager,
h->cfg->uri,
@@ -2660,7 +2657,7 @@ static int qemuProcessHook(void *data)
true,
&fd) < 0)
goto cleanup;
- if (virSecurityManagerClearSocketLabel(h->driver->securityManager, h->vm->def) < 0)
+ if (qemuSecurityClearSocketLabel(h->driver->securityManager, h->vm->def) < 0)
goto cleanup;
if (qemuDomainBuildNamespace(h->driver, h->vm) < 0)
@@ -3260,8 +3257,8 @@ qemuProcessBuildDestroyHugepagesPath(virQEMUDriverPtr driver,
goto cleanup;
}
- if (virSecurityManagerDomainSetPathLabel(driver->securityManager,
- vm->def, hugepagePath) < 0) {
+ if (qemuSecurityDomainSetPathLabel(driver->securityManager,
+ vm->def, hugepagePath) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("Unable to set huge path in security driver"));
goto cleanup;
@@ -3437,13 +3434,13 @@ qemuProcessReconnect(void *opaque)
/* if domain requests security driver we haven't loaded, report error, but
* do not kill the domain
*/
- ignore_value(virSecurityManagerCheckAllLabel(driver->securityManager,
- obj->def));
+ ignore_value(qemuSecurityCheckAllLabel(driver->securityManager,
+ obj->def));
if (qemuDomainRefreshVcpuInfo(driver, obj, QEMU_ASYNC_JOB_NONE, true) < 0)
goto error;
- if (virSecurityManagerReserveLabel(driver->securityManager, obj->def, obj->pid) < 0)
+ if (qemuSecurityReserveLabel(driver->securityManager, obj->def, obj->pid) < 0)
goto error;
if (qemuProcessNotifyNets(obj->def) < 0)
@@ -4451,8 +4448,8 @@ qemuProcessMakeDir(virQEMUDriverPtr driver,
goto cleanup;
}
- if (virSecurityManagerDomainSetPathLabel(driver->securityManager,
- vm->def, path) < 0)
+ if (qemuSecurityDomainSetPathLabel(driver->securityManager,
+ vm->def, path) < 0)
goto cleanup;
ret = 0;
@@ -4647,7 +4644,7 @@ qemuProcessStartValidate(virQEMUDriverPtr driver,
}
VIR_DEBUG("Checking domain and device security labels");
- if (virSecurityManagerCheckAllLabel(driver->securityManager, vm->def) < 0)
+ if (qemuSecurityCheckAllLabel(driver->securityManager, vm->def) < 0)
return -1;
}
@@ -5202,7 +5199,7 @@ qemuProcessPrepareDomain(virConnectPtr conn,
/* If you are using a SecurityDriver with dynamic labelling,
then generate a security label for isolation */
VIR_DEBUG("Generating domain security label (if required)");
- if (virSecurityManagerGenLabel(driver->securityManager, vm->def) < 0) {
+ if (qemuSecurityGenLabel(driver->securityManager, vm->def) < 0) {
virDomainAuditSecurityLabel(vm, false);
goto cleanup;
}
@@ -5513,8 +5510,8 @@ qemuProcessLaunch(virConnectPtr conn,
virCommandSetUmask(cmd, 0x002);
VIR_DEBUG("Setting up security labelling");
- if (virSecurityManagerSetChildProcessLabel(driver->securityManager,
- vm->def, cmd) < 0)
+ if (qemuSecuritySetChildProcessLabel(driver->securityManager,
+ vm->def, cmd) < 0)
goto cleanup;
virCommandSetOutputFD(cmd, &logfile);
@@ -5524,10 +5521,10 @@ qemuProcessLaunch(virConnectPtr conn,
virCommandDaemonize(cmd);
virCommandRequireHandshake(cmd);
- if (virSecurityManagerPreFork(driver->securityManager) < 0)
+ if (qemuSecurityPreFork(driver->securityManager) < 0)
goto cleanup;
rv = virCommandRun(cmd, NULL);
- virSecurityManagerPostFork(driver->securityManager);
+ qemuSecurityPostFork(driver->securityManager);
/* wait for qemu process to show up */
if (rv == 0) {
@@ -5604,8 +5601,8 @@ qemuProcessLaunch(virConnectPtr conn,
goto cleanup;
}
if (S_ISFIFO(stdin_sb.st_mode) &&
- virSecurityManagerSetImageFDLabel(driver->securityManager,
- vm->def, incoming->fd) < 0)
+ qemuSecuritySetImageFDLabel(driver->securityManager,
+ vm->def, incoming->fd) < 0)
goto cleanup;
}
@@ -6122,7 +6119,7 @@ void qemuProcessStop(virQEMUDriverPtr driver,
qemuSecurityRestoreAllLabel(driver, vm,
!!(flags & VIR_QEMU_PROCESS_STOP_MIGRATED));
- virSecurityManagerReleaseLabel(driver->securityManager, vm->def);
+ qemuSecurityReleaseLabel(driver->securityManager, vm->def);
for (i = 0; i < vm->def->ndisks; i++) {
virDomainDeviceDef dev;
@@ -6366,13 +6363,13 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
vm->pid = pid;
VIR_DEBUG("Detect security driver config");
- sec_managers = virSecurityManagerGetNested(driver->securityManager);
+ sec_managers = qemuSecurityGetNested(driver->securityManager);
if (sec_managers == NULL)
goto error;
for (i = 0; sec_managers[i]; i++) {
seclabelgen = false;
- model = virSecurityManagerGetModel(sec_managers[i]);
+ model = qemuSecurityGetModel(sec_managers[i]);
seclabeldef = virDomainDefGetSecurityLabelDef(vm->def, model);
if (seclabeldef == NULL) {
if (!(seclabeldef = virSecurityLabelDefNew(model)))
@@ -6382,8 +6379,8 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
seclabeldef->type = VIR_DOMAIN_SECLABEL_STATIC;
if (VIR_ALLOC(seclabel) < 0)
goto error;
- if (virSecurityManagerGetProcessLabel(sec_managers[i],
- vm->def, vm->pid, seclabel) < 0)
+ if (qemuSecurityGetProcessLabel(sec_managers[i], vm->def,
+ vm->pid, seclabel) < 0)
goto error;
if (VIR_STRDUP(seclabeldef->model, model) < 0)
@@ -6400,9 +6397,9 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
}
}
- if (virSecurityManagerCheckAllLabel(driver->securityManager, vm->def) < 0)
+ if (qemuSecurityCheckAllLabel(driver->securityManager, vm->def) < 0)
goto error;
- if (virSecurityManagerGenLabel(driver->securityManager, vm->def) < 0)
+ if (qemuSecurityGenLabel(driver->securityManager, vm->def) < 0)
goto error;
if (qemuDomainPerfRestart(vm) < 0)
diff --git a/src/qemu/qemu_security.h b/src/qemu/qemu_security.h
index 54638908d..d86db3f6b 100644
--- a/src/qemu/qemu_security.h
+++ b/src/qemu/qemu_security.h
@@ -28,6 +28,7 @@
# include "qemu_conf.h"
# include "domain_conf.h"
+# include "security/security_manager.h"
int qemuSecuritySetAllLabel(virQEMUDriverPtr driver,
virDomainObjPtr vm,
@@ -60,4 +61,35 @@ int qemuSecuritySetHostdevLabel(virQEMUDriverPtr driver,
int qemuSecurityRestoreHostdevLabel(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainHostdevDefPtr hostdev);
+
+/* Please note that for these APIs there is no wrapper yet. Do NOT blindly add
+ * new APIs here. If an API can touch a /dev file add a proper wrapper instead.
+ */
+# define qemuSecurityCheckAllLabel virSecurityManagerCheckAllLabel
+# define qemuSecurityClearSocketLabel virSecurityManagerClearSocketLabel
+# define qemuSecurityDomainSetPathLabel virSecurityManagerDomainSetPathLabel
+# define qemuSecurityGenLabel virSecurityManagerGenLabel
+# define qemuSecurityGetBaseLabel virSecurityManagerGetBaseLabel
+# define qemuSecurityGetDOI virSecurityManagerGetDOI
+# define qemuSecurityGetModel virSecurityManagerGetModel
+# define qemuSecurityGetMountOptions virSecurityManagerGetMountOptions
+# define qemuSecurityGetNested virSecurityManagerGetNested
+# define qemuSecurityGetProcessLabel virSecurityManagerGetProcessLabel
+# define qemuSecurityNew virSecurityManagerNew
+# define qemuSecurityNewDAC virSecurityManagerNewDAC
+# define qemuSecurityNewStack virSecurityManagerNewStack
+# define qemuSecurityPostFork virSecurityManagerPostFork
+# define qemuSecurityPreFork virSecurityManagerPreFork
+# define qemuSecurityReleaseLabel virSecurityManagerReleaseLabel
+# define qemuSecurityReserveLabel virSecurityManagerReserveLabel
+# define qemuSecurityRestoreSavedStateLabel virSecurityManagerRestoreSavedStateLabel
+# define qemuSecuritySetChildProcessLabel virSecurityManagerSetChildProcessLabel
+# define qemuSecuritySetDaemonSocketLabel virSecurityManagerSetDaemonSocketLabel
+# define qemuSecuritySetImageFDLabel virSecurityManagerSetImageFDLabel
+# define qemuSecuritySetSavedStateLabel virSecurityManagerSetSavedStateLabel
+# define qemuSecuritySetSocketLabel virSecurityManagerSetSocketLabel
+# define qemuSecuritySetTapFDLabel virSecurityManagerSetTapFDLabel
+# define qemuSecurityStackAddNested virSecurityManagerStackAddNested
+# define qemuSecurityVerify virSecurityManagerVerify
+
#endif /* __QEMU_SECURITY_H__ */
--
2.11.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Feb 14, 2017 at 15:30:44 +0100, Michal Privoznik wrote: > Now that we have some qemuSecurity wrappers over > virSecurityManager APIs, lets make sure everybody sticks with > them. We have them for a reason and calling virSecurityManager > API directly instead of wrapper may lead into accidentally > labelling a file on the host instead of namespace. > > Signed-off-by: Michal Privoznik <mprivozn@redhat.com> > --- > > This is an alternative approach to: > > https://www.redhat.com/archives/libvir-list/2017-February/msg00271.html > > cfg.mk | 5 ++++ > src/qemu/qemu_command.c | 7 +++--- > src/qemu/qemu_conf.c | 9 ++++--- > src/qemu/qemu_domain.c | 17 ++++++------- > src/qemu/qemu_driver.c | 63 ++++++++++++++++++++++------------------------- > src/qemu/qemu_hotplug.c | 4 +-- > src/qemu/qemu_migration.c | 13 +++++----- > src/qemu/qemu_process.c | 61 ++++++++++++++++++++++----------------------- > src/qemu/qemu_security.h | 32 ++++++++++++++++++++++++ > 9 files changed, 122 insertions(+), 89 deletions(-) > [...] > diff --git a/src/qemu/qemu_security.h b/src/qemu/qemu_security.h > index 54638908d..d86db3f6b 100644 > --- a/src/qemu/qemu_security.h > +++ b/src/qemu/qemu_security.h > @@ -28,6 +28,7 @@ > > # include "qemu_conf.h" > # include "domain_conf.h" > +# include "security/security_manager.h" > > int qemuSecuritySetAllLabel(virQEMUDriverPtr driver, > virDomainObjPtr vm, > @@ -60,4 +61,35 @@ int qemuSecuritySetHostdevLabel(virQEMUDriverPtr driver, > int qemuSecurityRestoreHostdevLabel(virQEMUDriverPtr driver, > virDomainObjPtr vm, > virDomainHostdevDefPtr hostdev); > + > +/* Please note that for these APIs there is no wrapper yet. Do NOT blindly add > + * new APIs here. If an API can touch a /dev file add a proper wrapper instead. > + */ > +# define qemuSecurityCheckAllLabel virSecurityManagerCheckAllLabel > +# define qemuSecurityClearSocketLabel virSecurityManagerClearSocketLabel > +# define qemuSecurityDomainSetPathLabel virSecurityManagerDomainSetPathLabel > +# define qemuSecurityGenLabel virSecurityManagerGenLabel > +# define qemuSecurityGetBaseLabel virSecurityManagerGetBaseLabel > +# define qemuSecurityGetDOI virSecurityManagerGetDOI > +# define qemuSecurityGetModel virSecurityManagerGetModel > +# define qemuSecurityGetMountOptions virSecurityManagerGetMountOptions > +# define qemuSecurityGetNested virSecurityManagerGetNested > +# define qemuSecurityGetProcessLabel virSecurityManagerGetProcessLabel > +# define qemuSecurityNew virSecurityManagerNew > +# define qemuSecurityNewDAC virSecurityManagerNewDAC > +# define qemuSecurityNewStack virSecurityManagerNewStack > +# define qemuSecurityPostFork virSecurityManagerPostFork > +# define qemuSecurityPreFork virSecurityManagerPreFork > +# define qemuSecurityReleaseLabel virSecurityManagerReleaseLabel > +# define qemuSecurityReserveLabel virSecurityManagerReserveLabel > +# define qemuSecurityRestoreSavedStateLabel virSecurityManagerRestoreSavedStateLabel > +# define qemuSecuritySetChildProcessLabel virSecurityManagerSetChildProcessLabel > +# define qemuSecuritySetDaemonSocketLabel virSecurityManagerSetDaemonSocketLabel > +# define qemuSecuritySetImageFDLabel virSecurityManagerSetImageFDLabel > +# define qemuSecuritySetSavedStateLabel virSecurityManagerSetSavedStateLabel > +# define qemuSecuritySetSocketLabel virSecurityManagerSetSocketLabel > +# define qemuSecuritySetTapFDLabel virSecurityManagerSetTapFDLabel > +# define qemuSecurityStackAddNested virSecurityManagerStackAddNested > +# define qemuSecurityVerify virSecurityManagerVerify I don't like this either for similar reasons that I've stated on the original series. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Feb 20, 2017 at 12:50:23PM +0100, Peter Krempa wrote: > On Tue, Feb 14, 2017 at 15:30:44 +0100, Michal Privoznik wrote: > > Now that we have some qemuSecurity wrappers over > > virSecurityManager APIs, lets make sure everybody sticks with > > them. We have them for a reason and calling virSecurityManager > > API directly instead of wrapper may lead into accidentally > > labelling a file on the host instead of namespace. > > > > Signed-off-by: Michal Privoznik <mprivozn@redhat.com> > > --- > > > > This is an alternative approach to: > > > > https://www.redhat.com/archives/libvir-list/2017-February/msg00271.html > > > > cfg.mk | 5 ++++ > > src/qemu/qemu_command.c | 7 +++--- > > src/qemu/qemu_conf.c | 9 ++++--- > > src/qemu/qemu_domain.c | 17 ++++++------- > > src/qemu/qemu_driver.c | 63 ++++++++++++++++++++++------------------------- > > src/qemu/qemu_hotplug.c | 4 +-- > > src/qemu/qemu_migration.c | 13 +++++----- > > src/qemu/qemu_process.c | 61 ++++++++++++++++++++++----------------------- > > src/qemu/qemu_security.h | 32 ++++++++++++++++++++++++ > > 9 files changed, 122 insertions(+), 89 deletions(-) > > > > [...] > > > diff --git a/src/qemu/qemu_security.h b/src/qemu/qemu_security.h > > index 54638908d..d86db3f6b 100644 > > --- a/src/qemu/qemu_security.h > > +++ b/src/qemu/qemu_security.h > > @@ -28,6 +28,7 @@ > > > > # include "qemu_conf.h" > > # include "domain_conf.h" > > +# include "security/security_manager.h" > > > > int qemuSecuritySetAllLabel(virQEMUDriverPtr driver, > > virDomainObjPtr vm, > > @@ -60,4 +61,35 @@ int qemuSecuritySetHostdevLabel(virQEMUDriverPtr driver, > > int qemuSecurityRestoreHostdevLabel(virQEMUDriverPtr driver, > > virDomainObjPtr vm, > > virDomainHostdevDefPtr hostdev); > > + > > +/* Please note that for these APIs there is no wrapper yet. Do NOT blindly add > > + * new APIs here. If an API can touch a /dev file add a proper wrapper instead. > > + */ > > +# define qemuSecurityCheckAllLabel virSecurityManagerCheckAllLabel > > +# define qemuSecurityClearSocketLabel virSecurityManagerClearSocketLabel > > +# define qemuSecurityDomainSetPathLabel virSecurityManagerDomainSetPathLabel > > +# define qemuSecurityGenLabel virSecurityManagerGenLabel > > +# define qemuSecurityGetBaseLabel virSecurityManagerGetBaseLabel > > +# define qemuSecurityGetDOI virSecurityManagerGetDOI > > +# define qemuSecurityGetModel virSecurityManagerGetModel > > +# define qemuSecurityGetMountOptions virSecurityManagerGetMountOptions > > +# define qemuSecurityGetNested virSecurityManagerGetNested > > +# define qemuSecurityGetProcessLabel virSecurityManagerGetProcessLabel > > +# define qemuSecurityNew virSecurityManagerNew > > +# define qemuSecurityNewDAC virSecurityManagerNewDAC > > +# define qemuSecurityNewStack virSecurityManagerNewStack > > +# define qemuSecurityPostFork virSecurityManagerPostFork > > +# define qemuSecurityPreFork virSecurityManagerPreFork > > +# define qemuSecurityReleaseLabel virSecurityManagerReleaseLabel > > +# define qemuSecurityReserveLabel virSecurityManagerReserveLabel > > +# define qemuSecurityRestoreSavedStateLabel virSecurityManagerRestoreSavedStateLabel > > +# define qemuSecuritySetChildProcessLabel virSecurityManagerSetChildProcessLabel > > +# define qemuSecuritySetDaemonSocketLabel virSecurityManagerSetDaemonSocketLabel > > +# define qemuSecuritySetImageFDLabel virSecurityManagerSetImageFDLabel > > +# define qemuSecuritySetSavedStateLabel virSecurityManagerSetSavedStateLabel > > +# define qemuSecuritySetSocketLabel virSecurityManagerSetSocketLabel > > +# define qemuSecuritySetTapFDLabel virSecurityManagerSetTapFDLabel > > +# define qemuSecurityStackAddNested virSecurityManagerStackAddNested > > +# define qemuSecurityVerify virSecurityManagerVerify > > I don't like this either for similar reasons that I've stated on the > original series. I think the interesting question here is whether we can figure out a way to push the qemuSecurity* functionality into the security drivers and thus remove the need for all these wrappers.... Currently all the wrappers look the same doing if (namespace enabled) { ...start transaction... } ... call real method... if (namespace enabled) { ...commit transaction... } The key observation to me is that there is only ever a single method called inside the transaction. That ought to mean we can push the transaction functionality down a layer. eg in the virSecurityManagerNew* methods we could pass in a "bool useTransactions" flag and then the security_manager.c could take care of start/commit/rollback Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Feb 20, 2017 at 11:55:06AM +0000, Daniel P. Berrange wrote: >On Mon, Feb 20, 2017 at 12:50:23PM +0100, Peter Krempa wrote: >> On Tue, Feb 14, 2017 at 15:30:44 +0100, Michal Privoznik wrote: >> > Now that we have some qemuSecurity wrappers over >> > virSecurityManager APIs, lets make sure everybody sticks with >> > them. We have them for a reason and calling virSecurityManager >> > API directly instead of wrapper may lead into accidentally >> > labelling a file on the host instead of namespace. >> > >> > Signed-off-by: Michal Privoznik <mprivozn@redhat.com> >> > --- >> > >> > This is an alternative approach to: >> > >> > https://www.redhat.com/archives/libvir-list/2017-February/msg00271.html >> > >> > cfg.mk | 5 ++++ >> > src/qemu/qemu_command.c | 7 +++--- >> > src/qemu/qemu_conf.c | 9 ++++--- >> > src/qemu/qemu_domain.c | 17 ++++++------- >> > src/qemu/qemu_driver.c | 63 ++++++++++++++++++++++------------------------- >> > src/qemu/qemu_hotplug.c | 4 +-- >> > src/qemu/qemu_migration.c | 13 +++++----- >> > src/qemu/qemu_process.c | 61 ++++++++++++++++++++++----------------------- >> > src/qemu/qemu_security.h | 32 ++++++++++++++++++++++++ >> > 9 files changed, 122 insertions(+), 89 deletions(-) >> > >> >> [...] >> >> > diff --git a/src/qemu/qemu_security.h b/src/qemu/qemu_security.h >> > index 54638908d..d86db3f6b 100644 >> > --- a/src/qemu/qemu_security.h >> > +++ b/src/qemu/qemu_security.h >> > @@ -28,6 +28,7 @@ >> > >> > # include "qemu_conf.h" >> > # include "domain_conf.h" >> > +# include "security/security_manager.h" >> > >> > int qemuSecuritySetAllLabel(virQEMUDriverPtr driver, >> > virDomainObjPtr vm, >> > @@ -60,4 +61,35 @@ int qemuSecuritySetHostdevLabel(virQEMUDriverPtr driver, >> > int qemuSecurityRestoreHostdevLabel(virQEMUDriverPtr driver, >> > virDomainObjPtr vm, >> > virDomainHostdevDefPtr hostdev); >> > + >> > +/* Please note that for these APIs there is no wrapper yet. Do NOT blindly add >> > + * new APIs here. If an API can touch a /dev file add a proper wrapper instead. >> > + */ >> > +# define qemuSecurityCheckAllLabel virSecurityManagerCheckAllLabel >> > +# define qemuSecurityClearSocketLabel virSecurityManagerClearSocketLabel >> > +# define qemuSecurityDomainSetPathLabel virSecurityManagerDomainSetPathLabel >> > +# define qemuSecurityGenLabel virSecurityManagerGenLabel >> > +# define qemuSecurityGetBaseLabel virSecurityManagerGetBaseLabel >> > +# define qemuSecurityGetDOI virSecurityManagerGetDOI >> > +# define qemuSecurityGetModel virSecurityManagerGetModel >> > +# define qemuSecurityGetMountOptions virSecurityManagerGetMountOptions >> > +# define qemuSecurityGetNested virSecurityManagerGetNested >> > +# define qemuSecurityGetProcessLabel virSecurityManagerGetProcessLabel >> > +# define qemuSecurityNew virSecurityManagerNew >> > +# define qemuSecurityNewDAC virSecurityManagerNewDAC >> > +# define qemuSecurityNewStack virSecurityManagerNewStack >> > +# define qemuSecurityPostFork virSecurityManagerPostFork >> > +# define qemuSecurityPreFork virSecurityManagerPreFork >> > +# define qemuSecurityReleaseLabel virSecurityManagerReleaseLabel >> > +# define qemuSecurityReserveLabel virSecurityManagerReserveLabel >> > +# define qemuSecurityRestoreSavedStateLabel virSecurityManagerRestoreSavedStateLabel >> > +# define qemuSecuritySetChildProcessLabel virSecurityManagerSetChildProcessLabel >> > +# define qemuSecuritySetDaemonSocketLabel virSecurityManagerSetDaemonSocketLabel >> > +# define qemuSecuritySetImageFDLabel virSecurityManagerSetImageFDLabel >> > +# define qemuSecuritySetSavedStateLabel virSecurityManagerSetSavedStateLabel >> > +# define qemuSecuritySetSocketLabel virSecurityManagerSetSocketLabel >> > +# define qemuSecuritySetTapFDLabel virSecurityManagerSetTapFDLabel >> > +# define qemuSecurityStackAddNested virSecurityManagerStackAddNested >> > +# define qemuSecurityVerify virSecurityManagerVerify >> >> I don't like this either for similar reasons that I've stated on the >> original series. > >I think the interesting question here is whether we can figure out a >way to push the qemuSecurity* functionality into the security drivers >and thus remove the need for all these wrappers.... > >Currently all the wrappers look the same doing > > if (namespace enabled) { > ...start transaction... > } > ... call real method... > if (namespace enabled) { > ...commit transaction... > } > >The key observation to me is that there is only ever a single method >called inside the transaction. That ought to mean we can push the >transaction functionality down a layer. eg in the virSecurityManagerNew* >methods we could pass in a "bool useTransactions" flag and then the >security_manager.c could take care of start/commit/rollback > Yeah, I was thinking about that too before, one of the ideas I had before. However, you need the bool to be there per-domain, not per-driver, and I haven't found out how to automatically take care of commit (and rollback). You'd have to call that anyway. >Regards, >Daniel >-- >|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| >|: http://libvirt.org -o- http://virt-manager.org :| >|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :| > >-- >libvir-list mailing list >libvir-list@redhat.com >https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 20.02.2017 12:55, Daniel P. Berrange wrote: > On Mon, Feb 20, 2017 at 12:50:23PM +0100, Peter Krempa wrote: >> On Tue, Feb 14, 2017 at 15:30:44 +0100, Michal Privoznik wrote: >>> Now that we have some qemuSecurity wrappers over >>> virSecurityManager APIs, lets make sure everybody sticks with >>> them. We have them for a reason and calling virSecurityManager >>> API directly instead of wrapper may lead into accidentally >>> labelling a file on the host instead of namespace. >>> >>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> >>> --- >>> >>> This is an alternative approach to: >>> >>> https://www.redhat.com/archives/libvir-list/2017-February/msg00271.html >>> >>> cfg.mk | 5 ++++ >>> src/qemu/qemu_command.c | 7 +++--- >>> src/qemu/qemu_conf.c | 9 ++++--- >>> src/qemu/qemu_domain.c | 17 ++++++------- >>> src/qemu/qemu_driver.c | 63 ++++++++++++++++++++++------------------------- >>> src/qemu/qemu_hotplug.c | 4 +-- >>> src/qemu/qemu_migration.c | 13 +++++----- >>> src/qemu/qemu_process.c | 61 ++++++++++++++++++++++----------------------- >>> src/qemu/qemu_security.h | 32 ++++++++++++++++++++++++ >>> 9 files changed, 122 insertions(+), 89 deletions(-) >>> >> >> [...] >> >>> diff --git a/src/qemu/qemu_security.h b/src/qemu/qemu_security.h >>> index 54638908d..d86db3f6b 100644 >>> --- a/src/qemu/qemu_security.h >>> +++ b/src/qemu/qemu_security.h >>> @@ -28,6 +28,7 @@ >>> >>> # include "qemu_conf.h" >>> # include "domain_conf.h" >>> +# include "security/security_manager.h" >>> >>> int qemuSecuritySetAllLabel(virQEMUDriverPtr driver, >>> virDomainObjPtr vm, >>> @@ -60,4 +61,35 @@ int qemuSecuritySetHostdevLabel(virQEMUDriverPtr driver, >>> int qemuSecurityRestoreHostdevLabel(virQEMUDriverPtr driver, >>> virDomainObjPtr vm, >>> virDomainHostdevDefPtr hostdev); >>> + >>> +/* Please note that for these APIs there is no wrapper yet. Do NOT blindly add >>> + * new APIs here. If an API can touch a /dev file add a proper wrapper instead. >>> + */ >>> +# define qemuSecurityCheckAllLabel virSecurityManagerCheckAllLabel >>> +# define qemuSecurityClearSocketLabel virSecurityManagerClearSocketLabel >>> +# define qemuSecurityDomainSetPathLabel virSecurityManagerDomainSetPathLabel >>> +# define qemuSecurityGenLabel virSecurityManagerGenLabel >>> +# define qemuSecurityGetBaseLabel virSecurityManagerGetBaseLabel >>> +# define qemuSecurityGetDOI virSecurityManagerGetDOI >>> +# define qemuSecurityGetModel virSecurityManagerGetModel >>> +# define qemuSecurityGetMountOptions virSecurityManagerGetMountOptions >>> +# define qemuSecurityGetNested virSecurityManagerGetNested >>> +# define qemuSecurityGetProcessLabel virSecurityManagerGetProcessLabel >>> +# define qemuSecurityNew virSecurityManagerNew >>> +# define qemuSecurityNewDAC virSecurityManagerNewDAC >>> +# define qemuSecurityNewStack virSecurityManagerNewStack >>> +# define qemuSecurityPostFork virSecurityManagerPostFork >>> +# define qemuSecurityPreFork virSecurityManagerPreFork >>> +# define qemuSecurityReleaseLabel virSecurityManagerReleaseLabel >>> +# define qemuSecurityReserveLabel virSecurityManagerReserveLabel >>> +# define qemuSecurityRestoreSavedStateLabel virSecurityManagerRestoreSavedStateLabel >>> +# define qemuSecuritySetChildProcessLabel virSecurityManagerSetChildProcessLabel >>> +# define qemuSecuritySetDaemonSocketLabel virSecurityManagerSetDaemonSocketLabel >>> +# define qemuSecuritySetImageFDLabel virSecurityManagerSetImageFDLabel >>> +# define qemuSecuritySetSavedStateLabel virSecurityManagerSetSavedStateLabel >>> +# define qemuSecuritySetSocketLabel virSecurityManagerSetSocketLabel >>> +# define qemuSecuritySetTapFDLabel virSecurityManagerSetTapFDLabel >>> +# define qemuSecurityStackAddNested virSecurityManagerStackAddNested >>> +# define qemuSecurityVerify virSecurityManagerVerify >> >> I don't like this either for similar reasons that I've stated on the >> original series. > > I think the interesting question here is whether we can figure out a > way to push the qemuSecurity* functionality into the security drivers > and thus remove the need for all these wrappers.... > > Currently all the wrappers look the same doing > > if (namespace enabled) { > ...start transaction... > } > ... call real method... > if (namespace enabled) { > ...commit transaction... > } > > The key observation to me is that there is only ever a single method > called inside the transaction. That ought to mean we can push the > transaction functionality down a layer. eg in the virSecurityManagerNew* > methods we could pass in a "bool useTransactions" flag and then the > security_manager.c could take care of start/commit/rollback That would not fly. As Martin pointed out we need to use transactions on per-domain basis. The decision whether to use transactions is based on a boolean stored in the domain object and not the domain definition (well, technically it's not a boolean rather than bitmap, but that's not the point here). I thought of registering a callback in the secdriver which would return true/false depending whether transactions should be used. But that would not fly either - secdriver takes domain def and not domain obj. So it can only feed the callback with domain def which lacks the information whether namespaces are used or not. The other solution I can come up with is to use transactions unconditionally - even for domains that don't run in a namespace. The only drawback there is useless fork to enter the namespace we are already in. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Feb 14, 2017 at 15:30:44 +0100, Michal Privoznik wrote: > Now that we have some qemuSecurity wrappers over > virSecurityManager APIs, lets make sure everybody sticks with > them. We have them for a reason and calling virSecurityManager > API directly instead of wrapper may lead into accidentally > labelling a file on the host instead of namespace. > > Signed-off-by: Michal Privoznik <mprivozn@redhat.com> > --- > > This is an alternative approach to: > > https://www.redhat.com/archives/libvir-list/2017-February/msg00271.html While I think that by putting some more effor to the script checking the rules it would be possible to achieve the same even without having to have macros for the APIs which don't require wrapping I must agree that it's better to have a check with techincal debt rather than a bug. ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 06.03.2017 12:43, Peter Krempa wrote: > On Tue, Feb 14, 2017 at 15:30:44 +0100, Michal Privoznik wrote: >> Now that we have some qemuSecurity wrappers over >> virSecurityManager APIs, lets make sure everybody sticks with >> them. We have them for a reason and calling virSecurityManager >> API directly instead of wrapper may lead into accidentally >> labelling a file on the host instead of namespace. >> >> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> >> --- >> >> This is an alternative approach to: >> >> https://www.redhat.com/archives/libvir-list/2017-February/msg00271.html > > While I think that by putting some more effor to the script checking the > rules it would be possible to achieve the same even without having to > have macros for the APIs which don't require wrapping I must agree that > it's better to have a check with techincal debt rather than a bug. Agreed. I believe we might turn some of those dummy #define-s into actual functions one day. But at least for now we will not introduce new bugs. > > ACK > Thank you, pushed. One thing less to remember while reviewing qemu patches. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.