From 999ceaef436867f1eb3f069882e43a21967766e9 Mon Sep 17 00:00:00 2001 From: maclane Date: Fri, 22 May 2026 23:02:00 -0500 Subject: [PATCH] feat(frost/signing): enforce AttemptContextHash on receive (RFC-21 Phase-6 milestone) Closes the Phase-6 milestone the RFC named but the implementation skipped: receive callbacks now reject messages whose AttemptContextHash does not match the session's bound AttemptContext. Default builds and sessions without a ROAST- attempt binding skip enforcement entirely, so the change is observationally identical to pre-Phase-6 behaviour outside the ROAST path. The Phase 1B AttemptContextHash field was structural-only (present, 32 bytes) until now. Senders could populate it but receivers ignored the value -- meaning a peer could send a message bound to attempt N to a receiver running attempt N+1 of the same session and the receiver would accept it as long as SessionID matched. This PR closes that gap. * pkg/frost/signing/attempt_context_binding_validation_frost_native.go (new, gated frost_native) - attemptContextHashCarrier interface so the helper covers all three FROST/tbtc-signer message types via their existing GetAttemptContextHash methods. - verifyMessageAttemptContextHash: looks up the session's handle binding via currentAttemptHandleForCollect. No binding -> return nil (legacy / default build). Binding present + matching hash -> return nil. Binding present + missing hash -> ErrAttemptContextHashMissing. Binding present + mismatched hash -> ErrAttemptContextHashMismatch. * pkg/frost/signing/native_frost_protocol_frost_native.go and * pkg/frost/signing/native_ffi_primitive_transitional_frost_native.go Three receive callbacks updated. After the existing shouldAcceptNativeFROSTMessage gate, each callback now calls verifyMessageAttemptContextHash. Failure paths call evidence.RecordReject(senderID, "attempt_context_hash_mismatch") so the policy can permanently exclude peers that consistently send stale-attempt messages. Tests: * attempt_context_binding_validation_frost_native_test.go (gated frost_native && frost_roast_retry, 5 cases) - No binding -> any message passes - Binding + matching hash -> passes - Binding + missing hash -> ErrAttemptContextHashMissing - Binding + mismatched hash -> ErrAttemptContextHashMismatch - Integration with a real nativeFROSTRoundOneCommitmentMessage via SetAttemptContextHash; rebinding to a different context produces a mismatch * attempt_context_binding_validation_default_build_test.go (gated frost_native && !frost_roast_retry, 1 case) - In the default build the helper always passes regardless of message contents, matching the rollback promise. Verification: * go build ./... + go build -tags 'frost_native frost_tbtc_signer frost_roast_retry' ./... -- both clean * go test ./pkg/frost/... -- pass * go test -tags 'frost_native frost_tbtc_signer' ./pkg/frost/... -- pass * go test -tags 'frost_native frost_tbtc_signer frost_roast_retry' ./pkg/frost/... -- pass (5 packages) * staticcheck -checks '-SA1019' ./pkg/frost/... -- silent * gofmt -l ./pkg/frost/signing/ -- silent * go vet ./pkg/frost/... -- clean Migration path: * Phase 1B (already shipped): AttemptContextHash is structurally validated when present, optional otherwise. * This PR: the field is enforced *only* when the session has a ROAST-attempt binding. Sessions without a binding -- including every default-build session and every non-ROAST tagged-build session -- continue to ignore the field. * Future PR: once production has rolled out a version that populates the field on every outbound message, enforcement can be made unconditional (binding-or-not). --- ...t_binding_validation_default_build_test.go | 34 ++++ ...context_binding_validation_frost_native.go | 82 +++++++++ ...xt_binding_validation_frost_native_test.go | 159 ++++++++++++++++++ ...ffi_primitive_transitional_frost_native.go | 5 + .../native_frost_protocol_frost_native.go | 10 ++ 5 files changed, 290 insertions(+) create mode 100644 pkg/frost/signing/attempt_context_binding_validation_default_build_test.go create mode 100644 pkg/frost/signing/attempt_context_binding_validation_frost_native.go create mode 100644 pkg/frost/signing/attempt_context_binding_validation_frost_native_test.go diff --git a/pkg/frost/signing/attempt_context_binding_validation_default_build_test.go b/pkg/frost/signing/attempt_context_binding_validation_default_build_test.go new file mode 100644 index 0000000000..288758f241 --- /dev/null +++ b/pkg/frost/signing/attempt_context_binding_validation_default_build_test.go @@ -0,0 +1,34 @@ +//go:build frost_native && !frost_roast_retry + +package signing + +import ( + "testing" +) + +func TestVerifyMessageAttemptContextHash_DefaultBuildPassesEverything(t *testing.T) { + // Without the frost_roast_retry tag, currentAttemptHandleForCollect + // always returns ok=false, so the helper short-circuits to nil + // for every input. This guarantees that the receive-loop wiring + // never enforces the AttemptContextHash binding in the default + // build, matching the rollback promise made in the rollout + // guide (docs/development/frost-roast-retry-rollout.adoc). + msg := stubDefaultBuildMessage{} + if err := verifyMessageAttemptContextHash(msg, "any-session"); err != nil { + t.Fatalf( + "default build must always pass; got %v", + err, + ) + } +} + +// stubDefaultBuildMessage is the equivalent of the tagged-build +// test's stubMessage. Kept separate to avoid the tagged-build +// definition leaking into this build's compilation unit. +type stubDefaultBuildMessage struct{} + +func (stubDefaultBuildMessage) GetAttemptContextHash() ( + [AttemptContextHashFieldLength]byte, bool, +) { + return [AttemptContextHashFieldLength]byte{}, false +} diff --git a/pkg/frost/signing/attempt_context_binding_validation_frost_native.go b/pkg/frost/signing/attempt_context_binding_validation_frost_native.go new file mode 100644 index 0000000000..24b19435ad --- /dev/null +++ b/pkg/frost/signing/attempt_context_binding_validation_frost_native.go @@ -0,0 +1,82 @@ +//go:build frost_native + +package signing + +import ( + "errors" + "fmt" +) + +// attemptContextHashCarrier is implemented by every protocol +// message type that carries the optional AttemptContextHash field +// introduced in RFC-21 Phase 1B. The validation helper below uses +// the interface so a single implementation covers all three +// FROST/tbtc-signer message types without duplicating per-type +// logic. +type attemptContextHashCarrier interface { + // GetAttemptContextHash returns the message's hash and a + // presence flag. Implementations are generated by the per-type + // Set/Get helpers in attempt_context_binding.go. + GetAttemptContextHash() ([AttemptContextHashFieldLength]byte, bool) +} + +// ErrAttemptContextHashMissing is returned when a message lacks +// the AttemptContextHash field while the session is bound to a +// ROAST attempt that requires it. Distinct sentinel so callers +// can map it to a specific RecordReject reason. +var ErrAttemptContextHashMissing = errors.New( + "attempt context hash required: session is ROAST-active but message omits the binding field", +) + +// ErrAttemptContextHashMismatch is returned when a message's +// AttemptContextHash does not match the session's currently-bound +// AttemptContext.Hash(). The peer is either talking about a stale +// attempt (post-transition) or trying to inject a message for a +// different context. +var ErrAttemptContextHashMismatch = errors.New( + "attempt context hash mismatch: message bound to a different attempt", +) + +// verifyMessageAttemptContextHash enforces the RFC-21 Phase-6 +// milestone that promotes the AttemptContextHash field from +// optional to required at the receive boundary, but only when the +// session has a ROAST-attempt binding registered. +// +// When no session-handle binding exists for sessionID (the typical +// state for non-ROAST sessions and for default builds), this +// function returns nil and lets the message through. The receive +// loop's other gates (shouldAcceptNativeFROSTMessage, etc.) still +// apply. +// +// When a binding exists -- i.e. the orchestration layer has begun +// an attempt for this session and is expecting the receive loops +// to participate -- the message must carry an AttemptContextHash +// that equals the bound context's Hash(). Returns +// ErrAttemptContextHashMissing or ErrAttemptContextHashMismatch on +// failure so the caller can RecordReject with a precise reason. +func verifyMessageAttemptContextHash( + msg attemptContextHashCarrier, + sessionID string, +) error { + _, ctx, ok := currentAttemptHandleForCollect(sessionID) + if !ok { + // No binding: legacy / non-ROAST mode. Skip enforcement + // so default builds and non-ROAST sessions stay + // observationally identical to pre-Phase-6 behaviour. + return nil + } + msgHash, present := msg.GetAttemptContextHash() + if !present { + return ErrAttemptContextHashMissing + } + expected := ctx.Hash() + if msgHash != expected { + return fmt.Errorf( + "%w: message=%x, current attempt=%x", + ErrAttemptContextHashMismatch, + msgHash[:4], + expected[:4], + ) + } + return nil +} diff --git a/pkg/frost/signing/attempt_context_binding_validation_frost_native_test.go b/pkg/frost/signing/attempt_context_binding_validation_frost_native_test.go new file mode 100644 index 0000000000..1a4338283b --- /dev/null +++ b/pkg/frost/signing/attempt_context_binding_validation_frost_native_test.go @@ -0,0 +1,159 @@ +//go:build frost_native && frost_roast_retry + +package signing + +import ( + "errors" + "testing" + + "github.com/keep-network/keep-core/pkg/frost/roast" + "github.com/keep-network/keep-core/pkg/frost/roast/attempt" + "github.com/keep-network/keep-core/pkg/protocol/group" +) + +// stubMessage is a minimal attemptContextHashCarrier implementation +// for unit tests. The receive callbacks use the three real message +// types; the helper itself is exercised via this stub so the test +// surface stays small. +type stubMessage struct { + hash [AttemptContextHashFieldLength]byte + present bool +} + +func (s stubMessage) GetAttemptContextHash() ( + [AttemptContextHashFieldLength]byte, bool, +) { + return s.hash, s.present +} + +func newOrchestrationTestContextForValidation(t *testing.T) attempt.AttemptContext { + t.Helper() + ctx, err := attempt.NewAttemptContext( + "validation-test", + "key-group", + []byte{0x01, 0x02}, + [attempt.MessageDigestLength]byte{0x77}, + 0, + []group.MemberIndex{1, 2, 3, 4, 5}, + nil, + ) + if err != nil { + t.Fatalf("ctx: %v", err) + } + return ctx +} + +func TestVerifyMessageAttemptContextHash_NoBindingPasses(t *testing.T) { + // In the default build, no session-handle bindings exist so + // every call returns nil regardless of message contents. The + // receive loop's other gates still apply. + ResetSessionHandleRegistryForTest() + t.Cleanup(ResetSessionHandleRegistryForTest) + + cases := []stubMessage{ + {present: false}, + {present: true, hash: [AttemptContextHashFieldLength]byte{0x01}}, + } + for _, msg := range cases { + if err := verifyMessageAttemptContextHash(msg, "session-x"); err != nil { + t.Fatalf( + "no-binding path must pass; got %v for msg %+v", + err, msg, + ) + } + } +} + +func TestVerifyMessageAttemptContextHash_BindingPresent_MatchingHashPasses(t *testing.T) { + ResetSessionHandleRegistryForTest() + t.Cleanup(ResetSessionHandleRegistryForTest) + + ctx := newOrchestrationTestContextForValidation(t) + SetCurrentAttemptHandleForSession("session-match", roast.AttemptHandle{}, ctx) + + expected := ctx.Hash() + msg := stubMessage{hash: expected, present: true} + if err := verifyMessageAttemptContextHash(msg, "session-match"); err != nil { + t.Fatalf("matching hash must pass; got %v", err) + } +} + +func TestVerifyMessageAttemptContextHash_BindingPresent_MissingHashFails(t *testing.T) { + ResetSessionHandleRegistryForTest() + t.Cleanup(ResetSessionHandleRegistryForTest) + + ctx := newOrchestrationTestContextForValidation(t) + SetCurrentAttemptHandleForSession("session-missing", roast.AttemptHandle{}, ctx) + + msg := stubMessage{present: false} + err := verifyMessageAttemptContextHash(msg, "session-missing") + if !errors.Is(err, ErrAttemptContextHashMissing) { + t.Fatalf( + "expected ErrAttemptContextHashMissing; got %v", + err, + ) + } +} + +func TestVerifyMessageAttemptContextHash_BindingPresent_MismatchedHashFails(t *testing.T) { + ResetSessionHandleRegistryForTest() + t.Cleanup(ResetSessionHandleRegistryForTest) + + ctx := newOrchestrationTestContextForValidation(t) + SetCurrentAttemptHandleForSession("session-mismatch", roast.AttemptHandle{}, ctx) + + wrong := [AttemptContextHashFieldLength]byte{} + for i := range wrong { + wrong[i] = 0xff + } + msg := stubMessage{hash: wrong, present: true} + err := verifyMessageAttemptContextHash(msg, "session-mismatch") + if !errors.Is(err, ErrAttemptContextHashMismatch) { + t.Fatalf( + "expected ErrAttemptContextHashMismatch; got %v", + err, + ) + } +} + +func TestVerifyMessageAttemptContextHash_RealMessageTypeIntegration(t *testing.T) { + // Exercise the helper against a real protocol message type + // (the round-one commitment from Phase 1B) rather than just + // the stub, so the test surface covers the actual Set/Get + // helpers code path. + ResetSessionHandleRegistryForTest() + t.Cleanup(ResetSessionHandleRegistryForTest) + + ctx := newOrchestrationTestContextForValidation(t) + SetCurrentAttemptHandleForSession("session-real-msg", roast.AttemptHandle{}, ctx) + + expected := ctx.Hash() + msg := &nativeFROSTRoundOneCommitmentMessage{ + SenderIDValue: 1, + SessionIDValue: "session-real-msg", + ParticipantIdentifier: "p1", + CommitmentData: []byte{0x01}, + } + msg.SetAttemptContextHash(expected) + + if err := verifyMessageAttemptContextHash(msg, "session-real-msg"); err != nil { + t.Fatalf("real-message integration must pass; got %v", err) + } + + // Now mutate the context to break the binding. + differentCtx, _ := attempt.NewAttemptContext( + "session-real-msg", + "key-group", + []byte{0x99}, + [attempt.MessageDigestLength]byte{0x77}, + 1, + []group.MemberIndex{1, 2, 3, 4, 5}, + nil, + ) + SetCurrentAttemptHandleForSession("session-real-msg", roast.AttemptHandle{}, differentCtx) + + err := verifyMessageAttemptContextHash(msg, "session-real-msg") + if !errors.Is(err, ErrAttemptContextHashMismatch) { + t.Fatalf("rebinding must cause mismatch; got %v", err) + } +} diff --git a/pkg/frost/signing/native_ffi_primitive_transitional_frost_native.go b/pkg/frost/signing/native_ffi_primitive_transitional_frost_native.go index 1a2671bd68..07d09c778e 100644 --- a/pkg/frost/signing/native_ffi_primitive_transitional_frost_native.go +++ b/pkg/frost/signing/native_ffi_primitive_transitional_frost_native.go @@ -1002,6 +1002,11 @@ func collectBuildTaggedTBTCSignerRoundContributionMessages( return } + if err := verifyMessageAttemptContextHash(payload, request.SessionID); err != nil { + evidence.RecordReject(payload.SenderID(), "attempt_context_hash_mismatch") + return + } + _ = enqueueOrRecordOverflow(payload, messageChan, evidence) }) diff --git a/pkg/frost/signing/native_frost_protocol_frost_native.go b/pkg/frost/signing/native_frost_protocol_frost_native.go index 5fcb8e9cff..d0c08e0a47 100644 --- a/pkg/frost/signing/native_frost_protocol_frost_native.go +++ b/pkg/frost/signing/native_frost_protocol_frost_native.go @@ -640,6 +640,11 @@ func collectNativeFROSTRoundOneMessages( return } + if err := verifyMessageAttemptContextHash(payload, request.SessionID); err != nil { + evidence.RecordReject(payload.SenderID(), "attempt_context_hash_mismatch") + return + } + _ = enqueueOrRecordOverflow(payload, messageChan, evidence) }) @@ -722,6 +727,11 @@ func collectNativeFROSTRoundTwoMessages( return } + if err := verifyMessageAttemptContextHash(payload, request.SessionID); err != nil { + evidence.RecordReject(payload.SenderID(), "attempt_context_hash_mismatch") + return + } + _ = enqueueOrRecordOverflow(payload, messageChan, evidence) })