Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <a507ef3302d3afff58d82528ee17e82df1f21de0.camel@HansenPartnership.com>
Subject: Re: [PATCH 0/4] keys: Introduce a keys frontend for attestation
 reports
From:   James Bottomley <James.Bottomley@HansenPartnership.com>
To:     Dan Williams <dan.j.williams@intel.com>, dhowells@redhat.com
Cc:     Brijesh Singh <brijesh.singh@amd.com>,
        Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@linux.intel.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Tom Lendacky <thomas.lendacky@amd.com>,
        Dionna Amalie Glaze <dionnaglaze@google.com>,
        Borislav Petkov <bp@alien8.de>,
        Jarkko Sakkinen <jarkko@kernel.org>,
        Samuel Ortiz <sameo@rivosinc.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        linux-coco@lists.linux.dev, keyrings@vger.kernel.org,
        x86@kernel.org, linux-kernel@vger.kernel.org
Date:   Sat, 29 Jul 2023 14:17:18 -0400
In-Reply-To: <169057265210.180586.7950140104251236598.stgit@dwillia2-xfh.jf.intel.com>
References: <169057265210.180586.7950140104251236598.stgit@dwillia2-xfh.jf.intel.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.42.4 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

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.

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?

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.

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

James