Hi
On Fri, Oct 11, 2024 at 5:49 PM Stefan Berger <stefanb@xxxxxxxxxxxxx> wrote:
>
>
>
> On 10/4/24 9:32 AM, marcandre.lureau@xxxxxxxxxx wrote:
> > From: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
> >
> > Learn to parse a file path for the TPM state.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
> > ---
> > docs/formatdomain.rst | 19 ++++++++++++++
> > src/conf/domain_conf.c | 28 +++++++++++++++++++++
> > src/conf/domain_conf.h | 9 +++++++
> > src/conf/schemas/domaincommon.rng | 14 +++++++++++
> > tests/qemuxmlconfdata/tpm-emulator-tpm2.xml | 1 +
> > 5 files changed, 71 insertions(+)
> >
> > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> > index 4336cff3ac..992bb98730 100644
> > --- a/docs/formatdomain.rst
> > +++ b/docs/formatdomain.rst
> > @@ -8173,6 +8173,25 @@ Example: usage of the TPM Emulator
> > The default version used depends on the combination of hypervisor, guest
> > architecture, TPM model and backend.
> >
> > +``source``
> > + The ``source`` element specifies the location of the TPM state storage . This
> > + element only works with the ``emulator`` backend.
> > +
> > + If not specified, the storage configuration is left to libvirt discretion.
> > +
> > + This element requires that swtpm v0.7 or later is installed.
> > +
> > + The following attributes are supported:
> > +
> > + ``type``
> > + The type of storage. It's possible to provide "file" to utilize a single
> > + file or block device where the TPM state will be stored.
> > +
> > + ``path``
> > + The path to the TPM state storage.
>
> The file backend of swtpm does not do the locking similar to what the
> dir backend does because those who added the file backend didn't
> need/want it. If we now give full control to the path of the TPM state
> file to the user via the domain XML then whose fault is it if two VMs
> use the same path to a file backend and stomp on the TPM state file? Is
> it the fault of the user because of how he defined the path in the XMLs?
Imho, it's desirable to have a similar locking behaviour regardless of
the backend and prevent users for mistakenly using the same file.
>
>
> > +
> > + :since:`Since v10.8.0`
> > +
>
> > ``persistent_state``
> > The ``persistent_state`` attribute indicates whether 'swtpm' TPM state is
> > kept or not when a transient domain is powered off or undefined. This
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 284a3815b3..9dd8b6b55d 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -1322,6 +1322,12 @@ VIR_ENUM_IMPL(virDomainTPMVersion,
> > "2.0",
> > );
> >
> > +VIR_ENUM_IMPL(virDomainTPMSourceType,
> > + VIR_DOMAIN_TPM_SOURCE_TYPE_LAST,
> > + "default",
> > + "file",
> > +);
> > +
> > VIR_ENUM_IMPL(virDomainTPMPcrBank,
> > VIR_DOMAIN_TPM_PCR_BANK_LAST,
> > "sha1",
> > @@ -10784,6 +10790,7 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
> > int nbackends;
> > int nnodes;
> > size_t i;
> > + xmlNodePtr source_node = NULL;
> > g_autofree char *path = NULL;
> > g_autofree char *secretuuid = NULL;
> > g_autofree char *persistent_state = NULL;
> > @@ -10857,6 +10864,22 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
> > def->data.emulator.hassecretuuid = true;
> > }
> >
> > + source_node = virXPathNode("./backend/source", ctxt);
> > + if (source_node) {
> > + if (virXMLPropEnum(source_node, "type",
> > + virDomainTPMSourceTypeTypeFromString,
> > + VIR_XML_PROP_NONZERO,
> > + &def->data.emulator.source_type) < 0)
> > + goto error;
> > + path = virXMLPropString(source_node, "path");
> > + if (!path) {
> > + virReportError(VIR_ERR_XML_ERROR, "%s",
> > + _("missing TPM source path"));
> > + goto error;
> > + }
> > + def->data.emulator.source_path = g_steal_pointer(&path);
> > + }
> > +
> > persistent_state = virXMLPropString(backends[0], "persistent_state");
> > if (persistent_state) {
> > if (virStringParseYesNo(persistent_state,
> > @@ -25070,6 +25093,11 @@ virDomainTPMDefFormat(virBuffer *buf,
> >
> > virXMLFormatElement(&backendChildBuf, "active_pcr_banks", NULL, &activePcrBanksBuf);
> > }
> > + if (def->data.emulator.source_type != VIR_DOMAIN_TPM_SOURCE_TYPE_DEFAULT) {
> > + virBufferAsprintf(&backendChildBuf, "<source type='%s'",
> > + virDomainTPMSourceTypeTypeToString(def->data.emulator.source_type));
> > + virBufferEscapeString(&backendChildBuf, " path='%s'/>\n", def->data.emulator.source_path);
> > + }
> > break;
> > case VIR_DOMAIN_TPM_TYPE_EXTERNAL:
> > if (def->data.external.source->type == VIR_DOMAIN_CHR_TYPE_UNIX) {
> > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> > index 6b27322e3e..7a70f68177 100644
> > --- a/src/conf/domain_conf.h
> > +++ b/src/conf/domain_conf.h
> > @@ -1463,6 +1463,13 @@ typedef enum {
> > VIR_DOMAIN_TPM_PCR_BANK_LAST
> > } virDomainPcrBank;
> >
> > +typedef enum {
> > + VIR_DOMAIN_TPM_SOURCE_TYPE_DEFAULT = 0,
> > + VIR_DOMAIN_TPM_SOURCE_TYPE_FILE,
> > +
> > + VIR_DOMAIN_TPM_SOURCE_TYPE_LAST
> > +} virDomainTPMSourceType;
> > +
> > #define VIR_DOMAIN_TPM_DEFAULT_DEVICE "/dev/tpm0"
> >
> > struct _virDomainTPMDef {
> > @@ -1478,6 +1485,7 @@ struct _virDomainTPMDef {
> > struct {
> > virDomainTPMVersion version;
> > virDomainChrSourceDef *source;
> > + virDomainTPMSourceType source_type;
> > char *source_path;
> > char *logfile;
> > unsigned int debug;
> > @@ -4277,6 +4285,7 @@ VIR_ENUM_DECL(virDomainRNGBackend);
> > VIR_ENUM_DECL(virDomainTPMModel);
> > VIR_ENUM_DECL(virDomainTPMBackend);
> > VIR_ENUM_DECL(virDomainTPMVersion);
> > +VIR_ENUM_DECL(virDomainTPMSourceType);
> > VIR_ENUM_DECL(virDomainTPMPcrBank);
> > VIR_ENUM_DECL(virDomainMemoryModel);
> > VIR_ENUM_DECL(virDomainMemoryBackingModel);
> > diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
> > index efb5f00d77..72c8b6c694 100644
> > --- a/src/conf/schemas/domaincommon.rng
> > +++ b/src/conf/schemas/domaincommon.rng
> > @@ -5923,6 +5923,7 @@
> > <interleave>
> > <ref name="tpm-backend-emulator-encryption"/>
> > <ref name="tpm-backend-emulator-active-pcr-banks"/>
> > + <ref name="tpm-backend-emulator-source"/>
> > </interleave>
> > <optional>
> > <attribute name="persistent_state">
> > @@ -5981,6 +5982,19 @@
> > </optional>
> > </define>
> >
> > + <define name="tpm-backend-emulator-source">
> > + <optional>
> > + <element name="source">
> > + <attribute name="type">
> > + <value>file</value>
> > + </attribute>
> > + <attribute name="path">
> > + <ref name="filePath"/>
> > + </attribute>
> > + </element>
> > + </optional>
> > + </define>
> > +
> > <define name="tpm-backend-emulator-encryption">
> > <optional>
> > <element name="encryption">
> > diff --git a/tests/qemuxmlconfdata/tpm-emulator-tpm2.xml b/tests/qemuxmlconfdata/tpm-emulator-tpm2.xml
> > index 8a613db456..3d6300f544 100644
> > --- a/tests/qemuxmlconfdata/tpm-emulator-tpm2.xml
> > +++ b/tests/qemuxmlconfdata/tpm-emulator-tpm2.xml
> > @@ -34,6 +34,7 @@
> > <sha256/>
> > <sha512/>
> > </active_pcr_banks>
> > + <source type='file' path='/path/to/state'/>
> > </backend>
> > </tpm>
> > <audio id='1' type='none'/>
>
