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

[Daniel P . Berrangé]

On Mon, Jan 06, 2025 at 06:11:09PM -0800, James Bottomley wrote:
> On Thu, 2024-12-19 at 17:39 +0000, Daniel P. Berrangé wrote:
> > On Thu, Dec 12, 2024 at 12:05:28PM -0500, James Bottomley wrote:
> > > 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
> > > <James.Bottomley@HansenPartnership.com>
> > > Acked-by: Markus Armbruster <armbru@redhat.com>
> > > 
> > > ---
> > > 
> > > v2: convert to SocketAddr json and use
> > > qio_channel_socket_connect_sync()
> > > v3: gate control power off by migration state keep control socket
> > > disconnected
> > >     to test outside influence and add docs.
> > > v7: TPMmssim -> TPMMssim; doc and json fixes
> > >     Make command socket open each time (makes OS debugging easier)
> > > v11: add startup method to make sure TPM is reset on reboot
> > > ---
> > >  MAINTAINERS              |   6 +
> > >  backends/tpm/Kconfig     |   5 +
> > >  backends/tpm/meson.build |   1 +
> > >  backends/tpm/tpm_mssim.c | 335
> > > +++++++++++++++++++++++++++++++++++++++
> > >  backends/tpm/tpm_mssim.h |  44 +++++
> > >  docs/specs/tpm.rst       |  39 +++++
> > >  qapi/tpm.json            |  31 +++-
> > >  system/tpm-hmp-cmds.c    |   9 ++
> > >  8 files changed, 466 insertions(+), 4 deletions(-)
> > >  create mode 100644 backends/tpm/tpm_mssim.c
> > >  create mode 100644 backends/tpm/tpm_mssim.h
> > > 
> > 
> > 
> > > diff --git a/backends/tpm/tpm_mssim.c b/backends/tpm/tpm_mssim.c
> > > new file mode 100644
> > > index 0000000000..8f105fc924
> > > --- /dev/null
> > > +++ b/backends/tpm/tpm_mssim.c
> > > @@ -0,0 +1,335 @@
> > > +/*
> > > + * Emulator TPM driver which connects over the mssim protocol
> > > + * SPDX-License-Identifier: GPL-2.0-or-later
> > > + *
> > > + * Copyright (c) 2022
> > 
> > Copyright by whom ? I presume yourself, but I wouldn't normally
> > assume the 'Author' line applies to the Copyright line.
> 
> I'll fix up this one (and all the others below) but:
> 
> [...]
> > > diff --git a/backends/tpm/tpm_mssim.h b/backends/tpm/tpm_mssim.h
> > > new file mode 100644
> > > index 0000000000..397474e4f6
> > > --- /dev/null
> > > +++ b/backends/tpm/tpm_mssim.h
> > > @@ -0,0 +1,44 @@
> > > +/*
> > > + * SPDX-License-Identifier: BSD-2-Clause
> > > + *
> > > + * The code below is copied from the Microsoft/TCG Reference
> > > implementation
> > > + *
> > > + *  https://github.com/Microsoft/ms-tpm-20-ref.git
> > > + *
> > > + * In file TPMCmd/Simulator/include/TpmTcpProtocol.h
> > > + */
> > 
> > That file has a volumous copyright header that I would expect to be
> > preserved here.
> 
> Actually, the file itself has no copyright header at all (seems to be
> standard practice for all header files in the repository). Did you want
> me to paste the copyright lines from the LICENSE file; i.e.

Oh, my local checkout was out of date. A commit in Oct last year purged
all the copyright headers that the files previously had:

  
https://github.com/microsoft/ms-tpm-20-ref/blob/482aaafc0523bb94de203c4975f72f0230becec4/TPMCmd/Simulator/include/TpmTcpProtocol.h

> Copyright 2010-2022 Microsoft Corporation
> Copyright 2022-2024 Trusted Computing Group and its contributors

Yes, I think adding those, would be sufficient.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|