Trail of Bits tBTC v2 AC Extension — security remediation

cmd/flags.go

@@ -344,6 +344,12 @@ func initCovenantSignerFlags(cmd *cobra.Command, cfg *config.Config) {
 		false,
 		"Fail startup when enabled covenant routes are missing route-level approval trust roots. Request-time validation still enforces exact reserve/network trust-root matches.",
 	)
+	cmd.Flags().BoolVar(
+		&cfg.CovenantSigner.BridgeCovenantFraudDefenseConfirmed,
+		"covenantSigner.bridgeCovenantFraudDefenseConfirmed",
+		false,
+		"Set only after confirming the tBTC Bridge covenant fraud-defense path is deployed. Until set, the covenant signer fails closed and refuses to sign, because a covenant signature would otherwise expose the wallet to an undefeatable fraud challenge.",
+	)
 }
 
 // Initialize flags for Maintainer configuration.

cmd/flags_test.go

@@ -226,6 +226,13 @@ var cmdFlagsTests = map[string]struct {
 		expectedValueFromFlag: true,
 		defaultValue:          false,
 	},
+	"covenantSigner.bridgeCovenantFraudDefenseConfirmed": {
+		readValueFunc:         func(c *config.Config) interface{} { return c.CovenantSigner.BridgeCovenantFraudDefenseConfirmed },
+		flagName:              "--covenantSigner.bridgeCovenantFraudDefenseConfirmed",
+		flagValue:             "",
+		expectedValueFromFlag: true,
+		defaultValue:          false,
+	},
 	"tbtc.preParamsPoolSize": {
 		readValueFunc:         func(c *config.Config) interface{} { return c.Tbtc.PreParamsPoolSize },
 		flagName:              "--tbtc.preParamsPoolSize",

cmd/start.go

@@ -100,6 +100,7 @@ type startDeps struct {
 		clientInfoRegistry *clientinfo.Registry,
 		perfMetrics *clientinfo.PerformanceMetrics,
 		minActiveOutpointConfirmations uint,
+		bridgeCovenantFraudDefenseConfirmed bool,
 	) (covenantsigner.Engine, error)
 	initializeSigner func(
 		ctx context.Context,
@@ -254,6 +255,7 @@ func startWithDeps(cmd *cobra.Command, deps startDeps) error {
 			clientInfoRegistry,
 			perfMetrics, // Pass the existing performance metrics instance to avoid duplicate registrations
 			clientConfig.CovenantSigner.MinActiveOutpointConfirmations,
+			clientConfig.CovenantSigner.BridgeCovenantFraudDefenseConfirmed,
 		)
 		if err != nil {
 			return fmt.Errorf("error initializing TBTC: [%v]", err)

pkg/chain/ethereum/tbtc.go

@@ -1251,6 +1251,32 @@ func (tc *TbtcChain) PastDepositRevealedEvents(
 	return convertedEvents, err
 }
 
+// convertPastRedemptionRequestedEvent maps a single on-chain RedemptionRequested
+// event to its off-chain tbtc.RedemptionRequestedEvent representation. The event
+// carries two distinct fee fields, TreasuryFee and TxMaxFee, that must be mapped
+// to their matching destination fields independently; keeping the mapping in a
+// standalone helper lets that field-by-field correspondence be unit tested.
+func convertPastRedemptionRequestedEvent(
+	event *tbtcabi.BridgeRedemptionRequested,
+) (*tbtc.RedemptionRequestedEvent, error) {
+	redeemerOutputScript, err := bitcoin.NewScriptFromVarLenData(
+		event.RedeemerOutputScript,
+	)
+	if err != nil {
+		return nil, err
+	}
+
+	return &tbtc.RedemptionRequestedEvent{
+		WalletPublicKeyHash:  event.WalletPubKeyHash,
+		RedeemerOutputScript: redeemerOutputScript,
+		Redeemer:             chain.Address(event.Redeemer.Hex()),
+		RequestedAmount:      event.RequestedAmount,
+		TreasuryFee:          event.TreasuryFee,
+		TxMaxFee:             event.TxMaxFee,
+		BlockNumber:          event.Raw.BlockNumber,
+	}, nil
+}
+
 func (tc *TbtcChain) PastRedemptionRequestedEvents(
 	filter *tbtc.RedemptionRequestedEventFilter,
 ) ([]*tbtc.RedemptionRequestedEvent, error) {
@@ -1282,23 +1308,11 @@ func (tc *TbtcChain) PastRedemptionRequestedEvents(
 
 	convertedEvents := make([]*tbtc.RedemptionRequestedEvent, 0)
 	for _, event := range events {
-		redeemerOutputScript, err := bitcoin.NewScriptFromVarLenData(
-			event.RedeemerOutputScript,
-		)
+		convertedEvent, err := convertPastRedemptionRequestedEvent(event)
 		if err != nil {
 			return nil, err
 		}
 
-		convertedEvent := &tbtc.RedemptionRequestedEvent{
-			WalletPublicKeyHash:  event.WalletPubKeyHash,
-			RedeemerOutputScript: redeemerOutputScript,
-			Redeemer:             chain.Address(event.Redeemer.Hex()),
-			RequestedAmount:      event.RequestedAmount,
-			TreasuryFee:          event.TreasuryFee,
-			TxMaxFee:             event.TreasuryFee,
-			BlockNumber:          event.Raw.BlockNumber,
-		}
-
 		convertedEvents = append(convertedEvents, convertedEvent)
 	}
 

pkg/chain/ethereum/tbtc_test.go

@@ -15,6 +15,7 @@ import (
 	"github.com/keep-network/keep-core/pkg/chain"
 
 	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/core/types"
 	commonEthereum "github.com/keep-network/keep-common/pkg/chain/ethereum"
 
 	"github.com/keep-network/keep-core/internal/testutils"
@@ -24,6 +25,91 @@ import (
 	tbtcpkg "github.com/keep-network/keep-core/pkg/tbtc"
 )
 
+func TestConvertPastRedemptionRequestedEvent(t *testing.T) {
+	// RedeemerOutputScript is stored on-chain as variable-length data (a
+	// CompactSizeUint length prefix followed by the script bytes), so build the
+	// fixture from a known script to guarantee the helper can parse it back.
+	redeemerOutputScript := bitcoin.Script([]byte{0x76, 0xa9, 0x14})
+	redeemerOutputScriptVarLen, err := redeemerOutputScript.ToVarLenData()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	walletPublicKeyHash := [20]byte{1, 2, 3}
+	redeemer := common.HexToAddress("0x1111111111111111111111111111111111111111")
+
+	// The treasury fee and the per-transaction max fee are two independent
+	// on-chain fields. They are set to distinct values so that a mapping which
+	// reads the wrong source (e.g. populating TxMaxFee from TreasuryFee) is
+	// caught rather than masked by equal values.
+	const (
+		requestedAmount = uint64(1_000_000)
+		treasuryFee     = uint64(2_000)
+		txMaxFee        = uint64(7_500)
+		blockNumber     = uint64(123_456)
+	)
+
+	event := &tbtcabi.BridgeRedemptionRequested{
+		WalletPubKeyHash:     walletPublicKeyHash,
+		RedeemerOutputScript: redeemerOutputScriptVarLen,
+		Redeemer:             redeemer,
+		RequestedAmount:      requestedAmount,
+		TreasuryFee:          treasuryFee,
+		TxMaxFee:             txMaxFee,
+		Raw:                  types.Log{BlockNumber: blockNumber},
+	}
+
+	converted, err := convertPastRedemptionRequestedEvent(event)
+	if err != nil {
+		t.Fatalf("unexpected error: [%v]", err)
+	}
+
+	testutils.AssertUintsEqual(
+		t,
+		"requested amount",
+		requestedAmount,
+		converted.RequestedAmount,
+	)
+
+	// Both fee fields must be populated from their own distinct source field.
+	testutils.AssertUintsEqual(
+		t,
+		"treasury fee",
+		treasuryFee,
+		converted.TreasuryFee,
+	)
+	testutils.AssertUintsEqual(
+		t,
+		"tx max fee",
+		txMaxFee,
+		converted.TxMaxFee,
+	)
+
+	testutils.AssertUintsEqual(
+		t,
+		"block number",
+		blockNumber,
+		converted.BlockNumber,
+	)
+
+	testutils.AssertBytesEqual(t, redeemerOutputScript, converted.RedeemerOutputScript)
+
+	testutils.AssertStringsEqual(
+		t,
+		"redeemer",
+		chain.Address(redeemer.Hex()).String(),
+		converted.Redeemer.String(),
+	)
+
+	if converted.WalletPublicKeyHash != walletPublicKeyHash {
+		t.Errorf(
+			"unexpected wallet public key hash\nexpected: [%x]\nactual:   [%x]",
+			walletPublicKeyHash,
+			converted.WalletPublicKeyHash,
+		)
+	}
+}
+
 func TestComputeOperatorsIDsHash(t *testing.T) {
 	operatorIDs := []chain.OperatorID{
 		5, 1, 55, 45435534, 33, 345, 23, 235, 3333, 2,

pkg/covenantsigner/config.go

@@ -37,6 +37,14 @@ type Config struct {
 	// covenant signer accepts it. When zero (unset), the system defaults to 6
 	// to align with the deposit sweep finality threshold.
 	MinActiveOutpointConfirmations uint `mapstructure:"minActiveOutpointConfirmations"`
+	// BridgeCovenantFraudDefenseConfirmed must be set only when the operator has
+	// confirmed that the tBTC Bridge recognizes covenant active UTXO spends as
+	// honest spends in Fraud.defeatFraudChallenge (the covenant fraud-defense
+	// path is deployed). Until then the covenant signer fails closed and refuses
+	// to produce signatures, because a covenant signature over a covenant active
+	// UTXO would otherwise be a valid, undefeatable tBTC fraud proof that could
+	// slash the signing wallet.
+	BridgeCovenantFraudDefenseConfirmed bool `mapstructure:"bridgeCovenantFraudDefenseConfirmed"`
 	// DataDir is the base directory path used by the disk persistence handle.
 	// When set, the store acquires an exclusive file lock to prevent concurrent
 	// process corruption. When empty, file locking is skipped.

pkg/covenantsigner/server.go

@@ -47,7 +47,9 @@ func Initialize(
 		listenAddress = DefaultListenAddress
 	}
 
-	if !isLoopbackListenAddress(listenAddress) && strings.TrimSpace(config.AuthToken) == "" {
+	isLoopback := isLoopbackListenAddress(listenAddress)
+
+	if !isLoopback && strings.TrimSpace(config.AuthToken) == "" {
 		return nil, false, fmt.Errorf(
 			"covenant signer authToken is required for non-loopback listenAddress [%s]",
 			listenAddress,
@@ -66,7 +68,12 @@ func Initialize(
 	if err != nil {
 		return nil, false, err
 	}
-	if err := validateRequiredApprovalTrustRoots(config, service); err != nil {
+	// A non-loopback (production) listen address is treated as requiring the
+	// full multi-party approval model: the signer approval verifier and the
+	// route trust roots must be configured, mirroring the non-loopback authToken
+	// requirement above. Loopback deployments may still run with warnings unless
+	// requireApprovalTrustRoots is set explicitly.
+	if err := validateRequiredApprovalTrustRoots(config, service, !isLoopback); err != nil {
 		return nil, false, err
 	}
 	if service.signerApprovalVerifier == nil {
@@ -181,8 +188,9 @@ func Initialize(
 func validateRequiredApprovalTrustRoots(
 	config Config,
 	service *Service,
+	requireForNonLoopbackListenAddress bool,
 ) error {
-	if !config.RequireApprovalTrustRoots {
+	if !config.RequireApprovalTrustRoots && !requireForNonLoopbackListenAddress {
 		return nil
 	}
 
@@ -192,7 +200,7 @@ func validateRequiredApprovalTrustRoots(
 			TemplateSelfV1,
 		) {
 		return fmt.Errorf(
-			"covenant signer self_v1 routes require depositorTrustRoots when covenantSigner.requireApprovalTrustRoots=true",
+			"covenant signer self_v1 routes require depositorTrustRoots when covenantSigner.requireApprovalTrustRoots=true or for a non-loopback listen address",
 		)
 	}
 
@@ -201,7 +209,7 @@ func validateRequiredApprovalTrustRoots(
 		TemplateQcV1,
 	) {
 		return fmt.Errorf(
-			"covenant signer qc_v1 routes require depositorTrustRoots when covenantSigner.requireApprovalTrustRoots=true",
+			"covenant signer qc_v1 routes require depositorTrustRoots when covenantSigner.requireApprovalTrustRoots=true or for a non-loopback listen address",
 		)
 	}
 
@@ -210,13 +218,13 @@ func validateRequiredApprovalTrustRoots(
 		TemplateQcV1,
 	) {
 		return fmt.Errorf(
-			"covenant signer qc_v1 routes require custodianTrustRoots when covenantSigner.requireApprovalTrustRoots=true",
+			"covenant signer qc_v1 routes require custodianTrustRoots when covenantSigner.requireApprovalTrustRoots=true or for a non-loopback listen address",
 		)
 	}
 
 	if service.signerApprovalVerifier == nil {
 		return fmt.Errorf(
-			"covenant signer requires a signerApprovalVerifier when covenantSigner.requireApprovalTrustRoots=true",
+			"covenant signer requires a signerApprovalVerifier when covenantSigner.requireApprovalTrustRoots=true or for a non-loopback listen address",
 		)
 	}
 

pkg/covenantsigner/server_test.go

@@ -415,6 +415,67 @@ func TestInitializeRequiresSignerApprovalVerifierWhenConfigured(t *testing.T) {
 	}
 }
 
+func TestInitializeRequiresTrustRootsForNonLoopbackListenAddress(t *testing.T) {
+	handle := newMemoryHandle()
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	// A non-loopback (production) listen address must fail startup when the
+	// required approval trust roots are missing, even without
+	// RequireApprovalTrustRoots being set explicitly. The engine provides a
+	// verifier, so the failure is attributable to the missing trust roots.
+	_, enabled, err := Initialize(
+		ctx,
+		Config{
+			Port:          availableLoopbackPort(t),
+			ListenAddress: "0.0.0.0",
+			AuthToken:     "test-token",
+		},
+		handle,
+		&scriptedVerifierEngine{},
+	)
+	if err == nil || enabled {
+		t.Fatalf("expected non-loopback startup without trust roots to fail, got enabled=%v err=%v", enabled, err)
+	}
+	if !strings.Contains(err.Error(), "non-loopback listen address") {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}
+
+func TestInitializeRequiresSignerApprovalVerifierForNonLoopbackListenAddress(t *testing.T) {
+	handle := newMemoryHandle()
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	// A non-loopback (production) listen address with all trust roots present
+	// but no signer approval verifier must fail startup.
+	_, enabled, err := Initialize(
+		ctx,
+		Config{
+			Port:          availableLoopbackPort(t),
+			ListenAddress: "0.0.0.0",
+			AuthToken:     "test-token",
+			DepositorTrustRoots: []DepositorTrustRoot{
+				testDepositorTrustRoot(TemplateQcV1),
+			},
+			CustodianTrustRoots: []CustodianTrustRoot{
+				testCustodianTrustRoot(TemplateQcV1),
+			},
+		},
+		handle,
+		&scriptedEngine{},
+	)
+	if err == nil || enabled {
+		t.Fatalf("expected non-loopback startup without a verifier to fail, got enabled=%v err=%v", enabled, err)
+	}
+	if !strings.Contains(err.Error(), "requires a signerApprovalVerifier") {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if !strings.Contains(err.Error(), "non-loopback listen address") {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}
+
 func TestIsLoopbackListenAddressAcceptsBracketedIPv6Loopback(t *testing.T) {
 	if !isLoopbackListenAddress("[::1]") {
 		t.Fatal("expected bracketed IPv6 loopback address to be recognized")