Switch from clear_on_drop to zeroize (fixes #281)

`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.
This commit is contained in:
Tony Arcieri 2019-09-30 11:51:57 -07:00
parent 620d17ef40
commit 9480844b8d
12 changed files with 85 additions and 36 deletions

View file

@ -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 = []

View file

@ -128,6 +128,8 @@ use core::ops::{Add, Neg, Sub};
use subtle::Choice;
use subtle::ConditionallySelectable;
use zeroize::Zeroize;
use constants;
use edwards::EdwardsPoint;
@ -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".
///
@ -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
// ------------------------------------------------------------------------

View file

@ -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;
@ -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() {
@ -137,6 +137,7 @@ impl MultiscalarMul for Straus {
Q = (&Q + &R_i).to_extended();
}
}
Q
}
}

View file

@ -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)\\).
///
@ -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 {

View file

@ -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
@ -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 {

View file

@ -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)\\).
///
@ -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 {

View file

@ -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
@ -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 {

View file

@ -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;
@ -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).
//
@ -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]);
@ -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 {

View file

@ -12,7 +12,7 @@
use core::borrow::Borrow;
use clear_on_drop::ClearOnDrop;
use zeroize::Zeroizing;
use backend::vector::{CachedPoint, ExtendedPoint};
use edwards::EdwardsPoint;
@ -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() {

View file

@ -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;

View file

@ -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()
}
}
@ -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();

View file

@ -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 \\).
///
@ -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,
@ -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]);