Re: [PATCH v4 00/22] lib: Rust implementation of SPDM

From: Alistair Francis

Date: Thu Oct 22 2026 - 00:34:46 EST


On Thu, Oct 15, 2026 at 6:02 PM Alexandre Courbot <acourbot@xxxxxxxxxx> wrote:
>
> On Thu Oct 15, 2026 at 2:23 PM JST, Alistair Francis wrote:
> > On Thu, Oct 15, 2026 at 6:54 AM Alice Ryhl <aliceryhl@xxxxxxxxxx> wrote:
> >>
> >> On Mon, Sep 28, 2026 at 11:11:01AM +1000, alistair23@xxxxxxxxx wrote:
> >> > From: Alistair Francis <alistair.francis@xxxxxxx>
> >> >
> >> > Security Protocols and Data Models (SPDM) [1] is used for authentication,
> >> > attestation and key exchange. SPDM is generally used over a range of
> >> > transports, such as PCIe, MCTP/SMBus/I3C, ATA, SCSI, NVMe or TCP.
> >> >
> >> > From the kernels perspective SPDM is used to authenticate and attest devices.
> >> > In this threat model a device is considered untrusted until it can be verified
> >> > by the kernel and userspace using SPDM. As such SPDM data is untrusted data
> >> > that can be mallicious.
> >> >
> >> > The SPDM specification is also complex, with the 1.2.1 spec being almost 200
> >> > pages and the 1.3.0 spec being almost 250 pages long.
> >> >
> >> > As such we have the kernel parsing untrusted responses from a complex
> >> > specification, which sounds like a possible exploit vector. This is the type
> >> > of place where Rust excels!
> >> >
> >> > This series implements a SPDM requester in Rust.
> >>
> >> So I looked a bit at this, and here are some comments. I will not repeat
> >> what Alexandre has already said.
> >>
> >> First, I'd like to say that it's good to see that all wire-format
> >> parsing in lib/rspdm/validator.rs is written in completely safe Rust.
> >> That's exactly what we want to see for parsing in Rust. We want the code
> >> to have basically no unsafe code whatsoever so that we really get to
> >> rely on the bullet-proof guarantees about bugs being caught at compile
> >> time, or at least at runtime.
> >>
> >> Unfortunately it looks like the same cannot be said for the state
> >> machine in lib/rspdm/state.rs, which has way too much unsafe code in its
> >> interaction with the rest of the kernel. There needs to be more
> >> intermediate Rust abstractions here. This file should not have *any*
> >> unsafe code, except maybe 1-2 blocks for using the C transport and
> >> validate function pointers.
> >
> > Yeah, it's the crypto functions. There are currently no bindings, so
> > it's just directly calling the C functions.
> >
> > Without the crypto functions it's just a single unsafe call to
> > `transport_function()` in `state.rs` and a few in `lib.rs` to handl
> > the C <-> FFI boundy.
> >
> > I was trying to avoid adding crypto and X.509 bindings in this series,
> > as that's a few more subsystems this ends up touching. We need to add
> > PQC as well, so crypto bindings are next on the todo list once this is
> > merged.
> >
> > Is this going to stall on adding crypto bindings?
>
> So without going all the way to fully-fledged crypto bindings, you will
> spare yourself and your reviewers quite a bit of pain if you at least
> implement a safe and lightweight crypto wrapper that is local to SPDM
> and could eventually grow into official crypto bindings as a future
> effort. As it is the code is very hard to review: for instance
> `crypto_free_shash` is always called after `free_desc`, and looks like
> it could/should be part of it, but that's hard to assess as the crypto
> code is mixed with the rest and called here and there. I just cannot
> tell whether all these unsafe blocks are safe of not, which is a problem
> since the goal of writing this in Rust is to prevent exploits.
>
> A self-contained crypto module exposing a safe interface would remove
> that burden, reduce the number of `unsafe` blocks, and make the series
> easier to review globally while not necessarily resulting in more code.
> You just need to store the relevant pointers into a new data structure,
> enforce the required invariants at construction time, and codify the
> actions that SPDM is already doing in the form of methods.

Oh! That is a great idea. I have added that locally and it's pretty
nice. I'll tidy it up a bit more and then send a v5

Alistair