On 05/10/2018 05:57 PM, Stefan Berger wrote:
> Implement functions for managing the storage of the external swtpm as well
> as starting and stopping it. Also implement functions to use swtpm_setup,
> which simulates the manufacturing of a TPM, which includes creation of
> certificates for the device.
>
> Further, the external TPM needs storage on the host that we need to set
> up before it can be run. We can clean up the host once the domain is
> undefined.
>
> This patch also implements a small layer for external device support that
> calls into the TPM device layer if a domain has an attached TPM. This is
> the layer we will wire up later on.
>
> Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxxxxxxx>
> ---
> src/qemu/Makefile.inc.am | 4 +
> src/qemu/qemu_domain.c | 2 +
> src/qemu/qemu_driver.c | 5 +
> src/qemu/qemu_extdevice.c | 154 ++++++++++
> src/qemu/qemu_extdevice.h | 53 ++++
> src/qemu/qemu_migration.c | 3 +
> src/qemu/qemu_process.c | 12 +
> src/qemu/qemu_tpm.c | 753 ++++++++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_tpm.h | 50 +++
> 9 files changed, 1036 insertions(+)
> create mode 100644 src/qemu/qemu_extdevice.c
> create mode 100644 src/qemu/qemu_extdevice.h
> create mode 100644 src/qemu/qemu_tpm.c
> create mode 100644 src/qemu/qemu_tpm.h
>
[...]
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 37876b8..9370de3 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -48,6 +48,7 @@
> #include "qemu_migration_params.h"
> #include "qemu_interface.h"
> #include "qemu_security.h"
> +#include "qemu_extdevice.h"
>
> #include "cpu/cpu.h"
> #include "datatypes.h"
> @@ -5872,6 +5873,10 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver,
> if (qemuProcessPrepareHostStorage(driver, vm, flags) < 0)
> goto cleanup;
>
> + VIR_DEBUG("Preparing external devices");
> + if (qemuExtDevicesPrepareHost(driver, vm->def) < 0)
> + goto cleanup;
> +
> ret = 0;
> cleanup:
> virObjectUnref(cfg);
> @@ -5955,6 +5960,9 @@ qemuProcessLaunch(virConnectPtr conn,
> goto cleanup;
> logfile = qemuDomainLogContextGetWriteFD(logCtxt);
>
> + if (qemuExtDevicesStart(driver, vm->def, logCtxt) < 0)
> + goto cleanup;
> +
> VIR_DEBUG("Building emulator command line");
> if (!(cmd = qemuBuildCommandLine(driver,
> qemuDomainLogContextGetManager(logCtxt),
> @@ -6194,6 +6202,8 @@ qemuProcessLaunch(virConnectPtr conn,
> ret = 0;
>
> cleanup:
> + if (ret)
if (ret < 0), right?
> + qemuExtDevicesStop(driver, vm->def);
> qemuDomainSecretDestroy(vm);
> virCommandFree(cmd);
> virObjectUnref(logCtxt);
> @@ -6614,6 +6624,8 @@ void qemuProcessStop(virQEMUDriverPtr driver,
>
> qemuDomainCleanupRun(driver, vm);
>
> + qemuExtDevicesStop(driver, vm->def);
> +
> /* Stop autodestroy in case guest is restarted */
> qemuProcessAutoDestroyRemove(driver, vm);
>
> diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
> new file mode 100644
> index 0000000..024d24d
> --- /dev/null
> +++ b/src/qemu/qemu_tpm.c
> @@ -0,0 +1,753 @@
> +/*
> + * qemu_tpm.c: QEMU TPM support
> + *
> + * Copyright (C) 2018 IBM Corporation
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library. If not, see
> + * <http://www.gnu.org/licenses/>.
> + *
> + * Author: Stefan Berger <stefanb@xxxxxxxxxxxxxxxxxx>
> + */
> +
[...]
> +/*
> + * qemuTPMCreateEmulatorStoragePath
> + *
> + * @swtpmStorageDir: directory for swtpm persistent state
> + * @vmname: The name of the VM for which to create the storage
> + *
> + * Create the swtpm's storage path
> + */
> +static char *
> +qemuTPMCreateEmulatorStoragePath(const char *swtpmStorageDir,
> + const char *vmname)
You changed at some point to use the uuidstr - probably should change
this too. Comment and argument
> +{
> + char *path = NULL;
> +
> + ignore_value(virAsprintf(&path, "%s/%s/tpm1.2", swtpmStorageDir, vmname));
> +
> + return path;
> +}
> +
> +
[...]
> +/*
> + * qemuTPMCreateEmulatorStorage
> + *
> + * @storagepath: directory for swtpm's pesistent state
persistent
> + * @created: a pointer to a bool that will be set to true if the
> + * storage was created because it did not exist yet
> + * @swtpm_user: The uid that needs to be able to access the directory
> + * @swtpm_group: The gid that needs to be able to access the directory
> + *
> + * Unless the storage path for the swtpm for the given VM
> + * already exists, create it and make it accessible for the given userid.
> + * Adapt ownership of the directory and all swtpm's state files there.
> + */
> +static int
> +qemuTPMCreateEmulatorStorage(const char *storagepath,
> + bool *created,
> + uid_t swtpm_user,
> + gid_t swtpm_group)
> +{
> + int ret = -1;
> + char *swtpmStorageDir = qemuTPMGetTPMStorageDir(storagepath);
> +
> + if (!swtpmStorageDir)
> + return -1;
> +
> + if (qemuTPMEmulatorInitStorage(swtpmStorageDir) < 0)
> + goto cleanup;
> +
> + *created = false;
> +
> + if (!virFileExists(storagepath))
> + *created = true;
> +
> + if (virDirCreate(storagepath, 0700, swtpm_user, swtpm_group,
> + VIR_DIR_CREATE_ALLOW_EXIST) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Could not create directory %s as %u:%d"),
> + storagepath, swtpm_user, swtpm_group);
> + goto cleanup;
> + }
> +
> + if (virFileChownFiles(storagepath, swtpm_user, swtpm_group) < 0)
> + goto cleanup;
> +
> + ret = 0;
> +
> + cleanup:
> + VIR_FREE(swtpmStorageDir);
> +
> + return ret;
> +}
> +
> +
[...]
> +/*
> + * qemuTPMEmulatorPrepareHost:
> + *
> + * @tpm: tpm definition
> + * @logDir: directory where swtpm writes its logs into
> + * @vmname: name of the VM
> + * @swtpm_user: uid to run the swtpm with
> + * @swtpm_group: gid to run the swtpm with
> + * @swtpmStateDir: directory for swtpm's persistent state
> + * @qemu_user: uid that qemu will run with; we share the socket file with it
> + * @shortName: short and unique name of the domain
> + *
> + * Prepare the log directory for the swtpm and adjust ownership of it and the
> + * log file we will be using. Prepare the state directory where we will share
> + * the socket between tss and qemu users.
> + */
> +static int
> +qemuTPMEmulatorPrepareHost(virDomainTPMDefPtr tpm,
> + const char *logDir,
> + const char *vmname,
> + uid_t swtpm_user,
> + gid_t swtpm_group,
> + const char *swtpmStateDir,
> + uid_t qemu_user,
> + const char *shortName)
> +{
> + int ret = -1;
> +
> + if (qemuTPMEmulatorInit() < 0)
> + return -1;
> +
> + /* create log dir ... */
> + if (virFileMakePathWithMode(logDir, 0730) < 0)
> + return -1;
> +
> + /* ... and adjust ownership */
> + if (virDirCreate(logDir, 0730, swtpm_user, swtpm_group,
> + VIR_DIR_CREATE_ALLOW_EXIST) < 0)
> + goto cleanup;
Do we really need both? IOW: Why not just virDirCreate
> +
> + /* create logfile name ... */
> + if (!tpm->data.emulator.logfile &&
> + virAsprintf(&tpm->data.emulator.logfile, "%s/%s-swtpm.log",
> + logDir, vmname) < 0)
> + goto cleanup;
> +
> + /* ... and make sure it can be accessed by swtpm_user */
> + if (virFileExists(tpm->data.emulator.logfile) &&
> + chown(tpm->data.emulator.logfile, swtpm_user, swtpm_group) < 0) {
> + virReportSystemError(errno,
> + _("Could not chown on swtpm logfile %s"),
> + tpm->data.emulator.logfile);
> + goto cleanup;
> + }
> +
> + /*
> + create our swtpm state dir ...
> + - QEMU user needs to be able to access the socket there
> + - swtpm group needs to be able to create files there
> + - in privileged mode 0570 would be enough, for non-privileged mode
> + we need 0770
> + */
> + if (virDirCreate(swtpmStateDir, 0770, qemu_user, swtpm_group,
> + VIR_DIR_CREATE_ALLOW_EXIST) < 0)
> + goto cleanup;
> +
> + /* create the socket filename */
> + if (!tpm->data.emulator.source.data.nix.path &&
> + !(tpm->data.emulator.source.data.nix.path =
> + qemuTPMCreateEmulatorSocket(swtpmStateDir, shortName)))
> + goto cleanup;
> + tpm->data.emulator.source.type = VIR_DOMAIN_CHR_TYPE_UNIX;
> +
> + ret = 0;
> +
> + cleanup:
> +
> + return ret;
> +}
[...]
> +int
> +qemuExtTPMInitPaths(virQEMUDriverPtr driver,
> + virDomainDefPtr def)
When I see qemuExt - I generally expect the code to be in
qemu_extdevice.c... Similar for anything qemuExtTPM. I understand why
the Ext is there (e.g. external), but not sure it's necessary.
FWIW: Although I didn't win my argument when last discussed, my feeling
is static functions should be something like "tpmEmulatorInitPaths";
whereas, external functions should be qemuTPMEmulatorInitPaths.
John
> +{
> + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> + int ret = 0;
> +
> + switch (def->tpm->type) {
> + case VIR_DOMAIN_TPM_TYPE_EMULATOR:
> + ret = qemuTPMEmulatorInitPaths(def->tpm, cfg->swtpmStorageDir,
> + def->uuid);
> + break;
> + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
> + case VIR_DOMAIN_TPM_TYPE_LAST:
> + break;
> + }
> +
> + virObjectUnref(cfg);
> +
> + return ret;
> +}
> +
> +
[...]
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
