On 05/04/2018 04:21 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.
>
> The file and process labels now look as follows:
>
> Directory: /var/lib/libvirt/swtpm
>
> [root@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@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@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@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 [..]
>
> Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxxxxxxx>
> ---
> src/libvirt_private.syms | 1 +
> src/qemu/qemu_extdevice.c | 22 ++++++++++-
> src/security/security_driver.h | 4 ++
> src/security/security_manager.c | 17 +++++++++
> src/security/security_manager.h | 3 ++
> src/security/security_selinux.c | 82 +++++++++++++++++++++++++++++++++++++++++
> src/security/security_stack.c | 19 ++++++++++
> 7 files changed, 147 insertions(+), 1 deletion(-)
>
I think this looks OK - not my specialty 0-) though. I see
security_manager, selinux, etc. and my eyes start glazing over!
Anyway, I assume the reason there's no Restore processing is because
everything is deleted at shutdown, right?
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index e533b95..79b8afa 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1334,6 +1334,7 @@ virSecurityManagerSetProcessLabel;
> virSecurityManagerSetSavedStateLabel;
> virSecurityManagerSetSocketLabel;
> virSecurityManagerSetTapFDLabel;
> +virSecurityManagerSetTPMLabels;
> virSecurityManagerStackAddNested;
> virSecurityManagerTransactionAbort;
> virSecurityManagerTransactionCommit;
> diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
> index f3f337d..eb7220d 100644
> --- a/src/qemu/qemu_extdevice.c
> +++ b/src/qemu/qemu_extdevice.c
> @@ -166,12 +166,32 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
>
> virCommandSetErrorBuffer(cmd, &errbuf);
>
> - if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
> + if (virSecurityManagerSetTPMLabels(driver->securityManager,
> + def) < 0)
> + goto error;
> +
> + if (virSecurityManagerSetChildProcessLabel(driver->securityManager,
> + def, cmd) < 0)
> + goto error;
> +
> + if (virSecurityManagerPreFork(driver->securityManager) < 0)
> + goto error;
> +
> + /* 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\n"
> "stderr: %s\n", exitstatus, errbuf);
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Could not start 'swtpm'. exitstatus: %d, "
> "error: %s"), exitstatus, errbuf);
> + ret = -1;
> goto error;
> }
>
> diff --git a/src/security/security_driver.h b/src/security/security_driver.h
> index 95e7c4d..4aa415f 100644
> --- a/src/security/security_driver.h
> +++ b/src/security/security_driver.h
> @@ -149,6 +149,8 @@ typedef int (*virSecurityDomainRestoreChardevLabel) (virSecurityManagerPtr mgr,
> virDomainDefPtr def,
> virDomainChrSourceDefPtr dev_source,
> bool chardevStdioLogd);
> +typedef int (*virSecurityDomainSetTPMLabels) (virSecurityManagerPtr mgr,
> + virDomainDefPtr def);
>
>
> struct _virSecurityDriver {
> @@ -213,6 +215,8 @@ struct _virSecurityDriver {
>
> virSecurityDomainSetChardevLabel domainSetSecurityChardevLabel;
> virSecurityDomainRestoreChardevLabel domainRestoreSecurityChardevLabel;
> +
> + virSecurityDomainSetTPMLabels domainSetSecurityTPMLabels;
> };
>
> virSecurityDriverPtr virSecurityDriverLookup(const char *name,
> diff --git a/src/security/security_manager.c b/src/security/security_manager.c
> index 71f7f59..48777bb 100644
> --- a/src/security/security_manager.c
> +++ b/src/security/security_manager.c
> @@ -1204,3 +1204,20 @@ virSecurityManagerRestoreChardevLabel(virSecurityManagerPtr mgr,
> virReportUnsupportedError();
> return -1;
> }
> +
> +
> +int virSecurityManagerSetTPMLabels(virSecurityManagerPtr mgr,
> + virDomainDefPtr vm)
int
virSecurity...
> +{
> + int ret;
> +
> + if (mgr->drv->domainSetSecurityTPMLabels) {
> + virObjectLock(mgr);
> + ret = mgr->drv->domainSetSecurityTPMLabels(mgr, vm);
> + virObjectUnlock(mgr);
> +
> + return ret;
> + }
> +
> + return 0;
> +}
> diff --git a/src/security/security_manager.h b/src/security/security_manager.h
> index c36a8b4..671f6a8 100644
> --- a/src/security/security_manager.h
> +++ b/src/security/security_manager.h
> @@ -194,4 +194,7 @@ int virSecurityManagerRestoreChardevLabel(virSecurityManagerPtr mgr,
> virDomainChrSourceDefPtr dev_source,
> bool chardevStdioLogd);
>
> +int virSecurityManagerSetTPMLabels(virSecurityManagerPtr mgr,
> + virDomainDefPtr vm);
> +
> #endif /* VIR_SECURITY_MANAGER_H__ */
> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index 17bc07a..42a940b 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -3047,6 +3047,86 @@ virSecuritySELinuxDomainSetPathLabel(virSecurityManagerPtr mgr,
> return virSecuritySELinuxSetFilecon(mgr, path, seclabel->imagelabel);
> }
>
2 blank lines
> +/*
> + * _virSecuritySELinuxSetSecurityFileLabels:
> + *
> + * @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
> +_virSecuritySELinuxSetSecurityFileLabels(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 (virDirOpen(&dir, path) < 0)
> + return 0;
> +
> + 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)
if (ret < 0)
Things look reasonable to my untrained eyes - on the next series -
hopefully someone else can poke into this particular patch as well.
John
> + break;
> + }
> + if (ret)
> + virReportSystemError(errno, _("Unable to label files under %s"),
> + path);
> +
> + virDirClose(&dir);
> +
> + return ret;
> +}
> +
> +static int
> +virSecuritySELinuxSetSecurityTPMLabels(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 = _virSecuritySELinuxSetSecurityFileLabels(
> + mgr, def->tpm->data.emulator.storagepath,
> + seclabel);
> + if (ret == 0 && def->tpm->data.emulator.logfile)
> + ret = _virSecuritySELinuxSetSecurityFileLabels(
> + mgr, def->tpm->data.emulator.logfile,
> + seclabel);
> + break;
> + case VIR_DOMAIN_TPM_TYPE_LAST:
> + break;
> + }
> +
> + return ret;
> +}
> +
> virSecurityDriver virSecurityDriverSELinux = {
> .privateDataLen = sizeof(virSecuritySELinuxData),
> .name = SECURITY_SELINUX_NAME,
> @@ -3106,4 +3186,6 @@ virSecurityDriver virSecurityDriverSELinux = {
>
> .domainSetSecurityChardevLabel = virSecuritySELinuxSetChardevLabel,
> .domainRestoreSecurityChardevLabel = virSecuritySELinuxRestoreChardevLabel,
> +
> + .domainSetSecurityTPMLabels = virSecuritySELinuxSetSecurityTPMLabels,
> };
> diff --git a/src/security/security_stack.c b/src/security/security_stack.c
> index 9615f9f..7f10ef0 100644
> --- a/src/security/security_stack.c
> +++ b/src/security/security_stack.c
> @@ -760,6 +760,23 @@ virSecurityStackDomainRestoreChardevLabel(virSecurityManagerPtr mgr,
> return rc;
> }
>
> +static int
> +virSecurityStackSetSecurityTPMLabels(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;
> +}
> +
> virSecurityDriver virSecurityDriverStack = {
> .privateDataLen = sizeof(virSecurityStackData),
> .name = "stack",
> @@ -822,4 +839,6 @@ virSecurityDriver virSecurityDriverStack = {
>
> .domainSetSecurityChardevLabel = virSecurityStackDomainSetChardevLabel,
> .domainRestoreSecurityChardevLabel = virSecurityStackDomainRestoreChardevLabel,
> +
> + .domainSetSecurityTPMLabels = virSecurityStackSetSecurityTPMLabels,
> };
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
