Re: [PATCH v4 22/22] lib: rspdm: Support SPDM challenge

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] rspdm: missing CertChainHash verification in CHALLENGE_AUTH response
--

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

lib: rspdm: Support SPDM challenge

This commit adds support for the CHALLENGE SPDM command. It introduces the
state handling and response validation for SPDM challenges to authenticate
the responder.

> diff --git a/lib/rspdm/state.rs b/lib/rspdm/state.rs
> index 9f7f6d0ffac65..d6ece65a6fc05 100644
> --- a/lib/rspdm/state.rs
> +++ b/lib/rspdm/state.rs

[ ... ]

> @@ -907,4 +957,165 @@ pub(crate) fn validate_cert_chain(&mut self, slot: u8) -> Result<(), Error> {

[ ... ]

> +    pub(crate) fn challenge(&mut self, slot: u8) -> Result<(), Error> {

[ ... ]

> +        let mut response: ChallengeRsp =
> +            Untrusted::new(response_vec.as_slice()).validate(&*self)?;
> +
> +        if request.context != response.context {
> +            pr_err!("Mismatched challenge context response\n");
> +            return Err(EIO);
> +        }
> +
> +        self.transcript
> +            .extend_from_slice(response_vec.get(..response.len()).ok_or(EIO)?, GFP_KERNEL)?;
> +
> +        match self.verify_signature(&mut response.signature) {

[Severity: Medium]
Does this code skip the verification of the certificate chain hash?

The SPDM specification (e.g., DSP0274 Section 11.2) explicitly mandates that
the requester must verify that the response cert_chain_hash matches the hash
of the certificate chain that the requester possesses for the specified slot.

Since response.cert_chain_hash is ignored here instead of being verified
against the expected hash of self.certs[slot], could this allow protocol
confusion or downgrade attacks if a responder uses a different certificate
chain for the signature than the one the requester holds?

> +            Ok(()) => {
> +                pr_info!("Authenticated with certificate slot {slot}\n");
> +                Ok(())
> +            }
> +            Err(e) => {
> +                pr_err!("Cannot verify challenge_auth signature: {e:?}\n");
> +                Err(EPROTO)
> +            }
> +        }
> +    }
>  }

> diff --git a/lib/rspdm/validator.rs b/lib/rspdm/validator.rs
> index f4e9485179d01..9992679f7c8a3 100644
> --- a/lib/rspdm/validator.rs
> +++ b/lib/rspdm/validator.rs

[ ... ]

> @@ -946,3 +943,149 @@ fn validate(unvalidated: &[u8], context: &'c SpdmState<'c>) -> Result<Self, Self

[ ... ]

> +impl<'a, 'c> Validate<'c, Untrusted<&'a [u8]>> for ChallengeRsp {

[ ... ]

> +        let mut cert_chain_hash = KVec::new();
> +        cert_chain_hash.extend_from_slice(
> +            unvalidated
> +                .get(mem::size_of::<SpdmHeader>()..nonce_offset)
> +                .ok_or(EIO)?,
> +            GFP_KERNEL,
> +        )?;

This isn't a bug in the extraction itself, but the cert_chain_hash is parsed
from the response here, confirming it is available to the requester, yet it
appears to be ultimately left unused in SpdmState::challenge().

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