Merge pull request #126 from poanetwork/remove-clear-on-drop

Replaced clear-on-drop with 'std::ptr::write_volatile'.

Cargo.toml

@@ -19,7 +19,6 @@ travis-ci = { repository = "poanetwork/hbbft" }
 [dependencies]
 bincode = "1.0.0"
 byteorder = "1.2.3"
-clear_on_drop = "0.2.3"
 derive_deref = "1.0.1"
 env_logger = "0.5.10"
 error-chain = "0.11.0"

src/broadcast.rs

@@ -44,12 +44,10 @@
 //! ## Example usage
 //!
 //! ```rust
-//!# extern crate clear_on_drop;
 //!# extern crate hbbft;
 //!# extern crate rand;
 //!# fn main() {
 //!#
-//! use clear_on_drop::ClearOnDrop;
 //! use hbbft::broadcast::Broadcast;
 //! use hbbft::crypto::SecretKeySet;
 //! use hbbft::messaging::{DistAlgorithm, NetworkInfo, Target, TargetedMessage};

src/crypto/mod.rs

@@ -10,9 +10,9 @@ pub mod serde_impl;
 
 use std::fmt;
 use std::hash::{Hash, Hasher};
+use std::ptr::write_volatile;
 
 use byteorder::{BigEndian, ByteOrder};
-use clear_on_drop::ClearOnDrop;
 use init_with::InitWith;
 use pairing::bls12_381::{Bls12, Fr, FrRepr, G1, G1Affine, G2, G2Affine};
 use pairing::{CurveAffine, CurveProjective, Engine, Field, PrimeField};
@@ -130,6 +130,15 @@ impl Default for SecretKey {
     }
 }
 
