TSS keysign security: IsNodeKeys excludes retiring vault members & blame.Round not validated
Two security issues identified during review of !4654 (also applicable to `huginn/batch-2vdmp`): ## 1. `IsNodeKeys` excludes retiring vault members from `LastRoundCount` `handler_tss_keysign.go` — `IsNodeKeys` filters by `NodeStatus_Active`: ```go // type_node_account.go:210 func (nas NodeAccounts) IsNodeKeys(addr cosmos.AccAddress) bool { for _, na := range nas { if na.Status == NodeStatus_Active && addr.Equals(na.NodeAddress) { ``` During churn, vault members transition to `Retiring`. The validate function explicitly accepts keysign fail messages from retiring vault members (via the retiring-vault membership check), but their round-7 reports never increment `LastRoundCount` because `IsNodeKeys` returns false for non-Active nodes. This means during churn — when vaults are most vulnerable — the freeze heuristic is weakened. **Fix:** Check vault membership directly instead of active status. `vaultMemberNodes` already contains all vault members regardless of status: ```diff - if vaultMemberNodes.IsNodeKeys(msg.Signer) { - voter.LastRoundCount++ + for _, na := range vaultMemberNodes { + if msg.Signer.Equals(na.NodeAddress) { + voter.LastRoundCount++ + break + } } ``` ## 2. `blame.Round` not validated — enables voter ID pollution `blame.Round` is now included in the keysign fail ID hash (`msg_tss_sign_fail.go:49`), but no validation restricts it to known TSS round constants. A node can submit a keysign fail message with an arbitrary round string (e.g. `"FakeRound7"`), creating a separate voter that dilutes the genuine consensus. The cost is only one cycle's observe slash points. **Fix:** Add a versioned validate that rejects unknown round values: ```go var validTssKeysignRounds = map[string]bool{ tssMessages.KEYSIGN1aUnicast: true, tssMessages.KEYSIGN1b: true, tssMessages.KEYSIGN2Unicast: true, tssMessages.KEYSIGN3: true, tssMessages.KEYSIGN4: true, tssMessages.KEYSIGN5: true, tssMessages.KEYSIGN6: true, tssMessages.KEYSIGN7: true, tssMessages.EDDSAKEYSIGN1: true, tssMessages.EDDSAKEYSIGN2: true, tssMessages.EDDSAKEYSIGN3: true, } // In validate (or versioned equivalent): if msg.Blame.Round != "" && !validTssKeysignRounds[msg.Blame.Round] { return cosmos.ErrUnknownRequest("invalid blame round: " + msg.Blame.Round) } ``` Empty round is allowed since the TSS blame manager may report failures before reaching a specific protocol round. --- **Source:** [Comment by @ahdzib on !4654](https://gitlab.com/thorchain/thornode/-/merge_requests/4654#note_3175336760)
issue