[RFC 0/6] lib: Rust implementation of SPDM

[RFC 0/6] lib: Rust implementation of SPDM

[Alistair Francis]

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.

As such this series implements a SPDM requester in Rust.

This is very similar to Lukas' implementation [2]. This series applies on top
of Lukas' tree [3] and is heavily based on Lukas' work. At build time a user can
choose to use either the Rust of the C SPDM implementation. The two are
interchangable, although you can only use one at a time.

To help with maintaining compatibility it's designed in a way to match Lukas'
design and the state struct stores the same information, although in a Rust
struct instead of the original C one.

The Rust implementation currently supports less features, but my end goal
is to consolidate to a single Rust implementation eventually. That will
probably have to wait until Rust in the kernel is no longer experimental
as SPDM is looking to be an important feature to support for all platforms.

This series is based on the latest rust-next tree.

This seris depends on the Untrusted abstraction work [4].

This seris also depends on the recent bindgen support for static inlines  [5].

The entire tree can be seen here: https://github.com/alistair23/linux/tree/alistair/spdm-rust

based-on: https://lore.kernel.org/all/cover.1719771133.git.lukas@wunner.de/
based-on: https://lore.kernel.org/rust-for-linux/20240925205244.873020-1-benno.lossin@proton.me/
based-on: https://lore.kernel.org/all/20241114005631.818440-1-alistair@alistair23.me/

1: https://www.dmtf.org/standards/spdm
2: https://lore.kernel.org/all/cover.1719771133.git.lukas@wunner.de/
3: https://github.com/l1k/linux/commits/spdm-future/
4: https://lore.kernel.org/rust-for-linux/20240925205244.873020-1-benno.lossin@proton.me/
5: https://lore.kernel.org/all/20241114005631.818440-1-alistair@alistair23.me/

Alistair Francis (6):
  rust: bindings: Support SPDM bindings
  drivers: pci: Change CONFIG_SPDM to a dependency
  lib: rspdm: Initial commit of Rust SPDM
  lib: rspdm: Support SPDM get_version
  lib: rspdm: Support SPDM get_capabilities
  lib: rspdm: Support SPDM negotiate_algorithms

 MAINTAINERS                     |   6 +
 drivers/pci/Kconfig             |   2 +-
 lib/Kconfig                     |  47 ++-
 lib/Makefile                    |   1 +
 lib/rspdm/Makefile              |  11 +
 lib/rspdm/consts.rs             | 123 +++++++
 lib/rspdm/lib.rs                | 146 +++++++++
 lib/rspdm/req-sysfs.c           | 174 ++++++++++
 lib/rspdm/state.rs              | 556 ++++++++++++++++++++++++++++++++
 lib/rspdm/sysfs.rs              |  27 ++
 lib/rspdm/validator.rs          | 301 +++++++++++++++++
 rust/bindgen_static_functions   |   4 +
 rust/bindings/bindings_helper.h |   2 +
 13 files changed, 1384 insertions(+), 16 deletions(-)
 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/req-sysfs.c
 create mode 100644 lib/rspdm/state.rs
 create mode 100644 lib/rspdm/sysfs.rs
 create mode 100644 lib/rspdm/validator.rs

-- 
2.47.0

[RFC 1/6] rust: bindings: Support SPDM bindings

[Alistair Francis]

In preparation for a Rust SPDM library we need to include the SPDM
functions in the Rust bindings.

Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
 rust/bindings/bindings_helper.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 7847b2b3090b..8283e6a79ac9 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -24,6 +24,7 @@
 #include <linux/sched/signal.h>
 #include <linux/sched/task.h>
 #include <linux/slab.h>
+#include <linux/spdm.h>
 #include <linux/uaccess.h>
 #include <linux/wait.h>
 #include <linux/workqueue.h>
-- 
2.47.0

[RFC 2/6] drivers: pci: Change CONFIG_SPDM to a dependency

[Alistair Francis]

In preparation for adding a Rust SPDM library change SPDM to a
dependency so that the user can select which SPDM library to use at
build time.

Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
 drivers/pci/Kconfig |  2 +-
 lib/Kconfig         | 30 +++++++++++++++---------------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index f1c39a6477a5..690a2a38cb52 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -128,7 +128,7 @@ config PCI_CMA
 	select CRYPTO_SHA256
 	select CRYPTO_SHA512
 	select PCI_DOE
-	select SPDM
+	depends on SPDM
 	help
 	  Authenticate devices on enumeration per PCIe r6.2 sec 6.31.
 	  A PCI DOE mailbox is used as transport for DMTF SPDM based
diff --git a/lib/Kconfig b/lib/Kconfig
index 68f46e4a72a6..4db9bc8e29f8 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -739,6 +739,21 @@ config LWQ_TEST
 	help
           Run boot-time test of light-weight queuing.
 
+config SPDM
+	bool "SPDM"
+	select CRYPTO
+	select KEYS
+	select ASYMMETRIC_KEY_TYPE
+	select ASYMMETRIC_PUBLIC_KEY_SUBTYPE
+	select X509_CERTIFICATE_PARSER
+	help
+	  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.  Drivers selecting SPDM therefore need to also select
+	  any algorithms they deem mandatory.
+
 endmenu
 
 config GENERIC_IOREMAP
@@ -777,18 +792,3 @@ config POLYNOMIAL
 
 config FIRMWARE_TABLE
 	bool
-
-config SPDM
-	tristate
-	select CRYPTO
-	select KEYS
-	select ASYMMETRIC_KEY_TYPE
-	select ASYMMETRIC_PUBLIC_KEY_SUBTYPE
-	select X509_CERTIFICATE_PARSER
-	help
-	  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.  Drivers selecting SPDM therefore need to also select
-	  any algorithms they deem mandatory.
-- 
2.47.0

[RFC 3/6] lib: rspdm: Initial commit of Rust SPDM

[Alistair Francis]

This is the initial commit of the Rust SPDM library. It is based on and
compatible with the C SPDM library in the kernel (lib/spdm).

Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
 MAINTAINERS            |   6 ++
 drivers/pci/Kconfig    |   2 +-
 lib/Kconfig            |  17 ++++
 lib/Makefile           |   1 +
 lib/rspdm/Makefile     |  11 +++
 lib/rspdm/consts.rs    |  39 ++++++++
 lib/rspdm/lib.rs       | 134 +++++++++++++++++++++++++
 lib/rspdm/req-sysfs.c  | 174 +++++++++++++++++++++++++++++++++
 lib/rspdm/state.rs     | 217 +++++++++++++++++++++++++++++++++++++++++
 lib/rspdm/sysfs.rs     |  27 +++++
 lib/rspdm/validator.rs |  65 ++++++++++++
 11 files changed, 692 insertions(+), 1 deletion(-)
 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/req-sysfs.c
 create mode 100644 lib/rspdm/state.rs
 create mode 100644 lib/rspdm/sysfs.rs
 create mode 100644 lib/rspdm/validator.rs

diff --git a/MAINTAINERS b/MAINTAINERS
index a30aca77b17b..2c2d4ddcfb1d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20785,6 +20785,12 @@ F:	drivers/pci/cma*
 F:	include/linux/spdm.h
 F:	lib/spdm/
 
+RUST SECURITY PROTOCOL AND DATA MODEL (SPDM)
+M:	Alistair Francis <alistair@alistair23.me>
+L:	linux-pci@vger.kernel.org
+S:	Maintained
+F:	lib/rspdm/
+
 SECURITY SUBSYSTEM
 M:	Paul Moore <paul@paul-moore.com>
 M:	James Morris <jmorris@namei.org>
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index 690a2a38cb52..744d35d28dc7 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -128,7 +128,7 @@ config PCI_CMA
 	select CRYPTO_SHA256
 	select CRYPTO_SHA512
 	select PCI_DOE
-	depends on SPDM
+	depends on SPDM || RSPDM
 	help
 	  Authenticate devices on enumeration per PCIe r6.2 sec 6.31.
 	  A PCI DOE mailbox is used as transport for DMTF SPDM based
diff --git a/lib/Kconfig b/lib/Kconfig
index 4db9bc8e29f8..a47650a6757c 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -754,6 +754,23 @@ config SPDM
 	  in .config.  Drivers selecting SPDM therefore need to also select
 	  any algorithms they deem mandatory.
 
+config RSPDM
+	bool "Rust SPDM"
+	select CRYPTO
+	select KEYS
+	select ASYMMETRIC_KEY_TYPE
+	select ASYMMETRIC_PUBLIC_KEY_SUBTYPE
+	select X509_CERTIFICATE_PARSER
+	depends on SPDM = "n"
+	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.  Drivers selecting SPDM therefore need to also select
+	  any algorithms they deem mandatory.
+
 endmenu
 
 config GENERIC_IOREMAP
diff --git a/lib/Makefile b/lib/Makefile
index 5c6a48365993..4b55542c1aed 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -302,6 +302,7 @@ obj-$(CONFIG_ASN1) += asn1_decoder.o
 obj-$(CONFIG_ASN1_ENCODER) += asn1_encoder.o
 
 obj-$(CONFIG_SPDM) += spdm/
+obj-$(CONFIG_RSPDM) += rspdm/
 
 obj-$(CONFIG_FONT_SUPPORT) += fonts/
 
