[RFC PATCH v1 0/6] 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 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 potentially disabled crypto algorithms Also extend the documentation but point the user to swtpm and libtpms documentation for further details. Stefan Stefan Berger (6): 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: Run swtpm_setup with --profile option if profile given docs/formatdomain.rst | 20 ++++++++++++++++ src/conf/domain_conf.c | 39 +++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 11 +++++++++ src/conf/domain_validate.c | 7 ++++++ src/conf/schemas/basictypes.rng | 6 +++++ src/conf/schemas/domaincommon.rng | 17 ++++++++++++++ src/qemu/qemu_tpm.c | 26 +++++++++++++++++++-- src/util/virtpm.c | 1 + src/util/virtpm.h | 1 + tests/testutilsqemu.c | 1 + 10 files changed, 127 insertions(+), 2 deletions(-) -- 2.46.0

Add support for parsing swtpm_setup 'cmdarg-profile' capability (since v0.10). 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 81fd6166cf..d991657696 100644 --- a/src/util/virtpm.c +++ b/src/util/virtpm.c @@ -50,6 +50,7 @@ VIR_ENUM_IMPL(virTPMSwtpmSetupFeature, "cmdarg-reconfigure-pcr-banks", "tpm-1.2", "tpm-2.0", + "cmdarg-profile", ); /** diff --git a/src/util/virtpm.h b/src/util/virtpm.h index fb330effa8..18c2877c03 100644 --- a/src/util/virtpm.h +++ b/src/util/virtpm.h @@ -42,6 +42,7 @@ typedef enum { VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_RECONFIGURE_PCR_BANKS, VIR_TPM_SWTPM_SETUP_FEATURE_TPM_1_2, VIR_TPM_SWTPM_SETUP_FEATURE_TPM_2_0, + 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 ee6cae218a..ba4677fb4c 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -71,6 +71,7 @@ virTPMSwtpmSetupCapsGet(virTPMSwtpmSetupFeature cap) case VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_CREATE_CONFIG_FILES: 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_CMDARG_PROFILE: case VIR_TPM_SWTPM_SETUP_FEATURE_LAST: break; } -- 2.46.0

On Thu, Sep 19, 2024 at 9:00 PM Stefan Berger <stefanb@linux.ibm.com> wrote:
Add support for parsing swtpm_setup 'cmdarg-profile' capability (since v0.10).
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.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 81fd6166cf..d991657696 100644 --- a/src/util/virtpm.c +++ b/src/util/virtpm.c @@ -50,6 +50,7 @@ VIR_ENUM_IMPL(virTPMSwtpmSetupFeature, "cmdarg-reconfigure-pcr-banks", "tpm-1.2", "tpm-2.0", + "cmdarg-profile", );
/** diff --git a/src/util/virtpm.h b/src/util/virtpm.h index fb330effa8..18c2877c03 100644 --- a/src/util/virtpm.h +++ b/src/util/virtpm.h @@ -42,6 +42,7 @@ typedef enum { VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_RECONFIGURE_PCR_BANKS, VIR_TPM_SWTPM_SETUP_FEATURE_TPM_1_2, VIR_TPM_SWTPM_SETUP_FEATURE_TPM_2_0, + 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 ee6cae218a..ba4677fb4c 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -71,6 +71,7 @@ virTPMSwtpmSetupCapsGet(virTPMSwtpmSetupFeature cap) case VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_CREATE_CONFIG_FILES: 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_CMDARG_PROFILE: case VIR_TPM_SWTPM_SETUP_FEATURE_LAST: break; } -- 2.46.0

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 7f6a91c427..1c8fffdfa5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1330,6 +1330,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 a15af4fae3..97972f9909 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1463,6 +1463,14 @@ typedef enum { VIR_DOMAIN_TPM_PCR_BANK_LAST } virDomainPcrBank; +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 _virDomainTPMDef { @@ -4278,6 +4286,7 @@ VIR_ENUM_DECL(virDomainTPMModel); VIR_ENUM_DECL(virDomainTPMBackend); VIR_ENUM_DECL(virDomainTPMVersion); VIR_ENUM_DECL(virDomainTPMPcrBank); +VIR_ENUM_DECL(virDomainTPMProfileRemoveDisabled); VIR_ENUM_DECL(virDomainMemoryModel); VIR_ENUM_DECL(virDomainMemoryBackingModel); VIR_ENUM_DECL(virDomainMemorySource); -- 2.46.0

On Thu, Sep 19, 2024 at 9:01 PM Stefan Berger <stefanb@linux.ibm.com> wrote:
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.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 7f6a91c427..1c8fffdfa5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1330,6 +1330,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 a15af4fae3..97972f9909 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1463,6 +1463,14 @@ typedef enum { VIR_DOMAIN_TPM_PCR_BANK_LAST } virDomainPcrBank;
+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 _virDomainTPMDef { @@ -4278,6 +4286,7 @@ VIR_ENUM_DECL(virDomainTPMModel); VIR_ENUM_DECL(virDomainTPMBackend); VIR_ENUM_DECL(virDomainTPMVersion); VIR_ENUM_DECL(virDomainTPMPcrBank); +VIR_ENUM_DECL(virDomainTPMProfileRemoveDisabled); VIR_ENUM_DECL(virDomainMemoryModel); VIR_ENUM_DECL(virDomainMemoryBackingModel); VIR_ENUM_DECL(virDomainMemorySource); -- 2.46.0

Extend the schema for the TPM emulator profile node. Require that the profile the user provides looks like a JSON map that at least starts with '{' and ends with '}'. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/conf/schemas/basictypes.rng | 6 ++++++ src/conf/schemas/domaincommon.rng | 17 +++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/src/conf/schemas/basictypes.rng b/src/conf/schemas/basictypes.rng index 2931e316b7..06df0fe67e 100644 --- a/src/conf/schemas/basictypes.rng +++ b/src/conf/schemas/basictypes.rng @@ -677,4 +677,10 @@ </element> </define> + <define name="JSONMap"> + <data type="string"> + <param name="pattern">\{.*\}</param> + </data> + </define> + </grammar> diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index efb5f00d77..f80a6afc06 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -5923,6 +5923,7 @@ <interleave> <ref name="tpm-backend-emulator-encryption"/> <ref name="tpm-backend-emulator-active-pcr-banks"/> + <ref name="tpm-backend-emulator-profile"/> </interleave> <optional> <attribute name="persistent_state"> @@ -6020,6 +6021,22 @@ </optional> </define> + <define name="tpm-backend-emulator-profile"> + <optional> + <element name="profile"> + <optional> + <attribute name="remove_disabled"> + <choice> + <value>check</value> + <value>fips-host</value> + </choice> + </attribute> + </optional> + <ref name="JSONMap"/> + </element> + </optional> + </define> + <define name="vsock"> <element name="vsock"> <optional> -- 2.46.0

Hi On Thu, Sep 19, 2024 at 10:05 PM Stefan Berger <stefanb@linux.ibm.com> wrote:
Extend the schema for the TPM emulator profile node. Require that the profile the user provides looks like a JSON map that at least starts with '{' and ends with '}'.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/conf/schemas/basictypes.rng | 6 ++++++ src/conf/schemas/domaincommon.rng | 17 +++++++++++++++++ 2 files changed, 23 insertions(+)
diff --git a/src/conf/schemas/basictypes.rng b/src/conf/schemas/basictypes.rng index 2931e316b7..06df0fe67e 100644 --- a/src/conf/schemas/basictypes.rng +++ b/src/conf/schemas/basictypes.rng @@ -677,4 +677,10 @@ </element> </define>
+ <define name="JSONMap"> + <data type="string"> + <param name="pattern">\{.*\}</param> + </data> + </define>
It's unfortunate, but I think this should rather be XML and converted to JSON internally (after all, that's part of what libvirt does with QEMU configuration, somehow) if there is a precedent for such mixing of languages, and it's acceptable I am okay with it too
+ </grammar> diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index efb5f00d77..f80a6afc06 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -5923,6 +5923,7 @@ <interleave> <ref name="tpm-backend-emulator-encryption"/> <ref name="tpm-backend-emulator-active-pcr-banks"/> + <ref name="tpm-backend-emulator-profile"/> </interleave> <optional> <attribute name="persistent_state"> @@ -6020,6 +6021,22 @@ </optional> </define>
+ <define name="tpm-backend-emulator-profile"> + <optional> + <element name="profile"> + <optional> + <attribute name="remove_disabled"> + <choice> + <value>check</value> + <value>fips-host</value> + </choice> + </attribute> + </optional> + <ref name="JSONMap"/> + </element> + </optional> + </define> + <define name="vsock"> <element name="vsock"> <optional> -- 2.46.0

On Fri, Sep 20, 2024 at 15:24:03 +0400, Marc-André Lureau wrote:
Hi
On Thu, Sep 19, 2024 at 10:05 PM Stefan Berger <stefanb@linux.ibm.com> wrote:
Extend the schema for the TPM emulator profile node. Require that the profile the user provides looks like a JSON map that at least starts with '{' and ends with '}'.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/conf/schemas/basictypes.rng | 6 ++++++ src/conf/schemas/domaincommon.rng | 17 +++++++++++++++++ 2 files changed, 23 insertions(+)
diff --git a/src/conf/schemas/basictypes.rng b/src/conf/schemas/basictypes.rng index 2931e316b7..06df0fe67e 100644 --- a/src/conf/schemas/basictypes.rng +++ b/src/conf/schemas/basictypes.rng @@ -677,4 +677,10 @@ </element> </define>
+ <define name="JSONMap"> + <data type="string"> + <param name="pattern">\{.*\}</param> + </data> + </define>
It's unfortunate, but I think this should rather be XML and converted to JSON internally (after all, that's part of what libvirt does with QEMU configuration, somehow)
Yeah, having arbitrary JSON is weird and also bypasses the philosophy that libvirt should express in the schema only what we really support (E.g. no raw arbitrary value passthrough, unless explicitly marked as without guarantees)
if there is a precedent for such mixing of languages, and it's acceptable I am okay with it too
We don't have anything like that.

On Fri, Sep 20, 2024 at 01:53:41PM +0200, Peter Krempa wrote:
On Fri, Sep 20, 2024 at 15:24:03 +0400, Marc-André Lureau wrote:
Hi
On Thu, Sep 19, 2024 at 10:05 PM Stefan Berger <stefanb@linux.ibm.com> wrote:
Extend the schema for the TPM emulator profile node. Require that the profile the user provides looks like a JSON map that at least starts with '{' and ends with '}'.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/conf/schemas/basictypes.rng | 6 ++++++ src/conf/schemas/domaincommon.rng | 17 +++++++++++++++++ 2 files changed, 23 insertions(+)
diff --git a/src/conf/schemas/basictypes.rng b/src/conf/schemas/basictypes.rng index 2931e316b7..06df0fe67e 100644 --- a/src/conf/schemas/basictypes.rng +++ b/src/conf/schemas/basictypes.rng @@ -677,4 +677,10 @@ </element> </define>
+ <define name="JSONMap"> + <data type="string"> + <param name="pattern">\{.*\}</param> + </data> + </define>
It's unfortunate, but I think this should rather be XML and converted to JSON internally (after all, that's part of what libvirt does with QEMU configuration, somehow)
Yeah, having arbitrary JSON is weird and also bypasses the philosophy that libvirt should express in the schema only what we really support (E.g. no raw arbitrary value passthrough, unless explicitly marked as without guarantees)
IMHO, we should not be defining raw crypto profiles in the XML at all, whether JSON or not. I don't see profile definitions as being something that needs to change per-VM definition. This is a case where there ought to be a set of common profiles defined, and just referenced by name at the VM configuration level. IOW swtpm itself is fully configurable which makes sense, but we don't need to expose this up to the libvirt level. Instead I think there should be a defined standard for how an distro package, or host sysadmin, would "drop in" a profile definition to a well defined directory, where upon we can reference it by name in libvirt, eg define two dirs /usr/share/swptm/profiles/<name>.json (for os distro) /etc/swptm/profiles/<name>.json (for local deployment) With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On 9/20/24 8:55 AM, Daniel P. Berrangé wrote:
On Fri, Sep 20, 2024 at 01:53:41PM +0200, Peter Krempa wrote:
On Fri, Sep 20, 2024 at 15:24:03 +0400, Marc-André Lureau wrote:
Hi
On Thu, Sep 19, 2024 at 10:05 PM Stefan Berger <stefanb@linux.ibm.com> wrote:
Extend the schema for the TPM emulator profile node. Require that the profile the user provides looks like a JSON map that at least starts with '{' and ends with '}'.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/conf/schemas/basictypes.rng | 6 ++++++ src/conf/schemas/domaincommon.rng | 17 +++++++++++++++++ 2 files changed, 23 insertions(+)
diff --git a/src/conf/schemas/basictypes.rng b/src/conf/schemas/basictypes.rng index 2931e316b7..06df0fe67e 100644 --- a/src/conf/schemas/basictypes.rng +++ b/src/conf/schemas/basictypes.rng @@ -677,4 +677,10 @@ </element> </define>
+ <define name="JSONMap"> + <data type="string"> + <param name="pattern">\{.*\}</param> + </data> + </define>
It's unfortunate, but I think this should rather be XML and converted to JSON internally (after all, that's part of what libvirt does with QEMU configuration, somehow)
Yeah, having arbitrary JSON is weird and also bypasses the philosophy that libvirt should express in the schema only what we really support (E.g. no raw arbitrary value passthrough, unless explicitly marked as without guarantees)
IMHO, we should not be defining raw crypto profiles in the XML at all, whether JSON or not. I don't see profile definitions as being something that needs to change per-VM definition. This is a case where there ought to be a set of common profiles defined, and just referenced by name at the VM configuration level.
IOW swtpm itself is fully configurable which makes sense, but we don't need to expose this up to the libvirt level.
FYI: Currently the following profiles are supported: swtpm_ioctl --tcp :2322 --info 0x40 | jq { "AvailableProfiles": [ { "Name": "default-v1", "StateFormatLevel": 7, "Commands": "0x11f-0x122,0x124-0x12e,0x130-0x140,0x142-0x159,0x15b-0x15e,0x160-0x165,0x167-0x174,0x176-0x178,0x17a-0x193,0x197,0x199-0x19c", "Algorithms": "rsa,rsa-min-size=1024,tdes,tdes-min-size=128,sha1,hmac,aes,aes-min-size=128,mgf1,keyedhash,xor,sha256,sha384,sha512,null,rsassa,rsaes,rsapss,oaep,ecdsa,ecdh,ecdaa,sm2,ecschnorr,ecmqv,kdf1-sp800-56a,kdf2,kdf1-sp800-108,ecc,ecc-min-size=192,ecc-nist,ecc-bn,ecc-sm2-p256,symcipher,camellia,camellia-min-size=128,cmac,ctr,ofb,cbc,cfb,ecb", "Description": "This profile enables all libtpms v0.10-supported commands and algorithms. This profile is compatible with libtpms >= v0.10." }, { "Name": "null", "StateFormatLevel": 1, "Commands": "0x11f-0x122,0x124-0x12e,0x130-0x140,0x142-0x159,0x15b-0x15e,0x160-0x165,0x167-0x174,0x176-0x178,0x17a-0x193,0x197", "Algorithms": "rsa,rsa-min-size=1024,tdes,tdes-min-size=128,sha1,hmac,aes,aes-min-size=128,mgf1,keyedhash,xor,sha256,sha384,sha512,null,rsassa,rsaes,rsapss,oaep,ecdsa,ecdh,ecdaa,sm2,ecschnorr,ecmqv,kdf1-sp800-56a,kdf2,kdf1-sp800-108,ecc,ecc-min-size=192,ecc-nist,ecc-bn,ecc-sm2-p256,symcipher,camellia,camellia-min-size=128,cmac,ctr,ofb,cbc,cfb,ecb", "Description": "The profile enables the commands and algorithms that were enabled in libtpms v0.9. This profile is automatically used when the state does not have a profile, for example when it was created by libtpms v0.9 or before. This profile enables compatibility with libtpms >= v0.9." }, { "Name": "custom", "StateFormatLevel": 2, "Commands": "0x11f-0x122,0x124-0x12e,0x130-0x140,0x142-0x159,0x15b-0x15e,0x160-0x165,0x167-0x174,0x176-0x178,0x17a-0x193,0x197,0x199-0x19c", "Algorithms": "rsa,rsa-min-size=1024,tdes,tdes-min-size=128,sha1,hmac,aes,aes-min-size=128,mgf1,keyedhash,xor,sha256,sha384,sha512,null,rsassa,rsaes,rsapss,oaep,ecdsa,ecdh,ecdaa,sm2,ecschnorr,ecmqv,kdf1-sp800-56a,kdf2,kdf1-sp800-108,ecc,ecc-min-size=192,ecc-nist,ecc-bn,ecc-sm2-p256,symcipher,camellia,camellia-min-size=128,cmac,ctr,ofb,cbc,cfb,ecb", "Description": "This profile allows customization of enabled algorithms and commands. This profile requires at least libtpms v0.10." } ] } The null profile is an implicit profile for all libtpms v0.9 instances upgrade to later version of libtpms. It is implicit because it is not part of the TPM state. It can be passed as '{"Name":"null"}' to a TPM 2 created with lintpms v0.10 that can then also work with libtpms v0.9. The default-v1 profile will be applied to any instance that does not provide a profile explicitly. A TPM created with it cannot be read by libtpms v0.9. Only the custom profile can be modified but that leaves enough room for distros to design a profile except they need to call it 'custom'. They can change the rest within the limits of what is allowed to be removed and of course what doesn't throw off any potential application (test suites?) that may now be missing some TPM functionality.
Instead I think there should be a defined standard for how an distro package, or host sysadmin, would "drop in" a profile definition to a well defined directory, where upon we can reference it by name in libvirt,
eg define two dirs
/usr/share/swptm/profiles/<name>.json (for os distro) /etc/swptm/profiles/<name>.json (for local deployment)
With the above: <profile name='null' type='built-in'/> <profile name='default-v1' type='built-in'/> <profile name='custom' type='built-in' remove_disabled='check'/> <profile name='restricted' type='distro'/> --> name is a filename now <profile name='test' type='local' remove_disabled='check'/> --> name is a filename now I suppose the above paths should be part of a config file? Stefan
With regards, Daniel

On 9/20/24 10:00 AM, Stefan Berger wrote:
On 9/20/24 8:55 AM, Daniel P. Berrangé wrote:
On Fri, Sep 20, 2024 at 01:53:41PM +0200, Peter Krempa wrote:
On Fri, Sep 20, 2024 at 15:24:03 +0400, Marc-André Lureau wrote:
Hi
On Thu, Sep 19, 2024 at 10:05 PM Stefan Berger <stefanb@linux.ibm.com> wrote:
Extend the schema for the TPM emulator profile node. Require that the profile the user provides looks like a JSON map that at least starts with '{' and ends with '}'.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/conf/schemas/basictypes.rng | 6 ++++++ src/conf/schemas/domaincommon.rng | 17 +++++++++++++++++ 2 files changed, 23 insertions(+)
diff --git a/src/conf/schemas/basictypes.rng b/src/conf/schemas/basictypes.rng index 2931e316b7..06df0fe67e 100644 --- a/src/conf/schemas/basictypes.rng +++ b/src/conf/schemas/basictypes.rng @@ -677,4 +677,10 @@ </element> </define>
+ <define name="JSONMap"> + <data type="string"> + <param name="pattern">\{.*\}</param> + </data> + </define>
It's unfortunate, but I think this should rather be XML and converted to JSON internally (after all, that's part of what libvirt does with QEMU configuration, somehow)
Yeah, having arbitrary JSON is weird and also bypasses the philosophy that libvirt should express in the schema only what we really support (E.g. no raw arbitrary value passthrough, unless explicitly marked as without guarantees)
IMHO, we should not be defining raw crypto profiles in the XML at all, whether JSON or not. I don't see profile definitions as being something that needs to change per-VM definition. This is a case where there ought to be a set of common profiles defined, and just referenced by name at the VM configuration level.
IOW swtpm itself is fully configurable which makes sense, but we don't need to expose this up to the libvirt level.
FYI: Currently the following profiles are supported:
swtpm_ioctl --tcp :2322 --info 0x40 | jq { "AvailableProfiles": [ { "Name": "default-v1", "StateFormatLevel": 7, "Commands": "0x11f-0x122,0x124-0x12e,0x130-0x140,0x142-0x159,0x15b-0x15e,0x160-0x165,0x167-0x174,0x176-0x178,0x17a-0x193,0x197,0x199-0x19c", "Algorithms": "rsa,rsa-min-size=1024,tdes,tdes-min-size=128,sha1,hmac,aes,aes-min-size=128,mgf1,keyedhash,xor,sha256,sha384,sha512,null,rsassa,rsaes,rsapss,oaep,ecdsa,ecdh,ecdaa,sm2,ecschnorr,ecmqv,kdf1-sp800-56a,kdf2,kdf1-sp800-108,ecc,ecc-min-size=192,ecc-nist,ecc-bn,ecc-sm2-p256,symcipher,camellia,camellia-min-size=128,cmac,ctr,ofb,cbc,cfb,ecb", "Description": "This profile enables all libtpms v0.10-supported commands and algorithms. This profile is compatible with libtpms >= v0.10." }, { "Name": "null", "StateFormatLevel": 1, "Commands": "0x11f-0x122,0x124-0x12e,0x130-0x140,0x142-0x159,0x15b-0x15e,0x160-0x165,0x167-0x174,0x176-0x178,0x17a-0x193,0x197", "Algorithms": "rsa,rsa-min-size=1024,tdes,tdes-min-size=128,sha1,hmac,aes,aes-min-size=128,mgf1,keyedhash,xor,sha256,sha384,sha512,null,rsassa,rsaes,rsapss,oaep,ecdsa,ecdh,ecdaa,sm2,ecschnorr,ecmqv,kdf1-sp800-56a,kdf2,kdf1-sp800-108,ecc,ecc-min-size=192,ecc-nist,ecc-bn,ecc-sm2-p256,symcipher,camellia,camellia-min-size=128,cmac,ctr,ofb,cbc,cfb,ecb", "Description": "The profile enables the commands and algorithms that were enabled in libtpms v0.9. This profile is automatically used when the state does not have a profile, for example when it was created by libtpms v0.9 or before. This profile enables compatibility with libtpms >= v0.9." }, { "Name": "custom", "StateFormatLevel": 2, "Commands": "0x11f-0x122,0x124-0x12e,0x130-0x140,0x142-0x159,0x15b-0x15e,0x160-0x165,0x167-0x174,0x176-0x178,0x17a-0x193,0x197,0x199-0x19c", "Algorithms": "rsa,rsa-min-size=1024,tdes,tdes-min-size=128,sha1,hmac,aes,aes-min-size=128,mgf1,keyedhash,xor,sha256,sha384,sha512,null,rsassa,rsaes,rsapss,oaep,ecdsa,ecdh,ecdaa,sm2,ecschnorr,ecmqv,kdf1-sp800-56a,kdf2,kdf1-sp800-108,ecc,ecc-min-size=192,ecc-nist,ecc-bn,ecc-sm2-p256,symcipher,camellia,camellia-min-size=128,cmac,ctr,ofb,cbc,cfb,ecb", "Description": "This profile allows customization of enabled algorithms and commands. This profile requires at least libtpms v0.10." } ] }
The null profile is an implicit profile for all libtpms v0.9 instances upgrade to later version of libtpms. It is implicit because it is not part of the TPM state. It can be passed as '{"Name":"null"}' to a TPM 2 created with lintpms v0.10 that can then also work with libtpms v0.9.
The default-v1 profile will be applied to any instance that does not provide a profile explicitly. A TPM created with it cannot be read by libtpms v0.9.
Only the custom profile can be modified but that leaves enough room for distros to design a profile except they need to call it 'custom'. They can change the rest within the limits of what is allowed to be removed and of course what doesn't throw off any potential application (test suites?) that may now be missing some TPM functionality.
Instead I think there should be a defined standard for how an distro package, or host sysadmin, would "drop in" a profile definition to a well defined directory, where upon we can reference it by name in libvirt,
eg define two dirs
/usr/share/swptm/profiles/<name>.json (for os distro) /etc/swptm/profiles/<name>.json (for local deployment)
With the above:
<profile name='null' type='built-in'/> <profile name='default-v1' type='built-in'/> <profile name='custom' type='built-in' remove_disabled='check'/>
<profile name='restricted' type='distro'/> --> name is a filename now <profile name='test' type='local' remove_disabled='check'/> --> name is a filename now
Better: <profile builtin="null"/> <profile builtin="default-v1"/> <profile builtin=custom" remove_disabled='check'/> <profile distro='restricted'/> <profile local='test' remove_disabled='check'/>
I suppose the above paths should be part of a config file?
Stefan
With regards, Daniel

On Fri, Sep 20, 2024 at 10:00:40AM -0400, Stefan Berger wrote:
On 9/20/24 8:55 AM, Daniel P. Berrangé wrote:
Instead I think there should be a defined standard for how an distro package, or host sysadmin, would "drop in" a profile definition to a well defined directory, where upon we can reference it by name in libvirt,
eg define two dirs
/usr/share/swptm/profiles/<name>.json (for os distro) /etc/swptm/profiles/<name>.json (for local deployment)
With the above:
<profile name='null' type='built-in'/> <profile name='default-v1' type='built-in'/> <profile name='custom' type='built-in' remove_disabled='check'/>
<profile name='restricted' type='distro'/> --> name is a filename now <profile name='test' type='local' remove_disabled='check'/> --> name is a filename now
Do we really need to express a "type" attribute ? How about if swtpm itself were to load profiles from the /usr/share/swtpm and /etc/swtpm directories, so that from a users' POV there is no distinction between built-in & file defined profiles ? I guess you want to resolve naming clashes. A couple of options - <name>.json in /etc/ overrides <name>.json in /usr/ which overrides <name> built-in. - <name>.json in /etc is ignored if it clashes with <name>.json in /usr or built-in - swtpm gives the profile name a prefix itself, based on where it came from eg "system:blah" or "local:blah" for /usr/ and /etc respectively. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On 9/23/24 12:55 PM, Daniel P. Berrangé wrote:
On Fri, Sep 20, 2024 at 10:00:40AM -0400, Stefan Berger wrote:
On 9/20/24 8:55 AM, Daniel P. Berrangé wrote:
Instead I think there should be a defined standard for how an distro package, or host sysadmin, would "drop in" a profile definition to a well defined directory, where upon we can reference it by name in libvirt,
eg define two dirs
/usr/share/swptm/profiles/<name>.json (for os distro) /etc/swptm/profiles/<name>.json (for local deployment)
With the above:
<profile name='null' type='built-in'/> <profile name='default-v1' type='built-in'/> <profile name='custom' type='built-in' remove_disabled='check'/>
<profile name='restricted' type='distro'/> --> name is a filename now <profile name='test' type='local' remove_disabled='check'/> --> name is a filename now
Do we really need to express a "type" attribute ? How about if swtpm itself were to load profiles from the /usr/share/swtpm and /etc/swtpm directories, so that from a users' POV there is no distinction between built-in & file defined profiles ?
I guess you want to resolve naming clashes. A couple of options
- <name>.json in /etc/ overrides <name>.json in /usr/ which overrides <name> built-in.
I think this makes it easier for a user to choose from: <profile builtin="null"/> <profile builtin="default-v1"/> <profile builtin=custom" remove_disabled='check'/> <profile distro='restricted'/> <profile local='test' remove_disabled='check'/>
- <name>.json in /etc is ignored if it clashes with <name>.json in /usr or built-in
- swtpm gives the profile name a prefix itself, based on where it came from eg "system:blah" or "local:blah" for /usr/ and /etc respectively.
With regards, Daniel

On Mon, Sep 23, 2024 at 01:30:50PM -0400, Stefan Berger wrote:
On 9/23/24 12:55 PM, Daniel P. Berrangé wrote:
On Fri, Sep 20, 2024 at 10:00:40AM -0400, Stefan Berger wrote:
On 9/20/24 8:55 AM, Daniel P. Berrangé wrote:
Instead I think there should be a defined standard for how an distro package, or host sysadmin, would "drop in" a profile definition to a well defined directory, where upon we can reference it by name in libvirt,
eg define two dirs
/usr/share/swptm/profiles/<name>.json (for os distro) /etc/swptm/profiles/<name>.json (for local deployment)
With the above:
<profile name='null' type='built-in'/> <profile name='default-v1' type='built-in'/> <profile name='custom' type='built-in' remove_disabled='check'/>
<profile name='restricted' type='distro'/> --> name is a filename now <profile name='test' type='local' remove_disabled='check'/> --> name is a filename now
Do we really need to express a "type" attribute ? How about if swtpm itself were to load profiles from the /usr/share/swtpm and /etc/swtpm directories, so that from a users' POV there is no distinction between built-in & file defined profiles ?
I guess you want to resolve naming clashes. A couple of options
- <name>.json in /etc/ overrides <name>.json in /usr/ which overrides <name> built-in.
I think this makes it easier for a user to choose from:
<profile builtin="null"/> <profile builtin="default-v1"/> <profile builtin=custom" remove_disabled='check'/> <profile distro='restricted'/> <profile local='test' remove_disabled='check'/>
I think that creates unneccessary upgrade drama. Consider that new swtpm defines a new built-in profile "default-v3", but your current host does not have "default-v3" as a built-in profile. You should be able to define that as a local profile or system profile with the same name, and have an upgrade path to future swtpm which has it as a built-in profile *without* having to change the XML. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On 9/24/24 4:35 AM, Daniel P. Berrangé wrote:
On Mon, Sep 23, 2024 at 01:30:50PM -0400, Stefan Berger wrote:
On 9/23/24 12:55 PM, Daniel P. Berrangé wrote:
On Fri, Sep 20, 2024 at 10:00:40AM -0400, Stefan Berger wrote:
On 9/20/24 8:55 AM, Daniel P. Berrangé wrote:
Instead I think there should be a defined standard for how an distro package, or host sysadmin, would "drop in" a profile definition to a well defined directory, where upon we can reference it by name in libvirt,
eg define two dirs
/usr/share/swptm/profiles/<name>.json (for os distro) /etc/swptm/profiles/<name>.json (for local deployment)
With the above:
<profile name='null' type='built-in'/> <profile name='default-v1' type='built-in'/> <profile name='custom' type='built-in' remove_disabled='check'/>
<profile name='restricted' type='distro'/> --> name is a filename now <profile name='test' type='local' remove_disabled='check'/> --> name is a filename now
Do we really need to express a "type" attribute ? How about if swtpm itself were to load profiles from the /usr/share/swtpm and /etc/swtpm directories, so that from a users' POV there is no distinction between built-in & file defined profiles ?
I guess you want to resolve naming clashes. A couple of options
- <name>.json in /etc/ overrides <name>.json in /usr/ which overrides <name> built-in.
I think this makes it easier for a user to choose from:
<profile builtin="null"/> <profile builtin="default-v1"/> <profile builtin=custom" remove_disabled='check'/> <profile distro='restricted'/> <profile local='test' remove_disabled='check'/>
I think that creates unneccessary upgrade drama. Consider that new swtpm defines a new built-in profile "default-v3", but your current host does not have "default-v3" as a built-in profile. You should
Exactly these default-v{1,2,3,...} profiles will all be built into libtpms and the latest one will be activated if the user chooses no profile when starting swtpm the first time and it creates initial state. swtpm_setup, which is used to 'manufacture' a TPM, tries to read a default profile from its configuration file (/etc/swtpm_setup.conf) and set that one if the user didn't provide one on the swtpm_setup command line. If nothing is set it's back to the default-v{1,..} profile on libtpms level enabling the latest crypto algorithms the TPM 2 supports. The profile default-v3 would be there because new crypto algorithms etc, were added to (future) libtpms v0.12 and they are then enable in this profile. If there weren't any new crypto algorithms or other verbs added to the profile language, libtpms would still be offering only default-v2 (from future libtpms v0.11 presumably). So you will need to have (future) v0.12 of libtpms to run default-v3. And it won't be possible to run this instance on a previous version of libtpms, for this you would have to pick an older default-v{1,2} profile that will still be made available as built-in's. Maybe a search order through directories should be supported on the swtpm_setup level using its configuration file that is either picked up from /etc/swtpm_local.conf or ~/.config/swtpm_setup.conf where one can then specify a local and a distro directory (or the distro one hardcoded in swtpm_setup?) for profiles to search through. The tricky part about other than the built-in profiles will be to decide which algorithms to disable without upsetting applications. But I agree, access to local or distro profiles will be useful along with 'swtpm_setup --tpm2 --print-profiles' for displaying them. I was going to do the directory configuration on the libvirt level...
be able to define that as a local profile or system profile with the same name, and have an upgrade path to future swtpm which has it as a built-in profile *without* having to change the XML. >
With regards, Daniel

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 | 32 ++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/conf/domain_validate.c | 7 +++++++ 3 files changed, 41 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1c8fffdfa5..8dab1cabea 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3471,6 +3471,7 @@ void virDomainTPMDefFree(virDomainTPMDef *def) g_free(def->data.emulator.storagepath); g_free(def->data.emulator.logfile); virBitmapFree(def->data.emulator.activePcrBanks); + g_free(def->data.emulator.profile); break; case VIR_DOMAIN_TPM_TYPE_EXTERNAL: virObjectUnref(def->data.external.source); @@ -10779,6 +10780,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 remove_disabled='check'>{"Name":"custom"}</profile> + * </backend> + * </tpm> + * */ static virDomainTPMDef * virDomainTPMDefParseXML(virDomainXMLOption *xmlopt, @@ -10797,6 +10807,9 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt, g_autofree xmlNodePtr *backends = NULL; g_autofree xmlNodePtr *nodes = NULL; g_autofree char *type = NULL; + g_autofree char *profile = NULL; + virDomainTPMProfileRemoveDisabled profile_remove_disabled; + xmlNodePtr tmp; int bank; if (!(def = virDomainTPMDefNew(xmlopt))) @@ -10887,6 +10900,18 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt, } virBitmapSetBitExpand(def->data.emulator.activePcrBanks, bank); } + + def->data.emulator.profile = virXPathString("string(./backend/profile[1])", ctxt); + if ((tmp = virXPathNode("./backend/profile[1]", ctxt))) { + if (virXMLPropEnum(tmp, "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))) { @@ -25077,6 +25102,13 @@ virDomainTPMDefFormat(virBuffer *buf, virXMLFormatElement(&backendChildBuf, "active_pcr_banks", NULL, &activePcrBanksBuf); } + if (def->data.emulator.profile) { + virBufferAddLit(&backendChildBuf, "<profile"); + if (def->data.emulator.profile_remove_disabled) + virBufferAsprintf(&backendChildBuf, " remove_disabled='%s'", + def->data.emulator.profile_remove_disabled); + virBufferAsprintf(&backendChildBuf, ">%s</profile>\n", def->data.emulator.profile); + } 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 97972f9909..4a171ee4da 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1493,6 +1493,8 @@ struct _virDomainTPMDef { bool hassecretuuid; bool persistent_state; virBitmap *activePcrBanks; + char *profile; + const char *profile_remove_disabled; } emulator; struct { virDomainChrSourceDef *source; diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index eddb4a5e74..efab3aa958 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -3025,6 +3025,13 @@ virDomainTPMDevValidate(const virDomainTPMDef *tpm) virDomainTPMVersionTypeToString(VIR_DOMAIN_TPM_VERSION_2_0)); return -1; } + if (tpm->data.emulator.profile && + 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.46.0

Add documentation for the TPM backend profile node and point the reader to further documentation about TPM profiles available in the swtpm and TPMLIB_SetProfile man pages. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- docs/formatdomain.rst | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 4336cff3ac..abb16df6fc 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -8119,6 +8119,7 @@ Example: usage of the TPM Emulator <active_pcr_banks> <sha256/> </active_pcr_banks> + <profile remove_disabled='check'>{"Name":"custom"}</profile> </backend> </tpm> </devices> @@ -8191,6 +8192,25 @@ 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. 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 + 'default' profile. The '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 '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, 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` + + For further information about TPM profiles see the man pages for ``swtpm`` + (swtpm v0.10) and libtpms's ``TPMLIB_SetProfile`` (libtpms 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.46.0

Hi Stefan On Thu, Sep 19, 2024 at 9:00 PM Stefan Berger <stefanb@linux.ibm.com> wrote:
Add documentation for the TPM backend profile node and point the reader to further documentation about TPM profiles available in the swtpm and TPMLIB_SetProfile man pages.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- docs/formatdomain.rst | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 4336cff3ac..abb16df6fc 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -8119,6 +8119,7 @@ Example: usage of the TPM Emulator <active_pcr_banks> <sha256/> </active_pcr_banks> + <profile remove_disabled='check'>{"Name":"custom"}</profile> </backend> </tpm> </devices> @@ -8191,6 +8192,25 @@ 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. 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 + 'default' profile. The '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 '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, 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` + + For further information about TPM profiles see the man pages for ``swtpm`` + (swtpm v0.10) and libtpms's ``TPMLIB_SetProfile`` (libtpms v0.10).
Here is my feedback, hopefully libvirt maintainers can also comment: - it is a bit confusing: the name of the profile is inside the element json configuration, but you further tune/configure it with element attributes. - it's specific to swtpm, and not self-explanatory (you need to look into swtpm manual page) - remove_disabled="check" vs "fips-host", I hope we can come up with something clearer. What are "algorithms that are currently disabled" vs "potentially disabled algorithms".
+ ``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.46.0

On 9/20/24 7:45 AM, Marc-André Lureau wrote:
Hi Stefan
On Thu, Sep 19, 2024 at 9:00 PM Stefan Berger <stefanb@linux.ibm.com> wrote:
Add documentation for the TPM backend profile node and point the reader to further documentation about TPM profiles available in the swtpm and TPMLIB_SetProfile man pages.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- docs/formatdomain.rst | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 4336cff3ac..abb16df6fc 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -8119,6 +8119,7 @@ Example: usage of the TPM Emulator <active_pcr_banks> <sha256/> </active_pcr_banks> + <profile remove_disabled='check'>{"Name":"custom"}</profile> </backend> </tpm> </devices> @@ -8191,6 +8192,25 @@ 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. 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 + 'default' profile. The '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 '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, 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` + + For further information about TPM profiles see the man pages for ``swtpm`` + (swtpm v0.10) and libtpms's ``TPMLIB_SetProfile`` (libtpms v0.10).
Here is my feedback, hopefully libvirt maintainers can also comment: - it is a bit confusing: the name of the profile is inside the element json configuration, but you further tune/configure it with element attributes.
Ok, I will change this. No JSON will be embedded in the XML.
- it's specific to swtpm, and not self-explanatory (you need to look into swtpm manual page)
For someone to dive into this they need good documentation and where do I all replicate it? TPMLIB_SetProfile needs it and swtpm needs it as well. swtpm_setup man page points to swtpm and TPMLIB_SetProfile for more details. Should there be a wiki page? Does libvirt need all the details of the (design of) profiles as part of its documentation?
- remove_disabled="check" vs "fips-host", I hope we can come up with something clearer. What are "algorithms that are currently disabled" vs "potentially disabled algorithms".
currently disabled: The candidate algorithms (tdes, camellia, support for RSA 1024 bit keys, support for 192 EC keys, unpadded RSA encryption, PKCS1 padded RSA encryption, signing with SHA1) are all individually checked whether they are disabled. Fedora, CentOS, and RHEL, and others have a different set of crypto algorithms disabled when on a FIPS versus when on a non-FIPS host. https://github.com/stefanberger/libtpms/wiki/Algorithms-Restrictions:-FIPS-m.... potentially disabled: all the algorithm candidates that will be tested for whether they are available when 'check' is chosen are just simply disabled without testing for them.
+ ``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.46.0

Runs swtpm_setup with the --profile option if the user provided a profile and swtpm_setup supports the option. Also use the --profile-remove-disabled option if the user provided a value in the remove_disabled attribute in the profile XML node. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/qemu/qemu_tpm.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 2f17918cbb..ec0e456163 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -355,6 +355,8 @@ qemuTPMVirCommandAddEncryption(virCommand *cmd, * @tpmversion: The version of the TPM, either a TPM 1.2 or TPM 2 * @encryption: pointer to virStorageEncryption holding secret * @incomingMigration: whether we have an incoming migration + * @profile: optional TPM 2 profile + * @profile_remove_disabled: value for remove_disabled option parameter * * Setup the external swtpm by creating endorsement key and * certificates for it. @@ -369,7 +371,9 @@ qemuTPMEmulatorRunSetup(const char *storagepath, const char *logfile, const virDomainTPMVersion tpmversion, const unsigned char *secretuuid, - bool incomingMigration) + bool incomingMigration, + const char *profile, + const char *profile_remove_disabled) { g_autoptr(virCommand) cmd = NULL; int exitstatus; @@ -422,6 +426,22 @@ qemuTPMEmulatorRunSetup(const char *storagepath, "--lock-nvram", "--not-overwrite", NULL); + if (profile) { + 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", profile, + NULL); + if (profile_remove_disabled) + virCommandAddArgList(cmd, + "--profile-remove-disable", + profile_remove_disabled, + NULL); + } } else { virCommandAddArgList(cmd, "--tpm-state", storagepath, @@ -584,7 +604,9 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm, privileged, swtpm_user, swtpm_group, tpm->data.emulator.logfile, tpm->data.emulator.version, - secretuuid, incomingMigration) < 0) + secretuuid, incomingMigration, + tpm->data.emulator.profile, + tpm->data.emulator.profile_remove_disabled) < 0) goto error; if (!incomingMigration && -- 2.46.0
participants (4)
-
Daniel P. Berrangé
-
Marc-André Lureau
-
Peter Krempa
-
Stefan Berger