From 4936b23dae289422171d0575eb6d18957b598372 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Thu, 5 Mar 2020 11:28:32 +0400 Subject: [PATCH] adr-047: evidence handling (#4429) Closes #4213 and #4182 --- docs/architecture/README.md | 1 + ...047-handling-evidence-from-light-client.md | 186 ++++++++++++++++++ 2 files changed, 187 insertions(+) create mode 100644 docs/architecture/adr-047-handling-evidence-from-light-client.md diff --git a/docs/architecture/README.md b/docs/architecture/README.md index 9b3c2f661..6eb6ebc4f 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -67,5 +67,6 @@ Note the context/background should be written in the present tense. - [ADR-044-Lite-Client-With-Weak-Subjectivity](./adr-044-lite-client-with-weak-subjectivity.md) - [ADR-045-ABCI-Evidence](./adr-045-abci-evidence.md) - [ADR-046-Light-Client-Implementation](./adr-046-light-client-implementation.md) +- [ADR-047-Handling-Evidence-From-Light-Client](./adr-047-handling-evidence-from-light-client.md) - [ADR-051-Double-Signing-Risk-Reduction](./adr-051-double-signing-risk-reduction.md) - [ADR-052-Tendermint-Mode](./adr-052-tendermint-mode.md) diff --git a/docs/architecture/adr-047-handling-evidence-from-light-client.md b/docs/architecture/adr-047-handling-evidence-from-light-client.md new file mode 100644 index 000000000..8b3a850ba --- /dev/null +++ b/docs/architecture/adr-047-handling-evidence-from-light-client.md @@ -0,0 +1,186 @@ +# ADR 047: Handling evidence from light client + +## Changelog +* 18-02-2020: Initial draft +* 24-02-2020: Second version + +## Context + +If the light client is under attack, either directly -> lunatic/phantom +validators (light fork) or indirectly -> full fork, it's supposed to halt and +send evidence of misbehavior to a correct full node. Upon receiving an +evidence, the full node should punish malicious validators (if possible). + +## Decision + +When a light client sees two conflicting headers (`H1.Hash() != H2.Hash()`, +`H1.Height == H2.Height`), both having 1/3+ of the voting power of the +currently trusted validator set, it will submit a `ConflictingHeadersEvidence` +to all full nodes it's connected to. Evidence needs to be submitted to all full +nodes since there's no way to determine which full node is correct (honest). + +```go +type ConflictingHeadersEvidence struct { + H1 types.SignedHeader + H2 types.SignedHeader +} +``` + +When a full node receives the `ConflictingHeadersEvidence` evidence, it should +a) validate it b) figure out if malicious behaviour is obvious (immediately +slashable) or the fork accountability protocol needs to be started. + +### Validating headers + +Check both headers are valid (`ValidateBasic`), have the same height, and +signed by 1/3+ of the validator set that the full node had at height +`H1.Height-1`. + +- Q: What if light client validator set is not equal to full node's validator + set (i.e. from full node's point of view both headers are not properly signed; + this includes the case where none of the two headers were committed on the + main chain) + + Reject the evidence. It means light client is following a fork, but, hey, at + least it will halt. + +- Q: Don't we want to punish validators who signed something else even if they + have less or equal than 1/3? + + No consensus so far. Ethan said no, Zarko said yes. + https://github.com/tendermint/spec/pull/71#discussion_r374210533 + +### Figuring out if malicious behaviour is immediately slashable + +Let's say H1 was committed from this full node's perspective (see Appendix A). +Intersect validator sets of H1 and H2. + +* if there are signers(H2) that are not part of validators(H1), they misbehaved as +they are signing protocol messages in heights they are not validators => +immediately slashable (#F4). + +* if `H1.Round == H2.Round`, and some signers signed different precommit +messages in both commits, then it is an equivocation misbehavior => immediately +slashable (#F1). + +* if `H1.Round != H2.Round` we need to run full detection procedure => not +immediately slashable. + +* if `ValidatorsHash`, `NextValidatorsHash`, `ConsensusHash`, +`AppHash`, and `LastResultsHash` in H2 are different (incorrect application +state transition), then it is a lunatic misbehavior => immediately slashable (#F5). + +If evidence is not immediately slashable, fork accountability needs to invoked +(ADR does not yet exist). + +It's unclear if we should further break up `ConflictingHeadersEvidence` or +gossip and commit it directly. See +https://github.com/tendermint/tendermint/issues/4182#issuecomment-590339233 + +If we'd go without breaking evidence, all we'll need to do is to strip the +committed header from `ConflictingHeadersEvidence` (H1) and leave only the +uncommitted header (H2): + +```go +type ConflictingHeaderEvidence struct { + H types.SignedHeader +} +``` + +If we'd go with breaking evidence, here are the types we'll need: + +### F1. Equivocation + +Existing `DuplicateVoteEvidence` needs to be created and gossiped. + +### F4. Phantom validators + +A new type of evidence needs to be created: + +```go +type PhantomValidatorEvidence struct { + PubKey crypto.PubKey + Vote types.Vote +} +``` + +It contains a validator's public key and a vote for a block, where this +validator is not part of the validator set. + +### F5. Lunatic validator + +```go +type LunaticValidatorEvidence struct { + Header types.Header + Vote types.Vote +} +``` + +To punish this attack, we need support for a new Evidence type - +`LunaticValidatorEvidence`. This type includes a vote and a header. The header +must contain fields that are invalid with respect to the previous block, and a +vote for that header by a validator that was in a validator set within the +unbonding period. While the attack is only possible if +1/3 of some validator +set colludes, the evidence should be verifiable independently for each +individual validator. This means the total evidence can be split into one piece +of evidence per attacking validator and gossipped to nodes to be verified one +piece at a time, reducing the DoS attack surface at the peer layer. + +Note it is not sufficient to simply compare this header with that committed for +the corresponding height, as an honest node may vote for a header that is not +ultimately committed. Certain fields may also be variable, for instance the +`LastCommitHash` and the `Time` may depend on which votes the proposer includes. +Thus, the header must be explicitly checked for invalid data. + +For the attack to succeed, VC must sign a header that changes the validator set +to consist of something they control. Without doing this, they can not +otherwise attack the light client, since the client verifies commits according +to validator sets. Thus, it should be sufficient to check only that +`ValidatorsHash` and `NextValidatorsHash` are correct with respect to the +header that was committed at the corresponding height. + +That said, if the attack is conducted by +2/3 of the validator set, they don't +need to make an invalid change to the validator set, since they already control +it. Instead they would make invalid changes to the `AppHash`, or possibly other +fields. In order to punish them, then, we would have to check all header +fields. + +Note some header fields require the block itself to verify, which the light +client, by definition, does not possess, so it may not be possible to check +these fields. For now, then, `LunaticValidatorEvidence` must be checked against +all header fields which are a function of the application at previous blocks. +This includes `ValidatorsHash`, `NextValidatorsHash`, `ConsensusHash`, +`AppHash`, and `LastResultsHash`. These should all match what's in the header +for the block that was actually committed at the corresponding height, and +should thus be easy to check. + +## Status + +Proposed. + +## Consequences + +### Positive + +* Tendermint will be able to detect & punish new types of misbehavior +* light clients connected to multiple full nodes can help full nodes notice a + fork faster + +### Negative + +* Accepting `ConflictingHeadersEvidence` from light clients opens up a DDOS +attack vector (same is fair for any RPC endpoint open to public; remember that +RPC is not open by default). + +### Neutral + +## References + +* [Fork accountability spec](https://github.com/tendermint/spec/blob/master/spec/consensus/light-client/accountability.md) + +## Appendix A + +If there is an actual fork (full fork), a full node may follow either one or +another branch. So both H1 or H2 can be considered committed depending on which +branch the full node is following. It's supposed to halt if it notices an +actual fork, but there's a small chance it doesn't.