Re: [PATCH v2 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:
- [Low] `validate_cert_chain` incorrectly accepts empty certificate chains as valid, silently bypassing all certificate parsing, signature verification, and the caller-provided validation callback.
--

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

lib: rspdm: Support SPDM certificate validation

This patch adds basic validation for SPDM certificate chains, checking that
the chain is structurally correct, signatures are supported, and certificates
aren't blacklisted. It also extracts and stores the first leaf certificate
for later use.

> diff --git a/lib/rspdm/state.rs b/lib/rspdm/state.rs
> --- a/lib/rspdm/state.rs
> +++ b/lib/rspdm/state.rs
[ ... ]
> +    pub(crate) fn validate_cert_chain(&mut self, slot: u8) -> Result<(), Error> {
> +        let cert_chain_buf = &self.certs[slot as usize];
> +        let cert_chain_len = cert_chain_buf.len();
> +        // We skip over the RootHash
> +        let header_len = 4 + self.hash_len;
> +
> +        let mut offset = header_len;
> +        let mut prev_cert: Option<*mut bindings::x509_certificate> = None;
> +
> +        while offset < cert_chain_len {

[Severity: Low]
Does this incorrectly accept empty certificate chains as valid?

If a responder provides an empty certificate chain where cert_chain_len is
less than or equal to header_len, this while loop is skipped entirely. The
function will fall through and implicitly return Ok(()) at the end, silently
bypassing all certificate parsing and caller-provided validation callbacks.

While the subsequent CHALLENGE phase safely catches the missing leaf key and
fails gracefully, should the structural validation phase explicitly reject an
empty chain with an error rather than silently accepting it?

> +            // SAFETY: `cert_chain_buf[offset..]` is a non-empty slice of
> +            // bytes valid for at least `cert_chain_len` bytes.
[ ... ]

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