Re: [edk2-devel] [PATCH 18/23] OvmfPkg: Enable Tdx in SecMain.c

["Yao, Jiewen" <jiewen.yao@intel.com>]

Comment below:

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd
> Hoffmann
> Sent: Thursday, August 26, 2021 4:32 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: devel@edk2.groups.io; Ard Biesheuvel <ardb@kernel.org>; Xu, Min M
> <min.m.xu@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Justen,
> Jordan L <jordan.l.justen@intel.com>; Brijesh Singh <brijesh.singh@amd.com>;
> Erdem Aktas <erdemaktas@google.com>; James Bottomley
> <jejb@linux.ibm.com>; Tom Lendacky <thomas.lendacky@amd.com>;
> Yamahata, Isaku <isaku.yamahata@intel.com>
> Subject: Re: [edk2-devel] [PATCH 18/23] OvmfPkg: Enable Tdx in SecMain.c
> 
>   Hi,
> 
> > Some reference for QEMU:
> > https://lists.nongnu.org/archive/html/qemu-devel/2021-07/msg01682.html
> 
> Ah, good.  /me adds an entry to the todo list.
> 
> > > > The fw_cfg is still allowed in the TDVF design guide, just because we
> > > > feel it is a burden to convert everything suddenly.
> > >
> > > What is the longer-term plan here?
> > >
> > > Does it make sense to special-case the memory map?
> > >
> > > If we want handle other fw_cfg items that way too later on, shouldn't we
> > > better check how we can improve the fw_cfg interface so it works better
> > > with confidential computing?
> >
> > [Jiewen] So far, my hope is to limit the fw_cfg as much as possible.
> > My worry is that we have to measure fw_cfg everywhere. If we miss one place,
> it will be a completeness vulnerability for trusted computing.
> >
> > I also think if we can add measurement code inside of fw_cfg get function.
> > Then we need improve the FwCfg API - Current style: QemuFwCfgSelectItem()
> + QemuFwCfgReadxxx() is not friendly for measurement. For example, we can
> combine them and do QemuFwCfgSelectRead ().
> 
> I was more thinking about a completely different way to pass (constant)
> fw_cfg data.  Something like defining a fw_cfg hob and adding that to the
> td hob.  QemuFwCfgLib could lookup the hob and use that when it finds
> the needed entry there.
> 
> In case the entry is not there try use io instead.  We'll continue to
> need that for the acpi tables for example, these entries are not
> constant.  qemu will adapt them when the firmware maps hardware
> resources referenced in acpi tables (mmconfig region, power management
> registers, ...).

[Jiewen] That is great idea. I really like it.


> 
> > The QemuFwCfgWritexxx() interface may also bring inconsistency issue.
> > If we use this API, we have 2 copy data.
> 
> Do you need any writable fw_cfg entries in TDX mode?

[Jiewen] I hope NOT to support writable fw_cfg.
In our TDX design, we even don't want to support SetVariable to NV Storage, just to reduce the risk.


> 
> 'git grep' shows the ramfb driver, smi feature negotiation and s3
> support use QemuFwCfgWrite()

[Jiewen] TDVF does not support SMM, and TDVF does not support S3. 

> 
> > One is in TDVF (trusted), and
> > the other is in VMM/QEMU (untrusted). What if the VMM modifies its
> > untrusted copy?
> 
> > What I can see is many potential attack surfaces. :-(
> 
> Well, you have to trust VMM/QEMU to a certain degree.  TDX can prevent
> data leaking, but it can't prevent VMM misbehaving.

[Jiewen] Yes, you are right. It is "in certain degree".

The threat model is :
TD cannot resist the deny-of-service (DOS) attack from VMM/QEMU.
TD need maintain the integrity and confidentiality, to avoid tamper and information disclosure.


If VMM misbehaving causes the system hang or guest device error, it is OK.
But if VMM misbehaving causes a TD secret leak to QEMU or TD tampered without being detected by measurement register (MRTD or RTMR), that is NOT acceptable.

If we allow the misbehaving, then we have to do thorough analysis to understand the impact.
If we can think of a way to avoid the possibility of misbehaving, then we know we are good. :-) That is our preference so far.


> 
> > Please let me know if you need any other information.
> 
> Sure.  For now I have to read more docs and patches ...
> 
> take care,
>   Gerd
> 
> 
> 
> 
>