Received-SPF: pass (google.com: domain of linux-crypto+bounces-3907-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Received-SPF: Pass (protection.outlook.com: domain of amd.com designates
 165.204.84.17 as permitted sender) receiver=protection.outlook.com;
 client-ip=165.204.84.17; helo=SATLEXMB04.amd.com; pr=C
Date: Fri, 26 Apr 2024 20:32:10 -0500
From: Michael Roth <michael.roth@amd.com>
To: Sean Christopherson <seanjc@google.com>
CC: <kvm@vger.kernel.org>, <linux-coco@lists.linux.dev>, <linux-mm@kvack.org>,
	<linux-crypto@vger.kernel.org>, <x86@kernel.org>,
	<linux-kernel@vger.kernel.org>, <tglx@linutronix.de>, <mingo@redhat.com>,
	<jroedel@suse.de>, <thomas.lendacky@amd.com>, <hpa@zytor.com>,
	<ardb@kernel.org>, <pbonzini@redhat.com>, <vkuznets@redhat.com>,
	<jmattson@google.com>, <luto@kernel.org>, <dave.hansen@linux.intel.com>,
	<slp@redhat.com>, <pgonda@google.com>, <peterz@infradead.org>,
	<srinivas.pandruvada@linux.intel.com>, <rientjes@google.com>,
	<dovmurik@linux.ibm.com>, <tobin@ibm.com>, <bp@alien8.de>, <vbabka@suse.cz>,
	<kirill@shutemov.name>, <ak@linux.intel.com>, <tony.luck@intel.com>,
	<sathyanarayanan.kuppuswamy@linux.intel.com>, <alpergun@google.com>,
	<jarkko@kernel.org>, <ashish.kalra@amd.com>, <nikunj.dadhania@amd.com>,
	<pankaj.gupta@amd.com>, <liam.merwick@oracle.com>, <Larry.Dewey@amd.com>
Subject: Re: [PATCH v14 21/22] crypto: ccp: Add the
 SNP_{PAUSE,RESUME}_ATTESTATION commands
Message-ID: <20240427013210.ioz7mv3yuu2r5un6@amd.com>
References: <20240421180122.1650812-1-michael.roth@amd.com>
 <20240421180122.1650812-22-michael.roth@amd.com>
 <ZimgrDQ_j2QTM6s5@google.com>
 <20240426173515.6pio42iqvjj2aeac@amd.com>
 <ZiwHFMfExfXvqDIr@google.com>
 <20240426214633.myecxgh6ci3qshmi@amd.com>
 <ZixCYlKn5OYUFWEq@google.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <ZixCYlKn5OYUFWEq@google.com>
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Apr 2024 01:32:28.7472
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 0cfdf1de-0253-410b-3523-08dc6659e6d7
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=3dd8961f-e488-4e60-8e11-a82d994e183d;Ip=[165.204.84.17];Helo=[SATLEXMB04.amd.com]
X-MS-Exchange-CrossTenant-AuthSource:
	CO1PEPF000042AC.namprd03.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR12MB9150

On Fri, Apr 26, 2024 at 05:10:10PM -0700, Sean Christopherson wrote:
> On Fri, Apr 26, 2024, Michael Roth wrote:
> > On Fri, Apr 26, 2024 at 12:57:08PM -0700, Sean Christopherson wrote:
> > > On Fri, Apr 26, 2024, Michael Roth wrote:
> > > What is "management"?  I assume its some userspace daemon?
> > 
> > It could be a daemon depending on cloud provider, but the main example
> > we have in mind is something more basic like virtee[1] being used to
> > interactively perform an update at the command-line. E.g. you point it
> > at the new VLEK, the new cert, and it will handle updating the certs at
> > some known location and issuing the SNP_LOAD_VLEK command. With this
>   ^^^^^^^^^^^^^^^^^^^
> > interface, it can take the additional step of PAUSE'ing attestations
> > before performing either update to keep the 2 actions in sync with the
> > guest view.
> 
> ...
> 
> > > without having to bounce through the kernel.  It doesn't even require a push
> > > model, e.g. wrap/redirect the certs with a file that has a "pause" flag and a
> > > sequence counter.
> > 
> > We could do something like flag the certificate file itself, it does
> > sounds less painful than the above. But what defines that spec?
> 
> Whoever defines "some known location".  And it doesn't need to be a file wrapper,

"some known location" is a necessary and simple parameter controlled by
the cloud provider, so it's easy to make a tool like virtee aware of those
what those parameters should be for any particular environment. But it's not
easy to make it aware of a particular providers internal way of synchronizing
guests and certs access. We'd be somewhat dependent of those providers either
providing hooks to allow for better integration, which is "work" that might
encourage them to just brew their own solutions, versus...

Providing a simple reference scheme that's clearly defined, and easily
adoptable across the board, which is "less work", and makes adoption of common
tools/libraries SNP/certs/key management easier because we don't need to
directly involve a provider's internal guest management mechanisms into those
tools.

> e.g. put the cert in a directory along with a lock.  Actually, IIUC, there doesn't
> even need to be a separate lock file.  I know very little about userspace programming,
> but common sense and a quick search tells me that file locks are a solved problem.
> 
> E.g. it took me ~5 minutes of Googling to come up with this, which AFAICT does
> exactly what you want.
> 
> touch ~/vlek.cert
> (
>   flock -e 200
>   echo "Locked the cert, sleeping for 10 seconds"
>   sleep 10
>   echo "Igor, it's alive!!!!!!"
> ) 200< vlek.cert
> 
> touch ~/vlek.cert
> (
>   flock -s 201
>   echo "Got me a shared lock, no updates for you!"
> ) 201< vlek.cert
> 

Hmm... I did completely miss this option. But I think there are still some
issues here. IIUC you're suggesting (for example):

  "Management":
  a) writelock vlek.cert
  b) perform SNP_LOAD_VLEK and update vlek.cert contents
  c) unlock vlek.cert

  "QEMU":
  a) readlock vlek.cert
  b) copy cert into guest buffer
  c) unlock vlek.cert

The issue is that after "QEMU" unlocks and return the cert to KVM we'll
have:

  "KVM"
  a) return from EXT_GUEST_REQ exit to userspace
  b) issue the attestation report to firmware
  c) return the attestation report and cert to the guest

Between a) and b), "Management" can complete another entire update, but
the cert that it passes back to the guest will be stale relative to the
key used to sign the attestation report.

If we need to take more time to explore other options it's not
absolutely necessary to have the kernel solve this now. But every userspace
will need to solve it in some way so it seemed like it might be nice to
have a simple reference implementation to start with.

-Mike