linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI
@ 2024-01-28 21:25 Samuel Ortiz
  2024-01-28 21:25 ` [RFC PATCH v2 1/4] tsm: Runtime measurement register support Samuel Ortiz
                   ` (5 more replies)
  0 siblings, 6 replies; 40+ messages in thread
From: Samuel Ortiz @ 2024-01-28 21:25 UTC (permalink / raw)
  To: Dan Williams
  Cc: Samuel Ortiz, Kuppuswamy Sathyanarayanan, Qinkun Bao, Yao,
	Jiewen, Xing, Cedric, Dionna Amalie Glaze, biao.lu, linux-coco,
	linux-integrity, linux-kernel

Some confidential computing architectures (Intel TDX, ARM CCA, RISC-V
CoVE) provide their guests with a set of measurements registers that can
be extended at runtime, i.e. after the initial, host-initiated
measurements of the TVM are finalized. Those runtime measurement
registers (RTMR) are isolated from the host accessible ones but TSMs
include them in their signed attestation reports.

All architectures supporting RTMRs expose a similar interface to their
TVMs: An extension command/call that takes a measurement value and an
RTMR index to extend it with, and a readback command for reading an RTMR
value back (taking an RTMR index as an argument as well). This patch series
builds an architecture agnostic, configfs-based ABI for userspace to extend
and read RTMR values back. It extends the current TSM ops structure and
each confidential computing architecture can implement this extension to
provide RTMR support.

Changes since v1 [1]:
- Removed the abilty for userspace to configure the TCG PCR mappings. The
  configfs attribute for the TCG PCR mapping is now RO, and the mapping is
  passed from the TSM provider as a static bitmap.
- Document the added tsm-configs attributes.

TODO:

- Event log support.

[1] https://lore.kernel.org/lkml/20240114223532.290550-1-sameo@rivosinc.com/

---

Samuel Ortiz (4):
  tsm: Runtime measurement register support
  tsm: Add RTMRs to the configfs-tsm hierarchy
  tsm: Map RTMRs to TCG TPM PCRs
  tsm: Allow for extending and reading configured RTMRs

 Documentation/ABI/testing/configfs-tsm |  36 +++
 drivers/virt/coco/Kconfig              |   1 +
 drivers/virt/coco/tsm.c                | 376 +++++++++++++++++++++++++
 include/linux/tsm.h                    |  39 ++-
 4 files changed, 451 insertions(+), 1 deletion(-)

