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

Mitigations against event type manipulation in UEFI eventlog #816

Merged
merged 3 commits into from Jan 12, 2022

Conversation

THS-on
Copy link
Member

@THS-on THS-on commented Dec 23, 2021

This provides the initial hardening of the example policy against event type manipulation.
This not directly an issue with Keylime, because the UEFI eventlog does not include the metadata in the hash that is used to extend the PCR. The original issue can be found here: https://github.com/google/go-attestation/blob/master/docs/event-log-disclosure.md

The most effective way to mitigate against that is to reduce the usage of acceptAll and limit how often they can be used.

As discussed with @mpeters we will first update our example policy and then provide documentation on how to write custom policies.

@THS-on
Copy link
Member Author

THS-on commented Jan 3, 2022

@kkaarreell the Fedora rawhide tests are currently failing. Is there something that we can fix them or can we just disable them until the issue in rawhide itself is solved?

@kkaarreell
Copy link
Contributor

According to @sergio-correia same patches as in
https://bugzilla.redhat.com/show_bug.cgi?id=1984634
are missing in rawhide.
Sergio, did you open a rawhide bug regarding it?

Re tests, do you mean disable them all or just on Rawhide?

@sergio-correia
Copy link
Contributor

@kkaarreell: there was a bug open for it already [1], so I just added a comment there some days ago. The maintainer of tpm2-tss in Fedora is currently waiting for a newer release -- to be released soon(tm) --, which should support OpenSSL 3.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=2008179

@kkaarreell
Copy link
Contributor

@THS-on Would you mind adding a commit that replaces fedora-all with fedora-stable in . packit. yaml?

@THS-on
Copy link
Member Author

THS-on commented Jan 3, 2022

@kkaarreell done. The tests now pass.

EvSeperatorTest and EvEfiActionTest are test for common constant values
in the event log.

OnceTest is a test that can be only run once successfully. This is useful
for events that we only expect once in the log. It prohibits that this
test can be reused to validate other entries by changing the event type.

Signed-off-by: Thore Sommer <mail@thson.de>
Signed-off-by: Thore Sommer <mail@thson.de>
Signed-off-by: Thore Sommer <mail@thson.de>
@mpeters
Copy link
Member

mpeters commented Jan 10, 2022

This looks good to me, but probably needs someone from @maugustosilva 's team to look it over since they did the initial implementation.

@mpeters
Copy link
Member

mpeters commented Jan 11, 2022

@maugustosilva any chance you could take a look at these changes?

@maugustosilva
Copy link
Contributor

Passes all my tests, let's merge

@mpeters mpeters merged commit 97bf8c5 into keylime:master Jan 12, 2022
@THS-on THS-on deleted the mb-mitigations branch February 6, 2022 18:26
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.

None yet

6 participants