Re: [edk2-devel] [PATCH V5 1/2] OvmfPkg: Introduce Tdx BFV/CFV PCDs and PcdOvmfImageSizeInKb

["Ard Biesheuvel" <ardb@kernel.org>]

On Wed, 1 Sept 2021 at 08:10, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
>   Hi,
>
> > > I didn't fully investigate what kind of attacks one can do.  I'm pretty sure simply
> > > making the variable store larger and the spare smaller works, so parts of the
> > > variable store are outside the area you are measuring.  Not fully sure whenever
> > > one can actually reorder the sections to move the varstore completely into the
> > > unmeasured area.  Or play out other attacks with the same effect, like bloating
> > > some header struct.
> > >
> > > Simply measuring everything (including the spare) will stop all that.
> > > Changes wouldn't go unnoticed, period.  No ifs and buts.  So I'm wondering why
> > > you not doing that?  Performance?  Wouldn't be the first time a performance
> > > optimization pokes a hole into a security concept ...
> > >
> > The measurement value of the CFV (provisioned configuration data) is extended to
> > RTMR registers (similar to TPM PCRs). At the same time it is recorded in the TD Event
> > log.
> > These information will be used by the Attestation server (This is the so-called Attestation).
> > In other words there is a known *good* CFV measurement value. Any changes to
> > the CFV, for example the layout, the order of the variables, the content of the variables
> > will produce a *bad* CFV measurement.
>
> Yes.  The attacker would need a varstore with a modified layout being
> approved by the attestation server as first step, then he would be able
> to modify variables unnoticed in a second step.
>
> So, assuming an attacker isn't able to carry out the first step it
> should be all fine in theory.  When it comes to security it never hurts
> to have another line of defense though, so I would still strongly
> recommend to measure the complete varstore (including spare).
>
> At the end of the day it is your call, I'm not going to veto the patch.
> But I'll reserve the right to pull a "told you so" in case someone
> manages to exploit that some day.
>

Have to agree with Gerd here: if those contents are being interpreted
by the code, and may therefore affect its execution, I don't think it
should be omitted from the measurement unless there is a compelling
reason for it. Omitting it simply because you can doesn't seem
sufficient justification to me.

-- 
Ard.