Message-ID: <ZOYOXTsveEhPCHq2@vermeer>
Date:   Wed, 23 Aug 2023 15:49:17 +0200
From:   Samuel Ortiz <sameo@...osinc.com>
To:     Dan Williams <dan.j.williams@...el.com>
Cc:     linux-coco@...ts.linux.dev,
        Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        Dionna Amalie Glaze <dionnaglaze@...gle.com>,
        James Bottomley <James.Bottomley@...senpartnership.com>,
        Peter Gonda <pgonda@...gle.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        peterz@...radead.org, x86@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/5] tsm: Introduce a shared ABI for attestation
 reports

Hi Dan,

On Mon, Aug 14, 2023 at 12:43:21AM -0700, Dan Williams wrote:
> One of the common operations of a TSM (Trusted Security Module) is to
> provide a way for a TVM (confidential computing guest execution
> environment) to take a measurement of its launch state, sign it and
> submit it to a verifying party. Upon successful attestation that
> verifies the integrity of the TVM additional secrets may be deployed.
> The concept is common across TSMs, but the implementations are
> unfortunately vendor specific. While the industry grapples with a common
> definition of this attestation format [1], Linux need not make this
> problem worse by defining a new ABI per TSM that wants to perform a
> similar operation. The current momentum has been to invent new ioctl-ABI
> per TSM per function which at best is an abdication of the kernel's
> responsibility to make common infrastructure concepts share common ABI.
> 
> The proposal, targeted to conceptually work with TDX, SEV, COVE if not
> more, is to define a sysfs interface to retrieve the TSM-specific blob.
> 
>     echo $hex_encoded_userdata_plus_nonce > /sys/class/tsm/tsm0/inhex
>     hexdump /sys/class/tsm/tsm0/outblob

My concern with that interface is that one could easily get an
attestation report with a nonce set by another userspace component or
thread. I realize there is a generation counter to detect if a
configuration changed between the caller's last config setting and the
report it got, but I think that this shows that this may not be the best
interface. IMHO an attestation report request from userspace should be
an atomic call that includes multiple platform independent attibutes
like e.g. an attestation nonce.

> This approach later allows for the standardization of the attestation
> blob format without needing to change the Linux ABI. Until then, the
> format of 'outblob' is determined by the parent device for 'tsm0'.
> 
> The expectation is that this is a boot time exchange that need not be
> regenerated, 

This works well with the encrypted boot disk that's decrypted through
attestation use-case, but this is just one use case. Although I don't
expect attestation requests to be frequent, we should not assume this is
only a boot time operation. Not only it can happen after the guest is
fully booted, but it can also happen multiple times. An attestation flow
where a guest gets an attestation token back from a validated report is
something we'd want to support. Those token's validity are time limited,
and userspace would want to regenerate a report, with a fresh,
attestation service provided nonce.
Another thing to keep in mind is that an attestation report could be
amended by userspace itself, for TEE that support runtime measurement
(The RTMR things...). So the TVM measurement itself could change during
the lifecycle of a TVM.

> making it amenable to a sysfs interface. In case userspace
> does try to generate multiple attestation reports it includes conflict
> detection so userspace can be sure no other thread changed the
> parameters from its last configuration step to the blob retrieval.
> 
> TSM specific options are encoded as 'extra' attributes on the TSM device
> with the expectation that vendors reuse the same options for similar
> concepts. The current options are defined by SEV-SNP's need for a
> 'privilege level' concept (VMPL), and the option to retrieve a
> certificate chain in addition to the attestation report ("extended"
> format).
> 
> Link: http://lore.kernel.org/r/64961c3baf8ce_142af829436@dwillia2-xfh.jf.intel.com.notmuch [1]
> Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>
> Cc: Dionna Amalie Glaze <dionnaglaze@...gle.com>
> Cc: James Bottomley <James.Bottomley@...senPartnership.com>
> Cc: Peter Gonda <pgonda@...gle.com>
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Cc: Samuel Ortiz <sameo@...osinc.com>
> Signed-off-by: Dan Williams <dan.j.williams@...el.com>
> ---
>  Documentation/ABI/testing/sysfs-class-tsm |   47 +++++
>  MAINTAINERS                               |    8 +
>  drivers/virt/coco/Kconfig                 |    4 
>  drivers/virt/coco/Makefile                |    1 
>  drivers/virt/coco/tdx-guest/Kconfig       |    1 
>  drivers/virt/coco/tsm.c                   |  290 +++++++++++++++++++++++++++++
>  include/linux/tsm.h                       |   45 +++++
>  7 files changed, 396 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-tsm
>  create mode 100644 drivers/virt/coco/tsm.c
>  create mode 100644 include/linux/tsm.h
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-tsm b/Documentation/ABI/testing/sysfs-class-tsm
> new file mode 100644
> index 000000000000..37017bde626d
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-tsm
> @@ -0,0 +1,47 @@
> +What:		/sys/class/tsm/tsm0/inhex
> +Date:		August, 2023
> +KernelVersion:	v6.6
> +Contact:	linux-cxl@...r.kernel.org

Did you mean linux-coco@ ?


> +Description:
> +		(RW) Hex encoded userdata to be included in the attestation
> +		report. For replay protection this should include a nonce, but
> +		the kernel does not place any restrictions on the content.
> +
> +What:		/sys/class/tsm/tsm0/outblob
> +Date:		August, 2023
> +KernelVersion:	v6.6
> +Contact:	linux-cxl@...r.kernel.org
> +Description:
> +		(RO) Binary attestation report generated from @inhex translated
> +		to binary and any options. The format of the report is vendor
> +		specific and determined by the parent device of 'tsm0'.
> +
> +What:		/sys/class/tsm/tsm0/generation
> +Date:		August, 2023
> +KernelVersion:	v6.6
> +Contact:	linux-cxl@...r.kernel.org
> +Description:
> +		(RO) The value in this attribute increments each time @inhex or
> +		any option is written. Userspace can detect conflicts by
> +		checking generation before writing to any attribute and making
> +		sure the number of writes matches expectations after reading
> +		@outblob.

One note here is that an attestation userspace thread could get an
invalid counter although the report is valid, if e.g. a separate thread
modified a config between the first thread report generation and it
reading the counter back. So an attestation report generating thread
could get false negatives with that interface.

Cheers,
Samuel.

Powered by blists - more mailing lists