lib: Rust implementation of SPDM
Subsystems (10)
v6 cover lib: Rust implementation of SPDM
From: Alistair Francis <[email protected]>
Security Protocols and Data Models (SPDM) [1] is used for authentication, attestation and key exchange. SPDM is generally used over a range of transports, such as PCIe, MCTP/SMBus/I3C, ATA, SCSI, NVMe or TCP.
From the kernels perspective SPDM is used to authenticate and attest devices. In this threat model a device is considered untrusted until it can be verified by the kernel and userspace using SPDM. As such SPDM data is untrusted data that can be mallicious.
The SPDM specification is also complex, with the 1.2.1 spec being almost 200 pages and the 1.3.0 spec being almost 250 pages long.
As such we have the kernel parsing untrusted responses from a complex specification, which sounds like a possible exploit vector. This is the type of place where Rust excels!
This series implements a SPDM requester in Rust.
This is done using the Validate and Untrusted traits. The validate() function has been updated updated to allow context which we use as part of the validation.
The validation has also swapped from a C style pointer cast to using a valid Rust struct and copying the data from the original buffer to the new struct. This obvious has some overhead, but as SPDM packets are relativly small, and not used as part of a constant hot path this seems fine. The big advantage is that we can validate everyting at once, convert the endianess when creating the struct and we get left with valid (not repr[c, packed]) Rust structs.
This makes the code a lot cleaner and more robust. It also removes a large number of unsafes and using get() for the arrays removes a large number of possible panics. Thanks for everyone at Kangrejos for the help with this!
This series also adds the required Rust Crypto bindings for SPDM.
I have kept Jonathon's reviews of the SPDM code, as the changes since the last vesion are mostly just mechanical, convertting the repr[c, packed] structs to copy structs.
This is based on Lukas' C implementation [2], but has been refacted during the first few RFCs. I have included some of the relevent patchesfrom Lukas' C SPDM implementation in this series where they are required.
This is a standalone series and doesn't depend on Lukas' implementation.
The goal of this series is to get the smallest possible SPDM implementation upstream. That will provide building blocks for us to continue working on.
As such we don't yet provide evidence or certificates to userspace, allow userspace to provide a nonce, support PQC or more advanced SPDM features. This is enough to communicate with a device and return "authenticated" to userspace.
Note that RFC v3 did provide evidence and certificates to userspace and allowed a custom nonce. Showing that it's possible. I also have patches that build apon [4] to do this via a TSM driver, again showing it's possible with the current approach.
We just don't support it yet and for TSM I need [4] upstream first.
This series is different to Lukas' original approach and the approach taken in the previous RFCs and instead adds the PCI-CMA support as a TSM driver. This was described by Dan in [3] and [5]. The advantage here is that for PCIe we can leverage the TSM work for a lot of the features and provide userspace a consistient interface between PCI TSM and CMA.
This series also doesn't check the certificate chain against the kernel keyring and will instead leave that to userspace once [4] is merged.
Other transport mode (such as ATA, SCSI, NVMe and MCTP) will therefore need slightly different approaches, as TSM doesn't apply. The library can support this though, it will just need some netlink and sysfs wrappers added as applicable. This way each transport can support SPDM in the way it sees fit.
The entire tree can be seen here:
https://github.com/alistair23/linux/tree/alistair/spdm-rust-tsm
I'm testing this by running the following
```shell cargo run -- --qemu-server response qemu-system-x86_64 \ -nic none \ -object rng-random,filename=/dev/urandom,id=rng0 \ -device virtio-rng-pci,rng=rng0 \ -drive file=deploy/images/qemux86-64/core-image-pcie-qemux86-64.rootfs.ext4,if=virtio,format=raw \ -usb -device usb-tablet -usb -device usb-kbd \ -cpu Skylake-Client \ -machine q35,i8042=off \ -smp 4 -m 2G \ -drive file=blknvme,if=none,id=mynvme,format=raw \ -device pcie-root-port,chassis=1,id=pci.1 \ -device nvme,drive=mynvme,serial=deadbeef,spdm_port=2323,spdm_trans=doe,bus=pci.1 \ -snapshot \ -serial mon:stdio -serial null -nographic \ -kernel deploy/images/qemux86-64/bzImage \ -append 'root=/dev/vda rw console=ttyS0 console=ttyS1 oprofile.timer=1 tsc=reliable no_timer_check rcupdate.rcu_expedited=1 swiotlb=0 ' echo tsm0 > /sys/bus/pci/devices/0000:01:00.0/tsm/connect ```
1: https://www.dmtf.org/standards/spdm
2: https://lore.kernel.org/all/[email protected]/
3: http://lore.kernel.org/[email protected]
4: https://lore.kernel.org/all/[email protected]/
5: https://lore.kernel.org/lkml/[email protected]/
v6:
- Add crypto bindings
- Expose public Rust API
- Address LLM generated reviews
v5:
- Split out crypto into Rust kernel wrappers
- Use ZeroCopy for SpdmHeader
- Add the Transport trait and CTransport type
- Bind the device driver
- Address bugs and issues from reviews
v4:
- Based on discussions at Kangrejos the following changes were made
- Add context to the validate functions
- Convert the validate() functions to be complete Rust structs and copy the data, instead of C style pointer casts
- Use ForeignOwnable trait for SpdmState
- Use get() for array accesses
- Address review comments
v3:
- Rebase on 7.3-rc1
v2:
- Tidy up the PCI/TSM function naming and is_pci_tsm_host()
- A large number of changes (mostly to the Rust code) based on Sashiko reviews
- This includes a few local runs of Sashiko
- This has fixed a few undefined behviour bugs in the Rust code
- Rebase on v4 of Benno's validate patches (patch 1, 2, 3)
v1:
- Add CMA as a TSM driver
- Initial support for SPDM 1.4
- Cleanup a range of comments and concerns from RFC
- Remove kernel keyring checks
RFC v3:
- Use netlink to send information to userspace
- Don't autogenerate Rust helpers
RFC v2:
- Drop support for Rust and C implementations
- Include patches from Lukas to reduce series deps
- Large code cleanups based on more testing
- Support for authentication
Alistair Francis (19):
virt: coco: change tsm_register() to use a const struct
rust: add bindings for hash.h
rust: device: Expose Device as_raw() publicly
lib: rspdm: Initial commit of Rust SPDM PCI/TSM: Rename pf0 to host
PCI/TSM: Support connecting to PCIe CMA devices PCI/CMA: Add a PCI TSM CMA driver using SPDM
lib: rspdm: Support SPDM get_version
lib: rspdm: Support SPDM get_capabilities
rust: kernel: Add crypto hash bindings
lib: rspdm: Support SPDM negotiate_algorithms
lib: rspdm: Support SPDM get_digests
lib: rspdm: Support SPDM get_certificate
rust: public_key: Add crypto public key bindings
keys: Guard asymmetric-type.h behind CONFIG_KEYS
rust: x509: Add X509 certificate bindings
lib: rspdm: Support SPDM certificate validation
lib: rspdm: Support SPDM challenge
lib: rspdm: Expose public Rust API
Benno Lossin (2):
rust: create basic untrusted data API
rust: validate: add Validate trait
Lukas Wunner (4):
X.509: Make certificate parser public
X.509: Parse Subject Alternative Name in certificates X.509: Move certificate length retrieval into new helper PCI/CMA: Validate Subject Alternative Name in certificates
MAINTAINERS | 13 + crypto/asymmetric_keys/x509_cert_parser.c | 9 + crypto/asymmetric_keys/x509_loader.c | 38 +- crypto/asymmetric_keys/x509_parser.h | 42 +- drivers/crypto/ccp/sev-dev-tio.h | 4 +- drivers/crypto/ccp/sev-dev-tsm.c | 14 +- drivers/pci/Kconfig | 14 + drivers/pci/Makefile | 5 + drivers/pci/cma.asn1 | 41 + drivers/pci/cma.c | 271 +++++ drivers/pci/doe.c | 6 +- drivers/pci/tsm.c | 185 ++-- drivers/virt/coco/tsm-core.c | 4 +- include/keys/asymmetric-type.h | 5 + include/keys/x509-parser.h | 57 ++ include/linux/oid_registry.h | 3 + include/linux/pci-doe.h | 4 + include/linux/pci-tsm.h | 48 +- include/linux/pci.h | 1 + include/linux/spdm.h | 71 ++ include/linux/tsm.h | 2 +- lib/Kconfig | 18 + lib/Makefile | 2 + lib/rspdm/Makefile | 10 + lib/rspdm/consts.rs | 123 +++ lib/rspdm/lib.rs | 317 ++++++ lib/rspdm/state.rs | 1000 ++++++++++++++++++ lib/rspdm/validator.rs | 1140 +++++++++++++++++++++ rust/bindings/bindings_helper.h | 5 + rust/helpers/hash.c | 22 + rust/helpers/helpers.c | 1 + rust/kernel/device.rs | 3 +- rust/kernel/hash.rs | 141 +++ rust/kernel/lib.rs | 7 + rust/kernel/public_key.rs | 83 ++ rust/kernel/validate.rs | 281 +++++ rust/kernel/x509.rs | 236 +++++ 37 files changed, 4082 insertions(+), 144 deletions(-) create mode 100644 drivers/pci/cma.asn1 create mode 100644 drivers/pci/cma.c create mode 100644 include/keys/x509-parser.h create mode 100644 include/linux/spdm.h create mode 100644 lib/rspdm/Makefile create mode 100644 lib/rspdm/consts.rs create mode 100644 lib/rspdm/lib.rs create mode 100644 lib/rspdm/state.rs create mode 100644 lib/rspdm/validator.rs create mode 100644 rust/helpers/hash.c create mode 100644 rust/kernel/hash.rs create mode 100644 rust/kernel/public_key.rs create mode 100644 rust/kernel/validate.rs create mode 100644 rust/kernel/x509.rs
Signature · 1 line
2.56.0
PATCH v6 1/25 virt: coco: change tsm_register() to use a const struct
From: Alistair Francis <[email protected]>
The tsm_register() function takes a struct pci_tsm_ops *ops argument. This eventually is stored in struct tsm_dev as a const, so we should just change the tsm_register() register function argument to be a const.
--- drivers/crypto/ccp/sev-dev-tsm.c | 2 +- drivers/virt/coco/tsm-core.c | 4 ++-- include/linux/tsm.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-)
drivers/crypto/ccp/sev-dev-tsm.c +1 -1
@@ -339,7 +339,7 @@ static void dsm_disconnect(struct pci_dev *pdev)streams_teardown (dev_data->ide); } -static struct pci_tsm_ops sev_tsm_ops = { +static const struct pci_tsm_ops sev_tsm_ops = { .probe = dsm_probe, .remove = dsm_remove, .connect = dsm_connect,
drivers/virt/coco/tsm-core.c +2 -2
@@ -57,7 +57,7 @@ static struct tsm_dev *alloc_tsm_dev(struct device *parent) }static struct tsm_dev *tsm_register_pci_or_reset (struct tsm_dev *tsm_dev, -struct pci_tsm_ops *pci_ops) +const struct pci_tsm_ops *pci_ops) {int rc;
@@ -78,7 +78,7 @@ static struct tsm_dev *tsm_register_pci_or_reset(struct tsm_dev *tsm_dev,return tsm_dev; } -struct tsm_dev *tsm_register (struct device *parent,struct pci_tsm_ops *pci_ops) +struct tsm_dev *tsm_register (struct device *parent,const struct pci_tsm_ops *pci_ops) { struct tsm_dev *tsm_dev __free(put_tsm_dev) = alloc_tsm_dev(parent);struct device *dev;
include/linux/tsm.h +1 -1
@@ -120,7 +120,7 @@ DEFINE_FREE(put_tsm_dev, struct tsm_dev *,int tsm_report_register (const struct tsm_report_ops *ops,void *priv);int tsm_report_unregister (const struct tsm_report_ops *ops); -struct tsm_dev *tsm_register (struct device *parent,struct pci_tsm_ops *ops); +struct tsm_dev *tsm_register (struct device *parent,const struct pci_tsm_ops *ops);void tsm_unregister (struct tsm_dev *tsm_dev);struct tsm_dev *find_tsm_dev (int id);#endif /* __TSM_H */
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v6 01/25] virt: coco: change tsm_register() to use a const struct
Sashiko has reviewed this patch and found no issues.
Quoted 7 lines
From: Alistair Francis <[email protected]>
The
tsm_register()function takes a structpci_tsm_ops*ops argument. This eventually is stored in structtsm_devas a const, so we should just change thetsm_register()register function argument to be a const.Signed-off-by Alistair Francis <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
PATCH v6 2/25 rust: create basic untrusted data API
In reply to [email protected]From: Benno Lossin <[email protected]>
When the kernel receives external data (e.g. from userspace), it usually is a very bad idea to directly use the data for logic decision in the kernel. For this reason, such data should be explicitly marked and validated before making decision based on its value.
The Untrusted<T> wrapper type marks a value of type T as untrusted. The particular meaning of "untrusted" highly depends on the type T. For example T = u8 ensures that the value of the byte cannot be retrieved. However, T = [u8] still allows to access the length of the slice. Similarly, T = KVec<U> allows modifications.
Message-ID: <[email protected]>
--- rust/kernel/lib.rs | 1 + rust/kernel/validate.rs | 198 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 199 insertions(+) create mode 100644 rust/kernel/validate.rs
rust/kernel/lib.rs +1 -0
@@ -147,6 +147,7 @@pub mod uaccess;#[cfg( CONFIG_USB = "y" )] pub mod usb; +pub mod validate;pub mod workqueue;pub mod xarray;
rust/kernel/validate.rs new file +198 -0
198 lines not on this page · raw message
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v6 02/25] rust: create basic untrusted data API
Sashiko has reviewed this patch and found no issues.
Quoted 13 lines
From: Benno Lossin <[email protected]>
When the kernel receives external data (e.g. from userspace), it usually is a very bad idea to directly use the data for logic decision in the kernel. For this reason, such data should be explicitly marked and validated before making decision based on its value.
The
Untrusted<T>wrapper type marks a value of typeTas untrusted. The particular meaning of "untrusted" highly depends on the typeT. For exampleT = u8ensures that the value of the byte cannot be retrieved. However,T = [u8]still allows to access the length of the slice. Similarly,T = KVec<U>allows modifications.
Quoted 3 lines
Signed-off-by Benno Lossin <[email protected]>Message-ID: <[email protected]>
Signed-off-by Alistair Francis <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
PATCH v6 3/25 rust: validate: add `Validate` trait
In reply to [email protected]From: Benno Lossin <[email protected]>
Introduce the Validate<Input> trait and functions to validate Untrusted<T> using said trait. This allows one to access the inner value of Untrusted<T> via validate{,_ref,_mut} functions which subsequently delegate the validation to user-implemented Validate trait.
The Validate trait is the only entry point for validation code, making it easy to spot where data is being validated.
The reason for restricting the types that can be inputs to Validate::validate is to be able to have the validate... functions on Untrusted. This is also the reason for the suggestions in the Usage in API Design section in the commit that introduced Untrusted<T>.
The Validate::validate() function allows additional context to be passed, which can be used as part of the validation process. For example comparing data against already negotiated information.
Message-ID: <[email protected]>
--- rust/kernel/validate.rs | 85 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 84 insertions(+), 1 deletion(-)
rust/kernel/validate.rs +84 -1
@@ -11,6 +11,9 @@//! APIs that write back into userspace usually allow writing untrusted bytes directly, allowing //! direct copying of untrusted user data back into userspace without validation. //! +//! The only way to access untrusted data is to [`Validate::validate`] it. This is facilitated by +//! the [`Validate`] trait. +//! //! # Rationale //! //! When reading data from an untrusted source, it must be validated before it can be used for
@@ -53,7 +56,7 @@/// untrusted, as it would otherwise violate normal Rust rules. For this reason, one can easily /// convert that reference to `&[Untrusted<u8>]`. Another such example is `Untrusted<KVec<T>>`, it /// derefs to `[Untrusted<T>]`. Raw bytes however do not behave in this way, `Untrusted<u8>` is -/// totally opaque. +/// totally opaque and one can only access its value by calling [`Untrusted::validate()`]. /// /// # Usage in API Design ///
@@ -116,6 +119,42 @@ pub fn new(value: T) -> Self {Self ::from (value) } + +/// Validate the underlying untrusted data. +/// +/// See the [`Validate`] trait for more information. +#[inline] +pub fn validate <'c,Context ,V :Validate <'c,Self ,Context >>( + self, + context:Context , + ) ->Result <V ,V ::Err > +where +T :Sized , + { +V ::validate (self.0 , context) + } + +/// Validate the underlying untrusted data. +/// +/// See the [`Validate`] trait for more information. +#[inline] +pub fn validate_ref <'a, 'c,Context ,V :Validate <'c, &'aSelf ,Context >>( + &'a self, + context:Context , + ) ->Result <V ,V ::Err > { +V ::validate (&self.0 , context) + } + +/// Validate the underlying untrusted data. +/// +/// See the [`Validate`] trait for more information. +#[inline] +pub fn validate_mut <'a, 'c,Context ,V :Validate <'c, &'amut Self ,Context >>( + &'amut self, + context:Context , + ) ->Result <V ,V ::Err > { +V ::validate (&mut self.0 , context) + } }impl <T >Untrusted <T > {
@@ -196,3 +235,47 @@ pub fn from_trusted(value: Self) -> Vec<Untrusted<T>, A> {unsafe {Vec ::from_raw_parts (ptr.cast ::<Untrusted <T >>(), length, capacity) } } } + +/// Marks valid input for the [`Validate`] trait. +pub trait ValidateInput : private::Sealed { +/// Type of the inner data. +type Inner : ?Sized ; +} + +impl <T : ?Sized >ValidateInput for Untrusted <T > { +type Inner =T ; +} + +impl <'a,T : ?Sized >ValidateInput for &'aUntrusted <T > { +type Inner = &'aT ; +} + +impl <'a,T : ?Sized >ValidateInput for &'amut Untrusted <T > { +type Inner = &'amut T ; +} + +mod private { +use super ::Untrusted ; + +pub trait Sealed {} + +impl <T : ?Sized >Sealed for Untrusted <T > {} +impl <'a,T : ?Sized >Sealed for &'aUntrusted <T > {} +impl <'a,T : ?Sized >Sealed for &'amut Untrusted <T > {} +} + +/// Validate [`Untrusted`] data. +/// +/// Care must be taken when implementing this trait, as unprotected access to unvalidated data is +/// given to the [`Validate::validate`] function. The implementer must ensure that the data is only +/// used for logic after successful validation. +pub trait Validate <'c,Input :ValidateInput ,Context >:Sized { +/// Validation error. +type Err ; + +/// Validate the raw input. +/// +/// `Context` is additional context needed for validation. The `'c` lifetime is the lifetime of +/// the borrowed context (`&'c Foo`). +fn validate (raw:Input ::Inner , context:Context ) ->Result <Self ,Self ::Err >; +}
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v6 03/25] rust: validate: add `Validate` trait
Sashiko has reviewed this patch and found no issues.
Quoted 17 lines
From: Benno Lossin <[email protected]>
Introduce the
Validate<Input>trait and functions to validateUntrusted<T>using said trait. This allows one to access the inner value ofUntrusted<T>viavalidate{,_ref,_mut}functions which subsequently delegate the validation to user-implementedValidatetrait.The
Validatetrait is the only entry point for validation code, making it easy to spot where data is being validated.The reason for restricting the types that can be inputs to
Validate::validateis to be able to have thevalidate...functions onUntrusted. This is also the reason for the suggestions in theUsage in API Designsection in the commit that introducedUntrusted<T>.
Quoted 7 lines
The
Validate::validate()function allows additional context to be passed, which can be used as part of the validation process. For example comparing data against already negotiated information.Signed-off-by Benno Lossin <[email protected]>Message-ID: <[email protected]>
Signed-off-by Alistair Francis <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
PATCH v6 4/25 X.509: Make certificate parser public
In reply to [email protected]From: Lukas Wunner <[email protected]>
The upcoming support for PCI device authentication with CMA-SPDM (PCIe r6.1 sec 6.31) requires validating the Subject Alternative Name in X.509 certificates.
High-level functions for X.509 parsing such as key_create_or_update() throw away the internal, low-level struct x509_certificate after extracting the struct public_key and public_key_signature from it. The Subject Alternative Name is thus inaccessible when using those functions.
Afford CMA-SPDM access to the Subject Alternative Name by making struct x509_certificate public, together with the functions for parsing an X.509 certificate into such a struct and freeing such a struct.
No functional change intended.
--- crypto/asymmetric_keys/x509_parser.h | 42 +-------------------- include/keys/x509-parser.h | 55 ++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 41 deletions(-) create mode 100644 include/keys/x509-parser.h
crypto/asymmetric_keys/x509_parser.h +1 -41
@@ -5,51 +5,11 @@* Written by David Howells ([email protected]) */ -#include <linux/cleanup.h> -#include <linux/time.h> -#include <crypto/public_key.h> -#include <keys/asymmetric-type.h> -#include <crypto/sha2.h> - -struct x509_certificate { -struct x509_certificate *next; -struct x509_certificate *signer;/* Certificate that signed this one */ -struct public_key *pub;/* Public key details */ -struct public_key_signature *sig;/* Signature parameters */ -u8 sha256[SHA256_DIGEST_SIZE ];/* Hash for blacklist purposes */ -char *issuer;/* Name of certificate issuer */ -char *subject;/* Name of certificate subject */ -struct asymmetric_key_id *id;/* Issuer + Serial number */ -struct asymmetric_key_id *skid;/* Subject + subjectKeyId (optional) */ -time64_t valid_from; -time64_t valid_to; -const void *tbs;/* Signed data */ -unsigned tbs_size;/* Size of signed data */ -unsigned raw_sig_size;/* Size of signature */ -const void *raw_sig;/* Signature data */ -const void *raw_serial;/* Raw serial number in ASN.1 */ -unsigned raw_serial_size; -unsigned raw_issuer_size; -const void *raw_issuer;/* Raw issuer name in ASN.1 */ -const void *raw_subject;/* Raw subject name in ASN.1 */ -unsigned raw_subject_size; -unsigned raw_skid_size; -const void *raw_skid;/* Raw subjectKeyId in ASN.1 */ -unsigned index; -bool seen;/* Infinite recursion prevention */ -bool verified; -bool self_signed;/* T if self-signed (check unsupported_sig too) */ -bool unsupported_sig;/* T if signature uses unsupported crypto */ -bool blacklisted; -}; +#include <keys/x509-parser.h> /* * x509_cert_parser.c */ -extern void x509_free_certificate (struct x509_certificate *cert); -DEFINE_FREE(x509_free_certificate, struct x509_certificate *, - if (!IS_ERR(_T)) x509_free_certificate(_T)) -extern struct x509_certificate *x509_cert_parse (const void *data,size_t datalen);extern int x509_decode_time (time64_t *_t,size_t hdrlen,unsigned char tag,const unsigned char *value,size_t vlen);
include/keys/x509-parser.h new file +55 -0
55 lines not on this page · raw message
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v6 04/25] X.509: Make certificate parser public
Sashiko has reviewed this patch and found no issues.
Quoted 16 lines
From: Lukas Wunner <[email protected]>
The upcoming support for PCI device authentication with CMA-SPDM (PCIe r6.1 sec 6.31) requires validating the Subject Alternative Name in X.509 certificates.
High-level functions for X.509 parsing such as
key_create_or_update()throw away the internal, low-level structx509_certificateafter extracting the structpublic_keyandpublic_key_signaturefrom it. The Subject Alternative Name is thus inaccessible when using those functions.Afford CMA-SPDM access to the Subject Alternative Name by making struct
x509_certificatepublic, together with the functions for parsing an X.509 certificate into such a struct and freeing such a struct.
Quoted 8 lines
No functional change intended.
Signed-off-by Lukas Wunner <[email protected]>Reviewed-by Dan Williams <[email protected]>Reviewed-by Alistair Francis <[email protected]>Reviewed-by Ilpo Järvinen <[email protected]>Reviewed-by Jonathan Cameron <[email protected]>Signed-off-by Alistair Francis <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
PATCH v6 5/25 X.509: Parse Subject Alternative Name in certificates
In reply to [email protected]From: Lukas Wunner <[email protected]>
The upcoming support for PCI device authentication with CMA-SPDM (PCIe r6.1 sec 6.31) requires validating the Subject Alternative Name in X.509 certificates.
Store a pointer to the Subject Alternative Name upon parsing for consumption by CMA-SPDM.
--- crypto/asymmetric_keys/x509_cert_parser.c | 9 +++++++++ include/keys/x509-parser.h | 2 ++ 2 files changed, 11 insertions(+)
crypto/asymmetric_keys/x509_cert_parser.c +9 -0
@@ -596,6 +596,15 @@ int x509_process_extension(void *context, size_t hdrlen,return 0 ; } +if (ctx->last_oid == OID_subjectAltName) { +if (ctx->cert->raw_san) +return -EBADMSG ; + + ctx->cert->raw_san = v; + ctx->cert->raw_san_size = vlen; +return 0 ; + } +if (ctx->last_oid == OID_keyUsage) {/* * Get hold of the keyUsage bit string
include/keys/x509-parser.h +2 -0
@@ -38,6 +38,8 @@ struct x509_certificate {unsigned raw_subject_size;unsigned raw_skid_size;const void *raw_skid;/* Raw subjectKeyId in ASN.1 */ +const void *raw_san;/* Raw subjectAltName in ASN.1 */ +unsigned raw_san_size;unsigned index;bool seen;/* Infinite recursion prevention */ bool verified;
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v6 05/25] X.509: Parse Subject Alternative Name in certificates
Sashiko has reviewed this patch and found no issues.
Quoted 9 lines
From: Lukas Wunner <[email protected]>
The upcoming support for PCI device authentication with CMA-SPDM (PCIe r6.1 sec 6.31) requires validating the Subject Alternative Name in X.509 certificates.
Store a pointer to the Subject Alternative Name upon parsing for consumption by CMA-SPDM.
Quoted 7 lines
Signed-off-by Lukas Wunner <[email protected]>Reviewed-by Wilfred Mallawa <[email protected]>Reviewed-by Alistair Francis <[email protected]>Reviewed-by Ilpo Järvinen <[email protected]>Reviewed-by Jonathan Cameron <[email protected]>Acked-by Dan Williams <[email protected]>Signed-off-by Alistair Francis <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5
PATCH v6 6/25 X.509: Move certificate length retrieval into new helper
In reply to [email protected]From: Lukas Wunner <[email protected]>
The upcoming in-kernel SPDM library (Security Protocol and Data Model,
https://www.dmtf.org/dsp/DSP0274) needs to retrieve the length from ASN.1 DER-encoded X.509 certificates.
Such code already exists in x509_load_certificate_list(), so move it into a new helper for reuse by SPDM.
Export the helper so that SPDM can be tristate. (Some upcoming users of the SPDM library may be modular, such as SCSI and ATA.)
No functional change intended.
--- crypto/asymmetric_keys/x509_loader.c | 38 +++++++++++++++++++--------- include/keys/asymmetric-type.h | 2 ++ 2 files changed, 28 insertions(+), 12 deletions(-)
crypto/asymmetric_keys/x509_loader.c +26 -12
@@ -4,28 +4,42 @@#include <linux/key.h> #include <keys/asymmetric-type.h> +ssize_t x509_get_certificate_length (const u8 *p,unsigned long buflen) +{ +ssize_t plen; + +/* Each cert begins with an ASN.1 SEQUENCE tag and must be more +* than 256 bytes in size. +*/ +if (buflen <4 ) +return -EINVAL ; + +if (p[0 ] !=0x30 || + p[1 ] !=0x82 ) +return -EINVAL ; + + plen = (p[2 ] <<8 ) | p[3 ]; + plen +=4 ; +if (plen > buflen) +return -EINVAL ; + +return plen; +} +EXPORT_SYMBOL_GPL (x509_get_certificate_length); +int x509_load_certificate_list (const u8 cert_list[],const unsigned long list_size,const struct key *keyring) {key_ref_t key;const u8 *p, *end; -size_t plen; +ssize_t plen; p = cert_list; end = p + list_size;while (p < end) { -/* Each cert begins with an ASN.1 SEQUENCE tag and must be more -* than 256 bytes in size. -*/ -if (end - p <4 ) - goto dodgy_cert; -if (p[0 ] !=0x30 || - p[1 ] !=0x82 ) - goto dodgy_cert; - plen = (p[2 ] <<8 ) | p[3 ]; - plen +=4 ; -if (plen > end - p) + plen =x509_get_certificate_length (p, end - p); +if (plen <0 ) goto dodgy_cert; key = key_create_or_update(make_key_ref(keyring, 1),
include/keys/asymmetric-type.h +2 -0
@@ -84,6 +84,8 @@ extern struct key *find_asymmetric_key(struct key *keyring,const struct asymmetric_key_id *id_2, bool partial); +ssize_t x509_get_certificate_length (const u8 *p,unsigned long buflen); +int x509_load_certificate_list (const u8 cert_list[],const unsigned long list_size,const struct key *keyring);
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v6 06/25] X.509: Move certificate length retrieval into new helper
Sashiko has reviewed this patch and found no issues.
Quoted 12 lines
From: Lukas Wunner <[email protected]>
The upcoming in-kernel SPDM library (Security Protocol and Data Model,
https://www.dmtf.org/dsp/DSP0274) needs to retrieve the length from ASN.1 DER-encoded X.509 certificates.Such code already exists in
x509_load_certificate_list(), so move it into a new helper for reuse by SPDM.Export the helper so that SPDM can be tristate. (Some upcoming users of the SPDM library may be modular, such as SCSI and ATA.)
Quoted 7 lines
No functional change intended.
Signed-off-by Lukas Wunner <[email protected]>Reviewed-by Dan Williams <[email protected]>Reviewed-by Alistair Francis <[email protected]>Reviewed-by Jonathan Cameron <[email protected]>Signed-off-by Alistair Francis <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6
PATCH v6 7/25 rust: add bindings for hash.h
In reply to [email protected]From: Alistair Francis <[email protected]>
Make the functions crypto_shash_descsize(), crypto_shash_digestsize() and crypto_free_shash() available to Rust.
--- rust/bindings/bindings_helper.h | 1 + rust/helpers/hash.c | 22 ++++++++++++++++++++++ rust/helpers/helpers.c | 1 + 3 files changed, 24 insertions(+) create mode 100644 rust/helpers/hash.c
rust/bindings/bindings_helper.h +1 -0
@@ -28,6 +28,7 @@*/ #include <linux/hrtimer_types.h> +#include <crypto/hash.h> #include <linux/acpi.h> #include <linux/gpu_buddy.h> #include <drm/drm_device.h>
rust/helpers/hash.c new file +22 -0
@@ -0,0 +1,22 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <crypto/hash.h> + +#ifdef CONFIG_CRYPTO_HASH2 + +__rust_helper unsigned int rust_helper_crypto_shash_descsize(struct crypto_shash *tfm) +{ +return crypto_shash_descsize (tfm); +} + +__rust_helper unsigned int rust_helper_crypto_shash_digestsize(struct crypto_shash *tfm) +{ +return crypto_shash_digestsize (tfm); +} + +__rust_helper void rust_helper_crypto_free_shash(struct crypto_shash *tfm) +{ +crypto_free_shash (tfm); +} + +#endif /* CONFIG_CRYPTO_HASH2 */
rust/helpers/helpers.c +1 -0
@@ -67,6 +67,7 @@#include "fs.c" #include "fwctl.c" #include "gpu.c" +#include "hash.c" #include "interrupt.c" #include "io.c" #include "irq.c"
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v6 07/25] rust: add bindings for hash.h
Sashiko has reviewed this patch and found no issues.
Quoted 6 lines
From: Alistair Francis <[email protected]>
Make the functions
crypto_shash_descsize(),crypto_shash_digestsize()andcrypto_free_shash()available to Rust.Signed-off-by Alistair Francis <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7
PATCH v6 8/25 rust: device: Expose Device as_raw() publicly
In reply to [email protected]From: Alistair Francis <[email protected]>
We are going to call the as_raw() function later as part of the SPDM library, so expose it publicly.
--- rust/kernel/device.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
rust/kernel/device.rs +2 -1
@@ -262,7 +262,8 @@ pub unsafe fn drvdata_borrow<T>(&self) -> Pin<&T> {impl <Ctx :DeviceContext >Device <Ctx > {/// Obtain the raw `struct device *`. -pub (crate )fn as_raw (&self) -> *mut bindings::device { +#[inline] +pub fn as_raw (&self) -> *mut bindings::device { self.0 .get () }
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v6 08/25] rust: device: Expose Device as_raw() publicly
Sashiko has reviewed this patch and found no issues.
Quoted 6 lines
From: Alistair Francis <[email protected]>
We are going to call the
as_raw()function later as part of the SPDM library, so expose it publicly.Signed-off-by Alistair Francis <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8
PATCH v6 9/25 lib: rspdm: Initial commit of Rust SPDM
In reply to [email protected]From: Alistair Francis <[email protected]>
This is the initial commit of the Rust SPDM library.
Security Protocols and Data Model (SPDM) [1] is used for authentication, attestation and key exchange. SPDM is generally used over a range of transports, such as PCIe, MCTP/SMBus/I3C, ATA, SCSI, NVMe or TCP.
From the kernels perspective SPDM is used to authenticate and attest devices.
This initial commit setups the framework for SPDM support. This uses the Validate and Untrusted traits, which means we can treat the responses as Untrusted until we validate them.
The next few patches add SPDM functionality, which allows us to perform basic authentication.
1: https://www.dmtf.org/standards/spdm
--- MAINTAINERS | 12 ++ include/linux/spdm.h | 71 +++++++++ lib/Kconfig | 17 ++ lib/Makefile | 2 + lib/rspdm/Makefile | 10 ++ lib/rspdm/consts.rs | 20 +++ lib/rspdm/lib.rs | 274 ++++++++++++++++++++++++++++++++ lib/rspdm/state.rs | 148 +++++++++++++++++ lib/rspdm/validator.rs | 92 +++++++++++ rust/bindings/bindings_helper.h | 1 + 10 files changed, 647 insertions(+) create mode 100644 include/linux/spdm.h create mode 100644 lib/rspdm/Makefile create mode 100644 lib/rspdm/consts.rs create mode 100644 lib/rspdm/lib.rs create mode 100644 lib/rspdm/state.rs create mode 100644 lib/rspdm/validator.rs
MAINTAINERS +12 -0
@@ -25384,6 +25384,18 @@ M: Security Officers <[email protected]> S: Supported F: Documentation/process/security-bugs.rst +SECURITY PROTOCOL AND DATA MODEL (SPDM) +M: Alistair Francis <[email protected]> +M: Lukas Wunner <[email protected]> +M: Jonathan Cameron <[email protected]> +L: [email protected] +L: [email protected] +L: [email protected] +S: Maintained +T: git git://git.kernel.org/pub/scm/linux/kernel/git/devsec/spdm.git +F: include/linux/spdm.h +F: lib/rspdm/ + SECURITY SUBSYSTEM M: Paul Moore <[email protected]> M: James Morris <[email protected]>
include/linux/spdm.h new file +71 -0
71 lines not on this page · raw message
lib/Kconfig +17 -0
@@ -588,6 +588,23 @@ config LWQ_TEST help Run boot-time test of light-weight queuing. +config RSPDM + bool "Rust SPDM" + depends on RUST + select KEYS + select CRYPTO + select ASYMMETRIC_KEY_TYPE + select ASYMMETRIC_PUBLIC_KEY_SUBTYPE + select X509_CERTIFICATE_PARSER + help + The Rust implementation of the Security Protocol and Data Model (SPDM) + allows for device authentication, measurement, key exchange and + encrypted sessions. + + Crypto algorithms negotiated with SPDM are limited to those enabled + in .config. Users of SPDM therefore need to also select + any algorithms they deem mandatory. + endmenu config GENERIC_IOREMAP
lib/Makefile +2 -0
@@ -297,6 +297,8 @@ obj-$(CONFIG_PERCPU_TEST) += percpu_test.o obj-$(CONFIG_ASN1) += asn1_decoder.o obj-$(CONFIG_ASN1_ENCODER) += asn1_encoder.o +obj-$(CONFIG_RSPDM) += rspdm/ + obj-$(CONFIG_FONT_SUPPORT) += fonts/ #
lib/rspdm/Makefile new file +10 -0
@@ -0,0 +1,10 @@ +# SPDX-License-Identifier: GPL-2.0 +# +# Rust implementation of the DMTF Security Protocol and Data Model (SPDM) +# https://www.dmtf.org/dsp/DSP0274 +# +# Copyright (C) 2026 Western Digital + +obj-$(CONFIG_RSPDM) += spdm.o + +spdm-y := lib.o
lib/rspdm/consts.rs new file +20 -0
@@ -0,0 +1,20 @@ +// SPDX-License-Identifier: GPL-2.0 + +// Copyright (C) 2026 Western Digital + +//! Constants used by the library +//! +//! Rust implementation of the DMTF Security Protocol and Data Model (SPDM) +//! <https://www.dmtf.org/dsp/DSP0274> + +// SPDM versions supported by this implementation +pub (crate )const SPDM_VER_10 :u8 =0x10 ; + +pub (crate )const SPDM_MIN_VER :u8 =SPDM_VER_10 ; + +#[allow(dead_code)] +pub (crate )const SPDM_REQ :u8 =0x80 ; +#[allow(dead_code)] +pub (crate )const SPDM_ERROR :u8 =0x7f ; + +pub (crate )const SPDM_MIN_DATA_TRANSFER_SIZE :u32 =42 ;// SPDM 1.2.0 margin no 226
lib/rspdm/lib.rs new file +274 -0
274 lines not on this page · raw message
lib/rspdm/state.rs new file +148 -0
148 lines not on this page · raw message
lib/rspdm/validator.rs new file +92 -0
92 lines not on this page · raw message
rust/bindings/bindings_helper.h +1 -0
@@ -90,6 +90,7 @@#include <linux/security.h> #include <linux/serdev.h> #include <linux/slab.h> +#include <linux/spdm.h> #include <linux/sys_soc.h> #include <linux/task_work.h> #include <linux/tracepoint.h>
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v6 09/25] lib: rspdm: Initial commit of Rust SPDM
Sashiko has reviewed this patch and found no issues.
Quoted 14 lines
From: Alistair Francis <[email protected]>
This is the initial commit of the Rust SPDM library.
Security Protocols and Data Model (SPDM) [1] is used for authentication, attestation and key exchange. SPDM is generally used over a range of transports, such as PCIe, MCTP/SMBus/I3C, ATA, SCSI, NVMe or TCP.
From the kernels perspective SPDM is used to authenticate and attest devices.
This initial commit setups the framework for SPDM support. This uses the
ValidateandUntrustedtraits, which means we can treat the responses asUntrusteduntil we validate them.
Quoted 6 lines
The next few patches add SPDM functionality, which allows us to perform basic authentication.
1: https://www.dmtf.org/standards/spdm
Signed-off-by Alistair Francis <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=9
PATCH v6 10/25 PCI/TSM: Rename pf0 to host
In reply to [email protected]From: Alistair Francis <[email protected]>
Rename pci_tsm_pf0 to pci_tsm_host (and rename variables and function names from pf0 to host) as part of converting pci_tsm_host to be any device that knows how to speak any of CMA, IDE, or TDISP.
This commit just renames the functions and provides no functional change. That will happen in the next commit.
--- drivers/crypto/ccp/sev-dev-tio.h | 4 +- drivers/crypto/ccp/sev-dev-tsm.c | 12 ++--- drivers/pci/tsm.c | 84 ++++++++++++++++---------------- include/linux/pci-tsm.h | 18 ++++--- 4 files changed, 61 insertions(+), 57 deletions(-)
drivers/crypto/ccp/sev-dev-tio.h +2 -2
@@ -53,9 +53,9 @@ struct tsm_dsm_tio {struct pci_ide *ide[TIO_IDE_MAX_TC ]; }; -/* Describes TSM structure for PF0 pointed by pci_dev->tsm */ +/* Describes TSM structure for the link host pointed by pci_dev->tsm */ struct tio_dsm { -struct pci_tsm_pf0 tsm; +struct pci_tsm_host tsm;struct tsm_dsm_tio data;struct sev_device *sev; };
drivers/crypto/ccp/sev-dev-tsm.c +6 -6
@@ -205,7 +205,7 @@ static int stream_alloc(struct pci_dev *pdev, struct pci_ide **ide,return 0 ; } -static struct pci_tsm *tio_pf0_probe (struct pci_dev *pdev,struct sev_device *sev) +static struct pci_tsm *tio_host_probe (struct pci_dev *pdev,struct sev_device *sev) { struct tio_dsm *dsm __free(kfree) = kzalloc_obj(*dsm);int rc;
@@ -213,7 +213,7 @@ static struct pci_tsm *tio_pf0_probe(struct pci_dev *pdev, struct sev_device *seif (!dsm)return NULL ; - rc =pci_tsm_pf0_constructor (pdev, &dsm->tsm, sev->tsmdev); + rc =pci_tsm_host_constructor (pdev, &dsm->tsm, sev->tsmdev);if (rc)return NULL ;
@@ -226,8 +226,8 @@ static struct pci_tsm *dsm_probe(struct tsm_dev *tsmdev, struct pci_dev *pdev) {struct sev_device *sev =tsm_dev_to_sev (tsmdev); -if (is_pci_tsm_pf0 (pdev)) -return tio_pf0_probe (pdev, sev); +if (is_pci_tsm_host (pdev)) +return tio_host_probe (pdev, sev);return NULL ; }
@@ -237,10 +237,10 @@ static void dsm_remove(struct pci_tsm *tsm)pci_dbg (pdev,"TSM disabled\n" ); -if (is_pci_tsm_pf0 (pdev)) { +if (is_pci_tsm_host (pdev)) { struct tio_dsm *dsm = container_of(tsm, struct tio_dsm, tsm.base_tsm); -pci_tsm_pf0_destructor (&dsm->tsm); +pci_tsm_host_destructor (&dsm->tsm);kfree (dsm); } }
drivers/pci/tsm.c +42 -42
250 lines not on this page · raw message
include/linux/pci-tsm.h +11 -7
@@ -40,7 +40,7 @@ struct pci_tsm_ops {* pci_tsm_rwsem held for write to sync with TSM unregistration and * mutual exclusion of @connect and @disconnect. @connect and * @disconnect additionally run under the DSM lock (struct -* pci_tsm_pf0::lock) as well as @probe and @remove of the subfunctions. +* pci_tsm_host::lock) as well as @probe and @remove of the subfunctions. * @bind, @unbind, and @guest_req run under pci_tsm_rwsem held for read * and the DSM lock. */
@@ -115,19 +115,23 @@ struct pci_tsm { };/** -* struct pci_tsm_pf0 - Physical Function 0 TDISP link context +* struct pci_tsm_host - TSM host link context (CMA, IDE, or TDISP) * @base_tsm: generic core "tsm" context * @lock: mutual exclustion for pci_tsm_ops invocation * @doe_mb: PCIe Data Object Exchange mailbox +* +* The host of a TSM link is the device that knows how to speak +* CMA, IDE, or TDISP. For TDISP / IDE that is a Physical Function 0. +* For CMA-only it is a CMA DOE mailbox. */ -struct pci_tsm_pf0 { +struct pci_tsm_host {struct pci_tsm base_tsm;struct mutex lock;struct pci_doe_mb *doe_mb; };/* physical function0 and capable of 'connect' */ -static inline bool is_pci_tsm_pf0 (struct pci_dev *pdev) +static inline bool is_pci_tsm_host (struct pci_dev *pdev) {if (!pdev)return false;
@@ -204,9 +208,9 @@ int pci_tsm_register(struct tsm_dev *tsm_dev);void pci_tsm_unregister (struct tsm_dev *tsm_dev);int pci_tsm_link_constructor (struct pci_dev *pdev,struct pci_tsm *tsm,struct tsm_dev *tsm_dev); -int pci_tsm_pf0_constructor (struct pci_dev *pdev,struct pci_tsm_pf0 *tsm, -struct tsm_dev *tsm_dev); -void pci_tsm_pf0_destructor (struct pci_tsm_pf0 *tsm); +int pci_tsm_host_constructor (struct pci_dev *pdev,struct pci_tsm_host *tsm, +struct tsm_dev *tsm_dev); +void pci_tsm_host_destructor (struct pci_tsm_host *tsm);int pci_tsm_doe_transfer (struct pci_dev *pdev,u8 type,const void *req,size_t req_sz,void *resp,size_t resp_sz);int pci_tsm_bind (struct pci_dev *pdev,struct kvm *kvm,u32 tdi_id);
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v6 10/25] PCI/TSM: Rename pf0 to host
Sashiko has reviewed this patch and found no issues.
Quoted 11 lines
From: Alistair Francis <[email protected]>
Rename
pci_tsm_pf0topci_tsm_host(and rename variables and function names from pf0 to host) as part of convertingpci_tsm_hostto be any device that knows how to speak any of CMA, IDE, or TDISP.This commit just renames the functions and provides no functional change. That will happen in the next commit.
Signed-off-by Alistair Francis <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=10
PATCH v6 11/25 PCI/TSM: Support connecting to PCIe CMA devices
In reply to [email protected]From: Alistair Francis <[email protected]>
In the next patch we are going to add a PCIe CMA TSM driver, as such we need to ensure that is_pci_tsm_host() will allow us to connect to CMA capable devices. These devices don't necessarily has DEVCAP_TEE or IDE support.
To avoid calling pci_find_doe_mailbox() everytime is_pci_tsm_host() is called we can cache the CMA support in struct pci_dev and just check against that.
--- drivers/crypto/ccp/sev-dev-tsm.c | 4 +- drivers/pci/doe.c | 3 + drivers/pci/tsm.c | 107 +++++++++++++++++++++++-------- include/linux/pci-tsm.h | 32 ++++++++- include/linux/pci.h | 1 + 5 files changed, 115 insertions(+), 32 deletions(-)
drivers/crypto/ccp/sev-dev-tsm.c +2 -2
@@ -226,7 +226,7 @@ static struct pci_tsm *dsm_probe(struct tsm_dev *tsmdev, struct pci_dev *pdev) {struct sev_device *sev =tsm_dev_to_sev (tsmdev); -if (is_pci_tsm_host (pdev)) +if (is_pci_tsm_dsm (pdev))return tio_host_probe (pdev, sev);return NULL ; }
@@ -237,7 +237,7 @@ static void dsm_remove(struct pci_tsm *tsm)pci_dbg (pdev,"TSM disabled\n" ); -if (is_pci_tsm_host (pdev)) { +if (is_pci_tsm_dsm (pdev)) { struct tio_dsm *dsm = container_of(tsm, struct tio_dsm, tsm.base_tsm);pci_tsm_host_destructor (&dsm->tsm);
drivers/pci/doe.c +3 -0
@@ -871,6 +871,9 @@ void pci_doe_init(struct pci_dev *pdev)pci_doe_destroy_mb (doe_mb); } } + + pdev->doe_cma =pci_find_doe_mailbox (pdev,PCI_VENDOR_ID_PCI_SIG , +PCI_DOE_FEATURE_CMA ); }void pci_doe_destroy (struct pci_dev *pdev)
drivers/pci/tsm.c +79 -28
195 lines not on this page · raw message
include/linux/pci-tsm.h +30 -2
@@ -3,6 +3,7 @@#define __PCI_TSM_H#include <linux/mutex.h> #include <linux/pci.h> +#include <linux/pci-doe.h> #include <linux/sockptr.h> struct pci_tsm ;
@@ -130,8 +131,11 @@ struct pci_tsm_host {struct pci_doe_mb *doe_mb; }; -/* physical function0 and capable of 'connect' */ -static inline bool is_pci_tsm_host (struct pci_dev *pdev) + +/* device is a Device Security Manager (DSM), similar to +* is_pci_tsm_host() but doesnt' support CMA +*/ +static inline bool is_pci_tsm_dsm (struct pci_dev *pdev) {if (!pdev)return false;
@@ -163,6 +167,30 @@ static inline bool is_pci_tsm_host(struct pci_dev *pdev)return PCI_FUNC (pdev->devfn) ==0 ; } +/* device is a TSM host and capable of 'connect' */ +static inline bool is_pci_tsm_host (struct pci_dev *pdev) +{ +if (!pdev) +return false; + +if (!pci_is_pcie (pdev)) +return false; + +if (pdev->is_virtfn) +return false; + +/* +* Report capable if CMA is supported, which can be supported on any PCIe +* device. +*/ +#ifdef CONFIG_PCI_DOE +if (pdev->doe_cma) +return true; +#endif + +return is_pci_tsm_dsm (pdev); +} +/** * enum pci_tsm_req_scope - Scope of guest requests to be validated by TSM *
include/linux/pci.h +1 -0
@@ -566,6 +566,7 @@ struct pci_dev {#endif #ifdef CONFIG_PCI_DOE struct xarray doe_mbs;/* Data Object Exchange mailboxes */ +bool doe_cma;/* A CMA/SPDM DOE mailbox is present */ #endif #ifdef CONFIG_PCI_NPEM struct npem *npem;/* Native PCIe Enclosure Management */
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v6 11/25] PCI/TSM: Support connecting to PCIe CMA devices
Thank you for your contribution! Sashiko AI review found 4 potential issues to consider: - [High] PCI/TSM: find_dsm_dev() treats plain CMA PF0/uport as DSM for subfunctions - [High] PCI/TSM…
Thank you for your contribution! Sashiko AI review found 4 potential issues to consider:
- [High] PCI/TSM: find_dsm_dev() treats plain CMA PF0/uport as DSM for subfunctions
- [High] PCI/TSM: probe_fn() constructs subordinate pci_tsm_host without connect()
- [Medium] PCI/TSM: remove_fn() skips link_sysfs_disable() during pci_tsm_unregister()
- [Medium] PCI/TSM: disconnect_store() does not verify is_dsm() under pci_tsm_rwsem
--
commit 71288b5ca3a729f150677c17fac34c3e0b86742a
Author: Alistair Francis <[email protected]>
PCI/TSM: Support connecting to PCIe CMA devices
This commit adds a helper to distinguish DSM devices from general TSM hosts, caches CMA DOE mailbox presence on struct pci_dev, and updates TSM connect, disconnect, and lookup paths to support independent CMA-capable PCIe functions.
Quoted 4 lines
drivers/pci/tsm.c +0
-0
[ ... ]
Quoted 5 lines
drivers/pci/tsm.c +1
-0@@ -221,8 +224,13 @@ static int pci_tsm_connect(struct pci_dev *pdev, struct tsm_dev *tsm_dev)* is the ultimate arbiter of security state relative to a given * interface id, and if it says it can manage TDISP state of a function, * let it. +*
drivers/pci/tsm.c static int pci_tsm_connect(struct pci_dev *pdev, struct tsm_dev *tsm_dev) · line 236
Quoted 10 lines
drivers/pci/tsm.c +5
-1+* Only a function in a valid DSM position (function0 or upstream +* port) with TEE capability manages subordinate functions. A plain +* CMA host manages only itself, leaving sibling functions available +* to be connected independently. */ -if (has_tee (pdev)) +if (has_tee (pdev) &&is_pci_tsm_dsm (pdev))pci_tsm_walk_fns (pdev, probe_fn, pdev);return 0 ; }
[Severity: High]
Can pci_tsm_walk_fns() here still construct a subordinate struct pci_tsm_host via probe_fn() without ever calling ops->connect()?
When PF0 of a multi-function device (or an upstream switch port) has PCI_EXP_DEVCAP_TEE set, pci_tsm_connect() walks all sibling or downstream functions and calls probe_fn() -> ops->probe(). Because pci_tsm_host_constructor() delegates to pci_tsm_link_constructor() -> find_dsm_dev() without requiring dsm_dev == pdev, any TSM driver whose probe() allocates a struct pci_tsm_host for matching devices (such as a CMA TSM driver matching pdev->doe_cma on PF1, or dsm_probe() in sev-dev-tsm.c matching is_pci_tsm_dsm() on a downstream endpoint PF0 below an upstream port DSM) succeeds in probe_fn() with base_tsm.dsm_dev set to the parent DSM and without ops->connect() running.
For a sibling CMA function, sysfs then reports it as connected and authenticated while hiding tsm/connect and tsm/disconnect, preventing userspace from authenticating it. Should pci_tsm_host_constructor() reject subordinate probing when find_dsm_dev() does not return pdev (or should subordinate probing be skipped for TSM drivers that only manage self-hosted connections)?
Quoted 10 lines
drivers/pci/tsm.c +6
-0@@ -287,6 +295,19 @@ static DEVICE_ATTR_RW(connect);static int remove_fn (struct pci_dev *pdev,void *data) { +struct pci_dev *host = data; +struct pci_tsm_host *tsm_host; + +/* +* Only teardown the security context of functions that belong to the +* DSM being disconnected, leaving any sibling DSM in the same slot
drivers/pci/tsm.c static DEVICE_ATTR_RW(connect); · line 313
Quoted 10 lines
drivers/pci/tsm.c +7
-0+* untouched. +*/ +if (!pdev->tsm || pdev->tsm->dsm_dev != host) +return 0 ; + + tsm_host =to_pci_tsm_host (pdev->tsm); +guard (mutex)(&tsm_host->lock);tsm_remove (pdev->tsm);link_sysfs_disable (pdev);return 0 ;
[Severity: Medium]
Does returning early on !pdev->tsm skip link_sysfs_disable() for subordinate functions during pci_tsm_unregister()?
pci_tsm_unregister() walks PCI devices in reverse order via for_each_pci_dev_reverse(), visiting VFs and higher-numbered PFs before PF0. On each subordinate function, __pci_tsm_destroy() only calls link_sysfs_disable() when is_pci_tsm_host(pdev) && !pci_tsm_link_count, and then calls pci_tsm_fn_exit() -> tsm_remove(), which clears pdev->tsm to NULL:
drivers/pci/tsm.c:__pci_tsm_destroy() {
if (is_link_tsm(tsm_dev) && is_pci_tsm_host(pdev) && !pci_tsm_link_count)
link_sysfs_disable(pdev);
...
if (is_link_tsm(tsm_dev) && is_dsm(pdev))
pci_tsm_disconnect(pdev);
else
pci_tsm_fn_exit(pdev);
}When __pci_tsm_destroy() later reaches the DSM and runs pci_tsm_disconnect() -> pci_tsm_walk_fns_reverse(pdev, remove_fn, pdev), pdev->tsm is already NULL on those subordinates, so remove_fn() now returns early before calling link_sysfs_disable(pdev). That leaves stale sysfs attribute visibility on VFs and non-host PFs (or on subordinate CMA PFs when another link TSM remains registered). Calling link_sysfs_disable(pdev) in pci_tsm_fn_exit() would ensure subordinate sysfs groups are updated before pdev->tsm is cleared.
[ ... ]
Quoted 8 lines
drivers/pci/tsm.c +3
-1@@ -587,8 +618,17 @@ static umode_t pci_tsm_attr_visible(struct kobject *kobj,if (attr == &dev_attr_connect.attr || attr == &dev_attr_disconnect.attr) { -if (is_pci_tsm_host (pdev)) +if (is_pci_tsm_host (pdev)) { +/* +* Hide while the function is managed by
drivers/pci/tsm.c static umode_t pci_tsm_attr_visible(struct kobject *kobj, · line 633
Quoted 10 lines
drivers/pci/tsm.c +7
-0+* another DSM, so userspace cannot connect() +* it independently or disconnect() a context +* that connect() never established. +*/ +if (pdev->tsm && !is_dsm (pdev)) +return 0 ;return attr->mode; + } } }
[Severity: Medium]
Should disconnect_store() also verify is_dsm(pdev) while holding pci_tsm_rwsem?
pci_tsm_attr_visible() hides tsm/disconnect when pdev->tsm && !is_dsm(pdev), but if userspace opens a subordinate host-capable function's tsm/disconnect attribute before its DSM connects and writes to it afterwards, disconnect_store() only checks !pdev->tsm:
drivers/pci/tsm.c:disconnect_store() {
...
if (!pdev->tsm)
return -ENXIO;
...
pci_tsm_disconnect(pdev);
}That calls ops->disconnect() and tsm_remove() on a subordinate context that was never established via ops->connect(). Checking !is_dsm(pdev) in disconnect_store() prevents this race.
Quoted 49 lines
drivers/pci/tsm.c +19
-17@@ -662,14 +702,15 @@ static struct pci_dev *find_dsm_dev(struct pci_dev *pdev)struct device *grandparent;struct pci_dev *uport; -if (is_pci_tsm_host (pdev)) -return pdev; - struct pci_dev *pf0 __free(pci_dev_put) = pf0_dev_get(pdev); -if (!pf0) -return NULL ; -if (is_dsm (pf0)) +/* +* Functions subordinate to an active DSM are managed by that DSM, +* even if the function itself has a CMA DOE mailbox. Check for the +* DSM context before considering this function to be its own TSM +* host. +*/ +if (pf0 && pf0 != pdev &&is_dsm (pf0))return pf0;/* @@ -677,20 +718,24 @@ static struct pci_dev *find_dsm_dev(struct pci_dev *pdev)* downstream devices, check the first upstream port relative to this * endpoint. */ -if (!pdev->dev.parent) -return NULL ; - grandparent = pdev->dev.parent->parent; -if (!grandparent) -return NULL ; -if (!dev_is_pci (grandparent)) -return NULL ; - uport =to_pci_dev (grandparent); -if (!pci_is_pcie (uport) || -pci_pcie_type (uport) !=PCI_EXP_TYPE_UPSTREAM ) -return NULL ; +if (pdev->dev.parent) { + grandparent = pdev->dev.parent->parent; +if (grandparent &&dev_is_pci (grandparent)) { + uport =to_pci_dev (grandparent); +if (pci_is_pcie (uport) && +pci_pcie_type (uport) ==PCI_EXP_TYPE_UPSTREAM && +is_dsm (uport)) +return uport; + } + } + +/*
drivers/pci/tsm.c static struct pci_dev *find_dsm_dev(struct pci_dev *pdev) · line 740
Quoted 10 lines
drivers/pci/tsm.c +5
-2+* With no DSM context above it, a TSM host capable function (e.g. +* one with a CMA DOE mailbox) is its own host. +*/ +if (is_pci_tsm_host (pdev)) +return pdev; -if (is_dsm (uport)) -return uport;return NULL ; }
[Severity: High]
Does find_dsm_dev() misidentify a plain CMA host on PF0 (or on an upstream port) as the DSM for sibling or downstream functions?
is_dsm() only checks pdev->tsm && pdev->tsm->dsm_dev == pdev, which is true for any self-hosted connected function, including a plain CMA host without PCI_EXP_DEVCAP_TEE. When PF0 on a multi-function CMA device is connected first, pci_tsm_connect() skips subordinate probing because has_tee(pf0) is false, leaving PF1 to be connected independently. However, when PF1 is later connected via pci_tsm_connect() -> pci_tsm_host_constructor() -> pci_tsm_link_constructor() -> find_dsm_dev(pf1), is_dsm(pf0) is true, so find_dsm_dev(pf1) returns pf0 instead of pf1.
That sets pf1->tsm->dsm_dev = pf0 on PF1's own struct pci_tsm_host:to_pci_tsm_host(pf1->tsm) then resolves to pf0's pci_tsm_host (using pf0's lock and DOE mailbox in pci_tsm_doe_transfer()), is_dsm(pf1) becomes false (hiding PF1's connect and disconnect sysfs attributes), and disconnecting pf0 or destroying pf1 tears down pf1->tsm via remove_fn() or pci_tsm_fn_exit() without calling ops->disconnect(pf1).
Should find_dsm_dev() (and pci_tsm_init()) also require has_tee() and is_pci_tsm_dsm() on pf0 and uport, matching the check in pci_tsm_connect()?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=11
PATCH v6 12/25 PCI/CMA: Add a PCI TSM CMA driver using SPDM
In reply to [email protected]From: Alistair Francis <[email protected]>
Component Measurement and Authentication (CMA, PCIe r6.2 sec 6.31) allows for measurement and authentication of PCIe devices. It is based on the Security Protocol and Data Model specification (SPDM,
https://www.dmtf.org/dsp/DSP0274).
CMA-SPDM in turn forms the basis for Integrity and Data Encryption (IDE, PCIe r6.2 sec 6.33) because the key material used by IDE is transmitted over a CMA-SPDM session.
As a first step, add support for authentication via a CMA TSM driver.
This was previously discussed here:
http://lore.kernel.org/[email protected]
By utilising a TSM driver we get a lot of the TSM driver probe policies "for free". Currently there is no mechanism to provide evidence to userspace, as the TSM system doesn't support that at the moment. That can be added later when support by TSM.
Credits: Jonathan wrote the original proof-of-concept for a CMA implementation. Lukas reworked that for upstream. Wilfred contributed fixes for issues discovered during testing. Alistair reworked it as a TSM driver.
--- MAINTAINERS | 1 + drivers/pci/Kconfig | 14 ++++ drivers/pci/Makefile | 2 + drivers/pci/cma.c | 150 ++++++++++++++++++++++++++++++++++++++++ drivers/pci/doe.c | 3 - include/linux/pci-doe.h | 4 ++ 6 files changed, 171 insertions(+), 3 deletions(-) create mode 100644 drivers/pci/cma.c
MAINTAINERS +1 -0
@@ -25393,6 +25393,7 @@ L: [email protected] L: [email protected] S: Maintained T: git git://git.kernel.org/pub/scm/linux/kernel/git/devsec/spdm.git +F: drivers/pci/cma* F: include/linux/spdm.h F: lib/rspdm/
drivers/pci/Kconfig +14 -0
@@ -124,6 +124,20 @@ config PCI_ATS config PCI_IDE bool +config PCI_CMA + bool "Component Measurement and Authentication (CMA-SPDM)" + depends on RSPDM + select CRYPTO_ECDSA + select CRYPTO_RSA + select CRYPTO_SHA256 + select CRYPTO_SHA512 + select PCI_DOE + select PCI_TSM + help + Authenticate devices on enumeration per PCIe r6.2 sec 6.31. + A PCI DOE mailbox is used as transport for DMTF SPDM based + authentication, measurement and secure channel establishment. + config PCI_TSM bool "PCI TSM: Device security protocol support" select PCI_IDE
drivers/pci/Makefile +2 -0
@@ -41,6 +41,8 @@ obj-$(CONFIG_PCI_NPEM) += npem.o obj-$(CONFIG_PCIE_TPH) += tph.o obj-$(CONFIG_CARDBUS) += setup-cardbus.o +obj-$(CONFIG_PCI_CMA) += cma.o + # Endpoint library must be initialized before its users obj-$(CONFIG_PCI_ENDPOINT) += endpoint/
drivers/pci/cma.c new file +150 -0
150 lines not on this page · raw message
drivers/pci/doe.c +0 -3
@@ -31,9 +31,6 @@#define PCI_DOE_FLAG_CANCEL 0#define PCI_DOE_FLAG_DEAD 1 -/* Max data object length is 2^18 dwords */ -#define PCI_DOE_MAX_LENGTH (1 << 18) -/** * struct pci_doe_mb - State for a single DOE mailbox *
include/linux/pci-doe.h +4 -0
@@ -19,6 +19,10 @@ struct pci_doe_mb;#define PCI_DOE_FEATURE_CMA 1#define PCI_DOE_FEATURE_SSESSION 2 +/* Max data object length is 2^18 dwords (including 2 dwords for header) */ +#define PCI_DOE_MAX_LENGTH (1 << 18) +#define PCI_DOE_MAX_PAYLOAD ((PCI_DOE_MAX_LENGTH - 2) * sizeof(u32)) +struct pci_doe_mb *pci_find_doe_mailbox (struct pci_dev *pdev,u16 vendor,u8 type);
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v6 12/25] PCI/CMA: Add a PCI TSM CMA driver using SPDM
Thank you for your contribution! Sashiko AI review found 2 potential issues to consider: - [High] PCI/CMA: pci_cma_tsm_probe() misattaches subordinate and sibling functions - [Low] PCI/CMA…
Thank you for your contribution! Sashiko AI review found 2 potential issues to consider:
- [High] PCI/CMA: pci_cma_tsm_probe() misattaches subordinate and sibling functions
- [Low] PCI/CMA: write-only static variable pci_cma_tsm_dev in pci_cma_tsm_init()
--
commit 8928bf8b82ed0687cad0e8da916dd5dcfd9c6e73
Author: Alistair Francis <[email protected]>
PCI/CMA: Add a PCI TSM CMA driver using SPDM
This commit adds a PCI TSM driver for Component Measurement and Authentication (CMA-SPDM) over PCI DOE mailboxes, enabling device authentication via SPDM.
Quoted 5 lines
drivers/pci/cma.c new file +0
-0
[ ... ]
Quoted 19 lines
drivers/pci/cma.c new file +19
-0+static struct pci_tsm *pci_cma_tsm_probe (struct tsm_dev *tsm_dev, +struct pci_dev *pdev) +{ +struct pci_cma_tsm *cma; +int rc; + +if (!pdev->doe_cma || pdev->is_virtfn) +return NULL ; + + cma =kzalloc (sizeof (*cma),GFP_KERNEL ); +if (!cma) +return NULL ; + + rc =pci_tsm_host_constructor (pdev, &cma->host, tsm_dev); +if (rc) { +kfree (cma); +return NULL ; + } +
drivers/pci/cma.c line 75
Quoted 10 lines
drivers/pci/cma.c new file +10
-0+ cma->spdm =spdm_create (&pdev->dev, pci_doe_transport, cma->host.doe_mb, +PCI_DOE_MAX_PAYLOAD ,NULL ); +if (!cma->spdm) { +pci_tsm_host_destructor (&cma->host); +kfree (cma); +return NULL ; + } + +return &cma->host.base_tsm; +}
[Severity: High]
Can pci_cma_tsm_probe() attach subordinate or sibling functions without authenticating them, and bind sibling functions' pci_tsm_host contexts to function 0?
pci_cma_tsm_probe() succeeds for any physical function with pdev->doe_cma set, even when called from probe_fn() for a subordinate function where pci_cma_tsm_connect() is never run:
drivers/pci/tsm.c:probe_fn() {
...
pdev->tsm = ops->probe(dsm_dev->tsm->tsm_dev, pdev);
pci_dbg(pdev, "setup TSM context: DSM: %s status: %s\n",
pci_name(dsm_dev), pdev->tsm ? "success" : "failed");
if (pdev->tsm)
link_sysfs_enable(pdev);
return 0;
}When a TEE-capable function 0 or upstream switch port is connected to pci_cma_tsm (via pci_tsm_connect() -> pci_tsm_walk_fns()), or when a sibling PF or downstream device is later initialized via pci_tsm_init() while function 0 or the upstream port is connected, probe_fn() populates pdev->tsm without calling spdm_authenticate(). Because authenticated_show() reports the TSM device name whenever pdev->tsm is non-NULL, sysfs reports those unauthenticated functions as authenticated and hides their tsm/connect and tsm/disconnect attributes.
In addition, once function 0 is connected to pci_cma_tsm, is_dsm(pf0) is true even on a non-TEE multi-function device:
drivers/pci/tsm.c:find_dsm_dev() {
...
if (pf0 && pf0 != pdev && is_dsm(pf0))
return pf0;
...
}When a sibling PF is then connected via pci_tsm_connect() -> pci_tsm_host_constructor() -> pci_tsm_link_constructor(), find_dsm_dev() sets the sibling PF's base_tsm.dsm_dev to pf0 instead of the sibling PF itself. That breaks the struct pci_tsm_host invariant (dsm_dev == pdev), causing to_pci_tsm_host() and pci_tsm_doe_transfer() on the sibling PF to use function 0's lock and DOE mailbox, hiding the sibling PF's disconnect attribute, and causing __pci_tsm_destroy() to tear down the sibling PF via pci_tsm_fn_exit() without holding tsm_host->lock.
[ ... ]
drivers/pci/cma.c line 149
Quoted 13 lines
drivers/pci/cma.c new file +13
-0+static struct tsm_dev *pci_cma_tsm_dev; + +static int __init pci_cma_tsm_init (void ) +{ +struct tsm_dev *tsm_dev; + + tsm_dev =tsm_register (NULL , &pci_cma_tsm_ops); +if (IS_ERR (tsm_dev)) +return PTR_ERR (tsm_dev); + + pci_cma_tsm_dev = tsm_dev; +return 0 ; +}
[Severity: Low]pci_cma_tsm_dev is assigned in pci_cma_tsm_init() but never read anywhere, which triggers a -Wunused-but-set-variable warning under W=1.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=12
PATCH v6 13/25 PCI/CMA: Validate Subject Alternative Name in certificates
In reply to [email protected]From: Lukas Wunner <[email protected]>
PCIe r6.1 sec 6.31.3 stipulates requirements for Leaf Certificates presented by devices, in particular the presence of a Subject Alternative Name which encodes the Vendor ID, Device ID, Device Serial Number, etc.
This prevents a mismatch between the device identity in Config Space and the certificate. A device cannot misappropriate a certificate from a different device without also spoofing Config Space. As a corollary, it cannot dupe an arbitrary driver into binding to it. Only drivers which bind to the device identity in the Subject Alternative Name work (PCIe r6.1 sec 6.31 "Implementation Note: Overview of Threat Model").
The Subject Alternative Name is signed, hence constitutes a signed copy of a Config Space portion. It's the same concept as web certificates which contain a set of domain names in the Subject Alternative Name for identity verification.
Parse the Subject Alternative Name using a small ASN.1 module and validate its contents. The theory of operation is explained in a comment at the top of the newly inserted code.
This functionality is introduced in a separate commit on top of basic CMA-SPDM support to split the code into digestible, reviewable chunks.
The CMA OID added here is taken from the official OID Repository (it's not documented in the PCIe Base Spec):
https://oid-rep.orange-labs.fr/get/2.23.147
Side notes:
* PCIe r6.2 removes the spec language on the Subject Alternative Name. It still "requires the leaf certificate to include the information typically used by system software for device driver binding", but no longer specifies how that information is encoded into the certificate.
According to the editor of the PCIe Base Spec and the author of the CMA 1.1 ECN (which caused this change), FPGA cards which mutate their device identity at runtime (due to a firmware update) were thought as unable to satisfy the previous spec language. The Protocol Working Group could not agree on a better solution and therefore dropped the spec language entirely. They acknowledge that the requirement is now under-spec'd. Because products already exist which adhere to the Subject Alternative Name requirement per PCIe r6.1 sec 6.31.3, they recommended to "push through" and use it as the de facto standard.
The FPGA concerns are easily overcome by reauthenticating the device after a firmware update, either via sysfs or pci_cma_reauthenticate() (added by a subsequent commit).
* PCIe r6.1 sec 6.31.3 strongly recommends to verify that "the information provided in the Subject Alternative Name entry is signed by the vendor indicated by the Vendor ID." In other words, the root certificate on pci_cma_keyring which signs the device's certificate chain must have been created for a particular Vendor ID.
Unfortunately the spec neglects to define how the Vendor ID shall be encoded into the root certificate. So the recommendation cannot be implemented at this point and it is thus possible that a vendor signs device certificates of a different vendor.
* Instead of a Subject Alternative Name, Leaf Certificates may include "a Reference Integrity Manifest, e.g., see Trusted Computing Group" or "a pointer to a location where such a Reference Integrity Manifest can be obtained" (PCIe r6.1 sec 6.31.3).
A Reference Integrity Manifest contains "golden" measurements which can be compared to actual measurements retrieved from a device. It serves a different purpose than the Subject Alternative Name, hence it is unclear why the spec says only either of them is necessary. It is also unclear how a Reference Integrity Manifest shall be encoded into a certificate.
Hence ignore the Reference Integrity Manifest requirement.
[ Changed by AF:
- Fixup a few issues caught by Sashiko
]
--- drivers/pci/Makefile | 5 +- drivers/pci/cma.asn1 | 41 ++++++++++++ drivers/pci/cma.c | 123 ++++++++++++++++++++++++++++++++++- include/linux/oid_registry.h | 3 + 4 files changed, 170 insertions(+), 2 deletions(-) create mode 100644 drivers/pci/cma.asn1
drivers/pci/Makefile +4 -1
@@ -41,7 +41,10 @@ obj-$(CONFIG_PCI_NPEM) += npem.o obj-$(CONFIG_PCIE_TPH) += tph.o obj-$(CONFIG_CARDBUS) += setup-cardbus.o -obj-$(CONFIG_PCI_CMA) += cma.o +obj-$(CONFIG_PCI_CMA) += pci-cma.o +pci-cma-y := cma.o cma.asn1.o +$(obj)/cma.o: $(obj)/cma.asn1.h +$(obj)/cma.asn1.o: $(obj)/cma.asn1.c $(obj)/cma.asn1.h # Endpoint library must be initialized before its users obj-$(CONFIG_PCI_ENDPOINT) += endpoint/
drivers/pci/cma.asn1 new file +41 -0
41 lines not on this page · raw message
drivers/pci/cma.c +122 -1
143 lines not on this page · raw message
include/linux/oid_registry.h +3 -0
@@ -150,6 +150,9 @@ enum OID { OID_id_ml_dsa_65,/* 2.16.840.1.101.3.4.3.18 */ OID_id_ml_dsa_87,/* 2.16.840.1.101.3.4.3.19 */ +/* PCI */ +OID_CMA ,/* 2.23.147 */ + OID__NR };
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v6 13/25] PCI/CMA: Validate Subject Alternative Name in certificates
Sashiko has reviewed this patch and found no issues.
Quoted 13 lines
From: Lukas Wunner <[email protected]>
PCIe r6.1 sec 6.31.3 stipulates requirements for Leaf Certificates presented by devices, in particular the presence of a Subject Alternative Name which encodes the Vendor ID, Device ID, Device Serial Number, etc.
This prevents a mismatch between the device identity in Config Space and the certificate. A device cannot misappropriate a certificate from a different device without also spoofing Config Space. As a corollary, it cannot dupe an arbitrary driver into binding to it. Only drivers which bind to the device identity in the Subject Alternative Name work (PCIe r6.1 sec 6.31 "Implementation Note: Overview of Threat Model").
Quoted 8 lines
The Subject Alternative Name is signed, hence constitutes a signed copy of a Config Space portion. It's the same concept as web certificates
[ ... ][ Changed by AF:
- Fixup a few issues caught by Sashiko
]Signed-off-by Alistair Francis <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=13
PATCH v6 14/25 lib: rspdm: Support SPDM get_version
In reply to [email protected]From: Alistair Francis <[email protected]>
Support the GET_VERSION SPDM command.
--- lib/rspdm/consts.rs | 17 +++++++- lib/rspdm/lib.rs | 25 +++++++++++- lib/rspdm/state.rs | 50 ++++++++++++++++++++++- lib/rspdm/validator.rs | 90 +++++++++++++++++++++++++++++++++++++++++- 4 files changed, 176 insertions(+), 6 deletions(-)
lib/rspdm/consts.rs +15 -2
@@ -7,14 +7,27 @@//! Rust implementation of the DMTF Security Protocol and Data Model (SPDM) //! <https://www.dmtf.org/dsp/DSP0274> +use crate ::validator::SpdmHeader ; +use core::mem; +// SPDM versions supported by this implementation pub (crate )const SPDM_VER_10 :u8 =0x10 ; +#[expect(dead_code)] +pub (crate )const SPDM_VER_11 :u8 =0x11 ; +#[expect(dead_code)] +pub (crate )const SPDM_VER_12 :u8 =0x12 ; +#[expect(dead_code)] +pub (crate )const SPDM_VER_13 :u8 =0x13 ; +pub (crate )const SPDM_VER_14 :u8 =0x14 ;pub (crate )const SPDM_MIN_VER :u8 =SPDM_VER_10 ; +pub (crate )const SPDM_MAX_VER :u8 =SPDM_VER_14 ; -#[allow(dead_code)] pub (crate )const SPDM_REQ :u8 =0x80 ; -#[allow(dead_code)] pub (crate )const SPDM_ERROR :u8 =0x7f ; +pub (crate )const SPDM_GET_VERSION :u8 =0x84 ; +pub (crate )const SPDM_GET_VERSION_LEN :usize = + mem::size_of ::<SpdmHeader >() +2 + (u8::MAX as usize ) * mem::size_of ::<u16 >(); +pub (crate )const SPDM_MIN_DATA_TRANSFER_SIZE :u32 =42 ;// SPDM 1.2.0 margin no 226
lib/rspdm/lib.rs +24 -1
@@ -244,7 +244,30 @@ fn from(code: SpdmErrorCode) -> Self {/// `state_ptr` must be a valid pointer returned by a previous call to /// `spdm_create()` that has not yet been passed to `spdm_destroy()`. #[export] -pub unsafe extern "C" fn spdm_authenticate (_state_ptr: *mut spdm_state ) ->c_int { +pub unsafe extern "C" fn spdm_authenticate (state_ptr: *mut spdm_state ) ->c_int { +if state_ptr.is_null () { +return -(bindings::EINVAL as c_int ); + } + +// SAFETY: `state_ptr` was returned from `spdm_create()` which used `into_foreign()` +// to create the pointer, and it remains valid until `spdm_destroy()` is called. +// We only borrow here (rather than `from_foreign()`) so that ownership stays +// with the foreign (C) caller. +// The exclusive `&mut SpdmState` lives entirely inside the lock guard, so +// concurrent FFI callers serialize on the mutex and can never form +// aliased `&mut SpdmState` references. +let mutex =unsafe { + <Pin <KBox <Mutex <SpdmState <CTransport >>>>as ForeignOwnable >::borrow ( + state_ptr.cast ::<c_void >(), + ) + }; + +let mut state = mutex.lock (); + +if let Err (e) = state.get_version () { +return e.to_errno ()as c_int ; + } + -(EPROTONOSUPPORT as i32 ) }
lib/rspdm/state.rs +49 -1
70 lines not on this page · raw message
lib/rspdm/validator.rs +88 -2
116 lines not on this page · raw message
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v6 14/25] lib: rspdm: Support SPDM get_version
Sashiko has reviewed this patch and found no issues.
Quoted 5 lines
From: Alistair Francis <[email protected]>
Support the
GET_VERSIONSPDM command.Signed-off-by Alistair Francis <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=14
PATCH v6 15/25 lib: rspdm: Support SPDM get_capabilities
In reply to [email protected]From: Alistair Francis <[email protected]>
Support the GET_CAPABILITIES SPDM command.
--- lib/rspdm/consts.rs | 22 ++- lib/rspdm/lib.rs | 4 + lib/rspdm/state.rs | 76 +++++++++- lib/rspdm/validator.rs | 318 ++++++++++++++++++++++++++++++++++++++++- 4 files changed, 415 insertions(+), 5 deletions(-)
lib/rspdm/consts.rs +20 -2
@@ -9,12 +9,14 @@use crate ::validator::SpdmHeader ;use core::mem; +use kernel::bits::{ + bit_u32, + bit_u8,// +};// SPDM versions supported by this implementation pub (crate )const SPDM_VER_10 :u8 =0x10 ; -#[expect(dead_code)] pub (crate )const SPDM_VER_11 :u8 =0x11 ; -#[expect(dead_code)] pub (crate )const SPDM_VER_12 :u8 =0x12 ;#[expect(dead_code)] pub (crate )const SPDM_VER_13 :u8 =0x13 ;
@@ -30,4 +32,20 @@pub (crate )const SPDM_GET_VERSION_LEN :usize = mem::size_of ::<SpdmHeader >() +2 + (u8::MAX as usize ) * mem::size_of ::<u16 >(); +pub (crate )const SPDM_GET_CAPABILITIES :u8 =0xe1 ; +pub (crate )const SPDM_CAP_SUPPORTED_ALGORITHMS :u8 =bit_u8 (0 );pub (crate )const SPDM_MIN_DATA_TRANSFER_SIZE :u32 =42 ;// SPDM 1.2.0 margin no 226 + +// SPDM cryptographic timeout of this implementation: +// Assume calculations may take up to 1 sec on a busy machine, which equals +// roughly 1 << 20. That's within the limits mandated for responders by CMA +// (1 << 23 usec, PCIe r6.2 sec 6.31.3) and DOE (1 sec, PCIe r6.2 sec 6.30.2). +// Used in GET_CAPABILITIES exchange. +pub (crate )const SPDM_CTEXPONENT :u8 =20 ; + +pub (crate )const SPDM_CERT_CAP :u32 =bit_u32 (1 ); +pub (crate )const SPDM_CHAL_CAP :u32 =bit_u32 (2 ); +pub (crate )const SPDM_MULTI_KEY_CAP :u32 =bit_u32 (26 ) |bit_u32 (27 ); + +pub (crate )const SPDM_REQ_CAPS :u32 =SPDM_CERT_CAP |SPDM_CHAL_CAP ; +pub (crate )const SPDM_RSP_MIN_CAPS :u32 =SPDM_CERT_CAP |SPDM_CHAL_CAP ;
lib/rspdm/lib.rs +4 -0
@@ -268,6 +268,10 @@ fn from(code: SpdmErrorCode) -> Self {return e.to_errno ()as c_int ; } +if let Err (e) = state.get_capabilities () { +return e.to_errno ()as c_int ; + } + -(EPROTONOSUPPORT as i32 ) }
lib/rspdm/state.rs +74 -2
109 lines not on this page · raw message
lib/rspdm/validator.rs +317 -1
328 lines not on this page · raw message
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v6 15/25] lib: rspdm: Support SPDM get_capabilities
Sashiko has reviewed this patch and found no issues.
Quoted 5 lines
From: Alistair Francis <[email protected]>
Support the
GET_CAPABILITIESSPDM command.Signed-off-by Alistair Francis <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=15
PATCH v6 16/25 rust: kernel: Add crypto hash bindings
In reply to [email protected]From: Alistair Francis <[email protected]>
Initial support for the kernel crypto hash bindings, which will be used by the SPDM library.
This is just a few functions that the Rust SPDM implementation will use in the next few commits.
--- lib/Kconfig | 1 + rust/kernel/hash.rs | 141 ++++++++++++++++++++++++++++++++++++++++++++ rust/kernel/lib.rs | 2 + 3 files changed, 144 insertions(+) create mode 100644 rust/kernel/hash.rs
lib/Kconfig +1 -0
@@ -593,6 +593,7 @@ config RSPDM depends on RUST select KEYS select CRYPTO + select CRYPTO_HASH2 select ASYMMETRIC_KEY_TYPE select ASYMMETRIC_PUBLIC_KEY_SUBTYPE select X509_CERTIFICATE_PARSER
rust/kernel/hash.rs new file +141 -0
141 lines not on this page · raw message
rust/kernel/lib.rs +2 -0
@@ -82,6 +82,8 @@pub mod fwctl;#[cfg( CONFIG_GPU_BUDDY = "y" )] pub mod gpu; +#[cfg( CONFIG_CRYPTO_HASH2 = "y" )] +pub mod hash;#[cfg( CONFIG_I2C = "y" )] pub mod i2c;pub mod id_pool;
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v6 16/25] rust: kernel: Add crypto hash bindings
Sashiko has reviewed this patch and found no issues.
Quoted 9 lines
From: Alistair Francis <[email protected]>
Initial support for the kernel crypto hash bindings, which will be used by the SPDM library.
This is just a few functions that the Rust SPDM implementation will use in the next few commits.
Signed-off-by Alistair Francis <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=16
PATCH v6 17/25 lib: rspdm: Support SPDM negotiate_algorithms
In reply to [email protected]From: Alistair Francis <[email protected]>
Support the NEGOTIATE_ALGORITHMS SPDM command.
--- lib/rspdm/consts.rs | 61 +++++++++++ lib/rspdm/lib.rs | 4 + lib/rspdm/state.rs | 169 +++++++++++++++++++++++++++++- lib/rspdm/validator.rs | 233 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 466 insertions(+), 1 deletion(-)
lib/rspdm/consts.rs +61 -0
@@ -45,7 +45,68 @@pub (crate )const SPDM_CERT_CAP :u32 =bit_u32 (1 );pub (crate )const SPDM_CHAL_CAP :u32 =bit_u32 (2 ); +pub (crate )const SPDM_MEAS_RESP_CAP :u32 =bit_u32 (3 ) |bit_u32 (4 ); +pub (crate )const SPDM_KEY_EX_CAP :u32 =bit_u32 (9 );pub (crate )const SPDM_MULTI_KEY_CAP :u32 =bit_u32 (26 ) |bit_u32 (27 );pub (crate )const SPDM_REQ_CAPS :u32 =SPDM_CERT_CAP |SPDM_CHAL_CAP ;pub (crate )const SPDM_RSP_MIN_CAPS :u32 =SPDM_CERT_CAP |SPDM_CHAL_CAP ; + +pub (crate )const SPDM_NEGOTIATE_ALGS :u8 =0xe3 ; + +pub (crate )const SPDM_MEAS_SPEC_DMTF :u8 =bit_u8 (0 ); + +#[cfg( CONFIG_CRYPTO_RSA )] +pub (crate )const SPDM_ASYM_RSASSA_2048 :u32 =bit_u32 (0 ); +pub (crate )const _SPDM_ASYM_RSAPSS_2048:u32 =bit_u32 (1 ); +#[cfg( CONFIG_CRYPTO_RSA )] +pub (crate )const SPDM_ASYM_RSASSA_3072 :u32 =bit_u32 (2 ); +pub (crate )const _SPDM_ASYM_RSAPSS_3072:u32 =bit_u32 (3 ); +#[cfg( CONFIG_CRYPTO_ECDSA )] +pub (crate )const SPDM_ASYM_ECDSA_ECC_NIST_P256 :u32 =bit_u32 (4 ); +#[cfg( CONFIG_CRYPTO_RSA )] +pub (crate )const SPDM_ASYM_RSASSA_4096 :u32 =bit_u32 (5 ); +pub (crate )const _SPDM_ASYM_RSAPSS_4096:u32 =bit_u32 (6 ); +#[cfg( CONFIG_CRYPTO_ECDSA )] +pub (crate )const SPDM_ASYM_ECDSA_ECC_NIST_P384 :u32 =bit_u32 (7 ); +#[cfg( CONFIG_CRYPTO_ECDSA )] +pub (crate )const SPDM_ASYM_ECDSA_ECC_NIST_P521 :u32 =bit_u32 (8 ); +pub (crate )const _SPDM_ASYM_SM2_ECC_SM2_P256:u32 =bit_u32 (9 ); +pub (crate )const _SPDM_ASYM_EDDSA_ED25519:u32 =bit_u32 (10 ); +pub (crate )const _SPDM_ASYM_EDDSA_ED448:u32 =bit_u32 (11 ); + +#[cfg( CONFIG_CRYPTO_SHA256 )] +pub (crate )const SPDM_HASH_SHA_256 :u32 =bit_u32 (0 ); +#[cfg( CONFIG_CRYPTO_SHA512 )] +pub (crate )const SPDM_HASH_SHA_384 :u32 =bit_u32 (1 ); +#[cfg( CONFIG_CRYPTO_SHA512 )] +pub (crate )const SPDM_HASH_SHA_512 :u32 =bit_u32 (2 ); + +// If the crypto support isn't enabled don't offer the algorithms +// to the responder +#[cfg( CONFIG_CRYPTO_RSA )] +pub (crate )const SPDM_ASYM_RSA :u32 = +SPDM_ASYM_RSASSA_2048 |SPDM_ASYM_RSASSA_3072 |SPDM_ASYM_RSASSA_4096 ; +#[cfg(not( CONFIG_CRYPTO_RSA ))] +pub (crate )const SPDM_ASYM_RSA :u32 =0 ; + +#[cfg( CONFIG_CRYPTO_ECDSA )] +pub (crate )const SPDM_ASYM_ECDSA :u32 = +SPDM_ASYM_ECDSA_ECC_NIST_P256 |SPDM_ASYM_ECDSA_ECC_NIST_P384 |SPDM_ASYM_ECDSA_ECC_NIST_P521 ; +#[cfg(not( CONFIG_CRYPTO_ECDSA ))] +pub (crate )const SPDM_ASYM_ECDSA :u32 =0 ; + +#[cfg( CONFIG_CRYPTO_SHA256 )] +pub (crate )const SPDM_HASH_SHA2_256 :u32 =SPDM_HASH_SHA_256 ; +#[cfg(not( CONFIG_CRYPTO_SHA256 ))] +pub (crate )const SPDM_HASH_SHA2_256 :u32 =0 ; + +#[cfg( CONFIG_CRYPTO_SHA512 )] +pub (crate )const SPDM_HASH_SHA2_384_512 :u32 =SPDM_HASH_SHA_384 |SPDM_HASH_SHA_512 ; +#[cfg(not( CONFIG_CRYPTO_SHA512 ))] +pub (crate )const SPDM_HASH_SHA2_384_512 :u32 =0 ; + +pub (crate )const SPDM_ASYM_ALGOS :u32 =SPDM_ASYM_RSA |SPDM_ASYM_ECDSA ; +pub (crate )const SPDM_HASH_ALGOS :u32 =SPDM_HASH_SHA2_256 |SPDM_HASH_SHA2_384_512 ; + +pub (crate )const SPDM_OPAQUE_DATA_FMT_GENERAL :u8 =bit_u8 (1 );
lib/rspdm/lib.rs +4 -0
@@ -272,6 +272,10 @@ fn from(code: SpdmErrorCode) -> Self {return e.to_errno ()as c_int ; } +if let Err (e) = state.negotiate_algs () { +return e.to_errno ()as c_int ; + } + -(EPROTONOSUPPORT as i32 ) }
lib/rspdm/state.rs +168 -1
215 lines not on this page · raw message
lib/rspdm/validator.rs +233 -0
248 lines not on this page · raw message
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v6 17/25] lib: rspdm: Support SPDM negotiate_algorithms
Thank you for your contribution! Sashiko AI review found 2 potential issues to consider: - [Medium] lib: rspdm: build failure in update_response_algs() without crypto Kconfigs - [Low] lib: rspdm…
Thank you for your contribution! Sashiko AI review found 2 potential issues to consider:
- [Medium] lib: rspdm: build failure in update_response_algs() without crypto Kconfigs
- [Low] lib: rspdm: unreachable extended algorithm loops in NegotiateAlgsRsp::validate()
--
commit 69781025c793b90efaa465da8436c6f48e861229
Author: Alistair Francis <[email protected]>
lib: rspdm: Support SPDM negotiate_algorithms
This commit adds support for the SPDM NEGOTIATE_ALGORITHMS command, including request construction, response validation, and algorithm state selection.
Quoted 4 lines
lib/rspdm/state.rs +0
-0
[ ... ]
Quoted 62 lines
lib/rspdm/state.rs +58
-0@@ -265,4 +323,113 @@ pub(crate) fn get_capabilities(&mut self) -> Result<(), Error> {Ok (()) } + +fn update_response_algs (&mut self) ->Result <(),Error > { +match self.base_asym_alg { +#[cfg( CONFIG_CRYPTO_RSA )] +SPDM_ASYM_RSASSA_2048 => { + self.sig_len =256 ; + self.base_asym_enc =c"pkcs1" ; + } +#[cfg( CONFIG_CRYPTO_RSA )] +SPDM_ASYM_RSASSA_3072 => { + self.sig_len =384 ; + self.base_asym_enc =c"pkcs1" ; + } +#[cfg( CONFIG_CRYPTO_RSA )] +SPDM_ASYM_RSASSA_4096 => { + self.sig_len =512 ; + self.base_asym_enc =c"pkcs1" ; + } +#[cfg( CONFIG_CRYPTO_ECDSA )] +SPDM_ASYM_ECDSA_ECC_NIST_P256 => { + self.sig_len =64 ; + self.base_asym_enc =c"p1363" ; + } +#[cfg( CONFIG_CRYPTO_ECDSA )] +SPDM_ASYM_ECDSA_ECC_NIST_P384 => { + self.sig_len =96 ; + self.base_asym_enc =c"p1363" ; + } +#[cfg( CONFIG_CRYPTO_ECDSA )] +SPDM_ASYM_ECDSA_ECC_NIST_P521 => { + self.sig_len =132 ; + self.base_asym_enc =c"p1363" ; + } + _ => { +dev_err! (&self.dev,"Unknown asym algorithm\n" ); +return Err (EPROTO ); + } + } + +let hash_name =match self.base_hash_alg { +#[cfg( CONFIG_CRYPTO_SHA256 )] +SPDM_HASH_SHA_256 =>c"sha256" , +#[cfg( CONFIG_CRYPTO_SHA512 )] +SPDM_HASH_SHA_384 =>c"sha384" , +#[cfg( CONFIG_CRYPTO_SHA512 )] +SPDM_HASH_SHA_512 =>c"sha512" , + _ => { +dev_err! (&self.dev,"Unknown hash algorithm\n" ); +return Err (EPROTO ); + } + }; + + self.crypto =Some (Shash ::new (hash_name)?); + +Ok (()) + } + +pub (crate )fn negotiate_algs (&mut self) ->Result <(),Error > {
[ ... ]
lib/rspdm/state.rs pub(crate) fn get_capabilities(&mut self) -> Result<(), Error> { · line 412
Quoted 8 lines
lib/rspdm/state.rs +8
-0+ self.base_asym_alg = response.base_asym_sel; + self.base_hash_alg = response.base_hash_sel; + self.meas_hash_alg = response.measurement_hash_algo; + +if self.base_asym_alg &SPDM_ASYM_ALGOS ==0 || self.base_hash_alg &SPDM_HASH_ALGOS ==0 { +dev_err! (&self.dev,"No common supported algorithms\n" ); +return Err (EPROTO ); + }
[Severity: Medium]
Does this fail to build when CONFIG_RSPDM=y is enabled without any asymmetric crypto option (CONFIG_CRYPTO_RSA and CONFIG_CRYPTO_ECDSA both disabled) or without any hash option (CONFIG_CRYPTO_SHA256 and CONFIG_CRYPTO_SHA512 both disabled)?
When both options in either pair are disabled, the corresponding match in update_response_algs() collapses to the diverging wildcard arm alone, which triggers rustc unreachable_code and unused_variables errors as well as clippy::match_single_binding. In addition, SPDM_ASYM_ALGOS or SPDM_HASH_ALGOS evaluates to 0, so the bitmask check in negotiate_algs() triggers clippy::bad_bit_mask.
Quoted 4 lines
lib/rspdm/validator.rs +0
-0
[ ... ]
Quoted 46 lines
lib/rspdm/validator.rs +46
-0+// We currently never request extended algorithms or algorithm structures +if header.param1 !=0 || ext_asym_sel_count !=0 || ext_hash_sel_count !=0 { +dev_err! ( + &context.dev, +"Responder selected unsupported extended algorithms\n" + ); +return Err (EPROTO ); + } + +let mut offset =NEGOTIATE_ALGS_RSP_SZ ; + +let mut ext_asym =KVec ::new (); +for _in 0 ..ext_asym_sel_count { + ext_asym.push (read_le32 (offset)?,GFP_KERNEL )?; + offset += mem::size_of ::<u32 >(); + } + +let mut ext_hash =KVec ::new (); +for _in 0 ..ext_hash_sel_count { + ext_hash.push (read_le32 (offset)?,GFP_KERNEL )?; + offset += mem::size_of ::<u32 >(); + } + +let mut resp_alg_struct =KVec ::new (); +for _in 0 ..header.param1 { +let alg_type = *unvalidated.get (offset).ok_or (EPROTO )?; +let alg_count = *unvalidated.get (offset +1 ).ok_or (EPROTO )?; +let fixed_alg_count = (alg_count >>4 )as usize ; +let ext_alg_count = (alg_count &0xf )as usize ; + offset +=2 ; + +let mut alg_supported =KVec ::new (); + alg_supported.extend_from_slice ( + unvalidated + .get (offset..offset + fixed_alg_count) + .ok_or (EPROTO )?, +GFP_KERNEL , + )?; + offset += fixed_alg_count; + +let mut alg_external =KVec ::new (); +for _in 0 ..ext_alg_count { + alg_external.push (read_le32 (offset)?,GFP_KERNEL )?; + offset += mem::size_of ::<u32 >(); + } +
lib/rspdm/validator.rs fn validate(unvalidated: KVec<u8>, context: &'c SpdmState<T>) -> Result<Self, Se · line 703
Quoted 10 lines
lib/rspdm/validator.rs +10
-0+ resp_alg_struct.push ( +RespAlgStruct { + alg_type, + alg_count, + alg_supported, + alg_external, + }, +GFP_KERNEL , + )?; + }
[Severity: Low]
This isn't a bug, but since NegotiateAlgsRsp::validate() returns Err(EPROTO) above whenever header.param1, ext_asym_sel_count, or ext_hash_sel_count is non-zero, the three loops that follow can never execute (and duplicate the parsing in SupportedAlgorithms::from_bytes()).
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=17
PATCH v6 18/25 lib: rspdm: Support SPDM get_digests
In reply to [email protected]From: Alistair Francis <[email protected]>
Support the GET_DIGESTS SPDM command.
--- lib/rspdm/consts.rs | 5 +- lib/rspdm/lib.rs | 4 ++ lib/rspdm/state.rs | 70 +++++++++++++++++++++++++++- lib/rspdm/validator.rs | 101 ++++++++++++++++++++++++++++++++++++++++- 4 files changed, 177 insertions(+), 3 deletions(-)
lib/rspdm/consts.rs +4 -1
@@ -18,10 +18,11 @@pub (crate )const SPDM_VER_10 :u8 =0x10 ;pub (crate )const SPDM_VER_11 :u8 =0x11 ;pub (crate )const SPDM_VER_12 :u8 =0x12 ; -#[expect(dead_code)] pub (crate )const SPDM_VER_13 :u8 =0x13 ;pub (crate )const SPDM_VER_14 :u8 =0x14 ; +pub (crate )const SPDM_SLOTS :usize =8 ; +pub (crate )const SPDM_MIN_VER :u8 =SPDM_VER_10 ;pub (crate )const SPDM_MAX_VER :u8 =SPDM_VER_14 ;
@@ -82,6 +83,8 @@#[cfg( CONFIG_CRYPTO_SHA512 )] pub (crate )const SPDM_HASH_SHA_512 :u32 =bit_u32 (2 ); +pub (crate )const SPDM_GET_DIGESTS :u8 =0x81 ; +// If the crypto support isn't enabled don't offer the algorithms // to the responder #[cfg( CONFIG_CRYPTO_RSA )]
lib/rspdm/lib.rs +4 -0
@@ -276,6 +276,10 @@ fn from(code: SpdmErrorCode) -> Self {return e.to_errno ()as c_int ; } +if let Err (e) = state.get_digests () { +return e.to_errno ()as c_int ; + } + -(EPROTONOSUPPORT as i32 ) }
lib/rspdm/state.rs +69 -1
118 lines not on this page · raw message
lib/rspdm/validator.rs +100 -1
118 lines not on this page · raw message
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v6 18/25] lib: rspdm: Support SPDM get_digests
Sashiko has reviewed this patch and found no issues.
Quoted 5 lines
From: Alistair Francis <[email protected]>
Support the
GET_DIGESTSSPDM command.Signed-off-by Alistair Francis <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=18
PATCH v6 19/25 lib: rspdm: Support SPDM get_certificate
In reply to [email protected]From: Alistair Francis <[email protected]>
Support the GET_CERTIFICATE SPDM command.
The kernel will send a GET_CERTIFICATE request to the responder and then iterate over all of the certificates returned.
Certificate validation happens in the next commit.
--- lib/rspdm/consts.rs | 2 + lib/rspdm/lib.rs | 15 ++++ lib/rspdm/state.rs | 100 +++++++++++++++++++++++- lib/rspdm/validator.rs | 170 ++++++++++++++++++++++++++++++++++++++++- 4 files changed, 285 insertions(+), 2 deletions(-)
lib/rspdm/consts.rs +2 -0
@@ -85,6 +85,8 @@pub (crate )const SPDM_GET_DIGESTS :u8 =0x81 ; +pub (crate )const SPDM_GET_CERTIFICATE :u8 =0x82 ; +// If the crypto support isn't enabled don't offer the algorithms // to the responder #[cfg( CONFIG_CRYPTO_RSA )]
lib/rspdm/lib.rs +15 -0
@@ -280,6 +280,21 @@ fn from(code: SpdmErrorCode) -> Self {return e.to_errno ()as c_int ; } +if state.provisioned_slots ==0 { +return -(bindings::EIO as c_int ); + } + +let mut provisioned_slots = state.provisioned_slots; +while (provisioned_slotsas usize ) >0 { +let slot = provisioned_slots.trailing_zeros ()as u8 ; + +if let Err (e) = state.get_certificate (slot) { +return e.to_errno ()as c_int ; + } + + provisioned_slots &= !(1 << slot); + } + -(EPROTONOSUPPORT as i32 ) }
lib/rspdm/state.rs +99 -1
119 lines not on this page · raw message
lib/rspdm/validator.rs +169 -1
185 lines not on this page · raw message
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v6 19/25] lib: rspdm: Support SPDM get_certificate
Thank you for your contribution! Sashiko AI review found 2 potential issues to consider: - [High] lib: rspdm: u16 length truncation in get_certificate() on 64K+ page kernels - [Low] lib: rspdm…
Thank you for your contribution! Sashiko AI review found 2 potential issues to consider:
- [High] lib: rspdm: u16 length truncation in get_certificate() on 64K+ page kernels
- [Low] lib: rspdm: duplicated initial exchange and missing check in get_certificate()
--
commit af8f6804281e40cfc39810cbc243efa8a576944b
Author: Alistair Francis <[email protected]>
lib: rspdm: Support SPDM get_certificate
This commit adds support for the SPDM GET_CERTIFICATE command to retrieve and store certificate chains for each provisioned slot from the responder.
Quoted 4 lines
lib/rspdm/state.rs +0
-0
[ ... ]
Quoted 5 lines
lib/rspdm/state.rs +1
-0@@ -500,4 +505,97 @@ pub(crate) fn get_digests(&mut self) -> Result<(), Error> {Ok (()) } +
lib/rspdm/state.rs pub(crate) fn get_digests(&mut self) -> Result<(), Error> { · line 518
Quoted 10 lines
lib/rspdm/state.rs +10
-0+pub (crate )fn get_certificate (&mut self, slot:u8 ) ->Result <(),Error > { +let mut request =GetCertificateReq ::default (); + request.header.version = self.version; + request.header.param1 = slot; + +// Bound the per-chunk allocation to one page +let rsp_sz = (GET_CERTIFICATE_RSP_HDR_SZ +PAGE_SIZE ).min (self.data_transfer_szas usize ); + + request.offset =0 ; + request.length = (rsp_sz -GET_CERTIFICATE_RSP_HDR_SZ )as u16 ;
[Severity: High]
Does casting rsp_sz - GET_CERTIFICATE_RSP_HDR_SZ to u16 truncate request.length to 0 on 64K and 256K page kernels?
When self.data_transfer_sz is at least GET_CERTIFICATE_RSP_HDR_SZ + PAGE_SIZE (such as with PCI_DOE_MAX_PAYLOAD), rsp_sz - GET_CERTIFICATE_RSP_HDR_SZ equals PAGE_SIZE (0x10000 on 64K-page kernels), which wraps to 0 when cast to u16 and causes certificate retrieval and device authentication to fail. Adding GET_CERTIFICATE_RSP_HDR_SZ to PAGE_SIZE also makes rsp_sz equal PAGE_SIZE + 8, so KVec::from_elem() allocates two pages instead of one.
Bounding rsp_sz to PAGE_SIZE.min(self.data_transfer_sz as usize) and clamping the requested length to u16::MAX avoids the truncation and keeps the buffer within a single page.
Quoted 41 lines
lib/rspdm/state.rs +41
-0+ +let response_vec:KVec <u8 > =KVec ::from_elem (0u8 , rsp_sz,GFP_KERNEL )?; + +let mut request_buf = request.to_bytes ()?; +let response:GetCertificateRsp = self + .spdm_exchange (request_buf.as_mut_slice (), response_vec)? + .validate (&*self)?; + +if (response.header.param1 &0xF ) != slot { +dev_err! (&self.dev,"Invalid slot response\n" ); +return Err (EPROTO ); + } + +let portion_length = response.portion_length; +let rem_length = response.remainder_length; + +let total_cert_len = portion_lengthas usize + rem_lengthas usize ; + +let mut certs_buf:KVec <u8 > =KVec ::new (); + + certs_buf.extend_from_slice (&response.cert_chain,GFP_KERNEL )?; + +let mut offset:u16 = portion_length; +let mut remainder_length = rem_lengthas usize ; + +while remainder_length >0 { + request.offset = offset; + request.length = (remainder_length.min (rsp_sz -GET_CERTIFICATE_RSP_HDR_SZ ))as u16 ; + +let mut request_buf = request.to_bytes ()?; + +let response_vec:KVec <u8 > =KVec ::from_elem ( +0u8 , + request.lengthas usize +GET_CERTIFICATE_RSP_HDR_SZ , +GFP_KERNEL , + )?; + +let response:GetCertificateRsp = self + .spdm_exchange (request_buf.as_mut_slice (), response_vec)? + .validate (&*self)?; +
lib/rspdm/state.rs pub(crate) fn get_digests(&mut self) -> Result<(), Error> { · line 569
Quoted 10 lines
lib/rspdm/state.rs +10
-0+let portion_length = response.portion_length; +let rem_length = response.remainder_length; + +if portion_length ==0 + || (response.header.param1 &0xF ) != slot + || offsetas usize + portion_lengthas usize + rem_lengthas usize != total_cert_len + { +dev_err! (&self.dev,"Malformed certificate response\n" ); +return Err (EPROTO ); + }
[Severity: Low]
Can the initial exchange before the while loop be folded into the loop so that the first response also checks portion_length == 0 and avoids duplicating the request/response sequence? Right now, if a responder returns portion_length == 0 with a non-zero remainder_length on the first chunk, or a total_cert_len exceeding u16::MAX, get_certificate() performs an extra exchange before failing.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=19
PATCH v6 20/25 rust: public_key: Add crypto public key bindings
In reply to [email protected]From: Alistair Francis <[email protected]>
Initial support for the kernel crypto public key (asymmetric-keys) bindings, which will be used by the SPDM library.
This is just a few functions that the Rust SPDM implementation will use in the next few commits.
--- rust/bindings/bindings_helper.h | 1 + rust/kernel/lib.rs | 2 + rust/kernel/public_key.rs | 83 +++++++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+) create mode 100644 rust/kernel/public_key.rs
rust/bindings/bindings_helper.h +1 -0
@@ -29,6 +29,7 @@#include <linux/hrtimer_types.h> #include <crypto/hash.h> +#include <crypto/public_key.h> #include <linux/acpi.h> #include <linux/gpu_buddy.h> #include <drm/drm_device.h>
rust/kernel/lib.rs +2 -0
@@ -122,6 +122,8 @@pub mod print;pub mod processor;pub mod ptr; +#[cfg( CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE = "y" )] +pub mod public_key;#[cfg( CONFIG_RUST_PWM_ABSTRACTIONS )] pub mod pwm;pub mod ratelimit;
rust/kernel/public_key.rs new file +83 -0
83 lines not on this page · raw message
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v6 20/25] rust: public_key: Add crypto public key bindings
Thank you for your contribution! Sashiko AI review found 2 potential issues to consider: - [Low] rust: public_key: missing Invariants and incomplete Safety in from_raw() - [Low] rust: public_key…
Thank you for your contribution! Sashiko AI review found 2 potential issues to consider:
- [Low] rust: public_key: missing Invariants and incomplete Safety in from_raw()
- [Low] rust: public_key: split kernel import blocks violate vertical import style
--
commit 5a9f44bbde86bec3d5be61549a4f56d64bb30ac2
Author: Alistair Francis <[email protected]>
rust: public_key: Add crypto public key bindings
This commit adds Rust bindings and a PublicKey wrapper for the kernel crypto public key APIs, providing PublicKey::from_raw(), PublicKey::verify_signature(), and a Drop implementation.
Quoted 12 lines
rust/kernel/public_key.rs new file +6
-0@@ -0,0 +1,83 @@ +// SPDX-License-Identifier: GPL-2.0 + +// Copyright (C) 2026 Western Digital + +//! Wrappers around the Linux kernel crypto public key APIs. +//!
rust/kernel/public_key.rs line 16
Quoted 10 lines
rust/kernel/public_key.rs new file +10
-0+//! C headers: [`include/crypto/public_key.h`](srctree/include/crypto/public_key.h). +//! +//! Reference: <https://www.kernel.org/doc/html/latest/crypto/asymmetric-keys.html> + +use core::ptr::NonNull ; +use kernel::prelude::*; +use kernel::{ + bindings, + error::to_result,// +};
[Severity: Low]
This isn't a bug, but the two kernel crate imports can be combined into a single vertical import block per Documentation/rust/coding-guidelines.rst.
Quoted 23 lines
rust/kernel/public_key.rs new file +23
-0+ +/// Owned `struct public_key` wrapper. +pub struct PublicKey { + key:NonNull <bindings::public_key >, +} + +impl PublicKey { +/// Create a Rust `PublicKey` from a valid C `struct public_key` pointer. +/// +/// # Safety +/// +/// `key` must point to a valid, exclusively-owned `struct public_key` +/// whose ownership is transferred to the returned `PublicKey`. The key +/// is freed with `public_key_free()` when the `PublicKey` is dropped, +/// so the caller must not free it itself or pass the same pointer to +/// `from_raw()` again. +#[inline] +pub unsafe fn from_raw (key:NonNull <bindings::public_key >) ->Self { +Self { key } + } + +/// Verify a signature using this public key. +#[inline]
rust/kernel/public_key.rs line 49
Quoted 10 lines
rust/kernel/public_key.rs new file +10
-0+pub fn verify_signature ( + &self, + signature: &[u8 ], + encoding: &'staticCStr , + hash_algo: &'staticCStr , + message: &[u8 ], + ) ->Result { +// SAFETY: `self.key` is valid per the struct invariants; its +// `pkey_algo` is populated by the key parser. +let pkey_algo =unsafe { (*self.key.as_ptr ()).pkey_algo };
[Severity: Low]
Should PublicKey document an # Invariants section and PublicKey::from_raw() require those invariants in its # Safety section (with a matching // INVARIANT: comment in from_raw())?
verify_signature() cites struct invariants for self.key and pkey_algo that are not documented on PublicKey, and public_key_verify_signature() and public_key_free() also require pkey_algo to be a valid NUL-terminated string and key (keylen bytes), params (paramlen bytes), and the struct public_key itself to be valid kmalloc allocations.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=20
PATCH v6 21/25 keys: Guard asymmetric-type.h behind CONFIG_KEYS
In reply to [email protected]From: Alistair Francis <[email protected]>
The header keys/asymmetric-type.h includes linux/key-type.h, which is where struct key_type is defined.
linux/key-type.h has a #ifdef CONFIG_KEYS guard around the entire file, yet keys/asymmetric-type.h doesn't. So if CONFIG_KEYS is not defined including keys/asymmetric-type.h will cause build failures as struct key_type isn't defined.
Guard keys/asymmetric-type.h with a #ifdef CONFIG_KEYS to fix the failures seen when including keys/asymmetric-type.h in the Rust bindgen.
--- include/keys/asymmetric-type.h | 3 +++ 1 file changed, 3 insertions(+)
include/keys/asymmetric-type.h +3 -0
@@ -13,6 +13,8 @@#include <linux/key-type.h> #include <linux/verification.h> +#ifdef CONFIG_KEYS +extern struct key_type key_type_asymmetric;/*
@@ -93,4 +95,5 @@ int x509_load_certificate_list(const u8 cert_list[], const unsigned long list_si* The payload is at the discretion of the subtype. */ +#endif /* CONFIG_KEYS */ #endif /* _KEYS_ASYMMETRIC_TYPE_H */
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v6 21/25] keys: Guard asymmetric-type.h behind CONFIG_KEYS
Sashiko has reviewed this patch and found no issues.
Quoted 10 lines
From: Alistair Francis <[email protected]>
The header
keys/asymmetric-type.hincludeslinux/key-type.h, which is wherestruct key_typeis defined.
linux/key-type.hhas a#ifdef CONFIG_KEYSguard around the entire file, yetkeys/asymmetric-type.hdoesn't. So ifCONFIG_KEYSis not defined includingkeys/asymmetric-type.hwill cause build failures asstruct key_typeisn't defined.
Quoted 4 lines
Guard
keys/asymmetric-type.hwith a#ifdef CONFIG_KEYSto fix the failures seen when includingkeys/asymmetric-type.hin the Rust bindgen.Signed-off-by Alistair Francis <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=21
PATCH v6 22/25 rust: x509: Add X509 certificate bindings
In reply to [email protected]From: Alistair Francis <[email protected]>
Initial support for the kernel x509 parser bindings, which will be used by the SPDM library.
This is just a few functions that the Rust SPDM implementation will use in the next few commits.
--- rust/bindings/bindings_helper.h | 2 + rust/kernel/lib.rs | 2 + rust/kernel/x509.rs | 236 ++++++++++++++++++++++++++++++++ 3 files changed, 240 insertions(+) create mode 100644 rust/kernel/x509.rs
rust/bindings/bindings_helper.h +2 -0
@@ -39,6 +39,8 @@#include <drm/drm_gem_shmem_helper.h> #include <drm/drm_gpuvm.h> #include <drm/drm_ioctl.h> +#include <keys/asymmetric-type.h> +#include <keys/x509-parser.h> #include <kunit/test.h> #include <linux/auxiliary_bus.h> #include <linux/bitmap.h>
rust/kernel/lib.rs +2 -0
@@ -153,6 +153,8 @@pub mod usb;pub mod validate;pub mod workqueue; +#[cfg( CONFIG_X509_CERTIFICATE_PARSER = "y" )] +pub mod x509;pub mod xarray;#[doc(hidden)]
rust/kernel/x509.rs new file +236 -0
236 lines not on this page · raw message
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v6 22/25] rust: x509: Add X509 certificate bindings
Thank you for your contribution! Sashiko AI review found 4 potential issues to consider: - [Medium] rust: x509: missing unsupported_sig check in Certificate::verify_signed() - [Medium] rust: x509…
Thank you for your contribution! Sashiko AI review found 4 potential issues to consider:
- [Medium] rust: x509: missing unsupported_sig check in Certificate::verify_signed()
- [Medium] rust: x509: pkey_len() returns ASN.1 DER sequence size instead of RSA modulus
- [Low] rust: x509: missing inline attribute on small methods in x509.rs
- [Low] rust: x509: imports do not follow vertical import grouping style --
commit b7f2b997e084d89e8ffc231afb5c218c305c3ae1
Author: Alistair Francis <[email protected]>
rust: x509: Add X509 certificate bindings
This commit adds Rust wrappers around the kernel X.509 certificate parser APIs for parsing DER certificates, inspecting their fields, and verifying signatures.
Quoted 18 lines
rust/kernel/x509.rs new file +12
-0@@ -0,0 +1,236 @@ +// SPDX-License-Identifier: GPL-2.0 + +// Copyright (C) 2026 Western Digital + +//! Wrappers around the Linux kernel X509 parser APIs. +//! +//! C headers: [`include/keys/x509-parser.h`](srctree/include/keys/x509-parser.h). + +use core::marker::PhantomData ; +use core::ptr::{ + self, +NonNull ,//
rust/kernel/x509.rs line 22
Quoted 10 lines
rust/kernel/x509.rs new file +10
-0+}; +use kernel::prelude::*; +use kernel::{ + bindings, + error::{ + from_err_ptr, + to_result,// + }, + public_key::PublicKey , +};
[Severity: Low]
This isn't a bug, but the imports from core and kernel are split across separate use statements instead of being grouped into single vertical import blocks per the Rust coding guidelines.
[ ... ]
Quoted 14 lines
rust/kernel/x509.rs new file +14
-0+impl KeyExtFlags { +/// Whether the CA basic constraint is asserted. +pub fn is_ca (self) ->bool { + self.0 & (1 << bindings::KEY_EFLAG_CA ) !=0 + } + +/// Whether the keyCertSign key usage is asserted. +pub fn key_cert_sign (self) ->bool { + self.0 & (1 << bindings::KEY_EFLAG_KEYCERTSIGN ) !=0 + } + +/// Whether the digitalSignature key usage is asserted. +pub fn digital_signature (self) ->bool { + self.0 & (1 << bindings::KEY_EFLAG_DIGITALSIG ) !=0
rust/kernel/x509.rs line 59
Quoted 10 lines
rust/kernel/x509.rs new file +10
-0+ } +} + +impl <'a>Certificate <'a> { +/// Parses a DER certificate from `data` and returns it with the length +/// of the certificate in bytes. +/// +/// The returned certificate borrows `data` (`x509_cert_parse()` keeps +/// pointers into the DER buffer), so `data` must outlive it. +pub fn parse (data: &'a [u8 ]) ->Result <(Self ,usize )> {
[Severity: Low]
This isn't a bug, but in the kernel crate, small functions and functions that forward to C bindings should be marked #[inline]; KeyExtFlags::is_ca(), KeyExtFlags::key_cert_sign(), KeyExtFlags::digital_signature(), and Certificate::parse() omit it.
[ ... ]
rust/kernel/x509.rs line 169
Quoted 11 lines
rust/kernel/x509.rs new file +11
-0+/// Length of the raw public key (in bytes). For RSA keys this is the +/// modulus length; for ECDSA keys the uncompressed public key encoding. +#[inline] +pub fn pkey_len (&self) ->Result <usize > { +// SAFETY: `self.cert` is valid per the struct invariants. +let pub_key =unsafe { (*self.cert.as_ptr ()).pub_ }; +let pub_key =NonNull ::new (pub_key).ok_or (EINVAL )?; + +// SAFETY: `pub_key` is valid and non-NULL as checked above. +Ok (unsafe { (*pub_key.as_ptr ()).keylen }as usize ) + }
[Severity: Medium]
Does Certificate::pkey_len() return the ASN.1 DER sequence size instead of the modulus length for RSA keys?
In crypto/asymmetric_keys/x509_cert_parser.c, x509_extract_key_data() sets cert->pub->keylen to the entire SubjectPublicKeyInfo BIT STRING payload, which for RSA is the DER-encoded RSAPublicKey SEQUENCE (including ASN.1 headers and the public exponent, e.g. ~270 bytes for a 2048-bit key rather than 256). Callers using pkey_len() to check RSA key sizes as documented will get the wrong length.
[ ... ]
Quoted 9 lines
rust/kernel/x509.rs new file +9
-0+/// Verify that `child`'s signature is signed by this cert's public/private key. +#[inline] +pub fn verify_signed (&self, child: &Self ) ->Result { +// SAFETY: both certificate pointers were produced by `x509_cert_parse()` +// and remain valid until dropped. +let (prev, child) =unsafe { (&*self.cert.as_ptr (), &*child.cert.as_ptr ()) }; + +let prev_pub =NonNull ::new (prev.pub_).ok_or (EINVAL )?; +let child_sig =NonNull ::new (child.sig).ok_or (EINVAL )?;
rust/kernel/x509.rs line 203
Quoted 10 lines
rust/kernel/x509.rs new file +10
-0+ +// SAFETY: `prev_pub` and `child_sig` are valid for at least the duration +// of this call. +unsafe { +to_result (bindings::public_key_verify_signature ( + prev_pub.as_ptr (), + child_sig.as_ptr (), + )) + } + }
[Severity: Medium]
Can Certificate::verify_signed() pass a NULL digest to public_key_verify_signature() when child.unsupported_sig is set?
When the signature hash algorithm is unavailable (-ENOENT), x509_get_sig_params() sets cert->unsupported_sig = true and returns 0 without allocating cert->sig->m:
crypto/asymmetric_keys/x509_public_key.c:x509_get_sig_params() {
...
tfm = crypto_alloc_shash(sig->hash_algo, 0, 0);
if (IS_ERR(tfm)) {
if (PTR_ERR(tfm) == -ENOENT) {
cert->unsupported_sig = true;
return 0;
}
return PTR_ERR(tfm);
}
...
}x509_cert_parse() then returns a certificate where child.sig is non-NULL but child.sig->m is NULL and child.sig->m_size is 0. If a caller invokes verify_signed() without first calling check_accepted(), an ECDSA issuer key reaches ecdsa_verify(), which clamps bufsize to dlen (0) and zero-fills the hash via ecc_digits_from_bytes(), verifying the signature against an all-zero digest. Checking child.unsupported_sig (or child_sig->m) in verify_signed() would prevent this.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=22
PATCH v6 23/25 lib: rspdm: Support SPDM certificate validation
In reply to [email protected]From: Alistair Francis <[email protected]>
Support validating the SPDM certificate chain. This only performs basic sanity checks on the chain before we continue on. This does not ensure that the root CA is trusted, we leave that for userspace to check and enforce. Instead we just make sure that the chain is correct, uses supported signatures and that it isn't blacklisted in the kernel.
We then store the first leaf certificate for use later.
--- lib/rspdm/lib.rs | 12 ++++ lib/rspdm/state.rs | 172 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 181 insertions(+), 3 deletions(-)
lib/rspdm/lib.rs +12 -0
@@ -295,6 +295,18 @@ fn from(code: SpdmErrorCode) -> Self { provisioned_slots &= !(1 << slot); } +let mut provisioned_slots = state.provisioned_slots; +while (provisioned_slotsas usize ) >0 { +let slot = provisioned_slots.trailing_zeros ()as u8 ; + +if let Err (e) = state.validate_cert_chain (slot) { +pr_err! ("Certificate in slot {slot} failed to verify: {e:?}\n" ); +return e.to_errno ()as c_int ; + } + + provisioned_slots &= !(1 << slot); + } + -(EPROTONOSUPPORT as i32 ) }
lib/rspdm/state.rs +169 -3
215 lines not on this page · raw message
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v6 23/25] lib: rspdm: Support SPDM certificate validation
Thank you for your contribution! Sashiko AI review found 2 potential issues to consider: - [Medium] rspdm: premature take_public_key() before validate in validate_cert_chain() - [Medium] rspdm…
Thank you for your contribution! Sashiko AI review found 2 potential issues to consider:
- [Medium] rspdm: premature take_public_key() before validate in validate_cert_chain()
- [Medium] rspdm: missing leaf key algorithm and size check in validate_cert_chain()
--
commit 90c6956b70972b83efecdd912863a7121463499f
Author: Alistair Francis <[email protected]>
lib: rspdm: Support SPDM certificate validation
This commit adds certificate chain digest verification, X.509 chain signature validation, and leaf public key extraction to the Rust SPDM implementation.
Quoted 4 lines
lib/rspdm/state.rs +0
-0
[ ... ]
Quoted 1 line
lib/rspdm/state.rs +0
-0@@ -593,9 +604,164 @@ pub(crate) fn get_certificate(&mut self, slot: u8) -> Result<(), Error> {
[ ... ]
lib/rspdm/state.rs pub(crate) fn get_certificate(&mut self, slot: u8) -> Result<(), Error> { · line 749
Quoted 9 lines
lib/rspdm/state.rs +9
-0+let Some (mut leaf) = prev_cert.take ()else { +return Err (EPROTO ); + }; + +let flags = leaf.key_ext_flags ()?; +if flags.is_ca () || !flags.digital_signature () { +dev_err! (&self.dev,"Invalid leaf certificate constraints\n" ); +return Err (EKEYREJECTED ); + }
[Severity: Medium]
Should validate_cert_chain() verify that leaf.pkey_algo() and leaf.pkey_len() match the session's negotiated self.base_asym_alg?
Without checking the leaf key's algorithm and size against self.base_asym_alg, a responder that negotiated a strong algorithm in NEGOTIATE_ALGORITHMS (for example ECDSA P-384 or RSA-4096) can supply a leaf certificate with a different or weaker key accepted by the X.509 parser, and validate_cert_chain() will accept the chain and store the mismatched key in self.leaf_key.
Quoted 8 lines
lib/rspdm/state.rs +8
-0+ +// The leaf key is the same for all slots (MULT_KEY_CAP == 0), +// so just store the first one. +if self.leaf_key.is_none () { + self.leaf_key = leaf.take_public_key (); + } + +if let Some (validate) = self.validate {
lib/rspdm/state.rs pub(crate) fn get_certificate(&mut self, slot: u8) -> Result<(), Error> { · line 767
Quoted 10 lines
lib/rspdm/state.rs +9
-0+// SAFETY: Call the `validate` function provided with a valid +// certificate pointer, which the callback must not modify or +// free. +let rc =unsafe {validate (self.dev.as_raw (), slot, leaf.as_raw ()) }; +to_result (rc)?; + } + +Ok (()) + } }
[Severity: Medium]
Does calling leaf.take_public_key() before invoking self.validate pass a struct x509_certificate with a NULL pub pointer to the callback on the first slot?
Certificate::take_public_key() transfers ownership of the key by clearing cert->pub to NULL in place:
rust/kernel/x509.rs:Certificate::take_public_key() {
...
unsafe {
(*self.cert.as_ptr()).pub_ = ptr::null_mut();
}
...
}As a result, on the first validated slot a validate callback that inspects leaf_cert->pub will dereference a NULL pointer, and if the callback rejects the certificate, self.leaf_key remains populated with the rejected certificate's key. Moving the self.leaf_key assignment after the self.validate check avoids both issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=23
PATCH v6 24/25 lib: rspdm: Support SPDM challenge
In reply to [email protected]From: Alistair Francis <[email protected]>
Support the CHALLENGE SPDM command.
--- lib/rspdm/consts.rs | 6 ++ lib/rspdm/lib.rs | 10 +- lib/rspdm/state.rs | 206 ++++++++++++++++++++++++++++++++++++++++- lib/rspdm/validator.rs | 156 ++++++++++++++++++++++++++++++- 4 files changed, 365 insertions(+), 13 deletions(-)
lib/rspdm/consts.rs +6 -0
@@ -87,6 +87,8 @@pub (crate )const SPDM_GET_CERTIFICATE :u8 =0x82 ; +pub (crate )const SPDM_CHALLENGE :u8 =0x83 ; +// If the crypto support isn't enabled don't offer the algorithms // to the responder #[cfg( CONFIG_CRYPTO_RSA )]
@@ -115,3 +117,7 @@pub (crate )const SPDM_HASH_ALGOS :u32 =SPDM_HASH_SHA2_256 |SPDM_HASH_SHA2_384_512 ;pub (crate )const SPDM_OPAQUE_DATA_FMT_GENERAL :u8 =bit_u8 (1 ); + +pub (crate )const SPDM_PREFIX_SZ :usize =64 ; +pub (crate )const SPDM_COMBINED_PREFIX_SZ :usize =100 ; +pub (crate )const SPDM_MAX_OPAQUE_DATA :usize =1024 ;
lib/rspdm/lib.rs +7 -3
@@ -11,8 +11,7 @@//! from other subsytems. use crate ::bindings::{ - spdm_state, -EPROTONOSUPPORT ,// + spdm_state,// };use core::ptr;use kernel::prelude::*;
@@ -307,7 +306,12 @@ fn from(code: SpdmErrorCode) -> Self { provisioned_slots &= !(1 << slot); } - -(EPROTONOSUPPORT as i32 ) +let provisioned_slots = state.provisioned_slots.trailing_zeros (); +if let Err (e) = state.challenge (provisioned_slotsas u8 ) { +return e.to_errno ()as c_int ; + } + +0 }/// Destroys an SPDM session.
lib/rspdm/state.rs +201 -5
324 lines not on this page · raw message
lib/rspdm/validator.rs +151 -5
201 lines not on this page · raw message
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v6 24/25] lib: rspdm: Support SPDM challenge
Sashiko has reviewed this patch and found no issues.
Quoted 5 lines
From: Alistair Francis <[email protected]>
Support the CHALLENGE SPDM command.
Signed-off-by Alistair Francis <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=24
PATCH v6 25/25 lib: rspdm: Expose public Rust API
In reply to [email protected]From: Alistair Francis <[email protected]>
Expose a public Rust API and an example that can be used to authenticate an SPDM responder.
--- lib/rspdm/lib.rs | 69 ++++++++++++++++------------------------------ lib/rspdm/state.rs | 43 +++++++++++++++++++++++++++-- 2 files changed, 63 insertions(+), 49 deletions(-)
lib/rspdm/lib.rs +23 -46
@@ -9,6 +9,26 @@//! //! Top level library, including C compatible public functions to be called //! from other subsytems. +//! +//! ```rust,ignore +//! struct MyTransport { } +//! +//! impl Transport for MyTransport { +//! fn exchange( +//! &self, +//! dev: &Device, +//! request: &[u8], +//! response: &mut [u8], +//! ) -> Result<usize, Error> { +//! todo!() +//! } +//! } +//! +//! fn authenticate_device(dev: &Device, transport: &MyTransport) -> Result { +//! let mut state = SpdmState::new(ARef::from(dev), transport, 512, None); +//! state.authenticate() +//! } +//! ``` use crate ::bindings::{ spdm_state,//
@@ -30,7 +50,7 @@ };use crate ::consts::SPDM_MIN_DATA_TRANSFER_SIZE ; -use crate ::state::{ +pub use crate ::state::{SpdmState ,Transport ,// };
@@ -263,51 +283,8 @@ fn from(code: SpdmErrorCode) -> Self {let mut state = mutex.lock (); -if let Err (e) = state.get_version () { -return e.to_errno ()as c_int ; - } - -if let Err (e) = state.get_capabilities () { -return e.to_errno ()as c_int ; - } - -if let Err (e) = state.negotiate_algs () { -return e.to_errno ()as c_int ; - } - -if let Err (e) = state.get_digests () { -return e.to_errno ()as c_int ; - } - -if state.provisioned_slots ==0 { -return -(bindings::EIO as c_int ); - } - -let mut provisioned_slots = state.provisioned_slots; -while (provisioned_slotsas usize ) >0 { -let slot = provisioned_slots.trailing_zeros ()as u8 ; - -if let Err (e) = state.get_certificate (slot) { -return e.to_errno ()as c_int ; - } - - provisioned_slots &= !(1 << slot); - } - -let mut provisioned_slots = state.provisioned_slots; -while (provisioned_slotsas usize ) >0 { -let slot = provisioned_slots.trailing_zeros ()as u8 ; - -if let Err (e) = state.validate_cert_chain (slot) { -pr_err! ("Certificate in slot {slot} failed to verify: {e:?}\n" ); -return e.to_errno ()as c_int ; - } - - provisioned_slots &= !(1 << slot); - } - -let provisioned_slots = state.provisioned_slots.trailing_zeros (); -if let Err (e) = state.challenge (provisioned_slotsas u8 ) { +if let Err (e) = state.authenticate () { +dev_err! (&state.dev,"Authentication failed: {e:?}\n" );return e.to_errno ()as c_int ; }
lib/rspdm/state.rs +40 -3
67 lines not on this page · raw message
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH v6 25/25] lib: rspdm: Expose public Rust API
Thank you for your contribution! Sashiko AI review found 4 potential issues to consider: - [Medium] lib: rspdm: unsound safe SpdmState::new() and authenticate() with C callback - [Medium] lib…
Thank you for your contribution! Sashiko AI review found 4 potential issues to consider:
- [Medium] lib: rspdm: unsound safe SpdmState::new() and authenticate() with C callback
- [Medium] lib: rspdm: missing transport_sz check in SpdmState::new() and authenticate()
- [Medium] lib: rspdm: public Rust API not built as an importable crate in Kbuild
- [Low] lib: rspdm: Transport not implemented for &T in SpdmState::new() example
--
commit c488a6ca9367e06e871d7f271b768271d53fc789
Author: Alistair Francis <[email protected]>
lib: rspdm: Expose public Rust API
This commit moves the SPDM authentication sequence into a public SpdmState::authenticate() method, makes SpdmState and Transport public, and adds a crate-level usage example.
Quoted 18 lines
lib/rspdm/lib.rs +10
-0@@ -9,6 +9,26 @@//! //! Top level library, including C compatible public functions to be called //! from other subsytems. +//! +//! ```rust,ignore +//! struct MyTransport { } +//! +//! impl Transport for MyTransport { +//! fn exchange( +//! &self, +//! dev: &Device, +//! request: &[u8], +//! response: &mut [u8],
lib/rspdm/lib.rs line 31
Quoted 10 lines
lib/rspdm/lib.rs +10
-0+//! ) -> Result<usize, Error> { +//! todo!() +//! } +//! } +//! +//! fn authenticate_device(dev: &Device, transport: &MyTransport) -> Result { +//! let mut state = SpdmState::new(ARef::from(dev), transport, 512, None); +//! state.authenticate() +//! } +//! ```
[Severity: Low]
Does Transport need a blanket implementation for shared references (&T) so that passing transport: &MyTransport to SpdmState::new() satisfies the
T: Transport bound?
[ ... ]
lib/rspdm/lib.rs line 56
Quoted 9 lines
lib/rspdm/lib.rs +1
-1@@ -30,7 +50,7 @@ };use crate ::consts::SPDM_MIN_DATA_TRANSFER_SIZE ; -use crate ::state::{ +pub use crate ::state::{SpdmState ,Transport ,// };
[Severity: Medium]
Can other Rust crates in the kernel import SpdmState and Transport with the current Kbuild setup?
lib/rspdm/Makefile builds spdm-y := lib.o as a plain object file without emitting crate metadata (.rmeta), exporting Rust symbols, or passing --extern flags to downstream Rust crates in scripts/Makefile.build, so no other Rust crate can reference or link against this public API.
Quoted 4 lines
lib/rspdm/state.rs +0
-0
[ ... ]
lib/rspdm/state.rs impl<T: Transport> SpdmState<T> { · line 200
Quoted 9 lines
lib/rspdm/state.rs +1
-1@@ -194,7 +194,7 @@ impl<T: Transport> SpdmState<T> {/// /// `transport_sz` is the maximum message size the transport can carry, /// and must be at least the SPDM minimum data transfer size (42 bytes). -pub (crate )fn new ( +pub fn new ( dev:ARef <Device >, transport:T , transport_sz: u32,
[Severity: Medium]
Can a Rust caller pass transport_sz < SPDM_MIN_DATA_TRANSFER_SIZE (42) to SpdmState::new() and underflow rsp_sz in SpdmState::get_certificate()?
The SPDM_MIN_DATA_TRANSFER_SIZE check remains only in the C entry point spdm_create(). When Rust code calls SpdmState::new() and SpdmState::authenticate() with a smaller transport_sz, data_transfer_sz stays below the minimum; if transport_sz < GET_CERTIFICATE_RSP_HDR_SZ (8), computing request.length in get_certificate() underflows usize:
lib/rspdm/state.rs:SpdmState::get_certificate() {
...
let rsp_sz = (GET_CERTIFICATE_RSP_HDR_SZ + PAGE_SIZE).min(self.data_transfer_sz as usize);
request.offset = 0;
request.length = (rsp_sz - GET_CERTIFICATE_RSP_HDR_SZ) as u16;
...
}Quoted 21 lines
lib/rspdm/state.rs +17
-0@@ -232,6 +232,43 @@ pub(crate) fn hash_len(&self) -> usize { .unwrap_or(0) } +/// Authenticates the responder through a sequence of GET_VERSION, +/// GET_CAPABILITIES, NEGOTIATE_ALGORITHMS, GET_DIGESTS, GET_CERTIFICATE +/// and CHALLENGE exchanges. +pub fn authenticate (&mut self) ->Result { + self.get_version ()?; + self.get_capabilities ()?; + self.negotiate_algs ()?; + self.get_digests ()?; + +if self.provisioned_slots ==0 { +return Err (EIO ); + } + +let mut provisioned_slots = self.provisioned_slots; +while (provisioned_slotsas usize ) >0 { +let slot = provisioned_slots.trailing_zeros ()as u8 ; +
lib/rspdm/state.rs pub(crate) fn hash_len(&self) -> usize { · line 261
Quoted 10 lines
lib/rspdm/state.rs +10
-0+ self.get_certificate (slot)?; + + provisioned_slots &= !(1 << slot); + } + +let mut provisioned_slots = self.provisioned_slots; +while (provisioned_slotsas usize ) >0 { +let slot = provisioned_slots.trailing_zeros ()as u8 ; + + self.validate_cert_chain (slot)?;
[Severity: Medium]
Is it sound for SpdmState::new() and SpdmState::authenticate() to be safe public functions while SpdmState::new() accepts a raw bindings::spdm_validate callback?
bindings::spdm_validate is an Option<unsafe extern "C" fn(...)>, which safe Rust code can construct and pass to SpdmState::new(). Calling state.authenticate() then invokes that unsafe function pointer inside SpdmState::validate_cert_chain() without an unsafe block at the call site, allowing safe Rust callers to bypass callback preconditions (such as device type assumptions) while also leaking raw kernel::bindings types into the public Rust API:
lib/rspdm/state.rs:SpdmState::validate_cert_chain() {
...
if let Some(validate) = self.validate {
// SAFETY: Call the `validate` function provided with a valid
// certificate pointer, which the callback must not modify or
// free.
let rc = unsafe { validate(self.dev.as_raw(), slot, leaf.as_raw()) };
to_result(rc)?;
}
...
}Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=25