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.

Sign up for GitHub

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

Jump to bottom

Refactor verifier for issue #418 #419

Merged
merged 15 commits into from
Mar 15, 2024
Merged

Refactor verifier for issue #418 #419

merged 15 commits into from
Mar 15, 2024

Conversation

Ruide
Copy link
Collaborator

@Ruide Ruide commented Mar 5, 2024 •
edited
Loading

See Issue #418

This PR makes the following changes:

  • move package verifier out of package launcher. Package verifier is responsible to request to Verifier services, thus it is not only useful to package launcher.
  • remove cloudLogger in package agent. Package agent is responsible for setting up evidence management for launcher, and launcher does not need the cloudLog flag option.
  • refactor token command to depend on package verifier and remove its dependency on agent package.
  • move rest_network_test.go test to agent_test. Not doing so lead to cyclic dependencies.
  • move package oci to package verifier. Because oci is part of request to Verifier services.
  • create package util for hosting common functions related to package verifier
  • move fake_attestation_server, fake_metadata, fake_oauth2_server to package util. Because they are common functions for unit testing related to Google verifier service.
  • extract common functions, GetAttestation function, GetRegion, GetRESTClient, to util. Because token command and package agent have these duplicate functions. And add unit test for the extracted functions.

@Ruide
Copy link
Collaborator Author

Ruide commented Mar 5, 2024

/gcbrun

@Ruide Ruide requested review from alexmwu and jkl73 March 5, 2024 19:43
@Ruide
Copy link
Collaborator Author

Ruide commented Mar 5, 2024

/gcbrun

@Ruide Ruide force-pushed the refactor-verifier branch 6 times, most recently from 23ed48a to 9ac26cb Compare March 7, 2024 02:00
@Ruide
Copy link
Collaborator Author

Ruide commented Mar 7, 2024

/gcbrun

@Ruide
Copy link
Collaborator Author

Ruide commented Mar 11, 2024

Hi @alexmwu ! Could you take a look?

alexmwu
alexmwu reviewed Mar 13, 2024
View reviewed changes
Copy link
Contributor

@alexmwu alexmwu left a comment

Choose a reason for hiding this comment

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

This PR description should mention what changes are breaking.

@Ruide
Copy link
Collaborator Author

Ruide commented Mar 13, 2024

Hi @alexmwu , I have added the description. Thanks for the comment!

@Ruide Ruide requested a review from alexmwu March 13, 2024 01:42
@Ruide
Copy link
Collaborator Author

Ruide commented Mar 14, 2024

/gcbrun

alexmwu
alexmwu reviewed Mar 14, 2024
View reviewed changes
Copy link
Contributor

@alexmwu alexmwu left a comment

Choose a reason for hiding this comment

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

Please also have @jkl73 approve before submitting

verifier/util/fake_attestation_server.go Outdated Show resolved Hide resolved
verifier/util/fake_attestation_server.go Outdated Show resolved Hide resolved
verifier/util/util_test.go Outdated Show resolved Hide resolved
cmd/token.go Outdated Show resolved Hide resolved
verifier/util/fake_attestation_server.go Outdated Show resolved Hide resolved
verifier/util/util.go Outdated Show resolved Hide resolved
@alexmwu alexmwu requested a review from yawangwang March 14, 2024 19:45
@Ruide Ruide merged commit cf9fd6d into google:main Mar 15, 2024
11 checks passed
@Ruide Ruide deleted the refactor-verifier branch March 15, 2024 21:43
@Ruide Ruide mentioned this pull request Mar 15, 2024
Closed
alexmwu added a commit to alexmwu/go-tpm-tools that referenced this pull request Mar 29, 2024
@alexmwu
Release v0.4.4
777192a 
Breaking Changes:
[launcher/cmd] Refactor verifier for issue google#419
* Unexport `cmd.Instance`, `cmd.MetadataServer`,
  `cmd.NewMetadataServer`.
* Move package `verifier` from launcher to go-tpm-tools.
  * `verifier.Client`, `verifier.Challenge`, etc.
* Move package `fake` from launcher to go-tpm-tools.
  * `fake.Claims`, `fake.NewClient`, etc.
