Re: [PATCH v6 2/2] tpm: add backend for mssim

[Markus Armbruster]

Found this cleaning out old mail, sorry for missing it until now!

I think we owe James a quick decision wether we're willing to take the
feature.  Stefan, thoughts?

James Bottomley <jejb@linux.ibm.com> writes:

> From: James Bottomley <James.Bottomley@HansenPartnership.com>
>
> The Microsoft Simulator (mssim) is the reference emulation platform
> for the TCG TPM 2.0 specification.
>
> https://github.com/Microsoft/ms-tpm-20-ref.git
>
> It exports a fairly simple network socket based protocol on two
> sockets, one for command (default 2321) and one for control (default
> 2322).  This patch adds a simple backend that can speak the mssim
> protocol over the network.  It also allows the two sockets to be
> specified on the command line.  The benefits are twofold: firstly it
> gives us a backend that actually speaks a standard TPM emulation
> protocol instead of the linux specific TPM driver format of the
> current emulated TPM backend and secondly, using the microsoft
> protocol, the end point of the emulator can be anywhere on the
> network, facilitating the cloud use case where a central TPM service
> can be used over a control network.
>
> The implementation does basic control commands like power off/on, but
> doesn't implement cancellation or startup.  The former because
> cancellation is pretty much useless on a fast operating TPM emulator
> and the latter because this emulator is designed to be used with OVMF
> which itself does TPM startup and I wanted to validate that.
>
> To run this, simply download an emulator based on the MS specification
> (package ibmswtpm2 on openSUSE) and run it, then add these two lines
> to the qemu command and it will use the emulator.
>
>     -tpmdev mssim,id=tpm0 \
>     -device tpm-crb,tpmdev=tpm0 \
>
> to use a remote emulator replace the first line with
>
>     -tpmdev 
> "{'type':'mssim','id':'tpm0','command':{'type':inet,'host':'remote','port':'2321'}}"
>
> tpm-tis also works as the backend.
>
> Signed-off-by: James Bottomley <jejb@linux.ibm.com>

[...]

> diff --git a/docs/specs/tpm.rst b/docs/specs/tpm.rst
> index 535912a92b..1398735956 100644
> --- a/docs/specs/tpm.rst
> +++ b/docs/specs/tpm.rst
> @@ -270,6 +270,38 @@ available as a module (assuming a TPM 2 is passed 
> through):
>    /sys/devices/LNXSYSTEM:00/LNXSYBUS:00/MSFT0101:00/tpm/tpm0/pcr-sha256/9
>    ...
>  
> +The QEMU TPM Microsoft Simulator Device
> +---------------------------------------
> +
> +The TCG provides a reference implementation for TPM 2.0 written by


Suggest to copy the cover letter's nice introductory paragraph here:

  The Microsoft Simulator (mssim) is the reference emulation platform
  for the TCG TPM 2.0 specification.

  It provides a reference implementation for TPM 2.0 written by

> +Microsoft (See `ms-tpm-20-ref`_ on github).  The reference implementation
> +starts a network server and listens for TPM commands on port 2321 and
> +TPM Platform control commands on port 2322, although these can be
> +altered.  The QEMU mssim TPM backend talks to this implementation.  By
> +default it connects to the default ports on localhost:
> +
> +.. code-block:: console
> +
> +  qemu-system-x86_64 <qemu-options> \
> +    -tpmdev mssim,id=tpm0 \
> +    -device tpm-crb,tpmdev=tpm0
> +
> +
> +Although it can also communicate with a remote host, which must be
> +specified as a SocketAddress via json on the command line for each of

Is the "via JSON" part in "must be specified ... on the command line"
correct?  I'd expect to be able to use dotted keys as well, like

    -tpmdev 
type=mssim,id=tpm0,command.type=inet,command.host=remote,command.port=2321',control.type=inet,control.host=remote,control.port=2322

Aside: I do recommend management applications stick to JSON.

> +the command and control ports:
> +
> +.. code-block:: console
> +
> +  qemu-system-x86_64 <qemu-options> \
> +    -tpmdev 
> "{'type':'mssim','id':'tpm0','command':{'type':'inet','host':'remote','port':'2321'},'control':{'type':'inet','host':'remote','port':'2322'}}"
>  \
> +    -device tpm-crb,tpmdev=tpm0
> +
> +
> +The mssim backend supports snapshotting and migration, but the state
> +of the Microsoft Simulator server must be preserved (or the server
> +kept running) outside of QEMU for restore to be successful.
> +
>  The QEMU TPM emulator device
>  ----------------------------
>  
> @@ -526,3 +558,6 @@ the following:
>  
>  .. _SWTPM protocol:
>     
> https://github.com/stefanberger/swtpm/blob/master/man/man3/swtpm_ioctls.pod
> +
> +.. _ms-tpm-20-ref:
> +   https://github.com/microsoft/ms-tpm-20-ref
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index ed78a87ddd..12482368d0 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -731,6 +731,7 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
>      unsigned int c = 0;
>      TPMPassthroughOptions *tpo;
>      TPMEmulatorOptions *teo;
> +    TPMmssimOptions *tmo;
>  
>      info_list = qmp_query_tpm(&err);
>      if (err) {
> @@ -764,6 +765,14 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
>              teo = ti->options->u.emulator.data;
>              monitor_printf(mon, ",chardev=%s", teo->chardev);
>              break;
> +        case TPM_TYPE_MSSIM:
> +            tmo = &ti->options->u.mssim;
> +            monitor_printf(mon, ",command=%s:%s,control=%s:%s",
> +                           tmo->command->u.inet.host,
> +                           tmo->command->u.inet.port,
> +                           tmo->control->u.inet.host,
> +                           tmo->control->u.inet.port);
> +            break;
>          case TPM_TYPE__MAX:
>              break;
>          }
> diff --git a/qapi/tpm.json b/qapi/tpm.json
> index 2b491c28b4..f9dde35377 100644
> --- a/qapi/tpm.json
> +++ b/qapi/tpm.json
> @@ -5,6 +5,7 @@
>  ##
>  # = TPM (trusted platform module) devices
>  ##

Blank line, please.

> +{ 'include': 'sockets.json' }
>  
>  ##
>  # @TpmModel:
> @@ -49,7 +50,7 @@
   #
   # @passthrough: TPM passthrough type
   #
   # @emulator: Software Emulator TPM type (since 2.11)
>  #

Missing member documentation:

   # @mssim: <brief description here> (since 8.2)

>  # Since: 1.5
>  ##
> -{ 'enum': 'TpmType', 'data': [ 'passthrough', 'emulator' ],
> +{ 'enum': 'TpmType', 'data': [ 'passthrough', 'emulator', 'mssim' ],
>    'if': 'CONFIG_TPM' }
>  
>  ##
> @@ -64,7 +65,7 @@
>  # Example:
>  #
>  # -> { "execute": "query-tpm-types" }
> -# <- { "return": [ "passthrough", "emulator" ] }
> +# <- { "return": [ "passthrough", "emulator", "mssim" ] }

Thanks for updating the example.

>  #
>  ##
>  { 'command': 'query-tpm-types', 'returns': ['TpmType'],
> @@ -117,6 +118,22 @@
>    'data': { 'data': 'TPMEmulatorOptions' },
>    'if': 'CONFIG_TPM' }
>  
> +##
> +# @TPMmssimOptions:

Please capitalize similar to TPMPassthroughOptions and
TPMEmulatorOptions: TPMMssimOptions.

> +#
> +# Information for the mssim emulator connection
> +#
> +# @command: command socket for the TPM emulator

Blank line, please.

> +# @control: control socket for the TPM emulator
> +#
> +# Since: 7.2.0

Since 8.2

> +##
> +{ 'struct': 'TPMmssimOptions',
> +  'data': {
> +      '*command': 'SocketAddress',
> +      '*control': 'SocketAddress' },

Locally consistent indentation is

     'data': { '*command': 'SocketAddress',
               '*control': 'SocketAddress' },

> +  'if': 'CONFIG_TPM' }
> +
>  ##
>  # @TpmTypeOptions:
>  #
> @@ -124,6 +141,7 @@
>  #
>  # @type: - 'passthrough' The configuration options for the TPM passthrough 
> type
>  #        - 'emulator' The configuration options for TPM emulator backend type
> +#        - 'mssim' The configuration options for TPM emulator mssim type
>  #
>  # Since: 1.5
>  ##
> @@ -131,7 +149,8 @@
>    'base': { 'type': 'TpmType' },
>    'discriminator': 'type',
>    'data': { 'passthrough' : 'TPMPassthroughOptionsWrapper',
> -            'emulator': 'TPMEmulatorOptionsWrapper' },
> +            'emulator': 'TPMEmulatorOptionsWrapper',
> +            'mssim' : 'TPMmssimOptions' },
>    'if': 'CONFIG_TPM' }
>  
>  ##
> @@ -150,7 +169,8 @@
>              'id' : 'str' },
>    'discriminator': 'type',
>    'data': { 'passthrough' : 'TPMPassthroughOptions',
> -            'emulator': 'TPMEmulatorOptions' },
> +            'emulator': 'TPMEmulatorOptions',
> +            'mssim': 'TPMmssimOptions' },
>    'if': 'CONFIG_TPM' }
>  
>  ##

Address my nitpicking, and you may add

Acked-by: Markus Armbruster <armbru@redhat.com>