diff --git a/lib/rspdm/Makefile b/lib/rspdm/Makefile
new file mode 100644
index 000000000000..f15b1437196b
--- /dev/null
+++ b/lib/rspdm/Makefile
@@ -0,0 +1,11 @@
+# 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) 2024 Western Digital
+
+obj-$(CONFIG_RSPDM) += spdm.o
+
+spdm-y := lib.o
+spdm-$(CONFIG_SYSFS) += req-sysfs.o
diff --git a/lib/rspdm/consts.rs b/lib/rspdm/consts.rs
new file mode 100644
index 000000000000..017e673ff194
--- /dev/null
+++ b/lib/rspdm/consts.rs
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0
+//! Rust implementation of the DMTF Security Protocol and Data Model (SPDM)
+//! https://www.dmtf.org/dsp/DSP0274
+//!
+//! Constants used by the library
+//!
+//! Copyright (C) 2024 Western Digital
+
+pub(crate) const SPDM_REQ: u8 = 0x80;
+pub(crate) const SPDM_ERROR: u8 = 0x7f;
+
+#[allow(dead_code)]
+#[repr(u8)]
+#[derive(Clone, Copy)]
+pub(crate) enum SpdmErrorCode {
+    InvalidRequest = 0x01,
+    InvalidSession = 0x02,
+    Busy = 0x03,
+    UnexpectedRequest = 0x04,
+    Unspecified = 0x05,
+    DecryptError = 0x06,
+    UnsupportedRequest = 0x07,
+    RequestInFlight = 0x08,
+    InvalidResponseCode = 0x09,
+    SessionLimitExceeded = 0x0a,
+    SessionRequired = 0x0b,
+    ResetRequired = 0x0c,
+    ResponseTooLarge = 0x0d,
+    RequestTooLarge = 0x0e,
+    LargeResponse = 0x0f,
+    MessageLost = 0x10,
+    InvalidPolicy = 0x11,
+    VersionMismatch = 0x41,
+    ResponseNotReady = 0x42,
+    RequestResynch = 0x43,
+    OperationFailed = 0x44,
+    NoPendingRequests = 0x45,
+    VendorDefinedError = 0xff,
+}
diff --git a/lib/rspdm/lib.rs b/lib/rspdm/lib.rs
new file mode 100644
index 000000000000..edfd94ab56dd
--- /dev/null
+++ b/lib/rspdm/lib.rs
@@ -0,0 +1,134 @@
+// SPDX-License-Identifier: GPL-2.0
+//! Rust implementation of the DMTF Security Protocol and Data Model (SPDM)
+//! https://www.dmtf.org/dsp/DSP0274
+//!
+//! Top level library, including C compatible public functions to be called
+//! from other subsytems.
+//!
+//! This mimics the C SPDM implementation in the kernel
+//!
+//! Copyright (C) 2024 Western Digital
+
+use core::ffi::{c_int, c_void};
+use core::ptr;
+use core::slice::from_raw_parts_mut;
+use kernel::prelude::*;
+use kernel::{alloc::flags, bindings};
+
+use crate::state::SpdmState;
+
+const __LOG_PREFIX: &[u8] = b"spdm\0";
+
+mod consts;
+mod state;
+pub mod sysfs;
+mod validator;
+
+/// spdm_create() - Allocate SPDM session
+///
+/// @dev: Responder device
+/// @transport: Transport function to perform one message exchange
+/// @transport_priv: Transport private data
+/// @transport_sz: Maximum message size the transport is capable of (in bytes)
+/// @keyring: Trusted root certificates
+/// @validate: Function to validate additional leaf certificate requirements
+///	(optional, may be %NULL)
+///
+/// Return a pointer to the allocated SPDM session state or NULL on error.
+#[no_mangle]
+pub unsafe extern "C" fn spdm_create(
+    dev: *mut bindings::device,
+    transport: bindings::spdm_transport,
+    transport_priv: *mut c_void,
+    transport_sz: u32,
+    keyring: *mut bindings::key,
+    validate: bindings::spdm_validate,
+) -> *mut SpdmState {
+    match KBox::new(
+        SpdmState::new(
+            dev,
+            transport,
+            transport_priv,
+            transport_sz,
+            keyring,
+            validate,
+        ),
+        flags::GFP_KERNEL,
+    ) {
+        Ok(ret) => KBox::into_raw(ret) as *mut SpdmState,
+        Err(_) => ptr::null_mut(),
+    }
+}
+
+/// spdm_exchange() - Perform SPDM message exchange with device
+///
+/// @spdm_state: SPDM session state
+/// @req: Request message
+/// @req_sz: Size of @req
+/// @rsp: Response message
+/// @rsp_sz: Size of @rsp
+///
+/// Send the request @req to the device via the @transport in @spdm_state and
+/// receive the response into @rsp, respecting the maximum buffer size @rsp_sz.
+/// The request version is automatically populated.
+///
+/// Return response size on success or a negative errno.  Response size may be
+/// less than @rsp_sz and the caller is responsible for checking that.  It may
+/// also be more than expected (though never more than @rsp_sz), e.g. if the
+/// transport receives only dword-sized chunks.
+#[no_mangle]
+pub unsafe extern "C" fn spdm_exchange(
+    state: &'static mut SpdmState,
+    req: *mut c_void,
+    req_sz: usize,
+    rsp: *mut c_void,
+    rsp_sz: usize,
+) -> isize {
+    let request_buf: &mut [u8] = unsafe { from_raw_parts_mut(req as *mut u8, req_sz) };
+    let response_buf: &mut [u8] = unsafe { from_raw_parts_mut(rsp as *mut u8, rsp_sz) };
+
+    match state.spdm_exchange(request_buf, response_buf) {
+        Ok(ret) => ret as isize,
+        Err(e) => e.to_errno() as isize,
+    }
+}
+
+/// spdm_authenticate() - Authenticate device
+///
+/// @spdm_state: SPDM session state
+///
+/// Authenticate a device through a sequence of GET_VERSION, GET_CAPABILITIES,
+/// NEGOTIATE_ALGORITHMS, GET_DIGESTS, GET_CERTIFICATE and CHALLENGE exchanges.
+///
+/// Perform internal locking to serialize multiple concurrent invocations.
+/// Can be called repeatedly for reauthentication.
+///
+/// Return 0 on success or a negative errno.  In particular, -EPROTONOSUPPORT
+/// indicates authentication is not supported by the device.
+#[no_mangle]
+pub unsafe extern "C" fn spdm_authenticate(_state: &'static mut SpdmState) -> c_int {
+    0
+}
+
+/// spdm_publish_log() - Publish log of received SPDM signatures in sysfs
+///
+/// @spdm_state: SPDM session state
+///
+/// sysfs attributes representing received SPDM signatures are not static,
+/// but created dynamically upon authentication or measurement.  If a device
+/// was authenticated or measured before it became visible in sysfs, the
+/// attributes could not be created.  This function retroactively creates
+/// those attributes in sysfs after the device has become visible through
+/// device_add().
+#[no_mangle]
+pub unsafe extern "C" fn spdm_publish_log(_spdm_state: *mut SpdmState) {
+    todo!()
+}
+
+/// spdm_destroy() - Destroy SPDM session
+///
+/// @spdm_state: SPDM session state
+#[no_mangle]
+pub unsafe extern "C" fn spdm_destroy(_spdm_state: *mut SpdmState) {
+    todo!()
+}
diff --git a/lib/rspdm/req-sysfs.c b/lib/rspdm/req-sysfs.c
new file mode 100644
index 000000000000..aff79eb0c4ee
--- /dev/null
+++ b/lib/rspdm/req-sysfs.c
@@ -0,0 +1,174 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Rust implementation of the DMTF Security Protocol and Data Model (SPDM)
+ * https://www.dmtf.org/dsp/DSP0274
+ *
+ * Requester role: sysfs interface
+ *
+ * Copyright (C) 2023-24 Intel Corporation
+ * Copyright (C) 2024 Western Digital
+ */
+
+#include <linux/pci.h>
+
+#define SPDM_NONCE_SZ 32 /* SPDM 1.0.0 table 20 */
+
+int rust_authenticated_show(void *spdm_state, char *buf);
+
+/**
+ * dev_to_spdm_state() - Retrieve SPDM session state for given device
+ *
+ * @dev: Responder device
+ *
+ * Returns a pointer to the device's SPDM session state,
+ *	   %NULL if the device doesn't have one or
+ *	   %ERR_PTR if it couldn't be determined whether SPDM is supported.
+ *
+ * In the %ERR_PTR case, attributes are visible but return an error on access.
+ * This prevents downgrade attacks where an attacker disturbs memory allocation
+ * or communication with the device in order to create the appearance that SPDM
+ * is unsupported.  E.g. with PCI devices, the attacker may foil CMA or DOE
+ * initialization by simply hogging memory.
+ */
+static void *dev_to_spdm_state(struct device *dev)
+{
+	if (dev_is_pci(dev))
+		return pci_dev_to_spdm_state(to_pci_dev(dev));
+
+	/* Insert mappers for further bus types here. */
+
+	return NULL;
+}
+
+/* authenticated attribute */
+
+static umode_t spdm_attrs_are_visible(struct kobject *kobj,
+				      struct attribute *a, int n)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	void *spdm_state = dev_to_spdm_state(dev);
+
+	if (IS_ERR_OR_NULL(spdm_state))
+		return SYSFS_GROUP_INVISIBLE;
+
+	return a->mode;
+}
+
+static ssize_t authenticated_store(struct device *dev,
+				   struct device_attribute *attr,
+				   const char *buf, size_t count)
+{
+	void *spdm_state = dev_to_spdm_state(dev);
+	int rc;
+
+	if (IS_ERR_OR_NULL(spdm_state))
+		return PTR_ERR(spdm_state);
+
+	if (sysfs_streq(buf, "re")) {
+		rc = spdm_authenticate(spdm_state);
+		if (rc)
+			return rc;
+	} else {
+		return -EINVAL;
+	}
+
+	return count;
+}
+
+static ssize_t authenticated_show(struct device *dev,
+				  struct device_attribute *attr, char *buf)
+{
+	void *spdm_state = dev_to_spdm_state(dev);
+
+	if (IS_ERR_OR_NULL(spdm_state))
+		return PTR_ERR(spdm_state);
+
+	return rust_authenticated_show(spdm_state, buf);
+}
+static DEVICE_ATTR_RW(authenticated);
+
+static struct attribute *spdm_attrs[] = {
+	&dev_attr_authenticated.attr,
+	NULL
+};
+
+const struct attribute_group spdm_attr_group = {
+	.attrs = spdm_attrs,
+	.is_visible = spdm_attrs_are_visible,
+};
+
+/* certificates attributes */
+
+static umode_t spdm_certificates_are_visible(struct kobject *kobj,
+					     struct bin_attribute *a, int n)
+{
+	return SYSFS_GROUP_INVISIBLE;
+}
+
+static ssize_t spdm_cert_read(struct file *file, struct kobject *kobj,
+			      struct bin_attribute *a, char *buf, loff_t off,
+			      size_t count)
+{
+	return 0;
+}
+
+static BIN_ATTR(slot0, 0444, spdm_cert_read, NULL, 0xffff);
+static BIN_ATTR(slot1, 0444, spdm_cert_read, NULL, 0xffff);
+static BIN_ATTR(slot2, 0444, spdm_cert_read, NULL, 0xffff);
+static BIN_ATTR(slot3, 0444, spdm_cert_read, NULL, 0xffff);
+static BIN_ATTR(slot4, 0444, spdm_cert_read, NULL, 0xffff);
+static BIN_ATTR(slot5, 0444, spdm_cert_read, NULL, 0xffff);
+static BIN_ATTR(slot6, 0444, spdm_cert_read, NULL, 0xffff);
+static BIN_ATTR(slot7, 0444, spdm_cert_read, NULL, 0xffff);
+
+static struct bin_attribute *spdm_certificates_bin_attrs[] = {
+	&bin_attr_slot0,
+	&bin_attr_slot1,
+	&bin_attr_slot2,
+	&bin_attr_slot3,
+	&bin_attr_slot4,
+	&bin_attr_slot5,
+	&bin_attr_slot6,
+	&bin_attr_slot7,
+	NULL
+};
+
+const struct attribute_group spdm_certificates_group = {
+	.name = "certificates",
+	.bin_attrs = spdm_certificates_bin_attrs,
+	.is_bin_visible = spdm_certificates_are_visible,
+};
+
+/* signatures attributes */
+
+static umode_t spdm_signatures_are_visible(struct kobject *kobj,
+					   struct bin_attribute *a, int n)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	void *spdm_state = dev_to_spdm_state(dev);
+
+	if (IS_ERR_OR_NULL(spdm_state))
+		return SYSFS_GROUP_INVISIBLE;
+
+	return a->attr.mode;
+}
+
+static ssize_t next_requester_nonce_write(struct file *file,
+					  struct kobject *kobj,
+					  struct bin_attribute *attr,
+					  char *buf, loff_t off, size_t count)
+{
+	return 0;
+}
+static BIN_ATTR_WO(next_requester_nonce, SPDM_NONCE_SZ);
+
+static struct bin_attribute *spdm_signatures_bin_attrs[] = {
+	&bin_attr_next_requester_nonce,
+	NULL
+};
+
+const struct attribute_group spdm_signatures_group = {
+	.name = "signatures",
+	.bin_attrs = spdm_signatures_bin_attrs,
+	.is_bin_visible = spdm_signatures_are_visible,
+};
diff --git a/lib/rspdm/state.rs b/lib/rspdm/state.rs
new file mode 100644
index 000000000000..5b55c4655e2e
--- /dev/null
+++ b/lib/rspdm/state.rs
@@ -0,0 +1,217 @@
+// SPDX-License-Identifier: GPL-2.0
+//! Rust implementation of the DMTF Security Protocol and Data Model (SPDM)
+//! https://www.dmtf.org/dsp/DSP0274
+//!
+//! The `SpdmState` struct and implementation.
+//!
+//! Copyright (C) 2024 Western Digital
+
+use core::ffi::c_void;
+use kernel::prelude::*;
+use kernel::{
+    bindings,
+    error::{code::EINVAL, to_result, Error},
+    validate::Untrusted,
+};
+
+use crate::consts::{SpdmErrorCode, SPDM_ERROR, SPDM_REQ};
+use crate::validator::{SpdmErrorRsp, SpdmHeader};
+
+/// The current SPDM session state for a device. Based on the
+/// C `struct spdm_state`.
+///
+/// @dev: Responder device.  Used for error reporting and passed to @transport.
+/// @transport: Transport function to perform one message exchange.
+/// @transport_priv: Transport private data.
+/// @transport_sz: Maximum message size the transport is capable of (in bytes).
+///  Used as DataTransferSize in GET_CAPABILITIES exchange.
+/// @keyring: Keyring against which to check the first certificate in
+///  responder's certificate chain.
+/// @validate: Function to validate additional leaf certificate requirements.
+///
+/// @version: Maximum common supported version of requester and responder.
+///  Negotiated during GET_VERSION exchange.
+///
+/// @authenticated: Whether device was authenticated successfully.
+#[allow(dead_code)]
+pub struct SpdmState {
+    pub(crate) dev: *mut bindings::device,
+    pub(crate) transport: bindings::spdm_transport,
+    pub(crate) transport_priv: *mut c_void,
+    pub(crate) transport_sz: u32,
+    pub(crate) keyring: *mut bindings::key,
+    pub(crate) validate: bindings::spdm_validate,
+
+    /* Negotiated state */
+    pub(crate) version: u8,
+
+    pub(crate) authenticated: bool,
+}
+
+impl SpdmState {
+    pub(crate) fn new(
+        dev: *mut bindings::device,
+        transport: bindings::spdm_transport,
+        transport_priv: *mut c_void,
+        transport_sz: u32,
+        keyring: *mut bindings::key,
+        validate: bindings::spdm_validate,
+    ) -> Self {
+        SpdmState {
+            dev,
+            transport,
+            transport_priv,
+            transport_sz,
+            keyring,
+            validate,
+            version: 0x10,
+            authenticated: false,
+        }
+    }
+
+    fn spdm_err(&self, rsp: &SpdmErrorRsp) -> Result<(), Error> {
+        let ret = match rsp.error_code {
+            SpdmErrorCode::InvalidRequest => {
+                pr_err!("Invalid request\n");
+                bindings::EINVAL
+            }
+            SpdmErrorCode::InvalidSession => {
+                if rsp.version == 0x11 {
+                    pr_err!("Invalid session {:#x}\n", rsp.error_data);
+                    bindings::EINVAL
+                } else {
+                    pr_err!("Undefined error {:#x}\n", rsp.error_code as u8);
+                    bindings::EINVAL
+                }
+            }
+            SpdmErrorCode::Busy => {
+                pr_err!("Busy\n");
+                bindings::EBUSY
+            }
+            SpdmErrorCode::UnexpectedRequest => {
+                pr_err!("Unexpected request\n");
+                bindings::EINVAL
+            }
+            SpdmErrorCode::Unspecified => {
+                pr_err!("Unspecified error\n");
+                bindings::EINVAL
+            }
+            SpdmErrorCode::DecryptError => {
+                pr_err!("Decrypt error\n");
+                bindings::EIO
+            }
+            SpdmErrorCode::UnsupportedRequest => {
+                pr_err!("Unsupported request {:#x}\n", rsp.error_data);
+                bindings::EINVAL
+            }
+            SpdmErrorCode::RequestInFlight => {
+                pr_err!("Request in flight\n");
+                bindings::EINVAL
+            }
+            SpdmErrorCode::InvalidResponseCode => {
+                pr_err!("Invalid response code\n");
+                bindings::EINVAL
+            }
+            SpdmErrorCode::SessionLimitExceeded => {
+                pr_err!("Session limit exceeded\n");
+                bindings::EBUSY
+            }
+            SpdmErrorCode::SessionRequired => {
+                pr_err!("Session required\n");
+                bindings::EINVAL
+            }
+            SpdmErrorCode::ResetRequired => {
+                pr_err!("Reset required\n");
+                bindings::ECONNRESET
+            }
+            SpdmErrorCode::ResponseTooLarge => {
+                pr_err!("Response too large\n");
+                bindings::EINVAL
+            }
+            SpdmErrorCode::RequestTooLarge => {
+                pr_err!("Request too large\n");
+                bindings::EINVAL
+            }
+            SpdmErrorCode::LargeResponse => {
+                pr_err!("Large response\n");
+                bindings::EMSGSIZE
+            }
+            SpdmErrorCode::MessageLost => {
+                pr_err!("Message lost\n");
+                bindings::EIO
+            }
+            SpdmErrorCode::InvalidPolicy => {
+                pr_err!("Invalid policy\n");
+                bindings::EINVAL
+            }
+            SpdmErrorCode::VersionMismatch => {
+                pr_err!("Version mismatch\n");
+                bindings::EINVAL
+            }
+            SpdmErrorCode::ResponseNotReady => {
+                pr_err!("Response not ready\n");
+                bindings::EINPROGRESS
+            }
+            SpdmErrorCode::RequestResynch => {
+                pr_err!("Request resynchronization\n");
+                bindings::ECONNRESET
+            }
+            SpdmErrorCode::OperationFailed => {
+                pr_err!("Operation failed\n");
+                bindings::EINVAL
+            }
+            SpdmErrorCode::NoPendingRequests => bindings::ENOENT,
+            SpdmErrorCode::VendorDefinedError => {
+                pr_err!("Vendor defined error\n");
+                bindings::EINVAL
+            }
+        };
+
+        to_result(-(ret as i32))
+    }
+
+    /// Start a SPDM exchange
+    ///
+    /// The data in `request_buf` is sent to the device and the response is
+    /// stored in `response_buf`.
+    pub(crate) fn spdm_exchange(
+        &self,
+        request_buf: &mut [u8],
+        response_buf: &mut [u8],
+    ) -> Result<i32, Error> {
+        let header_size = core::mem::size_of::<SpdmHeader>();
+        let request: &mut SpdmHeader = Untrusted::new_mut(request_buf).validate_mut()?;
+        let response: &SpdmHeader = Untrusted::new_ref(response_buf).validate()?;
+
+        let transport_function = self.transport.ok_or(EINVAL)?;
+        let length = unsafe {
+            transport_function(
+                self.transport_priv,
+                self.dev,
+                request_buf.as_ptr() as *const c_void,
+                request_buf.len(),
+                response_buf.as_mut_ptr() as *mut c_void,
+                response_buf.len(),
+            ) as i32
+        };
+        to_result(length)?;
+
+        if (length as usize) < header_size {
+            return Ok(length); /* Truncated response is handled by callers */
+        }
+        if response.code == SPDM_ERROR {
+            self.spdm_err(unsafe { &*(response_buf.as_ptr() as *const SpdmErrorRsp) })?;
+        }
+
+        if response.code != request.code & !SPDM_REQ {
+            pr_err!(
+                "Response code {:#x} does not match request code {:#x}\n",
+                response.code,
+                request.code
+            );
+            to_result(-(bindings::EPROTO as i32))?;
+        }
+
+        Ok(length)
+    }
+}
diff --git a/lib/rspdm/sysfs.rs b/lib/rspdm/sysfs.rs
new file mode 100644
index 000000000000..5deffdbf2b0c
--- /dev/null
+++ b/lib/rspdm/sysfs.rs
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-2.0
+//! Rust implementation of the DMTF Security Protocol and Data Model (SPDM)
+//! https://www.dmtf.org/dsp/DSP0274
+//!
+//! Rust sysfs helper functions
+//!
+//! Copyright (C) 2024 Western Digital
+
+use crate::SpdmState;
+use kernel::prelude::*;
+use kernel::{bindings, fmt, str::CString};
+
+/// Helper function for the sysfs `authenticated_show()`.
+#[no_mangle]
+pub unsafe extern "C" fn rust_authenticated_show(
+    spdm_state: *mut SpdmState,
+    buf: *mut core::ffi::c_char,
+) -> isize {
+    let state = unsafe { KBox::from_raw(spdm_state) };
+
+    let fmt = match CString::try_from_fmt(fmt!("{}\n", state.authenticated)) {
+        Ok(f) => f,
+        Err(_e) => return 0,
+    };
+
+    unsafe { bindings::sysfs_emit(buf, fmt.as_char_ptr()) as isize }
+}
diff --git a/lib/rspdm/validator.rs b/lib/rspdm/validator.rs
new file mode 100644
index 000000000000..5004804f85c8
--- /dev/null
+++ b/lib/rspdm/validator.rs
@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: GPL-2.0
+//! Rust implementation of the DMTF Security Protocol and Data Model (SPDM)
+//! https://www.dmtf.org/dsp/DSP0274
+//!
+//! Related structs and their Validate implementations.
+//!
+//! Copyright (C) 2024 Western Digital
+
+use crate::consts::SpdmErrorCode;
+use core::mem;
+use kernel::prelude::*;
+use kernel::{
+    error::{code::EINVAL, Error},
+    validate::{Unvalidated, Validate},
+};
+
+#[repr(C, packed)]
+pub(crate) struct SpdmHeader {
+    pub(crate) version: u8,
+    pub(crate) code: u8, /* RequestResponseCode */
+    pub(crate) param1: u8,
+    pub(crate) param2: u8,
+}
+
+impl Validate<&Unvalidated<[u8]>> for &SpdmHeader {
+    type Err = Error;
+
+    fn validate(unvalidated: &Unvalidated<[u8]>) -> Result<Self, Self::Err> {
+        let raw = unvalidated.raw();
+        if raw.len() < mem::size_of::<SpdmHeader>() {
+            return Err(EINVAL);
+        }
+
+        let ptr = raw.as_ptr();
+        // CAST: `SpdmHeader` only contains integers and has `repr(C)`.
+        let ptr = ptr.cast::<SpdmHeader>();
+        // SAFETY: `ptr` came from a reference and the cast above is valid.
+        Ok(unsafe { &*ptr })
+    }
+}
+
+impl Validate<&mut Unvalidated<[u8]>> for &mut SpdmHeader {
+    type Err = Error;
+
+    fn validate(unvalidated: &mut Unvalidated<[u8]>) -> Result<Self, Self::Err> {
+        let raw = unvalidated.raw_mut();
+        if raw.len() < mem::size_of::<SpdmHeader>() {
+            return Err(EINVAL);
+        }
+
+        let ptr = raw.as_mut_ptr();
+        // CAST: `SpdmHeader` only contains integers and has `repr(C)`.
+        let ptr = ptr.cast::<SpdmHeader>();
+        // SAFETY: `ptr` came from a reference and the cast above is valid.
+        Ok(unsafe { &mut *ptr })
+    }
+}
+
+#[repr(C, packed)]
+pub(crate) struct SpdmErrorRsp {
+    pub(crate) version: u8,
+    pub(crate) code: u8,
+    pub(crate) error_code: SpdmErrorCode,
+    pub(crate) error_data: u8,
+}
-- 
2.47.0

[RFC 4/6] lib: rspdm: Support SPDM get_version

[Alistair Francis]

Support the GET_VERSION SPDM command.

Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
 lib/rspdm/consts.rs    | 17 ++++++++++++
 lib/rspdm/lib.rs       |  6 ++++-
 lib/rspdm/state.rs     | 61 +++++++++++++++++++++++++++++++++++++++---
 lib/rspdm/validator.rs | 54 +++++++++++++++++++++++++++++++++++++
 4 files changed, 134 insertions(+), 4 deletions(-)

diff --git a/lib/rspdm/consts.rs b/lib/rspdm/consts.rs
index 017e673ff194..e263d62fa648 100644
--- a/lib/rspdm/consts.rs
+++ b/lib/rspdm/consts.rs
@@ -6,6 +6,20 @@
 //!
 //! Copyright (C) 2024 Western Digital
 
+use crate::validator::SpdmHeader;
+use core::mem;
+
+/* SPDM versions supported by this implementation */
+pub(crate) const SPDM_VER_10: u8 = 0x10;
+#[allow(dead_code)]
+pub(crate) const SPDM_VER_11: u8 = 0x11;
+#[allow(dead_code)]
+pub(crate) const SPDM_VER_12: u8 = 0x12;
+pub(crate) const SPDM_VER_13: u8 = 0x13;
+
+pub(crate) const SPDM_MIN_VER: u8 = SPDM_VER_10;
+pub(crate) const SPDM_MAX_VER: u8 = SPDM_VER_13;
+
 pub(crate) const SPDM_REQ: u8 = 0x80;
 pub(crate) const SPDM_ERROR: u8 = 0x7f;
 
@@ -37,3 +51,6 @@ pub(crate) enum SpdmErrorCode {
     NoPendingRequests = 0x45,
     VendorDefinedError = 0xff,
 }
