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 checks for platform firmware protections #316

Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion efi/preinstall/check_fw_protections_amd64.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,19 @@ func checkPlatformFirmwareProtections(env internal_efi.HostEnvironment, log *tcg
return 0, fmt.Errorf("encountered an error when determining platform firmware protections using Intel MEI: %w", err)
}
if amd64Env.HasCPUIDFeature(cpuid.SMX) {
// The Intel TXT spec says that locality 3 is only available to ACMs
// The Intel TXT spec says that locality 4 is basically only avilable
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo: available

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've fixed this now.

// to microcode, and is locked before handing over to an ACM which
// has access to locality 3. Access to this is meant to be locked at the
// hardware level before running non-Intel code, although I'm not sure if
// this is only relevant in the D-CRTM case where the SINIT ACM has access
//to locality 3, and it locks access to it, leaving access to localities 2
Copy link
Collaborator

Choose a reason for hiding this comment

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

strange formatting

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It looks like this one was already fixed in a recent push I made

// and 1 to the measured launch environment and dynamic OS respectively. We
// rely on the property of locality 3 being protected somewhat in order to
// attempt to mitigate discrete TPM reset attacks on Intel platforms (basically
// by including PCR0 in the policy, even though it's otherwise useless to include
// it, but locality 3 access is required in order to reconstruct PCR0 after a TPM
// reset. Mark locality 3 as protected if we have the right instructions for
// implementing a D-CRTM with Intel TXT (which I think is SMX).
result |= platformFirmwareProtectionsTPMLocality3IsProtected
}
case cpuVendorAMD:
Expand Down
Loading