Hi Marc-André On 07/27/2015 11:39 PM, Marc-André Lureau wrote:
HiOn Thu, Jul 23, 2015 at 12:13 PM, Luyao Huang<lhu...@redhat.com> wrote:A new api to help set/restore the shmem deivce dac/selinux label.typo: deivce / device.
thanks, i will fix this in next version
Signed-off-by: Luyao Huang<lhu...@redhat.com> --- src/libvirt_private.syms | 2 ++ src/security/security_dac.c | 67 +++++++++++++++++++++++++++++++++++++++ src/security/security_driver.h | 11 +++++++ src/security/security_manager.c | 38 ++++++++++++++++++++++ src/security/security_manager.h | 8 +++++ src/security/security_selinux.c | 70 +++++++++++++++++++++++++++++++++++++++++ src/security/security_stack.c | 41 ++++++++++++++++++++++++ 7 files changed, 237 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 588b1c4..af73177 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1038,6 +1038,7 @@ virSecurityManagerRestoreDiskLabel; virSecurityManagerRestoreHostdevLabel; virSecurityManagerRestoreImageLabel; virSecurityManagerRestoreSavedStateLabel; +virSecurityManagerRestoreShmemLabel; virSecurityManagerSetAllLabel; virSecurityManagerSetChildProcessLabel; virSecurityManagerSetDaemonSocketLabel; @@ -1048,6 +1049,7 @@ virSecurityManagerSetImageFDLabel; virSecurityManagerSetImageLabel; virSecurityManagerSetProcessLabel; virSecurityManagerSetSavedStateLabel; +virSecurityManagerSetShmemLabel; virSecurityManagerSetSocketLabel; virSecurityManagerSetTapFDLabel; virSecurityManagerStackAddNested; diff --git a/src/security/security_dac.c b/src/security/security_dac.c index deb6980..f954aa5 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -39,6 +39,7 @@ #include "virstoragefile.h" #include "virstring.h" #include "virutil.h" +#include "virshm.h"This header doesn't exist (yet)
I forgot remove this, thanks for pointing out. (and i won't remove this, since i need a function in virshm.h, so i will move it this patch after patch 3/4 in next version)
#define VIR_FROM_THIS VIR_FROM_SECURITY @@ -922,6 +923,69 @@ virSecurityDACRestoreSecurityTPMFileLabel(virSecurityManagerPtr mgr, static int +virSecurityDACSetShmemLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainShmemDefPtr shmem, + char *path) +{ + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityLabelDefPtr seclabel; + virSecurityDeviceLabelDefPtr shmem_seclabel = NULL; + char *tmppath;could make it const
Okay
+ uid_t user; + gid_t group; + + if (shmem->server.enabled) + tmppath = shmem->server.chr.data.nix.path; + else + tmppath = path; + + if (!tmppath) + return 0; + + shmem_seclabel = virDomainShmemDefGetSecurityLabelDef(shmem, SECURITY_DAC_NAME); + + if (shmem_seclabel && !shmem_seclabel->relabel) + return 0; + + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); +The function is similar to virSecurityDACSetSecurityImageLabel and yet subtly different: there is a early dynamicOwnership condition that seems to be general, the domain seclabel->relabel is checked first. It would be nice to align the behaviour.
Indeed, i will move the shmem_seclabel and seclabel check more early.
+ if (shmem_seclabel && shmem_seclabel->label) { + if (virParseOwnershipIds(shmem_seclabel->label, &user, &group) < 0) + return -1; + } else { + if (virSecurityDACGetIds(seclabel, priv, &user, &group, NULL, NULL) < 0) + return -1; + } + + return virSecurityDACSetOwnership(tmppath, user, group); +} + + +static int +virSecurityDACRestoreShmemLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainShmemDefPtr shmem, + char *path) +{ + virSecurityDeviceLabelDefPtr shmem_seclabel = NULL; + + shmem_seclabel = virDomainShmemDefGetSecurityLabelDef(shmem, SECURITY_DAC_NAME); + + if (shmem_seclabel && !shmem_seclabel->relabel) + return 0; + + if (shmem->server.enabled) + return virSecurityDACRestoreChardevLabel(mgr, def, NULL, &shmem->server.chr); + + if (!path) + return 0; + + return virSecurityDACRestoreSecurityFileLabel(path); +} + + +static int virSecurityDACRestoreSecurityAllLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, bool migrated) @@ -1433,4 +1497,7 @@ virSecurityDriver virSecurityDriverDAC = { .domainGetSecurityMountOptions = virSecurityDACGetMountOptions, .getBaseLabel = virSecurityDACGetBaseLabel, + + .domainSetSecurityShmemLabel = virSecurityDACSetShmemLabel, + .domainRestoreSecurityShmemLabel = virSecurityDACRestoreShmemLabel, }; diff --git a/src/security/security_driver.h b/src/security/security_driver.h index f0dca09..37e4527 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -118,6 +118,14 @@ typedef int (*virSecurityDomainSetImageLabel) (virSecurityManagerPtr mgr, typedef int (*virSecurityDomainRestoreImageLabel) (virSecurityManagerPtr mgr, virDomainDefPtr def, virStorageSourcePtr src); +typedef int (*virSecurityDomainSetShmemLabel) (virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainShmemDefPtr shmem, + char *path); +typedef int (*virSecurityDomainRestoreShmemLabel) (virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainShmemDefPtr shmem, + char *path); struct _virSecurityDriver { @@ -168,6 +176,9 @@ struct _virSecurityDriver { virSecurityDomainSetHugepages domainSetSecurityHugepages; virSecurityDriverGetBaseLabel getBaseLabel; + + virSecurityDomainSetShmemLabel domainSetSecurityShmemLabel; + virSecurityDomainRestoreShmemLabel domainRestoreSecurityShmemLabel; }; virSecurityDriverPtr virSecurityDriverLookup(const char *name, diff --git a/src/security/security_manager.c b/src/security/security_manager.c index b0cd9e8..72ca7e2 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -991,3 +991,41 @@ virSecurityManagerSetHugepages(virSecurityManagerPtr mgr, return 0; } + + +int +virSecurityManagerRestoreShmemLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + virDomainShmemDefPtr shmem, + char *path) +{ + if (mgr->drv->domainRestoreSecurityShmemLabel) { + int ret; + virObjectLock(mgr); + ret = mgr->drv->domainRestoreSecurityShmemLabel(mgr, vm, shmem, path); + virObjectUnlock(mgr); + return ret; + } + + virReportUnsupportedError(); + return -1; +} + + +int +virSecurityManagerSetShmemLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + virDomainShmemDefPtr shmem, + char *path) +{ + if (mgr->drv->domainSetSecurityShmemLabel) { + int ret; + virObjectLock(mgr); + ret = mgr->drv->domainSetSecurityShmemLabel(mgr, vm, shmem, path); + virObjectUnlock(mgr); + return ret; + } + + virReportUnsupportedError(); + return -1; +} diff --git a/src/security/security_manager.h b/src/security/security_manager.h index 13468db..ce37c91 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -149,5 +149,13 @@ int virSecurityManagerSetImageLabel(virSecurityManagerPtr mgr, int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm, virStorageSourcePtr src); +int virSecurityManagerRestoreShmemLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + virDomainShmemDefPtr shmem, + char *path); +int virSecurityManagerSetShmemLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + virDomainShmemDefPtr shmem, + char *path);const path
Okay
#endif /* VIR_SECURITY_MANAGER_H__ */ diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 6e67a86..cbf89ee 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -46,6 +46,7 @@ #include "virconf.h" #include "virtpm.h" #include "virstring.h" +#include "virshm.h"remove that too#define VIR_FROM_THIS VIR_FROM_SECURITY @@ -1888,6 +1889,37 @@ virSecuritySELinuxRestoreSecuritySmartcardCallback(virDomainDefPtr def, } +static int +virSecuritySELinuxRestoreShmemLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainShmemDefPtr shmem, + char *path)const path+{ + char *tmppath = NULL;make it const too+ virSecurityLabelDefPtr seclabel; + virSecurityDeviceLabelDefPtr shmem_seclabel = NULL; + + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (!seclabel || !seclabel->relabel) + return 0; + + shmem_seclabel = virDomainShmemDefGetSecurityLabelDef(shmem, SECURITY_SELINUX_NAME); + + if (shmem_seclabel && !shmem_seclabel->relabel) + return 0; + + if (shmem->server.enabled) + tmppath = shmem->server.chr.data.nix.path; + else + tmppath = path; + + if (!tmppath) + return 0; + + return virSecuritySELinuxRestoreSecurityFileLabel(mgr, tmppath); +} + + static const char * virSecuritySELinuxGetBaseLabel(virSecurityManagerPtr mgr, int virtType) { @@ -2284,6 +2316,41 @@ virSecuritySELinuxSetSecuritySmartcardCallback(virDomainDefPtr def, static int +virSecuritySELinuxSetShmemLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + virDomainShmemDefPtr shmem, + char *path) +{ + virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr); + char *tmppath = NULL; + virSecurityLabelDefPtr seclabel; + virSecurityDeviceLabelDefPtr shmem_seclabel = NULL; + + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (!seclabel || !seclabel->relabel) + return 0; + + shmem_seclabel = virDomainShmemDefGetSecurityLabelDef(shmem, SECURITY_SELINUX_NAME); + + if (shmem_seclabel && !shmem_seclabel->relabel) + return 0; + + if (shmem->server.enabled) + tmppath = shmem->server.chr.data.nix.path; + else + tmppath = path;I am not sure it's a good idea to either set the server socket policy or the shm. Why not set both?
Hmm...these $path are the shm path, if the shmem device is server enabled, the used shm is created by ivshmem-server, which will depends on ivshmem-server's label. And qemu process will connect to the socket created by ivshmem-server, so change the label for shm created by ivshmem-sever is useless (also we don't know the shm name created by ivshmem-server).
However, never mind, i will do a refactor and remove the variable $path here to make this function easy to put in virSecuritySELinuxSetSecurityAllLabel and virSecuritySELinuxSetSecurityAllLabel.
+ if (!tmppath) + return 0; + + if (shmem_seclabel && shmem_seclabel->label) + return virSecuritySELinuxSetFilecon(tmppath, shmem_seclabel->label); + else + return virSecuritySELinuxSetFilecon(tmppath, data->file_context); +} + + +static int virSecuritySELinuxSetSecurityAllLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, const char *stdin_path) @@ -2549,4 +2616,7 @@ virSecurityDriver virSecurityDriverSELinux = { .domainGetSecurityMountOptions = virSecuritySELinuxGetSecurityMountOptions, .getBaseLabel = virSecuritySELinuxGetBaseLabel, + + .domainSetSecurityShmemLabel = virSecuritySELinuxSetShmemLabel, + .domainRestoreSecurityShmemLabel = virSecuritySELinuxRestoreShmemLabel, }; diff --git a/src/security/security_stack.c b/src/security/security_stack.c index 1ded57b..22c1b56 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -599,6 +599,44 @@ virSecurityStackRestoreSecurityImageLabel(virSecurityManagerPtr mgr, return rc; } +static int +virSecurityStackSetShmemLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + virDomainShmemDefPtr shmem, + char *path) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; + int rc = 0; + + for (; item; item = item->next) { + if (virSecurityManagerSetShmemLabel(item->securityManager, + vm, shmem, path) < 0) + rc = -1; + } + + return rc; +} + +static int +virSecurityStackRestoreShmemLabel(virSecurityManagerPtr mgr, + virDomainDefPtr vm, + virDomainShmemDefPtr shmem, + char *path) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; + int rc = 0; + + for (; item; item = item->next) { + if (virSecurityManagerRestoreShmemLabel(item->securityManager, + vm, shmem, path) < 0) + rc = -1; + } + + return rc; +} + virSecurityDriver virSecurityDriverStack = { .privateDataLen = sizeof(virSecurityStackData), .name = "stack", @@ -648,4 +686,7 @@ virSecurityDriver virSecurityDriverStack = { .domainSetSecurityHugepages = virSecurityStackSetHugepages, .getBaseLabel = virSecurityStackGetBaseLabel, + + .domainSetSecurityShmemLabel = virSecurityStackSetShmemLabel, + .domainRestoreSecurityShmemLabel = virSecurityStackRestoreShmemLabel, }; -- 1.8.3.1Shouldn't it be implemented for the nop virSecurityDriver too? (note: I don't know what it is for)
Good catch, i didn't notice that, i will add it in next version. Luyao
-- 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
