Re: [RFC v3 24/27] lib: rspdm: Support SPDM challenge

[Jonathan Cameron]

On Wed, 11 Feb 2026 13:29:31 +1000
alistair23@xxxxxxxxx wrote:

> From: Alistair Francis <alistair@xxxxxxxxxxxxx>
> 
> Support the CHALLENGE SPDM command.
> 
> Signed-off-by: Alistair Francis <alistair@xxxxxxxxxxxxx>

Nicely broken out.  I was wondering when the transcript for the
hash might show up, but you sensibly kept that delight for only
being done when you need it.  Might be worth talking a bit more
about that in the patch description!

Minor comments inline.

J

> diff --git a/lib/rspdm/state.rs b/lib/rspdm/state.rs
> index 728b920beace..a4d803af48fe 100644
> --- a/lib/rspdm/state.rs
> +++ b/lib/rspdm/state.rs

...

> @@ -834,4 +877,165 @@ pub(crate) fn validate_cert_chain(&mut self, slot: u8) -> Result<(), Error> {
>  
>          Ok(())
>      }
> +
> +    pub(crate) fn challenge_rsp_len(&mut self, nonce_len: usize, opaque_len: usize) -> usize {
> +        let mut length =
> +            core::mem::size_of::<SpdmHeader>() + self.hash_len + nonce_len + opaque_len + 2;

As below, perhaps add a comment at least that MSHLength == 0

> +
> +        if self.version >= 0x13 {
> +            length += 8;
> +        }
> +
> +        length + self.sig_len
> +    }
> +
> +    fn verify_signature(&mut self, response_vec: &mut [u8]) -> Result<(), Error> {
> +        let sig_start = response_vec.len() - self.sig_len;
> +        let mut sig = bindings::public_key_signature::default();
> +        let mut mhash: KVec<u8> = KVec::new();
> +
> +        sig.s = &mut response_vec[sig_start..] as *mut _ as *mut u8;
> +        sig.s_size = self.sig_len as u32;

Perhaps it makes sense to extract the signature at the caller and only pass
that in here rather than the whole response_vec.

> +        sig.encoding = self.base_asym_enc.as_ptr() as *const u8;
> +        sig.hash_algo = self.base_hash_alg_name.as_ptr() as *const u8;
> +
...

> +    pub(crate) fn challenge(&mut self, slot: u8, verify: bool) -> Result<(), Error> {
> +        let mut request = ChallengeReq::default();
> +        request.version = self.version;
> +        request.param1 = slot;
> +
> +        let nonce_len = request.nonce.len();
> +
> +        if let Some(nonce) = &self.next_nonce {
> +            request.nonce.copy_from_slice(&nonce);
> +            self.next_nonce = None;
> +        } else {
> +            unsafe {
> +                bindings::get_random_bytes(&mut request.nonce as *mut _ as *mut c_void, nonce_len)
> +            };
> +        }
> +
> +        let req_sz = if self.version <= 0x12 {
> +            core::mem::size_of::<ChallengeReq>() - 8

No means to do offset_of type stuff in Rust?  Would make the sizing explicitly reflect the
structure.

> +        } else {
> +            core::mem::size_of::<ChallengeReq>()
> +        };
> +
> +        let rsp_sz = self.challenge_rsp_len(nonce_len, SPDM_MAX_OPAQUE_DATA);
> +
> +        // SAFETY: `request` is repr(C) and packed, so we can convert it to a slice
> +        let request_buf = unsafe { from_raw_parts_mut(&mut request as *mut _ as *mut u8, req_sz) };
> +
> +        let mut response_vec: KVec<u8> = KVec::with_capacity(rsp_sz, GFP_KERNEL)?;
> +        // SAFETY: `request` is repr(C) and packed, so we can convert it to a slice
> +        let response_buf = unsafe { from_raw_parts_mut(response_vec.as_mut_ptr(), rsp_sz) };
> +
> +        let rc = self.spdm_exchange(request_buf, response_buf)?;
> +
> +        if rc < (core::mem::size_of::<ChallengeRsp>() as i32) {
> +            pr_err!("Truncated challenge response\n");
> +            to_result(-(bindings::EIO as i32))?;
> +        }
> +
> +        // SAFETY: `rc` is the length of data read, which will be smaller
> +        // then the capacity of the vector
> +        unsafe { response_vec.inc_len(rc as usize) };
> +
> +        let _response: &mut ChallengeRsp = Untrusted::new_mut(&mut response_vec).validate_mut()?;
> +
> +        let opaque_len_offset = core::mem::size_of::<SpdmHeader>() + self.hash_len + nonce_len;

Might be worth adding something to reflect that this also includes the MSHLength but that is 0 as
we didn't ask for a measurement summary hash.  Would make it a tiny bit easier to correlate
with the spec.

> +        let opaque_len = u16::from_le_bytes(
> +            response_vec[opaque_len_offset..(opaque_len_offset + 2)]
> +                .try_into()
> +                .unwrap_or([0, 0]),
> +        );
> +
> +        let rsp_sz = self.challenge_rsp_len(nonce_len, opaque_len as usize);
> +
> +        if rc < rsp_sz as i32 {
> +            pr_err!("Truncated challenge response\n");
> +            to_result(-(bindings::EIO as i32))?;
> +        }
> +
> +        self.transcript
> +            .extend_from_slice(&response_vec[..rsp_sz], GFP_KERNEL)?;
> +
> +        if verify {
> +            /* Verify signature at end of transcript against leaf key */
> +            match self.verify_signature(&mut response_vec[..rsp_sz]) {
> +                Ok(()) => {
> +                    pr_info!("Authenticated with certificate slot {slot}");
> +                    self.authenticated = true;
> +                }
> +                Err(e) => {
> +                    pr_err!("Cannot verify challenge_auth signature: {e:?}");
> +                    self.authenticated = false;
> +                }
> +            };
> +        }
> +
> +        Ok(())
> +    }
>  }
> diff --git a/lib/rspdm/validator.rs b/lib/rspdm/validator.rs
> index a8bc3378676f..f8a5337841f0 100644
> --- a/lib/rspdm/validator.rs
> +++ b/lib/rspdm/validator.rs

