Re: [PATCH v2 07/18] spdm: Introduce library to authenticate devices

Dan Williams
2024-07-09T05:09:57Z

Lukas Wunner wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> The Security Protocol and Data Model (SPDM) allows for device
> authentication, measurement, key exchange and encrypted sessions.
>
> SPDM was conceived by the Distributed Management Task Force (DMTF).
> Its specification defines a request/response protocol spoken between
> host and attached devices over a variety of transports:
>
> https://www.dmtf.org/dsp/DSP0274
>
> This implementation supports SPDM 1.0 through 1.3 (the latest version).
> It is designed to be transport-agnostic as the kernel already supports
> four different SPDM-capable transports:
>
> * PCIe Data Object Exchange, which is a mailbox in PCI config space
> (PCIe r6.2 sec 6.30, drivers/pci/doe.c)
> * Management Component Transport Protocol
> (MCTP, Documentation/networking/mctp.rst)
> * TCP/IP (in draft stage)
> https://www.dmtf.org/sites/default/files/standards/documents/DSP0287_1.0.0WIP99.pdf
> * SCSI and ATA (in draft stage)
> "SECURITY PROTOCOL IN/OUT" and "TRUSTED SEND/RECEIVE" commands
>
> Use cases for SPDM include, but are not limited to:
>
> * PCIe Component Measurement and Authentication (PCIe r6.2 sec 6.31)
> * Compute Express Link (CXL r3.0 sec 14.11.6)
> * Open Compute Project (Attestation of System Components v1.0)
> https://www.opencompute.org/documents/attestation-v1-0-20201104-pdf
> * Open Compute Project (Datacenter NVMe SSD Specification v2.0)
> https://www.opencompute.org/documents/datacenter-nvme-ssd-specification-v2-0r21-pdf
>
> The initial focus of this implementation is enabling PCIe CMA device
> authentication. As such, only a subset of the SPDM specification is
> contained herein, namely the request/response sequence GET_VERSION,
> GET_CAPABILITIES, NEGOTIATE_ALGORITHMS, GET_DIGESTS, GET_CERTIFICATE
> and CHALLENGE.
>
> This sequence first negotiates the SPDM protocol version, capabilities
> and algorithms with the device. It then retrieves the up to eight
> certificate chains which may be provisioned on the device. Finally it
> performs challenge-response authentication with the device using one of
> those eight certificate chains and the algorithms negotiated before.
> The challenge-response authentication comprises computing a hash over
> all exchanged messages to detect modification by a man-in-the-middle
> or media error. The hash is then signed with the device's private key
> and the resulting signature is verified by the kernel using the device's
> public key from the certificate chain. Nonces are included in the
> message sequence to protect against replay attacks.
>
> A simple API is provided for subsystems wishing to authenticate devices:
> spdm_create(), spdm_authenticate() (can be called repeatedly for
> reauthentication) and spdm_destroy(). Certificates presented by devices
> are validated against an in-kernel keyring of trusted root certificates.
> A pointer to the keyring is passed to spdm_create().
>
> The set of supported cryptographic algorithms is limited to those
> declared mandatory in PCIe r6.2 sec 6.31.3. Adding more algorithms
> is straightforward as long as the crypto subsystem supports them.
>
> Future commits will extend this implementation with support for
> measurement, key exchange and encrypted sessions.
>
> So far, only the SPDM requester role is implemented. Care was taken to
> allow for effortless addition of the responder role at a later stage.
> This could be needed for a PCIe host bridge operating in endpoint mode.
> The responder role will be able to reuse struct definitions and helpers
> such as spdm_create_combined_prefix().

Nice changelog.

