[libvirt] [PATCH v4 09/11] security: Label the external swtpm with SELinux labels
Stefan Berger
stefanb at linux.vnet.ibm.com
Tue May 15 18:13:41 UTC 2018
On 05/10/2018 05:57 PM, Stefan Berger wrote:
> In this patch we label the swtpm process with SELinux labels. We give it the
> same label as the QEMU process has. We label its state directory and files
> as well. We restore the old security labels once the swtpm has terminated.
>
> The file and process labels now look as follows:
>
> Directory: /var/lib/libvirt/swtpm
>
> [root at localhost swtpm]# ls -lZ
> total 4
> rwx------. 2 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 4096 Apr 5 16:46 testvm
>
> [root at localhost testvm]# ls -lZ
> total 8
> -rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 3648 Apr 5 16:46 tpm-00.permall
>
> The log in /var/log/swtpm/libvirt/qemu is labeled as follows:
>
> -rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 2237 Apr 5 16:46 vtpm.log
>
> [root at localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep swtpm | grep ctrl | grep -v grep
> system_u:system_r:svirt_t:s0:c254,c932 tss 25664 0.0 0.0 28172 3892 ? Ss 16:57 0:00 /usr/bin/swtpm socket --daemon --ctrl type=unixio,path=/var/run/libvirt/qemu/swtpm/testvm-swtpm.sock,mode=0660 --tpmstate dir=/var/lib/libvirt/swtpm/testvm/tpm1.2 --log file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log
>
> [root at localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep qemu | grep tpm | grep -v grep
> system_u:system_r:svirt_t:s0:c254,c932 qemu 25669 99.0 0.0 3096704 48500 ? Sl 16:57 3:28 /bin/qemu-system-x86_64 [..]
An outstanding issue with this patch are the additional svirt rules that
are needed. A challenge here is to find that subset of rules that are
really needed.
One of my machines with Fedora 27 needs only this one rule (following
permissive mode and audit2allow):
allow svirt_t swtpm_exec_t:file map;
The other FC27 machine needs these:
allow svirt_t bin_t:file entrypoint;
allow svirt_t swtpm_exec_t:file { entrypoint execute map read };
I have seen it needing several more rules than these.
I suppose it won't be a problem to get them accepted, assuming there
will be an swtpm_exec_t...
Stefan
>
> Signed-off-by: Stefan Berger <stefanb at linux.vnet.ibm.com>
> ---
> src/libvirt_private.syms | 2 +
> src/qemu/qemu_tpm.c | 24 +++++-
> src/security/security_driver.h | 7 ++
> src/security/security_manager.c | 36 +++++++++
> src/security/security_manager.h | 6 ++
> src/security/security_selinux.c | 164 ++++++++++++++++++++++++++++++++++++++++
> src/security/security_stack.c | 40 ++++++++++
> 7 files changed, 278 insertions(+), 1 deletion(-)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 75b8932..2ce67e7 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1319,6 +1319,7 @@ virSecurityManagerRestoreImageLabel;
> virSecurityManagerRestoreInputLabel;
> virSecurityManagerRestoreMemoryLabel;
> virSecurityManagerRestoreSavedStateLabel;
> +virSecurityManagerRestoreTPMLabels;
> virSecurityManagerSetAllLabel;
> virSecurityManagerSetChardevLabel;
> virSecurityManagerSetChildProcessLabel;
> @@ -1333,6 +1334,7 @@ virSecurityManagerSetProcessLabel;
> virSecurityManagerSetSavedStateLabel;
> virSecurityManagerSetSocketLabel;
> virSecurityManagerSetTapFDLabel;
> +virSecurityManagerSetTPMLabels;
> virSecurityManagerStackAddNested;
> virSecurityManagerTransactionAbort;
> virSecurityManagerTransactionCommit;
> diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
> index 024d24d..62f0146 100644
> --- a/src/qemu/qemu_tpm.c
> +++ b/src/qemu/qemu_tpm.c
> @@ -684,7 +684,26 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
>
> virCommandSetErrorBuffer(cmd, &errbuf);
>
> - if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
> + if (virSecurityManagerSetTPMLabels(driver->securityManager,
> + def) < 0)
> + goto cleanup;
> +
> + if (virSecurityManagerSetChildProcessLabel(driver->securityManager,
> + def, cmd) < 0)
> + goto cleanup;
> +
> + if (virSecurityManagerPreFork(driver->securityManager) < 0)
> + goto cleanup;
> +
> + /* make sure we run this with the appropriate user */
> + virCommandSetUID(cmd, cfg->swtpm_user);
> + virCommandSetGID(cmd, cfg->swtpm_group);
> +
> + ret = virCommandRun(cmd, &exitstatus);
> +
> + virSecurityManagerPostFork(driver->securityManager);
> +
> + if (ret < 0 || exitstatus != 0) {
> VIR_ERROR(_("Could not start 'swtpm'. exitstatus: %d "
> "stderr: %s"), exitstatus, errbuf);
> virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -696,6 +715,8 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
> ret = 0;
>
> cleanup:
> + if (ret < 0)
> + virSecurityManagerRestoreTPMLabels(driver->securityManager, def);
> VIR_FREE(shortName);
> VIR_FREE(errbuf);
> virCommandFree(cmd);
> @@ -741,6 +762,7 @@ qemuExtTPMStop(virQEMUDriverPtr driver,
> goto cleanup;
>
> qemuTPMEmulatorStop(cfg->swtpmStateDir, shortName);
> + virSecurityManagerRestoreTPMLabels(driver->securityManager, def);
> break;
> case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
> case VIR_DOMAIN_TPM_TYPE_LAST:
> diff --git a/src/security/security_driver.h b/src/security/security_driver.h
> index 95e7c4d..cbf0ecf 100644
> --- a/src/security/security_driver.h
> +++ b/src/security/security_driver.h
> @@ -149,6 +149,10 @@ typedef int (*virSecurityDomainRestoreChardevLabel) (virSecurityManagerPtr mgr,
> virDomainDefPtr def,
> virDomainChrSourceDefPtr dev_source,
> bool chardevStdioLogd);
> +typedef int (*virSecurityDomainSetTPMLabels) (virSecurityManagerPtr mgr,
> + virDomainDefPtr def);
> +typedef int (*virSecurityDomainRestoreTPMLabels) (virSecurityManagerPtr mgr,
> + virDomainDefPtr def);
>
>
> struct _virSecurityDriver {
> @@ -213,6 +217,9 @@ struct _virSecurityDriver {
>
> virSecurityDomainSetChardevLabel domainSetSecurityChardevLabel;
> virSecurityDomainRestoreChardevLabel domainRestoreSecurityChardevLabel;
> +
> + virSecurityDomainSetTPMLabels domainSetSecurityTPMLabels;
> + virSecurityDomainRestoreTPMLabels domainRestoreSecurityTPMLabels;
> };
>
> virSecurityDriverPtr virSecurityDriverLookup(const char *name,
> diff --git a/src/security/security_manager.c b/src/security/security_manager.c
> index 71f7f59..8683ad7 100644
> --- a/src/security/security_manager.c
> +++ b/src/security/security_manager.c
> @@ -1204,3 +1204,39 @@ virSecurityManagerRestoreChardevLabel(virSecurityManagerPtr mgr,
> virReportUnsupportedError();
> return -1;
> }
> +
> +
> +int
> +virSecurityManagerSetTPMLabels(virSecurityManagerPtr mgr,
> + virDomainDefPtr vm)
> +{
> + int ret;
> +
> + if (mgr->drv->domainSetSecurityTPMLabels) {
> + virObjectLock(mgr);
> + ret = mgr->drv->domainSetSecurityTPMLabels(mgr, vm);
> + virObjectUnlock(mgr);
> +
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +
> +int
> +virSecurityManagerRestoreTPMLabels(virSecurityManagerPtr mgr,
> + virDomainDefPtr vm)
> +{
> + int ret;
> +
> + if (mgr->drv->domainRestoreSecurityTPMLabels) {
> + virObjectLock(mgr);
> + ret = mgr->drv->domainRestoreSecurityTPMLabels(mgr, vm);
> + virObjectUnlock(mgr);
> +
> + return ret;
> + }
> +
> + return 0;
> +}
> diff --git a/src/security/security_manager.h b/src/security/security_manager.h
> index c36a8b4..e772b61 100644
> --- a/src/security/security_manager.h
> +++ b/src/security/security_manager.h
> @@ -194,4 +194,10 @@ int virSecurityManagerRestoreChardevLabel(virSecurityManagerPtr mgr,
> virDomainChrSourceDefPtr dev_source,
> bool chardevStdioLogd);
>
> +int virSecurityManagerSetTPMLabels(virSecurityManagerPtr mgr,
> + virDomainDefPtr vm);
> +
> +int virSecurityManagerRestoreTPMLabels(virSecurityManagerPtr mgr,
> + virDomainDefPtr vm);
> +
> #endif /* VIR_SECURITY_MANAGER_H__ */
> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index 92e8415..6377fb7 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -3048,6 +3048,167 @@ virSecuritySELinuxDomainSetPathLabel(virSecurityManagerPtr mgr,
> return virSecuritySELinuxSetFilecon(mgr, path, seclabel->imagelabel);
> }
>
> +
> +/*
> + * _virSecuritySELinuxSetFileLabels:
> + *
> + * @mgr: the virSecurityManager
> + * @path: path to a directory or a file
> + * @seclabel: the security label
> + *
> + * Set the file labels on the given path; if the path is a directory
> + * we label all files found there, including the directory itself,
> + * otherwise we just label the file.
> + */
> +static int
> +_virSecuritySELinuxSetFileLabels(virSecurityManagerPtr mgr,
> + const char *path,
> + virSecurityLabelDefPtr seclabel)
> +{
> + int ret = 0;
> + struct dirent *ent;
> + char *filename = NULL;
> + DIR *dir;
> +
> + if ((ret = virSecuritySELinuxSetFilecon(mgr, path, seclabel->imagelabel)))
> + return ret;
> +
> + if (!virFileIsDir(path))
> + return 0;
> +
> + if (virDirOpen(&dir, path) < 0)
> + return -1;
> +
> + while ((ret = virDirRead(dir, &ent, path)) > 0) {
> + if (ent->d_type != DT_REG)
> + continue;
> +
> + if (virAsprintf(&filename, "%s/%s", path, ent->d_name) < 0) {
> + ret = -1;
> + break;
> + }
> + ret = virSecuritySELinuxSetFilecon(mgr, filename,
> + seclabel->imagelabel);
> + VIR_FREE(filename);
> + if (ret < 0)
> + break;
> + }
> + if (ret < 0)
> + virReportSystemError(errno, _("Unable to label files under %s"),
> + path);
> +
> + virDirClose(&dir);
> +
> + return ret;
> +}
> +
> +
> +/*
> + * _virSecuritySELinuxRestoreFileLabels:
> + *
> + * @mgr: the virSecurityManager
> + * @path: path to a directory or a file
> + *
> + * Restore the file labels on the given path; if the path is a directory
> + * we restore all file labels found there, including the label of the
> + * directory itself, otherwise we just restore the label on the file.
> + */
> +static int
> +_virSecuritySELinuxRestoreFileLabels(virSecurityManagerPtr mgr,
> + const char *path)
> +{
> + int ret = 0;
> + struct dirent *ent;
> + char *filename = NULL;
> + DIR *dir;
> +
> + if ((ret = virSecuritySELinuxRestoreFileLabel(mgr, path)))
> + return ret;
> +
> + if (!virFileIsDir(path))
> + return 0;
> +
> + if (virDirOpen(&dir, path) < 0)
> + return -1;
> +
> + while ((ret = virDirRead(dir, &ent, path)) > 0) {
> + if (ent->d_type != DT_REG)
> + continue;
> +
> + if (virAsprintf(&filename, "%s/%s", path, ent->d_name) < 0) {
> + ret = -1;
> + break;
> + }
> + ret = virSecuritySELinuxRestoreFileLabel(mgr, filename);
> + VIR_FREE(filename);
> + if (ret < 0)
> + break;
> + }
> + if (ret < 0)
> + virReportSystemError(errno, _("Unable to restore file labels under %s"),
> + path);
> +
> + virDirClose(&dir);
> +
> + return ret;
> +}
> +
> +
> +static int
> +virSecuritySELinuxSetTPMLabels(virSecurityManagerPtr mgr,
> + virDomainDefPtr def)
> +{
> + int ret = 0;
> + virSecurityLabelDefPtr seclabel;
> +
> + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
> + if (seclabel == NULL)
> + return 0;
> +
> + switch (def->tpm->type) {
> + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
> + break;
> + case VIR_DOMAIN_TPM_TYPE_EMULATOR:
> + ret = _virSecuritySELinuxSetFileLabels(
> + mgr, def->tpm->data.emulator.storagepath,
> + seclabel);
> + if (ret == 0 && def->tpm->data.emulator.logfile)
> + ret = _virSecuritySELinuxSetFileLabels(
> + mgr, def->tpm->data.emulator.logfile,
> + seclabel);
> + break;
> + case VIR_DOMAIN_TPM_TYPE_LAST:
> + break;
> + }
> +
> + return ret;
> +}
> +
> +
> +static int
> +virSecuritySELinuxRestoreTPMLabels(virSecurityManagerPtr mgr,
> + virDomainDefPtr def)
> +{
> + int ret = 0;
> +
> + switch (def->tpm->type) {
> + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
> + break;
> + case VIR_DOMAIN_TPM_TYPE_EMULATOR:
> + ret = _virSecuritySELinuxRestoreFileLabels(
> + mgr, def->tpm->data.emulator.storagepath);
> + if (ret == 0 && def->tpm->data.emulator.logfile)
> + ret = _virSecuritySELinuxRestoreFileLabels(
> + mgr, def->tpm->data.emulator.logfile);
> + break;
> + case VIR_DOMAIN_TPM_TYPE_LAST:
> + break;
> + }
> +
> + return ret;
> +}
> +
> +
> virSecurityDriver virSecurityDriverSELinux = {
> .privateDataLen = sizeof(virSecuritySELinuxData),
> .name = SECURITY_SELINUX_NAME,
> @@ -3107,4 +3268,7 @@ virSecurityDriver virSecurityDriverSELinux = {
>
> .domainSetSecurityChardevLabel = virSecuritySELinuxSetChardevLabel,
> .domainRestoreSecurityChardevLabel = virSecuritySELinuxRestoreChardevLabel,
> +
> + .domainSetSecurityTPMLabels = virSecuritySELinuxSetTPMLabels,
> + .domainRestoreSecurityTPMLabels = virSecuritySELinuxRestoreTPMLabels,
> };
> diff --git a/src/security/security_stack.c b/src/security/security_stack.c
> index 9615f9f..e37a681 100644
> --- a/src/security/security_stack.c
> +++ b/src/security/security_stack.c
> @@ -760,6 +760,43 @@ virSecurityStackDomainRestoreChardevLabel(virSecurityManagerPtr mgr,
> return rc;
> }
>
> +
> +static int
> +virSecurityStackSetTPMLabels(virSecurityManagerPtr mgr,
> + virDomainDefPtr vm)
> +{
> + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr);
> + virSecurityStackItemPtr item = priv->itemsHead;
> + int rc = 0;
> +
> + for (; item; item = item->next) {
> + if (virSecurityManagerSetTPMLabels(item->securityManager,
> + vm) < 0)
> + rc = -1;
> + }
> +
> + return rc;
> +}
> +
> +
> +static int
> +virSecurityStackRestoreTPMLabels(virSecurityManagerPtr mgr,
> + virDomainDefPtr vm)
> +{
> + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr);
> + virSecurityStackItemPtr item = priv->itemsHead;
> + int rc = 0;
> +
> + for (; item; item = item->next) {
> + if (virSecurityManagerRestoreTPMLabels(item->securityManager,
> + vm) < 0)
> + rc = -1;
> + }
> +
> + return rc;
> +}
> +
> +
> virSecurityDriver virSecurityDriverStack = {
> .privateDataLen = sizeof(virSecurityStackData),
> .name = "stack",
> @@ -822,4 +859,7 @@ virSecurityDriver virSecurityDriverStack = {
>
> .domainSetSecurityChardevLabel = virSecurityStackDomainSetChardevLabel,
> .domainRestoreSecurityChardevLabel = virSecurityStackDomainRestoreChardevLabel,
> +
> + .domainSetSecurityTPMLabels = virSecurityStackSetTPMLabels,
> + .domainRestoreSecurityTPMLabels = virSecurityStackRestoreTPMLabels,
> };
More information about the libvir-list
mailing list