Merge branch 'paulliu/malicious-ecdsa-test' into 'master'

test(consensus): add malicious ecdsa test to consensus test framework CON-1075

We add `minority_maliciouly_ecdsa_dealers_would_pass`, a malicious ECDSA test to the consensus test framework. 

See merge request dfinity-lab/public/ic!17138

rs/consensus/Cargo.toml

@@ -13,7 +13,7 @@ ic-consensus-utils = { path = "./utils" }
 ic-constants = { path = "../constants" }
 ic-crypto = { path = "../crypto" }
 ic-crypto-prng = { path = "../crypto/prng" }
-ic-crypto-test-utils-canister-threshold-sigs = { path = "../crypto/test_utils/canister_threshold_sigs", optional = true }
+ic-crypto-test-utils-canister-threshold-sigs = { path = "../crypto/test_utils/canister_threshold_sigs" }
 ic-crypto-utils-threshold-sig-der = { path = "../crypto/utils/threshold_sig_der" }
 ic-error-types = { path = "../types/error_types" }
 ic-https-outcalls-consensus = { path = "../https_outcalls/consensus" }
@@ -78,4 +78,4 @@ harness = false
 
 [features]
 default = []
-malicious_code = ["ic-crypto-test-utils-canister-threshold-sigs"]
+malicious_code = []

rs/consensus/src/ecdsa.rs

@@ -205,8 +205,7 @@ use std::sync::Arc;
 use std::time::{Duration, Instant};
 
 pub(crate) mod complaints;
-#[cfg(any(feature = "malicious_code", test))]
-pub(crate) mod malicious_pre_signer;
+pub mod malicious_pre_signer;
 pub(crate) mod payload_builder;
 pub(crate) mod payload_verifier;
 pub(crate) mod pre_signer;
@@ -230,7 +229,8 @@ pub const INACTIVE_TRANSCRIPT_PURGE_SECS: Duration = Duration::from_secs(60);
 /// `EcdsaImpl` is the consensus component responsible for processing threshold
 /// ECDSA payloads.
 pub struct EcdsaImpl {
-    pre_signer: Box<EcdsaPreSignerImpl>,
+    /// The Pre-Signer subcomponent
+    pub pre_signer: Box<EcdsaPreSignerImpl>,
     signer: Box<dyn EcdsaSigner>,
     complaint_handler: Box<dyn EcdsaComplaintHandler>,
     consensus_block_cache: Arc<dyn ConsensusBlockCache>,

rs/consensus/src/ecdsa/pre_signer.rs

@@ -37,7 +37,8 @@ pub(crate) trait EcdsaPreSigner: Send {
     ) -> EcdsaChangeSet;
 }
 
