Add SecureBootState to ParseMachineState #139
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SecureBootState contains enabled, db, dbx, and (post-separator) authority for the time being.
Ran `go generate ./...`.
Add the dbx files from https://uefi.org/revocationlistfile These are the the commonly expected and dbx files.
MachineStateError is a struct that wraps multiple parsing errors when calling ParseMachineState.
This includes the Secure Boot variable (whether enabled), db, dbx, and post-separator Authority. As with PlatformState, ParseMachineState may return a zero value SecureBootState in MachineState if Secure Boot parsing failed. This commit also changes the output of ParseMachineState to return *MachineStateError (implements error) so that dependents can get a detailed view of which parts of the event log fail parsing. As the type implements the Error interface, this is not a breaking change.
GroupedError allows the error that wraps a slice of errors to be reused for Policy evaluation. This adds a helper function as well and fixes an issue with the return value of ParseMachineState. Previously, we always returned a non-nil MachineStateError from ParseMachineState. A zero value MachineStateError evaluate as non-nil in error catching blocks, so we want to instead return either nil or a pointer to a valid MachineStateError.
Often when dealing with GroupedErrors (e.g., parsing a MachineState), there may be a known issue that a user will want to ignore. These functions support filtering through all the errors and looking for a matching substring and also asserting if there is only one match.
The Ubuntu2104NoDbxGCE and Ubuntu2104NoSecureBootGCE event logs fail to parse due to a known issue where go-attestation does not handle SBAT entries: google/go-attestation#222. The Arch Linux event log has a Secure Boot variable with length 0, as opposed to length 1 and data 0. This also causes parsing errors in go-attestation: https://github.com/google/go-attestation/blob/master/attest/secureboot.go#L165-L167 For now, we should expect certain errors and let the test succeed.
iKevinY
approved these changes
Nov 2, 2021
jkl73
reviewed
Nov 3, 2021
alexmwu
added a commit
to alexmwu/go-tpm-tools
that referenced
this pull request
Nov 5, 2021
Will need to rebase on google#139
alexmwu
added a commit
to alexmwu/go-tpm-tools
that referenced
this pull request
Nov 18, 2021
DONOTMERGE: Need to rebase against google#139.
jkl73
approved these changes
Nov 23, 2021
iKevinY
approved these changes
Nov 24, 2021
josephlr
approved these changes
Dec 2, 2021
There was a problem hiding this comment.
Looks good. Some things that we want to address in followup PRs:
We should add more tests to make sure that the parsed out DB/DBX works like we expect
I think we can improve the ergonomics of GroupedError to work better with the Golang standard Is and As methods. But that's fine to do in a followup PR
Regenerate protos and change references.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Also modifies the output of ParseMachineState to return
GroupedErrors to better extract errors in parsing for certain parts of the event log.Add testing on ParseGCENonHostInfo
Fixes #142