> +
> +#[repr(C, packed)]
> +pub(crate) struct ChallengeRsp {
> +    pub(crate) version: u8,
> +    pub(crate) code: u8,
> +    pub(crate) param1: u8,
> +    pub(crate) param2: u8,
> +
> +    pub(crate) cert_chain_hash: __IncompleteArrayField<u8>,
> +    pub(crate) nonce: [u8; 32],
> +    pub(crate) message_summary_hash: __IncompleteArrayField<u8>,
> +
> +    pub(crate) opaque_data_len: u16,
Similar to other places, I'd use a __le16 and convert at place of use
only.


> +    pub(crate) opaque_data: __IncompleteArrayField<u8>,
> +
> +    pub(crate) context: [u8; 8],
> +    pub(crate) signature: __IncompleteArrayField<u8>,
> +}
> +
> +impl Validate<&mut Unvalidated<KVec<u8>>> for &mut ChallengeRsp {
> +    type Err = Error;
> +
> +    fn validate(unvalidated: &mut Unvalidated<KVec<u8>>) -> Result<Self, Self::Err> {
> +        let raw = unvalidated.raw_mut();
> +        if raw.len() < mem::size_of::<ChallengeRsp>() {
> +            return Err(EINVAL);
> +        }
> +
> +        let ptr = raw.as_mut_ptr();
> +        // CAST: `ChallengeRsp` only contains integers and has `repr(C)`.
> +        let ptr = ptr.cast::<ChallengeRsp>();
> +        // SAFETY: `ptr` came from a reference and the cast above is valid.
> +        let rsp: &mut ChallengeRsp = unsafe { &mut *ptr };
> +
> +        // rsp.opaque_data_len = rsp.opaque_data_len.to_le();
Not sure why this is commented out. But as above, I'd leave it alone anyway.

> +
> +        Ok(rsp)
> +    }
> +}