>
> Credits: Jonathan wrote a proof-of-concept of this SPDM implementation.
> Lukas reworked it for upstream.
>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Co-developed-by: Lukas Wunner <lukas@wunner.de>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
> MAINTAINERS | 11 +
> include/linux/spdm.h | 33 ++
> lib/Kconfig | 15 +
> lib/Makefile | 2 +
> lib/spdm/Makefile | 10 +
> lib/spdm/core.c | 425 ++++++++++++++++++++++
> lib/spdm/req-authenticate.c | 704 ++++++++++++++++++++++++++++++++++++
> lib/spdm/spdm.h | 520 ++++++++++++++++++++++++++
> 8 files changed, 1720 insertions(+)
> create mode 100644 include/linux/spdm.h
> create mode 100644 lib/spdm/Makefile
> create mode 100644 lib/spdm/core.c
> create mode 100644 lib/spdm/req-authenticate.c
> create mode 100644 lib/spdm/spdm.h

I only have some quibbles below, but the broad strokes look good to me.

>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d6c90161c7bf..dbe16eea8818 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -20145,6 +20145,17 @@ M: Security Officers <security@kernel.org>
> S: Supported
> F: Documentation/process/security-bugs.rst
>
> +SECURITY PROTOCOL AND DATA MODEL (SPDM)
> +M: Jonathan Cameron <jic23@kernel.org>
> +M: Lukas Wunner <lukas@wunner.de>
> +L: linux-coco@lists.linux.dev
> +L: linux-cxl@vger.kernel.org
> +L: linux-pci@vger.kernel.org
> +S: Maintained
> +T: git git://git.kernel.org/pub/scm/linux/kernel/git/devsec/spdm.git
> +F: include/linux/spdm.h
> +F: lib/spdm/
> +
> SECURITY SUBSYSTEM
> M: Paul Moore <paul@paul-moore.com>
> M: James Morris <jmorris@namei.org>
> diff --git a/include/linux/spdm.h b/include/linux/spdm.h
> new file mode 100644
> index 000000000000..0da7340020c4
> --- /dev/null
> +++ b/include/linux/spdm.h
> @@ -0,0 +1,33 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * DMTF Security Protocol and Data Model (SPDM)
> + * https://www.dmtf.org/dsp/DSP0274
> + *
> + * Copyright (C) 2021-22 Huawei
> + * Jonathan Cameron <Jonathan.Cameron@huawei.com>
> + *
> + * Copyright (C) 2022-24 Intel Corporation
> + */
> +
> +#ifndef _SPDM_H_
> +#define _SPDM_H_
> +
> +#include <linux/types.h>
> +
> +struct key;
> +struct device;
> +struct spdm_state;
> +
> +typedef ssize_t (spdm_transport)(void *priv, struct device *dev,
> + const void *request, size_t request_sz,
> + void *response, size_t response_sz);
> +
> +struct spdm_state *spdm_create(struct device *dev, spdm_transport *transport,
> + void *transport_priv, u32 transport_sz,
> + struct key *keyring);
> +
> +int spdm_authenticate(struct spdm_state *spdm_state);
> +
> +void spdm_destroy(struct spdm_state *spdm_state);
> +
> +#endif
> diff --git a/lib/Kconfig b/lib/Kconfig
> index d33a268bc256..9011fa32af45 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -782,3 +782,18 @@ 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.
> diff --git a/lib/Makefile b/lib/Makefile
> index 3b1769045651..b2ef14d1fa71 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -301,6 +301,8 @@ obj-$(CONFIG_PERCPU_TEST) += percpu_test.o
> obj-$(CONFIG_ASN1) += asn1_decoder.o
> obj-$(CONFIG_ASN1_ENCODER) += asn1_encoder.o
>
> +obj-$(CONFIG_SPDM) += spdm/
> +
> obj-$(CONFIG_FONT_SUPPORT) += fonts/
>
> hostprogs := gen_crc32table
> diff --git a/lib/spdm/Makefile b/lib/spdm/Makefile
> new file mode 100644
> index 000000000000..f579cc898dbc
> --- /dev/null
> +++ b/lib/spdm/Makefile
> @@ -0,0 +1,10 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# DMTF Security Protocol and Data Model (SPDM)
> +# https://www.dmtf.org/dsp/DSP0274
> +#
> +# Copyright (C) 2024 Intel Corporation
> +
> +obj-$(CONFIG_SPDM) += spdm.o
> +
> +spdm-y := core.o req-authenticate.o
> diff --git a/lib/spdm/core.c b/lib/spdm/core.c
> new file mode 100644
> index 000000000000..f06402f6d127
> --- /dev/null
> +++ b/lib/spdm/core.c
> @@ -0,0 +1,425 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * DMTF Security Protocol and Data Model (SPDM)
> + * https://www.dmtf.org/dsp/DSP0274
> + *
> + * Core routines for message exchange, message transcript,
> + * signature verification and session state lifecycle
> + *
> + * Copyright (C) 2021-22 Huawei
> + * Jonathan Cameron <Jonathan.Cameron@huawei.com>
> + *
> + * Copyright (C) 2022-24 Intel Corporation
> + */
> +
> +#include "spdm.h"
> +
> +#include <linux/dev_printk.h>
> +#include <linux/module.h>
> +
> +#include <crypto/hash.h>
> +#include <crypto/public_key.h>
> +
> +static int spdm_err(struct device *dev, struct spdm_error_rsp *rsp)

