[libvirt] [PATCH v4 06/11] qemu: Extend QEMU with external TPM support
Stefan Berger
stefanb at linux.vnet.ibm.com
Tue May 15 17:26:38 UTC 2018
On 05/15/2018 09:53 AM, John Ferlan wrote:
>
> 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 at linux.vnet.ibm.com>
>> ---
>> 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?
Fixed.
>
>> + 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 at linux.vnet.ibm.com>
>> + */
>> +
> [...]
>
>> +/*
>> + * 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
Fixed.
>
>> +{
>> + char *path = NULL;
>> +
>> + ignore_value(virAsprintf(&path, "%s/%s/tpm1.2", swtpmStorageDir, vmname));
>> +
>> + return path;
>> +}
>> +
>> +
> [...]
>
>> +/*
>> + * qemuTPMCreateEmulatorStorage
>> + *
>> + * @storagepath: directory for swtpm's pesistent state
> persistent
Fixed.
>
>> + * @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
virFileMakePathWithMode creates all missing directories before. What is
wrong about that call is that it needs to have permissions 0711. If we
don't call it we'll get an error because /var/log/swtpm and
/var/log/swtpm/libvirt are missing.
error: failed to create directory '/var/log/swtpm/libvirt/qemu': No such
file or directory
The rpm does this correctly.
>
>> +
>> + /* 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.
There's some more TPM specific code in the qemuExtTPM* functions and I
thought I'd rather not clutter up the qemu_extdevice.c with the TPM
specifics but keep it at def->tpm checks and push the rest into qemu_tpm.c.
>
>
> 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.
I am following existing pattern when prefixing all functions with 'qemu'
for code in src/qemu/qemu_*.c. qemu_driver.c seems to do the same.
Stefan
More information about the libvir-list
mailing list