Support image hash measurements following proposed breaking change to attested-tls-proxy - #78
Draft
ameba23 wants to merge 3 commits into
Draft
Support image hash measurements following proposed breaking change to attested-tls-proxy#78ameba23 wants to merge 3 commits into
ameba23 wants to merge 3 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for typed attestation measurements and portable DCAP image-hash policies across validation, matching, persistence, tests, and documentation.
Changes:
- Parses and matches typed measurement headers.
- Adds image-hash policy storage and validation.
- Updates schema, fixtures, tests, documentation, and development configuration.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Summary | Final review comments |
|---|---|---|
testdata/get-measurements.json |
Adds portable image policy fixture. | None |
scripts/ci/e2e-test.hurl |
Updates typed DCAP test data. | None |
schema/005_dcap_image_hashes.sql |
Adds image-hash storage and constraints. | Critical (1 vote): Require exactly one representation; both columns may currently be NULL, creating allow-any policies. |
README.md |
Documents new headers and policies. | Nit (3 votes): Add the missing space before an inline code span. |
ports/types.go |
Adds API fields and policy validation. | Critical (1 vote): Reject non-empty measurements for none policies and retain a matcher-side guard. Moderate (3 votes): Restrict unsupported image-policy paths to gcp-tdx. Moderate (2 votes): Preserve empty register-policy maps in API responses. |
ports/types_test.go |
Tests parsing and conversion. | None |
ports/http_handler.go |
Parses typed authentication headers. | None |
ports/admin_handler.go |
Handles invalid measurement policies. | None |
Makefile |
Updates development seed data. | None |
httpserver/e2e_test.go |
Adds typed-header and portable-authentication tests. | None |
domain/types.go |
Adds image-hash domain types and validation. | None |
docs/devenv-setup.md |
Updates setup examples. | None |
docker/mock-proxy/nginx-default.conf |
Emits the new typed header format. | None |
application/service.go |
Matches typed measurement policies. | Critical (1 vote): Prevent none policies with register measurements from becoming allow-any. |
application/service_test.go |
Tests policy matching. | None |
adapters/database/types.go |
Reads nullable policy representations. | None |
adapters/database/service.go |
Persists and retrieves policies. | None |
adapters/database/service_test.go |
Tests portable policy persistence. | None |
Suppressed comments (3)
application/service.go:127
- This branch treats a
dcap-tdximage policy as a validExpectedMeasurementImage, but the paired verifier only supports portable image policies forgcp-tdx. Consequently, any such row loaded from the database is advertised as usable even though the upstream proxy will not produce a matching header. Use onlygcp-tdxfor image policies here.
if template.DcapImageHashes != nil {
if template.AttestationType == "dcap-tdx" || template.AttestationType == "gcp-tdx" {
return domain.ExpectedMeasurementImage, true
ports/http_handler.go:122
- The header parser also claims that an
imageheader is valid fordcap-tdx, although the paired attestation library currently emits portable image policies only forgcp-tdx. This makes the accepted API contract inconsistent and lets administrators configure an unreachable policy. Restrict this match togcp-tdx(and update the corresponding tests).
case domain.ExpectedMeasurementImage:
return attestationType == "dcap-tdx" || attestationType == "gcp-tdx"
schema/005_dcap_image_hashes.sql:3
- This schema change is only copied into
/docker-entrypoint-initdb.d/, which PostgreSQL runs only when initializing a new data directory (docker/database/Dockerfile:4). Existing databases will therefore lackdcap_image_hashes; the updatedAddMeasurementINSERT will fail when the server tries to create a portable policy, and reads cannot expose stored image hashes. Add/run an upgrade migration for existing database instances and document that rollout instead of relying on fresh initialization.
ADD COLUMN dcap_image_hashes JSONB,
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| {"type":"dcap","measurements":{"0":["<MRTD SHA-384>"],"3":["<RTMR2 SHA-384>"]}} | ||
| ``` | ||
|
|
||
| Portable image policies use `"type": "image"`. If no attestation was provided, it will have`"type": "no_attestation"` and no `measurements` field. Legacy headers containing a plain register-to-string object are accepted for backwards compatibility. |
| case domain.ExpectedMeasurementImage: | ||
| return equalImageHashes(measurement.DcapImageHashes, template.DcapImageHashes) | ||
| case domain.ExpectedMeasurementNoAttestation: | ||
| return true |
Comment on lines
+142
to
+144
| if measurement.DcapImageHashes != nil { | ||
| if measurement.AttestationType != "dcap-tdx" && measurement.AttestationType != "gcp-tdx" { | ||
| return domain.Measurement{}, errors.New("dcap_image_hashes require dcap-tdx or gcp-tdx attestation type") |
Author
There was a problem hiding this comment.
Support for portable image policies on bare metal is being implemeneted right now, and the paired PRs will accommodate that change before i take this out of draft. So this shouldnt be an issue.
| Name string `json:"measurement_id"` | ||
| AttestationType string `json:"attestation_type"` | ||
| Measurements map[string]domain.SingleMeasurement `json:"measurements"` | ||
| Measurements map[string]domain.SingleMeasurement `json:"measurements,omitempty"` |
Comment on lines
+151
to
+154
| measurements := measurement.Measurements | ||
| if measurements == nil && measurement.DcapImageHashes == nil { | ||
| measurements = make(map[string]domain.SingleMeasurement) | ||
| } |
Comment on lines
+4
to
+6
| ADD CONSTRAINT measurement_policy_shape CHECK ( | ||
| measurement IS NULL OR dcap_image_hashes IS NULL | ||
| ); |
This file contains hidden or 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
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.
📝 Summary
This pairs with flashbots/attested-tls-proxy#174 and in turn with flashbots/attested-tls#79
Those PRs mean that measurement HTTP header values are formatted differently from before, in order to support both measurements given as TDX register values (as before) as well as matched OS image hashes.
This PR accepts the new measurements header format, and also allows measurement policies to be submitted with this new format to allow matching a set of OS image hashes. This means changing the database schema.
⛱ Motivation and Context
On GCP we have had issues with RTMR0 register requiring many possible allowed values, as well as Google updating firmware which effect MRTD. The current mitigation for this issue is to only match RTMR1 and RTMR2. This works but comes at a security cost as we cannot ensure that Google endorsed firmware is running.
Recent work in https://github.com/Easy-TEE/attest means we can include platform metadata with an attestation allowing the verifier to compute an expected RTMR0 value. Also, MRTD values are checked against Google's public bucket mapping them to signed firmware. This is implemented in attested-tls-proxy v2.0.0 and above. But to take advantage of this, measurement policies have to be expressed as a set of OS image hashes rather than TDX register values. Hence the breaking change to measurements format.
📚 References
✅ I have run these commands
make lintmake testgo mod tidy