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

[Jonathan Cameron]

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.

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

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

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