Re: [PATCH v3 3/3] selftests/tdx: Test GetQuote TDX attestation feature

[gregkh@xxxxxxxxxxxxxxxxxxx]

On Wed, Jun 28, 2023 at 08:56:30AM +0000, Huang, Kai wrote:
> On Wed, 2023-06-28 at 08:46 +0200, gregkh@xxxxxxxxxxxxxxxxxxx wrote:
> > On Wed, Jun 28, 2023 at 02:16:45AM +0000, Huang, Kai wrote:
> > > > You really shouldn't be putting attestation validation logic in the
> > > > kernel.
> > > 
> > > Agreed.  The data blob for remote verification should be just some data blob to
> > > the kernel.  I think the kernel shouldn't even try to understand the data blob
> > > is for which architecture.  From the kernel's perspective, it should be just
> > > some data blob that the kernel gets from hardware/firmware or whatever embedded
> > > in the root-of-trust in the hardware after taking some input from usrspace for
> > > the unique identity of the blob that can be used to, e.g., mitigate replay-
> > > attack, etc.
> > 
> > Great, then use the common "data blob" api that we have in the kernel
> > for a very long time now, the "firwmare download" api, or the sysfs
> > binary file api.  Both of them just use the kernel as a pass-through and
> > do not touch the data at all.  No need for crazy custom ioctls and all
> > that mess :)
> > 
> 
> I guess I was talking about from "kernel shouldn't try to parse attestation data
> blob" perspective.  Looking at AMD's attestation flow (I have no deep
> understanding of AMD's attestation flow), the assumption of "one remote
> verifiable data blob" isn't even true -- AMD can return "attestation report"
> (remote verifiable) and the "certificate" to verify it separately:
> 
> https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/snp-attestation.html
> 
> On the other hand, AFAICT Intel SGX-based attestation doesn't have a mechanism
> "for the kernel" to return certificate(s), but choose to embed the
> certificate(s) to the Quote itself.  I believe we can add such mechanism (e.g.,
> another TDVMCALL) for the kernel to get certificate(s) separately, but AFAICT it
> doesn't exist yet.
> 
> Btw, getting "remote verifiable blob" is only one step of the attestation flow.
> For instance, before the blob can be generated, there must be a step to
> establish the attestation key between the machine and the attestation service. 
> And the flow to do this could be very different between vendors too.
> 
> That being said, while I believe all those differences can be unified in some
> way, I think the question is whether it is worth to put such effort to try to
> unify attestation flow for all vendors.  As Erdem Aktas mentioned earlier, "the
> number of CPU vendors for confidential computing seems manageable".

So you think that there should be a custom user/kernel api for every
single different CPU vendor?  That's not how kernel development works,
sorry.  Let's try to unify them to make both the kernel, and userspace,
sane.

And Dan is right, if this is handling keys, then the key subsystem needs
to be used here instead of custom ioctls.

thanks,

greg k-h