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.

Sign up for GitHub

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

Jump to bottom

[do not merge] Combined TPM eventlog #517

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

[do not merge] Combined TPM eventlog #517

wants to merge 10 commits into from

Conversation

SergiiDmytruk
Copy link
Member

@SergiiDmytruk SergiiDmytruk commented Jun 12, 2024 •
edited
Loading

Actual code changes are in EDK, coreboot only needs to not publish related ACPI entries.

Testing on APUs won't work without reverting modifications in src/drivers/pc80/tpm/tis.c which somehow disables a working TPM.

EDK PR: Dasharo/edk2#139

This now also includes picking TCG log format at runtime.

@SergiiDmytruk SergiiDmytruk mentioned this pull request Jun 12, 2024
Open
@SergiiDmytruk SergiiDmytruk mentioned this pull request Jul 4, 2024
Open
krystian-hebel
krystian-hebel previously approved these changes Jul 5, 2024
View reviewed changes
Copy link
Contributor

@krystian-hebel krystian-hebel left a comment

Choose a reason for hiding this comment

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

Approved, but edk2 must be merged first.

@m-iwanicki
Copy link

@SergiiDmytruk Not sure if it's due to your PR but when I tested latest artifact: https://github.com/Dasharo/coreboot/actions/runs/9800367204/job/27062155366 on Protectli VP2420 Dasharo System Features menu disappeared.

@krystian-hebel
Copy link
Contributor

@SergiiDmytruk Not sure if it's due to your PR but when I tested latest artifact: https://github.com/Dasharo/coreboot/actions/runs/9800367204/job/27062155366 on Protectli VP2420 Dasharo System Features menu disappeared.

Probably caused by lack of Dasharo/edk2#148 after the bug was introduced in Dasharo/edk2#144 / Dasharo/DasharoModulePkg#50

@SergiiDmytruk
Copy link
Member Author

@m-iwanicki I saw that but assumed a messed up .config. It's actually an issue fixed by Dasharo/edk2@d130aec commit which was missing on rebased. Added it there and updated both PRs to include that fix.

@miczyg1
Copy link
Contributor

miczyg1 commented Jul 8, 2024

Testing on APUs won't work without reverting modifications in src/drivers/pc80/tpm/tis.c which somehow disables a working TPM.

This was supposed to fix it: d3b8531

Do we have some insights why it doesn't work/what is the problem exactly?

@SergiiDmytruk
Copy link
Member Author

Testing on APUs won't work without reverting modifications in src/drivers/pc80/tpm/tis.c which somehow disables a working TPM.

This was supposed to fix it: d3b8531

Do we have some insights why it doesn't work/what is the problem exactly?

This PR is older than that commit.

@miczyg1
Copy link
Contributor

miczyg1 commented Jul 16, 2024 •
edited
Loading

@macpijan do we have some working setups with socketable TPM1.2 and TPM2.0 in the lab?

@krystian-hebel
Copy link
Contributor

IIRC one of KGPEs should still have TPM1.2 which was used for AEM. Note that it currently has ASUS BIOS, on 16Mb chip.

@miczyg1
Copy link
Contributor

miczyg1 commented Jul 16, 2024

IIRC one of KGPEs should still have TPM1.2 which was used for AEM. Note that it currently has ASUS BIOS, on 16Mb chip.

@krystian-hebel something that will run on this coreboot version :)

@SergiiDmytruk
Copy link
Member Author

Force-pushed to get deterministic variable measurements.

@m-iwanicki m-iwanicki mentioned this pull request Jul 30, 2024
Merged
@SergiiDmytruk SergiiDmytruk mentioned this pull request Aug 7, 2024
Open
@SergiiDmytruk
configs: bump Dasharo EDK2 to get combined TPM log
1de3f15 
Change-Id: Iacee23253d2766d9f3835860d0d64d84b5bcd880
Signed-off-by: Sergii Dmytruk <[email protected]>
@SergiiDmytruk
acpi: don't publish TPM logs if EDK payload is used
7630081 
EDK will publish its own version of the log after parsing and
importing coreboot's log discovered through CBMEM.

