From 4e6a2cff0422d49ef3b1cff3aa037c0586e112f8 Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Tue, 18 Jun 2024 12:51:56 -0600 Subject: [PATCH] SECURITY: fix timing variability in backend/serial/u32/scalar.rs Similar security fix to #659, but for the 32-bit backend. See that PR for more information about the problem. --- curve25519-dalek/src/backend/serial/u32/scalar.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/curve25519-dalek/src/backend/serial/u32/scalar.rs b/curve25519-dalek/src/backend/serial/u32/scalar.rs index 2d135d1d..29d4df5f 100644 --- a/curve25519-dalek/src/backend/serial/u32/scalar.rs +++ b/curve25519-dalek/src/backend/serial/u32/scalar.rs @@ -185,6 +185,14 @@ 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; @@ -199,7 +207,7 @@ impl Scalar29 { let underflow_mask = ((borrow >> 31) ^ 1).wrapping_sub(1); let mut carry: u32 = 0; for i in 0..9 { - carry = (carry >> 29) + difference[i] + (constants::L[i] & underflow_mask); + carry = (carry >> 29) + difference[i] + (constants::L[i] & black_box(underflow_mask)); difference[i] = carry & mask; }