In TDX guest, the attestation process is used to verify the TDX guest trustworthiness to other entities before provisioning secrets to the guest. The First step in the attestation process is TDREPORT generation, which involves getting the guest measurement data in the format of TDREPORT, which is further used to validate the authenticity of the TDX guest. TDREPORT by design is integrity-protected and can only be verified on the local machine.

To support remote verification of the TDREPORT (in a SGX-based attestation), the TDREPORT needs to be sent to the SGX Quoting Enclave (QE) to convert it to a remote verifiable Quote. SGX QE by design can only run outside of the TDX guest (i.e. in a host process or in a normal VM) and guest can use communication channels like vsock or TCP/IP to send the TDREPORT to the QE. But for security concerns, the TDX guest may not support these communication channels. To handle such cases, TDX defines a GetQuote hypercall which can be used by the guest to request the host VMM to communicate with the SGX QE. More details about GetQuote hypercall can be found in TDX Guest-Host Communication Interface (GHCI) for Intel TDX 1.0, section titled "TDG.VP.VMCALL<GetQuote>".

Add support for TDX_CMD_GET_QUOTE IOCTL to allow an attestation agent to submit GetQuote requests from the user space using GetQuote hypercall.

Since GetQuote is an asynchronous request hypercall, VMM will use the callback interrupt vector configured by the SetupEventNotifyInterrupt hypercall to notify the guest about Quote generation completion or failure. So register an IRQ handler for it.

GetQuote TDVMCALL requires TD guest pass a 4K aligned shared buffer with TDREPORT data as input, which is further used by the VMM to copy the TD Quote result after successful Quote generation. To create the shared buffer, allocate a large enough memory and mark it shared using set_memory_decrypted() in tdx_guest_init(). This buffer will be re-used for GetQuote requests in TDX_CMD_GET_QUOTE IOCTL handler.

Although this method reserves a fixed chunk of memory for GetQuote requests, such one-time allocation is preferable to the alternative choice of repeatedly allocating/freeing the shared buffer in the TDX_CMD_GET_QUOTE IOCTL handler, which will damage the direct map (because the sharing/unsharing process modifies the direct map). This allocation model is similar to that used by the AMD SEV guest driver.

Since the Quote generation process is not time-critical or frequently used, the current version does not support parallel GetQuote requests.

Reviewed-by: Tony Luck tony.luck@intel.com Reviewed-by: Andi Kleen ak@linux.intel.com Reviewed-by: Mika Westerberg mika.westerberg@linux.intel.com Reviewed-by: Erdem Aktas erdemaktas@google.com Acked-by: Kirill A. Shutemov kirill.shutemov@linux.intel.com Signed-off-by: Kuppuswamy Sathyanarayanan sathyanarayanan.kuppuswamy@linux.intel.com ---

Changes since v2: * Fixed commit log and comments as per review suggestion. * Renamed struct tdx_quote_hdr -> struct tdx_quote_buf. * Used alloc_pages_exact() instead of alloc_pages() in alloc_shared_pages(). * Clear quote buffer before each re-use in tdx_get_quote().

Changes since v1: * Removed platform bus device support. * Instead of allocating the shared buffers using DMA APIs in IOCTL handler, allocated it once in tdx_guest_init() and re-used it in GetQuote IOCTL handler. * To simplify the design, removed the support for parallel GetQuote requests. It can be added when there is a real requirement for it. * Fixed commit log and comments to reflect the latest changes.

Documentation/virt/coco/tdx-guest.rst | 11 ++ arch/x86/coco/tdx/tdx.c | 35 +++++ arch/x86/include/asm/tdx.h | 2 + drivers/virt/coco/tdx-guest/tdx-guest.c | 175 +++++++++++++++++++++++- include/uapi/linux/tdx-guest.h | 44 ++++++ 5 files changed, 266 insertions(+), 1 deletion(-)

diff --git a/Documentation/virt/coco/tdx-guest.rst b/Documentation/virt/coco/tdx-guest.rst index 46e316db6bb4..35e829466265 100644 --- a/Documentation/virt/coco/tdx-guest.rst +++ b/Documentation/virt/coco/tdx-guest.rst @@ -42,6 +42,17 @@ ABI. However, in the future, if the TDX Module supports more than one subtype, a new IOCTL CMD will be created to handle it. To keep the IOCTL naming consistent, a subtype index is added as part of the IOCTL CMD.

+2.2 TDX_CMD_GET_QUOTE +---------------------- + +:Input parameters: struct tdx_quote_req +:Output: Return 0 on success, -EIO on TDCALL failure or standard error number + on common failures. Upon successful execution, Quote data is copied + to tdx_quote_req.buf. + +The TDX_CMD_GET_QUOTE IOCTL can be used by attestation software to generate +Quote for the given TDREPORT using TDG.VP.VMCALL<GetQuote> hypercall. + Reference ---------

diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c index 49d7e11066c1..7e0af10515a6 100644 --- a/arch/x86/coco/tdx/tdx.c +++ b/arch/x86/coco/tdx/tdx.c @@ -33,6 +33,7 @@ #define TDVMCALL_MAP_GPA 0x10001 #define TDVMCALL_REPORT_FATAL_ERROR 0x10003 #define TDVMCALL_SETUP_NOTIFY_INTR 0x10004 +#define TDVMCALL_GET_QUOTE 0x10002

/* MMIO direction */ #define EPT_READ 0 @@ -198,6 +199,40 @@ static void __noreturn tdx_panic(const char *msg) __tdx_hypercall(&args); }

+/** + * tdx_hcall_get_quote() - Wrapper to request TD Quote using GetQuote + * hypercall. + * @buf: Address of the directly mapped shared kernel buffer which + * contains TDREPORT data. The same buffer will be used by + * VMM to store the generated TD Quote output. + * @size: size of the tdquote buffer (4KB-aligned). + * + * Refer to section titled "TDG.VP.VMCALL<GetQuote>" in the TDX GHCI + * v1.0 specification for more information on GetQuote hypercall. + * It is used in the TDX guest driver module to get the TD Quote. + * + * Return 0 on success or error code on failure. + */ +int tdx_hcall_get_quote(u8 *buf, size_t size) +{ + struct tdx_hypercall_args args = {0}; + + args.r10 = TDX_HYPERCALL_STANDARD; + args.r11 = TDVMCALL_GET_QUOTE; + /* Since buf is a shared memory, set the shared (decrypted) bits */ + args.r12 = cc_mkdec(virt_to_phys(buf)); + args.r13 = size; + + /* + * Pass the physical address of TDREPORT to the VMM and + * trigger the Quote generation. It is not a blocking + * call, hence completion of this request will be notified to + * the TD guest via a callback interrupt. + */ + return __tdx_hypercall(&args); +} +EXPORT_SYMBOL_GPL(tdx_hcall_get_quote); + static void tdx_parse_tdinfo(u64 *cc_mask) { struct tdx_module_output out; diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h index 8807fe1b1f3f..254468448a1b 100644 --- a/arch/x86/include/asm/tdx.h +++ b/arch/x86/include/asm/tdx.h @@ -75,6 +75,8 @@ int tdx_register_event_irq_cb(tdx_event_irq_cb_t handler, void *data);

int tdx_unregister_event_irq_cb(tdx_event_irq_cb_t handler, void *data);

+int tdx_hcall_get_quote(u8 *buf, size_t size); + #else

static inline void tdx_early_init(void) { }; diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c b/drivers/virt/coco/tdx-guest/tdx-guest.c index 5e44a0fa69bd..388491fa63a1 100644 --- a/drivers/virt/coco/tdx-guest/tdx-guest.c +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c @@ -12,12 +12,106 @@ #include <linux/mod_devicetable.h> #include <linux/string.h> #include <linux/uaccess.h> +#include <linux/set_memory.h>

#include <uapi/linux/tdx-guest.h>

#include <asm/cpu_device_id.h> #include <asm/tdx.h>

+/* + * Intel's SGX QE implementation generally uses Quote size less + * than 8K; Use 16K as MAX size to handle future updates and other + * 3rd party implementations. + */ +#define GET_QUOTE_MAX_SIZE (4 * PAGE_SIZE) + +/** + * struct quote_entry - Quote request struct + * @valid: Flag to check validity of the GetQuote request. + * @buf: Kernel buffer to share data with VMM (size is page aligned). + * @buf_len: Size of the buf in bytes. + * @compl: Completion object to track completion of GetQuote request. + */ +struct quote_entry { + bool valid; + void *buf; + size_t buf_len; + struct completion compl; +}; + +/* Quote data entry */ +static struct quote_entry *qentry; + +/* Lock to streamline quote requests */ +static DEFINE_MUTEX(quote_lock); + +static int quote_cb_handler(void *dev_id) +{ + struct quote_entry *entry = dev_id; + struct tdx_quote_buf *quote_buf = entry->buf; + + if (entry->valid && quote_buf->status != GET_QUOTE_IN_FLIGHT) + complete(&entry->compl); + + return 0; +} + +static void free_shared_pages(void *buf, size_t len) +{ + unsigned int count = PAGE_ALIGN(len) >> PAGE_SHIFT; + + set_memory_encrypted((unsigned long)buf, count); + + free_pages_exact(buf, PAGE_ALIGN(len)); +} + +static void *alloc_shared_pages(size_t len) +{ + unsigned int count = PAGE_ALIGN(len) >> PAGE_SHIFT; + void *addr; + int ret; + + addr = alloc_pages_exact(len, GFP_KERNEL); + if (!addr) + return NULL; + + ret = set_memory_decrypted((unsigned long)addr, count); + if (ret) { + free_pages_exact(addr, PAGE_ALIGN(len)); + return NULL; + } + + return addr; +} + +static struct quote_entry *alloc_quote_entry(size_t len) +{ + struct quote_entry *entry = NULL; + + entry = kmalloc(sizeof(*entry), GFP_KERNEL); + if (!entry) + return NULL; + + entry->buf = alloc_shared_pages(len); + if (!entry->buf) { + kfree(entry); + return NULL; + } + + entry->buf_len = PAGE_ALIGN(len); + init_completion(&entry->compl); + entry->valid = false; + + return entry; +} + +static void free_quote_entry(struct quote_entry *entry) +{ + free_shared_pages(entry->buf, entry->buf_len); + kfree(entry); +} + static long tdx_get_report0(struct tdx_report_req __user *req) { u8 *reportdata, *tdreport; @@ -53,12 +147,65 @@ static long tdx_get_report0(struct tdx_report_req __user *req) return ret; }

+static long tdx_get_quote(struct tdx_quote_req __user *ureq) +{ + struct tdx_quote_req req; + long ret; + + if (copy_from_user(&req, ureq, sizeof(req))) + return -EFAULT; + + mutex_lock(&quote_lock); + + if (!req.len || req.len > qentry->buf_len) { + ret = -EINVAL; + goto quote_failed; + } + + memset(qentry->buf, 0, qentry->buf_len); + reinit_completion(&qentry->compl); + qentry->valid = true; + + if (copy_from_user(qentry->buf, (void __user *)req.buf, req.len)) { + ret = -EFAULT; + goto quote_failed; + } + + /* Submit GetQuote Request using GetQuote hypercall */ + ret = tdx_hcall_get_quote(qentry->buf, qentry->buf_len); + if (ret) { + pr_err("GetQuote hypercall failed, status:%lx\n", ret); + ret = -EIO; + goto quote_failed; + } + + /* + * Although the GHCI specification does not state explicitly that + * the VMM must not wait indefinitely for the Quote request to be + * completed, a sane VMM should always notify the guest after a + * certain time, regardless of whether the Quote generation is + * successful or not. For now just assume the VMM will do so. + */ + wait_for_completion(&qentry->compl); + + if (copy_to_user((void __user *)req.buf, qentry->buf, req.len)) + ret = -EFAULT; + +quote_failed: + qentry->valid = false; + mutex_unlock(&quote_lock); + + return ret; +} + static long tdx_guest_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { switch (cmd) { case TDX_CMD_GET_REPORT0: return tdx_get_report0((struct tdx_report_req __user *)arg); + case TDX_CMD_GET_QUOTE: + return tdx_get_quote((struct tdx_quote_req *)arg); default: return -ENOTTY; } @@ -84,15 +231,41 @@ MODULE_DEVICE_TABLE(x86cpu, tdx_guest_ids);

static int __init tdx_guest_init(void) { + int ret; + if (!x86_match_cpu(tdx_guest_ids)) return -ENODEV;

- return misc_register(&tdx_misc_dev); + ret = misc_register(&tdx_misc_dev); + if (ret) + return ret; + + qentry = alloc_quote_entry(GET_QUOTE_MAX_SIZE); + if (!qentry) { + pr_err("Failed to allocate Quote buffer\n"); + ret = -ENOMEM; + goto free_misc; + } + + ret = tdx_register_event_irq_cb(quote_cb_handler, qentry); + if (ret) + goto free_quote; + + return 0; + +free_quote: + free_quote_entry(qentry); +free_misc: + misc_deregister(&tdx_misc_dev); + + return ret; } module_init(tdx_guest_init);

static void __exit tdx_guest_exit(void) { + tdx_unregister_event_irq_cb(quote_cb_handler, qentry); + free_quote_entry(qentry); misc_deregister(&tdx_misc_dev); } module_exit(tdx_guest_exit); diff --git a/include/uapi/linux/tdx-guest.h b/include/uapi/linux/tdx-guest.h index a6a2098c08ff..8a6ade299090 100644 --- a/include/uapi/linux/tdx-guest.h +++ b/include/uapi/linux/tdx-guest.h @@ -17,6 +17,12 @@ /* Length of TDREPORT used in TDG.MR.REPORT TDCALL */ #define TDX_REPORT_LEN 1024

+/* TD Quote status codes */ +#define GET_QUOTE_SUCCESS 0 +#define GET_QUOTE_IN_FLIGHT 0xffffffffffffffff +#define GET_QUOTE_ERROR 0x8000000000000000 +#define GET_QUOTE_SERVICE_UNAVAILABLE 0x8000000000000001 + /** * struct tdx_report_req - Request struct for TDX_CMD_GET_REPORT0 IOCTL. * @@ -30,6 +36,36 @@ struct tdx_report_req { __u8 tdreport[TDX_REPORT_LEN]; };

+/* struct tdx_quote_buf: Format of Quote request buffer. + * @version: Quote format version, filled by TD. + * @status: Status code of Quote request, filled by VMM. + * @in_len: Length of TDREPORT, filled by TD. + * @out_len: Length of Quote data, filled by VMM. + * @data: Quote data on output or TDREPORT on input. + * + * More details of Quote request buffer can be found in TDX + * Guest-Host Communication Interface (GHCI) for Intel TDX 1.0, + * section titled "TDG.VP.VMCALL<GetQuote>" + */ +struct tdx_quote_buf { + __u64 version; + __u64 status; + __u32 in_len; + __u32 out_len; + __u64 data[]; +}; + +/* struct tdx_quote_req: Request struct for TDX_CMD_GET_QUOTE IOCTL. + * @buf: Address of user buffer in the format of struct tdx_quote_buf. + * Upon successful completion of IOCTL, output is copied back to + * the same buffer (in struct tdx_quote_buf.data). + * @len: Length of the Quote buffer. + */ +struct tdx_quote_req { + __u64 buf; + __u64 len; +}; + /* * TDX_CMD_GET_REPORT0 - Get TDREPORT0 (a.k.a. TDREPORT subtype 0) using * TDCALL[TDG.MR.REPORT] @@ -39,4 +75,12 @@ struct tdx_report_req { */ #define TDX_CMD_GET_REPORT0 _IOWR('T', 1, struct tdx_report_req)

+/* + * TDX_CMD_GET_QUOTE - Get TD Guest Quote from QE/QGS using GetQuote + * TDVMCALL. + * + * Returns 0 on success or standard errno on other failures. + */ +#define TDX_CMD_GET_QUOTE _IOWR('T', 2, struct tdx_quote_req) + #endif /* _UAPI_LINUX_TDX_GUEST_H_ */

Dan Williams wrote:

[ add David, Brijesh, and Atish]

Kuppuswamy Sathyanarayanan wrote:

...

In TDX guest, the second stage of the attestation process is Quote generation. This process is required to convert the locally generated TDREPORT into a remotely verifiable Quote. It involves sending the TDREPORT data to a Quoting Enclave (QE) which will verify the integrity of the TDREPORT and sign it with an attestation key.

Intel's TDX attestation driver exposes TDX_CMD_GET_QUOTE IOCTL to allow the user agent to get the TD Quote.

Add a kernel selftest module to verify the Quote generation feature.

TD Quote generation involves following steps:

• Get the TDREPORT data using TDX_CMD_GET_REPORT IOCTL.
• Embed the TDREPORT data in quote buffer and request for quote generation via TDX_CMD_GET_QUOTE IOCTL request.
• Upon completion of the GetQuote request, check for non zero value in the status field of Quote header to make sure the generated quote is valid.

What this cover letter does not say is that this is adding another instance of the similar pattern as SNP_GET_REPORT.

Linux is best served when multiple vendors trying to do similar operations are brought together behind a common ABI. We see this in the history of wrangling SCSI vendors behind common interfaces. Now multiple confidential computing vendors trying to develop similar flows with differentiated formats where that differentiation need not leak over the ABI boundary.

[..]

Below is a rough mock up of this approach to demonstrate the direction. Again, the goal is to define an ABI that can support any vendor's arch-specific attestation method and key provisioning flows without leaking vendor-specific details, or confidential material over the user/kernel ABI.

The observation is that there are a sufficient number of attestation flows available to review where Linux can define a superset ABI to contain them all. The other observation is that the implementations have features that may cross-polinate over time. For example the SEV privelege level consideration ("vmpl"), and the TDX RTMR (think TPM PCRs) mechanisms address generic Confidential Computing use cases.

Vendor specific ioctls for all of this feels like surrender when Linux already has the keys subsystem which has plenty of degrees of freedom for tracking blobs with signatures and using those blobs to instantiate other blobs. It already serves as the ABI wrapping various TPM implementations and marshaling keys for storage encryption and other use cases that intersect Confidential Computing.

The benefit of deprecating vendor-specific abstraction layers in userspace is secondary. The primary benefit is collaboration. It enables kernel developers from various architectures to collaborate on common infrastructure. If, referring back to my previous example, SEV adopts an RTMR-like mechanism and TDX adopts a vmpl-like mechanism it would be unfortunate if those efforts were siloed, duplicated, and needlessly differentiated to userspace. So while there are arguably a manageable number of basic arch attestation methods the planned expansion of those to build incremental functionality is where I believe we, as a community, will be glad that we invested in a "Linux format" for all of this.

An example, to show what the strawman patch below enables: (req_key is the sample program from "man 2 request_key")

# ./req_key guest_attest guest_attest:0:0-$desc $(cat user_data | base64) Key ID is 10e2f3a7 # keyctl pipe 0x10e2f3a7 | hexdump -C 00000000 54 44 58 20 47 65 6e 65 72 61 74 65 64 20 51 75 |TDX Generated Qu| 00000010 6f 74 65 00 00 00 00 00 00 00 00 00 00 00 00 00 |ote.............| 00000020 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| * 00004000

This is the kernel instantiating a TDX Quote without the TDREPORT implementation detail ever leaving the kernel. Now, this is only the top-half of what is needed. The missing bottom half takes that material and uses it to instantiate derived key material like the storage decryption key internal to the kernel. See "The Process" in Documentation/security/keys/request-key.rst for how the Keys subsystem handles the "keys for keys" use case.

--- diff --git a/drivers/virt/Kconfig b/drivers/virt/Kconfig index f79ab13a5c28..0f775847028e 100644 --- a/drivers/virt/Kconfig +++ b/drivers/virt/Kconfig @@ -54,4 +54,8 @@ source "drivers/virt/coco/sev-guest/Kconfig"

source "drivers/virt/coco/tdx-guest/Kconfig"

+config GUEST_ATTEST + tristate + select KEYS + endif diff --git a/drivers/virt/Makefile b/drivers/virt/Makefile index e9aa6fc96fab..66f6b838f8f4 100644 --- a/drivers/virt/Makefile +++ b/drivers/virt/Makefile @@ -12,3 +12,4 @@ obj-$(CONFIG_ACRN_HSM) += acrn/ obj-$(CONFIG_EFI_SECRET) += coco/efi_secret/ obj-$(CONFIG_SEV_GUEST) += coco/sev-guest/ obj-$(CONFIG_INTEL_TDX_GUEST) += coco/tdx-guest/ +obj-$(CONFIG_GUEST_ATTEST) += coco/guest-attest/ diff --git a/drivers/virt/coco/guest-attest/Makefile b/drivers/virt/coco/guest-attest/Makefile new file mode 100644 index 000000000000..5581c5a27588 --- /dev/null +++ b/drivers/virt/coco/guest-attest/Makefile @@ -0,0 +1,2 @@ +obj-$(CONFIG_GUEST_ATTEST) += guest_attest.o +guest_attest-y := key.o diff --git a/drivers/virt/coco/guest-attest/key.c b/drivers/virt/coco/guest-attest/key.c new file mode 100644 index 000000000000..2a494b6dd7a7 --- /dev/null +++ b/drivers/virt/coco/guest-attest/key.c @@ -0,0 +1,159 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* Copyright(c) 2023 Intel Corporation. All rights reserved. */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#include <linux/seq_file.h> +#include <linux/key-type.h> +#include <linux/module.h> +#include <linux/base64.h> + +#include <keys/request_key_auth-type.h> +#include <keys/user-type.h> + +#include "guest-attest.h" + +static LIST_HEAD(guest_attest_list); +static DECLARE_RWSEM(guest_attest_rwsem); + +static struct guest_attest_ops *fetch_ops(void) +{ + return list_first_entry_or_null(&guest_attest_list, + struct guest_attest_ops, list); +} + +static struct guest_attest_ops *get_ops(void) +{ + down_read(&guest_attest_rwsem); + return fetch_ops(); +} + +static void put_ops(void) +{ + up_read(&guest_attest_rwsem); +} + +int register_guest_attest_ops(struct guest_attest_ops *ops) +{ + struct guest_attest_ops *conflict; + int rc; + + down_write(&guest_attest_rwsem); + conflict = fetch_ops(); + if (conflict) { + pr_err(""%s" ops already registered\n", conflict->name); + rc = -EEXIST; + goto out; + } + list_add(&ops->list, &guest_attest_list); + try_module_get(ops->module); + rc = 0; +out: + up_write(&guest_attest_rwsem); + return rc; +} +EXPORT_SYMBOL_GPL(register_guest_attest_ops); + +void unregister_guest_attest_ops(struct guest_attest_ops *ops) +{ + down_write(&guest_attest_rwsem); + list_del(&ops->list); + up_write(&guest_attest_rwsem); + module_put(ops->module); +} +EXPORT_SYMBOL_GPL(unregister_guest_attest_ops); + +static int __guest_attest_request_key(struct key *key, int level, + struct key *dest_keyring, + const char *callout_info, int callout_len, + struct key *authkey) +{ + struct guest_attest_ops *ops; + void *payload = NULL; + int rc, payload_len; + + ops = get_ops(); + if (!ops) + return -ENOKEY; + + payload = kzalloc(max(GUEST_ATTEST_DATALEN, callout_len), GFP_KERNEL); + if (!payload) { + rc = -ENOMEM; + goto out; + } + + payload_len = base64_decode(callout_info, callout_len, payload); + if (payload_len < 0 || payload_len > GUEST_ATTEST_DATALEN) { + rc = -EINVAL; + goto out; + } + + rc = ops->request_attest(key, level, dest_keyring, payload, payload_len, + authkey); +out: + kfree(payload); + put_ops(); + return rc; +} + +static int guest_attest_request_key(struct key *authkey, void *data) +{ + struct request_key_auth *rka = get_request_key_auth(authkey); + struct key *key = rka->target_key; + unsigned long long id; + int rc, level; + + pr_debug("desc: %s op: %s callout: %s\n", key->description, rka->op, + rka->callout_info ? (char *)rka->callout_info : ""none""); + + if (sscanf(key->description, "guest_attest:%d:%llu", &level, &id) != 2) + return -EINVAL; + + if (!rka->callout_info) { + rc = -EINVAL; + goto out; + } + + rc = __guest_attest_request_key(key, level, rka->dest_keyring, + rka->callout_info, rka->callout_len, + authkey); +out: + complete_request_key(authkey, rc); + return rc; +} + +static int guest_attest_vet_description(const char *desc) +{ + unsigned long long id; + int level; + + if (sscanf(desc, "guest_attest:%d:%llu", &level, &id) != 2) + return -EINVAL; + return 0; +} + +static struct key_type key_type_guest_attest = { + .name = "guest_attest", + .preparse = user_preparse, + .free_preparse = user_free_preparse, + .instantiate = generic_key_instantiate, + .revoke = user_revoke, + .destroy = user_destroy, + .describe = user_describe, + .read = user_read, + .vet_description = guest_attest_vet_description, + .request_key = guest_attest_request_key, +}; + +static int __init guest_attest_init(void) +{ + return register_key_type(&key_type_guest_attest); +} + +static void __exit guest_attest_exit(void) +{ + unregister_key_type(&key_type_guest_attest); +} + +module_init(guest_attest_init); +module_exit(guest_attest_exit); +MODULE_LICENSE("GPL v2"); diff --git a/drivers/virt/coco/tdx-guest/Kconfig b/drivers/virt/coco/tdx-guest/Kconfig index 14246fc2fb02..9a1ec85369fe 100644 --- a/drivers/virt/coco/tdx-guest/Kconfig +++ b/drivers/virt/coco/tdx-guest/Kconfig @@ -1,6 +1,7 @@ config TDX_GUEST_DRIVER tristate "TDX Guest driver" depends on INTEL_TDX_GUEST + select GUEST_ATTEST help The driver provides userspace interface to communicate with the TDX module to request the TDX guest details like attestation diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c b/drivers/virt/coco/tdx-guest/tdx-guest.c index 388491fa63a1..65b5aab284d9 100644 --- a/drivers/virt/coco/tdx-guest/tdx-guest.c +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c @@ -13,11 +13,13 @@ #include <linux/string.h> #include <linux/uaccess.h> #include <linux/set_memory.h> +#include <linux/key-type.h>

#include <uapi/linux/tdx-guest.h>

#include <asm/cpu_device_id.h> #include <asm/tdx.h> +#include "../guest-attest/guest-attest.h"

/* * Intel's SGX QE implementation generally uses Quote size less @@ -229,6 +231,62 @@ static const struct x86_cpu_id tdx_guest_ids[] = { }; MODULE_DEVICE_TABLE(x86cpu, tdx_guest_ids);

+static int tdx_request_attest(struct key *key, int level, + struct key *dest_keyring, void *payload, + int payload_len, struct key *authkey) +{ + u8 *tdreport; + long ret; + + tdreport = kzalloc(TDX_REPORT_LEN, GFP_KERNEL); + if (!tdreport) + return -ENOMEM; + + /* Generate TDREPORT0 using "TDG.MR.REPORT" TDCALL */ + ret = tdx_mcall_get_report0(payload, tdreport); + if (ret) + goto out; + + mutex_lock(&quote_lock); + + memset(qentry->buf, 0, qentry->buf_len); + reinit_completion(&qentry->compl); + qentry->valid = true; + + /* Submit GetQuote Request using GetQuote hyperetall */ + ret = tdx_hcall_get_quote(qentry->buf, qentry->buf_len); + if (ret) { + pr_err("GetQuote hyperetall failed, status:%lx\n", ret); + ret = -EIO; + goto quote_failed; + } + + /* + * Although the GHCI specification does not state explicitly that + * the VMM must not wait indefinitely for the Quote request to be + * completed, a sane VMM should always notify the guest after a + * certain time, regardless of whether the Quote generation is + * successful or not. For now just assume the VMM will do so. + */ + wait_for_completion(&qentry->compl); + + ret = key_instantiate_and_link(key, qentry->buf, qentry->buf_len, + dest_keyring, authkey); + +quote_failed: + qentry->valid = false; + mutex_unlock(&quote_lock); +out: + kfree(tdreport); + return ret; +} + +static struct guest_attest_ops tdx_attest_ops = { + .name = KBUILD_MODNAME, + .module = THIS_MODULE, + .request_attest = tdx_request_attest, +}; + static int __init tdx_guest_init(void) { int ret; @@ -251,8 +309,14 @@ static int __init tdx_guest_init(void) if (ret) goto free_quote;

+ ret = register_guest_attest_ops(&tdx_attest_ops); + if (ret) + goto free_irq; + return 0;

+free_irq: + tdx_unregister_event_irq_cb(quote_cb_handler, qentry); free_quote: free_quote_entry(qentry); free_misc: @@ -264,6 +328,7 @@ module_init(tdx_guest_init);

static void __exit tdx_guest_exit(void) { + unregister_guest_attest_ops(&tdx_attest_ops); tdx_unregister_event_irq_cb(quote_cb_handler, qentry); free_quote_entry(qentry); misc_deregister(&tdx_misc_dev);

Hi Dan,

On 6/23/23 3:27 PM, Dan Williams wrote:

Dan Williams wrote:

...

[ add David, Brijesh, and Atish]

Kuppuswamy Sathyanarayanan wrote:

...

In TDX guest, the second stage of the attestation process is Quote generation. This process is required to convert the locally generated TDREPORT into a remotely verifiable Quote. It involves sending the TDREPORT data to a Quoting Enclave (QE) which will verify the integrity of the TDREPORT and sign it with an attestation key.

Intel's TDX attestation driver exposes TDX_CMD_GET_QUOTE IOCTL to allow the user agent to get the TD Quote.

Add a kernel selftest module to verify the Quote generation feature.

TD Quote generation involves following steps:

• Get the TDREPORT data using TDX_CMD_GET_REPORT IOCTL.
• Embed the TDREPORT data in quote buffer and request for quote generation via TDX_CMD_GET_QUOTE IOCTL request.
• Upon completion of the GetQuote request, check for non zero value in the status field of Quote header to make sure the generated quote is valid.

What this cover letter does not say is that this is adding another instance of the similar pattern as SNP_GET_REPORT.

Linux is best served when multiple vendors trying to do similar operations are brought together behind a common ABI. We see this in the history of wrangling SCSI vendors behind common interfaces. Now multiple confidential computing vendors trying to develop similar flows with differentiated formats where that differentiation need not leak over the ABI boundary.

[..]

Below is a rough mock up of this approach to demonstrate the direction. Again, the goal is to define an ABI that can support any vendor's arch-specific attestation method and key provisioning flows without leaking vendor-specific details, or confidential material over the user/kernel ABI.

Thanks for working on this mock code and helping out. It gives me the general idea about your proposal.

The observation is that there are a sufficient number of attestation flows available to review where Linux can define a superset ABI to contain them all. The other observation is that the implementations have features that may cross-polinate over time. For example the SEV privelege level consideration ("vmpl"), and the TDX RTMR (think TPM PCRs) mechanisms address generic Confidential Computing use cases.

I agree with your point about VMPL and RTMR feature cases. This observation is valid for AMD SEV and TDX attestation flows. But I am not sure whether it will hold true for other vendor implementations. Our sample set is not good enough to make this conclusion. The reason for my concern is, if you check the ABI interface used in the S390 arch attestation driver (drivers/s390/char/uvdevice.c), you would notice that there is a significant difference between the ABI used in that driver and SEV/TDX drivers. The S390 driver attestation request appears to accept two data blobs as input, as well as a variety of vendor-specific header configurations.

Maybe the s390 attestation model is a special case, but, I think we consider this issue. Since we don't have a common spec, there is chance that any superset ABI we define now may not meet future vendor requirements. One way to handle it to leave enough space in the generic ABI to handle future vendor requirements.

I think it would be better if other vendors (like ARM or RISC) can comment and confirm whether this proposal meets their demands.

Vendor specific ioctls for all of this feels like surrender when Linux already has the keys subsystem which has plenty of degrees of freedom for tracking blobs with signatures and using those blobs to instantiate other blobs. It already serves as the ABI wrapping various TPM implementations and marshaling keys for storage encryption and other use cases that intersect Confidential Computing.

The benefit of deprecating vendor-specific abstraction layers in userspace is secondary. The primary benefit is collaboration. It enables kernel developers from various architectures to collaborate on common infrastructure. If, referring back to my previous example, SEV adopts an RTMR-like mechanism and TDX adopts a vmpl-like mechanism it would be unfortunate if those efforts were siloed, duplicated, and needlessly differentiated to userspace. So while there are arguably a manageable number of basic arch attestation methods the planned expansion of those to build incremental functionality is where I believe we, as a community, will be glad that we invested in a "Linux format" for all of this.

An example, to show what the strawman patch below enables: (req_key is the sample program from "man 2 request_key")

# ./req_key guest_attest guest_attest:0:0-$desc $(cat user_data | base64) Key ID is 10e2f3a7 # keyctl pipe 0x10e2f3a7 | hexdump -C 00000000 54 44 58 20 47 65 6e 65 72 61 74 65 64 20 51 75 |TDX Generated Qu| 00000010 6f 74 65 00 00 00 00 00 00 00 00 00 00 00 00 00 |ote.............| 00000020 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|

00004000

This is the kernel instantiating a TDX Quote without the TDREPORT implementation detail ever leaving the kernel. Now, this is only the

IIUC, the idea here is to cache the quote data and return it to the user whenever possible, right? If yes, I think such optimization may not be very useful for our case. AFAIK, the quote data will change whenever there is a change in the guest measurement data. Since the validity of the generated quote will not be long, and the frequency of quote generation requests is expected to be less, we may not get much benefit from caching the quote data. I think we can keep this logic simple by directly retrieving the quote data from the quoting enclave whenever there is a request from the user.

top-half of what is needed. The missing bottom half takes that material and uses it to instantiate derived key material like the storage decryption key internal to the kernel. See "The Process" in Documentation/security/keys/request-key.rst for how the Keys subsystem handles the "keys for keys" use case.

This is only useful for key-server use case, right? Attestation can also be used for use cases like pattern matching or uploading some secure data, etc. Since key-server is not the only use case, does it make sense to suppport this derived key feature?

---

diff --git a/drivers/virt/Kconfig b/drivers/virt/Kconfig index f79ab13a5c28..0f775847028e 100644 --- a/drivers/virt/Kconfig +++ b/drivers/virt/Kconfig @@ -54,4 +54,8 @@ source "drivers/virt/coco/sev-guest/Kconfig" source "drivers/virt/coco/tdx-guest/Kconfig" +config GUEST_ATTEST

• tristate
• select KEYS

endif diff --git a/drivers/virt/Makefile b/drivers/virt/Makefile index e9aa6fc96fab..66f6b838f8f4 100644 --- a/drivers/virt/Makefile +++ b/drivers/virt/Makefile @@ -12,3 +12,4 @@ obj-$(CONFIG_ACRN_HSM) += acrn/ obj-$(CONFIG_EFI_SECRET) += coco/efi_secret/ obj-$(CONFIG_SEV_GUEST) += coco/sev-guest/ obj-$(CONFIG_INTEL_TDX_GUEST) += coco/tdx-guest/ +obj-$(CONFIG_GUEST_ATTEST) += coco/guest-attest/ diff --git a/drivers/virt/coco/guest-attest/Makefile b/drivers/virt/coco/guest-attest/Makefile new file mode 100644 index 000000000000..5581c5a27588 --- /dev/null +++ b/drivers/virt/coco/guest-attest/Makefile @@ -0,0 +1,2 @@ +obj-$(CONFIG_GUEST_ATTEST) += guest_attest.o +guest_attest-y := key.o diff --git a/drivers/virt/coco/guest-attest/key.c b/drivers/virt/coco/guest-attest/key.c new file mode 100644 index 000000000000..2a494b6dd7a7 --- /dev/null +++ b/drivers/virt/coco/guest-attest/key.c @@ -0,0 +1,159 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* Copyright(c) 2023 Intel Corporation. All rights reserved. */

+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#include <linux/seq_file.h> +#include <linux/key-type.h> +#include <linux/module.h> +#include <linux/base64.h>

+#include <keys/request_key_auth-type.h> +#include <keys/user-type.h>

+#include "guest-attest.h"

Can you share you guest-attest.h?

+static LIST_HEAD(guest_attest_list); +static DECLARE_RWSEM(guest_attest_rwsem);

+static struct guest_attest_ops *fetch_ops(void) +{

• return list_first_entry_or_null(&guest_attest_list,

• 			struct guest_attest_ops, list);
  

+}

+static struct guest_attest_ops *get_ops(void) +{

• down_read(&guest_attest_rwsem);
• return fetch_ops();

+}

+static void put_ops(void) +{

• up_read(&guest_attest_rwsem);

+}

+int register_guest_attest_ops(struct guest_attest_ops *ops) +{

• struct guest_attest_ops *conflict;
• int rc;
• down_write(&guest_attest_rwsem);
• conflict = fetch_ops();
• if (conflict) {

• pr_err("\"%s\" ops already registered\n", conflict->name);
  

• rc = -EEXIST;
  

• goto out;
  

• }
• list_add(&ops->list, &guest_attest_list);
• try_module_get(ops->module);
• rc = 0;

+out:

• up_write(&guest_attest_rwsem);
• return rc;

+} +EXPORT_SYMBOL_GPL(register_guest_attest_ops);

+void unregister_guest_attest_ops(struct guest_attest_ops *ops) +{

• down_write(&guest_attest_rwsem);
• list_del(&ops->list);
• up_write(&guest_attest_rwsem);
• module_put(ops->module);

+} +EXPORT_SYMBOL_GPL(unregister_guest_attest_ops);

+static int __guest_attest_request_key(struct key *key, int level,

• 		      struct key *dest_keyring,
  

• 		      const char *callout_info, int callout_len,
  

• 		      struct key *authkey)
  

+{

• struct guest_attest_ops *ops;
• void *payload = NULL;
• int rc, payload_len;
• ops = get_ops();
• if (!ops)

• return -ENOKEY;
  

• payload = kzalloc(max(GUEST_ATTEST_DATALEN, callout_len), GFP_KERNEL);
• if (!payload) {

• rc = -ENOMEM;
  

• goto out;
  

• }

Is the idea to get the values like vmpl part of the payload?

• payload_len = base64_decode(callout_info, callout_len, payload);
• if (payload_len < 0 || payload_len > GUEST_ATTEST_DATALEN) {

• rc = -EINVAL;
  

• goto out;
  

• }
• rc = ops->request_attest(key, level, dest_keyring, payload, payload_len,

• 		 authkey);
  

+out:

• kfree(payload);
• put_ops();
• return rc;

+}

+static int guest_attest_request_key(struct key *authkey, void *data) +{

• struct request_key_auth *rka = get_request_key_auth(authkey);
• struct key *key = rka->target_key;
• unsigned long long id;
• int rc, level;
• pr_debug("desc: %s op: %s callout: %s\n", key->description, rka->op,

•  rka->callout_info ? (char *)rka->callout_info : "\"none\"");
  

• if (sscanf(key->description, "guest_attest:%d:%llu", &level, &id) != 2)

• return -EINVAL;
  

Can you explain some details about the id and level? It is not very clear why we need it.

• if (!rka->callout_info) {

• rc = -EINVAL;
  

• goto out;
  

• }
• rc = __guest_attest_request_key(key, level, rka->dest_keyring,

• 			rka->callout_info, rka->callout_len,
  

• 			authkey);
  

+out:

• complete_request_key(authkey, rc);
• return rc;

+}

+static int guest_attest_vet_description(const char *desc) +{

• unsigned long long id;
• int level;
• if (sscanf(desc, "guest_attest:%d:%llu", &level, &id) != 2)

• return -EINVAL;
  

• return 0;

+}

+static struct key_type key_type_guest_attest = {

• .name = "guest_attest",
• .preparse = user_preparse,
• .free_preparse = user_free_preparse,
• .instantiate = generic_key_instantiate,
• .revoke = user_revoke,
• .destroy = user_destroy,
• .describe = user_describe,
• .read = user_read,
• .vet_description = guest_attest_vet_description,
• .request_key = guest_attest_request_key,

+};

+static int __init guest_attest_init(void) +{

• return register_key_type(&key_type_guest_attest);

+}

+static void __exit guest_attest_exit(void) +{

• unregister_key_type(&key_type_guest_attest);

+}

+module_init(guest_attest_init); +module_exit(guest_attest_exit); +MODULE_LICENSE("GPL v2"); diff --git a/drivers/virt/coco/tdx-guest/Kconfig b/drivers/virt/coco/tdx-guest/Kconfig index 14246fc2fb02..9a1ec85369fe 100644 --- a/drivers/virt/coco/tdx-guest/Kconfig +++ b/drivers/virt/coco/tdx-guest/Kconfig @@ -1,6 +1,7 @@ config TDX_GUEST_DRIVER tristate "TDX Guest driver" depends on INTEL_TDX_GUEST

• select GUEST_ATTEST help The driver provides userspace interface to communicate with the TDX module to request the TDX guest details like attestation

diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c b/drivers/virt/coco/tdx-guest/tdx-guest.c index 388491fa63a1..65b5aab284d9 100644 --- a/drivers/virt/coco/tdx-guest/tdx-guest.c +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c @@ -13,11 +13,13 @@ #include <linux/string.h> #include <linux/uaccess.h> #include <linux/set_memory.h> +#include <linux/key-type.h> #include <uapi/linux/tdx-guest.h> #include <asm/cpu_device_id.h> #include <asm/tdx.h> +#include "../guest-attest/guest-attest.h" /*

• Intel's SGX QE implementation generally uses Quote size less

@@ -229,6 +231,62 @@ static const struct x86_cpu_id tdx_guest_ids[] = { }; MODULE_DEVICE_TABLE(x86cpu, tdx_guest_ids); +static int tdx_request_attest(struct key *key, int level,

• 	      struct key *dest_keyring, void *payload,
  

• 	      int payload_len, struct key *authkey)
  

+{

• u8 *tdreport;
• long ret;
• tdreport = kzalloc(TDX_REPORT_LEN, GFP_KERNEL);
• if (!tdreport)

• return -ENOMEM;
  

• /* Generate TDREPORT0 using "TDG.MR.REPORT" TDCALL */
• ret = tdx_mcall_get_report0(payload, tdreport);
• if (ret)

• goto out;
  

• mutex_lock(&quote_lock);
• memset(qentry->buf, 0, qentry->buf_len);
• reinit_completion(&qentry->compl);
• qentry->valid = true;
• /* Submit GetQuote Request using GetQuote hyperetall */
• ret = tdx_hcall_get_quote(qentry->buf, qentry->buf_len);
• if (ret) {

• pr_err("GetQuote hyperetall failed, status:%lx\n", ret);
  

• ret = -EIO;
  

• goto quote_failed;
  

• }
• /*

• * Although the GHCI specification does not state explicitly that
  

• * the VMM must not wait indefinitely for the Quote request to be
  

• * completed, a sane VMM should always notify the guest after a
  

• * certain time, regardless of whether the Quote generation is
  

• * successful or not.  For now just assume the VMM will do so.
  

• */
  

• wait_for_completion(&qentry->compl);
• ret = key_instantiate_and_link(key, qentry->buf, qentry->buf_len,

• 		       dest_keyring, authkey);
  

+quote_failed:

• qentry->valid = false;
• mutex_unlock(&quote_lock);

+out:

• kfree(tdreport);
• return ret;

+}

+static struct guest_attest_ops tdx_attest_ops = {

• .name = KBUILD_MODNAME,
• .module = THIS_MODULE,
• .request_attest = tdx_request_attest,

+};

static int __init tdx_guest_init(void) { int ret; @@ -251,8 +309,14 @@ static int __init tdx_guest_init(void) if (ret) goto free_quote;

• ret = register_guest_attest_ops(&tdx_attest_ops);
• if (ret)

• goto free_irq;
  

• return 0;

+free_irq:

• tdx_unregister_event_irq_cb(quote_cb_handler, qentry);

free_quote: free_quote_entry(qentry); free_misc: @@ -264,6 +328,7 @@ module_init(tdx_guest_init); static void __exit tdx_guest_exit(void) {

• unregister_guest_attest_ops(&tdx_attest_ops); tdx_unregister_event_irq_cb(quote_cb_handler, qentry); free_quote_entry(qentry); misc_deregister(&tdx_misc_dev);

+Atish

Atish, any comments on this topic from RISC-v?

On 6/26/23 11:57 AM, Dionna Amalie Glaze wrote:

On Sun, Jun 25, 2023 at 8:06 PM Sathyanarayanan Kuppuswamy sathyanarayanan.kuppuswamy@linux.intel.com wrote:

...

Hi Dan,

On 6/23/23 3:27 PM, Dan Williams wrote:

...

Dan Williams wrote:

...

[ add David, Brijesh, and Atish]

Kuppuswamy Sathyanarayanan wrote:

...

In TDX guest, the second stage of the attestation process is Quote generation. This process is required to convert the locally generated TDREPORT into a remotely verifiable Quote. It involves sending the TDREPORT data to a Quoting Enclave (QE) which will verify the integrity of the TDREPORT and sign it with an attestation key.

Intel's TDX attestation driver exposes TDX_CMD_GET_QUOTE IOCTL to allow the user agent to get the TD Quote.

Add a kernel selftest module to verify the Quote generation feature.

TD Quote generation involves following steps:

• Get the TDREPORT data using TDX_CMD_GET_REPORT IOCTL.
• Embed the TDREPORT data in quote buffer and request for quote generation via TDX_CMD_GET_QUOTE IOCTL request.
• Upon completion of the GetQuote request, check for non zero value in the status field of Quote header to make sure the generated quote is valid.

What this cover letter does not say is that this is adding another instance of the similar pattern as SNP_GET_REPORT.

Linux is best served when multiple vendors trying to do similar operations are brought together behind a common ABI. We see this in the history of wrangling SCSI vendors behind common interfaces. Now multiple confidential computing vendors trying to develop similar flows with differentiated formats where that differentiation need not leak over the ABI boundary.

[..]

Below is a rough mock up of this approach to demonstrate the direction. Again, the goal is to define an ABI that can support any vendor's arch-specific attestation method and key provisioning flows without leaking vendor-specific details, or confidential material over the user/kernel ABI.

Thanks for working on this mock code and helping out. It gives me the general idea about your proposal.

...

The observation is that there are a sufficient number of attestation flows available to review where Linux can define a superset ABI to contain them all. The other observation is that the implementations have features that may cross-polinate over time. For example the SEV privelege level consideration ("vmpl"), and the TDX RTMR (think TPM PCRs) mechanisms address generic Confidential Computing use cases.

I agree with your point about VMPL and RTMR feature cases. This observation is valid for AMD SEV and TDX attestation flows. But I am not sure whether it will hold true for other vendor implementations. Our sample set is not good enough to make this conclusion. The reason for my concern is, if you check the ABI interface used in the S390 arch attestation driver (drivers/s390/char/uvdevice.c), you would notice that there is a significant difference between the ABI used in that driver and SEV/TDX drivers. The S390 driver attestation request appears to accept two data blobs as input, as well as a variety of vendor-specific header configurations.

Maybe the s390 attestation model is a special case, but, I think we consider this issue. Since we don't have a common spec, there is chance that any superset ABI we define now may not meet future vendor requirements. One way to handle it to leave enough space in the generic ABI to handle future vendor requirements.

I think it would be better if other vendors (like ARM or RISC) can comment and confirm whether this proposal meets their demands.

The VMPL-based separation that will house the supervisor module known as SVSM can have protocols that implement a TPM command interface, or an RTMR-extension interface, and will also need to have an SVSM-specific protocol attestation report format to keep the secure chain of custody apparent. We'd have different formats and protocols in the kernel, at least, to speak to each technology. I'm not sure it's worth the trouble of papering over all the... 3-4 technologies with similar but still weirdly different formats and ways of doing things with an abstracted attestation ABI, especially since the output all has to be interpreted in an architecture-specific way anyway.

ARM's Confidential Computing Realm Management Extensions (RME) seems to be going along the lines of a runtime measurement register model with their hardware enforced security. The number of registers isn't prescribed in the spec.

+Joey Gouly +linux-coco@lists.linux.dev as far as RME is concerned, do you know who would be best to weigh in on this discussion of a unified attestation model?

...

...

Vendor specific ioctls for all of this feels like surrender when Linux already has the keys subsystem which has plenty of degrees of freedom for tracking blobs with signatures and using those blobs to instantiate other blobs. It already serves as the ABI wrapping various TPM implementations and marshaling keys for storage encryption and other use cases that intersect Confidential Computing.

The benefit of deprecating vendor-specific abstraction layers in userspace is secondary. The primary benefit is collaboration. It enables kernel developers from various architectures to collaborate on common infrastructure. If, referring back to my previous example, SEV adopts an RTMR-like mechanism and TDX adopts a vmpl-like mechanism it would be unfortunate if those efforts were siloed, duplicated, and needlessly differentiated to userspace. So while there are arguably a manageable number of basic arch attestation methods the planned expansion of those to build incremental functionality is where I believe we, as a community, will be glad that we invested in a "Linux format" for all of this.

An example, to show what the strawman patch below enables: (req_key is the sample program from "man 2 request_key")

# ./req_key guest_attest guest_attest:0:0-$desc $(cat user_data | base64) Key ID is 10e2f3a7 # keyctl pipe 0x10e2f3a7 | hexdump -C 00000000 54 44 58 20 47 65 6e 65 72 61 74 65 64 20 51 75 |TDX Generated Qu| 00000010 6f 74 65 00 00 00 00 00 00 00 00 00 00 00 00 00 |ote.............| 00000020 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|

00004000

This is the kernel instantiating a TDX Quote without the TDREPORT implementation detail ever leaving the kernel. Now, this is only the

IIUC, the idea here is to cache the quote data and return it to the user whenever possible, right? If yes, I think such optimization may not be very useful for our case. AFAIK, the quote data will change whenever there is a change in the guest measurement data. Since the validity of the generated quote will not be long, and the frequency of quote generation requests is expected to be less, we may not get much benefit from caching the quote data. I think we can keep this logic simple by directly retrieving the quote data from the quoting enclave whenever there is a request from the user.

...

top-half of what is needed. The missing bottom half takes that material and uses it to instantiate derived key material like the storage decryption key internal to the kernel. See "The Process" in Documentation/security/keys/request-key.rst for how the Keys subsystem handles the "keys for keys" use case.

This is only useful for key-server use case, right? Attestation can also be used for use cases like pattern matching or uploading some secure data, etc. Since key-server is not the only use case, does it make sense to suppport this derived key feature?

...

---

diff --git a/drivers/virt/Kconfig b/drivers/virt/Kconfig index f79ab13a5c28..0f775847028e 100644 --- a/drivers/virt/Kconfig +++ b/drivers/virt/Kconfig @@ -54,4 +54,8 @@ source "drivers/virt/coco/sev-guest/Kconfig"

source "drivers/virt/coco/tdx-guest/Kconfig"

+config GUEST_ATTEST

• tristate
  

• select KEYS
  

endif diff --git a/drivers/virt/Makefile b/drivers/virt/Makefile index e9aa6fc96fab..66f6b838f8f4 100644 --- a/drivers/virt/Makefile +++ b/drivers/virt/Makefile @@ -12,3 +12,4 @@ obj-$(CONFIG_ACRN_HSM) += acrn/ obj-$(CONFIG_EFI_SECRET) += coco/efi_secret/ obj-$(CONFIG_SEV_GUEST) += coco/sev-guest/ obj-$(CONFIG_INTEL_TDX_GUEST) += coco/tdx-guest/ +obj-$(CONFIG_GUEST_ATTEST) += coco/guest-attest/ diff --git a/drivers/virt/coco/guest-attest/Makefile b/drivers/virt/coco/guest-attest/Makefile new file mode 100644 index 000000000000..5581c5a27588 --- /dev/null +++ b/drivers/virt/coco/guest-attest/Makefile @@ -0,0 +1,2 @@ +obj-$(CONFIG_GUEST_ATTEST) += guest_attest.o +guest_attest-y := key.o diff --git a/drivers/virt/coco/guest-attest/key.c b/drivers/virt/coco/guest-attest/key.c new file mode 100644 index 000000000000..2a494b6dd7a7 --- /dev/null +++ b/drivers/virt/coco/guest-attest/key.c @@ -0,0 +1,159 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* Copyright(c) 2023 Intel Corporation. All rights reserved. */

+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#include <linux/seq_file.h> +#include <linux/key-type.h> +#include <linux/module.h> +#include <linux/base64.h>

+#include <keys/request_key_auth-type.h> +#include <keys/user-type.h>

+#include "guest-attest.h"

Can you share you guest-attest.h?

...

+static LIST_HEAD(guest_attest_list); +static DECLARE_RWSEM(guest_attest_rwsem);

+static struct guest_attest_ops *fetch_ops(void) +{

• return list_first_entry_or_null(&guest_attest_list,
  

•                                 struct guest_attest_ops, list);
  

+}

+static struct guest_attest_ops *get_ops(void) +{

• down_read(&guest_attest_rwsem);
  

• return fetch_ops();
  

+}

+static void put_ops(void) +{

• up_read(&guest_attest_rwsem);
  

+}

+int register_guest_attest_ops(struct guest_attest_ops *ops) +{

• struct guest_attest_ops *conflict;
  

• int rc;
  

• down_write(&guest_attest_rwsem);
  

• conflict = fetch_ops();
  

• if (conflict) {
  

•         pr_err("\"%s\" ops already registered\n", conflict->name);
  

•         rc = -EEXIST;
  

•         goto out;
  

• }
  

• list_add(&ops->list, &guest_attest_list);
  

• try_module_get(ops->module);
  

• rc = 0;
  

+out:

• up_write(&guest_attest_rwsem);
  

• return rc;
  

+} +EXPORT_SYMBOL_GPL(register_guest_attest_ops);

+void unregister_guest_attest_ops(struct guest_attest_ops *ops) +{

• down_write(&guest_attest_rwsem);
  

• list_del(&ops->list);
  

• up_write(&guest_attest_rwsem);
  

• module_put(ops->module);
  

+} +EXPORT_SYMBOL_GPL(unregister_guest_attest_ops);

+static int __guest_attest_request_key(struct key *key, int level,

•                               struct key *dest_keyring,
  

•                               const char *callout_info, int callout_len,
  

•                               struct key *authkey)
  

+{

• struct guest_attest_ops *ops;
  

• void *payload = NULL;
  

• int rc, payload_len;
  

• ops = get_ops();
  

• if (!ops)
  

•         return -ENOKEY;
  

• payload = kzalloc(max(GUEST_ATTEST_DATALEN, callout_len), GFP_KERNEL);
  

• if (!payload) {
  

•         rc = -ENOMEM;
  

•         goto out;
  

• }
  

Is the idea to get the values like vmpl part of the payload?

...

• payload_len = base64_decode(callout_info, callout_len, payload);
  

• if (payload_len < 0 || payload_len > GUEST_ATTEST_DATALEN) {
  

•         rc = -EINVAL;
  

•         goto out;
  

• }
  

• rc = ops->request_attest(key, level, dest_keyring, payload, payload_len,
  

•                          authkey);
  

+out:

• kfree(payload);
  

• put_ops();
  

• return rc;
  

+}

+static int guest_attest_request_key(struct key *authkey, void *data) +{

• struct request_key_auth *rka = get_request_key_auth(authkey);
  

• struct key *key = rka->target_key;
  

• unsigned long long id;
  

• int rc, level;
  

• pr_debug("desc: %s op: %s callout: %s\n", key->description, rka->op,
  

•          rka->callout_info ? (char *)rka->callout_info : "\"none\"");
  

• if (sscanf(key->description, "guest_attest:%d:%llu", &level, &id) != 2)
  

•         return -EINVAL;
  

Can you explain some details about the id and level? It is not very clear why we need it.

...

• if (!rka->callout_info) {
  

•         rc = -EINVAL;
  

•         goto out;
  

• }
  

• rc = __guest_attest_request_key(key, level, rka->dest_keyring,
  

•                                 rka->callout_info, rka->callout_len,
  

•                                 authkey);
  

+out:

• complete_request_key(authkey, rc);
  

• return rc;
  

+}

+static int guest_attest_vet_description(const char *desc) +{

• unsigned long long id;
  

• int level;
  

• if (sscanf(desc, "guest_attest:%d:%llu", &level, &id) != 2)
  

•         return -EINVAL;
  

• return 0;
  

+}

+static struct key_type key_type_guest_attest = {

• .name = "guest_attest",
  

• .preparse = user_preparse,
  

• .free_preparse = user_free_preparse,
  

• .instantiate = generic_key_instantiate,
  

• .revoke = user_revoke,
  

• .destroy = user_destroy,
  

• .describe = user_describe,
  

• .read = user_read,
  

• .vet_description = guest_attest_vet_description,
  

• .request_key = guest_attest_request_key,
  

+};

+static int __init guest_attest_init(void) +{

• return register_key_type(&key_type_guest_attest);
  

+}

+static void __exit guest_attest_exit(void) +{

• unregister_key_type(&key_type_guest_attest);
  

+}

+module_init(guest_attest_init); +module_exit(guest_attest_exit); +MODULE_LICENSE("GPL v2"); diff --git a/drivers/virt/coco/tdx-guest/Kconfig b/drivers/virt/coco/tdx-guest/Kconfig index 14246fc2fb02..9a1ec85369fe 100644 --- a/drivers/virt/coco/tdx-guest/Kconfig +++ b/drivers/virt/coco/tdx-guest/Kconfig @@ -1,6 +1,7 @@ config TDX_GUEST_DRIVER tristate "TDX Guest driver" depends on INTEL_TDX_GUEST

• select GUEST_ATTEST
  help
    The driver provides userspace interface to communicate with
    the TDX module to request the TDX guest details like attestation
  

diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c b/drivers/virt/coco/tdx-guest/tdx-guest.c index 388491fa63a1..65b5aab284d9 100644 --- a/drivers/virt/coco/tdx-guest/tdx-guest.c +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c @@ -13,11 +13,13 @@ #include <linux/string.h> #include <linux/uaccess.h> #include <linux/set_memory.h> +#include <linux/key-type.h>

#include <uapi/linux/tdx-guest.h>

#include <asm/cpu_device_id.h> #include <asm/tdx.h> +#include "../guest-attest/guest-attest.h"

/*

• Intel's SGX QE implementation generally uses Quote size less

@@ -229,6 +231,62 @@ static const struct x86_cpu_id tdx_guest_ids[] = { }; MODULE_DEVICE_TABLE(x86cpu, tdx_guest_ids);

+static int tdx_request_attest(struct key *key, int level,

•                       struct key *dest_keyring, void *payload,
  

•                       int payload_len, struct key *authkey)
  

+{

• u8 *tdreport;
  

• long ret;
  

• tdreport = kzalloc(TDX_REPORT_LEN, GFP_KERNEL);
  

• if (!tdreport)
  

•         return -ENOMEM;
  

• /* Generate TDREPORT0 using "TDG.MR.REPORT" TDCALL */
  

• ret = tdx_mcall_get_report0(payload, tdreport);
  

• if (ret)
  

•         goto out;
  

• mutex_lock(&quote_lock);
  

• memset(qentry->buf, 0, qentry->buf_len);
  

• reinit_completion(&qentry->compl);
  

• qentry->valid = true;
  

• /* Submit GetQuote Request using GetQuote hyperetall */
  

• ret = tdx_hcall_get_quote(qentry->buf, qentry->buf_len);
  

• if (ret) {
  

•         pr_err("GetQuote hyperetall failed, status:%lx\n", ret);
  

•         ret = -EIO;
  

•         goto quote_failed;
  

• }
  

• /*
  

•  * Although the GHCI specification does not state explicitly that
  

•  * the VMM must not wait indefinitely for the Quote request to be
  

•  * completed, a sane VMM should always notify the guest after a
  

•  * certain time, regardless of whether the Quote generation is
  

•  * successful or not.  For now just assume the VMM will do so.
  

•  */
  

• wait_for_completion(&qentry->compl);
  

• ret = key_instantiate_and_link(key, qentry->buf, qentry->buf_len,
  

•                                dest_keyring, authkey);
  

+quote_failed:

• qentry->valid = false;
  

• mutex_unlock(&quote_lock);
  

+out:

• kfree(tdreport);
  

• return ret;
  

+}

+static struct guest_attest_ops tdx_attest_ops = {

• .name = KBUILD_MODNAME,
  

• .module = THIS_MODULE,
  

• .request_attest = tdx_request_attest,
  

+};

static int __init tdx_guest_init(void) { int ret; @@ -251,8 +309,14 @@ static int __init tdx_guest_init(void) if (ret) goto free_quote;

• ret = register_guest_attest_ops(&tdx_attest_ops);
  

• if (ret)
  

•         goto free_irq;
  

• return 0;
  
  

+free_irq:

• tdx_unregister_event_irq_cb(quote_cb_handler, qentry);
  

free_quote: free_quote_entry(qentry); free_misc: @@ -264,6 +328,7 @@ module_init(tdx_guest_init);

static void __exit tdx_guest_exit(void) {

• unregister_guest_attest_ops(&tdx_attest_ops);
  tdx_unregister_event_irq_cb(quote_cb_handler, qentry);
  free_quote_entry(qentry);
  misc_deregister(&tdx_misc_dev);
  

-- Sathyanarayanan Kuppuswamy Linux Kernel Developer

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

Sathyanarayanan Kuppuswamy wrote:

Hi Dan,

On 6/23/23 3:27 PM, Dan Williams wrote:

...

Dan Williams wrote:

...

[ add David, Brijesh, and Atish]

Kuppuswamy Sathyanarayanan wrote:

...

In TDX guest, the second stage of the attestation process is Quote generation. This process is required to convert the locally generated TDREPORT into a remotely verifiable Quote. It involves sending the TDREPORT data to a Quoting Enclave (QE) which will verify the integrity of the TDREPORT and sign it with an attestation key.

Intel's TDX attestation driver exposes TDX_CMD_GET_QUOTE IOCTL to allow the user agent to get the TD Quote.

Add a kernel selftest module to verify the Quote generation feature.

TD Quote generation involves following steps:

• Get the TDREPORT data using TDX_CMD_GET_REPORT IOCTL.
• Embed the TDREPORT data in quote buffer and request for quote generation via TDX_CMD_GET_QUOTE IOCTL request.
• Upon completion of the GetQuote request, check for non zero value in the status field of Quote header to make sure the generated quote is valid.

What this cover letter does not say is that this is adding another instance of the similar pattern as SNP_GET_REPORT.

Linux is best served when multiple vendors trying to do similar operations are brought together behind a common ABI. We see this in the history of wrangling SCSI vendors behind common interfaces. Now multiple confidential computing vendors trying to develop similar flows with differentiated formats where that differentiation need not leak over the ABI boundary.

[..]

Below is a rough mock up of this approach to demonstrate the direction. Again, the goal is to define an ABI that can support any vendor's arch-specific attestation method and key provisioning flows without leaking vendor-specific details, or confidential material over the user/kernel ABI.

Thanks for working on this mock code and helping out. It gives me the general idea about your proposal.

...

The observation is that there are a sufficient number of attestation flows available to review where Linux can define a superset ABI to contain them all. The other observation is that the implementations have features that may cross-polinate over time. For example the SEV privelege level consideration ("vmpl"), and the TDX RTMR (think TPM PCRs) mechanisms address generic Confidential Computing use cases.

I agree with your point about VMPL and RTMR feature cases. This observation is valid for AMD SEV and TDX attestation flows. But I am not sure whether it will hold true for other vendor implementations. Our sample set is not good enough to make this conclusion. The reason for my concern is, if you check the ABI interface used in the S390 arch attestation driver (drivers/s390/char/uvdevice.c), you would notice that there is a significant difference between the ABI used in that driver and SEV/TDX drivers. The S390 driver attestation request appears to accept two data blobs as input, as well as a variety of vendor-specific header configurations.

I would need more time to investigate. It's also the case that if both major x86 vendors plus ARM and/or RISC-V can all get behind the same frontend then that is already success in my mind.

Maybe the s390 attestation model is a special case, but, I think we consider this issue. Since we don't have a common spec, there is chance that any superset ABI we define now may not meet future vendor requirements. One way to handle it to leave enough space in the generic ABI to handle future vendor requirements.

Perhaps, but the goal here is to clearly indicate "this is how Linux conveys confidential computing attestation concepts". If there is future vendor innovation in this space it needs to consider how it meets the established needs of Linux, not the other way round.

I think it would be better if other vendors (like ARM or RISC) can comment and confirm whether this proposal meets their demands.

The more participation the better. Open source definitely involves a component speaking up when the definition of things are still malleable, or catching issues before they go upstream.

...

Vendor specific ioctls for all of this feels like surrender when Linux already has the keys subsystem which has plenty of degrees of freedom for tracking blobs with signatures and using those blobs to instantiate other blobs. It already serves as the ABI wrapping various TPM implementations and marshaling keys for storage encryption and other use cases that intersect Confidential Computing.

The benefit of deprecating vendor-specific abstraction layers in userspace is secondary. The primary benefit is collaboration. It enables kernel developers from various architectures to collaborate on common infrastructure. If, referring back to my previous example, SEV adopts an RTMR-like mechanism and TDX adopts a vmpl-like mechanism it would be unfortunate if those efforts were siloed, duplicated, and needlessly differentiated to userspace. So while there are arguably a manageable number of basic arch attestation methods the planned expansion of those to build incremental functionality is where I believe we, as a community, will be glad that we invested in a "Linux format" for all of this.

An example, to show what the strawman patch below enables: (req_key is the sample program from "man 2 request_key")

# ./req_key guest_attest guest_attest:0:0-$desc $(cat user_data | base64) Key ID is 10e2f3a7 # keyctl pipe 0x10e2f3a7 | hexdump -C 00000000 54 44 58 20 47 65 6e 65 72 61 74 65 64 20 51 75 |TDX Generated Qu| 00000010 6f 74 65 00 00 00 00 00 00 00 00 00 00 00 00 00 |ote.............| 00000020 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|

00004000

This is the kernel instantiating a TDX Quote without the TDREPORT implementation detail ever leaving the kernel. Now, this is only the

IIUC, the idea here is to cache the quote data and return it to the user whenever possible, right? If yes, I think such optimization may not be very useful for our case. AFAIK, the quote data will change whenever there is a change in the guest measurement data. Since the validity of the generated quote will not be long, and the frequency of quote generation requests is expected to be less, we may not get much benefit from caching the quote data. I think we can keep this logic simple by directly retrieving the quote data from the quoting enclave whenever there is a request from the user.

The Keys subsystem already supports the concept of keys that expire immediately, so no need for special consideration here that I can see.

...

top-half of what is needed. The missing bottom half takes that material and uses it to instantiate derived key material like the storage decryption key internal to the kernel. See "The Process" in Documentation/security/keys/request-key.rst for how the Keys subsystem handles the "keys for keys" use case.

This is only useful for key-server use case, right? Attestation can also be used for use cases like pattern matching or uploading some secure data, etc. Since key-server is not the only use case, does it make sense to suppport this derived key feature?

The Keys subsystem is just a way for both userspace and kernel space to request the instantiation of blobs that mediate access to another resource be it another key or something else. So key-server is only one example client.

The other reason for defining a common frontend is so the kernel can understand and mediate resource access as a kernel is wont to do. The ioctl() approach blinds the kernel and requires userspace to repeatedly solve problems in vendor specific ways. They Keys proposal also has the property of not requiring userspace round trips for things like unlocking storage. The driver can just do request_keys() and kick off the process on its own.

...

---

diff --git a/drivers/virt/Kconfig b/drivers/virt/Kconfig index f79ab13a5c28..0f775847028e 100644 --- a/drivers/virt/Kconfig +++ b/drivers/virt/Kconfig @@ -54,4 +54,8 @@ source "drivers/virt/coco/sev-guest/Kconfig" source "drivers/virt/coco/tdx-guest/Kconfig" +config GUEST_ATTEST

• tristate
• select KEYS

endif diff --git a/drivers/virt/Makefile b/drivers/virt/Makefile index e9aa6fc96fab..66f6b838f8f4 100644 --- a/drivers/virt/Makefile +++ b/drivers/virt/Makefile @@ -12,3 +12,4 @@ obj-$(CONFIG_ACRN_HSM) += acrn/ obj-$(CONFIG_EFI_SECRET) += coco/efi_secret/ obj-$(CONFIG_SEV_GUEST) += coco/sev-guest/ obj-$(CONFIG_INTEL_TDX_GUEST) += coco/tdx-guest/ +obj-$(CONFIG_GUEST_ATTEST) += coco/guest-attest/ diff --git a/drivers/virt/coco/guest-attest/Makefile b/drivers/virt/coco/guest-attest/Makefile new file mode 100644 index 000000000000..5581c5a27588 --- /dev/null +++ b/drivers/virt/coco/guest-attest/Makefile @@ -0,0 +1,2 @@ +obj-$(CONFIG_GUEST_ATTEST) += guest_attest.o +guest_attest-y := key.o diff --git a/drivers/virt/coco/guest-attest/key.c b/drivers/virt/coco/guest-attest/key.c new file mode 100644 index 000000000000..2a494b6dd7a7 --- /dev/null +++ b/drivers/virt/coco/guest-attest/key.c @@ -0,0 +1,159 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* Copyright(c) 2023 Intel Corporation. All rights reserved. */

+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#include <linux/seq_file.h> +#include <linux/key-type.h> +#include <linux/module.h> +#include <linux/base64.h>

+#include <keys/request_key_auth-type.h> +#include <keys/user-type.h>

+#include "guest-attest.h"

Can you share you guest-attest.h?

Apologies, missed a 'git add':

/* SPDX-License-Identifier: GPL-2.0-only */ /* Copyright(c) 2023 Intel Corporation. */

#ifndef __GUEST_ATTEST_H__ #define __GUEST_ATTEST_H__ #include <linux/list.h>

/* * arch specific ops, only one is expected to be registered at a time * i.e. either SEV or TDX, but not both */ struct guest_attest_ops { const char *name; struct module *module; struct list_head list; int (*request_attest)(struct key *key, int level, struct key *dest_keyring, void *payload, int payload_len, struct key *authkey); };

#define GUEST_ATTEST_DATALEN 64

int register_guest_attest_ops(struct guest_attest_ops *ops); void unregister_guest_attest_ops(struct guest_attest_ops *ops);

#endif /*__GUEST_ATTEST_H__ */

...

+static LIST_HEAD(guest_attest_list); +static DECLARE_RWSEM(guest_attest_rwsem);

+static struct guest_attest_ops *fetch_ops(void) +{

• return list_first_entry_or_null(&guest_attest_list,

• 			struct guest_attest_ops, list);
  

+}

+static struct guest_attest_ops *get_ops(void) +{

• down_read(&guest_attest_rwsem);
• return fetch_ops();

+}

+static void put_ops(void) +{

• up_read(&guest_attest_rwsem);

+}

+int register_guest_attest_ops(struct guest_attest_ops *ops) +{

• struct guest_attest_ops *conflict;
• int rc;
• down_write(&guest_attest_rwsem);
• conflict = fetch_ops();
• if (conflict) {

• pr_err("\"%s\" ops already registered\n", conflict->name);
  

• rc = -EEXIST;
  

• goto out;
  

• }
• list_add(&ops->list, &guest_attest_list);
• try_module_get(ops->module);
• rc = 0;

+out:

• up_write(&guest_attest_rwsem);
• return rc;

+} +EXPORT_SYMBOL_GPL(register_guest_attest_ops);

+void unregister_guest_attest_ops(struct guest_attest_ops *ops) +{

• down_write(&guest_attest_rwsem);
• list_del(&ops->list);
• up_write(&guest_attest_rwsem);
• module_put(ops->module);

+} +EXPORT_SYMBOL_GPL(unregister_guest_attest_ops);

+static int __guest_attest_request_key(struct key *key, int level,

• 		      struct key *dest_keyring,
  

• 		      const char *callout_info, int callout_len,
  

• 		      struct key *authkey)
  

+{

• struct guest_attest_ops *ops;
• void *payload = NULL;
• int rc, payload_len;
• ops = get_ops();
• if (!ops)

• return -ENOKEY;
  

• payload = kzalloc(max(GUEST_ATTEST_DATALEN, callout_len), GFP_KERNEL);
• if (!payload) {

• rc = -ENOMEM;
  

• goto out;
  

• }

Is the idea to get the values like vmpl part of the payload?

No, to me vmpl likely needs to be conveyed in the key-description. Payload is simply the 64-bytes that both SEV and TDX take as input for the attestation request. The AMD specification seems to imply that payload is itself a public key? If that is the expectation then it may be more appropriate to make that a separate retrieval vs something passed in directly from userspace.

...

• payload_len = base64_decode(callout_info, callout_len, payload);
• if (payload_len < 0 || payload_len > GUEST_ATTEST_DATALEN) {

• rc = -EINVAL;
  

• goto out;
  

• }
• rc = ops->request_attest(key, level, dest_keyring, payload, payload_len,

• 		 authkey);
  

+out:

• kfree(payload);
• put_ops();
• return rc;

+}

+static int guest_attest_request_key(struct key *authkey, void *data) +{

• struct request_key_auth *rka = get_request_key_auth(authkey);
• struct key *key = rka->target_key;
• unsigned long long id;
• int rc, level;
• pr_debug("desc: %s op: %s callout: %s\n", key->description, rka->op,

•  rka->callout_info ? (char *)rka->callout_info : "\"none\"");
  

• if (sscanf(key->description, "guest_attest:%d:%llu", &level, &id) != 2)

• return -EINVAL;
  

Can you explain some details about the id and level? It is not very clear why we need it.

@level is my mockup of the vmpl concept, and @id is something free-form for now that the requester of the attestation knows what it means. This id-scheme would follow a Linux-defined format. More discussion is needed here about what attestation data is used for and opportunities to define a common naming scheme. For example, what is the common name of the guest_attest-key that supports retrieving the storage decryption key from the key-server.

On Mon, Jun 26, 2023 at 05:39:12PM -0700, Sathyanarayanan Kuppuswamy wrote:

+Atish

Atish, any comments on this topic from RISC-v?

The CoVE (RISC-V confidential computing specification) would benefit from the proposed abstract API. Similar to at least both TDX and SEV, the CoVE attestation evidence generation proposed (The spec is not ratified yet) interface [1] basically takes some binary blobs in (a TVM public key and an attestation challenge blob) and requests the TSM to generate an attesation evidence for the caller. The TSM will generate such evidence from all static and runtime measurements and sign it with its DICE derived key. Attestation lingo set aside, the pattern here is similar to SEV and TDX. Having a common API for a generic attestation evidence generation interface would avoid having to add yet another ioctl based interface specific to CoVE.

Another interface we could think about commonizing is the measurement extension one. I think both TDX and CoVE allow for a guest to dynamically extend its measurements (to dedicated, runtime PCRs).

Cheers, Samuel.

[1] https://github.com/riscv-non-isa/riscv-ap-tee/blob/main/specification/sbi_co...

On 6/26/23 11:57 AM, Dionna Amalie Glaze wrote:

...

On Sun, Jun 25, 2023 at 8:06 PM Sathyanarayanan Kuppuswamy sathyanarayanan.kuppuswamy@linux.intel.com wrote:

...

Hi Dan,

On 6/23/23 3:27 PM, Dan Williams wrote:

...

Dan Williams wrote:

...

[ add David, Brijesh, and Atish]

Kuppuswamy Sathyanarayanan wrote:

...

In TDX guest, the second stage of the attestation process is Quote generation. This process is required to convert the locally generated TDREPORT into a remotely verifiable Quote. It involves sending the TDREPORT data to a Quoting Enclave (QE) which will verify the integrity of the TDREPORT and sign it with an attestation key.

Intel's TDX attestation driver exposes TDX_CMD_GET_QUOTE IOCTL to allow the user agent to get the TD Quote.

Add a kernel selftest module to verify the Quote generation feature.

TD Quote generation involves following steps:

• Get the TDREPORT data using TDX_CMD_GET_REPORT IOCTL.
• Embed the TDREPORT data in quote buffer and request for quote generation via TDX_CMD_GET_QUOTE IOCTL request.
• Upon completion of the GetQuote request, check for non zero value in the status field of Quote header to make sure the generated quote is valid.

What this cover letter does not say is that this is adding another instance of the similar pattern as SNP_GET_REPORT.

Linux is best served when multiple vendors trying to do similar operations are brought together behind a common ABI. We see this in the history of wrangling SCSI vendors behind common interfaces. Now multiple confidential computing vendors trying to develop similar flows with differentiated formats where that differentiation need not leak over the ABI boundary.

[..]

Below is a rough mock up of this approach to demonstrate the direction. Again, the goal is to define an ABI that can support any vendor's arch-specific attestation method and key provisioning flows without leaking vendor-specific details, or confidential material over the user/kernel ABI.

Thanks for working on this mock code and helping out. It gives me the general idea about your proposal.

...

The observation is that there are a sufficient number of attestation flows available to review where Linux can define a superset ABI to contain them all. The other observation is that the implementations have features that may cross-polinate over time. For example the SEV privelege level consideration ("vmpl"), and the TDX RTMR (think TPM PCRs) mechanisms address generic Confidential Computing use cases.

I agree with your point about VMPL and RTMR feature cases. This observation is valid for AMD SEV and TDX attestation flows. But I am not sure whether it will hold true for other vendor implementations. Our sample set is not good enough to make this conclusion. The reason for my concern is, if you check the ABI interface used in the S390 arch attestation driver (drivers/s390/char/uvdevice.c), you would notice that there is a significant difference between the ABI used in that driver and SEV/TDX drivers. The S390 driver attestation request appears to accept two data blobs as input, as well as a variety of vendor-specific header configurations.

Maybe the s390 attestation model is a special case, but, I think we consider this issue. Since we don't have a common spec, there is chance that any superset ABI we define now may not meet future vendor requirements. One way to handle it to leave enough space in the generic ABI to handle future vendor requirements.

I think it would be better if other vendors (like ARM or RISC) can comment and confirm whether this proposal meets their demands.

The VMPL-based separation that will house the supervisor module known as SVSM can have protocols that implement a TPM command interface, or an RTMR-extension interface, and will also need to have an SVSM-specific protocol attestation report format to keep the secure chain of custody apparent. We'd have different formats and protocols in the kernel, at least, to speak to each technology. I'm not sure it's worth the trouble of papering over all the... 3-4 technologies with similar but still weirdly different formats and ways of doing things with an abstracted attestation ABI, especially since the output all has to be interpreted in an architecture-specific way anyway.

ARM's Confidential Computing Realm Management Extensions (RME) seems to be going along the lines of a runtime measurement register model with their hardware enforced security. The number of registers isn't prescribed in the spec.

+Joey Gouly +linux-coco@lists.linux.dev as far as RME is concerned, do you know who would be best to weigh in on this discussion of a unified attestation model?

...

...

...

Vendor specific ioctls for all of this feels like surrender when Linux already has the keys subsystem which has plenty of degrees of freedom for tracking blobs with signatures and using those blobs to instantiate other blobs. It already serves as the ABI wrapping various TPM implementations and marshaling keys for storage encryption and other use cases that intersect Confidential Computing.

The benefit of deprecating vendor-specific abstraction layers in userspace is secondary. The primary benefit is collaboration. It enables kernel developers from various architectures to collaborate on common infrastructure. If, referring back to my previous example, SEV adopts an RTMR-like mechanism and TDX adopts a vmpl-like mechanism it would be unfortunate if those efforts were siloed, duplicated, and needlessly differentiated to userspace. So while there are arguably a manageable number of basic arch attestation methods the planned expansion of those to build incremental functionality is where I believe we, as a community, will be glad that we invested in a "Linux format" for all of this.

An example, to show what the strawman patch below enables: (req_key is the sample program from "man 2 request_key")

# ./req_key guest_attest guest_attest:0:0-$desc $(cat user_data | base64) Key ID is 10e2f3a7 # keyctl pipe 0x10e2f3a7 | hexdump -C 00000000 54 44 58 20 47 65 6e 65 72 61 74 65 64 20 51 75 |TDX Generated Qu| 00000010 6f 74 65 00 00 00 00 00 00 00 00 00 00 00 00 00 |ote.............| 00000020 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|

00004000

This is the kernel instantiating a TDX Quote without the TDREPORT implementation detail ever leaving the kernel. Now, this is only the

IIUC, the idea here is to cache the quote data and return it to the user whenever possible, right? If yes, I think such optimization may not be very useful for our case. AFAIK, the quote data will change whenever there is a change in the guest measurement data. Since the validity of the generated quote will not be long, and the frequency of quote generation requests is expected to be less, we may not get much benefit from caching the quote data. I think we can keep this logic simple by directly retrieving the quote data from the quoting enclave whenever there is a request from the user.

...

top-half of what is needed. The missing bottom half takes that material and uses it to instantiate derived key material like the storage decryption key internal to the kernel. See "The Process" in Documentation/security/keys/request-key.rst for how the Keys subsystem handles the "keys for keys" use case.

This is only useful for key-server use case, right? Attestation can also be used for use cases like pattern matching or uploading some secure data, etc. Since key-server is not the only use case, does it make sense to suppport this derived key feature?

...

---

diff --git a/drivers/virt/Kconfig b/drivers/virt/Kconfig index f79ab13a5c28..0f775847028e 100644 --- a/drivers/virt/Kconfig +++ b/drivers/virt/Kconfig @@ -54,4 +54,8 @@ source "drivers/virt/coco/sev-guest/Kconfig"

source "drivers/virt/coco/tdx-guest/Kconfig"

+config GUEST_ATTEST

• tristate
  

• select KEYS
  

endif diff --git a/drivers/virt/Makefile b/drivers/virt/Makefile index e9aa6fc96fab..66f6b838f8f4 100644 --- a/drivers/virt/Makefile +++ b/drivers/virt/Makefile @@ -12,3 +12,4 @@ obj-$(CONFIG_ACRN_HSM) += acrn/ obj-$(CONFIG_EFI_SECRET) += coco/efi_secret/ obj-$(CONFIG_SEV_GUEST) += coco/sev-guest/ obj-$(CONFIG_INTEL_TDX_GUEST) += coco/tdx-guest/ +obj-$(CONFIG_GUEST_ATTEST) += coco/guest-attest/ diff --git a/drivers/virt/coco/guest-attest/Makefile b/drivers/virt/coco/guest-attest/Makefile new file mode 100644 index 000000000000..5581c5a27588 --- /dev/null +++ b/drivers/virt/coco/guest-attest/Makefile @@ -0,0 +1,2 @@ +obj-$(CONFIG_GUEST_ATTEST) += guest_attest.o +guest_attest-y := key.o diff --git a/drivers/virt/coco/guest-attest/key.c b/drivers/virt/coco/guest-attest/key.c new file mode 100644 index 000000000000..2a494b6dd7a7 --- /dev/null +++ b/drivers/virt/coco/guest-attest/key.c @@ -0,0 +1,159 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* Copyright(c) 2023 Intel Corporation. All rights reserved. */

+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#include <linux/seq_file.h> +#include <linux/key-type.h> +#include <linux/module.h> +#include <linux/base64.h>

+#include <keys/request_key_auth-type.h> +#include <keys/user-type.h>

+#include "guest-attest.h"

Can you share you guest-attest.h?

...

+static LIST_HEAD(guest_attest_list); +static DECLARE_RWSEM(guest_attest_rwsem);

+static struct guest_attest_ops *fetch_ops(void) +{

• return list_first_entry_or_null(&guest_attest_list,
  

•                                 struct guest_attest_ops, list);
  

+}

+static struct guest_attest_ops *get_ops(void) +{

• down_read(&guest_attest_rwsem);
  

• return fetch_ops();
  

+}

+static void put_ops(void) +{

• up_read(&guest_attest_rwsem);
  

+}

+int register_guest_attest_ops(struct guest_attest_ops *ops) +{

• struct guest_attest_ops *conflict;
  

• int rc;
  

• down_write(&guest_attest_rwsem);
  

• conflict = fetch_ops();
  

• if (conflict) {
  

•         pr_err("\"%s\" ops already registered\n", conflict->name);
  

•         rc = -EEXIST;
  

•         goto out;
  

• }
  

• list_add(&ops->list, &guest_attest_list);
  

• try_module_get(ops->module);
  

• rc = 0;
  

+out:

• up_write(&guest_attest_rwsem);
  

• return rc;
  

+} +EXPORT_SYMBOL_GPL(register_guest_attest_ops);

+void unregister_guest_attest_ops(struct guest_attest_ops *ops) +{

• down_write(&guest_attest_rwsem);
  

• list_del(&ops->list);
  

• up_write(&guest_attest_rwsem);
  

• module_put(ops->module);
  

+} +EXPORT_SYMBOL_GPL(unregister_guest_attest_ops);

+static int __guest_attest_request_key(struct key *key, int level,

•                               struct key *dest_keyring,
  

•                               const char *callout_info, int callout_len,
  

•                               struct key *authkey)
  

+{

• struct guest_attest_ops *ops;
  

• void *payload = NULL;
  

• int rc, payload_len;
  

• ops = get_ops();
  

• if (!ops)
  

•         return -ENOKEY;
  

• payload = kzalloc(max(GUEST_ATTEST_DATALEN, callout_len), GFP_KERNEL);
  

• if (!payload) {
  

•         rc = -ENOMEM;
  

•         goto out;
  

• }
  

Is the idea to get the values like vmpl part of the payload?

...

• payload_len = base64_decode(callout_info, callout_len, payload);
  

• if (payload_len < 0 || payload_len > GUEST_ATTEST_DATALEN) {
  

•         rc = -EINVAL;
  

•         goto out;
  

• }
  

• rc = ops->request_attest(key, level, dest_keyring, payload, payload_len,
  

•                          authkey);
  

+out:

• kfree(payload);
  

• put_ops();
  

• return rc;
  

+}

+static int guest_attest_request_key(struct key *authkey, void *data) +{

• struct request_key_auth *rka = get_request_key_auth(authkey);
  

• struct key *key = rka->target_key;
  

• unsigned long long id;
  

• int rc, level;
  

• pr_debug("desc: %s op: %s callout: %s\n", key->description, rka->op,
  

•          rka->callout_info ? (char *)rka->callout_info : "\"none\"");
  

• if (sscanf(key->description, "guest_attest:%d:%llu", &level, &id) != 2)
  

•         return -EINVAL;
  

Can you explain some details about the id and level? It is not very clear why we need it.

...

• if (!rka->callout_info) {
  

•         rc = -EINVAL;
  

•         goto out;
  

• }
  

• rc = __guest_attest_request_key(key, level, rka->dest_keyring,
  

•                                 rka->callout_info, rka->callout_len,
  

•                                 authkey);
  

+out:

• complete_request_key(authkey, rc);
  

• return rc;
  

+}

+static int guest_attest_vet_description(const char *desc) +{

• unsigned long long id;
  

• int level;
  

• if (sscanf(desc, "guest_attest:%d:%llu", &level, &id) != 2)
  

•         return -EINVAL;
  

• return 0;
  

+}

+static struct key_type key_type_guest_attest = {

• .name = "guest_attest",
  

• .preparse = user_preparse,
  

• .free_preparse = user_free_preparse,
  

• .instantiate = generic_key_instantiate,
  

• .revoke = user_revoke,
  

• .destroy = user_destroy,
  

• .describe = user_describe,
  

• .read = user_read,
  

• .vet_description = guest_attest_vet_description,
  

• .request_key = guest_attest_request_key,
  

+};

+static int __init guest_attest_init(void) +{

• return register_key_type(&key_type_guest_attest);
  

+}

+static void __exit guest_attest_exit(void) +{

• unregister_key_type(&key_type_guest_attest);
  

+}

+module_init(guest_attest_init); +module_exit(guest_attest_exit); +MODULE_LICENSE("GPL v2"); diff --git a/drivers/virt/coco/tdx-guest/Kconfig b/drivers/virt/coco/tdx-guest/Kconfig index 14246fc2fb02..9a1ec85369fe 100644 --- a/drivers/virt/coco/tdx-guest/Kconfig +++ b/drivers/virt/coco/tdx-guest/Kconfig @@ -1,6 +1,7 @@ config TDX_GUEST_DRIVER tristate "TDX Guest driver" depends on INTEL_TDX_GUEST

• select GUEST_ATTEST
  help
    The driver provides userspace interface to communicate with
    the TDX module to request the TDX guest details like attestation
  

diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c b/drivers/virt/coco/tdx-guest/tdx-guest.c index 388491fa63a1..65b5aab284d9 100644 --- a/drivers/virt/coco/tdx-guest/tdx-guest.c +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c @@ -13,11 +13,13 @@ #include <linux/string.h> #include <linux/uaccess.h> #include <linux/set_memory.h> +#include <linux/key-type.h>

#include <uapi/linux/tdx-guest.h>

#include <asm/cpu_device_id.h> #include <asm/tdx.h> +#include "../guest-attest/guest-attest.h"

/*

• Intel's SGX QE implementation generally uses Quote size less

@@ -229,6 +231,62 @@ static const struct x86_cpu_id tdx_guest_ids[] = { }; MODULE_DEVICE_TABLE(x86cpu, tdx_guest_ids);

+static int tdx_request_attest(struct key *key, int level,

•                       struct key *dest_keyring, void *payload,
  

•                       int payload_len, struct key *authkey)
  

+{

• u8 *tdreport;
  

• long ret;
  

• tdreport = kzalloc(TDX_REPORT_LEN, GFP_KERNEL);
  

• if (!tdreport)
  

•         return -ENOMEM;
  

• /* Generate TDREPORT0 using "TDG.MR.REPORT" TDCALL */
  

• ret = tdx_mcall_get_report0(payload, tdreport);
  

• if (ret)
  

•         goto out;
  

• mutex_lock(&quote_lock);
  

• memset(qentry->buf, 0, qentry->buf_len);
  

• reinit_completion(&qentry->compl);
  

• qentry->valid = true;
  

• /* Submit GetQuote Request using GetQuote hyperetall */
  

• ret = tdx_hcall_get_quote(qentry->buf, qentry->buf_len);
  

• if (ret) {
  

•         pr_err("GetQuote hyperetall failed, status:%lx\n", ret);
  

•         ret = -EIO;
  

•         goto quote_failed;
  

• }
  

• /*
  

•  * Although the GHCI specification does not state explicitly that
  

•  * the VMM must not wait indefinitely for the Quote request to be
  

•  * completed, a sane VMM should always notify the guest after a
  

•  * certain time, regardless of whether the Quote generation is
  

•  * successful or not.  For now just assume the VMM will do so.
  

•  */
  

• wait_for_completion(&qentry->compl);
  

• ret = key_instantiate_and_link(key, qentry->buf, qentry->buf_len,
  

•                                dest_keyring, authkey);
  

+quote_failed:

• qentry->valid = false;
  

• mutex_unlock(&quote_lock);
  

+out:

• kfree(tdreport);
  

• return ret;
  

+}

+static struct guest_attest_ops tdx_attest_ops = {

• .name = KBUILD_MODNAME,
  

• .module = THIS_MODULE,
  

• .request_attest = tdx_request_attest,
  

+};

static int __init tdx_guest_init(void) { int ret; @@ -251,8 +309,14 @@ static int __init tdx_guest_init(void) if (ret) goto free_quote;

• ret = register_guest_attest_ops(&tdx_attest_ops);
  

• if (ret)
  

•         goto free_irq;
  

• return 0;
  
  

+free_irq:

• tdx_unregister_event_irq_cb(quote_cb_handler, qentry);
  

free_quote: free_quote_entry(qentry); free_misc: @@ -264,6 +328,7 @@ module_init(tdx_guest_init);

static void __exit tdx_guest_exit(void) {

• unregister_guest_attest_ops(&tdx_attest_ops);
  tdx_unregister_event_irq_cb(quote_cb_handler, qentry);
  free_quote_entry(qentry);
  misc_deregister(&tdx_misc_dev);
  

-- Sathyanarayanan Kuppuswamy Linux Kernel Developer

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

-- Sathyanarayanan Kuppuswamy Linux Kernel Developer