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

[James Bottomley]

On Fri, 2023-09-22 at 08:00 +0200, Markus Armbruster wrote:
> 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

Sure, that's easy.

> > +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

Yes, I've verified that the dot notation works as well.  However, I
thought QEMU was calling all stuff like this JSON notation?  If not,
what do you usually call it? "json or dot notation"?

> 
> 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':'remot
> > e','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.

OK

> 
> > +#
> > +# Information for the mssim emulator connection
> > +#
> > +# @command: command socket for the TPM emulator
> 
> Blank line, please.

OK

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

Heh, yes, that keeps creeping with every release ..

> 
> > +##
> > +{ '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>

James