Skip to content

Commit 7ad6b0b

Browse files
committed
fix(reshare): source-version agreement to prevent master-secret corruption
PR #110 agreed on the dealer set WITHIN a round but not on the source version dealers deal FROM ACROSS rounds. When one operator aborts a round (via #110's abort-retry) while others complete, the cluster splits: the aborting node stays on version N-1, the rest advance to N. The next round then interpolates over dealer polynomials anchored on two different source polynomials, silently reconstructing a different secret S'' != S. The served MPK still maps to S, so every decrypt fails 'all combinations exhausted'. Reproduced live on kms-preprod-sepolia (v0.3.3_fd99a16). Three layers (docs/012): - Layer 1: ValidateReshareMasterPublicKey recomputes the group public key from the agreed dealers' commitments and requires it to equal the carried-forward MPK before persisting; abort-retry on mismatch. Corruption is now impossible (implements the docs/011 step-5 'validate before commit' that was never built). - Layer 2: dealers advertise CommitmentMessage.SourceVersion; SelectMajoritySourceVersion keeps only dealers on the majority source version and drops laggards (which resync as recipients). Ties and sub-threshold majorities abort. Adds keystore.GetPrivateShareForVersion (exact-match, errors on absence -- never a nearest-match). - Layer 3a: generated shares are retained at the node level (bounded to the last 4 rounds, memory-only) past session teardown; the share-fetch handler falls back to them, removing the 503 that triggered the lag. Restart degrades to abort-retry, never corruption. Regression coverage: cross-round mixed-source reproduction proving unfiltered reconstructs S'' != S while filtering preserves S, plus MPK validation, source-version selection, exact-version accessor, and share retention unit tests. Existing dealer-agreement integration tests unchanged.
1 parent fe29adc commit 7ad6b0b

11 files changed

Lines changed: 1089 additions & 14 deletions

File tree

docs/012_reshareSourceVersionAgreement.md

Lines changed: 362 additions & 0 deletions
Large diffs are not rendered by default.

pkg/keystore/keystore.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,28 @@ func (ks *KeyStore) ClearPendingVersion() {
113113
ks.pendingVersion = nil
114114
}
115115

