From 9622b67b1c7ddf999c5f1b68bab3b2531695647e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 1 Jul 2020 16:00:49 +0200 Subject: [PATCH] Check candidate signatures before including them in `set_heads` This work around the bug described in: https://github.com/paritytech/polkadot/issues/1327 We check the candidate signatures in `create_inherent` to ensure that all are valid. If one isn't valid, we don't include any candidate for the current inherent. --- runtime/common/src/parachains.rs | 260 ++++++++++++++++++------------- 1 file changed, 149 insertions(+), 111 deletions(-) diff --git a/runtime/common/src/parachains.rs b/runtime/common/src/parachains.rs index ef67adb031f1..cb510483d43e 100644 --- a/runtime/common/src/parachains.rs +++ b/runtime/common/src/parachains.rs @@ -684,6 +684,69 @@ fn localized_payload( encoded } +/// Iterator that returns groups of validators that are assigned to the same chain. +/// +/// Assumes that the inner validators are sorted by chain id. +struct GroupedDutyIter<'a> { + next_idx: usize, + inner: &'a [(usize, ParaId)], +} + +impl<'a> GroupedDutyIter<'a> { + fn new(inner: &'a [(usize, ParaId)]) -> Self { + GroupedDutyIter { next_idx: 0, inner } + } + + fn group_for(&mut self, wanted_id: ParaId) -> Option<&'a [(usize, ParaId)]> { + while let Some((id, keys)) = self.next() { + if wanted_id == id { + return Some(keys) + } + } + + None + } +} + +impl<'a> Iterator for GroupedDutyIter<'a> { + type Item = (ParaId, &'a [(usize, ParaId)]); + + fn next(&mut self) -> Option { + if self.next_idx == self.inner.len() { return None } + let start_idx = self.next_idx; + self.next_idx += 1; + let start_id = self.inner[start_idx].1; + + while self.inner.get(self.next_idx).map_or(false, |&(_, ref id)| id == &start_id) { + self.next_idx += 1; + } + + Some((start_id, &self.inner[start_idx..self.next_idx])) + } +} + +/// Convert a duty roster, which is originally a Vec, where each +/// item corresponds to the same position in the session keys, into +/// a list containing (index, parachain duty) where indices are into the session keys. +/// This list is sorted ascending by parachain duty, just like the +/// parachain candidates are. +fn make_sorted_duties(duty: &[Chain]) -> Vec<(usize, ParaId)> { + let mut sorted_duties = Vec::with_capacity(duty.len()); + for (val_idx, duty) in duty.iter().enumerate() { + let id = match duty { + Chain::Relay => continue, + Chain::Parachain(id) => id, + }; + + let idx = sorted_duties.binary_search_by_key(&id, |&(_, ref id)| id) + .unwrap_or_else(|idx| idx); + + sorted_duties.insert(idx, (val_idx, *id)); + } + + sorted_duties +} + impl Module { /// Initialize the state of a new parachain/parathread. pub fn initialize_para( @@ -1107,79 +1170,78 @@ impl Module { T::ActiveParachains::active_paras() } - // check the attestations on these candidates. The candidates should have been checked - // that each candidates' chain ID is valid. - fn check_candidates( - schedule: &GlobalValidationSchedule, - attested_candidates: &[AttestedCandidate], - active_parachains: &[(ParaId, Option<(CollatorId, Retriable)>)] - ) -> sp_std::result::Result, sp_runtime::DispatchError> - { - // returns groups of slices that have the same chain ID. - // assumes the inner slice is sorted by id. - struct GroupedDutyIter<'a> { - next_idx: usize, - inner: &'a [(usize, ParaId)], - } - - impl<'a> GroupedDutyIter<'a> { - fn new(inner: &'a [(usize, ParaId)]) -> Self { - GroupedDutyIter { next_idx: 0, inner } - } + /// Verify the signatures of all candidates. + /// + /// Returns `false` if a signature is not correct. + fn verify_candidate_signatures( + candidate: &AttestedCandidate, + authorities: &[ValidatorId], + validator_group: &[(usize, ParaId)], + signing_context: &SigningContext, + ) -> DispatchResult { + let mut expected_votes_len = 0; + let mut encoded_implicit = None; + let mut encoded_explicit = None; + let candidate_hash = candidate.candidate().hash(); - fn group_for(&mut self, wanted_id: ParaId) -> Option<&'a [(usize, ParaId)]> { - while let Some((id, keys)) = self.next() { - if wanted_id == id { - return Some(keys) - } + for (vote_index, (auth_index, _)) in candidate.validator_indices + .iter() + .enumerate() + .filter(|(_, bit)| **bit) + .enumerate() + { + let validity_attestation = match candidate.validity_votes.get(vote_index) { + None => Err(Error::::NotEnoughValidityVotes)?, + Some(v) => { + expected_votes_len = vote_index + 1; + v } + }; - None + if validator_group.iter().find(|&(idx, _)| *idx == auth_index).is_none() { + Err(Error::::WrongValidatorAttesting)? } - } - impl<'a> Iterator for GroupedDutyIter<'a> { - type Item = (ParaId, &'a [(usize, ParaId)]); + let (payload, sig) = match validity_attestation { + ValidityAttestation::Implicit(sig) => { + let payload = encoded_implicit.get_or_insert_with(|| localized_payload( + Statement::Candidate(candidate_hash), signing_context, + )); - fn next(&mut self) -> Option { - if self.next_idx == self.inner.len() { return None } - let start_idx = self.next_idx; - self.next_idx += 1; - let start_id = self.inner[start_idx].1; + (payload, sig) + } + ValidityAttestation::Explicit(sig) => { + let payload = encoded_explicit.get_or_insert_with(|| localized_payload( + Statement::Valid(candidate_hash), signing_context, + )); - while self.inner.get(self.next_idx).map_or(false, |&(_, ref id)| id == &start_id) { - self.next_idx += 1; + (payload, sig) } + }; - Some((start_id, &self.inner[start_idx..self.next_idx])) - } + ensure!( + sig.verify(&payload[..], &authorities[auth_index]), + Error::::InvalidSignature, + ); } + if candidate.validity_votes.len() == expected_votes_len { + Ok(()) + } else { + Err(Error::::UntaggedVotes.into()) + } + } + + // check the attestations on these candidates. The candidates should have been checked + // that each candidates' chain ID is valid. + fn check_candidates( + schedule: &GlobalValidationSchedule, + attested_candidates: &[AttestedCandidate], + active_parachains: &[(ParaId, Option<(CollatorId, Retriable)>)] + ) -> sp_std::result::Result, sp_runtime::DispatchError> { let authorities = Self::authorities(); let (duty_roster, random_seed) = Self::calculate_duty_roster(); - // convert a duty roster, which is originally a Vec, where each - // item corresponds to the same position in the session keys, into - // a list containing (index, parachain duty) where indices are into the session keys. - // this list is sorted ascending by parachain duty, just like the - // parachain candidates are. - let make_sorted_duties = |duty: &[Chain]| { - let mut sorted_duties = Vec::with_capacity(duty.len()); - for (val_idx, duty) in duty.iter().enumerate() { - let id = match duty { - Chain::Relay => continue, - Chain::Parachain(id) => id, - }; - - let idx = sorted_duties.binary_search_by_key(&id, |&(_, ref id)| id) - .unwrap_or_else(|idx| idx); - - sorted_duties.insert(idx, (val_idx, *id)); - } - - sorted_duties - }; - // computes the omitted validation data for a particular parachain. // // pass the perceived relay chain height of the para-block. This is the block number of @@ -1297,58 +1359,9 @@ impl Module { T::ParachainCurrency::deduct(para_id, fees)?; - let candidate_hash = candidate.candidate().hash(); - let mut encoded_implicit = None; - let mut encoded_explicit = None; - - let mut expected_votes_len = 0; - for (vote_index, (auth_index, _)) in candidate.validator_indices - .iter() - .enumerate() - .filter(|(_, bit)| **bit) - .enumerate() - { - let validity_attestation = match candidate.validity_votes.get(vote_index) { - None => Err(Error::::NotEnoughValidityVotes)?, - Some(v) => { - expected_votes_len = vote_index + 1; - v - } - }; - - if validator_group.iter().find(|&(idx, _)| *idx == auth_index).is_none() { - Err(Error::::WrongValidatorAttesting)? - } - - let (payload, sig) = match validity_attestation { - ValidityAttestation::Implicit(sig) => { - let payload = encoded_implicit.get_or_insert_with(|| localized_payload( - Statement::Candidate(candidate_hash), - )); + Self::verify_candidate_signatures(candidate, &authorities, validator_group, &signing_context)?; - (payload, sig) - } - ValidityAttestation::Explicit(sig) => { - let payload = encoded_explicit.get_or_insert_with(|| localized_payload( - Statement::Valid(candidate_hash), - )); - - (payload, sig) - } - }; - - ensure!( - sig.verify(&payload[..], &authorities[auth_index]), - Error::::InvalidSignature, - ); - } - - ensure!( - candidate.validity_votes.len() == expected_votes_len, - Error::::UntaggedVotes - ); - - para_block_hashes.push(candidate_hash); + para_block_hashes.push(candidate.candidate.hash()); } Ok(IncludedBlocks { @@ -1360,6 +1373,25 @@ impl Module { }) } + /// Checks all signatures from all given `candidates`. + /// + /// Returns an error if any signature verification failed. + fn check_candidates_signatures(candidates: &[AttestedCandidate]) -> DispatchResult { + let authorities = Self::authorities(); + let duty_roster = Self::calculate_duty_roster().0; + let sorted_validators = make_sorted_duties(&duty_roster.validator_duty); + let signing_context = Self::signing_context(); + let mut validator_groups = GroupedDutyIter::new(&sorted_validators[..]); + + candidates.iter().try_for_each(|c| { + let para_id = c.parachain_index(); + let validator_group = validator_groups.group_for(para_id) + .ok_or(Error::::NoValidatorGroup)?; + + Self::verify_candidate_signatures(c, &authorities, validator_group, &signing_context) + }) + } + fn initialize_authorities(authorities: &[ValidatorId]) { if !authorities.is_empty() { assert!(Authorities::get().is_empty(), "Authorities are already initialized!"); @@ -1420,7 +1452,13 @@ impl ProvideInherent for Module { .expect("Parachain heads could not be decoded.") .expect("No parachain heads found in inherent data."); - Some(Call::set_heads(data)) + // Temporary solution for: + // https://github.com/paritytech/polkadot/issues/1327 + if Self::check_candidates_signatures(&data).is_ok() { + Some(Call::set_heads(data)) + } else { + Some(Call::set_heads(Vec::new())) + } } }