Message-ID: <64c7ee68b56d8_51ad029422@dwillia2-xfh.jf.intel.com.notmuch>
Date:   Mon, 31 Jul 2023 10:24:56 -0700
From:   Dan Williams <dan.j.williams@...el.com>
To:     James Bottomley <James.Bottomley@...senpartnership.com>,
        Dan Williams <dan.j.williams@...el.com>, <dhowells@...hat.com>
CC:     Brijesh Singh <brijesh.singh@....com>,
        Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Tom Lendacky <thomas.lendacky@....com>,
        "Dionna Amalie Glaze" <dionnaglaze@...gle.com>,
        Borislav Petkov <bp@...en8.de>,
        Jarkko Sakkinen <jarkko@...nel.org>,
        Samuel Ortiz <sameo@...osinc.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        <linux-coco@...ts.linux.dev>, <keyrings@...r.kernel.org>,
        <x86@...nel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 0/4] keys: Introduce a keys frontend for attestation
 reports

James Bottomley wrote:
> On Sat, 2023-07-29 at 21:56 -0700, Dan Williams wrote:
> > James Bottomley wrote:
> > > On Fri, 2023-07-28 at 12:30 -0700, Dan Williams wrote:
> > > > The bulk of the justification for this patch kit is in "[PATCH
> > > > 1/4] keys: Introduce tsm keys". The short summary is that the
> > > > current approach of adding new char devs and new ioctls, for what
> > > > amounts to the same functionality with minor formatting
> > > > differences across vendors, is untenable. Common concepts and the
> > > > community benefit from common infrastructure.
> > > 
> > > I agree with this, but ...
> > > 
> > > > Use Keys to build common infrastructure for confidential
> > > > computing attestation report blobs, convert sevguest to use it
> > > > (leaving the deprecation question alone for now), and pave the
> > > > way for tdx-guest and the eventual risc-v equivalent to use it in
> > > > lieu of new ioctls.
> > > > 
> > > > The sevguest conversion is only compile-tested.
> > > > 
> > > > This submission is To:David since he needs to sign-off on the
> > > > idea of a new Keys type, the rest is up to the confidential-
> > > > computing driver maintainers to adopt.
> > > 
> > > So why is this a keys subsystem thing?  The keys in question cannot
> > > be used to do any key operations.  It looks like a transport layer
> > > for attestation reports rather than anything key like.
> > 
> > Yes, it has ended up as just a transport layer.
> > 
> > > To give an analogy with the TPM: We do have a TPM interface to keys
> > > because it can be used for things like sealing (TPM stores a
> > > symmetric key) and even asymmetric operations (although TPM key
> > > support for that in 1.2 was just removed).  However, in direct
> > > analogy with confidential computing: the TPM does have an
> > > attestation interface: TPM2_Quote and TPM2_Certify (among others)
> > > which is deliberately *not* wired in to the keys subsystem because
> > > the outputs are intended for external verifiers.
> > > 
> > > If the goal is to unify the interface for transporting attestation
> > > reports, why not pull the attestation ioctls out of sevguest into
> > > something common?
> > 
> > That's fair. I originally started out with a draft trusted-keys
> > implementation, but abandoned it because that really wants a vTPM
> > backend. There is no kernel consumer for attestation reports like
> > other key blobs, so that leaves either a key-type that is just a
> > transport layer or a new ABI.
> >  
> > I have a personal distaste for ioctls and the presence of user-
> > defined blobs in the Keyring subsystem made me think "why not just
> > have a key-type to convey the per-TSM attestation reports". Is that a
> > fair observation?
> 
> The trouble with this argument is that it's an argument for every new
> ioctl becoming a key type.  

Yeah, that's a danger, I don't want Linux keyring to become the blob
transporter subsystem.

While this usage is "security" adjacent the precedent is not a great
one.

> We have a ton of interfaces for transporting information across the
> kernel to user boundary: sysfs, filesystem, configfs, debugfs, etc ...
> although to be fair the fashionably acceptable one does seem to change
> each year.  Since there's nothing really transactional about this,
> what about a simple sysfs one?  You echo in the nonce to a binary
> attribute and cat the report.  Any additional stuff, like the cert
> chain, can appear as additional attributes?

That should be straightforward to mock up and it keeps the property I
like of common ABI with optional per-TSM modifiers.

> > An ioctl interface would make sense for a common report format, but
> > the presence of per-TSM options and per-TSM format modifiers (like
> > SEV privilege level and "extended" attestation reports) attracted me
> > to the ability to just have "options" specified at report
> > instantiation time.
> 
> The "extended" report is nothing but a way of getting the signing key
> cert chain.  It's really just a glorified caching mechanism to relieve
> the relying party from the job of doing the lookup themselves.
> 
> > I.e. like the options specified to trusted-key instantiation.
> > 
> > > I also don't see in your interface where the nonce goes?  Most
> > > attestation reports combine the report output with a user supplied
> > > nonce which gets added to the report signature to defend against
> > > replay.
> > 
> > The user supplied data is another argument to instantiate the report
> > blob. The instantiation format is:
> > 
> >     auth <ascii hex blob user data> [options]
> > 
> > ...for example:
> > 
> >     # dd if=/dev/urandom of=pubkey bs=1 count=64
> >     # keyctl add tsm tsm_test "auth $(xxd -p -c 0 < pubkey)
> > privlevel=2" @u
> > 
> > > Finally, I can see the logic in using this to do key release,
> > > because the external relying entity usually wishes to transport
> > > secrets into the enclave, but the currently developing use case for
> > > that seems to be to use a confidential guest vTPM because then we
> > > can use the existing TPM disk key interfaces.  Inventing something
> > > completely new isn't going to fly because all consumers have to be
> > > updated to use it (even though keys is a common interface, using
> > > key payloads isn't ... plus the systemd TPM disk encryption key
> > > doesn't even use kernel keys, it unwraps in userspace).
> > 
> > I do think the eventual vTPM enabling is separate from this and I
> > mention that in the changelogs.
> 
> vTPM requires no enabling: it will just work with the existing trusted
> key interface.

Oh, I had not seen a TSM implemenetation that presented an TPM
API-interface so I had been thining one had to be built around
facilities like derived keys. I agree the best vTPM is just a TPM.

> >  That functionality like SNP_GET_DERIVED_KEY is amenable to a
> > trusted-keys frontend and being unified with existing TPM paths.
> 
> To get a bit off topic, I'm not sure derived keys are much use.  The
> problem is in SNP that by the time the PSP does the derivation, the key
> is both tied to the physical system and derived from a measurement too
> general to differentiate between VM images (so one VM could read
> another VMs stored secrets).
> 
> > 
> > This report interface on the other hand just needs a single ABI to
> > retrieve all these vendor formats (until industry standardization
> > steps in) and it needs to be flexible (within reason) for all the
> > TSM-specific options to be conveyed. I do not trust my ioctl ABI
> > minefield avoidance skills to get that right. Key blob instantiation
> > feels up to the task.
> 
> To repeat: there's nothing keylike about it.
> 
> If you think that the keyctl mechanism for transporting information
> across the kernel boundary should be generalised and presented as an
> alternative to our fashion of the year interface for this, then that's
> what you should do (and, I'm afraid to add, cc all the other
> opinionated people who've also produced the flavour of the year
> interfaces).  Sneaking it in as a one-off is the wrong way to proceed
> on something like this.

Fair enough, I'll take a look at the sysfs conversion and we can go from
there.

Powered by blists - more mailing lists