116+
// GetPrivateShareForVersion returns a copy of the private share for the EXACT version.
117+
//
118+
// Unlike GetKeyVersionAtTime, this does NOT fall back to a nearest/earlier version: it
119+
// errors if the exact version is absent. Reshare source-version agreement (docs/012)
120+
// depends on this — a lagging node that asked for the quorum's version and silently got
121+
// its own stale version back would deal from a mismatched-source polynomial and re-corrupt
122+
// the master secret. Callers must treat the error as "I must catch up, not deal."
123+
func (ks *KeyStore) GetPrivateShareForVersion(version int64) (*fr.Element, error) {
124+
ks.mu.RLock()
125+
defer ks.mu.RUnlock()
126+
127+
for _, v := range ks.keyVersions {
128+
if v.Version == version {
129+
if v.PrivateShare == nil {
130+
return nil, fmt.Errorf("key version %d has no private share", version)
131+
}
132+
return new(fr.Element).Set(v.PrivateShare), nil
133+
}
134+
}
135+
return nil, fmt.Errorf("no key version %d in keystore", version)
136+
}
137+
116138
// GetKeyVersionAtTime returns the key version that was active at the given timestamp.
117139
// It returns the latest version whose Version (block timestamp) is <= the given timestamp.
118140
func (ks *KeyStore) GetKeyVersionAtTime(timestamp int64) *types.KeyShareVersion {

pkg/keystore/keystore_test.go

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,3 +85,64 @@ func TestGetKeyVersionAtTime(t *testing.T) {
8585
}
8686
})
8787
}
88+
89+
// TestGetPrivateShareForVersion covers the exact-match version accessor added for
90+
// reshare source-version agreement (docs/012). Unlike GetKeyVersionAtTime, it MUST
91+
// return an error on absence and MUST NOT fall back to a nearest/earlier version — a
92+
// nearest-match would let a lagging node deal from a stale share and reintroduce the
93+
// master-secret corruption this accessor exists to prevent.
94+
func TestGetPrivateShareForVersion(t *testing.T) {
95+
t.Run("returns the share for an exact version match", func(t *testing.T) {
96+
ks := NewKeyStore()
97+
ks.AddVersion(makeVersion(1_700_000_100))
98+
ks.AddVersion(makeVersion(1_700_000_200))
99+
100+
got, err := ks.GetPrivateShareForVersion(1_700_000_200)
101+
if err != nil {
102+
t.Fatalf("unexpected error: %v", err)
103+
}
104+
want := new(fr.Element).SetInt64(1_700_000_200)
105+
if !got.Equal(want) {
106+
t.Fatalf("wrong share returned for version 1_700_000_200")
107+
}
108+
})
109+
110+
t.Run("errors when the version is absent (no nearest-match fallback)", func(t *testing.T) {
111+
ks := NewKeyStore()
112+
ks.AddVersion(makeVersion(1_700_000_100))
113+
ks.AddVersion(makeVersion(1_700_000_300))
114+
115+
// 1_700_000_200 does not exist. GetKeyVersionAtTime would return the
116+
// 1_700_000_100 version here; this accessor must NOT — it must error.
117+
if _, err := ks.GetPrivateShareForVersion(1_700_000_200); err == nil {
118+
t.Fatal("expected an error for an absent version, got nil (nearest-match fallback would reintroduce the corruption bug)")
119+
}
120+
})
121+
122+
t.Run("errors on empty keystore", func(t *testing.T) {
123+
ks := NewKeyStore()
124+
if _, err := ks.GetPrivateShareForVersion(1_700_000_100); err == nil {
125+
t.Fatal("expected an error on empty keystore, got nil")
126+
}
127+
})
128+
129+
t.Run("returns a copy, not the stored element", func(t *testing.T) {
130+
ks := NewKeyStore()
131+
ks.AddVersion(makeVersion(1_700_000_100))
132+
133+
got, err := ks.GetPrivateShareForVersion(1_700_000_100)
134+
if err != nil {
135+
t.Fatalf("unexpected error: %v", err)
136+
}
137+
// Mutating the returned element must not corrupt the stored share.
138+
got.SetInt64(42)
139+
again, err := ks.GetPrivateShareForVersion(1_700_000_100)
140+
if err != nil {
141+
t.Fatalf("unexpected error: %v", err)
142+
}
143+
want := new(fr.Element).SetInt64(1_700_000_100)
144+
if !again.Equal(want) {
145+
t.Fatal("stored share was mutated through the returned element; accessor must return a copy")
146+
}
147+
})
148+
}

pkg/node/handlers.go

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/Layr-Labs/eigenx-kms-go/pkg/attestation"
1212
"github.com/Layr-Labs/eigenx-kms-go/pkg/peering"
1313
"github.com/Layr-Labs/eigenx-kms-go/pkg/types"
14+
"github.com/consensys/gnark-crypto/ecc/bls12-381/fr"
1415
"github.com/ethereum/go-ethereum/common"
1516
ethcrypto "github.com/ethereum/go-ethereum/crypto"
1617
)
@@ -592,6 +593,9 @@ func (s *Server) handleReshareCommitment(w http.ResponseWriter, r *http.Request)
592593
http.Error(w, err.Error(), http.StatusBadRequest)
593594
return
594595
}
596+
// Record the source version this dealer dealt from, so finalize can drop dealers on a
597+
// stale source version (docs/012 Layer 2).
598+
session.SetSourceVersion(senderAddr, commitMsg.SourceVersion)
595599

