Skip to content

Commit

Permalink
Factor out most-significant-limb masking and test it.
Browse files Browse the repository at this point in the history
  • Loading branch information
briansmith committed Aug 22, 2016
1 parent 70e0734 commit 3486e57
Showing 1 changed file with 49 additions and 12 deletions.
61 changes: 49 additions & 12 deletions src/limb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ use {rand, polyfill, c, core, error};
#[cfg(target_pointer_width = "32")] pub type Limb = u32;
#[cfg(target_pointer_width = "64")] pub const LIMB_BITS: usize = 64;
#[cfg(target_pointer_width = "32")] pub const LIMB_BITS: usize = 32;
#[cfg(target_pointer_width = "64")] const LIMB_BITS_U32: u32 = 64;
#[cfg(target_pointer_width = "32")] const LIMB_BITS_U32: u32 = 32;


pub const LIMB_BYTES: usize = (LIMB_BITS + 7) / 8;

Expand All @@ -44,12 +47,6 @@ impl <'a> Range<'a> {
}
}

fn leading_zero_bits(&self) -> usize {
let most_significant_limb =
self.max_exclusive[self.max_exclusive.len() - 1];
most_significant_limb.leading_zeros() as usize
}

/// Are these little-endian limbs within the range?
///
/// Checks in constant time.
Expand All @@ -75,7 +72,12 @@ impl <'a> Range<'a> {
-> Result<(), error::Unspecified> {
assert_eq!(self.max_exclusive.len(), dest.len());

let bits_to_mask = self.leading_zero_bits();
let most_significant_limb =
self.max_exclusive[self.max_exclusive.len() - 1];
debug_assert!(most_significant_limb > 0);
let most_significant_limb_mask =
most_significant_limb_mask_variable_time(most_significant_limb);
debug_assert!(most_significant_limb_mask != 0);

// XXX: The value 100 was chosen to match OpenSSL due to uncertainty of
// what specific value would be better, but it seems bad to try 100
Expand All @@ -85,11 +87,7 @@ impl <'a> Range<'a> {
let mut dest_as_bytes = limbs_as_bytes_mut(dest);
try!(rng.fill(&mut dest_as_bytes));
}

if bits_to_mask > 0 {
let mask: Limb = (1 << (LIMB_BITS - bits_to_mask)) - 1;
dest[self.max_exclusive.len() - 1] &= mask;
}
dest[self.max_exclusive.len() - 1] &= most_significant_limb_mask;

if self.are_limbs_within(&dest) {
return Ok(());
Expand All @@ -98,7 +96,20 @@ impl <'a> Range<'a> {

Err(error::Unspecified)
}
}

/// Returns a mask that has the same number of leading zeros as
/// `most_significant_limb`, with all the following bits set.
fn most_significant_limb_mask_variable_time(most_significant_limb: Limb)
-> Limb {
const ONE: Limb = 1;

let bits_to_mask_off = most_significant_limb.leading_zeros();
if bits_to_mask_off == 0 {
Limb::max_value()
} else {
(ONE << (LIMB_BITS_U32 - bits_to_mask_off)) - 1

This comment has been minimized.

Copy link
@djudd

djudd Aug 22, 2016

I don't understand the necessity for this new constant. Even if we need a u32 for some reason I don't understand, couldn't we do usize as u32 safely?

This comment has been minimized.

Copy link
@briansmith

briansmith Aug 22, 2016

Author Owner

Yes, we could. But in general I'm trying to avoid the use of as. See #31. I agree that it isn't clearly better. Not clearly worse either, though.

}
}

#[allow(unsafe_code)]
Expand Down Expand Up @@ -162,8 +173,34 @@ extern {
#[cfg(test)]
mod tests {
use super::*;
use super::most_significant_limb_mask_variable_time;
use rand;

#[test]
fn test_most_significant_limb_mask() {
assert_eq!(0, most_significant_limb_mask_variable_time(0));

for i in 0..LIMB_BITS {
let x = 1 << i;
let expected = if i == LIMB_BITS - 1 {
Limb::max_value()
} else {
(1 << (i + 1)) - 1
};
assert_eq!(expected, most_significant_limb_mask_variable_time(x),
"for {:?}", x);
assert_eq!(expected,
most_significant_limb_mask_variable_time(x | 1),
"for {:?}", x | 1);
assert_eq!(expected,
most_significant_limb_mask_variable_time(x | (x >> 1)),
"for {:?}", x | (x >> 1));
assert_eq!(expected,
most_significant_limb_mask_variable_time(expected),
"for {:?}", expected);
}
}

#[test]
fn test_limbs_in_range() {
let limbs = &[Limb::max_value(), Limb::max_value()];
Expand Down

0 comments on commit 3486e57

Please sign in to comment.