[PATCH v2 2/2] qemu: tpm: Extend TPM domain XML with PCR banks to activate
Stefan Berger
stefanb at linux.ibm.com
Tue Nov 2 12:54:12 UTC 2021
On 11/2/21 04:43, Marc-André Lureau wrote:
> Hi
>
> On Mon, Nov 1, 2021 at 9:23 PM Stefan Berger <stefanb at 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 at 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
>
More information about the libvir-list
mailing list