From 1fbf4a22796f530632772c8fd1d5f98b1b1def11 Mon Sep 17 00:00:00 2001 From: maclane Date: Fri, 22 May 2026 16:25:32 -0500 Subject: [PATCH] Refuse scaffold key-group by default; tighten witness and message hygiene MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four findings from the independent PR #3866 review, bundled because they all sit in the same code seam (frost_native scaffold path + receive loops). 1. H1+H4 — scaffold key-group must be opt-in (was silently accepted) `pkg/tbtc/signer_material_resolver_build_frost_native_tbtc_signer.go` previously built signer material whose `KeyGroupSource` was the string `"legacy-wallet-pubkey"` — a placeholder derived from a sha256 of the legacy wallet public key rather than the output of a real FROST DKG run — and the FFI primitive at `pkg/frost/signing/native_ffi_primitive_transitional_frost_native.go` silently substituted the Rust signer's RunDKG key group when the payload's source was that placeholder. Together that meant a production deployment with placeholder material would have routed signing through whatever key group the Rust side happened to return without any operator-facing signal. Add an explicit, refuse-by-default opt-in: `KEEP_CORE_FROST_TBTC_SIGNER_ACCEPT_SCAFFOLD_KEY_GROUP=1`. The new `signing.AcceptScaffoldKeyGroupEnabled` helper is per-call (not cached), so flipping the env back to unset recovers fail-closed behavior without a restart. Both the resolver and the FFI primitive check the flag; both refuse with a clear error message that names the env var and the placeholder source. Existing scaffold-using tests (`TestBuildTaggedTBTCSignerRoundKeyGroup`, `..._LegacyKeyGroupSourceUsesRunDKGResult`, `TestRegisterSignerMaterialResolverForBuild_UsesDefaultProvider`, the `signingExecutor` suite in `pkg/tbtc`) opt in via `t.Setenv` to continue exercising the scaffold path; a new `TestRegisterSignerMaterialResolverForBuild_DefaultProviderRefusesScaffoldWithoutOptIn` pins the refuse-by-default behavior, and a new `TestBuildTaggedTBTCSignerRoundKeyGroup/legacy_source_mismatch_refused_without_opt_in` covers the FFI side. 2. M2+M3 — Bitcoin witness restoration refuses unsupported shapes `pkg/bitcoin/transaction_builder.go`'s `ReplaceUnsignedTransaction` restoration path handled only `len(previousInput.Witness) == 1` (the P2WSH-style single redeem script). Multi-element previous witnesses — what a P2TR script-path spend would carry — were silently dropped, leaving the replaced input with an empty witness that signing later couldn't recover. Out-of-scope for the current P2TR key-path FROST migration but a footgun the next person to touch this code would hit. Switch to an explicit `switch` over previous witness length: 0 leaves the replacement empty, 1 restores the redeem script as before, anything else fails loudly with a clear "only zero- or single-element pre-signing witnesses are currently supported" error. Lifting this to support multi-element witnesses needs a per-input policy (the replacement could legitimately differ in witness shape from the previous), so failing loudly is the safer shape today. Also remove the tautological inner `len(replacedInput.X) == 0` checks that the two outer refusals already guarantee. New regression test `TestTransactionBuilder_ReplaceUnsignedTransaction_RejectsMultiElementPreviousWitness`. 3. M5 — first-write-wins on peer messages The three round-message receive loops (`native_ffi_primitive_transitional_frost_native.go` tbtc-signer contribution, `native_frost_protocol_frost_native.go` round one and round two) all did `receivedMessages[message.SenderID()] = message`, last-write-wins. That let a peer mutate its own contribution after the first send. ROAST evidence semantics call for first-write-wins, with bit-identical retransmissions being idempotent and conflicting retransmissions being dropped with a structured log entry. Each receive loop now checks `receivedMessages[senderID]` first. If present and the new message is byte-equal on the relevant payload fields (`Contribution{Identifier,Data}` for tbtc-signer, `Commitment{Identifier,Data}` for round one, `SignatureShare{Identifier,Data}` for round two), the duplicate is ignored; if different, the new message is dropped with a `protocolLogger.Warnf` line that names the sender. Three equality helpers (`buildTaggedTBTCSignerRoundContributionMessagesEqual`, `nativeFROSTRoundOneCommitmentMessagesEqual`, `nativeFROSTRoundTwoSignatureShareMessagesEqual`) plus a new package-level `protocolLogger` log channel. Verification (local, GOCACHE under /private/tmp): go test ./pkg/frost/... ./pkg/bitcoin go test -tags 'frost_native frost_tbtc_signer' ./pkg/frost/... ./pkg/bitcoin go test -tags 'frost_native frost_tbtc_signer' ./pkg/tbtc -run \ 'TestConfigureFrostSigningBackend|TestNewNode_ConfiguresFrostSigningBackend|TestSigningExecutor_Sign|TestRegisterSignerMaterialResolverForBuild|TestBuildTaggedTBTCSignerRoundKeyGroup|TestBuildTaggedLegacyCompatibleNativeExecutionFFISigningPrimitive|TestTransactionBuilder_ReplaceUnsignedTransaction' All pass. This is the safe-by-default tier of the PR #3866 review remediation; the M4 (ROAST bounded transition evidence) and M7 (ROAST-aware retry replacing the byte-identical tECDSA shuffle) tracks are separate multi-PR efforts, and L5 (FFI status-code semantics) is paired with a forthcoming tbtc-signer change. Co-Authored-By: Claude Opus 4.7 (1M context) --- pkg/bitcoin/transaction_builder.go | 36 ++++++++--- pkg/bitcoin/transaction_builder_test.go | 52 ++++++++++++++++ .../native_ffi_primitive_registration.go | 1 + ...ffi_primitive_transitional_frost_native.go | 51 +++++++++++++++- ...rimitive_transitional_frost_native_test.go | 57 +++++++++++++++--- .../native_frost_protocol_frost_native.go | 59 ++++++++++++++++++- .../signing/native_tbtc_signer_material.go | 41 ++++++++++++- ...resolver_build_frost_native_tbtc_signer.go | 15 +++++ ...terial_resolver_build_frost_native_test.go | 46 +++++++++++++++ pkg/tbtc/signing_test.go | 8 +++ 10 files changed, 346 insertions(+), 20 deletions(-) diff --git a/pkg/bitcoin/transaction_builder.go b/pkg/bitcoin/transaction_builder.go index 7b40758cce..c74b4562eb 100644 --- a/pkg/bitcoin/transaction_builder.go +++ b/pkg/bitcoin/transaction_builder.go @@ -354,18 +354,40 @@ func (tb *TransactionBuilder) ReplaceUnsignedTransaction( ) } + // The replacement's SignatureScript and Witness are both empty here + // because of the two refusals above, so the per-input restore below + // only has to decide what to copy *from* the previous input. if tb.sigHashArgs[i].witness { - if len(replacedInput.Witness) == 0 && len(previousInput.Witness) == 1 { + // Witness inputs may carry a single-element pre-signing witness + // that holds a P2WSH-style redeem script. Multi-element witnesses + // belong to P2TR script-path spends or other workflows that are + // not in scope for the current FROST migration, and silently + // dropping them produced malformed transactions later — refuse + // instead so the unsupported case fails loudly. Lifting this to + // support multi-element witnesses requires a per-input policy + // rather than a blanket copy because the replacement could + // legitimately differ in witness shape from the previous input. + switch len(previousInput.Witness) { + case 0: + // Nothing to restore (typical P2TR key-path or P2WPKH). + case 1: redeemScript := append([]byte{}, previousInput.Witness[0]...) replacedInput.Witness = wire.TxWitness{redeemScript} - } - } else { - if len(replacedInput.SignatureScript) == 0 && len(previousInput.SignatureScript) > 0 { - replacedInput.SignatureScript = append( - []byte{}, - previousInput.SignatureScript..., + default: + return fmt.Errorf( + "replacement transaction input [%d] previous witness has "+ + "[%d] elements; only zero- or single-element "+ + "pre-signing witnesses are currently supported for "+ + "restoration", + i, + len(previousInput.Witness), ) } + } else if len(previousInput.SignatureScript) > 0 { + replacedInput.SignatureScript = append( + []byte{}, + previousInput.SignatureScript..., + ) } } diff --git a/pkg/bitcoin/transaction_builder_test.go b/pkg/bitcoin/transaction_builder_test.go index 2720febb7d..8349946f6c 100644 --- a/pkg/bitcoin/transaction_builder_test.go +++ b/pkg/bitcoin/transaction_builder_test.go @@ -451,6 +451,58 @@ func TestTransactionBuilder_ReplaceUnsignedTransaction_RejectsNonEmptyReplacemen } } +func TestTransactionBuilder_ReplaceUnsignedTransaction_RejectsMultiElementPreviousWitness( + t *testing.T, +) { + builder := NewTransactionBuilder(nil) + + var txHash chainhash.Hash + previousInput := wire.NewTxIn(wire.NewOutPoint(&txHash, 0), nil, nil) + // Pre-signing witness that mimics a P2TR script-path spend: [script, + // controlBlock]. The restoration path supports only zero- or + // single-element previous witnesses today; the multi-element case must + // fail loudly rather than silently dropping data later in signing. + previousInput.Witness = wire.TxWitness{ + []byte{0x51, 0x52}, + []byte{0xc0, 0xab, 0xcd}, + } + builder.internal.AddTxIn(previousInput) + builder.sigHashArgs = append( + builder.sigHashArgs, + &inputSigHashArgs{value: 1, scriptCode: []byte{0x51}, witness: true}, + ) + + err := builder.ReplaceUnsignedTransaction( + &Transaction{ + Inputs: []*TransactionInput{ + { + Outpoint: &TransactionOutpoint{ + TransactionHash: Hash(txHash), + OutputIndex: 0, + }, + Sequence: 0xffffffff, + }, + }, + }, + ) + if err == nil { + t.Fatal("expected multi-element witness restoration error") + } + + if !strings.Contains( + err.Error(), + "previous witness has [2] elements", + ) { + t.Fatalf("unexpected error: [%v]", err) + } + if !strings.Contains( + err.Error(), + "only zero- or single-element", + ) { + t.Fatalf("unexpected error: [%v]", err) + } +} + func TestTransactionBuilder_UnsignedTransactionIO(t *testing.T) { builder := NewTransactionBuilder(nil) diff --git a/pkg/frost/signing/native_ffi_primitive_registration.go b/pkg/frost/signing/native_ffi_primitive_registration.go index 516330a56f..a62a38b19f 100644 --- a/pkg/frost/signing/native_ffi_primitive_registration.go +++ b/pkg/frost/signing/native_ffi_primitive_registration.go @@ -9,6 +9,7 @@ import ( var ( registrationLogger = log.Logger("keep-frost-signing-registration") + protocolLogger = log.Logger("keep-frost-signing-protocol") registrationErrorMu sync.RWMutex lastRegistrationError error ) 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 e38b12eecb..6bdac5a854 100644 --- a/pkg/frost/signing/native_ffi_primitive_transitional_frost_native.go +++ b/pkg/frost/signing/native_ffi_primitive_transitional_frost_native.go @@ -3,6 +3,7 @@ package signing import ( + "bytes" "context" "crypto/sha256" "encoding/hex" @@ -466,6 +467,23 @@ func buildTaggedTBTCSignerRoundKeyGroup( if payload.KeyGroupSource == NativeTBTCSignerKeyGroupSourceLegacyWalletPubKey { // Scaffold compatibility: legacy-wallet-pubkey key groups are // placeholder-only and expected to diverge from coarse RunDKG output. + // Refuse the substitution by default so a production deployment that + // somehow ended up with placeholder material does not silently route + // signing through whatever key group the Rust side happens to return. + // The operator must explicitly opt into the scaffold path via + // AcceptScaffoldKeyGroupEnvVar; the env-var check is per-call (not + // cached) so flipping it off recovers fail-closed behavior without a + // restart. + if !AcceptScaffoldKeyGroupEnabled() { + return "", false, fmt.Errorf( + "tbtc-signer key group source %q is scaffold-era placeholder "+ + "material and may not be silently substituted with the "+ + "RunDKG output; set %s=true to opt in for local/CI use "+ + "only, never in production", + NativeTBTCSignerKeyGroupSourceLegacyWalletPubKey, + AcceptScaffoldKeyGroupEnvVar, + ) + } return dkgResult.KeyGroup, true, nil } @@ -902,13 +920,44 @@ func collectBuildTaggedTBTCSignerRoundContributionMessages( ) case message := <-messageChan: - receivedMessages[message.SenderID()] = message + // First-write-wins / equal-or-reject. A peer that retransmits the + // same contribution is idempotent; a peer that mutates its own + // contribution after the first send is a ROAST evidence concern + // and must not be allowed to overwrite the persisted view. + senderID := message.SenderID() + if existing, ok := receivedMessages[senderID]; ok { + if !buildTaggedTBTCSignerRoundContributionMessagesEqual( + existing, + message, + ) { + protocolLogger.Warnf( + "dropping conflicting tbtc-signer round contribution "+ + "from sender [%d]; first-write-wins keeps the "+ + "originally accepted contribution", + senderID, + ) + } + continue + } + receivedMessages[senderID] = message } } return receivedMessages, nil } +func buildTaggedTBTCSignerRoundContributionMessagesEqual( + left, right *buildTaggedTBTCSignerRoundContributionMessage, +) bool { + if left == nil || right == nil { + return left == right + } + return left.SenderIDValue == right.SenderIDValue && + left.SessionIDValue == right.SessionIDValue && + left.ContributionIdentifier == right.ContributionIdentifier && + bytes.Equal(left.ContributionData, right.ContributionData) +} + func buildTaggedTBTCSignerSyntheticRoundContributions( roundState *NativeTBTCSignerRoundState, includedMembersIndexes []group.MemberIndex, diff --git a/pkg/frost/signing/native_ffi_primitive_transitional_frost_native_test.go b/pkg/frost/signing/native_ffi_primitive_transitional_frost_native_test.go index 03a01951cc..3bcce62fc3 100644 --- a/pkg/frost/signing/native_ffi_primitive_transitional_frost_native_test.go +++ b/pkg/frost/signing/native_ffi_primitive_transitional_frost_native_test.go @@ -1289,12 +1289,14 @@ func TestExecuteBuildTaggedTBTCSignerBootstrapCoarseRound_FailsWhenRoundStateSig func TestBuildTaggedTBTCSignerRoundKeyGroup(t *testing.T) { testCases := []struct { - name string - payload *NativeTBTCSignerMaterialPayload - dkgResult *NativeTBTCSignerDKGResult - expected string - substituted bool - expectError bool + name string + payload *NativeTBTCSignerMaterialPayload + dkgResult *NativeTBTCSignerDKGResult + acceptScaffoldOptIn bool + expected string + substituted bool + expectError bool + expectScaffoldRefuse bool }{ { name: "exact match", @@ -1308,7 +1310,19 @@ func TestBuildTaggedTBTCSignerRoundKeyGroup(t *testing.T) { substituted: false, }, { - name: "legacy source mismatch uses dkg key group", + name: "legacy source mismatch refused without opt-in", + payload: &NativeTBTCSignerMaterialPayload{ + KeyGroup: "legacy-group", + KeyGroupSource: NativeTBTCSignerKeyGroupSourceLegacyWalletPubKey, + }, + dkgResult: &NativeTBTCSignerDKGResult{ + KeyGroup: "dkg-group", + }, + expectError: true, + expectScaffoldRefuse: true, + }, + { + name: "legacy source mismatch uses dkg key group with opt-in", payload: &NativeTBTCSignerMaterialPayload{ KeyGroup: "legacy-group", KeyGroupSource: NativeTBTCSignerKeyGroupSourceLegacyWalletPubKey, @@ -1316,8 +1330,9 @@ func TestBuildTaggedTBTCSignerRoundKeyGroup(t *testing.T) { dkgResult: &NativeTBTCSignerDKGResult{ KeyGroup: "dkg-group", }, - expected: "dkg-group", - substituted: true, + acceptScaffoldOptIn: true, + expected: "dkg-group", + substituted: true, }, { name: "non-legacy source mismatch rejects", @@ -1334,12 +1349,30 @@ func TestBuildTaggedTBTCSignerRoundKeyGroup(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + if tc.acceptScaffoldOptIn { + t.Setenv(AcceptScaffoldKeyGroupEnvVar, "true") + } else { + // Force the env to "" so a stray external value from a + // containing process cannot suppress the scaffold refusal + // during this test case. + t.Setenv(AcceptScaffoldKeyGroupEnvVar, "") + } + actual, substituted, err := buildTaggedTBTCSignerRoundKeyGroup(tc.payload, tc.dkgResult) if tc.expectError { if err == nil { t.Fatal("expected error") } + if tc.expectScaffoldRefuse && + !strings.Contains(err.Error(), AcceptScaffoldKeyGroupEnvVar) { + t.Fatalf( + "expected scaffold-refusal error referencing %s; got: [%v]", + AcceptScaffoldKeyGroupEnvVar, + err, + ) + } + return } @@ -1802,6 +1835,12 @@ func TestBuildTaggedLegacyCompatibleNativeExecutionFFISigningPrimitive_Sign_TBTC func TestBuildTaggedLegacyCompatibleNativeExecutionFFISigningPrimitive_Sign_TBTCSignerPath_BootstrapVersion_LegacyKeyGroupSourceUsesRunDKGResult( t *testing.T, ) { + // Scaffold-era path: legacy-wallet-pubkey signer material is refused by + // default; the operator opt-in via AcceptScaffoldKeyGroupEnvVar is what + // lets this test exercise the substitution. Production deployments must + // never set this. + t.Setenv(AcceptScaffoldKeyGroupEnvVar, "true") + engine := &mockBuildTaggedTBTCSignerEngine{ version: "tbtc-signer/0.1.0-bootstrap", runDKGResult: &NativeTBTCSignerDKGResult{ diff --git a/pkg/frost/signing/native_frost_protocol_frost_native.go b/pkg/frost/signing/native_frost_protocol_frost_native.go index e2c496be73..3dcc1af4d8 100644 --- a/pkg/frost/signing/native_frost_protocol_frost_native.go +++ b/pkg/frost/signing/native_frost_protocol_frost_native.go @@ -3,6 +3,7 @@ package signing import ( + "bytes" "context" "encoding/json" "errors" @@ -582,13 +583,39 @@ func collectNativeFROSTRoundOneMessages( ) case message := <-messageChan: - receivedMessages[message.SenderID()] = message + // First-write-wins / equal-or-reject. See the matching comment in + // native_ffi_primitive_transitional_frost_native.go. + senderID := message.SenderID() + if existing, ok := receivedMessages[senderID]; ok { + if !nativeFROSTRoundOneCommitmentMessagesEqual(existing, message) { + protocolLogger.Warnf( + "dropping conflicting native FROST round one "+ + "commitment from sender [%d]; first-write-wins "+ + "keeps the originally accepted commitment", + senderID, + ) + } + continue + } + receivedMessages[senderID] = message } } return receivedMessages, nil } +func nativeFROSTRoundOneCommitmentMessagesEqual( + left, right *nativeFROSTRoundOneCommitmentMessage, +) bool { + if left == nil || right == nil { + return left == right + } + return left.SenderIDValue == right.SenderIDValue && + left.SessionIDValue == right.SessionIDValue && + left.ParticipantIdentifier == right.ParticipantIdentifier && + bytes.Equal(left.CommitmentData, right.CommitmentData) +} + func collectNativeFROSTRoundTwoMessages( ctx context.Context, request *NativeExecutionFFISigningRequest, @@ -638,13 +665,41 @@ func collectNativeFROSTRoundTwoMessages( ) case message := <-messageChan: - receivedMessages[message.SenderID()] = message + // First-write-wins / equal-or-reject. See round one above. + senderID := message.SenderID() + if existing, ok := receivedMessages[senderID]; ok { + if !nativeFROSTRoundTwoSignatureShareMessagesEqual( + existing, + message, + ) { + protocolLogger.Warnf( + "dropping conflicting native FROST round two "+ + "signature share from sender [%d]; first-write-wins "+ + "keeps the originally accepted share", + senderID, + ) + } + continue + } + receivedMessages[senderID] = message } } return receivedMessages, nil } +func nativeFROSTRoundTwoSignatureShareMessagesEqual( + left, right *nativeFROSTRoundTwoSignatureShareMessage, +) bool { + if left == nil || right == nil { + return left == right + } + return left.SenderIDValue == right.SenderIDValue && + left.SessionIDValue == right.SessionIDValue && + left.ParticipantIdentifier == right.ParticipantIdentifier && + bytes.Equal(left.SignatureShareData, right.SignatureShareData) +} + func shouldAcceptNativeFROSTMessage( request *NativeExecutionFFISigningRequest, includedMembersSet map[group.MemberIndex]struct{}, diff --git a/pkg/frost/signing/native_tbtc_signer_material.go b/pkg/frost/signing/native_tbtc_signer_material.go index ad8b443ad9..3b5391e391 100644 --- a/pkg/frost/signing/native_tbtc_signer_material.go +++ b/pkg/frost/signing/native_tbtc_signer_material.go @@ -1,12 +1,27 @@ package signing +import ( + "os" + "strings" +) + const ( // NativeSignerMaterialFormatFrostTBTCSignerV1 carries signer material for // tbtc-signer coarse session APIs. NativeSignerMaterialFormatFrostTBTCSignerV1 = "frost-tbtc-signer-v1" // NativeTBTCSignerKeyGroupSourceLegacyWalletPubKey marks scaffold-era - // key-group derivation from the legacy wallet public key. + // key-group derivation from the legacy wallet public key. Material built + // with this source is placeholder data, not the output of a real FROST DKG + // run, and is refused by default at signing time. See + // `AcceptScaffoldKeyGroupEnvVar` for the opt-in escape hatch. NativeTBTCSignerKeyGroupSourceLegacyWalletPubKey = "legacy-wallet-pubkey" + + // AcceptScaffoldKeyGroupEnvVar is the operator-facing opt-in that allows + // the FROST tbtc-signer FFI path to accept signer material whose + // `KeyGroupSource` is `legacy-wallet-pubkey`. Production deployments must + // not set this; it exists for local dev, CI, and integration rehearsals + // where a real DKG hand-off is not yet wired. + AcceptScaffoldKeyGroupEnvVar = "KEEP_CORE_FROST_TBTC_SIGNER_ACCEPT_SCAFFOLD_KEY_GROUP" ) // NativeTBTCSignerMaterialPayload is the signer-material payload schema for @@ -16,3 +31,27 @@ type NativeTBTCSignerMaterialPayload struct { KeyGroupSource string `json:"keyGroupSource,omitempty"` LegacyPrivateKeyShareHex string `json:"legacyPrivateKeyShareHex,omitempty"` } + +// AcceptScaffoldKeyGroupEnabled reports whether the operator has opted into +// accepting scaffold-era (legacy-wallet-pubkey) key-group material. Without +// this, the signer material resolver and the FFI signing primitive both +// refuse legacy material rather than silently signing with placeholder +// cryptographic context. +// +// The env var is parsed identically to the bootstrap-mode flag in +// `pkg/frost/signing/backend.go`: case-insensitive `1`, `true`, `yes`, or +// `on`. Anything else (including missing/empty) is treated as disabled, so +// the safe-by-default behavior is to refuse. +func AcceptScaffoldKeyGroupEnabled() bool { + raw, ok := os.LookupEnv(AcceptScaffoldKeyGroupEnvVar) + if !ok { + return false + } + + switch strings.ToLower(strings.TrimSpace(raw)) { + case "1", "true", "yes", "on": + return true + default: + return false + } +} diff --git a/pkg/tbtc/signer_material_resolver_build_frost_native_tbtc_signer.go b/pkg/tbtc/signer_material_resolver_build_frost_native_tbtc_signer.go index ef6a07a252..268b53a521 100644 --- a/pkg/tbtc/signer_material_resolver_build_frost_native_tbtc_signer.go +++ b/pkg/tbtc/signer_material_resolver_build_frost_native_tbtc_signer.go @@ -58,6 +58,21 @@ func (btnsmr *buildTaggedNativeSignerMaterialResolver) ResolveSignerMaterial( keyGroupDigest := sha256.Sum256(walletPublicKeyBytes) + // Scaffold-era key-group derivation: the current value identifies + // placeholder material derived from the legacy wallet public-key hash, + // not the output of a real FROST DKG run. Refuse to surface that material + // at all unless the operator has explicitly opted in via + // AcceptScaffoldKeyGroupEnvVar — production deployments must never set + // this. See native_tbtc_signer_material.go for the env-var contract. + if !frostsigning.AcceptScaffoldKeyGroupEnabled() { + return nil, fmt.Errorf( + "refusing to build scaffold-era %q signer material; set %s=true to "+ + "opt in for local/CI use only, never in production", + frostsigning.NativeTBTCSignerKeyGroupSourceLegacyWalletPubKey, + frostsigning.AcceptScaffoldKeyGroupEnvVar, + ) + } + // TODO: Replace this placeholder key-group derivation with Rust DKG output. // The current value identifies scaffold-era material only. payload, err := json.Marshal(frostsigning.NativeTBTCSignerMaterialPayload{ diff --git a/pkg/tbtc/signer_material_resolver_build_frost_native_test.go b/pkg/tbtc/signer_material_resolver_build_frost_native_test.go index 45680db2ad..23f4b36b8b 100644 --- a/pkg/tbtc/signer_material_resolver_build_frost_native_test.go +++ b/pkg/tbtc/signer_material_resolver_build_frost_native_test.go @@ -7,6 +7,7 @@ import ( "encoding/hex" "encoding/json" "errors" + "strings" "testing" frostsigning "github.com/keep-network/keep-core/pkg/frost/signing" @@ -16,6 +17,10 @@ import ( func TestRegisterSignerMaterialResolverForBuild_UsesDefaultProvider( t *testing.T, ) { + // Default scaffold-era resolver builds legacy-wallet-pubkey signer + // material; production refuses it but local/CI tests can opt in. + t.Setenv(frostsigning.AcceptScaffoldKeyGroupEnvVar, "true") + UnregisterSignerMaterialResolver() UnregisterSignerMaterialResolverProviderForBuild() t.Cleanup(UnregisterSignerMaterialResolver) @@ -194,3 +199,44 @@ func TestRegisterSignerMaterialResolverForBuild_ProviderReturnsNilResolver( t.Fatal("expected build resolver registration error") } } + +func TestRegisterSignerMaterialResolverForBuild_DefaultProviderRefusesScaffoldWithoutOptIn( + t *testing.T, +) { + // Force the env var to "" so a stray external value cannot suppress the + // scaffold refusal during this regression test. + t.Setenv(frostsigning.AcceptScaffoldKeyGroupEnvVar, "") + + UnregisterSignerMaterialResolver() + UnregisterSignerMaterialResolverProviderForBuild() + t.Cleanup(UnregisterSignerMaterialResolver) + t.Cleanup(UnregisterSignerMaterialResolverProviderForBuild) + + err := RegisterSignerMaterialResolverForBuild() + if err != nil { + t.Fatalf("unexpected build resolver registration error: [%v]", err) + } + + privateKeyShare := createMockSigner(t).privateKeyShare + _, err = resolveSignerMaterial(privateKeyShare) + if err == nil { + t.Fatal( + "expected scaffold-refusal error from default resolver without opt-in", + ) + } + + if !strings.Contains(err.Error(), frostsigning.AcceptScaffoldKeyGroupEnvVar) { + t.Fatalf( + "expected scaffold-refusal error to reference %s; got: [%v]", + frostsigning.AcceptScaffoldKeyGroupEnvVar, + err, + ) + } + if !strings.Contains(err.Error(), frostsigning.NativeTBTCSignerKeyGroupSourceLegacyWalletPubKey) { + t.Fatalf( + "expected scaffold-refusal error to reference %s; got: [%v]", + frostsigning.NativeTBTCSignerKeyGroupSourceLegacyWalletPubKey, + err, + ) + } +} diff --git a/pkg/tbtc/signing_test.go b/pkg/tbtc/signing_test.go index 5505e63c72..9ac73be7ee 100644 --- a/pkg/tbtc/signing_test.go +++ b/pkg/tbtc/signing_test.go @@ -110,6 +110,14 @@ func TestSigningExecutor_SignBatch(t *testing.T) { // setupSigningExecutor sets up an instance of the signing executor ready // to perform test signing. func setupSigningExecutor(t *testing.T) *signingExecutor { + // Tests in this suite exercise the keep-tbtc signing executor against + // in-process tECDSA fixtures. Under the `frost_native frost_tbtc_signer` + // build tags, the signer-material resolver refuses scaffold-era + // (legacy-wallet-pubkey) material by default; the fixtures here are + // inherently scaffold-era so the executor needs the operator opt-in to + // continue running. Production deployments must never set this env var. + t.Setenv("KEEP_CORE_FROST_TBTC_SIGNER_ACCEPT_SCAFFOLD_KEY_GROUP", "true") + groupParameters := &GroupParameters{ GroupSize: 5, GroupQuorum: 4,