[libvirt] [PATCH 10/12] conf: Add support for choosing emulation of a TPM 2

[Stefan Berger]

On 05/23/2018 11:55 AM, Ján Tomko wrote:
> On Tue, May 22, 2018 at 04:44:51PM -0400, Stefan Berger wrote:
>> This patch extends the TPM's device XML with TPM 2 support. This only 
>> works
>> for the emulator type backend and looks as follows:
>>
>>    <tpm model='tpm-tis'>
>>      <backend type='emulator' version='2'/>
>>    </tpm>
>>
>> The swtpm process now has --tpm2 as an additional parameter:
>>
>> system_u:system_r:svirt_t:s0:c597,c632 tss 18477 11.8  0.0 28364  
>> 3868 ?        Rs   11:13  13:50 /usr/bin/swtpm socket --daemon --ctrl 
>> type=unixio,path=/var/run/libvirt/qemu/swtpm/testvm-swtpm.sock,mode=0660 
>> --tpmstate dir=/var/lib/libvirt/swtpm/testvm/tpm2,mode=0640 --log 
>> file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log --tpm2 --pid 
>> file=/var/run/libvirt/qemu/swtpm/testvm-swtpm.pid
>>
>> The version of the TPM can be changed and the state of the TPM is 
>> preserved.
>>
>> Signed-off-by: Stefan Berger <stefanb at linux.vnet.ibm.com>
>> Reviewed-by: John Ferlan <jferlan at redhat.com>
>> ---
>> docs/formatdomain.html.in                          | 15 ++++-
>> docs/schemas/domaincommon.rng                      | 12 ++++
>> src/conf/domain_conf.c                             | 27 ++++++++-
>> src/conf/domain_conf.h                             |  6 ++
>> src/qemu/qemu_tpm.c                                | 64 
>> +++++++++++++++++++++-
>> .../tpm-emulator-tpm2.x86_64-latest.args           | 33 +++++++++++
>> tests/qemuxml2argvdata/tpm-emulator-tpm2.xml       | 30 ++++++++++
>> tests/qemuxml2argvtest.c                           |  1 +
>> tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml     | 34 ++++++++++++
>> 9 files changed, 217 insertions(+), 5 deletions(-)
>> create mode 100644 
>> tests/qemuxml2argvdata/tpm-emulator-tpm2.x86_64-latest.args
>> create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2.xml
>> create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml
>>
>
>> @@ -24941,6 +24963,9 @@ virDomainTPMDefFormat(virBufferPtr buf,
>>     virBufferAsprintf(buf, "<backend type='%s'",
>> virDomainTPMBackendTypeToString(def->type));
>>
>> +    if (def->version == VIR_DOMAIN_TPM_VERSION_2)
>> +        virBufferAddLit(buf, " version='2'");
>> +
>
> Any reason for not formatting version 1.2?
> We should format implicit defaults in the XML if possible.

Basically I did it because the previous default 1.2 didn't have it. So I 
though I'd keep it as is for 1.2 and only write out 2.

>
>>     switch (def->type) {
>>     case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
>>         virBufferAddLit(buf, ">\n");
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index 92466278ab..e2409899bc 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -1291,12 +1291,18 @@ typedef enum {
>>     VIR_DOMAIN_TPM_TYPE_LAST
>> } virDomainTPMBackendType;
>>
>> +typedef enum {
>> +    VIR_DOMAIN_TPM_VERSION_1_2,
>> +    VIR_DOMAIN_TPM_VERSION_2,
>> +} virDomainTPMVersion;
>
> With a corresponding VIR_ENUM_IMPL and VIR_ENUM_DECL,
> you can use the *{To,From}String functions for parsing/formatting
> the version.
>
>> +
>> # define VIR_DOMAIN_TPM_DEFAULT_DEVICE "/dev/tpm0"
>>
>> struct _virDomainTPMDef {
>>     virDomainTPMBackendType type;
>>     virDomainDeviceInfo info;
>>     virDomainTPMModel model;
>> +    virDomainTPMVersion version;
>>     union {
>>         struct {
>>             virDomainChrSourceDef source;
>> diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
>> index 11b91aa915..508685c455 100644
>> --- a/src/qemu/qemu_tpm.c
>> +++ b/src/qemu/qemu_tpm.c
>> @@ -54,6 +54,41 @@ static char *swtpm_path;
>> static char *swtpm_setup;
>> static char *swtpm_ioctl;
>>
>> +bool swtpm_supports_tpm2;
>> +
>> +/*
>> + * qemuTPMCheckForTPM2Support
>> + *
>> + * Check whether swtpm_setup supports TPM 2
>> + */
>> +static void
>> +qemuTPMCheckForTPM2Support(void)
>> +{
>> +    virCommandPtr cmd;
>> +    char *help = NULL;
>> +
>> +    if (!swtpm_setup)
>> +        return;
>> +
>> +    cmd = virCommandNew(swtpm_setup);
>> +    if (!cmd)
>> +        return;
>> +
>> +    virCommandAddArg(cmd, "--help");
>> +    virCommandSetOutputBuffer(cmd, &help);
>> +
>> +    if (virCommandRun(cmd, NULL) < 0)
>> +        goto cleanup;
>> +
>> +    if (strstr(help, "--tpm2"))
>> +        swtpm_supports_tpm2 = true;
>
> This bool is never read.

Maybe while doing some of the recent changes the reading of the variable 
got lost.

>
> Given that version 2 has to be requested in the XML and we don't try to
> use it automatically, I'd suggest just dropping this function. We don't
> need to parse another tool's --help output to make up for the removal
> of parsing --help of qemu and qemu-img.

swtpm doesn't have all the bells and whistles of QEMU that we would have 
a JSON interface to query the features from. So if a bad command line 
parameter is passed to swtpm, it will dump the help screen. Here's the 
output I get from trying to run a VM with an attached TPM 2 but there's 
no TPM 2 support compiled into swtpm (basically because it was created 
from the master branch not from the preview branch):

# virsh start testvm-tpm2
Error: Failed to start domain testvm-tpm2
error: internal error: Could not start 'swtpm'. exitstatus: 1, error: 
socket: unrecognized option '--tpm2'
Usage: /usr/bin/swtpm socket [options]

The following options are supported:

-p|--port <port> : use the given port
-f|--fd <fd>     : use the given socket file descriptor
[...]


I think a more controlled error message would be better basically 
stating 'Local swtpm installation does not support TPM 2'.

    Stefan

>
> Jano