[libvirt] [PATCH 2/4] security: add security part for shmem device
Marc-André Lureau
marcandre.lureau at gmail.com
Mon Jul 27 15:39:30 UTC 2015
Hi
On Thu, Jul 23, 2015 at 12:13 PM, Luyao Huang <lhuang at redhat.com> wrote:
> A new api to help set/restore the shmem deivce dac/selinux label.
typo: deivce / device.
>
> Signed-off-by: Luyao Huang <lhuang at 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)
>
> #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
> + 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.
> + 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
>
> #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?
> + 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.1
Shouldn't it be implemented for the nop virSecurityDriver too? (note:
I don't know what it is for)
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
Marc-André Lureau
7346 2483 9404 4E20 ABFF 7D48 D864 9487 F43F 0992
More information about the libvir-list
mailing list