[libvirt] [PATCH v3 10/14] qemu: Add support for external swtpm TPM emulator
Stefan Berger
stefanb at linux.vnet.ibm.com
Wed May 9 20:11:18 UTC 2018
On 05/08/2018 05:07 PM, John Ferlan wrote:
>
> On 05/04/2018 04:21 PM, Stefan Berger wrote:
>> This patch adds support for an external swtpm TPM emulator. The XML for
>> this type of TPM looks as follows:
>>
>> <tpm model='tpm-tis'>
>> <backend type='emulator'/>
>> </tpm>
>>
>> The XML will currently only start a TPM 1.2.
>>
>> Upon first start, libvirt will run `swtpm_setup`, which will simulate the
>> manufacturing of a TPM and create certificates for it and write them into
>> NVRAM locations of the emulated TPM.
>>
>> After that libvirt starts the swtpm TPM emulator using the `swtpm` executable.
>>
>> Once the VM terminates, libvirt uses the swtpm_ioctl executable to gracefully
>> shut down the `swtpm` in case it is still running (QEMU did not send shutdown)
>> or clean up the socket file.
>>
>> The above mentioned executables must be found in the PATH.
>>
>> The executables can either be run as root or started as root and switch to
>> the tss user. The requirement for the tss user comes through 'tcsd', which
>> is used for the simulation of the manufacturing. Which user is used can be
>> configured through qemu.conf. By default 'tss' is used.
>>
>> The swtpm writes out state into files. The state is kept in /var/lib/libvirt/swtpm:
>>
>> [root at localhost libvirt]# ls -lZ | grep swtpm
>>
>> drwx--x--x. 7 root root unconfined_u:object_r:virt_var_lib_t:s0 4096 Apr 5 16:22 swtpm
>>
>> The directory /var/lib/libvirt/swtpm maintains per-TPM state directories.
>> (Using the uuid of the VM for that since the name can change per VM renaming but
>> we need a stable directory name.)
>>
>> [root at localhost swtpm]# ls -lZ
>> total 4
>> drwx------. 2 tss tss system_u:object_r:virt_var_lib_t:s0 4096 Apr 5 16:46 485d0004-a48f-436a-8457-8a3b73e28568
>>
>> [root at localhost 485d0004-a48f-436a-8457-8a3b73e28568]# ls -lZ
>> total 4
>> drwx------. 2 tss tss system_u:object_r:virt_var_lib_t:s0 4096 Apr 10 21:34 tpm1.2
>>
>> [root at localhost tpm1.2]# ls -lZ
>> total 8
>> -rw-r--r--. 1 tss tss system_u:object_r:virt_var_lib_t:s0 3648 Apr 5 16:46 tpm-00.permall
>>
>> The directory /var/run/libvirt/qemu/swtpm/ hosts the swtpm.sock that
>> QEMU uses to communicate with the swtpm:
>>
>> root at localhost domain-1-testvm]# ls -lZ
>> total 0
>> srw-------. 1 qemu qemu system_u:object_r:svirt_image_t:s0:c597,c632 0 Apr 6 10:24 1-testvm-swtpm.sock
>>
>> The logfile for the swtpm is in /var/log/swtpm/libvirt/qemu:
>>
>> [root at localhost-3 qemu]# ls -lZ
>> total 4
>> -rw-------. 1 tss tss unconfined_u:object_r:var_log_t:s0 2199 Apr 6 14:01 testvm-swtpm.log
>>
>> The processes are labeled as follows:
>>
>> [root at localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep swtpm | grep socket | grep -v grep
>> system_u:system_r:virtd_t:s0-s0:c0.c1023 tss 18697 0.0 0.0 28172 3892 ? Ss 16:46 0:00 /usr/bin/swtpm socket --daemon --ctrl type=unixio,path=/var/run/libvirt/qemu/swtpm/1-testvm-swtpm.sock,mode=0600 --tpmstate dir=/var/lib/libvirt/swtpm/485d0004-a48f-436a-8457-8a3b73e28568/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:c413,c430 qemu 18702 2.5 0.0 3036052 48676 ? Sl 16:46 0:08 /bin/qemu-system-x86_64 [...]
>>
>> Signed-off-by: Stefan Berger <stefanb at linux.vnet.ibm.com>
>> ---
>> src/conf/domain_conf.c | 22 ++++++++++++++++++++++
>> src/libvirt_private.syms | 1 +
>> src/qemu/qemu_command.c | 39 +++++++++++++++++++++++++++++++++------
>> src/qemu/qemu_domain.c | 3 +++
>> src/qemu/qemu_driver.c | 7 +++++++
>> 5 files changed, 66 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index d9945dd..a42574a 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -2593,6 +2593,24 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def)
>> }
>> }
>>
> 2 blank lines and
> void
> virDomainTPMDelete(virDomainDefPtr def)
>
>> +void virDomainTPMDelete(virDomainDefPtr def)
>> +{
>> + virDomainTPMDefPtr tpm = def->tpm;
>> +
>> + if (!tpm)
>> + return;
>> +
>> + switch (tpm->type) {
>> + case VIR_DOMAIN_TPM_TYPE_EMULATOR:
>> + virTPMDeleteEmulatorStorage(tpm);
>> + break;
>> + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
>> + case VIR_DOMAIN_TPM_TYPE_LAST:
>> + /* nothing to do */
>> + break;
>> + }
>> +}
>> +
>> void virDomainTPMDefFree(virDomainTPMDefPtr def)
>> {
>> if (!def)
>> @@ -27614,6 +27632,10 @@ virDomainDeleteConfig(const char *configDir,
>> goto cleanup;
>> }
>>
>> + /* in case domain is NOT running, remove any TPM storage */
>> + if (!dom->persistent)
>> + virDomainTPMDelete(dom->def);
>> +
>> ret = 0;
>>
>> cleanup:
>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>> index eebfc72..e533b95 100644
>> --- a/src/libvirt_private.syms
>> +++ b/src/libvirt_private.syms
>> @@ -559,6 +559,7 @@ virDomainTimerTrackTypeToString;
>> virDomainTPMBackendTypeFromString;
>> virDomainTPMBackendTypeToString;
>> virDomainTPMDefFree;
>> +virDomainTPMDelete;
>> virDomainTPMModelTypeFromString;
>> virDomainTPMModelTypeToString;
>> virDomainUSBDeviceDefForeach;
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index bb330bf..c02b783 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -9425,21 +9425,31 @@ qemuBuildTPMDevStr(const virDomainDef *def,
>>
>>
>> static char *
>> -qemuBuildTPMBackendStr(const virDomainDef *def,
>> +qemuBuildTPMBackendStr(virDomainDef *def,
> Don't lose the "const"
a left-over from some previous call that made changes to 'tpm'. fixed.
>
>> virCommandPtr cmd,
>> virQEMUCapsPtr qemuCaps,
>> int *tpmfd,
>> - int *cancelfd)
>> + int *cancelfd,
>> + char **chardev)
>> {
>> - const virDomainTPMDef *tpm = def->tpm;
>> + virDomainTPMDef *tpm = def->tpm;
> Don't lose the "const"
>
>> virBuffer buf = VIR_BUFFER_INITIALIZER;
>> - const char *type = virDomainTPMBackendTypeToString(tpm->type);
>> + const char *type = NULL;
>> char *cancel_path = NULL, *devset = NULL;
>> const char *tpmdev;
>>
>> *tpmfd = -1;
>> *cancelfd = -1;
>>
>> + switch (tpm->type) {
>> + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
>> + case VIR_DOMAIN_TPM_TYPE_EMULATOR:
>> + type = virDomainTPMBackendTypeToString(tpm->type);
>> + break;
>> + case VIR_DOMAIN_TPM_TYPE_LAST:
> default:
> virReportEnumRangeError(virDomainTPMBackendType, tpm->type);
>
> We need some sort of error message otherwise we get failed for some
> reason which is never fun to diagnose.
All other cases I see use the same function without error message. Not
sure what you mean. We seem to follow a pattern with this now.
>
>> + goto error;
>> + }
>> +
>> virBufferAsprintf(&buf, "%s,id=tpm-%s", type, tpm->info.alias);
>>
>> switch (tpm->type) {
>> @@ -9491,6 +9501,16 @@ qemuBuildTPMBackendStr(const virDomainDef *def,
>>
>> break;
>> case VIR_DOMAIN_TPM_TYPE_EMULATOR:
>> + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_TPM_EMULATOR))
>> + goto no_support;
>> +
>> + virBufferAddLit(&buf, ",chardev=chrtpm");
>> +
>> + if (virAsprintf(chardev, "socket,id=chrtpm,path=%s",
>> + tpm->data.emulator.source.data.nix.path) < 0)
>> + goto error;
>> +
>> + break;
>> case VIR_DOMAIN_TPM_TYPE_LAST:
>> goto error;
>> }
>> @@ -9517,10 +9537,11 @@ qemuBuildTPMBackendStr(const virDomainDef *def,
>>
>> static int
>> qemuBuildTPMCommandLine(virCommandPtr cmd,
>> - const virDomainDef *def,
>> + virDomainDef *def,
> Don't lose the "const"
>
>> virQEMUCapsPtr qemuCaps)
>> {
>> char *optstr;
>> + char *chardev = NULL;
>> int tpmfd = -1;
>> int cancelfd = -1;
>> char *fdset;
>> @@ -9529,12 +9550,18 @@ qemuBuildTPMCommandLine(virCommandPtr cmd,
>> return 0;
>>
>> if (!(optstr = qemuBuildTPMBackendStr(def, cmd, qemuCaps,
>> - &tpmfd, &cancelfd)))
>> + &tpmfd, &cancelfd,
>> + &chardev)))
>> return -1;
>>
>> virCommandAddArgList(cmd, "-tpmdev", optstr, NULL);
>> VIR_FREE(optstr);
>>
>> + if (chardev) {
>> + virCommandAddArgList(cmd, "-chardev", chardev, NULL);
>> + VIR_FREE(chardev);
>> + }
>> +
>> if (tpmfd >= 0) {
>> fdset = qemuVirCommandGetFDSet(cmd, tpmfd);
>> if (!fdset)
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index d3eac43..57a82dc 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -34,6 +34,7 @@
>> #include "qemu_migration.h"
>> #include "qemu_migration_params.h"
>> #include "qemu_security.h"
>> +#include "qemu_extdevice.h"
>> #include "viralloc.h"
>> #include "virlog.h"
>> #include "virerror.h"
>> @@ -7166,6 +7167,8 @@ qemuDomainRemoveInactive(virQEMUDriverPtr driver,
>> VIR_WARN("unable to remove snapshot directory %s", snapDir);
>> VIR_FREE(snapDir);
>> }
>> + if (!qemuExtDevicesInitPaths(driver, vm->def))
> I know it's more or less functionally equivalent, but it's better to use
> "if (qemuExtDevicesInitPaths(driver, vm->def) == 0)" since the function
> is not a boolean or pointer returning function.
>
> With suggested adjustments,
Done. Thanks.
>
> Reviewed-by: John Ferlan <jferlan at redhat.com>
>
> John
>
>> + virDomainTPMDelete(vm->def);
>>
>> virObjectRef(vm);
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 9ce97ea..f496f89 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -60,6 +60,7 @@
>> #include "qemu_migration_params.h"
>> #include "qemu_blockjob.h"
>> #include "qemu_security.h"
>> +#include "qemu_extdevice.h"
>>
>> #include "virerror.h"
>> #include "virlog.h"
>> @@ -7349,6 +7350,9 @@ qemuDomainCreateWithFlags(virDomainPtr dom, unsigned int flags)
>> goto endjob;
>> }
>>
>> + if (qemuExtDevicesInitPaths(driver, vm->def) < 0)
>> + goto endjob;
>> +
>> if (qemuDomainObjStart(dom->conn, driver, vm, flags,
>> QEMU_ASYNC_JOB_START) < 0)
>> goto endjob;
>> @@ -7494,6 +7498,9 @@ qemuDomainUndefineFlags(virDomainPtr dom,
>> if (!(vm = qemuDomObjFromDomain(dom)))
>> return -1;
>>
>> + if (qemuExtDevicesInitPaths(driver, vm->def) < 0)
>> + return -1;
>> +
>> cfg = virQEMUDriverGetConfig(driver);
>>
>> if (virDomainUndefineFlagsEnsureACL(dom->conn, vm->def) < 0)
>>
More information about the libvir-list
mailing list