From: Alistair Francis <alistair@alistair23.me>
Support validating the SPDM certificate chain.
Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
lib/rspdm/lib.rs | 17 ++++++
lib/rspdm/state.rs | 93 ++++++++++++++++++++++++++++++++-
rust/bindings/bindings_helper.h | 2 +
3 files changed, 111 insertions(+), 1 deletion(-)
diff --git a/lib/rspdm/lib.rs b/lib/rspdm/lib.rs
index b065b3f70356..c016306116a3 100644
--- a/lib/rspdm/lib.rs
+++ b/lib/rspdm/lib.rs
@@ -136,6 +136,17 @@
provisioned_slots &= !(1 << slot);
}
+ let mut provisioned_slots = state.provisioned_slots;
+ while (provisioned_slots as usize) > 0 {
+ let slot = provisioned_slots.trailing_zeros() as u8;
+
+ if let Err(e) = state.validate_cert_chain(slot) {
+ return e.to_errno() as c_int;
+ }
+
+ provisioned_slots &= !(1 << slot);
+ }
+
0
}
@@ -144,6 +155,12 @@
/// @spdm_state: SPDM session state
#[no_mangle]
pub unsafe extern "C" fn spdm_destroy(state: &'static mut SpdmState) {
+ if let Some(leaf_key) = &mut state.leaf_key {
+ unsafe {
+ bindings::public_key_free(*leaf_key);
+ }
+ }
+
if let Some(desc) = &mut state.desc {
unsafe {
bindings::kfree(*desc as *mut _ as *mut c_void);
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
@@ -76,7 +76,8 @@
/// @slot: Certificate chain in each of the 8 slots. NULL pointer if a slot is
/// not populated. Prefixed by the 4 + H header per SPDM 1.0.0 table 15.
/// @slot_sz: Certificate chain size (in bytes).
-#[expect(dead_code)]
+/// @leaf_key: Public key portion of leaf certificate against which to check
+/// responder's signatures.
pub struct SpdmState {
pub(crate) dev: *mut bindings::device,
pub(crate) transport: bindings::spdm_transport,
@@ -106,6 +107,7 @@ pub struct SpdmState {
// Certificates
pub(crate) certs: [KVec<u8>; SPDM_SLOTS],
+ pub(crate) leaf_key: Option<*mut bindings::public_key>,
}
#[repr(C, packed)]
@@ -146,6 +148,7 @@ pub(crate) fn new(
desc: None,
hash_len: 0,
certs: [const { KVec::new() }; SPDM_SLOTS],
+ leaf_key: None,
}
}
@@ -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
+ };
+
+ 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
+ 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(())
+ }
}
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 5043eee2a8d6..52ad3e98e036 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -64,6 +64,8 @@
#include <linux/hash.h>
#include <linux/jiffies.h>
#include <linux/jump_label.h>
+#include <keys/asymmetric-type.h>
+#include <keys/x509-parser.h>
#include <linux/mdio.h>
#include <linux/mm.h>
#include <linux/miscdevice.h>
--
2.52.0On Wed, 11 Feb 2026 13:29:29 +1000
alistair23@gmail.com wrote:
> From: Alistair Francis <alistair@alistair23.me>
>
> Support validating the SPDM certificate chain.
>
> Signed-off-by: Alistair Francis <alistair@alistair23.me>
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;
> +
> + 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.
> + 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(())
> + }
> }
On Wed, Mar 4, 2026 at 1:00 AM Jonathan Cameron
<jonathan.cameron@huawei.com> wrote:
>
> On Wed, 11 Feb 2026 13:29:29 +1000
> alistair23@gmail.com wrote:
>
> > From: Alistair Francis <alistair@alistair23.me>
> >
> > Support validating the SPDM certificate chain.
> >
> > Signed-off-by: Alistair Francis <alistair@alistair23.me>
>
> 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(())
> > + }
> > }
>
>
On Tue, Mar 31, 2026 at 5:30 AM Alistair Francis <alistair23@gmail.com> wrote: > > Yep, same thing. My way is the Rust-y way to do it Hmm... In what way? I would say it is the other way around. Clippy should be flagging this, too. Cheers, Miguel
On Tue, Mar 31, 2026 at 8:11 PM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > On Tue, Mar 31, 2026 at 5:30 AM Alistair Francis <alistair23@gmail.com> wrote: > > > > Yep, same thing. My way is the Rust-y way to do it > > Hmm... In what way? I would say it is the other way around. I think it's Rust-ier as it uses the fact that * if statements can be used as expressions, which is a cool feature of Rust * Rust is explicit about comparisons being bools (no `if (value)` for example) So I find the if statement expression returning explicit bools very Rust-y and easier to read Alistair > > Clippy should be flagging this, too. > > Cheers, > Miguel
On Wed, Apr 1, 2026 at 3:48 AM Alistair Francis <alistair23@gmail.com> wrote: > > I think it's Rust-ier as it uses the fact that > * if statements can be used as expressions, which is a cool feature of Rust Sure, but a feature being nice doesn't mean it is idiomatic to apply it in every context. More generally, regarding cool features (whether in C or C++ or Rust etc.), I would say one should strive to keep code as simple as possible, i.e. we use fancy features when they buy us something, not just because we can. The kernel applies this principle too. > * Rust is explicit about comparisons being bools (no `if (value)` for example) Yes, but I am not sure how this is related. > So I find the if statement expression returning explicit bools very > Rust-y and easier to read If you mean that that seeing `true` and `false` may be clearer because it is more explicit, then that sounds OK (whether someone agrees or not), but it is a different argument than being idiomatic. In any case, I replied initially because the code should be Clippy-clean, and currently we have that lint enabled -- does it not trigger in your particular case? i.e. even if you have removed the code, it would be nice to know why it didn't trigger, or if you were not testing with `CLIPPY=1`. Anyway, if you think it is idiomatic Rust, then I would suggest discussing it with the Clippy folks, because they have it as a warning by default. Thanks! Cheers, Miguel
© 2016 - 2026 Red Hat, Inc.