Re: [PATCH v3 21/21] lib: rspdm: Support SPDM challenge
From: Alistair
Date: Thu Sep 10 2026 - 23:49:26 EST
On Wed, 2026-09-09 at 02:37 +0100, Jonathan Cameron wrote:
> On Tue, 1 Sep 2026 11:03:47 +1000
> alistair23@xxxxxxxxx wrote:
>
> > From: Alistair Francis <alistair@xxxxxxxxxxxxx>
> >
> > Support the CHALLENGE SPDM command.
> >
> > Signed-off-by: Alistair Francis <alistair@xxxxxxxxxxxxx>
>
> 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.
Thanks so much for your review!
I'm including fixes for all of your comments to my series.
I'm adding comments for the crypto as it is confusing (and some of it
is outside of SPDM and part of how RSA/ECDSA signature verification
works).
I'm hoping to get some Rust people to look at it over the new few
weeks, so your SPDM eyes are very helpful!
Alistair
>
> 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)
> > + }
> > + }
> > + }
> > }
>