fix(teeattestation): length-prefix tag and data in DomainHash [CL112-14]#2172
fix(teeattestation): length-prefix tag and data in DomainHash [CL112-14]#2172nadahalli wants to merge 2 commits into
Conversation
DomainHash delimited DomainSeparator, tag, and data with newlines, so a tag or data value containing a newline could make distinct (tag, data) pairs share a pre-image. Length-prefix each field instead, matching ComputeRequest.Hash's writeWithLength scheme. Not exploitable today (tags are newline-free constants); hardens against future tag changes. Note: changes DomainHash output, so producers and verifiers must run the same version. Acceptable as confidential workflows is unreleased.
|
👋 nadahalli, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
📊 API Diff Results
|
There was a problem hiding this comment.
Pull request overview
This PR hardens teeattestation.DomainHash against tag/data boundary ambiguity by replacing newline-delimited concatenation with a canonical, length-prefixed encoding (addressing audit finding CL112-14 / PRIV-467). This prevents distinct (tag, data) pairs from producing the same pre-hash byte sequence when either field contains newline characters.
Changes:
- Updated
DomainHashto hashDomainSeparator || len(tag) || tag || len(data) || datausing an 8-byte big-endian length prefix per field. - Added a small internal
writeWithLengthhelper to implement the encoding. - Added a regression test ensuring the historical newline-boundary collision case no longer collides.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/teeattestation/hash.go | Switches DomainHash to a length-prefixed encoding and adds the helper used to write the canonical preimage. |
| pkg/teeattestation/hash_test.go | Updates the reference computation for TestDomainHash and adds a boundary-collision regression test (CL112-14). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
DomainHashdelimitedDomainSeparator,tag, anddatawith newlines, so a tag or data value containing a newline could make distinct(tag, data)pairs share a pre-image (e.g.("A\nB", "C")and("A", "B\nC")). It length-prefixes each variable field instead, matching thewriteWithLengthschemeComputeRequest.Hashalready uses.Not exploitable today (tags are newline-free constants); this hardens against future tag changes. Adds a boundary-collision regression test.
Note: this changes
DomainHash's output, so attestation producers and verifiers must run the same version. Acceptable since confidential workflows is unreleased.Audit finding CL112-14 / PRIV-467.