One of the common operations of a TSM (Trusted Security Module) is to
provide a way for a TVM (confidential computing guest execution
environment) to take a measurement of its launch state, sign it and
submit it to a verifying party. Upon successful attestation that
verifies the integrity of the TVM additional secrets may be deployed.
The concept is common across TSMs, but the implementations are
unfortunately vendor specific. While the industry grapples with a common
definition of this attestation format [1], Linux need not make this
problem worse by defining a new ABI per TSM that wants to perform a
similar operation. The current momentum has been to invent new ioctl-ABI
per TSM per function which at best is an abdication of the kernel's
responsibility to make common infrastructure concepts share common ABI.
The proposal, targeted to conceptually work with TDX, SEV, COVE if not
more, is to define a sysfs interface to retrieve the TSM-specific blob.
echo $hex_encoded_userdata_plus_nonce > /sys/class/tsm/tsm0/inhex
hexdump /sys/class/tsm/tsm0/outblob
This approach later allows for the standardization of the attestation
blob format without needing to change the Linux ABI. Until then, the
format of 'outblob' is determined by the parent device for 'tsm0'.
The expectation is that this is a boot time exchange that need not be
regenerated, making it amenable to a sysfs interface. In case userspace
does try to generate multiple attestation reports it includes conflict
detection so userspace can be sure no other thread changed the
parameters from its last configuration step to the blob retrieval.
TSM specific options are encoded as 'extra' attributes on the TSM device
with the expectation that vendors reuse the same options for similar
concepts. The current options are defined by SEV-SNP's need for a
'privilege level' concept (VMPL), and the option to retrieve a
certificate chain in addition to the attestation report ("extended"
format).
Link: http://lore.kernel.org/r/64961c3baf8ce_142af829436@dwillia2-xfh.jf.intel.com.notmuch [1]
Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Cc: Dionna Amalie Glaze <dionnaglaze@google.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Peter Gonda <pgonda@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Samuel Ortiz <sameo@rivosinc.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
Documentation/ABI/testing/sysfs-class-tsm | 47 +++++
MAINTAINERS | 8 +
drivers/virt/coco/Kconfig | 4
drivers/virt/coco/Makefile | 1
drivers/virt/coco/tdx-guest/Kconfig | 1
drivers/virt/coco/tsm.c | 290 +++++++++++++++++++++++++++++
include/linux/tsm.h | 45 +++++
7 files changed, 396 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-class-tsm
create mode 100644 drivers/virt/coco/tsm.c
create mode 100644 include/linux/tsm.h
diff --git a/Documentation/ABI/testing/sysfs-class-tsm b/Documentation/ABI/testing/sysfs-class-tsm
new file mode 100644
index 000000000000..37017bde626d
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-tsm
@@ -0,0 +1,47 @@
+What: /sys/class/tsm/tsm0/inhex
+Date: August, 2023
+KernelVersion: v6.6
+Contact: linux-cxl@vger.kernel.org
+Description:
+ (RW) Hex encoded userdata to be included in the attestation
+ report. For replay protection this should include a nonce, but
+ the kernel does not place any restrictions on the content.
+
+What: /sys/class/tsm/tsm0/outblob
+Date: August, 2023
+KernelVersion: v6.6
+Contact: linux-cxl@vger.kernel.org
+Description:
+ (RO) Binary attestation report generated from @inhex translated
+ to binary and any options. The format of the report is vendor
+ specific and determined by the parent device of 'tsm0'.
+
+What: /sys/class/tsm/tsm0/generation
+Date: August, 2023
+KernelVersion: v6.6
+Contact: linux-cxl@vger.kernel.org
+Description:
+ (RO) The value in this attribute increments each time @inhex or
+ any option is written. Userspace can detect conflicts by
+ checking generation before writing to any attribute and making
+ sure the number of writes matches expectations after reading
+ @outblob.
+
+What: /sys/class/tsm/tsm0/privlevel
+Date: August, 2023
+KernelVersion: v6.6
+Contact: linux-cxl@vger.kernel.org
+Description:
+ (RW) If a TSM implementation supports the concept of attestation
+ reports for TVMs running at different privilege levels, like
+ SEV-SNP "VMPL", specify the privilege level via this attribute.
+
+What: /sys/class/tsm/tsm0/format
+Date: August, 2023
+KernelVersion: v6.6
+Contact: linux-cxl@vger.kernel.org
+Description:
+ (RW) If a TSM implementation supports the concept of attestation
+ reports with "extended" contents, like SEV-SNP extended reports
+ with certificate chains, specify "extended" vs "default" via
+ this attribute.
diff --git a/MAINTAINERS b/MAINTAINERS
index 3be1bdfe8ecc..97f74d344c8a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21625,6 +21625,14 @@ W: https://github.com/srcres258/linux-doc
T: git git://github.com/srcres258/linux-doc.git doc-zh-tw
F: Documentation/translations/zh_TW/
+TRUSTED SECURITY MODULE (TSM) ATTESTATION REPORTS
+M: Dan Williams <dan.j.williams@intel.com>
+L: linux-coco@lists.linux.dev
+S: Maintained
+F: Documentation/ABI/testing/sysfs-class-tsm
+F: drivers/virt/coco/tsm.c
+F: include/linux/tsm.h
+
TTY LAYER
M: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
M: Jiri Slaby <jirislaby@kernel.org>
diff --git a/drivers/virt/coco/Kconfig b/drivers/virt/coco/Kconfig
index fc5c64f04c4a..d92f07019f38 100644
--- a/drivers/virt/coco/Kconfig
+++ b/drivers/virt/coco/Kconfig
@@ -2,6 +2,10 @@
#
# Confidential computing related collateral
#
+
+config TSM_REPORTS
+ tristate
+
source "drivers/virt/coco/efi_secret/Kconfig"
source "drivers/virt/coco/sev-guest/Kconfig"
diff --git a/drivers/virt/coco/Makefile b/drivers/virt/coco/Makefile
index 55302ef719ad..18c1aba5edb7 100644
--- a/drivers/virt/coco/Makefile
+++ b/drivers/virt/coco/Makefile
@@ -2,6 +2,7 @@
#
# Confidential computing related collateral
#
+obj-$(CONFIG_TSM_REPORTS) += tsm.o
obj-$(CONFIG_EFI_SECRET) += efi_secret/
obj-$(CONFIG_SEV_GUEST) += sev-guest/
obj-$(CONFIG_INTEL_TDX_GUEST) += tdx-guest/
diff --git a/drivers/virt/coco/tdx-guest/Kconfig b/drivers/virt/coco/tdx-guest/Kconfig
index 14246fc2fb02..22dd59e19431 100644
--- a/drivers/virt/coco/tdx-guest/Kconfig
+++ b/drivers/virt/coco/tdx-guest/Kconfig
@@ -1,6 +1,7 @@
config TDX_GUEST_DRIVER
tristate "TDX Guest driver"
depends on INTEL_TDX_GUEST
+ select TSM_REPORTS
help
The driver provides userspace interface to communicate with
the TDX module to request the TDX guest details like attestation
diff --git a/drivers/virt/coco/tsm.c b/drivers/virt/coco/tsm.c
new file mode 100644
index 000000000000..1bf2ee82eb94
--- /dev/null
+++ b/drivers/virt/coco/tsm.c
@@ -0,0 +1,290 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright(c) 2023 Intel Corporation. All rights reserved. */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/tsm.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/rwsem.h>
+#include <linux/device.h>
+#include <linux/string.h>
+#include <linux/module.h>
+#include <linux/cleanup.h>
+
+struct class *tsm_class;
+static struct tsm_provider {
+ const struct tsm_ops *ops;
+ struct device *dev;
+} provider;
+static DECLARE_RWSEM(tsm_rwsem);
+
+/**
+ * DOC: Trusted Security Module (TSM) Attestation Report Interface
+ *
+ * The TSM report interface is a common provider of blobs that facilitate
+ * attestation of a TVM (confidential computing guest) by an attestation
+ * service. A TSM report combines a user-defined blob (likely a public-key with
+ * a nonce for a key-exchange protocol) with a signed attestation report. That
+ * combined blob is then used to obtain secrets provided by an agent that can
+ * validate the attestation report. The expectation is that this interface is
+ * invoked infrequently, likely only once at TVM boot time.
+ *
+ * The attestation report format is TSM provider specific, when / if a standard
+ * materializes that can be published instead of the vendor layout.
+ */
+
+/**
+ * struct tsm_report - track state of report generation relative to options
+ * @desc: report generation options / cached report state
+ * @outblob: generated evidence to provider to the attestation agent
+ * @outblob_len: sizeof(outblob)
+ * @write_generation: conflict detection, and report regeneration tracking
+ * @read_generation: cached report invalidation tracking
+ */
+struct tsm_report {
+ struct tsm_desc desc;
+ size_t outblob_len;
+ u8 *outblob;
+ unsigned long write_generation;
+ unsigned long read_generation;
+} tsm_report;
+
+static ssize_t privlevel_store(struct device *dev,
+ struct device_attribute *attr, const char *buf,
+ size_t len)
+{
+ unsigned int val;
+ int rc;
+
+ rc = kstrtouint(buf, 0, &val);
+ if (rc)
+ return rc;
+
+ guard(rwsem_write)(&tsm_rwsem);
+ if (tsm_report.desc.privlevel == val)
+ return len;
+ tsm_report.desc.privlevel = val;
+ tsm_report.write_generation++;
+
+ return len;
+}
+
+static ssize_t privlevel_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "%u\n", tsm_report.desc.privlevel);
+}
+
+static DEVICE_ATTR_RW(privlevel);
+
+static ssize_t format_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ enum tsm_format format;
+
+ if (sysfs_streq(buf, "default"))
+ format = TSM_FORMAT_DEFAULT;
+ else if (sysfs_streq(buf, "extended"))
+ format = TSM_FORMAT_EXTENDED;
+ else
+ return -EINVAL;
+
+ guard(rwsem_write)(&tsm_rwsem);
+ if (tsm_report.desc.outblob_format == format)
+ return len;
+ tsm_report.desc.outblob_format = format;
+ tsm_report.write_generation++;
+
+ return len;
+}
+
+static ssize_t format_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ if (tsm_report.desc.outblob_format == TSM_FORMAT_DEFAULT)
+ return sysfs_emit(buf, "default\n");
+ return sysfs_emit(buf, "extended\n");
+}
+
+static DEVICE_ATTR_RW(format);
+
+static struct attribute *tsm_extra_attributes[] = {
+ &dev_attr_format.attr,
+ &dev_attr_privlevel.attr,
+ NULL,
+};
+
+struct attribute_group tsm_extra_attribute_group = {
+ .attrs = tsm_extra_attributes,
+};
+EXPORT_SYMBOL_GPL(tsm_extra_attribute_group);
+
+/*
+ * Input is a small hex blob, rather than a writable binary attribute, so that
+ * it is conveyed atomically.
+ */
+static ssize_t inhex_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ u8 inblob[TSM_INBLOB_MAX];
+ size_t inblob_len;
+ int rc;
+
+ inblob_len = len;
+ if (buf[len - 1] == '\n')
+ inblob_len--;
+ if (inblob_len & 1)
+ return -EINVAL;
+ inblob_len /= 2;
+ if (inblob_len > TSM_INBLOB_MAX)
+ return -EINVAL;
+
+ rc = hex2bin(inblob, buf, inblob_len);
+ if (rc < 0)
+ return rc;
+
+ guard(rwsem_write)(&tsm_rwsem);
+ if (memcmp(tsm_report.desc.inblob, inblob, inblob_len) == 0)
+ return len;
+ memcpy(tsm_report.desc.inblob, inblob, inblob_len);
+ tsm_report.desc.inblob_len = inblob_len;
+ tsm_report.write_generation++;
+
+ return len;
+}
+
+static ssize_t inhex_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ char *end;
+
+ guard(rwsem_read)(&tsm_rwsem);
+ if (!tsm_report.desc.inblob_len)
+ return 0;
+ end = bin2hex(buf, tsm_report.desc.inblob, tsm_report.desc.inblob_len);
+ *end++ = '\n';
+ return end - buf;
+}
+static DEVICE_ATTR_RW(inhex);
+
+static ssize_t generation_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ guard(rwsem_read)(&tsm_rwsem);
+ return sysfs_emit(buf, "%lu\n", tsm_report.write_generation);
+}
+static DEVICE_ATTR_RO(generation);
+
+static struct attribute *tsm_attributes[] = {
+ &dev_attr_inhex.attr,
+ &dev_attr_generation.attr,
+ NULL,
+};
+
+static ssize_t outblob_read(struct file *f, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t offset, size_t count)
+{
+ guard(rwsem_read)(&tsm_rwsem);
+ if (!tsm_report.desc.inblob_len)
+ return -EINVAL;
+
+ if (!tsm_report.outblob ||
+ tsm_report.read_generation != tsm_report.write_generation) {
+ const struct tsm_ops *ops = provider.ops;
+ size_t outblob_len;
+ u8 *outblob;
+
+ kvfree(tsm_report.outblob);
+ outblob = ops->report_new(provider.dev->parent,
+ &tsm_report.desc, &outblob_len);
+ if (IS_ERR(outblob))
+ return PTR_ERR(outblob);
+ tsm_report.outblob_len = outblob_len;
+ tsm_report.outblob = outblob;
+ tsm_report.read_generation = tsm_report.write_generation;
+ }
+
+ return memory_read_from_buffer(buf, count, &offset,
+ tsm_report.outblob,
+ tsm_report.outblob_len);
+}
+static BIN_ATTR_RO(outblob, 0);
+
+static struct bin_attribute *tsm_bin_attributes[] = {
+ &bin_attr_outblob,
+ NULL,
+};
+
+struct attribute_group tsm_default_attribute_group = {
+ .bin_attrs = tsm_bin_attributes,
+ .attrs = tsm_attributes,
+};
+EXPORT_SYMBOL_GPL(tsm_default_attribute_group);
+
+static const struct attribute_group *tsm_default_attribute_groups[] = {
+ &tsm_default_attribute_group,
+ NULL,
+};
+
+int register_tsm(const struct tsm_ops *ops, struct device *parent,
+ const struct attribute_group **groups)
+{
+ const struct tsm_ops *conflict;
+ struct device *dev;
+ int rc;
+
+ if (!parent)
+ return -EINVAL;
+
+ if (!groups)
+ groups = tsm_default_attribute_groups;
+
+ guard(rwsem_write)(&tsm_rwsem);
+ conflict = provider.ops;
+ if (conflict) {
+ pr_err("\"%s\" ops already registered\n", conflict->name);
+ return rc;
+ }
+
+ dev = device_create_with_groups(tsm_class, parent, 0, NULL, groups,
+ "tsm0");
+ if (IS_ERR(dev))
+ return PTR_ERR(dev);
+
+ provider.ops = ops;
+ provider.dev = dev;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(register_tsm);
+
+int unregister_tsm(const struct tsm_ops *ops)
+{
+ guard(rwsem_write)(&tsm_rwsem);
+ if (ops != provider.ops)
+ return -EBUSY;
+ provider.ops = NULL;
+ device_unregister(provider.dev);
+ provider.dev = NULL;
+ kvfree(tsm_report.outblob);
+ tsm_report.outblob = NULL;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(unregister_tsm);
+
+static int __init tsm_init(void)
+{
+ tsm_class = class_create("tsm");
+ return PTR_ERR_OR_ZERO(tsm_class);
+}
+module_init(tsm_init);
+
+static void __exit tsm_exit(void)
+{
+ class_destroy(tsm_class);
+}
+module_exit(tsm_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Provide Trusted Security Module attestation reports via sysfs");
diff --git a/include/linux/tsm.h b/include/linux/tsm.h
new file mode 100644
index 000000000000..6dc2f07543b8
--- /dev/null
+++ b/include/linux/tsm.h
@@ -0,0 +1,45 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __TSM_H
+#define __TSM_H
+
+#include <linux/types.h>
+#include <linux/device.h>
+
+#define TSM_INBLOB_MAX 64
+
+enum tsm_format {
+ TSM_FORMAT_DEFAULT,
+ TSM_FORMAT_EXTENDED,
+};
+
+/**
+ * struct tsm_desc - option descriptor for generating tsm report blobs
+ * @privlevel: optional privilege level to associate with @outblob
+ * @inblob_len: sizeof @inblob
+ * @inblob: arbitrary input data
+ * @outblob_format: for TSMs with an "extended" format
+ */
+struct tsm_desc {
+ unsigned int privlevel;
+ size_t inblob_len;
+ u8 inblob[TSM_INBLOB_MAX];
+ enum tsm_format outblob_format;
+};
+
+/*
+ * arch specific ops, only one is expected to be registered at a time
+ * i.e. only one of SEV, TDX, COVE, etc.
+ */
+struct tsm_ops {
+ const char *name;
+ u8 *(*report_new)(struct device *dev, const struct tsm_desc *desc,
+ size_t *outblob_len);
+};
+
+extern struct attribute_group tsm_default_attribute_group;
+extern struct attribute_group tsm_extra_attribute_group;
+
+int register_tsm(const struct tsm_ops *ops, struct device *parent,
+ const struct attribute_group **groups);
+int unregister_tsm(const struct tsm_ops *ops);
+#endif /* __TSM_H */Hi Dan,
On Mon, Aug 14, 2023 at 12:43:21AM -0700, Dan Williams wrote:
> One of the common operations of a TSM (Trusted Security Module) is to
> provide a way for a TVM (confidential computing guest execution
> environment) to take a measurement of its launch state, sign it and
> submit it to a verifying party. Upon successful attestation that
> verifies the integrity of the TVM additional secrets may be deployed.
> The concept is common across TSMs, but the implementations are
> unfortunately vendor specific. While the industry grapples with a common
> definition of this attestation format [1], Linux need not make this
> problem worse by defining a new ABI per TSM that wants to perform a
> similar operation. The current momentum has been to invent new ioctl-ABI
> per TSM per function which at best is an abdication of the kernel's
> responsibility to make common infrastructure concepts share common ABI.
>
> The proposal, targeted to conceptually work with TDX, SEV, COVE if not
> more, is to define a sysfs interface to retrieve the TSM-specific blob.
>
> echo $hex_encoded_userdata_plus_nonce > /sys/class/tsm/tsm0/inhex
> hexdump /sys/class/tsm/tsm0/outblob
My concern with that interface is that one could easily get an
attestation report with a nonce set by another userspace component or
thread. I realize there is a generation counter to detect if a
configuration changed between the caller's last config setting and the
report it got, but I think that this shows that this may not be the best
interface. IMHO an attestation report request from userspace should be
an atomic call that includes multiple platform independent attibutes
like e.g. an attestation nonce.
> This approach later allows for the standardization of the attestation
> blob format without needing to change the Linux ABI. Until then, the
> format of 'outblob' is determined by the parent device for 'tsm0'.
>
> The expectation is that this is a boot time exchange that need not be
> regenerated,
This works well with the encrypted boot disk that's decrypted through
attestation use-case, but this is just one use case. Although I don't
expect attestation requests to be frequent, we should not assume this is
only a boot time operation. Not only it can happen after the guest is
fully booted, but it can also happen multiple times. An attestation flow
where a guest gets an attestation token back from a validated report is
something we'd want to support. Those token's validity are time limited,
and userspace would want to regenerate a report, with a fresh,
attestation service provided nonce.
Another thing to keep in mind is that an attestation report could be
amended by userspace itself, for TEE that support runtime measurement
(The RTMR things...). So the TVM measurement itself could change during
the lifecycle of a TVM.
> making it amenable to a sysfs interface. In case userspace
> does try to generate multiple attestation reports it includes conflict
> detection so userspace can be sure no other thread changed the
> parameters from its last configuration step to the blob retrieval.
>
> TSM specific options are encoded as 'extra' attributes on the TSM device
> with the expectation that vendors reuse the same options for similar
> concepts. The current options are defined by SEV-SNP's need for a
> 'privilege level' concept (VMPL), and the option to retrieve a
> certificate chain in addition to the attestation report ("extended"
> format).
>
> Link: http://lore.kernel.org/r/64961c3baf8ce_142af829436@dwillia2-xfh.jf.intel.com.notmuch [1]
> Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> Cc: Dionna Amalie Glaze <dionnaglaze@google.com>
> Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> Cc: Peter Gonda <pgonda@google.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Samuel Ortiz <sameo@rivosinc.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> Documentation/ABI/testing/sysfs-class-tsm | 47 +++++
> MAINTAINERS | 8 +
> drivers/virt/coco/Kconfig | 4
> drivers/virt/coco/Makefile | 1
> drivers/virt/coco/tdx-guest/Kconfig | 1
> drivers/virt/coco/tsm.c | 290 +++++++++++++++++++++++++++++
> include/linux/tsm.h | 45 +++++
> 7 files changed, 396 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-class-tsm
> create mode 100644 drivers/virt/coco/tsm.c
> create mode 100644 include/linux/tsm.h
>
> diff --git a/Documentation/ABI/testing/sysfs-class-tsm b/Documentation/ABI/testing/sysfs-class-tsm
> new file mode 100644
> index 000000000000..37017bde626d
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-tsm
> @@ -0,0 +1,47 @@
> +What: /sys/class/tsm/tsm0/inhex
> +Date: August, 2023
> +KernelVersion: v6.6
> +Contact: linux-cxl@vger.kernel.org
Did you mean linux-coco@ ?
> +Description:
> + (RW) Hex encoded userdata to be included in the attestation
> + report. For replay protection this should include a nonce, but
> + the kernel does not place any restrictions on the content.
> +
> +What: /sys/class/tsm/tsm0/outblob
> +Date: August, 2023
> +KernelVersion: v6.6
> +Contact: linux-cxl@vger.kernel.org
> +Description:
> + (RO) Binary attestation report generated from @inhex translated
> + to binary and any options. The format of the report is vendor
> + specific and determined by the parent device of 'tsm0'.
> +
> +What: /sys/class/tsm/tsm0/generation
> +Date: August, 2023
> +KernelVersion: v6.6
> +Contact: linux-cxl@vger.kernel.org
> +Description:
> + (RO) The value in this attribute increments each time @inhex or
> + any option is written. Userspace can detect conflicts by
> + checking generation before writing to any attribute and making
> + sure the number of writes matches expectations after reading
> + @outblob.
One note here is that an attestation userspace thread could get an
invalid counter although the report is valid, if e.g. a separate thread
modified a config between the first thread report generation and it
reading the counter back. So an attestation report generating thread
could get false negatives with that interface.
Cheers,
Samuel.
On Wed, Aug 23, 2023 at 03:49:17PM +0200, Samuel Ortiz wrote: Good morning, I hope the week is starting well for everyone. Some further background to hopefully assist decision making on how to handle the export of attestation information for COCO TSM's. > Hi Dan, > > On Mon, Aug 14, 2023 at 12:43:21AM -0700, Dan Williams wrote: > > One of the common operations of a TSM (Trusted Security Module) is to > > provide a way for a TVM (confidential computing guest execution > > environment) to take a measurement of its launch state, sign it and > > submit it to a verifying party. Upon successful attestation that > > verifies the integrity of the TVM additional secrets may be deployed. > > The concept is common across TSMs, but the implementations are > > unfortunately vendor specific. While the industry grapples with a common > > definition of this attestation format [1], Linux need not make this > > problem worse by defining a new ABI per TSM that wants to perform a > > similar operation. The current momentum has been to invent new ioctl-ABI > > per TSM per function which at best is an abdication of the kernel's > > responsibility to make common infrastructure concepts share common ABI. > > > > The proposal, targeted to conceptually work with TDX, SEV, COVE if not > > more, is to define a sysfs interface to retrieve the TSM-specific blob. > > > > echo $hex_encoded_userdata_plus_nonce > /sys/class/tsm/tsm0/inhex > > hexdump /sys/class/tsm/tsm0/outblob > My concern with that interface is that one could easily get an > attestation report with a nonce set by another userspace component > or thread. I realize there is a generation counter to detect if a > configuration changed between the caller's last config setting and > the report it got, but I think that this shows that this may not be > the best interface. IMHO an attestation report request from > userspace should be an atomic call that includes multiple platform > independent attibutes like e.g. an attestation nonce. The challenge in all of this would seem to be the need to get a 20+ year ABI 'right' on the first try, so some issues to consider based on our experiences building trusted systems, albeit with a focus on endpoint devices. The issue about needing to implement attestation atomically is on point. This requirement would seem to suggest that the optimum path forward for confidential computing will be to have attestation support implemented in the resource orchestration infrastructure inside the TVM. Since the goal is confidentiality, you need to trust everything that executes inside the execution domain/VM. This is particularly true given the reality of micro-architectural side channel issues, which don't seem to be going away and which Intel explicitly documents that you must address with TDX. Given that, there would seem to be no reason not to have the attestation support all in one place and then provide access to that functionality to anything in the TVM that would want to make an integrity statement to a relying party. A sysfs interface is perfectly suited to the this model, since the trust orchestrator, if we use our terminology, can address the concurrency and atomicity issues presented by a sysfs interface. From an Intel/SGX centric perspective, which is our area of expertise when it comes to the notion of a TSM, the attestation process is reasonably complex, even more so in a TDX environment, since you need to go outside the trusted execution domain in order to conduct the full attestation process. I don't know how many people have direct experience with all of this, but we can offer it on good authority that the COCO model will benefit from having all the necessary procedures done correctly and in one place > > This approach later allows for the standardization of the > > attestation blob format without needing to change the Linux > > ABI. Until then, the format of 'outblob' is determined by the > > parent device for 'tsm0'. > > > > The expectation is that this is a boot time exchange that need not > > be regenerated, > This works well with the encrypted boot disk that's decrypted > through attestation use-case, but this is just one use > case. Although I don't expect attestation requests to be frequent, > we should not assume this is only a boot time operation. Not only it > can happen after the guest is fully booted, but it can also happen > multiple times. An attestation flow where a guest gets an > attestation token back from a validated report is something we'd > want to support. Those token's validity are time limited, and > userspace would want to regenerate a report, with a fresh, > attestation service provided nonce. There would certainly seem to be no argument that a trusted execution domain may want to conduct multiple attestations, Jeremi suggested this is something the Confidential Containers initiative would want as well. The model of having attestation support centralized in the resource orchestration infrastructure is not only consistent with that need but would also appear to be the optimal approach. Let us consider TDX as an example, since it will undoubtedly be a major player in COCO. In the Data Center Attestation Primitives (DCAP) model, one needs to choregraph a dance between the Provisioning Certification Enclave (PCE) and the Quoting Enclave (QE) in order to conduct the attestation process. That requires access to SGX functionality, something that is not supported by Intel in a TDX mediated virtual machine. This imposes a need to go outside of the TD guest, to the host, in order to get access to SGX in order to load and run the PCE and QE enclaves. The call into the kernel to access the hypervisor interface is simply the starting point for the attestation process. The purpose is to have the TDX module generate a CPU specific report (SEAMREPORT) that will ultimately authenticate material, usually ECDH key exchange components, as coming from an execution domain with known provenance. That report then needs to be processed through the PCE and QE enclave infrastructure in order to fully prove the identity of the report and thus authenticate its contents. Mixed into all of this is a need to verify the MROWNER and MRSIGNER signatures on the involved enclaves as well as the Security Version Numbers (SVN's) on the enclaves and the TDX infrastructure. This process ends up authenticating a certificate chain which proves that a secret to be conveyed into the TD is actually being injected into a valid TD, with known provenance and one that is 'Genuine Intel' in origin. All of this doesn't seem to speak of infrastructure you would want to replicate multiple times over. > Another thing to keep in mind is that an attestation report could be > amended by userspace itself, for TEE that support runtime > measurement (The RTMR things...). So the TVM measurement itself > could change during the lifecycle of a TVM. Once again, speaking only to TDX, it isn't apparent that the RTMR registers are going to be useful for the task of confirming general application level integrity once the OS is started. Their primary utility will be to provide a root of trust for the resource orchestration infrastructure and in turn VTPM's, or something like TSEM's root security modeling namespace, if the goal is to provide a foundation for generalized trust/integrity mechanisms inside of the TVM. To wit: The formal usage recommendations for the TDX RTMR registers are as follows: RTMR[0]: PCR[1,7] RTMR[1]: PCR[4,5] RTMR[2]: PCR[8,15] RTMR[3]: Special use Where RTMR[2] is specified for usage by the OS/application for measurement purposes, so only the equivalent of one PCR is available. This would automatically preclude the use of systemd in a 'trusted' VM implementation, given that the latest Linux TPM PCR registry has systemd consuming PCR registers 11-15 for its own use. Once again, if you are really serious about confidentiality, you need to have an attestable integrity guarantee for everything that happens inside the trusted VM. IMA uses register 10 if you go that route and we will be using a register above what systemd is using for the TSEM root namespace. So we believe the lone trusted virtual machine measurement register will need to stay constant, in order to easily validate whatever root of trust that is in turn designated to guarantee the integrity of what goes on inside of the TVM from an OS/application perspective. A functional attestation scheme is thus going to require not only attestation of the state of the TVM but also what has gone on inside of the VM after it has booted. Another reason for having a single shopping site for the attestation needs of whatever runs inside of the trusted VM. FWIW, since we copied the LSM list on this, we haven't come by the design and implementation of TSEM and the notion of trust orchestration idly. If COCO is going to become a relevant reality there needs to be consideration given to the concept of trust orchestraton and management. Once gain, FWIW, and something that Jeremi from Microsoft might be able to comment on. It is unclear how COCO based Confidential Containers fits into the standard IMA attestation model, where one has to review an event log documented by a linear extension trust measurement to determine whether or not the system is to be trusted. At a minimum, this would require that a relying party for attestation of one container be able to verify what has gone on in all other container invocations. Unless the containers are being run for only a single entity, this would seem to imply a violation of the notion of privacy and confidentiality. > Cheers, > Samuel. Hopefully all of the above is of assistance in getting the ABI right and the scope of issues involved. Best wishes for a productive week. As always, Dr. Greg The Quixote Project - Flailing at the Travails of Cybersecurity
On Mon, 2023-08-14 at 00:43 -0700, Dan Williams wrote:
> One of the common operations of a TSM (Trusted Security Module) is to
> provide a way for a TVM (confidential computing guest execution
> environment) to take a measurement of its launch state, sign it and
> submit it to a verifying party. Upon successful attestation that
> verifies the integrity of the TVM additional secrets may be deployed.
> The concept is common across TSMs, but the implementations are
> unfortunately vendor specific. While the industry grapples with a common
> definition of this attestation format [1], Linux need not make this
> problem worse by defining a new ABI per TSM that wants to perform a
> similar operation. The current momentum has been to invent new ioctl-ABI
> per TSM per function which at best is an abdication of the kernel's
> responsibility to make common infrastructure concepts share common ABI.
>
> The proposal, targeted to conceptually work with TDX, SEV, COVE if not
> more, is to define a sysfs interface to retrieve the TSM-specific blob.
>
> echo $hex_encoded_userdata_plus_nonce > /sys/class/tsm/tsm0/inhex
> hexdump /sys/class/tsm/tsm0/outblob
>
> This approach later allows for the standardization of the attestation
> blob format without needing to change the Linux ABI. Until then, the
> format of 'outblob' is determined by the parent device for 'tsm0'.
>
> The expectation is that this is a boot time exchange that need not be
> regenerated, making it amenable to a sysfs interface. In case userspace
> does try to generate multiple attestation reports it includes conflict
> detection so userspace can be sure no other thread changed the
> parameters from its last configuration step to the blob retrieval.
>
> TSM specific options are encoded as 'extra' attributes on the TSM device
> with the expectation that vendors reuse the same options for similar
> concepts. The current options are defined by SEV-SNP's need for a
> 'privilege level' concept (VMPL), and the option to retrieve a
> certificate chain in addition to the attestation report ("extended"
> format).
>
> Link:
> http://lore.kernel.org/r/64961c3baf8ce_142af829436@dwillia2-xfh.jf.intel.com.notmuch
> [1]
> Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> Cc: Dionna Amalie Glaze <dionnaglaze@google.com>
> Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> Cc: Peter Gonda <pgonda@google.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Samuel Ortiz <sameo@rivosinc.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> Documentation/ABI/testing/sysfs-class-tsm | 47 +++++
> MAINTAINERS | 8 +
> drivers/virt/coco/Kconfig | 4
> drivers/virt/coco/Makefile | 1
> drivers/virt/coco/tdx-guest/Kconfig | 1
> drivers/virt/coco/tsm.c | 290
> +++++++++++++++++++++++++++++
> include/linux/tsm.h | 45 +++++
> 7 files changed, 396 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-class-tsm
> create mode 100644 drivers/virt/coco/tsm.c
> create mode 100644 include/linux/tsm.h
> +static ssize_t outblob_read(struct file *f, struct kobject *kobj,
> + struct bin_attribute *bin_attr, char *buf,
> + loff_t offset, size_t count)
> +{
> + guard(rwsem_read)(&tsm_rwsem);
> + if (!tsm_report.desc.inblob_len)
> + return -EINVAL;
> +
> + if (!tsm_report.outblob ||
> + tsm_report.read_generation != tsm_report.write_generation) {
> + const struct tsm_ops *ops = provider.ops;
> + size_t outblob_len;
> + u8 *outblob;
> +
> + kvfree(tsm_report.outblob);
I think tsm_report.outblob needs to be set to NULL here otherwise there is
the possibility of a double-free if ops->report_new returns an error.
Roy
> + outblob = ops->report_new(provider.dev->parent,
> + &tsm_report.desc, &outblob_len);
> + if (IS_ERR(outblob))
> + return PTR_ERR(outblob);
> + tsm_report.outblob_len = outblob_len;
> + tsm_report.outblob = outblob;
> + tsm_report.read_generation = tsm_report.write_generation;
> + }
> +
> + return memory_read_from_buffer(buf, count, &offset,
> + tsm_report.outblob,
> + tsm_report.outblob_len);
> +}
On 8/14/23 02:43, Dan Williams wrote:
> One of the common operations of a TSM (Trusted Security Module) is to
> provide a way for a TVM (confidential computing guest execution
> environment) to take a measurement of its launch state, sign it and
> submit it to a verifying party. Upon successful attestation that
> verifies the integrity of the TVM additional secrets may be deployed.
> The concept is common across TSMs, but the implementations are
> unfortunately vendor specific. While the industry grapples with a common
> definition of this attestation format [1], Linux need not make this
> problem worse by defining a new ABI per TSM that wants to perform a
> similar operation. The current momentum has been to invent new ioctl-ABI
> per TSM per function which at best is an abdication of the kernel's
> responsibility to make common infrastructure concepts share common ABI.
>
> The proposal, targeted to conceptually work with TDX, SEV, COVE if not
> more, is to define a sysfs interface to retrieve the TSM-specific blob.
>
> echo $hex_encoded_userdata_plus_nonce > /sys/class/tsm/tsm0/inhex
> hexdump /sys/class/tsm/tsm0/outblob
>
> This approach later allows for the standardization of the attestation
> blob format without needing to change the Linux ABI. Until then, the
> format of 'outblob' is determined by the parent device for 'tsm0'.
>
> The expectation is that this is a boot time exchange that need not be
> regenerated, making it amenable to a sysfs interface. In case userspace
> does try to generate multiple attestation reports it includes conflict
> detection so userspace can be sure no other thread changed the
> parameters from its last configuration step to the blob retrieval.
>
> TSM specific options are encoded as 'extra' attributes on the TSM device
> with the expectation that vendors reuse the same options for similar
> concepts. The current options are defined by SEV-SNP's need for a
> 'privilege level' concept (VMPL), and the option to retrieve a
> certificate chain in addition to the attestation report ("extended"
> format).
>
> Link: http://lore.kernel.org/r/64961c3baf8ce_142af829436@dwillia2-xfh.jf.intel.com.notmuch [1]
> Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> Cc: Dionna Amalie Glaze <dionnaglaze@google.com>
> Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> Cc: Peter Gonda <pgonda@google.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Samuel Ortiz <sameo@rivosinc.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> Documentation/ABI/testing/sysfs-class-tsm | 47 +++++
> MAINTAINERS | 8 +
> drivers/virt/coco/Kconfig | 4
> drivers/virt/coco/Makefile | 1
> drivers/virt/coco/tdx-guest/Kconfig | 1
> drivers/virt/coco/tsm.c | 290 +++++++++++++++++++++++++++++
> include/linux/tsm.h | 45 +++++
> 7 files changed, 396 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-class-tsm
> create mode 100644 drivers/virt/coco/tsm.c
> create mode 100644 include/linux/tsm.h
>
> +static ssize_t privlevel_store(struct device *dev,
> + struct device_attribute *attr, const char *buf,
> + size_t len)
> +{
> + unsigned int val;
> + int rc;
> +
> + rc = kstrtouint(buf, 0, &val);
> + if (rc)
> + return rc;
> +
> + guard(rwsem_write)(&tsm_rwsem);
> + if (tsm_report.desc.privlevel == val)
> + return len;
> + tsm_report.desc.privlevel = val;
> + tsm_report.write_generation++;
So I'm wondering if this use of write_generation helps or not. Since it
isn't incremented if the levels are the same, that might present race
conditions.
What if user A requests vmpl 2 and privlevel is already 2, then
write_generation is not incremented. But before user A can read back the
generation value user B can request vmpl 3 and cause write_generation to
be incremented.
This may not be a problem for VMPL, since that can be checked in the
returned attestation report, but it could be for the report format. If the
extended format is requested but changed to default, then the additional
certs might not be returned and the guest may think there aren't any...?
Maybe incrementing the write_generation no matter what is best.
Thanks,
Tom
> +
> + return len;
> +}
> +
> +static ssize_t privlevel_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + return sysfs_emit(buf, "%u\n", tsm_report.desc.privlevel);
> +}
> +
> +static DEVICE_ATTR_RW(privlevel);
> +
> +static ssize_t format_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + enum tsm_format format;
> +
> + if (sysfs_streq(buf, "default"))
> + format = TSM_FORMAT_DEFAULT;
> + else if (sysfs_streq(buf, "extended"))
> + format = TSM_FORMAT_EXTENDED;
> + else
> + return -EINVAL;
> +
> + guard(rwsem_write)(&tsm_rwsem);
> + if (tsm_report.desc.outblob_format == format)
> + return len;
> + tsm_report.desc.outblob_format = format;
> + tsm_report.write_generation++;
> +
> + return len;
> +}
> +
> +static ssize_t format_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + if (tsm_report.desc.outblob_format == TSM_FORMAT_DEFAULT)
> + return sysfs_emit(buf, "default\n");
> + return sysfs_emit(buf, "extended\n");
> +}
> +
> +static DEVICE_ATTR_RW(format);
> +
> +static struct attribute *tsm_extra_attributes[] = {
> + &dev_attr_format.attr,
> + &dev_attr_privlevel.attr,
> + NULL,
> +};
> +
> +struct attribute_group tsm_extra_attribute_group = {
> + .attrs = tsm_extra_attributes,
> +};
> +EXPORT_SYMBOL_GPL(tsm_extra_attribute_group);
> +
> +/*
> + * Input is a small hex blob, rather than a writable binary attribute, so that
> + * it is conveyed atomically.
> + */
> +static ssize_t inhex_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + u8 inblob[TSM_INBLOB_MAX];
> + size_t inblob_len;
> + int rc;
> +
> + inblob_len = len;
> + if (buf[len - 1] == '\n')
> + inblob_len--;
> + if (inblob_len & 1)
> + return -EINVAL;
> + inblob_len /= 2;
> + if (inblob_len > TSM_INBLOB_MAX)
> + return -EINVAL;
> +
> + rc = hex2bin(inblob, buf, inblob_len);
> + if (rc < 0)
> + return rc;
> +
> + guard(rwsem_write)(&tsm_rwsem);
> + if (memcmp(tsm_report.desc.inblob, inblob, inblob_len) == 0)
> + return len;
> + memcpy(tsm_report.desc.inblob, inblob, inblob_len);
> + tsm_report.desc.inblob_len = inblob_len;
> + tsm_report.write_generation++;
> +
> + return len;
> +}
> +
> +static ssize_t inhex_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + char *end;
> +
> + guard(rwsem_read)(&tsm_rwsem);
> + if (!tsm_report.desc.inblob_len)
> + return 0;
> + end = bin2hex(buf, tsm_report.desc.inblob, tsm_report.desc.inblob_len);
> + *end++ = '\n';
> + return end - buf;
> +}
> +static DEVICE_ATTR_RW(inhex);
> +
> +static ssize_t generation_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + guard(rwsem_read)(&tsm_rwsem);
> + return sysfs_emit(buf, "%lu\n", tsm_report.write_generation);
> +}
> +static DEVICE_ATTR_RO(generation);
> +
> +static struct attribute *tsm_attributes[] = {
> + &dev_attr_inhex.attr,
> + &dev_attr_generation.attr,
> + NULL,
> +};
> +
> +static ssize_t outblob_read(struct file *f, struct kobject *kobj,
> + struct bin_attribute *bin_attr, char *buf,
> + loff_t offset, size_t count)
> +{
> + guard(rwsem_read)(&tsm_rwsem);
> + if (!tsm_report.desc.inblob_len)
> + return -EINVAL;
> +
> + if (!tsm_report.outblob ||
> + tsm_report.read_generation != tsm_report.write_generation) {
> + const struct tsm_ops *ops = provider.ops;
> + size_t outblob_len;
> + u8 *outblob;
> +
> + kvfree(tsm_report.outblob);
> + outblob = ops->report_new(provider.dev->parent,
> + &tsm_report.desc, &outblob_len);
> + if (IS_ERR(outblob))
> + return PTR_ERR(outblob);
> + tsm_report.outblob_len = outblob_len;
> + tsm_report.outblob = outblob;
> + tsm_report.read_generation = tsm_report.write_generation;
> + }
> +
> + return memory_read_from_buffer(buf, count, &offset,
> + tsm_report.outblob,
> + tsm_report.outblob_len);
> +}
> +static BIN_ATTR_RO(outblob, 0);
> +
> +static struct bin_attribute *tsm_bin_attributes[] = {
> + &bin_attr_outblob,
> + NULL,
> +};
> +
> +struct attribute_group tsm_default_attribute_group = {
> + .bin_attrs = tsm_bin_attributes,
> + .attrs = tsm_attributes,
> +};
> +EXPORT_SYMBOL_GPL(tsm_default_attribute_group);
> +
> +static const struct attribute_group *tsm_default_attribute_groups[] = {
> + &tsm_default_attribute_group,
> + NULL,
> +};
> +
> +int register_tsm(const struct tsm_ops *ops, struct device *parent,
> + const struct attribute_group **groups)
> +{
> + const struct tsm_ops *conflict;
> + struct device *dev;
> + int rc;
> +
> + if (!parent)
> + return -EINVAL;
> +
> + if (!groups)
> + groups = tsm_default_attribute_groups;
> +
> + guard(rwsem_write)(&tsm_rwsem);
> + conflict = provider.ops;
> + if (conflict) {
> + pr_err("\"%s\" ops already registered\n", conflict->name);
> + return rc;
> + }
> +
> + dev = device_create_with_groups(tsm_class, parent, 0, NULL, groups,
> + "tsm0");
> + if (IS_ERR(dev))
> + return PTR_ERR(dev);
> +
> + provider.ops = ops;
> + provider.dev = dev;
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(register_tsm);
> +
> +int unregister_tsm(const struct tsm_ops *ops)
> +{
> + guard(rwsem_write)(&tsm_rwsem);
> + if (ops != provider.ops)
> + return -EBUSY;
> + provider.ops = NULL;
> + device_unregister(provider.dev);
> + provider.dev = NULL;
> + kvfree(tsm_report.outblob);
> + tsm_report.outblob = NULL;
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(unregister_tsm);
> +
> +static int __init tsm_init(void)
> +{
> + tsm_class = class_create("tsm");
> + return PTR_ERR_OR_ZERO(tsm_class);
> +}
> +module_init(tsm_init);
> +
> +static void __exit tsm_exit(void)
> +{
> + class_destroy(tsm_class);
> +}
> +module_exit(tsm_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Provide Trusted Security Module attestation reports via sysfs");
> diff --git a/include/linux/tsm.h b/include/linux/tsm.h
> new file mode 100644
> index 000000000000..6dc2f07543b8
> --- /dev/null
> +++ b/include/linux/tsm.h
> @@ -0,0 +1,45 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __TSM_H
> +#define __TSM_H
> +
> +#include <linux/types.h>
> +#include <linux/device.h>
> +
> +#define TSM_INBLOB_MAX 64
> +
> +enum tsm_format {
> + TSM_FORMAT_DEFAULT,
> + TSM_FORMAT_EXTENDED,
> +};
> +
> +/**
> + * struct tsm_desc - option descriptor for generating tsm report blobs
> + * @privlevel: optional privilege level to associate with @outblob
> + * @inblob_len: sizeof @inblob
> + * @inblob: arbitrary input data
> + * @outblob_format: for TSMs with an "extended" format
> + */
> +struct tsm_desc {
> + unsigned int privlevel;
> + size_t inblob_len;
> + u8 inblob[TSM_INBLOB_MAX];
> + enum tsm_format outblob_format;
> +};
> +
> +/*
> + * arch specific ops, only one is expected to be registered at a time
> + * i.e. only one of SEV, TDX, COVE, etc.
> + */
> +struct tsm_ops {
> + const char *name;
> + u8 *(*report_new)(struct device *dev, const struct tsm_desc *desc,
> + size_t *outblob_len);
> +};
> +
> +extern struct attribute_group tsm_default_attribute_group;
> +extern struct attribute_group tsm_extra_attribute_group;
> +
> +int register_tsm(const struct tsm_ops *ops, struct device *parent,
> + const struct attribute_group **groups);
> +int unregister_tsm(const struct tsm_ops *ops);
> +#endif /* __TSM_H */
>
>
Tom Lendacky wrote:
> > +static ssize_t privlevel_store(struct device *dev,
> > + struct device_attribute *attr, const char *buf,
> > + size_t len)
> > +{
> > + unsigned int val;
> > + int rc;
> > +
> > + rc = kstrtouint(buf, 0, &val);
> > + if (rc)
> > + return rc;
> > +
> > + guard(rwsem_write)(&tsm_rwsem);
> > + if (tsm_report.desc.privlevel == val)
> > + return len;
> > + tsm_report.desc.privlevel = val;
> > + tsm_report.write_generation++;
>
> So I'm wondering if this use of write_generation helps or not. Since it
> isn't incremented if the levels are the same, that might present race
> conditions.
>
> What if user A requests vmpl 2 and privlevel is already 2, then
> write_generation is not incremented. But before user A can read back the
> generation value user B can request vmpl 3 and cause write_generation to
> be incremented.
>
> This may not be a problem for VMPL, since that can be checked in the
> returned attestation report, but it could be for the report format. If the
> extended format is requested but changed to default, then the additional
> certs might not be returned and the guest may think there aren't any...?
>
> Maybe incrementing the write_generation no matter what is best.
True, and good eye. If write_generation does not always increment once
per write there is no way to assume the state of the parameters. Will
fix.
On 8/14/23 02:43, Dan Williams wrote:
> One of the common operations of a TSM (Trusted Security Module) is to
> provide a way for a TVM (confidential computing guest execution
> environment) to take a measurement of its launch state, sign it and
> submit it to a verifying party. Upon successful attestation that
> verifies the integrity of the TVM additional secrets may be deployed.
> The concept is common across TSMs, but the implementations are
> unfortunately vendor specific. While the industry grapples with a common
> definition of this attestation format [1], Linux need not make this
> problem worse by defining a new ABI per TSM that wants to perform a
> similar operation. The current momentum has been to invent new ioctl-ABI
> per TSM per function which at best is an abdication of the kernel's
> responsibility to make common infrastructure concepts share common ABI.
>
> The proposal, targeted to conceptually work with TDX, SEV, COVE if not
> more, is to define a sysfs interface to retrieve the TSM-specific blob.
>
> echo $hex_encoded_userdata_plus_nonce > /sys/class/tsm/tsm0/inhex
> hexdump /sys/class/tsm/tsm0/outblob
>
> This approach later allows for the standardization of the attestation
> blob format without needing to change the Linux ABI. Until then, the
> format of 'outblob' is determined by the parent device for 'tsm0'.
>
> The expectation is that this is a boot time exchange that need not be
> regenerated, making it amenable to a sysfs interface. In case userspace
> does try to generate multiple attestation reports it includes conflict
> detection so userspace can be sure no other thread changed the
> parameters from its last configuration step to the blob retrieval.
>
> TSM specific options are encoded as 'extra' attributes on the TSM device
> with the expectation that vendors reuse the same options for similar
> concepts. The current options are defined by SEV-SNP's need for a
> 'privilege level' concept (VMPL), and the option to retrieve a
> certificate chain in addition to the attestation report ("extended"
> format).
>
> Link: http://lore.kernel.org/r/64961c3baf8ce_142af829436@dwillia2-xfh.jf.intel.com.notmuch [1]
> Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> Cc: Dionna Amalie Glaze <dionnaglaze@google.com>
> Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> Cc: Peter Gonda <pgonda@google.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Samuel Ortiz <sameo@rivosinc.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> Documentation/ABI/testing/sysfs-class-tsm | 47 +++++
> MAINTAINERS | 8 +
> drivers/virt/coco/Kconfig | 4
> drivers/virt/coco/Makefile | 1
> drivers/virt/coco/tdx-guest/Kconfig | 1
> drivers/virt/coco/tsm.c | 290 +++++++++++++++++++++++++++++
> include/linux/tsm.h | 45 +++++
> 7 files changed, 396 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-class-tsm
> create mode 100644 drivers/virt/coco/tsm.c
> create mode 100644 include/linux/tsm.h
>
> +/*
> + * Input is a small hex blob, rather than a writable binary attribute, so that
> + * it is conveyed atomically.
> + */
> +static ssize_t inhex_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + u8 inblob[TSM_INBLOB_MAX];
> + size_t inblob_len;
> + int rc;
> +
> + inblob_len = len;
> + if (buf[len - 1] == '\n')
> + inblob_len--;
> + if (inblob_len & 1)
> + return -EINVAL;
> + inblob_len /= 2;
> + if (inblob_len > TSM_INBLOB_MAX)
> + return -EINVAL;
Is an array_index_nospec() call needed after this check?
> +
> + rc = hex2bin(inblob, buf, inblob_len);
> + if (rc < 0)
> + return rc;
> +
> + guard(rwsem_write)(&tsm_rwsem);
> + if (memcmp(tsm_report.desc.inblob, inblob, inblob_len) == 0)
> + return len;
> + memcpy(tsm_report.desc.inblob, inblob, inblob_len);
Should the portion of tsm_report.desc.inblob that is not updated be
cleared if inblob_len != TSM_INBLOB_MAX?
> + tsm_report.desc.inblob_len = inblob_len;
> + tsm_report.write_generation++;
> +
> + return len;
> +}
> +
> +int register_tsm(const struct tsm_ops *ops, struct device *parent,
> + const struct attribute_group **groups)
> +{
> + const struct tsm_ops *conflict;
> + struct device *dev;
> + int rc;
> +
> + if (!parent)
> + return -EINVAL;
> +
> + if (!groups)
> + groups = tsm_default_attribute_groups;
> +
> + guard(rwsem_write)(&tsm_rwsem);
> + conflict = provider.ops;
> + if (conflict) {
> + pr_err("\"%s\" ops already registered\n", conflict->name);
> + return rc;
Looks like rc is used uninitialized.
> + }
> +
> + dev = device_create_with_groups(tsm_class, parent, 0, NULL, groups,
> + "tsm0");
You can go out to 100 characters now, so this could all be one line.
Thanks,
Tom
> + if (IS_ERR(dev))
> + return PTR_ERR(dev);
> +
> + provider.ops = ops;
> + provider.dev = dev;
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(register_tsm);
> +
On Mon, Aug 14, 2023 at 12:43:21AM -0700, Dan Williams wrote:
> One of the common operations of a TSM (Trusted Security Module) is to
> provide a way for a TVM (confidential computing guest execution
> environment) to take a measurement of its launch state, sign it and
> submit it to a verifying party. Upon successful attestation that
> verifies the integrity of the TVM additional secrets may be deployed.
> The concept is common across TSMs, but the implementations are
> unfortunately vendor specific. While the industry grapples with a common
> definition of this attestation format [1], Linux need not make this
> problem worse by defining a new ABI per TSM that wants to perform a
> similar operation. The current momentum has been to invent new ioctl-ABI
> per TSM per function which at best is an abdication of the kernel's
> responsibility to make common infrastructure concepts share common ABI.
>
> The proposal, targeted to conceptually work with TDX, SEV, COVE if not
> more, is to define a sysfs interface to retrieve the TSM-specific blob.
>
> echo $hex_encoded_userdata_plus_nonce > /sys/class/tsm/tsm0/inhex
> hexdump /sys/class/tsm/tsm0/outblob
Why is one way a hex-encode file, that the kernel has to parse, and the
other not? Binary sysfs files should be "pass through" if at all
possible, why not make them both binary and not mess with hex at all?
That keeps the kernel simpler, and if userspace wants the hex format,
they can provide it much easier (with less potential parsing errors).
>
> This approach later allows for the standardization of the attestation
> blob format without needing to change the Linux ABI. Until then, the
> format of 'outblob' is determined by the parent device for 'tsm0'.
>
> The expectation is that this is a boot time exchange that need not be
> regenerated, making it amenable to a sysfs interface. In case userspace
> does try to generate multiple attestation reports it includes conflict
> detection so userspace can be sure no other thread changed the
> parameters from its last configuration step to the blob retrieval.
>
> TSM specific options are encoded as 'extra' attributes on the TSM device
> with the expectation that vendors reuse the same options for similar
> concepts. The current options are defined by SEV-SNP's need for a
> 'privilege level' concept (VMPL), and the option to retrieve a
> certificate chain in addition to the attestation report ("extended"
> format).
>
> Link: http://lore.kernel.org/r/64961c3baf8ce_142af829436@dwillia2-xfh.jf.intel.com.notmuch [1]
> Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> Cc: Dionna Amalie Glaze <dionnaglaze@google.com>
> Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> Cc: Peter Gonda <pgonda@google.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Samuel Ortiz <sameo@rivosinc.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> Documentation/ABI/testing/sysfs-class-tsm | 47 +++++
> MAINTAINERS | 8 +
> drivers/virt/coco/Kconfig | 4
> drivers/virt/coco/Makefile | 1
> drivers/virt/coco/tdx-guest/Kconfig | 1
> drivers/virt/coco/tsm.c | 290 +++++++++++++++++++++++++++++
> include/linux/tsm.h | 45 +++++
> 7 files changed, 396 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-class-tsm
> create mode 100644 drivers/virt/coco/tsm.c
> create mode 100644 include/linux/tsm.h
>
> diff --git a/Documentation/ABI/testing/sysfs-class-tsm b/Documentation/ABI/testing/sysfs-class-tsm
> new file mode 100644
> index 000000000000..37017bde626d
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-tsm
> @@ -0,0 +1,47 @@
> +What: /sys/class/tsm/tsm0/inhex
> +Date: August, 2023
> +KernelVersion: v6.6
> +Contact: linux-cxl@vger.kernel.org
> +Description:
> + (RW) Hex encoded userdata to be included in the attestation
> + report. For replay protection this should include a nonce, but
> + the kernel does not place any restrictions on the content.
"inhex" and it's read/write? Naming is hard :(
> +
> +What: /sys/class/tsm/tsm0/outblob
> +Date: August, 2023
> +KernelVersion: v6.6
> +Contact: linux-cxl@vger.kernel.org
> +Description:
> + (RO) Binary attestation report generated from @inhex translated
> + to binary and any options. The format of the report is vendor
> + specific and determined by the parent device of 'tsm0'.
> +
> +What: /sys/class/tsm/tsm0/generation
> +Date: August, 2023
> +KernelVersion: v6.6
> +Contact: linux-cxl@vger.kernel.org
> +Description:
> + (RO) The value in this attribute increments each time @inhex or
> + any option is written. Userspace can detect conflicts by
> + checking generation before writing to any attribute and making
> + sure the number of writes matches expectations after reading
> + @outblob.
> +
> +What: /sys/class/tsm/tsm0/privlevel
> +Date: August, 2023
> +KernelVersion: v6.6
> +Contact: linux-cxl@vger.kernel.org
> +Description:
> + (RW) If a TSM implementation supports the concept of attestation
> + reports for TVMs running at different privilege levels, like
> + SEV-SNP "VMPL", specify the privilege level via this attribute.
Where is the list of potential values for this file at?
> +
> +What: /sys/class/tsm/tsm0/format
> +Date: August, 2023
> +KernelVersion: v6.6
> +Contact: linux-cxl@vger.kernel.org
> +Description:
> + (RW) If a TSM implementation supports the concept of attestation
> + reports with "extended" contents, like SEV-SNP extended reports
> + with certificate chains, specify "extended" vs "default" via
> + this attribute.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3be1bdfe8ecc..97f74d344c8a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -21625,6 +21625,14 @@ W: https://github.com/srcres258/linux-doc
> T: git git://github.com/srcres258/linux-doc.git doc-zh-tw
> F: Documentation/translations/zh_TW/
>
> +TRUSTED SECURITY MODULE (TSM) ATTESTATION REPORTS
> +M: Dan Williams <dan.j.williams@intel.com>
> +L: linux-coco@lists.linux.dev
> +S: Maintained
> +F: Documentation/ABI/testing/sysfs-class-tsm
> +F: drivers/virt/coco/tsm.c
> +F: include/linux/tsm.h
> +
> TTY LAYER
> M: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> M: Jiri Slaby <jirislaby@kernel.org>
> diff --git a/drivers/virt/coco/Kconfig b/drivers/virt/coco/Kconfig
> index fc5c64f04c4a..d92f07019f38 100644
> --- a/drivers/virt/coco/Kconfig
> +++ b/drivers/virt/coco/Kconfig
> @@ -2,6 +2,10 @@
> #
> # Confidential computing related collateral
> #
> +
> +config TSM_REPORTS
> + tristate
> +
> source "drivers/virt/coco/efi_secret/Kconfig"
>
> source "drivers/virt/coco/sev-guest/Kconfig"
> diff --git a/drivers/virt/coco/Makefile b/drivers/virt/coco/Makefile
> index 55302ef719ad..18c1aba5edb7 100644
> --- a/drivers/virt/coco/Makefile
> +++ b/drivers/virt/coco/Makefile
> @@ -2,6 +2,7 @@
> #
> # Confidential computing related collateral
> #
> +obj-$(CONFIG_TSM_REPORTS) += tsm.o
> obj-$(CONFIG_EFI_SECRET) += efi_secret/
> obj-$(CONFIG_SEV_GUEST) += sev-guest/
> obj-$(CONFIG_INTEL_TDX_GUEST) += tdx-guest/
> diff --git a/drivers/virt/coco/tdx-guest/Kconfig b/drivers/virt/coco/tdx-guest/Kconfig
> index 14246fc2fb02..22dd59e19431 100644
> --- a/drivers/virt/coco/tdx-guest/Kconfig
> +++ b/drivers/virt/coco/tdx-guest/Kconfig
> @@ -1,6 +1,7 @@
> config TDX_GUEST_DRIVER
> tristate "TDX Guest driver"
> depends on INTEL_TDX_GUEST
> + select TSM_REPORTS
> help
> The driver provides userspace interface to communicate with
> the TDX module to request the TDX guest details like attestation
> diff --git a/drivers/virt/coco/tsm.c b/drivers/virt/coco/tsm.c
> new file mode 100644
> index 000000000000..1bf2ee82eb94
> --- /dev/null
> +++ b/drivers/virt/coco/tsm.c
> @@ -0,0 +1,290 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright(c) 2023 Intel Corporation. All rights reserved. */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/tsm.h>
> +#include <linux/err.h>
> +#include <linux/slab.h>
> +#include <linux/rwsem.h>
> +#include <linux/device.h>
> +#include <linux/string.h>
> +#include <linux/module.h>
> +#include <linux/cleanup.h>
> +
> +struct class *tsm_class;
Nit, we are moving away from using class_create(), please make this a
const static class and register it with the driver core instead. That
way we don't have to fix it up in the future when we finally catch up
with all of the existing class_create() calls we have left.
See the patches in this -rc cycle for a bunch of them already, with many
more on lkml for examples of how to convert this. Here's one example:
https://lore.kernel.org/r/20230810174618.7844-1-ivan.orlov0322@gmail.com
> +static struct tsm_provider {
> + const struct tsm_ops *ops;
> + struct device *dev;
> +} provider;
> +static DECLARE_RWSEM(tsm_rwsem);
> +
> +/**
> + * DOC: Trusted Security Module (TSM) Attestation Report Interface
> + *
> + * The TSM report interface is a common provider of blobs that facilitate
> + * attestation of a TVM (confidential computing guest) by an attestation
> + * service. A TSM report combines a user-defined blob (likely a public-key with
> + * a nonce for a key-exchange protocol) with a signed attestation report. That
> + * combined blob is then used to obtain secrets provided by an agent that can
> + * validate the attestation report. The expectation is that this interface is
> + * invoked infrequently, likely only once at TVM boot time.
> + *
> + * The attestation report format is TSM provider specific, when / if a standard
> + * materializes that can be published instead of the vendor layout.
Published where?
> +/*
> + * Input is a small hex blob, rather than a writable binary attribute, so that
> + * it is conveyed atomically.
> + */
> +static ssize_t inhex_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + u8 inblob[TSM_INBLOB_MAX];
Can this get too big for the stack?
> +static ssize_t inhex_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + char *end;
> +
> + guard(rwsem_read)(&tsm_rwsem);
I like seeing the guard() usage, very nice :)
Overall, the sysfs api is ok, except for the hex values, which is easy
to change. The usage of sysfs is ok as well, no complaints from me
there.
thanks,
greg k-h
Greg Kroah-Hartman wrote:
> On Mon, Aug 14, 2023 at 12:43:21AM -0700, Dan Williams wrote:
> > One of the common operations of a TSM (Trusted Security Module) is to
> > provide a way for a TVM (confidential computing guest execution
> > environment) to take a measurement of its launch state, sign it and
> > submit it to a verifying party. Upon successful attestation that
> > verifies the integrity of the TVM additional secrets may be deployed.
> > The concept is common across TSMs, but the implementations are
> > unfortunately vendor specific. While the industry grapples with a common
> > definition of this attestation format [1], Linux need not make this
> > problem worse by defining a new ABI per TSM that wants to perform a
> > similar operation. The current momentum has been to invent new ioctl-ABI
> > per TSM per function which at best is an abdication of the kernel's
> > responsibility to make common infrastructure concepts share common ABI.
> >
> > The proposal, targeted to conceptually work with TDX, SEV, COVE if not
> > more, is to define a sysfs interface to retrieve the TSM-specific blob.
> >
> > echo $hex_encoded_userdata_plus_nonce > /sys/class/tsm/tsm0/inhex
> > hexdump /sys/class/tsm/tsm0/outblob
>
> Why is one way a hex-encode file, that the kernel has to parse, and the
> other not? Binary sysfs files should be "pass through" if at all
> possible, why not make them both binary and not mess with hex at all?
> That keeps the kernel simpler, and if userspace wants the hex format,
> they can provide it much easier (with less potential parsing errors).
I can do that. The concern was the contract around what to do with
partial writes since binary attributes allow writing the middle of the
buffer. So either the attribute needs to enforce that @offset is always
zero, or that the unwritten portion of the buffer is zeroed. I will go
with just enforcing offset=zero writes.
> > This approach later allows for the standardization of the attestation
> > blob format without needing to change the Linux ABI. Until then, the
> > format of 'outblob' is determined by the parent device for 'tsm0'.
> >
> > The expectation is that this is a boot time exchange that need not be
> > regenerated, making it amenable to a sysfs interface. In case userspace
> > does try to generate multiple attestation reports it includes conflict
> > detection so userspace can be sure no other thread changed the
> > parameters from its last configuration step to the blob retrieval.
> >
> > TSM specific options are encoded as 'extra' attributes on the TSM device
> > with the expectation that vendors reuse the same options for similar
> > concepts. The current options are defined by SEV-SNP's need for a
> > 'privilege level' concept (VMPL), and the option to retrieve a
> > certificate chain in addition to the attestation report ("extended"
> > format).
> >
> > Link: http://lore.kernel.org/r/64961c3baf8ce_142af829436@dwillia2-xfh.jf.intel.com.notmuch [1]
> > Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> > Cc: Dionna Amalie Glaze <dionnaglaze@google.com>
> > Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> > Cc: Peter Gonda <pgonda@google.com>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Samuel Ortiz <sameo@rivosinc.com>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > ---
> > Documentation/ABI/testing/sysfs-class-tsm | 47 +++++
> > MAINTAINERS | 8 +
> > drivers/virt/coco/Kconfig | 4
> > drivers/virt/coco/Makefile | 1
> > drivers/virt/coco/tdx-guest/Kconfig | 1
> > drivers/virt/coco/tsm.c | 290 +++++++++++++++++++++++++++++
> > include/linux/tsm.h | 45 +++++
> > 7 files changed, 396 insertions(+)
> > create mode 100644 Documentation/ABI/testing/sysfs-class-tsm
> > create mode 100644 drivers/virt/coco/tsm.c
> > create mode 100644 include/linux/tsm.h
> >
> > diff --git a/Documentation/ABI/testing/sysfs-class-tsm b/Documentation/ABI/testing/sysfs-class-tsm
> > new file mode 100644
> > index 000000000000..37017bde626d
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-class-tsm
> > @@ -0,0 +1,47 @@
> > +What: /sys/class/tsm/tsm0/inhex
> > +Date: August, 2023
> > +KernelVersion: v6.6
> > +Contact: linux-cxl@vger.kernel.org
> > +Description:
> > + (RW) Hex encoded userdata to be included in the attestation
> > + report. For replay protection this should include a nonce, but
> > + the kernel does not place any restrictions on the content.
>
> "inhex" and it's read/write? Naming is hard :(
True, could be write-only.
>
>
> > +
> > +What: /sys/class/tsm/tsm0/outblob
> > +Date: August, 2023
> > +KernelVersion: v6.6
> > +Contact: linux-cxl@vger.kernel.org
> > +Description:
> > + (RO) Binary attestation report generated from @inhex translated
> > + to binary and any options. The format of the report is vendor
> > + specific and determined by the parent device of 'tsm0'.
> > +
> > +What: /sys/class/tsm/tsm0/generation
> > +Date: August, 2023
> > +KernelVersion: v6.6
> > +Contact: linux-cxl@vger.kernel.org
> > +Description:
> > + (RO) The value in this attribute increments each time @inhex or
> > + any option is written. Userspace can detect conflicts by
> > + checking generation before writing to any attribute and making
> > + sure the number of writes matches expectations after reading
> > + @outblob.
> > +
> > +What: /sys/class/tsm/tsm0/privlevel
> > +Date: August, 2023
> > +KernelVersion: v6.6
> > +Contact: linux-cxl@vger.kernel.org
> > +Description:
> > + (RW) If a TSM implementation supports the concept of attestation
> > + reports for TVMs running at different privilege levels, like
> > + SEV-SNP "VMPL", specify the privilege level via this attribute.
>
> Where is the list of potential values for this file at?
Good idea, let me bound this with some reasonable values.
>
> > +
> > +What: /sys/class/tsm/tsm0/format
> > +Date: August, 2023
> > +KernelVersion: v6.6
> > +Contact: linux-cxl@vger.kernel.org
> > +Description:
> > + (RW) If a TSM implementation supports the concept of attestation
> > + reports with "extended" contents, like SEV-SNP extended reports
> > + with certificate chains, specify "extended" vs "default" via
> > + this attribute.
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 3be1bdfe8ecc..97f74d344c8a 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -21625,6 +21625,14 @@ W: https://github.com/srcres258/linux-doc
> > T: git git://github.com/srcres258/linux-doc.git doc-zh-tw
> > F: Documentation/translations/zh_TW/
> >
> > +TRUSTED SECURITY MODULE (TSM) ATTESTATION REPORTS
> > +M: Dan Williams <dan.j.williams@intel.com>
> > +L: linux-coco@lists.linux.dev
> > +S: Maintained
> > +F: Documentation/ABI/testing/sysfs-class-tsm
> > +F: drivers/virt/coco/tsm.c
> > +F: include/linux/tsm.h
> > +
> > TTY LAYER
> > M: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > M: Jiri Slaby <jirislaby@kernel.org>
> > diff --git a/drivers/virt/coco/Kconfig b/drivers/virt/coco/Kconfig
> > index fc5c64f04c4a..d92f07019f38 100644
> > --- a/drivers/virt/coco/Kconfig
> > +++ b/drivers/virt/coco/Kconfig
> > @@ -2,6 +2,10 @@
> > #
> > # Confidential computing related collateral
> > #
> > +
> > +config TSM_REPORTS
> > + tristate
> > +
> > source "drivers/virt/coco/efi_secret/Kconfig"
> >
> > source "drivers/virt/coco/sev-guest/Kconfig"
> > diff --git a/drivers/virt/coco/Makefile b/drivers/virt/coco/Makefile
> > index 55302ef719ad..18c1aba5edb7 100644
> > --- a/drivers/virt/coco/Makefile
> > +++ b/drivers/virt/coco/Makefile
> > @@ -2,6 +2,7 @@
> > #
> > # Confidential computing related collateral
> > #
> > +obj-$(CONFIG_TSM_REPORTS) += tsm.o
> > obj-$(CONFIG_EFI_SECRET) += efi_secret/
> > obj-$(CONFIG_SEV_GUEST) += sev-guest/
> > obj-$(CONFIG_INTEL_TDX_GUEST) += tdx-guest/
> > diff --git a/drivers/virt/coco/tdx-guest/Kconfig b/drivers/virt/coco/tdx-guest/Kconfig
> > index 14246fc2fb02..22dd59e19431 100644
> > --- a/drivers/virt/coco/tdx-guest/Kconfig
> > +++ b/drivers/virt/coco/tdx-guest/Kconfig
> > @@ -1,6 +1,7 @@
> > config TDX_GUEST_DRIVER
> > tristate "TDX Guest driver"
> > depends on INTEL_TDX_GUEST
> > + select TSM_REPORTS
> > help
> > The driver provides userspace interface to communicate with
> > the TDX module to request the TDX guest details like attestation
> > diff --git a/drivers/virt/coco/tsm.c b/drivers/virt/coco/tsm.c
> > new file mode 100644
> > index 000000000000..1bf2ee82eb94
> > --- /dev/null
> > +++ b/drivers/virt/coco/tsm.c
> > @@ -0,0 +1,290 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/* Copyright(c) 2023 Intel Corporation. All rights reserved. */
> > +
> > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > +
> > +#include <linux/tsm.h>
> > +#include <linux/err.h>
> > +#include <linux/slab.h>
> > +#include <linux/rwsem.h>
> > +#include <linux/device.h>
> > +#include <linux/string.h>
> > +#include <linux/module.h>
> > +#include <linux/cleanup.h>
> > +
> > +struct class *tsm_class;
>
> Nit, we are moving away from using class_create(), please make this a
> const static class and register it with the driver core instead. That
> way we don't have to fix it up in the future when we finally catch up
> with all of the existing class_create() calls we have left.
>
> See the patches in this -rc cycle for a bunch of them already, with many
> more on lkml for examples of how to convert this. Here's one example:
> https://lore.kernel.org/r/20230810174618.7844-1-ivan.orlov0322@gmail.com
Got it.
>
> > +static struct tsm_provider {
> > + const struct tsm_ops *ops;
> > + struct device *dev;
> > +} provider;
> > +static DECLARE_RWSEM(tsm_rwsem);
> > +
> > +/**
> > + * DOC: Trusted Security Module (TSM) Attestation Report Interface
> > + *
> > + * The TSM report interface is a common provider of blobs that facilitate
> > + * attestation of a TVM (confidential computing guest) by an attestation
> > + * service. A TSM report combines a user-defined blob (likely a public-key with
> > + * a nonce for a key-exchange protocol) with a signed attestation report. That
> > + * combined blob is then used to obtain secrets provided by an agent that can
> > + * validate the attestation report. The expectation is that this interface is
> > + * invoked infrequently, likely only once at TVM boot time.
> > + *
> > + * The attestation report format is TSM provider specific, when / if a standard
> > + * materializes that can be published instead of the vendor layout.
>
> Published where?
Likely via a new attribute so that old scripts dependent on vendor
specific attestation formats do not break.
>
> > +/*
> > + * Input is a small hex blob, rather than a writable binary attribute, so that
> > + * it is conveyed atomically.
> > + */
> > +static ssize_t inhex_store(struct device *dev, struct device_attribute *attr,
> > + const char *buf, size_t len)
> > +{
> > + u8 inblob[TSM_INBLOB_MAX];
>
> Can this get too big for the stack?
As it stands the current implementations only allow 64-byte inputs. The
question would be whether RISC-V or ARM need more space for their
attestation report input buffers. Will keep an eye on it.
> > +static ssize_t inhex_show(struct device *dev, struct device_attribute *attr,
> > + char *buf)
> > +{
> > + char *end;
> > +
> > + guard(rwsem_read)(&tsm_rwsem);
>
> I like seeing the guard() usage, very nice :)
>
> Overall, the sysfs api is ok, except for the hex values, which is easy
> to change. The usage of sysfs is ok as well, no complaints from me
> there.
Thanks Greg.
On Mon, Aug 14, 2023 at 09:43:37AM -0700, Dan Williams wrote: > Greg Kroah-Hartman wrote: > > On Mon, Aug 14, 2023 at 12:43:21AM -0700, Dan Williams wrote: > > > One of the common operations of a TSM (Trusted Security Module) is to > > > provide a way for a TVM (confidential computing guest execution > > > environment) to take a measurement of its launch state, sign it and > > > submit it to a verifying party. Upon successful attestation that > > > verifies the integrity of the TVM additional secrets may be deployed. > > > The concept is common across TSMs, but the implementations are > > > unfortunately vendor specific. While the industry grapples with a common > > > definition of this attestation format [1], Linux need not make this > > > problem worse by defining a new ABI per TSM that wants to perform a > > > similar operation. The current momentum has been to invent new ioctl-ABI > > > per TSM per function which at best is an abdication of the kernel's > > > responsibility to make common infrastructure concepts share common ABI. > > > > > > The proposal, targeted to conceptually work with TDX, SEV, COVE if not > > > more, is to define a sysfs interface to retrieve the TSM-specific blob. > > > > > > echo $hex_encoded_userdata_plus_nonce > /sys/class/tsm/tsm0/inhex > > > hexdump /sys/class/tsm/tsm0/outblob > > > > Why is one way a hex-encode file, that the kernel has to parse, and the > > other not? Binary sysfs files should be "pass through" if at all > > possible, why not make them both binary and not mess with hex at all? > > That keeps the kernel simpler, and if userspace wants the hex format, > > they can provide it much easier (with less potential parsing errors). > > I can do that. The concern was the contract around what to do with > partial writes since binary attributes allow writing the middle of the > buffer. So either the attribute needs to enforce that @offset is always > zero, or that the unwritten portion of the buffer is zeroed. I will go > with just enforcing offset=zero writes. Enforcing that sounds sane, thanks. greg k-h
On 8/14/2023 9:43 AM, Dan Williams wrote:
> One of the common operations of a TSM (Trusted Security Module) is to
> provide a way for a TVM (confidential computing guest execution
> environment) to take a measurement of its launch state, sign it and
> submit it to a verifying party. Upon successful attestation that
> verifies the integrity of the TVM additional secrets may be deployed.
> The concept is common across TSMs, but the implementations are
> unfortunately vendor specific. While the industry grapples with a common
> definition of this attestation format [1], Linux need not make this
> problem worse by defining a new ABI per TSM that wants to perform a
> similar operation. The current momentum has been to invent new ioctl-ABI
> per TSM per function which at best is an abdication of the kernel's
> responsibility to make common infrastructure concepts share common ABI.
>
> The proposal, targeted to conceptually work with TDX, SEV, COVE if not
> more, is to define a sysfs interface to retrieve the TSM-specific blob.
>
> echo $hex_encoded_userdata_plus_nonce > /sys/class/tsm/tsm0/inhex
> hexdump /sys/class/tsm/tsm0/outblob
>
> This approach later allows for the standardization of the attestation
> blob format without needing to change the Linux ABI. Until then, the
> format of 'outblob' is determined by the parent device for 'tsm0'.
>
> The expectation is that this is a boot time exchange that need not be
> regenerated, making it amenable to a sysfs interface. In case userspace
> does try to generate multiple attestation reports it includes conflict
> detection so userspace can be sure no other thread changed the
> parameters from its last configuration step to the blob retrieval.
>
> TSM specific options are encoded as 'extra' attributes on the TSM device
> with the expectation that vendors reuse the same options for similar
> concepts. The current options are defined by SEV-SNP's need for a
> 'privilege level' concept (VMPL), and the option to retrieve a
> certificate chain in addition to the attestation report ("extended"
> format).
>
> Link: http://lore.kernel.org/r/64961c3baf8ce_142af829436@dwillia2-xfh.jf.intel.com.notmuch [1]
> Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> Cc: Dionna Amalie Glaze <dionnaglaze@google.com>
> Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> Cc: Peter Gonda <pgonda@google.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Samuel Ortiz <sameo@rivosinc.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> Documentation/ABI/testing/sysfs-class-tsm | 47 +++++
> MAINTAINERS | 8 +
> drivers/virt/coco/Kconfig | 4
> drivers/virt/coco/Makefile | 1
> drivers/virt/coco/tdx-guest/Kconfig | 1
> drivers/virt/coco/tsm.c | 290 +++++++++++++++++++++++++++++
> include/linux/tsm.h | 45 +++++
> 7 files changed, 396 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-class-tsm
> create mode 100644 drivers/virt/coco/tsm.c
> create mode 100644 include/linux/tsm.h
>
> diff --git a/Documentation/ABI/testing/sysfs-class-tsm b/Documentation/ABI/testing/sysfs-class-tsm
> new file mode 100644
> index 000000000000..37017bde626d
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-tsm
> @@ -0,0 +1,47 @@
> +What: /sys/class/tsm/tsm0/inhex
> +Date: August, 2023
> +KernelVersion: v6.6
> +Contact: linux-cxl@vger.kernel.org
> +Description:
> + (RW) Hex encoded userdata to be included in the attestation
> + report. For replay protection this should include a nonce, but
> + the kernel does not place any restrictions on the content.
> +
> +What: /sys/class/tsm/tsm0/outblob
> +Date: August, 2023
> +KernelVersion: v6.6
> +Contact: linux-cxl@vger.kernel.org
> +Description:
> + (RO) Binary attestation report generated from @inhex translated
> + to binary and any options. The format of the report is vendor
> + specific and determined by the parent device of 'tsm0'.
> +
> +What: /sys/class/tsm/tsm0/generation
> +Date: August, 2023
> +KernelVersion: v6.6
> +Contact: linux-cxl@vger.kernel.org
> +Description:
> + (RO) The value in this attribute increments each time @inhex or
> + any option is written. Userspace can detect conflicts by
> + checking generation before writing to any attribute and making
> + sure the number of writes matches expectations after reading
> + @outblob.
> +
> +What: /sys/class/tsm/tsm0/privlevel
> +Date: August, 2023
> +KernelVersion: v6.6
> +Contact: linux-cxl@vger.kernel.org
> +Description:
> + (RW) If a TSM implementation supports the concept of attestation
> + reports for TVMs running at different privilege levels, like
> + SEV-SNP "VMPL", specify the privilege level via this attribute.
> +
> +What: /sys/class/tsm/tsm0/format
> +Date: August, 2023
> +KernelVersion: v6.6
> +Contact: linux-cxl@vger.kernel.org
> +Description:
> + (RW) If a TSM implementation supports the concept of attestation
> + reports with "extended" contents, like SEV-SNP extended reports
> + with certificate chains, specify "extended" vs "default" via
> + this attribute.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3be1bdfe8ecc..97f74d344c8a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -21625,6 +21625,14 @@ W: https://github.com/srcres258/linux-doc
> T: git git://github.com/srcres258/linux-doc.git doc-zh-tw
> F: Documentation/translations/zh_TW/
>
> +TRUSTED SECURITY MODULE (TSM) ATTESTATION REPORTS
> +M: Dan Williams <dan.j.williams@intel.com>
> +L: linux-coco@lists.linux.dev
> +S: Maintained
> +F: Documentation/ABI/testing/sysfs-class-tsm
> +F: drivers/virt/coco/tsm.c
> +F: include/linux/tsm.h
> +
> TTY LAYER
> M: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> M: Jiri Slaby <jirislaby@kernel.org>
> diff --git a/drivers/virt/coco/Kconfig b/drivers/virt/coco/Kconfig
> index fc5c64f04c4a..d92f07019f38 100644
> --- a/drivers/virt/coco/Kconfig
> +++ b/drivers/virt/coco/Kconfig
> @@ -2,6 +2,10 @@
> #
> # Confidential computing related collateral
> #
> +
> +config TSM_REPORTS
> + tristate
> +
> source "drivers/virt/coco/efi_secret/Kconfig"
>
> source "drivers/virt/coco/sev-guest/Kconfig"
> diff --git a/drivers/virt/coco/Makefile b/drivers/virt/coco/Makefile
> index 55302ef719ad..18c1aba5edb7 100644
> --- a/drivers/virt/coco/Makefile
> +++ b/drivers/virt/coco/Makefile
> @@ -2,6 +2,7 @@
> #
> # Confidential computing related collateral
> #
> +obj-$(CONFIG_TSM_REPORTS) += tsm.o
> obj-$(CONFIG_EFI_SECRET) += efi_secret/
> obj-$(CONFIG_SEV_GUEST) += sev-guest/
> obj-$(CONFIG_INTEL_TDX_GUEST) += tdx-guest/
> diff --git a/drivers/virt/coco/tdx-guest/Kconfig b/drivers/virt/coco/tdx-guest/Kconfig
> index 14246fc2fb02..22dd59e19431 100644
> --- a/drivers/virt/coco/tdx-guest/Kconfig
> +++ b/drivers/virt/coco/tdx-guest/Kconfig
> @@ -1,6 +1,7 @@
> config TDX_GUEST_DRIVER
> tristate "TDX Guest driver"
> depends on INTEL_TDX_GUEST
> + select TSM_REPORTS
> help
> The driver provides userspace interface to communicate with
> the TDX module to request the TDX guest details like attestation
> diff --git a/drivers/virt/coco/tsm.c b/drivers/virt/coco/tsm.c
> new file mode 100644
> index 000000000000..1bf2ee82eb94
> --- /dev/null
> +++ b/drivers/virt/coco/tsm.c
> @@ -0,0 +1,290 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright(c) 2023 Intel Corporation. All rights reserved. */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/tsm.h>
> +#include <linux/err.h>
> +#include <linux/slab.h>
> +#include <linux/rwsem.h>
> +#include <linux/device.h>
> +#include <linux/string.h>
> +#include <linux/module.h>
> +#include <linux/cleanup.h>
> +
> +struct class *tsm_class;
> +static struct tsm_provider {
> + const struct tsm_ops *ops;
> + struct device *dev;
> +} provider;
> +static DECLARE_RWSEM(tsm_rwsem);
> +
> +/**
> + * DOC: Trusted Security Module (TSM) Attestation Report Interface
> + *
> + * The TSM report interface is a common provider of blobs that facilitate
> + * attestation of a TVM (confidential computing guest) by an attestation
> + * service. A TSM report combines a user-defined blob (likely a public-key with
> + * a nonce for a key-exchange protocol) with a signed attestation report. That
> + * combined blob is then used to obtain secrets provided by an agent that can
> + * validate the attestation report. The expectation is that this interface is
> + * invoked infrequently, likely only once at TVM boot time.
> + *
> + * The attestation report format is TSM provider specific, when / if a standard
> + * materializes that can be published instead of the vendor layout.
> + */
> +
> +/**
> + * struct tsm_report - track state of report generation relative to options
> + * @desc: report generation options / cached report state
> + * @outblob: generated evidence to provider to the attestation agent
> + * @outblob_len: sizeof(outblob)
> + * @write_generation: conflict detection, and report regeneration tracking
> + * @read_generation: cached report invalidation tracking
> + */
> +struct tsm_report {
> + struct tsm_desc desc;
> + size_t outblob_len;
> + u8 *outblob;
> + unsigned long write_generation;
> + unsigned long read_generation;
> +} tsm_report;
> +
> +static ssize_t privlevel_store(struct device *dev,
> + struct device_attribute *attr, const char *buf,
> + size_t len)
> +{
> + unsigned int val;
> + int rc;
> +
> + rc = kstrtouint(buf, 0, &val);
> + if (rc)
> + return rc;
> +
> + guard(rwsem_write)(&tsm_rwsem);
> + if (tsm_report.desc.privlevel == val)
> + return len;
> + tsm_report.desc.privlevel = val;
> + tsm_report.write_generation++;
> +
> + return len;
> +}
> +
> +static ssize_t privlevel_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + return sysfs_emit(buf, "%u\n", tsm_report.desc.privlevel);
> +}
> +
> +static DEVICE_ATTR_RW(privlevel);
> +
> +static ssize_t format_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + enum tsm_format format;
> +
> + if (sysfs_streq(buf, "default"))
> + format = TSM_FORMAT_DEFAULT;
> + else if (sysfs_streq(buf, "extended"))
> + format = TSM_FORMAT_EXTENDED;
> + else
> + return -EINVAL;
> +
> + guard(rwsem_write)(&tsm_rwsem);
> + if (tsm_report.desc.outblob_format == format)
> + return len;
> + tsm_report.desc.outblob_format = format;
> + tsm_report.write_generation++;
> +
> + return len;
> +}
> +
> +static ssize_t format_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + if (tsm_report.desc.outblob_format == TSM_FORMAT_DEFAULT)
> + return sysfs_emit(buf, "default\n");
> + return sysfs_emit(buf, "extended\n");
> +}
> +
> +static DEVICE_ATTR_RW(format);
> +
> +static struct attribute *tsm_extra_attributes[] = {
> + &dev_attr_format.attr,
> + &dev_attr_privlevel.attr,
> + NULL,
> +};
> +
> +struct attribute_group tsm_extra_attribute_group = {
> + .attrs = tsm_extra_attributes,
> +};
> +EXPORT_SYMBOL_GPL(tsm_extra_attribute_group);
> +
> +/*
> + * Input is a small hex blob, rather than a writable binary attribute, so that
> + * it is conveyed atomically.
> + */
> +static ssize_t inhex_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + u8 inblob[TSM_INBLOB_MAX];
> + size_t inblob_len;
> + int rc;
> +
> + inblob_len = len;
> + if (buf[len - 1] == '\n')
> + inblob_len--;
> + if (inblob_len & 1)
> + return -EINVAL;
> + inblob_len /= 2;
> + if (inblob_len > TSM_INBLOB_MAX)
> + return -EINVAL;
> +
> + rc = hex2bin(inblob, buf, inblob_len);
> + if (rc < 0)
> + return rc;
> +
> + guard(rwsem_write)(&tsm_rwsem);
> + if (memcmp(tsm_report.desc.inblob, inblob, inblob_len) == 0)
> + return len;
> + memcpy(tsm_report.desc.inblob, inblob, inblob_len);
> + tsm_report.desc.inblob_len = inblob_len;
> + tsm_report.write_generation++;
> +
> + return len;
> +}
> +
> +static ssize_t inhex_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + char *end;
> +
> + guard(rwsem_read)(&tsm_rwsem);
> + if (!tsm_report.desc.inblob_len)
> + return 0;
> + end = bin2hex(buf, tsm_report.desc.inblob, tsm_report.desc.inblob_len);
> + *end++ = '\n';
> + return end - buf;
> +}
> +static DEVICE_ATTR_RW(inhex);
> +
> +static ssize_t generation_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + guard(rwsem_read)(&tsm_rwsem);
> + return sysfs_emit(buf, "%lu\n", tsm_report.write_generation);
> +}
> +static DEVICE_ATTR_RO(generation);
> +
> +static struct attribute *tsm_attributes[] = {
> + &dev_attr_inhex.attr,
> + &dev_attr_generation.attr,
> + NULL,
> +};
> +
> +static ssize_t outblob_read(struct file *f, struct kobject *kobj,
> + struct bin_attribute *bin_attr, char *buf,
> + loff_t offset, size_t count)
> +{
> + guard(rwsem_read)(&tsm_rwsem);
This is unfortunate but it would need to be a rwsem_write otherwise two
processes can race to reach the kvfree and both call report_new at the
same time (unlikely as it may be).
Jeremi
> + if (!tsm_report.desc.inblob_len)
> + return -EINVAL;
> +
> + if (!tsm_report.outblob ||
> + tsm_report.read_generation != tsm_report.write_generation) {
> + const struct tsm_ops *ops = provider.ops;
> + size_t outblob_len;
> + u8 *outblob;
> +
> + kvfree(tsm_report.outblob);
> + outblob = ops->report_new(provider.dev->parent,
> + &tsm_report.desc, &outblob_len);
> + if (IS_ERR(outblob))
> + return PTR_ERR(outblob);
> + tsm_report.outblob_len = outblob_len;
> + tsm_report.outblob = outblob;
> + tsm_report.read_generation = tsm_report.write_generation;
> + }
> +
> + return memory_read_from_buffer(buf, count, &offset,
> + tsm_report.outblob,
> + tsm_report.outblob_len);
> +}
> +static BIN_ATTR_RO(outblob, 0);
> +
> +static struct bin_attribute *tsm_bin_attributes[] = {
> + &bin_attr_outblob,
> + NULL,
> +};
> +
> +struct attribute_group tsm_default_attribute_group = {
> + .bin_attrs = tsm_bin_attributes,
> + .attrs = tsm_attributes,
> +};
> +EXPORT_SYMBOL_GPL(tsm_default_attribute_group);
> +
> +static const struct attribute_group *tsm_default_attribute_groups[] = {
> + &tsm_default_attribute_group,
> + NULL,
> +};
> +
> +int register_tsm(const struct tsm_ops *ops, struct device *parent,
> + const struct attribute_group **groups)
> +{
> + const struct tsm_ops *conflict;
> + struct device *dev;
> + int rc;
> +
> + if (!parent)
> + return -EINVAL;
> +
> + if (!groups)
> + groups = tsm_default_attribute_groups;
> +
> + guard(rwsem_write)(&tsm_rwsem);
> + conflict = provider.ops;
> + if (conflict) {
> + pr_err("\"%s\" ops already registered\n", conflict->name);
> + return rc;
> + }
> +
> + dev = device_create_with_groups(tsm_class, parent, 0, NULL, groups,
> + "tsm0");
> + if (IS_ERR(dev))
> + return PTR_ERR(dev);
> +
> + provider.ops = ops;
> + provider.dev = dev;
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(register_tsm);
> +
> +int unregister_tsm(const struct tsm_ops *ops)
> +{
> + guard(rwsem_write)(&tsm_rwsem);
> + if (ops != provider.ops)
> + return -EBUSY;
> + provider.ops = NULL;
> + device_unregister(provider.dev);
> + provider.dev = NULL;
> + kvfree(tsm_report.outblob);
> + tsm_report.outblob = NULL;
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(unregister_tsm);
> +
> +static int __init tsm_init(void)
> +{
> + tsm_class = class_create("tsm");
> + return PTR_ERR_OR_ZERO(tsm_class);
> +}
> +module_init(tsm_init);
> +
> +static void __exit tsm_exit(void)
> +{
> + class_destroy(tsm_class);
> +}
> +module_exit(tsm_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Provide Trusted Security Module attestation reports via sysfs");
> diff --git a/include/linux/tsm.h b/include/linux/tsm.h
> new file mode 100644
> index 000000000000..6dc2f07543b8
> --- /dev/null
> +++ b/include/linux/tsm.h
> @@ -0,0 +1,45 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __TSM_H
> +#define __TSM_H
> +
> +#include <linux/types.h>
> +#include <linux/device.h>
> +
> +#define TSM_INBLOB_MAX 64
> +
> +enum tsm_format {
> + TSM_FORMAT_DEFAULT,
> + TSM_FORMAT_EXTENDED,
> +};
> +
> +/**
> + * struct tsm_desc - option descriptor for generating tsm report blobs
> + * @privlevel: optional privilege level to associate with @outblob
> + * @inblob_len: sizeof @inblob
> + * @inblob: arbitrary input data
> + * @outblob_format: for TSMs with an "extended" format
> + */
> +struct tsm_desc {
> + unsigned int privlevel;
> + size_t inblob_len;
> + u8 inblob[TSM_INBLOB_MAX];
> + enum tsm_format outblob_format;
> +};
> +
> +/*
> + * arch specific ops, only one is expected to be registered at a time
> + * i.e. only one of SEV, TDX, COVE, etc.
> + */
> +struct tsm_ops {
> + const char *name;
> + u8 *(*report_new)(struct device *dev, const struct tsm_desc *desc,
> + size_t *outblob_len);
> +};
> +
> +extern struct attribute_group tsm_default_attribute_group;
> +extern struct attribute_group tsm_extra_attribute_group;
> +
> +int register_tsm(const struct tsm_ops *ops, struct device *parent,
> + const struct attribute_group **groups);
> +int unregister_tsm(const struct tsm_ops *ops);
> +#endif /* __TSM_H */
>
Jeremi Piotrowski wrote:
> On 8/14/2023 9:43 AM, Dan Williams wrote:
> > One of the common operations of a TSM (Trusted Security Module) is to
> > provide a way for a TVM (confidential computing guest execution
> > environment) to take a measurement of its launch state, sign it and
> > submit it to a verifying party. Upon successful attestation that
> > verifies the integrity of the TVM additional secrets may be deployed.
> > The concept is common across TSMs, but the implementations are
> > unfortunately vendor specific. While the industry grapples with a common
> > definition of this attestation format [1], Linux need not make this
> > problem worse by defining a new ABI per TSM that wants to perform a
> > similar operation. The current momentum has been to invent new ioctl-ABI
> > per TSM per function which at best is an abdication of the kernel's
> > responsibility to make common infrastructure concepts share common ABI.
> >
> > The proposal, targeted to conceptually work with TDX, SEV, COVE if not
> > more, is to define a sysfs interface to retrieve the TSM-specific blob.
> >
> > echo $hex_encoded_userdata_plus_nonce > /sys/class/tsm/tsm0/inhex
> > hexdump /sys/class/tsm/tsm0/outblob
> >
> > This approach later allows for the standardization of the attestation
> > blob format without needing to change the Linux ABI. Until then, the
> > format of 'outblob' is determined by the parent device for 'tsm0'.
> >
> > The expectation is that this is a boot time exchange that need not be
> > regenerated, making it amenable to a sysfs interface. In case userspace
> > does try to generate multiple attestation reports it includes conflict
> > detection so userspace can be sure no other thread changed the
> > parameters from its last configuration step to the blob retrieval.
> >
> > TSM specific options are encoded as 'extra' attributes on the TSM device
> > with the expectation that vendors reuse the same options for similar
> > concepts. The current options are defined by SEV-SNP's need for a
> > 'privilege level' concept (VMPL), and the option to retrieve a
> > certificate chain in addition to the attestation report ("extended"
> > format).
> >
> > Link: http://lore.kernel.org/r/64961c3baf8ce_142af829436@dwillia2-xfh.jf.intel.com.notmuch [1]
> > Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> > Cc: Dionna Amalie Glaze <dionnaglaze@google.com>
> > Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> > Cc: Peter Gonda <pgonda@google.com>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Samuel Ortiz <sameo@rivosinc.com>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > ---
[..]
> > +static ssize_t outblob_read(struct file *f, struct kobject *kobj,
> > + struct bin_attribute *bin_attr, char *buf,
> > + loff_t offset, size_t count)
> > +{
> > + guard(rwsem_read)(&tsm_rwsem);
>
> This is unfortunate but it would need to be a rwsem_write otherwise two
> processes can race to reach the kvfree and both call report_new at the
> same time (unlikely as it may be).
Ugh, yup, good eye, will fix.
© 2016 - 2025 Red Hat, Inc.