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

patches/coreboot-4.8.1: Measure firmware into PCR2 #793

Merged
merged 1 commit into from
Aug 11, 2020

Conversation

PatrickRudolph
Copy link
Contributor

As part of migration to coreboot 4.12, which includes measured boot
without additional patches, measure all parts of the firmware and the
payload into PCR2.

The same is done in coreboot 4.12. This commit ensures that boards not
migrated yet will show the same behaviour.

TODO: Update heads-wiki.

Signed-off-by: Patrick Rudolph [email protected]

@tlaurion
Copy link
Collaborator

tlaurion commented Aug 10, 2020

This is a big change for Heads and needs testing. Ill do the x230 right away but it need to be tested for all TPM enforcing boards and the more eyes the better. This is first step changing Heads for coreboot 4.8.1 to move away of it for newer versions.

Tagged names taken from #692:

xx20:
x220: @SebastianMcMillan @techge @eganonoa @shamen123
t420: @SebastianMcMillan

xx30:
t430: @flawedworld
x230: @tlaurion @osresearch @merge @jan23 @flawedworld @MrChromebox @shamen123 @eganonoa

Thanks

@tlaurion
Copy link
Collaborator

tlaurion commented Aug 10, 2020

Builds for xx30 will be available here in a little while(+2 hours since musl-cross-make and other boards needs to be rebuild, kernel, coreboot, modules), since I had to change the local cache environment variable from 2 to 3 to build from scratch and create a new cache ( ./build/coreboot-4.8.1 directory is created only per archive decompression on which patches are applied, requiring that directory to be wiped out for the patches to be deployed on top of extracted archive. Planned fix here. Until then, Cache variable inside of builder's CI configuration needs to be changed manually.)

@tlaurion
Copy link
Collaborator

tlaurion commented Aug 10, 2020

Not functional as is:
BadPCRUsage

High level causes:

  • Current Heads scripts using those measurements were not modified so heads still uses previously expected PCRs which changed EDIT: reviewed all Heads code: extends PCR 4-5-6-7, doesn't touch 0-1-2-3 so shouldn't conflict. Will dig more.

  • The patch provided (0061-measure-to-pcr2.patch) is applied on top of previously deployed patches ( instead of modifying the original patch files directly: 0000-measuredboot.patch 0009-Add-heads-TPM-measurements-to-Skylake-Kabylake.patch 0030-sandybridge.patch 0061-measure-to-pcr2.patch). Original files should be modified ale users cannot track from where the changes in coreboot actually come from.

Patch 0061-measure-to-pcr2.patch modifies the same source code file patches by precedent patches.

user@x230-master:~/heads/patches$ grep -Rn tlcl_measure
coreboot-4.8.1/0009-Add-heads-TPM-measurements-to-Skylake-Kabylake.patch:21:+		tlcl_measure(2, (const void*) start, size);
coreboot-4.8.1/0009-Add-heads-TPM-measurements-to-Skylake-Kabylake.patch:54:+		tlcl_measure(0, bootblock, bootblock_size);
coreboot-4.8.1/0009-Add-heads-TPM-measurements-to-Skylake-Kabylake.patch:56:+		tlcl_measure(1, _romstage, _eromstage - _romstage);
coreboot-4.8.1/0009-Add-heads-TPM-measurements-to-Skylake-Kabylake.patch:69:+		tlcl_measure(1, (const void*) start, size);
coreboot-4.8.1/0009-Add-heads-TPM-measurements-to-Skylake-Kabylake.patch:90:+		tlcl_measure(1, (const void*) dest, size);
coreboot-4.8.1/0030-sandybridge.patch:25:+		tlcl_measure(0, bootblock, bootblock_size);
coreboot-4.8.1/0030-sandybridge.patch:28:+		tlcl_measure(1, &_romstage, &_eromstage - &_romstage);
coreboot-4.8.1/0030-sandybridge.patch:55:+		tlcl_measure(2, (const void*) start, size);
coreboot-4.8.1/0000-measuredboot.patch:170:+		tlcl_measure(3, (const void*) start, size);
coreboot-4.8.1/0000-measuredboot.patch:435:+uint32_t tlcl_measure(int pcr_num, const void * start, size_t len);
coreboot-4.8.1/0000-measuredboot.patch:459:+uint32_t tlcl_measure(int pcr_num, const void * start, size_t len)
coreboot-4.8.1/0061-measure-to-pcr2.patch:9:-		tlcl_measure(0, bootblock, bootblock_size);
coreboot-4.8.1/0061-measure-to-pcr2.patch:10:+		tlcl_measure(2, bootblock, bootblock_size);
coreboot-4.8.1/0061-measure-to-pcr2.patch:12:-		tlcl_measure(1, _romstage, _eromstage - _romstage);
coreboot-4.8.1/0061-measure-to-pcr2.patch:13:+		tlcl_measure(2, _romstage, _eromstage - _romstage);
coreboot-4.8.1/0061-measure-to-pcr2.patch:21:-		tlcl_measure(1, (const void*) start, size);
coreboot-4.8.1/0061-measure-to-pcr2.patch:22:+		tlcl_measure(2, (const void*) start, size);
coreboot-4.8.1/0061-measure-to-pcr2.patch:34:-		tlcl_measure(1, (const void*) dest, size);
coreboot-4.8.1/0061-measure-to-pcr2.patch:35:+		tlcl_measure(2, (const void*) dest, size);
coreboot-4.8.1/0061-measure-to-pcr2.patch:53:-		tlcl_measure(3, (const void*) start, size);
coreboot-4.8.1/0061-measure-to-pcr2.patch:54:+		tlcl_measure(2, (const void*) start, size);
coreboot-4.8.1/0061-measure-to-pcr2.patch:66:-		tlcl_measure(0, bootblock, bootblock_size);
coreboot-4.8.1/0061-measure-to-pcr2.patch:67:+		tlcl_measure(2, bootblock, bootblock_size);
coreboot-4.8.1/0061-measure-to-pcr2.patch:70:-		tlcl_measure(1, &_romstage, &_eromstage - &_romstage);
coreboot-4.8.1/0061-measure-to-pcr2.patch:71:+		tlcl_measure(2, &_romstage, &_eromstage - &_romstage);

Which produces the right outcome while 0061 should not exist at all and modify other patches:

user@x230-master:~/heads/build/coreboot-4.8.1$ grep -Rn tlcl_measure
...
src/drivers/intel/fsp2_0/memory_init.c:494:		tlcl_measure(2, bootblock, bootblock_size);
src/drivers/intel/fsp2_0/memory_init.c:496:		tlcl_measure(2, _romstage, _eromstage - _romstage);
src/drivers/intel/fsp2_0/memory_init.c:509:		tlcl_measure(2, (const void*) start, size);
src/drivers/intel/fsp2_0/silicon_init.c:106:		tlcl_measure(2, (const void*) dest, size);
src/lib/hardwaremain.c:555:		tlcl_measure(2, (const void*) start, size);
src/security/tpm/tss/tcg-1.2/tss.c:360:uint32_t tlcl_measure(int pcr_num, const void * start, size_t len)
src/security/tpm/tss.h:152:uint32_t tlcl_measure(int pcr_num, const void * start, size_t len);
src/arch/x86/postcar.c:51:		tlcl_measure(2, (const void*) start, size);
src/northbridge/intel/sandybridge/romstage.c:84:		tlcl_measure(2, bootblock, bootblock_size);
src/northbridge/intel/sandybridge/romstage.c:87:		tlcl_measure(2, &_romstage, &_eromstage - &_romstage);
src/northbridge/intel/sandybridge/romstage.c:151:		tlcl_measure(2, (const void*) start, size);

@PatrickRudolph Will comment directly if needed while #721 (comment) was really clear.

@MrChromebox
Copy link
Contributor

I modified existing patches, built/tested on a Librem 13v2, seems to work properly from the handful of boots I did

As part of migration to coreboot 4.12, which includes measured boot
without additional patches, measure all parts of the firmware and the
payload into PCR2.

The same is done in coreboot 4.12. This commit ensures that boards not
migrated yet will show the same behaviour.

TODO: Update heads-wiki.

Signed-off-by: Patrick Rudolph <[email protected]>
@PatrickRudolph
Copy link
Contributor Author

Merged the changes into existing patches.

@tlaurion
Copy link
Collaborator

@tlaurion
Copy link
Collaborator

@tlaurion
Copy link
Collaborator

signal-2020-08-11-142501
All good.

Ready to merge.

tlaurion added a commit to linuxboot/heads-wiki that referenced this pull request Aug 11, 2020
Documentation changes linked to PCR usage in coreboot 4.8.1 and newer versions, to be applied at the same time as linuxboot/heads#793 is merged.
Linked to VBOOT+Measured boot/Measured boot changes applied directly in coreboot so that we have a common base prior of going linuxboot/heads#709 and linuxboot/heads#721 

@PatrickRudolph comments welcome
@tlaurion
Copy link
Collaborator

@PatrickRudolph would you add anything else to linuxboot/heads-wiki#42 prior of merging both at the same time?

@tlaurion tlaurion merged commit 28d3b7c into linuxboot:master Aug 11, 2020
tlaurion added a commit to linuxboot/heads-wiki that referenced this pull request Aug 11, 2020
…#42)

* Update Keys.md

Documentation changes linked to PCR usage in coreboot 4.8.1 and newer versions, to be applied at the same time as linuxboot/heads#793 is merged.
Linked to VBOOT+Measured boot/Measured boot changes applied directly in coreboot so that we have a common base prior of going linuxboot/heads#709 and linuxboot/heads#721 

@PatrickRudolph comments welcome

* Update Keys.md

Removed comment about MRC needing to be measured into different PCR since coreboot is merging them all under PCR2 in newer VBOOT+measured boot/measuredboot
Ref:https://doc.coreboot.org/security/vboot/measured_boot.html
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