Re: [PATCH v3 19/21] lib: rspdm: Support SPDM certificate validation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The SPDM implementation incorrectly assumes all certificate slots share the same leaf key, violating the SPDM specification and restricting support for multi-slot responders.
--

commit b0152a815fb6a558a0d32bf4c0f287cb4d4ee1a3
Author: Alistair Francis <alistair@alistair23.me>

lib: rspdm: Support SPDM certificate validation

Support validating the SPDM certificate chain. This performs basic
sanity checks on the chain, ensures supported signatures are used,
checks against the blacklist, and stores the first leaf certificate.

> diff --git a/lib/rspdm/state.rs b/lib/rspdm/state.rs
> index 1e8a4402e6342..fc7df9dd7b973 100644
> --- a/lib/rspdm/state.rs
> +++ b/lib/rspdm/state.rs

[ ... ]

> @@ -846,4 +858,136 @@ pub(crate) fn get_certificate(&mut self, slot: u8) -> Result<(), Error> {

[ ... ]

> +        if let Some(prev) = prev_cert {
> +            if let Some(validate) = self.validate {
> +                // SAFETY: Call the `validate` function provided.
> +                let rc = unsafe { validate(self.dev, slot, prev) };
> +                if let Err(e) = to_result(rc) {
> +                    // SAFETY: `prev_cert` is the previously parsed
> +                    // certificate from a prior loop iteration.
> +                    unsafe { bindings::x509_free_certificate(prev) };
> +                    return Err(e);
> +                }
> +            }
> +
> +            // The leaf key is the same for all slots, so just store the first one.
> +            if self.leaf_key.is_none() {
> +                // SAFETY: `prev_cert` is the previously parsed
> +                // certificate from a prior loop iteration.
> +                self.leaf_key = unsafe { Some((*prev).pub_) };

[Severity: Medium]
Is it safe to assume all certificate slots share the same leaf key here?

According to the SPDM specification (DSP0274), devices are allowed up to 8
slots, and they can be provisioned with distinct certificate chains and
cryptographic keys (e.g., one RSA chain and one ECDSA chain).

When an SPDM responder with multiple provisioned slots connects, this logic
skips storing the leaf keys for all slots after the first one. If the responder
negotiates an algorithm that directs a challenge to a slot other than the
first one, won't authentication use the incorrect leaf key from the first slot
and cause signature verification to fail?

> +                // SAFETY: `prev_cert` is the previously parsed
> +                // certificate from a prior loop iteration. We are setting
> +                // the `pub` key to null so it isn't freed below
> +                unsafe { (*prev).pub_ = core::ptr::null_mut() };
> +            }
> +
> +            // SAFETY: `prev_cert` is the previously parsed
> +            // certificate from a prior loop iteration.
> +            unsafe { bindings::x509_free_certificate(prev) };
> +        }
> +
> +        Ok(())
> +    }
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901010347.2614656-1-alistair.francis@wdc.com?part=19