Re: [PATCH v2 12/18] PCI/CMA: Expose certificates in sysfs

Dan Williams
2024-07-18T02:43:53Z

Lukas Wunner wrote:
> The kernel already caches certificate chains retrieved from a device
> upon authentication. Expose them in "slot[0-7]" files in sysfs for
> examination by user space.
>
> As noted in the ABI documentation, the "slot[0-7]" files always have a
> file size of 65535 bytes (the maximum size of a certificate chain per
> SPDM 1.0.0 table 18), even if the certificate chain in the slot is
> actually smaller. Although it would be possible to use the certifiate
> chain's actual size as the file size, doing so would require a separate
> struct attribute_group for each device, which would occupy additional
> memory.
>
> Slots are visible in sysfs even if they're currently unprovisioned
> because a future commit will add support for certificate provisioning
> by writing to the "slot[0-7]" files.
>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
> Documentation/ABI/testing/sysfs-devices-spdm | 49 ++++++++++++
> drivers/pci/pci-sysfs.c | 1 +
> include/linux/spdm.h | 1 +
> lib/spdm/req-authenticate.c | 30 +++++++-
> lib/spdm/req-sysfs.c | 80 ++++++++++++++++++++
> lib/spdm/spdm.h | 3 +
> 6 files changed, 163 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-devices-spdm b/Documentation/ABI/testing/sysfs-devices-spdm
> index 2d6e5d513231..ed61405770d6 100644
> --- a/Documentation/ABI/testing/sysfs-devices-spdm
> +++ b/Documentation/ABI/testing/sysfs-devices-spdm
> @@ -29,3 +29,52 @@ Description:
> The reason why authentication support could not be determined
> is apparent from "dmesg". To re-probe authentication support
> of PCI devices, exercise the "remove" and "rescan" attributes.
> +
> +
> +What: /sys/devices/.../certificates/
> +What: /sys/devices/.../certificates/slot[0-7]
> +Date: June 2024
> +Contact: Lukas Wunner <lukas@wunner.de>
> +Description:
> + The "certificates" directory provides access to the certificate
> + chains contained in the up to 8 slots of a device.
> +
> + A certificate chain is the concatenation of one or more ASN.1
> + DER-encoded X.509 v3 certificates (SPDM 1.0.0 sec 4.9.2.1).
> + It can be examined as follows::
> +
> + # openssl storeutl -text certificates/slot0
> +
> + A common use case is to add the first certificate in a chain
> + to the keyring of trusted root certificates (".cma" in this
> + example) after comparing its fingerprint to the one provided
> + by the device manufacturer::
> +
> + # openssl x509 -in certificates/slot0 -fingerprint -nocert
> + # openssl x509 -in certificates/slot0 -outform DER | \
> + keyctl padd asymmetric "" %:.cma
> + # echo re > authenticated
> +
> + The file size of each slot is always 65535 bytes (the maximum
> + size of a certificate chain per SPDM 1.0.0 table 18), even if
> + the certificate chain in the slot is actually smaller.
> +
> + Unprovisioned slots are represented as empty files.
> +
> + Unsupported slots (introduced by SPDM 1.3 margin no 366) are
> + not visible. If the device only supports SPDM version 1.2 or
> + earlier, all 8 slots are assumed to be supported and therefore
> + visible.
> +
> + The kernel learns which slots are supported when authenticating
> + the device for the first time. Hence, no slots are visible
> + until at least one authentication attempt has been performed.
> +
> + SPDM doesn't support on-demand retrieval of certificate chains,
> + so the kernel caches them when (re-)authenticating the device.
> + SPDM allows provisioning slots behind the kernel's back by
> + sending a SET_CERTIFICATE request through a different transport
> + (e.g. via MCTP from a Baseboard Management Controller).
> + SPDM does not specify how to notify the kernel of such events,
> + so unless reauthentication is manually initiated to update the
> + kernel's cache, the "slot[0-7]" files may contain stale data.

> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index d9e467cbec6e..a85388211104 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -1664,6 +1664,7 @@ const struct attribute_group *pci_dev_attr_groups[] = {
> #endif
> #ifdef CONFIG_PCI_CMA
> &spdm_attr_group,
> + &spdm_certificates_group,
> #endif
> NULL,
> };
> diff --git a/include/linux/spdm.h b/include/linux/spdm.h
> index 9835a3202a0e..97c7d4feab76 100644
> --- a/include/linux/spdm.h
> +++ b/include/linux/spdm.h
> @@ -35,5 +35,6 @@ int spdm_authenticate(struct spdm_state *spdm_state);
> void spdm_destroy(struct spdm_state *spdm_state);
>
> extern const struct attribute_group spdm_attr_group;
> +extern const struct attribute_group spdm_certificates_group;
>
> #endif
> diff --git a/lib/spdm/req-authenticate.c b/lib/spdm/req-authenticate.c
> index 90f7a7f2629c..1f701d07ad46 100644
> --- a/lib/spdm/req-authenticate.c
> +++ b/lib/spdm/req-authenticate.c
> @@ -14,6 +14,7 @@
> #include "spdm.h"
>
> #include <linux/dev_printk.h>
> +#include <linux/device.h>
> #include <linux/key.h>
> #include <linux/random.h>
>
> @@ -288,9 +289,9 @@ static int spdm_get_digests(struct spdm_state *spdm_state)
> struct spdm_get_digests_req *req = spdm_state->transcript_end;
> struct spdm_get_digests_rsp *rsp;
> unsigned long deprovisioned_slots;
> + u8 slot, supported_slots;
> int rc, length;
> size_t rsp_sz;
> - u8 slot;
>
> *req = (struct spdm_get_digests_req) {
> .code = SPDM_GET_DIGESTS,
> @@ -338,6 +339,33 @@ static int spdm_get_digests(struct spdm_state *spdm_state)
> return -EPROTO;
> }
>
> + /*
> + * If a bit is set in ProvisionedSlotMask, the corresponding bit in
> + * SupportedSlotMask shall also be set (SPDM 1.3.0 table 35).
> + */
> + if (spdm_state->version >= 0x13 && rsp->param2 & ~rsp->param1) {
> + dev_err(spdm_state->dev, "Malformed digests response\n");
> + return -EPROTO;
> + }
> +
> + if (spdm_state->version >= 0x13)
> + supported_slots = rsp->param1;
> + else
> + supported_slots = GENMASK(7, 0);
> +
> + if (spdm_state->supported_slots != supported_slots) {
> + spdm_state->supported_slots = supported_slots;
> +
> + if (device_is_registered(spdm_state->dev)) {
> + rc = sysfs_update_group(&spdm_state->dev->kobj,
> + &spdm_certificates_group);
> + if (rc)
> + dev_err(spdm_state->dev,
> + "Cannot update certificates in sysfs: "
> + "%d\n", rc);
> + }
> + }
> +
> return 0;
> }
>
> diff --git a/lib/spdm/req-sysfs.c b/lib/spdm/req-sysfs.c
> index 9bbed7abc153..afba3c5a2e8f 100644
> --- a/lib/spdm/req-sysfs.c
> +++ b/lib/spdm/req-sysfs.c
> @@ -93,3 +93,83 @@ 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)
> +{
> + struct device *dev = kobj_to_dev(kobj);
> + struct spdm_state *spdm_state = dev_to_spdm_state(dev);
> + u8 slot = a->attr.name[4] - '0';

This is clever, but the @n parameter already conveys the index.

> +
> + if (IS_ERR_OR_NULL(spdm_state))
> + return SYSFS_GROUP_INVISIBLE;
> +
> + if (!(spdm_state->supported_slots & BIT(slot)))
> + return 0;
> +
> + return a->attr.mode;
> +}
> +
> +static ssize_t spdm_cert_read(struct file *file, struct kobject *kobj,
> + struct bin_attribute *a, 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);
> + u8 slot = a->attr.name[4] - '0';

Similar comment on cleverness, I will note that the way this is
typically handled is something like this which is just slightly less
error prone if someone in the future changes the naming scheme.

#define CERT_ATTR(n) \
static ssize_t slot##n##_show(struct file *file, struct kobject *kobj, \
struct bin_attribute *a, char *buf, loff_t off, \
size_t count) \
{ \
return spdm_cert_read(kobj_to_dev(kobj), buf, off, count, (n)); \
} \
static BIN_ATTR_RO(slot##n);