Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Publish EDK's TPM eventlog that contains coreboot's entries #139

Open
wants to merge 5 commits into
base: dasharo
Choose a base branch
from

Conversation

SergiiDmytruk
Copy link
Member

@SergiiDmytruk SergiiDmytruk commented Jun 12, 2024

Missing entries from coreboot's log are filled in as 0x01 0x00.... which affects expected SHA-1 PCR-2 value computed by tpm2_eventlog, but TCG specification says that all entries need to have identical set of digests listed in the same order, hence I've added those fake values as described by TXT spec because I don't think TCG covers such a case.

See commit messages for more details.

coreboot PR: Dasharo/coreboot#517

UefiPayloadPkg/Library/CbParseLib/CbParseLib.c Outdated Show resolved Hide resolved
SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c Show resolved Hide resolved
Copy link
Member Author

@SergiiDmytruk SergiiDmytruk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Force-pushed to get rid of a stray formatting fix in CbParseLib.c.

UefiPayloadPkg/Library/CbParseLib/CbParseLib.c Outdated Show resolved Hide resolved
SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c Show resolved Hide resolved
@macpijan
Copy link
Contributor

@miczyg1 any more comments here?

@miczyg1
Copy link
Contributor

miczyg1 commented Jul 16, 2024

@miczyg1 any more comments here?

Actually not anymore. I would like to give this PR and this a try on some HW that has a socketable TPM 1.2 and TPM 2.0. Dasharo/coreboot#517

@SergiiDmytruk
Copy link
Member Author

Added a fix to make measurements of Dasharo variables reproducible (c0078b2) here because it's kind of related. https://github.com/Dasharo/DasharoModulePkg has the issue as well, but don't know if we need to fix it there.

@miczyg1
Copy link
Contributor

miczyg1 commented Jul 18, 2024

https://github.com/Dasharo/DasharoModulePkg has the issue as well, but don't know if we need to fix it there.

We should, there are still releases coming that will use DasharoModulePkg submodule in EDKII

Find the log using DasharoPayloadPkg/CbParseLib in
DasharoPayloadPkg/BlSupportPei and create HOBs like those produced by
TcgPei and Tcg2Pei all of which will be picked up by TcgDxe and Tcg2Dxe.

TPM1 case is quite simple:
 - use coreboot's Spec ID Event as EDK doesn't seem to add one of its
   own

TPM2 case is more advanced and is more complicated:
 - don't create a HOB for coreboot's Spec ID Event (the first entry)
   because TPM2 can have multiple digests and coreboot produces at most
   one
 - when importing HOBs in Tcg2Dxe add missing hashes of OneDigest kind
   from TXT spec (0x01 followed by 0x00 bytes) just to not come up with
   some custom placeholder

Signed-off-by: Sergii Dmytruk <[email protected]>
Basically a copy&paste from Tcg2Smm.  Intentionally not making any
changes (like dropping use of PCDs to pass data) beyond what's necessary
to make it work.

No need for an analogous change for TPM1 because TcgDxe already
publishes the log.

Signed-off-by: Sergii Dmytruk <[email protected]>
This fixes "SecurityPkg: measure Dasharo variables before boot".

gRT->GetNextVariableName() doesn't return variables in any fixed order.
Seems like the order matches order in SMMSTORE.  This means that
measuring variables while enumerating them will produce different
results depending on which variables were update last (setting a
variable in SMMSTORE is marking old entry as deleted and appending of a
new one).  Sort list of variables that share the same GUID before
measuring any of them to impose a fixed order.

Also fix spacing in several places.

Signed-off-by: Sergii Dmytruk <[email protected]>
Prior to this change the code could only disable banks unsupported by
the BIOS and not enable those which are supported.  This resulted in not
touching TPM configuration if an unsupported bank was already selected
instead of automatically switching on supported banks.

Signed-off-by: Sergii Dmytruk <[email protected]>
Selecting them won't result in enabling them, so they shouldn't show up
in the UI.

Signed-off-by: Sergii Dmytruk <[email protected]>
@SergiiDmytruk
Copy link
Member Author

Rebased.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants