Re: [PATCH v4 06/11] qemu: Extend QEMU with external TPM support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux