[libvirt] [PATCH 0/3] qemu: Fix how files are being opened
There were some places in the code, where files were being opened with uid:gid of the daemon instead of the qemu process related to the file. First patch exposes the parseIds() function in order for it to be used somewhere else in the code than in the DAC security driver. The next patch fixes how the files are opened and the last one fixes occurences of open() that should use different uid:gid for opening files. There maybe should be a check for whether the file being opened is an image and whether the label used to open the file should be imagelabel or not. But, the QEMU process opening the file is running as the label (not imagelabel) and accessing the files as such. Martin Kletzander (3): Expose ownership ID parsing Make qemuOpenFile aware of per-VM DAC seclabel. Use qemuOpenFile in qemu_driver.c src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 87 +++++++++++++++++++++++++++++++-------------- src/security/security_dac.c | 51 ++------------------------ src/util/virutil.c | 56 +++++++++++++++++++++++++++++ src/util/virutil.h | 2 ++ 5 files changed, 122 insertions(+), 75 deletions(-) -- 1.8.2.1
Parsing 'user:group' is useful even outside the DAC security driver, so expose the most abstract function which has no DAC security driver bits in itself. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/libvirt_private.syms | 1 + src/security/security_dac.c | 51 +++-------------------------------------- src/util/virutil.c | 56 +++++++++++++++++++++++++++++++++++++++++++++ src/util/virutil.h | 2 ++ 4 files changed, 62 insertions(+), 48 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9d5f74b..1927451 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1951,6 +1951,7 @@ virIsCapableVport; virIsDevMapperDevice; virManageVport; virParseNumber; +virParseOwnershipIds; virParseVersionString; virPipeReadUntilEOF; virReadFCHost; diff --git a/src/security/security_dac.c b/src/security/security_dac.c index b8d1a92..0264c28 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -33,6 +33,7 @@ #include "virscsi.h" #include "virstoragefile.h" #include "virstring.h" +#include "virutil.h" #define VIR_FROM_THIS VIR_FROM_SECURITY #define SECURITY_DAC_NAME "dac" @@ -70,52 +71,6 @@ virSecurityDACSetDynamicOwnership(virSecurityManagerPtr mgr, priv->dynamicOwnership = dynamicOwnership; } -static int -parseIds(const char *label, uid_t *uidPtr, gid_t *gidPtr) -{ - int rc = -1; - uid_t theuid; - gid_t thegid; - char *tmp_label = NULL; - char *sep = NULL; - char *owner = NULL; - char *group = NULL; - - if (VIR_STRDUP(tmp_label, label) < 0) - goto cleanup; - - /* Split label */ - sep = strchr(tmp_label, ':'); - if (sep == NULL) { - virReportError(VIR_ERR_INVALID_ARG, - _("Missing separator ':' in DAC label \"%s\""), - label); - goto cleanup; - } - *sep = '\0'; - owner = tmp_label; - group = sep + 1; - - /* Parse owner and group, error message is defined by - * virGetUserID or virGetGroupID. - */ - if (virGetUserID(owner, &theuid) < 0 || - virGetGroupID(group, &thegid) < 0) - goto cleanup; - - if (uidPtr) - *uidPtr = theuid; - if (gidPtr) - *gidPtr = thegid; - - rc = 0; - -cleanup: - VIR_FREE(tmp_label); - - return rc; -} - /* returns 1 if label isn't found, 0 on success, -1 on error */ static int virSecurityDACParseIds(virDomainDefPtr def, uid_t *uidPtr, gid_t *gidPtr) @@ -133,7 +88,7 @@ virSecurityDACParseIds(virDomainDefPtr def, uid_t *uidPtr, gid_t *gidPtr) return 1; } - if (parseIds(seclabel->label, &uid, &gid) < 0) + if (virParseOwnershipIds(seclabel->label, &uid, &gid) < 0) return -1; if (uidPtr) @@ -194,7 +149,7 @@ virSecurityDACParseImageIds(virDomainDefPtr def, return 1; } - if (parseIds(seclabel->imagelabel, &uid, &gid) < 0) + if (virParseOwnershipIds(seclabel->imagelabel, &uid, &gid) < 0) return -1; if (uidPtr) diff --git a/src/util/virutil.c b/src/util/virutil.c index 028f1d1..450e5e3 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -2071,3 +2071,59 @@ virCompareLimitUlong(unsigned long long a, unsigned long b) return -1; } + +/** + * virParseOwnershipIds: + * + * Parse the usual "uid:gid" ownership specification into uid_t and + * gid_t passed as parameters. NULL value for those parameters mean + * the information is not needed. Also, none of those values are + * changed in case of any error. + * + * Returns -1 on error, 0 otherwise. + */ +int +virParseOwnershipIds(const char *label, uid_t *uidPtr, gid_t *gidPtr) +{ + int rc = -1; + uid_t theuid; + gid_t thegid; + char *tmp_label = NULL; + char *sep = NULL; + char *owner = NULL; + char *group = NULL; + + if (VIR_STRDUP(tmp_label, label) < 0) + goto cleanup; + + /* Split label */ + sep = strchr(tmp_label, ':'); + if (sep == NULL) { + virReportError(VIR_ERR_INVALID_ARG, + _("Failed to parse uid and gid from '%s'"), + label); + goto cleanup; + } + *sep = '\0'; + owner = tmp_label; + group = sep + 1; + + /* Parse owner and group, error message is defined by + * virGetUserID or virGetGroupID. + */ + if (virGetUserID(owner, &theuid) < 0 || + virGetGroupID(group, &thegid) < 0) + goto cleanup; + + if (uidPtr) + *uidPtr = theuid; + if (gidPtr) + *gidPtr = thegid; + + rc = 0; + +cleanup: + VIR_FREE(tmp_label); + + return rc; +} diff --git a/src/util/virutil.h b/src/util/virutil.h index 280a18d..0f6bcc1 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -166,4 +166,6 @@ char *virFindFCHostCapableVport(const char *sysfs_prefix); int virCompareLimitUlong(unsigned long long a, unsigned long b); +int virParseOwnershipIds(const char *label, uid_t *uidPtr, gid_t *gidPtr); + #endif /* __VIR_UTIL_H__ */ -- 1.8.2.1
On 24.05.2013 22:25, Martin Kletzander wrote:
Parsing 'user:group' is useful even outside the DAC security driver, so expose the most abstract function which has no DAC security driver bits in itself.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/libvirt_private.syms | 1 + src/security/security_dac.c | 51 +++-------------------------------------- src/util/virutil.c | 56 +++++++++++++++++++++++++++++++++++++++++++++ src/util/virutil.h | 2 ++ 4 files changed, 62 insertions(+), 48 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9d5f74b..1927451 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1951,6 +1951,7 @@ virIsCapableVport; virIsDevMapperDevice; virManageVport; virParseNumber; +virParseOwnershipIds; virParseVersionString; virPipeReadUntilEOF; virReadFCHost; diff --git a/src/security/security_dac.c b/src/security/security_dac.c index b8d1a92..0264c28 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -33,6 +33,7 @@ #include "virscsi.h" #include "virstoragefile.h" #include "virstring.h" +#include "virutil.h"
#define VIR_FROM_THIS VIR_FROM_SECURITY #define SECURITY_DAC_NAME "dac" @@ -70,52 +71,6 @@ virSecurityDACSetDynamicOwnership(virSecurityManagerPtr mgr, priv->dynamicOwnership = dynamicOwnership; }
-static int -parseIds(const char *label, uid_t *uidPtr, gid_t *gidPtr) -{ - int rc = -1; - uid_t theuid; - gid_t thegid; - char *tmp_label = NULL; - char *sep = NULL; - char *owner = NULL; - char *group = NULL; - - if (VIR_STRDUP(tmp_label, label) < 0) - goto cleanup; - - /* Split label */ - sep = strchr(tmp_label, ':'); - if (sep == NULL) { - virReportError(VIR_ERR_INVALID_ARG, - _("Missing separator ':' in DAC label \"%s\""), - label); - goto cleanup; - } - *sep = '\0'; - owner = tmp_label; - group = sep + 1; - - /* Parse owner and group, error message is defined by - * virGetUserID or virGetGroupID. - */ - if (virGetUserID(owner, &theuid) < 0 || - virGetGroupID(group, &thegid) < 0) - goto cleanup; - - if (uidPtr) - *uidPtr = theuid; - if (gidPtr) - *gidPtr = thegid; - - rc = 0; - -cleanup: - VIR_FREE(tmp_label); - - return rc; -} - /* returns 1 if label isn't found, 0 on success, -1 on error */ static int virSecurityDACParseIds(virDomainDefPtr def, uid_t *uidPtr, gid_t *gidPtr) @@ -133,7 +88,7 @@ virSecurityDACParseIds(virDomainDefPtr def, uid_t *uidPtr, gid_t *gidPtr) return 1; }
- if (parseIds(seclabel->label, &uid, &gid) < 0) + if (virParseOwnershipIds(seclabel->label, &uid, &gid) < 0) return -1;
if (uidPtr) @@ -194,7 +149,7 @@ virSecurityDACParseImageIds(virDomainDefPtr def, return 1; }
- if (parseIds(seclabel->imagelabel, &uid, &gid) < 0) + if (virParseOwnershipIds(seclabel->imagelabel, &uid, &gid) < 0) return -1;
if (uidPtr) diff --git a/src/util/virutil.c b/src/util/virutil.c index 028f1d1..450e5e3 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -2071,3 +2071,59 @@ virCompareLimitUlong(unsigned long long a, unsigned long b)
return -1; } + +/** + * virParseOwnershipIds: + * + * Parse the usual "uid:gid" ownership specification into uid_t and + * gid_t passed as parameters. NULL value for those parameters mean + * the information is not needed. Also, none of those values are + * changed in case of any error. + * + * Returns -1 on error, 0 otherwise. + */ +int +virParseOwnershipIds(const char *label, uid_t *uidPtr, gid_t *gidPtr) +{ + int rc = -1; + uid_t theuid; + gid_t thegid; + char *tmp_label = NULL; + char *sep = NULL; + char *owner = NULL; + char *group = NULL; + + if (VIR_STRDUP(tmp_label, label) < 0) + goto cleanup; + + /* Split label */ + sep = strchr(tmp_label, ':'); + if (sep == NULL) { + virReportError(VIR_ERR_INVALID_ARG, + _("Failed to parse uid and gid from '%s'"),
This is the only change to the original impl.
+ label); + goto cleanup; + } + *sep = '\0'; + owner = tmp_label; + group = sep + 1; + + /* Parse owner and group, error message is defined by + * virGetUserID or virGetGroupID. + */ + if (virGetUserID(owner, &theuid) < 0 || + virGetGroupID(group, &thegid) < 0) + goto cleanup; + + if (uidPtr) + *uidPtr = theuid; + if (gidPtr) + *gidPtr = thegid; + + rc = 0; + +cleanup: + VIR_FREE(tmp_label); + + return rc; +} diff --git a/src/util/virutil.h b/src/util/virutil.h index 280a18d..0f6bcc1 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -166,4 +166,6 @@ char *virFindFCHostCapableVport(const char *sysfs_prefix);
int virCompareLimitUlong(unsigned long long a, unsigned long b);
+int virParseOwnershipIds(const char *label, uid_t *uidPtr, gid_t *gidPtr); + #endif /* __VIR_UTIL_H__ */
Michal
Function qemuOpenFile() haven't had any idea about seclabels applied to VMs only, so in case the seclabel differed from the "user:group" from configuration, there might have been issues with opening files. Make qemuOpenFile() VM-aware, but only optionally, passing NULL argument means skipping VM seclabel info completely. However, all current qemuOpenFile() calls look like they should use VM seclabel info in case there is any, so convert these calls as well. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=869053 Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_driver.c | 71 ++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 55 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4a76f14..87dedef 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -147,6 +147,16 @@ static int qemuDomainGetMaxVcpus(virDomainPtr dom); static int qemuDomainManagedSaveLoad(virDomainObjPtr vm, void *opaque); +static int qemuOpenFile(virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *path, int oflags, + bool *needUnlink, bool *bypassSecurityDriver); + +static int qemuOpenFileAs(uid_t fallback_uid, gid_t fallback_gid, + bool dynamicOwnership, + const char *path, int oflags, + bool *needUnlink, bool *bypassSecurityDriver); + virQEMUDriverPtr qemu_driver = NULL; @@ -2551,11 +2561,41 @@ qemuCompressGetCommand(virQEMUSaveFormat compression) } /* Internal function to properly create or open existing files, with - * ownership affected by qemu driver setup. */ + * ownership affected by qemu driver setup and domain DAC label. */ static int -qemuOpenFile(virQEMUDriverPtr driver, const char *path, int oflags, +qemuOpenFile(virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *path, int oflags, bool *needUnlink, bool *bypassSecurityDriver) { + int ret = -1; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + uid_t user = cfg->user; + gid_t group = cfg->group; + bool dynamicOwnership = cfg->dynamicOwnership; + virSecurityLabelDefPtr seclabel; + + virObjectUnref(cfg); + + /* TODO: Take imagelabel into account? */ + if (vm && + (seclabel = virDomainDefGetSecurityLabelDef(vm->def, "dac")) != NULL && + (virParseOwnershipIds(seclabel->label, &user, &group) < 0)) + goto cleanup; + + ret = qemuOpenFileAs(user, group, dynamicOwnership, + path, oflags, needUnlink, bypassSecurityDriver); + + cleanup: + return ret; +} + +static int +qemuOpenFileAs(uid_t fallback_uid, gid_t fallback_gid, + bool dynamicOwnership, + const char *path, int oflags, + bool *needUnlink, bool *bypassSecurityDriver) +{ struct stat sb; bool is_reg = true; bool need_unlink = false; @@ -2565,7 +2605,6 @@ qemuOpenFile(virQEMUDriverPtr driver, const char *path, int oflags, int path_shared = virStorageFileIsSharedFS(path); uid_t uid = getuid(); gid_t gid = getgid(); - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); /* path might be a pre-existing block dev, in which case * we need to skip the create step, and also avoid unlink @@ -2575,7 +2614,7 @@ qemuOpenFile(virQEMUDriverPtr driver, const char *path, int oflags, /* Don't force chown on network-shared FS * as it is likely to fail. */ - if (path_shared <= 0 || cfg->dynamicOwnership) + if (path_shared <= 0 || dynamicOwnership) vfoflags |= VIR_FILE_OPEN_FORCE_OWNER; if (stat(path, &sb) == 0) { @@ -2583,7 +2622,7 @@ qemuOpenFile(virQEMUDriverPtr driver, const char *path, int oflags, /* If the path is regular file which exists * already and dynamic_ownership is off, we don't * want to change it's ownership, just open it as-is */ - if (is_reg && !cfg->dynamicOwnership) { + if (is_reg && !dynamicOwnership) { uid = sb.st_uid; gid = sb.st_gid; } @@ -2602,10 +2641,10 @@ qemuOpenFile(virQEMUDriverPtr driver, const char *path, int oflags, /* If we failed as root, and the error was permission-denied (EACCES or EPERM), assume it's on a network-connected share where root access is restricted (eg, root-squashed NFS). If the - qemu user (cfg->user) is non-root, just set a flag to + qemu user is non-root, just set a flag to bypass security driver shenanigans, and retry the operation after doing setuid to qemu user */ - if ((fd != -EACCES && fd != -EPERM) || cfg->user == getuid()) + if ((fd != -EACCES && fd != -EPERM) || fallback_uid == getuid()) goto error; /* On Linux we can also verify the FS-type of the directory. */ @@ -2631,11 +2670,11 @@ qemuOpenFile(virQEMUDriverPtr driver, const char *path, int oflags, goto error; } - /* Retry creating the file as cfg->user */ + /* Retry creating the file as qemu user */ if ((fd = virFileOpenAs(path, oflags, S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP, - cfg->user, cfg->group, + fallback_uid, fallback_gid, vfoflags | VIR_FILE_OPEN_FORK)) < 0) { virReportSystemError(-fd, oflags & O_CREAT ? _("Error from child process creating '%s'") @@ -2656,7 +2695,6 @@ cleanup: *needUnlink = need_unlink; if (bypassSecurityDriver) *bypassSecurityDriver = bypass_security; - virObjectUnref(cfg); return fd; error: @@ -2733,7 +2771,8 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver, goto cleanup; } } - fd = qemuOpenFile(driver, path, O_WRONLY | O_TRUNC | O_CREAT | directFlag, + fd = qemuOpenFile(driver, vm, path, + O_WRONLY | O_TRUNC | O_CREAT | directFlag, &needUnlink, &bypassSecurityDriver); if (fd < 0) goto cleanup; @@ -2766,7 +2805,7 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver, if (virFileWrapperFdClose(wrapperFd) < 0) goto cleanup; - if ((fd = qemuOpenFile(driver, path, O_WRONLY, NULL, NULL)) < 0) + if ((fd = qemuOpenFile(driver, vm, path, O_WRONLY, NULL, NULL)) < 0) goto cleanup; memcpy(header.magic, QEMU_SAVE_MAGIC, sizeof(header.magic)); @@ -3178,7 +3217,7 @@ doCoreDump(virQEMUDriverPtr driver, /* Core dumps usually imply last-ditch analysis efforts are * desired, so we intentionally do not unlink even if a file was * created. */ - if ((fd = qemuOpenFile(driver, path, + if ((fd = qemuOpenFile(driver, vm, path, O_CREAT | O_TRUNC | O_WRONLY | directFlag, NULL, NULL)) < 0) goto cleanup; @@ -4628,7 +4667,7 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto error; - if ((fd = qemuOpenFile(driver, path, oflags, NULL, NULL)) < 0) + if ((fd = qemuOpenFile(driver, NULL, path, oflags, NULL, NULL)) < 0) goto error; if (bypass_cache && !(*wrapperFd = virFileWrapperFdNew(&fd, path, @@ -11232,7 +11271,7 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, /* create the stub file and set selinux labels; manipulate disk in * place, in a way that can be reverted on failure. */ if (!reuse) { - fd = qemuOpenFile(driver, source, O_WRONLY | O_TRUNC | O_CREAT, + fd = qemuOpenFile(driver, vm, source, O_WRONLY | O_TRUNC | O_CREAT, &need_unlink, NULL); if (fd < 0) goto cleanup; @@ -13574,7 +13613,7 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *path, } if (!(flags & VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT)) { - int fd = qemuOpenFile(driver, dest, O_WRONLY | O_TRUNC | O_CREAT, + int fd = qemuOpenFile(driver, vm, dest, O_WRONLY | O_TRUNC | O_CREAT, &need_unlink, NULL); if (fd < 0) goto endjob; -- 1.8.2.1
On two places, the usage of open() is replaced with qemuOpenFile as that is the preferred method in those cases. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=963881 Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_driver.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 87dedef..a08a72a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9165,6 +9165,7 @@ qemuDomainBlockPeek(virDomainPtr dom, void *buffer, unsigned int flags) { + virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; int fd = -1, ret = -1; const char *actual; @@ -9188,13 +9189,9 @@ qemuDomainBlockPeek(virDomainPtr dom, } path = actual; - /* The path is correct, now try to open it and get its size. */ - fd = open(path, O_RDONLY); - if (fd == -1) { - virReportSystemError(errno, - _("%s: failed to open"), path); + fd = qemuOpenFile(driver, vm, path, O_RDONLY, NULL, NULL); + if (fd == -1) goto cleanup; - } /* Seek and read. */ /* NB. Because we configure with AC_SYS_LARGEFILE, off_t should @@ -9351,12 +9348,9 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom, path = disk->src; /* The path is correct, now try to open it and get its size. */ - fd = open(path, O_RDONLY); - if (fd == -1) { - virReportSystemError(errno, - _("failed to open path '%s'"), path); + fd = qemuOpenFile(driver, vm, path, O_RDONLY, NULL, NULL); + if (fd == -1) goto cleanup; - } /* Probe for magic formats */ if (disk->format) { -- 1.8.2.1
On 05/24/2013 10:25 PM, Martin Kletzander wrote:
There were some places in the code, where files were being opened with uid:gid of the daemon instead of the qemu process related to the file.
First patch exposes the parseIds() function in order for it to be used somewhere else in the code than in the DAC security driver. The next patch fixes how the files are opened and the last one fixes occurences of open() that should use different uid:gid for opening files.
There maybe should be a check for whether the file being opened is an image and whether the label used to open the file should be imagelabel or not. But, the QEMU process opening the file is running as the label (not imagelabel) and accessing the files as such.
Martin Kletzander (3): Expose ownership ID parsing Make qemuOpenFile aware of per-VM DAC seclabel. Use qemuOpenFile in qemu_driver.c
src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 87 +++++++++++++++++++++++++++++++-------------- src/security/security_dac.c | 51 ++------------------------ src/util/virutil.c | 56 +++++++++++++++++++++++++++++ src/util/virutil.h | 2 ++ 5 files changed, 122 insertions(+), 75 deletions(-)
Ping?
On 06/24/2013 12:19 PM, Martin Kletzander wrote:
On 05/24/2013 10:25 PM, Martin Kletzander wrote:
There were some places in the code, where files were being opened with uid:gid of the daemon instead of the qemu process related to the file.
First patch exposes the parseIds() function in order for it to be used somewhere else in the code than in the DAC security driver. The next patch fixes how the files are opened and the last one fixes occurences of open() that should use different uid:gid for opening files.
There maybe should be a check for whether the file being opened is an image and whether the label used to open the file should be imagelabel or not. But, the QEMU process opening the file is running as the label (not imagelabel) and accessing the files as such.
Martin Kletzander (3): Expose ownership ID parsing Make qemuOpenFile aware of per-VM DAC seclabel. Use qemuOpenFile in qemu_driver.c
src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 87 +++++++++++++++++++++++++++++++-------------- src/security/security_dac.c | 51 ++------------------------ src/util/virutil.c | 56 +++++++++++++++++++++++++++++ src/util/virutil.h | 2 ++ 5 files changed, 122 insertions(+), 75 deletions(-)
Ping?
Ping? Still applicable on master, fixes at least two bugs... Martin
On 24.05.2013 22:25, Martin Kletzander wrote:
There were some places in the code, where files were being opened with uid:gid of the daemon instead of the qemu process related to the file.
First patch exposes the parseIds() function in order for it to be used somewhere else in the code than in the DAC security driver. The next patch fixes how the files are opened and the last one fixes occurences of open() that should use different uid:gid for opening files.
There maybe should be a check for whether the file being opened is an image and whether the label used to open the file should be imagelabel or not. But, the QEMU process opening the file is running as the label (not imagelabel) and accessing the files as such.
Martin Kletzander (3): Expose ownership ID parsing Make qemuOpenFile aware of per-VM DAC seclabel. Use qemuOpenFile in qemu_driver.c
src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 87 +++++++++++++++++++++++++++++++-------------- src/security/security_dac.c | 51 ++------------------------ src/util/virutil.c | 56 +++++++++++++++++++++++++++++ src/util/virutil.h | 2 ++ 5 files changed, 122 insertions(+), 75 deletions(-)
-- 1.8.2.1
ACK series,
On Wed 24 Jul 2013 10:56:56 AM CEST, Michal Privoznik wrote:
On 24.05.2013 22:25, Martin Kletzander wrote:
There were some places in the code, where files were being opened with uid:gid of the daemon instead of the qemu process related to the file.
First patch exposes the parseIds() function in order for it to be used somewhere else in the code than in the DAC security driver. The next patch fixes how the files are opened and the last one fixes occurences of open() that should use different uid:gid for opening files.
There maybe should be a check for whether the file being opened is an image and whether the label used to open the file should be imagelabel or not. But, the QEMU process opening the file is running as the label (not imagelabel) and accessing the files as such.
Martin Kletzander (3): Expose ownership ID parsing Make qemuOpenFile aware of per-VM DAC seclabel. Use qemuOpenFile in qemu_driver.c
src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 87 +++++++++++++++++++++++++++++++-------------- src/security/security_dac.c | 51 ++------------------------ src/util/virutil.c | 56 +++++++++++++++++++++++++++++ src/util/virutil.h | 2 ++ 5 files changed, 122 insertions(+), 75 deletions(-)
-- 1.8.2.1
ACK series,
Thanks, pushed. Martin
participants (2)
-
Martin Kletzander -
Michal Privoznik