596600
s.node.logger.Sugar().Debugw("Received reshare commitments",
597601
"operator_address", s.node.OperatorAddress.Hex(),
@@ -681,19 +685,24 @@ func (s *Server) handleReshareShareRequest(w http.ResponseWriter, r *http.Reques
681685
return
682686
}
683687

684-
// Tolerate slight delivery-ordering skew (matches the push /reshare/share handler),
685-
// in case a request arrives on a node that hasn't created the session yet.
686-
session := s.node.waitForSession(reqMsg.SessionTimestamp, 5*time.Second)
687-
if session == nil {
688-
http.Error(w, "Session not found", http.StatusServiceUnavailable)
689-
return
690-
}
691-
692688
// Serve only the requester's own share (the authenticated sender), never another
693689
// operator's. The requester address is the authenticated identity, not a field the
694690
// caller can spoof.
695691
requester := senderPeer.OperatorAddress
696-
share := session.GetMyGeneratedShareFor(requester)
692+
693+
// Resolve the share the requester is missing. Prefer a live session (tolerating slight
694+
// delivery-ordering skew, in case the request arrives before this node created the
695+
// session), but fall back to the node-level retained store: the common case in the
696+
// live incident is a peer that lagged and asks for our share AFTER we already finished
697+
// the round and tore down our session. Retained shares (docs/012 Layer 3a) let that
698+
// fetch succeed instead of 503-ing the peer into a corrupting version split.
699+
var share *fr.Element
700+
if session := s.node.waitForSession(reqMsg.SessionTimestamp, 5*time.Second); session != nil {
701+
share = session.GetMyGeneratedShareFor(requester)
702+
}
703+
if share == nil {
704+
share = s.node.getRetainedGeneratedShare(reqMsg.SessionTimestamp, requester)
705+
}
697706
if share == nil {
698707
s.node.logger.Sugar().Warnw("No generated share to serve for requester",
699708
"operator_address", s.node.OperatorAddress.Hex(),

pkg/node/node.go

Lines changed: 161 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,17 @@ type Node struct {
7272
sessionNotify map[int64]chan struct{} // Notifies when session is created
7373
sessionNotifyLock sync.Mutex
7474

75+
// retainedGeneratedShares holds the per-recipient reshare shares this node dealt,
76+
// keyed by session timestamp, kept PAST session teardown so a lagging peer can still
77+
// fetch the share it missed (docs/012 Layer 3a). Without this, a dealer that finished
78+
// a round and cleaned up its session would 503 the fetch, the peer would abort and
79+
// fall a version behind, and the next round would corrupt the master secret. Bounded
80+
// to the last retainedShareRounds sessions to cap memory (in-memory only — a restart
81+
// drops it, which degrades to the pre-existing abort-and-retry, never to corruption).
82+
retainedGeneratedShares map[int64]map[common.Address]*fr.Element
83+
retainedGeneratedShareOrder []int64
84+
retainedSharesMutex sync.RWMutex
85+
7586
// Scheduling
7687
enableAutoReshare bool
7788
lastProcessedBoundary int64
@@ -107,6 +118,12 @@ type ProtocolSession struct {
107118
commitments map[common.Address][]types.G2Point
108119
acks map[common.Address]map[common.Address]*types.Acknowledgement
109120

121+
// sourceVersions records, per reshare dealer, the key version it dealt FROM (carried
122+
// in its commitment broadcast). Used at finalize to drop dealers on a stale source
123+
// version so the refreshed shares all descend from one polynomial (docs/012 Layer 2).
124+
// Empty for DKG (no source version).
125+
sourceVersions map[common.Address]int64
126+
110127
// myGeneratedShares retains the per-recipient shares THIS node generated as a
111128
// dealer (recipient address -> share). Unlike `shares` (which holds shares this
112129
// node RECEIVED from others), this is what we DEALT, kept so we can answer an
@@ -161,6 +178,97 @@ func (s *ProtocolSession) GetMyGeneratedShareFor(recipient common.Address) *fr.E
161178
return new(fr.Element).Set(sh)
162179
}
163180

181+
// retainedShareRounds bounds how many recent reshare rounds' generated shares are kept
182+
// for on-demand fetch after session teardown (docs/012 Layer 3a). At the ~2-minute
183+
// reshare cadence this covers several minutes of catch-up window, ample for the
184+
// second-scale receipt skew that triggered the live incident, while capping memory.
185+
const retainedShareRounds = 4
186+
187+
// retainGeneratedShares stores (a copy of) the per-recipient shares this node dealt for
188+
// the given session so they can be served after the session is torn down. Bounded to the
189+
// most recent retainedShareRounds sessions; the oldest is evicted first.
190+
func (n *Node) retainGeneratedShares(sessionTimestamp int64, shares map[common.Address]*fr.Element) {
191+
n.retainedSharesMutex.Lock()
192+
defer n.retainedSharesMutex.Unlock()
193+
194+
if n.retainedGeneratedShares == nil {
195+
n.retainedGeneratedShares = make(map[int64]map[common.Address]*fr.Element)
196+
}
197+
198+
if _, exists := n.retainedGeneratedShares[sessionTimestamp]; !exists {
199+
n.retainedGeneratedShareOrder = append(n.retainedGeneratedShareOrder, sessionTimestamp)
200+
}
201+
202+
cp := make(map[common.Address]*fr.Element, len(shares))
203+
for addr, sh := range shares {
204+
// Deep-copy: the caller retains the source map (GenerateNewShares' return value),
205+
// so storing the pointer would alias cryptographic material.
206+
cp[addr] = new(fr.Element).Set(sh)
207+
}
208+
n.retainedGeneratedShares[sessionTimestamp] = cp
209+
210+
// Evict oldest beyond the bound.
211+
for len(n.retainedGeneratedShareOrder) > retainedShareRounds {
212+
oldest := n.retainedGeneratedShareOrder[0]
213+
n.retainedGeneratedShareOrder = n.retainedGeneratedShareOrder[1:]
214+
delete(n.retainedGeneratedShares, oldest)
215+
}
216+
}
217+
218+
// getRetainedGeneratedShare returns a copy of the share this node dealt to recipient for
219+
// the given session, or nil if not retained (unknown session or recipient).
220+
func (n *Node) getRetainedGeneratedShare(sessionTimestamp int64, recipient common.Address) *fr.Element {
221+
n.retainedSharesMutex.RLock()
222+
defer n.retainedSharesMutex.RUnlock()
223+
224+
byRecipient, ok := n.retainedGeneratedShares[sessionTimestamp]
225+
if !ok {
226+
return nil
227+
}
228+
sh, ok := byRecipient[recipient]
229+
if !ok || sh == nil {
230+
return nil
231+
}
232+
return new(fr.Element).Set(sh)
233+
}
234+
235+
// SetSourceVersion records the source key version a reshare dealer dealt from (from its
236+
// commitment broadcast). See docs/012 Layer 2.
237+
func (s *ProtocolSession) SetSourceVersion(dealer common.Address, version int64) {
238+
s.mu.Lock()
239+
defer s.mu.Unlock()
240+
if s.sourceVersions == nil {
241+
s.sourceVersions = make(map[common.Address]int64)
242+
}
243+
s.sourceVersions[dealer] = version
244+
}
245+
246+
// GetSourceVersions returns a copy of the per-dealer source versions recorded this session.
247+
func (s *ProtocolSession) GetSourceVersions() map[common.Address]int64 {
248+
s.mu.RLock()
249+
defer s.mu.RUnlock()
250+
out := make(map[common.Address]int64, len(s.sourceVersions))
251+
for k, v := range s.sourceVersions {
252+
out[k] = v
253+
}
254+
return out
255+
}
256+
257+
// GetCommitmentsFor returns a copy of the polynomial commitments this session received
258+
// from the given dealer, or nil if none. Used by the post-reshare MPK validation to
259+
// recompute the group public key from the agreed dealers' commitments (docs/012 Layer 1).
260+
func (s *ProtocolSession) GetCommitmentsFor(dealer common.Address) []types.G2Point {
261+
s.mu.RLock()
262+
defer s.mu.RUnlock()
263+
c, ok := s.commitments[dealer]
264+
if !ok {
265+
return nil
266+
}
267+
out := make([]types.G2Point, len(c))
268+
copy(out, c)
269+
return out
270+
}
271+
164272
// HandleReceivedShare stores a share and signals completion if all shares received
165273
// Returns error if duplicate share detected
166274
func (s *ProtocolSession) HandleReceivedShare(sender common.Address, share *fr.Element) error {
@@ -1045,6 +1153,7 @@ func (n *Node) createSession(sessionType string, operators []*peering.OperatorSe
10451153
shares: make(map[common.Address]*fr.Element),
10461154
commitments: make(map[common.Address][]types.G2Point),
10471155
acks: make(map[common.Address]map[common.Address]*types.Acknowledgement),
1156+
sourceVersions: make(map[common.Address]int64),
10481157
sharesCompleteChan: make(chan bool, 1),
10491158
commitmentsCompleteChan: make(chan bool, 1),
10501159
acksCompleteChan: make(chan bool, 1),
@@ -1554,11 +1663,17 @@ func (n *Node) RunReshareAsExistingOperator(sessionTimestamp int64, triggerBlock
15541663
// Create reshare instance with current operators
15551664
n.resharer = reshare.NewReshare(n.OperatorAddress, operators)
15561665

1557-
// Get current share
1666+
// Get current share and the version we are dealing FROM. All finalized dealers must
1667+
// deal from the same source version or the refreshed shares won't descend from one
1668+
// polynomial (docs/012 Layer 2); we advertise this version in our commitment broadcast.
15581669
currentShare, err := n.keyStore.GetActivePrivateShare()
15591670
if err != nil {
15601671
return err
15611672
}
1673+
var sourceVersion int64
1674+
if activeVersion := n.keyStore.GetActiveVersion(); activeVersion != nil {
1675+
sourceVersion = activeVersion.Version
1676+
}
15621677

15631678
// Phase 1: Generate dealer polynomials anchored at each dealer's current share.
15641679
// Each dealer i samples f_i with f_i(0)=x_i and broadcasts commitments + per-recipient shares.
@@ -1590,16 +1705,22 @@ func (n *Node) RunReshareAsExistingOperator(sessionTimestamp int64, triggerBlock
15901705
// "dealer commitments unavailable".
15911706
_ = session.HandleReceivedShare(n.OperatorAddress, shares[n.OperatorAddress])
15921707
_ = session.HandleReceivedCommitment(n.OperatorAddress, commitments)
1708+
session.SetSourceVersion(n.OperatorAddress, sourceVersion)
15931709

1594-
// Broadcast commitments
1595-
if err := n.transport.BroadcastReshareCommitments(operators, commitments, session.SessionTimestamp); err != nil {
1710+
// Broadcast commitments (advertising the source version we dealt from)
1711+
if err := n.transport.BroadcastReshareCommitments(operators, commitments, session.SessionTimestamp, sourceVersion); err != nil {
15961712
n.logger.Sugar().Errorw("Failed to broadcast reshare commitments", "operator_address", n.OperatorAddress.Hex(), "error", err)
15971713
// Continue anyway - other nodes may have received
15981714
}
15991715

16001716
// Retain the shares we generated as a dealer so we can re-serve any of them to a
16011717
// peer that missed our original send (see on-demand share fetch during finalize).
16021718
session.SetMyGeneratedShares(shares)
1719+
// Also retain at the node level, keyed by session, so we can still serve an on-demand
1720+
// fetch AFTER this session is torn down on completion (docs/012 Layer 3a). This is the
1721+
// fix for the live incident's 503 trigger: a lagging peer fetching our share after we
1722+
// finished the round must succeed, not abort.
1723+
n.retainGeneratedShares(session.SessionTimestamp, shares)
16031724

16041725
// Send shares to all operators
16051726
for _, op := range operators {
@@ -1921,6 +2042,22 @@ func (n *Node) RunReshareAsExistingOperator(sessionTimestamp int64, triggerBlock
19212042
len(agreedDealers), newThreshold)
19222043
}
19232044

2045+
// SOURCE-VERSION AGREEMENT (docs/012 Layer 2). #110 agrees on WHICH dealers deal, but
2046+
// not on WHICH source version they deal FROM. A dealer that lagged a prior round deals
2047+
// from a stale share; mixing it in shifts the reconstructed secret. Keep only dealers on
2048+
// the majority source version (deterministic across nodes — all see the same broadcasts)
2049+
// and drop laggards. An excluded dealer still recomputes its own refreshed share as a
2050+
// recipient of the kept dealers, resyncing implicitly. If the majority is below
2051+
// threshold or ambiguous, abort-retry (Layer 1's MPK check is the ultimate backstop).
2052+
agreedDealers, srcVersion, err := reshare.SelectMajoritySourceVersion(
2053+
agreedDealers, session.GetSourceVersions(), newThreshold)
2054+
if err != nil {
2055+
n.logger.Sugar().Warnw("Aborting reshare finalize: no source-version-agreed dealer set",
2056+
"operator_address", n.OperatorAddress.Hex(),
2057+
"error", err)
2058+
return fmt.Errorf("reshare aborted: %w; will retry next interval", err)
2059+
}
2060+
19242061
// Ensure we hold a verified share from every dealer in D. For any we are missing
19252062
// (we were lagging / dropped that send), fetch it on demand from the dealer and
19262063
// verify it with the same polynomial-commitment check as the push path. If we still
@@ -1952,6 +2089,7 @@ func (n *Node) RunReshareAsExistingOperator(sessionTimestamp int64, triggerBlock
19522089
n.logger.Sugar().Infow("Finalizing reshare on agreed dealer set",
19532090
"operator_address", n.OperatorAddress.Hex(),
19542091
"agreed_dealers", len(participantIDsForFinalize),
2092+
"source_version", srcVersion,
19552093
"pinned_block", session.TriggerBlockNumber)
19562094

19572095
// Compute refreshed share using the same Lagrange reconstruction as the new-operator path.
@@ -1970,6 +2108,26 @@ func (n *Node) RunReshareAsExistingOperator(sessionTimestamp int64, triggerBlock
19702108
if currentVersion := n.keyStore.GetActiveVersion(); currentVersion != nil && currentVersion.MasterPublicKey != nil {
19712109
mpkCopy := *currentVersion.MasterPublicKey
19722110
newKeyVersion.MasterPublicKey = &mpkCopy
2111+
2112+
// VALIDATE BEFORE COMMIT (docs/011 § step 5, docs/012 Layer 1). Recompute the
2113+
// group public key implied by the agreed dealers' commitments and require it to
2114+
// equal the carried-forward MPK. If any dealer dealt from a mismatched source
2115+
// share (a cross-round version split) or the dealer sets diverged, the refreshed
2116+
// shares would not reconstruct the served MPK — decrypt would fail cluster-wide
2117+
// with "all combinations exhausted" and the corruption would be permanent. Abort
2118+
// loudly and retry next interval instead of persisting a poisoned share.
2119+
commitmentsByDealer := make(map[common.Address][]types.G2Point, len(participantIDsForFinalize))
2120+
for _, dealer := range participantIDsForFinalize {
2121+
commitmentsByDealer[dealer] = session.GetCommitmentsFor(dealer)
2122+
}
2123+
if verr := reshare.ValidateReshareMasterPublicKey(participantIDsForFinalize, commitmentsByDealer, &mpkCopy); verr != nil {
2124+
n.logger.Sugar().Errorw("ABORTING reshare finalize: post-reshare MPK validation failed",
2125+
"operator_address", n.OperatorAddress.Hex(),
2126+
"agreed_dealers", len(participantIDsForFinalize),
2127+
"new_version", newKeyVersion.Version,
2128+
"error", verr)
2129+
return fmt.Errorf("reshare aborted: post-reshare MPK validation failed: %w; will retry next interval", verr)
2130+
}
19732131
}
19742132

19752133
// Persist new key version BEFORE adding to keystore

0 commit comments

Comments
 (0)