This approach to error singnaling looks unprecedented, and not in a good
way. I like the idea of a SPDM-error-code to errno converter, and
separate SPDM-error-code to error string, but not an SPDM-error-code to
errno conversion that has a log emitting side effect.

Would this not emit ambiguous messages like:

cxl_pci 0000:35:00.0: Unexpected request

How does I know that error message is from CXL SPDM functionality and
not some other part of the driver?

What if the SPDM authentication is optional, how does the consumer of
this library avoid log spam? What about rate limiting?

Did you consider leaving all error logging to the caller?

I have less problem if these all become dev_dbg() or tracepoints, but
dev_err() seems awkward.

> +{
> + switch (rsp->error_code) {
> + case SPDM_INVALID_REQUEST:
> + dev_err(dev, "Invalid request\n");
> + return -EINVAL;
> + case SPDM_INVALID_SESSION:
> + if (rsp->version == 0x11) {
> + dev_err(dev, "Invalid session %#x\n", rsp->error_data);
> + return -EINVAL;
> + }
> + break;
> + case SPDM_BUSY:
> + dev_err(dev, "Busy\n");
> + return -EBUSY;
> + case SPDM_UNEXPECTED_REQUEST:
> + dev_err(dev, "Unexpected request\n");
> + return -EINVAL;
> + case SPDM_UNSPECIFIED:
> + dev_err(dev, "Unspecified error\n");
> + return -EINVAL;
> + case SPDM_DECRYPT_ERROR:
> + dev_err(dev, "Decrypt error\n");
> + return -EIO;
> + case SPDM_UNSUPPORTED_REQUEST:
> + dev_err(dev, "Unsupported request %#x\n", rsp->error_data);
> + return -EINVAL;
> + case SPDM_REQUEST_IN_FLIGHT:
> + dev_err(dev, "Request in flight\n");
> + return -EINVAL;
> + case SPDM_INVALID_RESPONSE_CODE:
> + dev_err(dev, "Invalid response code\n");
> + return -EINVAL;
> + case SPDM_SESSION_LIMIT_EXCEEDED:
> + dev_err(dev, "Session limit exceeded\n");
> + return -EBUSY;
> + case SPDM_SESSION_REQUIRED:
> + dev_err(dev, "Session required\n");
> + return -EINVAL;
> + case SPDM_RESET_REQUIRED:
> + dev_err(dev, "Reset required\n");
> + return -ECONNRESET;
> + case SPDM_RESPONSE_TOO_LARGE:
> + dev_err(dev, "Response too large\n");
> + return -EINVAL;
> + case SPDM_REQUEST_TOO_LARGE:
> + dev_err(dev, "Request too large\n");
> + return -EINVAL;
> + case SPDM_LARGE_RESPONSE:
> + dev_err(dev, "Large response\n");
> + return -EMSGSIZE;
> + case SPDM_MESSAGE_LOST:
> + dev_err(dev, "Message lost\n");
> + return -EIO;
> + case SPDM_INVALID_POLICY:
> + dev_err(dev, "Invalid policy\n");
> + return -EINVAL;
> + case SPDM_VERSION_MISMATCH:
> + dev_err(dev, "Version mismatch\n");
> + return -EINVAL;
> + case SPDM_RESPONSE_NOT_READY:
> + dev_err(dev, "Response not ready\n");
> + return -EINPROGRESS;
> + case SPDM_REQUEST_RESYNCH:
> + dev_err(dev, "Request resynchronization\n");
> + return -ECONNRESET;
> + case SPDM_OPERATION_FAILED:
> + dev_err(dev, "Operation failed\n");
> + return -EINVAL;
> + case SPDM_NO_PENDING_REQUESTS:
> + return -ENOENT;
> + case SPDM_VENDOR_DEFINED_ERROR:
> + dev_err(dev, "Vendor defined error\n");
> + return -EINVAL;
> + }
> +
> + dev_err(dev, "Undefined error %#x\n", rsp->error_code);
> + return -EINVAL;
> +}
> +
> +/**
> + * 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.
> + */
> +ssize_t spdm_exchange(struct spdm_state *spdm_state,
> + void *req, size_t req_sz, void *rsp, size_t rsp_sz)
> +{
> + struct spdm_header *request = req;
> + struct spdm_header *response = rsp;
> + ssize_t rc, length;

What's the locking assumption with this public function? I see that the
internal to this file usages wrap it in the state lock. Should that
assumption be codified with a:

lockdep_assert_held(&spdm_state->lock)

?

Or can this function be marked static?

> +
> + if (req_sz < sizeof(struct spdm_header) ||
> + rsp_sz < sizeof(struct spdm_header))
> + return -EINVAL;
> +
> + request->version = spdm_state->version;
> +
> + rc = spdm_state->transport(spdm_state->transport_priv, spdm_state->dev,
> + req, req_sz, rsp, rsp_sz);
> + if (rc < 0)
> + return rc;
> +
> + length = rc;
> + if (length < sizeof(struct spdm_header))
> + return length; /* Truncated response is handled by callers */
> +
> + if (response->code == SPDM_ERROR)
> + return spdm_err(spdm_state->dev, (struct spdm_error_rsp *)rsp);
> +
> + if (response->code != (request->code & ~SPDM_REQ)) {
> + dev_err(spdm_state->dev,
> + "Response code %#x does not match request code %#x\n",
> + response->code, request->code);
> + return -EPROTO;
> + }
> +
> + return length;
> +}
> +
> +/**
> + * spdm_alloc_transcript() - Allocate transcript buffer
> + *
> + * @spdm_state: SPDM session state
> + *
> + * Allocate a buffer to accommodate the concatenation of all SPDM messages
> + * exchanged during an authentication sequence. Used to verify the signature,
> + * as it is computed over the hashed transcript.
> + *
> + * Transcript size is initially one page. It grows by additional pages as
> + * needed. Minimum size of an authentication sequence is 1k (only one slot
> + * occupied, only one ECC P256 certificate in chain, SHA 256 hash selected).
> + * Maximum can be several MBytes. Between 4k and 64k is probably typical.
> + *
> + * Return 0 on success or a negative errno.
> + */
> +int spdm_alloc_transcript(struct spdm_state *spdm_state)
> +{
> + spdm_state->transcript = kvmalloc(PAGE_SIZE, GFP_KERNEL);
> + if (!spdm_state->transcript)
> + return -ENOMEM;
> +
> + spdm_state->transcript_end = spdm_state->transcript;
> + spdm_state->transcript_max = PAGE_SIZE;
> +
> + return 0;
> +}
> +
> +/**
> + * spdm_free_transcript() - Free transcript buffer
> + *
> + * @spdm_state: SPDM session state
> + *
> + * Free the transcript buffer after performing authentication. Reset the
> + * pointer to the current end of transcript as well as the allocation size.
> + */
> +void spdm_free_transcript(struct spdm_state *spdm_state)
> +{
> + kvfree(spdm_state->transcript);
> + spdm_state->transcript_end = NULL;
> + spdm_state->transcript_max = 0;

Similar locking context with public functions concern, but also the
cleverness of why it is ok to not set ->transcript to NULL that really
only hold true if spdm_append_transcript() and and
spdm_free_transcript() are guaranteed to be serialized.

> +}
> +
> +/**
> + * spdm_append_transcript() - Append a message to transcript buffer
> + *
> + * @spdm_state: SPDM session state
> + * @msg: SPDM message
> + * @msg_sz: Size of @msg
> + *
> + * Append an SPDM message to the transcript after reception or transmission.
> + * Reallocate a larger transcript buffer if the message exceeds its current
> + * allocation size.
> + *
> + * If the message to be appended is known to fit into the allocation size,
> + * it may be directly received into or transmitted from the transcript buffer
> + * instead of calling this function: Simply use the @transcript_end pointer in
> + * struct spdm_state as the position to store the message, then advance the
> + * pointer by the message size.
> + *
> + * Return 0 on success or a negative errno.
> + */
> +int spdm_append_transcript(struct spdm_state *spdm_state,
> + const void *msg, size_t msg_sz)
> +{
> + size_t transcript_sz = spdm_state->transcript_end -
> + spdm_state->transcript;
> +
> + if (transcript_sz + msg_sz > spdm_state->transcript_max) {
> + size_t new_sz = round_up(transcript_sz + msg_sz, PAGE_SIZE);
> + void *new = kvrealloc(spdm_state->transcript,
> + spdm_state->transcript_max,
> + new_sz, GFP_KERNEL);
> + if (!new)
> + return -ENOMEM;
> +
> + spdm_state->transcript = new;
> + spdm_state->transcript_end = new + transcript_sz;
> + spdm_state->transcript_max = new_sz;
> + }
> +
> + memcpy(spdm_state->transcript_end, msg, msg_sz);
> + spdm_state->transcript_end += msg_sz;
> +
> + return 0;
> +}
> +
> +/**
> + * spdm_create_combined_prefix() - Create combined_spdm_prefix for a hash
> + *
> + * @version: SPDM version negotiated during GET_VERSION exchange
> + * @spdm_context: SPDM context of signature generation (or verification)
> + * @buf: Buffer to receive combined_spdm_prefix (100 bytes)
> + *
> + * From SPDM 1.2, a hash is prefixed with the SPDM version and context before
> + * a signature is generated (or verified) over the resulting concatenation
> + * (SPDM 1.2.0 section 15). Create that prefix.
> + */
> +void spdm_create_combined_prefix(u8 version, const char *spdm_context,
> + void *buf)
> +{
> + u8 major = FIELD_GET(0xf0, version);
> + u8 minor = FIELD_GET(0x0f, version);
> + size_t len = strlen(spdm_context);
> + int rc, zero_pad;
> +
> + rc = snprintf(buf, SPDM_PREFIX_SZ + 1,
> + "dmtf-spdm-v%hhx.%hhx.*dmtf-spdm-v%hhx.%hhx.*"
> + "dmtf-spdm-v%hhx.%hhx.*dmtf-spdm-v%hhx.%hhx.*",
> + major, minor, major, minor, major, minor, major, minor);
> + WARN_ON(rc != SPDM_PREFIX_SZ);

Does this need a dynamic runtime check?

> +
> + zero_pad = SPDM_COMBINED_PREFIX_SZ - SPDM_PREFIX_SZ - 1 - len;
> + WARN_ON(zero_pad < 0);

Worth crashing the system here for panic_on_warn folks?

> +
> + memset(buf + SPDM_PREFIX_SZ + 1, 0, zero_pad);
> + memcpy(buf + SPDM_PREFIX_SZ + 1 + zero_pad, spdm_context, len);
> +}
> +
> +/**
> + * spdm_verify_signature() - Verify signature against leaf key
> + *
> + * @spdm_state: SPDM session state
> + * @spdm_context: SPDM context (used to create combined_spdm_prefix)
> + *
> + * Implementation of the abstract SPDMSignatureVerify() function described in
> + * SPDM 1.2.0 section 16: Compute the hash over @spdm_state->transcript and
> + * verify that the signature at the end of the transcript was generated by
> + * @spdm_state->leaf_key. Hashing the entire transcript allows detection
> + * of message modification by a man-in-the-middle or media error.
> + *
> + * Return 0 on success or a negative errno.
> + */
> +int spdm_verify_signature(struct spdm_state *spdm_state,
> + const char *spdm_context)
> +{
> + struct public_key_signature sig = {
> + .s = spdm_state->transcript_end - spdm_state->sig_len,
> + .s_size = spdm_state->sig_len,
> + .encoding = spdm_state->base_asym_enc,
> + .hash_algo = spdm_state->base_hash_alg_name,
> + };
> + u8 *mhash __free(kfree) = NULL;
> + u8 *m __free(kfree);
> + int rc;
> +
> + m = kmalloc(SPDM_COMBINED_PREFIX_SZ + spdm_state->hash_len, GFP_KERNEL);

I am not a fan of forward declared scoped-based resource management
variables [1], although PeterZ never responded to those suggestions.

[1]: http://lore.kernel.org/171175585714.2192972.12661675876300167762.stgit@dwillia2-xfh.jf.intel.com

> + if (!m)
> + return -ENOMEM;
> +
> + /* Hash the transcript (sans trailing signature) */
> + rc = crypto_shash_digest(spdm_state->desc, spdm_state->transcript,
> + (void *)sig.s - spdm_state->transcript,
> + m + SPDM_COMBINED_PREFIX_SZ);
> + if (rc)
> + return rc;
> +
> + if (spdm_state->version <= 0x11) {
> + /*
> + * SPDM 1.0 and 1.1 compute the signature only over the hash
> + * (SPDM 1.0.0 section 4.9.2.7).
> + */
> + sig.digest = m + SPDM_COMBINED_PREFIX_SZ;
> + sig.digest_size = spdm_state->hash_len;
> + } else {
> + /*
> + * From SPDM 1.2, the hash is prefixed with spdm_context before
> + * computing the signature over the resulting message M
> + * (SPDM 1.2.0 sec 15).
> + */
> + spdm_create_combined_prefix(spdm_state->version, spdm_context,
> + m);
> +
> + /*
> + * RSA and ECDSA algorithms require that M is hashed once more.
> + * EdDSA and SM2 algorithms omit that step.
> + * The switch statement prepares for their introduction.
> + */
> + switch (spdm_state->base_asym_alg) {
> + default:
> + mhash = kmalloc(spdm_state->hash_len, GFP_KERNEL);
> + if (!mhash)
> + return -ENOMEM;
> +
> + rc = crypto_shash_digest(spdm_state->desc, m,
> + SPDM_COMBINED_PREFIX_SZ + spdm_state->hash_len,
> + mhash);
> + if (rc)
> + return rc;
> +
> + sig.digest = mhash;
> + sig.digest_size = spdm_state->hash_len;
> + break;
> + }
> + }
> +
> + return public_key_verify_signature(spdm_state->leaf_key, &sig);
> +}
> +
> +/**
> + * spdm_reset() - Free cryptographic data structures
> + *
> + * @spdm_state: SPDM session state
> + *
> + * Free cryptographic data structures when an SPDM session is destroyed or
> + * when the device is reauthenticated.
> + */
> +void spdm_reset(struct spdm_state *spdm_state)
> +{
> + public_key_free(spdm_state->leaf_key);
> + spdm_state->leaf_key = NULL;
> +
> + kfree(spdm_state->desc);
> + spdm_state->desc = NULL;
> +
> + crypto_free_shash(spdm_state->shash);
> + spdm_state->shash = NULL;
> +}
> +
> +/**
> + * 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
> + *
> + * Return a pointer to the allocated SPDM session state or NULL on error.
> + */
> +struct spdm_state *spdm_create(struct device *dev, spdm_transport *transport,
> + void *transport_priv, u32 transport_sz,
> + struct key *keyring)
> +{
> + struct spdm_state *spdm_state = kzalloc(sizeof(*spdm_state), GFP_KERNEL);
> +
> + if (!spdm_state)
> + return NULL;
> +
> + spdm_state->dev = dev;
> + spdm_state->transport = transport;
> + spdm_state->transport_priv = transport_priv;
> + spdm_state->transport_sz = transport_sz;
> + spdm_state->root_keyring = keyring;
> +
> + mutex_init(&spdm_state->lock);
> +
> + return spdm_state;
> +}
> +EXPORT_SYMBOL_GPL(spdm_create);
> +
> +/**
> + * spdm_destroy() - Destroy SPDM session
> + *
> + * @spdm_state: SPDM session state
> + */
> +void spdm_destroy(struct spdm_state *spdm_state)
> +{
> + u8 slot;
> +
> + for_each_set_bit(slot, &spdm_state->provisioned_slots, SPDM_SLOTS)
> + kvfree(spdm_state->slot[slot]);
> +
> + spdm_reset(spdm_state);
> + mutex_destroy(&spdm_state->lock);
> + kfree(spdm_state);
> +}
> +EXPORT_SYMBOL_GPL(spdm_destroy);
> +
> +MODULE_LICENSE("GPL");
> diff --git a/lib/spdm/req-authenticate.c b/lib/spdm/req-authenticate.c
> new file mode 100644
> index 000000000000..51fdb88f519b
> --- /dev/null
> +++ b/lib/spdm/req-authenticate.c
[..]
> +/**
> + * 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.
> + */
> +int spdm_authenticate(struct spdm_state *spdm_state)
> +{
> + u8 slot;
> + int rc;
> +
> + mutex_lock(&spdm_state->lock);
> + spdm_reset(spdm_state);
> +
> + rc = spdm_alloc_transcript(spdm_state);
> + if (rc)
> + goto unlock;
> +
> + rc = spdm_get_version(spdm_state);
> + if (rc)
> + goto unlock;
> +
> + rc = spdm_get_capabilities(spdm_state);
> + if (rc)
> + goto unlock;
> +
> + rc = spdm_negotiate_algs(spdm_state);
> + if (rc)
> + goto unlock;
> +
> + rc = spdm_get_digests(spdm_state);
> + if (rc)
> + goto unlock;
> +
> + for_each_set_bit(slot, &spdm_state->provisioned_slots, SPDM_SLOTS) {
> + rc = spdm_get_certificate(spdm_state, slot);
> + if (rc)
> + goto unlock;
> + }
> +
> + for_each_set_bit(slot, &spdm_state->provisioned_slots, SPDM_SLOTS) {
> + rc = spdm_validate_cert_chain(spdm_state, slot);
> + if (rc == 0)
> + break;
> + }
> + if (rc)
> + goto unlock;
> +
> + rc = spdm_challenge(spdm_state, slot);
> +
> +unlock:
> + if (rc)
> + spdm_reset(spdm_state);

The above looks like it is asking for an __spdm_authenticate helper.

int spdm_authenticate(struct spdm_state *spdm_state)
{
guard(mutex)(&spdm_state->lock);
rc = __spdm_authenticate(spdm_state);
if (rc)
spdm_reset(spdm_state);
spdm_state->authenticated = !rc;
spdm_free_transcript(spdm_state);
return rc;
}

Otherwise, looks good to me.