+
+pub(crate) const SPDM_GET_VERSION: u8 = 0x84;
+pub(crate) const SPDM_GET_VERSION_LEN: usize = mem::size_of::<SpdmHeader>() + 255;
diff --git a/lib/rspdm/lib.rs b/lib/rspdm/lib.rs
index edfd94ab56dd..670ecc02a471 100644
--- a/lib/rspdm/lib.rs
+++ b/lib/rspdm/lib.rs
@@ -106,7 +106,11 @@
 /// Return 0 on success or a negative errno.  In particular, -EPROTONOSUPPORT
 /// indicates authentication is not supported by the device.
 #[no_mangle]
-pub unsafe extern "C" fn spdm_authenticate(_state: &'static mut SpdmState) -> c_int {
+pub unsafe extern "C" fn spdm_authenticate(state: &'static mut SpdmState) -> c_int {
+    if let Err(e) = state.get_version() {
+        return e.to_errno() as c_int;
+    }
+
     0
 }
 
diff --git a/lib/rspdm/state.rs b/lib/rspdm/state.rs
index 5b55c4655e2e..9ace0bbaa21a 100644
--- a/lib/rspdm/state.rs
+++ b/lib/rspdm/state.rs
@@ -7,6 +7,7 @@
 //! Copyright (C) 2024 Western Digital
 
 use core::ffi::c_void;
+use core::slice::from_raw_parts_mut;
 use kernel::prelude::*;
 use kernel::{
     bindings,
@@ -14,8 +15,11 @@
     validate::Untrusted,
 };
 
-use crate::consts::{SpdmErrorCode, SPDM_ERROR, SPDM_REQ};
-use crate::validator::{SpdmErrorRsp, SpdmHeader};
+use crate::consts::{
+    SpdmErrorCode, SPDM_ERROR, SPDM_GET_VERSION, SPDM_GET_VERSION_LEN, SPDM_MAX_VER, SPDM_MIN_VER,
+    SPDM_REQ,
+};
+use crate::validator::{GetVersionReq, GetVersionRsp, SpdmErrorRsp, SpdmHeader};
 
 /// The current SPDM session state for a device. Based on the
 /// C `struct spdm_state`.
@@ -64,7 +68,7 @@ pub(crate) fn new(
             transport_sz,
             keyring,
             validate,
-            version: 0x10,
+            version: SPDM_MIN_VER,
             authenticated: false,
         }
     }
@@ -214,4 +218,55 @@ pub(crate) fn spdm_exchange(
 
         Ok(length)
     }
+
+    /// Negoiate a supported SPDM version and store the information
+    /// in the `SpdmState`.
+    pub(crate) fn get_version(&mut self) -> Result<(), Error> {
+        let mut request = GetVersionReq {
+            version: self.version,
+            code: SPDM_GET_VERSION,
+            param1: 0,
+            param2: 0,
+        };
+        // SAFETY: `request` is repr(C) and packed, so we can convert it to a slice
+        let request_buf = unsafe {
+            from_raw_parts_mut(
+                &mut request as *mut _ as *mut u8,
+                core::mem::size_of::<GetVersionReq>(),
+            )
+        };
+
+        let mut response_vec: KVec<u8> = KVec::with_capacity(SPDM_GET_VERSION_LEN, GFP_KERNEL)?;
+        // SAFETY: `request` is repr(C) and packed, so we can convert it to a slice
+        let response_buf =
+            unsafe { from_raw_parts_mut(response_vec.as_mut_ptr(), SPDM_GET_VERSION_LEN) };
+
+        let rc = self.spdm_exchange(request_buf, response_buf)?;
+
+        // SAFETY: `rc` bytes where inserted to the raw pointer by spdm_exchange
+        unsafe { response_vec.set_len(rc as usize) };
+
+        let response: &mut GetVersionRsp = Untrusted::new_mut(&mut response_vec).validate_mut()?;
+
+        let mut foundver = false;
+        for i in 0..response.version_number_entry_count {
+            // Creating a reference on a packed struct will result in
+            // undefined behaviour, so we operate on the raw data directly
+            let unaligned = core::ptr::addr_of_mut!(response.version_number_entries) as *mut u16;
+            let addr = unaligned.wrapping_add(i as usize);
+            let version = (unsafe { core::ptr::read_unaligned::<u16>(addr) } >> 8) as u8;
+
+            if version >= self.version && version <= SPDM_MAX_VER {
+                self.version = version;
+                foundver = true;
+            }
+        }
+
+        if !foundver {
+            pr_err!("No common supported version\n");
+            to_result(-(bindings::EPROTO as i32))?;
+        }
+
+        Ok(())
+    }
 }
diff --git a/lib/rspdm/validator.rs b/lib/rspdm/validator.rs
index 5004804f85c8..05f1ba155920 100644
--- a/lib/rspdm/validator.rs
+++ b/lib/rspdm/validator.rs
@@ -6,6 +6,7 @@
 //!
 //! Copyright (C) 2024 Western Digital
 
+use crate::bindings::{__IncompleteArrayField, __le16};
 use crate::consts::SpdmErrorCode;
 use core::mem;
 use kernel::prelude::*;
@@ -63,3 +64,56 @@ pub(crate) struct SpdmErrorRsp {
     pub(crate) error_code: SpdmErrorCode,
     pub(crate) error_data: u8,
 }
+
+#[repr(C, packed)]
+pub(crate) struct GetVersionReq {
+    pub(crate) version: u8,
+    pub(crate) code: u8,
+    pub(crate) param1: u8,
+    pub(crate) param2: u8,
+}
+
+#[repr(C, packed)]
+pub(crate) struct GetVersionRsp {
+    pub(crate) version: u8,
+    pub(crate) code: u8,
+    param1: u8,
+    param2: u8,
+    reserved: u8,
+    pub(crate) version_number_entry_count: u8,
+    pub(crate) version_number_entries: __IncompleteArrayField<__le16>,
+}
+
+impl Validate<&mut Unvalidated<KVec<u8>>> for &mut GetVersionRsp {
+    type Err = Error;
+
+    fn validate(unvalidated: &mut Unvalidated<KVec<u8>>) -> Result<Self, Self::Err> {
+        let raw = unvalidated.raw_mut();
+        if raw.len() < mem::size_of::<GetVersionRsp>() {
+            return Err(EINVAL);
+        }
+
+        let version_number_entries = *(raw.get(5).ok_or(ENOMEM))? as usize;
+        let total_expected_size = version_number_entries * 2 + 6;
+        if raw.len() < total_expected_size {
+            return Err(EINVAL);
+        }
+
+        let ptr = raw.as_mut_ptr();
+        // CAST: `GetVersionRsp` only contains integers and has `repr(C)`.
+        let ptr = ptr.cast::<GetVersionRsp>();
+        // SAFETY: `ptr` came from a reference and the cast above is valid.
+        let rsp: &mut GetVersionRsp = unsafe { &mut *ptr };
+
+        // Creating a reference on a packed struct will result in
+        // undefined behaviour, so we operate on the raw data directly
+        let unaligned = core::ptr::addr_of_mut!(rsp.version_number_entries) as *mut u16;
+        for version_offset in 0..version_number_entries {
+            let addr = unaligned.wrapping_add(version_offset);
+            let version = unsafe { core::ptr::read_unaligned::<u16>(addr) };
+            unsafe { core::ptr::write_unaligned::<u16>(addr, version.to_le()) }
+        }
+
+        Ok(rsp)
+    }
+}
-- 
2.47.0

[RFC 5/6] lib: rspdm: Support SPDM get_capabilities

[Alistair Francis]

Support the GET_CAPABILITIES SPDM command.

Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
 lib/rspdm/consts.rs    | 18 ++++++++--
 lib/rspdm/lib.rs       |  4 +++
 lib/rspdm/state.rs     | 73 +++++++++++++++++++++++++++++++++++++++--
 lib/rspdm/validator.rs | 74 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 164 insertions(+), 5 deletions(-)

diff --git a/lib/rspdm/consts.rs b/lib/rspdm/consts.rs
index e263d62fa648..d60f8302f389 100644
--- a/lib/rspdm/consts.rs
+++ b/lib/rspdm/consts.rs
@@ -11,9 +11,7 @@
 
 /* SPDM versions supported by this implementation */
 pub(crate) const SPDM_VER_10: u8 = 0x10;
-#[allow(dead_code)]
 pub(crate) const SPDM_VER_11: u8 = 0x11;
-#[allow(dead_code)]
 pub(crate) const SPDM_VER_12: u8 = 0x12;
 pub(crate) const SPDM_VER_13: u8 = 0x13;
 
@@ -54,3 +52,19 @@ pub(crate) enum SpdmErrorCode {
 
 pub(crate) const SPDM_GET_VERSION: u8 = 0x84;
 pub(crate) const SPDM_GET_VERSION_LEN: usize = mem::size_of::<SpdmHeader>() + 255;
+
+pub(crate) const SPDM_GET_CAPABILITIES: u8 = 0xe1;
+pub(crate) const SPDM_MIN_DATA_TRANSFER_SIZE: u32 = 42;
+
+// 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 = 1 << 1;
+pub(crate) const SPDM_CHAL_CAP: u32 = 1 << 2;
+
+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;
diff --git a/lib/rspdm/lib.rs b/lib/rspdm/lib.rs
index 670ecc02a471..bbd755854acd 100644
--- a/lib/rspdm/lib.rs
+++ b/lib/rspdm/lib.rs
@@ -111,6 +111,10 @@
         return e.to_errno() as c_int;
     }
 
+    if let Err(e) = state.get_capabilities() {
+        return e.to_errno() as c_int;
+    }
+
     0
 }
 
diff --git a/lib/rspdm/state.rs b/lib/rspdm/state.rs
index 9ace0bbaa21a..05a7faf17d47 100644
--- a/lib/rspdm/state.rs
+++ b/lib/rspdm/state.rs
@@ -16,10 +16,13 @@
 };
 
 use crate::consts::{
-    SpdmErrorCode, SPDM_ERROR, SPDM_GET_VERSION, SPDM_GET_VERSION_LEN, SPDM_MAX_VER, SPDM_MIN_VER,
-    SPDM_REQ,
+    SpdmErrorCode, SPDM_CTEXPONENT, SPDM_ERROR, SPDM_GET_CAPABILITIES, SPDM_GET_VERSION,
+    SPDM_GET_VERSION_LEN, SPDM_MAX_VER, SPDM_MIN_DATA_TRANSFER_SIZE, SPDM_MIN_VER, SPDM_REQ,
+    SPDM_REQ_CAPS, SPDM_RSP_MIN_CAPS, SPDM_VER_10, SPDM_VER_11, SPDM_VER_12,
+};
+use crate::validator::{
+    GetCapabilitiesReq, GetCapabilitiesRsp, GetVersionReq, GetVersionRsp, SpdmErrorRsp, SpdmHeader,
 };
-use crate::validator::{GetVersionReq, GetVersionRsp, SpdmErrorRsp, SpdmHeader};
 
 /// The current SPDM session state for a device. Based on the
 /// C `struct spdm_state`.
@@ -35,6 +38,8 @@
 ///
 /// @version: Maximum common supported version of requester and responder.
 ///  Negotiated during GET_VERSION exchange.
+/// @rsp_caps: Cached capabilities of responder.
+///  Received during GET_CAPABILITIES exchange.
 ///
 /// @authenticated: Whether device was authenticated successfully.
 #[allow(dead_code)]
@@ -48,6 +53,7 @@ pub struct SpdmState {
 
     /* Negotiated state */
     pub(crate) version: u8,
+    pub(crate) rsp_caps: u32,
 
     pub(crate) authenticated: bool,
 }
@@ -69,6 +75,7 @@ pub(crate) fn new(
             keyring,
             validate,
             version: SPDM_MIN_VER,
+            rsp_caps: 0,
             authenticated: false,
         }
     }
@@ -269,4 +276,64 @@ pub(crate) fn get_version(&mut self) -> Result<(), Error> {
 
         Ok(())
     }
