Message-ID: <64961c3baf8ce_142af829436@dwillia2-xfh.jf.intel.com.notmuch>
Date:   Fri, 23 Jun 2023 15:27:07 -0700
From:   Dan Williams <dan.j.williams@...el.com>
To:     Dan Williams <dan.j.williams@...el.com>,
        Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, <x86@...nel.org>,
        Shuah Khan <shuah@...nel.org>, Jonathan Corbet <corbet@....net>
CC:     "H . Peter Anvin" <hpa@...or.com>,
        Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        "Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
        Tony Luck <tony.luck@...el.com>,
        "Wander Lairson Costa" <wander@...hat.com>,
        Erdem Aktas <erdemaktas@...gle.com>,
        Dionna Amalie Glaze <dionnaglaze@...gle.com>,
        Chong Cai <chongc@...gle.com>, Qinkun Bao <qinkun@...che.org>,
        Guorui Yu <GuoRui.Yu@...ux.alibaba.com>,
        "Du Fan" <fan.du@...el.com>, <linux-kernel@...r.kernel.org>,
        <linux-kselftest@...r.kernel.org>, <linux-doc@...r.kernel.org>,
        <dhowells@...hat.com>, <brijesh.singh@....com>,
        <atishp@...osinc.com>, <gregkh@...uxfoundation.org>
Subject: RE: [PATCH v3 3/3] selftests/tdx: Test GetQuote TDX attestation
 feature

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);

Powered by blists - more mailing lists