Re: [PATCH v3 21/21] lib: rspdm: Support SPDM challenge

[Jonathan Cameron <jic23@kernel.org>]

On Tue,  1 Sep 2026 11:03:47 +1000
alistair23@gmail.com wrote:

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

Hi Alistair

I probably need to come at this with fresh eyes as I struggled to align
the generation of what was being signed with the spec.  Anyhow out of
time for today and unfortunately I'm not sure when I'll get back to this.

Jonathan

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

> @@ -990,4 +1051,180 @@ 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 {
> +        // No measurement summary hash requested (MSHLength == 0)
> +        let mut length =
> +            core::mem::size_of::<SpdmHeader>() + self.hash_len + nonce_len + opaque_len + 2;
> +
> +        if self.version >= SPDM_VER_13 {
> +            length += 8;
> +        }
> +
> +        length + self.sig_len
> +    }
> +
> +    fn verify_signature(&mut self, signature: &mut [u8]) -> Result<(), Error> {
> +        let mut sig = bindings::public_key_signature::default();
> +        let mut mhash: KVec<u8> = KVec::new();

Can we move this down nearer where it is used?

> +
> +        sig.s = signature as *mut _ as *mut u8;
> +        sig.s_size = self.sig_len as u32;
> +        sig.encoding = self.base_asym_enc.as_ptr() as *const u8;
> +        sig.hash_algo = self.base_hash_alg_name.as_ptr() as *const u8;
> +
> +        let mut m: KVec<u8> = KVec::new();
> +        m.extend_with(SPDM_COMBINED_PREFIX_SZ + self.hash_len, 0, GFP_KERNEL)?;
> +
> +        if let Some(desc) = &mut self.desc {
> +            desc.tfm = self.shash;
> +
> +            unsafe {
> +                to_result(bindings::crypto_shash_digest(
> +                    *desc,
> +                    self.transcript.as_ptr(),
> +                    (self.transcript.len() - self.sig_len) as u32,
> +                    m[SPDM_COMBINED_PREFIX_SZ..].as_mut_ptr(),
> +                ))?;
> +            };
> +        } else {
> +            return Err(EPROTO);
> +        }
> +
> +        if self.version <= SPDM_VER_11 {
> +            sig.m = m[SPDM_COMBINED_PREFIX_SZ..].as_mut_ptr();
> +        } else {
> +            let major = self.version >> 4;
> +            let minor = self.version & 0xF;
> +
> +            let prefix = CString::try_from_fmt(fmt!("dmtf-spdm-v{major:x}.{minor:x}.*dmtf-spdm-v{major:x}.{minor:x}.*dmtf-spdm-v{major:x}.{minor:x}.*dmtf-spdm-v{major:x}.{minor:x}.*"))?;

Sometimes I think they added this bit just to make sure all implementations break
the coding style and generally look ugly. 

> +            let mut buf = prefix.into_vec();
> +            let zero_pad_len = SPDM_COMBINED_PREFIX_SZ - SPDM_PREFIX_SZ - SPDM_CONTEXT.len() - 1;
> +
> +            buf.extend_with(zero_pad_len, 0, GFP_KERNEL)?;
> +            buf.extend_from_slice(SPDM_CONTEXT.as_bytes(), GFP_KERNEL)?;
> +
> +            if buf.len() != SPDM_COMBINED_PREFIX_SZ {
> +                pr_err!("combined_spdm_prefix calculation is incorrect");
> +                return Err(EPROTO);
> +            }
> +

> +            m[..SPDM_COMBINED_PREFIX_SZ].copy_from_slice(&buf);
> +
> +            mhash.extend_with(self.hash_len, 0, GFP_KERNEL)?;
> +
> +            if let Some(desc) = &mut self.desc {
> +                desc.tfm = self.shash;
> +
> +                unsafe {
> +                    to_result(bindings::crypto_shash_digest(
> +                        *desc,
> +                        m.as_ptr(),
> +                        m.len() as u32,
> +                        mhash.as_mut_ptr(),
> +                    ))?;

Could do with a few comments or spec references. It's a challenge
to align the spec with the code. I've read this a couple of times and
can't figure out why we have a hash of something we already hashed.
I may well have forgotten a spec detail though.


> +                };
> +            } else {
> +                return Err(EPROTO);
> +            }
> +
> +            sig.m = mhash.as_mut_ptr();
> +        }
> +
> +        sig.m_size = self.hash_len as u32;
> +
> +        if let Some(leaf_key) = self.leaf_key {
> +            unsafe { to_result(bindings::public_key_verify_signature(leaf_key, &sig)) }
> +        } else {
> +            return Err(EPROTO);
> +        }
> +    }
> +
> +    pub(crate) fn challenge(&mut self, slot: u8) -> Result<(), Error> {
> +        let mut request = ChallengeReq::default();
> +        request.version = self.version;
> +        request.param1 = slot;
> +
> +        let nonce_len = request.nonce.len();
> +
> +        if self.next_nonce.len() > 0 {
> +            let request_nonce_len = request.nonce.len();
> +
> +            if self.next_nonce.len() == request_nonce_len {
> +                request
> +                    .nonce
> +                    .copy_from_slice(&self.next_nonce[..request_nonce_len]);
> +            } else {
> +                return Err(EINVAL);
> +            }
> +
> +            self.next_nonce.clear();
> +        } else {
> +            unsafe {
> +                bindings::get_random_bytes(&mut request.nonce as *mut _ as *mut c_void, nonce_len)
> +            };
> +        }
> +
> +        let req_sz = if self.version <= SPDM_VER_12 {
> +            offset_of!(ChallengeReq, context)
> +        } 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::from_elem(0u8, rsp_sz, GFP_KERNEL)?;
> +
> +        let rc = self.spdm_exchange(request_buf, response_vec.as_mut_slice())? as usize;
> +
> +        // The transport must report a length within the buffer we provided.
> +        if rc < core::mem::size_of::<ChallengeRsp>() {
> +            pr_err!("Truncated challenge response\n");
> +            return Err(EIO);
> +        }
> +        response_vec.truncate(rc);
> +
> +        let _response: &ChallengeRsp = Untrusted::new(response_vec.as_slice()).validate()?;
> +
> +        // MSHLength is 0 as no measurement summary hash requested
> +        let opaque_len_offset = core::mem::size_of::<SpdmHeader>() + self.hash_len + nonce_len;
> +
> +        if opaque_len_offset + 2 > response_vec.len() {
> +            return Err(EIO);
> +        }
> +
> +        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 rsp_sz > response_vec.len() {
> +            pr_err!("Truncated challenge response\n");
> +            return Err(EIO);
> +        }
> +

We should sanity check the context matches that in requester.
Probably just a zero check given we aren't setting it in request.

> +        self.transcript
> +            .extend_from_slice(&response_vec[..rsp_sz], GFP_KERNEL)?;
> +
> +        /* Verify signature at end of transcript against leaf key */
> +        let sig_start = rsp_sz - self.sig_len;
> +        let signature = &mut response_vec[sig_start..rsp_sz];
> +
> +        match self.verify_signature(signature) {
> +            Ok(()) => {
> +                pr_info!("Authenticated with certificate slot {slot}\n");
> +                Ok(())
> +            }
> +            Err(e) => {
> +                pr_err!("Cannot verify challenge_auth signature: {e:?}\n");
> +                Err(EPROTO)
> +            }
> +        }
> +    }
>  }