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

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

Comment below only:

> 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.

I don't understand why you want separate them.  Improving OvmfPkg
security is good for both OVMF and TDVF.  For TDVF it is clearly much
more important due to the different threat model, but it is good for
OVMF too.  Likewise edk2 in general.

[Jiewen] My answer is very clear as I mentioned multiple times.
The threat model is different.
IMHO, talking about "Improving OvmfPkg security" without a clear threat model is *meaningless*.
All mitigation must be based upon threat mode and security objective.
Otherwise, what you are doing might be either unnecessary or insufficient.



> 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?

Bugs happen.  There could also be bugs in the additional SEC code you
need for platform setup in a non-PEI configuration ...

[Jiewen] I agree. That is why we need smaller code.
We can just focus on review that small portion of code what we have written for TDVF, instead of the whole PEI.
We have process to review and maintain the extra TDVF code.

I think it is totally *unfair* that you force me to add PEI, without knowing the quality of PEI.


Thank you
Yao Jiewen


> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Tuesday, November 23, 2021 8:38 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,
> 
> > > 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.
> 
> Yes, of course.  But even though there are differences in both
> implementation and supported features both platforms have similar goals
> and there is quite some overlap in concepts too.
> 
> > 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.
> 
> Yes.  Whenever they are close enough that merging them makes sense
> remains to be seen.
> 
> > > 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.
> 
> On the other hand there are probably more eyes are looking at PEI Core
> because it is used by a lot of platforms, increasing the chance that
> bugs are actually spotted.
> 
> > 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.
> 
> The maintenance problem isn't a non-PEI flow.  If a platform chooses
> that -- fine.  The problem having to maintain both PEI and non-PEI
> workflow.
> 
> > [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".
> 
> I never stated "config-B should be adopt what AmdSev chooses".
> 
> I stated "TDVF boot workflow should be simlar to the other OVMF
> platforms to simplify maintenance".
> 
> AmdSev is just an example showing that you can easily strip down the
> firmware  build without putting the boot workflow upside down (which one
> of the things config-b wants too).
> 
> > > 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.
> 
> Does not using PEI phase have any advantages from a security point of
> view (other than "not using PEI Core code which might have bugs")?
> 
> > > 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.
> 
> Do you have a concrete example for that?
> 
> I can't think of a good reason to skip checks on OVMF.  When we -- for
> example -- review and improve virtio drivers to make sure they can't be
> used by the VMM to exploit a TDVF guest:  We surely would use the
> improved sanity checks on OVMF too, for better OVMF stability and also
> for better test coverage of the sanity checks.
> 
> > > 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.
> 
> Yes.  We have that in quite a few places.  IoLib for example.
> It's required for Config-A, obviously.
> 
> So, what is your plan for IoLib for Config-B?
> 
> > 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.
> 
> Sure.  Config-A will need some drivers from OvmfPkg/AmdSev/ so both SEV
> and TDX work, whereas Config-B will not.
> 
> > I even propose config-a skip PEI phase.
> 
> Care to back your proposal by posting patches to the list?
> 
> I think it's easier to discuss the advantages + disadvantages
> with concrete code at hand.
> 
> > 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.
> 
> I don't understand why you want separate them.  Improving OvmfPkg
> security is good for both OVMF and TDVF.  For TDVF it is clearly much
> more important due to the different threat model, but it is good for
> OVMF too.  Likewise edk2 in general.
> 
> > 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?
> 
> Bugs happen.  There could also be bugs in the additional SEC code you
> need for platform setup in a non-PEI configuration ...
> 
> take care,
>   Gerd