Re: [PATCH 1/4] keys: Introduce tsm keys

[Dan Williams <dan.j.williams@intel.com>]

Jarkko Sakkinen wrote:
> On Mon Jul 31, 2023 at 7:33 PM EEST, Peter Gonda wrote:
> > What is the purpose of this report? What does it prove to whom? I'm a
> > bit confused because it doesn't seem like there is an ability for a
> > remote party to participate in a challenge and response to introduce
> > any freshness into this protocol.
> >
> > Also shouldn't the report have a little more context into the key we
> > are signing? For instance what type of public key is this? And what is
> > its purpose? In your example this isn't even a valid public key.
> 
> Yeah, I agree.
> 
> It is pretty hard to even evaluate whether this should be in kernel or
> could handled by the user space (perhaps with something less intrusive
> added to the kernel).
> 
> With cover letter starting with not one but two three letter acronyms
> that are not common vocabulary is already a red flag for me at least.
> 
> A lot more clarity is needed on what the heck this thing is anyway.

Apologies Jarkko, the assumption of this code was that because
drivers/virt/coco/sevguest.c was already exporting an ABI that put no
definition on the payload that this new key type would not need to
either.

However, your questioning proves the point that stashing security
relevant ABI in drivers/virt/coco/ is not a recipe to get worthwhile
review from security minded kernel practitioners.

So I can see why my explanation of "just do what sevguest was getting
away with" is not a great answer. Lets try again.

As mentioned in the AMD whitepaper [1]. Confidential computing has a use
case for a guest to prove to an attestation agent that it is running in
a valid configuration before that agent deploys other keys and secrets
to that VM to run a workload. One way to do that is to wrap a report of
the launch state of the VM with a public-key and if that report passes
validation use that key to send encrypted payloads back to the guest.

In this model the kernel is only a conduit to get 64-bytes of data
signed by the report, and the kernel need not have any requirements on
that data. That said, it could. That's where I would look to
recommendations from Dionna and James and others about what contract the
kernel *could*  enforce to ensure that best security practices are being
deployed.  I expect that also helps this implementation cross the
threshold from "blob store" to "key" as the Keyring expects.

[1]: Section "VM Launch & Attestation" https://www.amd.com/system/files/TechDocs/SEV-SNP-strengthening-vm-isolation-with-integrity-protection-and-more.pdf