+
+    /// Obtain the supported capabilities from an SPDM session and store the
+    /// information in the `SpdmState`.
+    pub(crate) fn get_capabilities(&mut self) -> Result<(), Error> {
+        let mut request = GetCapabilitiesReq::default();
+
+        request.version = self.version;
+        request.code = SPDM_GET_CAPABILITIES;
+        request.ctexponent = SPDM_CTEXPONENT;
+        request.flags = (SPDM_REQ_CAPS as u32).to_le();
+
+        let (req_sz, rsp_sz) = match self.version {
+            SPDM_VER_10 => (4, 8),
+            SPDM_VER_11 => (8, 8),
+            _ => {
+                request.data_transfer_size = self.transport_sz.to_le();
+                request.max_spdm_msg_size = request.data_transfer_size;
+
+                (
+                    core::mem::size_of::<GetCapabilitiesReq>(),
+                    core::mem::size_of::<GetCapabilitiesRsp>(),
+                )
+            }
+        };
+
+        // SAFETY: `request` is repr(C) and packed, so we can convert it to a slice
+        let request_buf = unsafe { from_raw_parts_mut(&mut request as *mut _ as *mut u8, req_sz) };
+
+        let mut response_vec: KVec<u8> = KVec::with_capacity(rsp_sz, GFP_KERNEL)?;
+        // SAFETY: `request` is repr(C) and packed, so we can convert it to a slice
+        let response_buf = unsafe { from_raw_parts_mut(response_vec.as_mut_ptr(), rsp_sz) };
+
+        let rc = self.spdm_exchange(request_buf, response_buf)?;
+
+        if rc < (rsp_sz as i32) {
+            pr_err!("Truncated capabilities response\n");
+            to_result(-(bindings::EIO as i32))?;
+        }
+
+        // SAFETY: `rc` bytes where inserted to the raw pointer by spdm_exchange
+        unsafe { response_vec.set_len(rc as usize) };
+
+        let response: &mut GetCapabilitiesRsp =
+            Untrusted::new_mut(&mut response_vec).validate_mut()?;
+
+        self.rsp_caps = u32::from_le(response.flags);
+        if (self.rsp_caps & SPDM_RSP_MIN_CAPS) != SPDM_RSP_MIN_CAPS {
+            to_result(-(bindings::EPROTONOSUPPORT as i32))?;
+        }
+
+        if self.version >= SPDM_VER_12 {
+            if response.data_transfer_size < SPDM_MIN_DATA_TRANSFER_SIZE {
+                pr_err!("Malformed capabilities response\n");
+                to_result(-(bindings::EPROTO as i32))?;
+            }
+            self.transport_sz = self.transport_sz.min(response.data_transfer_size);
+        }
+
+        Ok(())
+    }
 }
diff --git a/lib/rspdm/validator.rs b/lib/rspdm/validator.rs
index 05f1ba155920..cc998e70f235 100644
--- a/lib/rspdm/validator.rs
+++ b/lib/rspdm/validator.rs
@@ -117,3 +117,77 @@ fn validate(unvalidated: &mut Unvalidated<KVec<u8>>) -> Result<Self, Self::Err>
         Ok(rsp)
     }
 }
+
+#[repr(C, packed)]
+pub(crate) struct GetCapabilitiesReq {
+    pub(crate) version: u8,
+    pub(crate) code: u8,
+    pub(crate) param1: u8,
+    pub(crate) param2: u8,
+
+    reserved1: u8,
+    pub(crate) ctexponent: u8,
+    reserved2: u16,
+
+    pub(crate) flags: u32,
+
+    /* End of SPDM 1.1 structure */
+    pub(crate) data_transfer_size: u32,
+    pub(crate) max_spdm_msg_size: u32,
+}
+
+impl Default for GetCapabilitiesReq {
+    fn default() -> Self {
+        GetCapabilitiesReq {
+            version: 0,
+            code: 0,
+            param1: 0,
+            param2: 0,
+            reserved1: 0,
+            ctexponent: 0,
+            reserved2: 0,
+            flags: 0,
+            data_transfer_size: 0,
+            max_spdm_msg_size: 0,
+        }
+    }
+}
+
+#[repr(C, packed)]
+pub(crate) struct GetCapabilitiesRsp {
+    pub(crate) version: u8,
+    pub(crate) code: u8,
+    pub(crate) param1: u8,
+    pub(crate) param2: u8,
+
+    reserved1: u8,
+    pub(crate) ctexponent: u8,
+    reserved2: u16,
+
+    pub(crate) flags: u32,
+
+    /* End of SPDM 1.1 structure */
+    pub(crate) data_transfer_size: u32,
+    pub(crate) max_spdm_msg_size: u32,
+
+    pub(crate) supported_algorithms: __IncompleteArrayField<__le16>,
+}
+
+impl Validate<&mut Unvalidated<KVec<u8>>> for &mut GetCapabilitiesRsp {
+    type Err = Error;
+
+    fn validate(unvalidated: &mut Unvalidated<KVec<u8>>) -> Result<Self, Self::Err> {
+        let raw = unvalidated.raw_mut();
+        if raw.len() < mem::size_of::<GetCapabilitiesRsp>() {
+            return Err(EINVAL);
+        }
+
+        let ptr = raw.as_mut_ptr();
+        // CAST: `GetCapabilitiesRsp` only contains integers and has `repr(C)`.
+        let ptr = ptr.cast::<GetCapabilitiesRsp>();
+        // SAFETY: `ptr` came from a reference and the cast above is valid.
+        let rsp: &mut GetCapabilitiesRsp = unsafe { &mut *ptr };
+
+        Ok(rsp)
+    }
+}
-- 
2.47.0

[RFC 6/6] lib: rspdm: Support SPDM negotiate_algorithms

[Alistair Francis]

Support the NEGOTIATE_ALGORITHMS SPDM command.

Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
 lib/rspdm/consts.rs             |  53 ++++++++
 lib/rspdm/lib.rs                |   4 +
 lib/rspdm/state.rs              | 225 +++++++++++++++++++++++++++++++-
 lib/rspdm/validator.rs          | 110 +++++++++++++++-
 rust/bindgen_static_functions   |   4 +
 rust/bindings/bindings_helper.h |   1 +
 6 files changed, 392 insertions(+), 5 deletions(-)

diff --git a/lib/rspdm/consts.rs b/lib/rspdm/consts.rs
index d60f8302f389..a1218874a524 100644
--- a/lib/rspdm/consts.rs
+++ b/lib/rspdm/consts.rs
@@ -65,6 +65,59 @@ pub(crate) enum SpdmErrorCode {
 
 pub(crate) const SPDM_CERT_CAP: u32 = 1 << 1;
 pub(crate) const SPDM_CHAL_CAP: u32 = 1 << 2;
+pub(crate) const SPDM_MEAS_CAP_MASK: u32 = 3 << 3;
+pub(crate) const SPDM_KEY_EX_CAP: u32 = 1 << 9;
 
 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 = 1 << 0;
+
+pub(crate) const SPDM_ASYM_RSASSA_2048: u32 = 1 << 0;
+pub(crate) const _SPDM_ASYM_RSAPSS_2048: u32 = 1 << 1;
+pub(crate) const SPDM_ASYM_RSASSA_3072: u32 = 1 << 2;
+pub(crate) const _SPDM_ASYM_RSAPSS_3072: u32 = 1 << 3;
+pub(crate) const SPDM_ASYM_ECDSA_ECC_NIST_P256: u32 = 1 << 4;
+pub(crate) const SPDM_ASYM_RSASSA_4096: u32 = 1 << 5;
+pub(crate) const _SPDM_ASYM_RSAPSS_4096: u32 = 1 << 6;
+pub(crate) const SPDM_ASYM_ECDSA_ECC_NIST_P384: u32 = 1 << 7;
+pub(crate) const SPDM_ASYM_ECDSA_ECC_NIST_P521: u32 = 1 << 8;
+pub(crate) const _SPDM_ASYM_SM2_ECC_SM2_P256: u32 = 1 << 9;
+pub(crate) const _SPDM_ASYM_EDDSA_ED25519: u32 = 1 << 10;
+pub(crate) const _SPDM_ASYM_EDDSA_ED448: u32 = 1 << 11;
+
+pub(crate) const SPDM_HASH_SHA_256: u32 = 1 << 0;
+pub(crate) const SPDM_HASH_SHA_384: u32 = 1 << 1;
+pub(crate) const SPDM_HASH_SHA_512: u32 = 1 << 2;
+
+#[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;
+
+/* Maximum number of ReqAlgStructs sent by this implementation */
+// pub(crate) const SPDM_MAX_REQ_ALG_STRUCT: usize = 4;
+
+pub(crate) const SPDM_OPAQUE_DATA_FMT_GENERAL: u8 = 1 << 1;
diff --git a/lib/rspdm/lib.rs b/lib/rspdm/lib.rs
index bbd755854acd..89be29f1b5dd 100644
--- a/lib/rspdm/lib.rs
+++ b/lib/rspdm/lib.rs
@@ -115,6 +115,10 @@
         return e.to_errno() as c_int;
     }
 
+    if let Err(e) = state.negotiate_algs() {
+        return e.to_errno() as c_int;
+    }
+
     0
 }
 
diff --git a/lib/rspdm/state.rs b/lib/rspdm/state.rs
index 05a7faf17d47..80027bbde673 100644
--- a/lib/rspdm/state.rs
+++ b/lib/rspdm/state.rs
@@ -12,16 +12,22 @@
 use kernel::{
     bindings,
     error::{code::EINVAL, to_result, Error},
+    str::CStr,
     validate::Untrusted,
 };
 
 use crate::consts::{
-    SpdmErrorCode, SPDM_CTEXPONENT, SPDM_ERROR, SPDM_GET_CAPABILITIES, SPDM_GET_VERSION,
-    SPDM_GET_VERSION_LEN, SPDM_MAX_VER, SPDM_MIN_DATA_TRANSFER_SIZE, SPDM_MIN_VER, SPDM_REQ,
-    SPDM_REQ_CAPS, SPDM_RSP_MIN_CAPS, SPDM_VER_10, SPDM_VER_11, SPDM_VER_12,
+    SpdmErrorCode, SPDM_ASYM_ALGOS, SPDM_ASYM_ECDSA_ECC_NIST_P256, SPDM_ASYM_ECDSA_ECC_NIST_P384,
+    SPDM_ASYM_ECDSA_ECC_NIST_P521, SPDM_ASYM_RSASSA_2048, SPDM_ASYM_RSASSA_3072,
+    SPDM_ASYM_RSASSA_4096, SPDM_CTEXPONENT, SPDM_ERROR, SPDM_GET_CAPABILITIES, SPDM_GET_VERSION,
+    SPDM_GET_VERSION_LEN, SPDM_HASH_ALGOS, SPDM_HASH_SHA_256, SPDM_HASH_SHA_384, SPDM_HASH_SHA_512,
+    SPDM_KEY_EX_CAP, SPDM_MAX_VER, SPDM_MEAS_CAP_MASK, SPDM_MEAS_SPEC_DMTF,
+    SPDM_MIN_DATA_TRANSFER_SIZE, SPDM_MIN_VER, SPDM_NEGOTIATE_ALGS, SPDM_OPAQUE_DATA_FMT_GENERAL,
+    SPDM_REQ, SPDM_REQ_CAPS, SPDM_RSP_MIN_CAPS, SPDM_VER_10, SPDM_VER_11, SPDM_VER_12,
 };
 use crate::validator::{
-    GetCapabilitiesReq, GetCapabilitiesRsp, GetVersionReq, GetVersionRsp, SpdmErrorRsp, SpdmHeader,
+    GetCapabilitiesReq, GetCapabilitiesRsp, GetVersionReq, GetVersionRsp, NegotiateAlgsReq,
+    NegotiateAlgsRsp, RegAlg, SpdmErrorRsp, SpdmHeader,
 };
 
 /// The current SPDM session state for a device. Based on the
@@ -40,6 +46,28 @@
 ///  Negotiated during GET_VERSION exchange.
 /// @rsp_caps: Cached capabilities of responder.
 ///  Received during GET_CAPABILITIES exchange.
