light: expand on errors and docs by cmwaters · Pull Request #5443 · tendermint/tendermint

cmwaters · 2020-10-01T15:25:06Z

Description

Updates light docs with a section on security and adds a more informative error in the case of a light client attack.

codecov · 2020-10-01T15:56:27Z

Codecov Report

Merging #5443 into master will decrease coverage by 1.97%.
The diff coverage is 53.82%.

@@            Coverage Diff             @@
##           master    #5443      +/-   ##
==========================================
- Coverage   63.29%   61.31%   -1.98%     
==========================================
  Files         181      259      +78     
  Lines       19080    23456    +4376     
==========================================
+ Hits        12077    14383    +2306     
- Misses       5973     7615    +1642     
- Partials     1030     1458     +428

Impacted Files	Coverage Δ
abci/client/client.go	`40.00% <ø> (ø)`
abci/client/grpc_client.go	`0.00% <0.00%> (ø)`
abci/client/local_client.go	`0.00% <0.00%> (ø)`
abci/types/pubkey.go	`0.00% <0.00%> (ø)`
abci/types/util.go	`0.00% <0.00%> (ø)`
behaviour/reporter.go	`46.42% <ø> (-2.06%)`	⬇️
blockchain/v0/pool.go	`76.01% <0.00%> (-2.65%)`	⬇️
blockchain/v1/reactor_fsm.go	`70.81% <0.00%> (-3.62%)`	⬇️
blockchain/v2/io.go	`0.00% <0.00%> (ø)`
blockchain/v2/processor_context.go	`68.00% <0.00%> (+4.84%)`	⬆️
... and 299 more

melekes · 2020-10-02T08:50:41Z

docs/tendermint-core/light-client.md

+primary with witnesses. Therefore light clients should be set with enough witnesses.

-[Trust Options](https://pkg.go.dev/github.com/tendermint/tendermint/light?tab=doc#TrustOptions)
+If the detector observes a faulty provider it will report it to another provider


I think we should state that the light client is not safe when a) more than {trust_level} of validators are malicious b) all witnesses are malicious

Sure, this would be in replacement of lines 44 - 49 right? or the entire section

I'd just add it, not replace.

not a replacement, but rather an addition which clearly states the fact.

tessr · 2020-10-02T10:18:57Z

docs/tendermint-core/light-client.md

-The objective of the light client protocol is to get a commit for a recent
-block hash where the commit includes a majority of signatures from the last
-known validator set. From there, all the application state is verifiable with
+The the light client protocol verifies headers by retrieving a chain of headers,


Suggested change

The the light client protocol verifies headers by retrieving a chain of headers,

The light client protocol verifies headers by retrieving a chain of headers,

tessr · 2020-10-02T10:20:37Z

docs/tendermint-core/light-client.md

+primary with witnesses. Therefore light clients should be set with enough witnesses.

-[Trust Options](https://pkg.go.dev/github.com/tendermint/tendermint/light?tab=doc#TrustOptions)
+If the detector observes a faulty provider it will report it to another provider


I'd just add it, not replace.

tessr · 2020-10-02T10:23:28Z

light/detector.go

 				CommonHeight:     commonHeight, // the first block in the bisection is common to both providers
 			}
-			c.logger.Error("Attack detected. Sending evidence againt primary by witness", "ev", ev,
+			c.logger.Error("Attack detected. Sending evidence againt primary by witness", "ev", primaryEv,


Shall we clarify that this attack was just attempted, in the spirit of not spooking any users?

Suggested change

c.logger.Error("Attack detected. Sending evidence againt primary by witness", "ev", primaryEv,

c.logger.Error("Attempted attack detected. Sending evidence against primary by witness", "ev", primaryEv,

tessr · 2020-10-02T11:15:55Z

light/errors.go


+// ErrLightClientAttack is returned when the light client has detected an attempt
+// to verify a false header and has sent the evidence to either a witness or primary.
+var ErrLightClientAttack = errors.New("ATTACK DETECTED. Light client received valid conflicting header from witness." +


Suggested change

var ErrLightClientAttack = errors.New("ATTACK DETECTED. Light client received valid conflicting header from witness." +

var ErrLightClientAttack = errors.New("Attempted attack detected. Light client received valid conflicting header from witness." +

tessr · 2020-10-02T11:19:19Z

This looks pretty good to me. Maybe we can add a message to the logs for detected attack attempts that's something like

The light client has detected, and prevented, an attempted amnesia attack. We think this attack is pretty unlikely, so if you see it, that's interesting to us. Can you let us know by opening an issue through https://github.com/tendermint/tendermint/issues/new?

* docs: specify TM version in go tutorials (#5427) Closes #5425 * privval: allow passing options to NewSignerDialerEndpoint (#5434) Required for #5291 to set timeouts for remote signers. * config: set statesync.rpc_servers when generating config file (#5433) Required for #5291, to generate configuration files with state sync RPC servers. * consensus: check block parts don't exceed maximum block bytes (#5431) * ci: docker remvoe circleci and add github action (#5420) * privval: fix ping message encoding (#5441) Fixes #5371. * docs: revise ADR 56, documenting short term decision around amnesia evidence (#5440) * light: expand on errors and docs (#5443) * test: add end-to-end testing framework (#5435) Partial fix for #5291. For details, see [README.md](https://github.com/tendermint/tendermint/blob/erik/e2e-tests/test/e2e/README.md) and [RFC-001](https://github.com/tendermint/tendermint/blob/master/docs/rfc/rfc-001-end-to-end-testing.md). This only includes a single test case under `test/e2e/tests/`, as a proof of concept - additional test cases will be submitted separately. A randomized testnet generator will also be submitted separately, there a currently just a handful of static testnets under `test/e2e/networks/`. This will eventually replace the current P2P tests and run in CI. * changelog: add missing date to v0.33.5 release, fix indentation (#5454) I forgot to add the date when we cut 0.33.5. This fixes that. It also fixes a header indentation issue for 0.33.8. * test: add basic end-to-end test cases (#5450) Partial fix for #5291. This adds a basic set of test cases for core network invariants. Although small, it is sufficient to replace and extend the current set of P2P tests. Further test cases can be added later. * test: add GitHub action for end-to-end tests (#5452) Partial fix for #5291. * fix RPC blockresults reutrn (#5459) ## Description In blocks_results we use the proto definition of abciResponses: https://github.com/tendermint/tendermint/blob/2672b91ab099b8b02f3afabae4a0a745acd93c3f/rpc/core/blocks.go#L152-L155, this leads to the use of the proto definition of the pubkey which is an interface in go (oneof). The interface must be registered with the JSON encoder to have it work correctly. A clearer divide between proto types and native types is needed. Closes: #XXX * circleci: remove Gitian reproducible_builds job (#5462) * docs: fix links to adr 56 (#5464) ## Description fix broken link from a previous change * test: remove P2P tests (#5453) Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com> Co-authored-by: Erik Grinaker <erik@interchain.berlin> Co-authored-by: Callum Waters <cmwaters19@gmail.com> Co-authored-by: Marko <marbar3778@yahoo.com> Co-authored-by: Tess Rinearson <tess.rinearson@gmail.com>

* docs: specify TM version in go tutorials (#5427) Closes #5425 * privval: allow passing options to NewSignerDialerEndpoint (#5434) Required for #5291 to set timeouts for remote signers. * config: set statesync.rpc_servers when generating config file (#5433) Required for #5291, to generate configuration files with state sync RPC servers. * consensus: check block parts don't exceed maximum block bytes (#5431) * ci: docker remvoe circleci and add github action (#5420) * privval: fix ping message encoding (#5441) Fixes #5371. * docs: revise ADR 56, documenting short term decision around amnesia evidence (#5440) * light: expand on errors and docs (#5443) * makefile: config build-docs for branch and path prefix * update versions with new 0.33 branch Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com> Co-authored-by: Erik Grinaker <erik@interchain.berlin> Co-authored-by: Callum Waters <cmwaters19@gmail.com> Co-authored-by: Marko <marbar3778@yahoo.com>

expanded on light errors and docs

fb9081a

cmwaters added the C:light Component: Light label Oct 1, 2020

modify error and lint

861b34e

Merge branch 'master' into callum/light-errors

6dca232

cmwaters marked this pull request as ready for review October 2, 2020 07:28

cmwaters requested review from ebuchman, erikgrinaker, melekes, tac0turtle and tessr as code owners October 2, 2020 07:28

melekes reviewed Oct 2, 2020

View reviewed changes

tessr reviewed Oct 2, 2020

View reviewed changes

cmwaters added 2 commits October 2, 2020 16:51

implement suggested changes

f222115

minor syntax change

c8daf3d

cmwaters requested review from melekes and tessr October 2, 2020 15:02

tessr approved these changes Oct 2, 2020

View reviewed changes

Merge branch 'master' into callum/light-errors

e6447f7

cmwaters merged commit a4b7018 into master Oct 2, 2020

cmwaters deleted the callum/light-errors branch October 2, 2020 18:05

melekes pushed a commit that referenced this pull request Oct 9, 2020

light: expand on errors and docs (#5443)

04bf83f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

light: expand on errors and docs#5443

light: expand on errors and docs#5443
cmwaters merged 6 commits intomasterfrom
callum/light-errors

cmwaters commented Oct 1, 2020 •

edited

Loading

Uh oh!

codecov bot commented Oct 1, 2020 •

edited

Loading

Uh oh!

melekes Oct 2, 2020

Uh oh!

cmwaters Oct 2, 2020

Uh oh!

tessr Oct 2, 2020

Uh oh!

melekes Oct 2, 2020

Uh oh!

tessr Oct 2, 2020

Uh oh!

tessr Oct 2, 2020

Uh oh!

tessr Oct 2, 2020

Uh oh!

tessr Oct 2, 2020

Uh oh!

tessr commented Oct 2, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	The the light client protocol verifies headers by retrieving a chain of headers,
	The light client protocol verifies headers by retrieving a chain of headers,

	c.logger.Error("Attack detected. Sending evidence againt primary by witness", "ev", primaryEv,
	c.logger.Error("Attempted attack detected. Sending evidence against primary by witness", "ev", primaryEv,

	var ErrLightClientAttack = errors.New("ATTACK DETECTED. Light client received valid conflicting header from witness." +
	var ErrLightClientAttack = errors.New("Attempted attack detected. Light client received valid conflicting header from witness." +

Conversation

cmwaters commented Oct 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

codecov bot commented Oct 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tessr commented Oct 2, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cmwaters commented Oct 1, 2020 •

edited

Loading

codecov bot commented Oct 1, 2020 •

edited

Loading