Message-ID: <5ef943d9-1ce6-4b20-8c6c-7cba4f1b3ea9@amd.com>
Date: Mon, 4 Nov 2024 19:40:37 +1100
From: Alexey Kardashevskiy <aik@....com>
To: Cedric Xing <cedric.xing@...el.com>,
 Dan Williams <dan.j.williams@...el.com>, Samuel Ortiz <sameo@...osinc.com>,
 James Bottomley <James.Bottomley@...senPartnership.com>,
 Lukas Wunner <lukas@...ner.de>, Dionna Amalie Glaze
 <dionnaglaze@...gle.com>, Qinkun Bao <qinkun@...gle.com>,
 Mikko Ylinen <mikko.ylinen@...ux.intel.com>,
 Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>
Cc: linux-kernel@...r.kernel.org, linux-coco@...ts.linux.dev
Subject: Re: [PATCH RFC v2 2/2] tsm: Add TVM Measurement Sample Code

On 1/11/24 03:50, Cedric Xing wrote:
> This sample kernel module demonstrates how to make MRs accessible to user mode
> through TSM.
> 
> Once loaded, this module registers a virtual measurement provider with the TSM
> core and will result in the directory tree below.
> 
> /sys/kernel/tsm/
> └── measurement-example
>      ├── config_mr
>      │   ├── digest
>      │   └── hash_algo

Do we actually need this many nodes? A digest is 64bytes long (or 128 
chars), hash_algo is lot less, "config_mr" could just print 
human-readable 2 lines (one with the algo, one with the digest), just 
like many other things in sysfs. And with one node per MR - no need in 
that suspicios _kset_put_children() (which at least belongs to 
lib/kobject.c). Or more nodes are coming?

(ignore if it's been discussed)

>      ├── full_report
>      ├── report_digest
>      │   ├── digest
>      │   └── hash_algo
>      ├── rtmr0
>      │   ├── digest
>      │   └── hash_algo
>      ├── rtmr1
>      │   ├── digest
>      │   └── hash_algo
>      ├── static_mr
>      │   ├── digest
>      │   └── hash_algo
>      └── user_data
>          ├── digest
>          └── hash_algo
> 
> Among the MRs in this example:
> 
> - `static_mr` and `config_mr` are static MRs.
> - `full_report` illustrates that a complete architecture-dependent attestation
>    report structure (for instance, the `TDREPORT` structure in Intel TDX) can be
>    presented as an MR. It also demonstrates exposing measurements in a file.

It looks that /sys/kernel/tsm/full_report is a binary concatenation of 6 
digests, with no hash_algo and no hint which digest is which, hardly a 
"structure". I do understand it is an example though :)



