[PATCH v4 00/11] swtpm: Add support for profiles
Upcoming libtpms v0.10 and swtpm v0.10 will have TPM profile support that allows to restrict a TPM's provided set of crypto algorithms and commands and through which backwards compatibility and migration from newer versions of libtpms to older ones (up to libtpms v0.9) is supported. For the latter to work it is necessary that the user chooses the right ('null') profile. This series adds support for passing a profile choice to swtpm_setup by setting it in the domain XML using the <profile/> XML node. An optional attribute 'remove_disabled' can be set in this node and accepts two values: "check": test a few crypto algorithms (tdes, camellia, unpadded encryption, and others) for whether they are currently disabled due to FIPS mode on the host and remove these algorithms in the 'custom' profile if they are disabled; "fips-host": do not test but remove all the possibly disabled crypto algorithms (from list above) Also extend the documentation but point the user to swtpm and libtpms documentation for further details. Follow Deniel's suggestions there's now a PR for swtpm_setup to support searching for profiles though a configurable local directory, distro directory and if no profile could be found there (with appended ".json" suffix) it will fall back to try to use a built-in profile by the provided name: https://github.com/stefanberger/swtpm/pull/918 Stefan v4: - Renamed previous 'name' attribute in profile XML node to 'source' to indicate that the profile was created from some sort of 'source'. The 'name' is now set from the name of the profile read from the swtpm instance's state once it has been created. v3: - 2/10: Adjustments to due rebase - Applied Marc-André's R-b tags - 10/10: Read back profile name from swtpm and adjust it in emulator defs Stefan Berger (11): conf: Move TPM emulator parameters into own struct qemu: Pass virQEMUDriverConfig rather than some of its fields util: Add parsing support for swtpm_setup's cmdarg-profile capability conf: Define enum virDomainTPMProfileRemoveDisabled schema: Extend schema for TPM emulator profile node conf: Add support for profile parameter on TPM emulator in domain XML docs: Add documentation for the TPM backend profile node qemu: Extend swtpm_setup command line to set a profile by its name qemu: Move adding of keys to swtpm command line into own function qemu: Move adding --tpmstate to swtpm command line into own function qemu: Read back the profile name after creation of a TPM instance docs/formatdomain.rst | 32 +++ src/conf/domain_conf.c | 47 ++++ src/conf/domain_conf.h | 38 ++-- src/conf/domain_validate.c | 7 + src/conf/schemas/domaincommon.rng | 32 +++ src/conf/virconftypes.h | 2 + src/qemu/qemu_extdevice.c | 5 +- src/qemu/qemu_tpm.c | 344 ++++++++++++++++++++---------- src/qemu/qemu_tpm.h | 3 +- src/util/virtpm.c | 2 + src/util/virtpm.h | 2 + tests/testutilsqemu.c | 1 + 12 files changed, 386 insertions(+), 129 deletions(-) -- 2.47.0
To avoid passing TPM emulator parameters around individually, move them into a structure and pass around the structure. Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- v3: - Made virDomainTPMEmulatorDef first parameter to functions - Applied Marc-André's R-b --- src/conf/domain_conf.h | 26 +++++++++-------- src/conf/virconftypes.h | 2 ++ src/qemu/qemu_tpm.c | 64 ++++++++++++++--------------------------- 3 files changed, 37 insertions(+), 55 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 45c52107e8..08c6526711 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1473,6 +1473,19 @@ typedef enum { #define VIR_DOMAIN_TPM_DEFAULT_DEVICE "/dev/tpm0" +struct _virDomainTPMEmulatorDef { + virDomainTPMVersion version; + virDomainChrSourceDef *source; + virDomainTPMSourceType source_type; + char *source_path; + char *logfile; + unsigned int debug; + unsigned char secretuuid[VIR_UUID_BUFLEN]; + bool hassecretuuid; + bool persistent_state; + virBitmap *activePcrBanks; +}; + struct _virDomainTPMDef { virObject *privateData; @@ -1483,18 +1496,7 @@ struct _virDomainTPMDef { struct { virDomainChrSourceDef *source; } passthrough; - struct { - virDomainTPMVersion version; - virDomainChrSourceDef *source; - virDomainTPMSourceType source_type; - char *source_path; - char *logfile; - unsigned int debug; - unsigned char secretuuid[VIR_UUID_BUFLEN]; - bool hassecretuuid; - bool persistent_state; - virBitmap *activePcrBanks; - } emulator; + virDomainTPMEmulatorDef emulator; struct { virDomainChrSourceDef *source; } external; diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h index f18ebcca10..59be61cea4 100644 --- a/src/conf/virconftypes.h +++ b/src/conf/virconftypes.h @@ -234,6 +234,8 @@ typedef struct _virDomainAudioDef virDomainAudioDef; typedef struct _virDomainTPMDef virDomainTPMDef; +typedef struct _virDomainTPMEmulatorDef virDomainTPMEmulatorDef; + typedef struct _virDomainThreadSchedParam virDomainThreadSchedParam; typedef struct _virDomainTimerCatchupDef virDomainTimerCatchupDef; diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index edd10ca2f6..6d7625f6f4 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -368,33 +368,26 @@ qemuTPMGetSwtpmSetupStateArg(const virDomainTPMSourceType source_type, /* * qemuTPMEmulatorRunSetup * - * @source_type: type of storage - * @source_path: path to the directory for TPM state + * @emulator: emulator parameters * @vmname: the name of the VM * @vmuuid: the UUID of the VM * @privileged: whether we are running in privileged mode * @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 - * @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 - * @encryption: pointer to virStorageEncryption holding secret + * @secretuuid: UUID describing virStorageEncryption holding secret * @incomingMigration: whether we have an incoming migration * * Setup the external swtpm by creating endorsement key and * certificates for it. */ static int -qemuTPMEmulatorRunSetup(const virDomainTPMSourceType source_type, - const char *source_path, +qemuTPMEmulatorRunSetup(const virDomainTPMEmulatorDef *emulator, const char *vmname, const unsigned char *vmuuid, bool privileged, uid_t swtpm_user, gid_t swtpm_group, - const char *logfile, - const virDomainTPMVersion tpmversion, const unsigned char *secretuuid, bool incomingMigration) { @@ -403,14 +396,15 @@ qemuTPMEmulatorRunSetup(const virDomainTPMSourceType source_type, char uuid[VIR_UUID_STRING_BUFLEN]; g_autofree char *vmid = NULL; g_autofree char *swtpm_setup = virTPMGetSwtpmSetup(); - g_autofree char *tpm_state = qemuTPMGetSwtpmSetupStateArg(source_type, source_path); + g_autofree char *tpm_state = qemuTPMGetSwtpmSetupStateArg(emulator->source_type, + emulator->source_path); if (!swtpm_setup) return -1; - if (!privileged && tpmversion == VIR_DOMAIN_TPM_VERSION_1_2 && + if (!privileged && emulator->version == VIR_DOMAIN_TPM_VERSION_1_2 && !virTPMSwtpmSetupCapsGet(VIR_TPM_SWTPM_SETUP_FEATURE_TPM12_NOT_NEED_ROOT)) { - return virFileWriteStr(logfile, + return virFileWriteStr(emulator->logfile, _("Did not create EK and certificates since this requires privileged mode for a TPM 1.2\n"), 0600); } @@ -425,7 +419,7 @@ qemuTPMEmulatorRunSetup(const virDomainTPMSourceType source_type, virCommandSetUID(cmd, swtpm_user); virCommandSetGID(cmd, swtpm_group); - switch (tpmversion) { + switch (emulator->version) { case VIR_DOMAIN_TPM_VERSION_1_2: break; case VIR_DOMAIN_TPM_VERSION_2_0: @@ -443,7 +437,7 @@ qemuTPMEmulatorRunSetup(const virDomainTPMSourceType source_type, virCommandAddArgList(cmd, "--tpm-state", tpm_state, "--vmid", vmid, - "--logfile", logfile, + "--logfile", emulator->logfile, "--createek", "--create-ek-cert", "--create-platform-cert", @@ -453,7 +447,7 @@ qemuTPMEmulatorRunSetup(const virDomainTPMSourceType source_type, } else { virCommandAddArgList(cmd, "--tpm-state", tpm_state, - "--logfile", logfile, + "--logfile", emulator->logfile, "--overwrite", NULL); } @@ -463,7 +457,7 @@ qemuTPMEmulatorRunSetup(const virDomainTPMSourceType source_type, if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not run '%1$s'. exitstatus: %2$d; Check error log '%3$s' for details."), - swtpm_setup, exitstatus, logfile); + swtpm_setup, exitstatus, emulator->logfile); return -1; } @@ -492,41 +486,32 @@ qemuTPMPcrBankBitmapToStr(virBitmap *activePcrBanks) /* * qemuTPMEmulatorReconfigure * - * - * @source_type: type of storage - * @source_path: path to the directory for TPM state + * @emulator: emulator parameters * @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 - * @activePcrBanks: 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 virDomainTPMSourceType source_type, - const char *source_path, +qemuTPMEmulatorReconfigure(const virDomainTPMEmulatorDef *emulator, uid_t swtpm_user, gid_t swtpm_group, - virBitmap *activePcrBanks, - const char *logfile, - const virDomainTPMVersion tpmversion, const unsigned char *secretuuid) { g_autoptr(virCommand) cmd = NULL; int exitstatus; g_autofree char *activePcrBanksStr = NULL; g_autofree char *swtpm_setup = virTPMGetSwtpmSetup(); - g_autofree char *tpm_state = qemuTPMGetSwtpmSetupStateArg(source_type, source_path); + g_autofree char *tpm_state = qemuTPMGetSwtpmSetupStateArg(emulator->source_type, + emulator->source_path); if (!swtpm_setup) return -1; - if (tpmversion != VIR_DOMAIN_TPM_VERSION_2_0 || - (activePcrBanksStr = qemuTPMPcrBankBitmapToStr(activePcrBanks)) == NULL || + if (emulator->version != VIR_DOMAIN_TPM_VERSION_2_0 || + (activePcrBanksStr = qemuTPMPcrBankBitmapToStr(emulator->activePcrBanks)) == NULL || !virTPMSwtpmSetupCapsGet(VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_RECONFIGURE_PCR_BANKS)) return 0; @@ -542,7 +527,7 @@ qemuTPMEmulatorReconfigure(const virDomainTPMSourceType source_type, virCommandAddArgList(cmd, "--tpm-state", tpm_state, - "--logfile", logfile, + "--logfile", emulator->logfile, "--pcr-banks", activePcrBanksStr, "--reconfigure", NULL); @@ -552,7 +537,7 @@ qemuTPMEmulatorReconfigure(const virDomainTPMSourceType source_type, if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not run '%1$s --reconfigure'. exitstatus: %2$d; Check error log '%3$s' for details."), - swtpm_setup, exitstatus, logfile); + swtpm_setup, exitstatus, emulator->logfile); return -1; } @@ -628,21 +613,14 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm, secretuuid = tpm->data.emulator.secretuuid; if (run_setup && - qemuTPMEmulatorRunSetup(tpm->data.emulator.source_type, - tpm->data.emulator.source_path, vmname, vmuuid, + qemuTPMEmulatorRunSetup(&tpm->data.emulator, vmname, vmuuid, privileged, swtpm_user, swtpm_group, - tpm->data.emulator.logfile, - tpm->data.emulator.version, secretuuid, incomingMigration) < 0) goto error; if (!incomingMigration && - qemuTPMEmulatorReconfigure(tpm->data.emulator.source_type, - tpm->data.emulator.source_path, + qemuTPMEmulatorReconfigure(&tpm->data.emulator, swtpm_user, swtpm_group, - tpm->data.emulator.activePcrBanks, - tpm->data.emulator.logfile, - tpm->data.emulator.version, secretuuid) < 0) goto error; -- 2.47.0
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- v3: - Adjustments due to rebase - Applied Marc-André's R-b --- src/qemu/qemu_tpm.c | 52 +++++++++++++++++---------------------------- 1 file changed, 20 insertions(+), 32 deletions(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 6d7625f6f4..757bb16d7b 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -372,9 +372,7 @@ qemuTPMGetSwtpmSetupStateArg(const virDomainTPMSourceType source_type, * @vmname: the name of the VM * @vmuuid: the UUID of the VM * @privileged: whether we are running in privileged mode - * @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 + * @cfg: virQEMUDriverConfig * @secretuuid: UUID describing virStorageEncryption holding secret * @incomingMigration: whether we have an incoming migration * @@ -386,8 +384,7 @@ qemuTPMEmulatorRunSetup(const virDomainTPMEmulatorDef *emulator, const char *vmname, const unsigned char *vmuuid, bool privileged, - uid_t swtpm_user, - gid_t swtpm_group, + const virQEMUDriverConfig *cfg, const unsigned char *secretuuid, bool incomingMigration) { @@ -416,8 +413,8 @@ qemuTPMEmulatorRunSetup(const virDomainTPMEmulatorDef *emulator, virUUIDFormat(vmuuid, uuid); vmid = g_strdup_printf("%s:%s", vmname, uuid); - virCommandSetUID(cmd, swtpm_user); - virCommandSetGID(cmd, swtpm_group); + virCommandSetUID(cmd, cfg->swtpm_user); /* should be uid of 'tss' or 'root' */ + virCommandSetGID(cmd, cfg->swtpm_group); switch (emulator->version) { case VIR_DOMAIN_TPM_VERSION_1_2: @@ -487,17 +484,14 @@ qemuTPMPcrBankBitmapToStr(virBitmap *activePcrBanks) * qemuTPMEmulatorReconfigure * * @emulator: emulator parameters - * @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 + * @cfg: virQEMUDriverConfig * @secretuuid: The secret's UUID needed for state encryption * * Reconfigure the active PCR banks of a TPM 2. */ static int qemuTPMEmulatorReconfigure(const virDomainTPMEmulatorDef *emulator, - uid_t swtpm_user, - gid_t swtpm_group, + const virQEMUDriverConfig *cfg, const unsigned char *secretuuid) { g_autoptr(virCommand) cmd = NULL; @@ -517,8 +511,8 @@ qemuTPMEmulatorReconfigure(const virDomainTPMEmulatorDef *emulator, cmd = virCommandNew(swtpm_setup); - virCommandSetUID(cmd, swtpm_user); - virCommandSetGID(cmd, swtpm_group); + virCommandSetUID(cmd, cfg->swtpm_user); /* should be uid of 'tss' or 'root' */ + virCommandSetGID(cmd, cfg->swtpm_group); virCommandAddArgList(cmd, "--tpm2", NULL); @@ -552,9 +546,7 @@ qemuTPMEmulatorReconfigure(const virDomainTPMEmulatorDef *emulator, * @vmname: The name of the VM * @vmuuid: The UUID of the VM * @privileged: whether we are running in privileged mode - * @swtpm_user: The uid for the swtpm to run as (drop privileges to from root) - * @swtpm_group: The gid for the swtpm to run as - * @sharedFilesystems: list of filesystem to consider shared + * @cfg: virQEMUDriverConfig * @incomingMigration: whether we have an incoming migration * * Create the virCommand use for starting the emulator @@ -566,9 +558,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm, const char *vmname, const unsigned char *vmuuid, bool privileged, - uid_t swtpm_user, - gid_t swtpm_group, - char *const *sharedFilesystems, + const virQEMUDriverConfig *cfg, bool incomingMigration) { g_autoptr(virCommand) cmd = NULL; @@ -599,12 +589,14 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm, /* Do not create storage and run swtpm_setup on incoming migration over * shared storage */ - on_shared_storage = virFileIsSharedFS(tpm->data.emulator.source_path, sharedFilesystems) == 1; + on_shared_storage = virFileIsSharedFS(tpm->data.emulator.source_path, + cfg->sharedFilesystems) == 1; if (incomingMigration && on_shared_storage) create_storage = false; if (create_storage) { - if (qemuTPMEmulatorCreateStorage(tpm, &created, swtpm_user, swtpm_group) < 0) + if (qemuTPMEmulatorCreateStorage(tpm, &created, + cfg->swtpm_user, cfg->swtpm_group) < 0) return NULL; run_setup = created; } @@ -614,14 +606,12 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm, if (run_setup && qemuTPMEmulatorRunSetup(&tpm->data.emulator, vmname, vmuuid, - privileged, swtpm_user, swtpm_group, - secretuuid, incomingMigration) < 0) + privileged, cfg, secretuuid, + incomingMigration) < 0) goto error; if (!incomingMigration && - qemuTPMEmulatorReconfigure(&tpm->data.emulator, - swtpm_user, swtpm_group, - secretuuid) < 0) + qemuTPMEmulatorReconfigure(&tpm->data.emulator, cfg, secretuuid) < 0) goto error; unlink(tpm->data.emulator.source->data.nix.path); @@ -657,8 +647,8 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm, virCommandAddArg(cmd, "--terminate"); - virCommandSetUID(cmd, swtpm_user); - virCommandSetGID(cmd, swtpm_group); + virCommandSetUID(cmd, cfg->swtpm_user); + virCommandSetGID(cmd, cfg->swtpm_group); switch (tpm->data.emulator.version) { case VIR_DOMAIN_TPM_VERSION_1_2: @@ -979,9 +969,7 @@ qemuTPMEmulatorStart(virQEMUDriver *driver, if (!(cmd = qemuTPMEmulatorBuildCommand(tpm, vm->def->name, vm->def->uuid, driver->privileged, - cfg->swtpm_user, - cfg->swtpm_group, - cfg->sharedFilesystems, + cfg, incomingMigration))) return -1; -- 2.47.0
Add support for parsing swtpm_setup 'cmdarg-profile' capability (since v0.10). Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/util/virtpm.c | 1 + src/util/virtpm.h | 1 + tests/testutilsqemu.c | 1 + 3 files changed, 3 insertions(+) diff --git a/src/util/virtpm.c b/src/util/virtpm.c index 8dcd3f90d9..1c736b0229 100644 --- a/src/util/virtpm.c +++ b/src/util/virtpm.c @@ -53,6 +53,7 @@ VIR_ENUM_IMPL(virTPMSwtpmSetupFeature, "tpm-1.2", "tpm-2.0", "tpmstate-opt-lock", + "cmdarg-profile", ); /** diff --git a/src/util/virtpm.h b/src/util/virtpm.h index 279cb7e976..9ca09c2d80 100644 --- a/src/util/virtpm.h +++ b/src/util/virtpm.h @@ -45,6 +45,7 @@ typedef enum { VIR_TPM_SWTPM_SETUP_FEATURE_TPM_1_2, VIR_TPM_SWTPM_SETUP_FEATURE_TPM_2_0, VIR_TPM_SWTPM_SETUP_FEATURE_TPMSTATE_OPT_LOCK, + VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PROFILE, VIR_TPM_SWTPM_SETUP_FEATURE_LAST } virTPMSwtpmSetupFeature; diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index f40bfa873c..5caccbc6b4 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -72,6 +72,7 @@ virTPMSwtpmSetupCapsGet(virTPMSwtpmSetupFeature cap) case VIR_TPM_SWTPM_SETUP_FEATURE_TPM12_NOT_NEED_ROOT: case VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_RECONFIGURE_PCR_BANKS: case VIR_TPM_SWTPM_SETUP_FEATURE_TPMSTATE_OPT_LOCK: + case VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PROFILE: case VIR_TPM_SWTPM_SETUP_FEATURE_LAST: break; } -- 2.47.0
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/conf/domain_conf.c | 7 +++++++ src/conf/domain_conf.h | 9 +++++++++ 2 files changed, 16 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3a32e50890..a5627ada88 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1337,6 +1337,13 @@ VIR_ENUM_IMPL(virDomainTPMPcrBank, "sha512", ); +VIR_ENUM_IMPL(virDomainTPMProfileRemoveDisabled, + VIR_DOMAIN_TPM_PROFILE_REMOVE_DISABLED_LAST, + "none", + "check", + "fips-host", +); + VIR_ENUM_IMPL(virDomainIOMMUModel, VIR_DOMAIN_IOMMU_MODEL_LAST, "intel", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 08c6526711..e1103c3655 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1471,6 +1471,14 @@ typedef enum { VIR_DOMAIN_TPM_SOURCE_TYPE_LAST } virDomainTPMSourceType; +typedef enum { + VIR_DOMAIN_TPM_PROFILE_REMOVE_DISABLED_NONE = 0, + VIR_DOMAIN_TPM_PROFILE_REMOVE_DISABLED_CHECK, + VIR_DOMAIN_TPM_PROFILE_REMOVE_DISABLED_FIPS_HOST, + + VIR_DOMAIN_TPM_PROFILE_REMOVE_DISABLED_LAST +} virDomainTPMProfileRemoveDisabled; + #define VIR_DOMAIN_TPM_DEFAULT_DEVICE "/dev/tpm0" struct _virDomainTPMEmulatorDef { @@ -4290,6 +4298,7 @@ VIR_ENUM_DECL(virDomainTPMBackend); VIR_ENUM_DECL(virDomainTPMVersion); VIR_ENUM_DECL(virDomainTPMSourceType); VIR_ENUM_DECL(virDomainTPMPcrBank); +VIR_ENUM_DECL(virDomainTPMProfileRemoveDisabled); VIR_ENUM_DECL(virDomainMemoryModel); VIR_ENUM_DECL(virDomainMemoryBackingModel); VIR_ENUM_DECL(virDomainMemorySource); -- 2.47.0
On 11/13/24 18:39, Stefan Berger wrote:
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/conf/domain_conf.c | 7 +++++++ src/conf/domain_conf.h | 9 +++++++++ 2 files changed, 16 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3a32e50890..a5627ada88 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1337,6 +1337,13 @@ VIR_ENUM_IMPL(virDomainTPMPcrBank, "sha512", );
+VIR_ENUM_IMPL(virDomainTPMProfileRemoveDisabled, + VIR_DOMAIN_TPM_PROFILE_REMOVE_DISABLED_LAST, + "none", + "check", + "fips-host", +); + VIR_ENUM_IMPL(virDomainIOMMUModel, VIR_DOMAIN_IOMMU_MODEL_LAST, "intel", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 08c6526711..e1103c3655 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1471,6 +1471,14 @@ typedef enum { VIR_DOMAIN_TPM_SOURCE_TYPE_LAST } virDomainTPMSourceType;
+typedef enum { + VIR_DOMAIN_TPM_PROFILE_REMOVE_DISABLED_NONE = 0, + VIR_DOMAIN_TPM_PROFILE_REMOVE_DISABLED_CHECK, + VIR_DOMAIN_TPM_PROFILE_REMOVE_DISABLED_FIPS_HOST, + + VIR_DOMAIN_TPM_PROFILE_REMOVE_DISABLED_LAST +} virDomainTPMProfileRemoveDisabled; + #define VIR_DOMAIN_TPM_DEFAULT_DEVICE "/dev/tpm0"
struct _virDomainTPMEmulatorDef { @@ -4290,6 +4298,7 @@ VIR_ENUM_DECL(virDomainTPMBackend); VIR_ENUM_DECL(virDomainTPMVersion); VIR_ENUM_DECL(virDomainTPMSourceType); VIR_ENUM_DECL(virDomainTPMPcrBank); +VIR_ENUM_DECL(virDomainTPMProfileRemoveDisabled); VIR_ENUM_DECL(virDomainMemoryModel); VIR_ENUM_DECL(virDomainMemoryBackingModel); VIR_ENUM_DECL(virDomainMemorySource);
Don't forget to add these new symbols to libvirt_private.syms Michal
Extend the schema for the TPM emulator profile node. Require that the profile the user provides is described in a 'source' attribute. An optional remove_disabled attribute is also supported for swtpm to automatically remove algorithms from the 'custom' profile if they are disabled by FIPS mode on the host. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/conf/schemas/domaincommon.rng | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 8360eeae3f..d94ff9b4c3 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -5924,6 +5924,7 @@ <ref name="tpm-backend-emulator-encryption"/> <ref name="tpm-backend-emulator-active-pcr-banks"/> <ref name="tpm-backend-emulator-source"/> + <ref name="tpm-backend-emulator-profile"/> </interleave> <optional> <attribute name="persistent_state"> @@ -6046,6 +6047,30 @@ </optional> </define> + <define name="profileName"> + <data type="string"> + <param name="pattern">[A-Za-z0-9.\-:]+</param> + </data> + </define> + + <define name="tpm-backend-emulator-profile"> + <optional> + <element name="profile"> + <attribute name="source"> + <ref name="profileName"/> + </attribute> + <optional> + <attribute name="remove_disabled"> + <choice> + <value>check</value> + <value>fips-host</value> + </choice> + </attribute> + </optional> + </element> + </optional> + </define> + <define name="vsock"> <element name="vsock"> <optional> -- 2.47.0
On 11/13/24 18:39, Stefan Berger wrote:
Extend the schema for the TPM emulator profile node. Require that the profile the user provides is described in a 'source' attribute. An optional remove_disabled attribute is also supported for swtpm to automatically remove algorithms from the 'custom' profile if they are disabled by FIPS mode on the host.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/conf/schemas/domaincommon.rng | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 8360eeae3f..d94ff9b4c3 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -5924,6 +5924,7 @@ <ref name="tpm-backend-emulator-encryption"/> <ref name="tpm-backend-emulator-active-pcr-banks"/> <ref name="tpm-backend-emulator-source"/> + <ref name="tpm-backend-emulator-profile"/> </interleave> <optional> <attribute name="persistent_state"> @@ -6046,6 +6047,30 @@ </optional> </define>
+ <define name="profileName"> + <data type="string"> + <param name="pattern">[A-Za-z0-9.\-:]+</param> + </data> + </define> + + <define name="tpm-backend-emulator-profile"> + <optional> + <element name="profile"> + <attribute name="source"> + <ref name="profileName"/> + </attribute> + <optional> + <attribute name="remove_disabled">
How about "removeDisabled" instead? I think camelCase is preferred when it comes to multiple worded attributes.
+ <choice> + <value>check</value> + <value>fips-host</value> + </choice> + </attribute> + </optional> + </element> + </optional> + </define> + <define name="vsock"> <element name="vsock"> <optional>
Michal
Extend the parser and XML builder with support for the profile parameter and its remove_disabled attribute. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/conf/domain_conf.c | 36 ++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/conf/domain_validate.c | 7 +++++++ 3 files changed, 45 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a5627ada88..7d91e3e958 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3478,6 +3478,7 @@ void virDomainTPMDefFree(virDomainTPMDef *def) g_free(def->data.emulator.source_path); g_free(def->data.emulator.logfile); virBitmapFree(def->data.emulator.activePcrBanks); + g_free(def->data.emulator.profile_source); break; case VIR_DOMAIN_TPM_TYPE_EXTERNAL: virObjectUnref(def->data.external.source); @@ -10786,6 +10787,15 @@ virDomainSmartcardDefParseXML(virDomainXMLOption *xmlopt, * <tpm model='tpm-tis'> * <backend type='emulator' version='2.0' persistent_state='yes'> * </tpm> + * + * A profile for a TPM 2.0 can be added like this: + * + * <tpm model='tpm-crb'> + * <backend type='emulator' version='2.0'> + * <profile source='local:restricted' remove_disabled='check'/> + * </backend> + * </tpm> + * */ static virDomainTPMDef * virDomainTPMDefParseXML(virDomainXMLOption *xmlopt, @@ -10805,6 +10815,8 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt, g_autofree xmlNodePtr *backends = NULL; g_autofree xmlNodePtr *nodes = NULL; g_autofree char *type = NULL; + virDomainTPMProfileRemoveDisabled profile_remove_disabled; + xmlNodePtr profile; int bank; if (!(def = virDomainTPMDefNew(xmlopt))) @@ -10911,6 +10923,22 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt, } virBitmapSetBitExpand(def->data.emulator.activePcrBanks, bank); } + + 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, + &profile_remove_disabled) < 0) + goto error; + if (profile_remove_disabled != VIR_DOMAIN_TPM_PROFILE_REMOVE_DISABLED_NONE) + def->data.emulator.profile_remove_disabled = + virDomainTPMProfileRemoveDisabledTypeToString(profile_remove_disabled); + } break; case VIR_DOMAIN_TPM_TYPE_EXTERNAL: if (!(type = virXPathString("string(./backend/source/@type)", ctxt))) { @@ -25106,6 +25134,14 @@ 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_remove_disabled) + virBufferAsprintf(&backendChildBuf, " remove_disabled='%s'", + def->data.emulator.profile_remove_disabled); + virBufferAddLit(&backendChildBuf, "/>\n"); + } 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 e1103c3655..bd2740af26 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1492,6 +1492,8 @@ struct _virDomainTPMEmulatorDef { bool hassecretuuid; bool persistent_state; virBitmap *activePcrBanks; + char *profile_source; /* 'source' profile was created from */ + const char *profile_remove_disabled; }; struct _virDomainTPMDef { diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index b8ae9ed79d..7573430b6b 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -3026,6 +3026,13 @@ virDomainTPMDevValidate(const virDomainTPMDef *tpm) virDomainTPMVersionTypeToString(VIR_DOMAIN_TPM_VERSION_2_0)); return -1; } + if (tpm->data.emulator.profile_source && + tpm->data.emulator.version != VIR_DOMAIN_TPM_VERSION_2_0) { + virReportError(VIR_ERR_XML_ERROR, + _("<profile/> requires TPM version '%1$s'"), + virDomainTPMVersionTypeToString(VIR_DOMAIN_TPM_VERSION_2_0)); + return -1; + } break; case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: -- 2.47.0
On 11/13/24 18:39, Stefan Berger wrote:
Extend the parser and XML builder with support for the profile parameter and its remove_disabled attribute.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/conf/domain_conf.c | 36 ++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/conf/domain_validate.c | 7 +++++++ 3 files changed, 45 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a5627ada88..7d91e3e958 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3478,6 +3478,7 @@ void virDomainTPMDefFree(virDomainTPMDef *def) g_free(def->data.emulator.source_path); g_free(def->data.emulator.logfile); virBitmapFree(def->data.emulator.activePcrBanks); + g_free(def->data.emulator.profile_source); break; case VIR_DOMAIN_TPM_TYPE_EXTERNAL: virObjectUnref(def->data.external.source); @@ -10786,6 +10787,15 @@ virDomainSmartcardDefParseXML(virDomainXMLOption *xmlopt, * <tpm model='tpm-tis'> * <backend type='emulator' version='2.0' persistent_state='yes'> * </tpm> + * + * A profile for a TPM 2.0 can be added like this: + * + * <tpm model='tpm-crb'> + * <backend type='emulator' version='2.0'> + * <profile source='local:restricted' remove_disabled='check'/> + * </backend> + * </tpm> + * */ static virDomainTPMDef * virDomainTPMDefParseXML(virDomainXMLOption *xmlopt, @@ -10805,6 +10815,8 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt, g_autofree xmlNodePtr *backends = NULL; g_autofree xmlNodePtr *nodes = NULL; g_autofree char *type = NULL; + virDomainTPMProfileRemoveDisabled profile_remove_disabled; + xmlNodePtr profile; int bank;
if (!(def = virDomainTPMDefNew(xmlopt))) @@ -10911,6 +10923,22 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt, } virBitmapSetBitExpand(def->data.emulator.activePcrBanks, bank); } + + 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, + &profile_remove_disabled) < 0) + goto error; + if (profile_remove_disabled != VIR_DOMAIN_TPM_PROFILE_REMOVE_DISABLED_NONE) + def->data.emulator.profile_remove_disabled = + virDomainTPMProfileRemoveDisabledTypeToString(profile_remove_disabled); + } break; case VIR_DOMAIN_TPM_TYPE_EXTERNAL: if (!(type = virXPathString("string(./backend/source/@type)", ctxt))) { @@ -25106,6 +25134,14 @@ 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_remove_disabled) + virBufferAsprintf(&backendChildBuf, " remove_disabled='%s'", + def->data.emulator.profile_remove_disabled); + virBufferAddLit(&backendChildBuf, "/>\n");
Alternatively, virXMLFormatElement() makes this simpler.
+ } 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 e1103c3655..bd2740af26 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1492,6 +1492,8 @@ struct _virDomainTPMEmulatorDef { bool hassecretuuid; bool persistent_state; virBitmap *activePcrBanks; + char *profile_source; /* 'source' profile was created from */ + const char *profile_remove_disabled;
Why not store the enum instead of this const string? Oh, and while at it, these two (soon three) variables can be moved into a separate struct so that their "profile_" prefix can be dropped. E.g. struct { char *source; virDomainTPMProfileRemoveDisabled removeDisabled; } profile;
};
struct _virDomainTPMDef { diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index b8ae9ed79d..7573430b6b 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -3026,6 +3026,13 @@ virDomainTPMDevValidate(const virDomainTPMDef *tpm) virDomainTPMVersionTypeToString(VIR_DOMAIN_TPM_VERSION_2_0)); return -1; } + if (tpm->data.emulator.profile_source && + tpm->data.emulator.version != VIR_DOMAIN_TPM_VERSION_2_0) { + virReportError(VIR_ERR_XML_ERROR, + _("<profile/> requires TPM version '%1$s'"), + virDomainTPMVersionTypeToString(VIR_DOMAIN_TPM_VERSION_2_0)); + return -1; + } break;
case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
Michal
On 11/15/24 4:19 AM, Michal Prívozník wrote:
On 11/13/24 18:39, Stefan Berger wrote:
+ } 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 e1103c3655..bd2740af26 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1492,6 +1492,8 @@ struct _virDomainTPMEmulatorDef { bool hassecretuuid; bool persistent_state; virBitmap *activePcrBanks; + char *profile_source; /* 'source' profile was created from */ + const char *profile_remove_disabled;
Why not store the enum instead of this const string?
Oh, and while at it, these two (soon three) variables can be moved into a separate struct so that their "profile_" prefix can be dropped. E.g.
struct { char *source; virDomainTPMProfileRemoveDisabled removeDisabled; } profile;
Yes, all profile-related stuff together is better. Thanks. Stefan
Add documentation for the TPM backend profile node and point the reader to further documentation about TPM profiles available in the swtpm man page. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- docs/formatdomain.rst | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index c50744b57b..6539f620fa 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -8131,6 +8131,7 @@ Example: usage of the TPM Emulator <active_pcr_banks> <sha256/> </active_pcr_banks> + <profile source='local:restricted' remove_disabled='check'/> </backend> </tpm> </devices> @@ -8225,6 +8226,35 @@ Example: usage of the TPM Emulator and may not have any effect otherwise. The selection of PCR banks only works with the ``emulator`` backend. :since:`Since 7.10.0` +``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. + + The built-in 'null' profile provides backwards compatibility with + libtpms v0.9 but also restricts the user to use only TPM features that were + available at the time of libtpms v0.9. The built-in 'custom' profile is the + only profile that a user can modify and where the ``remove_disabled`` + attribute has any effect. This attribute is particularly useful when a host + is running in FIPS mode and therefore some crypto algorithms (camellia, + tdes, unpadded RSA encryption, 1024-bit RSA keys, and others) are + disabled. When it is set to ``check`` (recommended) then only those + algorithms that are currently disabled will automatically be removed from + the 'custom' profile, while when it is set to ``fips-host`` then all + potentially disabled algorithms will be removed. :since:`Since 10.??.0` + + TPM profiles provided by a distro can be referenced with the 'distro:' + prefix. Locally created TPM profiles can be referenced with the + 'local:' prefix. + + For further information about TPM profiles see the man pages for ``swtpm`` + (swtpm v0.10). + ``encryption`` The ``encryption`` element allows the state of a TPM emulator to be encrypted. The ``secret`` must reference a secret object that holds the -- 2.47.0
Run swtpm_setup with the --profile-name option if the user provided the name of a profile. swtpm_setup will try to load the profile from directories with local profiles and distro profiles and if no profile by this name with appended '.json' suffix could be found there, it will fall back to try to use an internal profile with the given name. Also set the --profile-remove-disabled option if the user provided a value in the remove_disabled attribute in the profile XML node. Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/qemu/qemu_tpm.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 757bb16d7b..34db6494a5 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -365,6 +365,41 @@ qemuTPMGetSwtpmSetupStateArg(const virDomainTPMSourceType source_type, } +/* + * Add a (optional) profile to the swtpm_setup command line. + * + * @cmd: virCommand to add options to + * @emulator: emulator parameters + * + * Returns 0 on success, -1 on failure. + */ +static int +qemuTPMVirCommandAddProfile(virCommand *cmd, + const virDomainTPMEmulatorDef *emulator) +{ + if (!emulator->profile_source) + return 0; + + if (!virTPMSwtpmSetupCapsGet( + VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PROFILE)) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("swtpm_setup has no support for profiles")); + return -1; + } + + virCommandAddArgList(cmd, + "--profile-name", emulator->profile_source, + NULL); + + if (emulator->profile_remove_disabled) + virCommandAddArgList(cmd, + "--profile-remove-disable", + emulator->profile_remove_disabled, + NULL); + return 0; +} + + /* * qemuTPMEmulatorRunSetup * @@ -441,6 +476,8 @@ qemuTPMEmulatorRunSetup(const virDomainTPMEmulatorDef *emulator, "--lock-nvram", "--not-overwrite", NULL); + if (qemuTPMVirCommandAddProfile(cmd, emulator) < 0) + return -1; } else { virCommandAddArgList(cmd, "--tpm-state", tpm_state, -- 2.47.0
Factor-out code related to adding key to the swtpm command line into its own function. Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/qemu/qemu_tpm.c | 60 +++++++++++++++++++++++++++------------------ 1 file changed, 36 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 34db6494a5..bf07b86793 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -575,6 +575,38 @@ qemuTPMEmulatorReconfigure(const virDomainTPMEmulatorDef *emulator, return 0; } +static int +qemuTPMVirCommandSwtpmAddEncryption(virCommand *cmd, + const virDomainTPMEmulatorDef *emulator, + const char *swtpm) +{ + int pwdfile_fd = -1; + int migpwdfile_fd = -1; + + if (emulator->hassecretuuid) { + if (!virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_CMDARG_PWD_FD)) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, + _("%1$s does not support passing passphrase via file descriptor"), + swtpm); + return -1; + } + + if (qemuTPMSetupEncryption(emulator->secretuuid, + cmd, &pwdfile_fd) < 0) + return -1; + + if (qemuTPMSetupEncryption(emulator->secretuuid, + cmd, &migpwdfile_fd) < 0) + return -1; + + virCommandAddArg(cmd, "--key"); + virCommandAddArgFormat(cmd, "pwdfd=%d,mode=aes-256-cbc", pwdfile_fd); + + virCommandAddArg(cmd, "--migration-key"); + virCommandAddArgFormat(cmd, "pwdfd=%d,mode=aes-256-cbc", migpwdfile_fd); + } + return 0; +} /* * qemuTPMEmulatorBuildCommand: @@ -602,8 +634,6 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm, bool created = false; bool run_setup = false; g_autofree char *swtpm = virTPMGetSwtpm(); - int pwdfile_fd = -1; - int migpwdfile_fd = -1; const unsigned char *secretuuid = NULL; bool create_storage = true; bool on_shared_storage; @@ -698,28 +728,10 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm, break; } - if (tpm->data.emulator.hassecretuuid) { - if (!virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_CMDARG_PWD_FD)) { - virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, - _("%1$s does not support passing passphrase via file descriptor"), - swtpm); - goto error; - } - - if (qemuTPMSetupEncryption(tpm->data.emulator.secretuuid, - cmd, &pwdfile_fd) < 0) - goto error; - - if (qemuTPMSetupEncryption(tpm->data.emulator.secretuuid, - cmd, &migpwdfile_fd) < 0) - goto error; - - virCommandAddArg(cmd, "--key"); - virCommandAddArgFormat(cmd, "pwdfd=%d,mode=aes-256-cbc", pwdfile_fd); - - virCommandAddArg(cmd, "--migration-key"); - virCommandAddArgFormat(cmd, "pwdfd=%d,mode=aes-256-cbc", migpwdfile_fd); - } + if (qemuTPMVirCommandSwtpmAddEncryption(cmd, + &tpm->data.emulator, + swtpm) < 0) + goto error; /* If swtpm supports it and the TPM state is stored on shared storage, * start swtpm with --migration release-lock-outgoing so it can migrate -- 2.47.0
On 11/13/24 18:39, Stefan Berger wrote:
Factor-out code related to adding key to the swtpm command line into its own function.
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/qemu/qemu_tpm.c | 60 +++++++++++++++++++++++++++------------------ 1 file changed, 36 insertions(+), 24 deletions(-)
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 34db6494a5..bf07b86793 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -575,6 +575,38 @@ qemuTPMEmulatorReconfigure(const virDomainTPMEmulatorDef *emulator, return 0; }
+static int +qemuTPMVirCommandSwtpmAddEncryption(virCommand *cmd, + const virDomainTPMEmulatorDef *emulator, + const char *swtpm) +{ + int pwdfile_fd = -1; + int migpwdfile_fd = -1; + + if (emulator->hassecretuuid) {
Alternatively: if (!emulator->hassecretuuid) return 0; ....
+ if (!virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_CMDARG_PWD_FD)) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, + _("%1$s does not support passing passphrase via file descriptor"), + swtpm); + return -1; + } + + if (qemuTPMSetupEncryption(emulator->secretuuid, + cmd, &pwdfile_fd) < 0) + return -1; + + if (qemuTPMSetupEncryption(emulator->secretuuid, + cmd, &migpwdfile_fd) < 0) + return -1; + + virCommandAddArg(cmd, "--key"); + virCommandAddArgFormat(cmd, "pwdfd=%d,mode=aes-256-cbc", pwdfile_fd); + + virCommandAddArg(cmd, "--migration-key"); + virCommandAddArgFormat(cmd, "pwdfd=%d,mode=aes-256-cbc", migpwdfile_fd); + } + return 0; +}
/* * qemuTPMEmulatorBuildCommand: @@ -602,8 +634,6 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm, bool created = false; bool run_setup = false; g_autofree char *swtpm = virTPMGetSwtpm(); - int pwdfile_fd = -1; - int migpwdfile_fd = -1; const unsigned char *secretuuid = NULL; bool create_storage = true; bool on_shared_storage; @@ -698,28 +728,10 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm, break; }
- if (tpm->data.emulator.hassecretuuid) { - if (!virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_CMDARG_PWD_FD)) { - virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, - _("%1$s does not support passing passphrase via file descriptor"), - swtpm); - goto error; - } - - if (qemuTPMSetupEncryption(tpm->data.emulator.secretuuid, - cmd, &pwdfile_fd) < 0) - goto error; - - if (qemuTPMSetupEncryption(tpm->data.emulator.secretuuid, - cmd, &migpwdfile_fd) < 0) - goto error; - - virCommandAddArg(cmd, "--key"); - virCommandAddArgFormat(cmd, "pwdfd=%d,mode=aes-256-cbc", pwdfile_fd); - - virCommandAddArg(cmd, "--migration-key"); - virCommandAddArgFormat(cmd, "pwdfd=%d,mode=aes-256-cbc", migpwdfile_fd); - } + if (qemuTPMVirCommandSwtpmAddEncryption(cmd, + &tpm->data.emulator, + swtpm) < 0) + goto error;
/* If swtpm supports it and the TPM state is stored on shared storage, * start swtpm with --migration release-lock-outgoing so it can migrate
Michal
Factor-out code related to adding the --tpmstate option to the swtpm command line into its own function. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/qemu/qemu_tpm.c | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index bf07b86793..a7eee501bf 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -608,6 +608,25 @@ qemuTPMVirCommandSwtpmAddEncryption(virCommand *cmd, return 0; } +static void +qemuTPMVirCommandSwtpmAddTPMState(virCommand *cmd, + const virDomainTPMEmulatorDef *emulator) +{ + virCommandAddArg(cmd, "--tpmstate"); + switch (emulator->source_type) { + case VIR_DOMAIN_TPM_SOURCE_TYPE_FILE: + virCommandAddArgFormat(cmd, "backend-uri=file://%s", + emulator->source_path); + break; + case VIR_DOMAIN_TPM_SOURCE_TYPE_DIR: + case VIR_DOMAIN_TPM_SOURCE_TYPE_DEFAULT: + case VIR_DOMAIN_TPM_SOURCE_TYPE_LAST: + virCommandAddArgFormat(cmd, "dir=%s,mode=0600", + emulator->source_path); + break; + } +} + /* * qemuTPMEmulatorBuildCommand: * @@ -691,19 +710,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm, virCommandAddArgFormat(cmd, "type=unixio,path=%s,mode=0600", tpm->data.emulator.source->data.nix.path); - virCommandAddArg(cmd, "--tpmstate"); - switch (tpm->data.emulator.source_type) { - case VIR_DOMAIN_TPM_SOURCE_TYPE_FILE: - virCommandAddArgFormat(cmd, "backend-uri=file://%s", - tpm->data.emulator.source_path); - break; - case VIR_DOMAIN_TPM_SOURCE_TYPE_DIR: - case VIR_DOMAIN_TPM_SOURCE_TYPE_DEFAULT: - case VIR_DOMAIN_TPM_SOURCE_TYPE_LAST: - virCommandAddArgFormat(cmd, "dir=%s,mode=0600", - tpm->data.emulator.source_path); - break; - } + qemuTPMVirCommandSwtpmAddTPMState(cmd, &tpm->data.emulator); virCommandAddArg(cmd, "--log"); if (tpm->data.emulator.debug != 0) -- 2.47.0
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@linux.ibm.com> --- 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"), + 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")); + + 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; +} + /* * qemuTPMEmulatorBuildCommand: * * @tpm: TPM definition + * @persistentTPMDef: TPM definition from the persistent domain definition * @vmname: The name of the VM * @vmuuid: The UUID of the VM * @privileged: whether we are running in privileged mode * @cfg: virQEMUDriverConfig * @incomingMigration: whether we have an incoming migration + * @saveDef: whether caller should save the persistent domain def * * Create the virCommand use for starting the emulator * Do some initializations on the way, such as creation of storage @@ -643,11 +717,13 @@ qemuTPMVirCommandSwtpmAddTPMState(virCommand *cmd, */ static virCommand * qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm, + virDomainTPMDef *persistentTPMDef, const char *vmname, const unsigned char *vmuuid, bool privileged, const virQEMUDriverConfig *cfg, - bool incomingMigration) + bool incomingMigration, + bool *saveDef) { g_autoptr(virCommand) cmd = NULL; bool created = false; @@ -696,6 +772,11 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm, incomingMigration) < 0) goto error; + if (run_setup && !incomingMigration && + qemuTPMEmulatorUpdateProfileName(&tpm->data.emulator, persistentTPMDef, + cfg, saveDef) < 0) + goto error; + if (!incomingMigration && qemuTPMEmulatorReconfigure(&tpm->data.emulator, cfg, secretuuid) < 0) goto error; @@ -995,6 +1076,7 @@ qemuExtTPMEmulatorSetupCgroup(const char *swtpmStateDir, * @driver: QEMU driver * @vm: the domain object * @tpm: TPM definition + * @persistentTPMDef: TPM definition from persistent domain definition * @shortName: short and unique name of the domain * @incomingMigration: whether we have an incoming migration * @@ -1007,6 +1089,7 @@ qemuTPMEmulatorStart(virQEMUDriver *driver, virDomainObj *vm, const char *shortName, virDomainTPMDef *tpm, + virDomainTPMDef *persistentTPMDef, bool incomingMigration) { g_autoptr(virCommand) cmd = NULL; @@ -1015,6 +1098,7 @@ qemuTPMEmulatorStart(virQEMUDriver *driver, g_autofree char *pidfile = NULL; virTimeBackOffVar timebackoff; const unsigned long long timeout = 1000; /* ms */ + bool saveDef = false; pid_t pid = -1; bool lockMetadataException = false; @@ -1023,12 +1107,18 @@ qemuTPMEmulatorStart(virQEMUDriver *driver, /* stop any left-over TPM emulator for this VM */ qemuTPMEmulatorStop(cfg->swtpmStateDir, shortName); - if (!(cmd = qemuTPMEmulatorBuildCommand(tpm, vm->def->name, vm->def->uuid, + if (!(cmd = qemuTPMEmulatorBuildCommand(tpm, persistentTPMDef, + vm->def->name, vm->def->uuid, driver->privileged, cfg, - incomingMigration))) + incomingMigration, + &saveDef))) return -1; + if (saveDef && + virDomainDefSave(vm->newDef, driver->xmlopt, cfg->configDir) < 0) + goto error; + if (qemuExtDeviceLogCommand(driver, vm, cmd, "TPM Emulator") < 0) return -1; @@ -1212,6 +1302,7 @@ int qemuExtTPMStart(virQEMUDriver *driver, virDomainObj *vm, virDomainTPMDef *tpm, + virDomainTPMDef *persistentTPMDef, bool incomingMigration) { g_autofree char *shortName = virDomainDefGetShortName(vm->def); @@ -1219,7 +1310,8 @@ qemuExtTPMStart(virQEMUDriver *driver, if (!shortName) return -1; - return qemuTPMEmulatorStart(driver, vm, shortName, tpm, incomingMigration); + return qemuTPMEmulatorStart(driver, vm, shortName, tpm, persistentTPMDef, + incomingMigration); } diff --git a/src/qemu/qemu_tpm.h b/src/qemu/qemu_tpm.h index 3071dc3f71..7096060a2a 100644 --- a/src/qemu/qemu_tpm.h +++ b/src/qemu/qemu_tpm.h @@ -44,9 +44,10 @@ void qemuExtTPMCleanupHost(virQEMUDriver *driver, int qemuExtTPMStart(virQEMUDriver *driver, virDomainObj *vm, virDomainTPMDef *def, + virDomainTPMDef *persistentDefTPM, bool incomingMigration) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) - ATTRIBUTE_NONNULL(3) + ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) G_GNUC_WARN_UNUSED_RESULT; void qemuExtTPMStop(virQEMUDriver *driver, diff --git a/src/util/virtpm.c b/src/util/virtpm.c index 1c736b0229..4016ad8fc4 100644 --- a/src/util/virtpm.c +++ b/src/util/virtpm.c @@ -42,6 +42,7 @@ VIR_ENUM_IMPL(virTPMSwtpmFeature, "cmdarg-migration", "nvram-backend-dir", "nvram-backend-file", + "cmdarg-print-info", ); VIR_ENUM_IMPL(virTPMSwtpmSetupFeature, diff --git a/src/util/virtpm.h b/src/util/virtpm.h index 9ca09c2d80..03fb92629a 100644 --- a/src/util/virtpm.h +++ b/src/util/virtpm.h @@ -33,6 +33,7 @@ typedef enum { VIR_TPM_SWTPM_FEATURE_CMDARG_MIGRATION, VIR_TPM_SWTPM_FEATURE_NVRAM_BACKEND_DIR, VIR_TPM_SWTPM_FEATURE_NVRAM_BACKEND_FILE, + VIR_TPM_SWTPM_FEATURE_CMDARG_PRINT_INFO, VIR_TPM_SWTPM_FEATURE_LAST } virTPMSwtpmFeature; -- 2.47.0
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@linux.ibm.com> --- 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
On 11/15/24 4:19 AM, Michal Prívozník wrote:
On 11/13/24 18:39, Stefan Berger wrote:
+ 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.
Correct, one too many g_strdup's here.
+ + 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
On 11/13/24 18:39, Stefan Berger wrote:
Upcoming libtpms v0.10 and swtpm v0.10 will have TPM profile support that allows to restrict a TPM's provided set of crypto algorithms and commands and through which backwards compatibility and migration from newer versions of libtpms to older ones (up to libtpms v0.9) is supported. For the latter to work it is necessary that the user chooses the right ('null') profile.
This series adds support for passing a profile choice to swtpm_setup by setting it in the domain XML using the <profile/> XML node. An optional attribute 'remove_disabled' can be set in this node and accepts two values:
"check": test a few crypto algorithms (tdes, camellia, unpadded encryption, and others) for whether they are currently disabled due to FIPS mode on the host and remove these algorithms in the 'custom' profile if they are disabled; "fips-host": do not test but remove all the possibly disabled crypto algorithms (from list above)
Also extend the documentation but point the user to swtpm and libtpms documentation for further details.
Follow Deniel's suggestions there's now a PR for swtpm_setup to support searching for profiles though a configurable local directory, distro directory and if no profile could be found there (with appended ".json" suffix) it will fall back to try to use a built-in profile by the provided name: https://github.com/stefanberger/swtpm/pull/918
Stefan
v4: - Renamed previous 'name' attribute in profile XML node to 'source' to indicate that the profile was created from some sort of 'source'. The 'name' is now set from the name of the profile read from the swtpm instance's state once it has been created.
v3: - 2/10: Adjustments to due rebase - Applied Marc-André's R-b tags - 10/10: Read back profile name from swtpm and adjust it in emulator defs
Stefan Berger (11): conf: Move TPM emulator parameters into own struct qemu: Pass virQEMUDriverConfig rather than some of its fields util: Add parsing support for swtpm_setup's cmdarg-profile capability conf: Define enum virDomainTPMProfileRemoveDisabled schema: Extend schema for TPM emulator profile node conf: Add support for profile parameter on TPM emulator in domain XML docs: Add documentation for the TPM backend profile node qemu: Extend swtpm_setup command line to set a profile by its name qemu: Move adding of keys to swtpm command line into own function qemu: Move adding --tpmstate to swtpm command line into own function qemu: Read back the profile name after creation of a TPM instance
docs/formatdomain.rst | 32 +++ src/conf/domain_conf.c | 47 ++++ src/conf/domain_conf.h | 38 ++-- src/conf/domain_validate.c | 7 + src/conf/schemas/domaincommon.rng | 32 +++ src/conf/virconftypes.h | 2 + src/qemu/qemu_extdevice.c | 5 +- src/qemu/qemu_tpm.c | 344 ++++++++++++++++++++---------- src/qemu/qemu_tpm.h | 3 +- src/util/virtpm.c | 2 + src/util/virtpm.h | 2 + tests/testutilsqemu.c | 1 + 12 files changed, 386 insertions(+), 129 deletions(-)
This adds new XML element and attributes but is lacking corresponding tests/qemuxmlconfdata/ addition to show parser/formatter working. I've uploaded my suggestions here: https://gitlab.com/MichalPrivoznik/libvirt/-/commits/review_swtpm?ref_type=h... If you are fine with them, I can squash those fixup commits and merge. Michal
On 11/15/24 4:19 AM, Michal Prívozník wrote:
On 11/13/24 18:39, Stefan Berger wrote:
Upcoming libtpms v0.10 and swtpm v0.10 will have TPM profile support that allows to restrict a TPM's provided set of crypto algorithms and commands and through which backwards compatibility and migration from newer versions of libtpms to older ones (up to libtpms v0.9) is supported. For the latter to work it is necessary that the user chooses the right ('null') profile.
This series adds support for passing a profile choice to swtpm_setup by setting it in the domain XML using the <profile/> XML node. An optional attribute 'remove_disabled' can be set in this node and accepts two values:
"check": test a few crypto algorithms (tdes, camellia, unpadded encryption, and others) for whether they are currently disabled due to FIPS mode on the host and remove these algorithms in the 'custom' profile if they are disabled; "fips-host": do not test but remove all the possibly disabled crypto algorithms (from list above)
Also extend the documentation but point the user to swtpm and libtpms documentation for further details.
Follow Deniel's suggestions there's now a PR for swtpm_setup to support searching for profiles though a configurable local directory, distro directory and if no profile could be found there (with appended ".json" suffix) it will fall back to try to use a built-in profile by the provided name: https://github.com/stefanberger/swtpm/pull/918
Stefan
v4: - Renamed previous 'name' attribute in profile XML node to 'source' to indicate that the profile was created from some sort of 'source'. The 'name' is now set from the name of the profile read from the swtpm instance's state once it has been created.
v3: - 2/10: Adjustments to due rebase - Applied Marc-André's R-b tags - 10/10: Read back profile name from swtpm and adjust it in emulator defs
Stefan Berger (11): conf: Move TPM emulator parameters into own struct qemu: Pass virQEMUDriverConfig rather than some of its fields util: Add parsing support for swtpm_setup's cmdarg-profile capability conf: Define enum virDomainTPMProfileRemoveDisabled schema: Extend schema for TPM emulator profile node conf: Add support for profile parameter on TPM emulator in domain XML docs: Add documentation for the TPM backend profile node qemu: Extend swtpm_setup command line to set a profile by its name qemu: Move adding of keys to swtpm command line into own function qemu: Move adding --tpmstate to swtpm command line into own function qemu: Read back the profile name after creation of a TPM instance
docs/formatdomain.rst | 32 +++ src/conf/domain_conf.c | 47 ++++ src/conf/domain_conf.h | 38 ++-- src/conf/domain_validate.c | 7 + src/conf/schemas/domaincommon.rng | 32 +++ src/conf/virconftypes.h | 2 + src/qemu/qemu_extdevice.c | 5 +- src/qemu/qemu_tpm.c | 344 ++++++++++++++++++++---------- src/qemu/qemu_tpm.h | 3 +- src/util/virtpm.c | 2 + src/util/virtpm.h | 2 + tests/testutilsqemu.c | 1 + 12 files changed, 386 insertions(+), 129 deletions(-)
This adds new XML element and attributes but is lacking corresponding tests/qemuxmlconfdata/ addition to show parser/formatter working.
I've uploaded my suggestions here:
https://gitlab.com/MichalPrivoznik/libvirt/-/commits/review_swtpm?ref_type=h...
If you are fine with them, I can squash those fixup commits and merge.
I tested it. The changes look good to me. Thanks also for the test case. Stefan
Michal
On 11/15/24 22:55, Stefan Berger wrote:
On 11/15/24 4:19 AM, Michal Prívozník wrote:
I've uploaded my suggestions here:
https://gitlab.com/MichalPrivoznik/libvirt/-/commits/review_swtpm? ref_type=heads
If you are fine with them, I can squash those fixup commits and merge.
I tested it. The changes look good to me. Thanks also for the test case.
Stefan
Awesome. Squashed in my suggestions and merged. Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
Hi On Wed, Nov 13, 2024 at 9:40 PM Stefan Berger <stefanb@linux.ibm.com> wrote:
Upcoming libtpms v0.10 and swtpm v0.10 will have TPM profile support that allows to restrict a TPM's provided set of crypto algorithms and commands and through which backwards compatibility and migration from newer versions of libtpms to older ones (up to libtpms v0.9) is supported. For the latter to work it is necessary that the user chooses the right ('null') profile.
This series adds support for passing a profile choice to swtpm_setup by setting it in the domain XML using the <profile/> XML node. An optional attribute 'remove_disabled' can be set in this node and accepts two values:
"check": test a few crypto algorithms (tdes, camellia, unpadded encryption, and others) for whether they are currently disabled due to FIPS mode on the host and remove these algorithms in the 'custom' profile if they are disabled; "fips-host": do not test but remove all the possibly disabled crypto algorithms (from list above)
Also extend the documentation but point the user to swtpm and libtpms documentation for further details.
Follow Deniel's suggestions there's now a PR for swtpm_setup to support searching for profiles though a configurable local directory, distro directory and if no profile could be found there (with appended ".json" suffix) it will fall back to try to use a built-in profile by the provided name: https://github.com/stefanberger/swtpm/pull/918
Stefan
v4: - Renamed previous 'name' attribute in profile XML node to 'source' to indicate that the profile was created from some sort of 'source'. The 'name' is now set from the name of the profile read from the swtpm instance's state once it has been created.
This difference between 'source' and 'name' is not described in the domain xml documentation. Also the doc still has 10.??.0. thanks
participants (3)
-
Marc-André Lureau -
Michal Prívozník -
Stefan Berger