+/// @base_asym_alg: Asymmetric key algorithm for signature verification of
+///  CHALLENGE_AUTH and MEASUREMENTS messages.
+///  Selected by responder during NEGOTIATE_ALGORITHMS exchange.
+/// @base_hash_alg: Hash algorithm for signature verification of
+///  CHALLENGE_AUTH and MEASUREMENTS messages.
+///  Selected by responder during NEGOTIATE_ALGORITHMS exchange.
+/// @meas_hash_alg: Hash algorithm for measurement blocks.
+///  Selected by responder during NEGOTIATE_ALGORITHMS exchange.
+/// @base_asym_enc: Human-readable name of @base_asym_alg's signature encoding.
+///  Passed to crypto subsystem when calling verify_signature().
+/// @sig_len: Signature length of @base_asym_alg (in bytes).
+///  S or SigLen in SPDM specification.
+/// @base_hash_alg_name: Human-readable name of @base_hash_alg.
+///  Passed to crypto subsystem when calling crypto_alloc_shash() and
+///  verify_signature().
+/// @base_hash_alg_name: Human-readable name of @base_hash_alg.
+///  Passed to crypto subsystem when calling crypto_alloc_shash() and
+///  verify_signature().
+/// @shash: Synchronous hash handle for @base_hash_alg computation.
+/// @desc: Synchronous hash context for @base_hash_alg computation.
+/// @hash_len: Hash length of @base_hash_alg (in bytes).
+///  H in SPDM specification.
 ///
 /// @authenticated: Whether device was authenticated successfully.
 #[allow(dead_code)]
@@ -54,6 +82,19 @@ pub struct SpdmState {
     /* Negotiated state */
     pub(crate) version: u8,
     pub(crate) rsp_caps: u32,
+    pub(crate) base_asym_alg: u32,
+    pub(crate) base_hash_alg: u32,
+    pub(crate) meas_hash_alg: u32,
+
+    /* Signature algorithm */
+    base_asym_enc: &'static CStr,
+    sig_len: usize,
+
+    /* Hash algorithm */
+    base_hash_alg_name: &'static CStr,
+    shash: Option<*mut bindings::crypto_shash>,
+    desc: Option<&'static mut bindings::shash_desc>,
+    hash_len: usize,
 
     pub(crate) authenticated: bool,
 }
@@ -76,6 +117,15 @@ pub(crate) fn new(
             validate,
             version: SPDM_MIN_VER,
             rsp_caps: 0,
+            base_asym_alg: 0,
+            base_hash_alg: 0,
+            meas_hash_alg: 0,
+            base_asym_enc: unsafe { CStr::from_bytes_with_nul_unchecked(b"\0") },
+            sig_len: 0,
+            base_hash_alg_name: unsafe { CStr::from_bytes_with_nul_unchecked(b"\0") },
+            shash: None,
+            desc: None,
+            hash_len: 0,
             authenticated: false,
         }
     }
@@ -336,4 +386,171 @@ pub(crate) fn get_capabilities(&mut self) -> Result<(), Error> {
 
         Ok(())
     }
+
+    fn update_response_algs(&mut self) -> Result<(), Error> {
+        match self.base_asym_alg {
+            SPDM_ASYM_RSASSA_2048 => {
+                self.sig_len = 256;
+                self.base_asym_enc = CStr::from_bytes_with_nul(b"pkcs1\0")?;
+            }
+            SPDM_ASYM_RSASSA_3072 => {
+                self.sig_len = 384;
+                self.base_asym_enc = CStr::from_bytes_with_nul(b"pkcs1\0")?;
+            }
+            SPDM_ASYM_RSASSA_4096 => {
+                self.sig_len = 512;
+                self.base_asym_enc = CStr::from_bytes_with_nul(b"pkcs1\0")?;
+            }
+            SPDM_ASYM_ECDSA_ECC_NIST_P256 => {
+                self.sig_len = 64;
+                self.base_asym_enc = CStr::from_bytes_with_nul(b"p1363\0")?;
+            }
+            SPDM_ASYM_ECDSA_ECC_NIST_P384 => {
+                self.sig_len = 96;
+                self.base_asym_enc = CStr::from_bytes_with_nul(b"p1363\0")?;
+            }
+            SPDM_ASYM_ECDSA_ECC_NIST_P521 => {
+                self.sig_len = 132;
+                self.base_asym_enc = CStr::from_bytes_with_nul(b"p1363\0")?;
+            }
+            _ => {
+                pr_err!("Unknown asym algorithm\n");
+                return Err(EINVAL);
+            }
+        }
+
+        match self.base_hash_alg {
+            SPDM_HASH_SHA_256 => {
+                self.base_hash_alg_name = CStr::from_bytes_with_nul(b"sha256\0")?;
+            }
+            SPDM_HASH_SHA_384 => {
+                self.base_hash_alg_name = CStr::from_bytes_with_nul(b"sha384\0")?;
+            }
+            SPDM_HASH_SHA_512 => {
+                self.base_hash_alg_name = CStr::from_bytes_with_nul(b"sha512\0")?;
+            }
+            _ => {
+                pr_err!("Unknown hash algorithm\n");
+                return Err(EINVAL);
+            }
+        }
+
+        /*
+         * shash and desc allocations are reused for subsequent measurement
+         * retrieval, hence are not freed until spdm_reset().
+         */
+        let shash =
+            unsafe { bindings::crypto_alloc_shash(self.base_hash_alg_name.as_char_ptr(), 0, 0) };
+        if shash.is_null() {
+            return Err(ENOMEM);
+        }
+
+        let desc_len = core::mem::size_of::<bindings::shash_desc>()
+            + unsafe { bindings::crypto_shash_descsize(shash) } as usize;
+        self.shash = Some(shash);
+
+        let mut desc_vec: KVec<u8> = KVec::with_capacity(desc_len, GFP_KERNEL)?;
+        // SAFETY: `desc_vec` is `desc_len` long
+        let desc_buf = unsafe { from_raw_parts_mut(desc_vec.as_mut_ptr(), desc_len) };
+
+        let desc = unsafe {
+            core::mem::transmute::<*mut c_void, &mut bindings::shash_desc>(
+                desc_buf.as_mut_ptr() as *mut c_void
+            )
+        };
+        desc.tfm = shash;
+
+        self.desc = Some(desc);
+
+        /* Used frequently to compute offsets, so cache H */
+        self.shash.map(|shash| {
+            self.hash_len = unsafe { bindings::crypto_shash_digestsize(shash) as usize };
+        });
+
+        if let Some(desc) = &mut self.desc {
+            unsafe {
+                to_result(bindings::crypto_shash_init(
+                    *desc as *mut bindings::shash_desc,
+                ))
+            }
+        } else {
+            Err(ENOMEM)
+        }
+    }
+
+    pub(crate) fn negotiate_algs(&mut self) -> Result<(), Error> {
+        let mut request = NegotiateAlgsReq::default();
+        let reg_alg_entries = 0;
+
+        request.version = self.version;
+        request.code = SPDM_NEGOTIATE_ALGS;
+        request.measurement_specification = SPDM_MEAS_SPEC_DMTF;
+        request.base_asym_algo = SPDM_ASYM_ALGOS.to_le();
+        request.base_hash_algo = SPDM_HASH_ALGOS.to_le();
+
+        if self.version >= SPDM_VER_12 && (self.rsp_caps & SPDM_KEY_EX_CAP) == SPDM_KEY_EX_CAP {
+            request.other_params_support = SPDM_OPAQUE_DATA_FMT_GENERAL;
+        }
+
+        let req_sz = core::mem::size_of::<NegotiateAlgsReq>()
+            + core::mem::size_of::<RegAlg>() * reg_alg_entries;
+        let rsp_sz = core::mem::size_of::<NegotiateAlgsRsp>()
+            + core::mem::size_of::<RegAlg>() * reg_alg_entries;
+
+        request.length = req_sz as u16;
+        request.param1 = reg_alg_entries as u8;
+
+        // SAFETY: `request` is repr(C) and packed, so we can convert it to a slice
+        let request_buf = unsafe { from_raw_parts_mut(&mut request as *mut _ as *mut u8, req_sz) };
+
+        let mut response_vec: KVec<u8> = KVec::with_capacity(rsp_sz, GFP_KERNEL)?;
+        // SAFETY: `request` is repr(C) and packed, so we can convert it to a slice
+        let response_buf = unsafe { from_raw_parts_mut(response_vec.as_mut_ptr(), rsp_sz) };
+
+        let rc = self.spdm_exchange(request_buf, response_buf)?;
+
+        if rc < (rsp_sz as i32) {
+            pr_err!("Truncated capabilities response\n");
+            to_result(-(bindings::EIO as i32))?;
+        }
+
+        // SAFETY: `rc` bytes where inserted to the raw pointer by spdm_exchange
+        unsafe { response_vec.set_len(rc as usize) };
+
+        let response: &mut NegotiateAlgsRsp =
+            Untrusted::new_mut(&mut response_vec).validate_mut()?;
+
+        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 {
+            pr_err!("No common supported algorithms\n");
+            to_result(-(bindings::EPROTO as i32))?;
+        }
+
+        // /* Responder shall select exactly 1 alg (SPDM 1.0.0 table 14) */
+        if self.base_asym_alg.count_ones() != 1
+            || self.base_hash_alg.count_ones() != 1
+            || response.ext_asym_sel_count != 0
+            || response.ext_hash_sel_count != 0
+            || response.param1 > request.param1
+            || response.other_params_sel != request.other_params_support
+        {
+            pr_err!("Malformed algorithms response\n");
+            to_result(-(bindings::EPROTO as i32))?;
+        }
+
+        if self.rsp_caps & SPDM_MEAS_CAP_MASK == SPDM_MEAS_CAP_MASK
+            && (self.meas_hash_alg.count_ones() != 1
+                || response.measurement_specification_sel != SPDM_MEAS_SPEC_DMTF)
+        {
+            pr_err!("Malformed algorithms response\n");
+            to_result(-(bindings::EPROTO as i32))?;
+        }
+
+        self.update_response_algs()?;
+
+        Ok(())
+    }
 }
diff --git a/lib/rspdm/validator.rs b/lib/rspdm/validator.rs
index cc998e70f235..5f64870e18d2 100644
--- a/lib/rspdm/validator.rs
+++ b/lib/rspdm/validator.rs
@@ -6,7 +6,7 @@
 //!
 //! Copyright (C) 2024 Western Digital
 
-use crate::bindings::{__IncompleteArrayField, __le16};
+use crate::bindings::{__IncompleteArrayField, __le16, __le32};
 use crate::consts::SpdmErrorCode;
 use core::mem;
 use kernel::prelude::*;
@@ -191,3 +191,111 @@ fn validate(unvalidated: &mut Unvalidated<KVec<u8>>) -> Result<Self, Self::Err>
         Ok(rsp)
     }
 }
