diff --git a/crypto/dalek-ff-group/src/field.rs b/crypto/dalek-ff-group/src/field.rs index 8c9a927d..2afe6396 100644 --- a/crypto/dalek-ff-group/src/field.rs +++ b/crypto/dalek-ff-group/src/field.rs @@ -1,5 +1,6 @@ -use core::ops::{Add, AddAssign, Sub, SubAssign, Neg, Mul, MulAssign}; +use core::ops::{DerefMut, Add, AddAssign, Sub, SubAssign, Neg, Mul, MulAssign}; +use zeroize::Zeroize; use rand_core::RngCore; use subtle::{ @@ -11,7 +12,7 @@ use crypto_bigint::{Integer, Encoding, U256, U512}; use group::ff::{Field, PrimeField, FieldBits, PrimeFieldBits}; -use crate::{constant_time, math, from_uint}; +use crate::{u8_from_bool, constant_time, math, from_uint}; // 2^255 - 19 // Uses saturating_sub because checked_sub isn't available at compile time @@ -200,10 +201,11 @@ impl FieldElement { let mut res = FieldElement::one(); let mut bits = 0; - for (i, bit) in other.to_le_bits().iter().rev().enumerate() { + for (i, mut bit) in other.to_le_bits().iter_mut().rev().enumerate() { bits <<= 1; - let bit = u8::from(*bit); + let mut bit = u8_from_bool(bit.deref_mut()); bits |= bit; + bit.zeroize(); if ((i + 1) % 4) == 0 { if i != 3 { diff --git a/crypto/dalek-ff-group/src/lib.rs b/crypto/dalek-ff-group/src/lib.rs index a05725ff..5a63375e 100644 --- a/crypto/dalek-ff-group/src/lib.rs +++ b/crypto/dalek-ff-group/src/lib.rs @@ -3,7 +3,7 @@ use core::{ borrow::Borrow, - ops::{Deref, Add, AddAssign, Sub, SubAssign, Neg, Mul, MulAssign}, + ops::{Deref, DerefMut, Add, AddAssign, Sub, SubAssign, Neg, Mul, MulAssign}, iter::{Iterator, Sum}, hash::{Hash, Hasher}, }; @@ -41,18 +41,34 @@ use group::{ pub mod field; -// Convert a boolean to a Choice in a *presumably* constant time manner -fn choice(value: bool) -> Choice { - #[cfg(not(feature = "black_box"))] - let res = Choice::from(u8::from(value)); - #[cfg(feature = "black_box")] - let res = { - use core::hint::black_box; - Choice::from(black_box(u8::from(black_box(value)))) - }; +// Feature gated due to MSRV requirements +#[cfg(feature = "black_box")] +pub(crate) fn black_box(val: T) -> T { + core::hint::black_box(val) +} + +#[cfg(not(feature = "black_box"))] +pub(crate) fn black_box(val: T) -> T { + val +} + +fn u8_from_bool(bit_ref: &mut bool) -> u8 { + let bit_ref = black_box(bit_ref); + + let mut bit = black_box(*bit_ref); + let res = black_box(bit as u8); + bit.zeroize(); + debug_assert!((res | 1) == 1); + + bit_ref.zeroize(); res } +// Convert a boolean to a Choice in a *presumably* constant time manner +fn choice(mut value: bool) -> Choice { + Choice::from(u8_from_bool(&mut value)) +} + macro_rules! deref_borrow { ($Source: ident, $Target: ident) => { impl Deref for $Source { @@ -202,10 +218,11 @@ impl Scalar { let mut res = Scalar::one(); let mut bits = 0; - for (i, bit) in other.to_le_bits().iter().rev().enumerate() { + for (i, mut bit) in other.to_le_bits().iter_mut().rev().enumerate() { bits <<= 1; - let bit = u8::from(*bit); + let mut bit = u8_from_bool(bit.deref_mut()); bits |= bit; + bit.zeroize(); if ((i + 1) % 4) == 0 { if i != 3 { @@ -288,7 +305,18 @@ impl PrimeField for Scalar { // The number of leading zero bits in the little-endian bit representation of (modulus - 1) const S: u32 = 2; fn is_odd(&self) -> Choice { - choice(self.to_le_bits()[0]) + // This is probably overkill? Yet it's better safe than sorry since this is a complete + // decomposition of the scalar + let mut bits = self.to_le_bits(); + let res = choice(bits[0]); + // This shouldn't need mut since it should be a mutable reference + // Per the bitvec docs, writing through a derefence requires mut, writing through one of its + // methods does not + // We do not use one of its methods to ensure we write via zeroize + for mut bit in bits.iter_mut() { + bit.deref_mut().zeroize(); + } + res } fn multiplicative_generator() -> Self { // This was calculated with the method from the ff crate docs diff --git a/crypto/dleq/Cargo.toml b/crypto/dleq/Cargo.toml index da114e0f..0032c447 100644 --- a/crypto/dleq/Cargo.toml +++ b/crypto/dleq/Cargo.toml @@ -39,8 +39,14 @@ transcript = { package = "flexible-transcript", path = "../transcript", features [features] std = [] serialize = ["std"] -experimental = ["std", "thiserror", "multiexp"] + +# Needed for cross-group DLEqs +black_box = [] secure_capacity_difference = [] +experimental = ["std", "thiserror", "multiexp"] # Only applies to experimental, yet is default to ensure security +# experimental doesn't mandate it itself in case two curves with extreme +# capacity differences are desired to be used together, in which case the user +# must specify experimental without default features default = ["secure_capacity_difference"] diff --git a/crypto/dleq/src/cross_group/mod.rs b/crypto/dleq/src/cross_group/mod.rs index e5095b45..b59a84fc 100644 --- a/crypto/dleq/src/cross_group/mod.rs +++ b/crypto/dleq/src/cross_group/mod.rs @@ -1,4 +1,6 @@ -use core::ops::Deref; +use core::ops::{Deref, DerefMut}; +#[cfg(feature = "serialize")] +use std::io::{Read, Write}; use thiserror::Error; @@ -27,8 +29,28 @@ pub(crate) mod aos; mod bits; use bits::{BitSignature, Bits}; -#[cfg(feature = "serialize")] -use std::io::{Read, Write}; +// Feature gated due to MSRV requirements +#[cfg(feature = "black_box")] +pub(crate) fn black_box(val: T) -> T { + core::hint::black_box(val) +} + +#[cfg(not(feature = "black_box"))] +pub(crate) fn black_box(val: T) -> T { + val +} + +fn u8_from_bool(bit_ref: &mut bool) -> u8 { + let bit_ref = black_box(bit_ref); + + let mut bit = black_box(*bit_ref); + let res = black_box(bit as u8); + bit.zeroize(); + debug_assert!((res | 1) == 1); + + bit_ref.zeroize(); + res +} #[cfg(feature = "serialize")] pub(crate) fn read_point(r: &mut R) -> std::io::Result { @@ -224,15 +246,13 @@ where let mut these_bits: u8 = 0; // Needed to zero out the bits #[allow(unused_assignments)] - for (i, mut raw_bit) in raw_bits.iter_mut().enumerate() { + for (i, mut bit) in raw_bits.iter_mut().enumerate() { if i == capacity { break; } - let mut bit = u8::from(*raw_bit); - *raw_bit = false; - // Accumulate this bit + let mut bit = u8_from_bool(bit.deref_mut()); these_bits |= bit << (i % bits_per_group); bit.zeroize(); diff --git a/crypto/dleq/src/cross_group/scalar.rs b/crypto/dleq/src/cross_group/scalar.rs index fefd819d..14ef71c9 100644 --- a/crypto/dleq/src/cross_group/scalar.rs +++ b/crypto/dleq/src/cross_group/scalar.rs @@ -1,21 +1,26 @@ +use core::ops::DerefMut; + use ff::PrimeFieldBits; use zeroize::Zeroize; +use crate::cross_group::u8_from_bool; + /// Convert a uniform scalar into one usable on both fields, clearing the top bits as needed. pub fn scalar_normalize( mut scalar: F0, ) -> (F0, F1) { let mutual_capacity = F0::CAPACITY.min(F1::CAPACITY); - // The security of a mutual key is the security of the lower field. Accordingly, this bans a - // difference of more than 4 bits + // A mutual key is only as secure as its weakest group + // Accordingly, this bans a capacity difference of more than 4 bits to prevent a curve generally + // offering n-bits of security from being forced into a situation with much fewer bits #[cfg(feature = "secure_capacity_difference")] - assert!((F0::CAPACITY.max(F1::CAPACITY) - mutual_capacity) < 4); + assert!((F0::CAPACITY.max(F1::CAPACITY) - mutual_capacity) <= 4); let mut res1 = F0::zero(); let mut res2 = F1::zero(); - // Uses the bit view API to ensure a consistent endianess + // Uses the bits API to ensure a consistent endianess let mut bits = scalar.to_le_bits(); scalar.zeroize(); // Convert it to big endian @@ -24,9 +29,9 @@ pub fn scalar_normalize( let mut skip = bits.len() - usize::try_from(mutual_capacity).unwrap(); // Needed to zero out the bits #[allow(unused_assignments)] - for mut raw_bit in bits.iter_mut() { + for mut bit in bits.iter_mut() { if skip > 0 { - *raw_bit = false; + bit.deref_mut().zeroize(); skip -= 1; continue; } @@ -34,9 +39,7 @@ pub fn scalar_normalize( res1 = res1.double(); res2 = res2.double(); - let mut bit = u8::from(*raw_bit); - *raw_bit = false; - + let mut bit = u8_from_bool(bit.deref_mut()); res1 += F0::from(bit.into()); res2 += F1::from(bit.into()); bit.zeroize(); diff --git a/crypto/ed448/Cargo.toml b/crypto/ed448/Cargo.toml index 50a98c2f..fb2da204 100644 --- a/crypto/ed448/Cargo.toml +++ b/crypto/ed448/Cargo.toml @@ -32,3 +32,6 @@ dalek-ff-group = { path = "../dalek-ff-group", version = "^0.1.2" } hex = "0.4" ff-group-tests = { path = "../ff-group-tests" } + +[features] +black_box = [] diff --git a/crypto/ed448/src/backend.rs b/crypto/ed448/src/backend.rs index 617dfb5e..7daae853 100644 --- a/crypto/ed448/src/backend.rs +++ b/crypto/ed448/src/backend.rs @@ -1,12 +1,36 @@ +use zeroize::Zeroize; + +// Feature gated due to MSRV requirements +#[cfg(feature = "black_box")] +pub(crate) fn black_box(val: T) -> T { + core::hint::black_box(val) +} + +#[cfg(not(feature = "black_box"))] +pub(crate) fn black_box(val: T) -> T { + val +} + +pub(crate) fn u8_from_bool(bit_ref: &mut bool) -> u8 { + let bit_ref = black_box(bit_ref); + + let mut bit = black_box(*bit_ref); + let res = black_box(bit as u8); + bit.zeroize(); + debug_assert!((res | 1) == 1); + + bit_ref.zeroize(); + res +} + #[doc(hidden)] #[macro_export] macro_rules! field { ($FieldName: ident, $MODULUS: ident, $WIDE_MODULUS: ident, $NUM_BITS: literal) => { - use core::ops::{Add, AddAssign, Neg, Sub, SubAssign, Mul, MulAssign}; - - use rand_core::RngCore; + use core::ops::{DerefMut, Add, AddAssign, Neg, Sub, SubAssign, Mul, MulAssign}; use subtle::{Choice, CtOption, ConstantTimeEq, ConstantTimeLess, ConditionallySelectable}; + use rand_core::RngCore; use generic_array::{typenum::U57, GenericArray}; use crypto_bigint::{Integer, Encoding}; @@ -18,6 +42,8 @@ macro_rules! field { use dalek_ff_group::{from_wrapper, math_op}; use dalek_ff_group::{constant_time, from_uint, math}; + use $crate::backend::u8_from_bool; + fn reduce(x: U1024) -> U512 { U512::from_le_slice(&x.reduce(&$WIDE_MODULUS).unwrap().to_le_bytes()[.. 64]) } @@ -59,10 +85,11 @@ macro_rules! field { let mut res = Self(U512::ONE); let mut bits = 0; - for (i, bit) in other.to_le_bits().iter().rev().enumerate() { + for (i, mut bit) in other.to_le_bits().iter_mut().rev().enumerate() { bits <<= 1; - let bit = u8::from(*bit); + let mut bit = u8_from_bool(bit.deref_mut()); bits |= bit; + bit.zeroize(); if ((i + 1) % 4) == 0 { if i != 3 { diff --git a/crypto/ed448/src/point.rs b/crypto/ed448/src/point.rs index 25916b95..54e8caac 100644 --- a/crypto/ed448/src/point.rs +++ b/crypto/ed448/src/point.rs @@ -1,5 +1,5 @@ use core::{ - ops::{Add, AddAssign, Neg, Sub, SubAssign, Mul, MulAssign}, + ops::{DerefMut, Add, AddAssign, Neg, Sub, SubAssign, Mul, MulAssign}, iter::Sum, }; @@ -19,6 +19,7 @@ use group::{ }; use crate::{ + backend::u8_from_bool, scalar::{Scalar, MODULUS as SCALAR_MODULUS}, field::{FieldElement, MODULUS as FIELD_MODULUS, Q_4}, }; @@ -218,7 +219,7 @@ impl<'a> Sum<&'a Point> for Point { impl Mul for Point { type Output = Point; - fn mul(self, other: Scalar) -> Point { + fn mul(self, mut other: Scalar) -> Point { // Precompute the optimal amount that's a multiple of 2 let mut table = [Point::identity(); 16]; table[1] = self; @@ -228,10 +229,11 @@ impl Mul for Point { let mut res = Self::identity(); let mut bits = 0; - for (i, bit) in other.to_le_bits().iter().rev().enumerate() { + for (i, mut bit) in other.to_le_bits().iter_mut().rev().enumerate() { bits <<= 1; - let bit = u8::from(*bit); + let mut bit = u8_from_bool(bit.deref_mut()); bits |= bit; + bit.zeroize(); if ((i + 1) % 4) == 0 { if i != 3 { @@ -243,6 +245,7 @@ impl Mul for Point { bits = 0; } } + other.zeroize(); res } } diff --git a/crypto/multiexp/Cargo.toml b/crypto/multiexp/Cargo.toml index a54ae0d8..24ebd399 100644 --- a/crypto/multiexp/Cargo.toml +++ b/crypto/multiexp/Cargo.toml @@ -27,4 +27,5 @@ k256 = { version = "0.12", features = ["bits"] } dalek-ff-group = { path = "../dalek-ff-group" } [features] +black_box = [] batch = ["rand_core"] diff --git a/crypto/multiexp/src/lib.rs b/crypto/multiexp/src/lib.rs index df4da3e3..9d309637 100644 --- a/crypto/multiexp/src/lib.rs +++ b/crypto/multiexp/src/lib.rs @@ -1,5 +1,7 @@ #![cfg_attr(docsrs, feature(doc_auto_cfg))] +use core::ops::DerefMut; + use zeroize::Zeroize; use ff::PrimeFieldBits; @@ -19,6 +21,29 @@ pub use batch::BatchVerifier; #[cfg(test)] mod tests; +// Feature gated due to MSRV requirements +#[cfg(feature = "black_box")] +pub(crate) fn black_box(val: T) -> T { + core::hint::black_box(val) +} + +#[cfg(not(feature = "black_box"))] +pub(crate) fn black_box(val: T) -> T { + val +} + +fn u8_from_bool(bit_ref: &mut bool) -> u8 { + let bit_ref = black_box(bit_ref); + + let mut bit = black_box(*bit_ref); + let res = black_box(bit as u8); + bit.zeroize(); + debug_assert!((res | 1) == 1); + + bit_ref.zeroize(); + res +} + // Convert scalars to `window`-sized bit groups, as needed to index a table // This algorithm works for `window <= 8` pub(crate) fn prep_bits(pairs: &[(G::Scalar, G)], window: u8) -> Vec> @@ -33,10 +58,8 @@ where let mut bits = pair.0.to_le_bits(); groupings.push(vec![0; (bits.len() + (w_usize - 1)) / w_usize]); - for (i, mut raw_bit) in bits.iter_mut().enumerate() { - let mut bit = u8::from(*raw_bit); - (*raw_bit).zeroize(); - + for (i, mut bit) in bits.iter_mut().enumerate() { + let mut bit = u8_from_bool(bit.deref_mut()); groupings[p][i / w_usize] |= bit << (i % w_usize); bit.zeroize(); }