From 8a17c614f0a737252ed80abce3323fa811422564 Mon Sep 17 00:00:00 2001
From: Robert Habermeier <rphmeier@gmail.com>
Date: Thu, 10 Mar 2022 14:49:35 -0600
Subject: [PATCH] Check signatures as "Compact" in statement distribution
(#5071)
* allow converting payloads _up_
* convert to superpayload in statement-distribution
* Update primitives/src/v2/signed.rs
Co-authored-by: Andronik <write@reusable.software>
Co-authored-by: Andronik <write@reusable.software>
---
polkadot/node/network/protocol/src/lib.rs | 8 +++
.../network/statement-distribution/src/lib.rs | 70 ++++++++++++++-----
.../statement-distribution/src/tests.rs | 32 ++++++---
polkadot/primitives/src/v2/mod.rs | 2 +
polkadot/primitives/src/v2/signed.rs | 24 +++++++
5 files changed, 109 insertions(+), 27 deletions(-)
diff --git a/polkadot/node/network/protocol/src/lib.rs b/polkadot/node/network/protocol/src/lib.rs
index e97cb882039..ae92bc0f7f2 100644
--- a/polkadot/node/network/protocol/src/lib.rs
+++ b/polkadot/node/network/protocol/src/lib.rs
@@ -357,6 +357,14 @@ pub mod v1 {
}
}
+ /// Get the signature from the statement.
+ pub fn get_signature(&self) -> ValidatorSignature {
+ match self {
+ Self::Statement(_, statement) => statement.unchecked_signature().clone(),
+ Self::LargeStatement(metadata) => metadata.signature.clone(),
+ }
+ }
+
/// Get contained relay parent.
pub fn get_relay_parent(&self) -> Hash {
match self {
diff --git a/polkadot/node/network/statement-distribution/src/lib.rs b/polkadot/node/network/statement-distribution/src/lib.rs
index 702e2db6a71..f68860c1529 100644
--- a/polkadot/node/network/statement-distribution/src/lib.rs
+++ b/polkadot/node/network/statement-distribution/src/lib.rs
@@ -36,7 +36,8 @@ use polkadot_node_subsystem_util::{self as util, MIN_GOSSIP_PEERS};
use polkadot_primitives::v2::{
AuthorityDiscoveryId, CandidateHash, CommittedCandidateReceipt, CompactStatement, Hash,
- SigningContext, ValidatorId, ValidatorIndex, ValidatorSignature,
+ SignedStatement, SigningContext, UncheckedSignedStatement, ValidatorId, ValidatorIndex,
+ ValidatorSignature,
};
use polkadot_subsystem::{
jaeger,
@@ -780,10 +781,10 @@ impl ActiveHeadData {
/// without modifying the internal state.
fn check_useful_or_unknown(
&self,
- statement: &UncheckedSignedFullStatement,
+ statement: &UncheckedSignedStatement,
) -> std::result::Result<(), DeniedStatement> {
let validator_index = statement.unchecked_validator_index();
- let compact = statement.unchecked_payload().to_compact();
+ let compact = statement.unchecked_payload();
let comparator = StoredStatementComparator {
compact: compact.clone(),
validator_index,
@@ -857,8 +858,8 @@ impl ActiveHeadData {
fn check_statement_signature(
head: &ActiveHeadData,
relay_parent: Hash,
- statement: UncheckedSignedFullStatement,
-) -> std::result::Result<SignedFullStatement, UncheckedSignedFullStatement> {
+ statement: UncheckedSignedStatement,
+) -> std::result::Result<SignedStatement, UncheckedSignedStatement> {
let signing_context =
SigningContext { session_index: head.session_index, parent_hash: relay_parent };
@@ -1387,24 +1388,57 @@ async fn handle_incoming_message<'a>(
return None
}
+ let checked_compact = {
+ let (compact, validator_index) = message.get_fingerprint();
+ let signature = message.get_signature();
+
+ let unchecked_compact = UncheckedSignedStatement::new(compact, validator_index, signature);
+
+ match active_head.check_useful_or_unknown(&unchecked_compact) {
+ Ok(()) => {},
+ Err(DeniedStatement::NotUseful) => return None,
+ Err(DeniedStatement::UsefulButKnown) => {
+ report_peer(ctx, peer, BENEFIT_VALID_STATEMENT).await;
+ return None
+ },
+ }
+
+ // check the signature on the statement.
+ match check_statement_signature(&active_head, relay_parent, unchecked_compact) {
+ Err(statement) => {
+ tracing::debug!(
+ target: LOG_TARGET,
+ ?peer,
+ ?statement,
+ "Invalid statement signature"
+ );
+ report_peer(ctx, peer, COST_INVALID_SIGNATURE).await;
+ return None
+ },
+ Ok(statement) => statement,
+ }
+ };
+
+ // Fetch from the network only after signature and usefulness checks are completed.
+ let is_large_statement = message.is_large_statement();
let statement =
retrieve_statement_from_message(peer, message, active_head, ctx, req_sender, metrics)
.await?;
- match active_head.check_useful_or_unknown(&statement) {
- Ok(()) => {},
- Err(DeniedStatement::NotUseful) => return None,
- Err(DeniedStatement::UsefulButKnown) => {
- report_peer(ctx, peer, BENEFIT_VALID_STATEMENT).await;
- return None
- },
- }
+ let payload = statement.unchecked_into_payload();
- // check the signature on the statement.
- let statement = match check_statement_signature(&active_head, relay_parent, statement) {
- Err(statement) => {
- tracing::debug!(target: LOG_TARGET, ?peer, ?statement, "Invalid statement signature");
- report_peer(ctx, peer, COST_INVALID_SIGNATURE).await;
+ // Upgrade the `Signed` wrapper from the compact payload to the full payload.
+ // This fails if the payload doesn't encode correctly.
+ let statement: SignedFullStatement = match checked_compact.convert_to_superpayload(payload) {
+ Err((compact, _)) => {
+ tracing::debug!(
+ target: LOG_TARGET,
+ ?peer,
+ ?compact,
+ is_large_statement,
+ "Full statement had bad payload."
+ );
+ report_peer(ctx, peer, COST_WRONG_HASH).await;
return None
},
Ok(statement) => statement,
diff --git a/polkadot/node/network/statement-distribution/src/tests.rs b/polkadot/node/network/statement-distribution/src/tests.rs
index 5e885e71e0c..eda99cae1d3 100644
--- a/polkadot/node/network/statement-distribution/src/tests.rs
+++ b/polkadot/node/network/statement-distribution/src/tests.rs
@@ -105,14 +105,16 @@ fn active_head_accepts_only_2_seconded_per_validator() {
.ok()
.flatten()
.expect("should be signed");
- assert!(head_data.check_useful_or_unknown(&a_seconded_val_0.clone().into()).is_ok());
+ assert!(head_data
+ .check_useful_or_unknown(&a_seconded_val_0.clone().convert_payload().into())
+ .is_ok());
let noted = head_data.note_statement(a_seconded_val_0.clone());
assert_matches!(noted, NotedStatement::Fresh(_));
// note A (duplicate)
assert_eq!(
- head_data.check_useful_or_unknown(&a_seconded_val_0.clone().into()),
+ head_data.check_useful_or_unknown(&a_seconded_val_0.clone().convert_payload().into()),
Err(DeniedStatement::UsefulButKnown),
);
let noted = head_data.note_statement(a_seconded_val_0);
@@ -130,7 +132,9 @@ fn active_head_accepts_only_2_seconded_per_validator() {
.ok()
.flatten()
.expect("should be signed");
- assert!(head_data.check_useful_or_unknown(&statement.clone().into()).is_ok());
+ assert!(head_data
+ .check_useful_or_unknown(&statement.clone().convert_payload().into())
+ .is_ok());
let noted = head_data.note_statement(statement);
assert_matches!(noted, NotedStatement::Fresh(_));
@@ -146,7 +150,7 @@ fn active_head_accepts_only_2_seconded_per_validator() {
.flatten()
.expect("should be signed");
assert_eq!(
- head_data.check_useful_or_unknown(&statement.clone().into()),
+ head_data.check_useful_or_unknown(&statement.clone().convert_payload().into()),
Err(DeniedStatement::NotUseful),
);
let noted = head_data.note_statement(statement);
@@ -163,7 +167,9 @@ fn active_head_accepts_only_2_seconded_per_validator() {
.ok()
.flatten()
.expect("should be signed");
- assert!(head_data.check_useful_or_unknown(&statement.clone().into()).is_ok());
+ assert!(head_data
+ .check_useful_or_unknown(&statement.clone().convert_payload().into())
+ .is_ok());
let noted = head_data.note_statement(statement);
assert_matches!(noted, NotedStatement::Fresh(_));
@@ -178,7 +184,9 @@ fn active_head_accepts_only_2_seconded_per_validator() {
.ok()
.flatten()
.expect("should be signed");
- assert!(head_data.check_useful_or_unknown(&statement.clone().into()).is_ok());
+ assert!(head_data
+ .check_useful_or_unknown(&statement.clone().convert_payload().into())
+ .is_ok());
let noted = head_data.note_statement(statement);
assert_matches!(noted, NotedStatement::Fresh(_));
}
@@ -428,7 +436,9 @@ fn peer_view_update_sends_messages() {
.ok()
.flatten()
.expect("should be signed");
- assert!(data.check_useful_or_unknown(&statement.clone().into()).is_ok());
+ assert!(data
+ .check_useful_or_unknown(&statement.clone().convert_payload().into())
+ .is_ok());
let noted = data.note_statement(statement);
assert_matches!(noted, NotedStatement::Fresh(_));
@@ -443,7 +453,9 @@ fn peer_view_update_sends_messages() {
.ok()
.flatten()
.expect("should be signed");
- assert!(data.check_useful_or_unknown(&statement.clone().into()).is_ok());
+ assert!(data
+ .check_useful_or_unknown(&statement.clone().convert_payload().into())
+ .is_ok());
let noted = data.note_statement(statement);
assert_matches!(noted, NotedStatement::Fresh(_));
@@ -458,7 +470,9 @@ fn peer_view_update_sends_messages() {
.ok()
.flatten()
.expect("should be signed");
- assert!(data.check_useful_or_unknown(&statement.clone().into()).is_ok());
+ assert!(data
+ .check_useful_or_unknown(&statement.clone().convert_payload().into())
+ .is_ok());
let noted = data.note_statement(statement);
assert_matches!(noted, NotedStatement::Fresh(_));
diff --git a/polkadot/primitives/src/v2/mod.rs b/polkadot/primitives/src/v2/mod.rs
index 36a9ad35ac1..9d47a90366d 100644
--- a/polkadot/primitives/src/v2/mod.rs
+++ b/polkadot/primitives/src/v2/mod.rs
@@ -660,6 +660,8 @@ impl From<BitVec<u8, bitvec::order::Lsb0>> for AvailabilityBitfield {
/// A signed compact statement, suitable to be sent to the chain.
pub type SignedStatement = Signed<CompactStatement>;
+/// A signed compact statement, with signature not yet checked.
+pub type UncheckedSignedStatement = UncheckedSigned<CompactStatement>;
/// A bitfield signed by a particular validator about the availability of pending candidates.
pub type SignedAvailabilityBitfield = Signed<AvailabilityBitfield>;
diff --git a/polkadot/primitives/src/v2/signed.rs b/polkadot/primitives/src/v2/signed.rs
index 9e199033a61..e58a09a0ea7 100644
--- a/polkadot/primitives/src/v2/signed.rs
+++ b/polkadot/primitives/src/v2/signed.rs
@@ -148,6 +148,30 @@ impl<Payload: EncodeAs<RealPayload>, RealPayload: Encode> Signed<Payload, RealPa
{
Signed(self.0.unchecked_convert_payload())
}
+
+ /// Convert `Payload` into some claimed `SuperPayload` if the encoding matches.
+ ///
+ /// Succeeds if and only if the super-payload provided actually encodes as
+ /// the expected payload.
+ pub fn convert_to_superpayload<SuperPayload>(
+ self,
+ claimed: SuperPayload,
+ ) -> Result<Signed<SuperPayload, RealPayload>, (Self, SuperPayload)>
+ where
+ SuperPayload: EncodeAs<RealPayload>,
+ Payload: Encode,
+ {
+ if claimed.encode_as() == self.0.payload.encode_as() {
+ Ok(Signed(UncheckedSigned {
+ payload: claimed,
+ validator_index: self.0.validator_index,
+ signature: self.0.signature,
+ real_payload: sp_std::marker::PhantomData,
+ }))
+ } else {
+ Err((self, claimed))
+ }
+ }
}
// We can't bound this on `Payload: Into<RealPayload>` because that conversion consumes
--
GitLab