Implement GCE PZID verification to validate TDX quote#108
Conversation
07538f9 to
fef68fb
Compare
|
This is a larger PR to review, could you add summaries to the PR description? |
| @@ -1,49 +1,124 @@ | |||
| # `provenance` CLI tool | |||
There was a problem hiding this comment.
Revisiting this README.md makes me realize both pzid and ppid checks are specific to gce platform, so could you update the CLI name as something like "gceprovenance" to be more specific?
There was a problem hiding this comment.
Good point. I renamed it to gceprovenance.
There was a problem hiding this comment.
I'd say gce-provenance for readability and convention, unless tdx-tools uses non dash separated tools elsewhere.
But are we sure it's just GCE? I believe GKE uses confidential nodes.
There was a problem hiding this comment.
good point, if the VMX rollout will touch GKE fleet or host PPID registration will support GKE eventually then a more generic name would be better.
There was a problem hiding this comment.
Having second thoughts, I think gceprovenance is the better name, because the verified identity is specifically GCE VM. If GKE ever to be supported, probably it has different setup considering it has workload, cluster etc., and we can add gke-provenance at that time.
5ac9398 to
06fab69
Compare
Done. |
yawangwang
left a comment
yawangwang
left a comment
There was a problem hiding this comment.
Overall LGTM, is it possible to test this binary on an actual GCE CVM before merging(not sure how the current rollout goes)
|
|
||
| // ResourceString returns the canonical GCE instance resource string when the | ||
| // numeric project and instance IDs are available. | ||
| func (i *InstanceInfo) ResourceString() string { |
There was a problem hiding this comment.
Consider implementing Stringer interface https://pkg.go.dev/fmt#Stringer if you want to print a formatted string for a custom type.
There was a problem hiding this comment.
Thanks! I prefer keeping this function, as this format might not be the best representation, like for better readability there is the richer logging LogString().
| if err != nil { | ||
| return nil, nil, fmt.Errorf("read quote file at %s: %w", path, err) | ||
| } | ||
| quote, err := abi.QuoteToProto(quoteBytes) |
There was a problem hiding this comment.
since abi.QuoteToProto natively supports both v4 and v5 conversion, it would be good to have v5 support for CLI in case GCE supports TDX v1.5
There was a problem hiding this comment.
Yes, QuoteToProto can parse V5, but there is no e2e support or verification. I'd prefer not silently support V5, but make this as a focused follow-up improvement when V5 is fully supported.
| zone, err := getMetadata(ctx, client, baseURL, "instance/zone") | ||
| if err != nil { | ||
| return nil, fmt.Errorf("fetch zone from metadata server: %w", err) | ||
| } | ||
| projectID, err := getMetadata(ctx, client, baseURL, "project/project-id") | ||
| if err != nil { | ||
| return nil, fmt.Errorf("fetch project ID from metadata server: %w", err) | ||
| } | ||
| projectNumber, err := getMetadataUint(ctx, client, baseURL, "project/numeric-project-id") | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| instanceName, err := getMetadata(ctx, client, baseURL, "instance/name") | ||
| if err != nil { | ||
| return nil, fmt.Errorf("fetch instance name from metadata server: %w", err) | ||
| } | ||
| instanceID, err := getMetadataUint(ctx, client, baseURL, "instance/id") |
There was a problem hiding this comment.
Is it possible to send single request to get everything we need rather than 5 requests?
There was a problem hiding this comment.
Yes, it could be done in one request with the recursion param. But it retrieves a lot more metadata that we don't use, and this this host-local request, latency is not an issue.
…t instance identity
3 participants
Adds GCE-specific TDX provenance verification through the new gceprovenance CLI.
verify,verify-host, andverify-instancecommands.Tests
go test ./...
go build ./tools/gceprovenance
Manual GCE TDX CVM testing covered: