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

preinstall: Add explicit checks for empty PCR banks #347

Merged

Conversation

chrisccoulson
Copy link
Collaborator

@chrisccoulson chrisccoulson commented Nov 13, 2024

Whilst this has no consequence for FDE because we seal against a good
bank, it breaks measured boot as required by remote verifiers, as an
empty PCR bank provides a means for an adversary to spoof any host
platform of their choosing.

We reject systems with empty PCR banks by default, but with an opt-in to
permit it. Permitting it is fine for now, but when we get to the point of a
fully verified and attestable runtime in the future, this will not be permitted.
We will need to take some action here, such as capping PCRs 0-7 with an
EV_SEPARATOR type event that indicates an error occurred in those banks.

Whilst an empty PCR bank is considered a firmware bug and we shouldn't be
seeing it on newer devices, it's not that uncommon to see this on devices from
the era when TPM2 devices started introducing SHA384 before the firmware
was ready to use it. We may want to see if we can design a scheme where we
can provide proof that a PCR bank was populated by the firmware and not by
an adversary.

@chrisccoulson chrisccoulson force-pushed the preinstall-detect-empty-pcr-banks branch from e49aa0c to 997a6d1 Compare November 13, 2024 19:17
@chrisccoulson chrisccoulson force-pushed the preinstall-detect-empty-pcr-banks branch 2 times, most recently from 4a6e606 to 82a4551 Compare November 23, 2024 22:49
@chrisccoulson chrisccoulson force-pushed the preinstall-detect-empty-pcr-banks branch from 82a4551 to 26ba971 Compare January 16, 2025 01:36
@chrisccoulson chrisccoulson force-pushed the preinstall-detect-empty-pcr-banks branch from 26ba971 to 82a4551 Compare January 16, 2025 10:54
Whilst this has no consequence for FDE because we seal against a good
bank, it breaks measured boot as required by remove verifiers, as an
empty PCR bank provides a means for an adversary to spoof any host
platform of their choosing.

We reject systems with empty PCR banks by default, but with an opt-in to
permit it.
@chrisccoulson chrisccoulson force-pushed the preinstall-detect-empty-pcr-banks branch from 82a4551 to 87e85f2 Compare January 16, 2025 10:57
@chrisccoulson chrisccoulson marked this pull request as ready for review January 16, 2025 10:57
Copy link
Collaborator

@pedronis pedronis left a comment

Choose a reason for hiding this comment

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

thx, one nitpick

internal_efi.PlatformManufacturerPCR,
internal_efi.SecureBootPolicyPCR,
},
true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

at least for the true is would be good to use a const:

const permitEmptyPCRBanks = true

before the call and use it here

@chrisccoulson chrisccoulson merged commit d70d576 into canonical:master Jan 16, 2025
2 checks passed
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.

2 participants