Skip to content

Commit

Permalink
Switch from clear_on_drop to zeroize (fixes #281)
Browse files Browse the repository at this point in the history
`zeroize` is WASM-friendly as it has no dependencies on C compilers.

Instead uses Rust's own volatile write semantics and compiler fences to
ensure zeroization is not elided by the compiler.
  • Loading branch information
tarcieri committed Oct 23, 2019
1 parent 620d17e commit 9480844
Show file tree
Hide file tree
Showing 12 changed files with 85 additions and 36 deletions.
6 changes: 3 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,16 @@ harness = false
rand_core = { version = "0.3.0", default-features = false }
byteorder = { version = "^1.2.3", default-features = false, features = ["i128"] }
digest = { version = "0.8", default-features = false }
clear_on_drop = "=0.2.3"
subtle = { version = "2", default-features = false }
serde = { version = "1.0", default-features = false, optional = true }
packed_simd = { version = "0.3.0", features = ["into_bits"], optional = true }
zeroize = { version = "1.0.0", default-features = false }

[features]
nightly = ["subtle/nightly", "clear_on_drop/nightly"]
nightly = ["subtle/nightly"]
default = ["std", "u64_backend"]
std = ["alloc", "subtle/std", "rand_core/std"]
alloc = []
alloc = ["zeroize/alloc"]

# The u32 backend uses u32s with u64 products.
u32_backend = []
Expand Down
19 changes: 19 additions & 0 deletions src/backend/serial/curve_models/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ use core::ops::{Add, Neg, Sub};
use subtle::Choice;
use subtle::ConditionallySelectable;

use zeroize::Zeroize;

use constants;

use edwards::EdwardsPoint;
Expand Down Expand Up @@ -182,6 +184,14 @@ pub struct AffineNielsPoint {
pub xy2d: FieldElement,
}

impl Zeroize for AffineNielsPoint {
fn zeroize(&mut self) {
self.y_plus_x.zeroize();
self.y_minus_x.zeroize();
self.xy2d.zeroize();
}
}

/// A pre-computed point on the \\( \mathbb P\^3 \\) model for the
/// curve, represented as \\((Y+X, Y-X, Z, 2dXY)\\) in "Niels coordinates".
///
Expand All @@ -195,6 +205,15 @@ pub struct ProjectiveNielsPoint {
pub T2d: FieldElement,
}

impl Zeroize for ProjectiveNielsPoint {
fn zeroize(&mut self) {
self.Y_plus_X.zeroize();
self.Y_minus_X.zeroize();
self.Z.zeroize();
self.T2d.zeroize();
}
}

// ------------------------------------------------------------------------
// Constructors
// ------------------------------------------------------------------------
Expand Down
7 changes: 4 additions & 3 deletions src/backend/serial/scalar_mul/straus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ impl MultiscalarMul for Straus {
J: IntoIterator,
J::Item: Borrow<EdwardsPoint>,
{
use clear_on_drop::ClearOnDrop;
use zeroize::Zeroizing;

use backend::serial::curve_models::ProjectiveNielsPoint;
use window::LookupTable;
Expand All @@ -119,12 +119,12 @@ impl MultiscalarMul for Straus {

// This puts the scalar digits into a heap-allocated Vec.
// To ensure that these are erased, pass ownership of the Vec into a
// ClearOnDrop wrapper.
// Zeroizing wrapper.
let scalar_digits_vec: Vec<_> = scalars
.into_iter()
.map(|s| s.borrow().to_radix_16())
.collect();
let scalar_digits = ClearOnDrop::new(scalar_digits_vec);
let scalar_digits = Zeroizing::new(scalar_digits_vec);

let mut Q = EdwardsPoint::identity();
for j in (0..64).rev() {
Expand All @@ -137,6 +137,7 @@ impl MultiscalarMul for Straus {
Q = (&Q + &R_i).to_extended();
}
}

Q
}
}
Expand Down
8 changes: 8 additions & 0 deletions src/backend/serial/u32/field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ use core::ops::{Sub, SubAssign};
use subtle::Choice;
use subtle::ConditionallySelectable;

use zeroize::Zeroize;

/// A `FieldElement2625` represents an element of the field
/// \\( \mathbb Z / (2\^{255} - 19)\\).
///
Expand Down Expand Up @@ -55,6 +57,12 @@ impl Debug for FieldElement2625 {
}
}

impl Zeroize for FieldElement2625 {
fn zeroize(&mut self) {
self.0.zeroize();
}
}

impl<'b> AddAssign<&'b FieldElement2625> for FieldElement2625 {
fn add_assign(&mut self, _rhs: &'b FieldElement2625) {
for i in 0..10 {
Expand Down
8 changes: 8 additions & 0 deletions src/backend/serial/u32/scalar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
use core::fmt::Debug;
use core::ops::{Index, IndexMut};

use zeroize::Zeroize;

use constants;

/// The `Scalar29` struct represents an element in ℤ/lℤ as 9 29-bit limbs
Expand All @@ -25,6 +27,12 @@ impl Debug for Scalar29 {
}
}

impl Zeroize for Scalar29 {
fn zeroize(&mut self) {
self.0.zeroize();
}
}

impl Index<usize> for Scalar29 {
type Output = u32;
fn index(&self, _index: usize) -> &u32 {
Expand Down
8 changes: 8 additions & 0 deletions src/backend/serial/u64/field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ use core::ops::{Sub, SubAssign};
use subtle::Choice;
use subtle::ConditionallySelectable;

use zeroize::Zeroize;

/// A `FieldElement51` represents an element of the field
/// \\( \mathbb Z / (2\^{255} - 19)\\).
///
Expand All @@ -44,6 +46,12 @@ impl Debug for FieldElement51 {
}
}

impl Zeroize for FieldElement51 {
fn zeroize(&mut self) {
self.0.zeroize();
}
}

impl<'b> AddAssign<&'b FieldElement51> for FieldElement51 {
fn add_assign(&mut self, _rhs: &'b FieldElement51) {
for i in 0..5 {
Expand Down
8 changes: 8 additions & 0 deletions src/backend/serial/u64/scalar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
use core::fmt::Debug;
use core::ops::{Index, IndexMut};

use zeroize::Zeroize;

use constants;

/// The `Scalar52` struct represents an element in
Expand All @@ -27,6 +29,12 @@ impl Debug for Scalar52 {
}
}

impl Zeroize for Scalar52 {
fn zeroize(&mut self) {
self.0.zeroize();
}
}

impl Index<usize> for Scalar52 {
type Output = u64;
fn index(&self, _index: usize) -> &u64 {
Expand Down
10 changes: 8 additions & 2 deletions src/backend/vector/avx2/field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ const D_LANES64: u8 = 0b11_00_00_00;

use core::ops::{Add, Mul, Neg};
use packed_simd::{i32x8, u32x8, u64x4, IntoBits};
use zeroize::Zeroize;

use backend::vector::avx2::constants::{P_TIMES_16_HI, P_TIMES_16_LO, P_TIMES_2_HI, P_TIMES_2_LO};
use backend::serial::u64::field::FieldElement51;
Expand Down Expand Up @@ -449,7 +450,7 @@ impl FieldElement2625x4 {
// hi (c(x3), c(y3), c(x2), c(y2), c(z3), c(w3), c(z2), c(w2))
// -> (c(x1), c(y1), c(x2), c(y2), c(z1), c(w1), c(z2), c(w2))
//
// which is exactly the vector of carryins for
// which is exactly the vector of carryins for
//
// ( x2, y2, x3, y3, z2, w2, z3, w3).
//
Expand All @@ -462,7 +463,7 @@ impl FieldElement2625x4 {

let mut v = self.0;

let c10 = rotated_carryout(v[0]);
let c10 = rotated_carryout(v[0]);
v[0] = (v[0] & masks) + combine(u32x8::splat(0), c10);

let c32 = rotated_carryout(v[1]);
Expand Down Expand Up @@ -873,6 +874,11 @@ impl<'a, 'b> Mul<&'b FieldElement2625x4> for &'a FieldElement2625x4 {
}
}

impl Zeroize for FieldElement2625x4 {
fn zeroize(&mut self) {
self.0.zeroize();
}
}

#[cfg(test)]
mod test {
Expand Down
6 changes: 3 additions & 3 deletions src/backend/vector/scalar_mul/straus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

use core::borrow::Borrow;

use clear_on_drop::ClearOnDrop;
use zeroize::Zeroizing;

use backend::vector::{CachedPoint, ExtendedPoint};
use edwards::EdwardsPoint;
Expand Down Expand Up @@ -54,8 +54,8 @@ impl MultiscalarMul for Straus {
.into_iter()
.map(|s| s.borrow().to_radix_16())
.collect();
// Pass ownership to a ClearOnDrop wrapper
let scalar_digits = ClearOnDrop::new(scalar_digits_vec);
// Pass ownership to a `Zeroizing` wrapper
let scalar_digits = Zeroizing::new(scalar_digits_vec);

let mut Q = ExtendedPoint::identity();
for j in (0..64).rev() {
Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ extern crate std;
extern crate packed_simd;

extern crate byteorder;
extern crate clear_on_drop;
pub extern crate digest;
extern crate rand_core;
#[cfg(test)]
extern crate rand_os;
extern crate zeroize;

// Used for traits related to constant-time code.
extern crate subtle;
Expand Down
11 changes: 4 additions & 7 deletions src/scalar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ impl<'a> Neg for &'a Scalar {
fn neg(self) -> Scalar {
let self_R = UnpackedScalar::mul_internal(&self.unpack(), &constants::R);
let self_mod_l = UnpackedScalar::montgomery_reduce(&self_R);
UnpackedScalar::sub(&UnpackedScalar::zero(), &self_mod_l).pack()
UnpackedScalar::sub(&UnpackedScalar::zero(), &self_mod_l).pack()
}
}

Expand Down Expand Up @@ -760,18 +760,15 @@ impl Scalar {
// externally, but there's no corresponding distinction for
// field elements.

use clear_on_drop::ClearOnDrop;
use clear_on_drop::clear::ZeroSafe;
// Mark UnpackedScalars as zeroable.
unsafe impl ZeroSafe for UnpackedScalar {}
use zeroize::Zeroizing;

let n = inputs.len();
let one: UnpackedScalar = Scalar::one().unpack().to_montgomery();

// Wrap the scratch storage in a ClearOnDrop to wipe it when
// Place scratch storage in a Zeroizing wrapper to wipe it when
// we pass out of scope.
let scratch_vec = vec![one; n];
let mut scratch = ClearOnDrop::new(scratch_vec);
let mut scratch = Zeroizing::new(scratch_vec);

// Keep an accumulator of all of the previous products
let mut acc = Scalar::one().unpack().to_montgomery();
Expand Down
28 changes: 11 additions & 17 deletions src/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ use edwards::EdwardsPoint;
use backend::serial::curve_models::ProjectiveNielsPoint;
use backend::serial::curve_models::AffineNielsPoint;

use zeroize::Zeroize;

/// A lookup table of precomputed multiples of a point \\(P\\), used to
/// compute \\( xP \\) for \\( -8 \leq x \leq 8 \\).
///
Expand All @@ -40,23 +42,6 @@ use backend::serial::curve_models::AffineNielsPoint;
#[derive(Copy, Clone)]
pub struct LookupTable<T>(pub(crate) [T; 8]);

use clear_on_drop::clear::ZeroSafe;

/// This type isn't actually zeroable (all zero bytes are not valid
/// points), but we want to be able to use `clear_on_drop` to erase slices
/// of `LookupTable`.
///
/// Since the `ZeroSafe` trait is only used by `clear_on_drop`, the only
/// situation where this would be a problem is if code attempted to use
/// a `ClearOnDrop` to erase a `LookupTable` and then used the table
/// afterwards.
///
/// Normally this is not a problem, since the table's storage is usually
/// dropped too.
///
/// XXX is this a good compromise?
unsafe impl<T> ZeroSafe for LookupTable<T> {}

impl<T> LookupTable<T>
where
T: Identity + ConditionallySelectable + ConditionallyNegatable,
Expand Down Expand Up @@ -120,6 +105,15 @@ impl<'a> From<&'a EdwardsPoint> for LookupTable<AffineNielsPoint> {
}
}

impl<T> Zeroize for LookupTable<T>
where
T: Copy + Default + Zeroize
{
fn zeroize(&mut self) {
self.0.zeroize();
}
}

/// Holds odd multiples 1A, 3A, ..., 15A of a point A.
#[derive(Copy, Clone)]
pub(crate) struct NafLookupTable5<T>(pub(crate) [T; 8]);
Expand Down

0 comments on commit 9480844

Please sign in to comment.