On 11/13/24 18:39, Stefan Berger wrote:
> Get the JSON profile that the swtpm instance was created with from the
> output of 'swtpm socket --tpm2 --print-info 0x20 --tpmstate ...'. Get the
> name of the profile from the JSON and set it in the current and persistent
> emulator descriptions as 'name' attribute and have the persistent
> description stored with this update. The user should avoid setting this
> 'name' attribute since it is meant to be read-only. The following is
> an example of how the XML could look like:
>
> <profile source='local:restricted' name='custom:restricted'/>
>
> If the user provided no profile node, and therefore swtpm_setup picked its
> default profile, the XML may now shows the 'name' attribute with the name
> of the profile. This makes the 'source' attribute now optional.
>
> <profile name='default-v1'/>
>
> Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxx>
> ---
> docs/formatdomain.rst | 16 ++---
> src/conf/domain_conf.c | 18 +++---
> src/conf/domain_conf.h | 1 +
> src/conf/schemas/domaincommon.rng | 13 +++-
> src/qemu/qemu_extdevice.c | 5 +-
> src/qemu/qemu_tpm.c | 100 ++++++++++++++++++++++++++++--
> src/qemu/qemu_tpm.h | 3 +-
> src/util/virtpm.c | 1 +
> src/util/virtpm.h | 1 +
> 9 files changed, 135 insertions(+), 23 deletions(-)
>
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index 6539f620fa..20c86087ef 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -8131,7 +8131,7 @@ Example: usage of the TPM Emulator
> <active_pcr_banks>
> <sha256/>
> </active_pcr_banks>
> - <profile source='local:restricted' remove_disabled='check'/>
> + <profile source='local:restricted' remove_disabled='check' name='custom:restricted'/>
> </backend>
> </tpm>
> </devices>
> @@ -8229,12 +8229,14 @@ Example: usage of the TPM Emulator
> ``profile``
> The ``profile`` node is used to set a profile for a TPM 2.0 given in the
> source attribute. This profile will be set when the TPM is initially
> - created and after that cannot be changed anymore. If no profile is provided,
> - then swtpm will use the latest built-in 'default' profile or the default
> - profile set in swtpm_setup.conf. Otherwise swtpm_setup will search for a
> - profile with the given name with appended .json suffix in a configurable
> - local and then in a distro directory. If none could be found in either, it
> - will fall back trying to use a built-in one.
> + created and after that cannot be changed anymore. Once a profile has been
> + set the name attribute will be updated with the name of the profile that
> + is running. If no profile is provided, then swtpm will use the latest
> + built-in 'default' profile or the default profile set in swtpm_setup.conf.
> + Otherwise swtpm_setup will search for a profile with the given name with
> + appended .json suffix in a configurable local and then in a distro
> + directory. If none could be found in either, it will fall back trying to
> + use a built-in one.
>
> The built-in 'null' profile provides backwards compatibility with
> libtpms v0.9 but also restricts the user to use only TPM features that were
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 7d91e3e958..6f6898014b 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -3479,6 +3479,7 @@ void virDomainTPMDefFree(virDomainTPMDef *def)
> g_free(def->data.emulator.logfile);
> virBitmapFree(def->data.emulator.activePcrBanks);
> g_free(def->data.emulator.profile_source);
> + g_free(def->data.emulator.profile_name);
> break;
> case VIR_DOMAIN_TPM_TYPE_EXTERNAL:
> virObjectUnref(def->data.external.source);
> @@ -10926,10 +10927,6 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
>
> if ((profile = virXPathNode("./backend/profile[1]", ctxt))) {
> def->data.emulator.profile_source = virXMLPropString(profile, "source");
> - if (!def->data.emulator.profile_source) {
> - virReportError(VIR_ERR_XML_ERROR, "%s", _("missing profile source"));
> - goto error;
> - }
> if (virXMLPropEnum(profile, "remove_disabled",
> virDomainTPMProfileRemoveDisabledTypeFromString,
> VIR_XML_PROP_NONZERO,
> @@ -10938,6 +10935,7 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
> if (profile_remove_disabled != VIR_DOMAIN_TPM_PROFILE_REMOVE_DISABLED_NONE)
> def->data.emulator.profile_remove_disabled =
> virDomainTPMProfileRemoveDisabledTypeToString(profile_remove_disabled);
> + def->data.emulator.profile_name = virXMLPropString(profile, "name");
> }
> break;
> case VIR_DOMAIN_TPM_TYPE_EXTERNAL:
> @@ -25134,12 +25132,18 @@ virDomainTPMDefFormat(virBuffer *buf,
> virDomainTPMSourceTypeTypeToString(def->data.emulator.source_type));
> virBufferEscapeString(&backendChildBuf, " path='%s'/>\n", def->data.emulator.source_path);
> }
> - if (def->data.emulator.profile_source) {
> - virBufferAsprintf(&backendChildBuf, "<profile source='%s'",
> - def->data.emulator.profile_source);
> + if (def->data.emulator.profile_source ||
> + def->data.emulator.profile_name) {
> + virBufferAddLit(&backendChildBuf, "<profile");
> + if (def->data.emulator.profile_source)
> + virBufferAsprintf(&backendChildBuf, " source='%s'",
> + def->data.emulator.profile_source);
> if (def->data.emulator.profile_remove_disabled)
> virBufferAsprintf(&backendChildBuf, " remove_disabled='%s'",
> def->data.emulator.profile_remove_disabled);
> + if (def->data.emulator.profile_name)
> + virBufferAsprintf(&backendChildBuf, " name='%s'",
> + def->data.emulator.profile_name);
> virBufferAddLit(&backendChildBuf, "/>\n");
> }
> break;
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index bd2740af26..45421d4772 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1493,6 +1493,7 @@ struct _virDomainTPMEmulatorDef {
> bool persistent_state;
> virBitmap *activePcrBanks;
> char *profile_source; /* 'source' profile was created from */
> + char *profile_name; /* name read from active profile */
> const char *profile_remove_disabled;
> };
>
> diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
> index d94ff9b4c3..e26e65fd7c 100644
> --- a/src/conf/schemas/domaincommon.rng
> +++ b/src/conf/schemas/domaincommon.rng
> @@ -6056,9 +6056,11 @@
> <define name="tpm-backend-emulator-profile">
> <optional>
> <element name="profile">
> - <attribute name="source">
> - <ref name="profileName"/>
> - </attribute>
> + <optional>
> + <attribute name="source">
> + <ref name="profileName"/>
> + </attribute>
> + </optional>
> <optional>
> <attribute name="remove_disabled">
> <choice>
> @@ -6067,6 +6069,11 @@
> </choice>
> </attribute>
> </optional>
> + <optional>
> + <attribute name="name">
> + <ref name="profileName"/>
> + </attribute>
> + </optional>
> </element>
> </optional>
> </define>
> diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
> index dc1bb56237..a6f31f9773 100644
> --- a/src/qemu/qemu_extdevice.c
> +++ b/src/qemu/qemu_extdevice.c
> @@ -175,6 +175,7 @@ qemuExtDevicesStart(virQEMUDriver *driver,
> virDomainObj *vm,
> bool incomingMigration)
> {
> + virDomainDef *persistentDef = vm->newDef;
> virDomainDef *def = vm->def;
> size_t i;
>
> @@ -189,9 +190,11 @@ qemuExtDevicesStart(virQEMUDriver *driver,
>
> for (i = 0; i < def->ntpms; i++) {
> virDomainTPMDef *tpm = def->tpms[i];
> + virDomainTPMDef *persistentTPMDef = persistentDef->tpms[i];
>
> if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR &&
> - qemuExtTPMStart(driver, vm, tpm, incomingMigration) < 0)
> + qemuExtTPMStart(driver, vm, tpm, persistentTPMDef,
> + incomingMigration) < 0)
> return -1;
> }
>
> diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
> index a7eee501bf..2fb3796910 100644
> --- a/src/qemu/qemu_tpm.c
> +++ b/src/qemu/qemu_tpm.c
> @@ -627,15 +627,89 @@ qemuTPMVirCommandSwtpmAddTPMState(virCommand *cmd,
> }
> }
>
> +/* qemuTPMEmulatorUpdateProfileName:
> + *
> + * @emulator: TPM emulator definition
> + * @persistentTPMDef: TPM definition from the persistent domain definition
> + * @cfg: virQEMUDriverConfig
> + * @saveDef: whether caller should save the persistent domain def
> + */
> +static int
> +qemuTPMEmulatorUpdateProfileName(virDomainTPMEmulatorDef *emulator,
> + virDomainTPMDef *persistentTPMDef,
> + const virQEMUDriverConfig *cfg,
> + bool *saveDef)
> +{
> + g_autoptr(virJSONValue) object = NULL;
> + g_autofree char *stderr_buf = NULL;
> + g_autofree char *stdout_buf = NULL;
> + g_autoptr(virCommand) cmd = NULL;
> + g_autofree char *swtpm = NULL;
> + virJSONValue *active_profile;
> + const char *profile_name;
> + int exitstatus;
> +
> + if (emulator->version != VIR_DOMAIN_TPM_VERSION_2_0 ||
> + !virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_CMDARG_PRINT_INFO))
> + return 0;
> +
> + swtpm = virTPMGetSwtpm();
> + if (!swtpm)
> + return -1;
> +
> + cmd = virCommandNew(swtpm);
> +
> + virCommandSetUID(cmd, cfg->swtpm_user); /* should be uid of 'tss' or 'root' */
> + virCommandSetGID(cmd, cfg->swtpm_group);
> +
> + virCommandAddArgList(cmd, "socket", "--print-info", "0x20", "--tpm2", NULL);
> +
> + qemuTPMVirCommandSwtpmAddTPMState(cmd, emulator);
> +
> + if (qemuTPMVirCommandSwtpmAddEncryption(cmd, emulator, swtpm) < 0)
> + return -1;
> +
> + virCommandClearCaps(cmd);
> +
> + virCommandSetOutputBuffer(cmd, &stdout_buf);
> + virCommandSetErrorBuffer(cmd, &stderr_buf);
> +
> + if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Could not run '%1$s --print-info'. exitstatus: %2$d; stderr: %3$s\n"),
We don't really put newlines at the end of messages.
> + swtpm, exitstatus, stderr_buf);
> + return -1;
> + }
> +
> + if (!(object = virJSONValueFromString(stdout_buf)))
> + return -1;
> +
> + if (!(active_profile = virJSONValueObjectGetObject(object, "ActiveProfile")))
> + return -1;
> +
> + profile_name = g_strdup(virJSONValueObjectGetString(active_profile, "Name"));
This g_strdup() looks suspicios and surely must lead to a memleak.
> +
> + g_free(emulator->profile_name);
> + emulator->profile_name = g_strdup(profile_name);
> +
> + *saveDef = true;
> + g_free(persistentTPMDef->data.emulator.profile_name);
> + persistentTPMDef->data.emulator.profile_name = g_strdup(profile_name);
> +
> + return 0;
> +}
> +
Michal
