cmov: impl optimized CmovEq for [u8] [BREAKING]

.github/workflows/ctutils.yml

@@ -45,10 +45,11 @@ jobs:
           targets: ${{ matrix.target }}
       - run: cargo build --target ${{ matrix.target }}
 
-  minimal-versions:
-    uses: RustCrypto/actions/.github/workflows/minimal-versions.yml@master
-    with:
-      working-directory: ${{ github.workflow }}
+  # Disabled until there's a stable `cmov` v0.5.0 release
+  #  minimal-versions:
+  #    uses: RustCrypto/actions/.github/workflows/minimal-versions.yml@master
+  #    with:
+  #      working-directory: ${{ github.workflow }}
 
   test:
     runs-on: ubuntu-latest

cmov/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "cmov"
-version = "0.4.6"
+version = "0.5.0-pre"
 authors = ["RustCrypto Developers"]
 edition = "2024"
 rust-version = "1.85"

cmov/src/array.rs

@@ -1,35 +1,22 @@
 //! Trait impls for core arrays.
 
-use crate::{
-    Cmov, CmovEq, Condition,
-    utils::{WORD_SIZE, Word, slice_as_chunks},
-};
+use crate::{Cmov, CmovEq, Condition, slice::cmovnz_slice_unchecked};
 
 /// Optimized implementation for byte arrays which coalesces them into word-sized chunks first,
 /// then performs [`Cmov`] at the word-level to cut down on the total number of instructions.
 impl<const N: usize> Cmov for [u8; N] {
     #[inline]
     fn cmovnz(&mut self, value: &Self, condition: Condition) {
-        self.as_mut_slice().cmovnz(value, condition);
+        // "unchecked" means it doesn't check the inputs are equal-length, however they are in this
+        // context because they're two equal-sized arrays
+        cmovnz_slice_unchecked(self, value, condition);
     }
 }
 
 /// Optimized implementation for byte arrays which coalesces them into word-sized chunks first,
 /// then performs [`CmovEq`] at the word-level to cut down on the total number of instructions.
 impl<const N: usize> CmovEq for [u8; N] {
     fn cmovne(&self, rhs: &Self, input: Condition, output: &mut Condition) {
-        let (self_chunks, self_remainder) = slice_as_chunks::<u8, WORD_SIZE>(self);
-        let (rhs_chunks, rhs_remainder) = slice_as_chunks::<u8, WORD_SIZE>(rhs);
-
-        for (self_chunk, rhs_chunk) in self_chunks.iter().zip(rhs_chunks.iter()) {
-            let a = Word::from_ne_bytes(*self_chunk);
-            let b = Word::from_ne_bytes(*rhs_chunk);
-            a.cmovne(&b, input, output);
-        }
-
-        // Process the remainder a byte-at-a-time.
-        for (a, b) in self_remainder.iter().zip(rhs_remainder.iter()) {
-            a.cmovne(b, input, output);
-        }
+        self.as_slice().cmovne(rhs, input, output);
     }
 }

cmov/src/lib.rs

@@ -28,7 +28,7 @@
 )]
 
 #[macro_use]
-mod utils;
+mod macros;
 
 #[cfg(not(miri))]
 #[cfg(target_arch = "aarch64")]

cmov/src/macros.rs

@@ -0,0 +1,62 @@
+//! Macro definitions.
+
+/// Generates a mask the width of the given unsigned integer type `$uint` if the input value is
+/// non-zero.
+///
+/// Uses `core::hint::black_box` to coerce our desired codegen based on real-world observations
+/// of the assembly generated by Rust/LLVM.
+///
+/// Implemented as a macro instead of a generic function because it uses functionality for which
+/// there aren't available `core` traits, e.g. `wrapping_neg`.
+///
+/// See also:
+/// - CVE-2026-23519
+/// - RustCrypto/utils#1332
+macro_rules! masknz {
+    ($value:tt : $uint:ident) => {{
+        let mut value: $uint = $value;
+        value |= value.wrapping_neg(); // has MSB `1` if non-zero, `0` if zero
+
+        // use `black_box` to obscure we're computing a 1-bit value
+        core::hint::black_box(
+            value >> ($uint::BITS - 1), // Extract MSB
+        )
+        .wrapping_neg() // Generate $uint::MAX mask if `black_box` outputs `1`
+    }};
+}
+
+#[cfg(test)]
+mod tests {
+    // Spot check up to a given limit
+    const TEST_LIMIT: u8 = 128;
+
+    macro_rules! masknz_test {
+        ( $($name:ident : $uint:ident),+ ) => {
+            $(
+                #[test]
+                fn $name() {
+                    assert_eq!(masknz!(0: $uint), 0);
+
+                    // Test lower values
+                    for i in 1..=$uint::from(TEST_LIMIT) {
+                        assert_eq!(masknz!(i: $uint), $uint::MAX);
+                    }
+
+                    // Test upper values
+                    for i in ($uint::MAX - $uint::from(TEST_LIMIT))..=$uint::MAX {
+                        assert_eq!(masknz!(i: $uint), $uint::MAX);
+                    }
+                }
+            )+
+        }
+    }
+
+    // Ensure the macro works with any types we might use it with (we only use u8, u32, and u64)
+    masknz_test!(
+        masknz_u8: u8,
+        masknz_u16: u16,
+        masknz_u32: u32,
+        masknz_u64: u64,
+        masknz_u128: u128
+    );
+}

