Re: [Qemu-devel] [PATCH v2 8/8] ovmf: add DxeTpm2MeasureBootLib

[Laszlo Ersek]

On 03/09/18 01:39, Yao, Jiewen wrote:
> Very good question.
> Comment below:
>
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:address@hidden
>> Sent: Friday, March 9, 2018 3:54 AM
>> To: address@hidden; address@hidden; Yao, Jiewen
>> <address@hidden>
>> Cc: address@hidden; address@hidden;
>> address@hidden; address@hidden
>> Subject: Re: [PATCH v2 8/8] ovmf: add DxeTpm2MeasureBootLib

>> However, it would be a problem for UEFI_DRIVER modules / apps
>> that come from external media (disk, network, PCI oprom, etc).

> [Jiewen] By design, the 3rd part module should not be invoked before
> EndOfDxe. All Arch Protocol Ready is not strong enough. :-) Please
> refer to
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.c
>
> If a non-FV image is loaded before EndOfDxe, it will be queued into
> mDeferred3rdPartyImage.

OK, thank you -- I suspected image deferral was somehow related to this.
I remember deferred execution from the OVMF log, even though at present
we hook only DxeImageVerificationLib into SecurityStubDxe.

> We also added EfiBootManagerDispatchDeferredImages() API in
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Include/Library/UefiBootManagerLib.h
> and implemented in
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
> A platform must call EfiBootManagerDispatchDeferredImages(), if the
> platform supports PCI OROM.

Yep, OVMF does that already. In
"OvmfPkg/Library/PlatformBootManagerLib", we have the following order of
operations, in PlatformBootManagerBeforeConsole():

(a) connect all PCI root bridges non-recursively (this produces PciIo
    instances and discovers the PCI oproms)

(b) kick QEMU so that it generates the ACPI tables that OVMF has to
    install (this depends on PciIo instances existing, from step (1a)),
    and actually install those tables

(c) signal End-of-Dxe (this depends on step (1b), because S3 support
    needs the FACS table coming from step (1b))

(d) write an INFO opcode to the S3 boot script so that the boot script
    is never empty

(e) install DxeSmmReadyToLock (this may come only after steps (1c) and
    (1d))

(f) we call EfiBootManagerDispatchDeferredImages(). This is from Ray's
    commit 9789894e3ba3 ("OvmfPkg/PlatformBds: Dispatch deferred images
    after EndOfDxe", 2016-11-10).

(The rest of PlatformBootManagerBeforeConsole() skipped here.)

So, we have all the right operations in place, I just missed that non-FV
images loaded before End-of-Dxe (such as PCI oproms) are deferred until
step (f), and that the deferral will cover the TPM measurements too.

This is great, because it's an even stronger guarantee than what I would
have wished for. I mean the argument I wrote up earlier -- about oproms
not being dispatched before entering BDS, at which point Tcg2Dxe will
have been dispatched already -- was already good enough, but now I know
that PCI oproms are delayed even *within* BDS until after we explicitly
end DXE and dispatch the deferred images.

> You can find the sample code in
> https://github.com/tianocore/edk2-platforms/blob/devel-MinPlatform/Platform/Intel/MinPlatformPkg/Bds/Library/DxePlatformBootManagerLib/BdsPlatform.c
>
>
>
>> However, such are loaded only in the BDS phase, and BDS is only
>> entered after all of the DXE drivers are dispatched from the firmware
>> volumes. In other words, the ordering between Tcg2Dxe and external
>> UEFI_DRIVER / UEFI_APPLICATION modules is ensured that Tcg2Dxe will
>> be dispatched in the DXE phase, while the latter will only be loaded
>> in BDS.
>>
>> Is this intentional? Is my understanding correct?
>
> [Jiewen] Right. The only assumption is: Tcg2Dxe is included in the
> firmware volume and it is dispatched before EndOfDxe.

Perfect. Thank you.

So, Marc-André, I would request the following commit message *addition*
(not replacement!) then:

--------------
The following order of operations ensures that 3rd party UEFI modules,
such as PCI option ROMs and other modules possibly loaded from outside
of firmware volumes, are measured into the TPM:

(1) Tcg2Dxe is included in DXEFV, therefore it produces the TCG2
    protocol sometime in the DXE phase (assuming a TPM2 chip is present,
    reported via PcdTpmInstanceGuid).

(2) The DXE core finds that no more drivers are left to dispatch from
    DXEFV, and we enter the BDS phase.

(3) OVMF's PlatformBootManagerLib connects all PCI root bridges
    non-recursively, producing PciIo instances and discovering PCI
    oproms.

(4) The dispatching of images that don't originate from FVs is deferred
    at this point, by
    "MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.c".

(5) OVMF's PlatformBootManagerLib signals EndOfDxe.

(6) OVMF's PlatformBootManagerLib calls
    EfiBootManagerDispatchDeferredImages() -- the images deferred in
    step (4) are now dispatched.

(7) Image dispatch invokes the Security / Security2 Arch protocols
    (produced by SecurityStubDxe). In this patch, we hook
    DxeTpm2MeasureBootLib into SecurityStubDxe, therefore image dispatch
    will try to locate the TCG2 protocol, and measure the image into the
    TPM2 chip with the protocol. Because of step (1), the TCG2 protocol
    will always be found and used (assuming a TPM2 chip is present).
--------------

Jiewen, does this look OK?

Thanks!
Laszlo