| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <64f2137362305_4c1f3294f4@dwillia2-mobl3.amr.corp.intel.com.notmuch>
Date: Fri, 1 Sep 2023 09:38:11 -0700
From: Dan Williams <dan.j.williams@...el.com>
To: Jeremi Piotrowski <jpiotrowski@...ux.microsoft.com>,
Dan Williams <dan.j.williams@...el.com>,
<linux-coco@...ts.linux.dev>
CC: Borislav Petkov <bp@...en8.de>,
Tom Lendacky <thomas.lendacky@....com>,
Dionna Glaze <dionnaglaze@...gle.com>,
Brijesh Singh <brijesh.singh@....com>, <peterz@...radead.org>,
<linux-kernel@...r.kernel.org>, <tglx@...utronix.de>
Subject: Re: [PATCH v3 5/5] virt: sevguest: Add TSM_REPORTS support for
SNP_{GET, GET_EXT}_REPORT
Jeremi Piotrowski wrote:
> On 8/30/2023 9:33 PM, Dan Williams wrote:
> > The sevguest driver was a first mover in the confidential computing
> > space. As a first mover that afforded some leeway to build the driver
> > without concern for common infrastructure.
> >
> > Now that sevguest is no longer a singleton [1] the common operation of
> > building and transmitting attestation report blobs can / should be made
> > common. In this model the so called "TSM-provider" implementations can
> > share a common envelope ABI even if the contents of that envelope remain
> > vendor-specific. When / if the industry agrees on an attestation record
> > format, that definition can also fit in the same ABI. In the meantime
> > the kernel's maintenance burden is reduced and collaboration on the
> > commons is increased.
> >
> > Convert sevguest to use CONFIG_TSM_REPORTS to retrieve the blobs that
> > the SNP_{GET,GET_EXT}_REPORT ioctls produce. An example flow follows for
> > retrieving the SNP_GET_REPORT blob via the TSM interface utility,
> > assuming no nonce and VMPL==2:
> >
> > report=/sys/kernel/config/tsm/report/report0
> > mkdir $report
> > echo 2 > $report/privlevel
> > dd if=/dev/urandom bs=64 count=1 > $report/inblob
> > hexdump -C $report/outblob
> > rmdir $report
> >
> > ...while the SNP_GET_EXT_REPORT flow needs to additionally set the
> > format to "extended":
> >
> > report=/sys/kernel/config/tsm/report/report1
> > mkdir $report
> > echo extended > $report/format
> > dd if=/dev/urandom bs=64 count=1 > $report/inblob
> > hexdump -C $report/outblob
> > rmdir $report
> >
> > The old ioctls can be lazily deprecated, the main motivation of this
> > effort is to stop the proliferation of new ioctls, and to increase
> > cross-vendor collaboration.
> >
> > Note, only compile-tested.
> >
> > Link: http://lore.kernel.org/r/64961c3baf8ce_142af829436@dwillia2-xfh.jf.intel.com.notmuch [1]
> > Cc: Borislav Petkov <bp@...en8.de>
> > Cc: Tom Lendacky <thomas.lendacky@....com>
> > Cc: Dionna Glaze <dionnaglaze@...gle.com>
> > Cc: Brijesh Singh <brijesh.singh@....com>
> > Signed-off-by: Dan Williams <dan.j.williams@...el.com>
> > ---
> > drivers/virt/coco/sev-guest/Kconfig | 1
> > drivers/virt/coco/sev-guest/sev-guest.c | 83 +++++++++++++++++++++++++++++++
> > 2 files changed, 84 insertions(+)
> >
> > diff --git a/drivers/virt/coco/sev-guest/Kconfig b/drivers/virt/coco/sev-guest/Kconfig
> > index da2d7ca531f0..1cffc72c41cb 100644
> > --- a/drivers/virt/coco/sev-guest/Kconfig
> > +++ b/drivers/virt/coco/sev-guest/Kconfig
> > @@ -5,6 +5,7 @@ config SEV_GUEST
> > select CRYPTO
> > select CRYPTO_AEAD2
> > select CRYPTO_GCM
> > + select TSM_REPORTS
> > help
> > SEV-SNP firmware provides the guest a mechanism to communicate with
> > the PSP without risk from a malicious hypervisor who wishes to read,
> > diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
> > index c3c9e9ea691f..c7bbb8f372a3 100644
> > --- a/drivers/virt/coco/sev-guest/sev-guest.c
> > +++ b/drivers/virt/coco/sev-guest/sev-guest.c
> > @@ -16,10 +16,12 @@
> > #include <linux/miscdevice.h>
> > #include <linux/set_memory.h>
> > #include <linux/fs.h>
> > +#include <linux/tsm.h>
> > #include <crypto/aead.h>
> > #include <linux/scatterlist.h>
> > #include <linux/psp-sev.h>
> > #include <linux/sockptr.h>
> > +#include <linux/cleanup.h>
> > #include <uapi/linux/sev-guest.h>
> > #include <uapi/linux/psp-sev.h>
> >
> > @@ -759,6 +761,79 @@ static u8 *get_vmpck(int id, struct snp_secrets_page_layout *layout, u32 **seqno
> > return key;
> > }
> >
> > +static u8 *sev_report_new(const struct tsm_desc *desc, void *data, size_t *outblob_len)
> > +{
> > + struct snp_guest_dev *snp_dev = data;
> > + const int report_size = SZ_4K;
> > + const int ext_size = SZ_16K;
> > + int ret, size;
> > +
> > + if (desc->inblob_len != 64)
> > + return ERR_PTR(-EINVAL);
> > +
> > + if (desc->outblob_format == TSM_FORMAT_EXTENDED)
> > + size = report_size + ext_size;
> > + else
> > + size = report_size;
> > +
> > + u8 *buf __free(kvfree) = kvzalloc(size, GFP_KERNEL);
> > +
> > + guard(mutex)(&snp_cmd_mutex);
> > + if (desc->outblob_format == TSM_FORMAT_EXTENDED) {
> > + struct snp_ext_report_req ext_req = {
> > + .data = { .vmpl = desc->privlevel },
> > + .certs_address = (__u64)buf + report_size,
> > + .certs_len = ext_size,
> > + };
> > + memcpy(&ext_req.data.user_data, desc->inblob, desc->inblob_len);
> > +
> > + struct snp_guest_request_ioctl input = {
> > + .msg_version = 1,
> > + .req_data = (__u64)&ext_req,
> > + .resp_data = (__u64)buf,
> > + };
> > + struct snp_req_resp io = {
> > + .req_data = KERNEL_SOCKPTR(&ext_req),
> > + .resp_data = KERNEL_SOCKPTR(buf),
> > + };
> > +
> > + ret = get_ext_report(snp_dev, &input, &io);
> > + } else {
> > + struct snp_report_req req = {
> > + .vmpl = desc->privlevel,
> > + };
> > + memcpy(&req.user_data, desc->inblob, desc->inblob_len);
> > +
> > + struct snp_guest_request_ioctl input = {
> > + .msg_version = 1,
> > + .req_data = (__u64)&req,
> > + .resp_data = (__u64)buf,
> > + };
> > + struct snp_req_resp io = {
> > + .req_data = KERNEL_SOCKPTR(&req),
> > + .resp_data = KERNEL_SOCKPTR(buf),
> > + };
> > +
> > + ret = get_report(snp_dev, &input, &io);
> > + }
> > +
> > + if (ret)
> > + return ERR_PTR(ret);
> > +
> > + *outblob_len = size;
> > + return_ptr(buf);
> > +}
> > +
> > +static const struct tsm_ops sev_tsm_ops = {
> > + .name = KBUILD_MODNAME,
> > + .report_new = sev_report_new,
> > +};
> > +
> > +static void unregister_sev_tsm(void *data)
> > +{
> > + unregister_tsm(&sev_tsm_ops);
> > +}
> > +
> > static int __init sev_guest_probe(struct platform_device *pdev)
> > {
> > struct snp_secrets_page_layout *layout;
> > @@ -832,6 +907,14 @@ static int __init sev_guest_probe(struct platform_device *pdev)
> > snp_dev->input.resp_gpa = __pa(snp_dev->response);
> > snp_dev->input.data_gpa = __pa(snp_dev->certs_data);
> >
> > + ret = register_tsm(&sev_tsm_ops, snp_dev, &tsm_report_ext_type);
> > + if (ret)
> > + goto e_free_cert_data;
> > +
> > + ret = devm_add_action_or_reset(&pdev->dev, unregister_sev_tsm, NULL);
> > + if (ret)
> > + goto e_free_cert_data;
> > +
> > ret = misc_register(misc);
> > if (ret)
> > goto e_free_cert_data;
> >
>
> I tried this with the non-extended request
Thanks for testing!
> ...and realized it's a bit awkward from
> a uapi point of view. The returned outblob has a header prepended (table 23 in [1])
> and is arbitrarily sized at 4096. It would be more natural to only return the report
> field and the count bytes that the report actually has. I've attached a rough patch
> below to give you an idea of what I mean.
It makes sense, especially if that is what the legacy ioctl output
buffer is emitting.
> The extended guest request is another topic, since userspace has to be aware of
> where the kernel choses to put the extended data, and fixup all the offsets in the
> table (section 4.1.8.1 in [2]). It would be better to return this data through a
> separate file.
I notice that the TDX report also includes a certificate blob, so if
that is a common concept then yes, it makes sense to have a separate
file for that.
Powered by blists - more mailing lists