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

[Alistair Francis]

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

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.

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

The hope is we can continue to march towards including libspdm as a
standard library in distros, which should make the entire process
easier.

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