Skip to content

Commit

Permalink
add coordinator checks
Browse files Browse the repository at this point in the history
  • Loading branch information
xoloki committed Nov 7, 2024
1 parent 0169c62 commit f36a68e
Show file tree
Hide file tree
Showing 3 changed files with 137 additions and 4 deletions.
4 changes: 4 additions & 0 deletions signer/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,10 @@ pub enum Error {
/// The smart contract has already been deployed
#[error("smart contract already deployed, contract name: {0}")]
ContractAlreadyDeployed(&'static str),

/// Received coordinator message wasn't from coordinator for this chain tip
#[error("not chain tip coordinator")]
NotChainTipCoordinator,
}

impl From<std::convert::Infallible> for Error {
Expand Down
88 changes: 88 additions & 0 deletions signer/src/transaction_coordinator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -787,6 +787,13 @@ where
coordinator_state_machine: &mut CoordinatorStateMachine,
txid: bitcoin::Txid,
) -> Result<WstsOperationResult, Error> {
// this assumes that the signer set doesn't change for the duration of this call,
// but we're already assuming that the bitcoin chain tip doesn't change
// alternately we could hit the DB every time we get a new message
let (_, signer_set) = self
.get_signer_set_and_aggregate_key(bitcoin_chain_tip)
.await?;

loop {
// Let's get the next message from the network or the
// TxSignerEventLoop.
Expand All @@ -797,6 +804,11 @@ where
continue;
}

if !msg.verify() {
tracing::warn!(?msg, "invalid signature");
continue;
}

let Payload::WstsMessage(wsts_msg) = msg.inner.payload else {
continue;
};
Expand All @@ -806,6 +818,82 @@ where
sig: Vec::new(),
};

let msg_public_key = msg.signer_pub_key.clone();

let sender_is_coordinator =
given_key_is_coordinator(msg_public_key, bitcoin_chain_tip, &signer_set);

let public_keys = &coordinator_state_machine.get_config().signer_public_keys;
let public_key_point = p256k1::point::Point::from(msg_public_key);
// check that messages were signed by correct key
match &packet.msg {
wsts::net::Message::DkgBegin(_)
| wsts::net::Message::DkgPrivateBegin(_)
| wsts::net::Message::DkgEndBegin(_)
| wsts::net::Message::NonceRequest(_)
| wsts::net::Message::SignatureShareRequest(_) => {
if !sender_is_coordinator {
tracing::warn!(
?packet,
reason = "got coordinator message from sender who is not coordinator",
"ignoring packet"
);
continue;
}
}

wsts::net::Message::DkgPublicShares(dkg_public_shares) => {
if &public_key_point != &public_keys[&dkg_public_shares.signer_id] {
tracing::warn!(
?packet,
reason = "message was signed by the wrong signer",
"ignoring packet"
);
continue;
}
}
wsts::net::Message::DkgPrivateShares(dkg_private_shares) => {
if &public_key_point != &public_keys[&dkg_private_shares.signer_id] {
tracing::warn!(
?packet,
reason = "message was signed by the wrong signer",
"ignoring packet"
);
continue;
}
}
wsts::net::Message::DkgEnd(dkg_end) => {
if &public_key_point != &public_keys[&dkg_end.signer_id] {
tracing::warn!(
?packet,
reason = "message was signed by the wrong signer",
"ignoring packet"
);
continue;
}
}
wsts::net::Message::NonceResponse(nonce_response) => {
if &public_key_point != &public_keys[&nonce_response.signer_id] {
tracing::warn!(
?packet,
reason = "message was signed by the wrong signer",
"ignoring packet"
);
continue;
}
}
wsts::net::Message::SignatureShareResponse(sig_share_response) => {
if &public_key_point != &public_keys[&sig_share_response.signer_id] {
tracing::warn!(
?packet,
reason = "message was signed by the wrong signer",
"ignoring packet"
);
continue;
}
}
}

let (outbound_packet, operation_result) =
match coordinator_state_machine.process_message(&packet) {
Ok(val) => val,
Expand Down
49 changes: 45 additions & 4 deletions signer/src/transaction_signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ where
}

(message::Payload::WstsMessage(wsts_msg), _, _) => {
self.handle_wsts_message(wsts_msg, &msg.bitcoin_chain_tip)
self.handle_wsts_message(wsts_msg, &msg.bitcoin_chain_tip, msg.signer_pub_key)
.await?;
}

Expand Down Expand Up @@ -579,10 +579,19 @@ where
&mut self,
msg: &message::WstsMessage,
bitcoin_chain_tip: &model::BitcoinBlockHash,
msg_public_key: PublicKey,
) -> Result<(), Error> {
tracing::info!("handling message");
let chain_tip_report = self
.inspect_msg_chain_tip(msg_public_key, bitcoin_chain_tip)
.await?;

match &msg.inner {
wsts::net::Message::DkgBegin(_) => {
if !chain_tip_report.sender_is_coordinator {
return Err(Error::NotChainTipCoordinator);
}

let signer_public_keys = self.get_signer_public_keys(bitcoin_chain_tip).await?;

let state_machine = wsts_state_machine::SignerStateMachine::new(
Expand All @@ -594,13 +603,38 @@ where
self.relay_message(msg.txid, &msg.inner, bitcoin_chain_tip)
.await?;
}
wsts::net::Message::DkgPublicShares(_)
| wsts::net::Message::DkgPrivateBegin(_)
| wsts::net::Message::DkgPrivateShares(_) => {
wsts::net::Message::DkgPrivateBegin(_) => {
if !chain_tip_report.sender_is_coordinator {
return Err(Error::NotChainTipCoordinator);
}

self.relay_message(msg.txid, &msg.inner, bitcoin_chain_tip)
.await?;
}
wsts::net::Message::DkgPublicShares(dkg_public_shares) => {
let public_keys = &self.wsts_state_machines[&msg.txid].public_keys;
let signer_public_key =
PublicKey::from(&public_keys.signers[&dkg_public_shares.signer_id]);
if signer_public_key != msg_public_key {
return Err(Error::InvalidSignature);
}
self.relay_message(msg.txid, &msg.inner, bitcoin_chain_tip)
.await?;
}
wsts::net::Message::DkgPrivateShares(dkg_private_shares) => {
let public_keys = &self.wsts_state_machines[&msg.txid].public_keys;
let signer_public_key =
PublicKey::from(&public_keys.signers[&dkg_private_shares.signer_id]);
if signer_public_key != msg_public_key {
return Err(Error::InvalidSignature);
}
self.relay_message(msg.txid, &msg.inner, bitcoin_chain_tip)
.await?;
}
wsts::net::Message::DkgEndBegin(_) => {
if !chain_tip_report.sender_is_coordinator {
return Err(Error::NotChainTipCoordinator);
}
self.relay_message(msg.txid, &msg.inner, bitcoin_chain_tip)
.await?;
self.store_dkg_shares(&msg.txid).await?;
Expand All @@ -614,6 +648,9 @@ where
// warning.
#[allow(clippy::map_entry)]
wsts::net::Message::NonceRequest(_) => {
if !chain_tip_report.sender_is_coordinator {
return Err(Error::NotChainTipCoordinator);
}
// TODO(296): Validate that message is the appropriate sighash
if !self.wsts_state_machines.contains_key(&msg.txid) {
let (maybe_aggregate_key, _) = self
Expand All @@ -634,6 +671,10 @@ where
.await?;
}
wsts::net::Message::SignatureShareRequest(_) => {
if !chain_tip_report.sender_is_coordinator {
return Err(Error::NotChainTipCoordinator);
}

// TODO(296): Validate that message is the appropriate sighash
self.relay_message(msg.txid, &msg.inner, bitcoin_chain_tip)
.await?;
Expand Down

0 comments on commit f36a68e

Please sign in to comment.