Re: [RFC PATCH v1 3/6] schema: Extend schema for TPM emulator profile node

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