Re: [PATCH v4 2/6] configfs-tsm: Introduce a shared ABI for attestation reports

[Kuppuswamy Sathyanarayanan]

On 9/25/2023 9:17 PM, Dan Williams wrote:
> One of the common operations of a TSM (Trusted Security Module) is to
> provide a way for a TVM (confidential computing guest execution
> environment) to take a measurement of its launch state, sign it and
> submit it to a verifying party. Upon successful attestation that
> verifies the integrity of the TVM additional secrets may be deployed.
> The concept is common across TSMs, but the implementations are
> unfortunately vendor specific. While the industry grapples with a common
> definition of this attestation format [1], Linux need not make this
> problem worse by defining a new ABI per TSM that wants to perform a
> similar operation. The current momentum has been to invent new ioctl-ABI
> per TSM per function which at best is an abdication of the kernel's
> responsibility to make common infrastructure concepts share common ABI.
> 
> The proposal, targeted to conceptually work with TDX, SEV-SNP, COVE if
> not more, is to define a configfs interface to retrieve the TSM-specific
> blob.
> 
>     report=/sys/kernel/config/tsm/report/report0
>     mkdir $report
>     dd if=binary_userdata_plus_nonce > $report/inblob
>     hexdump $report/outblob
> 
> This approach later allows for the standardization of the attestation
> blob format without needing to invent a new ABI. Once standardization
> happens the standard format can be emitted by $report/outblob and
> indicated by $report/provider, or a new attribute like
> "$report/tcg_coco_report" can emit the standard format alongside the
> vendor format.
> 
> Review of previous iterations of this interface identified that there is
> a need to scale report generation for multiple container environments
> [2]. Configfs enables a model where each container can bind mount one or
> more report generation item instances. Still, within a container only a
> single thread can be manipulating a given configuration instance at a
> time. A 'generation' count is provided to detect conflicts between
> multiple threads racing to configure a report instance.
> 
> The SEV-SNP concepts of "extended reports" and "privilege levels" are
> optionally enabled by selecting 'tsm_report_ext_type' at register_tsm()
> time. The expectation is that those concepts are generic enough that
> they may be adopted by other TSM implementations. In other words,
> configfs-tsm aims to address a superset of TSM specific functionality
> with a common ABI where attributes may appear, or not appear, based on the set
> of concepts the implementation supports.
> 
> Link: http://lore.kernel.org/r/64961c3baf8ce_142af829436@xxxxxxxxxxxxxxxxxxxxxxxxx.notmuch [1]
> Link: http://lore.kernel.org/r/57f3a05e-8fcd-4656-beea-56bb8365ae64@xxxxxxxxxxxxxxxxxxx [2]
> Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>
> Cc: Dionna Amalie Glaze <dionnaglaze@xxxxxxxxxx>
> Cc: James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>
> Cc: Peter Gonda <pgonda@xxxxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Cc: Samuel Ortiz <sameo@xxxxxxxxxxxx>
> Acked-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Acked-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
> ---
>  Documentation/ABI/testing/configfs-tsm |   67 +++++
>  MAINTAINERS                            |    8 +
>  drivers/virt/coco/Kconfig              |    5 
>  drivers/virt/coco/Makefile             |    1 
>  drivers/virt/coco/tsm.c                |  411 ++++++++++++++++++++++++++++++++
>  include/linux/tsm.h                    |   63 +++++
>  6 files changed, 555 insertions(+)
>  create mode 100644 Documentation/ABI/testing/configfs-tsm
>  create mode 100644 drivers/virt/coco/tsm.c
>  create mode 100644 include/linux/tsm.h
> 

[...]

> +
> +static ssize_t __read_report(struct tsm_report *report, void *buf, size_t count,
> +			     enum tsm_data_select select)
> +{
> +	loff_t offset = 0;
> +	u8 *out, len;

When testing I noticed that it reports incorrect buf len. After debugging, noticed
that using u8 for len is incorrect. It should be size_t.

> +
> +	if (select == TSM_REPORT) {
> +		out = report->outblob;
> +		len = report->outblob_len;
> +	} else {
> +		out = report->certs;
> +		len = report->certs_len;
> +	}
> +
> +	if (!buf)
> +		return len;
> +	return memory_read_from_buffer(buf, count, &offset, out, len);
> +}
> +

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer