Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add ConstantTimeEq implementation for [T; N]. #114

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add ConstantTimeEq implementation for [T; N]. #114

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
17edc81
Add ConstantTimeEq implementation for [T; N].
branlwyd Sep 7, 2023
a829f68
Fall back to slice ct_eq implementation.
branlwyd Sep 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 18 additions & 1 deletion src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
//! enable the `core_hint_black_box` feature.
//!
//! Rust versions from 1.51 or higher have const generics support. You may enable
//! `const-generics` feautre to have `subtle` traits implemented for arrays `[T; N]`.
//! `const-generics` feature to have `subtle` traits implemented for arrays `[T; N]`.
//!
//! Versions prior to `2.2` recommended use of the `nightly` feature to enable an
//! optimization barrier; this is not required in versions `2.2` and above.
Expand Down Expand Up @@ -597,6 +597,23 @@ where
}
}

#[cfg(feature = "const-generics")]
impl<T, const N: usize> ConstantTimeEq for [T; N]
where
T: ConstantTimeEq,
{
fn ct_eq(&self, other: &Self) -> Choice {
// This loop shouldn't be shortcircuitable, since the compiler
// shouldn't be able to reason about the value of the `u8`
// unwrapped from the `ct_eq` result.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the compiler were sufficiently smart, if it noticed x became 0 it could short circuit the loop because anything & 0 is 0.

Why not just use a Choice? (or use the slice impl)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering the same thing; I switched to falling back to the slice implementation. But the slice implementation uses literally the same code for its element-by-element comparison loop:

subtle/src/lib.rs

Lines 299 to 342 in 6b6a81a

impl<T: ConstantTimeEq> ConstantTimeEq for [T] {
/// Check whether two slices of `ConstantTimeEq` types are equal.
///
/// # Note
///
/// This function short-circuits if the lengths of the input slices
/// are different. Otherwise, it should execute in time independent
/// of the slice contents.
///
/// Since arrays coerce to slices, this function works with fixed-size arrays:
///
/// ```
/// # use subtle::ConstantTimeEq;
/// #
/// let a: [u8; 8] = [0,1,2,3,4,5,6,7];
/// let b: [u8; 8] = [0,1,2,3,0,1,2,3];
///
/// let a_eq_a = a.ct_eq(&a);
/// let a_eq_b = a.ct_eq(&b);
///
/// assert_eq!(a_eq_a.unwrap_u8(), 1);
/// assert_eq!(a_eq_b.unwrap_u8(), 0);
/// ```
#[inline]
fn ct_eq(&self, _rhs: &[T]) -> Choice {
let len = self.len();
// Short-circuit on the *lengths* of the slices, not their
// contents.
if len != _rhs.len() {
return Choice::from(0);
}
// This loop shouldn't be shortcircuitable, since the compiler
// shouldn't be able to reason about the value of the `u8`
// unwrapped from the `ct_eq` result.
let mut x = 1u8;
for (ai, bi) in self.iter().zip(_rhs.iter()) {
x &= ai.ct_eq(bi).unwrap_u8();
}
x.into()
}
}

If there's a problem with this code, there may be a pre-existing problem with the implementation for [T].

FWIW, I'm pretty comfortable falling back on the [T] implementation for the [T; N] implementation: a little experimentation showed that the Rust compiler can reason about slice lengths to optimize away length checks in similar code (but that's no guarantee that it will do so here, of course).

let mut x = 1u8;
for (ai, bi) in self.iter().zip(other.iter()) {
x &= ai.ct_eq(bi).unwrap_u8();
}
x.into()
}
}

/// A type which can be conditionally negated in constant time.
///
/// # Note
Expand Down
23 changes: 18 additions & 5 deletions tests/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,29 +8,42 @@ use subtle::*;
#[test]
#[should_panic]
fn slices_equal_different_lengths() {
let a: [u8; 3] = [0, 0, 0];
let b: [u8; 4] = [0, 0, 0, 0];
let a: &[u8] = &[0, 0, 0];
let b: &[u8] = &[0, 0, 0, 0];

assert_eq!((&a).ct_eq(&b).unwrap_u8(), 1);
}

#[test]
fn slices_equal() {
let a: [u8; 8] = [1, 2, 3, 4, 5, 6, 7, 8];
let b: [u8; 8] = [1, 2, 3, 4, 4, 3, 2, 1];
let a: &[u8] = &[1, 2, 3, 4, 5, 6, 7, 8];
let b: &[u8] = &[1, 2, 3, 4, 4, 3, 2, 1];

let a_eq_a = (&a).ct_eq(&a);
let a_eq_b = (&a).ct_eq(&b);

assert_eq!(a_eq_a.unwrap_u8(), 1);
assert_eq!(a_eq_b.unwrap_u8(), 0);

let c: [u8; 16] = [0u8; 16];
let c: &[u8] = &[0u8; 16];

let a_eq_c = (&a).ct_eq(&c);
assert_eq!(a_eq_c.unwrap_u8(), 0);
}

#[cfg(feature = "const-generics")]
#[test]
fn arrays_equal() {
let a: [u8; 8] = [1, 2, 3, 4, 5, 6, 7, 8];
let b: [u8; 8] = [1, 2, 3, 4, 4, 3, 2, 1];

let a_eq_a = (&a).ct_eq(&a);
let a_eq_b = (&a).ct_eq(&b);

assert_eq!(a_eq_a.unwrap_u8(), 1);
assert_eq!(a_eq_b.unwrap_u8(), 0);
}

#[test]
fn conditional_assign_i32() {
let mut a: i32 = 5;
Expand Down