+
+#[repr(C, packed)]
+pub(crate) struct RegAlg {
+    pub(crate) alg_type: u8,
+    pub(crate) alg_count: u8,
+    pub(crate) alg_supported: u16,
+    pub(crate) alg_external: __IncompleteArrayField<__le32>,
+}
+
+#[repr(C, packed)]
+pub(crate) struct NegotiateAlgsReq {
+    pub(crate) version: u8,
+    pub(crate) code: u8,
+    pub(crate) param1: u8,
+    pub(crate) param2: u8,
+
+    pub(crate) length: u16,
+    pub(crate) measurement_specification: u8,
+    pub(crate) other_params_support: u8,
+
+    pub(crate) base_asym_algo: u32,
+    pub(crate) base_hash_algo: u32,
+
+    reserved1: [u8; 12],
+
+    pub(crate) ext_asym_count: u8,
+    pub(crate) ext_hash_count: u8,
+    reserved2: u8,
+    pub(crate) mel_specification: u8,
+
+    pub(crate) ext_asym: __IncompleteArrayField<__le32>,
+    pub(crate) ext_hash: __IncompleteArrayField<__le32>,
+    pub(crate) req_alg_struct: __IncompleteArrayField<RegAlg>,
+}
+
+impl Default for NegotiateAlgsReq {
+    fn default() -> Self {
+        NegotiateAlgsReq {
+            version: 0,
+            code: 0,
+            param1: 0,
+            param2: 0,
+            length: 0,
+            measurement_specification: 0,
+            other_params_support: 0,
+            base_asym_algo: 0,
+            base_hash_algo: 0,
+            reserved1: [0u8; 12],
+            ext_asym_count: 0,
+            ext_hash_count: 0,
+            reserved2: 0,
+            mel_specification: 0,
+            ext_asym: __IncompleteArrayField::new(),
+            ext_hash: __IncompleteArrayField::new(),
+            req_alg_struct: __IncompleteArrayField::new(),
+        }
+    }
+}
+
+#[repr(C, packed)]
+pub(crate) struct NegotiateAlgsRsp {
+    pub(crate) version: u8,
+    pub(crate) code: u8,
+    pub(crate) param1: u8,
+    pub(crate) param2: u8,
+
+    pub(crate) length: u16,
+    pub(crate) measurement_specification_sel: u8,
+    pub(crate) other_params_sel: u8,
+
+    pub(crate) measurement_hash_algo: u32,
+    pub(crate) base_asym_sel: u32,
+    pub(crate) base_hash_sel: u32,
+
+    reserved1: [u8; 11],
+
+    pub(crate) mel_specification_sel: u8,
+    pub(crate) ext_asym_sel_count: u8,
+    pub(crate) ext_hash_sel_count: u8,
+    reserved2: [u8; 2],
+
+    pub(crate) ext_asym: __IncompleteArrayField<__le32>,
+    pub(crate) ext_hash: __IncompleteArrayField<__le32>,
+    pub(crate) req_alg_struct: __IncompleteArrayField<RegAlg>,
+}
+
+impl Validate<&mut Unvalidated<KVec<u8>>> for &mut NegotiateAlgsRsp {
+    type Err = Error;
+
+    fn validate(unvalidated: &mut Unvalidated<KVec<u8>>) -> Result<Self, Self::Err> {
+        let raw = unvalidated.raw_mut();
+        if raw.len() < mem::size_of::<NegotiateAlgsRsp>() {
+            return Err(EINVAL);
+        }
+
+        let ptr = raw.as_mut_ptr();
+        // CAST: `NegotiateAlgsRsp` only contains integers and has `repr(C)`.
+        let ptr = ptr.cast::<NegotiateAlgsRsp>();
+        // SAFETY: `ptr` came from a reference and the cast above is valid.
+        let rsp: &mut NegotiateAlgsRsp = unsafe { &mut *ptr };
+
+        rsp.base_asym_sel = rsp.base_asym_sel.to_le();
+        rsp.base_hash_sel = rsp.base_hash_sel.to_le();
+        rsp.measurement_hash_algo = rsp.measurement_hash_algo.to_le();
+
+        Ok(rsp)
+    }
+}
diff --git a/rust/bindgen_static_functions b/rust/bindgen_static_functions
index ec48ad2e8c78..617316ce1925 100644
--- a/rust/bindgen_static_functions
+++ b/rust/bindgen_static_functions
@@ -30,3 +30,7 @@
 
 --allowlist-function copy_from_user
 --allowlist-function copy_to_user
+
+--allowlist-function crypto_shash_descsize
+--allowlist-function crypto_shash_init
+--allowlist-function crypto_shash_digestsize
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 8283e6a79ac9..c2f6b9a471bc 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -6,6 +6,7 @@
  * Sorted alphabetically.
  */
 
+#include <crypto/hash.h>
 #include <kunit/test.h>
 #include <linux/blk-mq.h>
 #include <linux/blk_types.h>
-- 
2.47.0

Re: [RFC 3/6] lib: rspdm: Initial commit of Rust SPDM

[Miguel Ojeda]

Hi Alistair,

It is nice to see more Rust, thanks!

Some quick consistency nits I noticed for future non-RFC versions (I
didn't check the code). Some apply in several cases.

On Fri, Nov 15, 2024 at 6:46 AM Alistair Francis <alistair@alistair23.me> wrote:
>
> +#[allow(dead_code)]

It may be possible to use `expect` here instead of `allow` -- e.g. if
it does not depend on conditional compilation.

Also, later in the series, is it used? (I imagine it is a temporary
`allow`? If so, please delete it when you introduce the first use.
`expect` can help here to not forget to delete it.

> +#[repr(u8)]

It is probably a good idea to mention why it needs this `repr`. I
imagine it is related to `SpdmErrorRsp` being `packed` and so on, but
it wouldn't hurt documenting it.

> +    InvalidRequest = 0x01,

Please feel free to ignore this one (especially if the idea is to
replace the C implementation eventually, or to just showcase how it
would look like if the C one was removed), but one idea here would be
to pick the values from a common C header? i.e. moving that `enum` to
its own header that both use.

> +//! Top level library, including C compatible public functions to be called
> +//! from other subsytems.

Typo.

> +/// spdm_create() - Allocate SPDM session

I think these are copied from the C one, so it is fine for the RFC,
but the subsystem ends up accepting this, then please use the usual
Markdown style of the rest of the Rust code, instead of kernel-doc
style. While we don't render the docs of these just yet, we will start
doing it at some point, and e.g. IDEs may do so too. Even if we
didn't, the comments could be copied into other docs at some point, so
it is always useful to have them formatted properly.

> +    /* Negotiated state */
> +    pub(crate) version: u8,

Please use `//`.

> +                bindings::EINVAL
> +            }
> +        };
> +
> +        to_result(-(ret as i32))

These are errors you create directly, so you can do directly e.g.
`Err(EINVAL)`, i.e. please avoid `bindings::`.

> +        let length = unsafe {

Missing `SAFETY` comment.

If you based this on top of `rust-next` as you noted in the cover
letter, you should be getting a warning under `CLIPPY=1`. There may be
other cleanups under `CLIPPY=1` if you weren't using it so far.

> +            return Ok(length); /* Truncated response is handled by callers */

Please use `//` for comments.

> +// SPDX-License-Identifier: GPL-2.0

New line between SPDX and the crate docs.

> +//! Rust implementation of the DMTF Security Protocol and Data Model (SPDM)
> +//! https://www.dmtf.org/dsp/DSP0274

This should be a link (using <>) or a link for the "DMTF Security
Protocol and Data Model (SPDM)" text.

> +//! Rust sysfs helper functions

This should be the title, at the top. In Rust the first paragraph
(which typically should be short, e.g. a single line) is considered
the "short description"  and used e.g. for lists.

> +//! Copyright (C) 2024 Western Digital

This should be a comment (likely near the SPDX), rather than part of
the documentation -- see e.g. how it is done in `rust/kernel/list.rs`.

> +pub unsafe extern "C" fn rust_authenticated_show(

`unsafe` functions should have a `# Safety` section.

Thanks again!

Cheers,
Miguel

Re: [RFC 1/6] rust: bindings: Support SPDM bindings

[Bjorn Helgaas]

On Fri, Nov 15, 2024 at 03:46:11PM +1000, Alistair Francis wrote:
> In preparation for a Rust SPDM library we need to include the SPDM
> functions in the Rust bindings.
> 
> Signed-off-by: Alistair Francis <alistair@alistair23.me>
> ---
>  rust/bindings/bindings_helper.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 7847b2b3090b..8283e6a79ac9 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -24,6 +24,7 @@
>  #include <linux/sched/signal.h>
>  #include <linux/sched/task.h>
>  #include <linux/slab.h>
> +#include <linux/spdm.h>

Usually an additional #include goes in the same patch that makes use
of the new .h file.  Maybe there's a different convention in rust/?

>  #include <linux/uaccess.h>
>  #include <linux/wait.h>
>  #include <linux/workqueue.h>
> -- 
> 2.47.0
> 

Re: [RFC 2/6] drivers: pci: Change CONFIG_SPDM to a dependency

[Bjorn Helgaas]

On Fri, Nov 15, 2024 at 03:46:12PM +1000, Alistair Francis wrote:
> In preparation for adding a Rust SPDM library change SPDM to a
> dependency so that the user can select which SPDM library to use at
> build time.

Run "git log --oneline drivers/pci" and follow the existing
subject line convention.

> Signed-off-by: Alistair Francis <alistair@alistair23.me>
> ---
>  drivers/pci/Kconfig |  2 +-
>  lib/Kconfig         | 30 +++++++++++++++---------------
>  2 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index f1c39a6477a5..690a2a38cb52 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -128,7 +128,7 @@ config PCI_CMA
>  	select CRYPTO_SHA256
>  	select CRYPTO_SHA512
>  	select PCI_DOE
> -	select SPDM
> +	depends on SPDM
>  	help
>  	  Authenticate devices on enumeration per PCIe r6.2 sec 6.31.
>  	  A PCI DOE mailbox is used as transport for DMTF SPDM based
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 68f46e4a72a6..4db9bc8e29f8 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -739,6 +739,21 @@ config LWQ_TEST
>  	help
>            Run boot-time test of light-weight queuing.
>  
> +config SPDM
> +	bool "SPDM"

If this appears in a menuconfig or similar menu, I think expanding
"SPDM" would be helpful to users.

> +	select CRYPTO
> +	select KEYS
> +	select ASYMMETRIC_KEY_TYPE
> +	select ASYMMETRIC_PUBLIC_KEY_SUBTYPE
> +	select X509_CERTIFICATE_PARSER
> +	help
> +	  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.  Drivers selecting SPDM therefore need to also select
> +	  any algorithms they deem mandatory.
> +
>  endmenu
>  
>  config GENERIC_IOREMAP
> @@ -777,18 +792,3 @@ config POLYNOMIAL
>  
>  config FIRMWARE_TABLE
>  	bool
> -
> -config SPDM
> -	tristate
> -	select CRYPTO
> -	select KEYS
> -	select ASYMMETRIC_KEY_TYPE
> -	select ASYMMETRIC_PUBLIC_KEY_SUBTYPE
> -	select X509_CERTIFICATE_PARSER
> -	help
> -	  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.  Drivers selecting SPDM therefore need to also select
> -	  any algorithms they deem mandatory.
> -- 
> 2.47.0
> 

Re: [RFC 1/6] rust: bindings: Support SPDM bindings

[Miguel Ojeda]

On Fri, Nov 15, 2024 at 6:53 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> Usually an additional #include goes in the same patch that makes use
> of the new .h file.  Maybe there's a different convention in rust/?

I think doing it in the same patch makes more sense, i.e. please feel
free to avoid a "rust: bindings: ..."-titled patch.

It is also how it has been done in the past, e.g. see commit
de6582833db0 ("rust: add firmware abstractions").

Cheers,
Miguel

Re: [RFC 3/6] lib: rspdm: Initial commit of Rust SPDM

[Dan Williams]

Alistair Francis wrote:
> This is the initial commit of the Rust SPDM library. It is based on and
> compatible with the C SPDM library in the kernel (lib/spdm).
> 
> Signed-off-by: Alistair Francis <alistair@alistair23.me>
[..]
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 690a2a38cb52..744d35d28dc7 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -128,7 +128,7 @@ config PCI_CMA
>  	select CRYPTO_SHA256
>  	select CRYPTO_SHA512
>  	select PCI_DOE
> -	depends on SPDM
> +	depends on SPDM || RSPDM
>  	help
>  	  Authenticate devices on enumeration per PCIe r6.2 sec 6.31.
>  	  A PCI DOE mailbox is used as transport for DMTF SPDM based
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 4db9bc8e29f8..a47650a6757c 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -754,6 +754,23 @@ config SPDM
>  	  in .config.  Drivers selecting SPDM therefore need to also select
>  	  any algorithms they deem mandatory.
>  
> +config RSPDM
> +	bool "Rust SPDM"
> +	select CRYPTO
> +	select KEYS
> +	select ASYMMETRIC_KEY_TYPE
> +	select ASYMMETRIC_PUBLIC_KEY_SUBTYPE
> +	select X509_CERTIFICATE_PARSER
> +	depends on SPDM = "n"

I trust these last 2 diff hunks never actually go upstream, right? I.e.
if the kernel has no SPDM today it should not plan to carry 2
implementations just for language differences.

I am not sure if that is already the plan, but the cover letter seemed
ambiguous with its "maintaining compatibility" statement. On one hand,
that does not make sense when this is brand new upstream code (i.e. C
version is not even in linux-next), and there are no chances for
regressions. Just embrace the attempt to be a Rust library for C
consumers or otherwise help the C version along.

However, if "maintain compatibility" means "make it easy for the
work-in-progress C-effort to switch dependencies", then that makes
sense and is worth clarifying in the next posting.

Re: [RFC 3/6] lib: rspdm: Initial commit of Rust SPDM

[Alistair Francis]

On Sat, Nov 16, 2024 at 8:54 AM Dan Williams <dan.j.williams@intel.com> wrote:
>
> Alistair Francis wrote:
> > This is the initial commit of the Rust SPDM library. It is based on and
> > compatible with the C SPDM library in the kernel (lib/spdm).
> >
> > Signed-off-by: Alistair Francis <alistair@alistair23.me>
> [..]
> > diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> > index 690a2a38cb52..744d35d28dc7 100644
> > --- a/drivers/pci/Kconfig
> > +++ b/drivers/pci/Kconfig
> > @@ -128,7 +128,7 @@ config PCI_CMA
> >       select CRYPTO_SHA256
> >       select CRYPTO_SHA512
> >       select PCI_DOE
> > -     depends on SPDM
> > +     depends on SPDM || RSPDM
> >       help
> >         Authenticate devices on enumeration per PCIe r6.2 sec 6.31.
> >         A PCI DOE mailbox is used as transport for DMTF SPDM based
> > diff --git a/lib/Kconfig b/lib/Kconfig
> > index 4db9bc8e29f8..a47650a6757c 100644
> > --- a/lib/Kconfig
> > +++ b/lib/Kconfig
> > @@ -754,6 +754,23 @@ config SPDM
> >         in .config.  Drivers selecting SPDM therefore need to also select
> >         any algorithms they deem mandatory.
> >
> > +config RSPDM
> > +     bool "Rust SPDM"
> > +     select CRYPTO
> > +     select KEYS
> > +     select ASYMMETRIC_KEY_TYPE
> > +     select ASYMMETRIC_PUBLIC_KEY_SUBTYPE
> > +     select X509_CERTIFICATE_PARSER
> > +     depends on SPDM = "n"
>
> I trust these last 2 diff hunks never actually go upstream, right? I.e.
> if the kernel has no SPDM today it should not plan to carry 2
> implementations just for language differences.
>
> I am not sure if that is already the plan, but the cover letter seemed
> ambiguous with its "maintaining compatibility" statement. On one hand,
> that does not make sense when this is brand new upstream code (i.e. C
> version is not even in linux-next), and there are no chances for
> regressions. Just embrace the attempt to be a Rust library for C
> consumers or otherwise help the C version along.
>
> However, if "maintain compatibility" means "make it easy for the
> work-in-progress C-effort to switch dependencies", then that makes
> sense and is worth clarifying in the next posting.

It is ambiguous, because I'm not really sure what the best approach is.

I think everyone agrees SPDM in the kernel is the way to go and there
is a work in progress C implementation.

I think that SPDM is the exact type of library that would be great to
be written in Rust. My only worry is that Rust in the kernel is still
experimental. I don't want to have a Rust SPDM implementation that
isn't used or enabled by default because Rust is still experimental,
hence the option of supporting the C or the Rust implementation.

If a Rust only implementation is acceptable, then I'm happy to switch
this series to "make it easy for the work-in-progress C-effort to
switch dependencies". At which point I can continue to support more
SPDM features here and work on a better userspace interface as
discussed at Plumbers.

Alistair

Re: [RFC 2/6] drivers: pci: Change CONFIG_SPDM to a dependency

[Jonathan Cameron]

> > diff --git a/lib/Kconfig b/lib/Kconfig
> > index 68f46e4a72a6..4db9bc8e29f8 100644
> > --- a/lib/Kconfig
> > +++ b/lib/Kconfig
> > @@ -739,6 +739,21 @@ config LWQ_TEST
> >  	help
> >            Run boot-time test of light-weight queuing.
> >  
> > +config SPDM
> > +	bool "SPDM"  
> 
> If this appears in a menuconfig or similar menu, I think expanding
> "SPDM" would be helpful to users.

Not sure it will!  Security Protocol and Data Model
which to me is completely useless for hinting what it is ;)

Definitely keep (SPDM) on end of expanded name as I suspect most
people can't remember the terms (I had to look it up ;)

Jonathan

Re: [RFC 2/6] drivers: pci: Change CONFIG_SPDM to a dependency

[Miguel Ojeda]

On Fri, Nov 22, 2024 at 4:30 PM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> Not sure it will!  Security Protocol and Data Model
> which to me is completely useless for hinting what it is ;)
>
> Definitely keep (SPDM) on end of expanded name as I suspect most
> people can't remember the terms (I had to look it up ;)