Publishing is done by appending a table, so coreboot must avoid
adding corresponding log tables to let OS find a more complete one
from EDK.

Change-Id: Iec92c2b5c426ee003e81996937862d81cb4ead24
Signed-off-by: Sergii Dmytruk <[email protected]>
@SergiiDmytruk
drivers/crb: use crb_tpm_ prefix instead of tpm2_
189f5eb 
This prevents name clashes with drivers/spi/tpm and allows both to be
potentially compiled in at the same time.

Change-Id: I0aa2686103546e0696ab8dcf77e2b99bf9734915
Signed-off-by: Sergii Dmytruk <[email protected]>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/81860
Tested-by: build bot (Jenkins) <[email protected]>
Reviewed-by: Julius Werner <[email protected]>
@SergiiDmytruk
security/tpm: support compiling in multiple TPM drivers (upstreamed v…
eab21e1 
…ersion)

Starting from here CONFIG_TPM1 and CONFIG_TPM2 are no longer mutually
exclusive.

Change-Id: I44c5a1d825afe414c2f5c2c90f4cfe41ba9bef5f
Ticket: https://ticket.coreboot.org/issues/433
Signed-off-by: Sergii Dmytruk <[email protected]>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/69162
Reviewed-by: Julius Werner <[email protected]>
Tested-by: build bot (Jenkins) <[email protected]>
Signed-off-by: Sergii Dmytruk <[email protected]>
@SergiiDmytruk
security/tpm: replace TPM_MEASURE_ALGO with tpm_log_alg()
c4a1abe 
No functional changes.  This replaces a macro with an inline function to
make code more readable and more convenient to extend in the future.

Change-Id: I456bc3bb749a9b58fba72f5562195525e55290bf
Signed-off-by: Sergii Dmytruk <[email protected]>
@SergiiDmytruk
security/tpm: add CONFIG_TPM_LOG_TCG
70a0c6a 
This event log format option automatically selects TCG log format
depending on which TPM is present.

Change-Id: I1997396f24ff6362fe64ac56f8e61efcf2ffb0f7
Signed-off-by: Sergii Dmytruk <[email protected]>
@SergiiDmytruk
configs: run savedefconfig on Dasharo EDK boards
2825d4c 
iPXE configuration of the boards were broken after a rebase, so this
also involved setting CONFIG_BUILD_IPXE=y which now enables build.

Change-Id: Ib2af25f7259c31f2e89dc10f473331733c255796
Signed-off-by: Sergii Dmytruk <[email protected]>
@SergiiDmytruk
configs: bump Dasharo EDK2 to get complete PCR banks control
a681eb3 
Change-Id: I7117aa165596ea3b25d7582ebdad3a8591a72f38
Signed-off-by: Sergii Dmytruk <[email protected]>
@SergiiDmytruk
Copy link
Member Author

Rebased: updated EDK2 hash which also fixed merge conflict.

SergiiDmytruk and others added 2 commits August 14, 2024 17:56
@SergiiDmytruk
payloads/edk2: ask EDK2 to disable PCR banks unsupported by coreboot
1cb3194 
coreboot can only extend a single PCR bank of TPM2 and this change
results in all available PCRs being extended.

This is an alternative to making coreboot extend all active PCRs.

Change-Id: I4e21ab77f191e9b36cb467cd61ad0a3e347035cb
Signed-off-by: Sergii Dmytruk <[email protected]>
@macpijan
config: pcengines: v0.9.1-rc1
2e69210 
Change-Id: I8573a65bdbd7727dc11f0634fdbd62711b36ddac
Signed-off-by: Maciej Pijanowski <[email protected]>
@macpijan macpijan changed the title Combined TPM eventlog [do not merge] Combined TPM eventlog Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@krystian-hebel krystian-hebel krystian-hebel left review comments

@miczyg1 miczyg1 Awaiting requested review from miczyg1

@mkopec mkopec Awaiting requested review from mkopec

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@SergiiDmytruk @m-iwanicki @krystian-hebel @miczyg1 @macpijan