[libvirt] [PATCH v3 07/14] util: Extend virtpm.c with tpm-emulator support
Add 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. Signed-off-by: Stefan Berger --- src/libvirt_private.syms | 5 + src/util/virtpm.c| 536 ++- src/util/virtpm.h| 33 ++- 3 files changed, 572 insertions(+), 2 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 33fe75b..eebfc72 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2984,6 +2984,11 @@ virTimeStringThenRaw; # util/virtpm.h virTPMCreateCancelPath; +virTPMDeleteEmulatorStorage; +virTPMEmulatorBuildCommand; +virTPMEmulatorInitPaths; +virTPMEmulatorPrepareHost; +virTPMEmulatorStop; # util/virtypedparam.h diff --git a/src/util/virtpm.c b/src/util/virtpm.c index d5c10da..76bbb21 100644 --- a/src/util/virtpm.c +++ b/src/util/virtpm.c @@ -1,7 +1,7 @@ /* * virtpm.c: TPM support * - * Copyright (C) 2013 IBM Corporation + * Copyright (C) 2013,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 @@ -22,16 +22,36 @@ #include +#include #include +#include +#include +#include +#include "conf/domain_conf.h" +#include "viralloc.h" +#include "vircommand.h" #include "virstring.h" #include "virerror.h" #include "viralloc.h" #include "virfile.h" +#include "virkmod.h" +#include "virlog.h" #include "virtpm.h" +#include "virutil.h" +#include "configmake.h" #define VIR_FROM_THIS VIR_FROM_NONE +VIR_LOG_INIT("util.tpm") + +/* + * executables for the swtpm; to be found on the host + */ +static char *swtpm_path; +static char *swtpm_setup; +static char *swtpm_ioctl; + /** * virTPMCreateCancelPath: * @devpath: Path to the TPM device @@ -74,3 +94,517 @@ virTPMCreateCancelPath(const char *devpath) cleanup: return path; } + +/* + * virTPMEmulatorInit + * + * Initialize the Emulator functions by searching for necessary + * executables that we will use to start and setup the swtpm + */ +static int +virTPMEmulatorInit(void) +{ +if (!swtpm_path) { +swtpm_path = virFindFileInPath("swtpm"); +if (!swtpm_path) { +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not find swtpm 'swtpm' in PATH")); +return -1; +} +if (!virFileIsExecutable(swtpm_path)) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("TPM emulator %s is not an executable"), + swtpm_path); +VIR_FREE(swtpm_path); +return -1; +} +} + +if (!swtpm_setup) { +swtpm_setup = virFindFileInPath("swtpm_setup"); +if (!swtpm_setup) { +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not find 'swtpm_setup' in PATH")); +return -1; +} +if (!virFileIsExecutable(swtpm_setup)) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("'%s' is not an executable"), + swtpm_setup); +VIR_FREE(swtpm_setup); +return -1; +} +} + +if (!swtpm_ioctl) { +swtpm_ioctl = virFindFileInPath("swtpm_ioctl"); +if (!swtpm_ioctl) { +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not find swtpm_ioctl in PATH")); +return -1; +} +if (!virFileIsExecutable(swtpm_ioctl)) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("swtpm_ioctl program %s is not an executable"), + swtpm_ioctl); +VIR_FREE(swtpm_ioctl); +return -1; +} +} + +return 0; +} + +/* + * virTPMCreateEmulatorStoragePath + * + * @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 * +virTPMCreateEmulatorStoragePath(const char *swtpmStorageDir, +const char *vmname) +{ +char *path = NULL; + +ignore_value(virAsprintf(&path, "%s/%s/tpm1.2", swtpmStorageDir, vmname)); + +return path; +} + +/* + * virtTPMGetTPMStorageDir: + * + * @storagepath: directory for swtpm's pesistent state + * + * Derive the 'TPMStorageDir' from the storagepath by searching + * for the last '/'. + */ +static char * +virTPMGetTPMStorageDir(const char *storagepath) +{ +const char *tail = strrchr(storagepath, '/'); +char *path = NULL; + +if (!tail) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not get tail of storagedir %s"), + storagepath); +return NULL; +} +ignore_value(VIR_STRNDUP(pa
Re: [libvirt] [PATCH v3 07/14] util: Extend virtpm.c with tpm-emulator support
On 05/04/2018 04:21 PM, Stefan Berger wrote: > Add 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. > > Signed-off-by: Stefan Berger > --- > src/libvirt_private.syms | 5 + > src/util/virtpm.c| 536 > ++- > src/util/virtpm.h| 33 ++- > 3 files changed, 572 insertions(+), 2 deletions(-) > > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms > index 33fe75b..eebfc72 100644 > --- a/src/libvirt_private.syms > +++ b/src/libvirt_private.syms > @@ -2984,6 +2984,11 @@ virTimeStringThenRaw; > > # util/virtpm.h > virTPMCreateCancelPath; > +virTPMDeleteEmulatorStorage; > +virTPMEmulatorBuildCommand; > +virTPMEmulatorInitPaths; > +virTPMEmulatorPrepareHost; > +virTPMEmulatorStop; > > > # util/virtypedparam.h > diff --git a/src/util/virtpm.c b/src/util/virtpm.c > index d5c10da..76bbb21 100644 > --- a/src/util/virtpm.c > +++ b/src/util/virtpm.c > @@ -1,7 +1,7 @@ > /* > * virtpm.c: TPM support > * > - * Copyright (C) 2013 IBM Corporation > + * Copyright (C) 2013,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 > @@ -22,16 +22,36 @@ > > #include > > +#include > #include > +#include > +#include > +#include > > +#include "conf/domain_conf.h" syntax-check would have told you unsafe cross-directory include - IOW including conf/* files into util/* files is not allowed. So I think you need to rethink where some of these functions will go. I think they are mostly all used by the qemu_extdevice.c changes in patch 9, so perhaps they need to get folded into them. There at least you can grab the conf/domain_conf.h file. > +#include "viralloc.h" syntax-check would have told you not to include "viralloc.h" twice... > +#include "vircommand.h" > #include "virstring.h" > #include "virerror.h" > #include "viralloc.h" > #include "virfile.h" > +#include "virkmod.h" > +#include "virlog.h" > #include "virtpm.h" > +#include "virutil.h" #include "viruuid.h" to get virUUIDGenerate > +#include "configmake.h" > > #define VIR_FROM_THIS VIR_FROM_NONE > > +VIR_LOG_INIT("util.tpm") > + > +/* > + * executables for the swtpm; to be found on the host > + */ > +static char *swtpm_path; > +static char *swtpm_setup; > +static char *swtpm_ioctl; > + There's a love/hate relationship with static/globals... > /** > * virTPMCreateCancelPath: > * @devpath: Path to the TPM device > @@ -74,3 +94,517 @@ virTPMCreateCancelPath(const char *devpath) > cleanup: > return path; > } Two empty lines - pervasive comment here... > + > +/* > + * virTPMEmulatorInit > + * > + * Initialize the Emulator functions by searching for necessary > + * executables that we will use to start and setup the swtpm > + */ > +static int > +virTPMEmulatorInit(void) > +{ > +if (!swtpm_path) { > +swtpm_path = virFindFileInPath("swtpm"); > +if (!swtpm_path) { > +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("Could not find swtpm 'swtpm' in PATH")); The message feels redundant. > +return -1; > +} > +if (!virFileIsExecutable(swtpm_path)) { > +virReportError(VIR_ERR_INTERNAL_ERROR, > + _("TPM emulator %s is not an executable"), > + swtpm_path); > +VIR_FREE(swtpm_path); > +return -1; > +} > +} > + > +if (!swtpm_setup) { > +swtpm_setup = virFindFileInPath("swtpm_setup"); > +if (!swtpm_setup) { > +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("Could not find 'swtpm_setup' in PATH")); > +return -1; > +} > +if (!virFileIsExecutable(swtpm_setup)) { > +virReportError(VIR_ERR_INTERNAL_ERROR, > + _("'%s' is not an executable"), > + swtpm_setup); > +VIR_FREE(swtpm_setup); > +return -1; > +} > +} > + > +if (!swtpm_ioctl) { > +swtpm_ioctl = virFindFileInPath("swtpm_ioctl"); > +if (!swtpm_ioctl) { > +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("Could not find swtpm_ioctl in PATH")); > +return -1; > +} > +if (!virFileIsExecutable(swtpm_ioctl)) { > +virReportError(VIR_ERR_INTERNAL_ERROR, > + _("swtpm_ioctl program %s is not an executable"), > + swtpm_ioctl); > +VIR_FREE(swtpm_ioctl); > +return -1; > +} > +} > + > +return 0; > +} > + > +/* > + * virTPMCreateEmulatorStoragePath > + * > + * @swtpmStorageDi
Re: [libvirt] [PATCH v3 07/14] util: Extend virtpm.c with tpm-emulator support
On 05/08/2018 04:30 PM, John Ferlan wrote: On 05/04/2018 04:21 PM, Stefan Berger wrote: Add 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. Signed-off-by: Stefan Berger --- src/libvirt_private.syms | 5 + src/util/virtpm.c| 536 ++- src/util/virtpm.h| 33 ++- 3 files changed, 572 insertions(+), 2 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 33fe75b..eebfc72 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2984,6 +2984,11 @@ virTimeStringThenRaw; # util/virtpm.h virTPMCreateCancelPath; +virTPMDeleteEmulatorStorage; +virTPMEmulatorBuildCommand; +virTPMEmulatorInitPaths; +virTPMEmulatorPrepareHost; +virTPMEmulatorStop; # util/virtypedparam.h diff --git a/src/util/virtpm.c b/src/util/virtpm.c index d5c10da..76bbb21 100644 --- a/src/util/virtpm.c +++ b/src/util/virtpm.c @@ -1,7 +1,7 @@ /* * virtpm.c: TPM support * - * Copyright (C) 2013 IBM Corporation + * Copyright (C) 2013,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 @@ -22,16 +22,36 @@ #include +#include #include +#include +#include +#include +#include "conf/domain_conf.h" syntax-check would have told you unsafe cross-directory include - IOW including conf/* files into util/* files is not allowed. So I think you need to rethink where some of these functions will go. I think they are mostly all used by the qemu_extdevice.c changes in patch 9, so perhaps they need to get folded into them. There at least you can grab the conf/domain_conf.h file. Probably best to do that... rather than passing the fields of virDomainTPMDef into the functions instead. Currently the functions have the prefix virTPM. That will have to change - to qemuTPM? So I'll merge these functions into qemu_extdevice.c? or another new file qemu_tpm.c ? +#include "viralloc.h" syntax-check would have told you not to include "viralloc.h" twice... obviously 'forgot' to run it. +#include "vircommand.h" #include "virstring.h" #include "virerror.h" #include "viralloc.h" #include "virfile.h" +#include "virkmod.h" +#include "virlog.h" #include "virtpm.h" +#include "virutil.h" #include "viruuid.h" to get virUUIDGenerate +#include "configmake.h" #define VIR_FROM_THIS VIR_FROM_NONE +VIR_LOG_INIT("util.tpm") + +/* + * executables for the swtpm; to be found on the host + */ +static char *swtpm_path; +static char *swtpm_setup; +static char *swtpm_ioctl; + There's a love/hate relationship with static/globals... /** * virTPMCreateCancelPath: * @devpath: Path to the TPM device @@ -74,3 +94,517 @@ virTPMCreateCancelPath(const char *devpath) cleanup: return path; } Two empty lines - pervasive comment here... + +/* + * virTPMEmulatorInit + * + * Initialize the Emulator functions by searching for necessary + * executables that we will use to start and setup the swtpm + */ +static int +virTPMEmulatorInit(void) +{ +if (!swtpm_path) { +swtpm_path = virFindFileInPath("swtpm"); +if (!swtpm_path) { +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not find swtpm 'swtpm' in PATH")); The message feels redundant. You mean the repetition of 'swtpm' is redundant? Should I adapt the reporting to use this type of command ? if (!(qemunbd = virFindFileInPath("qemu-nbd"))) { virReportSystemError(ENOENT, "%s", _("Unable to find 'qemu-nbd' binary in $PATH")); goto cleanup; } + +/* + * virTPMEmulatorPrepareHost: + * + * @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. + */ +int virTPMEmulatorPrepareHost(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) one line each argument +{ +int ret = -1; + +if (virTPMEmulatorInit() < 0) +return -1; + +/* create log dir ... */ +if (virFileM
Re: [libvirt] [PATCH v3 07/14] util: Extend virtpm.c with tpm-emulator support
On 05/09/2018 01:47 PM, Stefan Berger wrote: > On 05/08/2018 04:30 PM, John Ferlan wrote: >> >> On 05/04/2018 04:21 PM, Stefan Berger wrote: >>> Add 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. >>> >>> Signed-off-by: Stefan Berger >>> --- >>> src/libvirt_private.syms | 5 + >>> src/util/virtpm.c | 536 >>> ++- >>> src/util/virtpm.h | 33 ++- >>> 3 files changed, 572 insertions(+), 2 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms >>> index 33fe75b..eebfc72 100644 >>> --- a/src/libvirt_private.syms >>> +++ b/src/libvirt_private.syms >>> @@ -2984,6 +2984,11 @@ virTimeStringThenRaw; >>> # util/virtpm.h >>> virTPMCreateCancelPath; >>> +virTPMDeleteEmulatorStorage; >>> +virTPMEmulatorBuildCommand; >>> +virTPMEmulatorInitPaths; >>> +virTPMEmulatorPrepareHost; >>> +virTPMEmulatorStop; >>> # util/virtypedparam.h >>> diff --git a/src/util/virtpm.c b/src/util/virtpm.c >>> index d5c10da..76bbb21 100644 >>> --- a/src/util/virtpm.c >>> +++ b/src/util/virtpm.c >>> @@ -1,7 +1,7 @@ >>> /* >>> * virtpm.c: TPM support >>> * >>> - * Copyright (C) 2013 IBM Corporation >>> + * Copyright (C) 2013,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 >>> @@ -22,16 +22,36 @@ >>> #include >>> +#include >>> #include >>> +#include >>> +#include >>> +#include >>> +#include "conf/domain_conf.h" >> syntax-check would have told you unsafe cross-directory include - IOW >> including conf/* files into util/* files is not allowed. >> >> So I think you need to rethink where some of these functions will go. I >> think they are mostly all used by the qemu_extdevice.c changes in patch >> 9, so perhaps they need to get folded into them. There at least you can >> grab the conf/domain_conf.h file. > > Probably best to do that... rather than passing the fields of > virDomainTPMDef into the functions instead. > Currently the functions have the prefix virTPM. That will have to change > - to qemuTPM? So I'll merge these functions into qemu_extdevice.c? or > another new file qemu_tpm.c ? > > qemu_tpm.c seems good for those specific things > >> >>> +#include "viralloc.h" >> syntax-check would have told you not to include "viralloc.h" twice... > > obviously 'forgot' to run it. > >> >>> +#include "vircommand.h" >>> #include "virstring.h" >>> #include "virerror.h" >>> #include "viralloc.h" >>> #include "virfile.h" >>> +#include "virkmod.h" >>> +#include "virlog.h" >>> #include "virtpm.h" >>> +#include "virutil.h" >> #include "viruuid.h" to get virUUIDGenerate >> >>> +#include "configmake.h" >>> #define VIR_FROM_THIS VIR_FROM_NONE >>> +VIR_LOG_INIT("util.tpm") >>> + >>> +/* >>> + * executables for the swtpm; to be found on the host >>> + */ >>> +static char *swtpm_path; >>> +static char *swtpm_setup; >>> +static char *swtpm_ioctl; >>> + >> There's a love/hate relationship with static/globals... >> >>> /** >>> * virTPMCreateCancelPath: >>> * @devpath: Path to the TPM device >>> @@ -74,3 +94,517 @@ virTPMCreateCancelPath(const char *devpath) >>> cleanup: >>> return path; >>> } >> Two empty lines - pervasive comment here... >> >>> + >>> +/* >>> + * virTPMEmulatorInit >>> + * >>> + * Initialize the Emulator functions by searching for necessary >>> + * executables that we will use to start and setup the swtpm >>> + */ >>> +static int >>> +virTPMEmulatorInit(void) >>> +{ >>> + if (!swtpm_path) { >>> + swtpm_path = virFindFileInPath("swtpm"); >>> + if (!swtpm_path) { >>> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", >>> + _("Could not find swtpm 'swtpm' in PATH")); >> The message feels redundant. > > You mean the repetition of 'swtpm' is redundant? yes. > > Should I adapt the reporting to use this type of command ? > > if (!(qemunbd = virFindFileInPath("qemu-nbd"))) { > virReportSystemError(ENOENT, "%s", > _("Unable to find 'qemu-nbd' binary in > $PATH")); > goto cleanup; > } > + That seems reasonable. >>> +/* >>> + * virTPMEmulatorPrepareHost: >>> + * >>> + * @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
Re: [libvirt] [PATCH v3 07/14] util: Extend virtpm.c with tpm-emulator support
On 05/10/2018 03:29 PM, John Ferlan wrote: On 05/09/2018 01:47 PM, Stefan Berger wrote: On 05/08/2018 04:30 PM, John Ferlan wrote: On 05/04/2018 04:21 PM, Stefan Berger wrote: Add 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. Signed-off-by: Stefan Berger --- src/libvirt_private.syms | 5 + src/util/virtpm.c| 536 ++- src/util/virtpm.h| 33 ++- 3 files changed, 572 insertions(+), 2 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 33fe75b..eebfc72 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2984,6 +2984,11 @@ virTimeStringThenRaw; # util/virtpm.h virTPMCreateCancelPath; +virTPMDeleteEmulatorStorage; +virTPMEmulatorBuildCommand; +virTPMEmulatorInitPaths; +virTPMEmulatorPrepareHost; +virTPMEmulatorStop; # util/virtypedparam.h diff --git a/src/util/virtpm.c b/src/util/virtpm.c index d5c10da..76bbb21 100644 --- a/src/util/virtpm.c +++ b/src/util/virtpm.c @@ -1,7 +1,7 @@ /* * virtpm.c: TPM support * - * Copyright (C) 2013 IBM Corporation + * Copyright (C) 2013,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 @@ -22,16 +22,36 @@ #include +#include #include +#include +#include +#include +#include "conf/domain_conf.h" syntax-check would have told you unsafe cross-directory include - IOW including conf/* files into util/* files is not allowed. So I think you need to rethink where some of these functions will go. I think they are mostly all used by the qemu_extdevice.c changes in patch 9, so perhaps they need to get folded into them. There at least you can grab the conf/domain_conf.h file. Probably best to do that... rather than passing the fields of virDomainTPMDef into the functions instead. Currently the functions have the prefix virTPM. That will have to change - to qemuTPM? So I'll merge these functions into qemu_extdevice.c? or another new file qemu_tpm.c ? qemu_tpm.c seems good for those specific things Will post v4 soon. Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list