Add Policy object for server-side remote attestation #103
Conversation
0a065da to
788ac80
Compare
|
@alexmwu I didn't have time to make any changes to this yesterday. But I opened #113 It should have all the necessary embedded data for this PR. So can you:
I didn't include the dbx binary files as they are unused (and shouldn't be in this PR). |
18168fb to
b5a463c
Compare
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
|
@googlebot I consent. |
|
@alexmwu this is all of the The big changes are in |
Signed-off-by: Joe Richey <joerichey@google.com>
Signed-off-by: Joe Richey <joerichey@google.com>
Signed-off-by: Joe Richey <joerichey@google.com>
This makes the meaning clearer. We also split out some functionality into a helper function to make future methods easier to implement. This method now returns a `pb.MachineState`, making it easier to add functionality going forward. Signed-off-by: Joe Richey <joerichey@google.com>
Signed-off-by: Joe Richey <joerichey@google.com>
This is for consistancy with VerifyOpts/SealOpts/UnsealOpts Signed-off-by: Joe Richey <joerichey@google.com>
Signed-off-by: Joe Richey <joerichey@google.com>
Signed-off-by: Joe Richey <joerichey@google.com>
Instead of hard-coding sha256, we now allow aribitary hash algorithms to be use for verification. We will iteratate through a defined list of supported hash algorithms, taking the first algorithm that produces a valid MachineState. We also now explictly check if the signing hash and the pcr hash are supported by the library. Signed-off-by: Joe Richey <joerichey@google.com>
server/eventlog.go
Outdated
|
|
||
| if (event.GetUntrustedType() == Separator) || bytes.Equal(event.GetDigest(), separatorDigest) { | ||
| // Make sure we have a valid separator event | ||
| if !(event.GetUntrustedType() == Separator) { |
There was a problem hiding this comment.
if you're just checking on the digest, why not remove the first statement in the or.
So something like:
if bytes.Equal(event.GetDigest(), separatorDigest) {
if event.GetUntrustedType() != Separator {
return nil, fmt.Errorf(...)
}
There was a problem hiding this comment.
We want to make sure that if we see a event with a type of Separator but having the wrong data, we raise an error.
I added some more comments, PTAL.
Signed-off-by: Joe Richey <joerichey@google.com>
Signed-off-by: Joe Richey <joerichey@google.com>
Signed-off-by: Joe Richey <joerichey@google.com>
Signed-off-by: Joe Richey <joerichey@google.com>
Signed-off-by: Joe Richey <joerichey@google.com>
Signed-off-by: Joe Richey <joerichey@google.com>
Signed-off-by: Joe Richey <joerichey@google.com>
Signed-off-by: Joe Richey <joerichey@google.com>
|
@alexmwu @iKevinY I've added the policy evaluation logic and some basic tests. Could you two add more tests and merge this when it looks good to you? Note: This PR should be Merged not squashed. Also when adding commits, be sure to not mess up the |
| @@ -46,7 +46,7 @@ | |||
|
|
|||
| #include <openssl/evp.h> | |||
| #include <openssl/ec.h> | |||
| #if OPENSSL_VERSION_NUMBER >= 0x10200000L | |||
| #if 0 // OPENSSL_VERSION_NUMBER >= 0x10200000L | |||
There was a problem hiding this comment.
Is this change intentional / documented anywhere?
There was a problem hiding this comment.
here's the failing test: https://github.com/google/go-tpm-tools/runs/3903970801?check_suite_focus=true.
The comment tells us to
// Check the bignum_st definition in crypto/bn/bn_lcl.h and either update the
// version check or provide the new definition for this version.
seeing as this version works for all builds, I think we can consider the version definition "updated"
This only tests PlatformPolicy at the moment, including GCE confidential technology, GCE firmware version, and SCRTM.
The collected gLinux workstation event log has issues where the firmware extends a separator event (four 0x00 bytes) into PCR0, using EV_S_CRTM_CONTENTS. This conflicts with the logic that checks for separator events using only the event data (and ignoring the untrustworthy event type).
The TPM PFP spec says that separator values must be four 00 bytes or four ff bytes. This commit extends support for the ff bytes. Also, fix linter issues
Return an error on bad platform state parsing. Not returning an error masks problems with parsing the MachineState; we will eventually need to support multiple parsing errors anyway. Change separator parsing error message to convey the concern about separator events getting changed to different event types. Remove unsupported gLinux event log from the tests.
Users with an attestation server may want to apply a policy against a validated event log. A common use case might be validating secure configuration against expected values. For example, a server may want to check that a machine has updated its dbx and is not vulnerable to Boothole.
The policy object includes a firmware and secure boot related subpolicies. The firmware policy is mostly GCE instance related.