From b6f0aa3a8874877771415d40f49798b16e86c3e5 Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Fri, 6 Mar 2020 20:05:20 +0100 Subject: [PATCH] lite2: replace primary when providing invalid header (#4523) Closes: #4420 Created a new error ErrInvalidHeaderwhich can be formed during the verification process verifier.go and will result in the replacement of the primary provider with a witness by executing: replacePrimaryProvider() Co-authored-by: Anton Kaliaev --- go.sum | 10 ------- lite2/client.go | 67 ++++++++++++++++++++++++++++++------------ lite2/errors.go | 10 +++++++ lite2/verifier.go | 61 ++++++++++++++++++++++++++++---------- lite2/verifier_test.go | 4 +-- 5 files changed, 106 insertions(+), 46 deletions(-) diff --git a/go.sum b/go.sum index 886eadee3..21cbdfc57 100644 --- a/go.sum +++ b/go.sum @@ -10,8 +10,6 @@ github.com/Shopify/sarama v1.19.0/go.mod h1:FVkBWblsNy7DGZRfXLU0O9RCGt5g3g3yEuWX github.com/Shopify/toxiproxy v2.1.4+incompatible/go.mod h1:OXgGpZ6Cli1/URJOF1DMxUHB2q5Ap20/P/eIdh4G0pI= github.com/VividCortex/gohistogram v1.0.0 h1:6+hBz+qvs0JOrrNhhmR7lFxo5sINxBCGXrdtl/UvroE= github.com/VividCortex/gohistogram v1.0.0/go.mod h1:Pf5mBqqDxYaXu3hDrrU+w6nw50o/4+TcAqDqk/vUH7g= -github.com/Workiva/go-datastructures v1.0.50 h1:slDmfW6KCHcC7U+LP3DDBbm4fqTwZGn1beOFPfGaLvo= -github.com/Workiva/go-datastructures v1.0.50/go.mod h1:Z+F2Rca0qCsVYDS8z7bAGm8f3UkzuWYS/oBZz5a7VVA= github.com/Workiva/go-datastructures v1.0.51 h1:LJHjjfcv+1gH+1D1SgrjcrF8iSZkgsAdCjclvHvVecQ= github.com/Workiva/go-datastructures v1.0.51/go.mod h1:Z+F2Rca0qCsVYDS8z7bAGm8f3UkzuWYS/oBZz5a7VVA= github.com/aead/siphash v1.0.1/go.mod h1:Nywa3cDsYNNK3gaciGTWPwHt0wlpNV15vwmswBAUSII= @@ -132,8 +130,6 @@ github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5y github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.2 h1:6nsPYzhq5kReh6QImI3k5qWzO4PEbvbIW2cwSfR/6xs= github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= -github.com/golang/protobuf v1.3.3 h1:gyjaxf+svBWX08ZjK86iN9geUJF0H6gp2IRKX6Nf6/I= -github.com/golang/protobuf v1.3.3/go.mod h1:vzj43D7+SQXF/4pzW/hwtAqwc6iTitCiVSaWz5lYuqw= github.com/golang/protobuf v1.3.4 h1:87PNWwrRvUSnqS4dlcBU/ftvOIBep4sYuBLlh6rX2wk= github.com/golang/protobuf v1.3.4/go.mod h1:vzj43D7+SQXF/4pzW/hwtAqwc6iTitCiVSaWz5lYuqw= github.com/golang/snappy v0.0.0-20180518054509-2e65f85255db/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q= @@ -386,22 +382,16 @@ github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXf github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= -github.com/stretchr/testify v1.5.0 h1:DMOzIV76tmoDNE9pX6RSN0aDtCYeCg5VueieJaAo1uw= -github.com/stretchr/testify v1.5.0/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= github.com/stretchr/testify v1.5.1 h1:nOGnQDM7FYENwehXlg/kFVnos3rEvtKTjRvOWSzb6H4= github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= github.com/subosito/gotenv v1.2.0 h1:Slr1R9HxAlEKefgq5jn9U+DnETlIUa6HfgEzj0g5d7s= github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw= github.com/syndtr/goleveldb v1.0.1-0.20190923125748-758128399b1d h1:gZZadD8H+fF+n9CmNhYL1Y0dJB+kLOmKd7FbPJLeGHs= github.com/syndtr/goleveldb v1.0.1-0.20190923125748-758128399b1d/go.mod h1:9OrXJhf154huy1nPWmuSrkgjPUtUNhA+Zmy+6AESzuA= -github.com/tecbot/gorocksdb v0.0.0-20191017175515-d217d93fd4c5 h1:gVwAW5OwaZlDB5/CfqcGFM9p9C+KxvQKyNOltQ8orj0= -github.com/tecbot/gorocksdb v0.0.0-20191017175515-d217d93fd4c5/go.mod h1:ahpPrc7HpcfEWDQRZEmnXMzHY03mLDYMCxeDzy46i+8= github.com/tecbot/gorocksdb v0.0.0-20191217155057-f0fad39f321c h1:g+WoO5jjkqGAzHWCjJB1zZfXPIAaDpzXIEJ0eS6B5Ok= github.com/tecbot/gorocksdb v0.0.0-20191217155057-f0fad39f321c/go.mod h1:ahpPrc7HpcfEWDQRZEmnXMzHY03mLDYMCxeDzy46i+8= github.com/tendermint/go-amino v0.14.1 h1:o2WudxNfdLNBwMyl2dqOJxiro5rfrEaU0Ugs6offJMk= github.com/tendermint/go-amino v0.14.1/go.mod h1:i/UKE5Uocn+argJJBb12qTZsCDBcAYMbR92AaJVmKso= -github.com/tendermint/tm-db v0.4.0 h1:iPbCcLbf4nwDFhS39Zo1lpdS1X/cT9CkTlUx17FHQgA= -github.com/tendermint/tm-db v0.4.0/go.mod h1:+Cwhgowrf7NBGXmsqFMbwEtbo80XmyrlY5Jsk95JubQ= github.com/tendermint/tm-db v0.4.1 h1:TvX7JWjJOVZ+N3y+I86wddrGttOdMmmBxXcu0/Y7ZJ0= github.com/tendermint/tm-db v0.4.1/go.mod h1:JsJ6qzYkCGiGwm5GHl/H5GLI9XLb6qZX7PRe425dHAY= github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= diff --git a/lite2/client.go b/lite2/client.go index 2eefd472d..22c080949 100644 --- a/lite2/client.go +++ b/lite2/client.go @@ -476,9 +476,10 @@ func (c *Client) ChainID() string { // and calls VerifyHeader. It returns header immediately if such exists in // trustedStore (no verification is needed). // +// height must be > 0. +// // It returns provider.ErrSignedHeaderNotFound if header is not found by // primary. -// It returns ErrOldHeaderExpired if header expired. func (c *Client) VerifyHeaderAtHeight(height int64, now time.Time) (*types.SignedHeader, error) { if height <= 0 { return nil, errors.New("negative or zero height") @@ -515,7 +516,10 @@ func (c *Client) VerifyHeaderAtHeight(height int64, now time.Time) (*types.Signe // intermediate headers will be requested. See the specification for details. // https://github.com/tendermint/spec/blob/master/spec/consensus/light-client.md // -// It returns ErrOldHeaderExpired if newHeader expired. +// It returns ErrOldHeaderExpired if the latest trusted header expired. +// +// If the primary provides an invalid header (ErrInvalidHeader), it is rejected +// and replaced by another provider until all are exhausted. // // If, at any moment, SignedHeader or ValidatorSet are not found by the primary // provider, provider.ErrSignedHeaderNotFound / @@ -676,7 +680,23 @@ func (c *Client) sequence( err = VerifyAdjacent(c.chainID, trustedHeader, interimHeader, interimVals, c.trustingPeriod, now) if err != nil { - return errors.Wrapf(err, "failed to verify the header #%d", height) + err = errors.Wrapf(err, "verify adjacent from #%d to #%d failed", + trustedHeader.Height, interimHeader.Height) + + switch errors.Cause(err).(type) { + case ErrInvalidHeader: + c.logger.Error("primary sent invalid header -> replacing", "err", err) + replaceErr := c.replacePrimaryProvider() + if replaceErr != nil { + c.logger.Error("Can't replace primary", "err", replaceErr) + return err // return original error + } + // attempt to verify header again + height-- + continue + default: + return err + } } // 3) Update trustedHeader @@ -729,8 +749,21 @@ func (c *Client) bisection( return err } + case ErrInvalidHeader: + c.logger.Error("primary sent invalid header -> replacing", "err", err) + replaceErr := c.replacePrimaryProvider() + if replaceErr != nil { + c.logger.Error("Can't replace primary", "err", replaceErr) + // return original error + return errors.Wrapf(err, "verify from #%d to #%d failed", + trustedHeader.Height, interimHeader.Height) + } + // attempt to verify the header again + continue + default: - return errors.Wrapf(err, "failed to verify the header #%d", newHeader.Height) + return errors.Wrapf(err, "verify from #%d to #%d failed", + trustedHeader.Height, interimHeader.Height) } } } @@ -772,7 +805,9 @@ func (c *Client) fetchHeaderAndValsAtHeight(height int64) (*types.SignedHeader, return h, vals, nil } -// Backwards verification (see VerifyHeaderBackwards func in the spec) +// backwards verification (see VerifyHeaderBackwards func in the spec) verifies +// headers before a trusted header. If a sent header is invalid the primary is +// replaced with another provider and the operation is repeated. func (c *Client) backwards( initiallyTrustedHeader *types.SignedHeader, newHeader *types.SignedHeader, @@ -794,20 +829,14 @@ func (c *Client) backwards( return errors.Wrapf(err, "failed to obtain the header at height #%d", trustedHeader.Height-1) } - if err := interimHeader.ValidateBasic(c.chainID); err != nil { - return errors.Wrap(err, "untrustedHeader.ValidateBasic failed") - } - - if !interimHeader.Time.Before(trustedHeader.Time) { - return errors.Errorf("expected older header time %v to be before newer header time %v", - interimHeader.Time, - trustedHeader.Time) - } - - if !bytes.Equal(interimHeader.Hash(), trustedHeader.LastBlockID.Hash) { - return errors.Errorf("older header hash %X does not match trusted header's last block %X", - interimHeader.Hash(), - trustedHeader.LastBlockID.Hash) + if err := VerifyBackwards(c.chainID, interimHeader, trustedHeader); err != nil { + c.logger.Error("primary sent invalid header -> replacing", "err", err) + if replaceErr := c.replacePrimaryProvider(); replaceErr != nil { + c.logger.Error("Can't replace primary", "err", replaceErr) + // return original error + return errors.Wrapf(err, "verify backwards from %d to %d failed", + trustedHeader.Height, interimHeader.Height) + } } trustedHeader = interimHeader diff --git a/lite2/errors.go b/lite2/errors.go index d0b5d2d31..13a6cf29d 100644 --- a/lite2/errors.go +++ b/lite2/errors.go @@ -28,3 +28,13 @@ type ErrNewValSetCantBeTrusted struct { func (e ErrNewValSetCantBeTrusted) Error() string { return fmt.Sprintf("cant trust new val set: %v", e.Reason) } + +// ErrInvalidHeader means the header either failed the basic validation or +// commit is not signed by 2/3+. +type ErrInvalidHeader struct { + Reason error +} + +func (e ErrInvalidHeader) Error() string { + return fmt.Sprintf("invalid header: %v", e.Reason) +} diff --git a/lite2/verifier.go b/lite2/verifier.go index 250c8b204..6d8459ab6 100644 --- a/lite2/verifier.go +++ b/lite2/verifier.go @@ -23,12 +23,12 @@ var ( // VerifyNonAdjacent verifies non-adjacent untrustedHeader against // trustedHeader. It ensures that: // -// a) trustedHeader can still be trusted (if not, ErrOldHeaderExpired is returned); -// b) untrustedHeader is valid; +// a) trustedHeader can still be trusted (if not, ErrOldHeaderExpired is returned) +// b) untrustedHeader is valid (if not, ErrInvalidHeader is returned) // c) trustLevel ([1/3, 1]) of trustedHeaderVals (or trustedHeaderNextVals) -// signed correctly (if not, ErrNewValSetCantBeTrusted is returned); -// d) more than 2/3 of untrustedVals have signed h2 (if not, -// ErrNotEnoughVotingPowerSigned is returned); +// signed correctly (if not, ErrNewValSetCantBeTrusted is returned) +// d) more than 2/3 of untrustedVals have signed h2 +// (otherwise, ErrInvalidHeader is returned) // e) headers are non-adjacent. func VerifyNonAdjacent( chainID string, @@ -49,7 +49,7 @@ func VerifyNonAdjacent( } if err := verifyNewHeaderAndVals(chainID, untrustedHeader, untrustedVals, trustedHeader, now); err != nil { - return err + return ErrInvalidHeader{err} } // Ensure that +`trustLevel` (default 1/3) or more of last trusted validators signed correctly. @@ -67,11 +67,11 @@ func VerifyNonAdjacent( // Ensure that +2/3 of new validators signed correctly. // // NOTE: this should always be the last check because untrustedVals can be - // intentionaly made very large to DOS the light client. not the case for + // intentionally made very large to DOS the light client. not the case for // VerifyAdjacent, where validator set is known in advance. if err := untrustedVals.VerifyCommit(chainID, untrustedHeader.Commit.BlockID, untrustedHeader.Height, untrustedHeader.Commit); err != nil { - return err + return ErrInvalidHeader{err} } return nil @@ -80,11 +80,11 @@ func VerifyNonAdjacent( // VerifyAdjacent verifies directly adjacent untrustedHeader against // trustedHeader. It ensures that: // -// a) trustedHeader can still be trusted (if not, ErrOldHeaderExpired is returned); -// b) untrustedHeader is valid; -// c) untrustedHeader.ValidatorsHash equals trustedHeader.NextValidatorsHash; -// d) more than 2/3 of new validators (untrustedVals) have signed h2 (if not, -// ErrNotEnoughVotingPowerSigned is returned); +// a) trustedHeader can still be trusted (if not, ErrOldHeaderExpired is returned) +// b) untrustedHeader is valid (if not, ErrInvalidHeader is returned) +// c) untrustedHeader.ValidatorsHash equals trustedHeader.NextValidatorsHash +// d) more than 2/3 of new validators (untrustedVals) have signed h2 +// (otherwise, ErrInvalidHeader is returned) // e) headers are adjacent. func VerifyAdjacent( chainID string, @@ -103,7 +103,7 @@ func VerifyAdjacent( } if err := verifyNewHeaderAndVals(chainID, untrustedHeader, untrustedVals, trustedHeader, now); err != nil { - return err + return ErrInvalidHeader{err} } // Check the validator hashes are the same @@ -118,7 +118,7 @@ func VerifyAdjacent( // Ensure that +2/3 of new validators signed correctly. if err := untrustedVals.VerifyCommit(chainID, untrustedHeader.Commit.BlockID, untrustedHeader.Height, untrustedHeader.Commit); err != nil { - return err + return ErrInvalidHeader{err} } return nil @@ -200,3 +200,34 @@ func HeaderExpired(h *types.SignedHeader, trustingPeriod time.Duration, now time expirationTime := h.Time.Add(trustingPeriod) return !expirationTime.After(now) } + +// VerifyBackwards verifies an untrusted header with a height one less than +// that of an adjacent trusted header. It ensures that: +// +// a) untrusted header is valid +// b) untrusted header has a time before the trusted header +// c) that the LastBlockID hash of the trusted header is the same as the hash +// of the trusted header +// +// For any of these cases ErrInvalidHeader is returned. +func VerifyBackwards(chainID string, untrustedHeader, trustedHeader *types.SignedHeader) error { + if err := untrustedHeader.ValidateBasic(chainID); err != nil { + return ErrInvalidHeader{err} + } + + if !untrustedHeader.Time.Before(trustedHeader.Time) { + return ErrInvalidHeader{ + errors.Errorf("expected older header time %v to be before new header time %v", + untrustedHeader.Time, + trustedHeader.Time)} + } + + if !bytes.Equal(untrustedHeader.Hash(), trustedHeader.LastBlockID.Hash) { + return ErrInvalidHeader{ + errors.Errorf("older header hash %X does not match trusted header's last block %X", + untrustedHeader.Hash(), + trustedHeader.LastBlockID.Hash)} + } + + return nil +} diff --git a/lite2/verifier_test.go b/lite2/verifier_test.go index 241d33b05..adc671516 100644 --- a/lite2/verifier_test.go +++ b/lite2/verifier_test.go @@ -113,7 +113,7 @@ func TestVerifyAdjacentHeaders(t *testing.T) { vals, 3 * time.Hour, bTime.Add(2 * time.Hour), - types.ErrNotEnoughVotingPowerSigned{Got: 50, Needed: 93}, + ErrInvalidHeader{Reason: types.ErrNotEnoughVotingPowerSigned{Got: 50, Needed: 93}}, "", }, // vals does not match with what we have -> error @@ -227,7 +227,7 @@ func TestVerifyNonAdjacentHeaders(t *testing.T) { vals, 3 * time.Hour, bTime.Add(2 * time.Hour), - types.ErrNotEnoughVotingPowerSigned{Got: 50, Needed: 93}, + ErrInvalidHeader{types.ErrNotEnoughVotingPowerSigned{Got: 50, Needed: 93}}, "", }, // 3/3 new vals signed, 2/3 old vals present -> no error