feat(frost/roast): domain-separate evidence-snapshot + transition-message signatures#4057
Merged
mswilkison merged 4 commits intoJun 14, 2026
Conversation
…sage signatures The node's operator key signs LocalEvidenceSnapshot, TransitionMessage, and (on #4056) the signing package, and their bodies are wire-compatible, so a signature over one body could be replayed as a signature over another. Each signed body now prepends a UNIQUE domain tag to the bytes it signs and verifies, while the body that travels on the wire stays the bare serialized body. - Add localEvidenceSnapshotSignatureDomain + transitionMessageSignatureDomain, each beginning with byte 0x00 (an illegal protobuf tag, field 0) so the signed payload is undecodable as any protobuf message. This separates the domains in both directions without relying on field layout: a signature over one body cannot be accepted on another envelope (its decoder rejects the 0x00-leading body), and a genuine protobuf body starts >= 0x08 so its signature can never verify against domain||body. Matches the signing-package fix on #4056. - Split each type into bodyBytes() (the bare wire body) and SignableBytes() (domain || body). wireEnvelopeBytes / Marshal now embed bodyBytes(), and both Unmarshal reset the signable-bytes cache so a reused receiver never verifies against stale bytes. Rename signedBody -> bodyCache; add signaturePayloadCache. Sign and verify both flow through SignableBytes(), so the change is transparent to the sign sites and the wire envelope structure is unchanged. It DOES change the bytes signed, so signatures are not compatible with the pre-change protocol - all nodes must run the new code together (acceptable pre-mainnet; the external-audit gate stands). Tests pin the domain tagging, undecodability, prefix-free distinctness, bare wire body, and the reused-receiver cache reset. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The package comment called all three signed bodies wire-compatible; in fact only TransitionMessageBody and the signing-package body share field-1 = attempt_context_hash. LocalEvidenceSnapshotBody's field 1 is sender_id (a varint), so it is only INCIDENTALLY separated - a difference a later proto change could erase, which is precisely why the domain tag makes the separation intentional. Comment-only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…es (review) Codex (PR #4057) flagged that the cross-language contract still documented snapshot and transition signatures as covering the bare body, while this PR makes Go sign/verify domain || body - so a non-Go implementation built from evidence.proto or RFC-21 would sign/verify different bytes and reject Go's evidence messages. Document the signed payload as `domain_tag || body` in evidence.proto (a file-level SIGNED PAYLOAD note with both exact tags + the four message comments) and in the RFC-21 wire-format decision. Regenerated evidence.pb.go is comment-only (descriptor and symbols unchanged). Same fix class as the signing-package proto contract on #4056. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rify race (P1) Gemini (PR #4057) flagged a data race the lazy-cache change introduced: setting signaturePayloadCache = nil in Unmarshal means the first SignableBytes call on a PARSED message writes the cache, so concurrent signature verification of one received message races on that write. This regressed a prior property - the pre-PR code primed the cache at Unmarshal because the signed payload equaled the received body, making a parsed message's SignableBytes a pure read. Prime the cache in both Unmarshal paths (clear it, then call SignableBytes once), restoring the race-free read on the verification path. This still discards any stale cache on a reused value. Tests: end-to-end cross-protocol rejection in both directions (a transition coordinator signature does not verify as a snapshot operator signature and vice versa, even with matching id/attempt), plus a -race regression guard that verifies a parsed snapshot concurrently. Full pkg/frost/roast suite passes under -race. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
mswilkison
added a commit
that referenced
this pull request
Jun 14, 2026
…verify race) Mirror of the #4057 fix. SigningPackage.Unmarshal reset signaturePayloadCache to nil, so the first SignableBytes call on a PARSED package was a racing write - concurrent signature verification (AuthenticateSigningPackage) of one received package would race on it. Prime the cache in Unmarshal (clear it, then call SignableBytes once), restoring a pure-read SignableBytes on the verify path; it still discards a stale cache on a reused value. Adds a -race regression guard that verifies a parsed package concurrently. Full pkg/frost/roast suite passes under -race. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
mswilkison
merged commit d65ac91
into
feat/frost-schnorr-migration-scaffold
16 checks passed
mswilkison
deleted the
feat/frost-roast-signed-body-domain-separation-2026-06-14
branch
mswilkison
added a commit
that referenced
this pull request
Jun 14, 2026
…ation) (#4058) ## What The wire foundation for the **context-bound, member-authenticated Round2 share submission** — the remaining 7.2b-2 piece. After a member authenticates the elected coordinator's `SignedSigningPackage` (#4056) and accepts its taproot root, it returns its FROST round-2 signature share **bound to that exact package**. That binding is the hard prerequisite for blame adjudication (Phase 7.2b-4): a member's share is provably tied to the specific package bytes it received, so coordinator equivocation across members is detectable and a member's submission is non-repudiable. This PR is the **wire type only** (mirrors how #4056 landed the signing-package envelope first). Member sign/retain + network distribution come next. ## How - **`share_submission.proto`**: `ShareSubmissionBody{attempt_context_hash, submitter_id, signing_package_hash, signature_share}` + `SignedShareSubmission{body, submitter_signature}`; generated `pb.go`. - **`ShareSubmission` Go wire type**, structurally identical to the signing-package envelope: - `bodyBytes()` (the bare body that travels) + `SignableBytes()` (`domain ‖ body`) with the leading-`0x00` illegal-tag domain `roast/signed-share-submission/v1` — consistent with the domain-separation sweep (#4057), so the share signature is non-confusable with the signing-package / snapshot / transition signatures. - `Marshal`/`Unmarshal`/`Validate`. `Unmarshal` bounds the envelope before `proto.Unmarshal`, rejects an over-cap `signature_share` before copying, and **primes the signable-bytes cache** so concurrent verification is race-free (incorporating the fix from #4056/#4057 up front). `submitter_id` is bounded to `group.MemberIndex`. ## Testing `gofmt`/`vet`/`build` clean; full `pkg/frost/roast` suite passes under `go test -race`. New tests: verbatim round-trip, non-canonical-encoding survival, domain-separated + undecodable-as-protobuf, **prefix-free distinctness vs the signing-package / snapshot / transition domains**, validate-rejects-malformed (incl. submitter out of member-index range, short hashes, over-cap share), marshal-requires-signature, unmarshal-rejects-oversize-before-copy, and a `-race` concurrency guard. ## Next Member-side `AuthenticateShareSubmission` (verify under the submitter's operator key + bind to the live attempt + the retained signing-package hash) and the network distribution path — then the equivocation compare + f+1 quorum blame (7.2b-4). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
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
1 participant
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.
What
Gives the two remaining operator-key-signed ROAST bodies —
LocalEvidenceSnapshotandTransitionMessage— their own distinct domain tags, completing the cross-protocol signature-confusion sweep started for the signing-package envelope on #4056.The node's operator key signs all of these bodies (snapshots as a member; transition messages + the signing package as the elected coordinator), and their protobuf bodies are wire-compatible (each begins with a field-1 attempt-context binding). Without domain separation, a signature over one body could be presented as a signature over another. #4056 added a tag to the signing package; a reviewer correctly noted the existing snapshot/transition bodies still signed bare bodies, so this PR closes that gap.
How
Distinct domain tags, each beginning with byte
0x00— an illegal protobuf tag (field number 0) — so the signed payload is undecodable as any protobuf message:\x00roast/signed-evidence-snapshot/v1\x00\x00roast/signed-transition-message/v1\x00This separates the domains in both directions without relying on field layout: a signature over one body can't be accepted on another envelope (its decoder
proto.Unmarshals and rejects the0x00-leading body), and a genuine serialized protobuf body always starts>= 0x08, so its signature can never verify againstdomain || body. Same construction as the signing-package fix on feat(frost/roast): Phase 7.2b-2 signed signing-package envelope (wire foundation) #4056.bodyBytes()/SignableBytes()split:bodyBytes()is the bare body that travels on the wire (embedded bywireEnvelopeBytes/Marshal);SignableBytes()isdomain || body, the only thing signed/verified. BothUnmarshalpaths reset the signable-bytes cache so a reused receiver never verifies against stale bytes.Renamed the bare-body cache
signedBody→bodyCacheand addedsignaturePayloadCache, so all three signed-body types share one structure.Compatibility
Sign and verify both flow through
SignableBytes(), so the change is transparent to the sign sites and the wire envelope structure is unchanged. It does change the bytes signed, so signatures are not compatible with the pre-change protocol — all nodes must run the new code together. Acceptable pre-mainnet; the external-audit gate is unchanged.Testing
gofmt/go vet/go build ./pkg/... ./cmd/...clean; fullpkg/frost/roastsuite green. Newdomain_separation_test.gopins, for both bodies: domain tagging, undecodability as protobuf, the bare (untagged) wire body, prefix-free distinctness of the tags, and the reused-receiver cache reset. Existing verbatim/round-trip/verify tests continue to pass.🤖 Generated with Claude Code