From: Dionna Amalie Glaze <dionnaglaze@google.com>
To: Dan Williams <dan.j.williams@intel.com>
Cc: linux-coco@lists.linux.dev, Borislav Petkov <bp@alien8.de>,
Tom Lendacky <thomas.lendacky@amd.com>,
Brijesh Singh <brijesh.singh@amd.com>,
Jeremi Piotrowski <jpiotrowski@linux.microsoft.com>,
peterz@infradead.org,
sathyanarayanan.kuppuswamy@linux.intel.com,
dave.hansen@linux.intel.com
Subject: Re: [PATCH v5 6/7] virt: sevguest: Add TSM_REPORTS support for SNP_GET_EXT_REPORT
Date: Wed, 11 Oct 2023 14:06:26 -0700 [thread overview]
Message-ID: <CAAH4kHb-V8q14L+eJh+iTYGwu10-MQ6maR48U5wJRC_bgktEbw@mail.gmail.com> (raw)
In-Reply-To: <65270896e90f3_780ef294c1@dwillia2-xfh.jf.intel.com.notmuch>
On Wed, Oct 11, 2023 at 1:42 PM Dan Williams <dan.j.williams@intel.com> wrote:
>
> Dionna Amalie Glaze wrote:
> [..]
> > > + /* Concatenate returned certs */
> > > + for (i = 0, offset = 0; i < cert_count; i++) {
> > > + struct snp_msg_cert_entry *certs = buf + report_size;
> > > +
> > > + memcpy(cbuf + offset, certs_address + certs[i].offset, certs[i].length);
> > > + offset += certs[i].length;
> > > + }
> >
> > This concatenation isn't going to work
>
> I will note that v4 had this as well, so the timing here is not great,
> but the feedback is still important because it affects an ABI decision
> and "ABI is hard".
Yes, thanks for taking the feedback.
>
> > since you lose the identity of
> > the certificates. The GUIDs of the certificate table matter, and the
> > concatenation isn't necessarily easily parsed out.
>
> I notice "isn't necessarily easily parsed out", leaves some wiggle room.
>
Yes, DER certificates can be concatenated and parsed without
additional metadata,
but I'm less sure about other formats. We could be looking at COSE or
binary-serialized
protocol buffers. It's an entirely avoidable complexity.
If you want to keep PEM, then you'd need to interpret the GUID table
and do the base64
encoding yourself
> The rationale for the concatenate proposal came from the observation
> that conveying certificates is a common capability between SNP reports
> and TDX quotes. In the TDX quote the certificate payload is a:
>
> "Concatenated PCK Cert Chain (PEM formatted)"
>
> ...so the thought was unify on that common denominator of "concatenated
> certificates". Is that an oversimplification?
>
It is. We'll need to get the quoting enclave to additionally provide
an integrity manifest of the firmware.
The TDX team hasn't implemented this functionality yet. Intel told us
this is the pathway we need to take.
> > Consider our use case of providing not just the ARK, ASK, and VCEK
> > certificates, but also a non-x.509 document that is a signed golden
> > measurement of the firmware. We may also want to provide the VLEK so
> > the endorsement key selection attribute of the MSG_REPORT_REQ can be
> > utilized to swap between VCEK and VLEK. I don't believe your patch
> > here allows for that selection either.
>
> To date the kernel's snp_report_req definition is:
>
> struct snp_report_req {
> /* user data that should be included in the report */
> __u8 user_data[64];
>
> /* The vmpl level to be included in the report */
> __u32 vmpl;
>
> /* Must be zero filled */
> __u8 rsvd[28];
> };
>
> ...so the design was only considering what was called out in the existing
> ioctl ABI.
>
> When would the key-select field of MSG_REPORT_REQ be non-zero, and would
> that be a per call decision, or is that selection a fleet wide policy?
>
This is more of a customer-wide policy or machine pool policy.
We have some folks that want VCEK endorsement and we have other folks
that want VLEK, and sole ownership vs fungibility for that choice is I
think is not where this ABI decision should come from.
> > By the GHCB specification, the host is allowed to document their own
> > GUIDs and provide their data to the guest in this data blob.
>
> What does the caller do with these certificates, or are they only
> conveyed to the verifier?
>
The caller could do anything it wants. They can be forwarded to a
verification service, yes, but they may also be analyzed locally and
added to a database for manual review provided some criteria are met.
Say we have a user that is skeptical of any new firmware and rejects
any document that the VM is not preconfigured to accept. This is a
kind of verifier, yes, but it's not RATS.
> > I think probably what's better is for the configfs to just create
> > GUID-named files with the contents of the entries, but I don't think
> > just inblob of length 64 is going to handle the VLEK/VCEK selection.
>
> That's a possibility. Would the kernel need to invent GUID to represent
> the TDX quote PCK cert chain, would the kernel need to un-concatenate
> that blob into separate files?
I don't think that Intel plans on utilizing this "certs" outblob since
it includes all collateral with the quote.
--
-Dionna Glaze, PhD (she/her)
next prev parent reply other threads:[~2023-10-11 21:06 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-11 5:27 [PATCH v5 0/7] configfs-tsm: Attestation Report ABI Dan Williams
2023-10-11 5:27 ` [PATCH v5 1/7] virt: sevguest: Fix passing a stack buffer as a scatterlist target Dan Williams
2023-10-11 5:27 ` [PATCH v5 2/7] virt: coco: Add a coco/Makefile and coco/Kconfig Dan Williams
2023-10-11 5:27 ` [PATCH v5 3/7] configfs-tsm: Introduce a shared ABI for attestation reports Dan Williams
2023-10-11 6:29 ` Kuppuswamy Sathyanarayanan
2023-10-11 5:27 ` [PATCH v5 4/7] virt: sevguest: Prep for kernel internal get_ext_report() Dan Williams
2023-10-11 5:27 ` [PATCH v5 5/7] mm/slab: Add __free() support for kvfree Dan Williams
2023-10-11 6:31 ` Kuppuswamy Sathyanarayanan
2023-10-11 5:27 ` [PATCH v5 6/7] virt: sevguest: Add TSM_REPORTS support for SNP_GET_EXT_REPORT Dan Williams
2023-10-11 16:13 ` Dionna Amalie Glaze
2023-10-11 20:41 ` Dan Williams
2023-10-11 21:06 ` Dionna Amalie Glaze [this message]
2023-10-11 19:24 ` Tom Lendacky
2023-10-11 21:30 ` Dan Williams
2023-10-11 22:21 ` Dionna Amalie Glaze
2023-10-11 22:24 ` Tom Lendacky
2023-10-12 0:38 ` Dan Williams
2023-10-11 5:27 ` [PATCH v5 7/7] virt: tdx-guest: Add Quote generation support using TSM_REPORTS Dan Williams
2023-10-11 6:44 ` [PATCH v5 0/7] configfs-tsm: Attestation Report ABI Kuppuswamy Sathyanarayanan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAAH4kHb-V8q14L+eJh+iTYGwu10-MQ6maR48U5wJRC_bgktEbw@mail.gmail.com \
--to=dionnaglaze@google.com \
--cc=bp@alien8.de \
--cc=brijesh.singh@amd.com \
--cc=dan.j.williams@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=jpiotrowski@linux.microsoft.com \
--cc=linux-coco@lists.linux.dev \
--cc=peterz@infradead.org \
--cc=sathyanarayanan.kuppuswamy@linux.intel.com \
--cc=thomas.lendacky@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.