Re: [PATCH V3 15/29] OvmfPkg: Update SecEntry.nasm to support Tdx

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

> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Friday, November 19, 2021 11:12 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Xu, Min M <min.m.xu@intel.com>; devel@edk2.groups.io; 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>
> Subject: Re: [PATCH V3 15/29] OvmfPkg: Update SecEntry.nasm to support Tdx
> 
>   Hi,
> 
> > Comment on config-B.
> 
> > > I'm sure I've asked this before:  Why skip the PEI phase?  So far
> > > I have not seen any convincing argument for it.
> >
> > Skipping PEI phase is valid architecture design.
> 
> Sure.
> 
> > Second, the confidential computing changes the threat model
> > completely.  One of our goal is to simplify the design for CC-firmware
> > (TDVF) - remove unnecessary modules, remove unnecessary interface,
> > make the image smaller and faster.  That will reduce the validation
> > effort, too.
> >
> > That is the main motivation.
> 
> That totally makes sense.  I expect TDVF Config-B will look alot like
> the existing AmdSev configuration variant which is stripped down too.

[Jiewen] I don't think TDVF config-B will be like the AMD SEV is right statement.
TDVF and SEV are two different platforms.
Intel mainly focuses on TDVF and we will let AMD defines the feature set in SEV.
They MAY be alike if possible.
But difference is also acceptable if there is architecture difference or different decision in different company.


> No SMM support, no network stack, ...
> 
> There wouldn't be left much in PEI beside PeiCore and
> OvmfPkg/PlatformPei.
> 
> But I don't see how dropping the PEI phase altogether helps much in
> stripping down the firmware image.  The initialization currently handled
> by OvmfPkg/PlatformPei must happen somewhere else instead.  Given SEC is
> a very restricted environment I don't expect the code can be shared
> easily, so we will probably end up with code duplication.  Also two
> different boot workflows which I fear can easily introduce subtle bugs
> due to differences like a initialization order changes.  This is what I
> see as maintenance problem.

[Jiewen] I don't think this is right statement.
In Tiano history, there were security bugs exposed in PEI phase, even the PEI Core on FV processing.
I do see the value to skip PEI core.

Again, I am very familiar with non-PEI flow.
Back to 10 years ago, I was maintainer of a non-PEI platform (named DUET) and we jumped from SEC to DXE directly.
I don't see any maintenance problem.


> 
> > Config-A is to keep current architecture, to maximum compatible with
> > OVMF. And we don't remove VMM out of TCB.
> 
> > Config-B is to have a new TDVF design, to maximum satisfy the security
> > requirement. And we remove VMM out of TCB.
> 
> Sure.
> 
> config-a is ovmf with tdx support added, all uefi features present, only
> basic tdx/sev support (basically support memory encryption).
> 
> config-b is simliar to AmdSev (maybe we'll merge them some day),
> stripped down uefi feature set (no network etc), full tdx support
> including attestation etc.

[Jiewen] I think we are debating two different things.
Your statement that "config-B is similar to AmdSev" does not support the statement "config-B should be adopt what AmdSev chooses".
TDVF config-B is proposed by Intel. AMD SEV is proposed by AMD. I respect SEV people and I *may* propose something, but I will ack their decision.
I would not force them to accept the TDVF config-B. And vice versa.



> 
> I don't want question all that.  I still don't see the point in dropping
> the PEI phase and make config-b work different that all other ovmf
> variants though.

[Jiewen] My point is simple - Threat Model is different.
That causes security objective difference and design difference.
Each CC env owner should have freedom to choose what they want to enforce.
IMHO, they are the policy decision maker.



> > > Jiewen argued this is a simplification.  Which is not completely wrong,
> > > but it's also only half the truth.  Switching all OVMF builds over to
> > > PEI-less boot doesn't work because some features supported by OVMF
> > > depend on PEI Modules.  Therefore TDX Config-B skipping the PEI phase
> > > means we would have to maintain two boot work flows (with and without
> > > PEI phase) for OVMF.  Which in turn would imply more work for
> > > maintenance, testing and so on.
> >
> > [Jiewen] I am not asking your to OVMF build to PEI-less.
> > But if you want to do, I will not object.
> 
> s3, smm, tpm and maybe more depends on pei modules, so dropping the PEi
> phase is not an option for a full-featured OVMF build.  So I'd very much
> prefer all ovmf variants (including tdvf) use the PEI phase.
> 
> > On contrast, if we keep PEI for config B, it adds extra burden from
> > security assurance perspective.  That means, every issue in PEI may be
> > exposed to TDVF.
> 
> Same for all other modules used by tdvf.
> 
> The list of affected PEI modules is rather short though.  I think it's
> only PeiCore and DxeIpl.  PlatformPei doesn't count as the code wouldn't
> go away but would be moved to SEC (and maybe parts of it to DXE).
> 
> > Comparing the effort to maintain the work flow and the effort to
> > handle potential security issue, I would choose to maintain the work
> > flow.  I have experience to wait for 1 year embargo to fix EDKII
> > security issue, it is very painful and brings huge burden.
> 
> The security workflow is a serious problem indeed.  Not only for TDVF,
> also for OVMF in general, and other platforms too.  We should certainly
> try to improve it.

[Jiewen] If you look at how we state config-A and config-B again, you will find we made difference statement.
I copy it here again.
1) Config-A is to keep current architecture, to maximum compatible with OVMF. And we don't remove VMM out of TCB.
2) Config-B is to have a new TDVF design, to maximum satisfy the security requirement. And we remove VMM out of TCB.

