Add support for parsing swtpm_setup 'cmdarg-profile' capability (since v0.10). Signed-off-by: Stefan Berger <stefanb(a)linux.ibm.com&gt;

--- 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(a)linux.ibm.com&gt;

--- 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(a)linux.ibm.com&gt; --- 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

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(a)linux.ibm.com&gt; 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(a)linux.ibm.com&gt; > > --- > > 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)

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(a)linux.ibm.com&gt; 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(a)linux.ibm.com&gt; >>>> --- >>>>   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

Hi Stefan On Thu, Sep 19, 2024 at 9:00 PM Stefan Berger <stefanb(a)linux.ibm.com&gt; 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(a)linux.ibm.com&gt; > --- > 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".