Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jul 15, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 0 additions & 2 deletions src/broadcast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
23 changes: 15 additions & 8 deletions src/crypto/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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>> {
ClearOnDrop::new(Box::new(SecretKey(
self.poly.evaluate(from_repr_plus_1::<Fr>(i.into())),
)))
pub fn secret_key_share<T: Into<FrRepr>>(&self, i: T) -> SecretKey {
SecretKey(self.poly.evaluate(from_repr_plus_1::<Fr>(i.into())))
}

/// Returns the corresponding public key set. That information can be shared publicly.
Expand Down Expand Up @@ -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(1));
assert_ne!(sk_set.secret_key(), *sk_set.secret_key_share(2));
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(2));

let msg = "Totally real news";

Expand Down
9 changes: 3 additions & 6 deletions src/dynamic_honey_badger/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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)]
Expand Down Expand Up @@ -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(|| {
ClearOnDrop::new(Box::new(self.netinfo.secret_key().clone()))
});
let sk = sk.unwrap_or_else(|| 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);
Expand Down Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
12 changes: 2 additions & 10 deletions src/messaging.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -131,20 +129,14 @@ 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,
all_uids: BTreeSet<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>,
Expand All @@ -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();
Expand Down
6 changes: 2 additions & 4 deletions src/sync_key_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -249,8 +248,7 @@ impl<NodeUid: Ord + Clone + Debug> SyncKeyGen<NodeUid> {
sk_val.add_assign(&row.evaluate(0));
}
}
let opt_sk =
opt_sk_val.map(|sk_val| ClearOnDrop::new(Box::new(SecretKey::from_value(sk_val))));
let opt_sk = opt_sk_val.map(SecretKey::from_value);
(pk_commit.into(), opt_sk)
}

Expand Down