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

[Dan Williams <dan.j.williams@intel.com>]

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);