Re: [PATCH v3 4/4] platform/x86: intel_tdx_attest: Add TDX Guest attestation interface driver

Kai Huang
2022-04-19T22:38:19Z

On Tue, 2022-04-19 at 07:00 -0700, Sathyanarayanan Kuppuswamy wrote:
>
> On 4/19/22 1:16 AM, Kai Huang wrote:
> > In fact after slightly thinking more, I think you can split TDREPORT TDCALL
> > support with GetQuote/SetupEventNotifyInterrupt support. The reason is as I
> > said, GetQuote isn't mandatory to support attestation. TD attestation agent can
> > use i.e. vsock, tcp/ip, to communicate to QE directly. Whether kernel needs to
> > support GetQuote is actually arguable.
>
> IMO, we should not use a usage model to categorize "GetQuote" support
> as a mandatory or non-mandatory requirement.
>
> For customers who use VSOCK, they can get away without GetQuote
> TDVMCALL support. But for customers who do not want to use
> VSOCK model, this is a required support. AFAIK, our current customer
> requirement is to use TDVMCALL approach for attestation support.
>
> If your suggestion is to split GetQuote support as separate
> patch to make it easier for review, I am fine with such
> suggestion.
>

I am not saying we should get rid of GetQuote support. If there's customer
wants this with a good reason, we can certainly support it. I understand that
some customer wants to deploy QE in host and don't want additional communication
channel (i.e. vsock) between guest and host, which may add additional attack
window and/or customer's validation resource.

My point is regardless whether we need to support GetQuote, logically this
driver can be split to two parts as I said: 1) basic TDREPORT support to
userspace; 2) additional GetQuote support. And I think there are many benefits
if you do in this way as I commented below.

> >
> > So IMHO you can split this attestation driver into two parts:
> >
> > 1) A "basic" driver which supports reporting TDREPORT to userspace
> > 2) Additional support of GetQuote/SetupEventNotifyInterrupt.
> >
> > The 1) can even be in a single patch (I guess it won't be complicated). It is
> > easy to review (and i.e. can be merged separately), and with it, you will
> > immediately have one way to support attestation.
> >
> > 2) can be reviewed separately, perhaps with one additional Kconfig option (i.e.
> > CONFIG_INTEL_TDX_ATTESTATION_GET_QUOTE). I think this part has most of the
>
>
> GetQuote IOCTL support is a very simple feature support, so, IMO, we
> don't need to complicate it with additional config.
>
> >

Additional Kconfig can reduce attack window by turning it off for people don't
need it. Anyway no strong opinion here.

--
Thanks,
-Kai