cmov/src/slice.rs

@@ -1,31 +1,43 @@
 //! Trait impls for core slices.
 
-use crate::utils::{WORD_SIZE, Word, slice_as_chunks, slice_as_chunks_mut};
 use crate::{Cmov, CmovEq, Condition};
+use core::slice;
+
+// Uses 64-bit words on 64-bit targets, 32-bit everywhere else
+#[cfg(not(target_pointer_width = "64"))]
+type Word = u32;
+#[cfg(target_pointer_width = "64")]
+type Word = u64;
+const WORD_SIZE: usize = size_of::<Word>();
+const _: () = assert!(size_of::<usize>() <= WORD_SIZE, "unexpected word size");
 
 /// Optimized implementation for byte slices which coalesces them into word-sized chunks first,
 /// then performs [`Cmov`] at the word-level to cut down on the total number of instructions.
+///
+/// # Panics
+/// - if slices have unequal lengths
 impl Cmov for [u8] {
     #[inline]
     fn cmovnz(&mut self, value: &Self, condition: Condition) {
-        let (self_chunks, self_remainder) = slice_as_chunks_mut::<u8, WORD_SIZE>(self);
-        let (value_chunks, value_remainder) = slice_as_chunks::<u8, WORD_SIZE>(value);
-
-        for (self_chunk, value_chunk) in self_chunks.iter_mut().zip(value_chunks.iter()) {
-            let mut a = Word::from_ne_bytes(*self_chunk);
-            let b = Word::from_ne_bytes(*value_chunk);
-            a.cmovnz(&b, condition);
-            self_chunk.copy_from_slice(&a.to_ne_bytes());
-        }
+        assert_eq!(
+            self.len(),
+            value.len(),
+            "source slice length ({}) does not match destination slice length ({})",
+            value.len(),
+            self.len()
+        );
 
-        // Process the remainder a byte-at-a-time.
-        for (a, b) in self_remainder.iter_mut().zip(value_remainder.iter()) {
-            a.cmovnz(b, condition);
-        }
+        cmovnz_slice_unchecked(self, value, condition);
     }
 }
 
