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

[James Bottomley]

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.

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

?

James