> - `rtmr0` is an RTMR with `TSM_MR_F_W` **cleared**, preventing direct
>    extensions; as a result, `rtmr0/digest` is read-only.
> - `rtmr1` is an RTMR with `TSM_MR_F_W` **set**, permitting direct extensions;
>    thus, `rtmr1/digest` is writable.
> - `user_data` represents the data provided by the software to be incorporated
>    into the attestation report. Writing to this MR and then reading from
>    `full_report` effectively triggers a request for an attestation report from
>    the underlying CC hardware.
> - `report_digest` serves as an example MR to demonstrate the use of the
>    `TSM_MR_F_L` flag.
> 
> See comments in `samples/tsm/measurement-example.c` for more details.
> 
> Signed-off-by: Cedric Xing <cedric.xing@...el.com>
> ---
>   include/linux/tsm.h               |  63 ++++++++++++++++++++
>   samples/Kconfig                   |   4 ++
>   samples/Makefile                  |   1 +
>   samples/tsm/Makefile              |   2 +
>   samples/tsm/measurement-example.c | 117 ++++++++++++++++++++++++++++++++++++++
>   5 files changed, 187 insertions(+)
> 
> diff --git a/include/linux/tsm.h b/include/linux/tsm.h
> index 11b0c525be30..291259fc85fc 100644
> --- a/include/linux/tsm.h
> +++ b/include/linux/tsm.h
> @@ -5,6 +5,7 @@
>   #include <linux/sizes.h>
>   #include <linux/types.h>
>   #include <linux/uuid.h>
> +#include <uapi/linux/hash_info.h>
>   
>   #define TSM_INBLOB_MAX 64
>   #define TSM_OUTBLOB_MAX SZ_32K
> @@ -109,4 +110,66 @@ struct tsm_ops {
>   
>   int tsm_register(const struct tsm_ops *ops, void *priv);
>   int tsm_unregister(const struct tsm_ops *ops);
> +
> +/**
> + * struct tsm_measurement_register - describes an architectural measurement
> + *                                   register (MR)
> + * @mr_name: name of the MR
> + * @mr_value: buffer containing the current value of the MR
> + * @mr_size: size of the MR - typically the digest size of @mr_hash
> + * @mr_flags: bitwise OR of flags defined in enum tsm_measurement_register_flag
> + * @mr_hash: optional hash identifier defined in include/uapi/linux/hash_info.h
> + */
> +struct tsm_measurement_register {
> +	const char *mr_name;
> +	void *mr_value;
> +	u32 mr_size;
> +	u32 mr_flags;
> +	enum hash_algo mr_hash;
> +};
> +
> +/**
> + * enum tsm_measurement_register_flag - properties of an MR
> + * @TSM_MR_F_X: this MR supports the extension semantics on write
> + * @TSM_MR_F_W: this MR is writable
> + * @TSM_MR_F_R: this MR is readable. This should typically be set
> + * @TSM_MR_F_L: this MR is live - writes to other MRs may change this MR
> + * @TSM_MR_F_F: present this MR as a file (instead of a directory)
> + * @TSM_MR_F_LIVE: shorthand for L (live) and R (readable)
> + * @TSM_MR_F_RTMR: shorthand for LIVE and X (extensible)
> + */
> +enum tsm_measurement_register_flag {
> +	TSM_MR_F_X = 1,
> +	TSM_MR_F_W = 2,
> +	TSM_MR_F_R = 4,
> +	TSM_MR_F_L = 8,
> +	TSM_MR_F_F = 16,

Usually these use hex.

> +	TSM_MR_F_LIVE = TSM_MR_F_L | TSM_MR_F_R,
> +	TSM_MR_F_RTMR = TSM_MR_F_LIVE | TSM_MR_F_X,
> +};
> +
> +#define TSM_MR_(mr, hash)                              \
> +	.mr_name = #mr, .mr_size = hash##_DIGEST_SIZE, \
> +	.mr_hash = HASH_ALGO_##hash, .mr_flags = TSM_MR_F_R
> +
> +/**
> + * struct tsm_measurement - define CC specific MRs and methods for updating
> + *                          them
> + * @name: name of the measurement provider
> + * @mrs: array of MR definitions ending with mr_name set to %NULL
> + * @refresh: invoked to update the specified MR
> + * @extend: invoked to extend the specified MR with mr_size bytes
> + */
> +struct tsm_measurement {
> +	const char *name;
> +	const struct tsm_measurement_register *mrs;
> +	int (*refresh)(struct tsm_measurement *,
> +		       const struct tsm_measurement_register *);
> +	int (*extend)(struct tsm_measurement *,
> +		      const struct tsm_measurement_register *, const u8 *);
> +};
> +
> +int tsm_register_measurement(struct tsm_measurement *);
> +int tsm_unregister_measurement(struct tsm_measurement *);
> +
>   #endif /* __TSM_H */
> diff --git a/samples/Kconfig b/samples/Kconfig
> index b288d9991d27..8159d3ca6487 100644
> --- a/samples/Kconfig
> +++ b/samples/Kconfig
> @@ -184,6 +184,10 @@ config SAMPLE_TIMER
>   	bool "Timer sample"
>   	depends on CC_CAN_LINK && HEADERS_INSTALL
>   
> +config SAMPLE_TSM
> +	tristate "TSM measurement sample"
> +	depends on TSM_REPORTS
> +
>   config SAMPLE_UHID
>   	bool "UHID sample"
>   	depends on CC_CAN_LINK && HEADERS_INSTALL
> diff --git a/samples/Makefile b/samples/Makefile
> index b85fa64390c5..891f5c12cd39 100644
> --- a/samples/Makefile
> +++ b/samples/Makefile
> @@ -39,3 +39,4 @@ obj-$(CONFIG_SAMPLE_KMEMLEAK)		+= kmemleak/
>   obj-$(CONFIG_SAMPLE_CORESIGHT_SYSCFG)	+= coresight/
>   obj-$(CONFIG_SAMPLE_FPROBE)		+= fprobe/
>   obj-$(CONFIG_SAMPLES_RUST)		+= rust/
> +obj-$(CONFIG_SAMPLE_TSM)		+= tsm/
> diff --git a/samples/tsm/Makefile b/samples/tsm/Makefile
> new file mode 100644
> index 000000000000..3969a59221e9
> --- /dev/null
> +++ b/samples/tsm/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +obj-$(CONFIG_SAMPLE_TSM) += measurement-example.o
> diff --git a/samples/tsm/measurement-example.c b/samples/tsm/measurement-example.c
> new file mode 100644
> index 000000000000..3abd67d3e569
> --- /dev/null
> +++ b/samples/tsm/measurement-example.c
> @@ -0,0 +1,117 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright(c) 2024 Intel Corporation. All rights reserved. */
> +
> +#include <linux/module.h>
> +#include <linux/tsm.h>
> +#include <crypto/hash_info.h>
> +#include <crypto/hash.h>
> +
> +struct __aligned(16)

Why this alignment?

> +{
> +	u8 static_mr[SHA384_DIGEST_SIZE];
> +	u8 config_mr[SHA512_DIGEST_SIZE];
> +	u8 rtmr0[SHA256_DIGEST_SIZE];
> +	u8 rtmr1[SHA384_DIGEST_SIZE];
> +	u8 user_data[SHA512_DIGEST_SIZE];
> +	u8 report_digest[SHA512_DIGEST_SIZE];
> +}
> +example_report = {
> +	.static_mr = "static_mr",
> +	.config_mr = "config_mr",
> +	.rtmr0 = "rtmr0",
> +	.rtmr1 = "rtmr1",
> +	.user_data = "user_data",
> +};
> +
> +DEFINE_FREE(shash, struct crypto_shash *,
> +	    if (!IS_ERR(_T)) crypto_free_shash(_T));
> +
> +static int _refresh_report(struct tsm_measurement *tmr,
> +			   const struct tsm_measurement_register *mr)
> +{
> +	pr_debug(KBUILD_MODNAME ": %s(%s,%s)\n", __func__, tmr->name,
> +		 mr->mr_name);
> +	struct crypto_shash *tfm __free(shash) =
> +		crypto_alloc_shash(hash_algo_name[HASH_ALGO_SHA512], 0, 0);
> +	if (IS_ERR(tfm))
> +		return PTR_ERR(tfm);
> +	return crypto_shash_tfm_digest(tfm, (u8 *)&example_report,
> +				       offsetof(typeof(example_report),
> +						report_digest),
> +				       example_report.report_digest);
> +}
> +
> +static int _extend_mr(struct tsm_measurement *tmr,
> +		      const struct tsm_measurement_register *mr, const u8 *data)
> +{
> +	SHASH_DESC_ON_STACK(desc, 0);
> +	int rc;
> +
> +	pr_debug(KBUILD_MODNAME ": %s(%s,%s)\n", __func__, tmr->name,
> +		 mr->mr_name);
> +
> +	desc->tfm = crypto_alloc_shash(hash_algo_name[mr->mr_hash], 0, 0);
> +	if (IS_ERR(desc->tfm))
> +		return PTR_ERR(desc->tfm);
> +
> +	BUG_ON(crypto_shash_digestsize(desc->tfm) != mr->mr_size);
> +
> +	rc = crypto_shash_init(desc);
> +	if (!rc)
> +		rc = crypto_shash_update(desc, mr->mr_value, mr->mr_size);
> +	if (!rc)
> +		rc = crypto_shash_finup(desc, data, mr->mr_size, mr->mr_value);
> +
> +	crypto_free_shash(desc->tfm);
> +	return rc;
> +}
> +
> +#define MR_(mr, hash) .mr_value = &example_report.mr, TSM_MR_(mr, hash)
> +static const struct tsm_measurement_register example_mrs[] = {
> +	/* the entire report can be considered as a LIVE MR */
> +	{ "full_report", &example_report, sizeof(example_report),
> +	  TSM_MR_F_LIVE | TSM_MR_F_F },
> +	/* static MR, read-only */
> +	{ MR_(static_mr, SHA384) },
> +	/* config MR, read-only */
> +	{ MR_(config_mr, SHA512) },
> +	/* RTMR, direct extension prohibited */

What is indirect extension here? Thanks,

> +	{ MR_(rtmr0, SHA256) | TSM_MR_F_RTMR },
> +	/* RTMR, direct extension allowed */
> +	{ MR_(rtmr1, SHA384) | TSM_MR_F_RTMR | TSM_MR_F_W },
> +	/* most CC archs allow including user data in attestation */
> +	{ MR_(user_data, SHA512) | TSM_MR_F_W },
> +	/* LIVE MR example, usually doesn't exist in real CC arch */
> +	{ MR_(report_digest, SHA512) | TSM_MR_F_L },
> +	/* terminating NULL entry */
> +	{}
> +};
> +#undef MR_
> +
> +static struct tsm_measurement example_measurement_provider = {
> +	.name = "measurement-example",
> +	.mrs = example_mrs,
> +	.refresh = _refresh_report,
> +	.extend = _extend_mr,
> +};
> +
> +static int __init measurement_example_init(void)
> +{
> +	int rc = tsm_register_measurement(&example_measurement_provider);
> +	pr_debug(KBUILD_MODNAME ": tsm_register_measurement(%p)=%d\n",
> +		 &example_measurement_provider, rc);
> +	return rc;
> +}
> +
> +static void __exit measurement_example_exit(void)
> +{
> +	int rc = tsm_unregister_measurement(&example_measurement_provider);
> +	pr_debug(KBUILD_MODNAME ": tsm_unregister_measurement(%p)=%d\n",
> +		 &example_measurement_provider, rc);
> +}
> +
> +module_init(measurement_example_init);
> +module_exit(measurement_example_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Sample tsm_measurement implementation");
> 

-- 
Alexey

Powered by blists - more mailing lists