From d53a8d0377fe975063dc76116285c2c47e87d09b Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Thu, 4 Jun 2020 13:45:39 +0200 Subject: [PATCH] light: implement validate basic (#4916) run a validate basic on inbound validator sets and headers before further processing them --- CHANGELOG_PENDING.md | 7 +++-- light/client.go | 67 +++++++++++++++++++++++++++++++---------- light/client_test.go | 71 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 126 insertions(+), 19 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index a7a5cd709..0bfb340f9 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -48,7 +48,7 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi - [statesync] Add state sync support, where a new node can be rapidly bootstrapped by fetching state snapshots from peers instead of replaying blocks. See the `[statesync]` config section. - [evidence] [\#4532](https://github.com/tendermint/tendermint/pull/4532) Handle evidence from light clients (@melekes) -- [lite2] [\#4532](https://github.com/tendermint/tendermint/pull/4532) Submit conflicting headers, if any, to a full node & all witnesses (@melekes) +- [light] [\#4532](https://github.com/tendermint/tendermint/pull/4532) Submit conflicting headers, if any, to a full node & all witnesses (@melekes) - [rpc] [\#4532](https://github.com/tendermint/tendermint/pull/4923) Support `BlockByHash` query (@fedekunze) ### IMPROVEMENTS: @@ -63,8 +63,9 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi - [types] [\#4905](https://github.com/tendermint/tendermint/pull/4905) Add ValidateBasic to validator and validator set (@cmwaters) - [consensus] [\#4578](https://github.com/tendermint/tendermint/issues/4578) Attempt to repair the consensus WAL file (`data/cs.wal/wal`) automatically in case of corruption (@alessio) The original WAL file will be backed up to `data/cs.wal/wal.CORRUPTED`. -- [lite2] [\#4935](https://github.com/tendermint/tendermint/pull/4935) Fetch and compare a new header with witnesses in parallel (@melekes) -- [lite2] [\#4929](https://github.com/tendermint/tendermint/pull/4929) compare header w/ witnesses only when doing bisection (@melekes) +- [light] [\#4935](https://github.com/tendermint/tendermint/pull/4935) Fetch and compare a new header with witnesses in parallel (@melekes) +- [light] [\#4929](https://github.com/tendermint/tendermint/pull/4929) compare header w/ witnesses only when doing bisection (@melekes) +- [light] [\#4916](https://github.com/tendermint/tendermint/pull/4916) validate basic for inbound validator sets and headers before further processing them (@cmwaters) ### BUG FIXES: diff --git a/light/client.go b/light/client.go index 8dd624425..7ac138c76 100644 --- a/light/client.go +++ b/light/client.go @@ -1071,6 +1071,7 @@ func (c *Client) replacePrimaryProvider() error { c.providerMutex.Lock() defer c.providerMutex.Unlock() + c.logger.Info("Primary is unavailable. Replacing with the first witness") if len(c.witnesses) <= 1 { return errNoWitnesses{} } @@ -1087,55 +1088,89 @@ func (c *Client) replacePrimaryProvider() error { func (c *Client) signedHeaderFromPrimary(height int64) (*types.SignedHeader, error) { for attempt := uint16(1); attempt <= c.maxRetryAttempts; attempt++ { c.providerMutex.Lock() - h, err := c.primary.SignedHeader(height) + h, providerErr := c.primary.SignedHeader(height) c.providerMutex.Unlock() - if err == nil { - // sanity check - if height > 0 && h.Height != height { - return nil, fmt.Errorf("expected %d height, got %d", height, h.Height) + if providerErr == nil { + err := c.validateHeader(h, height) + if err != nil { + replaceErr := c.replacePrimaryProvider() + if replaceErr != nil { + return nil, fmt.Errorf("%v. Tried to replace primary but: %w", err.Error(), replaceErr) + } + // replace primary and request signed header again + return c.signedHeaderFromPrimary(height) } + // valid header has been received return h, nil } - if err == provider.ErrSignedHeaderNotFound { - return nil, err + if providerErr == provider.ErrSignedHeaderNotFound { + return nil, providerErr } - c.logger.Error("Failed to get signed header from primary", "attempt", attempt, "err", err) + c.logger.Error("Failed to get signed header from primary", "attempt", attempt, "err", providerErr) time.Sleep(backoffTimeout(attempt)) } - c.logger.Info("Primary is unavailable. Replacing with the first witness") err := c.replacePrimaryProvider() if err != nil { - return nil, err + return nil, fmt.Errorf("primary dropped out. Tried to replace but: %w", err) } return c.signedHeaderFromPrimary(height) } +func (c *Client) validateHeader(h *types.SignedHeader, expectedHeight int64) error { + if h == nil { + return errors.New("nil header") + } + if expectedHeight > 0 && h.Height != expectedHeight { + return errors.New("height mismatch") + } + return h.ValidateBasic(c.chainID) +} + // validatorSetFromPrimary retrieves the ValidatorSet from the primary provider // at the specified height. Handles dropout by the primary provider after 5 // attempts by replacing it with an alternative provider. func (c *Client) validatorSetFromPrimary(height int64) (*types.ValidatorSet, error) { for attempt := uint16(1); attempt <= c.maxRetryAttempts; attempt++ { c.providerMutex.Lock() - vals, err := c.primary.ValidatorSet(height) + vals, providerErr := c.primary.ValidatorSet(height) c.providerMutex.Unlock() - if err == nil || err == provider.ErrValidatorSetNotFound { - return vals, err + if providerErr == nil { + err := c.validateValidatorSet(vals) + if err != nil { + replaceErr := c.replacePrimaryProvider() + if replaceErr != nil { + return nil, fmt.Errorf("%v. Tried to replace primary but: %w", err.Error(), replaceErr) + } + // replace primary and request signed header again + return c.validatorSetFromPrimary(height) + } + return vals, nil + } + + if providerErr == provider.ErrValidatorSetNotFound { + return vals, providerErr } - c.logger.Error("Failed to get validator set from primary", "attempt", attempt, "err", err) + c.logger.Error("Failed to get validator set from primary", "attempt", attempt, "err", providerErr) time.Sleep(backoffTimeout(attempt)) } - c.logger.Info("Primary is unavailable. Replacing with the first witness") err := c.replacePrimaryProvider() if err != nil { - return nil, err + return nil, fmt.Errorf("primary dropped out. Tried to replace but: %w", err) } return c.validatorSetFromPrimary(height) } +func (c *Client) validateValidatorSet(vals *types.ValidatorSet) error { + if vals == nil { + return errors.New("validator set is nil") + } + return vals.ValidateBasic() +} + // sendConflictingHeadersEvidence sends evidence to all witnesses and primary // on best effort basis. // diff --git a/light/client_test.go b/light/client_test.go index d51581558..19851d6e5 100644 --- a/light/client_test.go +++ b/light/client_test.go @@ -973,3 +973,74 @@ func TestClientReportsConflictingHeadersEvidence(t *testing.T) { assert.True(t, fullNode2.HasEvidence(ev)) assert.True(t, fullNode.HasEvidence(ev)) } + +func TestClientEnsureValidHeadersAndValSets(t *testing.T) { + emptyValSet := &types.ValidatorSet{ + Validators: nil, + Proposer: nil, + } + + testCases := []struct { + headers map[int64]*types.SignedHeader + vals map[int64]*types.ValidatorSet + err bool + }{ + { + headerSet, + valSet, + false, + }, + { + headerSet, + map[int64]*types.ValidatorSet{ + 1: vals, + 2: vals, + 3: nil, + }, + true, + }, + { + map[int64]*types.SignedHeader{ + 1: h1, + 2: h2, + 3: nil, + }, + valSet, + true, + }, + { + headerSet, + map[int64]*types.ValidatorSet{ + 1: vals, + 2: vals, + 3: emptyValSet, + }, + true, + }, + } + + for _, tc := range testCases { + badNode := mockp.New( + chainID, + tc.headers, + tc.vals, + ) + c, err := light.NewClient( + chainID, + trustOptions, + badNode, + []provider.Provider{badNode, badNode}, + dbs.New(dbm.NewMemDB(), chainID), + light.MaxRetryAttempts(1), + ) + require.NoError(t, err) + + _, err = c.VerifyHeaderAtHeight(3, bTime.Add(2*time.Hour)) + if tc.err { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + } + +}