diff --git a/docs/rfc/rfc-21-roast-coordinator-retry-and-transition-evidence.adoc b/docs/rfc/rfc-21-roast-coordinator-retry-and-transition-evidence.adoc index 04f5084fb9..d66bfd2218 100644 --- a/docs/rfc/rfc-21-roast-coordinator-retry-and-transition-evidence.adoc +++ b/docs/rfc/rfc-21-roast-coordinator-retry-and-transition-evidence.adoc @@ -669,20 +669,30 @@ optimization given the current key model. (`pkg/frost/roast/gen/pb/evidence.proto`), routed via the `net.Message` interface.* -Evidence signatures cover exact serialized body bytes, and those -bytes travel verbatim: a snapshot is +Evidence signatures cover a domain-separated byte stream -- a +fixed per-message-type tag followed by the exact serialized body -- +while the body itself travels verbatim. A snapshot is `SignedLocalEvidenceSnapshot{body, operator_signature}` where -`body` is the serialized `LocalEvidenceSnapshotBody`, and a -transition message is `SignedTransitionMessage{body, -coordinator_signature}` whose body embeds every member's signed -snapshot envelope verbatim (`repeated bytes signed_snapshots`). -A verifier checks each signature over the bytes it received and -only then parses them; nothing in the evidence chain is ever -re-encoded. Signature validity therefore never depends on any -serializer's canonical form -- across protobuf library versions -or across languages (the Phase 7 Rust signer verifies and parses -these same bytes). Producers marshal a body exactly once, at -signing time, and transmit those bytes. +`body` is the serialized `LocalEvidenceSnapshotBody` and the +operator signature covers `domain_tag || body`; a transition +message is `SignedTransitionMessage{body, coordinator_signature}` +whose body embeds every member's signed snapshot envelope verbatim +(`repeated bytes signed_snapshots`) and whose coordinator signature +covers `domain_tag || body`. Each `domain_tag` begins with byte +0x00 -- an illegal protobuf tag (field 0) -- so the signed payload +is undecodable as any protobuf message, and each message type uses +a distinct tag, so a signature over one body can never be accepted +for another (snapshot: `0x00 "roast/signed-evidence-snapshot/v1" +0x00`; transition: `0x00 "roast/signed-transition-message/v1" +0x00`; the Phase 7 signing package uses its own tag). The tag is +not carried on the wire -- only the body is. A verifier re-derives +the tag, checks the signature over the tag plus the body bytes it +received, and only then parses the body; nothing in the evidence +chain is ever re-encoded. Signature validity therefore never +depends on any serializer's canonical form -- across protobuf +library versions or across languages (the Phase 7 Rust signer +verifies and parses these same bytes). Producers marshal a body +exactly once, at signing time, and transmit those bytes. The original Phase 3 implementation used canonical JSON (`json.Marshal` over field-order-stable structs) as the signed diff --git a/pkg/frost/roast/domain_separation_test.go b/pkg/frost/roast/domain_separation_test.go new file mode 100644 index 0000000000..f4847469b5 --- /dev/null +++ b/pkg/frost/roast/domain_separation_test.go @@ -0,0 +1,242 @@ +package roast + +import ( + "bytes" + "errors" + "sync" + "testing" + + "google.golang.org/protobuf/proto" + + "github.com/keep-network/keep-core/pkg/frost/roast/attempt" + "github.com/keep-network/keep-core/pkg/frost/roast/gen/pb" + "github.com/keep-network/keep-core/pkg/protocol/group" +) + +// These pin the cross-protocol signature-confusion defense for the operator-key +// signed bodies: the signed bytes are domain-tagged and undecodable as +// protobuf, the bytes that travel on the wire stay the bare body, and the two +// body types use distinct tags so a signature over one can never verify as a +// signature over the other. + +func TestSnapshotSignableBytes_DomainSeparated(t *testing.T) { + snap := signedTestSnapshot(t, 7) + signable, err := snap.SignableBytes() + if err != nil { + t.Fatalf("signable: %v", err) + } + body, err := snap.bodyBytes() + if err != nil { + t.Fatalf("body: %v", err) + } + + // SignableBytes = snapshot domain tag || bare body. + if !bytes.HasPrefix(signable, localEvidenceSnapshotSignatureDomain) { + t.Fatal("snapshot signed bytes must carry the snapshot domain tag") + } + if !bytes.Equal(signable[len(localEvidenceSnapshotSignatureDomain):], body) { + t.Fatal("snapshot signed bytes must be the domain tag followed by the bare body") + } + + // The signed bytes begin with an illegal protobuf tag (field 0) and so are + // undecodable as any protobuf message - a snapshot signature can never be + // accepted on a transition (or other) envelope whose decoder parses the + // forged body. + if signable[0] != 0x00 { + t.Fatal("snapshot signed bytes must begin with an illegal protobuf tag (0x00)") + } + if err := proto.Unmarshal(signable, &pb.LocalEvidenceSnapshotBody{}); err == nil { + t.Fatal("snapshot signed bytes must not decode as a protobuf message") + } + + // The bare wire body, by contrast, carries no tag and IS a valid protobuf + // body - the domain tag never travels on the wire. + if bytes.HasPrefix(body, localEvidenceSnapshotSignatureDomain) { + t.Fatal("the wire body must not carry the domain tag") + } + if err := proto.Unmarshal(body, &pb.LocalEvidenceSnapshotBody{}); err != nil { + t.Fatalf("the bare wire body must be a valid protobuf body: %v", err) + } +} + +func TestTransitionSignableBytes_DomainSeparated(t *testing.T) { + msg := buildValidTransitionMessage() + signable, err := msg.SignableBytes() + if err != nil { + t.Fatalf("signable: %v", err) + } + body, err := msg.bodyBytes() + if err != nil { + t.Fatalf("body: %v", err) + } + + if !bytes.HasPrefix(signable, transitionMessageSignatureDomain) { + t.Fatal("transition signed bytes must carry the transition domain tag") + } + if !bytes.Equal(signable[len(transitionMessageSignatureDomain):], body) { + t.Fatal("transition signed bytes must be the domain tag followed by the bare body") + } + if signable[0] != 0x00 { + t.Fatal("transition signed bytes must begin with an illegal protobuf tag (0x00)") + } + if err := proto.Unmarshal(signable, &pb.TransitionMessageBody{}); err == nil { + t.Fatal("transition signed bytes must not decode as a protobuf message") + } + if bytes.HasPrefix(body, transitionMessageSignatureDomain) { + t.Fatal("the wire body must not carry the domain tag") + } + if err := proto.Unmarshal(body, &pb.TransitionMessageBody{}); err != nil { + t.Fatalf("the bare wire body must be a valid protobuf body: %v", err) + } +} + +func TestSignedBodyDomains_AreDistinctAndPrefixFree(t *testing.T) { + // Distinct, prefix-free tags make the signed-byte spaces of the two body + // types disjoint: domain_a || body_a == domain_b || body_b is impossible + // unless one tag is a prefix of the other, so a signature over one body can + // never verify as a signature over the other. + a := localEvidenceSnapshotSignatureDomain + b := transitionMessageSignatureDomain + if bytes.Equal(a, b) { + t.Fatal("each signed body type must use a distinct domain tag") + } + if bytes.HasPrefix(a, b) || bytes.HasPrefix(b, a) { + t.Fatal("no domain tag may be a prefix of another") + } + for _, tag := range [][]byte{a, b} { + if len(tag) == 0 || tag[0] != 0x00 { + t.Fatalf("domain tag %q must begin with an illegal protobuf tag (0x00)", tag) + } + } +} + +func TestSnapshotUnmarshal_ResetsSignableCache(t *testing.T) { + // A snapshot value reused across a SignableBytes call and then an Unmarshal + // must authenticate the newly decoded snapshot against the bytes it just + // received, never the stale cached payload. + reused := signedTestSnapshot(t, 7) + if _, err := reused.SignableBytes(); err != nil { // prime the cache + t.Fatalf("prime cache: %v", err) + } + + genuine := signedTestSnapshot(t, 9) + wire, err := genuine.Marshal() + if err != nil { + t.Fatalf("marshal: %v", err) + } + if err := reused.Unmarshal(wire); err != nil { + t.Fatalf("unmarshal into reused value: %v", err) + } + + got, _ := reused.SignableBytes() + want, _ := genuine.SignableBytes() + if !bytes.Equal(got, want) { + t.Fatal("Unmarshal must reset the snapshot signable-bytes cache") + } + if err := verifySnapshotSignature(fakeVerifier{}, reused); err != nil { + t.Fatalf("authenticate reused-decoded snapshot: %v", err) + } +} + +func TestTransitionUnmarshal_ResetsSignableCache(t *testing.T) { + reused := buildValidTransitionMessage() + if _, err := reused.SignableBytes(); err != nil { // prime the cache + t.Fatalf("prime cache: %v", err) + } + + // Decode a structurally different genuine bundle into the SAME value. + other := buildValidTransitionMessage() + other.CoordinatorIDValue = 2 + other.CoordinatorSignature = bytes.Repeat([]byte{0xcd}, 64) + wire, err := other.Marshal() + if err != nil { + t.Fatalf("marshal: %v", err) + } + if err := reused.Unmarshal(wire); err != nil { + t.Fatalf("unmarshal into reused value: %v", err) + } + + got, _ := reused.SignableBytes() + want, _ := other.SignableBytes() + if !bytes.Equal(got, want) { + t.Fatal("Unmarshal must reset the transition signable-bytes cache") + } +} + +func TestCrossProtocol_TransitionSignatureRejectedAsSnapshot(t *testing.T) { + // End-to-end: a coordinator signature over a transition message must not + // verify as an operator signature over a snapshot, even when the snapshot + // shares the signer id and attempt context. The distinct domain tags (and + // bodies) make the signed preimages disjoint. + const id group.MemberIndex = 7 + + transition := buildValidTransitionMessage() + transition.CoordinatorIDValue = uint32(id) + transition.CoordinatorSignature = nil + tPayload, err := transition.SignableBytes() + if err != nil { + t.Fatalf("transition signable: %v", err) + } + tSig, err := (&fakeSigner{id: id}).Sign(tPayload) + if err != nil { + t.Fatalf("sign transition: %v", err) + } + transition.CoordinatorSignature = tSig + // Control: the signature really is a valid bundle signature. + if err := verifyBundleSignature(fakeVerifier{}, transition, id); err != nil { + t.Fatalf("control: genuine transition signature must verify: %v", err) + } + + // Paste it onto a snapshot from the same signer + attempt. + snap := NewLocalEvidenceSnapshot(id, pinnedContextHash, attempt.Evidence{}) + snap.OperatorSignature = tSig + if err := verifySnapshotSignature(fakeVerifier{}, snap); !errors.Is(err, ErrSignatureInvalid) { + t.Fatalf("transition signature must not verify as a snapshot signature; got %v", err) + } +} + +func TestCrossProtocol_SnapshotSignatureRejectedAsTransition(t *testing.T) { + // The reverse direction: an operator signature over a snapshot must not + // verify as a coordinator signature over a transition message. + const id group.MemberIndex = 7 + snap := signedTestSnapshot(t, id) + // Control: it verifies as a snapshot signature. + if err := verifySnapshotSignature(fakeVerifier{}, snap); err != nil { + t.Fatalf("control: genuine snapshot signature must verify: %v", err) + } + + transition := buildValidTransitionMessage() + transition.CoordinatorIDValue = uint32(id) + transition.CoordinatorSignature = snap.OperatorSignature + if err := verifyBundleSignature(fakeVerifier{}, transition, id); !errors.Is(err, ErrSignatureInvalid) { + t.Fatalf("snapshot signature must not verify as a transition signature; got %v", err) + } +} + +func TestSnapshotSignableBytes_ConcurrentAfterUnmarshalIsRaceFree(t *testing.T) { + // Regression guard (run under -race): a parsed snapshot must carry a primed + // signable-bytes cache so concurrent signature verification reads a ready + // cache instead of racing on lazy initialization. Without priming in + // Unmarshal, the concurrent first SignableBytes calls below race on the + // cache write. + wire, err := signedTestSnapshot(t, 7).Marshal() + if err != nil { + t.Fatalf("marshal: %v", err) + } + var decoded LocalEvidenceSnapshot + if err := decoded.Unmarshal(wire); err != nil { + t.Fatalf("unmarshal: %v", err) + } + + var wg sync.WaitGroup + for i := 0; i < 8; i++ { + wg.Add(1) + go func() { + defer wg.Done() + if _, err := decoded.SignableBytes(); err != nil { + t.Errorf("signable: %v", err) + } + }() + } + wg.Wait() +} diff --git a/pkg/frost/roast/equivocation_test.go b/pkg/frost/roast/equivocation_test.go index e4568ac47b..03de8c4972 100644 --- a/pkg/frost/roast/equivocation_test.go +++ b/pkg/frost/roast/equivocation_test.go @@ -94,7 +94,8 @@ func TestOwnSnapshotMutatedInBundle_RetainsBothSignedEnvelopes(t *testing.T) { mutated := *selfSubmission mutated.OperatorSignature = bytes.Repeat([]byte{0xff}, 64) // Fresh caches: the mutated copy is a distinct signed object. - mutated.signedBody = nil + mutated.bodyCache = nil + mutated.signaturePayloadCache = nil mutated.wireEnvelope = nil bundle := &TransitionMessage{ diff --git a/pkg/frost/roast/gen/pb/evidence.pb.go b/pkg/frost/roast/gen/pb/evidence.pb.go index e026531675..d53f4e7d5d 100644 --- a/pkg/frost/roast/gen/pb/evidence.pb.go +++ b/pkg/frost/roast/gen/pb/evidence.pb.go @@ -20,8 +20,9 @@ const ( _ = protoimpl.EnforceVersion(protoimpl.MaxVersion - 20) ) -// The byte stream a signer's operator key signs. Carried as exact bytes in -// SignedLocalEvidenceSnapshot.body. +// The evidence-snapshot body. Carried verbatim as +// SignedLocalEvidenceSnapshot.body; the operator signature covers the +// domain-tagged form of these bytes (domain_tag || body), not the bare bytes. type LocalEvidenceSnapshotBody struct { state protoimpl.MessageState `protogen:"open.v1"` SenderId uint32 `protobuf:"varint,1,opt,name=sender_id,json=senderId,proto3" json:"sender_id,omitempty"` @@ -267,8 +268,9 @@ func (x *ConflictEntry) GetCount() uint64 { return 0 } -// The on-wire snapshot message: exact signed body bytes plus the operator -// signature over them. +// The on-wire snapshot message: the exact serialized LocalEvidenceSnapshotBody +// bytes plus the operator signature, which covers the domain-tagged body +// (domain_tag || body), not the bare body field. type SignedLocalEvidenceSnapshot struct { state protoimpl.MessageState `protogen:"open.v1"` Body []byte `protobuf:"bytes,1,opt,name=body,proto3" json:"body,omitempty"` @@ -321,11 +323,13 @@ func (x *SignedLocalEvidenceSnapshot) GetOperatorSignature() []byte { return nil } -// The byte stream the elected coordinator signs. signed_snapshots carries -// each member's SignedLocalEvidenceSnapshot envelope verbatim as received, -// so the coordinator attests to the exact signed snapshots it assembled, -// in order, and downstream verifiers re-check the operator signatures over -// those same exact bytes. +// The transition-message body. signed_snapshots carries each member's +// SignedLocalEvidenceSnapshot envelope verbatim as received, so the +// coordinator attests to the exact signed snapshots it assembled, in order, +// and downstream verifiers re-check the operator signatures over those same +// exact bytes. Carried verbatim as SignedTransitionMessage.body; the +// coordinator signature covers the domain-tagged form of these bytes +// (domain_tag || body), not the bare bytes. type TransitionMessageBody struct { state protoimpl.MessageState `protogen:"open.v1"` AttemptContextHash []byte `protobuf:"bytes,1,opt,name=attempt_context_hash,json=attemptContextHash,proto3" json:"attempt_context_hash,omitempty"` @@ -386,8 +390,9 @@ func (x *TransitionMessageBody) GetSignedSnapshots() [][]byte { return nil } -// The on-wire transition message: exact signed body bytes plus the -// coordinator signature over them. +// The on-wire transition message: the exact serialized TransitionMessageBody +// bytes plus the coordinator signature, which covers the domain-tagged body +// (domain_tag || body), not the bare body field. type SignedTransitionMessage struct { state protoimpl.MessageState `protogen:"open.v1"` Body []byte `protobuf:"bytes,1,opt,name=body,proto3" json:"body,omitempty"` diff --git a/pkg/frost/roast/gen/pb/evidence.proto b/pkg/frost/roast/gen/pb/evidence.proto index ac78895eb0..3b8075e459 100644 --- a/pkg/frost/roast/gen/pb/evidence.proto +++ b/pkg/frost/roast/gen/pb/evidence.proto @@ -5,15 +5,28 @@ package roast; // Evidence wire format (RFC-21 Layer B). // -// Signatures cover exact serialized body bytes, and those bytes travel -// verbatim: a verifier checks the signature over the bytes it received and -// only then parses them. Nothing in the evidence chain is ever re-encoded, -// so signature validity never depends on any serializer's canonical form - -// across protobuf library versions or across languages (the Phase 7 Rust -// signer verifies and parses these same bytes). +// The body bytes travel verbatim: a verifier checks the signature over the +// bytes it received and only then parses them. Nothing in the evidence chain +// is ever re-encoded, so signature validity never depends on any serializer's +// canonical form - across protobuf library versions or across languages (the +// Phase 7 Rust signer verifies and parses these same bytes). +// +// SIGNED PAYLOAD (cross-language contract). Operator and coordinator +// signatures do NOT cover the bare body; they cover a domain-separated byte +// stream, domain_tag || serialized body, where domain_tag is, in order: a +// single 0x00 byte, a fixed ASCII string, and a trailing 0x00 byte. The +// leading 0x00 is an illegal protobuf tag (field number 0), so the signed +// payload is undecodable as any protobuf message and a signature over one body +// type can never be accepted for another. The tag is NOT carried on the wire - +// only the body is, in the .body field. The ASCII strings are +// "roast/signed-evidence-snapshot/v1" (operator_signature) and +// "roast/signed-transition-message/v1" (coordinator_signature). Any +// implementation that signs or verifies the bare body (without the exact tag) +// will fail to interoperate. -// The byte stream a signer's operator key signs. Carried as exact bytes in -// SignedLocalEvidenceSnapshot.body. +// The evidence-snapshot body. Carried verbatim as +// SignedLocalEvidenceSnapshot.body; the operator signature covers the +// domain-tagged form of these bytes (domain_tag || body), not the bare bytes. message LocalEvidenceSnapshotBody { uint32 sender_id = 1; // 32-byte attempt context hash binding the evidence to one attempt. @@ -43,26 +56,30 @@ message ConflictEntry { uint64 count = 2; } -// The on-wire snapshot message: exact signed body bytes plus the operator -// signature over them. +// The on-wire snapshot message: the exact serialized LocalEvidenceSnapshotBody +// bytes plus the operator signature, which covers the domain-tagged body +// (domain_tag || body), not the bare body field. message SignedLocalEvidenceSnapshot { bytes body = 1; bytes operator_signature = 2; } -// The byte stream the elected coordinator signs. signed_snapshots carries -// each member's SignedLocalEvidenceSnapshot envelope verbatim as received, -// so the coordinator attests to the exact signed snapshots it assembled, -// in order, and downstream verifiers re-check the operator signatures over -// those same exact bytes. +// The transition-message body. signed_snapshots carries each member's +// SignedLocalEvidenceSnapshot envelope verbatim as received, so the +// coordinator attests to the exact signed snapshots it assembled, in order, +// and downstream verifiers re-check the operator signatures over those same +// exact bytes. Carried verbatim as SignedTransitionMessage.body; the +// coordinator signature covers the domain-tagged form of these bytes +// (domain_tag || body), not the bare bytes. message TransitionMessageBody { bytes attempt_context_hash = 1; uint32 coordinator_id = 2; repeated bytes signed_snapshots = 3; } -// The on-wire transition message: exact signed body bytes plus the -// coordinator signature over them. +// The on-wire transition message: the exact serialized TransitionMessageBody +// bytes plus the coordinator signature, which covers the domain-tagged body +// (domain_tag || body), not the bare body field. message SignedTransitionMessage { bytes body = 1; bytes coordinator_signature = 2; diff --git a/pkg/frost/roast/transition_message.go b/pkg/frost/roast/transition_message.go index f8499ad91f..4a63ff2550 100644 --- a/pkg/frost/roast/transition_message.go +++ b/pkg/frost/roast/transition_message.go @@ -101,15 +101,20 @@ type LocalEvidenceSnapshot struct { // observed. Conflicts []ConflictEntry // OperatorSignature is the signer's operator-key signature over - // SignableBytes(): the serialized protobuf body of (senderID, - // attemptContextHash, overflows, rejects, conflicts). + // SignableBytes(): the snapshot domain tag followed by the serialized + // protobuf body of (senderID, attemptContextHash, overflows, rejects, + // conflicts). OperatorSignature []byte - // signedBody caches the exact serialized body bytes the - // OperatorSignature covers: marshaled once at signing time for - // self-authored snapshots, or the received bytes verbatim for - // parsed ones. Evidence fields must not be mutated once set. - signedBody []byte + // bodyCache caches the exact serialized body bytes carried on the wire: + // marshaled once at signing time for self-authored snapshots, or the + // received bytes verbatim for parsed ones. Evidence fields must not be + // mutated once set. + bodyCache []byte + // signaturePayloadCache caches the domain-tagged bytes the + // OperatorSignature covers (localEvidenceSnapshotSignatureDomain || + // bodyCache); rebuilt from bodyCache and never carried on the wire. + signaturePayloadCache []byte // wireEnvelope caches the exact on-wire envelope (body + // signature): the received bytes verbatim for parsed snapshots, // or built once after signing for self-authored ones. @@ -246,8 +251,17 @@ func (s *LocalEvidenceSnapshot) Unmarshal(data []byte) error { } snapshotFieldsFromBody(s, &body) s.OperatorSignature = append([]byte(nil), envelope.OperatorSignature...) - s.signedBody = append([]byte(nil), envelope.Body...) + s.bodyCache = append([]byte(nil), envelope.Body...) s.wireEnvelope = append([]byte(nil), data...) + // Prime the signable-bytes cache from the body just received, discarding any + // cache a prior SignableBytes call left on a reused value. Priming here - + // rather than lazily in SignableBytes - keeps concurrent signature + // verification of a parsed snapshot race-free: verifiers read a ready cache + // instead of racing on lazy initialization. + s.signaturePayloadCache = nil + if _, err := s.SignableBytes(); err != nil { + return err + } return s.Validate() } @@ -326,15 +340,16 @@ type TransitionMessage struct { // Bundle is the canonical sorted-by-SenderID list of signed // evidence snapshots aggregated by the coordinator. Bundle []LocalEvidenceSnapshot - // CoordinatorSignature is the coordinator's operator-key - // signature over SignableBytes(): the serialized protobuf body - // embedding every snapshot's signed envelope verbatim. + // CoordinatorSignature is the coordinator's operator-key signature over + // SignableBytes(): the transition domain tag followed by the serialized + // protobuf body embedding every snapshot's signed envelope verbatim. CoordinatorSignature []byte - // signedBody and wireEnvelope cache exact bytes with the same - // semantics as the LocalEvidenceSnapshot caches. - signedBody []byte - wireEnvelope []byte + // bodyCache, signaturePayloadCache, and wireEnvelope cache exact bytes + // with the same semantics as the LocalEvidenceSnapshot caches. + bodyCache []byte + signaturePayloadCache []byte + wireEnvelope []byte } // CoordinatorID returns the coordinator member index as a @@ -373,7 +388,7 @@ func (m *TransitionMessage) Marshal() ([]byte, error) { "transition message: must be signed before wire encoding", ) } - body, err := m.SignableBytes() + body, err := m.bodyBytes() if err != nil { return nil, err } @@ -426,8 +441,15 @@ func (m *TransitionMessage) Unmarshal(data []byte) error { m.Bundle = append(m.Bundle, snapshot) } m.CoordinatorSignature = append([]byte(nil), envelope.CoordinatorSignature...) - m.signedBody = append([]byte(nil), envelope.Body...) + m.bodyCache = append([]byte(nil), envelope.Body...) m.wireEnvelope = append([]byte(nil), data...) + // Prime the signable-bytes cache so concurrent verification of a parsed + // bundle is race-free, and discard any stale cache on a reused value (see + // LocalEvidenceSnapshot.Unmarshal). + m.signaturePayloadCache = nil + if _, err := m.SignableBytes(); err != nil { + return err + } return m.Validate() } diff --git a/pkg/frost/roast/wire.go b/pkg/frost/roast/wire.go index a7932aae49..5da542c134 100644 --- a/pkg/frost/roast/wire.go +++ b/pkg/frost/roast/wire.go @@ -19,6 +19,33 @@ import ( // on any serializer's canonical form, across protobuf library versions or // across languages. Producers marshal a body exactly once (at signing // time) and cache it; parsed messages cache the received bytes. +// +// Domain separation. Several signed bodies share the node's operator key and +// are structurally similar - each carries an attempt-context hash and a member +// index. The TransitionMessageBody and the signing-package body are outright +// wire-compatible (both have attempt_context_hash as a length-delimited field +// 1). The LocalEvidenceSnapshotBody is only INCIDENTALLY distinguished - its +// field 1 is sender_id (a varint), not a length-delimited field - a difference +// a later proto change could erase. So a signature over one body must not be +// acceptable as a signature over another. Each signed-body type therefore +// prepends a UNIQUE domain tag to the bytes it signs and verifies +// (SignableBytes), making the separation intentional rather than incidental, +// while the body that travels on the wire stays the bare serialized body +// (bodyBytes). +// +// Each tag BEGINS with byte 0x00 - an illegal protobuf tag (field number 0) - +// so the signed payload is undecodable as any protobuf message. That separates +// the domains in both directions without relying on field layout: a signature +// over one body cannot be replayed onto another envelope (whose decoder +// proto.Unmarshals and rejects the 0x00-leading body), and a genuine +// serialized protobuf body always begins with a valid tag (>= 0x08), so its +// signature can never verify against domain || body. The tags are NOT carried +// on the wire; signer and verifier prepend the same constant. (The signed +// signing-package envelope follows the same scheme with its own tag.) +var ( + localEvidenceSnapshotSignatureDomain = []byte("\x00roast/signed-evidence-snapshot/v1\x00") + transitionMessageSignatureDomain = []byte("\x00roast/signed-transition-message/v1\x00") +) func snapshotBodyMessage(s *LocalEvidenceSnapshot) *pb.LocalEvidenceSnapshotBody { body := &pb.LocalEvidenceSnapshotBody{ @@ -74,28 +101,54 @@ func snapshotFieldsFromBody(s *LocalEvidenceSnapshot, body *pb.LocalEvidenceSnap } } -// SignableBytes returns the exact byte stream the OperatorSignature covers: -// the serialized LocalEvidenceSnapshotBody. For a self-authored snapshot -// the body is marshaled once and cached - sign exactly what will be -// transmitted. For a snapshot parsed off the wire this returns the -// received body bytes verbatim - verify exactly what was received. The -// snapshot's evidence fields must not be mutated afterwards, and the -// returned slice is the internal cache - callers must not mutate it. -func (s *LocalEvidenceSnapshot) SignableBytes() ([]byte, error) { +// bodyBytes returns the exact serialized LocalEvidenceSnapshotBody - the body +// carried verbatim in the SignedLocalEvidenceSnapshot envelope. Marshaled once +// and cached for a self-authored snapshot; the received bytes verbatim for a +// parsed one. The returned slice is the internal cache - callers must not +// mutate it. +func (s *LocalEvidenceSnapshot) bodyBytes() ([]byte, error) { if s == nil { return nil, errors.New("roast: cannot encode a nil snapshot") } - if s.signedBody != nil { - return s.signedBody, nil + if s.bodyCache != nil { + return s.bodyCache, nil } body, err := proto.Marshal(snapshotBodyMessage(s)) if err != nil { return nil, fmt.Errorf("roast: marshal snapshot body: %w", err) } - s.signedBody = body + s.bodyCache = body return body, nil } +// SignableBytes returns the exact byte stream the OperatorSignature covers: +// the snapshot domain tag (localEvidenceSnapshotSignatureDomain) followed by +// the serialized LocalEvidenceSnapshotBody. The domain tag is a fixed constant +// prepended by both signer and verifier and is NOT carried on the wire - it +// domain-separates this signature from the node's other signed bodies (see the +// package comment). The body half is the bytes that travel: marshaled once for +// a self-authored snapshot, or the received body verbatim for a parsed one +// (verify exactly what was received). Evidence fields must not be mutated +// afterwards, and the returned slice is the internal cache - callers must not +// mutate it. +func (s *LocalEvidenceSnapshot) SignableBytes() ([]byte, error) { + if s == nil { + return nil, errors.New("roast: cannot encode a nil snapshot") + } + if s.signaturePayloadCache != nil { + return s.signaturePayloadCache, nil + } + body, err := s.bodyBytes() + if err != nil { + return nil, err + } + payload := make([]byte, 0, len(localEvidenceSnapshotSignatureDomain)+len(body)) + payload = append(payload, localEvidenceSnapshotSignatureDomain...) + payload = append(payload, body...) + s.signaturePayloadCache = payload + return payload, nil +} + // wireEnvelopeBytes returns the exact on-wire SignedLocalEvidenceSnapshot // envelope. For parsed snapshots this is the received envelope verbatim; // for self-authored snapshots it is built once (after signing) and cached, @@ -110,7 +163,7 @@ func (s *LocalEvidenceSnapshot) wireEnvelopeBytes() ([]byte, error) { "roast: snapshot must be signed before wire encoding", ) } - body, err := s.SignableBytes() + body, err := s.bodyBytes() if err != nil { return nil, err } @@ -125,19 +178,17 @@ func (s *LocalEvidenceSnapshot) wireEnvelopeBytes() ([]byte, error) { return envelope, nil } -// SignableBytes returns the exact byte stream the CoordinatorSignature -// covers: the serialized TransitionMessageBody, which embeds every -// snapshot's signed envelope verbatim. The coordinator's signature -// attests that these specific signed snapshots were assembled in this -// specific order. For a message parsed off the wire this returns the -// received body bytes verbatim. The returned slice is the internal -// cache - callers must not mutate it. -func (m *TransitionMessage) SignableBytes() ([]byte, error) { +// bodyBytes returns the exact serialized TransitionMessageBody, which embeds +// every snapshot's signed envelope verbatim - the body carried in the +// SignedTransitionMessage envelope. Built and cached once for a self-authored +// message; the received bytes verbatim for a parsed one. The returned slice is +// the internal cache - callers must not mutate it. +func (m *TransitionMessage) bodyBytes() ([]byte, error) { if m == nil { return nil, errors.New("roast: cannot encode a nil transition message") } - if m.signedBody != nil { - return m.signedBody, nil + if m.bodyCache != nil { + return m.bodyCache, nil } body := &pb.TransitionMessageBody{ AttemptContextHash: m.AttemptContextHash, @@ -154,6 +205,33 @@ func (m *TransitionMessage) SignableBytes() ([]byte, error) { if err != nil { return nil, fmt.Errorf("roast: marshal transition body: %w", err) } - m.signedBody = bodyBytes + m.bodyCache = bodyBytes return bodyBytes, nil } + +// SignableBytes returns the exact byte stream the CoordinatorSignature covers: +// the transition domain tag (transitionMessageSignatureDomain) followed by the +// serialized TransitionMessageBody. The domain tag is a fixed constant +// prepended by both signer and verifier and is NOT carried on the wire - it +// domain-separates the coordinator's bundle signature from its other signed +// bodies (see the package comment). The coordinator's signature attests that +// these specific signed snapshots were assembled in this specific order; for a +// message parsed off the wire the body half is the received bytes verbatim. +// The returned slice is the internal cache - callers must not mutate it. +func (m *TransitionMessage) SignableBytes() ([]byte, error) { + if m == nil { + return nil, errors.New("roast: cannot encode a nil transition message") + } + if m.signaturePayloadCache != nil { + return m.signaturePayloadCache, nil + } + body, err := m.bodyBytes() + if err != nil { + return nil, err + } + payload := make([]byte, 0, len(transitionMessageSignatureDomain)+len(body)) + payload = append(payload, transitionMessageSignatureDomain...) + payload = append(payload, body...) + m.signaturePayloadCache = payload + return payload, nil +} diff --git a/pkg/frost/roast/wire_test.go b/pkg/frost/roast/wire_test.go index e9ddf2e3e8..d20eb879a2 100644 --- a/pkg/frost/roast/wire_test.go +++ b/pkg/frost/roast/wire_test.go @@ -66,7 +66,10 @@ func TestSnapshotWire_ReceivedBytesPreservedVerbatim(t *testing.T) { func TestSnapshotWire_NonCanonicalEnvelopeEncodingSurvives(t *testing.T) { original := signedTestSnapshot(t, 7) - body, _ := original.SignableBytes() + // The wire body is the bare serialized body (NOT the domain-tagged + // SignableBytes); the signature over SignableBytes still verifies against + // it after decode. + body, _ := original.bodyBytes() // Handcraft an envelope with the fields in REVERSE tag order // (operator_signature before body) - a wire-legal but non-canonical