Re: [PATCH v2] conf: Add support for keeping TPM emulator state

[Daniel Henrique Barboza <danielhb413@xxxxxxxxx>]

On 1/3/21 11:31 PM, Eiichi Tsukata wrote:
> Currently, swtpm TPM state file is removed when a transient domain is
> powered off or undefined. When we store TPM state on a shared storage
> such as NFS and use transient domain, TPM states should be kept as it is.
>
> Add per-TPM emulator option `persistent_sate` for keeping TPM state.
> This option only works for the emulator type backend and looks as follows:
>
>    <tpm model='tpm-tis'>
>      <backend type='emulator' persistent_state='yes'/>
>    </tpm>
>
> Signed-off-by: Eiichi Tsukata <eiichi.tsukata@xxxxxxxxxxx>
> Reviewed-by: Stefan Berger <stefanb@xxxxxxxxxxxxx>

Reviewed-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx>

> ---
>   docs/formatdomain.rst                         |  7 ++++
>   docs/schemas/domaincommon.rng                 | 12 ++++++
>   src/conf/domain_conf.c                        | 21 ++++++++++
>   src/conf/domain_conf.h                        |  1 +
>   src/qemu/qemu_tpm.c                           |  3 +-
>   ...pm-emulator-tpm2-pstate.x86_64-latest.args | 38 +++++++++++++++++++
>   .../tpm-emulator-tpm2-pstate.xml              | 30 +++++++++++++++
>   tests/qemuxml2argvtest.c                      |  1 +
>   ...tpm-emulator-tpm2-pstate.x86_64-latest.xml | 37 ++++++++++++++++++
>   tests/qemuxml2xmltest.c                       |  1 +
>   10 files changed, 150 insertions(+), 1 deletion(-)
>   create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2-pstate.x86_64-latest.args
>   create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2-pstate.xml
>   create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator-tpm2-pstate.x86_64-latest.xml
>
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index 512939679b..87bbd1a4f1 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -6986,6 +6986,13 @@ Example: usage of the TPM Emulator
>      -  '1.2' : creates a TPM 1.2
>      -  '2.0' : creates a TPM 2.0
>   
> +``persistent_state``
> +   The ``persistent_state`` attribute indicates whether 'swtpm' TPM state is
> +   kept or not when a transient domain is powered off or undefined. This
> +   option can be used for preserving TPM state. By default the value is ``no``.
> +   This attribute only works with the ``emulator`` backend. The accepted values
> +   are ``yes`` and ``no``.
> +
>   ``encryption``
>      The ``encryption`` element allows the state of a TPM emulator to be
>      encrypted. The ``secret`` must reference a secret object that holds the
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 795b654feb..d7cedc014c 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -4780,6 +4780,18 @@
>             </optional>
>           </group>
>         </choice>
> +      <choice>
> +        <group>
> +          <optional>
> +            <attribute name="persistent_state">
> +              <choice>
> +                <value>yes</value>
> +                <value>no</value>
> +              </choice>
> +           </attribute>
> +          </optional>
> +        </group>
> +      </choice>
>       </element>
>     </define>
>   
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 23415b323c..82c3a68347 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -13178,6 +13178,12 @@ virDomainSmartcardDefParseXML(virDomainXMLOptionPtr xmlopt,
>    *     <encryption secret='32ee7e76-2178-47a1-ab7b-269e6e348015'/>
>    *   </backend>
>    * </tpm>
> + *
> + * Emulator persistent_state is supported with the following:
> + *
> + * <tpm model='tpm-tis'>
> + *   <backend type='emulator' version='2.0' persistent_state='yes'>
> + * </tpm>
>    */
>   static virDomainTPMDefPtr
>   virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
> @@ -13193,6 +13199,7 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
>       g_autofree char *backend = NULL;
>       g_autofree char *version = NULL;
>       g_autofree char *secretuuid = NULL;
> +    g_autofree char *persistent_state = NULL;
>       g_autofree xmlNodePtr *backends = NULL;
>   
>       def = g_new0(virDomainTPMDef, 1);
> @@ -13265,6 +13272,18 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
>               }
>               def->data.emulator.hassecretuuid = true;
>           }
> +
> +        persistent_state = virXMLPropString(backends[0], "persistent_state");
> +        if (persistent_state) {
> +            if (virStringParseYesNo(persistent_state,
> +                                    &def->data.emulator.persistent_state) < 0) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                               _("Invalid persistent_state value, either 'yes' or 'no'"));
> +                goto error;
> +            }
> +        } else {
> +            def->data.emulator.persistent_state = false;
> +        }
>           break;
>       case VIR_DOMAIN_TPM_TYPE_LAST:
>           goto error;
> @@ -26952,6 +26971,8 @@ virDomainTPMDefFormat(virBufferPtr buf,
>       case VIR_DOMAIN_TPM_TYPE_EMULATOR:
>           virBufferAsprintf(buf, " version='%s'",
>                             virDomainTPMVersionTypeToString(def->version));
> +        if (def->data.emulator.persistent_state)
> +            virBufferAddLit(buf, " persistent_state='yes'");
>           if (def->data.emulator.hassecretuuid) {
>               char uuidstr[VIR_UUID_STRING_BUFLEN];
>               virBufferAddLit(buf, ">\n");
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 72771c46b9..109625828a 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1362,6 +1362,7 @@ struct _virDomainTPMDef {
>               char *logfile;
>               unsigned char secretuuid[VIR_UUID_BUFLEN];
>               bool hassecretuuid;
> +            bool persistent_state;
>           } emulator;
>       } data;
>   };
> diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
> index 872be16570..532e0912bd 100644
> --- a/src/qemu/qemu_tpm.c
> +++ b/src/qemu/qemu_tpm.c
> @@ -729,7 +729,8 @@ qemuExtTPMCleanupHost(virDomainDefPtr def)
>           if (def->tpms[i]->type != VIR_DOMAIN_TPM_TYPE_EMULATOR)
>               continue;
>   
> -        qemuTPMDeleteEmulatorStorage(def->tpms[i]);
> +        if (!def->tpms[i]->data.emulator.persistent_state)
> +            qemuTPMDeleteEmulatorStorage(def->tpms[i]);
>       }
>   }
>   
> diff --git a/tests/qemuxml2argvdata/tpm-emulator-tpm2-pstate.x86_64-latest.args b/tests/qemuxml2argvdata/tpm-emulator-tpm2-pstate.x86_64-latest.args
> new file mode 100644
> index 0000000000..90505c7a76
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/tpm-emulator-tpm2-pstate.x86_64-latest.args
> @@ -0,0 +1,38 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/tmp/lib/domain--1-TPM-VM \
> +USER=test \
> +LOGNAME=test \
> +XDG_DATA_HOME=/tmp/lib/domain--1-TPM-VM/.local/share \
> +XDG_CACHE_HOME=/tmp/lib/domain--1-TPM-VM/.cache \
> +XDG_CONFIG_HOME=/tmp/lib/domain--1-TPM-VM/.config \
> +QEMU_AUDIO_DRV=none \
> +/usr/bin/qemu-system-x86_64 \
> +-name guest=TPM-VM,debug-threads=on \
> +-S \
> +-object secret,id=masterKey0,format=raw,\
> +file=/tmp/lib/domain--1-TPM-VM/master-key.aes \
> +-machine pc-i440fx-2.12,accel=tcg,usb=off,dump-guest-core=off,\
> +memory-backend=pc.ram \
> +-cpu qemu64 \
> +-m 2048 \
> +-object memory-backend-ram,id=pc.ram,size=2147483648 \
> +-overcommit mem-lock=off \
> +-smp 1,sockets=1,cores=1,threads=1 \
> +-uuid 11d7cd22-da89-3094-6212-079a48a309a1 \
> +-display none \
> +-no-user-config \
> +-nodefaults \
> +-chardev socket,id=charmonitor,fd=1729,server,nowait \
> +-mon chardev=charmonitor,id=monitor,mode=control \
> +-rtc base=utc \
> +-no-shutdown \
> +-boot menu=on,strict=on \
> +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
> +-tpmdev emulator,id=tpm-tpm0,chardev=chrtpm \
> +-chardev socket,id=chrtpm,path=/dev/test \
> +-device tpm-tis,tpmdev=tpm-tpm0,id=tpm0 \
> +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \
> +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
> +resourcecontrol=deny \
> +-msg timestamp=on
> diff --git a/tests/qemuxml2argvdata/tpm-emulator-tpm2-pstate.xml b/tests/qemuxml2argvdata/tpm-emulator-tpm2-pstate.xml
> new file mode 100644
> index 0000000000..45fc4c0e1a
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/tpm-emulator-tpm2-pstate.xml
> @@ -0,0 +1,30 @@
> +<domain type='qemu'>
> +  <name>TPM-VM</name>
> +  <uuid>11d7cd22-da89-3094-6212-079a48a309a1</uuid>
> +  <memory unit='KiB'>2097152</memory>
> +  <currentMemory unit='KiB'>512288</currentMemory>
> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <type arch='x86_64' machine='pc-i440fx-2.12'>hvm</type>
> +    <boot dev='hd'/>
> +    <bootmenu enable='yes'/>
> +  </os>
> +  <features>
> +    <acpi/>
> +  </features>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>destroy</on_crash>
> +  <devices>
> +    <emulator>/usr/bin/qemu-system-x86_64</emulator>
> +    <controller type='usb' index='0'/>
> +    <controller type='pci' index='0' model='pci-root'/>
> +    <input type='mouse' bus='ps2'/>
> +    <input type='keyboard' bus='ps2'/>
> +    <tpm model='tpm-tis'>
> +      <backend type='emulator' version='2.0' persistent_state='yes'/>
> +    </tpm>
> +    <memballoon model='virtio'/>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 9b853c6d59..e96a51d18b 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -2460,6 +2460,7 @@ mymain(void)
>       DO_TEST_CAPS_LATEST("tpm-emulator");
>       DO_TEST_CAPS_LATEST("tpm-emulator-tpm2");
>       DO_TEST_CAPS_LATEST("tpm-emulator-tpm2-enc");
> +    DO_TEST_CAPS_LATEST("tpm-emulator-tpm2-pstate");
>       DO_TEST_CAPS_LATEST_PPC64("tpm-emulator-spapr");
>   
>       DO_TEST_PARSE_ERROR("pci-domain-invalid", NONE);
> diff --git a/tests/qemuxml2xmloutdata/tpm-emulator-tpm2-pstate.x86_64-latest.xml b/tests/qemuxml2xmloutdata/tpm-emulator-tpm2-pstate.x86_64-latest.xml
> new file mode 100644
> index 0000000000..08bc8d690c
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/tpm-emulator-tpm2-pstate.x86_64-latest.xml
> @@ -0,0 +1,37 @@
> +<domain type='qemu'>
> +  <name>TPM-VM</name>
> +  <uuid>11d7cd22-da89-3094-6212-079a48a309a1</uuid>
> +  <memory unit='KiB'>2097152</memory>
> +  <currentMemory unit='KiB'>512288</currentMemory>
> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <type arch='x86_64' machine='pc-i440fx-2.12'>hvm</type>
> +    <boot dev='hd'/>
> +    <bootmenu enable='yes'/>
> +  </os>
> +  <features>
> +    <acpi/>
> +  </features>
> +  <cpu mode='custom' match='exact' check='none'>
> +    <model fallback='forbid'>qemu64</model>
> +  </cpu>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>destroy</on_crash>
> +  <devices>
> +    <emulator>/usr/bin/qemu-system-x86_64</emulator>
> +    <controller type='usb' index='0' model='piix3-uhci'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
> +    </controller>
> +    <controller type='pci' index='0' model='pci-root'/>
> +    <input type='mouse' bus='ps2'/>
> +    <input type='keyboard' bus='ps2'/>
> +    <tpm model='tpm-tis'>
> +      <backend type='emulator' version='2.0' persistent_state='yes'/>
> +    </tpm>
> +    <memballoon model='virtio'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
> +    </memballoon>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index 1968be6782..f8bca9f559 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -761,6 +761,7 @@ mymain(void)
>       DO_TEST_CAPS_LATEST("tpm-emulator");
>       DO_TEST_CAPS_LATEST("tpm-emulator-tpm2");
>       DO_TEST_CAPS_LATEST("tpm-emulator-tpm2-enc");
> +    DO_TEST_CAPS_LATEST("tpm-emulator-tpm2-pstate");
>   
>       DO_TEST("metadata", NONE);
>       DO_TEST("metadata-duplicate", NONE);