Re: [PATCH v2 18/18] spdm: Allow control of next requester nonce through sysfs

Jonathan Cameron
2024-07-18T16:11:52Z

On Sun, 30 Jun 2024 21:53:00 +0200
Lukas Wunner <lukas@wunner.de> wrote:

> Remote attestation services may mistrust the kernel to always use a
> fresh nonce for SPDM authentication.
>
> So allow user space to set the next requester nonce by writing to a
> sysfs attribute.
>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> Cc: Jérôme Glisse <jglisse@google.com>
> Cc: Jason Gunthorpe <jgg@nvidia.com>
Why is the group visibility callback in this patch?

Otherwise looks fine to me,

Jonathan

> ---
> Documentation/ABI/testing/sysfs-devices-spdm | 29 ++++++++++++++++
> lib/spdm/core.c | 1 +
> lib/spdm/req-authenticate.c | 8 ++++-
> lib/spdm/req-sysfs.c | 35 ++++++++++++++++++++
> lib/spdm/spdm.h | 4 +++
> 5 files changed, 76 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-devices-spdm b/Documentation/ABI/testing/sysfs-devices-spdm
> index 5ce34ce10b9c..d315b47b4af0 100644
> --- a/Documentation/ABI/testing/sysfs-devices-spdm
> +++ b/Documentation/ABI/testing/sysfs-devices-spdm
> @@ -216,3 +216,32 @@ Description:
> necessary to parse the SPDM messages in the transcript to find
> and extract the nonces, which is cumbersome. That's why they
> are exposed as separate files.
> +
> +
> +What: /sys/devices/.../signatures/next_requester_nonce
> +Date: June 2024
> +Contact: Lukas Wunner <lukas@wunner.de>
> +Description:
> + If you do not trust the kernel to always use a fresh nonce,
> + write 32 bytes to this file to set the requester nonce used
> + in the next SPDM authentication sequence.
> +
> + Meant for remote attestation services. You are responsible
> + for providing a nonce with sufficient entropy. The kernel
> + only uses the nonce once, so provide a new one every time
> + you reauthenticate the device. If you do not provide a
> + nonce, the kernel generates a random one.
> +
> + After the nonce has been consumed, it becomes readable as
> + the newest [0-9]*_requester_nonce, which proves its usage::
> +
> + # dd if=/dev/random bs=32 count=1 | \
> + tee signatures/next_requester_nonce | hexdump
> + 0000000 e0 77 91 54 bd 56 99 c2 ea 4f 0b 1a 7f ba 6e 59
> + 0000010 8f ee f6 b2 26 82 58 34 9e e5 8c 8a 31 58 29 7e
> +
> + # echo re > authenticated
> +
> + # hexdump $(\ls -t signatures/[0-9]*_requester_nonce | head -1)
> + 0000000 e0 77 91 54 bd 56 99 c2 ea 4f 0b 1a 7f ba 6e 59
> + 0000010 8f ee f6 b2 26 82 58 34 9e e5 8c 8a 31 58 29 7e
> diff --git a/lib/spdm/core.c b/lib/spdm/core.c
> index b6a46bdbb2f9..7371adb7a52f 100644
> --- a/lib/spdm/core.c
> +++ b/lib/spdm/core.c
> @@ -434,6 +434,7 @@ void spdm_destroy(struct spdm_state *spdm_state)
> spdm_reset(spdm_state);
> spdm_destroy_log(spdm_state);
> mutex_destroy(&spdm_state->lock);
> + kfree(spdm_state->next_nonce);
> kfree(spdm_state);
> }
> EXPORT_SYMBOL_GPL(spdm_destroy);
> diff --git a/lib/spdm/req-authenticate.c b/lib/spdm/req-authenticate.c
> index 7c977f5835c1..489fc88de74d 100644
> --- a/lib/spdm/req-authenticate.c
> +++ b/lib/spdm/req-authenticate.c
> @@ -626,7 +626,13 @@ static int spdm_challenge(struct spdm_state *spdm_state, u8 slot, bool verify)
> };
> int rc, length;
>
> - get_random_bytes(&req.nonce, sizeof(req.nonce));
> + if (spdm_state->next_nonce) {
> + memcpy(&req.nonce, spdm_state->next_nonce, sizeof(req.nonce));
> + kfree(spdm_state->next_nonce);
> + spdm_state->next_nonce = NULL;
> + } else {
> + get_random_bytes(&req.nonce, sizeof(req.nonce));
> + }
>
> if (spdm_state->version <= 0x12)
> req_sz = offsetofend(typeof(req), nonce);
> diff --git a/lib/spdm/req-sysfs.c b/lib/spdm/req-sysfs.c
> index c782054f8e18..232d4a00a510 100644
> --- a/lib/spdm/req-sysfs.c
> +++ b/lib/spdm/req-sysfs.c
> @@ -176,13 +176,48 @@ const struct attribute_group spdm_certificates_group = {
>
> /* 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);
> + struct spdm_state *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)
> +{
> + struct device *dev = kobj_to_dev(kobj);
> + struct spdm_state *spdm_state = dev_to_spdm_state(dev);
> +
> + guard(mutex)(&spdm_state->lock);
> +
> + if (!spdm_state->next_nonce) {
> + spdm_state->next_nonce = kmalloc(SPDM_NONCE_SZ, GFP_KERNEL);
> + if (!spdm_state->next_nonce)
> + return -ENOMEM;
> + }
> +
> + memcpy(spdm_state->next_nonce + off, buf, count);
> + return count;
> +}
> +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,
> };
>
> static unsigned int spdm_max_log_sz = SZ_16M; /* per device */
> diff --git a/lib/spdm/spdm.h b/lib/spdm/spdm.h
> index 448107c92db7..aa36aa55e718 100644
> --- a/lib/spdm/spdm.h
> +++ b/lib/spdm/spdm.h
> @@ -475,6 +475,9 @@ struct spdm_error_rsp {
> * itself and the transcript with trailing signature.
> * @log_counter: Number of generated log entries so far. Will be prefixed to
> * the sysfs files of the next generated log entry.
> + * @next_nonce: Requester nonce to be used for the next authentication
> + * sequence. Populated from user space through sysfs.
> + * If user space does not provide a nonce, the kernel uses a random one.
> */
> struct spdm_state {
> struct device *dev;
> @@ -521,6 +524,7 @@ struct spdm_state {
> struct list_head log;
> size_t log_sz;
> u32 log_counter;
> + u8 *next_nonce;
> };
>
> extern struct list_head spdm_state_list;