From bc5b0e9752b8ce3140e5cc697b2383103419a648 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Wed, 16 Sep 2020 21:52:42 +0300 Subject: [PATCH] Verify GRANDPA justifications from runtime (#353) * verify justifications from runtime * unreachable --- bridges/modules/substrate/Cargo.toml | 33 +- .../modules/substrate/src/justification.rs | 329 ++++++++++++++++++ bridges/modules/substrate/src/lib.rs | 1 + 3 files changed, 361 insertions(+), 2 deletions(-) create mode 100644 bridges/modules/substrate/src/justification.rs diff --git a/bridges/modules/substrate/Cargo.toml b/bridges/modules/substrate/Cargo.toml index 40b7b0b894d59..365ce68993539 100644 --- a/bridges/modules/substrate/Cargo.toml +++ b/bridges/modules/substrate/Cargo.toml @@ -8,16 +8,35 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -serde = { version = "1.0", optional = true } +codec = { package = "parity-scale-codec", version = "1.3.4", default-features = false } +finality-grandpa = { version = "0.12.3", default-features = false } hash-db = { version = "0.15.2", default-features = false } # Substrate Based Dependencies +[dependencies.frame-support] +version = "2.0.0-rc6" +tag = 'v2.0.0-rc6' +default-features = false +git = "https://github.com/paritytech/substrate/" + +[dependencies.sp-finality-grandpa] +version = "2.0.0-rc6" +tag = 'v2.0.0-rc6' +default-features = false +git = "https://github.com/paritytech/substrate/" + [dependencies.sp-runtime] version = "2.0.0-rc6" tag = 'v2.0.0-rc6' default-features = false git = "https://github.com/paritytech/substrate/" +[dependencies.sp-std] +version = "2.0.0-rc6" +tag = 'v2.0.0-rc6' +default-features = false +git = "https://github.com/paritytech/substrate/" + [dependencies.sp-trie] version = "2.0.0-rc6" tag = 'v2.0.0-rc6' @@ -30,6 +49,12 @@ tag = 'v2.0.0-rc6' default-features = false git = "https://github.com/paritytech/substrate/" +[dev-dependencies.sp-keyring] +version = "2.0.0-rc6" +tag = 'v2.0.0-rc6' +default-features = false +git = "https://github.com/paritytech/substrate/" + [dev-dependencies.sp-state-machine] version = "0.8.0-rc6" tag = 'v2.0.0-rc6' @@ -39,7 +64,11 @@ git = "https://github.com/paritytech/substrate/" [features] default = ["std"] std = [ - "serde", + "codec/std", + "finality-grandpa/std", + "frame-support/std", + "sp-finality-grandpa/std", "sp-runtime/std", + "sp-std/std", "sp-trie/std", ] diff --git a/bridges/modules/substrate/src/justification.rs b/bridges/modules/substrate/src/justification.rs new file mode 100644 index 0000000000000..0457278197aa2 --- /dev/null +++ b/bridges/modules/substrate/src/justification.rs @@ -0,0 +1,329 @@ +// Copyright 2019-2020 Parity Technologies (UK) Ltd. +// This file is part of Parity Bridges Common. + +// Parity Bridges Common is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Parity Bridges Common is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Parity Bridges Common. If not, see . + +//! Adapted copy of substrate/client/finality-grandpa/src/justification.rs. If origin +//! will ever be moved to the sp_finality_grandpa, we should reuse that implementation. + +// TODO: remove on actual use +#![allow(dead_code)] + +use codec::Decode; +use finality_grandpa::{voter_set::VoterSet, Chain, Error as GrandpaError}; +use frame_support::RuntimeDebug; +use sp_finality_grandpa::{AuthorityId, AuthoritySignature, SetId}; +use sp_runtime::traits::Header as HeaderT; +use sp_std::collections::{btree_map::BTreeMap, btree_set::BTreeSet}; + +/// Justification verification error. +#[derive(RuntimeDebug, PartialEq)] +pub enum Error { + /// Failed to decode justification. + JustificationDecode, + /// Justification is finalizing unexpected header. + InvalidJustificationTarget, + /// Invalid commit in justification. + InvalidJustificationCommit, + /// Justification has invalid authority singature. + InvalidAuthoritySignature, + /// The justification has precommit for the header that has no route from the target header. + InvalidPrecommitAncestryProof, + /// The justification has 'unused' headers in its precommit ancestries. + InvalidPrecommitAncestries, +} + +/// Verify that justification, that is generated by given authority set, finalizes given header. +pub fn verify_justification( + finalized_target: (Header::Hash, Header::Number), + authorities_set_id: SetId, + authorities_set: VoterSet, + raw_justification: &[u8], +) -> Result<(), Error> +where + Header::Number: finality_grandpa::BlockNumberOps, +{ + // decode justification first + let justification = + GrandpaJustification::
::decode(&mut &raw_justification[..]).map_err(|_| Error::JustificationDecode)?; + + // ensure that it is justification for the expected header + if (justification.commit.target_hash, justification.commit.target_number) != finalized_target { + return Err(Error::InvalidJustificationTarget); + } + + // validate commit of the justification (it just assumes all signatures are valid) + let ancestry_chain = AncestryChain::new(&justification.votes_ancestries); + match finality_grandpa::validate_commit(&justification.commit, &authorities_set, &ancestry_chain) { + Ok(ref result) if result.ghost().is_some() => {} + _ => return Err(Error::InvalidJustificationCommit), + } + + // now that we know that the commit is correct, check authorities signatures + let mut buf = Vec::new(); + let mut visited_hashes = BTreeSet::new(); + for signed in &justification.commit.precommits { + if !sp_finality_grandpa::check_message_signature_with_buffer( + &finality_grandpa::Message::Precommit(signed.precommit.clone()), + &signed.id, + &signed.signature, + justification.round, + authorities_set_id, + &mut buf, + ) { + return Err(Error::InvalidAuthoritySignature); + } + + if justification.commit.target_hash == signed.precommit.target_hash { + continue; + } + + match ancestry_chain.ancestry(justification.commit.target_hash, signed.precommit.target_hash) { + Ok(route) => { + // ancestry starts from parent hash but the precommit target hash has been visited + visited_hashes.insert(signed.precommit.target_hash); + visited_hashes.extend(route); + } + _ => { + // could this happen in practice? I don't think so, but original code has this check + return Err(Error::InvalidPrecommitAncestryProof); + } + } + } + + let ancestry_hashes = justification + .votes_ancestries + .iter() + .map(|h: &Header| h.hash()) + .collect(); + if visited_hashes != ancestry_hashes { + return Err(Error::InvalidPrecommitAncestries); + } + + Ok(()) +} + +/// GRANDPA justification of the bridged chain +#[derive(Decode, RuntimeDebug)] +#[cfg_attr(test, derive(codec::Encode))] +struct GrandpaJustification { + round: u64, + commit: finality_grandpa::Commit, + votes_ancestries: Vec
, +} + +/// A utility trait implementing `finality_grandpa::Chain` using a given set of headers. +struct AncestryChain { + ancestry: BTreeMap, +} + +impl AncestryChain
{ + fn new(ancestry: &[Header]) -> AncestryChain
{ + AncestryChain { + ancestry: ancestry + .iter() + .map(|header| (header.hash(), *header.parent_hash())) + .collect(), + } + } +} + +impl finality_grandpa::Chain for AncestryChain
+where + Header::Number: finality_grandpa::BlockNumberOps, +{ + fn ancestry(&self, base: Header::Hash, block: Header::Hash) -> Result, GrandpaError> { + let mut route = Vec::new(); + let mut current_hash = block; + loop { + if current_hash == base { + break; + } + match self.ancestry.get(¤t_hash).cloned() { + Some(parent_hash) => { + current_hash = parent_hash; + route.push(current_hash); + } + _ => return Err(GrandpaError::NotDescendent), + } + } + route.pop(); // remove the base + + Ok(route) + } + + fn best_chain_containing(&self, _block: Header::Hash) -> Option<(Header::Hash, Header::Number)> { + unreachable!("is only used during voting; qed") + } +} + +#[cfg(test)] +mod tests { + use super::*; + use codec::Encode; + use sp_core::H256; + use sp_keyring::Ed25519Keyring; + use sp_runtime::traits::BlakeTwo256; + + const TEST_GRANDPA_ROUND: u64 = 1; + const TEST_GRANDPA_SET_ID: SetId = 1; + + type TestHeader = sp_runtime::generic::Header; + + fn header(index: u8) -> TestHeader { + TestHeader::new( + index as _, + Default::default(), + Default::default(), + if index == 0 { + Default::default() + } else { + header(index - 1).hash() + }, + Default::default(), + ) + } + + fn header_id(index: u8) -> (H256, u64) { + (header(index).hash(), index as _) + } + + fn authorities_set() -> VoterSet { + VoterSet::new(vec![ + (Ed25519Keyring::Alice.public().into(), 1), + (Ed25519Keyring::Bob.public().into(), 1), + (Ed25519Keyring::Charlie.public().into(), 1), + ]) + .unwrap() + } + + fn signed_precommit( + signer: Ed25519Keyring, + target: (H256, u64), + ) -> finality_grandpa::SignedPrecommit { + let precommit = finality_grandpa::Precommit { + target_hash: target.0, + target_number: target.1, + }; + let encoded = sp_finality_grandpa::localized_payload( + TEST_GRANDPA_ROUND, + TEST_GRANDPA_SET_ID, + &finality_grandpa::Message::Precommit(precommit.clone()), + ); + let signature = signer.sign(&encoded[..]).into(); + finality_grandpa::SignedPrecommit { + precommit, + signature, + id: signer.public().into(), + } + } + + fn make_justification_for_header_1() -> GrandpaJustification { + GrandpaJustification { + round: TEST_GRANDPA_ROUND, + commit: finality_grandpa::Commit { + target_hash: header_id(1).0, + target_number: header_id(1).1, + precommits: vec![ + signed_precommit(Ed25519Keyring::Alice, header_id(2)), + signed_precommit(Ed25519Keyring::Bob, header_id(3)), + signed_precommit(Ed25519Keyring::Charlie, header_id(4)), + ], + }, + votes_ancestries: vec![header(2), header(3), header(4)], + } + } + + #[test] + fn justification_with_invalid_encoding_rejected() { + assert_eq!( + verify_justification::(header_id(1), TEST_GRANDPA_SET_ID, authorities_set(), &[],), + Err(Error::JustificationDecode), + ); + } + + #[test] + fn justification_with_invalid_target_rejected() { + assert_eq!( + verify_justification::( + header_id(2), + TEST_GRANDPA_SET_ID, + authorities_set(), + &make_justification_for_header_1().encode(), + ), + Err(Error::InvalidJustificationTarget), + ); + } + + #[test] + fn justification_with_invalid_commit_rejected() { + let mut justification = make_justification_for_header_1(); + justification.commit.precommits.clear(); + + assert_eq!( + verify_justification::( + header_id(1), + TEST_GRANDPA_SET_ID, + authorities_set(), + &justification.encode(), + ), + Err(Error::InvalidJustificationCommit), + ); + } + + #[test] + fn justification_with_invalid_authority_signature_rejected() { + let mut justification = make_justification_for_header_1(); + justification.commit.precommits[0].signature = Default::default(); + + assert_eq!( + verify_justification::( + header_id(1), + TEST_GRANDPA_SET_ID, + authorities_set(), + &justification.encode(), + ), + Err(Error::InvalidAuthoritySignature), + ); + } + + #[test] + fn justification_with_invalid_precommit_ancestry() { + let mut justification = make_justification_for_header_1(); + justification.votes_ancestries.push(header(10)); + + assert_eq!( + verify_justification::( + header_id(1), + TEST_GRANDPA_SET_ID, + authorities_set(), + &justification.encode(), + ), + Err(Error::InvalidPrecommitAncestries), + ); + } + + #[test] + fn valid_justification_accepted() { + assert_eq!( + verify_justification::( + header_id(1), + TEST_GRANDPA_SET_ID, + authorities_set(), + &make_justification_for_header_1().encode(), + ), + Ok(()), + ); + } +} diff --git a/bridges/modules/substrate/src/lib.rs b/bridges/modules/substrate/src/lib.rs index 034c56a8a46ca..ec00311e6eabe 100644 --- a/bridges/modules/substrate/src/lib.rs +++ b/bridges/modules/substrate/src/lib.rs @@ -14,4 +14,5 @@ // You should have received a copy of the GNU General Public License // along with Parity Bridges Common. If not, see . +mod justification; mod storage_proof;