+impl Drop for SecretKey {
+    fn drop(&mut self) {
+        let ptr = self as *mut Self;
+        unsafe {
+            write_volatile(ptr, SecretKey::default());
+        }
+    }
+}
+
 impl SecretKey {
     /// Creates a secret key from an existing value
     pub fn from_value(f: Fr) -> Self {
@@ -295,10 +304,8 @@ impl SecretKeySet {
     }
 
     /// Returns the `i`-th secret key share.
-    pub fn secret_key_share<T: Into<FrRepr>>(&self, i: T) -> ClearOnDrop<Box<SecretKey>> {
+    pub fn secret_key_share<T: Into<FrRepr>>(&self, i: T) -> SecretKey {
-        ClearOnDrop::new(Box::new(SecretKey(
+        SecretKey(self.poly.evaluate(from_repr_plus_1::<Fr>(i.into())))
-            self.poly.evaluate(from_repr_plus_1::<Fr>(i.into())),
-        )))
     }
 
     /// Returns the corresponding public key set. That information can be shared publicly.
@@ -428,9 +435,9 @@ mod tests {
         assert_ne!(pk_set.public_key(), pk_set.public_key_share(2));
 
         // Make sure we don't hand out the main secret key to anyone.
-        assert_ne!(sk_set.secret_key(), *sk_set.secret_key_share(0));
+        assert_ne!(sk_set.secret_key(), sk_set.secret_key_share(0));
-        assert_ne!(sk_set.secret_key(), *sk_set.secret_key_share(1));
+        assert_ne!(sk_set.secret_key(), sk_set.secret_key_share(1));
-        assert_ne!(sk_set.secret_key(), *sk_set.secret_key_share(2));
+        assert_ne!(sk_set.secret_key(), sk_set.secret_key_share(2));
 
         let msg = "Totally real news";
 

src/dynamic_honey_badger/mod.rs

@@ -52,7 +52,6 @@ use std::mem;
 use std::sync::Arc;
 
 use bincode;
-use clear_on_drop::ClearOnDrop;
 use serde::{Deserialize, Serialize};
 
 use self::votes::{SignedVote, VoteCounter};
@@ -73,7 +72,7 @@ mod change;
 mod error;
 mod votes;
 
-type KeyGenOutput = (PublicKeySet, Option<ClearOnDrop<Box<SecretKey>>>);
+type KeyGenOutput = (PublicKeySet, Option<SecretKey>);
 
 /// The user input for `DynamicHoneyBadger`.
 #[derive(Clone, Debug)]
@@ -283,9 +282,7 @@ where
                 // If DKG completed, apply the change.
                 debug!("{:?} DKG for {:?} complete!", self.our_id(), change);
                 // If we are a validator, we received a new secret key. Otherwise keep the old one.
-                let sk = sk.unwrap_or_else(|| {
+                let sk = sk.unwrap_or_else(|| self.netinfo.secret_key().clone());
-                    ClearOnDrop::new(Box::new(self.netinfo.secret_key().clone()))
-                });
                 // Restart Honey Badger in the next epoch, and inform the user about the change.
                 self.apply_change(&change, pub_key_set, sk, batch.epoch + 1)?;
                 batch.change = ChangeState::Complete(change);
@@ -316,7 +313,7 @@ where
         &mut self,
         change: &Change<NodeUid>,
         pub_key_set: PublicKeySet,
-        sk: ClearOnDrop<Box<SecretKey>>,
+        sk: SecretKey,
         epoch: u64,
     ) -> Result<()> {
         self.key_gen = None;

src/lib.rs

@@ -100,7 +100,6 @@
 
 extern crate bincode;
 extern crate byteorder;
-extern crate clear_on_drop;
 #[macro_use(Deref, DerefMut)]
 extern crate derive_deref;
 #[macro_use]

src/messaging.rs

@@ -1,8 +1,6 @@
 use std::collections::{BTreeMap, BTreeSet};
 use std::fmt::Debug;
 
-use clear_on_drop::ClearOnDrop;
-
 use crypto::{PublicKey, PublicKeySet, SecretKey};
 use fault_log::FaultLog;
 
@@ -131,12 +129,6 @@ impl<'a, D: DistAlgorithm + 'a> Iterator for OutputIter<'a, D> {
 }
 
 /// Common data shared between algorithms.
-///
-/// *NOTE* `NetworkInfo` requires its `secret_key` to be heap allocated and
-/// wrapped by the `ClearOnDrop` type from the `clear_on_drop` crate. We
-/// use this construction to zero out the section of heap memory that is
-/// allocated for `secret_key` when the corresponding instance of
-/// `NetworkInfo` goes out of scope.
 #[derive(Debug, Clone)]
 pub struct NetworkInfo<NodeUid> {
     our_uid: NodeUid,
@@ -144,7 +136,7 @@ pub struct NetworkInfo<NodeUid> {
     num_nodes: usize,
     num_faulty: usize,
     is_validator: bool,
-    secret_key: ClearOnDrop<Box<SecretKey>>,
+    secret_key: SecretKey,
     public_key_set: PublicKeySet,
     public_keys: BTreeMap<NodeUid, PublicKey>,
     node_indices: BTreeMap<NodeUid, usize>,
@@ -154,7 +146,7 @@ impl<NodeUid: Clone + Ord> NetworkInfo<NodeUid> {
     pub fn new(
         our_uid: NodeUid,
         all_uids: BTreeSet<NodeUid>,
-        secret_key: ClearOnDrop<Box<SecretKey>>,
+        secret_key: SecretKey,
         public_key_set: PublicKeySet,
     ) -> Self {
         let num_nodes = all_uids.len();

src/sync_key_gen.rs

@@ -37,7 +37,6 @@ use std::collections::{BTreeMap, BTreeSet};
 use std::fmt::{self, Debug, Formatter};
 
 use bincode;
-use clear_on_drop::ClearOnDrop;
 use pairing::bls12_381::{Fr, G1Affine};
 use pairing::{CurveAffine, Field};
 use rand::OsRng;
@@ -238,7 +237,7 @@ impl<NodeUid: Ord + Clone + Debug> SyncKeyGen<NodeUid> {
     ///
     /// These are only secure if `is_ready` returned `true`. Otherwise it is not guaranteed that
     /// none of the nodes knows the secret master key.
-    pub fn generate(&self) -> (PublicKeySet, Option<ClearOnDrop<Box<SecretKey>>>) {
+    pub fn generate(&self) -> (PublicKeySet, Option<SecretKey>) {
         let mut pk_commit = Poly::zero().commitment();
         let mut opt_sk_val = self.our_idx.map(|_| Fr::zero());
         let is_complete = |proposal: &&ProposalState| proposal.is_complete(self.threshold);
@@ -249,8 +248,7 @@ impl<NodeUid: Ord + Clone + Debug> SyncKeyGen<NodeUid> {
                 sk_val.add_assign(&row.evaluate(0));
             }
         }
-        let opt_sk =
+        let opt_sk = opt_sk_val.map(SecretKey::from_value);
-            opt_sk_val.map(|sk_val| ClearOnDrop::new(Box::new(SecretKey::from_value(sk_val))));
         (pk_commit.into(), opt_sk)
     }