From f09952f3d77a48edfc644ac2d35db3d49be21c43 Mon Sep 17 00:00:00 2001 From: Greg Fitzgerald Date: Tue, 6 Mar 2018 16:34:14 -0700 Subject: [PATCH] DRY up transaction signing Cleanup the big mess I copy-pasted myself into. --- src/accountant.rs | 16 ++---- src/accountant_stub.rs | 28 +++------- src/bin/client-demo.rs | 30 ++++------- src/bin/demo.rs | 10 ++-- src/event.rs | 7 +-- src/genesis.rs | 14 ++--- src/historian.rs | 13 ++--- src/log.rs | 102 ++----------------------------------- src/logger.rs | 23 --------- src/transaction.rs | 113 +++++++++++++++++++++++++++-------------- 10 files changed, 109 insertions(+), 247 deletions(-) diff --git a/src/accountant.rs b/src/accountant.rs index 22cf75ccb..3af394965 100644 --- a/src/accountant.rs +++ b/src/accountant.rs @@ -4,8 +4,8 @@ use log::{Entry, Sha256Hash}; use event::Event; -use transaction::{sign_transaction_data, Transaction}; -use signature::{get_pubkey, PublicKey, Signature}; +use transaction::Transaction; +use signature::{PublicKey, Signature}; use genesis::Genesis; use historian::{reserve_signature, Historian}; use ring::signature::Ed25519KeyPair; @@ -137,16 +137,8 @@ impl Accountant { keypair: &Ed25519KeyPair, to: PublicKey, ) -> Result { - let from = get_pubkey(keypair); - let last_id = self.last_id; - let sig = sign_transaction_data(&n, keypair, &to, &last_id); - let tr = Transaction { - from, - to, - asset: n, - last_id, - sig, - }; + let tr = Transaction::new(keypair, to, n, self.last_id); + let sig = tr.sig; self.process_transaction(tr).map(|_| sig) } diff --git a/src/accountant_stub.rs b/src/accountant_stub.rs index 757a9905d..ea713a9b5 100644 --- a/src/accountant_stub.rs +++ b/src/accountant_stub.rs @@ -5,8 +5,8 @@ use std::net::UdpSocket; use std::io; use bincode::{deserialize, serialize}; -use transaction::{sign_transaction_data, Transaction}; -use signature::{get_pubkey, PublicKey, Signature}; +use transaction::Transaction; +use signature::{PublicKey, Signature}; use log::{Entry, Sha256Hash}; use ring::signature::Ed25519KeyPair; use accountant_skel::{Request, Response}; @@ -26,21 +26,8 @@ impl AccountantStub { } } - pub fn transfer_signed( - &self, - from: PublicKey, - to: PublicKey, - val: i64, - last_id: Sha256Hash, - sig: Signature, - ) -> io::Result { - let req = Request::Transaction(Transaction { - from, - to, - asset: val, - last_id, - sig, - }); + pub fn transfer_signed(&self, tr: Transaction) -> io::Result { + let req = Request::Transaction(tr); let data = serialize(&req).unwrap(); self.socket.send_to(&data, &self.addr) } @@ -52,10 +39,9 @@ impl AccountantStub { to: PublicKey, last_id: &Sha256Hash, ) -> io::Result { - let from = get_pubkey(keypair); - let sig = sign_transaction_data(&n, keypair, &to, last_id); - self.transfer_signed(from, to, n, *last_id, sig) - .map(|_| sig) + let tr = Transaction::new(keypair, to, n, *last_id); + let sig = tr.sig; + self.transfer_signed(tr).map(|_| sig) } pub fn get_balance(&self, pubkey: &PublicKey) -> io::Result> { diff --git a/src/bin/client-demo.rs b/src/bin/client-demo.rs index 36ee0cb8d..3005a1fa1 100644 --- a/src/bin/client-demo.rs +++ b/src/bin/client-demo.rs @@ -2,9 +2,8 @@ extern crate serde_json; extern crate silk; use silk::accountant_stub::AccountantStub; -use silk::event::Event; use silk::signature::{generate_keypair, get_pubkey}; -use silk::transaction::{sign_transaction_data, Transaction}; +use silk::transaction::Transaction; use silk::genesis::Genesis; use std::time::Instant; use std::net::UdpSocket; @@ -25,15 +24,12 @@ fn main() { let txs = acc.get_balance(&alice_pubkey).unwrap().unwrap(); println!("Alice's Initial Balance {}", txs); - let one = 1; println!("Signing transactions..."); let now = Instant::now(); - let sigs: Vec<(_, _)> = (0..txs) + let transactions: Vec<_> = (0..txs) .map(|_| { - let rando_keypair = generate_keypair(); - let rando_pubkey = get_pubkey(&rando_keypair); - let sig = sign_transaction_data(&one, &alice_keypair, &rando_pubkey, &last_id); - (rando_pubkey, sig) + let rando_pubkey = get_pubkey(&generate_keypair()); + Transaction::new(&alice_keypair, rando_pubkey, 1, last_id) }) .collect(); let duration = now.elapsed(); @@ -48,15 +44,8 @@ fn main() { println!("Verify signatures..."); let now = Instant::now(); - for &(k, s) in &sigs { - let e = Event::Transaction(Transaction { - from: alice_pubkey, - to: k, - asset: one, - last_id, - sig: s, - }); - assert!(e.verify()); + for tr in &transactions { + assert!(tr.verify()); } let duration = now.elapsed(); let ns = duration.as_secs() * 1_000_000_000 + duration.subsec_nanos() as u64; @@ -71,10 +60,9 @@ fn main() { println!("Transferring 1 unit {} times...", txs); let now = Instant::now(); let mut sig = Default::default(); - for (k, s) in sigs { - acc.transfer_signed(alice_pubkey, k, one, last_id, s) - .unwrap(); - sig = s; + for tr in transactions { + sig = tr.sig; + acc.transfer_signed(tr).unwrap(); } println!("Waiting for last transaction to be confirmed...",); acc.wait_on_signature(&sig).unwrap(); diff --git a/src/bin/demo.rs b/src/bin/demo.rs index 29571f2d7..bdbad0ba2 100644 --- a/src/bin/demo.rs +++ b/src/bin/demo.rs @@ -3,7 +3,7 @@ extern crate silk; use silk::historian::Historian; use silk::log::{verify_slice, Entry, Sha256Hash}; use silk::signature::{generate_keypair, get_pubkey}; -use silk::transaction::sign_claim_data; +use silk::transaction::Transaction; use silk::event::Event; use std::thread::sleep; use std::time::Duration; @@ -16,12 +16,8 @@ fn create_log( sleep(Duration::from_millis(15)); let asset = Sha256Hash::default(); let keypair = generate_keypair(); - let event0 = Event::new_claim( - get_pubkey(&keypair), - asset, - *seed, - sign_claim_data(&asset, &keypair, seed), - ); + let tr = Transaction::new(&keypair, get_pubkey(&keypair), asset, *seed); + let event0 = Event::Transaction(tr); hist.sender.send(event0)?; sleep(Duration::from_millis(10)); Ok(()) diff --git a/src/event.rs b/src/event.rs index b69c445d1..791edcd3a 100644 --- a/src/event.rs +++ b/src/event.rs @@ -1,9 +1,8 @@ //! The `event` crate provides the data structures for log events. -use signature::{PublicKey, Signature}; +use signature::Signature; use transaction::Transaction; use serde::Serialize; -use log::Sha256Hash; /// When 'event' is Tick, the event represents a simple clock tick, and exists for the /// sole purpose of improving the performance of event log verification. A tick can @@ -17,10 +16,6 @@ pub enum Event { } impl Event { - pub fn new_claim(to: PublicKey, asset: T, last_id: Sha256Hash, sig: Signature) -> Self { - Event::Transaction(Transaction::new_claim(to, asset, last_id, sig)) - } - pub fn get_signature(&self) -> Option { match *self { Event::Tick => None, diff --git a/src/genesis.rs b/src/genesis.rs index 16faf9efe..01bcd1416 100644 --- a/src/genesis.rs +++ b/src/genesis.rs @@ -1,7 +1,7 @@ //! A library for generating the chain's genesis block. use event::Event; -use transaction::{sign_transaction_data, Transaction}; +use transaction::Transaction; use signature::{generate_keypair, get_pubkey, PublicKey}; use log::{create_entries, hash, Entry, Sha256Hash}; use ring::rand::SystemRandom; @@ -55,16 +55,8 @@ impl Genesis { } pub fn create_transaction(&self, asset: i64, to: &PublicKey) -> Event { - let last_id = self.get_seed(); - let from = self.get_pubkey(); - let sig = sign_transaction_data(&asset, &self.get_keypair(), to, &last_id); - Event::Transaction(Transaction { - from, - to: *to, - asset, - last_id, - sig, - }) + let tr = Transaction::new(&self.get_keypair(), *to, asset, self.get_seed()); + Event::Transaction(tr) } pub fn create_events(&self) -> Vec> { diff --git a/src/historian.rs b/src/historian.rs index 8af1d0424..03605a247 100644 --- a/src/historian.rs +++ b/src/historian.rs @@ -69,8 +69,6 @@ mod tests { use super::*; use log::*; use event::*; - use transaction::*; - use signature::*; use std::thread::sleep; use std::time::Duration; @@ -112,15 +110,10 @@ mod tests { #[test] fn test_duplicate_event_signature() { - let keypair = generate_keypair(); - let to = get_pubkey(&keypair); - let data = b"hello, world"; - let zero = Sha256Hash::default(); - let sig = sign_claim_data(&data, &keypair, &zero); - let tr0 = Transaction::new_claim(to, &data, zero, sig); let mut sigs = HashSet::new(); - assert!(reserve_signature(&mut sigs, &tr0.sig)); - assert!(!reserve_signature(&mut sigs, &tr0.sig)); + let sig = Signature::default(); + assert!(reserve_signature(&mut sigs, &sig)); + assert!(!reserve_signature(&mut sigs, &sig)); } #[test] diff --git a/src/log.rs b/src/log.rs index a91b8b8c5..1da66d778 100644 --- a/src/log.rs +++ b/src/log.rs @@ -170,7 +170,7 @@ pub fn next_ticks(start_hash: &Sha256Hash, num_hashes: u64, len: usize) -> Vec Logger { } } } - -#[cfg(test)] -mod tests { - use super::*; - use log::*; - use event::*; - use signature::*; - use transaction::*; - - #[test] - fn test_bad_event_signature() { - let zero = Sha256Hash::default(); - let keypair = generate_keypair(); - let sig = sign_claim_data(&hash(b"hello, world"), &keypair, &zero); - let event0 = Event::new_claim( - get_pubkey(&keypair), - hash(b"goodbye cruel world"), - zero, - sig, - ); - assert!(!event0.verify()); - } -} diff --git a/src/transaction.rs b/src/transaction.rs index 6c6fe8228..f0be7c1d8 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -16,62 +16,99 @@ pub struct Transaction { } impl Transaction { - pub fn new_claim(to: PublicKey, asset: T, last_id: Sha256Hash, sig: Signature) -> Self { - Transaction { - from: to, + pub fn new( + from_keypair: &Ed25519KeyPair, + to: PublicKey, + asset: T, + last_id: Sha256Hash, + ) -> Self { + let mut tr = Transaction { + from: get_pubkey(&from_keypair), to, asset, last_id, - sig, - } + sig: Signature::default(), + }; + tr.sign(from_keypair); + tr + } + + fn get_sign_data(&self) -> Vec { + serialize(&(&self.from, &self.to, &self.asset, &self.last_id)).unwrap() + } + + pub fn sign(&mut self, keypair: &Ed25519KeyPair) { + let sign_data = self.get_sign_data(); + self.sig = Signature::clone_from_slice(keypair.sign(&sign_data).as_ref()); } pub fn verify(&self) -> bool { - let sign_data = serialize(&(&self.from, &self.to, &self.asset, &self.last_id)).unwrap(); - verify_signature(&self.from, &sign_data, &self.sig) + verify_signature(&self.from, &self.get_sign_data(), &self.sig) } } -fn sign_serialized(asset: &T, keypair: &Ed25519KeyPair) -> Signature { - let serialized = serialize(asset).unwrap(); - Signature::clone_from_slice(keypair.sign(&serialized).as_ref()) -} - -/// Return a signature for the given transaction data using the private key from the given keypair. -pub fn sign_transaction_data( - asset: &T, - keypair: &Ed25519KeyPair, - to: &PublicKey, - last_id: &Sha256Hash, -) -> Signature { - let from = &get_pubkey(keypair); - sign_serialized(&(from, to, asset, last_id), keypair) -} - -/// Return a signature for the given data using the private key from the given keypair. -pub fn sign_claim_data( - asset: &T, - keypair: &Ed25519KeyPair, - last_id: &Sha256Hash, -) -> Signature { - sign_transaction_data(asset, keypair, &get_pubkey(keypair), last_id) -} - #[cfg(test)] mod tests { use super::*; use bincode::{deserialize, serialize}; + use log::*; + use signature::*; + + #[test] + fn test_claim() { + let keypair = generate_keypair(); + let asset = hash(b"hello, world"); + let zero = Sha256Hash::default(); + let tr0 = Transaction::new(&keypair, get_pubkey(&keypair), asset, zero); + assert!(tr0.verify()); + } + + #[test] + fn test_transfer() { + let zero = Sha256Hash::default(); + let keypair0 = generate_keypair(); + let keypair1 = generate_keypair(); + let pubkey1 = get_pubkey(&keypair1); + let asset = hash(b"hello, world"); + let tr0 = Transaction::new(&keypair0, pubkey1, asset, zero); + assert!(tr0.verify()); + } #[test] fn test_serialize_claim() { - let claim0 = Transaction::new_claim( - Default::default(), - 0u8, - Default::default(), - Default::default(), - ); + let claim0 = Transaction { + from: Default::default(), + to: Default::default(), + asset: 0u8, + last_id: Default::default(), + sig: Default::default(), + }; let buf = serialize(&claim0).unwrap(); let claim1: Transaction = deserialize(&buf).unwrap(); assert_eq!(claim1, claim0); } + + #[test] + fn test_bad_event_signature() { + let zero = Sha256Hash::default(); + let keypair = generate_keypair(); + let pubkey = get_pubkey(&keypair); + let mut tr = Transaction::new(&keypair, pubkey, hash(b"hello, world"), zero); + tr.sign(&keypair); + tr.asset = hash(b"goodbye cruel world"); // <-- attack! + assert!(!tr.verify()); + } + + #[test] + fn test_hijack_attack() { + let keypair0 = generate_keypair(); + let keypair1 = generate_keypair(); + let thief_keypair = generate_keypair(); + let pubkey1 = get_pubkey(&keypair1); + let zero = Sha256Hash::default(); + let mut tr = Transaction::new(&keypair0, pubkey1, hash(b"hello, world"), zero); + tr.sign(&keypair0); + tr.to = get_pubkey(&thief_keypair); // <-- attack! + assert!(!tr.verify()); + } }