Extend the TPM domain XML with an attribute active_pcr_banks that allows
a user to specify the PCR banks to activate before starting a VM. A comma-
separated list of PCR banks with the choices of sha1, sha256, sha384 and
sha512 is allowed. When the XML attribute is provided, the set of active
PCR banks is 'enforced' by running swtpm_setup before every start of the
VM. The activation requires that swtpm_setup v0.7 or later is installed
and may not have any effect otherwise.
<tpm model='tpm-tis'>
<backend type='emulator' version='2.0' active_pcr_banks='sha256,sha384'/>
</tpm>
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2016599
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
docs/formatdomain.rst | 12 ++-
docs/schemas/basictypes.rng | 6 ++
docs/schemas/domaincommon.rng | 5 ++
src/conf/domain_conf.c | 21 ++++-
src/conf/domain_conf.h | 1 +
src/qemu/qemu_tpm.c | 80 +++++++++++++++++++
src/util/virtpm.c | 1 +
src/util/virtpm.h | 1 +
tests/qemuxml2argvdata/tpm-emulator-tpm2.xml | 2 +-
.../tpm-emulator-tpm2.x86_64-latest.xml | 2 +-
10 files changed, 127 insertions(+), 4 deletions(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 0651975c88..8785a7a682 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -7537,7 +7537,7 @@ Example: usage of the TPM Emulator
...
<devices>
<tpm model='tpm-tis'>
- <backend type='emulator' version='2.0'>
+ <backend type='emulator' version='2.0' active_pcr_banks='sha256'>
<encryption secret='6dd3e4a5-1d76-44ce-961f-f119f5aad935'/>
</backend>
</tpm>
@@ -7598,6 +7598,16 @@ Example: usage of the TPM Emulator
This attribute only works with the ``emulator`` backend. The accepted values
are ``yes`` and ``no``. :since:`Since 7.0.0`
+``active_pcr_banks``
+ The ``active_pcr_banks`` attribute indicates the names of the PCR banks
+ of a TPM 2.0 to activate. A comma separated list of PCR banks' names
+ must be provided. Valid names are for example sha1, sha256, sha384, and
+ sha512. If this attribute is provided, the set of PCR banks are activated
+ before every start of a VM and this step is logged in the swtpm's log.
+ This attribute requires that swtpm_setup v0.7 or later is installed
+ and may not have any effect otherwise. This attribute only works with the
+ ``emulator`` backend. since:`Since 7.10.0`
+
``encryption``
The ``encryption`` element allows the state of a TPM emulator to be
encrypted. The ``secret`` must reference a secret object that holds the
diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
index a221ff6295..3bd1eebdc4 100644
--- a/docs/schemas/basictypes.rng
+++ b/docs/schemas/basictypes.rng
@@ -88,6 +88,12 @@
</choice>
</define>
+ <define name="pcrBankList">
+ <data type="string">
+ <param name="pattern">(sha1|sha256|sha384|sha512){1}(,(sha1|sha256|sha384|sha512)){0,3}</param>
+ </data>
+ </define>
+
<define name="numaDistanceValue">
<data type="unsignedInt">
<param name="minInclusive">10</param>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 67df13d90d..6801673cf1 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -5331,6 +5331,11 @@
</choice>
</attribute>
</optional>
+ <optional>
+ <attribute name="active_pcr_banks">
+ <ref name="pcrBankList"/>
+ </attribute>
+ </optional>
</group>
</choice>
<optional>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4644d18120..bc8237fd0b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3207,6 +3207,7 @@ void virDomainTPMDefFree(virDomainTPMDef *def)
break;
case VIR_DOMAIN_TPM_TYPE_EMULATOR:
virDomainChrSourceDefClear(&def->data.emulator.source);
+ g_free(def->data.emulator.activePcrBanks);
g_free(def->data.emulator.storagepath);
g_free(def->data.emulator.logfile);
break;
@@ -11733,7 +11734,7 @@ virDomainSmartcardDefParseXML(virDomainXMLOption *xmlopt,
* Emulator state encryption is supported with the following:
*
* <tpm model='tpm-tis'>
- * <backend type='emulator' version='2.0'>
+ * <backend type='emulator' version='2.0' active_pcr_banks='sha256,sha384'>
* <encryption secret='32ee7e76-2178-47a1-ab7b-269e6e348015'/>
* </backend>
* </tpm>
@@ -11759,6 +11760,7 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
g_autofree char *version = NULL;
g_autofree char *secretuuid = NULL;
g_autofree char *persistent_state = NULL;
+ g_autofree char *activePcrBanks = NULL;
g_autofree xmlNodePtr *backends = NULL;
def = g_new0(virDomainTPMDef, 1);
@@ -11841,6 +11843,18 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
goto error;
}
}
+ if (def->version == VIR_DOMAIN_TPM_VERSION_2_0) {
+ activePcrBanks = virXMLPropString(backends[0], "active_pcr_banks");
+ if (activePcrBanks) {
+ if (!virStringMatch(activePcrBanks,
+ "(sha1|sha256|sha384|sha512)(,(sha1|sha256|sha384|sha512)){0,3}")) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Malformatted list of PCR banks"));
+ goto error;
+ }
+ def->data.emulator.activePcrBanks = g_steal_pointer(&activePcrBanks);
+ }
+ }
break;
case VIR_DOMAIN_TPM_TYPE_LAST:
goto error;
@@ -25433,6 +25447,11 @@ virDomainTPMDefFormat(virBuffer *buf,
virDomainTPMVersionTypeToString(def->version));
if (def->data.emulator.persistent_state)
virBufferAddLit(buf, " persistent_state='yes'");
+ if (def->version == VIR_DOMAIN_TPM_VERSION_2_0 &&
+ def->data.emulator.activePcrBanks) {
+ virBufferAsprintf(buf, " active_pcr_banks='%s'",
+ def->data.emulator.activePcrBanks);
+ }
if (def->data.emulator.hassecretuuid) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
virBufferAddLit(buf, ">\n");
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index cb6d8975b8..19597dba7e 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1381,6 +1381,7 @@ struct _virDomainTPMDef {
unsigned char secretuuid[VIR_UUID_BUFLEN];
bool hassecretuuid;
bool persistent_state;
+ char *activePcrBanks;
} emulator;
} data;
};
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 93cb04f49d..bb14228edc 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -566,6 +566,78 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
}
+/*
+ * qemuTPMEmulatorReconfigure
+ *
+ *
+ * @storagepath: path to the directory for TPM state
+ * @swtpm_user: The userid to switch to when setting up the TPM;
+ * typically this should be the uid of 'tss' or 'root'
+ * @swtpm_group: The group id to switch to
+ * @swtpmActivePcrBanks: The string describing the active PCR banks
+ * @logfile: The file to write the log into; it must be writable
+ * for the user given by userid or 'tss'
+ * @tpmversion: The version of the TPM, either a TPM 1.2 or TPM 2
+ * @secretuuid: The secret's UUID needed for state encryption
+ *
+ * Reconfigure the active PCR banks of a TPM 2.
+ */
+static int
+qemuTPMEmulatorReconfigure(const char *storagepath,
+ uid_t swtpm_user,
+ gid_t swtpm_group,
+ const char *swtpmActivePcrBanks,
+ const char *logfile,
+ const virDomainTPMVersion tpmversion,
+ const unsigned char *secretuuid)
+{
+ g_autoptr(virCommand) cmd = NULL;
+ int exitstatus;
+ g_autofree char *swtpm_setup = virTPMGetSwtpmSetup();
+ VIR_AUTOCLOSE pwdfile_fd = -1;
+
+ if (!swtpm_setup)
+ return -1;
+
+ if (tpmversion != VIR_DOMAIN_TPM_VERSION_2_0 ||
+ swtpmActivePcrBanks == NULL ||
+ !virTPMSwtpmSetupCapsGet(
+ VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_RECONFIGURE_PCR_BANKS))
+ return 0;
+
+ cmd = virCommandNew(swtpm_setup);
+ if (!cmd)
+ return -1;
+
+ virCommandSetUID(cmd, swtpm_user);
+ virCommandSetGID(cmd, swtpm_group);
+
+ virCommandAddArgList(cmd, "--tpm2", NULL);
+
+ if (qemuTPMVirCommandAddEncryption(cmd, swtpm_setup, secretuuid) < 0)
+ return -1;
+
+ virCommandAddArgList(cmd,
+ "--tpm-state", storagepath,
+ "--logfile", logfile,
+ "--pcr-banks", swtpmActivePcrBanks,
+ "--reconfigure",
+ NULL);
+
+ virCommandClearCaps(cmd);
+
+ if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not run '%s --reconfigure'. exitstatus: %d; "
+ "Check error log '%s' for details."),
+ swtpm_setup, exitstatus, logfile);
+ return -1;
+ }
+
+ return 0;
+}
+
+
/*
* qemuTPMEmulatorBuildCommand:
*
@@ -620,6 +692,14 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
secretuuid, incomingMigration) < 0)
goto error;
+ if (!incomingMigration &&
+ qemuTPMEmulatorReconfigure(tpm->data.emulator.storagepath,
+ swtpm_user, swtpm_group,
+ tpm->data.emulator.activePcrBanks,
+ tpm->data.emulator.logfile, tpm->version,
+ secretuuid) < 0)
+ goto error;
+
unlink(tpm->data.emulator.source.data.nix.path);
cmd = virCommandNew(swtpm);
diff --git a/src/util/virtpm.c b/src/util/virtpm.c
index 40d9272e66..7fa870b803 100644
--- a/src/util/virtpm.c
+++ b/src/util/virtpm.c
@@ -47,6 +47,7 @@ VIR_ENUM_IMPL(virTPMSwtpmSetupFeature,
"cmdarg-pwdfile-fd",
"cmdarg-create-config-files",
"tpm12-not-need-root",
+ "cmdarg-reconfigure-pcr-banks",
);
/**
diff --git a/src/util/virtpm.h b/src/util/virtpm.h
index b75eb84f31..defea6c106 100644
--- a/src/util/virtpm.h
+++ b/src/util/virtpm.h
@@ -40,6 +40,7 @@ typedef enum {
VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PWDFILE_FD,
VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_CREATE_CONFIG_FILES,
VIR_TPM_SWTPM_SETUP_FEATURE_TPM12_NOT_NEED_ROOT,
+ VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_RECONFIGURE_PCR_BANKS,
VIR_TPM_SWTPM_SETUP_FEATURE_LAST
} virTPMSwtpmSetupFeature;
diff --git a/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml b/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml
index 3e2f485ee7..ca9b38540d 100644
--- a/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml
+++ b/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml
@@ -23,7 +23,7 @@
<input type='mouse' bus='ps2'/>
<input type='keyboard' bus='ps2'/>
<tpm model='tpm-tis'>
- <backend type='emulator' version='2.0'/>
+ <backend type='emulator' version='2.0' active_pcr_banks='sha256,sha512'/>
</tpm>
<memballoon model='virtio'/>
</devices>
diff --git a/tests/qemuxml2xmloutdata/tpm-emulator-tpm2.x86_64-latest.xml b/tests/qemuxml2xmloutdata/tpm-emulator-tpm2.x86_64-latest.xml
index fe4e1aba19..2488f6ad29 100644
--- a/tests/qemuxml2xmloutdata/tpm-emulator-tpm2.x86_64-latest.xml
+++ b/tests/qemuxml2xmloutdata/tpm-emulator-tpm2.x86_64-latest.xml
@@ -28,7 +28,7 @@
<input type='mouse' bus='ps2'/>
<input type='keyboard' bus='ps2'/>
<tpm model='tpm-tis'>
- <backend type='emulator' version='2.0'/>
+ <backend type='emulator' version='2.0' active_pcr_banks='sha256,sha512'/>
</tpm>
<audio id='1' type='none'/>
<memballoon model='virtio'>
--
2.31.1
Hi
On Mon, Nov 1, 2021 at 9:23 PM Stefan Berger <stefanb@linux.ibm.com> wrote:
>
> Extend the TPM domain XML with an attribute active_pcr_banks that allows
> a user to specify the PCR banks to activate before starting a VM. A comma-
> separated list of PCR banks with the choices of sha1, sha256, sha384 and
> sha512 is allowed. When the XML attribute is provided, the set of active
> PCR banks is 'enforced' by running swtpm_setup before every start of the
> VM. The activation requires that swtpm_setup v0.7 or later is installed
> and may not have any effect otherwise.
>
Is this a configuration switch that the guest is expected to handle in general?
On real hw (or ftpm), is there some bios option or equivalent to
configure the pcr banks?
If not, shouldn't this be a first-time only configuration? (and
attempts to change the value further be rejected by libvirt)
> <tpm model='tpm-tis'>
> <backend type='emulator' version='2.0' active_pcr_banks='sha256,sha384'/>
> </tpm>
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2016599
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
> docs/formatdomain.rst | 12 ++-
> docs/schemas/basictypes.rng | 6 ++
> docs/schemas/domaincommon.rng | 5 ++
> src/conf/domain_conf.c | 21 ++++-
> src/conf/domain_conf.h | 1 +
> src/qemu/qemu_tpm.c | 80 +++++++++++++++++++
> src/util/virtpm.c | 1 +
> src/util/virtpm.h | 1 +
> tests/qemuxml2argvdata/tpm-emulator-tpm2.xml | 2 +-
> .../tpm-emulator-tpm2.x86_64-latest.xml | 2 +-
> 10 files changed, 127 insertions(+), 4 deletions(-)
>
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index 0651975c88..8785a7a682 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -7537,7 +7537,7 @@ Example: usage of the TPM Emulator
> ...
> <devices>
> <tpm model='tpm-tis'>
> - <backend type='emulator' version='2.0'>
> + <backend type='emulator' version='2.0' active_pcr_banks='sha256'>
> <encryption secret='6dd3e4a5-1d76-44ce-961f-f119f5aad935'/>
> </backend>
> </tpm>
> @@ -7598,6 +7598,16 @@ Example: usage of the TPM Emulator
> This attribute only works with the ``emulator`` backend. The accepted values
> are ``yes`` and ``no``. :since:`Since 7.0.0`
>
> +``active_pcr_banks``
> + The ``active_pcr_banks`` attribute indicates the names of the PCR banks
> + of a TPM 2.0 to activate. A comma separated list of PCR banks' names
> + must be provided. Valid names are for example sha1, sha256, sha384, and
> + sha512. If this attribute is provided, the set of PCR banks are activated
> + before every start of a VM and this step is logged in the swtpm's log.
> + This attribute requires that swtpm_setup v0.7 or later is installed
> + and may not have any effect otherwise. This attribute only works with the
> + ``emulator`` backend. since:`Since 7.10.0`
> +
> ``encryption``
> The ``encryption`` element allows the state of a TPM emulator to be
> encrypted. The ``secret`` must reference a secret object that holds the
> diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
> index a221ff6295..3bd1eebdc4 100644
> --- a/docs/schemas/basictypes.rng
> +++ b/docs/schemas/basictypes.rng
> @@ -88,6 +88,12 @@
> </choice>
> </define>
>
> + <define name="pcrBankList">
> + <data type="string">
> + <param name="pattern">(sha1|sha256|sha384|sha512){1}(,(sha1|sha256|sha384|sha512)){0,3}</param>
> + </data>
> + </define>
> +
> <define name="numaDistanceValue">
> <data type="unsignedInt">
> <param name="minInclusive">10</param>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 67df13d90d..6801673cf1 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -5331,6 +5331,11 @@
> </choice>
> </attribute>
> </optional>
> + <optional>
> + <attribute name="active_pcr_banks">
> + <ref name="pcrBankList"/>
> + </attribute>
> + </optional>
> </group>
> </choice>
> <optional>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 4644d18120..bc8237fd0b 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -3207,6 +3207,7 @@ void virDomainTPMDefFree(virDomainTPMDef *def)
> break;
> case VIR_DOMAIN_TPM_TYPE_EMULATOR:
> virDomainChrSourceDefClear(&def->data.emulator.source);
> + g_free(def->data.emulator.activePcrBanks);
> g_free(def->data.emulator.storagepath);
> g_free(def->data.emulator.logfile);
> break;
> @@ -11733,7 +11734,7 @@ virDomainSmartcardDefParseXML(virDomainXMLOption *xmlopt,
> * Emulator state encryption is supported with the following:
> *
> * <tpm model='tpm-tis'>
> - * <backend type='emulator' version='2.0'>
> + * <backend type='emulator' version='2.0' active_pcr_banks='sha256,sha384'>
> * <encryption secret='32ee7e76-2178-47a1-ab7b-269e6e348015'/>
> * </backend>
> * </tpm>
> @@ -11759,6 +11760,7 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
> g_autofree char *version = NULL;
> g_autofree char *secretuuid = NULL;
> g_autofree char *persistent_state = NULL;
> + g_autofree char *activePcrBanks = NULL;
> g_autofree xmlNodePtr *backends = NULL;
>
> def = g_new0(virDomainTPMDef, 1);
> @@ -11841,6 +11843,18 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
> goto error;
> }
> }
> + if (def->version == VIR_DOMAIN_TPM_VERSION_2_0) {
> + activePcrBanks = virXMLPropString(backends[0], "active_pcr_banks");
> + if (activePcrBanks) {
> + if (!virStringMatch(activePcrBanks,
> + "(sha1|sha256|sha384|sha512)(,(sha1|sha256|sha384|sha512)){0,3}")) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Malformatted list of PCR banks"));
> + goto error;
> + }
> + def->data.emulator.activePcrBanks = g_steal_pointer(&activePcrBanks);
> + }
> + }
> break;
> case VIR_DOMAIN_TPM_TYPE_LAST:
> goto error;
> @@ -25433,6 +25447,11 @@ virDomainTPMDefFormat(virBuffer *buf,
> virDomainTPMVersionTypeToString(def->version));
> if (def->data.emulator.persistent_state)
> virBufferAddLit(buf, " persistent_state='yes'");
> + if (def->version == VIR_DOMAIN_TPM_VERSION_2_0 &&
> + def->data.emulator.activePcrBanks) {
> + virBufferAsprintf(buf, " active_pcr_banks='%s'",
> + def->data.emulator.activePcrBanks);
> + }
> if (def->data.emulator.hassecretuuid) {
> char uuidstr[VIR_UUID_STRING_BUFLEN];
> virBufferAddLit(buf, ">\n");
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index cb6d8975b8..19597dba7e 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1381,6 +1381,7 @@ struct _virDomainTPMDef {
> unsigned char secretuuid[VIR_UUID_BUFLEN];
> bool hassecretuuid;
> bool persistent_state;
> + char *activePcrBanks;
> } emulator;
> } data;
> };
> diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
> index 93cb04f49d..bb14228edc 100644
> --- a/src/qemu/qemu_tpm.c
> +++ b/src/qemu/qemu_tpm.c
> @@ -566,6 +566,78 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
> }
>
>
> +/*
> + * qemuTPMEmulatorReconfigure
> + *
> + *
> + * @storagepath: path to the directory for TPM state
> + * @swtpm_user: The userid to switch to when setting up the TPM;
> + * typically this should be the uid of 'tss' or 'root'
> + * @swtpm_group: The group id to switch to
> + * @swtpmActivePcrBanks: The string describing the active PCR banks
> + * @logfile: The file to write the log into; it must be writable
> + * for the user given by userid or 'tss'
> + * @tpmversion: The version of the TPM, either a TPM 1.2 or TPM 2
> + * @secretuuid: The secret's UUID needed for state encryption
> + *
> + * Reconfigure the active PCR banks of a TPM 2.
> + */
> +static int
> +qemuTPMEmulatorReconfigure(const char *storagepath,
> + uid_t swtpm_user,
> + gid_t swtpm_group,
> + const char *swtpmActivePcrBanks,
> + const char *logfile,
> + const virDomainTPMVersion tpmversion,
> + const unsigned char *secretuuid)
> +{
> + g_autoptr(virCommand) cmd = NULL;
> + int exitstatus;
> + g_autofree char *swtpm_setup = virTPMGetSwtpmSetup();
> + VIR_AUTOCLOSE pwdfile_fd = -1;
> +
> + if (!swtpm_setup)
> + return -1;
> +
> + if (tpmversion != VIR_DOMAIN_TPM_VERSION_2_0 ||
> + swtpmActivePcrBanks == NULL ||
> + !virTPMSwtpmSetupCapsGet(
> + VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_RECONFIGURE_PCR_BANKS))
> + return 0;
> +
> + cmd = virCommandNew(swtpm_setup);
> + if (!cmd)
> + return -1;
> +
> + virCommandSetUID(cmd, swtpm_user);
> + virCommandSetGID(cmd, swtpm_group);
> +
> + virCommandAddArgList(cmd, "--tpm2", NULL);
> +
> + if (qemuTPMVirCommandAddEncryption(cmd, swtpm_setup, secretuuid) < 0)
> + return -1;
> +
> + virCommandAddArgList(cmd,
> + "--tpm-state", storagepath,
> + "--logfile", logfile,
> + "--pcr-banks", swtpmActivePcrBanks,
> + "--reconfigure",
> + NULL);
> +
> + virCommandClearCaps(cmd);
> +
> + if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Could not run '%s --reconfigure'. exitstatus: %d; "
> + "Check error log '%s' for details."),
> + swtpm_setup, exitstatus, logfile);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +
> /*
> * qemuTPMEmulatorBuildCommand:
> *
> @@ -620,6 +692,14 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
> secretuuid, incomingMigration) < 0)
> goto error;
>
> + if (!incomingMigration &&
> + qemuTPMEmulatorReconfigure(tpm->data.emulator.storagepath,
> + swtpm_user, swtpm_group,
> + tpm->data.emulator.activePcrBanks,
> + tpm->data.emulator.logfile, tpm->version,
> + secretuuid) < 0)
> + goto error;
> +
> unlink(tpm->data.emulator.source.data.nix.path);
>
> cmd = virCommandNew(swtpm);
> diff --git a/src/util/virtpm.c b/src/util/virtpm.c
> index 40d9272e66..7fa870b803 100644
> --- a/src/util/virtpm.c
> +++ b/src/util/virtpm.c
> @@ -47,6 +47,7 @@ VIR_ENUM_IMPL(virTPMSwtpmSetupFeature,
> "cmdarg-pwdfile-fd",
> "cmdarg-create-config-files",
> "tpm12-not-need-root",
> + "cmdarg-reconfigure-pcr-banks",
> );
>
> /**
> diff --git a/src/util/virtpm.h b/src/util/virtpm.h
> index b75eb84f31..defea6c106 100644
> --- a/src/util/virtpm.h
> +++ b/src/util/virtpm.h
> @@ -40,6 +40,7 @@ typedef enum {
> VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PWDFILE_FD,
> VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_CREATE_CONFIG_FILES,
> VIR_TPM_SWTPM_SETUP_FEATURE_TPM12_NOT_NEED_ROOT,
> + VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_RECONFIGURE_PCR_BANKS,
>
> VIR_TPM_SWTPM_SETUP_FEATURE_LAST
> } virTPMSwtpmSetupFeature;
> diff --git a/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml b/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml
> index 3e2f485ee7..ca9b38540d 100644
> --- a/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml
> +++ b/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml
> @@ -23,7 +23,7 @@
> <input type='mouse' bus='ps2'/>
> <input type='keyboard' bus='ps2'/>
> <tpm model='tpm-tis'>
> - <backend type='emulator' version='2.0'/>
> + <backend type='emulator' version='2.0' active_pcr_banks='sha256,sha512'/>
> </tpm>
> <memballoon model='virtio'/>
> </devices>
> diff --git a/tests/qemuxml2xmloutdata/tpm-emulator-tpm2.x86_64-latest.xml b/tests/qemuxml2xmloutdata/tpm-emulator-tpm2.x86_64-latest.xml
> index fe4e1aba19..2488f6ad29 100644
> --- a/tests/qemuxml2xmloutdata/tpm-emulator-tpm2.x86_64-latest.xml
> +++ b/tests/qemuxml2xmloutdata/tpm-emulator-tpm2.x86_64-latest.xml
> @@ -28,7 +28,7 @@
> <input type='mouse' bus='ps2'/>
> <input type='keyboard' bus='ps2'/>
> <tpm model='tpm-tis'>
> - <backend type='emulator' version='2.0'/>
> + <backend type='emulator' version='2.0' active_pcr_banks='sha256,sha512'/>
> </tpm>
> <audio id='1' type='none'/>
> <memballoon model='virtio'>
> --
> 2.31.1
>
lgtm otherwise
On 11/2/21 04:43, Marc-André Lureau wrote:
> Hi
>
> On Mon, Nov 1, 2021 at 9:23 PM Stefan Berger <stefanb@linux.ibm.com> wrote:
>> Extend the TPM domain XML with an attribute active_pcr_banks that allows
>> a user to specify the PCR banks to activate before starting a VM. A comma-
>> separated list of PCR banks with the choices of sha1, sha256, sha384 and
>> sha512 is allowed. When the XML attribute is provided, the set of active
>> PCR banks is 'enforced' by running swtpm_setup before every start of the
>> VM. The activation requires that swtpm_setup v0.7 or later is installed
>> and may not have any effect otherwise.
>>
> Is this a configuration switch that the guest is expected to handle in general?
I am not sure what you mean. swtpm_setup would run with the (new)
--reconfigure option every time when the XML indicates which PCR banks
to activate. The user wouldn't have to go through the process of
changing the PCR banks.
>
> On real hw (or ftpm), is there some bios option or equivalent to
> configure the pcr banks?
Yes there typically is a menu item to let you change the PCR banks. We
have that also on UEFI, SeaBIOS, and SLOF (ppc64).
>
> If not, shouldn't this be a first-time only configuration? (and
> attempts to change the value further be rejected by libvirt)
What drove this change to make this a lot more flexible than having this
a first-time only configuration was the fact that it seemed a more
user-friendly to implement it this way than checking whether the swtpm
storage directory exists already indicating that the VM had been started
before and swtpm_setup ran on first start to then either deny (storage
dir exists already) or allow (storage dir doesn't exists yet) the user
to re-configure the pcr banks by editing the domain XML. So I now allow
the user to always edit the domain XML and run 'swtpm_setup
--reconfigure' every time... We don't have configuration entries in the
XML that are 'stuck' all of a sudden and reject edits.
Michal writes in the other message:
(yes, as horrible as it sounds you can 'virsh define dom1.xml && virsh create dom2.xml' where dom1.xml and dom2.xml have nothing in common except domain <name/> and <uuid/>)
If the PCR banks config in the domain XML was to 'get stuck' then it could prevent this sequence of define/create IF the selected PCR banks were different.
Stefan
>
>> <tpm model='tpm-tis'>
>> <backend type='emulator' version='2.0' active_pcr_banks='sha256,sha384'/>
>> </tpm>
>>
>> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2016599
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>> ---
>> docs/formatdomain.rst | 12 ++-
>> docs/schemas/basictypes.rng | 6 ++
>> docs/schemas/domaincommon.rng | 5 ++
>> src/conf/domain_conf.c | 21 ++++-
>> src/conf/domain_conf.h | 1 +
>> src/qemu/qemu_tpm.c | 80 +++++++++++++++++++
>> src/util/virtpm.c | 1 +
>> src/util/virtpm.h | 1 +
>> tests/qemuxml2argvdata/tpm-emulator-tpm2.xml | 2 +-
>> .../tpm-emulator-tpm2.x86_64-latest.xml | 2 +-
>> 10 files changed, 127 insertions(+), 4 deletions(-)
>>
>> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
>> index 0651975c88..8785a7a682 100644
>> --- a/docs/formatdomain.rst
>> +++ b/docs/formatdomain.rst
>> @@ -7537,7 +7537,7 @@ Example: usage of the TPM Emulator
>> ...
>> <devices>
>> <tpm model='tpm-tis'>
>> - <backend type='emulator' version='2.0'>
>> + <backend type='emulator' version='2.0' active_pcr_banks='sha256'>
>> <encryption secret='6dd3e4a5-1d76-44ce-961f-f119f5aad935'/>
>> </backend>
>> </tpm>
>> @@ -7598,6 +7598,16 @@ Example: usage of the TPM Emulator
>> This attribute only works with the ``emulator`` backend. The accepted values
>> are ``yes`` and ``no``. :since:`Since 7.0.0`
>>
>> +``active_pcr_banks``
>> + The ``active_pcr_banks`` attribute indicates the names of the PCR banks
>> + of a TPM 2.0 to activate. A comma separated list of PCR banks' names
>> + must be provided. Valid names are for example sha1, sha256, sha384, and
>> + sha512. If this attribute is provided, the set of PCR banks are activated
>> + before every start of a VM and this step is logged in the swtpm's log.
>> + This attribute requires that swtpm_setup v0.7 or later is installed
>> + and may not have any effect otherwise. This attribute only works with the
>> + ``emulator`` backend. since:`Since 7.10.0`
>> +
>> ``encryption``
>> The ``encryption`` element allows the state of a TPM emulator to be
>> encrypted. The ``secret`` must reference a secret object that holds the
>> diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
>> index a221ff6295..3bd1eebdc4 100644
>> --- a/docs/schemas/basictypes.rng
>> +++ b/docs/schemas/basictypes.rng
>> @@ -88,6 +88,12 @@
>> </choice>
>> </define>
>>
>> + <define name="pcrBankList">
>> + <data type="string">
>> + <param name="pattern">(sha1|sha256|sha384|sha512){1}(,(sha1|sha256|sha384|sha512)){0,3}</param>
>> + </data>
>> + </define>
>> +
>> <define name="numaDistanceValue">
>> <data type="unsignedInt">
>> <param name="minInclusive">10</param>
>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>> index 67df13d90d..6801673cf1 100644
>> --- a/docs/schemas/domaincommon.rng
>> +++ b/docs/schemas/domaincommon.rng
>> @@ -5331,6 +5331,11 @@
>> </choice>
>> </attribute>
>> </optional>
>> + <optional>
>> + <attribute name="active_pcr_banks">
>> + <ref name="pcrBankList"/>
>> + </attribute>
>> + </optional>
>> </group>
>> </choice>
>> <optional>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 4644d18120..bc8237fd0b 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -3207,6 +3207,7 @@ void virDomainTPMDefFree(virDomainTPMDef *def)
>> break;
>> case VIR_DOMAIN_TPM_TYPE_EMULATOR:
>> virDomainChrSourceDefClear(&def->data.emulator.source);
>> + g_free(def->data.emulator.activePcrBanks);
>> g_free(def->data.emulator.storagepath);
>> g_free(def->data.emulator.logfile);
>> break;
>> @@ -11733,7 +11734,7 @@ virDomainSmartcardDefParseXML(virDomainXMLOption *xmlopt,
>> * Emulator state encryption is supported with the following:
>> *
>> * <tpm model='tpm-tis'>
>> - * <backend type='emulator' version='2.0'>
>> + * <backend type='emulator' version='2.0' active_pcr_banks='sha256,sha384'>
>> * <encryption secret='32ee7e76-2178-47a1-ab7b-269e6e348015'/>
>> * </backend>
>> * </tpm>
>> @@ -11759,6 +11760,7 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
>> g_autofree char *version = NULL;
>> g_autofree char *secretuuid = NULL;
>> g_autofree char *persistent_state = NULL;
>> + g_autofree char *activePcrBanks = NULL;
>> g_autofree xmlNodePtr *backends = NULL;
>>
>> def = g_new0(virDomainTPMDef, 1);
>> @@ -11841,6 +11843,18 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
>> goto error;
>> }
>> }
>> + if (def->version == VIR_DOMAIN_TPM_VERSION_2_0) {
>> + activePcrBanks = virXMLPropString(backends[0], "active_pcr_banks");
>> + if (activePcrBanks) {
>> + if (!virStringMatch(activePcrBanks,
>> + "(sha1|sha256|sha384|sha512)(,(sha1|sha256|sha384|sha512)){0,3}")) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> + _("Malformatted list of PCR banks"));
>> + goto error;
>> + }
>> + def->data.emulator.activePcrBanks = g_steal_pointer(&activePcrBanks);
>> + }
>> + }
>> break;
>> case VIR_DOMAIN_TPM_TYPE_LAST:
>> goto error;
>> @@ -25433,6 +25447,11 @@ virDomainTPMDefFormat(virBuffer *buf,
>> virDomainTPMVersionTypeToString(def->version));
>> if (def->data.emulator.persistent_state)
>> virBufferAddLit(buf, " persistent_state='yes'");
>> + if (def->version == VIR_DOMAIN_TPM_VERSION_2_0 &&
>> + def->data.emulator.activePcrBanks) {
>> + virBufferAsprintf(buf, " active_pcr_banks='%s'",
>> + def->data.emulator.activePcrBanks);
>> + }
>> if (def->data.emulator.hassecretuuid) {
>> char uuidstr[VIR_UUID_STRING_BUFLEN];
>> virBufferAddLit(buf, ">\n");
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index cb6d8975b8..19597dba7e 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -1381,6 +1381,7 @@ struct _virDomainTPMDef {
>> unsigned char secretuuid[VIR_UUID_BUFLEN];
>> bool hassecretuuid;
>> bool persistent_state;
>> + char *activePcrBanks;
>> } emulator;
>> } data;
>> };
>> diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
>> index 93cb04f49d..bb14228edc 100644
>> --- a/src/qemu/qemu_tpm.c
>> +++ b/src/qemu/qemu_tpm.c
>> @@ -566,6 +566,78 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
>> }
>>
>>
>> +/*
>> + * qemuTPMEmulatorReconfigure
>> + *
>> + *
>> + * @storagepath: path to the directory for TPM state
>> + * @swtpm_user: The userid to switch to when setting up the TPM;
>> + * typically this should be the uid of 'tss' or 'root'
>> + * @swtpm_group: The group id to switch to
>> + * @swtpmActivePcrBanks: The string describing the active PCR banks
>> + * @logfile: The file to write the log into; it must be writable
>> + * for the user given by userid or 'tss'
>> + * @tpmversion: The version of the TPM, either a TPM 1.2 or TPM 2
>> + * @secretuuid: The secret's UUID needed for state encryption
>> + *
>> + * Reconfigure the active PCR banks of a TPM 2.
>> + */
>> +static int
>> +qemuTPMEmulatorReconfigure(const char *storagepath,
>> + uid_t swtpm_user,
>> + gid_t swtpm_group,
>> + const char *swtpmActivePcrBanks,
>> + const char *logfile,
>> + const virDomainTPMVersion tpmversion,
>> + const unsigned char *secretuuid)
>> +{
>> + g_autoptr(virCommand) cmd = NULL;
>> + int exitstatus;
>> + g_autofree char *swtpm_setup = virTPMGetSwtpmSetup();
>> + VIR_AUTOCLOSE pwdfile_fd = -1;
>> +
>> + if (!swtpm_setup)
>> + return -1;
>> +
>> + if (tpmversion != VIR_DOMAIN_TPM_VERSION_2_0 ||
>> + swtpmActivePcrBanks == NULL ||
>> + !virTPMSwtpmSetupCapsGet(
>> + VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_RECONFIGURE_PCR_BANKS))
>> + return 0;
>> +
>> + cmd = virCommandNew(swtpm_setup);
>> + if (!cmd)
>> + return -1;
>> +
>> + virCommandSetUID(cmd, swtpm_user);
>> + virCommandSetGID(cmd, swtpm_group);
>> +
>> + virCommandAddArgList(cmd, "--tpm2", NULL);
>> +
>> + if (qemuTPMVirCommandAddEncryption(cmd, swtpm_setup, secretuuid) < 0)
>> + return -1;
>> +
>> + virCommandAddArgList(cmd,
>> + "--tpm-state", storagepath,
>> + "--logfile", logfile,
>> + "--pcr-banks", swtpmActivePcrBanks,
>> + "--reconfigure",
>> + NULL);
>> +
>> + virCommandClearCaps(cmd);
>> +
>> + if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("Could not run '%s --reconfigure'. exitstatus: %d; "
>> + "Check error log '%s' for details."),
>> + swtpm_setup, exitstatus, logfile);
>> + return -1;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +
>> /*
>> * qemuTPMEmulatorBuildCommand:
>> *
>> @@ -620,6 +692,14 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
>> secretuuid, incomingMigration) < 0)
>> goto error;
>>
>> + if (!incomingMigration &&
>> + qemuTPMEmulatorReconfigure(tpm->data.emulator.storagepath,
>> + swtpm_user, swtpm_group,
>> + tpm->data.emulator.activePcrBanks,
>> + tpm->data.emulator.logfile, tpm->version,
>> + secretuuid) < 0)
>> + goto error;
>> +
>> unlink(tpm->data.emulator.source.data.nix.path);
>>
>> cmd = virCommandNew(swtpm);
>> diff --git a/src/util/virtpm.c b/src/util/virtpm.c
>> index 40d9272e66..7fa870b803 100644
>> --- a/src/util/virtpm.c
>> +++ b/src/util/virtpm.c
>> @@ -47,6 +47,7 @@ VIR_ENUM_IMPL(virTPMSwtpmSetupFeature,
>> "cmdarg-pwdfile-fd",
>> "cmdarg-create-config-files",
>> "tpm12-not-need-root",
>> + "cmdarg-reconfigure-pcr-banks",
>> );
>>
>> /**
>> diff --git a/src/util/virtpm.h b/src/util/virtpm.h
>> index b75eb84f31..defea6c106 100644
>> --- a/src/util/virtpm.h
>> +++ b/src/util/virtpm.h
>> @@ -40,6 +40,7 @@ typedef enum {
>> VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PWDFILE_FD,
>> VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_CREATE_CONFIG_FILES,
>> VIR_TPM_SWTPM_SETUP_FEATURE_TPM12_NOT_NEED_ROOT,
>> + VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_RECONFIGURE_PCR_BANKS,
>>
>> VIR_TPM_SWTPM_SETUP_FEATURE_LAST
>> } virTPMSwtpmSetupFeature;
>> diff --git a/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml b/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml
>> index 3e2f485ee7..ca9b38540d 100644
>> --- a/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml
>> +++ b/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml
>> @@ -23,7 +23,7 @@
>> <input type='mouse' bus='ps2'/>
>> <input type='keyboard' bus='ps2'/>
>> <tpm model='tpm-tis'>
>> - <backend type='emulator' version='2.0'/>
>> + <backend type='emulator' version='2.0' active_pcr_banks='sha256,sha512'/>
>> </tpm>
>> <memballoon model='virtio'/>
>> </devices>
>> diff --git a/tests/qemuxml2xmloutdata/tpm-emulator-tpm2.x86_64-latest.xml b/tests/qemuxml2xmloutdata/tpm-emulator-tpm2.x86_64-latest.xml
>> index fe4e1aba19..2488f6ad29 100644
>> --- a/tests/qemuxml2xmloutdata/tpm-emulator-tpm2.x86_64-latest.xml
>> +++ b/tests/qemuxml2xmloutdata/tpm-emulator-tpm2.x86_64-latest.xml
>> @@ -28,7 +28,7 @@
>> <input type='mouse' bus='ps2'/>
>> <input type='keyboard' bus='ps2'/>
>> <tpm model='tpm-tis'>
>> - <backend type='emulator' version='2.0'/>
>> + <backend type='emulator' version='2.0' active_pcr_banks='sha256,sha512'/>
>> </tpm>
>> <audio id='1' type='none'/>
>> <memballoon model='virtio'>
>> --
>> 2.31.1
>>
> lgtm otherwise
>
On 11/1/21 6:23 PM, Stefan Berger wrote:
> Extend the TPM domain XML with an attribute active_pcr_banks that allows
> a user to specify the PCR banks to activate before starting a VM. A comma-
> separated list of PCR banks with the choices of sha1, sha256, sha384 and
> sha512 is allowed. When the XML attribute is provided, the set of active
> PCR banks is 'enforced' by running swtpm_setup before every start of the
> VM. The activation requires that swtpm_setup v0.7 or later is installed
> and may not have any effect otherwise.
>
> <tpm model='tpm-tis'>
> <backend type='emulator' version='2.0' active_pcr_banks='sha256,sha384'/>
> </tpm>
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2016599
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
> docs/formatdomain.rst | 12 ++-
> docs/schemas/basictypes.rng | 6 ++
> docs/schemas/domaincommon.rng | 5 ++
> src/conf/domain_conf.c | 21 ++++-
> src/conf/domain_conf.h | 1 +
> src/qemu/qemu_tpm.c | 80 +++++++++++++++++++
> src/util/virtpm.c | 1 +
> src/util/virtpm.h | 1 +
> tests/qemuxml2argvdata/tpm-emulator-tpm2.xml | 2 +-
> .../tpm-emulator-tpm2.x86_64-latest.xml | 2 +-
> 10 files changed, 127 insertions(+), 4 deletions(-)
>
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index 0651975c88..8785a7a682 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -7537,7 +7537,7 @@ Example: usage of the TPM Emulator
> ...
> <devices>
> <tpm model='tpm-tis'>
> - <backend type='emulator' version='2.0'>
> + <backend type='emulator' version='2.0' active_pcr_banks='sha256'>
> <encryption secret='6dd3e4a5-1d76-44ce-961f-f119f5aad935'/>
> </backend>
> </tpm>
> @@ -7598,6 +7598,16 @@ Example: usage of the TPM Emulator
> This attribute only works with the ``emulator`` backend. The accepted values
> are ``yes`` and ``no``. :since:`Since 7.0.0`
>
> +``active_pcr_banks``
> + The ``active_pcr_banks`` attribute indicates the names of the PCR banks
> + of a TPM 2.0 to activate. A comma separated list of PCR banks' names
> + must be provided. Valid names are for example sha1, sha256, sha384, and
> + sha512. If this attribute is provided, the set of PCR banks are activated
> + before every start of a VM and this step is logged in the swtpm's log.
> + This attribute requires that swtpm_setup v0.7 or later is installed
> + and may not have any effect otherwise. This attribute only works with the
> + ``emulator`` backend. since:`Since 7.10.0`
> +
> ``encryption``
> The ``encryption`` element allows the state of a TPM emulator to be
> encrypted. The ``secret`` must reference a secret object that holds the
> diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
> index a221ff6295..3bd1eebdc4 100644
> --- a/docs/schemas/basictypes.rng
> +++ b/docs/schemas/basictypes.rng
> @@ -88,6 +88,12 @@
> </choice>
> </define>
>
> + <define name="pcrBankList">
> + <data type="string">
> + <param name="pattern">(sha1|sha256|sha384|sha512){1}(,(sha1|sha256|sha384|sha512)){0,3}</param>
> + </data>
> + </define>
> +
Honestly, I'm not a big fan of comma separated lists. I think we could
do with nested elements, repeated for each option. But I'll let others
decide that.
> <define name="numaDistanceValue">
> <data type="unsignedInt">
> <param name="minInclusive">10</param>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 67df13d90d..6801673cf1 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -5331,6 +5331,11 @@
> </choice>
> </attribute>
> </optional>
> + <optional>
> + <attribute name="active_pcr_banks">
> + <ref name="pcrBankList"/>
> + </attribute>
> + </optional>
> </group>
> </choice>
> <optional>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 4644d18120..bc8237fd0b 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -3207,6 +3207,7 @@ void virDomainTPMDefFree(virDomainTPMDef *def)
> break;
> case VIR_DOMAIN_TPM_TYPE_EMULATOR:
> virDomainChrSourceDefClear(&def->data.emulator.source);
> + g_free(def->data.emulator.activePcrBanks);
> g_free(def->data.emulator.storagepath);
> g_free(def->data.emulator.logfile);
> break;
> @@ -11733,7 +11734,7 @@ virDomainSmartcardDefParseXML(virDomainXMLOption *xmlopt,
> * Emulator state encryption is supported with the following:
> *
> * <tpm model='tpm-tis'>
> - * <backend type='emulator' version='2.0'>
> + * <backend type='emulator' version='2.0' active_pcr_banks='sha256,sha384'>
> * <encryption secret='32ee7e76-2178-47a1-ab7b-269e6e348015'/>
> * </backend>
> * </tpm>
> @@ -11759,6 +11760,7 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
> g_autofree char *version = NULL;
> g_autofree char *secretuuid = NULL;
> g_autofree char *persistent_state = NULL;
> + g_autofree char *activePcrBanks = NULL;
> g_autofree xmlNodePtr *backends = NULL;
>
> def = g_new0(virDomainTPMDef, 1);
> @@ -11841,6 +11843,18 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
> goto error;
> }
> }
> + if (def->version == VIR_DOMAIN_TPM_VERSION_2_0) {
> + activePcrBanks = virXMLPropString(backends[0], "active_pcr_banks");
> + if (activePcrBanks) {
> + if (!virStringMatch(activePcrBanks,
> + "(sha1|sha256|sha384|sha512)(,(sha1|sha256|sha384|sha512)){0,3}")) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Malformatted list of PCR banks"));
> + goto error;
> + }
> + def->data.emulator.activePcrBanks = g_steal_pointer(&activePcrBanks);
> + }
> + }
> break;
> case VIR_DOMAIN_TPM_TYPE_LAST:
> goto error;
> @@ -25433,6 +25447,11 @@ virDomainTPMDefFormat(virBuffer *buf,
> virDomainTPMVersionTypeToString(def->version));
> if (def->data.emulator.persistent_state)
> virBufferAddLit(buf, " persistent_state='yes'");
> + if (def->version == VIR_DOMAIN_TPM_VERSION_2_0 &&
> + def->data.emulator.activePcrBanks) {
Technically, if version != 2.0 then parser didn't even bother parsing
and activePCrBanks would be NULL.
> + virBufferAsprintf(buf, " active_pcr_banks='%s'",
> + def->data.emulator.activePcrBanks);
> + }
> if (def->data.emulator.hassecretuuid) {
> char uuidstr[VIR_UUID_STRING_BUFLEN];
> virBufferAddLit(buf, ">\n");
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index cb6d8975b8..19597dba7e 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1381,6 +1381,7 @@ struct _virDomainTPMDef {
> unsigned char secretuuid[VIR_UUID_BUFLEN];
> bool hassecretuuid;
> bool persistent_state;
> + char *activePcrBanks;
> } emulator;
> } data;
> };
> diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
> index 93cb04f49d..bb14228edc 100644
> --- a/src/qemu/qemu_tpm.c
> +++ b/src/qemu/qemu_tpm.c
> @@ -566,6 +566,78 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
> }
>
>
> +/*
> + * qemuTPMEmulatorReconfigure
> + *
> + *
> + * @storagepath: path to the directory for TPM state
> + * @swtpm_user: The userid to switch to when setting up the TPM;
> + * typically this should be the uid of 'tss' or 'root'
> + * @swtpm_group: The group id to switch to
> + * @swtpmActivePcrBanks: The string describing the active PCR banks
> + * @logfile: The file to write the log into; it must be writable
> + * for the user given by userid or 'tss'
> + * @tpmversion: The version of the TPM, either a TPM 1.2 or TPM 2
> + * @secretuuid: The secret's UUID needed for state encryption
> + *
> + * Reconfigure the active PCR banks of a TPM 2.
> + */
> +static int
> +qemuTPMEmulatorReconfigure(const char *storagepath,
> + uid_t swtpm_user,
> + gid_t swtpm_group,
> + const char *swtpmActivePcrBanks,
> + const char *logfile,
> + const virDomainTPMVersion tpmversion,
> + const unsigned char *secretuuid)
> +{
> + g_autoptr(virCommand) cmd = NULL;
> + int exitstatus;
> + g_autofree char *swtpm_setup = virTPMGetSwtpmSetup();
> + VIR_AUTOCLOSE pwdfile_fd = -1;
> +
> + if (!swtpm_setup)
> + return -1;
> +
> + if (tpmversion != VIR_DOMAIN_TPM_VERSION_2_0 ||
> + swtpmActivePcrBanks == NULL ||
> + !virTPMSwtpmSetupCapsGet(
> + VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_RECONFIGURE_PCR_BANKS))
> + return 0;
> +
> + cmd = virCommandNew(swtpm_setup);
> + if (!cmd)
> + return -1;
> +
> + virCommandSetUID(cmd, swtpm_user);
> + virCommandSetGID(cmd, swtpm_group);
> +
> + virCommandAddArgList(cmd, "--tpm2", NULL);
> +
> + if (qemuTPMVirCommandAddEncryption(cmd, swtpm_setup, secretuuid) < 0)
> + return -1;
> +
> + virCommandAddArgList(cmd,
> + "--tpm-state", storagepath,
> + "--logfile", logfile,
> + "--pcr-banks", swtpmActivePcrBanks,
> + "--reconfigure",
> + NULL);
> +
> + virCommandClearCaps(cmd);
> +
> + if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Could not run '%s --reconfigure'. exitstatus: %d; "
> + "Check error log '%s' for details."),
> + swtpm_setup, exitstatus, logfile);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +
> /*
> * qemuTPMEmulatorBuildCommand:
> *
> @@ -620,6 +692,14 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
> secretuuid, incomingMigration) < 0)
> goto error;
>
> + if (!incomingMigration &&
> + qemuTPMEmulatorReconfigure(tpm->data.emulator.storagepath,
> + swtpm_user, swtpm_group,
> + tpm->data.emulator.activePcrBanks,
> + tpm->data.emulator.logfile, tpm->version,
> + secretuuid) < 0)
So this runs reconfigure on every cold boot of a guest. I wonder whether
there's a way to run it just once, when activePcrBanks have changed.
For instance, in qemuDomainDefineXMLFlags() the @oldDef is set to the
old domain definition and maybe we can use that to compare
activePcrBanks and run reconfigure at that time? That won't cover
transient domains though, nor it would cover domains which are
persistent but are started with a different XML (yes, as horrible as it
sounds you can 'virsh define dom1.xml && virsh create dom2.xml' where
dom1.xml and dom2.xml have nothing in common except domain <name/> and
<uuid/>).
Maybe I'm just complicating this needlessly, what do you think?
Michal
On Tue, Nov 02, 2021 at 10:38:05AM +0100, Michal Prívozník wrote:
> On 11/1/21 6:23 PM, Stefan Berger wrote:
> > Extend the TPM domain XML with an attribute active_pcr_banks that allows
> > a user to specify the PCR banks to activate before starting a VM. A comma-
> > separated list of PCR banks with the choices of sha1, sha256, sha384 and
> > sha512 is allowed. When the XML attribute is provided, the set of active
> > PCR banks is 'enforced' by running swtpm_setup before every start of the
> > VM. The activation requires that swtpm_setup v0.7 or later is installed
> > and may not have any effect otherwise.
> >
> > <tpm model='tpm-tis'>
> > <backend type='emulator' version='2.0' active_pcr_banks='sha256,sha384'/>
> > </tpm>
> >
> > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2016599
> >
> > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> > ---
> > docs/formatdomain.rst | 12 ++-
> > docs/schemas/basictypes.rng | 6 ++
> > docs/schemas/domaincommon.rng | 5 ++
> > src/conf/domain_conf.c | 21 ++++-
> > src/conf/domain_conf.h | 1 +
> > src/qemu/qemu_tpm.c | 80 +++++++++++++++++++
> > src/util/virtpm.c | 1 +
> > src/util/virtpm.h | 1 +
> > tests/qemuxml2argvdata/tpm-emulator-tpm2.xml | 2 +-
> > .../tpm-emulator-tpm2.x86_64-latest.xml | 2 +-
> > 10 files changed, 127 insertions(+), 4 deletions(-)
> >
> > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> > index 0651975c88..8785a7a682 100644
> > --- a/docs/formatdomain.rst
> > +++ b/docs/formatdomain.rst
> > @@ -7537,7 +7537,7 @@ Example: usage of the TPM Emulator
> > ...
> > <devices>
> > <tpm model='tpm-tis'>
> > - <backend type='emulator' version='2.0'>
> > + <backend type='emulator' version='2.0' active_pcr_banks='sha256'>
> > <encryption secret='6dd3e4a5-1d76-44ce-961f-f119f5aad935'/>
> > </backend>
> > </tpm>
> > @@ -7598,6 +7598,16 @@ Example: usage of the TPM Emulator
> > This attribute only works with the ``emulator`` backend. The accepted values
> > are ``yes`` and ``no``. :since:`Since 7.0.0`
> >
> > +``active_pcr_banks``
> > + The ``active_pcr_banks`` attribute indicates the names of the PCR banks
> > + of a TPM 2.0 to activate. A comma separated list of PCR banks' names
> > + must be provided. Valid names are for example sha1, sha256, sha384, and
> > + sha512. If this attribute is provided, the set of PCR banks are activated
> > + before every start of a VM and this step is logged in the swtpm's log.
> > + This attribute requires that swtpm_setup v0.7 or later is installed
> > + and may not have any effect otherwise. This attribute only works with the
> > + ``emulator`` backend. since:`Since 7.10.0`
> > +
> > ``encryption``
> > The ``encryption`` element allows the state of a TPM emulator to be
> > encrypted. The ``secret`` must reference a secret object that holds the
> > diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
> > index a221ff6295..3bd1eebdc4 100644
> > --- a/docs/schemas/basictypes.rng
> > +++ b/docs/schemas/basictypes.rng
> > @@ -88,6 +88,12 @@
> > </choice>
> > </define>
> >
> > + <define name="pcrBankList">
> > + <data type="string">
> > + <param name="pattern">(sha1|sha256|sha384|sha512){1}(,(sha1|sha256|sha384|sha512)){0,3}</param>
> > + </data>
> > + </define>
> > +
>
> Honestly, I'm not a big fan of comma separated lists. I think we could
> do with nested elements, repeated for each option. But I'll let others
> decide that.
Yes, the golden rule of XML design is that you should not have to write
a second parser to interpret the value of an attribute / element. Any
structure should be represented in the XML design itself.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 11/2/21 05:57, Daniel P. Berrangé wrote: > On Tue, Nov 02, 2021 at 10:38:05AM +0100, Michal Prívozník wrote: > > Yes, the golden rule of XML design is that you should not have to write > a second parser to interpret the value of an attribute / element. Any > structure should be represented in the XML design itself. Like this? <tpm model='tpm-tis'> <backend type='emulator' version='2.0'> <active_pcr_banks> <sha256/> <sha384/> </active_pcr_banks> </backend> </tpm> Regards, Stefan
On Wed, Nov 03, 2021 at 09:07:11AM -0400, Stefan Berger wrote: > > On 11/2/21 05:57, Daniel P. Berrangé wrote: > > On Tue, Nov 02, 2021 at 10:38:05AM +0100, Michal Prívozník wrote: > > > > Yes, the golden rule of XML design is that you should not have to write > > a second parser to interpret the value of an attribute / element. Any > > structure should be represented in the XML design itself. > > Like this? > > <tpm model='tpm-tis'> > <backend type='emulator' version='2.0'> > > <active_pcr_banks> > > <sha256/> > > <sha384/> > > </active_pcr_banks> > > </backend> > > </tpm> Yes, that would be viable and about as concise as you'll get with XML Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 11/2/21 05:38, Michal Prívozník wrote: > On 11/1/21 6:23 PM, Stefan Berger wrote: > > So this runs reconfigure on every cold boot of a guest. I wonder whether > there's a way to run it just once, when activePcrBanks have changed. > For instance, in qemuDomainDefineXMLFlags() the @oldDef is set to the > old domain definition and maybe we can use that to compare > activePcrBanks and run reconfigure at that time? That won't cover > transient domains though, nor it would cover domains which are > persistent but are started with a different XML (yes, as horrible as it > sounds you can 'virsh define dom1.xml && virsh create dom2.xml' where > dom1.xml and dom2.xml have nothing in common except domain <name/> and > <uuid/>). I think to 'enforce' what is shown in the XML is the simplest solution. Whatever the user may have done inside the VM, such as used firmware menu to reconfigure the active PCR banks doesn't matter since what will be enforced next time when the VM is cold-started is what is shown in the XML. Otherwise it's documented how it behaves. Stefan
© 2016 - 2026 Red Hat, Inc.