Because of the threat model difference, in config-A, we can safely make some assumption that the VMM is benign and VMM will not input malicious data. As such, we might not perform data validation. We just trust VMM input.
However, in config-B, VMM is malicious, which means we need be careful to NOT trust VMM at any time.

The code in config-A and config-B may do totally different thing to handle the difference situation.

I don't think it is hidden assumption that if TDVF need do some check, then a generic OVMF need do this check.
If OVMF trusts the VMM, the check might be totally unnecessary.



> I'm not going to open that discussion in this thread.  But let me drop
> two links two links to osfc talk and workshop (Not 30th + Dec 1st),
> titled "The firmware supply-chain security is broken: can we fix it?"
> 
> https://talks.osfc.io/osfc2021/talk/D9X39Z/
> https://talks.osfc.io/osfc2021/talk/E9YYJF/
> 
> > > I want TDVF be consistent with the rest of OVMF.  Makes long-term
> > > maintenance easier.  Building a single binary for both SEV and TDX with
> > > full confidential computing support (including config-b features) will
> > > be easier too.
> >
> > [Jiewen] I am not convinced that TDVF be consist with rest of OVMF.
> > The goal of project is different. The choice can be different.
> > I don't see a reason that one platform must be in this way, just because other
> platform does in this way.
> 
> Hmm?  Seeing TDVF as "other platform" is a rather strange view given
> that we are integrating tdx support into OVMF right now ...

[Jiewen] This is how Intel views the "platform".
In history, we call this one binary mode is "multiple-platform" or "multiple-SKU".
That means we only have one binary, and this single binary can boot different platforms, which has similar CPU or silicon but different platform board design.
And there will be platform specific code flow, such as
Switch (PlatformId) {
Case PlatformA:
   {do platformA init}
Case PlatformB:
   {do platformB init}
}

If you treat CC_TYPE to be platformID, it perfectly matches.

Also a platform may has extra module (a driver or an FV) to support the platform specific feature. Or a platform may much simpler design and skip some drivers.
The actual multi-platform design is even more complicated, because we also have group concept. So I will stop the discussion here.



> 
> > I think a PEI-less TDVF is much easier to maintain, comparing with
> > complicated OVMF flow, at least from security perspective.  The less
> > code we have, the less issue we have.
> 
> Well, we will have TDX support in the normal OVMF workflow anyway,
> because we need that for config-a.  Why use and maintain something
> different for config-b?  That looks rather pointless to me ...

[Jiewen] I think I have explained a lot above.
The key difference between config-a and config-b is threat mode.
I even propose config-a skip PEI phase. I am persuaded to let config-a adopt the OVMF way, because the threat model of config-A is same as the normal OVMF.
But config-B is NOT.
Different threat model drives different solution.
I completely don't understand why they must be same.

If you force me to add PEI to config-B. Finally, that causes TDVF config-B is compromised due to an issue in PEI.
Who will take the responsibility? Will you or RedHat take that?






> 
> take care,
>   Gerd