If that is the case, then perhaps it may make sense to put it at the
beginning if no one knows what the expanded form is:

    bool "SPDM (Security Protocol and Data Model)"

However, from a quick grep, this "at the beginning" style does not
seem common...

Cheers,
Miguel

Re: [RFC 2/6] drivers: pci: Change CONFIG_SPDM to a dependency

[Bjorn Helgaas]

On Fri, Nov 22, 2024 at 03:30:40PM +0000, Jonathan Cameron wrote:
> 
> > > diff --git a/lib/Kconfig b/lib/Kconfig
> > > index 68f46e4a72a6..4db9bc8e29f8 100644
> > > --- a/lib/Kconfig
> > > +++ b/lib/Kconfig
> > > @@ -739,6 +739,21 @@ config LWQ_TEST
> > >  	help
> > >            Run boot-time test of light-weight queuing.
> > >  
> > > +config SPDM
> > > +	bool "SPDM"  
> > 
> > If this appears in a menuconfig or similar menu, I think expanding
> > "SPDM" would be helpful to users.
> 
> Not sure it will!  Security Protocol and Data Model
> which to me is completely useless for hinting what it is ;)
> 
> Definitely keep (SPDM) on end of expanded name as I suspect most
> people can't remember the terms (I had to look it up ;)

Agree that the expansion doesn't add a whole lot, but I do think the
unadorned "SPDM" config prompt is not quite enough since this is in
the "Library routines" section and there's no useful context at all.

Maybe a hint that it's related to PCIe (although I'm not sure it's
*only* PCIe?) or device authentication or even some kind of general
"security" connection?

I admit that you're in good company; "parman" and "objagg" have zero
context and not even any help text at all.

Bjorn

Re: [RFC 3/6] lib: rspdm: Initial commit of Rust SPDM

[Bjorn Helgaas]

On Fri, Nov 15, 2024 at 03:46:13PM +1000, Alistair Francis wrote:
> This is the initial commit of the Rust SPDM library. It is based on and
> compatible with the C SPDM library in the kernel (lib/spdm).

> +++ b/lib/Kconfig
> @@ -754,6 +754,23 @@ config SPDM
>  	  in .config.  Drivers selecting SPDM therefore need to also select
>  	  any algorithms they deem mandatory.
>  
> +config RSPDM
> +	bool "Rust SPDM"
> +	select CRYPTO
> +	select KEYS
> +	select ASYMMETRIC_KEY_TYPE
> +	select ASYMMETRIC_PUBLIC_KEY_SUBTYPE
> +	select X509_CERTIFICATE_PARSER
> +	depends on SPDM = "n"
> +	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.  Drivers selecting SPDM therefore need to also select
> +	  any algorithms they deem mandatory.

Maybe this (and config SPDM) should be tweaked to mention drivers that
*depend* on SPDM or RSPDM, since they no longer use "select"?

PCI_CMA, which currently depends on SPDM, doesn't really look like a
"driver", so maybe it should say "users of SPDM" or "features
depending on SPDM" or something?

Re: [RFC 2/6] drivers: pci: Change CONFIG_SPDM to a dependency

[Jonathan Cameron]

On Fri, 22 Nov 2024 11:23:54 -0600
Bjorn Helgaas <helgaas@kernel.org> wrote:

> On Fri, Nov 22, 2024 at 03:30:40PM +0000, Jonathan Cameron wrote:
> >   
> > > > diff --git a/lib/Kconfig b/lib/Kconfig
> > > > index 68f46e4a72a6..4db9bc8e29f8 100644
> > > > --- a/lib/Kconfig
> > > > +++ b/lib/Kconfig
> > > > @@ -739,6 +739,21 @@ config LWQ_TEST
> > > >  	help
> > > >            Run boot-time test of light-weight queuing.
> > > >  
> > > > +config SPDM
> > > > +	bool "SPDM"    
> > > 
> > > If this appears in a menuconfig or similar menu, I think expanding
> > > "SPDM" would be helpful to users.  
> > 
> > Not sure it will!  Security Protocol and Data Model
> > which to me is completely useless for hinting what it is ;)
> > 
> > Definitely keep (SPDM) on end of expanded name as I suspect most
> > people can't remember the terms (I had to look it up ;)  
> 
> Agree that the expansion doesn't add a whole lot, but I do think the
> unadorned "SPDM" config prompt is not quite enough since this is in
> the "Library routines" section and there's no useful context at all.
> 
> Maybe a hint that it's related to PCIe (although I'm not sure it's
> *only* PCIe?) or device authentication or even some kind of general
> "security" connection?

It's much broader than PCIe (I believe originated in USB before
being standardized?)

Maybe something like...

SPDM for security related message exchange (with devices)

Attempting to distill the text in the Introduction of the spec.

"The Security Protocol and Data Model (SPDM) Specification defines messages, data objects, and sequences for
performing message exchanges over a variety of transport and physical media. The description of message
exchanges includes authentication and provisioning of hardware identities, measurement for firmware identities,
session key exchange protocols to enable confidentiality with integrity-protected data communication, and other
related capabilities. The SPDM enables efficient access to low-level security capabilities and operations. Other
mechanisms, including non-DMTF-defined mechanisms, can use the SPDM"

So clear as mud ;)

J


> 
> I admit that you're in good company; "parman" and "objagg" have zero
> context and not even any help text at all.
> 
> Bjorn

Re: [RFC 3/6] lib: rspdm: Initial commit of Rust SPDM

[Lukas Wunner]

On Fri, Nov 22, 2024 at 11:31:04AM -0600, Bjorn Helgaas wrote:
> On Fri, Nov 15, 2024 at 03:46:13PM +1000, Alistair Francis wrote:
> > +++ b/lib/Kconfig
> > @@ -754,6 +754,23 @@ config SPDM
> >  	  in .config.  Drivers selecting SPDM therefore need to also select
> >  	  any algorithms they deem mandatory.
> >  
> > +config RSPDM
> > +	bool "Rust SPDM"
> > +	select CRYPTO
> > +	select KEYS
> > +	select ASYMMETRIC_KEY_TYPE
> > +	select ASYMMETRIC_PUBLIC_KEY_SUBTYPE
> > +	select X509_CERTIFICATE_PARSER
> > +	depends on SPDM = "n"
> > +	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.  Drivers selecting SPDM therefore need to also select
> > +	  any algorithms they deem mandatory.
> 
> Maybe this (and config SPDM) should be tweaked to mention drivers that
> *depend* on SPDM or RSPDM, since they no longer use "select"?
> 
> PCI_CMA, which currently depends on SPDM, doesn't really look like a
> "driver", so maybe it should say "users of SPDM" or "features
> depending on SPDM" or something?

I anticipate that the SPDM library will eventually be used by at least
two actual drivers:  NVMe and an x86 platform driver for Intel SDSi
(Software Defined Silicon).  SCSI and ATA may follow suit.

Thus, although the PCI core may be the first user, the majority of
users will likely be actual drivers, which is why I've used that
term in the help text.

Referring to "users" instead of "drivers" may be misunderstood as
users in the sense of people using the kernel.  In particular because
the help text is seen by such users.  The terms "subsystems" or "features"
don't seem to be as clear as "drivers" IMO.

Thanks,

Lukas