diff --git a/.circleci/config.yml b/.circleci/config.yml index da1e87a..49f8358 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -38,7 +38,7 @@ jobs: command: | rustc --version cargo --version - cargo test --all --all-features + cargo test --all --all-features -- --test-threads 1 - run: name: audit command: | diff --git a/src/error.rs b/src/error.rs index 7e41f0e..a2cb553 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,5 +1,6 @@ // Error types +use crate::last_sign_state; use crate::prost; use abscissa::Error; use signatory; @@ -77,6 +78,10 @@ pub enum KmsErrorKind { /// Verification operation failed #[fail(display = "verification failed")] VerificationError, + + /// Signature invalid + #[fail(display = "attempted double sign")] + DoubleSign, } impl Display for KmsError { @@ -145,3 +150,9 @@ impl From for KmsError { err!(KmsErrorKind::InvalidMessageError, other).into() } } + +impl From for KmsError { + fn from(other: last_sign_state::LastSignError) -> Self { + err!(KmsErrorKind::DoubleSign, other).into() + } +} diff --git a/src/last_sign_state.rs b/src/last_sign_state.rs new file mode 100644 index 0000000..a3d1759 --- /dev/null +++ b/src/last_sign_state.rs @@ -0,0 +1,200 @@ +use abscissa::Error; +use serde_json; +use std::{ + fmt::{self, Display}, + fs::{File, OpenOptions}, + io::prelude::*, + path::Path, +}; +use tendermint::{block, chain}; + +#[derive(Serialize, Deserialize)] +struct LastSignData { + pub height: i64, + pub round: i64, + pub step: i8, + pub block_id: Option, +} + +const EMPTY_DATA: LastSignData = LastSignData { + height: 0, + round: 0, + step: 0, + block_id: None, +}; + +pub struct LastSignState { + data: LastSignData, + file: File, + _chain_id: chain::Id, +} + +/// Error type +#[derive(Debug)] +pub struct LastSignError(Error); + +#[derive(Copy, Clone, Eq, PartialEq, Debug, Fail)] +pub enum LastSignErrorKind { + #[fail(display = "height regression")] + HeightRegression, + #[fail(display = "step regression")] + StepRegression, + #[fail(display = "round regression")] + RoundRegression, + #[fail(display = "invalid block id")] + DoubleSign, +} + +impl From> for LastSignError { + fn from(other: Error) -> Self { + LastSignError(other) + } +} + +impl Display for LastSignError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.fmt(f) + } +} + +impl LastSignState { + pub fn load_state(path: &Path, chain_id: chain::Id) -> std::io::Result { + if !path.exists() { + let mut lst = LastSignState { + data: EMPTY_DATA, + file: File::create(path)?, + _chain_id: chain_id, + }; + lst.sync_to_disk()?; + return Ok(lst); + } + let mut lst = LastSignState { + data: EMPTY_DATA, + file: OpenOptions::new().read(true).write(true).open(path)?, + _chain_id: chain_id, + }; + + let mut contents = String::new(); + lst.file.read_to_string(&mut contents)?; + lst.data = serde_json::from_str(&contents)?; + Ok(lst) + } + + pub fn sync_to_disk(&mut self) -> std::io::Result<()> { + self.file + .write_all(serde_json::to_string(&self.data)?.as_ref())?; + self.file.sync_all()?; + Ok(()) + } + + pub fn check_and_update_hrs( + &mut self, + height: i64, + round: i64, + step: i8, + block_id: Option, + ) -> Result<(), LastSignError> { + if height < self.data.height { + fail!( + LastSignErrorKind::HeightRegression, + "last height:{} new height:{}", + self.data.height, + height + ); + } + if height == self.data.height { + if round < self.data.round { + fail!( + LastSignErrorKind::RoundRegression, + "round regression at height:{} last round:{} new round:{}", + height, + self.data.round, + round + ) + } + if round == self.data.round { + if step < self.data.step { + fail!( + LastSignErrorKind::StepRegression, + "round regression at height:{} round:{} last step:{} new step:{}", + height, + round, + self.data.step, + step + ) + } + + if block_id != None && self.data.block_id != None && self.data.block_id != block_id + { + fail!( + LastSignErrorKind::DoubleSign, + "Attempting to sign a second proposal at height:{} round:{} step:{} old block id:{} new block {}", + height, + round, + step, + self.data.block_id.clone().unwrap(), + block_id.unwrap() + ) + } + } + } + self.data.height = height; + self.data.round = round; + self.data.step = step; + self.data.block_id = block_id; + Ok(()) + } +} +#[cfg(test)] +mod tests { + use super::*; + use std::str::FromStr; + use tendermint::{block, chain}; + + const EXAMPLE_BLOCK_ID: &str = + "26C0A41F3243C6BCD7AD2DFF8A8D83A71D29D307B5326C227F734A1A512FE47D"; + + const EXAMPLE_DOUBLE_SIGN_BLOCK_ID: &str = + "2470A41F3243C6BCD7AD2DFF8A8D83A71D29D307B5326C227F734A1A512FE47D"; + + #[test] + fn hrs_test() { + let mut last_sign_state = LastSignState { + data: LastSignData { + height: 1, + round: 1, + step: 0, + block_id: None, + }, + file: File::create("/tmp/tmp_state.json").unwrap(), + _chain_id: "example-chain".parse::().unwrap(), + }; + assert_eq!( + last_sign_state.check_and_update_hrs(2, 0, 0, None).unwrap(), + () + ) + } + + #[test] + fn hrs_test_double_sign() { + let mut last_sign_state = LastSignState { + data: LastSignData { + height: 1, + round: 1, + step: 0, + block_id: Some(block::Id::from_str(EXAMPLE_BLOCK_ID).unwrap()), + }, + file: File::create("/tmp/tmp_state.json").unwrap(), + _chain_id: "example-chain".parse::().unwrap(), + }; + let double_sign_block = block::Id::from_str(EXAMPLE_DOUBLE_SIGN_BLOCK_ID).unwrap(); + let err = last_sign_state.check_and_update_hrs(1, 1, 1, Some(double_sign_block)); + + let double_sign_error = LastSignErrorKind::DoubleSign; + + assert_eq!( + err.expect_err("Expect Double Sign error").0.kind(), + &double_sign_error + ) + } +} diff --git a/src/lib.rs b/src/lib.rs index 078feb9..76a89b9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -34,6 +34,7 @@ mod client; mod commands; mod config; mod keyring; +mod last_sign_state; mod rpc; mod session; mod unix_connection; diff --git a/src/session.rs b/src/session.rs index fabd0cf..c791383 100644 --- a/src/session.rs +++ b/src/session.rs @@ -22,6 +22,7 @@ use tendermint::{ use crate::{ error::KmsError, keyring::KeyRing, + last_sign_state::LastSignState, prost::Message, rpc::{Request, Response, TendermintRequest}, unix_connection::UnixConnection, @@ -34,6 +35,9 @@ pub struct Session { /// TCP connection to a validator node connection: Connection, + + /// Stateful double sign defense that is synced to disk + last_sign_state: LastSignState, } impl Session> { @@ -50,10 +54,15 @@ impl Session> { let signer = Ed25519Signer::from(secret_connection_key); let public_key = SecretConnectionKey::from(ed25519::public_key(&signer)?); let connection = SecretConnection::new(socket, &public_key, &signer)?; + let last_sign_state = LastSignState::load_state( + Path::new(&(chain_id.to_string() + "_priv_validator_state.json")), + chain_id, + )?; Ok(Self { chain_id, connection, + last_sign_state, }) } } @@ -68,10 +77,15 @@ impl Session> { let socket = UnixStream::connect(socket_path)?; let connection = UnixConnection::new(socket); + let last_sign_state = LastSignState::load_state( + Path::new(&(chain_id.as_ref().to_owned() + "_priv_validator_state.json")), + chain_id, + )?; Ok(Self { chain_id, connection, + last_sign_state, }) } } @@ -120,6 +134,21 @@ where fn sign(&mut self, mut request: T) -> Result { request.validate()?; + if let Some(cs) = request.consensus_state() { + match self.last_sign_state.check_and_update_hrs( + cs.height, + cs.round, + cs.step, + cs.block_id, + ) { + Ok(()) => (), + Err(e) => { + debug! {"Double sign event: {}",e} + return Err(KmsError::from(e)); + } + } + } + self.last_sign_state.sync_to_disk()?; let mut to_sign = vec![]; request.sign_bytes(self.chain_id, &mut to_sign)?; diff --git a/tendermint-rs/src/amino_types/mod.rs b/tendermint-rs/src/amino_types/mod.rs index 282cdf4..c509398 100644 --- a/tendermint-rs/src/amino_types/mod.rs +++ b/tendermint-rs/src/amino_types/mod.rs @@ -21,7 +21,7 @@ pub use self::{ proposal::{SignProposalRequest, SignedProposalResponse, AMINO_NAME as PROPOSAL_AMINO_NAME}, remote_error::RemoteError, secret_connection::AuthSigMessage, - signature::{SignableMsg, SignedMsgType}, + signature::{ConsensusState, SignableMsg, SignedMsgType}, time::TimeMsg, validate::ConsensusMessage, vote::{SignVoteRequest, SignedVoteResponse, AMINO_NAME as VOTE_AMINO_NAME}, diff --git a/tendermint-rs/src/amino_types/proposal.rs b/tendermint-rs/src/amino_types/proposal.rs index 5211b12..b64977b 100644 --- a/tendermint-rs/src/amino_types/proposal.rs +++ b/tendermint-rs/src/amino_types/proposal.rs @@ -1,10 +1,11 @@ use super::{ block_id::{BlockId, CanonicalBlockId, CanonicalPartSetHeader}, remote_error::RemoteError, - signature::{SignableMsg, SignedMsgType}, + signature::{ConsensusState, SignableMsg, SignedMsgType}, time::TimeMsg, validate::{ConsensusMessage, ValidationError, ValidationErrorKind::*}, }; +use crate::block::ParseId; use crate::{block, chain, error::Error}; use bytes::BufMut; use prost::{EncodeError, Message}; @@ -129,6 +130,25 @@ impl SignableMsg for SignProposalRequest { None => Err(MissingConsensusMessage.into()), } } + fn consensus_state(&self) -> Option { + match self.proposal { + Some(ref p) => Some(ConsensusState { + height: p.height, + round: p.round, + step: 3, + block_id: { + match p.block_id { + Some(ref b) => match b.parse_block_id() { + Ok(id) => Some(id), + Err(_) => None, + }, + None => None, + } + }, + }), + None => None, + } + } } impl ConsensusMessage for Proposal { diff --git a/tendermint-rs/src/amino_types/signature.rs b/tendermint-rs/src/amino_types/signature.rs index 004ff11..068bd3e 100644 --- a/tendermint-rs/src/amino_types/signature.rs +++ b/tendermint-rs/src/amino_types/signature.rs @@ -1,5 +1,5 @@ use super::validate::ValidationError; -use crate::chain; +use crate::{block, chain}; use bytes::BufMut; use prost::{DecodeError, EncodeError}; use signatory::ed25519; @@ -15,8 +15,15 @@ pub trait SignableMsg { /// Set the Ed25519 signature on the underlying message fn set_signature(&mut self, sig: &ed25519::Signature); - fn validate(&self) -> Result<(), ValidationError>; + fn consensus_state(&self) -> Option; +} + +pub struct ConsensusState { + pub height: i64, + pub round: i64, + pub step: i8, + pub block_id: Option, } /// Signed message types. This follows: diff --git a/tendermint-rs/src/amino_types/vote.rs b/tendermint-rs/src/amino_types/vote.rs index 3ddfa06..bfc041b 100644 --- a/tendermint-rs/src/amino_types/vote.rs +++ b/tendermint-rs/src/amino_types/vote.rs @@ -1,11 +1,12 @@ use super::{ block_id::{BlockId, CanonicalBlockId, CanonicalPartSetHeader}, remote_error::RemoteError, - signature::SignableMsg, + signature::{ConsensusState, SignableMsg}, time::TimeMsg, validate::{ConsensusMessage, ValidationError, ValidationErrorKind::*}, SignedMsgType, }; +use crate::block::ParseId; use crate::{block, chain, error::Error}; use bytes::BufMut; use prost::{error::EncodeError, Message}; @@ -150,6 +151,25 @@ impl SignableMsg for SignVoteRequest { None => Err(MissingConsensusMessage.into()), } } + fn consensus_state(&self) -> Option { + match self.vote { + Some(ref v) => Some(ConsensusState { + height: v.height, + round: v.round, + step: 6, + block_id: { + match v.block_id { + Some(ref b) => match b.parse_block_id() { + Ok(id) => Some(id), + Err(_) => None, + }, + None => None, + } + }, + }), + None => None, + } + } } impl ConsensusMessage for Vote { diff --git a/tests/integration.rs b/tests/integration.rs index b4586a5..4fda8da 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -8,6 +8,7 @@ use rand::Rng; use signatory::{ed25519, encoding::Identity, Decode, Signature}; use signatory_dalek::{Ed25519Signer, Ed25519Verifier}; use std::{ + fs, io::{self, Cursor, Read, Write}, net::{TcpListener, TcpStream}, os::unix::net::{UnixListener, UnixStream}, @@ -222,6 +223,7 @@ impl Drop for ProtocolTester { fn drop(&mut self) { self.tcp_device.process.kill().unwrap(); self.unix_device.process.kill().unwrap(); + fs::remove_file("test_chain_id_priv_validator_state.json").unwrap(); } } @@ -291,7 +293,7 @@ fn test_handle_and_sign_proposal() { let proposal = amino_types::proposal::Proposal { msg_type: amino_types::SignedMsgType::Proposal.to_u32(), height: 12345, - round: 23456, + round: 1, timestamp: Some(t), pol_round: -1, block_id: None,