Re: [PATCH 2/3] backends: Initial support for SPDM socket support

[Jonathan Cameron]

On Thu, 21 Sep 2023 16:28:00 +1000
Alistair Francis <alistair23@gmail.com> wrote:

> On Mon, Sep 18, 2023 at 8:28 PM Jonathan Cameron
> <Jonathan.Cameron@huawei.com> wrote:
> >
> > On Mon, 18 Sep 2023 13:16:01 +1000
> > Alistair Francis <alistair23@gmail.com> wrote:
> >  
> > > On Sat, Sep 16, 2023 at 1:19 AM Jonathan Cameron
> > > <Jonathan.Cameron@huawei.com> wrote:  
> > > >
> > > > On Fri, 15 Sep 2023 21:27:22 +1000
> > > > Alistair Francis <alistair23@gmail.com> wrote:
> > > >  
> > > > > From: Huai-Cheng Kuo <hchkuo@avery-design.com.tw>  
> > > >
> > > > Great to see you taking this forwards!
> > > >
> > > >  
> > > > >
> > > > > SPDM enables authentication, attestation and key exchange to assist in
> > > > > providing infrastructure security enablement. It's a standard 
> > > > > published
> > > > > by the DMTF [1].
> > > > >
> > > > > SPDM currently supports PCIe DOE and MCTP transports, but it can be
> > > > > extended to support others in the future. This patch adds
> > > > > support to QEMU to connect to an external SPDM instance.  
> > > >
> > > > It supports way more that that these days.  I'd just say 'multiple'
> > > > transports.
> > > >  
> > > > >
> > > > > SPDM support can be added to any QEMU device by exposing a
> > > > > TCP socket to a SPDM server. The server can then implement the SPDM
> > > > > decoding/encoding support, generally using libspdm [2].
> > > > >
> > > > > This is similar to how the current TPM implementation works and means
> > > > > that the heavy lifting of setting up certificate chains, capabilities,
> > > > > measurements and complex crypto can be done outside QEMU by a well
> > > > > supported and tested library.  
> > > >
> > > > Is this sufficient for usecases beyond initial attestation flows?  
> > >
> > > I believe so.
> > >
> > > The SPDM responder would be in charge of doing all of this. For the
> > > rest of the discussion the responder is the software on the other end
> > > of the QEMU socket.
> > >  
> > > > How does measurement work for example?  We need settings from the  
> > >
> > > In a basic case the responder can generate measurement data. For
> > > example the responder can return digests of the firmware. Now there
> > > won't actually be "firmware", but the responder can still return
> > > measurement data.
> > >
> > > if you are trying to test an existing product, you could fake it and
> > > return the same values as a real device. Otherwise you could return
> > > example data.
> > >  
> > > > emulated device to squirt into the SPDM agent so that it can be
> > > > encrypted and signed etc.
> > > >
> > > > Measurement reports often need to include the status of various config
> > > > space registers + any device specific additional stuff - not sure
> > > > what is defined for NVME but I suspect the list will grow, particularly
> > > > when tdisp is included.  There are some things called out in the PCIe
> > > > state as must haves, like any debug features must be reported.  
> > >
> > > So this is probably the hard part. How can the responder measurements
> > > change based on configuration values set by the host.
> > >
> > > Just for completeness, the idea would be the host would set some state
> > > in the NVMe controller for example. Then we would expect the
> > > measurement values to change.
> > >
> > > That's trickier, but we could extend the socket to communicate state
> > > like that. So when a measurement state changes in the QEMU model, we
> > > relay that to the responder. Which then changes the measurements
> > >  
> > > > Also we need a way to mess with firmware revisions reported
> > > > as those are likely to be checked.  
> > >
> > > That seems like something you can do via command line arguments or
> > > configuration settings to the responder. This is separate to QEMU  
> > I'm not convinced it is because QEMU is emulating the firmware behavior
> > and that may well change with version.  Still it's just more metadata
> > to push out from QEMU to the SPDM instance.
> >
> > My gut feeling is you'd just add an interface for the whole measurement
> > record coming from QEMU.  No need to be clever with sending only small
> > subsets of info and building the measurement.  
> 
> That also works. libspdm delegates the measurement work anyway, so
> it's easy to handle either in the external socket wrapper or in QEMU.
> 
> >  
> > >  
> > > >
> > > > I'm not sure that model will work with the spdm-emu approach.  
> > >
> > > I don't think there is anything specifically in the socket approach
> > > that limits this from working. It comes down to passing information
> > > from the QEMU emulated device to the SPDM responder. That needs to be
> > > done either way. It probably is simpler to do if libspdm is included
> > > as part of QEMU, but that brings along other complexities.  
> >
> > Agreed that we can do this with an external agent.  So do you think
> > we can persuade dmtf tools lot to allow interfaces for this purpose
> > or are we looking at a fork of the spdm-emu examples?  
> 
> I think that depends on them. We can either convince them to add
> support, which doesn't seem impossible. Or we can just fork it.
> 
> It's worth pointing out that other tools besides spdm-emu can be used
> here. It's a pretty simple transport protocol.

True enough - though I hope we don't end up replicating the complexity of
libspdm.  Changing the the actual app is less of an issue.

> 
> >  
> > >
> > > As you pointed out in my original RFC, the complexity of configuration
> > > a responder via the QEMU command line will be very difficult. I think
> > > it's simpler to keep the responder outside and QEMU and just pass any
> > > relevant data to the responder as required.  
> >
> > I'm not sure how bad the interface would actually be.
> > My expectation is that we aren't going to need to emulate the
> > full flexibility of SPDM - for example we can keep to only the
> > required protocols etc.  As such, what do we need to pass in
> > beyond the cert chains?  We might provide options to do the more fun
> > stuff, but mostly defaults shoudl work.  
> 
> There are a range of configurations:
>  - The supported capabilities
>  - The asym algorithms
>  - The hash algorithms
>  - DHE, AED cipher suits
>  - Measurement specs
>  - Certificate chains
> 
> We could go down the route of saying "this is what the device
> supports" and not letting users customise it. That is more like
> modelling a real world device. That's handy for users attaching a
> device, but not as useful for testing guest software (like the LInux
> implementation).