-pub(crate) struct EcdsaPreSignerImpl {
+/// Pre-Signer subcomponent.
+pub struct EcdsaPreSignerImpl {
     pub(crate) node_id: NodeId,
     pub(crate) consensus_block_cache: Arc<dyn ConsensusBlockCache>,
     pub(crate) crypto: Arc<dyn ConsensusCrypto>,

rs/consensus/tests/framework/driver.rs

@@ -9,7 +9,7 @@ use ic_interfaces::{
     certification,
     consensus_pool::{ChangeAction, ChangeSet as ConsensusChangeSet},
     dkg::ChangeAction as DkgChangeAction,
-    ecdsa::EcdsaChangeAction,
+    ecdsa::{EcdsaChangeAction, EcdsaChangeSet},
     p2p::consensus::{ChangeSetProducer, MutablePool},
 };
 use ic_logger::{debug, ReplicaLogger};
@@ -30,7 +30,7 @@ impl<'a> ConsensusDriver<'a> {
         consensus: Box<dyn ChangeSetProducer<ConsensusPoolImpl, ChangeSet = ConsensusChangeSet>>,
         consensus_gossip: ConsensusGossipImpl,
         dkg: ic_consensus::dkg::DkgImpl,
-        ecdsa: ic_consensus::ecdsa::EcdsaImpl,
+        ecdsa: Box<dyn ChangeSetProducer<EcdsaPoolImpl, ChangeSet = EcdsaChangeSet>>,
         certifier: Box<
             dyn ChangeSetProducer<CertificationPoolImpl, ChangeSet = certification::ChangeSet> + 'a,
         >,

rs/consensus/tests/framework/malicious.rs

@@ -1,16 +1,19 @@
 //! Implementation of malicious behaviors in consensus.
 
-use super::ConsensusModifier;
+use super::ComponentModifier;
 use ic_consensus::consensus::ConsensusImpl;
+use ic_consensus::ecdsa::{malicious_pre_signer, EcdsaImpl};
 use ic_consensus_utils::pool_reader::PoolReader;
 use ic_interfaces::{
     consensus_pool::{ChangeAction::*, ChangeSet, ConsensusPool, ValidatedConsensusArtifact},
+    ecdsa::{EcdsaChangeSet, EcdsaPool},
     p2p::consensus::ChangeSetProducer,
 };
 use ic_protobuf::types::v1 as pb;
 use ic_types::consensus::{ConsensusMessageHashable, NotarizationShare};
 use ic_types::malicious_flags::MaliciousFlags;
 use ic_types::time::current_time;
+use std::cell::RefCell;
 
 /// Simulate a malicious notary that always produces a bad NotarizationShare
 /// by mutating the signature.
@@ -48,8 +51,13 @@ impl<T: ConsensusPool> ChangeSetProducer<T> for InvalidNotaryShareSignature {
     }
 }
 
-pub fn invalid_notary_share_signature() -> ConsensusModifier {
-    Box::new(|consensus: ConsensusImpl| Box::new(InvalidNotaryShareSignature { consensus }))
+pub fn invalid_notary_share_signature() -> ComponentModifier {
+    ComponentModifier {
+        consensus: Box::new(|consensus: ConsensusImpl| {
+            Box::new(InvalidNotaryShareSignature { consensus })
+        }),
+        ..Default::default()
+    }
 }
 
 /// Simulate a non-responding notary that does not sign on notary shares.
@@ -74,17 +82,20 @@ impl<T: ConsensusPool> ChangeSetProducer<T> for AbsentNotaryShare {
     }
 }
 
-pub fn absent_notary_share() -> ConsensusModifier {
-    Box::new(|consensus: ConsensusImpl| Box::new(AbsentNotaryShare { consensus }))
+pub fn absent_notary_share() -> ComponentModifier {
+    ComponentModifier {
+        consensus: Box::new(|consensus: ConsensusImpl| Box::new(AbsentNotaryShare { consensus })),
+        ..Default::default()
+    }
 }
 
-/// Simulate a malicious behavior via MaliciousFlags.
-pub struct WithMaliciousFlags {
+/// Simulate a malicious consensus behavior via MaliciousFlags.
+pub struct ConsensusWithMaliciousFlags {
     consensus: ConsensusImpl,
     malicious_flags: MaliciousFlags,
 }
 
-impl<T: ConsensusPool> ChangeSetProducer<T> for WithMaliciousFlags {
+impl<T: ConsensusPool> ChangeSetProducer<T> for ConsensusWithMaliciousFlags {
     type ChangeSet = ChangeSet;
     fn on_state_change(&self, pool: &T) -> ChangeSet {
         let changeset = self.consensus.on_state_change(pool);
@@ -106,11 +117,46 @@ impl<T: ConsensusPool> ChangeSetProducer<T> for WithMaliciousFlags {
     }
 }
 
-pub fn with_malicious_flags(malicious_flags: MaliciousFlags) -> super::ConsensusModifier {
-    Box::new(move |consensus: ConsensusImpl| {
-        Box::new(WithMaliciousFlags {
-            consensus,
-            malicious_flags: malicious_flags.clone(),
+/// Simulate a malicious ecdsa behavior via MaliciousFlags.
+pub struct EcdsaWithMaliciousFlags {
+    ecdsa: RefCell<EcdsaImpl>,
+    malicious_flags: MaliciousFlags,
+}
+
+impl<T: EcdsaPool> ChangeSetProducer<T> for EcdsaWithMaliciousFlags {
+    type ChangeSet = EcdsaChangeSet;
+    fn on_state_change(&self, pool: &T) -> EcdsaChangeSet {
+        let changeset = EcdsaImpl::on_state_change(&self.ecdsa.borrow(), pool);
+        if self.malicious_flags.is_ecdsa_malicious() {
+            malicious_pre_signer::maliciously_alter_changeset(
+                changeset,
+                &self.ecdsa.borrow().pre_signer,
+                &self.malicious_flags,
+            )
+        } else {
+            changeset
+        }
+    }
+}
+
+pub fn with_malicious_flags(malicious_flags: MaliciousFlags) -> ComponentModifier {
+    let mut modifier = ComponentModifier::default();
+    let malicious_flags_clone = malicious_flags.clone();
+    if malicious_flags_clone.is_consensus_malicious() {
+        modifier.consensus = Box::new(move |consensus: ConsensusImpl| {
+            Box::new(ConsensusWithMaliciousFlags {
+                consensus,
+                malicious_flags: malicious_flags_clone.clone(),
+            })
+        })
+    };
+    if malicious_flags.is_ecdsa_malicious() {
+        modifier.ecdsa = Box::new(move |ecdsa: EcdsaImpl| {
+            Box::new(EcdsaWithMaliciousFlags {
+                ecdsa: RefCell::new(ecdsa),
+                malicious_flags: malicious_flags.clone(),
+            })
         })
-    })
+    };
+    modifier
 }

rs/consensus/tests/framework/mod.rs

@@ -9,7 +9,7 @@ mod types;
 
 pub use runner::ConsensusRunner;
 pub use types::{
-    ConsensusDependencies, ConsensusDriver, ConsensusInstance, ConsensusModifier,
+    ComponentModifier, ConsensusDependencies, ConsensusDriver, ConsensusInstance,
     ConsensusRunnerConfig, StopPredicate,
 };
 

rs/consensus/tests/framework/runner.rs

@@ -129,7 +129,7 @@ impl<'a> ConsensusRunner<'a> {
         membership: Arc<Membership>,
         consensus_crypto: Arc<dyn ConsensusCrypto>,
         certification_crypto: Arc<dyn CertificationCrypto>,
-        modifier: Option<ConsensusModifier>,
+        modifier: Option<ComponentModifier>,
         deps: &'a ConsensusDependencies,
         pool_config: ArtifactPoolConfig,
         pool_reader: &PoolReader<'_>,
@@ -212,10 +212,10 @@ impl<'a> ConsensusRunner<'a> {
             driver: ConsensusDriver::new(
                 node_id,
                 pool_config,
-                modifier.unwrap_or_else(|| Box::new(|x| Box::new(x)))(consensus),
+                apply_modifier_consensus(&modifier, consensus),
                 consensus_gossip,
                 dkg,
-                ecdsa,
+                apply_modifier_ecdsa(&modifier, ecdsa),
                 Box::new(certifier),
                 deps.consensus_pool.clone(),
                 deps.dkg_pool.clone(),

rs/consensus/tests/framework/types.rs

@@ -13,6 +13,7 @@ use ic_interfaces::{
     batch_payload::BatchPayloadBuilder,
     certification::ChangeSet,
     consensus_pool::ChangeSet as ConsensusChangeSet,
+    ecdsa::EcdsaChangeSet,
     ingress_manager::IngressSelector,
     messaging::XNetPayloadBuilder,
     p2p::consensus::{ChangeSetProducer, PriorityFnAndFilterProducer},
@@ -292,12 +293,50 @@ impl<Artifact: ArtifactKind> PriorityFnState<Artifact> {
     }
 }
 
-/// Consensus modifier that can potentially change its behavior.
-pub type ConsensusModifier = Box<
-    dyn Fn(
-        ConsensusImpl,
-    ) -> Box<dyn ChangeSetProducer<ConsensusPoolImpl, ChangeSet = ConsensusChangeSet>>,
->;
+/// Modifier that can potentially change a component's behavior.
+pub struct ComponentModifier {
+    pub(crate) consensus: Box<
+        dyn Fn(
+            ConsensusImpl,
+        )
+            -> Box<dyn ChangeSetProducer<ConsensusPoolImpl, ChangeSet = ConsensusChangeSet>>,
+    >,
+    pub(crate) ecdsa: Box<
+        dyn Fn(
+            ecdsa::EcdsaImpl,
+        )
+            -> Box<dyn ChangeSetProducer<ecdsa_pool::EcdsaPoolImpl, ChangeSet = EcdsaChangeSet>>,
+    >,
+}
+
+impl Default for ComponentModifier {
+    fn default() -> Self {
+        Self {
+            consensus: Box::new(|x: ConsensusImpl| Box::new(x)),
+            ecdsa: Box::new(|x: ecdsa::EcdsaImpl| Box::new(x)),
+        }
+    }
+}
+
+pub fn apply_modifier_consensus(
+    modifier: &Option<ComponentModifier>,
+    consensus: ConsensusImpl,
+) -> Box<dyn ChangeSetProducer<ConsensusPoolImpl, ChangeSet = ConsensusChangeSet>> {
+    match modifier {
+        Some(f) => (f.consensus)(consensus),
+        _ => Box::new(consensus),
+    }
+}
+
+pub fn apply_modifier_ecdsa(
+    modifier: &Option<ComponentModifier>,
+    ecdsa: ecdsa::EcdsaImpl,
+) -> Box<dyn ChangeSetProducer<ecdsa_pool::EcdsaPoolImpl, ChangeSet = EcdsaChangeSet>> {
+    match modifier {
+        Some(f) => (f.ecdsa)(ecdsa),
+        _ => Box::new(ecdsa),
+    }
+}
 
 /// A ConsensusDriver mainly consists of the consensus component, and the
 /// consensus artifact pool and timer.
@@ -306,7 +345,8 @@ pub struct ConsensusDriver<'a> {
         Box<dyn ChangeSetProducer<ConsensusPoolImpl, ChangeSet = ConsensusChangeSet>>,
     pub(crate) consensus_gossip: ConsensusGossipImpl,
     pub(crate) dkg: dkg::DkgImpl,
-    pub(crate) ecdsa: ecdsa::EcdsaImpl,
+    pub(crate) ecdsa:
+        Box<dyn ChangeSetProducer<ecdsa_pool::EcdsaPoolImpl, ChangeSet = EcdsaChangeSet>>,
     pub(crate) certifier:
         Box<dyn ChangeSetProducer<CertificationPoolImpl, ChangeSet = ChangeSet> + 'a>,
     pub(crate) logger: ReplicaLogger,