diff --git a/curve25519-dalek/Cargo.toml b/curve25519-dalek/Cargo.toml index 8e480ada..dc3b2b4e 100644 --- a/curve25519-dalek/Cargo.toml +++ b/curve25519-dalek/Cargo.toml @@ -51,7 +51,7 @@ ff = { version = "0.13", default-features = false, optional = true } group = { version = "0.13", default-features = false, optional = true } rand_core = { version = "0.6.4", default-features = false, optional = true } digest = { version = "0.10", default-features = false, optional = true } -subtle = { version = "2.3.0", default-features = false } +subtle = { version = "2.6.0", default-features = false } serde = { version = "1.0", default-features = false, optional = true, features = ["derive"] } zeroize = { version = "1", default-features = false, optional = true } diff --git a/curve25519-dalek/src/backend/serial/u32/scalar.rs b/curve25519-dalek/src/backend/serial/u32/scalar.rs index d3df38c8..10eadd1b 100644 --- a/curve25519-dalek/src/backend/serial/u32/scalar.rs +++ b/curve25519-dalek/src/backend/serial/u32/scalar.rs @@ -12,6 +12,7 @@ use core::fmt::Debug; use core::ops::{Index, IndexMut}; +use subtle::BlackBox; #[cfg(feature = "zeroize")] use zeroize::Zeroize; @@ -185,30 +186,24 @@ impl Scalar29 { /// Compute `a - b` (mod l). pub fn sub(a: &Scalar29, b: &Scalar29) -> Scalar29 { - // Optimization barrier to prevent compiler from inserting branch instructions - // TODO(tarcieri): find a better home (or abstraction) for this - fn black_box(value: u32) -> u32 { - // SAFETY: `u32` is a simple integer `Copy` type and `value` lives on the stack so - // a pointer to it will be valid. - unsafe { core::ptr::read_volatile(&value) } - } - let mut difference = Scalar29::ZERO; - let mask = (1u32 << 29) - 1; + let mask = BlackBox::new((1u32 << 29) - 1); // a - b let mut borrow: u32 = 0; for i in 0..9 { borrow = a[i].wrapping_sub(b[i] + (borrow >> 31)); - difference[i] = borrow & mask; + difference[i] = borrow & mask.get(); } // conditionally add l if the difference is negative - let underflow_mask = ((borrow >> 31) ^ 1).wrapping_sub(1); + let underflow_mask = BlackBox::new(((borrow >> 31) ^ 1).wrapping_sub(1)); let mut carry: u32 = 0; for i in 0..9 { - carry = (carry >> 29) + difference[i] + (constants::L[i] & black_box(underflow_mask)); - difference[i] = carry & mask; + // SECURITY: `BlackBox` prevents LLVM from inserting a `jns` conditional on x86(_64) + // which can be used to bypass this section when `underflow_mask` is zero. + carry = (carry >> 29) + difference[i] + (constants::L[i] & underflow_mask.get()); + difference[i] = carry & mask.get(); } difference diff --git a/curve25519-dalek/src/backend/serial/u64/scalar.rs b/curve25519-dalek/src/backend/serial/u64/scalar.rs index 6c0eaf79..ade16f31 100644 --- a/curve25519-dalek/src/backend/serial/u64/scalar.rs +++ b/curve25519-dalek/src/backend/serial/u64/scalar.rs @@ -13,6 +13,7 @@ use core::fmt::Debug; use core::ops::{Index, IndexMut}; +use subtle::BlackBox; #[cfg(feature = "zeroize")] use zeroize::Zeroize; @@ -174,32 +175,24 @@ impl Scalar52 { /// Compute `a - b` (mod l) pub fn sub(a: &Scalar52, b: &Scalar52) -> Scalar52 { - // Optimization barrier to prevent compiler from inserting branch instructions - // TODO(tarcieri): find a better home (or abstraction) for this - fn black_box(value: u64) -> u64 { - // SAFETY: `u64` is a simple integer `Copy` type and `value` lives on the stack so - // a pointer to it will be valid. - unsafe { core::ptr::read_volatile(&value) } - } - let mut difference = Scalar52::ZERO; - let mask = (1u64 << 52) - 1; + let mask = BlackBox::new((1u64 << 52) - 1); // a - b let mut borrow: u64 = 0; for i in 0..5 { borrow = a[i].wrapping_sub(b[i] + (borrow >> 63)); - difference[i] = borrow & mask; + difference[i] = borrow & mask.get(); } // conditionally add l if the difference is negative - let underflow_mask = ((borrow >> 63) ^ 1).wrapping_sub(1); + let underflow_mask = BlackBox::new(((borrow >> 63) ^ 1).wrapping_sub(1)); let mut carry: u64 = 0; for i in 0..5 { - // SECURITY: `black_box` prevents LLVM from inserting a `jns` conditional on x86(_64) + // SECURITY: `BlackBox` prevents LLVM from inserting a `jns` conditional on x86(_64) // which can be used to bypass this section when `underflow_mask` is zero. - carry = (carry >> 52) + difference[i] + (constants::L[i] & black_box(underflow_mask)); - difference[i] = carry & mask; + carry = (carry >> 52) + difference[i] + (constants::L[i] & underflow_mask.get()); + difference[i] = carry & mask.get(); } difference