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

Implements TCG_CompactLogHashExtendEvent Call #54

Closed
wants to merge 1 commit into from

Conversation

geedo0
Copy link

@geedo0 geedo0 commented Oct 7, 2016

This replaces the original calls to grub_TPM_int1A_hashLogExtendEvent with calls
to the compact measurement routine. We use the truncated hash of a descriptive
string for the "Informative Value" defined by TCG documentation.

Addresses Issue #53

@johnwallace123
Copy link

I can confirm that this patch fixes our issue with the HP EliteDesk 705 G2. The only difference that I've seen is that the string logged to the measurement log is now "CompactHash" where before it was "IPL".

Thanks to all for the quick fix!

@neusdan
Copy link
Contributor

neusdan commented Oct 10, 2016

Thanks for the patch. Can you have a look at the merge conflict?

This replaces the original calls to grub_TPM_int1A_hashLogExtendEvent with calls
to the compact measurement routine. We use the truncated hash of a descriptive
string for the "Informative Value" defined by TCG documentation.

Addresses Issue Rohde-Schwarz#53
@geedo0
Copy link
Author

geedo0 commented Oct 10, 2016

Looks like Git got confused by the tab/space change from 23c23c1, rebasing my changes upstream fixes it.

@johnwallace123 johnwallace123 mentioned this pull request Oct 11, 2016
@johnwallace123
Copy link

While this patch resolves the crash that I'm having, it appears that the PCRs aren't being populated as expected. My external PCR calculation tool no longer works for PCRs 10 and 11 (12 is unused, and 13 is taken as gospel). When I take a sha1sum of the kernel and initramfs, it is different than what I see in the measurement logs.

I will continue to debug and keep you apprised of anything I find.

@johnwallace123
Copy link

From my reading of the spec, it appears that TCG_CompactLogHashExtendEvent does the hashing of the buffer within the interrupt. However, the TPM code already has hashed it by the time we're looking at tpm_int1A_compactHashLogExtendEvent, so we're essentially measuring everything as SHA1(SHA1(buffer)) instead of SHA1(buffer).

@geedo0
Copy link
Author

geedo0 commented Oct 17, 2016

Good catch, I never had hashes to compare this against so it slipped past me. I'll read the spec again and see what needs to be changed.

Just passing this along, we were able to get an issue open with Lenovo to look into this. I was told it has something to do with the UEFI backwards compatibility module. So that's a +1 for issue #15 at some point.

@neusdan
Copy link
Contributor

neusdan commented Oct 17, 2016

Previously (before the changes in this PR) there was grub_TPM_int1A_hashLogExtendEvent which takes an hash and passes it to the TPM and grub_TPM_int1A_hashLogExtendEventHashTPM where the TPM does the hashing of the buffer. Because i had performance concerns i've used grub_TPM_int1A_hashLogExtendEvent everywhere. Unfortunately now with TCG_CompactLogHashExtendEvent it seems we no longer have a choice and we must pass a buffer to the TPM.

@johnwallace123
Copy link

@neusdan: It's possible that instead of using the TCG_CompactLosHasExtendEvent, we could use a TPM_Extend (via TCG_PassThrougToTPM) coupled with a TCG_HashLogEvent. It's not ideal to have 2 TPM calls, but it might be easier than attempting to retrofit the hashing that's already taking place.

@securitykernel
Copy link
Contributor

Thanks for your contribution! Unfortunately, we decided to deprecate and no longer maintain this project. I will be closing this issue.

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.

4 participants