From 8961628e7c4597f0c76417fd8bd8995eeaf6e33d Mon Sep 17 00:00:00 2001
From: Andronik Ordian <write@reusable.software>
Date: Fri, 9 Apr 2021 19:31:34 +0200
Subject: [PATCH] bitfield-dist: check signatures once (#2868)
---
.../network/bitfield-distribution/src/lib.rs | 101 ++++++++++++------
1 file changed, 71 insertions(+), 30 deletions(-)
diff --git a/polkadot/node/network/bitfield-distribution/src/lib.rs b/polkadot/node/network/bitfield-distribution/src/lib.rs
index e53759b1eb2..8437fc41226 100644
--- a/polkadot/node/network/bitfield-distribution/src/lib.rs
+++ b/polkadot/node/network/bitfield-distribution/src/lib.rs
@@ -462,32 +462,35 @@ where
return;
};
+ let one_per_validator = &mut (job_data.one_per_validator);
+
+ // only relay_message a message of a validator once
+ if let Some(old_message) = one_per_validator.get(&validator) {
+ tracing::trace!(
+ target: LOG_TARGET,
+ validator_index,
+ "already received a message for validator",
+ );
+ if old_message.signed_availability == message.signed_availability {
+ modify_reputation(ctx, origin, BENEFIT_VALID_MESSAGE).await;
+ }
+ return;
+ }
if message
.signed_availability
.check_signature(&signing_context, &validator)
- .is_ok()
+ .is_err()
{
- metrics.on_bitfield_received();
- let one_per_validator = &mut (job_data.one_per_validator);
+ modify_reputation(ctx, origin, COST_SIGNATURE_INVALID).await;
+ return;
+ }
- // only relay_message a message of a validator once
- if one_per_validator.get(&validator).is_some() {
- tracing::trace!(
- target: LOG_TARGET,
- validator_index,
- "already received a message for validator",
- );
- modify_reputation(ctx, origin, BENEFIT_VALID_MESSAGE).await;
- return;
- }
- one_per_validator.insert(validator.clone(), message.clone());
+ metrics.on_bitfield_received();
+ one_per_validator.insert(validator.clone(), message.clone());
- relay_message(ctx, job_data, &mut state.peer_views, validator, message).await;
+ relay_message(ctx, job_data, &mut state.peer_views, validator, message).await;
- modify_reputation(ctx, origin, BENEFIT_VALID_MESSAGE_FIRST).await
- } else {
- modify_reputation(ctx, origin, COST_SIGNATURE_INVALID).await
- }
+ modify_reputation(ctx, origin, BENEFIT_VALID_MESSAGE_FIRST).await
}
/// Deal with network bridge updates and track what needs to be tracked
@@ -926,22 +929,47 @@ mod test {
// another validator not part of the validatorset
let keystore : SyncCryptoStorePtr = Arc::new(KeyStore::new());
let malicious = SyncCryptoStore::sr25519_generate_new(&*keystore, ValidatorId::ID, None)
- .expect("Malicious key created");
- let validator = SyncCryptoStore::sr25519_generate_new(&*keystore, ValidatorId::ID, None)
- .expect("Malicious key created");
+ .expect("Malicious key created");
+ let validator_0 = SyncCryptoStore::sr25519_generate_new(&*keystore, ValidatorId::ID, None)
+ .expect("key created");
+ let validator_1 = SyncCryptoStore::sr25519_generate_new(&*keystore, ValidatorId::ID, None)
+ .expect("key created");
let payload = AvailabilityBitfield(bitvec![bitvec::order::Lsb0, u8; 1u8; 32]);
- let signed = executor::block_on(Signed::<AvailabilityBitfield>::sign(
+ let invalid_signed = executor::block_on(Signed::<AvailabilityBitfield>::sign(
&keystore,
- payload,
+ payload.clone(),
&signing_context,
ValidatorIndex(0),
&malicious.into(),
)).ok().flatten().expect("should be signed");
+ let invalid_signed_2 = executor::block_on(Signed::<AvailabilityBitfield>::sign(
+ &keystore,
+ payload.clone(),
+ &signing_context,
+ ValidatorIndex(1),
+ &malicious.into(),
+ )).ok().flatten().expect("should be signed");
- let msg = BitfieldGossipMessage {
+ let valid_signed = executor::block_on(Signed::<AvailabilityBitfield>::sign(
+ &keystore,
+ payload,
+ &signing_context,
+ ValidatorIndex(0),
+ &validator_0.into(),
+ )).ok().flatten().expect("should be signed");
+
+ let invalid_msg = BitfieldGossipMessage {
relay_parent: hash_a.clone(),
- signed_availability: signed.clone(),
+ signed_availability: invalid_signed.clone(),
+ };
+ let invalid_msg_2 = BitfieldGossipMessage {
+ relay_parent: hash_a.clone(),
+ signed_availability: invalid_signed_2.clone(),
+ };
+ let valid_msg = BitfieldGossipMessage {
+ relay_parent: hash_a.clone(),
+ signed_availability: valid_signed.clone(),
};
let pool = sp_core::testing::TaskExecutor::new();
@@ -949,21 +977,34 @@ mod test {
make_subsystem_context::<BitfieldDistributionMessage, _>(pool);
let mut state = prewarmed_state(
- validator.into(),
+ validator_0.into(),
signing_context.clone(),
- msg.clone(),
+ valid_msg,
vec![peer_b.clone()],
);
+ state.per_relay_parent.get_mut(&hash_a)
+ .unwrap()
+ .validator_set
+ .push(validator_1.into());
executor::block_on(async move {
launch!(handle_network_msg(
&mut ctx,
&mut state,
&Default::default(),
- NetworkBridgeEvent::PeerMessage(peer_b.clone(), msg.into_network_message()),
+ NetworkBridgeEvent::PeerMessage(peer_b.clone(), invalid_msg.into_network_message()),
));
- // reputation change due to invalid validator index
+ // reputation doesn't change due to one_job_per_validator check
+ assert!(handle.recv().timeout(Duration::from_millis(10)).await.is_none());
+
+ launch!(handle_network_msg(
+ &mut ctx,
+ &mut state,
+ &Default::default(),
+ NetworkBridgeEvent::PeerMessage(peer_b.clone(), invalid_msg_2.into_network_message()),
+ ));
+ // reputation change due to invalid signature
assert_matches!(
handle.recv().await,
AllMessages::NetworkBridge(
--
GitLab