Skip to content

Commit 5b0fd65

Browse files
sainoeinsumity
sainoe
and
insumity
authored
fix!: verify the signatures of byzantine validators in misbehaviour handling (#1422)
* update byzantine validators extraction * nits * Update x/ccv/provider/keeper/misbehaviour.go Co-authored-by: insumity <insumity@users.noreply.github.com> * last changes * udpdate changelog --------- Co-authored-by: insumity <insumity@users.noreply.github.com>
1 parent 1d29ee2 commit 5b0fd65

File tree

4 files changed

+191
-78
lines changed

4 files changed

+191
-78
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,7 @@
44

55
Add an entry to the unreleased section whenever merging a PR to main that is not targeted at a specific release. These entries will eventually be included in a release.
66

7-
## v2.3.0-provider-lsm
8-
* (fix!) [#1404](https://github.com/cosmos/interchain-security/pull/1404) Add conditions to the misbehaviour handling ensuring that validators who vote nil
9-
are not getting punished.
7+
* (fix!) [#1422](https://github.com/cosmos/interchain-security/pull/1422) Fix the misbehaviour handling by verifying the signatures of byzantine validators.
108

119
## v2.2.0-provider-lsm
1210

‎tests/integration/misbehaviour.go

Lines changed: 125 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,119 @@ func (s *CCVTestSuite) TestGetByzantineValidators() {
135135
nil,
136136
false,
137137
},
138+
{
139+
"incorrect valset - shouldn't pass",
140+
func() *ibctmtypes.Misbehaviour {
141+
clientHeader := s.consumerChain.CreateTMClientHeader(
142+
s.consumerChain.ChainID,
143+
int64(clientHeight.RevisionHeight+1),
144+
clientHeight,
145+
altTime.Add(time.Minute),
146+
clientTMValset,
147+
clientTMValset,
148+
clientTMValset,
149+
clientSigners,
150+
)
151+
152+
clientHeaderWithCorruptedValset := s.consumerChain.CreateTMClientHeader(
153+
s.consumerChain.ChainID,
154+
int64(clientHeight.RevisionHeight+1),
155+
clientHeight,
156+
altTime.Add(time.Hour),
157+
clientTMValset,
158+
clientTMValset,
159+
clientTMValset,
160+
clientSigners,
161+
)
162+
163+
// change a validator public key in one the second header
164+
testutil.CorruptValidatorPubkeyInHeader(clientHeaderWithCorruptedValset, clientTMValset.Validators[0].Address)
165+
166+
return &ibctmtypes.Misbehaviour{
167+
ClientId: s.path.EndpointA.ClientID,
168+
Header1: clientHeader,
169+
Header2: clientHeaderWithCorruptedValset,
170+
}
171+
},
172+
[]*tmtypes.Validator{},
173+
false,
174+
},
175+
{
176+
"incorrect valset 2 - shouldn't pass",
177+
func() *ibctmtypes.Misbehaviour {
178+
clientHeader := s.consumerChain.CreateTMClientHeader(
179+
s.consumerChain.ChainID,
180+
int64(clientHeight.RevisionHeight+1),
181+
clientHeight,
182+
altTime.Add(time.Minute),
183+
clientTMValset,
184+
clientTMValset,
185+
clientTMValset,
186+
clientSigners,
187+
)
188+
189+
clientHeaderWithCorruptedSigs := s.consumerChain.CreateTMClientHeader(
190+
s.consumerChain.ChainID,
191+
int64(clientHeight.RevisionHeight+1),
192+
clientHeight,
193+
altTime.Add(time.Hour),
194+
clientTMValset,
195+
clientTMValset,
196+
clientTMValset,
197+
clientSigners,
198+
)
199+
200+
// change the valset in the header
201+
vs, _ := altValset.ToProto()
202+
clientHeader.ValidatorSet.Validators = vs.Validators[:3]
203+
clientHeaderWithCorruptedSigs.ValidatorSet.Validators = vs.Validators[:3]
204+
205+
return &ibctmtypes.Misbehaviour{
206+
ClientId: s.path.EndpointA.ClientID,
207+
Header1: clientHeader,
208+
Header2: clientHeaderWithCorruptedSigs,
209+
}
210+
},
211+
[]*tmtypes.Validator{},
212+
false,
213+
},
214+
{
215+
"incorrect signatures - shouldn't pass",
216+
func() *ibctmtypes.Misbehaviour {
217+
clientHeader := s.consumerChain.CreateTMClientHeader(
218+
s.consumerChain.ChainID,
219+
int64(clientHeight.RevisionHeight+1),
220+
clientHeight,
221+
altTime.Add(time.Minute),
222+
clientTMValset,
223+
clientTMValset,
224+
clientTMValset,
225+
clientSigners,
226+
)
227+
228+
clientHeaderWithCorruptedSigs := s.consumerChain.CreateTMClientHeader(
229+
s.consumerChain.ChainID,
230+
int64(clientHeight.RevisionHeight+1),
231+
clientHeight,
232+
altTime.Add(time.Hour),
233+
clientTMValset,
234+
clientTMValset,
235+
clientTMValset,
236+
clientSigners,
237+
)
238+
239+
// change the signature of one of the validator in the header
240+
testutil.CorruptCommitSigsInHeader(clientHeaderWithCorruptedSigs, clientTMValset.Validators[0].Address)
241+
242+
return &ibctmtypes.Misbehaviour{
243+
ClientId: s.path.EndpointA.ClientID,
244+
Header1: clientHeader,
245+
Header2: clientHeaderWithCorruptedSigs,
246+
}
247+
},
248+
[]*tmtypes.Validator{},
249+
false,
250+
},
138251
{
139252
"light client attack - lunatic attack",
140253
func() *ibctmtypes.Misbehaviour {
@@ -212,43 +325,6 @@ func (s *CCVTestSuite) TestGetByzantineValidators() {
212325
[]*tmtypes.Validator{},
213326
true,
214327
},
215-
{
216-
"validators who did vote nil should not be returned",
217-
func() *ibctmtypes.Misbehaviour {
218-
clientHeader := s.consumerChain.CreateTMClientHeader(
219-
s.consumerChain.ChainID,
220-
int64(clientHeight.RevisionHeight+1),
221-
clientHeight,
222-
altTime.Add(time.Minute),
223-
clientTMValset,
224-
clientTMValset,
225-
clientTMValset,
226-
clientSigners,
227-
)
228-
229-
// create conflicting header with 2/4 validators voting nil
230-
clientHeaderWithNilVotes := s.consumerChain.CreateTMClientHeader(
231-
s.consumerChain.ChainID,
232-
int64(clientHeight.RevisionHeight+1),
233-
clientHeight,
234-
altTime.Add(time.Hour),
235-
clientTMValset,
236-
clientTMValset,
237-
clientTMValset,
238-
clientSigners,
239-
)
240-
testutil.UpdateHeaderCommitWithNilVotes(clientHeaderWithNilVotes, clientTMValset.Validators[:2])
241-
242-
return &ibctmtypes.Misbehaviour{
243-
ClientId: s.path.EndpointA.ClientID,
244-
Header1: clientHeader,
245-
Header2: clientHeaderWithNilVotes,
246-
}
247-
},
248-
// Expect validators who did NOT vote nil
249-
clientTMValset.Validators[2:],
250-
true,
251-
},
252328
}
253329

254330
for _, tc := range testCases {
@@ -262,7 +338,7 @@ func (s *CCVTestSuite) TestGetByzantineValidators() {
262338
s.Equal(len(tc.expByzantineValidators), len(byzantineValidators))
263339

264340
// For both lunatic and equivocation attacks, all the validators
265-
// who signed both headers and didn't vote nil should be returned
341+
// who signed both headers
266342
if len(tc.expByzantineValidators) > 0 {
267343
expValset := tmtypes.NewValidatorSet(tc.expByzantineValidators)
268344
s.NoError(err)
@@ -299,10 +375,15 @@ func (s *CCVTestSuite) TestCheckMisbehaviour() {
299375

300376
// create an alternative validator set using more than 1/3 of the trusted validator set
301377
altValset := tmtypes.NewValidatorSet(s.consumerChain.Vals.Validators[0:2])
302-
altSigners := make(map[string]tmtypes.PrivValidator, 1)
378+
altSigners := make(map[string]tmtypes.PrivValidator, 2)
303379
altSigners[clientTMValset.Validators[0].Address.String()] = clientSigners[clientTMValset.Validators[0].Address.String()]
304380
altSigners[clientTMValset.Validators[1].Address.String()] = clientSigners[clientTMValset.Validators[1].Address.String()]
305381

382+
// create an alternative validator set using less than 1/3 of the trusted validator set
383+
altValset2 := tmtypes.NewValidatorSet(s.consumerChain.Vals.Validators[0:1])
384+
altSigners2 := make(map[string]tmtypes.PrivValidator, 1)
385+
altSigners2[clientTMValset.Validators[0].Address.String()] = clientSigners[clientTMValset.Validators[0].Address.String()]
386+
306387
clientHeader := s.consumerChain.CreateTMClientHeader(
307388
s.consumerChain.ChainID,
308389
int64(clientHeight.RevisionHeight+1),
@@ -315,19 +396,17 @@ func (s *CCVTestSuite) TestCheckMisbehaviour() {
315396
)
316397

317398
// create a conflicting client header with insufficient voting power
318-
// by changing 3/4 of its validators votes to nil
319-
clientHeaderWithNilVotes := s.consumerChain.CreateTMClientHeader(
399+
clientHeader2 := s.consumerChain.CreateTMClientHeader(
320400
s.consumerChain.ChainID,
321401
int64(clientHeight.RevisionHeight+1),
322402
clientHeight,
323403
// use a different block time to change the header BlockID
324404
headerTs.Add(time.Hour),
405+
altValset2,
406+
altValset2,
325407
clientTMValset,
326-
clientTMValset,
327-
clientTMValset,
328-
clientSigners,
408+
altSigners2,
329409
)
330-
testutil.UpdateHeaderCommitWithNilVotes(clientHeaderWithNilVotes, clientTMValset.Validators[:4])
331410

332411
testCases := []struct {
333412
name string
@@ -402,7 +481,7 @@ func (s *CCVTestSuite) TestCheckMisbehaviour() {
402481
&ibctmtypes.Misbehaviour{
403482
ClientId: s.path.EndpointA.ClientID,
404483
Header1: clientHeader,
405-
Header2: clientHeaderWithNilVotes,
484+
Header2: clientHeader2,
406485
},
407486
false,
408487
},

‎testutil/crypto/evidence.go

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
package crypto
22

33
import (
4-
"fmt"
54
"time"
65

76
ibctmtypes "github.com/cosmos/ibc-go/v4/modules/light-clients/07-tendermint/types"
87
"github.com/tendermint/tendermint/crypto/tmhash"
8+
"github.com/tendermint/tendermint/libs/bytes"
99
tmtypes "github.com/tendermint/tendermint/types"
1010
)
1111

@@ -92,39 +92,43 @@ func MakeAndSignVoteWithForgedValAddress(
9292
return vote
9393
}
9494

95-
// UpdateHeaderCommitWithNilVotes updates the given light client header
96-
// by changing the commit BlockIDFlag of the given validators to nil
97-
//
95+
// CorruptCommitSigsInHeader corrupts the header by changing the value
96+
// of the commit signature for given validator address.
9897
// Note that this method is solely used for testing purposes
99-
func UpdateHeaderCommitWithNilVotes(header *ibctmtypes.Header, validators []*tmtypes.Validator) {
100-
if len(validators) > len(header.ValidatorSet.Validators) {
101-
panic(fmt.Sprintf("cannot change more than %d validators votes: got %d",
102-
len(header.ValidatorSet.Validators), len(header.ValidatorSet.Validators)))
103-
}
104-
98+
func CorruptCommitSigsInHeader(header *ibctmtypes.Header, valAddress bytes.HexBytes) {
10599
commit, err := tmtypes.CommitFromProto(header.Commit)
106100
if err != nil {
107101
panic(err)
108102
}
109103

104+
for idx, sig := range commit.Signatures {
105+
if sig.ValidatorAddress.String() == valAddress.String() {
106+
sig.Signature = []byte("randomsig")
107+
commit.Signatures[idx] = sig
108+
}
109+
}
110+
// update the commit in client the header
111+
header.SignedHeader.Commit = commit.ToProto()
112+
}
113+
114+
// CorruptValidatorPubkeyInHeader corrupts the header by changing the validator pubkey
115+
// of the given validator address in the validator set.
116+
// Note that this method is solely used for testing purposes
117+
func CorruptValidatorPubkeyInHeader(header *ibctmtypes.Header, valAddress bytes.HexBytes) {
110118
valset, err := tmtypes.ValidatorSetFromProto(header.ValidatorSet)
111119
if err != nil {
112120
panic(err)
113121
}
114122

115-
for _, v := range validators {
116-
// get validator index in valset
117-
idx, _ := valset.GetByAddress(v.Address)
118-
if idx != -1 {
119-
// get validator commit sig
120-
s := commit.Signatures[idx]
121-
// change BlockIDFlag to nil
122-
s.BlockIDFlag = tmtypes.BlockIDFlagNil
123-
// update the signatures
124-
commit.Signatures[idx] = s
123+
for _, v := range valset.Validators {
124+
if v.Address.String() == valAddress.String() {
125+
v.PubKey = tmtypes.NewMockPV().PrivKey.PubKey()
125126
}
126127
}
127128

128-
// update the commit in client the header
129-
header.SignedHeader.Commit = commit.ToProto()
129+
vs, err := valset.ToProto()
130+
if err != nil {
131+
panic(err)
132+
}
133+
header.ValidatorSet = vs
130134
}

‎x/ccv/provider/keeper/misbehaviour.go

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -101,20 +101,28 @@ func (k Keeper) GetByzantineValidators(ctx sdk.Context, misbehaviour ibctmtypes.
101101
// and return the intersection of validators who signed both
102102

103103
// create a map with the validators' address that signed header1
104-
header1Signers := map[string]struct{}{}
105-
for _, sign := range lightBlock1.Commit.Signatures {
106-
if !sign.ForBlock() {
104+
header1Signers := map[string]int{}
105+
for idx, sign := range lightBlock1.Commit.Signatures {
106+
if sign.Absent() {
107107
continue
108108
}
109-
header1Signers[sign.ValidatorAddress.String()] = struct{}{}
109+
header1Signers[sign.ValidatorAddress.String()] = idx
110110
}
111111

112112
// iterate over the header2 signers and check if they signed header1
113-
for _, sign := range lightBlock2.Commit.Signatures {
114-
if !sign.ForBlock() {
113+
for sigIdxHeader2, sign := range lightBlock2.Commit.Signatures {
114+
if sign.Absent() {
115115
continue
116116
}
117-
if _, ok := header1Signers[sign.ValidatorAddress.String()]; ok {
117+
if sigIdxHeader1, ok := header1Signers[sign.ValidatorAddress.String()]; ok {
118+
if err := verifyLightBlockCommitSig(*lightBlock1, sigIdxHeader1); err != nil {
119+
return nil, err
120+
}
121+
122+
if err := verifyLightBlockCommitSig(*lightBlock2, sigIdxHeader2); err != nil {
123+
return nil, err
124+
}
125+
118126
_, val := lightBlock1.ValidatorSet.GetByAddress(sign.ValidatorAddress)
119127
validators = append(validators, val)
120128
}
@@ -192,3 +200,27 @@ func headersStateTransitionsAreConflicting(h1, h2 tmtypes.Header) bool {
192200
!bytes.Equal(h1.AppHash, h2.AppHash) ||
193201
!bytes.Equal(h1.LastResultsHash, h2.LastResultsHash)
194202
}
203+
204+
func verifyLightBlockCommitSig(lightBlock tmtypes.LightBlock, sigIdx int) error {
205+
// get signature
206+
sig := lightBlock.Commit.Signatures[sigIdx]
207+
208+
// get validator
209+
idx, val := lightBlock.ValidatorSet.GetByAddress(sig.ValidatorAddress)
210+
if idx == -1 {
211+
return fmt.Errorf("incorrect signature: validator address %s isn't part of the validator set", sig.ValidatorAddress.String())
212+
}
213+
214+
// verify validator pubkey corresponds to signature validator address
215+
if !bytes.Equal(val.PubKey.Address(), sig.ValidatorAddress) {
216+
return fmt.Errorf("validator public key doesn't correspond to signature validator address: %s!= %s", val.PubKey.Address(), sig.ValidatorAddress)
217+
}
218+
219+
// validate signature
220+
voteSignBytes := lightBlock.Commit.VoteSignBytes(lightBlock.ChainID, int32(sigIdx))
221+
if !val.PubKey.VerifySignature(voteSignBytes, sig.Signature) {
222+
return fmt.Errorf("wrong signature (#%d): %X", sigIdx, sig.Signature)
223+
}
224+
225+
return nil
226+
}

0 commit comments

Comments
 (0)