* Move package `rest` from launcher to go-tpm-tools.
  * `rest.NewClient`, `rest.BadRegionError`, etc.

New Features:
[cmd] Add new command token in the CLI tool google#375
[cmd] add records to cloud logging when fetching token from attestation verifier google#417

Bug Fixes:
Statically link binaries built by goreleaser google#425

Other Changes:
Update readme to include the instruction to use the prebuilt gotpm tool. google#424

New Contributors:
@Ruide in google#375
@qinkunbao in google#424
@alexmwu alexmwu mentioned this pull request Mar 29, 2024
Merged
alexmwu added a commit to alexmwu/go-tpm-tools that referenced this pull request Mar 29, 2024
@alexmwu
Release v0.4.4
0ae0aa4 
Breaking Changes:
[launcher/cmd] Refactor verifier for issue google#419
* Unexport `cmd.Instance`, `cmd.MetadataServer`,
  `cmd.NewMetadataServer`.
* Move package `verifier` from launcher to go-tpm-tools.
  * `verifier.Client`, `verifier.Challenge`, etc.
* Move package `fake` from launcher to go-tpm-tools.
  * `fake.Claims`, `fake.NewClient`, etc.
* Move package `rest` from launcher to go-tpm-tools.
  * `rest.NewClient`, `rest.BadRegionError`, etc.

New Features:
[cmd] Add new command token in the CLI tool google#375
[cmd] add records to cloud logging when fetching token from attestation verifier google#417

Bug Fixes:
Statically link binaries built by goreleaser google#425

Other Changes:
Update readme to gotpm CLi instructions. google#424, google#426