-impl<T: CmovEq> CmovEq for [T] {
+/// Optimized implementation for byte arrays which coalesces them into word-sized chunks first,
+/// then performs [`CmovEq`] at the word-level to cut down on the total number of instructions.
+///
+/// This is only constant-time for equal-length slices, and will short-circuit and set `output`
+/// in the event the slices are of unequal length.
+impl CmovEq for [u8] {
+    #[inline]
     fn cmovne(&self, rhs: &Self, input: Condition, output: &mut Condition) {
         // Short-circuit the comparison if the slices are of different lengths, and set the output
         // condition to the input condition.
@@ -34,9 +46,109 @@ impl<T: CmovEq> CmovEq for [T] {
             return;
         }
 
-        // Compare each byte.
-        for (a, b) in self.iter().zip(rhs.iter()) {
+        let (self_chunks, self_remainder) = slice_as_chunks::<u8, WORD_SIZE>(self);
+        let (rhs_chunks, rhs_remainder) = slice_as_chunks::<u8, WORD_SIZE>(rhs);
+
+        for (self_chunk, rhs_chunk) in self_chunks.iter().zip(rhs_chunks.iter()) {
+            let a = Word::from_ne_bytes(*self_chunk);
+            let b = Word::from_ne_bytes(*rhs_chunk);
+            a.cmovne(&b, input, output);
+        }
+
+        // Process the remainder a byte-at-a-time.
+        for (a, b) in self_remainder.iter().zip(rhs_remainder.iter()) {
             a.cmovne(b, input, output);
         }
     }
 }
+
+/// Conditionally move `src` to `dst` in constant-time if `condition` is non-zero.
+///
+/// This function does not check the slices are equal-length and expects the caller to do so first.
+#[inline(always)]
+pub(crate) fn cmovnz_slice_unchecked(dst: &mut [u8], src: &[u8], condition: Condition) {
+    let (dst_chunks, dst_remainder) = slice_as_chunks_mut::<u8, WORD_SIZE>(dst);
+    let (src_chunks, src_remainder) = slice_as_chunks::<u8, WORD_SIZE>(src);
+
+    for (dst_chunk, src_chunk) in dst_chunks.iter_mut().zip(src_chunks.iter()) {
+        let mut a = Word::from_ne_bytes(*dst_chunk);
+        let b = Word::from_ne_bytes(*src_chunk);
+        a.cmovnz(&b, condition);
+        dst_chunk.copy_from_slice(&a.to_ne_bytes());
+    }
+
+    // Process the remainder a byte-at-a-time.
+    for (a, b) in dst_remainder.iter_mut().zip(src_remainder.iter()) {
+        a.cmovnz(b, condition);
+    }
+}
+
+/// Rust core `[T]::as_chunks` vendored because of its 1.88 MSRV.
+/// TODO(tarcieri): use upstream function when we bump MSRV
+#[inline]
+#[track_caller]
+#[must_use]
+#[allow(clippy::integer_division_remainder_used)]
+fn slice_as_chunks<T, const N: usize>(slice: &[T]) -> (&[[T; N]], &[T]) {
+    assert!(N != 0, "chunk size must be non-zero");
+    let len_rounded_down = slice.len() / N * N;
+    // SAFETY: The rounded-down value is always the same or smaller than the
+    // original length, and thus must be in-bounds of the slice.
+    let (multiple_of_n, remainder) = unsafe { slice.split_at_unchecked(len_rounded_down) };
+    // SAFETY: We already panicked for zero, and ensured by construction
+    // that the length of the subslice is a multiple of N.
+    let array_slice = unsafe { slice_as_chunks_unchecked(multiple_of_n) };
+    (array_slice, remainder)
+}
+
+/// Rust core `[T]::as_chunks_mut` vendored because of its 1.88 MSRV.
+/// TODO(tarcieri): use upstream function when we bump MSRV
+#[inline]
+#[track_caller]
+#[must_use]
+#[allow(clippy::integer_division_remainder_used)]
+fn slice_as_chunks_mut<T, const N: usize>(slice: &mut [T]) -> (&mut [[T; N]], &mut [T]) {
+    assert!(N != 0, "chunk size must be non-zero");
+    let len_rounded_down = slice.len() / N * N;
+    // SAFETY: The rounded-down value is always the same or smaller than the
+    // original length, and thus must be in-bounds of the slice.
+    let (multiple_of_n, remainder) = unsafe { slice.split_at_mut_unchecked(len_rounded_down) };
+    // SAFETY: We already panicked for zero, and ensured by construction
+    // that the length of the subslice is a multiple of N.
+    let array_slice = unsafe { slice_as_chunks_unchecked_mut(multiple_of_n) };
+    (array_slice, remainder)
+}
+
+/// Rust core `[T]::as_chunks_unchecked` vendored because of its 1.88 MSRV.
+/// TODO(tarcieri): use upstream function when we bump MSRV
+#[inline]
+#[must_use]
+#[track_caller]
+#[allow(clippy::integer_division_remainder_used)]
+unsafe fn slice_as_chunks_unchecked<T, const N: usize>(slice: &[T]) -> &[[T; N]] {
+    // SAFETY: Caller must guarantee that `N` is nonzero and exactly divides the slice length
+    const { debug_assert!(N != 0) };
+    debug_assert_eq!(slice.len() % N, 0);
+    let new_len = slice.len() / N;
+
+    // SAFETY: We cast a slice of `new_len * N` elements into
+    // a slice of `new_len` many `N` elements chunks.
+    unsafe { slice::from_raw_parts(slice.as_ptr().cast(), new_len) }
+}
+
+/// Rust core `[T]::as_chunks_unchecked_mut` vendored because of its 1.88 MSRV.
+/// TODO(tarcieri): use upstream function when we bump MSRV
+#[inline]
+#[must_use]
+#[track_caller]
+#[allow(clippy::integer_division_remainder_used)]
+unsafe fn slice_as_chunks_unchecked_mut<T, const N: usize>(slice: &mut [T]) -> &mut [[T; N]] {
+    // SAFETY: Caller must guarantee that `N` is nonzero and exactly divides the slice length
+    const { debug_assert!(N != 0) };
+    debug_assert_eq!(slice.len() % N, 0);
+    let new_len = slice.len() / N;
+
+    // SAFETY: We cast a slice of `new_len * N` elements into
+    // a slice of `new_len` many `N` elements chunks.
+    unsafe { slice::from_raw_parts_mut(slice.as_mut_ptr().cast(), new_len) }
+}