Re: [RFC v3 22/27] lib: rspdm: Support SPDM certificate validation

[Alistair Francis]

On Wed, Mar 4, 2026 at 1:00 AM Jonathan Cameron
<jonathan.cameron@xxxxxxxxxx> wrote:
>
> On Wed, 11 Feb 2026 13:29:29 +1000
> alistair23@xxxxxxxxx wrote:
>
> > From: Alistair Francis <alistair@xxxxxxxxxxxxx>
> >
> > Support validating the SPDM certificate chain.
> >
> > Signed-off-by: Alistair Francis <alistair@xxxxxxxxxxxxx>
>
> Only minor thing inline + observation on the code that I think we want to remove
> for now wrt to checking the root cert.
>
> > diff --git a/lib/rspdm/state.rs b/lib/rspdm/state.rs
> > index 1e5656144611..728b920beace 100644
> > --- a/lib/rspdm/state.rs
> > +++ b/lib/rspdm/state.rs
>
> > @@ -743,4 +746,92 @@ pub(crate) fn get_certificate(&mut self, slot: u8) -> Result<(), Error> {
> >
> >          Ok(())
> >      }
> > +
> > +    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();
> > +        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 {
> > +            let cert_len = unsafe {
> > +                bindings::x509_get_certificate_length(
> > +                    &cert_chain_buf[offset..] as *const _ as *const u8,
> > +                    cert_chain_len - offset,
> > +                )
> > +            };
> > +
> > +            if cert_len < 0 {
> > +                pr_err!("Invalid certificate length\n");
> > +                to_result(cert_len as i32)?;
> > +            }
> > +
> > +            let _is_leaf_cert = if offset + cert_len as usize == cert_chain_len {
> > +                true
> > +            } else {
> > +                false
> > +            };
> Same as the following?
>                let _is_leaf_cert = offset + cert_len as usize == cert_chain_len;

Yep, same thing. My way is the Rust-y way to do it

Although _is_leaf_cert is unused, so I'll just remove it

>
>
> > +
> > +            let cert_ptr = unsafe {
> > +                from_err_ptr(bindings::x509_cert_parse(
> > +                    &cert_chain_buf[offset..] as *const _ as *const c_void,
> > +                    cert_len as usize,
> > +                ))?
> > +            };
> > +            let cert = unsafe { *cert_ptr };
> > +
> > +            if cert.unsupported_sig || cert.blacklisted {
> > +                to_result(-(bindings::EKEYREJECTED as i32))?;
> > +            }
> > +
> > +            if let Some(prev) = prev_cert {
> > +                // Check against previous certificate
> > +                let rc = unsafe { bindings::public_key_verify_signature((*prev).pub_, cert.sig) };
> > +
> > +                if rc < 0 {
> > +                    pr_err!("Signature validation error\n");
> > +                    to_result(rc)?;
> > +                }
> > +            } else {
> > +                // Check aginst root keyring
> So this is the check that Dan and Jason are proposing we drop (for now).
>
> Works for me as easy to bring back later and lets us move forward
> in the meantime. As long as userspace gets nothing to indicate
> that we have in any way checked this root cert we are making no false claims.

Will do!

Alistair

>
> > +                let key = unsafe {
> > +                    from_err_ptr(bindings::find_asymmetric_key(
> > +                        self.keyring,
> > +                        (*cert.sig).auth_ids[0],
> > +                        (*cert.sig).auth_ids[1],
> > +                        (*cert.sig).auth_ids[2],
> > +                        false,
> > +                    ))?
> > +                };
> > +
> > +                let rc = unsafe { bindings::verify_signature(key, cert.sig) };
> > +                unsafe { bindings::key_put(key) };
> > +
> > +                if rc < 0 {
> > +                    pr_err!("Root signature validation error\n");
> > +                    to_result(rc)?;
> > +                }
> > +            }
> > +
> > +            if let Some(prev) = prev_cert {
> > +                unsafe { bindings::x509_free_certificate(prev) };
> > +            }
> > +
> > +            prev_cert = Some(cert_ptr);
> > +            offset += cert_len as usize;
> > +        }
> > +
> > +        if let Some(prev) = prev_cert {
> > +            if let Some(validate) = self.validate {
> > +                let rc = unsafe { validate(self.dev, slot, prev) };
> > +                to_result(rc)?;
> > +            }
> > +
> > +            self.leaf_key = unsafe { Some((*prev).pub_) };
> > +        }
> > +
> > +        Ok(())
> > +    }
> >  }
>
>