New Contributors:
@Ruide in google#375
@qinkunbao in google#424
alexmwu added a commit to alexmwu/go-tpm-tools that referenced this pull request Apr 12, 2024
@alexmwu
Fix bug that dropped the CEL in the attestation
2af4e38 
The CEL getting dropped in the attestation was missed as part of google#419.
This PR adds it back for the launcher and adds a unit test that ensures
the CEL is verified when available.
@alexmwu alexmwu mentioned this pull request Apr 12, 2024
Closed
alexmwu added a commit to alexmwu/go-tpm-tools that referenced this pull request Apr 12, 2024
@alexmwu
Fix bug that dropped the CEL in the attestation
16e6b3a 
The CEL getting dropped in the attestation was missed as part of google#419.
This PR adds it back for the launcher and adds a unit test that ensures
the CEL is verified when available.
alexmwu added a commit to alexmwu/go-tpm-tools that referenced this pull request Apr 12, 2024
@alexmwu
Fix bug that dropped the CEL in the attestation
e5e91a8 
The CEL getting dropped in the attestation was missed as part of google#419.
This PR adds it back for the launcher and adds a unit test that ensures
the CEL is verified when available.
alexmwu added a commit to alexmwu/go-tpm-tools that referenced this pull request Apr 12, 2024
@alexmwu
Fix bug dropping CEL in the Attestation
2a93cdb 
The CEL getting dropped in the attestation was missed as part of google#419.
This PR adds it back for the launcher and adds a unit test that ensures
the CEL is verified when available.
alexmwu added a commit to alexmwu/go-tpm-tools that referenced this pull request Apr 12, 2024
@alexmwu
Fix bug dropping CEL in launcher attestations
bc9b175 
We inadvertently dropped the launcher's canonical event log when
refactoring in google#419. This fix adds back the launcher CEL and adds a unit
test that checks for CEL measurements in the MachineState.
@alexmwu alexmwu mentioned this pull request Apr 12, 2024
Merged
alexmwu added a commit to alexmwu/go-tpm-tools that referenced this pull request Apr 12, 2024
@alexmwu
Fix bug dropping CEL in launcher attestations
a70f39b 
We inadvertently dropped the launcher's canonical event log when
refactoring in google#419. This fix adds back the launcher CEL and adds a unit
test that checks for CEL measurements in the MachineState.
alexmwu added a commit to alexmwu/go-tpm-tools that referenced this pull request Apr 12, 2024
@alexmwu
Fix bug dropping CEL in launcher attestations
9386864 
We inadvertently dropped the launcher's canonical event log when
refactoring in google#419. This fix adds back the launcher CEL and adds a unit
test that checks for CEL measurements in the MachineState.
alexmwu added a commit to alexmwu/go-tpm-tools that referenced this pull request Apr 13, 2024
@alexmwu
Fix bug dropping CEL in launcher attestations
a169efa 
We inadvertently dropped the launcher's canonical event log when
refactoring in google#419. This fix adds back the launcher CEL and adds a unit
test that checks for CEL measurements in the MachineState.
alexmwu added a commit that referenced this pull request Apr 13, 2024
@alexmwu
Fix bug dropping CEL in launcher attestations (#438)
7309492 
We inadvertently dropped the launcher's canonical event log when
refactoring in #419. This fix adds back the launcher CEL and adds a unit
test that checks for CEL measurements in the MachineState.
alexmwu added a commit to alexmwu/go-tpm-tools that referenced this pull request May 7, 2024
@alexmwu
Move verifier package to its own submodule
41aef79 
This should remove the large increase in dependencies due to google#419.
We temporarily include replace directives in cmd/go.mod and
launcher/go.mod for now while we work on moving the package to a
submodule.
@alexmwu alexmwu mentioned this pull request May 7, 2024
Merged
alexmwu added a commit to alexmwu/go-tpm-tools that referenced this pull request May 7, 2024
@alexmwu
Move verifier package to its own submodule
905e95c 
This should remove the large increase in dependencies due to google#419.
We temporarily include replace directives in cmd/go.mod and
launcher/go.mod for now while we work on moving the package to a
submodule.
alexmwu added a commit to alexmwu/go-tpm-tools that referenced this pull request May 7, 2024
@alexmwu
Move verifier package to its own submodule
ad2f148 
This should remove the large increase in dependencies due to google#419.
We temporarily include replace directives in cmd/go.mod and
launcher/go.mod for now while we work on moving the package to a
submodule.
alexmwu added a commit to alexmwu/go-tpm-tools that referenced this pull request May 7, 2024
@alexmwu
Move verifier package to its own submodule
babc80a 
This should remove the large increase in dependencies due to google#419.
We temporarily include replace directives in cmd/go.mod and
launcher/go.mod for now while we work on moving the package to a
submodule.
alexmwu added a commit to alexmwu/go-tpm-tools that referenced this pull request May 7, 2024
@alexmwu
Move verifier package to its own submodule
004ad9f 
This should remove the large increase in dependencies due to google#419.
We temporarily include replace directives in cmd/go.mod and
launcher/go.mod for now while we work on moving the package to a
submodule.
alexmwu added a commit to alexmwu/go-tpm-tools that referenced this pull request May 7, 2024
@alexmwu
Move verifier package to its own submodule
303a0b6 
This should remove the large increase in dependencies due to google#419.
We temporarily include replace directives in cmd/go.mod and
launcher/go.mod for now while we work on moving the package to a
submodule.
alexmwu added a commit to alexmwu/go-tpm-tools that referenced this pull request May 7, 2024
@alexmwu
Move verifier package to its own submodule
790e8e0 
This should remove the large increase in dependencies due to google#419.
We temporarily include replace directives in cmd/go.mod and
launcher/go.mod for now while we work on moving the package to a
submodule.
alexmwu added a commit to alexmwu/go-tpm-tools that referenced this pull request May 7, 2024
@alexmwu
Move verifier package to its own submodule
0ef0d4d 
This should remove the large increase in dependencies due to google#419.
We temporarily include replace directives in cmd/go.mod and
launcher/go.mod for now while we work on moving the package to a
submodule.
alexmwu added a commit to alexmwu/go-tpm-tools that referenced this pull request May 7, 2024
@alexmwu
Move verifier package to its own submodule
eaeefd0 
This should remove the large increase in dependencies due to google#419.
We temporarily include replace directives in cmd/go.mod and
launcher/go.mod for now while we work on moving the package to a
submodule.
alexmwu added a commit to alexmwu/go-tpm-tools that referenced this pull request May 7, 2024
@alexmwu
Move verifier package to its own submodule
b6ae4fe 
This should remove the large increase in dependencies due to google#419.
We temporarily include replace directives in cmd/go.mod and
launcher/go.mod for now while we work on moving the package to a
submodule. We also remove some unnecessary utilities like
FetchAttestation and tests that won't work except on GCE.

Breaking Changes
* Move package `verifier` from go-tpm-tools to a new submodule github.com/google/go-tpm-tools/verifier
  * `verifier.Client`, `verifier.Challenge`, etc.
* Move package `fake` from go-tpm-tools to a new submodule github.com/google/go-tpm-tools/verifier.
  * `fake.Claims`, `fake.NewClient`, etc.
* Move package `oci` and `cosign` from go-tpm-tools to a new submodule github.com/google/go-tpm-tools/verifier.
  * `oci.Signature`, `cosign.Sig`, etc.
* Move package `rest`  from go-tpm-tools to a new submodule github.com/google/go-tpm-tools/verifier.
  * `rest.NewClient`, `rest.BadRegionError`, etc.
* Move package `util` from go-tpm-tools to a new submodule github.com/google/go-tpm-tools/verifier.
  * `util.Instance`, `util.MetadataServer`, `util.NewMetadataServer`, etc.
alexmwu added a commit to alexmwu/go-tpm-tools that referenced this pull request May 7, 2024
@alexmwu
Move verifier package to its own submodule
31911bd 
This should remove the large increase in dependencies due to google#419.
We temporarily include replace directives in cmd/go.mod and
launcher/go.mod for now while we work on moving the package to a
submodule. We also remove some unnecessary utilities like
FetchAttestation and tests that won't work except on GCE.

Breaking Changes
* Move package `verifier` from go-tpm-tools to a new submodule github.com/google/go-tpm-tools/verifier
  * `verifier.Client`, `verifier.Challenge`, etc.
* Move package `fake` from go-tpm-tools to a new submodule github.com/google/go-tpm-tools/verifier.
  * `fake.Claims`, `fake.NewClient`, etc.
* Move package `oci` and `cosign` from go-tpm-tools to a new submodule github.com/google/go-tpm-tools/verifier.
  * `oci.Signature`, `cosign.Sig`, etc.
* Move package `rest`  from go-tpm-tools to a new submodule github.com/google/go-tpm-tools/verifier.
  * `rest.NewClient`, `rest.BadRegionError`, etc.
* Move package `util` from go-tpm-tools to a new submodule github.com/google/go-tpm-tools/verifier.
  * `util.Instance`, `util.MetadataServer`, `util.NewMetadataServer`, etc.
alexmwu added a commit that referenced this pull request May 8, 2024
@alexmwu
Move verifier package to its own submodule (#447)
02ce9da 
This should remove the large increase in dependencies due to #419.
We temporarily include replace directives in cmd/go.mod and
launcher/go.mod for now while we work on moving the package to a
submodule. We also remove some unnecessary utilities like
FetchAttestation and tests that won't work except on GCE.

Breaking Changes
* Move package `verifier` from go-tpm-tools to a new submodule github.com/google/go-tpm-tools/verifier
  * `verifier.Client`, `verifier.Challenge`, etc.
* Move package `fake` from go-tpm-tools to a new submodule github.com/google/go-tpm-tools/verifier.
  * `fake.Claims`, `fake.NewClient`, etc.
* Move package `oci` and `cosign` from go-tpm-tools to a new submodule github.com/google/go-tpm-tools/verifier.
  * `oci.Signature`, `cosign.Sig`, etc.
* Move package `rest`  from go-tpm-tools to a new submodule github.com/google/go-tpm-tools/verifier.
  * `rest.NewClient`, `rest.BadRegionError`, etc.
* Move package `util` from go-tpm-tools to a new submodule github.com/google/go-tpm-tools/verifier.
  * `util.Instance`, `util.MetadataServer`, `util.NewMetadataServer`, etc.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexmwu alexmwu alexmwu left review comments

@yawangwang yawangwang yawangwang approved these changes

@jkl73 jkl73 Awaiting requested review from jkl73

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Ruide @alexmwu @yawangwang