Re: [PATCH v2 11/14] x86/sev: Extend the config-fs attestation support for an SVSM

From: Tom Lendacky
Date: Tue Mar 12 2024 - 09:29:52 EST


On 3/12/24 00:57, Kuppuswamy Sathyanarayanan wrote:

On 3/11/24 9:16 AM, Tom Lendacky wrote:
On 3/10/24 00:06, Kuppuswamy, Sathyanarayanan wrote:

On 3/8/24 10:35 AM, Tom Lendacky wrote:
When an SVSM is present, the guest can also request attestation reports
from the SVSM. These SVSM attestation reports can be used to attest the
SVSM and any services running within the SVSM.

Extend the config-fs attestation support to allow for an SVSM attestation
report. This involves creating four (4) new config-fs attributes:

   - 'svsm' (input)
     This attribute is used to determine whether the attestation request
     should be sent to the SVSM or to the SEV firmware.

   - 'service_guid' (input)
     Used for requesting the attestation of a single service within the
     SVSM. A null GUID implies that the SVSM_ATTEST_SERVICES call should
     be used to request the attestation report. A non-null GUID implies
     that the SVSM_ATTEST_SINGLE_SERVICE call should be used.

   - 'service_manifest_version' (input)
     Used with the SVSM_ATTEST_SINGLE_SERVICE call, the service version
     represents a specific service manifest version be used for the
     attestation report.

   - 'manifestblob' (output)
     Used to return the service manifest associated with the attestation
     report.

Signed-off-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
---
  Documentation/ABI/testing/configfs-tsm  |  59 ++++++++++
  arch/x86/include/asm/sev.h              |  31 ++++-
  arch/x86/kernel/sev.c                   |  50 ++++++++
  drivers/virt/coco/sev-guest/sev-guest.c | 147 ++++++++++++++++++++++++
  drivers/virt/coco/tsm.c                 |  95 ++++++++++++++-
  include/linux/tsm.h                     |  11 ++
  6 files changed, 390 insertions(+), 3 deletions(-)

diff --git a/Documentation/ABI/testing/configfs-tsm b/Documentation/ABI/testing/configfs-tsm
index dd24202b5ba5..a4663610bf7c 100644
--- a/Documentation/ABI/testing/configfs-tsm
+++ b/Documentation/ABI/testing/configfs-tsm


+
+What:        /sys/kernel/config/tsm/report/$name/service_guid
+Date:        January, 2024
+KernelVersion:    v6.9
+Contact:    linux-coco@xxxxxxxxxxxxxxx
+Description:
+        (WO) Attribute is visible if a TSM implementation provider
+        supports the concept of attestation reports for TVMs running
+        under an SVSM, like SEV-SNP. Specifying a empty or null GUID
+        (00000000-0000-0000-0000-000000) requests all active services
+        within the SVSM be part of the attestation report. Specifying
+        a non-null GUID requests an attestation report of just the
+        specified service using the manifest form specified by the
+        service_manifest_version attribute.
+        Secure VM Service Module for SEV-SNP Guests v1.00 Section 7.
+        https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/58019.pdf
+

I think it will be useful to the user if there is a attribute to list the service GUIDs
supported. It can help prevent user using incorrect or unsupported GUIDs.

A list of supported GUIDs can be obtained from the manifest of a all-services attestation request.

So they have to make a request twice? Once with a NULL GUID to get the
manifest with all service list, and another to make service-specific request?
There should be a fixed list of service GUIDs, right? Why not list them by
default?

It's not a fixed list. It may appear that way today, but as other services are added, then it is impossible for a kernel to know what services are present in the SVSM without querying it.






+    if (guid_len == UUID_STRING_LEN) {
+        rc = guid_parse(buf, &report->desc.service_guid);
+        if (rc)
+            return rc;
+    } else {
+        report->desc.service_guid = guid_null;

I think the default value will be guid_null right, why reset it to NULL for every failed attempt?

Default, yes. But what if it is written once, then a second time with an invalid GUID. Should the previously written GUID still be used?


If the user write fails, why update the state? IMO, we can leave it at the old value. But, lets see what others think.

Sounds good.

Thanks,
Tom


Thanks,
Tom