From 6fec8fad573623a23b2cf6a6cd66a9f80e542be1 Mon Sep 17 00:00:00 2001 From: Greg Fitzgerald Date: Mon, 2 Apr 2018 20:34:18 -0600 Subject: [PATCH 1/4] Adding from to the signature is redundant --- src/transaction.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/transaction.rs b/src/transaction.rs index 4754335573..09821ec7af 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -57,7 +57,7 @@ impl Transaction { } fn get_sign_data(&self) -> Vec { - serialize(&(&self.from, &self.plan, &self.tokens, &self.last_id)).unwrap() + serialize(&(&self.plan, &self.tokens, &self.last_id)).unwrap() } /// Sign this transaction. From 07aa2e12602b684eb8f328af4b15239ce0549fb1 Mon Sep 17 00:00:00 2001 From: Greg Fitzgerald Date: Mon, 2 Apr 2018 20:47:48 -0600 Subject: [PATCH 2/4] Add witness data to entry hash Otherwise, witnesses can be dropped or reordered by a malicious generator. --- src/entry.rs | 22 ++++++++++++++++++---- src/event.rs | 25 +++++++++++++++++++------ 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/src/entry.rs b/src/entry.rs index 985bab83cc..e9d13157f2 100644 --- a/src/entry.rs +++ b/src/entry.rs @@ -43,6 +43,23 @@ impl Entry { } } +fn add_event_data(hash_data: &mut Vec, event: &Event) { + match *event { + Event::Transaction(ref tr) => { + hash_data.push(0u8); + hash_data.extend_from_slice(&tr.sig); + } + Event::Signature { ref sig, .. } => { + hash_data.push(1u8); + hash_data.extend_from_slice(sig); + } + Event::Timestamp { ref sig, .. } => { + hash_data.push(2u8); + hash_data.extend_from_slice(sig); + } + } +} + /// Creates the hash `num_hashes` after `start_hash`. If the event contains /// signature, the final hash will be a hash of both the previous ID and /// the signature. @@ -55,10 +72,7 @@ pub fn next_hash(start_hash: &Hash, num_hashes: u64, events: &[Event]) -> Hash { // Hash all the event data let mut hash_data = vec![]; for event in events { - let sig = event.get_signature(); - if let Some(sig) = sig { - hash_data.extend_from_slice(&sig); - } + add_event_data(&mut hash_data, event); } if !hash_data.is_empty() { diff --git a/src/event.rs b/src/event.rs index 9940959a3d..dbd2a93412 100644 --- a/src/event.rs +++ b/src/event.rs @@ -33,12 +33,13 @@ impl Event { } } - // TODO: Rename this to transaction_signature(). - /// If the Event is a Transaction, return its Signature. - pub fn get_signature(&self) -> Option { - match *self { - Event::Transaction(ref tr) => Some(tr.sig), - Event::Signature { .. } | Event::Timestamp { .. } => None, + /// Create and sign a new Witness Signature. Used for unit-testing. + pub fn new_signature(from: &KeyPair, tx_sig: Signature) -> Self { + let sig = Signature::clone_from_slice(from.sign(&tx_sig).as_ref()); + Event::Signature { + from: from.pubkey(), + tx_sig, + sig, } } @@ -52,3 +53,15 @@ impl Event { } } } + +#[cfg(test)] +mod tests { + use super::*; + use signature::{KeyPair, KeyPairUtil}; + + #[test] + fn test_event_verify() { + assert!(Event::new_timestamp(&KeyPair::new(), Utc::now()).verify()); + assert!(Event::new_signature(&KeyPair::new(), Signature::default()).verify()); + } +} From fe32159673160c5db8dc2f30f9b9b913ffaaf16a Mon Sep 17 00:00:00 2001 From: Greg Fitzgerald Date: Mon, 2 Apr 2018 21:07:38 -0600 Subject: [PATCH 3/4] Add a test to ensure witness data continues to be hashed --- src/entry.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/entry.rs b/src/entry.rs index e9d13157f2..e672e71c04 100644 --- a/src/entry.rs +++ b/src/entry.rs @@ -113,6 +113,7 @@ pub fn next_tick(start_hash: &Hash, num_hashes: u64) -> Entry { #[cfg(test)] mod tests { use super::*; + use chrono::prelude::*; use entry::create_entry; use event::Event; use hash::hash; @@ -146,6 +147,23 @@ mod tests { assert!(!e0.verify(&zero)); } + #[test] + fn test_witness_reorder_attack() { + let zero = Hash::default(); + + // First, verify entries + let keypair = KeyPair::new(); + let tr0 = Event::new_timestamp(&keypair, Utc::now()); + let tr1 = Event::new_signature(&keypair, Default::default()); + let mut e0 = create_entry(&zero, 0, vec![tr0.clone(), tr1.clone()]); + assert!(e0.verify(&zero)); + + // Next, swap two witness events and ensure verification fails. + e0.events[0] = tr1; // <-- attack + e0.events[1] = tr0; + assert!(!e0.verify(&zero)); + } + #[test] fn test_next_tick() { let zero = Hash::default(); From 94eea3abecbad2fd1fcdfb466566b22dff1ddde9 Mon Sep 17 00:00:00 2001 From: Greg Fitzgerald Date: Mon, 2 Apr 2018 21:15:21 -0600 Subject: [PATCH 4/4] fmt --- src/accountant_skel.rs | 6 +++--- src/accountant_stub.rs | 2 +- src/bin/client-demo.rs | 4 ++-- src/bin/testnode.rs | 6 +++--- src/transaction.rs | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/accountant_skel.rs b/src/accountant_skel.rs index 8893fcb1f7..91c85e2405 100644 --- a/src/accountant_skel.rs +++ b/src/accountant_skel.rs @@ -3,12 +3,13 @@ //! in flux. Clients should use AccountantStub to interact with it. use accountant::Accountant; -use historian::Historian; -use recorder::Signal; use bincode::{deserialize, serialize}; use entry::Entry; use event::Event; use hash::Hash; +use historian::Historian; +use rayon::prelude::*; +use recorder::Signal; use result::Result; use serde_json; use signature::PublicKey; @@ -22,7 +23,6 @@ use std::thread::{spawn, JoinHandle}; use std::time::Duration; use streamer; use transaction::Transaction; -use rayon::prelude::*; pub struct AccountantSkel { acc: Accountant, diff --git a/src/accountant_stub.rs b/src/accountant_stub.rs index 0c7c528087..a83862ef20 100644 --- a/src/accountant_stub.rs +++ b/src/accountant_stub.rs @@ -87,8 +87,8 @@ impl AccountantStub { mod tests { use super::*; use accountant::Accountant; - use historian::Historian; use accountant_skel::AccountantSkel; + use historian::Historian; use mint::Mint; use signature::{KeyPair, KeyPairUtil}; use std::io::sink; diff --git a/src/bin/client-demo.rs b/src/bin/client-demo.rs index 75f2b2b295..5dc4c9f056 100644 --- a/src/bin/client-demo.rs +++ b/src/bin/client-demo.rs @@ -2,15 +2,15 @@ extern crate rayon; extern crate serde_json; extern crate solana; +use rayon::prelude::*; use solana::accountant_stub::AccountantStub; use solana::mint::Mint; use solana::signature::{KeyPair, KeyPairUtil}; use solana::transaction::Transaction; use std::io::stdin; use std::net::UdpSocket; -use std::time::{Duration, Instant}; use std::thread::sleep; -use rayon::prelude::*; +use std::time::{Duration, Instant}; fn main() { let addr = "127.0.0.1:8000"; diff --git a/src/bin/testnode.rs b/src/bin/testnode.rs index ef6700d309..6ef325c425 100644 --- a/src/bin/testnode.rs +++ b/src/bin/testnode.rs @@ -2,10 +2,10 @@ extern crate serde_json; extern crate solana; use solana::accountant::Accountant; -use solana::event::Event; -use solana::entry::Entry; -use solana::historian::Historian; use solana::accountant_skel::AccountantSkel; +use solana::entry::Entry; +use solana::event::Event; +use solana::historian::Historian; use std::io::{self, stdout, BufRead}; use std::sync::atomic::AtomicBool; use std::sync::{Arc, Mutex}; diff --git a/src/transaction.rs b/src/transaction.rs index 09821ec7af..c88a1eea90 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -2,9 +2,9 @@ use bincode::serialize; use chrono::prelude::*; -use rayon::prelude::*; use hash::Hash; use plan::{Condition, Payment, Plan}; +use rayon::prelude::*; use signature::{KeyPair, KeyPairUtil, PublicKey, Signature, SignatureUtil}; #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)]