There is a different between providing interfaces to play with all the
details, and expecting most users to care about them.  Defaults would
cover anyone not trying to shake out bugs in the transfers etc.

You are allowed to do a lot of things in SPDM, but CMA limits things
you must do support to far less I suspect for a while at least that's
all software stacks will bother with.

> 
> If we keep this external (as in this series) we can allow very
> customised implementations to be used, which will help test guest
> software. That will allow software like EDK2 and Linux to be
> thoroughly tested and even allow fuzzing or automated unit tests.
> 
> That doesn't preclude us from integrating libspdm directly in the
> future though, if we want to just hard code the setup for specific
> devices though. Which might end up happening when we start seeing real
> world hardware that we want to emulate that includes SPDM support.

I'm nervous that we will build a very complex set of interactions but
sure, if we allow for a path to link libSPDM in for the future
I'm fine with an alternative of an external agent.

I'm seeing more and more uses of SPDM surfacing and some of them
are going to involve a lot of complexity in that agent that is tightly
coupled to the hardware in QEMU. However we definitely shouldn't
let the quest for a 'perfect' solution get in the way of a something
that actually works.  So I'm fine with this solution for now.

For giggles I'll hook this up to an MCTP over I2C device as well when
I get time as looks like people also care about that path for key
programming etc.

Jonathan

> 
> Alistair
> 
> >  
> > >  
> > > >
> > > > Anyhow, I think we need to have gotten a little further figuring that
> > > > out before we merge a solution.  I've been carrying this on the CXL
> > > > staging tree for a long time because I couldn't figure out a good 
> > > > solution
> > > > to the amount of information that needs to go between them.
> > > >
> > > > For those not familiar with the fun of libSPDM it is a pain to work with
> > > > which is why Huai-Cheng instead connected with the demo app.
> > > >
> > > > Any more luck getting a reliable build to work?  
> > >
> > > Yes!
> > >
> > > libspdm is now packaged in buildroot:
> > > https://github.com/buildroot/buildroot/blob/master/package/libspdm/libspdm.mk
> > >
> > > You can now build libspdm with openSSL provided by your distro as you
> > > don't need to access any private openSSL APIs any more.  
> >
> > Great.
> >  
> > >
> > > The hope is we can continue to march towards including libspdm as a
> > > standard library in distros, which should make the entire process
> > > easier.  
> >
> > That is indeed good either way.
> >
> > Jonathan
> >  
> > >  
> > > >  
> > > > >
> > > > > 1: https://www.dmtf.org/standards/SPDM
> > > > > 2: https://github.com/DMTF/libspdm
> > > > >
> > > > > Signed-off-by: Huai-Cheng Kuo <hchkuo@avery-design.com.tw>
> > > > > Signed-off-by: Chris Browy <cbrowy@avery-design.com>
> > > > > Co-developed-by: Jonathan Cameron <Jonathan.cameron@huawei.com>
> > > > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > > > [ Changes by AF:
> > > > >  - Convert to be more QEMU-ified
> > > > >  - Move to backends as it isn't PCIe specific
> > > > > ]
> > > > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>  
> > > > Alistair, you sent this so I think your sign off should be last
> > > > + some indication of Wilfred's involvement would be good?
> > > > Probably another Co-developed-by
> > > >
> > > >
> > > >  
> > > > > Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
> > > > > ---  
> > > >
> > > > I've looked at this code too much in the past to give much
> > > > real review.  Still a few comments inline.
> > > > I'm very keen to get a solution to this upstream, though I think
> > > > we do need to discuss a few general points (no cover letter so I'll
> > > > do it here).  
> > >
> > > Yeah, I should have included a cover letter. V2 will
> > >
> > > Alistair
> > >  
> > > >
> > > >
> > > > ...
> > > >  
> > > > > diff --git a/backends/spdm-socket.c b/backends/spdm-socket.c
> > > > > new file mode 100644
> > > > > index 0000000000..2f31ba80ba
> > > > > --- /dev/null
> > > > > +++ b/backends/spdm-socket.c
> > > > > @@ -0,0 +1,215 @@  
> > > >
> > > >  
> > > > > +
> > > > > +int spdm_socket_connect(uint16_t port, Error **errp)
> > > > > +{
> > > > > +    int client_socket;
> > > > > +    struct sockaddr_in server_addr;
> > > > > +
> > > > > +    client_socket = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
> > > > > +    if (client_socket < 0) {
> > > > > +        error_setg(errp, "cannot create socket: %s", 
> > > > > strerror(errno));
> > > > > +        return -1;
> > > > > +    }
> > > > > +
> > > > > +    memset((char *)&server_addr, 0, sizeof(server_addr));
> > > > > +    server_addr.sin_family = AF_INET;
> > > > > +    server_addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
> > > > > +    server_addr.sin_port = htons(port);
> > > > > +
> > > > > +
> > > > > +    if (connect(client_socket, (struct sockaddr *)&server_addr, 
> > > > > sizeof(server_addr)) < 0) {  
> > > > Wrap the line.
> > > >  
> > > > > +        error_setg(errp, "cannot connect: %s", strerror(errno));
> > > > > +        close(client_socket);
> > > > > +        return -1;
> > > > > +    }
> > > > > +
> > > > > +    return client_socket;
> > > > > +}  
> > > >
> > > >  
> > >  
> >  
> 
>