Skip to content

Commit

Permalink
Route blame on sign between processor and coordinator
Browse files Browse the repository at this point in the history 
Doesn't yet act on it in coordinator.
  • Loading branch information
@kayabaNerve
kayabaNerve committed Nov 12, 2023
1 parent 3e962b9 commit c247cbe
Show file tree
Hide file tree
Showing 4 changed files with 147 additions and 82 deletions.
18 changes: 17 additions & 1 deletion coordinator/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,10 @@ async fn handle_processor_message<D: Db, P: P2p>(
},
// TODO: Review replacing key with Session in messages?
ProcessorMessage::Sign(inner_msg) => match inner_msg {
// We'll only receive Preprocess and Share if we're actively signing
// We'll only receive InvalidParticipant/Preprocess/Share if we're actively signing
sign::ProcessorMessage::InvalidParticipant { id, .. } => {
Some(SubstrateDb::<D>::session_for_key(&txn, &id.key).unwrap())
}
sign::ProcessorMessage::Preprocess { id, .. } => {
Some(SubstrateDb::<D>::session_for_key(&txn, &id.key).unwrap())
}
Expand Down Expand Up @@ -264,6 +267,9 @@ async fn handle_processor_message<D: Db, P: P2p>(
None
}
// We'll only fire these if we are the Substrate signer, making the Tributary relevant
coordinator::ProcessorMessage::InvalidParticipant { id, .. } => {
Some(SubstrateDb::<D>::session_for_key(&txn, &id.key).unwrap())
}
coordinator::ProcessorMessage::BatchPreprocess { id, .. } => {
Some(SubstrateDb::<D>::session_for_key(&txn, &id.key).unwrap())
}
Expand Down Expand Up @@ -532,6 +538,11 @@ async fn handle_processor_message<D: Db, P: P2p>(
}
},
ProcessorMessage::Sign(msg) => match msg {
sign::ProcessorMessage::InvalidParticipant { .. } => {
// TODO: Locally increase slash points to maximum (distinct from an explicitly fatal
// slash) and censor transactions (yet don't explicitly ban)
vec![]
}
sign::ProcessorMessage::Preprocess { id, preprocesses } => {
if id.attempt == 0 {
MainDb::<D>::save_first_preprocess(
Expand Down Expand Up @@ -582,6 +593,11 @@ async fn handle_processor_message<D: Db, P: P2p>(
},
ProcessorMessage::Coordinator(inner_msg) => match inner_msg {
coordinator::ProcessorMessage::SubstrateBlockAck { .. } => unreachable!(),
coordinator::ProcessorMessage::InvalidParticipant { .. } => {
// TODO: Locally increase slash points to maximum (distinct from an explicitly fatal
// slash) and censor transactions (yet don't explicitly ban)
vec![]
}
coordinator::ProcessorMessage::BatchPreprocess { id, block, preprocesses } => {
log::info!(
"informed of batch (sign ID {}, attempt {}) for block {}",
Expand Down
19 changes: 12 additions & 7 deletions processor/messages/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,10 @@ pub mod sign {
}
}

#[derive(Clone, PartialEq, Eq, Debug, Zeroize, Encode, Decode, Serialize, Deserialize)]
#[derive(Clone, PartialEq, Eq, Debug, Zeroize, Serialize, Deserialize)]
pub enum ProcessorMessage {
// Participant sent an invalid message during the sign protocol.
InvalidParticipant { id: SignId, participant: Participant },
// Created preprocess for the specified signing protocol.
Preprocess { id: SignId, preprocesses: Vec<Vec<u8>> },
// Signed share for the specified signing protocol.
Expand Down Expand Up @@ -198,9 +200,10 @@ pub mod coordinator {
pub id: [u8; 32],
}

#[derive(Clone, PartialEq, Eq, Debug, Zeroize, Encode, Decode, Serialize, Deserialize)]
#[derive(Clone, PartialEq, Eq, Debug, Zeroize, Serialize, Deserialize)]
pub enum ProcessorMessage {
SubstrateBlockAck { network: NetworkId, block: u64, plans: Vec<PlanMeta> },
InvalidParticipant { id: BatchSignId, participant: Participant },
BatchPreprocess { id: BatchSignId, block: BlockHash, preprocesses: Vec<Vec<u8>> },
BatchShare { id: BatchSignId, shares: Vec<[u8; 32]> },
}
Expand Down Expand Up @@ -401,10 +404,11 @@ impl ProcessorMessage {
ProcessorMessage::Sign(msg) => {
let (sub, id) = match msg {
// Unique since SignId
sign::ProcessorMessage::Preprocess { id, .. } => (0, id.encode()),
sign::ProcessorMessage::Share { id, .. } => (1, id.encode()),
sign::ProcessorMessage::InvalidParticipant { id, .. } => (0, id.encode()),
sign::ProcessorMessage::Preprocess { id, .. } => (1, id.encode()),
sign::ProcessorMessage::Share { id, .. } => (2, id.encode()),
// Unique since a processor will only sign a TX once
sign::ProcessorMessage::Completed { id, .. } => (2, id.to_vec()),
sign::ProcessorMessage::Completed { id, .. } => (3, id.to_vec()),
};

let mut res = vec![PROCESSSOR_UID, TYPE_SIGN_UID, sub];
Expand All @@ -417,8 +421,9 @@ impl ProcessorMessage {
(0, (network, block).encode())
}
// Unique since BatchSignId
coordinator::ProcessorMessage::BatchPreprocess { id, .. } => (1, id.encode()),
coordinator::ProcessorMessage::BatchShare { id, .. } => (2, id.encode()),
coordinator::ProcessorMessage::InvalidParticipant { id, .. } => (1, id.encode()),
coordinator::ProcessorMessage::BatchPreprocess { id, .. } => (2, id.encode()),
coordinator::ProcessorMessage::BatchShare { id, .. } => (3, id.encode()),
};

let mut res = vec![PROCESSSOR_UID, TYPE_COORDINATOR_UID, sub];
Expand Down
96 changes: 59 additions & 37 deletions processor/src/signer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use rand_core::OsRng;

use ciphersuite::group::GroupEncoding;
use frost::{
ThresholdKeys,
ThresholdKeys, FrostError,
sign::{Writable, PreprocessMachine, SignMachine, SignatureMachine},
};

Expand Down Expand Up @@ -470,7 +470,7 @@ impl<N: Network, D: Db> Signer<N, D> {
msg: CoordinatorMessage,
) -> Option<ProcessorMessage> {
match msg {
CoordinatorMessage::Preprocesses { id, mut preprocesses } => {
CoordinatorMessage::Preprocesses { id, preprocesses } => {
if self.verify_id(&id).is_err() {
return None;
}
Expand All @@ -487,23 +487,22 @@ impl<N: Network, D: Db> Signer<N, D> {
Some(machine) => machine,
};

let preprocesses = match preprocesses
.drain()
.map(|(l, preprocess)| {
let mut preprocess_ref = preprocess.as_ref();
let res = machines[0]
.read_preprocess::<&[u8]>(&mut preprocess_ref)
.map(|preprocess| (l, preprocess));
if !preprocess_ref.is_empty() {
todo!("malicious signer: extra bytes");
}
res
})
.collect::<Result<HashMap<_, _>, _>>()
{
Ok(preprocesses) => preprocesses,
Err(e) => todo!("malicious signer: {:?}", e),
};
let mut parsed = HashMap::new();
for l in {
let mut keys = preprocesses.keys().cloned().collect::<Vec<_>>();
keys.sort();
keys
} {
let mut preprocess_ref = preprocesses.get(&l).unwrap().as_slice();
let Ok(res) = machines[0].read_preprocess(&mut preprocess_ref) else {
return Some(ProcessorMessage::InvalidParticipant { id, participant: l });
};
if !preprocess_ref.is_empty() {
return Some(ProcessorMessage::InvalidParticipant { id, participant: l });
}
parsed.insert(l, res);
}
let preprocesses = parsed;

// Only keep a single machine as we only need one to get the signature
let mut signature_machine = None;
Expand All @@ -520,7 +519,18 @@ impl<N: Network, D: Db> Signer<N, D> {
// Use an empty message, as expected of TransactionMachines
let (machine, share) = match machine.sign(preprocesses, &[]) {
Ok(res) => res,
Err(e) => todo!("malicious signer: {:?}", e),
Err(e) => match e {
FrostError::InternalError(_) |
FrostError::InvalidParticipant(_, _) |
FrostError::InvalidSigningSet(_) |
FrostError::InvalidParticipantQuantity(_, _) |
FrostError::DuplicatedParticipant(_) |
FrostError::MissingParticipant(_) => unreachable!(),

FrostError::InvalidPreprocess(l) | FrostError::InvalidShare(l) => {
return Some(ProcessorMessage::InvalidParticipant { id, participant: l })
}
},
};
if m == 0 {
signature_machine = Some(machine);
Expand All @@ -534,7 +544,7 @@ impl<N: Network, D: Db> Signer<N, D> {
Some(ProcessorMessage::Share { id, shares: serialized_shares })
}

CoordinatorMessage::Shares { id, mut shares } => {
CoordinatorMessage::Shares { id, shares } => {
if self.verify_id(&id).is_err() {
return None;
}
Expand All @@ -557,29 +567,41 @@ impl<N: Network, D: Db> Signer<N, D> {
Some(machine) => machine,
};

let mut shares = match shares
.drain()
.map(|(l, share)| {
let mut share_ref = share.as_ref();
let res = machine.read_share::<&[u8]>(&mut share_ref).map(|share| (l, share));
if !share_ref.is_empty() {
todo!("malicious signer: extra bytes");
}
res
})
.collect::<Result<HashMap<_, _>, _>>()
{
Ok(shares) => shares,
Err(e) => todo!("malicious signer: {:?}", e),
};
let mut parsed = HashMap::new();
for l in {
let mut keys = shares.keys().cloned().collect::<Vec<_>>();
keys.sort();
keys
} {
let mut share_ref = shares.get(&l).unwrap().as_slice();
let Ok(res) = machine.read_share(&mut share_ref) else {
return Some(ProcessorMessage::InvalidParticipant { id, participant: l });
};
if !share_ref.is_empty() {
return Some(ProcessorMessage::InvalidParticipant { id, participant: l });
}
parsed.insert(l, res);
}
let mut shares = parsed;

for (i, our_share) in our_shares.into_iter().enumerate().skip(1) {
assert!(shares.insert(self.keys[i].params().i(), our_share).is_none());
}

let tx = match machine.complete(shares) {
Ok(res) => res,
Err(e) => todo!("malicious signer: {:?}", e),
Err(e) => match e {
FrostError::InternalError(_) |
FrostError::InvalidParticipant(_, _) |
FrostError::InvalidSigningSet(_) |
FrostError::InvalidParticipantQuantity(_, _) |
FrostError::DuplicatedParticipant(_) |
FrostError::MissingParticipant(_) => unreachable!(),

FrostError::InvalidPreprocess(l) | FrostError::InvalidShare(l) => {
return Some(ProcessorMessage::InvalidParticipant { id, participant: l })
}
},
};

// Save the transaction in case it's needed for recovery
Expand Down
96 changes: 59 additions & 37 deletions processor/src/substrate_signer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rand_core::OsRng;
use ciphersuite::group::GroupEncoding;
use frost::{
curve::Ristretto,
ThresholdKeys,
ThresholdKeys, FrostError,
algorithm::Algorithm,
sign::{
Writable, PreprocessMachine, SignMachine, SignatureMachine, AlgorithmMachine,
Expand Down Expand Up @@ -246,7 +246,7 @@ impl<D: Db> SubstrateSigner<D> {
msg: CoordinatorMessage,
) -> Option<messages::ProcessorMessage> {
match msg {
CoordinatorMessage::BatchPreprocesses { id, mut preprocesses } => {
CoordinatorMessage::BatchPreprocesses { id, preprocesses } => {
if self.verify_id(&id).is_err() {
return None;
}
Expand All @@ -263,23 +263,22 @@ impl<D: Db> SubstrateSigner<D> {
Some(preprocess) => preprocess,
};

let preprocesses = match preprocesses
.drain()
.map(|(l, preprocess)| {
let mut preprocess_ref = preprocess.as_ref();
let res = machines[0]
.read_preprocess::<&[u8]>(&mut preprocess_ref)
.map(|preprocess| (l, preprocess));
if !preprocess_ref.is_empty() {
todo!("malicious signer: extra bytes");
}
res
})
.collect::<Result<HashMap<_, _>, _>>()
{
Ok(preprocesses) => preprocesses,
Err(e) => todo!("malicious signer: {:?}", e),
};
let mut parsed = HashMap::new();
for l in {
let mut keys = preprocesses.keys().cloned().collect::<Vec<_>>();
keys.sort();
keys
} {
let mut preprocess_ref = preprocesses.get(&l).unwrap().as_slice();
let Ok(res) = machines[0].read_preprocess(&mut preprocess_ref) else {
return Some((ProcessorMessage::InvalidParticipant { id, participant: l }).into());
};
if !preprocess_ref.is_empty() {
return Some((ProcessorMessage::InvalidParticipant { id, participant: l }).into());
}
parsed.insert(l, res);
}
let preprocesses = parsed;

// Only keep a single machine as we only need one to get the signature
let mut signature_machine = None;
Expand All @@ -296,7 +295,18 @@ impl<D: Db> SubstrateSigner<D> {
let (machine, share) =
match machine.sign(preprocesses, &batch_message(&self.signable[&id.id])) {
Ok(res) => res,
Err(e) => todo!("malicious signer: {:?}", e),
Err(e) => match e {
FrostError::InternalError(_) |
FrostError::InvalidParticipant(_, _) |
FrostError::InvalidSigningSet(_) |
FrostError::InvalidParticipantQuantity(_, _) |
FrostError::DuplicatedParticipant(_) |
FrostError::MissingParticipant(_) => unreachable!(),

FrostError::InvalidPreprocess(l) | FrostError::InvalidShare(l) => {
return Some((ProcessorMessage::InvalidParticipant { id, participant: l }).into())
}
},
};
if m == 0 {
signature_machine = Some(machine);
Expand All @@ -314,7 +324,7 @@ impl<D: Db> SubstrateSigner<D> {
Some((ProcessorMessage::BatchShare { id, shares: serialized_shares }).into())
}

CoordinatorMessage::BatchShares { id, mut shares } => {
CoordinatorMessage::BatchShares { id, shares } => {
if self.verify_id(&id).is_err() {
return None;
}
Expand All @@ -337,29 +347,41 @@ impl<D: Db> SubstrateSigner<D> {
Some(signing) => signing,
};

let mut shares = match shares
.drain()
.map(|(l, share)| {
let mut share_ref = share.as_ref();
let res = machine.read_share::<&[u8]>(&mut share_ref).map(|share| (l, share));
if !share_ref.is_empty() {
todo!("malicious signer: extra bytes");
}
res
})
.collect::<Result<HashMap<_, _>, _>>()
{
Ok(shares) => shares,
Err(e) => todo!("malicious signer: {:?}", e),
};
let mut parsed = HashMap::new();
for l in {
let mut keys = shares.keys().cloned().collect::<Vec<_>>();
keys.sort();
keys
} {
let mut share_ref = shares.get(&l).unwrap().as_slice();
let Ok(res) = machine.read_share(&mut share_ref) else {
return Some((ProcessorMessage::InvalidParticipant { id, participant: l }).into());
};
if !share_ref.is_empty() {
return Some((ProcessorMessage::InvalidParticipant { id, participant: l }).into());
}
parsed.insert(l, res);
}
let mut shares = parsed;

for (i, our_share) in our_shares.into_iter().enumerate().skip(1) {
assert!(shares.insert(self.keys[i].params().i(), our_share).is_none());
}

let sig = match machine.complete(shares) {
Ok(res) => res,
Err(e) => todo!("malicious signer: {:?}", e),
Err(e) => match e {
FrostError::InternalError(_) |
FrostError::InvalidParticipant(_, _) |
FrostError::InvalidSigningSet(_) |
FrostError::InvalidParticipantQuantity(_, _) |
FrostError::DuplicatedParticipant(_) |
FrostError::MissingParticipant(_) => unreachable!(),

FrostError::InvalidPreprocess(l) | FrostError::InvalidShare(l) => {
return Some((ProcessorMessage::InvalidParticipant { id, participant: l }).into())
}
},
};

info!("signed batch {} with attempt #{}", hex::encode(id.id), id.attempt);
Expand Down

0 comments on commit c247cbe

Please sign in to comment.