On Fri, 15 Sep 2023 21:27:22 +1000 Alistair Francis <alistai...@gmail.com> wrote:
> From: Huai-Cheng Kuo <hch...@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? How does measurement work for example? We need settings from the 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. Also we need a way to mess with firmware revisions reported as those are likely to be checked. I'm not sure that model will work with the spdm-emu approach. 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? > > 1: https://www.dmtf.org/standards/SPDM > 2: https://github.com/DMTF/libspdm > > Signed-off-by: Huai-Cheng Kuo <hch...@avery-design.com.tw> > Signed-off-by: Chris Browy <cbr...@avery-design.com> > Co-developed-by: Jonathan Cameron <jonathan.came...@huawei.com> > Signed-off-by: Jonathan Cameron <jonathan.came...@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.fran...@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.mall...@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). ... > 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; > +}