From 8d335bacc704bb5b885147a7ef6085b3255dbc84 Mon Sep 17 00:00:00 2001 From: maclane Date: Sat, 13 Jun 2026 21:07:56 -0400 Subject: [PATCH 1/8] feat(frost/roast): Phase 7.2b-2 signed signing-package envelope (wire) The wire foundation for the coordinator-distributed signing package (frozen spec section 6): SigningPackageBody {attempt_context_hash, coordinator_id, signing_package, taproot_merkle_root} + SignedSigningPackage {body, coordinator_signature} protos, and a SigningPackage Go type following the established sign-what-you-transmit / verify-what-you-received discipline (wire.go): SignableBytes caches the body marshaled once; Unmarshal retains the received body + envelope verbatim; Validate does structural checks. Byte-preservation (the point of the envelope) is pinned by tests: received bytes survive re-broadcast verbatim, a non-canonical encoding is preserved, key-path and script-path roots round-trip, and Validate rejects malformed packages. proto generated with protoc-gen-go v1.36.3 into the existing gen/pb dir (no new Dockerfile gen-allowlist entry). Coordinator signing/distribution and member-side authentication (elected-coordinator check, signature verification, root binding) + retention + the context-bound member-authenticated share submission follow on this branch; the engine never sees this envelope (blame adjudication is Go-side). Co-Authored-By: Claude Opus 4.8 --- pkg/frost/roast/gen/pb/signing_package.pb.go | 230 +++++++++++++++++++ pkg/frost/roast/gen/pb/signing_package.proto | 38 +++ pkg/frost/roast/signing_package.go | 225 ++++++++++++++++++ pkg/frost/roast/signing_package_test.go | 177 ++++++++++++++ 4 files changed, 670 insertions(+) create mode 100644 pkg/frost/roast/gen/pb/signing_package.pb.go create mode 100644 pkg/frost/roast/gen/pb/signing_package.proto create mode 100644 pkg/frost/roast/signing_package.go create mode 100644 pkg/frost/roast/signing_package_test.go diff --git a/pkg/frost/roast/gen/pb/signing_package.pb.go b/pkg/frost/roast/gen/pb/signing_package.pb.go new file mode 100644 index 0000000000..98290c7e99 --- /dev/null +++ b/pkg/frost/roast/gen/pb/signing_package.pb.go @@ -0,0 +1,230 @@ +// Code generated by protoc-gen-go. DO NOT EDIT. +// versions: +// protoc-gen-go v1.36.3 +// protoc v6.33.4 +// source: pkg/frost/roast/gen/pb/signing_package.proto + +package pb + +import ( + protoreflect "google.golang.org/protobuf/reflect/protoreflect" + protoimpl "google.golang.org/protobuf/runtime/protoimpl" + reflect "reflect" + sync "sync" +) + +const ( + // Verify that this generated code is sufficiently up-to-date. + _ = protoimpl.EnforceVersion(20 - protoimpl.MinVersion) + // Verify that runtime/protoimpl is sufficiently up-to-date. + _ = protoimpl.EnforceVersion(protoimpl.MaxVersion - 20) +) + +// The byte stream the elected coordinator's operator key signs. Carried as +// exact bytes in SignedSigningPackage.body. +type SigningPackageBody struct { + state protoimpl.MessageState `protogen:"open.v1"` + // 32-byte attempt context hash binding the package to one attempt. + AttemptContextHash []byte `protobuf:"bytes,1,opt,name=attempt_context_hash,json=attemptContextHash,proto3" json:"attempt_context_hash,omitempty"` + // The elected coordinator's member index (RFC-21 Annex A). A member + // verifies this is the attempt's elected coordinator and that the + // signature verifies under that coordinator's operator key. + CoordinatorId uint32 `protobuf:"varint,2,opt,name=coordinator_id,json=coordinatorId,proto3" json:"coordinator_id,omitempty"` + // The serialized FROST SigningPackage the chosen subset signs over. + SigningPackage []byte `protobuf:"bytes,3,opt,name=signing_package,json=signingPackage,proto3" json:"signing_package,omitempty"` + // The 32-byte taproot script-tree root the signature is tweaked by; + // empty for a key-path spend. + TaprootMerkleRoot []byte `protobuf:"bytes,4,opt,name=taproot_merkle_root,json=taprootMerkleRoot,proto3" json:"taproot_merkle_root,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *SigningPackageBody) Reset() { + *x = SigningPackageBody{} + mi := &file_pkg_frost_roast_gen_pb_signing_package_proto_msgTypes[0] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *SigningPackageBody) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*SigningPackageBody) ProtoMessage() {} + +func (x *SigningPackageBody) ProtoReflect() protoreflect.Message { + mi := &file_pkg_frost_roast_gen_pb_signing_package_proto_msgTypes[0] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use SigningPackageBody.ProtoReflect.Descriptor instead. +func (*SigningPackageBody) Descriptor() ([]byte, []int) { + return file_pkg_frost_roast_gen_pb_signing_package_proto_rawDescGZIP(), []int{0} +} + +func (x *SigningPackageBody) GetAttemptContextHash() []byte { + if x != nil { + return x.AttemptContextHash + } + return nil +} + +func (x *SigningPackageBody) GetCoordinatorId() uint32 { + if x != nil { + return x.CoordinatorId + } + return 0 +} + +func (x *SigningPackageBody) GetSigningPackage() []byte { + if x != nil { + return x.SigningPackage + } + return nil +} + +func (x *SigningPackageBody) GetTaprootMerkleRoot() []byte { + if x != nil { + return x.TaprootMerkleRoot + } + return nil +} + +// The on-wire signing package: exact signed body bytes plus the elected +// coordinator's operator signature over them. +type SignedSigningPackage struct { + state protoimpl.MessageState `protogen:"open.v1"` + Body []byte `protobuf:"bytes,1,opt,name=body,proto3" json:"body,omitempty"` + CoordinatorSignature []byte `protobuf:"bytes,2,opt,name=coordinator_signature,json=coordinatorSignature,proto3" json:"coordinator_signature,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *SignedSigningPackage) Reset() { + *x = SignedSigningPackage{} + mi := &file_pkg_frost_roast_gen_pb_signing_package_proto_msgTypes[1] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *SignedSigningPackage) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*SignedSigningPackage) ProtoMessage() {} + +func (x *SignedSigningPackage) ProtoReflect() protoreflect.Message { + mi := &file_pkg_frost_roast_gen_pb_signing_package_proto_msgTypes[1] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use SignedSigningPackage.ProtoReflect.Descriptor instead. +func (*SignedSigningPackage) Descriptor() ([]byte, []int) { + return file_pkg_frost_roast_gen_pb_signing_package_proto_rawDescGZIP(), []int{1} +} + +func (x *SignedSigningPackage) GetBody() []byte { + if x != nil { + return x.Body + } + return nil +} + +func (x *SignedSigningPackage) GetCoordinatorSignature() []byte { + if x != nil { + return x.CoordinatorSignature + } + return nil +} + +var File_pkg_frost_roast_gen_pb_signing_package_proto protoreflect.FileDescriptor + +var file_pkg_frost_roast_gen_pb_signing_package_proto_rawDesc = []byte{ + 0x0a, 0x2c, 0x70, 0x6b, 0x67, 0x2f, 0x66, 0x72, 0x6f, 0x73, 0x74, 0x2f, 0x72, 0x6f, 0x61, 0x73, + 0x74, 0x2f, 0x67, 0x65, 0x6e, 0x2f, 0x70, 0x62, 0x2f, 0x73, 0x69, 0x67, 0x6e, 0x69, 0x6e, 0x67, + 0x5f, 0x70, 0x61, 0x63, 0x6b, 0x61, 0x67, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x12, 0x05, + 0x72, 0x6f, 0x61, 0x73, 0x74, 0x22, 0xc6, 0x01, 0x0a, 0x12, 0x53, 0x69, 0x67, 0x6e, 0x69, 0x6e, + 0x67, 0x50, 0x61, 0x63, 0x6b, 0x61, 0x67, 0x65, 0x42, 0x6f, 0x64, 0x79, 0x12, 0x30, 0x0a, 0x14, + 0x61, 0x74, 0x74, 0x65, 0x6d, 0x70, 0x74, 0x5f, 0x63, 0x6f, 0x6e, 0x74, 0x65, 0x78, 0x74, 0x5f, + 0x68, 0x61, 0x73, 0x68, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x12, 0x61, 0x74, 0x74, 0x65, + 0x6d, 0x70, 0x74, 0x43, 0x6f, 0x6e, 0x74, 0x65, 0x78, 0x74, 0x48, 0x61, 0x73, 0x68, 0x12, 0x25, + 0x0a, 0x0e, 0x63, 0x6f, 0x6f, 0x72, 0x64, 0x69, 0x6e, 0x61, 0x74, 0x6f, 0x72, 0x5f, 0x69, 0x64, + 0x18, 0x02, 0x20, 0x01, 0x28, 0x0d, 0x52, 0x0d, 0x63, 0x6f, 0x6f, 0x72, 0x64, 0x69, 0x6e, 0x61, + 0x74, 0x6f, 0x72, 0x49, 0x64, 0x12, 0x27, 0x0a, 0x0f, 0x73, 0x69, 0x67, 0x6e, 0x69, 0x6e, 0x67, + 0x5f, 0x70, 0x61, 0x63, 0x6b, 0x61, 0x67, 0x65, 0x18, 0x03, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x0e, + 0x73, 0x69, 0x67, 0x6e, 0x69, 0x6e, 0x67, 0x50, 0x61, 0x63, 0x6b, 0x61, 0x67, 0x65, 0x12, 0x2e, + 0x0a, 0x13, 0x74, 0x61, 0x70, 0x72, 0x6f, 0x6f, 0x74, 0x5f, 0x6d, 0x65, 0x72, 0x6b, 0x6c, 0x65, + 0x5f, 0x72, 0x6f, 0x6f, 0x74, 0x18, 0x04, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x11, 0x74, 0x61, 0x70, + 0x72, 0x6f, 0x6f, 0x74, 0x4d, 0x65, 0x72, 0x6b, 0x6c, 0x65, 0x52, 0x6f, 0x6f, 0x74, 0x22, 0x5f, + 0x0a, 0x14, 0x53, 0x69, 0x67, 0x6e, 0x65, 0x64, 0x53, 0x69, 0x67, 0x6e, 0x69, 0x6e, 0x67, 0x50, + 0x61, 0x63, 0x6b, 0x61, 0x67, 0x65, 0x12, 0x12, 0x0a, 0x04, 0x62, 0x6f, 0x64, 0x79, 0x18, 0x01, + 0x20, 0x01, 0x28, 0x0c, 0x52, 0x04, 0x62, 0x6f, 0x64, 0x79, 0x12, 0x33, 0x0a, 0x15, 0x63, 0x6f, + 0x6f, 0x72, 0x64, 0x69, 0x6e, 0x61, 0x74, 0x6f, 0x72, 0x5f, 0x73, 0x69, 0x67, 0x6e, 0x61, 0x74, + 0x75, 0x72, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x14, 0x63, 0x6f, 0x6f, 0x72, 0x64, + 0x69, 0x6e, 0x61, 0x74, 0x6f, 0x72, 0x53, 0x69, 0x67, 0x6e, 0x61, 0x74, 0x75, 0x72, 0x65, 0x42, + 0x06, 0x5a, 0x04, 0x2e, 0x2f, 0x70, 0x62, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, +} + +var ( + file_pkg_frost_roast_gen_pb_signing_package_proto_rawDescOnce sync.Once + file_pkg_frost_roast_gen_pb_signing_package_proto_rawDescData = file_pkg_frost_roast_gen_pb_signing_package_proto_rawDesc +) + +func file_pkg_frost_roast_gen_pb_signing_package_proto_rawDescGZIP() []byte { + file_pkg_frost_roast_gen_pb_signing_package_proto_rawDescOnce.Do(func() { + file_pkg_frost_roast_gen_pb_signing_package_proto_rawDescData = protoimpl.X.CompressGZIP(file_pkg_frost_roast_gen_pb_signing_package_proto_rawDescData) + }) + return file_pkg_frost_roast_gen_pb_signing_package_proto_rawDescData +} + +var file_pkg_frost_roast_gen_pb_signing_package_proto_msgTypes = make([]protoimpl.MessageInfo, 2) +var file_pkg_frost_roast_gen_pb_signing_package_proto_goTypes = []any{ + (*SigningPackageBody)(nil), // 0: roast.SigningPackageBody + (*SignedSigningPackage)(nil), // 1: roast.SignedSigningPackage +} +var file_pkg_frost_roast_gen_pb_signing_package_proto_depIdxs = []int32{ + 0, // [0:0] is the sub-list for method output_type + 0, // [0:0] is the sub-list for method input_type + 0, // [0:0] is the sub-list for extension type_name + 0, // [0:0] is the sub-list for extension extendee + 0, // [0:0] is the sub-list for field type_name +} + +func init() { file_pkg_frost_roast_gen_pb_signing_package_proto_init() } +func file_pkg_frost_roast_gen_pb_signing_package_proto_init() { + if File_pkg_frost_roast_gen_pb_signing_package_proto != nil { + return + } + type x struct{} + out := protoimpl.TypeBuilder{ + File: protoimpl.DescBuilder{ + GoPackagePath: reflect.TypeOf(x{}).PkgPath(), + RawDescriptor: file_pkg_frost_roast_gen_pb_signing_package_proto_rawDesc, + NumEnums: 0, + NumMessages: 2, + NumExtensions: 0, + NumServices: 0, + }, + GoTypes: file_pkg_frost_roast_gen_pb_signing_package_proto_goTypes, + DependencyIndexes: file_pkg_frost_roast_gen_pb_signing_package_proto_depIdxs, + MessageInfos: file_pkg_frost_roast_gen_pb_signing_package_proto_msgTypes, + }.Build() + File_pkg_frost_roast_gen_pb_signing_package_proto = out.File + file_pkg_frost_roast_gen_pb_signing_package_proto_rawDesc = nil + file_pkg_frost_roast_gen_pb_signing_package_proto_goTypes = nil + file_pkg_frost_roast_gen_pb_signing_package_proto_depIdxs = nil +} diff --git a/pkg/frost/roast/gen/pb/signing_package.proto b/pkg/frost/roast/gen/pb/signing_package.proto new file mode 100644 index 0000000000..a65dcc55e5 --- /dev/null +++ b/pkg/frost/roast/gen/pb/signing_package.proto @@ -0,0 +1,38 @@ +syntax = "proto3"; + +option go_package = "./pb"; +package roast; + +// Signed signing-package wire format (RFC-21 Phase 7.2b). +// +// The elected coordinator signs the exact serialized SigningPackageBody +// with its operator key and distributes the SignedSigningPackage to the +// chosen signing subset. A member verifies the coordinator signature over +// the bytes it received and only then parses them - the same signed-body, +// verify-what-you-received discipline as the evidence envelopes +// (evidence.proto): the body travels verbatim, so signature validity never +// depends on any serializer's canonical form across protobuf versions or +// languages. + +// The byte stream the elected coordinator's operator key signs. Carried as +// exact bytes in SignedSigningPackage.body. +message SigningPackageBody { + // 32-byte attempt context hash binding the package to one attempt. + bytes attempt_context_hash = 1; + // The elected coordinator's member index (RFC-21 Annex A). A member + // verifies this is the attempt's elected coordinator and that the + // signature verifies under that coordinator's operator key. + uint32 coordinator_id = 2; + // The serialized FROST SigningPackage the chosen subset signs over. + bytes signing_package = 3; + // The 32-byte taproot script-tree root the signature is tweaked by; + // empty for a key-path spend. + bytes taproot_merkle_root = 4; +} + +// The on-wire signing package: exact signed body bytes plus the elected +// coordinator's operator signature over them. +message SignedSigningPackage { + bytes body = 1; + bytes coordinator_signature = 2; +} diff --git a/pkg/frost/roast/signing_package.go b/pkg/frost/roast/signing_package.go new file mode 100644 index 0000000000..54ca17ec05 --- /dev/null +++ b/pkg/frost/roast/signing_package.go @@ -0,0 +1,225 @@ +package roast + +import ( + "errors" + "fmt" + + "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" +) + +// SignedSigningPackageType is the stable Type() string for the +// coordinator-distributed, operator-signed signing package (Phase 7.2b). +const SignedSigningPackageType = roastMessageTypePrefix + "signed_signing_package" + +// MaxSigningPackageBytes caps the embedded FROST SigningPackage length, +// rejecting pathological payloads at Unmarshal time so a misbehaving +// coordinator cannot exhaust receiver memory. Sized for a worst-case +// production signing subset's round-1 commitments plus generous headroom. +const MaxSigningPackageBytes = 1 << 20 // 1 MiB + +// TaprootMerkleRootLength is the byte length of a taproot script-tree +// root. A SigningPackage carries either exactly this many bytes or none +// (the key-path case). +const TaprootMerkleRootLength = 32 + +// SigningPackage is the coordinator-distributed signing package for one +// attempt, carried as a signed-body envelope (Phase 7.2b, frozen spec +// section 6). The elected coordinator signs the exact serialized +// SigningPackageBody with its operator key and distributes the +// SignedSigningPackage to the chosen signing subset; each member verifies +// the coordinator signature over the exact bytes it received and only then +// parses them - the same sign-what-you-transmit / verify-what-you-received +// discipline as the evidence envelopes (wire.go). +// +// This file defines the wire type and its byte-preservation contract only. +// Coordinator-side signing/distribution and member-side authentication +// (elected-coordinator check, signature verification, root binding) and +// retention land in later Phase 7.2b increments; the engine never sees this +// envelope (frozen spec: blame adjudication is Go-side). +type SigningPackage struct { + // AttemptContextHash binds the package to one attempt. Always exactly + // 32 bytes (attempt.MessageDigestLength). + AttemptContextHash []byte + // CoordinatorIDValue is the elected coordinator's member index + // (RFC-21 Annex A). A member authenticates the envelope by checking + // this equals the attempt's elected coordinator and that the + // signature verifies under that coordinator's operator key. + CoordinatorIDValue uint32 + // SigningPackageBytes is the serialized FROST SigningPackage the + // chosen subset signs over. + SigningPackageBytes []byte + // TaprootMerkleRoot is the taproot script-tree root the signature is + // tweaked by: exactly 32 bytes, or empty for a key-path spend. + TaprootMerkleRoot []byte + // CoordinatorSignature is the elected coordinator's operator-key + // signature over SignableBytes(). + CoordinatorSignature []byte + + // signedBody caches the exact serialized body bytes the + // CoordinatorSignature covers: marshaled once at signing time for a + // self-authored package, or the received bytes verbatim for a parsed + // one. Fields must not be mutated once set. + signedBody []byte + // wireEnvelope caches the exact on-wire envelope (body + signature): + // the received bytes verbatim for parsed packages, or built once + // after signing for self-authored ones. + wireEnvelope []byte +} + +func signingPackageBodyMessage(p *SigningPackage) *pb.SigningPackageBody { + return &pb.SigningPackageBody{ + AttemptContextHash: p.AttemptContextHash, + CoordinatorId: p.CoordinatorIDValue, + SigningPackage: p.SigningPackageBytes, + TaprootMerkleRoot: p.TaprootMerkleRoot, + } +} + +func signingPackageFieldsFromBody(p *SigningPackage, body *pb.SigningPackageBody) { + p.AttemptContextHash = append([]byte(nil), body.AttemptContextHash...) + p.CoordinatorIDValue = body.CoordinatorId + p.SigningPackageBytes = append([]byte(nil), body.SigningPackage...) + p.TaprootMerkleRoot = append([]byte(nil), body.TaprootMerkleRoot...) +} + +// SignableBytes returns the exact byte stream the CoordinatorSignature +// covers: the serialized SigningPackageBody. For a self-authored package +// the body is marshaled once and cached - sign exactly what will be +// transmitted. For a package parsed off the wire this returns the received +// body bytes verbatim - verify exactly what was received. The fields must +// not be mutated afterwards, and the returned slice is the internal cache - +// callers must not mutate it. +func (p *SigningPackage) SignableBytes() ([]byte, error) { + if p == nil { + return nil, errors.New("roast: cannot encode a nil signing package") + } + if p.signedBody != nil { + return p.signedBody, nil + } + body, err := proto.Marshal(signingPackageBodyMessage(p)) + if err != nil { + return nil, fmt.Errorf("roast: marshal signing package body: %w", err) + } + p.signedBody = body + return body, nil +} + +// Type implements net.TaggedUnmarshaler. +func (p *SigningPackage) Type() string { + return SignedSigningPackageType +} + +// Marshal serialises the package as a SignedSigningPackage envelope: the +// exact signed body bytes plus the coordinator signature. For a package +// parsed off the wire the received envelope is returned verbatim, so the +// bytes a member retains for the section-3 equivocation comparison are the +// exact bytes it received. The package must be signed first. The returned +// slice is the internal cache - callers must not mutate it. +func (p *SigningPackage) Marshal() ([]byte, error) { + if p.wireEnvelope != nil { + return p.wireEnvelope, nil + } + if len(p.CoordinatorSignature) == 0 { + return nil, errors.New( + "roast: signing package must be signed before wire encoding", + ) + } + body, err := p.SignableBytes() + if err != nil { + return nil, err + } + envelope, err := proto.Marshal(&pb.SignedSigningPackage{ + Body: body, + CoordinatorSignature: p.CoordinatorSignature, + }) + if err != nil { + return nil, fmt.Errorf("roast: marshal signing package envelope: %w", err) + } + p.wireEnvelope = envelope + return envelope, nil +} + +// Unmarshal parses a SignedSigningPackage envelope, retains the received +// body and envelope bytes verbatim (the coordinator signature is verified +// over exactly these bytes), populates the fields from the body, and +// validates the structure. +func (p *SigningPackage) Unmarshal(data []byte) error { + var envelope pb.SignedSigningPackage + if err := proto.Unmarshal(data, &envelope); err != nil { + return fmt.Errorf("signed signing package: parse envelope: %w", err) + } + if len(envelope.Body) == 0 { + return errors.New("signed signing package: empty body") + } + var body pb.SigningPackageBody + if err := proto.Unmarshal(envelope.Body, &body); err != nil { + return fmt.Errorf("signed signing package: parse body: %w", err) + } + signingPackageFieldsFromBody(p, &body) + p.CoordinatorSignature = append([]byte(nil), envelope.CoordinatorSignature...) + p.signedBody = append([]byte(nil), envelope.Body...) + p.wireEnvelope = append([]byte(nil), data...) + return p.Validate() +} + +// AttemptContextHashArray returns the attempt context hash as a fixed +// 32-byte array. Validate (or Unmarshal) must have confirmed the length +// first; it copies at most 32 bytes and zero-pads a short slice. +func (p *SigningPackage) AttemptContextHashArray() [attempt.MessageDigestLength]byte { + var out [attempt.MessageDigestLength]byte + copy(out[:], p.AttemptContextHash) + return out +} + +// CoordinatorID returns the elected coordinator's member index. +func (p *SigningPackage) CoordinatorID() group.MemberIndex { + return group.MemberIndex(p.CoordinatorIDValue) +} + +// Validate runs the structural checks Unmarshal applies after a decode. +// Exposed so callers that construct packages in memory (e.g. the +// coordinator) can validate without a marshal/unmarshal round-trip. It +// does not verify the coordinator signature - that is the member-side +// authentication step (a later Phase 7.2b increment), which checks the +// signature against the attempt's elected coordinator's operator key. +func (p *SigningPackage) Validate() error { + if len(p.AttemptContextHash) != attempt.MessageDigestLength { + return fmt.Errorf( + "signed signing package: attemptContextHash length [%d], expected [%d]", + len(p.AttemptContextHash), + attempt.MessageDigestLength, + ) + } + if p.CoordinatorIDValue == 0 { + return errors.New("signed signing package: coordinatorID is zero") + } + if len(p.SigningPackageBytes) == 0 { + return errors.New("signed signing package: empty signing package") + } + if len(p.SigningPackageBytes) > MaxSigningPackageBytes { + return fmt.Errorf( + "signed signing package: signingPackage length [%d] exceeds cap [%d]", + len(p.SigningPackageBytes), + MaxSigningPackageBytes, + ) + } + if n := len(p.TaprootMerkleRoot); n != 0 && n != TaprootMerkleRootLength { + return fmt.Errorf( + "signed signing package: taprootMerkleRoot length [%d], expected 0 (key-path) or %d", + n, + TaprootMerkleRootLength, + ) + } + if len(p.CoordinatorSignature) > MaxCoordinatorSignatureBytes { + return fmt.Errorf( + "signed signing package: coordinatorSignature length [%d] exceeds cap [%d]", + len(p.CoordinatorSignature), + MaxCoordinatorSignatureBytes, + ) + } + return nil +} diff --git a/pkg/frost/roast/signing_package_test.go b/pkg/frost/roast/signing_package_test.go new file mode 100644 index 0000000000..737a071257 --- /dev/null +++ b/pkg/frost/roast/signing_package_test.go @@ -0,0 +1,177 @@ +package roast + +import ( + "bytes" + "testing" + + "google.golang.org/protobuf/proto" + + "github.com/keep-network/keep-core/pkg/frost/roast/gen/pb" + "github.com/keep-network/keep-core/pkg/protocol/group" +) + +// These pin the signed-body envelope contract for the coordinator's signing +// package: the coordinator signs exactly the bytes that travel, those bytes +// survive re-broadcast verbatim, and parsing never depends on a serializer's +// canonical form. Coordinator-signature verification and member-side +// authentication arrive with a later Phase 7.2b increment. + +func signedTestSigningPackage( + t *testing.T, + coordinator group.MemberIndex, + root []byte, +) *SigningPackage { + t.Helper() + pkg := &SigningPackage{ + AttemptContextHash: append([]byte(nil), pinnedContextHash[:]...), + CoordinatorIDValue: uint32(coordinator), + SigningPackageBytes: []byte("frost-signing-package-bytes"), + TaprootMerkleRoot: root, + } + payload, err := pkg.SignableBytes() + if err != nil { + t.Fatalf("signable bytes: %v", err) + } + sig, err := (&fakeSigner{id: coordinator}).Sign(payload) + if err != nil { + t.Fatalf("sign: %v", err) + } + pkg.CoordinatorSignature = sig + return pkg +} + +func TestSigningPackageWire_ReceivedBytesPreservedVerbatim(t *testing.T) { + original := signedTestSigningPackage(t, 3, nil) + wire, err := original.Marshal() + if err != nil { + t.Fatalf("marshal: %v", err) + } + + decoded := &SigningPackage{} + if err := decoded.Unmarshal(wire); err != nil { + t.Fatalf("unmarshal: %v", err) + } + + rebroadcast, err := decoded.Marshal() + if err != nil { + t.Fatalf("re-marshal: %v", err) + } + if !bytes.Equal(rebroadcast, wire) { + t.Fatal("re-marshal of a received signing package must return the received bytes verbatim") + } + + producerBody, _ := original.SignableBytes() + receiverBody, _ := decoded.SignableBytes() + if !bytes.Equal(producerBody, receiverBody) { + t.Fatal("receiver must be able to verify over exactly the bytes the coordinator signed") + } + if decoded.CoordinatorIDValue != original.CoordinatorIDValue || + !bytes.Equal(decoded.AttemptContextHash, original.AttemptContextHash) || + !bytes.Equal(decoded.SigningPackageBytes, original.SigningPackageBytes) || + !bytes.Equal(decoded.CoordinatorSignature, original.CoordinatorSignature) { + t.Fatal("decoded fields must match the original") + } +} + +func TestSigningPackageWire_NonCanonicalEnvelopeEncodingSurvives(t *testing.T) { + original := signedTestSigningPackage(t, 3, nil) + body, _ := original.SignableBytes() + + // Handcraft an envelope with fields in REVERSE tag order + // (coordinator_signature before body) - wire-legal but non-canonical, no + // Go marshaler would emit it. Field 1 (body) tag 0x0a, field 2 + // (coordinator_signature) tag 0x12; both length-delimited. + var crafted []byte + crafted = append(crafted, 0x12, byte(len(original.CoordinatorSignature))) + crafted = append(crafted, original.CoordinatorSignature...) + crafted = append(crafted, 0x0a, byte(len(body))) + crafted = append(crafted, body...) + + var check pb.SignedSigningPackage + if err := proto.Unmarshal(crafted, &check); err != nil { + t.Fatalf("crafted envelope must be wire-legal: %v", err) + } + + decoded := &SigningPackage{} + if err := decoded.Unmarshal(crafted); err != nil { + t.Fatalf("unmarshal crafted: %v", err) + } + if gotBody, _ := decoded.SignableBytes(); !bytes.Equal(gotBody, body) { + t.Fatal("SignableBytes must return the embedded body bytes verbatim") + } + remarshaled, err := decoded.Marshal() + if err != nil { + t.Fatalf("re-marshal: %v", err) + } + if !bytes.Equal(remarshaled, crafted) { + t.Fatal("re-marshal must preserve even a non-canonical received encoding verbatim") + } +} + +func TestSigningPackageWire_RootRoundTrips(t *testing.T) { + for _, tc := range []struct { + name string + root []byte + }{ + {"key-path (empty root)", nil}, + {"script-path (32-byte root)", bytes.Repeat([]byte{0xab}, TaprootMerkleRootLength)}, + } { + t.Run(tc.name, func(t *testing.T) { + wire, err := signedTestSigningPackage(t, 5, tc.root).Marshal() + if err != nil { + t.Fatalf("marshal: %v", err) + } + decoded := &SigningPackage{} + if err := decoded.Unmarshal(wire); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if !bytes.Equal(decoded.TaprootMerkleRoot, tc.root) { + t.Fatalf("root mismatch: got %x want %x", decoded.TaprootMerkleRoot, tc.root) + } + }) + } +} + +func TestSigningPackage_ValidateRejectsMalformed(t *testing.T) { + valid := func() *SigningPackage { + return &SigningPackage{ + AttemptContextHash: append([]byte(nil), pinnedContextHash[:]...), + CoordinatorIDValue: 3, + SigningPackageBytes: []byte("pkg"), + } + } + if err := valid().Validate(); err != nil { + t.Fatalf("a well-formed package must validate: %v", err) + } + for _, tc := range []struct { + name string + mutate func(*SigningPackage) + }{ + {"short attempt hash", func(p *SigningPackage) { p.AttemptContextHash = []byte{1, 2, 3} }}, + {"zero coordinator", func(p *SigningPackage) { p.CoordinatorIDValue = 0 }}, + {"empty signing package", func(p *SigningPackage) { p.SigningPackageBytes = nil }}, + {"bad root length", func(p *SigningPackage) { p.TaprootMerkleRoot = []byte{0x01} }}, + {"oversize signing package", func(p *SigningPackage) { + p.SigningPackageBytes = make([]byte, MaxSigningPackageBytes+1) + }}, + } { + t.Run(tc.name, func(t *testing.T) { + p := valid() + tc.mutate(p) + if err := p.Validate(); err == nil { + t.Fatal("expected Validate to reject the malformed package") + } + }) + } +} + +func TestSigningPackage_MarshalRequiresSignature(t *testing.T) { + pkg := &SigningPackage{ + AttemptContextHash: append([]byte(nil), pinnedContextHash[:]...), + CoordinatorIDValue: 3, + SigningPackageBytes: []byte("pkg"), + } + if _, err := pkg.Marshal(); err == nil { + t.Fatal("Marshal must refuse an unsigned signing package") + } +} From 0ae2f79941f0c3580122a4d99887147fcd6fc400 Mon Sep 17 00:00:00 2001 From: maclane Date: Sat, 13 Jun 2026 21:22:17 -0400 Subject: [PATCH 2/8] fix(frost/roast): reject out-of-range coordinator_id in signing package Self-review of #4056: coordinator_id is a wire uint32 but a member index is uint8 (group.MemberIndex, max 255), so Validate's zero-only check let an out-of-range coordinator_id pass while CoordinatorID() silently truncated it. Validate now also rejects coordinator_id > group.MaxMemberIndex, so the later member-side elected-coordinator check compares a faithful value. Test added. Co-Authored-By: Claude Opus 4.8 --- pkg/frost/roast/signing_package.go | 11 +++++++++++ pkg/frost/roast/signing_package_test.go | 3 +++ 2 files changed, 14 insertions(+) diff --git a/pkg/frost/roast/signing_package.go b/pkg/frost/roast/signing_package.go index 54ca17ec05..45c13a035a 100644 --- a/pkg/frost/roast/signing_package.go +++ b/pkg/frost/roast/signing_package.go @@ -197,6 +197,17 @@ func (p *SigningPackage) Validate() error { if p.CoordinatorIDValue == 0 { return errors.New("signed signing package: coordinatorID is zero") } + // coordinator_id is a wire uint32 but a member index is a uint8 + // (group.MemberIndex); reject an out-of-range value here so CoordinatorID() + // never silently truncates and the member-side elected-coordinator check + // compares a faithful value. + if p.CoordinatorIDValue > group.MaxMemberIndex { + return fmt.Errorf( + "signed signing package: coordinatorID [%d] exceeds max member index [%d]", + p.CoordinatorIDValue, + group.MaxMemberIndex, + ) + } if len(p.SigningPackageBytes) == 0 { return errors.New("signed signing package: empty signing package") } diff --git a/pkg/frost/roast/signing_package_test.go b/pkg/frost/roast/signing_package_test.go index 737a071257..89c464820b 100644 --- a/pkg/frost/roast/signing_package_test.go +++ b/pkg/frost/roast/signing_package_test.go @@ -149,6 +149,9 @@ func TestSigningPackage_ValidateRejectsMalformed(t *testing.T) { }{ {"short attempt hash", func(p *SigningPackage) { p.AttemptContextHash = []byte{1, 2, 3} }}, {"zero coordinator", func(p *SigningPackage) { p.CoordinatorIDValue = 0 }}, + {"coordinator out of member-index range", func(p *SigningPackage) { + p.CoordinatorIDValue = group.MaxMemberIndex + 1 + }}, {"empty signing package", func(p *SigningPackage) { p.SigningPackageBytes = nil }}, {"bad root length", func(p *SigningPackage) { p.TaprootMerkleRoot = []byte{0x01} }}, {"oversize signing package", func(p *SigningPackage) { From 7279cce085a355e9434025485402097869169f9c Mon Sep 17 00:00:00 2001 From: maclane Date: Sun, 14 Jun 2026 08:33:51 -0400 Subject: [PATCH 3/8] fix(frost/roast): domain-separate signing-package signatures (P1) Review of #4056: SigningPackageBody is wire-compatible with TransitionMessageBody (matching tags/types for attempt_context_hash, coordinator_id, and a length-delimited field 3), and the elected coordinator's operator key signs both - so a coordinator signature over a signing package could be replayed as a valid transition-message signature (cross-protocol signature confusion). SignableBytes now prepends a fixed domain tag so the coordinator signs the domain-tagged body. The bare body still travels on the wire verbatim (new bodyBytes()); the verifier reconstructs domain||body. The signed byte stream is unambiguously a signing package and can never be a valid transition-message body (it does not start with the attempt_context_hash tag), regardless of protobuf field layout. Test added pinning the body-level collision and that the domain-tagged signed bytes do not present a valid transition body. Co-Authored-By: Claude Opus 4.8 --- pkg/frost/roast/signing_package.go | 83 ++++++++++++++++++------- pkg/frost/roast/signing_package_test.go | 48 +++++++++++++- 2 files changed, 107 insertions(+), 24 deletions(-) diff --git a/pkg/frost/roast/signing_package.go b/pkg/frost/roast/signing_package.go index 45c13a035a..7393b87998 100644 --- a/pkg/frost/roast/signing_package.go +++ b/pkg/frost/roast/signing_package.go @@ -15,6 +15,18 @@ import ( // coordinator-distributed, operator-signed signing package (Phase 7.2b). const SignedSigningPackageType = roastMessageTypePrefix + "signed_signing_package" +// signingPackageSignatureDomain is the fixed domain-separation tag prefixed +// to the bytes the coordinator signs (see SignableBytes). The elected +// coordinator's operator key also signs TransitionMessage bodies, and a +// SigningPackageBody is wire-compatible with a TransitionMessageBody (matching +// tags/types for attempt_context_hash and coordinator_id, and a +// length-delimited field 3). Prefixing a unique domain tag makes the signed +// byte stream unambiguously a signing package, so a coordinator signature over +// a signing package can never be replayed as a transition-message signature +// (or vice versa) regardless of protobuf field layout. The tag is NOT carried +// on the wire - it is a fixed constant both signer and verifier prepend. +var signingPackageSignatureDomain = []byte("roast/signed-signing-package/v1\x00") + // MaxSigningPackageBytes caps the embedded FROST SigningPackage length, // rejecting pathological payloads at Unmarshal time so a misbehaving // coordinator cannot exhaust receiver memory. Sized for a worst-case @@ -59,11 +71,14 @@ type SigningPackage struct { // signature over SignableBytes(). CoordinatorSignature []byte - // signedBody caches the exact serialized body bytes the - // CoordinatorSignature covers: marshaled once at signing time for a - // self-authored package, or the received bytes verbatim for a parsed - // one. Fields must not be mutated once set. - signedBody []byte + // bodyCache caches the exact serialized SigningPackageBody: marshaled + // once at signing time for a self-authored package, or the received body + // bytes verbatim for a parsed one. This is the body field carried on the + // wire; fields must not be mutated once set. + bodyCache []byte + // signaturePayloadCache caches the exact bytes the CoordinatorSignature + // covers - the domain tag followed by the body (see SignableBytes). + signaturePayloadCache []byte // wireEnvelope caches the exact on-wire envelope (body + signature): // the received bytes verbatim for parsed packages, or built once // after signing for self-authored ones. @@ -87,24 +102,49 @@ func signingPackageFieldsFromBody(p *SigningPackage, body *pb.SigningPackageBody } // SignableBytes returns the exact byte stream the CoordinatorSignature -// covers: the serialized SigningPackageBody. For a self-authored package -// the body is marshaled once and cached - sign exactly what will be -// transmitted. For a package parsed off the wire this returns the received -// body bytes verbatim - verify exactly what was received. The fields must -// not be mutated afterwards, and the returned slice is the internal cache - -// callers must not mutate it. +// covers: the signing-package domain tag (see signingPackageSignatureDomain) +// followed by the serialized SigningPackageBody. 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 coordinator's +// transition-message signatures (whose body is otherwise wire-compatible). +// The body half is the bytes the package transmits: marshaled once for a +// self-authored package, or the received body verbatim for a parsed one +// (verify exactly what was received). Fields must not be mutated afterwards, +// and the returned slice is the internal cache - callers must not mutate it. func (p *SigningPackage) SignableBytes() ([]byte, error) { if p == nil { return nil, errors.New("roast: cannot encode a nil signing package") } - if p.signedBody != nil { - return p.signedBody, nil + if p.signaturePayloadCache != nil { + return p.signaturePayloadCache, nil + } + body, err := p.bodyBytes() + if err != nil { + return nil, err + } + payload := make([]byte, 0, len(signingPackageSignatureDomain)+len(body)) + payload = append(payload, signingPackageSignatureDomain...) + payload = append(payload, body...) + p.signaturePayloadCache = payload + return payload, nil +} + +// bodyBytes returns the exact serialized SigningPackageBody - the body field +// carried in the SignedSigningPackage envelope. Marshaled once and cached for +// a self-authored package; the received bytes verbatim for a parsed one. The +// returned slice is the internal cache - callers must not mutate it. +func (p *SigningPackage) bodyBytes() ([]byte, error) { + if p == nil { + return nil, errors.New("roast: cannot encode a nil signing package") + } + if p.bodyCache != nil { + return p.bodyCache, nil } body, err := proto.Marshal(signingPackageBodyMessage(p)) if err != nil { return nil, fmt.Errorf("roast: marshal signing package body: %w", err) } - p.signedBody = body + p.bodyCache = body return body, nil } @@ -114,11 +154,12 @@ func (p *SigningPackage) Type() string { } // Marshal serialises the package as a SignedSigningPackage envelope: the -// exact signed body bytes plus the coordinator signature. For a package -// parsed off the wire the received envelope is returned verbatim, so the -// bytes a member retains for the section-3 equivocation comparison are the -// exact bytes it received. The package must be signed first. The returned -// slice is the internal cache - callers must not mutate it. +// serialized SigningPackageBody plus the coordinator signature (which covers +// the domain-tagged body, see SignableBytes). For a package parsed off the +// wire the received envelope is returned verbatim, so the bytes a member +// retains for the section-3 equivocation comparison are the exact bytes it +// received. The package must be signed first. The returned slice is the +// internal cache - callers must not mutate it. func (p *SigningPackage) Marshal() ([]byte, error) { if p.wireEnvelope != nil { return p.wireEnvelope, nil @@ -128,7 +169,7 @@ func (p *SigningPackage) Marshal() ([]byte, error) { "roast: signing package must be signed before wire encoding", ) } - body, err := p.SignableBytes() + body, err := p.bodyBytes() if err != nil { return nil, err } @@ -161,7 +202,7 @@ func (p *SigningPackage) Unmarshal(data []byte) error { } signingPackageFieldsFromBody(p, &body) p.CoordinatorSignature = append([]byte(nil), envelope.CoordinatorSignature...) - p.signedBody = append([]byte(nil), envelope.Body...) + p.bodyCache = append([]byte(nil), envelope.Body...) p.wireEnvelope = append([]byte(nil), data...) return p.Validate() } diff --git a/pkg/frost/roast/signing_package_test.go b/pkg/frost/roast/signing_package_test.go index 89c464820b..19965c7f37 100644 --- a/pkg/frost/roast/signing_package_test.go +++ b/pkg/frost/roast/signing_package_test.go @@ -6,6 +6,7 @@ import ( "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" ) @@ -75,7 +76,10 @@ func TestSigningPackageWire_ReceivedBytesPreservedVerbatim(t *testing.T) { func TestSigningPackageWire_NonCanonicalEnvelopeEncodingSurvives(t *testing.T) { original := signedTestSigningPackage(t, 3, nil) - body, _ := original.SignableBytes() + body, err := original.bodyBytes() + if err != nil { + t.Fatalf("body bytes: %v", err) + } // Handcraft an envelope with fields in REVERSE tag order // (coordinator_signature before body) - wire-legal but non-canonical, no @@ -96,8 +100,8 @@ func TestSigningPackageWire_NonCanonicalEnvelopeEncodingSurvives(t *testing.T) { if err := decoded.Unmarshal(crafted); err != nil { t.Fatalf("unmarshal crafted: %v", err) } - if gotBody, _ := decoded.SignableBytes(); !bytes.Equal(gotBody, body) { - t.Fatal("SignableBytes must return the embedded body bytes verbatim") + if gotBody, _ := decoded.bodyBytes(); !bytes.Equal(gotBody, body) { + t.Fatal("the received body must be preserved verbatim") } remarshaled, err := decoded.Marshal() if err != nil { @@ -108,6 +112,44 @@ func TestSigningPackageWire_NonCanonicalEnvelopeEncodingSurvives(t *testing.T) { } } +func TestSigningPackageWire_SignedBytesDomainSeparatedFromTransitionMessage(t *testing.T) { + pkg := signedTestSigningPackage(t, 3, nil) + signable, err := pkg.SignableBytes() + if err != nil { + t.Fatalf("signable: %v", err) + } + body, _ := pkg.bodyBytes() + + // The signed bytes are the domain tag followed by the body. + if !bytes.HasPrefix(signable, signingPackageSignatureDomain) { + t.Fatal("signed bytes must carry the signing-package domain tag") + } + if !bytes.Equal(signable[len(signingPackageSignatureDomain):], body) { + t.Fatal("signed bytes must be the domain tag followed by the body") + } + + // The bare body IS wire-compatible with a TransitionMessageBody - the + // collision the domain tag defends against: it presents the same 32-byte + // attempt_context_hash and coordinator_id a transition body would. + var asTransition pb.TransitionMessageBody + if err := proto.Unmarshal(body, &asTransition); err != nil { + t.Fatalf("the bare body is expected to parse as a TransitionMessageBody: %v", err) + } + if len(asTransition.AttemptContextHash) != attempt.MessageDigestLength || + asTransition.CoordinatorId != pkg.CoordinatorIDValue { + t.Fatal("sanity: the bare body must collide with TransitionMessageBody") + } + + // But the domain-tagged SIGNED bytes do NOT present a valid transition body + // (no 32-byte attempt_context_hash), so a coordinator signature over a + // signing package cannot be replayed as a transition-message signature. + var signableAsTransition pb.TransitionMessageBody + _ = proto.Unmarshal(signable, &signableAsTransition) + if len(signableAsTransition.AttemptContextHash) == attempt.MessageDigestLength { + t.Fatal("domain-tagged signed bytes must not present a valid transition attempt_context_hash") + } +} + func TestSigningPackageWire_RootRoundTrips(t *testing.T) { for _, tc := range []struct { name string From 7ec105701b99caaea3dd9863b3d9c6123b4fd819 Mon Sep 17 00:00:00 2001 From: maclane Date: Sun, 14 Jun 2026 08:47:57 -0400 Subject: [PATCH 4/8] fix(frost/roast): bound signing-package envelope before allocating (P2) Review of #4056: Unmarshal materialized the whole signing_package field and copied it into the struct + cached the body before Validate applied the MaxSigningPackageBytes cap, so an oversized peer envelope forced large allocations before rejection. Unmarshal now rejects len(data) > MaxSignedSigningPackageBytes before proto.Unmarshal, and rejects an over-cap signing_package field before the field/body copies. Test added. Co-Authored-By: Claude Opus 4.8 --- pkg/frost/roast/signing_package.go | 28 ++++++++++++++++++++++++ pkg/frost/roast/signing_package_test.go | 29 +++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/pkg/frost/roast/signing_package.go b/pkg/frost/roast/signing_package.go index 7393b87998..37d92021b9 100644 --- a/pkg/frost/roast/signing_package.go +++ b/pkg/frost/roast/signing_package.go @@ -38,6 +38,13 @@ const MaxSigningPackageBytes = 1 << 20 // 1 MiB // (the key-path case). const TaprootMerkleRootLength = 32 +// MaxSignedSigningPackageBytes bounds a whole SignedSigningPackage envelope so +// Unmarshal can reject a grossly oversized message before proto.Unmarshal +// materializes it (and before the body/field copies). Sized as the +// signing-package cap plus the coordinator-signature cap plus generous +// framing/field overhead, so a legitimate maximum-size package still fits. +const MaxSignedSigningPackageBytes = MaxSigningPackageBytes + MaxCoordinatorSignatureBytes + 512 + // SigningPackage is the coordinator-distributed signing package for one // attempt, carried as a signed-body envelope (Phase 7.2b, frozen spec // section 6). The elected coordinator signs the exact serialized @@ -189,6 +196,17 @@ func (p *SigningPackage) Marshal() ([]byte, error) { // over exactly these bytes), populates the fields from the body, and // validates the structure. func (p *SigningPackage) Unmarshal(data []byte) error { + // Bound the input before allocating: reject a grossly oversized envelope + // before proto.Unmarshal materializes it (and before the copies below), so + // the MaxSigningPackageBytes cap protects memory rather than only rejecting + // after the fact. + if len(data) > MaxSignedSigningPackageBytes { + return fmt.Errorf( + "signed signing package: envelope length [%d] exceeds cap [%d]", + len(data), + MaxSignedSigningPackageBytes, + ) + } var envelope pb.SignedSigningPackage if err := proto.Unmarshal(data, &envelope); err != nil { return fmt.Errorf("signed signing package: parse envelope: %w", err) @@ -200,6 +218,16 @@ func (p *SigningPackage) Unmarshal(data []byte) error { if err := proto.Unmarshal(envelope.Body, &body); err != nil { return fmt.Errorf("signed signing package: parse body: %w", err) } + // Enforce the signing-package cap on the parsed field before copying it + // into the struct (and before caching the body), so an over-cap field is + // rejected without the extra allocations. + if len(body.SigningPackage) > MaxSigningPackageBytes { + return fmt.Errorf( + "signed signing package: signingPackage length [%d] exceeds cap [%d]", + len(body.SigningPackage), + MaxSigningPackageBytes, + ) + } signingPackageFieldsFromBody(p, &body) p.CoordinatorSignature = append([]byte(nil), envelope.CoordinatorSignature...) p.bodyCache = append([]byte(nil), envelope.Body...) diff --git a/pkg/frost/roast/signing_package_test.go b/pkg/frost/roast/signing_package_test.go index 19965c7f37..fbd91a5626 100644 --- a/pkg/frost/roast/signing_package_test.go +++ b/pkg/frost/roast/signing_package_test.go @@ -210,6 +210,35 @@ func TestSigningPackage_ValidateRejectsMalformed(t *testing.T) { } } +func TestSigningPackageWire_UnmarshalRejectsOversizeBeforeCopy(t *testing.T) { + // A peer-supplied envelope whose signing_package exceeds the cap is + // rejected on receive, so the cap protects memory rather than only + // failing after the field is materialized and copied. + oversized := &SigningPackage{ + AttemptContextHash: append([]byte(nil), pinnedContextHash[:]...), + CoordinatorIDValue: 3, + SigningPackageBytes: make([]byte, MaxSigningPackageBytes+1), + } + payload, err := oversized.SignableBytes() + if err != nil { + t.Fatalf("signable: %v", err) + } + sig, err := (&fakeSigner{id: 3}).Sign(payload) + if err != nil { + t.Fatalf("sign: %v", err) + } + oversized.CoordinatorSignature = sig + wire, err := oversized.Marshal() + if err != nil { + t.Fatalf("marshal: %v", err) + } + + var decoded SigningPackage + if err := decoded.Unmarshal(wire); err == nil { + t.Fatal("Unmarshal must reject an over-cap signing package") + } +} + func TestSigningPackage_MarshalRequiresSignature(t *testing.T) { pkg := &SigningPackage{ AttemptContextHash: append([]byte(nil), pinnedContextHash[:]...), From d01493fea6f0b225f745b12f94a273ed00d78843 Mon Sep 17 00:00:00 2001 From: maclane Date: Sun, 14 Jun 2026 09:09:00 -0400 Subject: [PATCH 5/8] feat(frost/roast): Phase 7.2b-2 coordinator-sign + member-authenticate signing package The producer/consumer logic for the signed signing-package envelope: SignSigningPackage (the elected coordinator signs SignableBytes() - the domain-tagged body - with its operator key); AuthenticateSigningPackage (a member verifies the envelope is genuine evidence from the attempt's elected coordinator: it names the elected coordinator per RFC-21 Annex A, resolved by the caller via SelectCoordinator; the signature verifies under that coordinator's operator key; and attempt_context_hash matches the live attempt). Passing = attributable, so the member retains the exact received bytes before the sign/no-sign decision; failing = forgeable noise, rejected WITHOUT retention. Mirrors verifyBundleSignature. MatchesRoot is the root-binding sign/no-sign check, kept separate from authentication so a root-divergent but genuine-coordinator envelope is retained as equivocation evidence and then refused (retain-on-reject). Tests: round-trip authenticate; rejections (missing/tampered signature, non-elected coordinator, wrong attempt, a non-elected operator signing a body carrying the elected id); key/script-path root match. Still on this branch: member retention storage + retain-on-reject wiring, the context-bound Round2 share submission, and network distribution. Co-Authored-By: Claude Opus 4.8 --- pkg/frost/roast/signing_package_auth.go | 112 ++++++++++++++++ pkg/frost/roast/signing_package_auth_test.go | 133 +++++++++++++++++++ 2 files changed, 245 insertions(+) create mode 100644 pkg/frost/roast/signing_package_auth.go create mode 100644 pkg/frost/roast/signing_package_auth_test.go diff --git a/pkg/frost/roast/signing_package_auth.go b/pkg/frost/roast/signing_package_auth.go new file mode 100644 index 0000000000..dfe345a27b --- /dev/null +++ b/pkg/frost/roast/signing_package_auth.go @@ -0,0 +1,112 @@ +package roast + +import ( + "bytes" + "errors" + "fmt" + + "github.com/keep-network/keep-core/pkg/protocol/group" +) + +// ErrSigningPackageWrongCoordinator is returned by AuthenticateSigningPackage +// when a signed signing package names a coordinator other than the attempt's +// elected coordinator (RFC-21 Annex A). attempt_context_hash is public, so any +// operator could sign a body carrying it; an envelope from a non-elected +// coordinator is not attributable to the coordinator and MUST NOT be retained. +var ErrSigningPackageWrongCoordinator = errors.New( + "roast: signing package coordinator is not the attempt's elected coordinator", +) + +// ErrSigningPackageWrongAttempt is returned when a signed signing package's +// attempt_context_hash does not match the live attempt. +var ErrSigningPackageWrongAttempt = errors.New( + "roast: signing package attempt context hash does not match the live attempt", +) + +// SignSigningPackage signs pkg with the elected coordinator's operator key, +// setting pkg.CoordinatorSignature over pkg.SignableBytes() (the domain-tagged +// body). The elected coordinator calls this before distributing the +// SignedSigningPackage to the chosen signing subset. pkg must be structurally +// valid (call Validate first). +func SignSigningPackage(signer Signer, pkg *SigningPackage) error { + payload, err := pkg.SignableBytes() + if err != nil { + return err + } + signature, err := signer.Sign(payload) + if err != nil { + return fmt.Errorf("roast: sign signing package: %w", err) + } + pkg.CoordinatorSignature = signature + return nil +} + +// AuthenticateSigningPackage verifies that pkg is genuine evidence from the +// attempt's elected coordinator: it names electedCoordinator, its signature +// verifies under that coordinator's operator key over the domain-tagged body, +// and its attempt_context_hash matches the live attempt. (electedCoordinator +// is resolved by the caller from the attempt via SelectCoordinator, exactly as +// Coordinator.VerifyBundle resolves the bundle coordinator.) +// +// A package that passes is attributable to the coordinator, so the member MUST +// retain its exact received bytes - the section-3 cross-member equivocation +// comparison needs them - BEFORE deciding whether to sign over it. A package +// that fails any check is forgeable noise: the caller rejects it WITHOUT +// retaining it. +// +// This deliberately does NOT check the taproot root. Root binding is the +// separate sign/no-sign decision (see MatchesRoot): a root-divergent but +// genuine-coordinator envelope is still retained as equivocation evidence and +// then refused, so root verification must not gate retention here. +func AuthenticateSigningPackage( + verifier SignatureVerifier, + pkg *SigningPackage, + electedCoordinator group.MemberIndex, + liveAttemptContextHash []byte, +) error { + if len(pkg.CoordinatorSignature) == 0 { + return fmt.Errorf( + "%w: signing package has no coordinator signature", + ErrSignatureMissing, + ) + } + if pkg.CoordinatorID() != electedCoordinator { + return fmt.Errorf( + "%w: package coordinator %d, elected %d", + ErrSigningPackageWrongCoordinator, + pkg.CoordinatorID(), + electedCoordinator, + ) + } + if !bytes.Equal(pkg.AttemptContextHash, liveAttemptContextHash) { + return ErrSigningPackageWrongAttempt + } + payload, err := pkg.SignableBytes() + if err != nil { + return fmt.Errorf("signing package signable bytes: %w", err) + } + if err := verifier.Verify( + payload, + pkg.CoordinatorSignature, + pkg.CoordinatorID(), + ); err != nil { + return fmt.Errorf( + "%w: coordinator %d: %s", + ErrSignatureInvalid, + pkg.CoordinatorID(), + err.Error(), + ) + } + return nil +} + +// MatchesRoot reports whether the package's taproot_merkle_root equals the +// live session/signing root (both empty for a key-path spend). After +// authenticating and retaining a package, a member signs over its +// signing_package ONLY when this is true: a divergent root means the +// coordinator is committing the subset to a tweaked key other than the +// session's, so the member refuses to sign and the retained envelope stands as +// root-equivocation evidence for the section-3 comparison. +func (p *SigningPackage) MatchesRoot(liveRoot []byte) bool { + return bytes.Equal(p.TaprootMerkleRoot, liveRoot) +} diff --git a/pkg/frost/roast/signing_package_auth_test.go b/pkg/frost/roast/signing_package_auth_test.go new file mode 100644 index 0000000000..3237e86f14 --- /dev/null +++ b/pkg/frost/roast/signing_package_auth_test.go @@ -0,0 +1,133 @@ +package roast + +import ( + "bytes" + "errors" + "testing" + + "github.com/keep-network/keep-core/pkg/frost/roast/attempt" + "github.com/keep-network/keep-core/pkg/protocol/group" +) + +const testElectedCoordinator = group.MemberIndex(3) + +func TestSignSigningPackage_RoundTripAuthenticates(t *testing.T) { + pkg := &SigningPackage{ + AttemptContextHash: append([]byte(nil), pinnedContextHash[:]...), + CoordinatorIDValue: uint32(testElectedCoordinator), + SigningPackageBytes: []byte("frost-signing-package-bytes"), + } + if err := SignSigningPackage(&fakeSigner{id: testElectedCoordinator}, pkg); err != nil { + t.Fatalf("sign: %v", err) + } + if len(pkg.CoordinatorSignature) == 0 { + t.Fatal("SignSigningPackage must set a coordinator signature") + } + + // A member receives the envelope off the wire and authenticates it as + // genuine evidence from the attempt's elected coordinator. + wire, err := pkg.Marshal() + if err != nil { + t.Fatalf("marshal: %v", err) + } + var received SigningPackage + if err := received.Unmarshal(wire); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if err := AuthenticateSigningPackage( + fakeVerifier{}, + &received, + testElectedCoordinator, + pinnedContextHash[:], + ); err != nil { + t.Fatalf("authenticate a genuine package: %v", err) + } +} + +func TestAuthenticateSigningPackage_Rejections(t *testing.T) { + signed := func() *SigningPackage { + pkg := &SigningPackage{ + AttemptContextHash: append([]byte(nil), pinnedContextHash[:]...), + CoordinatorIDValue: uint32(testElectedCoordinator), + SigningPackageBytes: []byte("pkg"), + } + if err := SignSigningPackage(&fakeSigner{id: testElectedCoordinator}, pkg); err != nil { + t.Fatalf("sign: %v", err) + } + return pkg + } + otherAttempt := bytes.Repeat([]byte{0x09}, attempt.MessageDigestLength) + + t.Run("missing signature is rejected", func(t *testing.T) { + pkg := signed() + pkg.CoordinatorSignature = nil + err := AuthenticateSigningPackage(fakeVerifier{}, pkg, testElectedCoordinator, pinnedContextHash[:]) + if !errors.Is(err, ErrSignatureMissing) { + t.Fatalf("want ErrSignatureMissing, got %v", err) + } + }) + + t.Run("non-elected coordinator is rejected without retention", func(t *testing.T) { + err := AuthenticateSigningPackage(fakeVerifier{}, signed(), testElectedCoordinator+1, pinnedContextHash[:]) + if !errors.Is(err, ErrSigningPackageWrongCoordinator) { + t.Fatalf("want ErrSigningPackageWrongCoordinator, got %v", err) + } + }) + + t.Run("wrong attempt context is rejected", func(t *testing.T) { + err := AuthenticateSigningPackage(fakeVerifier{}, signed(), testElectedCoordinator, otherAttempt) + if !errors.Is(err, ErrSigningPackageWrongAttempt) { + t.Fatalf("want ErrSigningPackageWrongAttempt, got %v", err) + } + }) + + t.Run("tampered signature fails verification", func(t *testing.T) { + pkg := signed() + pkg.CoordinatorSignature[0] ^= 0xff + err := AuthenticateSigningPackage(fakeVerifier{}, pkg, testElectedCoordinator, pinnedContextHash[:]) + if !errors.Is(err, ErrSignatureInvalid) { + t.Fatalf("want ErrSignatureInvalid, got %v", err) + } + }) + + t.Run("package signed by a non-elected operator is rejected", func(t *testing.T) { + // A non-elected operator signs a body carrying the elected + // coordinator's id (attempt_context_hash is public, so it can). The + // signature does not verify under the elected coordinator's key. + pkg := &SigningPackage{ + AttemptContextHash: append([]byte(nil), pinnedContextHash[:]...), + CoordinatorIDValue: uint32(testElectedCoordinator), + SigningPackageBytes: []byte("pkg"), + } + if err := SignSigningPackage(&fakeSigner{id: testElectedCoordinator + 7}, pkg); err != nil { + t.Fatalf("sign: %v", err) + } + err := AuthenticateSigningPackage(fakeVerifier{}, pkg, testElectedCoordinator, pinnedContextHash[:]) + if !errors.Is(err, ErrSignatureInvalid) { + t.Fatalf("want ErrSignatureInvalid, got %v", err) + } + }) +} + +func TestSigningPackage_MatchesRoot(t *testing.T) { + root := bytes.Repeat([]byte{0xab}, TaprootMerkleRootLength) + other := bytes.Repeat([]byte{0xcd}, TaprootMerkleRootLength) + keyPath := &SigningPackage{} + scriptPath := &SigningPackage{TaprootMerkleRoot: root} + + if !keyPath.MatchesRoot(nil) { + t.Fatal("a key-path package must match an empty live root") + } + if keyPath.MatchesRoot(root) { + t.Fatal("a key-path package must not match a script-path live root") + } + if !scriptPath.MatchesRoot(root) { + t.Fatal("a script-path package must match its own root") + } + if scriptPath.MatchesRoot(nil) { + t.Fatal("a script-path package must not match an empty (key-path) live root") + } + if scriptPath.MatchesRoot(other) { + t.Fatal("a script-path package must not match a divergent root") + } +} From 51eabcd2e5ea17d352e3fbb12a2d3ae502ef979a Mon Sep 17 00:00:00 2001 From: maclane Date: Sun, 14 Jun 2026 09:55:06 -0400 Subject: [PATCH 6/8] docs(frost/roast): align proto signing contract with domain separation (P2) The signed signing-package wire spec documented bare-body signatures, but SignableBytes prefixes a fixed domain tag ("roast/signed-signing-package/v1" + 0x00) before the body. A Rust or other verifier built from this proto would sign/verify the bare body and fail to interoperate with AuthenticateSigningPackage. Document the signed payload as `domain_tag || SigningPackageBody` in a file-level cross-language contract note (with the exact tag bytes) and correct the two message comments. Regenerated signing_package.pb.go is comment-only; the descriptor path and symbols are unchanged. Co-Authored-By: Claude Opus 4.8 --- pkg/frost/roast/gen/pb/signing_package.pb.go | 10 +++-- pkg/frost/roast/gen/pb/signing_package.proto | 39 ++++++++++++++------ 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/pkg/frost/roast/gen/pb/signing_package.pb.go b/pkg/frost/roast/gen/pb/signing_package.pb.go index 98290c7e99..5d4af72486 100644 --- a/pkg/frost/roast/gen/pb/signing_package.pb.go +++ b/pkg/frost/roast/gen/pb/signing_package.pb.go @@ -20,8 +20,9 @@ const ( _ = protoimpl.EnforceVersion(protoimpl.MaxVersion - 20) ) -// The byte stream the elected coordinator's operator key signs. Carried as -// exact bytes in SignedSigningPackage.body. +// The signing-package body. Carried verbatim as SignedSigningPackage.body; +// the operator signature covers the domain-tagged form of these bytes +// (domain_tag || body), not the bare bytes. type SigningPackageBody struct { state protoimpl.MessageState `protogen:"open.v1"` // 32-byte attempt context hash binding the package to one attempt. @@ -97,8 +98,9 @@ func (x *SigningPackageBody) GetTaprootMerkleRoot() []byte { return nil } -// The on-wire signing package: exact signed body bytes plus the elected -// coordinator's operator signature over them. +// The on-wire signing package: the exact serialized SigningPackageBody bytes +// plus the elected coordinator's operator signature. The signature covers the +// domain-tagged body (domain_tag || body), not the bare body field. type SignedSigningPackage 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/signing_package.proto b/pkg/frost/roast/gen/pb/signing_package.proto index a65dcc55e5..acb8777cd8 100644 --- a/pkg/frost/roast/gen/pb/signing_package.proto +++ b/pkg/frost/roast/gen/pb/signing_package.proto @@ -5,17 +5,31 @@ package roast; // Signed signing-package wire format (RFC-21 Phase 7.2b). // -// The elected coordinator signs the exact serialized SigningPackageBody -// with its operator key and distributes the SignedSigningPackage to the -// chosen signing subset. A member verifies the coordinator signature over -// the bytes it received and only then parses them - the same signed-body, -// verify-what-you-received discipline as the evidence envelopes -// (evidence.proto): the body travels verbatim, so signature validity never -// depends on any serializer's canonical form across protobuf versions or -// languages. +// The elected coordinator signs the signing package with its operator key and +// distributes the SignedSigningPackage to the chosen signing subset. A member +// verifies the coordinator signature over the bytes it received and only then +// parses them - the same signed-body, verify-what-you-received discipline as +// the evidence envelopes (evidence.proto): the body travels verbatim, so +// signature validity never depends on any serializer's canonical form across +// protobuf versions or languages. +// +// SIGNED PAYLOAD (cross-language contract). The operator signature in +// SignedSigningPackage.coordinator_signature is computed over a +// domain-separated byte stream, NOT over the bare body: +// +// domain_tag || serialized SigningPackageBody +// +// where domain_tag is the fixed ASCII bytes "roast/signed-signing-package/v1" +// followed by a single 0x00 byte. The tag is a constant prepended by signer +// and verifier alike and is NOT carried on the wire - only the body is, in +// SignedSigningPackage.body. The tag domain-separates this signature from the +// coordinator's transition-message signatures, whose body is otherwise +// wire-compatible with SigningPackageBody. Any implementation that signs or +// verifies the bare body (without the tag) will fail to interoperate. -// The byte stream the elected coordinator's operator key signs. Carried as -// exact bytes in SignedSigningPackage.body. +// The signing-package body. Carried verbatim as SignedSigningPackage.body; +// the operator signature covers the domain-tagged form of these bytes +// (domain_tag || body), not the bare bytes. message SigningPackageBody { // 32-byte attempt context hash binding the package to one attempt. bytes attempt_context_hash = 1; @@ -30,8 +44,9 @@ message SigningPackageBody { bytes taproot_merkle_root = 4; } -// The on-wire signing package: exact signed body bytes plus the elected -// coordinator's operator signature over them. +// The on-wire signing package: the exact serialized SigningPackageBody bytes +// plus the elected coordinator's operator signature. The signature covers the +// domain-tagged body (domain_tag || body), not the bare body field. message SignedSigningPackage { bytes body = 1; bytes coordinator_signature = 2; From 6b89dd9a42053e25c998d0d9485ead02b7892c54 Mon Sep 17 00:00:00 2001 From: maclane Date: Sun, 14 Jun 2026 10:16:06 -0400 Subject: [PATCH 7/8] fix(frost/roast): harden signing-package domain separation + reset cache (P1/P2) P1: the domain tag began with ASCII "ro" (0x72 0x6f), which is valid protobuf wire data (field 14, length 111), so a parser skips it and resumes - letting a crafted signing_package embed a TransitionMessageBody at the resume point and re-enabling cross-protocol signature replay. Begin the tag with byte 0x00 (an illegal protobuf tag, field 0) so the signed payload is undecodable as any protobuf message: a signing-package signature is rejected when another envelope decodes the forged body, and another coordinator-signed message's signature (over a body starting >= 0x08) can never verify against domain||body. The test now asserts undecodability, including a body that embeds a full valid transition body. P2: Unmarshal reset bodyCache and wireEnvelope but not signaturePayloadCache, so a reused receiver could authenticate a freshly decoded package against stale signable bytes. Clear the cache on Unmarshal; test added. Proto SIGNED PAYLOAD note updated to the new tag bytes (file-level comment; generated .pb.go is unchanged). Co-Authored-By: Claude Opus 4.8 --- pkg/frost/roast/gen/pb/signing_package.proto | 17 ++-- pkg/frost/roast/signing_package.go | 39 +++++++-- pkg/frost/roast/signing_package_test.go | 86 ++++++++++++++++++-- 3 files changed, 119 insertions(+), 23 deletions(-) diff --git a/pkg/frost/roast/gen/pb/signing_package.proto b/pkg/frost/roast/gen/pb/signing_package.proto index acb8777cd8..0d04eca9f2 100644 --- a/pkg/frost/roast/gen/pb/signing_package.proto +++ b/pkg/frost/roast/gen/pb/signing_package.proto @@ -19,13 +19,16 @@ package roast; // // domain_tag || serialized SigningPackageBody // -// where domain_tag is the fixed ASCII bytes "roast/signed-signing-package/v1" -// followed by a single 0x00 byte. The tag is a constant prepended by signer -// and verifier alike and is NOT carried on the wire - only the body is, in -// SignedSigningPackage.body. The tag domain-separates this signature from the -// coordinator's transition-message signatures, whose body is otherwise -// wire-compatible with SigningPackageBody. Any implementation that signs or -// verifies the bare body (without the tag) will fail to interoperate. +// where domain_tag is, in order: a single 0x00 byte, the fixed ASCII bytes +// "roast/signed-signing-package/v1", and a trailing 0x00 byte. The LEADING +// 0x00 is an illegal protobuf tag (field number 0): it makes the signed +// payload undecodable as any protobuf message, so a signing-package signature +// can never be accepted on a transition-message (or other coordinator-signed) +// envelope whose body is otherwise wire-compatible with SigningPackageBody. +// The tag is a constant prepended by signer and verifier alike and is NOT +// carried on the wire - only the body is, in SignedSigningPackage.body. Any +// implementation that signs or verifies the bare body (without the exact tag) +// will fail to interoperate. // The signing-package body. Carried verbatim as SignedSigningPackage.body; // the operator signature covers the domain-tagged form of these bytes diff --git a/pkg/frost/roast/signing_package.go b/pkg/frost/roast/signing_package.go index 37d92021b9..83546c2e11 100644 --- a/pkg/frost/roast/signing_package.go +++ b/pkg/frost/roast/signing_package.go @@ -17,15 +17,32 @@ const SignedSigningPackageType = roastMessageTypePrefix + "signed_signing_packag // signingPackageSignatureDomain is the fixed domain-separation tag prefixed // to the bytes the coordinator signs (see SignableBytes). The elected -// coordinator's operator key also signs TransitionMessage bodies, and a -// SigningPackageBody is wire-compatible with a TransitionMessageBody (matching -// tags/types for attempt_context_hash and coordinator_id, and a -// length-delimited field 3). Prefixing a unique domain tag makes the signed -// byte stream unambiguously a signing package, so a coordinator signature over -// a signing package can never be replayed as a transition-message signature -// (or vice versa) regardless of protobuf field layout. The tag is NOT carried -// on the wire - it is a fixed constant both signer and verifier prepend. -var signingPackageSignatureDomain = []byte("roast/signed-signing-package/v1\x00") +// coordinator's operator key also signs TransitionMessage and evidence-snapshot +// bodies, and a SigningPackageBody is wire-compatible with a +// TransitionMessageBody (matching tags/types for attempt_context_hash and +// coordinator_id, and a length-delimited field 3), so the signed byte streams +// must not be confusable. +// +// The tag BEGINS with byte 0x00 - an illegal protobuf tag (field number 0) - +// which separates the domains in BOTH directions without relying on field +// layout: +// +// - A signing-package signature cannot be accepted on another envelope: +// presenting these signed bytes as that envelope's body fails when the +// receiver decodes the body, because every signed-body decoder +// proto.Unmarshals it and an illegal leading tag is rejected. (A +// valid-protobuf ASCII tag does NOT give this: a parser skips it as an +// unknown length-delimited field and resumes into a transition body +// crafted inside signing_package.) +// - Another message's signature cannot be accepted on a signing package: +// the signature is verified over signingPackageSignatureDomain || body, +// which begins with 0x00, whereas a serialized protobuf body always begins +// with a valid field tag (>= 0x08), so the two signed byte streams differ +// in their first byte and the signature cannot verify. +// +// The tag is NOT carried on the wire - it is a fixed constant both signer and +// verifier prepend. +var signingPackageSignatureDomain = []byte("\x00roast/signed-signing-package/v1\x00") // MaxSigningPackageBytes caps the embedded FROST SigningPackage length, // rejecting pathological payloads at Unmarshal time so a misbehaving @@ -232,6 +249,10 @@ func (p *SigningPackage) Unmarshal(data []byte) error { p.CoordinatorSignature = append([]byte(nil), envelope.CoordinatorSignature...) p.bodyCache = append([]byte(nil), envelope.Body...) p.wireEnvelope = append([]byte(nil), data...) + // Clear any signable-bytes cache a prior SignableBytes call left on a reused + // receiver, so the next call rebuilds it from the body just received - + // authentication must verify against the received bytes, never stale ones. + p.signaturePayloadCache = nil return p.Validate() } diff --git a/pkg/frost/roast/signing_package_test.go b/pkg/frost/roast/signing_package_test.go index fbd91a5626..9e50ff0f75 100644 --- a/pkg/frost/roast/signing_package_test.go +++ b/pkg/frost/roast/signing_package_test.go @@ -140,13 +140,85 @@ func TestSigningPackageWire_SignedBytesDomainSeparatedFromTransitionMessage(t *t t.Fatal("sanity: the bare body must collide with TransitionMessageBody") } - // But the domain-tagged SIGNED bytes do NOT present a valid transition body - // (no 32-byte attempt_context_hash), so a coordinator signature over a - // signing package cannot be replayed as a transition-message signature. - var signableAsTransition pb.TransitionMessageBody - _ = proto.Unmarshal(signable, &signableAsTransition) - if len(signableAsTransition.AttemptContextHash) == attempt.MessageDigestLength { - t.Fatal("domain-tagged signed bytes must not present a valid transition attempt_context_hash") + // But the domain-tagged SIGNED bytes begin with an illegal protobuf tag + // (field 0), so they are not decodable as ANY protobuf message - a + // signing-package signature therefore cannot be replayed onto a + // transition-message (or other coordinator-signed) envelope, whose decoder + // proto.Unmarshals and rejects the body. A valid-protobuf tag would not give + // this (see TestSigningPackageWire_SignedBytesResistEmbeddedTransitionBody). + if signable[0] != 0x00 { + t.Fatal("signed bytes must begin with an illegal protobuf tag (0x00)") + } + if err := proto.Unmarshal(signable, &pb.TransitionMessageBody{}); err == nil { + t.Fatal("domain-tagged signed bytes must not decode as a protobuf message") + } +} + +func TestSigningPackageWire_SignedBytesResistEmbeddedTransitionBody(t *testing.T) { + // A malicious coordinator controls signing_package, so it can embed a fully + // valid serialized TransitionMessageBody there. Under a domain tag that is + // itself valid protobuf wire data, a parser skips the tag as an unknown + // field and could resume into this embedded transition body, re-enabling + // cross-protocol signature confusion. The leading illegal tag must make the + // whole signed payload undecodable regardless of the embedded content. + embeddedTransition, err := proto.Marshal(&pb.TransitionMessageBody{ + AttemptContextHash: bytes.Repeat([]byte{0x07}, attempt.MessageDigestLength), + CoordinatorId: 3, + }) + if err != nil { + t.Fatalf("marshal embedded transition: %v", err) + } + // Sanity: the embedded payload really is a valid transition body. + var sanity pb.TransitionMessageBody + if err := proto.Unmarshal(embeddedTransition, &sanity); err != nil || + len(sanity.AttemptContextHash) != attempt.MessageDigestLength { + t.Fatal("sanity: embedded payload must be a valid transition body") + } + + pkg := &SigningPackage{ + AttemptContextHash: append([]byte(nil), pinnedContextHash[:]...), + CoordinatorIDValue: 3, + SigningPackageBytes: embeddedTransition, + } + signable, err := pkg.SignableBytes() + if err != nil { + t.Fatalf("signable: %v", err) + } + if err := proto.Unmarshal(signable, &pb.TransitionMessageBody{}); err == nil { + t.Fatal("signed bytes embedding a transition body must still be undecodable as protobuf") + } +} + +func TestSigningPackageWire_UnmarshalResetsSignableCache(t *testing.T) { + // A SigningPackage value reused across a SignableBytes call and then an + // Unmarshal must authenticate the newly decoded package against the bytes it + // just received, never the stale cached payload. + reused := &SigningPackage{ + AttemptContextHash: append([]byte(nil), pinnedContextHash[:]...), + CoordinatorIDValue: 3, + SigningPackageBytes: []byte("stale-package"), + } + if _, err := reused.SignableBytes(); err != nil { // prime the cache + t.Fatalf("prime cache: %v", err) + } + + // Decode a different, genuine package into the SAME value. + genuine := signedTestSigningPackage(t, 5, bytes.Repeat([]byte{0xab}, TaprootMerkleRootLength)) + 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 signable-bytes cache to the received body") + } + if err := AuthenticateSigningPackage(fakeVerifier{}, reused, 5, pinnedContextHash[:]); err != nil { + t.Fatalf("authenticate reused-decoded package: %v", err) } } From a2d888e5f411dcb7bbd2c7831c2c70c85305c686 Mon Sep 17 00:00:00 2001 From: maclane Date: Sun, 14 Jun 2026 12:20:59 -0400 Subject: [PATCH 8/8] fix(frost/roast): prime signing-package signable cache in Unmarshal (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 --- pkg/frost/roast/signing_package.go | 12 +++++++--- pkg/frost/roast/signing_package_test.go | 29 +++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/pkg/frost/roast/signing_package.go b/pkg/frost/roast/signing_package.go index 83546c2e11..8d163dfcff 100644 --- a/pkg/frost/roast/signing_package.go +++ b/pkg/frost/roast/signing_package.go @@ -249,10 +249,16 @@ func (p *SigningPackage) Unmarshal(data []byte) error { p.CoordinatorSignature = append([]byte(nil), envelope.CoordinatorSignature...) p.bodyCache = append([]byte(nil), envelope.Body...) p.wireEnvelope = append([]byte(nil), data...) - // Clear any signable-bytes cache a prior SignableBytes call left on a reused - // receiver, so the next call rebuilds it from the body just received - - // authentication must verify against the received bytes, never stale ones. + // 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 package race-free: verifiers read a ready cache + // instead of racing on lazy initialization (authentication must verify + // against the received bytes, never stale ones). p.signaturePayloadCache = nil + if _, err := p.SignableBytes(); err != nil { + return err + } return p.Validate() } diff --git a/pkg/frost/roast/signing_package_test.go b/pkg/frost/roast/signing_package_test.go index 9e50ff0f75..7ac8b2e924 100644 --- a/pkg/frost/roast/signing_package_test.go +++ b/pkg/frost/roast/signing_package_test.go @@ -2,6 +2,7 @@ package roast import ( "bytes" + "sync" "testing" "google.golang.org/protobuf/proto" @@ -321,3 +322,31 @@ func TestSigningPackage_MarshalRequiresSignature(t *testing.T) { t.Fatal("Marshal must refuse an unsigned signing package") } } + +func TestSigningPackage_ConcurrentSignableBytesAfterUnmarshalIsRaceFree(t *testing.T) { + // Regression guard (run under -race): a parsed signing package 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 := signedTestSigningPackage(t, 3, nil).Marshal() + if err != nil { + t.Fatalf("marshal: %v", err) + } + var decoded SigningPackage + 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() +}