-- 
2.42.0


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [RFC PATCH v2 1/4] tsm: Runtime measurement register support
  2024-01-28 21:25 [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI Samuel Ortiz
@ 2024-01-28 21:25 ` Samuel Ortiz
  2024-01-29 16:57   ` Dionna Amalie Glaze
  2024-02-01 22:03   ` Jarkko Sakkinen
  2024-01-28 21:25 ` [RFC PATCH v2 2/4] tsm: Add RTMRs to the configfs-tsm hierarchy Samuel Ortiz
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 40+ messages in thread
From: Samuel Ortiz @ 2024-01-28 21:25 UTC (permalink / raw)
  To: Dan Williams
  Cc: Samuel Ortiz, Kuppuswamy Sathyanarayanan, Qinkun Bao, Yao,
	Jiewen, Xing, Cedric, Dionna Amalie Glaze, biao.lu, linux-coco,
	linux-integrity, linux-kernel

Some confidential computing architecture (Intel TDX, ARM-CCA, RISC-V
CoVE) provide the TVM (confidential computing guest) with a set of
runtime measurement registers (RTMR). TVMs can extend those registers
with their measurements at runtime, i.e. after the TVM initial
measurements are finalized and the TVM actually runs.

RTMRs are separated from the initial measurement registers set, and TSMs
typically includes RTMR values into a distinct section of their signed
attestion reports.

We add support for extending and reading a TSM runtime measurement
registers by extending the TSM ops structure with resp. an rtmr_extend()
and an rtmr_read() function pointers. TSM providers/backends will
implement those ops if they are capable of exposing RTMRs to their
TVMs. This capability is now described by a tsm_capabilites structure,
passed by the TSM provider to the TSM framework at registration time.

TVMs can configure, extend and read RTMRs from the configfs-tsm interface.

Signed-off-by: Samuel Ortiz <sameo@rivosinc.com>
---
 drivers/virt/coco/tsm.c | 80 +++++++++++++++++++++++++++++++++++++++++
 include/linux/tsm.h     | 39 +++++++++++++++++++-
 2 files changed, 118 insertions(+), 1 deletion(-)

diff --git a/drivers/virt/coco/tsm.c b/drivers/virt/coco/tsm.c
index d1c2db83a8ca..1a8c3c096120 100644
--- a/drivers/virt/coco/tsm.c
+++ b/drivers/virt/coco/tsm.c
@@ -11,6 +11,7 @@
 #include <linux/module.h>
 #include <linux/cleanup.h>
 #include <linux/configfs.h>
+#include <linux/tpm.h>
 
 static struct tsm_provider {
 	const struct tsm_ops *ops;
@@ -50,6 +51,85 @@ enum tsm_data_select {
 	TSM_CERTS,
 };
 
+/**
+ * DOC: Trusted Security Module (TSM) Runtime Measurement Register (RTMR) Interface
+ *
+ * The TSM RTMR interface is a common ABI for allowing TVMs to extend
+ * and read measurement registers at runtime, i.e. after the TVM initial
+ * measurement is finalized. TSMs that support such capability will typically
+ * include all runtime measurement registers values into their signed
+ * attestation report, providing the TVM post-boot measurements to e.g. remote
+ * attestation services.
+ *
+ * A TVM uses the TSM RTMR configfs ABI to create all runtime measurement
+ * registers (RTMR) that it needs. Each created RTMR must be configured first
+ * before being readable and extensible. TVM configures an RTMR by setting its
+ * index and optionally by mapping it to one or more TCG PCR indexes.
+ *
+ * A TSM backend statically declares the number of RTMRs it supports and which
+ * hash algorithm must be used when extending them. This declaration is done
+ * through the tsm_capabilities structure, at TSM registration time (see
+ * tsm_register()).
+ */
+
+/**
+ * struct tsm_rtmr_state - tracks the state of a TSM RTMR.
+ * @index: The RTMR hardware index.
+ * @alg: The hash algorithm used for this RTMR.
+ * @digest: The RTMR cached digest value.
+ * @cached_digest: Is the RTMR cached digest valid or not.
+ * @cfg: The configfs item for this RTMR.
+ */
+struct tsm_rtmr_state {
+	u32 index;
+	enum hash_algo alg;
+	u8 digest[TSM_DIGEST_MAX];
+	bool cached_digest;
+	struct config_item cfg;
+};
+
+static bool is_rtmr_configured(struct tsm_rtmr_state *rtmr_state)
+{
+	return rtmr_state->index != U32_MAX;
+}
+
+/**
+ * struct tsm_rtmrs_state - tracks the state of all RTMRs for a TSM.
+ * @rtmrs: The array of all created RTMRs.
+ * @tcg_map: A mapping between TCG PCR and RTMRs, indexed by PCR indexes.
+ * Entry `i` on this map points to an RTMR that covers TCG PCR[i] for the TSM
+ * hash algorithm.
+ * @group: The configfs group for a TSM RTMRs.
+ */
+static struct tsm_rtmrs_state {
+	struct tsm_rtmr_state **rtmrs;
+	const struct tsm_rtmr_state *tcg_map[TPM2_PLATFORM_PCR];
+	struct config_group *group;
+} *tsm_rtmrs;
+
+static int tsm_rtmr_read(struct tsm_provider *tsm, u32 idx,
+			 u8 *digest, size_t digest_size)
+{
+	if (tsm->ops && tsm->ops->rtmr_read)
+		return tsm->ops->rtmr_read(idx, digest, digest_size);
+
+	return -ENXIO;
+}
+
+static int tsm_rtmr_extend(struct tsm_provider *tsm, u32 idx,
+			   const u8 *digest, size_t digest_size)
+{
+	if (tsm->ops && tsm->ops->rtmr_extend)
+		return tsm->ops->rtmr_extend(idx, digest, digest_size);
+
+	return -ENXIO;
+}
+
+static struct tsm_rtmr_state *to_tsm_rtmr_state(struct config_item *cfg)
+{
+	return container_of(cfg, struct tsm_rtmr_state, cfg);
+}
+
 static struct tsm_report *to_tsm_report(struct config_item *cfg)
 {
 	struct tsm_report_state *state =
diff --git a/include/linux/tsm.h b/include/linux/tsm.h
index de8324a2223c..a546983c24fc 100644
--- a/include/linux/tsm.h
+++ b/include/linux/tsm.h
@@ -2,11 +2,13 @@
 #ifndef __TSM_H
 #define __TSM_H
 
+#include <crypto/hash_info.h>
 #include <linux/sizes.h>
 #include <linux/types.h>
 
 #define TSM_INBLOB_MAX 64
 #define TSM_OUTBLOB_MAX SZ_32K
+#define TSM_DIGEST_MAX SHA512_DIGEST_SIZE
 
 /*
  * Privilege level is a nested permission concept to allow confidential
@@ -42,12 +44,44 @@ struct tsm_report {
 	u8 *auxblob;
 };
 
+#define TSM_MAX_RTMR 32
+
+/**
+ * struct tsm_rtmr_desc - Describes a TSM Runtime Measurement Register (RTMR).
+ * @hash_alg: The hash algorithm used to extend this runtime measurement
+ *            register.
+ * @tcg_pcr_mask: A bit mask of all TCG PCRs mapped to this RTMR.
+ */
+struct tsm_rtmr_desc {
+	enum hash_algo hash_alg;
+	unsigned long tcg_pcr_mask;
+};
+
+/**
+ * struct tsm_capabilities - Describes a TSM capabilities.
+ * @num_rtmrs: The number of Runtime Measurement Registers (RTMR) available from
+ *             a TSM.
+ * @rtmr_hash_alg: The hash algorithm used to extend a runtime measurement
+ *                 register.
+ */
+struct tsm_capabilities {
+	size_t num_rtmrs;
+	const struct tsm_rtmr_desc *rtmrs;
+};
+
 /**
  * struct tsm_ops - attributes and operations for tsm instances
  * @name: tsm id reflected in /sys/kernel/config/tsm/report/$report/provider
  * @privlevel_floor: convey base privlevel for nested scenarios
+ * @capabilities: Describe the TSM capabilities, e.g. the number of available
+ *                runtime measurement registers (see `struct tsm_capabilities`).
  * @report_new: Populate @report with the report blob and auxblob
- * (optional), return 0 on successful population, or -errno otherwise
+ *              (optional), return 0 on successful population, or -errno
+ *              otherwise
+ * @rtmr_extend: Extend an RTMR with the provided digest.
+ *               Return 0 on successful extension, or -errno otherwise.
+ * @rtmr_read: Reads the value of an RTMR.
+ *             Return the number of bytes read or -errno for errors.
  *
  * Implementation specific ops, only one is expected to be registered at
  * a time i.e. only one of "sev-guest", "tdx-guest", etc.
@@ -55,7 +89,10 @@ struct tsm_report {
 struct tsm_ops {
 	const char *name;
 	const unsigned int privlevel_floor;
+	const struct tsm_capabilities capabilities;
 	int (*report_new)(struct tsm_report *report, void *data);
+	int (*rtmr_extend)(u32 idx, const u8 *digest, size_t digest_size);
+	ssize_t (*rtmr_read)(u32 idx, u8 *digest, size_t digest_size);
 };
 
 extern const struct config_item_type tsm_report_default_type;
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [RFC PATCH v2 2/4] tsm: Add RTMRs to the configfs-tsm hierarchy
  2024-01-28 21:25 [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI Samuel Ortiz
  2024-01-28 21:25 ` [RFC PATCH v2 1/4] tsm: Runtime measurement register support Samuel Ortiz
@ 2024-01-28 21:25 ` Samuel Ortiz
  2024-01-28 22:38   ` Kuppuswamy Sathyanarayanan
                     ` (2 more replies)
  2024-01-28 21:25 ` [RFC PATCH v2 3/4] tsm: Map RTMRs to TCG TPM PCRs Samuel Ortiz
                   ` (3 subsequent siblings)
  5 siblings, 3 replies; 40+ messages in thread
From: Samuel Ortiz @ 2024-01-28 21:25 UTC (permalink / raw)
  To: Dan Williams
  Cc: Samuel Ortiz, Kuppuswamy Sathyanarayanan, Qinkun Bao, Yao,
	Jiewen, Xing, Cedric, Dionna Amalie Glaze, biao.lu, linux-coco,
	linux-integrity, linux-kernel

RTMRs are defined and managed by their corresponding TSM provider. As
such, they can be configured through the TSM configfs root.

An additional `rtmrs` directory is added by default under the `tsm` one,
where each supported RTMR can be configured:

mkdir /sys/kernel/config/tsm/rtmrs/rtmr0
echo 0 > /sys/kernel/config/tsm/rtmrs/rtmr0/index

An RTMR can not be extended nor read before its configured by assigning
it an index. It is the TSM backend responsibility and choice to map that
index to a hardware RTMR.

Signed-off-by: Samuel Ortiz <sameo@rivosinc.com>
---
 Documentation/ABI/testing/configfs-tsm |  11 ++
 drivers/virt/coco/tsm.c                | 164 +++++++++++++++++++++++++
 2 files changed, 175 insertions(+)

diff --git a/Documentation/ABI/testing/configfs-tsm b/Documentation/ABI/testing/configfs-tsm
index dd24202b5ba5..590e103a9bcd 100644
--- a/Documentation/ABI/testing/configfs-tsm
+++ b/Documentation/ABI/testing/configfs-tsm
@@ -80,3 +80,14 @@ Contact:	linux-coco@lists.linux.dev
 Description:
 		(RO) Indicates the minimum permissible value that can be written
 		to @privlevel.
+
+What:		/sys/kernel/config/tsm/rtmrs/$name/index
+Date:		January, 2024
+KernelVersion:	v6.8
+Contact:	linux-coco@lists.linux.dev
+Description:
+		(RW) A Runtime Measurement Register (RTMR) hardware index.
+                Once created under /sys/kernel/config/tsm/rtmrs/, an RTMR entry
+                can be mapped to a hardware RTMR by writing into its index
+                attribute. The TSM provider will then map the configfs entry to
+                its corresponding hardware register.
diff --git a/drivers/virt/coco/tsm.c b/drivers/virt/coco/tsm.c
index 1a8c3c096120..bb9ed2d2accc 100644
--- a/drivers/virt/coco/tsm.c
+++ b/drivers/virt/coco/tsm.c
@@ -419,6 +419,108 @@ static const struct config_item_type tsm_reports_type = {
 	.ct_group_ops = &tsm_report_group_ops,
 };
 
+static ssize_t tsm_rtmr_index_store(struct config_item *cfg,
+				    const char *buf, size_t len)
+{
+	struct tsm_rtmr_state *rtmr_state = to_tsm_rtmr_state(cfg);
+	const struct tsm_ops *ops;
+	unsigned int val;
+	int rc;
+
+	rc = kstrtouint(buf, 0, &val);
+	if (rc)
+		return rc;
+
+	guard(rwsem_write)(&tsm_rwsem);
+
+	/* Index can only be configured once */
+	if (is_rtmr_configured(rtmr_state))
+		return -EBUSY;
+
+	/* Check that index stays within the TSM provided capabilities */
+	ops = provider.ops;
+	if (!ops)
+		return -ENOTTY;
+
+	if (val > ops->capabilities.num_rtmrs - 1)
+		return -EINVAL;
+
+	/* Check that this index is available */
+	if (tsm_rtmrs->rtmrs[val])
+		return -EINVAL;
+
+	rtmr_state->index = val;
+	rtmr_state->alg = ops->capabilities.rtmrs[val].hash_alg;
+
+	tsm_rtmrs->rtmrs[val] = rtmr_state;
+
+	return len;
+}
+
+static ssize_t tsm_rtmr_index_show(struct config_item *cfg,
+				   char *buf)
+{
+	struct tsm_rtmr_state *rtmr_state = to_tsm_rtmr_state(cfg);
+
+	guard(rwsem_read)(&tsm_rwsem);
+
+	/* An RTMR is not available if it has not been configured */
+	if (!is_rtmr_configured(rtmr_state))
+		return -ENXIO;
+
+	return sysfs_emit(buf, "%u\n", rtmr_state->index);
+}
+CONFIGFS_ATTR(tsm_rtmr_, index);
+
+static struct configfs_attribute *tsm_rtmr_attrs[] = {
+	&tsm_rtmr_attr_index,
+	NULL,
+};
+
+static void tsm_rtmr_item_release(struct config_item *cfg)
+{
+	struct tsm_rtmr_state *state = to_tsm_rtmr_state(cfg);
+
+	kfree(state);
+}
+
+static struct configfs_item_operations tsm_rtmr_item_ops = {
+	.release = tsm_rtmr_item_release,
+};
+
+const struct config_item_type tsm_rtmr_type = {
+	.ct_owner = THIS_MODULE,
+	.ct_attrs = tsm_rtmr_attrs,
+	.ct_item_ops = &tsm_rtmr_item_ops,
+};
+
+static struct config_item *tsm_rtmrs_make_item(struct config_group *group,
+					       const char *name)
+{
+	struct tsm_rtmr_state *state;
+
+	guard(rwsem_read)(&tsm_rwsem);
+	if (!(provider.ops && (provider.ops->capabilities.num_rtmrs > 0)))
+		return ERR_PTR(-ENXIO);
+
+	state = kzalloc(sizeof(*state), GFP_KERNEL);
+	if (!state)
+		return ERR_PTR(-ENOMEM);
+	state->index = U32_MAX;
+
+	config_item_init_type_name(&state->cfg, name, &tsm_rtmr_type);
+	return &state->cfg;
+}
+
+static struct configfs_group_operations tsm_rtmrs_group_ops = {
+	.make_item = tsm_rtmrs_make_item,
+};
+
+static const struct config_item_type tsm_rtmrs_type = {
+	.ct_owner = THIS_MODULE,
+	.ct_group_ops = &tsm_rtmrs_group_ops,
+};
+
 static const struct config_item_type tsm_root_group_type = {
 	.ct_owner = THIS_MODULE,
 };
@@ -433,10 +535,48 @@ static struct configfs_subsystem tsm_configfs = {
 	.su_mutex = __MUTEX_INITIALIZER(tsm_configfs.su_mutex),
 };
 
+static int tsm_rtmr_register(const struct tsm_ops *ops)
+{
+	struct config_group *rtmrs_group;
+
+	lockdep_assert_held_write(&tsm_rwsem);
+
+	if (!ops || !ops->capabilities.num_rtmrs)
+		return 0;
+
+	if (ops->capabilities.num_rtmrs > TSM_MAX_RTMR)
+		return -EINVAL;
+
+	tsm_rtmrs = kzalloc(sizeof(struct tsm_rtmrs_state), GFP_KERNEL);
+	if (!tsm_rtmrs)
+		return -ENOMEM;
+
+	tsm_rtmrs->rtmrs = kcalloc(ops->capabilities.num_rtmrs,
+				   sizeof(struct tsm_rtmr_state *),
+				   GFP_KERNEL);
+	if (!tsm_rtmrs->rtmrs) {
+		kfree(tsm_rtmrs);
+		return -ENOMEM;
+	}
+
+	rtmrs_group = configfs_register_default_group(&tsm_configfs.su_group, "rtmrs",
+						      &tsm_rtmrs_type);
+	if (IS_ERR(rtmrs_group)) {
+		kfree(tsm_rtmrs->rtmrs);
+		kfree(tsm_rtmrs);
+		return PTR_ERR(rtmrs_group);
+	}
+
+	tsm_rtmrs->group = rtmrs_group;
+
+	return 0;
+}
+
 int tsm_register(const struct tsm_ops *ops, void *priv,
 		 const struct config_item_type *type)
 {
 	const struct tsm_ops *conflict;
+	int rc;
 
 	if (!type)
 		type = &tsm_report_default_type;
@@ -450,6 +590,10 @@ int tsm_register(const struct tsm_ops *ops, void *priv,
 		return -EBUSY;
 	}
 
+	rc = tsm_rtmr_register(ops);
+	if (rc < 0)
+		return rc;
+
 	provider.ops = ops;
 	provider.data = priv;
 	provider.type = type;
@@ -457,11 +601,31 @@ int tsm_register(const struct tsm_ops *ops, void *priv,
 }
 EXPORT_SYMBOL_GPL(tsm_register);
 
+static int tsm_rtmr_unregister(const struct tsm_ops *ops)
+{
+	lockdep_assert_held_write(&tsm_rwsem);
+
+	if ((ops) && (ops->capabilities.num_rtmrs > 0)) {
+		configfs_unregister_default_group(tsm_rtmrs->group);
+		kfree(tsm_rtmrs->rtmrs);
+		kfree(tsm_rtmrs);
+	}
+
+	return 0;
+}
+
 int tsm_unregister(const struct tsm_ops *ops)
 {
+	int rc;
+
 	guard(rwsem_write)(&tsm_rwsem);
 	if (ops != provider.ops)
 		return -EBUSY;
+
+	rc = tsm_rtmr_unregister(ops);
+	if (rc < 0)
+		return rc;
+
 	provider.ops = NULL;
 	provider.data = NULL;
 	provider.type = NULL;
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [RFC PATCH v2 3/4] tsm: Map RTMRs to TCG TPM PCRs
  2024-01-28 21:25 [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI Samuel Ortiz
  2024-01-28 21:25 ` [RFC PATCH v2 1/4] tsm: Runtime measurement register support Samuel Ortiz
  2024-01-28 21:25 ` [RFC PATCH v2 2/4] tsm: Add RTMRs to the configfs-tsm hierarchy Samuel Ortiz
@ 2024-01-28 21:25 ` Samuel Ortiz
  2024-01-28 22:44   ` Kuppuswamy Sathyanarayanan
  2024-01-28 21:25 ` [RFC PATCH v2 4/4] tsm: Allow for extending and reading configured RTMRs Samuel Ortiz
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 40+ messages in thread
From: Samuel Ortiz @ 2024-01-28 21:25 UTC (permalink / raw)
  To: Dan Williams
  Cc: Samuel Ortiz, Kuppuswamy Sathyanarayanan, Qinkun Bao, Yao,
	Jiewen, Xing, Cedric, Dionna Amalie Glaze, biao.lu, linux-coco,
	linux-integrity, linux-kernel

Many user space and internal kernel subsystems (e.g. the Linux IMA)
expect a Root of Trust for Storage (RTS) that allows for extending
and reading measurement registers that are compatible with the TCG TPM
PCRs layout, e.g. a TPM. In order to allow those components to
alternatively use a platform TSM as their RTS, a TVM could map the
available RTMRs to one or more TCG TPM PCRs. Once configured, those PCR
to RTMR mappings give the kernel TSM layer all the necessary information
to be a RTS for e.g. the Linux IMA or any other components that expects
a TCG compliant TPM PCRs layout.

TPM PCR mappings are statically configured through the TSM provider
capabilities. A TSM backend defines its number of RTMRs, and for each
one of them can define a bitmask of TCG TPM PCR it maps to. As they are
TSM backend specific, those mappings are to some extend architecture
specific. Each architecture is free to decide and choose how it builds
it, e.g. by requesting an EFI firmware when it supports the EFI_CC
protocol.

The tsm-configfs rtmrs/<rtmrN>tcg_map describes these static mappings.

Signed-off-by: Samuel Ortiz <sameo@rivosinc.com>
---
 Documentation/ABI/testing/configfs-tsm | 14 +++++
 drivers/virt/coco/tsm.c                | 74 ++++++++++++++++++++++++++
 2 files changed, 88 insertions(+)

diff --git a/Documentation/ABI/testing/configfs-tsm b/Documentation/ABI/testing/configfs-tsm
index 590e103a9bcd..5d20a872475e 100644
--- a/Documentation/ABI/testing/configfs-tsm
+++ b/Documentation/ABI/testing/configfs-tsm
@@ -91,3 +91,17 @@ Description:
                 can be mapped to a hardware RTMR by writing into its index
                 attribute. The TSM provider will then map the configfs entry to
                 its corresponding hardware register.
+
+What:		/sys/kernel/config/tsm/rtmrs/$name/tcg_map
+Date:		January, 2024
+KernelVersion:	v6.8
+Contact:	linux-coco@lists.linux.dev
+Description:
+		(RO) A representation of the architecturally defined mapping
+		between this RTMR and one or more TCG TPM PCRs [1]. When using
+		a TSM as Root of Trust for Storage (RTS), TCG TPM PCRs
+		associated semantics and indexes can be used when RTMRs are
+		logically mapped to TPM PCRs.
+
+		[1]: TCG PC Client Specific Platform Firmware Profile Specification
+		https://trustedcomputinggroup.org/resource/pc-client-specific-platform-firmware-profile-specification/
diff --git a/drivers/virt/coco/tsm.c b/drivers/virt/coco/tsm.c
index bb9ed2d2accc..d03cf5173bc9 100644
--- a/drivers/virt/coco/tsm.c
+++ b/drivers/virt/coco/tsm.c
@@ -419,6 +419,46 @@ static const struct config_item_type tsm_reports_type = {
 	.ct_group_ops = &tsm_report_group_ops,
 };
 
+static int tsm_rtmr_build_tcg_map(const struct tsm_provider *tsm,
+				const struct tsm_rtmr_state *rtmr_state,
+				u32 rtmr_idx)
+{
+	const struct tsm_ops *ops;
+	unsigned long pcr_mask;
+	int i;
+
+	lockdep_assert_held_write(&tsm_rwsem);
+
+	ops = tsm->ops;
+	if (!ops)
+		return -ENOTTY;
+
+	if (!ops->capabilities.rtmrs)
+		return -ENXIO;
+
+	pcr_mask = ops->capabilities.rtmrs[rtmr_idx].tcg_pcr_mask;
+
+	/* Check that the PCR mask is valid  */
+	for (i = 0; i < TPM2_PLATFORM_PCR; i++) {
+		if (!(pcr_mask & BIT(i)))
+			continue;
+
+		/* If another RTMR maps to this PCR, the mask is discarded */
+		if (tsm_rtmrs->tcg_map[i] &&
+			tsm_rtmrs->tcg_map[i] != rtmr_state)
+			return -EBUSY;
+	}
+
+	for (i = 0; i < TPM2_PLATFORM_PCR; i++) {
+		if (!(pcr_mask & BIT(i)))
+			continue;
+
+		tsm_rtmrs->tcg_map[i] = rtmr_state;
+	}
+
+	return 0;
+}
+
 static ssize_t tsm_rtmr_index_store(struct config_item *cfg,
 				    const char *buf, size_t len)
 {
@@ -449,6 +489,10 @@ static ssize_t tsm_rtmr_index_store(struct config_item *cfg,
 	if (tsm_rtmrs->rtmrs[val])
 		return -EINVAL;
 
+	rc = tsm_rtmr_build_tcg_map(&provider, rtmr_state, val);
+	if (rc)
+		return rc;
+
 	rtmr_state->index = val;
 	rtmr_state->alg = ops->capabilities.rtmrs[val].hash_alg;
 
@@ -472,8 +516,38 @@ static ssize_t tsm_rtmr_index_show(struct config_item *cfg,
 }
 CONFIGFS_ATTR(tsm_rtmr_, index);
 
+static ssize_t tsm_rtmr_tcg_map_show(struct config_item *cfg,
+				     char *buf)
+{
+	struct tsm_rtmr_state *rtmr_state = to_tsm_rtmr_state(cfg);
+	unsigned int nr_pcrs = ARRAY_SIZE(tsm_rtmrs->tcg_map), i;
+	unsigned long *pcr_mask;
+	ssize_t len;
+
+	/* Build a bitmap mask of all PCRs that this RTMR covers */
+	pcr_mask = bitmap_zalloc(nr_pcrs, GFP_KERNEL);
+	if (!pcr_mask)
+		return -ENOMEM;
+
+	guard(rwsem_read)(&tsm_rwsem);
+	for (i = 0; i < nr_pcrs; i++) {
+		if (tsm_rtmrs->tcg_map[i] != rtmr_state)
+			continue;
+
+		__set_bit(i, pcr_mask);
+	}
+
+	len = bitmap_print_list_to_buf(buf, pcr_mask, nr_pcrs, 0,
+				       nr_pcrs * 3 /* 2 ASCII digits and one comma */);
+	bitmap_free(pcr_mask);
+
+	return len;
+}
+CONFIGFS_ATTR_RO(tsm_rtmr_, tcg_map);
+
 static struct configfs_attribute *tsm_rtmr_attrs[] = {
 	&tsm_rtmr_attr_index,
+	&tsm_rtmr_attr_tcg_map,
 	NULL,
 };
 
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [RFC PATCH v2 4/4] tsm: Allow for extending and reading configured RTMRs
  2024-01-28 21:25 [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI Samuel Ortiz
                   ` (2 preceding siblings ...)
  2024-01-28 21:25 ` [RFC PATCH v2 3/4] tsm: Map RTMRs to TCG TPM PCRs Samuel Ortiz
@ 2024-01-28 21:25 ` Samuel Ortiz
  2024-05-11  2:57   ` James Bottomley
  2024-02-01 22:02 ` [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI Jarkko Sakkinen
  2024-02-02  6:24 ` James Bottomley
  5 siblings, 1 reply; 40+ messages in thread
From: Samuel Ortiz @ 2024-01-28 21:25 UTC (permalink / raw)
  To: Dan Williams
  Cc: Samuel Ortiz, Kuppuswamy Sathyanarayanan, Qinkun Bao, Yao,
	Jiewen, Xing, Cedric, Dionna Amalie Glaze, biao.lu, linux-coco,
	linux-integrity, linux-kernel

The whole purpose of TSM supported RTMRs is for userspace to extend them
with runtime measurements and to read them back.

This can be done through a binary configfs attribute for each RTMR:

rtmr0=/sys/kernel/config/tsm/rtmrs/rtmr0
mkdir $rtmr0
echo 0 > $rtmr0/index
dd if=software_layer_digest > $rtmr0/digest
hexdump $rtmr0/digest

An RTMR digest can not be extended or read before the RTMR is configured
by assigning it an index.

Signed-off-by: Samuel Ortiz <sameo@rivosinc.com>
---
 Documentation/ABI/testing/configfs-tsm | 11 +++++
 drivers/virt/coco/Kconfig              |  1 +
 drivers/virt/coco/tsm.c                | 58 ++++++++++++++++++++++++++
 3 files changed, 70 insertions(+)

diff --git a/Documentation/ABI/testing/configfs-tsm b/Documentation/ABI/testing/configfs-tsm
index 5d20a872475e..dc5c68a49625 100644
--- a/Documentation/ABI/testing/configfs-tsm
+++ b/Documentation/ABI/testing/configfs-tsm
@@ -81,6 +81,17 @@ Description:
 		(RO) Indicates the minimum permissible value that can be written
 		to @privlevel.
 
+What:		/sys/kernel/config/tsm/rtmrs/$name/digest
+Date:		January, 2024
+KernelVersion:	v6.8
+Contact:	linux-coco@lists.linux.dev
+Description:
+		(RW) The value in this attribute is the Runtime Measurement
+		Register (RTMR) digest. Callers can extend this digest with
+		additional hashes by writing into it. Binary blobs written to
+		this attribute must be of the exact length used by the hash
+		algorithm for this RTMR.
+
 What:		/sys/kernel/config/tsm/rtmrs/$name/index
 Date:		January, 2024
 KernelVersion:	v6.8
diff --git a/drivers/virt/coco/Kconfig b/drivers/virt/coco/Kconfig
index 87d142c1f932..5d924bae1ed8 100644
--- a/drivers/virt/coco/Kconfig
+++ b/drivers/virt/coco/Kconfig
@@ -5,6 +5,7 @@
 
 config TSM_REPORTS
 	select CONFIGFS_FS
+	select CRYPTO_HASH_INFO
 	tristate
 
 source "drivers/virt/coco/efi_secret/Kconfig"
diff --git a/drivers/virt/coco/tsm.c b/drivers/virt/coco/tsm.c
index d03cf5173bc9..b4f8cf6ca149 100644
--- a/drivers/virt/coco/tsm.c
+++ b/drivers/virt/coco/tsm.c
@@ -551,6 +551,63 @@ static struct configfs_attribute *tsm_rtmr_attrs[] = {
 	NULL,
 };
 
+static ssize_t tsm_rtmr_digest_read(struct config_item *cfg, void *buf,
+				    size_t count)
+{
+	struct tsm_rtmr_state *rtmr_state = to_tsm_rtmr_state(cfg);
+	int rc, digest_size = hash_digest_size[rtmr_state->alg];
+
+	/* configfs is asking for the digest size */
+	if (!buf)
+		return digest_size;
+
+	if (!is_rtmr_configured(rtmr_state))
+		return -ENXIO;
+
+	if (count > TSM_DIGEST_MAX || count < digest_size)
+		return -EINVAL;
+
+	/* Read from the cached digest */
+	if (rtmr_state->cached_digest) {
+		memcpy(buf, rtmr_state->digest, count);
+		return digest_size;
+	}
+
+	/* Slow path, this RTMR got extended */
+	guard(rwsem_write)(&tsm_rwsem);
+	rc = tsm_rtmr_read(&provider, rtmr_state->index, buf, count);
+	if (rc < 0)
+		return rc;
+
+	/* Update the cached digest */
+	memcpy(rtmr_state->digest, buf, count);
+	rtmr_state->cached_digest = true;
+
+	return rc;
+}
+
+static ssize_t tsm_rtmr_digest_write(struct config_item *cfg,
+				     const void *buf, size_t count)
+{
+	struct tsm_rtmr_state *rtmr_state = to_tsm_rtmr_state(cfg);
+
+	if (!is_rtmr_configured(rtmr_state))
+		return -ENXIO;
+
+	if (count > TSM_DIGEST_MAX || count < hash_digest_size[rtmr_state->alg])
+		return -EINVAL;
+
+	guard(rwsem_write)(&tsm_rwsem);
+	rtmr_state->cached_digest = false;
+	return tsm_rtmr_extend(&provider, rtmr_state->index, buf, count);
+}
+CONFIGFS_BIN_ATTR(tsm_rtmr_, digest, NULL, TSM_DIGEST_MAX);
+
+static struct configfs_bin_attribute *tsm_rtmr_bin_attrs[] = {
+	&tsm_rtmr_attr_digest,
+	NULL,
+};
+
 static void tsm_rtmr_item_release(struct config_item *cfg)
 {
 	struct tsm_rtmr_state *state = to_tsm_rtmr_state(cfg);
@@ -564,6 +621,7 @@ static struct configfs_item_operations tsm_rtmr_item_ops = {
 
 const struct config_item_type tsm_rtmr_type = {
 	.ct_owner = THIS_MODULE,
+	.ct_bin_attrs = tsm_rtmr_bin_attrs,
 	.ct_attrs = tsm_rtmr_attrs,
 	.ct_item_ops = &tsm_rtmr_item_ops,
 };
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 2/4] tsm: Add RTMRs to the configfs-tsm hierarchy
  2024-01-28 21:25 ` [RFC PATCH v2 2/4] tsm: Add RTMRs to the configfs-tsm hierarchy Samuel Ortiz
@ 2024-01-28 22:38   ` Kuppuswamy Sathyanarayanan
  2024-02-01 22:05   ` Jarkko Sakkinen
  2024-02-21 16:16   ` Mikko Ylinen
  2 siblings, 0 replies; 40+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2024-01-28 22:38 UTC (permalink / raw)
  To: Samuel Ortiz, Dan Williams
  Cc: Qinkun Bao, Yao, Jiewen, Xing, Cedric, Dionna Amalie Glaze,
	biao.lu, linux-coco, linux-integrity, linux-kernel


On 1/28/24 1:25 PM, Samuel Ortiz wrote:
> RTMRs are defined and managed by their corresponding TSM provider. As
> such, they can be configured through the TSM configfs root.
>
> An additional `rtmrs` directory is added by default under the `tsm` one,
> where each supported RTMR can be configured:
>
> mkdir /sys/kernel/config/tsm/rtmrs/rtmr0
> echo 0 > /sys/kernel/config/tsm/rtmrs/rtmr0/index
>
> An RTMR can not be extended nor read before its configured by assigning
> it an index. It is the TSM backend responsibility and choice to map that
> index to a hardware RTMR.
>
> Signed-off-by: Samuel Ortiz <sameo@rivosinc.com>
> ---
>  Documentation/ABI/testing/configfs-tsm |  11 ++
>  drivers/virt/coco/tsm.c                | 164 +++++++++++++++++++++++++
>  2 files changed, 175 insertions(+)
>
> diff --git a/Documentation/ABI/testing/configfs-tsm b/Documentation/ABI/testing/configfs-tsm
> index dd24202b5ba5..590e103a9bcd 100644
> --- a/Documentation/ABI/testing/configfs-tsm
> +++ b/Documentation/ABI/testing/configfs-tsm
> @@ -80,3 +80,14 @@ Contact:	linux-coco@lists.linux.dev
>  Description:
>  		(RO) Indicates the minimum permissible value that can be written
>  		to @privlevel.
> +
> +What:		/sys/kernel/config/tsm/rtmrs/$name/index
> +Date:		January, 2024
> +KernelVersion:	v6.8
v6.9?
> +Contact:	linux-coco@lists.linux.dev
> +Description:
> +		(RW) A Runtime Measurement Register (RTMR) hardware index.
> +                Once created under /sys/kernel/config/tsm/rtmrs/, an RTMR entry
> +                can be mapped to a hardware RTMR by writing into its index
> +                attribute. The TSM provider will then map the configfs entry to
> +                its corresponding hardware register.
> diff --git a/drivers/virt/coco/tsm.c b/drivers/virt/coco/tsm.c
> index 1a8c3c096120..bb9ed2d2accc 100644
> --- a/drivers/virt/coco/tsm.c
> +++ b/drivers/virt/coco/tsm.c
> @@ -419,6 +419,108 @@ static const struct config_item_type tsm_reports_type = {
>  	.ct_group_ops = &tsm_report_group_ops,
>  };
>  
> +static ssize_t tsm_rtmr_index_store(struct config_item *cfg,
> +				    const char *buf, size_t len)
> +{
> +	struct tsm_rtmr_state *rtmr_state = to_tsm_rtmr_state(cfg);
> +	const struct tsm_ops *ops;
> +	unsigned int val;
> +	int rc;
> +
> +	rc = kstrtouint(buf, 0, &val);
> +	if (rc)
> +		return rc;
> +
> +	guard(rwsem_write)(&tsm_rwsem);
> +
> +	/* Index can only be configured once */
> +	if (is_rtmr_configured(rtmr_state))
> +		return -EBUSY;
> +
> +	/* Check that index stays within the TSM provided capabilities */
> +	ops = provider.ops;
> +	if (!ops)
> +		return -ENOTTY;
> +
> +	if (val > ops->capabilities.num_rtmrs - 1)
> +		return -EINVAL;
> +
> +	/* Check that this index is available */
> +	if (tsm_rtmrs->rtmrs[val])
> +		return -EINVAL;
> +
> +	rtmr_state->index = val;
> +	rtmr_state->alg = ops->capabilities.rtmrs[val].hash_alg;
> +
> +	tsm_rtmrs->rtmrs[val] = rtmr_state;
> +
> +	return len;
> +}
> +
> +static ssize_t tsm_rtmr_index_show(struct config_item *cfg,
> +				   char *buf)
> +{
> +	struct tsm_rtmr_state *rtmr_state = to_tsm_rtmr_state(cfg);
> +
> +	guard(rwsem_read)(&tsm_rwsem);
> +
> +	/* An RTMR is not available if it has not been configured */
> +	if (!is_rtmr_configured(rtmr_state))
> +		return -ENXIO;
> +
> +	return sysfs_emit(buf, "%u\n", rtmr_state->index);
> +}
> +CONFIGFS_ATTR(tsm_rtmr_, index);
> +
> +static struct configfs_attribute *tsm_rtmr_attrs[] = {
> +	&tsm_rtmr_attr_index,
> +	NULL,
> +};
> +
> +static void tsm_rtmr_item_release(struct config_item *cfg)
> +{
> +	struct tsm_rtmr_state *state = to_tsm_rtmr_state(cfg);
> +
> +	kfree(state);

I think you need to clear the index history as well?

> +}
> +
> +static struct configfs_item_operations tsm_rtmr_item_ops = {
> +	.release = tsm_rtmr_item_release,
> +};
> +
> +const struct config_item_type tsm_rtmr_type = {
> +	.ct_owner = THIS_MODULE,
> +	.ct_attrs = tsm_rtmr_attrs,
> +	.ct_item_ops = &tsm_rtmr_item_ops,
> +};
> +
> +static struct config_item *tsm_rtmrs_make_item(struct config_group *group,
> +					       const char *name)
> +{
> +	struct tsm_rtmr_state *state;
> +
> +	guard(rwsem_read)(&tsm_rwsem);
> +	if (!(provider.ops && (provider.ops->capabilities.num_rtmrs > 0)))
> +		return ERR_PTR(-ENXIO);
> +
> +	state = kzalloc(sizeof(*state), GFP_KERNEL);
> +	if (!state)
> +		return ERR_PTR(-ENOMEM);
> +	state->index = U32_MAX;
> +
> +	config_item_init_type_name(&state->cfg, name, &tsm_rtmr_type);
> +	return &state->cfg;
> +}
> +
> +static struct configfs_group_operations tsm_rtmrs_group_ops = {
> +	.make_item = tsm_rtmrs_make_item,
> +};
> +
> +static const struct config_item_type tsm_rtmrs_type = {
> +	.ct_owner = THIS_MODULE,
> +	.ct_group_ops = &tsm_rtmrs_group_ops,
> +};
> +
>  static const struct config_item_type tsm_root_group_type = {
>  	.ct_owner = THIS_MODULE,
>  };
> @@ -433,10 +535,48 @@ static struct configfs_subsystem tsm_configfs = {
>  	.su_mutex = __MUTEX_INITIALIZER(tsm_configfs.su_mutex),
>  };
>  
> +static int tsm_rtmr_register(const struct tsm_ops *ops)
> +{
> +	struct config_group *rtmrs_group;
> +
> +	lockdep_assert_held_write(&tsm_rwsem);
> +
> +	if (!ops || !ops->capabilities.num_rtmrs)
> +		return 0;
> +
> +	if (ops->capabilities.num_rtmrs > TSM_MAX_RTMR)
> +		return -EINVAL;
> +
> +	tsm_rtmrs = kzalloc(sizeof(struct tsm_rtmrs_state), GFP_KERNEL);
> +	if (!tsm_rtmrs)
> +		return -ENOMEM;
> +
> +	tsm_rtmrs->rtmrs = kcalloc(ops->capabilities.num_rtmrs,
> +				   sizeof(struct tsm_rtmr_state *),
> +				   GFP_KERNEL);
> +	if (!tsm_rtmrs->rtmrs) {
> +		kfree(tsm_rtmrs);
> +		return -ENOMEM;
> +	}
> +
> +	rtmrs_group = configfs_register_default_group(&tsm_configfs.su_group, "rtmrs",
> +						      &tsm_rtmrs_type);
> +	if (IS_ERR(rtmrs_group)) {
> +		kfree(tsm_rtmrs->rtmrs);
> +		kfree(tsm_rtmrs);
> +		return PTR_ERR(rtmrs_group);
> +	}
> +
> +	tsm_rtmrs->group = rtmrs_group;
> +
> +	return 0;
> +}
> +
>  int tsm_register(const struct tsm_ops *ops, void *priv,
>  		 const struct config_item_type *type)
>  {
>  	const struct tsm_ops *conflict;
> +	int rc;
>  
>  	if (!type)
>  		type = &tsm_report_default_type;
> @@ -450,6 +590,10 @@ int tsm_register(const struct tsm_ops *ops, void *priv,
>  		return -EBUSY;
>  	}
>  
> +	rc = tsm_rtmr_register(ops);
> +	if (rc < 0)
> +		return rc;
> +
>  	provider.ops = ops;
>  	provider.data = priv;
>  	provider.type = type;
> @@ -457,11 +601,31 @@ int tsm_register(const struct tsm_ops *ops, void *priv,
>  }
>  EXPORT_SYMBOL_GPL(tsm_register);
>  
> +static int tsm_rtmr_unregister(const struct tsm_ops *ops)
> +{
> +	lockdep_assert_held_write(&tsm_rwsem);
> +
> +	if ((ops) && (ops->capabilities.num_rtmrs > 0)) {
This check is used in multiple places. May you can add a helper function
for it. is_valid_rtmr()?
> +		configfs_unregister_default_group(tsm_rtmrs->group);
> +		kfree(tsm_rtmrs->rtmrs);
> +		kfree(tsm_rtmrs);
> +	}
> +
> +	return 0;
> +}
> +
>  int tsm_unregister(const struct tsm_ops *ops)
>  {
> +	int rc;
> +
>  	guard(rwsem_write)(&tsm_rwsem);
>  	if (ops != provider.ops)
>  		return -EBUSY;
> +
> +	rc = tsm_rtmr_unregister(ops);
> +	if (rc < 0)
> +		return rc;
> +
>  	provider.ops = NULL;
>  	provider.data = NULL;
>  	provider.type = NULL;

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 3/4] tsm: Map RTMRs to TCG TPM PCRs
  2024-01-28 21:25 ` [RFC PATCH v2 3/4] tsm: Map RTMRs to TCG TPM PCRs Samuel Ortiz
@ 2024-01-28 22:44   ` Kuppuswamy Sathyanarayanan
  2024-02-02  6:18     ` James Bottomley
  0 siblings, 1 reply; 40+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2024-01-28 22:44 UTC (permalink / raw)
  To: Samuel Ortiz, Dan Williams
  Cc: Qinkun Bao, Yao, Jiewen, Xing, Cedric, Dionna Amalie Glaze,
	biao.lu, linux-coco, linux-integrity, linux-kernel


On 1/28/24 1:25 PM, Samuel Ortiz wrote:
> Many user space and internal kernel subsystems (e.g. the Linux IMA)
> expect a Root of Trust for Storage (RTS) that allows for extending
> and reading measurement registers that are compatible with the TCG TPM
> PCRs layout, e.g. a TPM. In order to allow those components to
> alternatively use a platform TSM as their RTS, a TVM could map the
> available RTMRs to one or more TCG TPM PCRs. Once configured, those PCR
> to RTMR mappings give the kernel TSM layer all the necessary information
> to be a RTS for e.g. the Linux IMA or any other components that expects
> a TCG compliant TPM PCRs layout.

Why expose the mapping to user space? IMO, the goal should be
to let user space application work without any changes. So we should
try to hide this conversion in kernel and let userspace code to use
PCR as usual.

>
> TPM PCR mappings are statically configured through the TSM provider
> capabilities. A TSM backend defines its number of RTMRs, and for each
> one of them can define a bitmask of TCG TPM PCR it maps to. As they are
> TSM backend specific, those mappings are to some extend architecture
> specific. Each architecture is free to decide and choose how it builds
> it, e.g. by requesting an EFI firmware when it supports the EFI_CC
> protocol.
>
> The tsm-configfs rtmrs/<rtmrN>tcg_map describes these static mappings.
>
> Signed-off-by: Samuel Ortiz <sameo@rivosinc.com>
> ---
>  Documentation/ABI/testing/configfs-tsm | 14 +++++
>  drivers/virt/coco/tsm.c                | 74 ++++++++++++++++++++++++++
>  2 files changed, 88 insertions(+)
>
> diff --git a/Documentation/ABI/testing/configfs-tsm b/Documentation/ABI/testing/configfs-tsm
> index 590e103a9bcd..5d20a872475e 100644
> --- a/Documentation/ABI/testing/configfs-tsm
> +++ b/Documentation/ABI/testing/configfs-tsm
> @@ -91,3 +91,17 @@ Description:
>                  can be mapped to a hardware RTMR by writing into its index
>                  attribute. The TSM provider will then map the configfs entry to
>                  its corresponding hardware register.
> +
> +What:		/sys/kernel/config/tsm/rtmrs/$name/tcg_map
> +Date:		January, 2024
> +KernelVersion:	v6.8
> +Contact:	linux-coco@lists.linux.dev
> +Description:
> +		(RO) A representation of the architecturally defined mapping
> +		between this RTMR and one or more TCG TPM PCRs [1]. When using
> +		a TSM as Root of Trust for Storage (RTS), TCG TPM PCRs
> +		associated semantics and indexes can be used when RTMRs are
> +		logically mapped to TPM PCRs.
> +
> +		[1]: TCG PC Client Specific Platform Firmware Profile Specification
> +		https://trustedcomputinggroup.org/resource/pc-client-specific-platform-firmware-profile-specification/
> diff --git a/drivers/virt/coco/tsm.c b/drivers/virt/coco/tsm.c
> index bb9ed2d2accc..d03cf5173bc9 100644
> --- a/drivers/virt/coco/tsm.c
> +++ b/drivers/virt/coco/tsm.c
> @@ -419,6 +419,46 @@ static const struct config_item_type tsm_reports_type = {
>  	.ct_group_ops = &tsm_report_group_ops,
>  };
>  
> +static int tsm_rtmr_build_tcg_map(const struct tsm_provider *tsm,
> +				const struct tsm_rtmr_state *rtmr_state,
> +				u32 rtmr_idx)
> +{
> +	const struct tsm_ops *ops;
> +	unsigned long pcr_mask;
> +	int i;
> +
> +	lockdep_assert_held_write(&tsm_rwsem);
> +
> +	ops = tsm->ops;
> +	if (!ops)
> +		return -ENOTTY;
> +
> +	if (!ops->capabilities.rtmrs)
> +		return -ENXIO;
> +
> +	pcr_mask = ops->capabilities.rtmrs[rtmr_idx].tcg_pcr_mask;
> +
> +	/* Check that the PCR mask is valid  */
> +	for (i = 0; i < TPM2_PLATFORM_PCR; i++) {
> +		if (!(pcr_mask & BIT(i)))
> +			continue;
> +
> +		/* If another RTMR maps to this PCR, the mask is discarded */
> +		if (tsm_rtmrs->tcg_map[i] &&
> +			tsm_rtmrs->tcg_map[i] != rtmr_state)
> +			return -EBUSY;
> +	}
> +
> +	for (i = 0; i < TPM2_PLATFORM_PCR; i++) {
> +		if (!(pcr_mask & BIT(i)))
> +			continue;
> +
> +		tsm_rtmrs->tcg_map[i] = rtmr_state;
> +	}
> +
> +	return 0;
> +}
> +
>  static ssize_t tsm_rtmr_index_store(struct config_item *cfg,
>  				    const char *buf, size_t len)
>  {
> @@ -449,6 +489,10 @@ static ssize_t tsm_rtmr_index_store(struct config_item *cfg,
>  	if (tsm_rtmrs->rtmrs[val])
>  		return -EINVAL;
>  
> +	rc = tsm_rtmr_build_tcg_map(&provider, rtmr_state, val);
> +	if (rc)
> +		return rc;
> +
>  	rtmr_state->index = val;
>  	rtmr_state->alg = ops->capabilities.rtmrs[val].hash_alg;
>  
> @@ -472,8 +516,38 @@ static ssize_t tsm_rtmr_index_show(struct config_item *cfg,
>  }
>  CONFIGFS_ATTR(tsm_rtmr_, index);
>  
> +static ssize_t tsm_rtmr_tcg_map_show(struct config_item *cfg,
> +				     char *buf)
> +{
> +	struct tsm_rtmr_state *rtmr_state = to_tsm_rtmr_state(cfg);
> +	unsigned int nr_pcrs = ARRAY_SIZE(tsm_rtmrs->tcg_map), i;
> +	unsigned long *pcr_mask;
> +	ssize_t len;
> +
> +	/* Build a bitmap mask of all PCRs that this RTMR covers */
> +	pcr_mask = bitmap_zalloc(nr_pcrs, GFP_KERNEL);
> +	if (!pcr_mask)
> +		return -ENOMEM;
> +
> +	guard(rwsem_read)(&tsm_rwsem);
> +	for (i = 0; i < nr_pcrs; i++) {
> +		if (tsm_rtmrs->tcg_map[i] != rtmr_state)
> +			continue;
> +
> +		__set_bit(i, pcr_mask);
> +	}
> +
> +	len = bitmap_print_list_to_buf(buf, pcr_mask, nr_pcrs, 0,
> +				       nr_pcrs * 3 /* 2 ASCII digits and one comma */);
> +	bitmap_free(pcr_mask);
> +
> +	return len;
> +}
> +CONFIGFS_ATTR_RO(tsm_rtmr_, tcg_map);
> +
>  static struct configfs_attribute *tsm_rtmr_attrs[] = {
>  	&tsm_rtmr_attr_index,
> +	&tsm_rtmr_attr_tcg_map,
>  	NULL,
>  };
>  

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 1/4] tsm: Runtime measurement register support
  2024-01-28 21:25 ` [RFC PATCH v2 1/4] tsm: Runtime measurement register support Samuel Ortiz
@ 2024-01-29 16:57   ` Dionna Amalie Glaze
  2024-02-01 22:03   ` Jarkko Sakkinen
  1 sibling, 0 replies; 40+ messages in thread
From: Dionna Amalie Glaze @ 2024-01-29 16:57 UTC (permalink / raw)
  To: Samuel Ortiz
  Cc: Dan Williams, Kuppuswamy Sathyanarayanan, Qinkun Bao, Yao,
	Jiewen, Xing, Cedric, biao.lu, linux-coco, linux-integrity,
	linux-kernel

The rtmr backend doesn't specify the digest size it expects to user
space, so rtmr_extend could be zero-fill, or provide a truncated
update, or be strict and return an error. Should the expected digest
size for writes not also be a RO attribute?

On Sun, Jan 28, 2024 at 1:27 PM Samuel Ortiz <sameo@rivosinc.com> wrote:
>
> Some confidential computing architecture (Intel TDX, ARM-CCA, RISC-V
> CoVE) provide the TVM (confidential computing guest) with a set of
> runtime measurement registers (RTMR). TVMs can extend those registers
> with their measurements at runtime, i.e. after the TVM initial
> measurements are finalized and the TVM actually runs.
>
> RTMRs are separated from the initial measurement registers set, and TSMs
> typically includes RTMR values into a distinct section of their signed
> attestion reports.
>
> We add support for extending and reading a TSM runtime measurement
> registers by extending the TSM ops structure with resp. an rtmr_extend()
> and an rtmr_read() function pointers. TSM providers/backends will
> implement those ops if they are capable of exposing RTMRs to their
> TVMs. This capability is now described by a tsm_capabilites structure,
> passed by the TSM provider to the TSM framework at registration time.
>
> TVMs can configure, extend and read RTMRs from the configfs-tsm interface.
>
> Signed-off-by: Samuel Ortiz <sameo@rivosinc.com>
> ---
>  drivers/virt/coco/tsm.c | 80 +++++++++++++++++++++++++++++++++++++++++
>  include/linux/tsm.h     | 39 +++++++++++++++++++-
>  2 files changed, 118 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/virt/coco/tsm.c b/drivers/virt/coco/tsm.c
> index d1c2db83a8ca..1a8c3c096120 100644
> --- a/drivers/virt/coco/tsm.c
> +++ b/drivers/virt/coco/tsm.c
> @@ -11,6 +11,7 @@
>  #include <linux/module.h>
>  #include <linux/cleanup.h>
>  #include <linux/configfs.h>
> +#include <linux/tpm.h>
>
>  static struct tsm_provider {
>         const struct tsm_ops *ops;
> @@ -50,6 +51,85 @@ enum tsm_data_select {
>         TSM_CERTS,
>  };
>
> +/**
> + * DOC: Trusted Security Module (TSM) Runtime Measurement Register (RTMR) Interface
> + *
> + * The TSM RTMR interface is a common ABI for allowing TVMs to extend
> + * and read measurement registers at runtime, i.e. after the TVM initial
> + * measurement is finalized. TSMs that support such capability will typically
> + * include all runtime measurement registers values into their signed
> + * attestation report, providing the TVM post-boot measurements to e.g. remote
> + * attestation services.
> + *
> + * A TVM uses the TSM RTMR configfs ABI to create all runtime measurement
> + * registers (RTMR) that it needs. Each created RTMR must be configured first
> + * before being readable and extensible. TVM configures an RTMR by setting its
> + * index and optionally by mapping it to one or more TCG PCR indexes.
> + *
> + * A TSM backend statically declares the number of RTMRs it supports and which
> + * hash algorithm must be used when extending them. This declaration is done
> + * through the tsm_capabilities structure, at TSM registration time (see
> + * tsm_register()).
> + */
> +
> +/**
> + * struct tsm_rtmr_state - tracks the state of a TSM RTMR.
> + * @index: The RTMR hardware index.
> + * @alg: The hash algorithm used for this RTMR.
> + * @digest: The RTMR cached digest value.
> + * @cached_digest: Is the RTMR cached digest valid or not.
> + * @cfg: The configfs item for this RTMR.
> + */
> +struct tsm_rtmr_state {
> +       u32 index;
> +       enum hash_algo alg;
> +       u8 digest[TSM_DIGEST_MAX];
> +       bool cached_digest;
> +       struct config_item cfg;
> +};
> +
> +static bool is_rtmr_configured(struct tsm_rtmr_state *rtmr_state)
> +{
> +       return rtmr_state->index != U32_MAX;
> +}
> +
> +/**
> + * struct tsm_rtmrs_state - tracks the state of all RTMRs for a TSM.
> + * @rtmrs: The array of all created RTMRs.
> + * @tcg_map: A mapping between TCG PCR and RTMRs, indexed by PCR indexes.
> + * Entry `i` on this map points to an RTMR that covers TCG PCR[i] for the TSM
> + * hash algorithm.
> + * @group: The configfs group for a TSM RTMRs.
> + */
> +static struct tsm_rtmrs_state {
> +       struct tsm_rtmr_state **rtmrs;
> +       const struct tsm_rtmr_state *tcg_map[TPM2_PLATFORM_PCR];
> +       struct config_group *group;
> +} *tsm_rtmrs;
> +
> +static int tsm_rtmr_read(struct tsm_provider *tsm, u32 idx,
> +                        u8 *digest, size_t digest_size)
> +{
> +       if (tsm->ops && tsm->ops->rtmr_read)
> +               return tsm->ops->rtmr_read(idx, digest, digest_size);
> +
> +       return -ENXIO;
> +}
> +
> +static int tsm_rtmr_extend(struct tsm_provider *tsm, u32 idx,
> +                          const u8 *digest, size_t digest_size)
> +{
> +       if (tsm->ops && tsm->ops->rtmr_extend)
> +               return tsm->ops->rtmr_extend(idx, digest, digest_size);
> +
> +       return -ENXIO;
> +}
> +
> +static struct tsm_rtmr_state *to_tsm_rtmr_state(struct config_item *cfg)
> +{
> +       return container_of(cfg, struct tsm_rtmr_state, cfg);
> +}
> +
>  static struct tsm_report *to_tsm_report(struct config_item *cfg)
>  {
>         struct tsm_report_state *state =
> diff --git a/include/linux/tsm.h b/include/linux/tsm.h
> index de8324a2223c..a546983c24fc 100644
> --- a/include/linux/tsm.h
> +++ b/include/linux/tsm.h
> @@ -2,11 +2,13 @@
>  #ifndef __TSM_H
>  #define __TSM_H
>
> +#include <crypto/hash_info.h>
>  #include <linux/sizes.h>
>  #include <linux/types.h>
>
>  #define TSM_INBLOB_MAX 64
>  #define TSM_OUTBLOB_MAX SZ_32K
> +#define TSM_DIGEST_MAX SHA512_DIGEST_SIZE
>
>  /*
>   * Privilege level is a nested permission concept to allow confidential
> @@ -42,12 +44,44 @@ struct tsm_report {
>         u8 *auxblob;
>  };
>
> +#define TSM_MAX_RTMR 32
> +
> +/**
> + * struct tsm_rtmr_desc - Describes a TSM Runtime Measurement Register (RTMR).
> + * @hash_alg: The hash algorithm used to extend this runtime measurement
> + *            register.
> + * @tcg_pcr_mask: A bit mask of all TCG PCRs mapped to this RTMR.
> + */
> +struct tsm_rtmr_desc {
> +       enum hash_algo hash_alg;
> +       unsigned long tcg_pcr_mask;
> +};
> +
> +/**
> + * struct tsm_capabilities - Describes a TSM capabilities.
> + * @num_rtmrs: The number of Runtime Measurement Registers (RTMR) available from
> + *             a TSM.
> + * @rtmr_hash_alg: The hash algorithm used to extend a runtime measurement
> + *                 register.
> + */
> +struct tsm_capabilities {
> +       size_t num_rtmrs;
> +       const struct tsm_rtmr_desc *rtmrs;
> +};
> +
>  /**
>   * struct tsm_ops - attributes and operations for tsm instances
>   * @name: tsm id reflected in /sys/kernel/config/tsm/report/$report/provider
>   * @privlevel_floor: convey base privlevel for nested scenarios
> + * @capabilities: Describe the TSM capabilities, e.g. the number of available
> + *                runtime measurement registers (see `struct tsm_capabilities`).
>   * @report_new: Populate @report with the report blob and auxblob
> - * (optional), return 0 on successful population, or -errno otherwise
> + *              (optional), return 0 on successful population, or -errno
> + *              otherwise
> + * @rtmr_extend: Extend an RTMR with the provided digest.
> + *               Return 0 on successful extension, or -errno otherwise.
> + * @rtmr_read: Reads the value of an RTMR.
> + *             Return the number of bytes read or -errno for errors.
>   *
>   * Implementation specific ops, only one is expected to be registered at
>   * a time i.e. only one of "sev-guest", "tdx-guest", etc.
> @@ -55,7 +89,10 @@ struct tsm_report {
>  struct tsm_ops {
>         const char *name;
>         const unsigned int privlevel_floor;
> +       const struct tsm_capabilities capabilities;
>         int (*report_new)(struct tsm_report *report, void *data);
> +       int (*rtmr_extend)(u32 idx, const u8 *digest, size_t digest_size);
> +       ssize_t (*rtmr_read)(u32 idx, u8 *digest, size_t digest_size);
>  };
>
>  extern const struct config_item_type tsm_report_default_type;
> --
> 2.42.0
>


-- 
-Dionna Glaze, PhD (she/her)

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI
  2024-01-28 21:25 [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI Samuel Ortiz
                   ` (3 preceding siblings ...)
  2024-01-28 21:25 ` [RFC PATCH v2 4/4] tsm: Allow for extending and reading configured RTMRs Samuel Ortiz
@ 2024-02-01 22:02 ` Jarkko Sakkinen
  2024-02-02  6:24 ` James Bottomley
  5 siblings, 0 replies; 40+ messages in thread
From: Jarkko Sakkinen @ 2024-02-01 22:02 UTC (permalink / raw)
  To: Samuel Ortiz, Dan Williams
  Cc: Kuppuswamy Sathyanarayanan, Qinkun Bao, Yao, Jiewen, Xing,
	Cedric, Dionna Amalie Glaze, biao.lu, linux-coco,
	linux-integrity, linux-kernel

On Sun Jan 28, 2024 at 11:25 PM EET, Samuel Ortiz wrote:
> Some confidential computing architectures (Intel TDX, ARM CCA, RISC-V
> CoVE) provide their guests with a set of measurements registers that can
> be extended at runtime, i.e. after the initial, host-initiated
> measurements of the TVM are finalized. Those runtime measurement
> registers (RTMR) are isolated from the host accessible ones but TSMs
> include them in their signed attestation reports.

Please expand "TSM" acronym and explain what it is.

> All architectures supporting RTMRs expose a similar interface to their

Please expand RTMR *everywhere* ot "measurement registers". It is
totally useless terminology.

> TVMs: An extension command/call that takes a measurement value and an

What is TVM?

> RTMR index to extend it with, and a readback command for reading an RTMR
> value back (taking an RTMR index as an argument as well). This patch series
> builds an architecture agnostic, configfs-based ABI for userspace to extend
> and read RTMR values back. It extends the current TSM ops structure and
> each confidential computing architecture can implement this extension to
> provide RTMR support.

This patch set should simplify its gibberish terminology to common
language.

BR, Jarkko

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 1/4] tsm: Runtime measurement register support
  2024-01-28 21:25 ` [RFC PATCH v2 1/4] tsm: Runtime measurement register support Samuel Ortiz
  2024-01-29 16:57   ` Dionna Amalie Glaze
@ 2024-02-01 22:03   ` Jarkko Sakkinen
  1 sibling, 0 replies; 40+ messages in thread
From: Jarkko Sakkinen @ 2024-02-01 22:03 UTC (permalink / raw)
  To: Samuel Ortiz, Dan Williams
  Cc: Kuppuswamy Sathyanarayanan, Qinkun Bao, Yao, Jiewen, Xing,
	Cedric, Dionna Amalie Glaze, biao.lu, linux-coco,
	linux-integrity, linux-kernel

On Sun Jan 28, 2024 at 11:25 PM EET, Samuel Ortiz wrote:
> Some confidential computing architecture (Intel TDX, ARM-CCA, RISC-V
> CoVE) provide the TVM (confidential computing guest) with a set of
> runtime measurement registers (RTMR). TVMs can extend those registers
> with their measurements at runtime, i.e. after the TVM initial
> measurements are finalized and the TVM actually runs.
>
> RTMRs are separated from the initial measurement registers set, and TSMs

"measurement registers" and you do not need to cross-check what the
heck RTMR was anyway.

BR, Jarkko

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 2/4] tsm: Add RTMRs to the configfs-tsm hierarchy
  2024-01-28 21:25 ` [RFC PATCH v2 2/4] tsm: Add RTMRs to the configfs-tsm hierarchy Samuel Ortiz
  2024-01-28 22:38   ` Kuppuswamy Sathyanarayanan
@ 2024-02-01 22:05   ` Jarkko Sakkinen
  2024-02-21 16:16   ` Mikko Ylinen
  2 siblings, 0 replies; 40+ messages in thread
From: Jarkko Sakkinen @ 2024-02-01 22:05 UTC (permalink / raw)
  To: Samuel Ortiz, Dan Williams
  Cc: Kuppuswamy Sathyanarayanan, Qinkun Bao, Yao, Jiewen, Xing,
	Cedric, Dionna Amalie Glaze, biao.lu, linux-coco,
	linux-integrity, linux-kernel

On Sun Jan 28, 2024 at 11:25 PM EET, Samuel Ortiz wrote:
> RTMRs are defined and managed by their corresponding TSM provider. As
> such, they can be configured through the TSM configfs root.
>
> An additional `rtmrs` directory is added by default under the `tsm` one,
> where each supported RTMR can be configured:
>
> mkdir /sys/kernel/config/tsm/rtmrs/rtmr0
> echo 0 > /sys/kernel/config/tsm/rtmrs/rtmr0/index

/sys/kernel/config/tsm/registers/0

Does not mean that I agree with "tsm" sub-path as I don't know what
TSM is by definition.

BR, Jarkko

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 3/4] tsm: Map RTMRs to TCG TPM PCRs
  2024-01-28 22:44   ` Kuppuswamy Sathyanarayanan
@ 2024-02-02  6:18     ` James Bottomley
  0 siblings, 0 replies; 40+ messages in thread
From: James Bottomley @ 2024-02-02  6:18 UTC (permalink / raw)
  To: Kuppuswamy Sathyanarayanan, Samuel Ortiz, Dan Williams
  Cc: Qinkun Bao, Yao, Jiewen, Xing, Cedric, Dionna Amalie Glaze,
	biao.lu, linux-coco, linux-integrity, linux-kernel

On Sun, 2024-01-28 at 14:44 -0800, Kuppuswamy Sathyanarayanan wrote:
> 
> On 1/28/24 1:25 PM, Samuel Ortiz wrote:
> > Many user space and internal kernel subsystems (e.g. the Linux IMA)
> > expect a Root of Trust for Storage (RTS) that allows for extending
> > and reading measurement registers that are compatible with the TCG
> > TPM PCRs layout, e.g. a TPM. In order to allow those components to
> > alternatively use a platform TSM as their RTS, a TVM could map the
> > available RTMRs to one or more TCG TPM PCRs. Once configured, those
> > PCR to RTMR mappings give the kernel TSM layer all the necessary
> > information to be a RTS for e.g. the Linux IMA or any other
> > components that expects a TCG compliant TPM PCRs layout.
> 
> Why expose the mapping to user space? IMO, the goal should be
> to let user space application work without any changes. So we should
> try to hide this conversion in kernel and let userspace code to use
> PCR as usual.

There's also the question about use case: if we're going to measure
into RTMRs as though they were PCRs, they will need to collect the
kernel measurements as well, which means the mapping will have to be
fixed in early boot when the first TPM measurement is done.

James


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI
  2024-01-28 21:25 [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI Samuel Ortiz
                   ` (4 preceding siblings ...)
  2024-02-01 22:02 ` [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI Jarkko Sakkinen
@ 2024-02-02  6:24 ` James Bottomley
  2024-02-02 23:07   ` Dan Middleton
  5 siblings, 1 reply; 40+ messages in thread
From: James Bottomley @ 2024-02-02  6:24 UTC (permalink / raw)
  To: Samuel Ortiz, Dan Williams
  Cc: Kuppuswamy Sathyanarayanan, Qinkun Bao, Yao, Jiewen, Xing,
	Cedric, Dionna Amalie Glaze, biao.lu, linux-coco,
	linux-integrity, linux-kernel

On Sun, 2024-01-28 at 22:25 +0100, Samuel Ortiz wrote:
> All architectures supporting RTMRs expose a similar interface to
> their TVMs: An extension command/call that takes a measurement value
> and an RTMR index to extend it with, and a readback command for
> reading an RTMR value back (taking an RTMR index as an argument as
> well). This patch series builds an architecture agnostic, configfs-
> based ABI for userspace to extend and read RTMR values back. It
> extends the current TSM ops structure and each confidential computing
> architecture can implement this extension to provide RTMR support.

What's the actual use case for this?  At the moment the TPM PCRs only
provide a read interface to userspace (via /sys/class/tpm/tpmX/pcr-
shaY/Z) and don't have any extension ability becuase nothing in
userspace currently extends them.

The only current runtime use for TPM PCRs is IMA, which is in-kernel
(and which this patch doesn't enable).

Without the ability to log, this interface is unusable anyway, but even
with that it's not clear that you need the ability separately to extend
PCRs because the extension and log entry should be done atomically to
prevent the log going out of sync with the PCRs, so it would seem a log
first interface would be the correct way of doing this rather than a
PCR first one.

James


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI
  2024-02-02  6:24 ` James Bottomley
@ 2024-02-02 23:07   ` Dan Middleton
  2024-02-03  6:03     ` James Bottomley
  0 siblings, 1 reply; 40+ messages in thread
From: Dan Middleton @ 2024-02-02 23:07 UTC (permalink / raw)
  To: James Bottomley, Samuel Ortiz, Dan Williams
  Cc: Kuppuswamy Sathyanarayanan, Qinkun Bao, Yao, Jiewen, Xing,
	Cedric, Dionna Amalie Glaze, biao.lu, linux-coco,
	linux-integrity, linux-kernel


On 2/2/24 12:24 AM, James Bottomley wrote:
> On Sun, 2024-01-28 at 22:25 +0100, Samuel Ortiz wrote:
>> All architectures supporting RTMRs expose a similar interface to
>> their TVMs: An extension command/call that takes a measurement value
>> and an RTMR index to extend it with, and a readback command for
>> reading an RTMR value back (taking an RTMR index as an argument as
>> well). This patch series builds an architecture agnostic, configfs-
>> based ABI for userspace to extend and read RTMR values back. It
>> extends the current TSM ops structure and each confidential computing
>> architecture can implement this extension to provide RTMR support.
> What's the actual use case for this?  At the moment the TPM PCRs only
> provide a read interface to userspace (via /sys/class/tpm/tpmX/pcr-
> shaY/Z) and don't have any extension ability becuase nothing in
> userspace currently extends them.
>
> The only current runtime use for TPM PCRs is IMA, which is in-kernel
> (and which this patch doesn't enable).
>
> Without the ability to log, this interface is unusable anyway, but even
> with that it's not clear that you need the ability separately to extend
> PCRs because the extension and log entry should be done atomically to
> prevent the log going out of sync with the PCRs, so it would seem a log
> first interface would be the correct way of doing this rather than a
> PCR first one.
>
> James
>
>

While we clearly need to cover PCR-like usages, I think Confidential
Computing affords usages that go beyond TPM.

For example, Attested Containers [1] (and similar explorations in CNCF
Confidential Containers [2]) extends the measurement chain into the guest.
There, a trusted agent measures container images, and extends an RTMR
with those measurements. Particularly in the case of containers, the 
existing
runtime infrastructure is user mode oriented. However the generalization
here is in providing a mechanism to strongly identify an application or
behavior provided by the TVM.

Less concretely, I think this is an area for developer creativity.
Attestation is one of the main APIs that CC gives application developers and
these runtime extendable fields provide a further degree of creativity.

[1] ACON https://github.com/intel/acon
[2] CoCo 
https://github.com/confidential-containers/guest-components/commit/3c75201a8ba0327fb41b68b7e1521ff517e3ca9f

Regards,
Dan


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI
  2024-02-02 23:07   ` Dan Middleton
@ 2024-02-03  6:03     ` James Bottomley
  2024-02-03  7:13       ` Kuppuswamy Sathyanarayanan
  0 siblings, 1 reply; 40+ messages in thread
From: James Bottomley @ 2024-02-03  6:03 UTC (permalink / raw)
  To: Dan Middleton, Samuel Ortiz, Dan Williams
  Cc: Kuppuswamy Sathyanarayanan, Qinkun Bao, Yao, Jiewen, Xing,
	Cedric, Dionna Amalie Glaze, biao.lu, linux-coco,
	linux-integrity, linux-kernel

On Fri, 2024-02-02 at 17:07 -0600, Dan Middleton wrote:
> 
> On 2/2/24 12:24 AM, James Bottomley wrote:
> > On Sun, 2024-01-28 at 22:25 +0100, Samuel Ortiz wrote:
> > > All architectures supporting RTMRs expose a similar interface to
> > > their TVMs: An extension command/call that takes a measurement
> > > value and an RTMR index to extend it with, and a readback command
> > > for reading an RTMR value back (taking an RTMR index as an
> > > argument as well). This patch series builds an architecture
> > > agnostic, configfs-based ABI for userspace to extend and read
> > > RTMR values back. It extends the current TSM ops structure and
> > > each confidential computing architecture can implement this
> > > extension to provide RTMR support.
> > What's the actual use case for this?  At the moment the TPM PCRs
> > only provide a read interface to userspace (via
> > /sys/class/tpm/tpmX/pcr-shaY/Z) and don't have any extension
> > ability becuase nothing in userspace currently extends them.
> > 
> > The only current runtime use for TPM PCRs is IMA, which is in-
> > kernel (and which this patch doesn't enable).
> > 
> > Without the ability to log, this interface is unusable anyway, but
> > even with that it's not clear that you need the ability separately
> > to extend PCRs because the extension and log entry should be done
> > atomically to prevent the log going out of sync with the PCRs, so
> > it would seem a log first interface would be the correct way of
> > doing this rather than a PCR first one.
> > 
> > James
> > 
> > 
> 
> While we clearly need to cover PCR-like usages, I think Confidential
> Computing affords usages that go beyond TPM.

Well, don't get me wrong, I think the ability to create non repudiable
log entries from userspace is very useful.  However, I think the
proposed ABI is wrong: it should take the log entry (which will contain
the PCR number and the hash) then do the extension and add it to the
log so we get the non-repudiable verifiability.  This should work
equally with TPM and RTMR (and anything else).

The issue, I suppose, is what log format?  The TCG has one which is
extensible and IMA uses a similar but different binary log format.

> For example, Attested Containers [1] (and similar explorations in
> CNCF Confidential Containers [2]) extends the measurement chain into
> the guest. There, a trusted agent measures container images, and
> extends an RTMR with those measurements. Particularly in the case of
> containers, the  existing runtime infrastructure is user mode
> oriented. However the generalization here is in providing a mechanism
> to strongly identify an application or behavior provided by the TVM.

There's a similar proposal for Keylime which was demo'd at Plumbers
last year, except it uses IMA to measure the container so you only have
to trust the kernel:

https://lpc.events/event/17/contributions/1571/

> Less concretely, I think this is an area for developer creativity.
> Attestation is one of the main APIs that CC gives application
> developers and
> these runtime extendable fields provide a further degree of
> creativity.
> 
> [1] ACON https://github.com/intel/acon

Just on this, lest we repeat the errors of the past (and believe me
there was a time people thought that simply extending TPM PCRs without
log entries was the way to do measurements), if you're extending a PCR
like entity you also need a log entry to tell people who come after you
what you've done.  Even in the one ephemeral VM per pod kata use case
(with RTMRs local to the VM), you'll still likely be starting several
sidecars and if you don't have a log to tell you the order you measured
the containers deriving the RTMR value is a combinatoric explosion.

James


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI
  2024-02-03  6:03     ` James Bottomley
@ 2024-02-03  7:13       ` Kuppuswamy Sathyanarayanan
  2024-02-03 10:27         ` James Bottomley
  0 siblings, 1 reply; 40+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2024-02-03  7:13 UTC (permalink / raw)
  To: James Bottomley, Dan Middleton, Samuel Ortiz, Dan Williams
  Cc: Qinkun Bao, Yao, Jiewen, Xing, Cedric, Dionna Amalie Glaze,
	biao.lu, linux-coco, linux-integrity, linux-kernel


On 2/2/24 10:03 PM, James Bottomley wrote:
> On Fri, 2024-02-02 at 17:07 -0600, Dan Middleton wrote:
>> On 2/2/24 12:24 AM, James Bottomley wrote:
>>> On Sun, 2024-01-28 at 22:25 +0100, Samuel Ortiz wrote:
>>>> All architectures supporting RTMRs expose a similar interface to
>>>> their TVMs: An extension command/call that takes a measurement
>>>> value and an RTMR index to extend it with, and a readback command
>>>> for reading an RTMR value back (taking an RTMR index as an
>>>> argument as well). This patch series builds an architecture
>>>> agnostic, configfs-based ABI for userspace to extend and read
>>>> RTMR values back. It extends the current TSM ops structure and
>>>> each confidential computing architecture can implement this
>>>> extension to provide RTMR support.
>>> What's the actual use case for this?  At the moment the TPM PCRs
>>> only provide a read interface to userspace (via
>>> /sys/class/tpm/tpmX/pcr-shaY/Z) and don't have any extension
>>> ability becuase nothing in userspace currently extends them.
>>>
>>> The only current runtime use for TPM PCRs is IMA, which is in-
>>> kernel (and which this patch doesn't enable).
>>>
>>> Without the ability to log, this interface is unusable anyway, but
>>> even with that it's not clear that you need the ability separately
>>> to extend PCRs because the extension and log entry should be done
>>> atomically to prevent the log going out of sync with the PCRs, so
>>> it would seem a log first interface would be the correct way of
>>> doing this rather than a PCR first one.
>>>
>>> James
>>>
>>>
>> While we clearly need to cover PCR-like usages, I think Confidential
>> Computing affords usages that go beyond TPM.
> Well, don't get me wrong, I think the ability to create non repudiable
> log entries from userspace is very useful.  However, I think the
> proposed ABI is wrong: it should take the log entry (which will contain
> the PCR number and the hash) then do the extension and add it to the
> log so we get the non-repudiable verifiability.  This should work
> equally with TPM and RTMR (and anything else).

Maybe I misunderstood your comments, but I am not sure why
the user ABI needs to change? I agree that logging after extension is
the right approach. But, IMO, it should be owned by the back end
TSM vendor drivers. The user ABI should just pass the digest and
RTMR index.

>
> The issue, I suppose, is what log format?  The TCG has one which is
> extensible and IMA uses a similar but different binary log format.

TDX uses EFI_TCG2_EVENT_LOG_FORMAT_TCG_2 log format. I think SEV is the
same.

https://uefi.org/specs/UEFI/2.10/38_Confidential_Computing.html#virtual-platform-cc-event-log

>
>> For example, Attested Containers [1] (and similar explorations in
>> CNCF Confidential Containers [2]) extends the measurement chain into
>> the guest. There, a trusted agent measures container images, and
>> extends an RTMR with those measurements. Particularly in the case of
>> containers, the  existing runtime infrastructure is user mode
>> oriented. However the generalization here is in providing a mechanism
>> to strongly identify an application or behavior provided by the TVM.
> There's a similar proposal for Keylime which was demo'd at Plumbers
> last year, except it uses IMA to measure the container so you only have
> to trust the kernel:
>
> https://lpc.events/event/17/contributions/1571/
>
>> Less concretely, I think this is an area for developer creativity.
>> Attestation is one of the main APIs that CC gives application
>> developers and
>> these runtime extendable fields provide a further degree of
>> creativity.
>>
>> [1] ACON https://github.com/intel/acon
> Just on this, lest we repeat the errors of the past (and believe me
> there was a time people thought that simply extending TPM PCRs without
> log entries was the way to do measurements), if you're extending a PCR
> like entity you also need a log entry to tell people who come after you
> what you've done.  Even in the one ephemeral VM per pod kata use case
> (with RTMRs local to the VM), you'll still likely be starting several
> sidecars and if you don't have a log to tell you the order you measured
> the containers deriving the RTMR value is a combinatoric explosion.
>
> James
>
>
-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI
  2024-02-03  7:13       ` Kuppuswamy Sathyanarayanan
@ 2024-02-03 10:27         ` James Bottomley
  2024-02-06  8:34           ` Xing, Cedric
  0 siblings, 1 reply; 40+ messages in thread
From: James Bottomley @ 2024-02-03 10:27 UTC (permalink / raw)
  To: Kuppuswamy Sathyanarayanan, Dan Middleton, Samuel Ortiz, Dan Williams
  Cc: Qinkun Bao, Yao, Jiewen, Xing, Cedric, Dionna Amalie Glaze,
	biao.lu, linux-coco, linux-integrity, linux-kernel

On Fri, 2024-02-02 at 23:13 -0800, Kuppuswamy Sathyanarayanan wrote:
> 
> On 2/2/24 10:03 PM, James Bottomley wrote:
> > On Fri, 2024-02-02 at 17:07 -0600, Dan Middleton wrote:
> > > On 2/2/24 12:24 AM, James Bottomley wrote:
> > > > On Sun, 2024-01-28 at 22:25 +0100, Samuel Ortiz wrote:
> > > > > All architectures supporting RTMRs expose a similar interface
> > > > > to
> > > > > their TVMs: An extension command/call that takes a
> > > > > measurement
> > > > > value and an RTMR index to extend it with, and a readback
> > > > > command
> > > > > for reading an RTMR value back (taking an RTMR index as an
> > > > > argument as well). This patch series builds an architecture
> > > > > agnostic, configfs-based ABI for userspace to extend and read
> > > > > RTMR values back. It extends the current TSM ops structure
> > > > > and
> > > > > each confidential computing architecture can implement this
> > > > > extension to provide RTMR support.
> > > > What's the actual use case for this?  At the moment the TPM
> > > > PCRs
> > > > only provide a read interface to userspace (via
> > > > /sys/class/tpm/tpmX/pcr-shaY/Z) and don't have any extension
> > > > ability becuase nothing in userspace currently extends them.
> > > > 
> > > > The only current runtime use for TPM PCRs is IMA, which is in-
> > > > kernel (and which this patch doesn't enable).
> > > > 
> > > > Without the ability to log, this interface is unusable anyway,
> > > > but
> > > > even with that it's not clear that you need the ability
> > > > separately
> > > > to extend PCRs because the extension and log entry should be
> > > > done
> > > > atomically to prevent the log going out of sync with the PCRs,
> > > > so
> > > > it would seem a log first interface would be the correct way of
> > > > doing this rather than a PCR first one.
> > > > 
> > > > James
> > > > 
> > > > 
> > > While we clearly need to cover PCR-like usages, I think
> > > Confidential
> > > Computing affords usages that go beyond TPM.
> > Well, don't get me wrong, I think the ability to create non
> > repudiable
> > log entries from userspace is very useful.  However, I think the
> > proposed ABI is wrong: it should take the log entry (which will
> > contain
> > the PCR number and the hash) then do the extension and add it to
> > the
> > log so we get the non-repudiable verifiability.  This should work
> > equally with TPM and RTMR (and anything else).
> 
> Maybe I misunderstood your comments, but I am not sure why
> the user ABI needs to change?

Well, there is no ABI currently, so I'm saying get it right before
there is one.

>  I agree that logging after extension is the right approach. But,
> IMO, it should be owned by the back end TSM vendor drivers. The user
> ABI should just pass the digest and RTMR index.

Well, lets wind back to the assumptions about the log.  The current
convention from IMA and Measured Boot is that the log is managed by the
kernel.  Given the potential problems with timing and serialization
(which can cause log mismatches) it would make sense for this ABI also
to have a kernel backed log (probably a new one from the other two). 
If you have a kernel backed log, the ABI for extending it should be
where you get the PCR extensions from, that way nothing can go wrong. 
An API to extend the PCRs separately will only cause pain for people
who get it wrong (and lead to ordering issues if more than one thing
wants to add to the log, which they will do because neither the TPM nor
the RTMRs have enough registers to do one per process that wants to use
it if this becomes popular).

James


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI
  2024-02-03 10:27         ` James Bottomley
@ 2024-02-06  8:34           ` Xing, Cedric
  2024-02-06  8:57             ` James Bottomley
  0 siblings, 1 reply; 40+ messages in thread
From: Xing, Cedric @ 2024-02-06  8:34 UTC (permalink / raw)
  To: James Bottomley, Kuppuswamy Sathyanarayanan, Dan Middleton,
	Samuel Ortiz, Dan Williams
  Cc: Qinkun Bao, Yao, Jiewen, Dionna Amalie Glaze, biao.lu,
	linux-coco, linux-integrity, linux-kernel

On 2/3/2024 2:27 AM, James Bottomley wrote:
> On Fri, 2024-02-02 at 23:13 -0800, Kuppuswamy Sathyanarayanan wrote:
>>
>> On 2/2/24 10:03 PM, James Bottomley wrote:
>>> On Fri, 2024-02-02 at 17:07 -0600, Dan Middleton wrote:
>>>> On 2/2/24 12:24 AM, James Bottomley wrote:
>>>>> On Sun, 2024-01-28 at 22:25 +0100, Samuel Ortiz wrote:
>>>>>> All architectures supporting RTMRs expose a similar interface
>>>>>> to
>>>>>> their TVMs: An extension command/call that takes a
>>>>>> measurement
>>>>>> value and an RTMR index to extend it with, and a readback
>>>>>> command
>>>>>> for reading an RTMR value back (taking an RTMR index as an
>>>>>> argument as well). This patch series builds an architecture
>>>>>> agnostic, configfs-based ABI for userspace to extend and read
>>>>>> RTMR values back. It extends the current TSM ops structure
>>>>>> and
>>>>>> each confidential computing architecture can implement this
>>>>>> extension to provide RTMR support.
>>>>> What's the actual use case for this?  At the moment the TPM
>>>>> PCRs
>>>>> only provide a read interface to userspace (via
>>>>> /sys/class/tpm/tpmX/pcr-shaY/Z) and don't have any extension
>>>>> ability becuase nothing in userspace currently extends them.
>>>>>
>>>>> The only current runtime use for TPM PCRs is IMA, which is in-
>>>>> kernel (and which this patch doesn't enable).
>>>>>
>>>>> Without the ability to log, this interface is unusable anyway,
>>>>> but
>>>>> even with that it's not clear that you need the ability
>>>>> separately
>>>>> to extend PCRs because the extension and log entry should be
>>>>> done
>>>>> atomically to prevent the log going out of sync with the PCRs,
>>>>> so
>>>>> it would seem a log first interface would be the correct way of
>>>>> doing this rather than a PCR first one.
>>>>>
>>>>> James
>>>>>
>>>>>
>>>> While we clearly need to cover PCR-like usages, I think
>>>> Confidential
>>>> Computing affords usages that go beyond TPM.
>>> Well, don't get me wrong, I think the ability to create non
>>> repudiable
>>> log entries from userspace is very useful.  However, I think the
>>> proposed ABI is wrong: it should take the log entry (which will
>>> contain
>>> the PCR number and the hash) then do the extension and add it to
>>> the
>>> log so we get the non-repudiable verifiability.  This should work
>>> equally with TPM and RTMR (and anything else).
>>
>> Maybe I misunderstood your comments, but I am not sure why
>> the user ABI needs to change?
> 
> Well, there is no ABI currently, so I'm saying get it right before
> there is one.
> 
>>   I agree that logging after extension is the right approach. But,
>> IMO, it should be owned by the back end TSM vendor drivers. The user
>> ABI should just pass the digest and RTMR index.
> 
> Well, lets wind back to the assumptions about the log.  The current
> convention from IMA and Measured Boot is that the log is managed by the
> kernel.  Given the potential problems with timing and serialization
> (which can cause log mismatches) it would make sense for this ABI also
> to have a kernel backed log (probably a new one from the other two).

I'm not familiar with existing TPM code. Per 
https://elixir.free-electrons.com/linux/latest/source/drivers/char/tpm/tpm-interface.c#L314, 
tpm_pcr_extend() doesn't seem to take/log the actual event, but only 
extends the PCR. IMA seems to maintain the measurement list/log by 
itself. Am I right? If so, why do we want logging to be part of TSM here?

For measured boots, I think UEFI BIOS has already maintained a log so 
what's needed here is just to expose the log somewhere in sysfs. IMHO, I 
don't think logging is even necessary because everything in the boot 
flow is static, hence a relying party can simply compare measurement 
registers against known good values without looking at any log. But 
please correct me if I have missed anything.

> If you have a kernel backed log, the ABI for extending it should be
> where you get the PCR extensions from, that way nothing can go wrong.
> An API to extend the PCRs separately will only cause pain for people
> who get it wrong (and lead to ordering issues if more than one thing
> wants to add to the log, which they will do because neither the TPM nor
> the RTMRs have enough registers to do one per process that wants to use
> it if this becomes popular).
> 
There's an easy way to solve the synchronization problem in user mode by 
applying flock() on the log file - i.e., a process can extend a 
measurement register only when holding an exclusive lock on the 
corresponding log file. A possible drawback is it'd allow a malicious 
process to starve all other processes by holding the lock forever, or to 
mess up the log file content intentionally. But that shouldn't be a 
practical problem because the existence of such malicious processes 
would have rendered the CVM untrustworthy anyway - i.e., should the CVM 
still be able to generate a valid attestation, that would only lead to a 
distrust decision by any sane relying party.

IMHO, if something can be easily solved in user mode, probably it 
shouldn't be solved in kernel mode.

> James
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI
  2024-02-06  8:34           ` Xing, Cedric
@ 2024-02-06  8:57             ` James Bottomley
  2024-02-07  2:02               ` Dan Williams
  0 siblings, 1 reply; 40+ messages in thread
From: James Bottomley @ 2024-02-06  8:57 UTC (permalink / raw)
  To: Xing, Cedric, Kuppuswamy Sathyanarayanan, Dan Middleton,
	Samuel Ortiz, Dan Williams
  Cc: Qinkun Bao, Yao, Jiewen, Dionna Amalie Glaze, biao.lu,
	linux-coco, linux-integrity, linux-kernel

On Tue, 2024-02-06 at 00:34 -0800, Xing, Cedric wrote:
[...]
> I'm not familiar with existing TPM code. Per 
> https://elixir.free-electrons.com/linux/latest/source/drivers/char/tpm/tpm-interface.c#L314
> ,
> tpm_pcr_extend() doesn't seem to take/log the actual event, but only 
> extends the PCR.

That's the low level code we build on.  The TPM doesn't maintain a log
at all, just the measuring entity.

>  IMA seems to maintain the measurement list/log by itself.

It does, yes.

>  Am I right? If so, why do we want logging to be part of TSM
> here?

Well, as I said above: without a log you have a combinatoric explosion
of events that lead to the PCR value.

> For measured boots, I think UEFI BIOS has already maintained a log so
> what's needed here is just to expose the log somewhere in sysfs.
> IMHO, I don't think logging is even necessary because everything in
> the boot flow is static, hence a relying party can simply compare
> measurement registers against known good values without looking at
> any log. But please correct me if I have missed anything.

Without the log the UEFI boot flow is way too brittle because
measurements aren't actually static and without knowing what happened
you can't reproduce the PCR value.  It was actually the earliest
insight from the keylime project that it couldn't just define state by
PCR values and had to parse the log instead.

> > If you have a kernel backed log, the ABI for extending it should be
> > where you get the PCR extensions from, that way nothing can go
> > wrong. An API to extend the PCRs separately will only cause pain
> > for people who get it wrong (and lead to ordering issues if more
> > than one thing wants to add to the log, which they will do because
> > neither the TPM nor the RTMRs have enough registers to do one per
> > process that wants to use it if this becomes popular).
> > 
> There's an easy way to solve the synchronization problem in user mode
> by applying flock() on the log file - i.e., a process can extend a 
> measurement register only when holding an exclusive lock on the 
> corresponding log file.

Which would be where exactly? and owned by whom?

>  A possible drawback is it'd allow a malicious
> process to starve all other processes by holding the lock forever, or
> to mess up the log file content intentionally. But that shouldn't be
> a practical problem because the existence of such malicious processes
> would have rendered the CVM untrustworthy anyway - i.e., should the
> CVM still be able to generate a valid attestation, that would only
> lead to a distrust decision by any sane relying party.
> 
> IMHO, if something can be easily solved in user mode, probably it 
> shouldn't be solved in kernel mode.

There isn't really anything more complex about an interface that takes
a log entry, and does the record an extend, than an interface which
takes a PCR extension value.  So best practice would say that you
should create the ABI that you can't get wrong (log and record) rather
than creating one that causes additional problems for userspace.

James


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI
  2024-02-06  8:57             ` James Bottomley
@ 2024-02-07  2:02               ` Dan Williams
  2024-02-07 20:16                 ` Xing, Cedric
  0 siblings, 1 reply; 40+ messages in thread
From: Dan Williams @ 2024-02-07  2:02 UTC (permalink / raw)
  To: James Bottomley, Xing, Cedric, Kuppuswamy Sathyanarayanan,
	Dan Middleton, Samuel Ortiz, Dan Williams
  Cc: Qinkun Bao, Yao, Jiewen, Dionna Amalie Glaze, biao.lu,
	linux-coco, linux-integrity, linux-kernel

James Bottomley wrote:
> There isn't really anything more complex about an interface that takes
> a log entry, and does the record an extend, than an interface which
> takes a PCR extension value.  So best practice would say that you
> should create the ABI that you can't get wrong (log and record) rather
> than creating one that causes additional problems for userspace.

Agree, there's no need for the kernel to leave deliberately pointy edges
for userspace to trip over.

Cedric, almost every time we, kernel community, build an interface where
userspace says "trust us, we know what we are doing" it inevitably
results later in "whoops, turns out it would have helped if the kernel
enforced structure here". So the log ABI adds that structure for the
primary use cases.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI
  2024-02-07  2:02               ` Dan Williams
@ 2024-02-07 20:16                 ` Xing, Cedric
  2024-02-07 21:08                   ` Kuppuswamy Sathyanarayanan
  0 siblings, 1 reply; 40+ messages in thread
From: Xing, Cedric @ 2024-02-07 20:16 UTC (permalink / raw)
  To: Dan Williams, James Bottomley, Kuppuswamy Sathyanarayanan,
	Dan Middleton, Samuel Ortiz
  Cc: Qinkun Bao, Yao, Jiewen, Dionna Amalie Glaze, biao.lu,
	linux-coco, linux-integrity, linux-kernel

On 2/6/2024 6:02 PM, Dan Williams wrote:
> James Bottomley wrote:
>> There isn't really anything more complex about an interface that takes
>> a log entry, and does the record an extend, than an interface which
>> takes a PCR extension value.  So best practice would say that you
>> should create the ABI that you can't get wrong (log and record) rather
>> than creating one that causes additional problems for userspace.
> 
> Agree, there's no need for the kernel to leave deliberately pointy edges
> for userspace to trip over.
> 
> Cedric, almost every time we, kernel community, build an interface where
> userspace says "trust us, we know what we are doing" it inevitably
> results later in "whoops, turns out it would have helped if the kernel
> enforced structure here". So the log ABI adds that structure for the
> primary use cases.

Dan, I agree with your statement generally. But with the precedent of 
TPM module not maintaining a log, I just wonder if the addition of log 
would cause problems or force more changes to existing usages than 
necessary. For example, IMA has its own log and if changed to use RTMR, 
how would those 2 logs interoperate? We would also need to decide on a 
log format that can accommodate all applications.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI
  2024-02-07 20:16                 ` Xing, Cedric
@ 2024-02-07 21:08                   ` Kuppuswamy Sathyanarayanan
  2024-02-07 21:46                     ` James Bottomley
  0 siblings, 1 reply; 40+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2024-02-07 21:08 UTC (permalink / raw)
  To: Xing, Cedric, Dan Williams, James Bottomley, Dan Middleton, Samuel Ortiz
  Cc: Qinkun Bao, Yao, Jiewen, Dionna Amalie Glaze, biao.lu,
	linux-coco, linux-integrity, linux-kernel


On 2/7/24 12:16 PM, Xing, Cedric wrote:
> On 2/6/2024 6:02 PM, Dan Williams wrote:
>> James Bottomley wrote:
>>> There isn't really anything more complex about an interface that takes
>>> a log entry, and does the record an extend, than an interface which
>>> takes a PCR extension value.  So best practice would say that you
>>> should create the ABI that you can't get wrong (log and record) rather
>>> than creating one that causes additional problems for userspace.
>>
>> Agree, there's no need for the kernel to leave deliberately pointy edges
>> for userspace to trip over.
>>
>> Cedric, almost every time we, kernel community, build an interface where
>> userspace says "trust us, we know what we are doing" it inevitably
>> results later in "whoops, turns out it would have helped if the kernel
>> enforced structure here". So the log ABI adds that structure for the
>> primary use cases.
>
> Dan, I agree with your statement generally. But with the precedent of TPM module not maintaining a log, I just wonder if the addition of log would cause problems or force more changes to existing usages than necessary. For example, IMA has its own log and if changed to use RTMR, how would those 2 logs interoperate? We would also need to decide on a log format that can accommodate all applications.


IIUC, CC event logging in firmware uses TCG2 format. Since IMA internally uses TPM calls, I assume it also uses the TCG2 format. I think we can follow the same format for RTMR extension.

I am wondering where will the event log be stored? Is it in the log_area region of CCEL table?

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI
  2024-02-07 21:08                   ` Kuppuswamy Sathyanarayanan
@ 2024-02-07 21:46                     ` James Bottomley
  2024-02-09 20:58                       ` Dan Williams
  2024-02-22 15:45                       ` Lukas Wunner
  0 siblings, 2 replies; 40+ messages in thread
From: James Bottomley @ 2024-02-07 21:46 UTC (permalink / raw)
  To: Kuppuswamy Sathyanarayanan, Xing, Cedric, Dan Williams,
	Dan Middleton, Samuel Ortiz
  Cc: Qinkun Bao, Yao, Jiewen, Dionna Amalie Glaze, biao.lu,
	linux-coco, linux-integrity, linux-kernel

On Wed, 2024-02-07 at 13:08 -0800, Kuppuswamy Sathyanarayanan wrote:
> 
> On 2/7/24 12:16 PM, Xing, Cedric wrote:
> > On 2/6/2024 6:02 PM, Dan Williams wrote:
> > > James Bottomley wrote:
> > > > There isn't really anything more complex about an interface
> > > > that takes a log entry, and does the record an extend, than an
> > > > interface which takes a PCR extension value.  So best practice
> > > > would say that you should create the ABI that you can't get
> > > > wrong (log and record) rather than creating one that causes
> > > > additional problems for userspace.
> > > 
> > > Agree, there's no need for the kernel to leave deliberately
> > > pointy edges for userspace to trip over.
> > > 
> > > Cedric, almost every time we, kernel community, build an
> > > interface where userspace says "trust us, we know what we are
> > > doing" it inevitably results later in "whoops, turns out it would
> > > have helped if the kernel enforced structure here". So the log
> > > ABI adds that structure for the primary use cases.
> > 
> > Dan, I agree with your statement generally. But with the precedent
> > of TPM module not maintaining a log, I just wonder if the addition
> > of log would cause problems or force more changes to existing
> > usages than necessary. For example, IMA has its own log and if
> > changed to use RTMR, how would those 2 logs interoperate? We would
> > also need to decide on a log format that can accommodate all
> > applications.
> 
> 
> IIUC, CC event logging in firmware uses TCG2 format. Since IMA
> internally uses TPM calls, I assume it also uses the TCG2 format. I
> think we can follow the same format for RTMR extension.

Just to correct this: IMA uses its own log format, but I think this was
a mistake long ago and the new log should use TCG2 format so all the
tools know how to parse it.

> I am wondering where will the event log be stored? Is it in the
> log_area region of CCEL table?

IMA stores its log in kernel memory and makes it visible in securityfs
(in the smae place as the measured boot log).  Since this interface is
using configfs, that's where I'd make the log visible.

Just to add a note about how UEFI works: the measured boot log is
effectively copied into kernel memory because the UEFI memory it once
occupied is freed after exit boot services, so no UEFI interface will
suffice for the log location.

I'd make the file exporting it root owned but probably readable by only
the people who can also extend it (presumably enforced by group?). 

James


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI
  2024-02-07 21:46                     ` James Bottomley
@ 2024-02-09 20:58                       ` Dan Williams
  2024-02-13  7:36                         ` Xing, Cedric
  2024-02-22 15:45                       ` Lukas Wunner
  1 sibling, 1 reply; 40+ messages in thread
From: Dan Williams @ 2024-02-09 20:58 UTC (permalink / raw)
  To: James Bottomley, Kuppuswamy Sathyanarayanan, Xing, Cedric,
	Dan Williams, Dan Middleton, Samuel Ortiz
  Cc: Qinkun Bao, Yao, Jiewen, Dionna Amalie Glaze, biao.lu,
	linux-coco, linux-integrity, linux-kernel

James Bottomley wrote:
> On Wed, 2024-02-07 at 13:08 -0800, Kuppuswamy Sathyanarayanan wrote:
> > 
> > On 2/7/24 12:16 PM, Xing, Cedric wrote:
> > > On 2/6/2024 6:02 PM, Dan Williams wrote:
> > > > James Bottomley wrote:
> > > > > There isn't really anything more complex about an interface
> > > > > that takes a log entry, and does the record an extend, than an
> > > > > interface which takes a PCR extension value.  So best practice
> > > > > would say that you should create the ABI that you can't get
> > > > > wrong (log and record) rather than creating one that causes
> > > > > additional problems for userspace.
> > > > 
> > > > Agree, there's no need for the kernel to leave deliberately
> > > > pointy edges for userspace to trip over.
> > > > 
> > > > Cedric, almost every time we, kernel community, build an
> > > > interface where userspace says "trust us, we know what we are
> > > > doing" it inevitably results later in "whoops, turns out it would
> > > > have helped if the kernel enforced structure here". So the log
> > > > ABI adds that structure for the primary use cases.
> > > 
> > > Dan, I agree with your statement generally. But with the precedent
> > > of TPM module not maintaining a log, I just wonder if the addition
> > > of log would cause problems or force more changes to existing
> > > usages than necessary. For example, IMA has its own log and if
> > > changed to use RTMR, how would those 2 logs interoperate? We would
> > > also need to decide on a log format that can accommodate all
> > > applications.
> > 
> > 
> > IIUC, CC event logging in firmware uses TCG2 format. Since IMA
> > internally uses TPM calls, I assume it also uses the TCG2 format. I
> > think we can follow the same format for RTMR extension.
> 
> Just to correct this: IMA uses its own log format, but I think this was
> a mistake long ago and the new log should use TCG2 format so all the
> tools know how to parse it.

Is this a chance to nudge IMA towards a standard log format? In other
words, one of the goals alongside userspace consumers of the RTMR log
would be for IMA to support it as well as an alternate in-kernel backend
next to TPM. IMA-over-TPM continues with its current format,
IMA-over-RTMR internally unifies with the log format that is shared with
RTMR-user-ABI.

...but be warned the above is a comment from someone who knows nothing
about IMA internals, just reacting to the comment.


> > I am wondering where will the event log be stored? Is it in the
> > log_area region of CCEL table?
> 
> IMA stores its log in kernel memory and makes it visible in securityfs
> (in the smae place as the measured boot log).  Since this interface is
> using configfs, that's where I'd make the log visible.
> 
> Just to add a note about how UEFI works: the measured boot log is
> effectively copied into kernel memory because the UEFI memory it once
> occupied is freed after exit boot services, so no UEFI interface will
> suffice for the log location.
> 
> I'd make the file exporting it root owned but probably readable by only
> the people who can also extend it (presumably enforced by group?). 

I assume EFI copying into kernel memory is ok because that log has a
limited number of entries. If this RTMR log gets large I assume it needs
some way cull entries that have been moved to storage. Maybe this is a
problem IMA has already solved.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI
  2024-02-09 20:58                       ` Dan Williams
@ 2024-02-13  7:36                         ` Xing, Cedric
  2024-02-13 16:05                           ` James Bottomley
                                             ` (2 more replies)
  0 siblings, 3 replies; 40+ messages in thread
From: Xing, Cedric @ 2024-02-13  7:36 UTC (permalink / raw)
  To: Dan Williams, James Bottomley, Kuppuswamy Sathyanarayanan,
	Dan Middleton, Samuel Ortiz
  Cc: Qinkun Bao, Yao, Jiewen, Dionna Amalie Glaze, biao.lu,
	linux-coco, linux-integrity, linux-kernel

On 2/9/2024 12:58 PM, Dan Williams wrote:
> James Bottomley wrote:
>> Just to correct this: IMA uses its own log format, but I think this was
>> a mistake long ago and the new log should use TCG2 format so all the
>> tools know how to parse it.
> 
> Is this a chance to nudge IMA towards a standard log format? In other
> words, one of the goals alongside userspace consumers of the RTMR log
> would be for IMA to support it as well as an alternate in-kernel backend
> next to TPM. IMA-over-TPM continues with its current format,
> IMA-over-RTMR internally unifies with the log format that is shared with
> RTMR-user-ABI.
> 
I'm not a TCG expert. As far as I know, 
https://trustedcomputinggroup.org/wp-content/uploads/TCG-PC-Client-Platform-Firmware-Profile-Version-1.06-Revision-52_pub-1.pdf 
defines the event types for TCG2 logs for firmware uses only. I cannot 
find a spec that defines event types for OS or applications. We may 
reuse the firmware event types for Linux but I doubt they can 
accommodate IMA.

IMHO, we don't have to follow TCG2 format because TDX is never TPM, nor 
are any other TEEs that support runtime measurements. The existing TCG2 
format looks to me somewhat like ASN.1 - well defined but schema is 
needed to decode. In contrast, JSON is a lot more popular than ASN.1 
nowadays because it's human readable and doesn't require a schema. I 
just wonder if we should introduce a text based log format. We could 
make the log a text file, in which each line is an event record and the 
digest of the line is extended to the specified runtime measurement 
register. The content of each line could be free-form at the ABI level, 
but we can still recommend a convention for applications - e.g., the 
first word/column must be an URL for readers to find out the 
format/syntax of the rest of the line. Thoughts?

> ...but be warned the above is a comment from someone who knows nothing
> about IMA internals, just reacting to the comment.
> 
> 
>>> I am wondering where will the event log be stored? Is it in the
>>> log_area region of CCEL table?
>>
>> IMA stores its log in kernel memory and makes it visible in securityfs
>> (in the smae place as the measured boot log).  Since this interface is
>> using configfs, that's where I'd make the log visible.
>>
>> Just to add a note about how UEFI works: the measured boot log is
>> effectively copied into kernel memory because the UEFI memory it once
>> occupied is freed after exit boot services, so no UEFI interface will
>> suffice for the log location.
>>
>> I'd make the file exporting it root owned but probably readable by only
>> the people who can also extend it (presumably enforced by group?).
> 
> I assume EFI copying into kernel memory is ok because that log has a
> limited number of entries. If this RTMR log gets large I assume it needs
> some way cull entries that have been moved to storage. Maybe this is a
> problem IMA has already solved.

We don't have to, and are also not supposed to I guess, append to the 
log generated by BIOS. The kernel can start a new log, and potentially 
in a different format. I think the BIOS log is exposed via securityfs 
today. Am I correct? For the new TEE measurement log, I don't think it 
has to be collocated with the BIOS log, because TEEs are never TPMs.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI
  2024-02-13  7:36                         ` Xing, Cedric
@ 2024-02-13 16:05                           ` James Bottomley
  2024-02-14  8:54                             ` Xing, Cedric
  2024-03-05  1:19                             ` Xing, Cedric
  2024-02-13 16:54                           ` Mikko Ylinen
  2024-02-15 22:44                           ` Dr. Greg
  2 siblings, 2 replies; 40+ messages in thread
From: James Bottomley @ 2024-02-13 16:05 UTC (permalink / raw)
  To: Xing, Cedric, Dan Williams, Kuppuswamy Sathyanarayanan,
	Dan Middleton, Samuel Ortiz
  Cc: Qinkun Bao, Yao, Jiewen, Dionna Amalie Glaze, biao.lu,
	linux-coco, linux-integrity, linux-kernel

On Mon, 2024-02-12 at 23:36 -0800, Xing, Cedric wrote:
> On 2/9/2024 12:58 PM, Dan Williams wrote:
> > James Bottomley wrote:
> > > Just to correct this: IMA uses its own log format, but I think
> > > this was a mistake long ago and the new log should use TCG2
> > > format so all the tools know how to parse it.
> > 
> > Is this a chance to nudge IMA towards a standard log format? In
> > other words, one of the goals alongside userspace consumers of the
> > RTMR log would be for IMA to support it as well as an alternate in-
> > kernel backend next to TPM. IMA-over-TPM continues with its current
> > format, IMA-over-RTMR internally unifies with the log format that
> > is shared with RTMR-user-ABI.
> > 
> I'm not a TCG expert. As far as I know, 
> https://trustedcomputinggroup.org/wp-content/uploads/TCG-PC-Client-Platform-Firmware-Profile-Version-1.06-Revision-52_pub-1.pdf
>  
> defines the event types for TCG2 logs for firmware uses only. I
> cannot  find a spec that defines event types for OS or applications.
> We may  reuse the firmware event types for Linux but I doubt they can
> accommodate IMA.

The TCG crypto agile log format is

 index (32 bit),
 event tag (32 bit),
 digests array,
 sized event entry (up to 4GB)

So an IMA log entry can definitely be transformed into this format
(providing someone agrees to the tag or set of tags).  The slight
problem would be that none of the current IMA tools would understand
it, but that could be solved over time (the kernel could use the TCG
format internally but transform to the IMA format for the current
securityfs IMA log).

> IMHO, we don't have to follow TCG2 format because TDX is never TPM,
> nor are any other TEEs that support runtime measurements. The
> existing TCG2 format looks to me somewhat like ASN.1 - well defined
> but schema is needed to decode. In contrast, JSON is a lot more
> popular than ASN.1  nowadays because it's human readable and doesn't
> require a schema. I just wonder if we should introduce a text based
> log format. We could make the log a text file, in which each line is
> an event record and the digest of the line is extended to the
> specified runtime measurement register. The content of each line
> could be free-form at the ABI level, but we can still recommend a
> convention for applications - e.g., the first word/column must be an
> URL for readers to find out the format/syntax of the rest of the
> line. Thoughts?

https://xkcd.com/927/

> > ...but be warned the above is a comment from someone who knows
> > nothing about IMA internals, just reacting to the comment.
> > 
> > 
> > > > I am wondering where will the event log be stored? Is it in the
> > > > log_area region of CCEL table?
> > > 
> > > IMA stores its log in kernel memory and makes it visible in
> > > securityfs (in the smae place as the measured boot log).  Since
> > > this interface is using configfs, that's where I'd make the log
> > > visible.
> > > 
> > > Just to add a note about how UEFI works: the measured boot log is
> > > effectively copied into kernel memory because the UEFI memory it
> > > once occupied is freed after exit boot services, so no UEFI
> > > interface will suffice for the log location.
> > > 
> > > I'd make the file exporting it root owned but probably readable
> > > by only the people who can also extend it (presumably enforced by
> > > group?).
> > 
> > I assume EFI copying into kernel memory is ok because that log has
> > a limited number of entries. If this RTMR log gets large I assume
> > it needs some way cull entries that have been moved to storage.
> > Maybe this is a problem IMA has already solved.
> 
> We don't have to, and are also not supposed to I guess, append to the
> log generated by BIOS.

We do actually: the EFI boot stub in the kernel appends entries for the
initrd and command line.

>  The kernel can start a new log, and potentially in a different
> format. I think the BIOS log is exposed via securityfs today. Am I
> correct?

I already said that, yes.

>  For the new TEE measurement log, I don't think it has to be
> collocated with the BIOS log, because TEEs are never TPMs.

This depends.  Logs are separable by PCRs.  As in every entry for the
same PCR could be in a separate, correctly ordered, log.  However, you
can't have separate logs that both use the same PCR because they won't
replay.

James




^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI
  2024-02-13  7:36                         ` Xing, Cedric
  2024-02-13 16:05                           ` James Bottomley
@ 2024-02-13 16:54                           ` Mikko Ylinen
  2024-02-15 22:44                           ` Dr. Greg
  2 siblings, 0 replies; 40+ messages in thread
From: Mikko Ylinen @ 2024-02-13 16:54 UTC (permalink / raw)
  To: Xing, Cedric
  Cc: Dan Williams, James Bottomley, Kuppuswamy Sathyanarayanan,
	Dan Middleton, Samuel Ortiz, Qinkun Bao, Yao, Jiewen,
	Dionna Amalie Glaze, biao.lu, linux-coco, linux-integrity,
	linux-kernel

On Mon, Feb 12, 2024 at 11:36:27PM -0800, Xing, Cedric wrote:
> On 2/9/2024 12:58 PM, Dan Williams wrote:
> > James Bottomley wrote:
> > > Just to correct this: IMA uses its own log format, but I think this was
> > > a mistake long ago and the new log should use TCG2 format so all the
> > > tools know how to parse it.
> > 
> > Is this a chance to nudge IMA towards a standard log format? In other
> > words, one of the goals alongside userspace consumers of the RTMR log
> > would be for IMA to support it as well as an alternate in-kernel backend
> > next to TPM. IMA-over-TPM continues with its current format,
> > IMA-over-RTMR internally unifies with the log format that is shared with
> > RTMR-user-ABI.
> > 
> I'm not a TCG expert. As far as I know, https://trustedcomputinggroup.org/wp-content/uploads/TCG-PC-Client-Platform-Firmware-Profile-Version-1.06-Revision-52_pub-1.pdf
> defines the event types for TCG2 logs for firmware uses only. I cannot find
> a spec that defines event types for OS or applications. We may reuse the
> firmware event types for Linux but I doubt they can accommodate IMA.
> 
> IMHO, we don't have to follow TCG2 format because TDX is never TPM, nor are
> any other TEEs that support runtime measurements. The existing TCG2 format
> looks to me somewhat like ASN.1 - well defined but schema is needed to
> decode. In contrast, JSON is a lot more popular than ASN.1 nowadays because
> it's human readable and doesn't require a schema. I just wonder if we should
> introduce a text based log format. We could make the log a text file, in
> which each line is an event record and the digest of the line is extended to
> the specified runtime measurement register. The content of each line could
> be free-form at the ABI level, but we can still recommend a convention for
> applications - e.g., the first word/column must be an URL for readers to
> find out the format/syntax of the rest of the line. Thoughts?

There's also the 'Canonical Event Log' format from TCG. It
covers IMA but it looks it's PC/client specific otherwise.

systemd seems to be following this format for its systemd-pcr*
services and exposing the log in JSON format under /run/log [1].

[1] https://www.freedesktop.org/software/systemd/man/latest/systemd-pcrphase.service.html

> 
> > ...but be warned the above is a comment from someone who knows nothing
> > about IMA internals, just reacting to the comment.
> > 
> > 
> > > > I am wondering where will the event log be stored? Is it in the
> > > > log_area region of CCEL table?
> > > 
> > > IMA stores its log in kernel memory and makes it visible in securityfs
> > > (in the smae place as the measured boot log).  Since this interface is
> > > using configfs, that's where I'd make the log visible.
> > > 
> > > Just to add a note about how UEFI works: the measured boot log is
> > > effectively copied into kernel memory because the UEFI memory it once
> > > occupied is freed after exit boot services, so no UEFI interface will
> > > suffice for the log location.
> > > 
> > > I'd make the file exporting it root owned but probably readable by only
> > > the people who can also extend it (presumably enforced by group?).
> > 
> > I assume EFI copying into kernel memory is ok because that log has a
> > limited number of entries. If this RTMR log gets large I assume it needs
> > some way cull entries that have been moved to storage. Maybe this is a
> > problem IMA has already solved.
> 
> We don't have to, and are also not supposed to I guess, append to the log
> generated by BIOS. The kernel can start a new log, and potentially in a
> different format. I think the BIOS log is exposed via securityfs today. Am I
> correct? For the new TEE measurement log, I don't think it has to be
> collocated with the BIOS log, because TEEs are never TPMs.

-- 
Regards, Mikko

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI
  2024-02-13 16:05                           ` James Bottomley
@ 2024-02-14  8:54                             ` Xing, Cedric
  2024-02-15  6:14                               ` Dan Williams
  2024-03-05  1:19                             ` Xing, Cedric
  1 sibling, 1 reply; 40+ messages in thread
From: Xing, Cedric @ 2024-02-14  8:54 UTC (permalink / raw)
  To: James Bottomley, Dan Williams, Kuppuswamy Sathyanarayanan,
	Dan Middleton, Samuel Ortiz
  Cc: Qinkun Bao, Yao, Jiewen, Dionna Amalie Glaze, biao.lu,
	linux-coco, linux-integrity, linux-kernel

On 2/13/2024 8:05 AM, James Bottomley wrote:
> On Mon, 2024-02-12 at 23:36 -0800, Xing, Cedric wrote:
>> On 2/9/2024 12:58 PM, Dan Williams wrote:
>>> James Bottomley wrote:
>>>> Just to correct this: IMA uses its own log format, but I think
>>>> this was a mistake long ago and the new log should use TCG2
>>>> format so all the tools know how to parse it.
>>>
>>> Is this a chance to nudge IMA towards a standard log format? In
>>> other words, one of the goals alongside userspace consumers of the
>>> RTMR log would be for IMA to support it as well as an alternate in-
>>> kernel backend next to TPM. IMA-over-TPM continues with its current
>>> format, IMA-over-RTMR internally unifies with the log format that
>>> is shared with RTMR-user-ABI.
>>>
>> I'm not a TCG expert. As far as I know,
>> https://trustedcomputinggroup.org/wp-content/uploads/TCG-PC-Client-Platform-Firmware-Profile-Version-1.06-Revision-52_pub-1.pdf
>>   
>> defines the event types for TCG2 logs for firmware uses only. I
>> cannot  find a spec that defines event types for OS or applications.
>> We may  reuse the firmware event types for Linux but I doubt they can
>> accommodate IMA.
> 
> The TCG crypto agile log format is
> 
>   index (32 bit),
>   event tag (32 bit),
>   digests array,
>   sized event entry (up to 4GB)
> 
> So an IMA log entry can definitely be transformed into this format
> (providing someone agrees to the tag or set of tags).  The slight
> problem would be that none of the current IMA tools would understand
> it, but that could be solved over time (the kernel could use the TCG
> format internally but transform to the IMA format for the current
> securityfs IMA log).
> 
Hi James,

As Mikko mentioned in his reply, TCG has defined the "Canonical Event 
Log Format" (aka. CEL) [1], while systemd-pcr* services use a subset of 
CEL format in their user space log.

I skimmed through the CEL spec today. Comparing to TCG2 log, CEL follows 
the same design (i.e., each event has a type field that determines the 
structure of the event data) but separates the encoding from the 
information model.

IMHO, CEL only works for applications defined in its information model 
(currently UEFI BIOS and IMA) but wouldn't work for any other 
applications like systemd. The systemd source code has documented the 
difference [2] between their log format and CEL.

One problem of CEL is its "content_type", which contains numeric values 
assigned by the spec. systemd doesn't have a numeric "content_type" 
assigned so has to use a string value - "systemd", which can only be 
encoded in JSON but not in TLV or CBOR. Technically, the systemd log is 
NOT CEL even though they claim that's a subset of it.

Another problem of CEL is that NOT every byte of an event is 
hashed/extended. CEL spec has defined for each "content_type" the subset 
of bytes to hash, so a verifier must understand ALL content types to be 
able to verify the integrity of a log. In other words, the integrity of 
a "systemd" log can never be verified by a CEL conformant verifier.

So I wouldn't recommend CEL to be the log format here.

We are looking for, as I believe, is a format that can accommodate all 
applications and allow application-agnostic verifiers. For every event, 
the kernel only needs to know what to store in the log, and what to 
hash/extend and to which measurement registers, but isn't concerned by 
the semantics of the event. If reusing CEL terms, what needs to be 
defined here is the "encoding" (so that every application can 
store/extend "something" in a log that every verifier knows how to 
replay); while every application should be allowed to define its own 
"information model".

-Cedric

[1] https://trustedcomputinggroup.org/resource/canonical-event-log-format/

[2] 
https://github.com/systemd/systemd/blob/e1390da0256bbe2017c4c2fbc636c54fe02c84cb/src/shared/tpm2-util.c#L6112

>> IMHO, we don't have to follow TCG2 format because TDX is never TPM,
>> nor are any other TEEs that support runtime measurements. The
>> existing TCG2 format looks to me somewhat like ASN.1 - well defined
>> but schema is needed to decode. In contrast, JSON is a lot more
>> popular than ASN.1  nowadays because it's human readable and doesn't
>> require a schema. I just wonder if we should introduce a text based
>> log format. We could make the log a text file, in which each line is
>> an event record and the digest of the line is extended to the
>> specified runtime measurement register. The content of each line
>> could be free-form at the ABI level, but we can still recommend a
>> convention for applications - e.g., the first word/column must be an
>> URL for readers to find out the format/syntax of the rest of the
>> line. Thoughts?
> 
> https://xkcd.com/927/
> 
That is funny :-D

I can't agree more, so "no log" I think is always an option.

>>   For the new TEE measurement log, I don't think it has to be
>> collocated with the BIOS log, because TEEs are never TPMs.
> 
> This depends.  Logs are separable by PCRs.  As in every entry for the
> same PCR could be in a separate, correctly ordered, log.  However, you
> can't have separate logs that both use the same PCR because they won't
> replay.
> 
We can have separate logs for the same PCR as long as there's a way to 
order those logs. A simple way is to record the current PCR value at the 
beginning of every log, then a verifier can always replay the log to get 
the PCR value at exit, and use that value to match the next log.

Anyway, those details are unimportant. What I intended to say was that 
those logs don't have to be in the same format.

> James
> 
> 
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI
  2024-02-14  8:54                             ` Xing, Cedric
@ 2024-02-15  6:14                               ` Dan Williams
  2024-02-16  2:05                                 ` Xing, Cedric
  0 siblings, 1 reply; 40+ messages in thread
From: Dan Williams @ 2024-02-15  6:14 UTC (permalink / raw)
  To: Xing, Cedric, James Bottomley, Dan Williams,
	Kuppuswamy Sathyanarayanan, Dan Middleton, Samuel Ortiz
  Cc: Qinkun Bao, Yao, Jiewen, Dionna Amalie Glaze, biao.lu,
	linux-coco, linux-integrity, linux-kernel

Xing, Cedric wrote:
> On 2/13/2024 8:05 AM, James Bottomley wrote:
[..]
> > The TCG crypto agile log format is
> > 
> >   index (32 bit),
> >   event tag (32 bit),
> >   digests array,
> >   sized event entry (up to 4GB)
> > 
> > So an IMA log entry can definitely be transformed into this format
> > (providing someone agrees to the tag or set of tags).  The slight
> > problem would be that none of the current IMA tools would understand
> > it, but that could be solved over time (the kernel could use the TCG
> > format internally but transform to the IMA format for the current
> > securityfs IMA log).
> > 
> Hi James,
> 
[..]
> Another problem of CEL is that NOT every byte of an event is 
> hashed/extended. CEL spec has defined for each "content_type" the subset 
> of bytes to hash, so a verifier must understand ALL content types to be 
> able to verify the integrity of a log. In other words, the integrity of 
> a "systemd" log can never be verified by a CEL conformant verifier.

Wait, James said, "crypto agile log format", not CEL. Crypto agile log
format looks more generic, no "recnum" for example.

[..]
> >> IMHO, we don't have to follow TCG2 format..
[..]
> > https://xkcd.com/927/
> > 
> That is funny :-D
> 
> I can't agree more, so "no log" I think is always an option.

So to me, "no log" means that instead of going from 14 standards going
to 15, the kernel is saying "whee, infinite userspace log formats!", an
abdication of its role to support a stable application ABI.

The job here to define a kernel de-facto standard for the tags that this
configs implementation of a cryto agile log emits, right? As James says:

"(providing someone agrees to the tag or set of tags)"

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI
  2024-02-13  7:36                         ` Xing, Cedric
  2024-02-13 16:05                           ` James Bottomley
  2024-02-13 16:54                           ` Mikko Ylinen
@ 2024-02-15 22:44                           ` Dr. Greg
  2 siblings, 0 replies; 40+ messages in thread
From: Dr. Greg @ 2024-02-15 22:44 UTC (permalink / raw)
  To: Xing, Cedric
  Cc: Dan Williams, James Bottomley, Kuppuswamy Sathyanarayanan,
	Dan Middleton, Samuel Ortiz, Qinkun Bao, Yao, Jiewen,
	Dionna Amalie Glaze, biao.lu, linux-coco, linux-integrity,
	linux-kernel

On Mon, Feb 12, 2024 at 11:36:27PM -0800, Xing, Cedric wrote:

Hi, I hope the week is going well for everyone.

> On 2/9/2024 12:58 PM, Dan Williams wrote:
> >James Bottomley wrote:
> >>Just to correct this: IMA uses its own log format, but I think this was
> >>a mistake long ago and the new log should use TCG2 format so all the
> >>tools know how to parse it.
> >
> >Is this a chance to nudge IMA towards a standard log format? In other
> >words, one of the goals alongside userspace consumers of the RTMR log
> >would be for IMA to support it as well as an alternate in-kernel backend
> >next to TPM. IMA-over-TPM continues with its current format,
> >IMA-over-RTMR internally unifies with the log format that is shared with
> >RTMR-user-ABI.

> I'm not a TCG expert. As far as I know,
> https://trustedcomputinggroup.org/wp-content/uploads/TCG-PC-Client-Platform-Firmware-Profile-Version-1.06-Revision-52_pub-1.pdf
> defines the event types for TCG2 logs for firmware uses only. I
> cannot find a spec that defines event types for OS or
> applications. We may reuse the firmware event types for Linux but I
> doubt they can accommodate IMA.
>
> IMHO, we don't have to follow TCG2 format because TDX is never TPM,
> nor are any other TEEs that support runtime measurements. The
> existing TCG2 format looks to me somewhat like ASN.1 - well defined
> but schema is needed to decode. In contrast, JSON is a lot more
> popular than ASN.1 nowadays because it's human readable and doesn't
> require a schema. I just wonder if we should introduce a text based
> log format. We could make the log a text file, in which each line is
> an event record and the digest of the line is extended to the
> specified runtime measurement register. The content of each line
> could be free-form at the ABI level, but we can still recommend a
> convention for applications - e.g., the first word/column must be an
> URL for readers to find out the format/syntax of the rest of the
> line. Thoughts?

A common text based security event description format, based on JSON
encoding of LSM security event descriptions, surfaced through
securityfs, has already been implemented, proposed and has been pushed
out for review twice.

The TSEM LSM is designed to be a generic security modeling and
security event description export architecture.

The V2 patches and discusion around those can be found here:

https://lore.kernel.org/lkml/20230710102319.19716-1-greg@enjellic.com/T/#t

We have a rather significant upgrade to that patchset that we are
staging up for a V3 release.

The fundamental premise for TSEM is the encoding and modeling of the
parameters that describe LSM based security events.  It is designed to
be a model/policy agnostic scheme for generating attestations on the
state of the platform at large or a workload.  Workload models are
supported by a concept known as a security modeling namespace, much
like any other namespace, that tracks events for an isolated process
heirarchy.

The most important review comment on the V1 patchset, that can also be
found on lore, was by Greg Kroah-Hartmann who suggested using a
standardized encoding scheme like JSON for the event descriptions.  If
you look at his comments, he indicated that there is little rationale
for not using an encoding format that the entire technology industry
trusts and uses.

FWIW, we made the change to JSON and have never looked back, it was
the most positive review comment we received.

The current format would not seem to have any issues supporting IMA
style attestation.  For example, the most important event for IMA
would be a file open event.  Here is an example of the encoding
generated for that event:

{
    "event": {
        "process": "quixote",
        "type": "file_open",
        "ttd": "219",
        "p_ttd": "219",
        "task_id": "20e07b3614ee37869391849278dfe7285f37ec2362f7d10c052e6715ad888584",
        "p_task_id": "20e07b3614ee37869391849278dfe7285f37ec2362f7d10c052e6715ad888584",
        "ts": "6535350020298"
    },
    "COE": {
        "uid": "0",
        "euid": "0",
        "suid": "0",
        "gid": "0",
        "egid": "0",
        "sgid": "0",
        "fsuid": "0",
        "fsgid": "0",
        "capeff": "0x3ffffffffff"
    },
    "file_open": {
        "file": {
            "flags": "32800",
            "inode": {
                "uid": "50",
                "gid": "50",
                "mode": "0100755",
                "s_magic": "0xef53",
                "s_id": "xvda",
                "s_uuid": "feadbeaffeadbeaffeadbeaffeadbeaf"
            },
            "path": {
                "dev": {
                    "major": "202",
                    "minor": "0"
                },
                "pathname": "/opt/Quixote/sbin/runc"
            },
            "digest": "81f73a59be3d122ab484d7dfe9ddc81030f595cc59968f61c113a9a38a2c113a"
        }
    }
}

There is sufficient information included to track the digests of
executable files, or any other type of file for that matter, for any
user on the system.

This isn't an attempt to pitch TSEM, but rather to suggest the utility
of a self-describing JSON format for security logging.

As GKH correctly noted in his review comments, there is a great deal
of utility to be had by using a format that has significant and mature
userspace tooling support.  Our own work and deployments have also
indicated a great deal of utility to having log entries that are
self-describing.

One additional observation that may be of use with respect to anyone
pursueing an alternate event log format has come out of our data
science team.  They indicate there has been significant work in the
Elastic search community with respect to the development of
standardized descriptions of events for logging and other purposes,
reference the following URL:

https://www.elastic.co/guide/en/ecs/current/index.html

Our data team is looking at modifying our current security event
descriptions to be as consistent as possible with existing standards
for identifying event parameters.  Given that attestation and host
based security event modeling are only going to become more important
in the future, there would seem to be utility in working towards
contributing to standardized descriptions for security relevant logs.

Hopefully the above reflections are of assistance in furthering the
various agendas that are involved.

Have a good remainder of the week.

As always,
Dr. Greg

The Quixote Project - Flailing at the Travails of Cybersecurity
              https://github.com/Quixote-Project

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI
  2024-02-15  6:14                               ` Dan Williams
@ 2024-02-16  2:05                                 ` Xing, Cedric
  0 siblings, 0 replies; 40+ messages in thread
From: Xing, Cedric @ 2024-02-16  2:05 UTC (permalink / raw)
  To: Dan Williams, James Bottomley, Kuppuswamy Sathyanarayanan,
	Dan Middleton, Samuel Ortiz
  Cc: Qinkun Bao, Yao, Jiewen, Dionna Amalie Glaze, biao.lu,
	linux-coco, linux-integrity, linux-kernel

On 2/14/2024 10:14 PM, Dan Williams wrote:
> Xing, Cedric wrote:
>> On 2/13/2024 8:05 AM, James Bottomley wrote:
> [..]
>>> The TCG crypto agile log format is
>>>
>>>    index (32 bit),
>>>    event tag (32 bit),
>>>    digests array,
>>>    sized event entry (up to 4GB)
>>>
>>> So an IMA log entry can definitely be transformed into this format
>>> (providing someone agrees to the tag or set of tags).  The slight
>>> problem would be that none of the current IMA tools would understand
>>> it, but that could be solved over time (the kernel could use the TCG
>>> format internally but transform to the IMA format for the current
>>> securityfs IMA log).
>>>
>> Hi James,
>>
> [..]
>> Another problem of CEL is that NOT every byte of an event is
>> hashed/extended. CEL spec has defined for each "content_type" the subset
>> of bytes to hash, so a verifier must understand ALL content types to be
>> able to verify the integrity of a log. In other words, the integrity of
>> a "systemd" log can never be verified by a CEL conformant verifier.
> 
> Wait, James said, "crypto agile log format", not CEL. Crypto agile log
> format looks more generic, no "recnum" for example.
> 
If I'm not mistaken, "crypto agile log" refers to the same format as 
"TCG2 log". It's "crypto agile" because it allows a plurality of hash 
algorithms/digests (specified in the "digests" array) to be extended to 
one PCR - each algorithm supported is called a "bank" of the PCR.

CEL is also "crypto agile". I'm not familiar with its history but it 
seems emerged after the TCG2 log format, as CEL's information model is a 
superset of TCG2's. Specifically, CEL's information model covers 3 
applications - "CEL management" (owned by TCG/CEL), "PC Client STD" 
(owned by TCG PC Client WG and equivalent to TCG2 log), and IMA. 
Supporting any new applications would require expanding CEL's 
information model - i.e., by changing the CEL spec.

> [..]
>>>> IMHO, we don't have to follow TCG2 format..
> [..]
>>> https://xkcd.com/927/
>>>
>> That is funny :-D
>>
>> I can't agree more, so "no log" I think is always an option.
> 
> So to me, "no log" means that instead of going from 14 standards going
> to 15, the kernel is saying "whee, infinite userspace log formats!", an
> abdication of its role to support a stable application ABI.
> 
If we look at how CEL is defined, it separates information model from 
encoding. Information models have to be contextualized within specific 
applications, but encodings don't. The reason for 14 standards is 
because there are 14 different applications. The 15th may be able to 
combine the existing 14 into a single one, but probably cannot 
accommodate the 16th.

So I think the only practical approach is to abandon the information 
model and focus on the encoding only. For example, JSON is just a set of 
encoding rules without an information model, hence can serialize data 
for all applications.

Coming back to the TSM log, the real question is: Can we just specify 
the encoding without an information model? The answer is yes and no. The 
kernel does need to know something, such as what to log, what to hash, 
and extend to which MR, but does NOT need to understand anything else 
about the event. So a potential ABI definition could be:
   - Take the MR index and an *encoded* log entry as parameters from 
user mode.
   - Hash the whole entry as-is using the same algorithm as used in MR 
extension, and extend the resulted digest to the specified MR.
   - Append the whole entry as-is to the log file.

The key difference between the aforementioned and CEL is that the former 
takes the *encoded* log entry as a single input to alleviate the kernel 
from the necessity of comprehending the logger's information model.

> The job here to define a kernel de-facto standard for the tags that this
> configs implementation of a cryto agile log emits, right? As James says:
> 
> "(providing someone agrees to the tag or set of tags)"

I don't think we should define any tags because that can only be done 
for existing applications but can never address the needs of future 
applications.

We don't have to maintain a log. The existing TPM module doesn't 
maintain a log either. systemd on the other hand is an example of 
keeping measurement logs in user mode.

But if we agree that a log is indeed necessary, I'd recommend the 
aforementioned approach. We can then focus discussions on the options 
for encoding log entries.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 2/4] tsm: Add RTMRs to the configfs-tsm hierarchy
  2024-01-28 21:25 ` [RFC PATCH v2 2/4] tsm: Add RTMRs to the configfs-tsm hierarchy Samuel Ortiz
  2024-01-28 22:38   ` Kuppuswamy Sathyanarayanan
  2024-02-01 22:05   ` Jarkko Sakkinen
@ 2024-02-21 16:16   ` Mikko Ylinen
  2 siblings, 0 replies; 40+ messages in thread
From: Mikko Ylinen @ 2024-02-21 16:16 UTC (permalink / raw)
  To: Samuel Ortiz
  Cc: Dan Williams, Kuppuswamy Sathyanarayanan, Qinkun Bao, Yao,
	Jiewen, Xing, Cedric, Dionna Amalie Glaze, biao.lu, linux-coco,
	linux-integrity, linux-kernel

Hi,

On Sun, Jan 28, 2024 at 10:25:21PM +0100, Samuel Ortiz wrote:
> RTMRs are defined and managed by their corresponding TSM provider. As
> such, they can be configured through the TSM configfs root.
> 
> An additional `rtmrs` directory is added by default under the `tsm` one,
> where each supported RTMR can be configured:
> 
> mkdir /sys/kernel/config/tsm/rtmrs/rtmr0
> echo 0 > /sys/kernel/config/tsm/rtmrs/rtmr0/index

I implemented the plumbing for TDX to experiment with this patchset a bit
and to try out some ideas for the event logging.

The first mkdir triggers the following, FYI:

[  353.984801] ======================================================
[  353.984805] WARNING: possible circular locking dependency detected
[  353.984808] 6.8.0-rc5+ #14 Not tainted
[  353.984812] ------------------------------------------------------
[  353.984814] mkdir/3374 is trying to acquire lock:
[  353.984817] ffffffff8ae59d90 (tsm_rwsem){++++}-{3:3}, at: tsm_rtmrs_make_item+0x26/0xa0
[  353.984830] 
               but task is already holding lock:
[  353.984832] ffffffff8ae59890 (tsm_configfs.su_mutex){+.+.}-{3:3}, at: configfs_mkdir+0x188/0x470
[  353.984842] 
               which lock already depends on the new lock.

[  353.984845] 
               the existing dependency chain (in reverse order) is:
[  353.984848] 
               -> #1 (tsm_configfs.su_mutex){+.+.}-{3:3}:
[  353.984853]        __lock_acquire+0x4d1/0xbb0
[  353.984861]        lock_acquire+0xcb/0x2b0
[  353.984863]        __mutex_lock+0x9b/0xb80
[  353.984874]        mutex_lock_nested+0x1f/0x30
[  353.984878]        configfs_register_group+0x7b/0x1d0
[  353.984880]        configfs_register_default_group+0x54/0x90
[  353.984883]        tsm_rtmr_register+0xb9/0x140
[  353.984886]        tsm_register+0x89/0xc0
[  353.984888]        tdx_guest_init+0x81/0x110
[  353.984896]        do_one_initcall+0x62/0x370
[  353.984903]        do_initcalls+0xe3/0x1a0
[  353.984909]        kernel_init_freeable+0x2ea/0x400
[  353.984912]        kernel_init+0x1e/0x1c0
[  353.984915]        ret_from_fork+0x3e/0x60
[  353.984921]        ret_from_fork_asm+0x11/0x20
[  353.984925] 
               -> #0 (tsm_rwsem){++++}-{3:3}:
[  353.984929]        check_prev_add+0xed/0xc60
[  353.984931]        validate_chain+0x488/0x530
[  353.984933]        __lock_acquire+0x4d1/0xbb0
[  353.984936]        lock_acquire+0xcb/0x2b0
[  353.984938]        down_read+0x45/0x190
[  353.984941]        tsm_rtmrs_make_item+0x26/0xa0
[  353.984944]        configfs_mkdir+0x349/0x470
[  353.984946]        vfs_mkdir+0x1a5/0x260
[  353.984955]        do_mkdirat+0x83/0x140
[  353.984958]        __x64_sys_mkdir+0x4e/0x70
[  353.984960]        do_syscall_64+0x67/0x110
[  353.984964]        entry_SYSCALL_64_after_hwframe+0x63/0x6b
[  353.984972] 
               other info that might help us debug this:

[  353.984975]  Possible unsafe locking scenario:

[  353.984977]        CPU0                    CPU1
[  353.984980]        ----                    ----
[  353.984982]   lock(tsm_configfs.su_mutex);
[  353.984985]                                lock(tsm_rwsem);
[  353.984988]                                lock(tsm_configfs.su_mutex);
[  353.984991]   rlock(tsm_rwsem);
[  353.984993] 
                *** DEADLOCK ***

[  353.984996] 3 locks held by mkdir/3374:
[  353.984998]  #0: ff3af65b4ae05408 (sb_writers#13){.+.+}-{0:0}, at: filename_create+0x61/0x190
[  353.985006]  #1: ff3af65b492a97f8 (&sb->s_type->i_mutex_key#6/1){+.+.}-{3:3}, at: filename_create+0x9d/
0x190
[  353.985014]  #2: ffffffff8ae59890 (tsm_configfs.su_mutex){+.+.}-{3:3}, at: configfs_mkdir+0x188/0x470
[  353.985020]
               stack backtrace:
[  353.985023] CPU: 36 PID: 3374 Comm: mkdir Not tainted 6.8.0-rc5+ #14
[  353.985027] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 2023.05-2+tdx1.0 11/05/2023
[  353.985031] Call Trace:
[  353.985033]  <TASK>
[  353.985034]  dump_stack_lvl+0x4e/0x90
[  353.985041]  dump_stack+0x14/0x20
[  353.985044]  print_circular_bug+0xec/0x110
[  353.985047]  check_noncircular+0x130/0x150
[  353.985051]  check_prev_add+0xed/0xc60
[  353.985053]  ? add_chain_cache+0x10e/0x2d0
[  353.985059]  validate_chain+0x488/0x530
[  353.985062]  __lock_acquire+0x4d1/0xbb0
[  353.985065]  lock_acquire+0xcb/0x2b0
[  353.985067]  ? tsm_rtmrs_make_item+0x26/0xa0
[  353.985071]  down_read+0x45/0x190
[  353.985074]  ? tsm_rtmrs_make_item+0x26/0xa0
[  353.985094]  tsm_rtmrs_make_item+0x26/0xa0
[  353.985097]  configfs_mkdir+0x349/0x470
[  353.985100]  vfs_mkdir+0x1a5/0x260
[  353.985105]  do_mkdirat+0x83/0x140
[  353.985109]  __x64_sys_mkdir+0x4e/0x70
[  353.985112]  do_syscall_64+0x67/0x110
[  353.985116]  entry_SYSCALL_64_after_hwframe+0x63/0x6b
[  353.985119] RIP: 0033:0x7f35aad19d4b
[  353.985124] Code: 0f 1e fa 48 89 f2 b9 00 01 00 00 48 89 fe bf 9c ff ff ff e9 d7 c6 ff ff 0f 1f 80 00 0
0 00 00 f3 0f 1e fa b8 53 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 05 c3 0f 1f 40 00 48 8b 15 99 40 0e 00 f7
d8
[  353.985132] RSP: 002b:00007ffc3f2ccc78 EFLAGS: 00000246 ORIG_RAX: 0000000000000053
[  353.985139] RAX: ffffffffffffffda RBX: 00007ffc3f2cd5a5 RCX: 00007f35aad19d4b
[  353.985143] RDX: 0000000000000000 RSI: 00000000000001ff RDI: 00007ffc3f2cd5a5
[  353.985147] RBP: 00000000000001ff R08: 00000000000001ff R09: 0000000000000000
[  353.985151] R10: 0000556f33617249 R11: 0000000000000246 R12: 0000000000000000
[  353.985155] R13: 00007ffc3f2cce18 R14: 0000000000000000 R15: 00007ffc3f2cd5a5
[  353.985161]  </TASK>

> 
> An RTMR can not be extended nor read before its configured by assigning
> it an index. It is the TSM backend responsibility and choice to map that
> index to a hardware RTMR.
> 
> Signed-off-by: Samuel Ortiz <sameo@rivosinc.com>
> ---
>  Documentation/ABI/testing/configfs-tsm |  11 ++
>  drivers/virt/coco/tsm.c                | 164 +++++++++++++++++++++++++
>  2 files changed, 175 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/configfs-tsm b/Documentation/ABI/testing/configfs-tsm
> index dd24202b5ba5..590e103a9bcd 100644
> --- a/Documentation/ABI/testing/configfs-tsm
> +++ b/Documentation/ABI/testing/configfs-tsm
> @@ -80,3 +80,14 @@ Contact:	linux-coco@lists.linux.dev
>  Description:
>  		(RO) Indicates the minimum permissible value that can be written
>  		to @privlevel.
> +
> +What:		/sys/kernel/config/tsm/rtmrs/$name/index
> +Date:		January, 2024
> +KernelVersion:	v6.8
> +Contact:	linux-coco@lists.linux.dev
> +Description:
> +		(RW) A Runtime Measurement Register (RTMR) hardware index.
> +                Once created under /sys/kernel/config/tsm/rtmrs/, an RTMR entry
> +                can be mapped to a hardware RTMR by writing into its index
> +                attribute. The TSM provider will then map the configfs entry to
> +                its corresponding hardware register.
> diff --git a/drivers/virt/coco/tsm.c b/drivers/virt/coco/tsm.c
> index 1a8c3c096120..bb9ed2d2accc 100644
> --- a/drivers/virt/coco/tsm.c
> +++ b/drivers/virt/coco/tsm.c
> @@ -419,6 +419,108 @@ static const struct config_item_type tsm_reports_type = {
>  	.ct_group_ops = &tsm_report_group_ops,
>  };
>  
> +static ssize_t tsm_rtmr_index_store(struct config_item *cfg,
> +				    const char *buf, size_t len)
> +{
> +	struct tsm_rtmr_state *rtmr_state = to_tsm_rtmr_state(cfg);
> +	const struct tsm_ops *ops;
> +	unsigned int val;
> +	int rc;
> +
> +	rc = kstrtouint(buf, 0, &val);
> +	if (rc)
> +		return rc;
> +
> +	guard(rwsem_write)(&tsm_rwsem);
> +
> +	/* Index can only be configured once */
> +	if (is_rtmr_configured(rtmr_state))
> +		return -EBUSY;
> +
> +	/* Check that index stays within the TSM provided capabilities */
> +	ops = provider.ops;
> +	if (!ops)
> +		return -ENOTTY;
> +
> +	if (val > ops->capabilities.num_rtmrs - 1)
> +		return -EINVAL;
> +
> +	/* Check that this index is available */
> +	if (tsm_rtmrs->rtmrs[val])
> +		return -EINVAL;
> +
> +	rtmr_state->index = val;
> +	rtmr_state->alg = ops->capabilities.rtmrs[val].hash_alg;
> +
> +	tsm_rtmrs->rtmrs[val] = rtmr_state;
> +
> +	return len;
> +}
> +
> +static ssize_t tsm_rtmr_index_show(struct config_item *cfg,
> +				   char *buf)
> +{
> +	struct tsm_rtmr_state *rtmr_state = to_tsm_rtmr_state(cfg);
> +
> +	guard(rwsem_read)(&tsm_rwsem);
> +
> +	/* An RTMR is not available if it has not been configured */
> +	if (!is_rtmr_configured(rtmr_state))
> +		return -ENXIO;
> +
> +	return sysfs_emit(buf, "%u\n", rtmr_state->index);
> +}
> +CONFIGFS_ATTR(tsm_rtmr_, index);
> +
> +static struct configfs_attribute *tsm_rtmr_attrs[] = {
> +	&tsm_rtmr_attr_index,
> +	NULL,
> +};
> +
> +static void tsm_rtmr_item_release(struct config_item *cfg)
> +{
> +	struct tsm_rtmr_state *state = to_tsm_rtmr_state(cfg);
> +
> +	kfree(state);
> +}
> +
> +static struct configfs_item_operations tsm_rtmr_item_ops = {
> +	.release = tsm_rtmr_item_release,
> +};
> +
> +const struct config_item_type tsm_rtmr_type = {
> +	.ct_owner = THIS_MODULE,
> +	.ct_attrs = tsm_rtmr_attrs,
> +	.ct_item_ops = &tsm_rtmr_item_ops,
> +};
> +
> +static struct config_item *tsm_rtmrs_make_item(struct config_group *group,
> +					       const char *name)
> +{
> +	struct tsm_rtmr_state *state;
> +
> +	guard(rwsem_read)(&tsm_rwsem);
> +	if (!(provider.ops && (provider.ops->capabilities.num_rtmrs > 0)))
> +		return ERR_PTR(-ENXIO);
> +
> +	state = kzalloc(sizeof(*state), GFP_KERNEL);
> +	if (!state)
> +		return ERR_PTR(-ENOMEM);
> +	state->index = U32_MAX;
> +
> +	config_item_init_type_name(&state->cfg, name, &tsm_rtmr_type);
> +	return &state->cfg;
> +}
> +
> +static struct configfs_group_operations tsm_rtmrs_group_ops = {
> +	.make_item = tsm_rtmrs_make_item,
> +};
> +
> +static const struct config_item_type tsm_rtmrs_type = {
> +	.ct_owner = THIS_MODULE,
> +	.ct_group_ops = &tsm_rtmrs_group_ops,
> +};
> +
>  static const struct config_item_type tsm_root_group_type = {
>  	.ct_owner = THIS_MODULE,
>  };
> @@ -433,10 +535,48 @@ static struct configfs_subsystem tsm_configfs = {
>  	.su_mutex = __MUTEX_INITIALIZER(tsm_configfs.su_mutex),
>  };
>  
> +static int tsm_rtmr_register(const struct tsm_ops *ops)
> +{
> +	struct config_group *rtmrs_group;
> +
> +	lockdep_assert_held_write(&tsm_rwsem);
> +
> +	if (!ops || !ops->capabilities.num_rtmrs)
> +		return 0;
> +
> +	if (ops->capabilities.num_rtmrs > TSM_MAX_RTMR)
> +		return -EINVAL;
> +
> +	tsm_rtmrs = kzalloc(sizeof(struct tsm_rtmrs_state), GFP_KERNEL);
> +	if (!tsm_rtmrs)
> +		return -ENOMEM;
> +
> +	tsm_rtmrs->rtmrs = kcalloc(ops->capabilities.num_rtmrs,
> +				   sizeof(struct tsm_rtmr_state *),
> +				   GFP_KERNEL);
> +	if (!tsm_rtmrs->rtmrs) {
> +		kfree(tsm_rtmrs);
> +		return -ENOMEM;
> +	}
> +
> +	rtmrs_group = configfs_register_default_group(&tsm_configfs.su_group, "rtmrs",
> +						      &tsm_rtmrs_type);
> +	if (IS_ERR(rtmrs_group)) {
> +		kfree(tsm_rtmrs->rtmrs);
> +		kfree(tsm_rtmrs);
> +		return PTR_ERR(rtmrs_group);
> +	}
> +
> +	tsm_rtmrs->group = rtmrs_group;
> +
> +	return 0;
> +}
> +
>  int tsm_register(const struct tsm_ops *ops, void *priv,
>  		 const struct config_item_type *type)
>  {
>  	const struct tsm_ops *conflict;
> +	int rc;
>  
>  	if (!type)
>  		type = &tsm_report_default_type;
> @@ -450,6 +590,10 @@ int tsm_register(const struct tsm_ops *ops, void *priv,
>  		return -EBUSY;
>  	}
>  
> +	rc = tsm_rtmr_register(ops);
> +	if (rc < 0)
> +		return rc;
> +
>  	provider.ops = ops;
>  	provider.data = priv;
>  	provider.type = type;
> @@ -457,11 +601,31 @@ int tsm_register(const struct tsm_ops *ops, void *priv,
>  }
>  EXPORT_SYMBOL_GPL(tsm_register);
>  
> +static int tsm_rtmr_unregister(const struct tsm_ops *ops)
> +{
> +	lockdep_assert_held_write(&tsm_rwsem);
> +
> +	if ((ops) && (ops->capabilities.num_rtmrs > 0)) {
> +		configfs_unregister_default_group(tsm_rtmrs->group);
> +		kfree(tsm_rtmrs->rtmrs);
> +		kfree(tsm_rtmrs);
> +	}
> +
> +	return 0;
> +}
> +
>  int tsm_unregister(const struct tsm_ops *ops)
>  {
> +	int rc;
> +
>  	guard(rwsem_write)(&tsm_rwsem);
>  	if (ops != provider.ops)
>  		return -EBUSY;
> +
> +	rc = tsm_rtmr_unregister(ops);
> +	if (rc < 0)
> +		return rc;
> +
>  	provider.ops = NULL;
>  	provider.data = NULL;
>  	provider.type = NULL;
> -- 
> 2.42.0
> 

-- Regards, Mikko

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI
  2024-02-07 21:46                     ` James Bottomley
  2024-02-09 20:58                       ` Dan Williams
@ 2024-02-22 15:45                       ` Lukas Wunner
  1 sibling, 0 replies; 40+ messages in thread
From: Lukas Wunner @ 2024-02-22 15:45 UTC (permalink / raw)
  To: James Bottomley
  Cc: Kuppuswamy Sathyanarayanan, Xing, Cedric, Dan Williams,
	Dan Middleton, Samuel Ortiz, Qinkun Bao, Yao, Jiewen,
	Dionna Amalie Glaze, biao.lu, linux-coco, linux-integrity,
	linux-kernel, Jonathan Cameron

Hi James,

On Wed, Feb 07, 2024 at 04:46:36PM -0500, James Bottomley wrote:
> Just to correct this: IMA uses its own log format, but I think this was
> a mistake long ago and the new log should use TCG2 format so all the
> tools know how to parse it.

At last year's Plumbers BoF on PCI device authentication & encryption,
you requested that the kernel exposes proof of SPDM signature validation
so that user space can verify after the fact that the kernel did
everything correctly.

Your above comment seems to indicate that you prefer TCG2 CEL as the
format to expose the information, however the format seems ill-suited
for the purpose:

Per TCG PFP v1.06r52 sec 3.3.7, an SPDM CHALLENGE event merely logs
the nonce used.  That's not sufficient to verify the signature:
The signature is computed over a hash of the concatenation of all
the messages exchanged with the device:

* GET_VERSION request + VERSION response
* GET_CAPABILITIES request + CAPABILITIES response
* NEGOTIATE_ALGORITHMS request + ALGORITHMS response
* GET_DIGESTS request + DIGESTS response
* GET_CERTIFICATE request + CERTIFICATE response (can be multiple)
* CHALLENGE request + CHALLENGE_AUTH response

The content of those SPDM messages is not necessarily static:
E.g. the SPDM requester (the kernel) presents all supported algorithms
and the SPDM responder (the device) selects one of them.

If only the nonce is saved in the log, the verifier in user space would
need to know exactly which algorithms were supported by the SPDM requester
at the time the request was sent (could since have changed through a
kernel update).  It also needs to know exactly which algorithm the
SPDM responder picked.

Armed with the knowledge which algorithm bits were set, the verifier
would have to reconstruct the messages that were exchanged between
SPDM requester and responder so that it can calculate the hash over
their concatenation and verify the signature.

The algorithm selection is but one example of bits that can vary between
different requesters/responders and between different points in time.
The SPDM protocol allows a great deal of flexibility/agility here.

The nonces sent by requester and responder are not the only bits that are
variable, is what I'm trying to say.  Storing the nonces in the log is
sufficient to prove their freshness, but it is not sufficient to prove
correct validation of the signature.

I'd have to store the full concatenation of all exchanged SPDM messages
in the log to facilitate that.  Personally I have no problem doing so,
but it won't be possible with the CEL format as currently specified by TCG.

So on the one hand I'd like to fulfil your Plumbers request to expose
proof of correct signature validation and on the other hand you're
requesting CEL format which is insufficient to fulfil the request.
I don't really know how to reconcile that.

I do see value in exposing the full concatenation of all exchanged
SPDM messages:  It would allow piping that into wireshark or tshark
to decode the messages into human-readable form, which might be useful
for debugging SPDM communication with the device.

In fact, an SPDM dissector for wireshark already exists, though it's
not up-to-date (last change 3 years ago) and probably needs a cleanup
before it can be upstreamed:  https://github.com/jyao1/wireshark-spdm/

I'm considering adding a custom sysfs interface which exposes the last,
say, 64 SPDM events of each device, comprising:

* type of event (CHALLENGE or GET_MEASUREMENTS)
* timestamp
* all exchanged messages
* hash of all exchanged messages
* hash algorithm used
* signature received from device
* certificate chain used for signature validation

The memory consumption for all that data would be significant and the
format wouldn't be TCG2 CEL, but it would fulfil your request to provide
proof of signature verification.

Thoughts?

Thanks,

Lukas

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI
  2024-02-13 16:05                           ` James Bottomley
  2024-02-14  8:54                             ` Xing, Cedric
@ 2024-03-05  1:19                             ` Xing, Cedric
  2024-04-17 20:23                               ` Dan Middleton
  1 sibling, 1 reply; 40+ messages in thread
From: Xing, Cedric @ 2024-03-05  1:19 UTC (permalink / raw)
  To: James Bottomley, Dan Williams, Kuppuswamy Sathyanarayanan,
	Dan Middleton, Samuel Ortiz
  Cc: Qinkun Bao, Yao, Jiewen, Dionna Amalie Glaze, biao.lu,
	linux-coco, linux-integrity, linux-kernel

Hi James,

In the past couple of weeks I've been thinking about what should be a 
good log format that can be conformant to existing standards and 
accommodate future applications at the same time. After discussing with 
folks from Alibaba and Intel internally, I created this issue - 
https://github.com/confidential-containers/guest-components/issues/495 
to document what I've found. Although it was written for CoCo, the 
design I believe is CEL (Canonical Event Log) conformant and generic 
enough to be adopted by the kernel. Hence, I revive this thread to 
solicit your opinion. Your valuable time and feedback will be highly 
appreciated!

Thanks!

-Cedric

On 2/13/2024 8:05 AM, James Bottomley wrote:
> On Mon, 2024-02-12 at 23:36 -0800, Xing, Cedric wrote:
>> On 2/9/2024 12:58 PM, Dan Williams wrote:
>>> James Bottomley wrote:
>>>> Just to correct this: IMA uses its own log format, but I think
>>>> this was a mistake long ago and the new log should use TCG2
>>>> format so all the tools know how to parse it.
>>>
>>> Is this a chance to nudge IMA towards a standard log format? In
>>> other words, one of the goals alongside userspace consumers of the
>>> RTMR log would be for IMA to support it as well as an alternate in-
>>> kernel backend next to TPM. IMA-over-TPM continues with its current
>>> format, IMA-over-RTMR internally unifies with the log format that
>>> is shared with RTMR-user-ABI.
>>>
>> I'm not a TCG expert. As far as I know,
>> https://trustedcomputinggroup.org/wp-content/uploads/TCG-PC-Client-Platform-Firmware-Profile-Version-1.06-Revision-52_pub-1.pdf
>>   
>> defines the event types for TCG2 logs for firmware uses only. I
>> cannot  find a spec that defines event types for OS or applications.
>> We may  reuse the firmware event types for Linux but I doubt they can
>> accommodate IMA.
> 
> The TCG crypto agile log format is
> 
>   index (32 bit),
>   event tag (32 bit),
>   digests array,
>   sized event entry (up to 4GB)
> 
> So an IMA log entry can definitely be transformed into this format
> (providing someone agrees to the tag or set of tags).  The slight
> problem would be that none of the current IMA tools would understand
> it, but that could be solved over time (the kernel could use the TCG
> format internally but transform to the IMA format for the current
> securityfs IMA log).
> 
>> IMHO, we don't have to follow TCG2 format because TDX is never TPM,
>> nor are any other TEEs that support runtime measurements. The
>> existing TCG2 format looks to me somewhat like ASN.1 - well defined
>> but schema is needed to decode. In contrast, JSON is a lot more
>> popular than ASN.1  nowadays because it's human readable and doesn't
>> require a schema. I just wonder if we should introduce a text based
>> log format. We could make the log a text file, in which each line is
>> an event record and the digest of the line is extended to the
>> specified runtime measurement register. The content of each line
>> could be free-form at the ABI level, but we can still recommend a
>> convention for applications - e.g., the first word/column must be an
>> URL for readers to find out the format/syntax of the rest of the
>> line. Thoughts?
> 
> https://xkcd.com/927/
> 
>>> ...but be warned the above is a comment from someone who knows
>>> nothing about IMA internals, just reacting to the comment.
>>>
>>>
>>>>> I am wondering where will the event log be stored? Is it in the
>>>>> log_area region of CCEL table?
>>>>
>>>> IMA stores its log in kernel memory and makes it visible in
>>>> securityfs (in the smae place as the measured boot log).  Since
>>>> this interface is using configfs, that's where I'd make the log
>>>> visible.
>>>>
>>>> Just to add a note about how UEFI works: the measured boot log is
>>>> effectively copied into kernel memory because the UEFI memory it
>>>> once occupied is freed after exit boot services, so no UEFI
>>>> interface will suffice for the log location.
>>>>
>>>> I'd make the file exporting it root owned but probably readable
>>>> by only the people who can also extend it (presumably enforced by
>>>> group?).
>>>
>>> I assume EFI copying into kernel memory is ok because that log has
>>> a limited number of entries. If this RTMR log gets large I assume
>>> it needs some way cull entries that have been moved to storage.
>>> Maybe this is a problem IMA has already solved.
>>
>> We don't have to, and are also not supposed to I guess, append to the
>> log generated by BIOS.
> 
> We do actually: the EFI boot stub in the kernel appends entries for the
> initrd and command line.
> 
>>   The kernel can start a new log, and potentially in a different
>> format. I think the BIOS log is exposed via securityfs today. Am I
>> correct?
> 
> I already said that, yes.
> 
>>   For the new TEE measurement log, I don't think it has to be
>> collocated with the BIOS log, because TEEs are never TPMs.
> 
> This depends.  Logs are separable by PCRs.  As in every entry for the
> same PCR could be in a separate, correctly ordered, log.  However, you
> can't have separate logs that both use the same PCR because they won't
> replay.
> 
> James
> 
> 
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI
  2024-03-05  1:19                             ` Xing, Cedric
@ 2024-04-17 20:23                               ` Dan Middleton
  0 siblings, 0 replies; 40+ messages in thread
From: Dan Middleton @ 2024-04-17 20:23 UTC (permalink / raw)
  To: Xing, Cedric, James Bottomley, Dan Williams,
	Kuppuswamy Sathyanarayanan, Samuel Ortiz
  Cc: Qinkun Bao, Yao, Jiewen, Dionna Amalie Glaze, biao.lu,
	linux-coco, linux-integrity, linux-kernel

On 3/4/24 7:19 PM, Xing, Cedric wrote:
> Hi James,
>
> In the past couple of weeks I've been thinking about what should be a 
> good log format that can be conformant to existing standards and 
> accommodate future applications at the same time. After discussing 
> with folks from Alibaba and Intel internally, I created this issue - 
> https://github.com/confidential-containers/guest-components/issues/495 
> to document what I've found. Although it was written for CoCo, the 
> design I believe is CEL (Canonical Event Log) conformant and generic 
> enough to be adopted by the kernel. Hence, I revive this thread to 
> solicit your opinion. Your valuable time and feedback will be highly 
> appreciated!
>
> Thanks!
>
> -Cedric
>

Hi,

Closing the loop on testing format options with CNCF CoCo as an adopter
community...
There was a robust discussion in the issue [1] posted ~1.5 months back 
on the
previous note on this thread.
It seems the conversation has tailed off with agreement that the NELR format
would work for that containers community.
I think that's a good signal for this approach to move forward.

[1] https://github.com/confidential-containers/guest-components/issues/495

Regards,
Dan


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 4/4] tsm: Allow for extending and reading configured RTMRs
  2024-01-28 21:25 ` [RFC PATCH v2 4/4] tsm: Allow for extending and reading configured RTMRs Samuel Ortiz
@ 2024-05-11  2:57   ` James Bottomley
  2024-05-13 10:16     ` Samuel Ortiz
  0 siblings, 1 reply; 40+ messages in thread
From: James Bottomley @ 2024-05-11  2:57 UTC (permalink / raw)
  To: Samuel Ortiz, Dan Williams
  Cc: Kuppuswamy Sathyanarayanan, Qinkun Bao, Yao, Jiewen, Xing,
	Cedric, Dionna Amalie Glaze, biao.lu, linux-coco,
	linux-integrity, linux-kernel

I'm not really sure where to hang this, since there's no posted agenda
or materials for the CCC meeting today.  I'm afraid I also don't have a
copy of the presentation to point people who weren't at the meeting to.
However, it struck me you missed a third option: use the ima log
format.  This has the advantage that we can define additional events
and have them published with a kernel patch (the IMA log format is
defined in the kernel).  Thanks to the TCG, it's also CEL compatible
but doesn't require any sort of TCG blessing of the events.  Plus we
also have existing kernel infrastructure to log to that format.

Regards,

James


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 4/4] tsm: Allow for extending and reading configured RTMRs
  2024-05-11  2:57   ` James Bottomley
@ 2024-05-13 10:16     ` Samuel Ortiz
  2024-05-13 14:03       ` James Bottomley
  0 siblings, 1 reply; 40+ messages in thread
From: Samuel Ortiz @ 2024-05-13 10:16 UTC (permalink / raw)
  To: James Bottomley
  Cc: Dan Williams, Kuppuswamy Sathyanarayanan, Qinkun Bao, Yao,
	Jiewen, Xing, Cedric, Dionna Amalie Glaze, biao.lu, linux-coco,
	linux-integrity, linux-kernel

On Fri, May 10, 2024 at 10:57:37PM -0400, James Bottomley wrote:
> I'm not really sure where to hang this, since there's no posted agenda
> or materials for the CCC meeting today.

The agenda was posted on the linux-coco ml [1]. I sent a link to the
presentation slides [2] to the thread.

> However, it struck me you missed a third option: use the ima log
> format.  This has the advantage that we can define additional events
> and have them published with a kernel patch (the IMA log format is
> defined in the kernel).  Thanks to the TCG, it's also CEL compatible
> but doesn't require any sort of TCG blessing of the events.  Plus we
> also have existing kernel infrastructure to log to that format.

That's an interesting idea. It may avoid having to extend the CEL spec
with a new Content Type, but otoh the current spec defines which IMA
events are supported. So adding new ones may require to also eventually
extend the spec. But I guess since IMA is a Linux kernel subsystem,
changing the kernel code and ABI would de-facto extend the TCG CEL IMA
spec.

Here I assume you're talking about the IMA_TEMPLATE CEL specified
format, which is designed to accomodate for the current kernel IMA log
format. The main drawback of this format is that the digest does not
include the whole content event, making the CEL content type, the IMA
tag name and both lengths (for the content event and the IMA content)
untrusted for event log verifiers.

CEL defines another IMA format (IMA_TLV), that hashes the whole event
content. I think we should at least use that format as our output ABI,
if we want to use a TCG specified IMA content type.

Cheers,
Samuel.

[1] https://lore.kernel.org/linux-coco/61b65115-5945-4e27-89e4-bb6cba657f7f@linux.intel.com/
[2] https://docs.google.com/presentation/d/1qMk-8TiMigVmVAEDWXqPu9Jd7OJ8AGvCR34Lp2WunhU/edit?usp=sharing

> Regards,
> 
> James
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 4/4] tsm: Allow for extending and reading configured RTMRs
  2024-05-13 10:16     ` Samuel Ortiz
@ 2024-05-13 14:03       ` James Bottomley
  2024-05-14  5:08         ` Samuel Ortiz
  0 siblings, 1 reply; 40+ messages in thread
From: James Bottomley @ 2024-05-13 14:03 UTC (permalink / raw)
  To: Samuel Ortiz
  Cc: Dan Williams, Kuppuswamy Sathyanarayanan, Qinkun Bao, Yao,
	Jiewen, Xing, Cedric, Dionna Amalie Glaze, biao.lu, linux-coco,
	linux-integrity, linux-kernel

On Mon, 2024-05-13 at 12:16 +0200, Samuel Ortiz wrote:
> On Fri, May 10, 2024 at 10:57:37PM -0400, James Bottomley wrote:
> > I'm not really sure where to hang this, since there's no posted
> > agenda
> > or materials for the CCC meeting today.
> 
> The agenda was posted on the linux-coco ml [1]. I sent a link to the
> presentation slides [2] to the thread.

That's great, thanks.

> > However, it struck me you missed a third option: use the ima log
> > format.  This has the advantage that we can define additional
> > events and have them published with a kernel patch (the IMA log
> > format is defined in the kernel).  Thanks to the TCG, it's also CEL
> > compatible but doesn't require any sort of TCG blessing of the
> > events.  Plus we also have existing kernel infrastructure to log to
> > that format.
> 
> That's an interesting idea. It may avoid having to extend the CEL
> spec with a new Content Type, but otoh the current spec defines which
> IMA events are supported. So adding new ones may require to also
> eventually extend the spec. But I guess since IMA is a Linux kernel
> subsystem, changing the kernel code and ABI would de-facto extend the
> TCG CEL IMA spec.

That's what I was assuming since the TCG is currently deferring to IMA
in that regard.

> Here I assume you're talking about the IMA_TEMPLATE CEL specified
> format, which is designed to accomodate for the current kernel IMA
> log format. The main drawback of this format is that the digest does
> not include the whole content event, making the CEL content type, the
> IMA tag name and both lengths (for the content event and the IMA
> content) untrusted for event log verifiers.

That's only because IMA doesn't yet have such an event.  If we're
assuming effectively designing an IMA log format for non repudiation of
external events, one can be added.  Although I wouldn't want to be
hasty: one of the big problems of all options is that no existing log
format really covers the measure container use case and we're not
completely sure what other use cases will arise (the firewall rules
measurements was one that regulated cloud providers seem to think would
be important ... and that has a periodic rush of events, but there will
be others).

However, the current IMA templates (event descriptions) are known by an
ASCII prefix (they all begin ima-):

https://docs.kernel.org/security/IMA-templates.html#supported-template-fields-and-descriptors

So it would be easy to add more with a non ima- prefix.  Note that this
doc is out of date an IMA does support hashes all the way to SHA256
although SHA384 isn't currently listed.

The current record fields are defined in

security/integrity/ima/ima_template.c

> CEL defines another IMA format (IMA_TLV), that hashes the whole event
> content. I think we should at least use that format as our output
> ABI, if we want to use a TCG specified IMA content type.

Possibly.  Although avoiding double hashing may be a useful performance
measure (not really sure how fast records will come in yet).

James


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 4/4] tsm: Allow for extending and reading configured RTMRs
  2024-05-13 14:03       ` James Bottomley
@ 2024-05-14  5:08         ` Samuel Ortiz
  2024-05-16  8:33           ` Xing, Cedric
  0 siblings, 1 reply; 40+ messages in thread
From: Samuel Ortiz @ 2024-05-14  5:08 UTC (permalink / raw)
  To: James Bottomley
  Cc: Dan Williams, Kuppuswamy Sathyanarayanan, Qinkun Bao, Yao,
	Jiewen, Xing, Cedric, Dionna Amalie Glaze, biao.lu, linux-coco,
	linux-integrity, linux-kernel

On Mon, May 13, 2024 at 08:03:53AM -0600, James Bottomley wrote:
> On Mon, 2024-05-13 at 12:16 +0200, Samuel Ortiz wrote:
> > > However, it struck me you missed a third option: use the ima log
> > > format.  This has the advantage that we can define additional
> > > events and have them published with a kernel patch (the IMA log
> > > format is defined in the kernel).  Thanks to the TCG, it's also CEL
> > > compatible but doesn't require any sort of TCG blessing of the
> > > events.  Plus we also have existing kernel infrastructure to log to
> > > that format.
> > 
> > That's an interesting idea. It may avoid having to extend the CEL
> > spec with a new Content Type, but otoh the current spec defines which
> > IMA events are supported. So adding new ones may require to also
> > eventually extend the spec. But I guess since IMA is a Linux kernel
> > subsystem, changing the kernel code and ABI would de-facto extend the
> > TCG CEL IMA spec.
> 
> That's what I was assuming since the TCG is currently deferring to IMA
> in that regard.
> 
> > Here I assume you're talking about the IMA_TEMPLATE CEL specified
> > format, which is designed to accomodate for the current kernel IMA
> > log format. The main drawback of this format is that the digest does
> > not include the whole content event, making the CEL content type, the
> > IMA tag name and both lengths (for the content event and the IMA
> > content) untrusted for event log verifiers.
> 
> That's only because IMA doesn't yet have such an event.  If we're
> assuming effectively designing an IMA log format for non repudiation of
> external events, one can be added. 

If we were to follow the IMA_TEMPLATE format as our output RTMR ABI for
the event log, adding one or more IMA events would not change the fact
that the event and content type would not be hashed into the extended
digest. Unless we want to specify a different behaviour for each IMA
event, and then verifiers would have interpret the digest construction
differently depending on the IMA_TEMPLATE nested event type. And that's
not pretty IMHO.

Using the IMA_TLV content type would make that cut cleaner at least. A
digest is built on the whole content event, for all event types. And the
content and event types are trusted, i.e. the verifier can securely map
events to the reported event types.

> Although I wouldn't want to be
> hasty: one of the big problems of all options is that no existing log
> format really covers the measure container use case and we're not
> completely sure what other use cases will arise (the firewall rules
> measurements was one that regulated cloud providers seem to think would
> be important ... and that has a periodic rush of events, but there will
> be others).

Right. A new CEL content type would give us more freedom in that regard,
as it would allow us to define our own event content value in a more
flexible way. Instead of the nested TLV approach that IMA_TLV follows,
having one where the T would be a max length string defining the creator
of the event (a.k.a. the attester), would avoid having to formally
define each and every new event. That's where option #2 in the
presentation was heading to.

Cheers,
Samuel.
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RFC PATCH v2 4/4] tsm: Allow for extending and reading configured RTMRs
  2024-05-14  5:08         ` Samuel Ortiz
@ 2024-05-16  8:33           ` Xing, Cedric
  0 siblings, 0 replies; 40+ messages in thread
From: Xing, Cedric @ 2024-05-16  8:33 UTC (permalink / raw)
  To: Samuel Ortiz, James Bottomley
  Cc: Dan Williams, Kuppuswamy Sathyanarayanan, Qinkun Bao, Yao,
	Jiewen, Dionna Amalie Glaze, biao.lu, linux-coco,
	linux-integrity, linux-kernel

On 5/13/2024 10:08 PM, Samuel Ortiz wrote:
> On Mon, May 13, 2024 at 08:03:53AM -0600, James Bottomley wrote:
>> On Mon, 2024-05-13 at 12:16 +0200, Samuel Ortiz wrote:
>>>> However, it struck me you missed a third option: use the ima log
>>>> format.  This has the advantage that we can define additional
>>>> events and have them published with a kernel patch (the IMA log
>>>> format is defined in the kernel).  Thanks to the TCG, it's also CEL
>>>> compatible but doesn't require any sort of TCG blessing of the
>>>> events.  Plus we also have existing kernel infrastructure to log to
>>>> that format.
>>>
>>> That's an interesting idea. It may avoid having to extend the CEL
>>> spec with a new Content Type, but otoh the current spec defines which
>>> IMA events are supported. So adding new ones may require to also
>>> eventually extend the spec. But I guess since IMA is a Linux kernel
>>> subsystem, changing the kernel code and ABI would de-facto extend the
>>> TCG CEL IMA spec.
>>
>> That's what I was assuming since the TCG is currently deferring to IMA
>> in that regard.
>>
>>> Here I assume you're talking about the IMA_TEMPLATE CEL specified
>>> format, which is designed to accomodate for the current kernel IMA
>>> log format. The main drawback of this format is that the digest does
>>> not include the whole content event, making the CEL content type, the
>>> IMA tag name and both lengths (for the content event and the IMA
>>> content) untrusted for event log verifiers.
>>
>> That's only because IMA doesn't yet have such an event.  If we're
>> assuming effectively designing an IMA log format for non repudiation of
>> external events, one can be added.
> 
> If we were to follow the IMA_TEMPLATE format as our output RTMR ABI for
> the event log, adding one or more IMA events would not change the fact
> that the event and content type would not be hashed into the extended
> digest. Unless we want to specify a different behaviour for each IMA
> event, and then verifiers would have interpret the digest construction
> differently depending on the IMA_TEMPLATE nested event type. And that's
> not pretty IMHO.
> 
Agreed. This misses the design objective of separating storage from 
semantics of event records.

> Using the IMA_TLV content type would make that cut cleaner at least. A
> digest is built on the whole content event, for all event types. And the
> content and event types are trusted, i.e. the verifier can securely map
> events to the reported event types.
> 
The numerical T would need to be allocated/tracked by a central 
registry. This won't work for applications designed/developed outside of 
the kernel community, as they won't have a reliable way to avoid 
conflicts with each other. Thus, T needs to be a string, but that 
violates IMA_TLV definition. This is kinda fitting a square peg into a 
round hole IMHO.

>> Although I wouldn't want to be
>> hasty: one of the big problems of all options is that no existing log
>> format really covers the measure container use case and we're not
>> completely sure what other use cases will arise (the firewall rules
>> measurements was one that regulated cloud providers seem to think would
>> be important ... and that has a periodic rush of events, but there will
>> be others).
> 
> Right. A new CEL content type would give us more freedom in that regard,
> as it would allow us to define our own event content value in a more
> flexible way. Instead of the nested TLV approach that IMA_TLV follows,
> having one where the T would be a max length string defining the creator
> of the event (a.k.a. the attester), would avoid having to formally
> define each and every new event. That's where option #2 in the
> presentation was heading to.
> 
Agreed. In fact, the 2 primary design objectives of this event log are 
(1) to separate storage from semantics of event records and (2) to allow 
applications to define custom events and avoid conflicts with each other 
reliably. IMA_TLV meets the 1st objective but misses the 2nd; while 
IMA_TEMPLATE meets the 2nd but misses the 1st. And that's how we came to 
this Option #2.

-Cedric

^ permalink raw reply	[flat|nested] 40+ messages in thread

end of thread, other threads:[~2024-05-16  8:33 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-28 21:25 [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI Samuel Ortiz
2024-01-28 21:25 ` [RFC PATCH v2 1/4] tsm: Runtime measurement register support Samuel Ortiz
2024-01-29 16:57   ` Dionna Amalie Glaze
2024-02-01 22:03   ` Jarkko Sakkinen
2024-01-28 21:25 ` [RFC PATCH v2 2/4] tsm: Add RTMRs to the configfs-tsm hierarchy Samuel Ortiz
2024-01-28 22:38   ` Kuppuswamy Sathyanarayanan
2024-02-01 22:05   ` Jarkko Sakkinen
2024-02-21 16:16   ` Mikko Ylinen
2024-01-28 21:25 ` [RFC PATCH v2 3/4] tsm: Map RTMRs to TCG TPM PCRs Samuel Ortiz
2024-01-28 22:44   ` Kuppuswamy Sathyanarayanan
2024-02-02  6:18     ` James Bottomley
2024-01-28 21:25 ` [RFC PATCH v2 4/4] tsm: Allow for extending and reading configured RTMRs Samuel Ortiz
2024-05-11  2:57   ` James Bottomley
2024-05-13 10:16     ` Samuel Ortiz
2024-05-13 14:03       ` James Bottomley
2024-05-14  5:08         ` Samuel Ortiz
2024-05-16  8:33           ` Xing, Cedric
2024-02-01 22:02 ` [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI Jarkko Sakkinen
2024-02-02  6:24 ` James Bottomley
2024-02-02 23:07   ` Dan Middleton
2024-02-03  6:03     ` James Bottomley
2024-02-03  7:13       ` Kuppuswamy Sathyanarayanan
2024-02-03 10:27         ` James Bottomley
2024-02-06  8:34           ` Xing, Cedric
2024-02-06  8:57             ` James Bottomley
2024-02-07  2:02               ` Dan Williams
2024-02-07 20:16                 ` Xing, Cedric
2024-02-07 21:08                   ` Kuppuswamy Sathyanarayanan
2024-02-07 21:46                     ` James Bottomley
2024-02-09 20:58                       ` Dan Williams
2024-02-13  7:36                         ` Xing, Cedric
2024-02-13 16:05                           ` James Bottomley
2024-02-14  8:54                             ` Xing, Cedric
2024-02-15  6:14                               ` Dan Williams
2024-02-16  2:05                                 ` Xing, Cedric
2024-03-05  1:19                             ` Xing, Cedric
2024-04-17 20:23                               ` Dan Middleton
2024-02-13 16:54                           ` Mikko Ylinen
2024-02-15 22:44                           ` Dr. Greg
2024-02-22 15:45                       ` Lukas Wunner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).