Constant-time square root and division #277

HastD · 2023-09-13T18:51:31Z

This un-deprecates Uint::sqrt and provides a constant-time implementation. (Unfortunately this makes it non-const, so it's technically a breaking change; I don't see a way to handle zero without using checked_div, which is why this can't be const. For const contexts, Uint::sqrt_vartime can still be used.) [Edit: this isn't an issue after all, see below discussion.] The algorithm is just a slight modification of Algorithm 1.13 of Brent-Zimmermann as used in sqrt_vartime: instead of iterating until xn increases, we just iterate a fixed number of times (logarithmic in the number of bits) and then take the minimum of guess and xn at the end.

The reason this works is because of two properties of this algorithm that I prove here: ct_sqrt.pdf. (We can upload this somewhere more permanent—not sure where would be best. Also, it'd be great if someone could look this over to make sure I didn't make any mistakes in the proof.) First, we have an upper bound the number of iterations required. Second, once the algorithm reaches the desired value sqrt(self), it will either repeat sqrt(self) or it will alternate between sqrt(self) and sqrt(self) + 1; thus, we can return the minimum of guess and xn.

I also removed some dead code in Uint::sqrt_vartime: unless I'm really missing something here, the first while loop can never run because the initial value of guess is defined such that it's never an underestimate.

Edit: Just noticed I forgot to un-deprecate checked_sqrt and wrapping_sqrt and make those point to sqrt rather than sqrt_vartime, but obviously that goes along with this and I've added those changes too.

The initial guess is 2^(ceil(bits/2)), which is always greater than sqrt(self), so initially `xn < guess`, which means the removed while loop never runs.

This un-deprecates sqrt, makes it non-const (to be able to use checked_div), and provides a constant-time implementation using essentially the same algorithm as sqrt_vartime, but run for a fixed number of iterations.

tarcieri · 2023-09-13T19:04:35Z

We can upload this somewhere more permanent—not sure where would be best.

arXiv perhaps?

HastD · 2023-09-13T20:41:41Z

Sure, I can upload it to arXiv (cs.DS probably?). I'd like to wait until at least one other person has looked it over it reduce the chance of errors slipping through, but after that I'll go ahead.

tarcieri · 2023-09-14T15:47:08Z

cc @fjarri

src/uint/sqrt.rs

fjarri · 2023-09-14T16:01:13Z

src/uint/sqrt.rs

+        let cap = Self::ONE.shl(max_bits);
+        let mut guess = cap; // ≥ √(`self`)
+        let mut xn = {
+            let q = self.wrapping_div(&guess);


I don't think wrapping_div() is currently constant-time (although it could be made so).

My mistake, I didn't see that wrapping_div is only constant-time for a fixed rhs. Actually, I think there might be a documentation issue here—I can't find anywhere in the public documentation that says this, only the documentation for ct_div_rem (which doesn't appear in the public docs since it's pub(crate)).

What do you think would be the better approach here: making a new function that's like ct_div_rem but constant-time with respect to both inputs, or modifying ct_div_rem to have this stronger constant-time guarantee and moving the "constant-time only for fixed rhs" behavior to a new function?

I agree that the name is misleading - it should have had a vartime suffix (and the whole div.rs is kind of a mess in terms of naming - see #268). So the proper way to proceed I think would be to rename the current one to _vartime, and implement a constant-time one in its place.

It appears this is a blocker on merging this PR. I guess we can go ahead and flip over to the v0.6 series per #268 and try to land this PR afterward.

Checking on this: wrapping_div calls const_div_rem, which claims:

/// This function is constant-time with respect to both `self` and `rhs`. pub(crate) const fn const_div_rem(&self, rhs: &Self) -> (Self, Self, CtChoice) {

Which commit are you looking at? The current master has no const_div_rem(), and Uint::wrapping_div() uses ct_div_rem(), which is not constant-time in rhs.

This PR changes wrapping_div to call const_div_rem, which is also added by this PR

fjarri · 2023-09-14T16:03:16Z

I don't see a way to handle zero without using checked_div, which is why this can't be const

I am not sure I see the problem here, you can always use the underlying Uint::ct_div_rem() and use the returned CtChoice to conditionally select the result.

HastD · 2023-09-14T16:47:27Z

I am not sure I see the problem here, you can always use the underlying Uint::ct_div_rem() and use the returned CtChoice to conditionally select the result.

I just overlooked that, so there's no obstruction to keeping it const. Actually, it looks like Uint::ct_div_rem already internally does that conditional selection (so the returned quotient is zero if rhs is zero); can that behavior be relied on or is it an internal implementation detail of ct_div_rem? For reference, the relevant lines of code in ct_div_rem:

    pub(crate) const fn ct_div_rem(&self, rhs: &Self) -> (Self, Self, CtChoice) {
        let mb = rhs.bits_vartime();
        // [...]
        let is_some = Limb(mb as Word).ct_is_nonzero();
        quo = Self::ct_select(&Self::ZERO, &quo, is_some);
        (quo, rem, is_some)
    }

fjarri · 2023-09-14T20:04:28Z

can that behavior be relied on or is it an internal implementation detail of ct_div_rem

I think it should be treated as an implementation detail.

fjarri · 2023-09-14T20:14:30Z

Also:

I'll go through your proof in a few days, looks like it'll require some time and concentration :)
Perhaps a proptest (see tests/proptests.rs) would be good to add here, for both vartime and const-time versions.

HastD · 2023-09-14T20:43:35Z

Okay, I just made the necessary changes so sqrt and wrapping_sqrt are const functions again, and I modified the wrapping_sqrt proptest to test both the vartime and constant-time versions. However, as discussed above, it's still not actually constant-time yet because ct_div_rem (and hence also wrapping_div) are currently only constant-time for fixed divisor.

This makes `Uint::sqrt` constant-time as well.

HastD · 2023-09-20T14:12:43Z

Okay, I added an implementation of division that's constant-time in both arguments, and moved the old division implementation (that's constant-time only for fixed divisor) to _vartime functions (div_rem_vartime, wrapping_div_vartime, etc). If this has been implemented correctly then Uint::sqrt should be fully constant-time now.

My constant-time division implementation is a straightforward modification of the vartime algorithm, just replacing vartime methods with their constant-time variants, and running for the maximum number of iterations with a ct_select to avoid modifying the outputs beyond when the loop would've terminated in the vartime version. I suspect it's possible to do this more efficiently but I'll put this out there as a starting point. How's it look?

fjarri · 2023-10-10T00:10:50Z

src/uint/sqrt.rs

+        // Repeat enough times to guarantee result has stabilized.
+        // See Hast, "Note on computation of integer square roots" for a proof of this bound.
+        let mut i = 0;
+        while i < usize::BITS - Self::BITS.leading_zeros() {


Self::LOG2_BITS could be used here

fjarri · 2023-10-10T00:11:18Z

src/uint/sqrt.rs

+        };
+
+        // Repeat enough times to guarantee result has stabilized.
+        // See Hast, "Note on computation of integer square roots" for a proof of this bound.


When your paper has a more permanent link (e.g. on arxiv), please make a PR referencing it here

fjarri · 2023-10-10T00:11:24Z

src/uint/sqrt.rs

+            guess = xn;
+            xn = {
+                let (q, _, is_some) = self.const_div_rem(&guess);
+                let q = Self::ct_select(&Self::ZERO, &q, is_some);


Is this needed specifically to handle the case of self == 0?

fjarri · 2023-10-10T00:15:01Z

src/uint/sqrt.rs

+            i += 1;
+        }
+
+        // at least one of `guess` and `xn` is now equal to √(`self`), so return the minimum


So at this point guess == x_n, and xn == x_{n+1}, where n = floor(log2(Self::BITS)). But in the paper it says that it should be n = floor(log2(Self::BITS)) + 1 - am I missing something?

fjarri · 2023-10-10T00:15:47Z

src/uint/sqrt.rs

-                t.shr_vartime(1)
-            };
-        }
+        // Note, xn <= guess at this point.

        // Repeat while guess decreases.
        while Uint::ct_gt(&guess, &xn).is_true_vartime() && xn.ct_is_nonzero().is_true_vartime() {


We don't need to use ct_gt() and ct_is_nonzero() here, those are constant-time

fjarri · 2023-10-10T00:16:07Z

src/uint/sqrt.rs

@@ -56,29 +64,26 @@ impl<const LIMBS: usize> Uint<LIMBS> {
        Self::ct_select(&Self::ZERO, &guess, self.ct_is_nonzero())


Similarly here, we don't need constant-timeness.

fjarri · 2023-10-10T00:17:53Z

Sorry for the delay, I was kind of swamped at work. I've read the paper, and it looks sound to me (although a bit too terse :). Have you submitted it anywhere yet? Would be nice to have a reference in the comments to the code.

I've also got some relatively minor comments to the code.

tarcieri · 2023-10-10T00:22:02Z

I've read the paper, and it looks sound to me (although a bit too terse :). Have you submitted it anywhere yet? Would be nice to have a reference in the comments to the code.

I think with a bit more polish on it you could throw it on the IACR ePrint service.

fjarri · 2023-10-10T04:42:47Z

src/uint/sqrt.rs

+        for i in 0..half.limbs.len() / 2 {
+            half.limbs[i] = Limb::MAX;
+        }
+        assert_eq!(U256::MAX.sqrt(), half);


An idea for the edge case test: a number that actually needs the maximum amount of iterations to converge. According to my tests, the ones before 10000 are 80, 99, 4224, 4355, 4488, 4623, 4760, 4899; but please check independently. (Also an interesting mathematical question - is there some rule for their distribution)

Based on PR #277. The constant-time square root algorithm is described here: https://github.com/RustCrypto/crypto-bigint/files/12600669/ct_sqrt.pdf Co-authored-by: Daniel Hast <32797673+HastD@users.noreply.github.com>

tarcieri · 2023-12-03T14:58:50Z

Merged in #376

Adapts the implementation originally from #277 to `BoxedUint`, adding the following methods: - `BoxedUint::div_rem` - `BoxedUint::rem` Additionally, `wrapping_div` and `checked_div` have been changed to use the constant-time versions, rather than `*_vartime`.

HastD added 3 commits September 13, 2023 13:49

Removed dead code in sqrt_vartime

52a4184

The initial guess is 2^(ceil(bits/2)), which is always greater than sqrt(self), so initially `xn < guess`, which means the removed while loop never runs.

Constant-time implementation of sqrt

9980209

This un-deprecates sqrt, makes it non-const (to be able to use checked_div), and provides a constant-time implementation using essentially the same algorithm as sqrt_vartime, but run for a fixed number of iterations.

avoid ilog2 to maintain MSRV

82d4b9d

HastD added 2 commits September 13, 2023 17:18

Constant-time wrapping and checked variants

f5e2ad7

test both ct and vartime versions

6e6c544

fjarri reviewed Sep 14, 2023

View reviewed changes

src/uint/sqrt.rs Show resolved Hide resolved

fjarri reviewed Sep 14, 2023

View reviewed changes

HastD marked this pull request as draft September 14, 2023 17:00

HastD added 2 commits September 14, 2023 16:19

restored const for sqrt and wrapping_sqrt

48aba50

proptest for both vartime and ct sqrt

bbce847

division that's constant-time in both arguments

5086d13

This makes `Uint::sqrt` constant-time as well.

HastD changed the title ~~Constant-time square root~~ Constant-time square root and division Sep 20, 2023

HastD marked this pull request as ready for review September 20, 2023 14:12

fjarri reviewed Oct 10, 2023

View reviewed changes

tarcieri mentioned this pull request Nov 18, 2023

Multi-exponentiation #248

Merged

tarcieri added a commit that referenced this pull request Dec 2, 2023

Address review comments from #277

92d231a

tarcieri mentioned this pull request Dec 2, 2023

Constant-time square root and division #376

Merged

tarcieri closed this Dec 3, 2023

tarcieri mentioned this pull request Dec 6, 2023

BoxedUint: add constant-time division implementation #398

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Constant-time square root and division #277

Constant-time square root and division #277

HastD commented Sep 13, 2023 •

edited

Loading

tarcieri commented Sep 13, 2023

HastD commented Sep 13, 2023

tarcieri commented Sep 14, 2023

fjarri Sep 14, 2023

HastD Sep 14, 2023

fjarri Sep 14, 2023

tarcieri Nov 18, 2023

tarcieri Dec 2, 2023

fjarri Dec 2, 2023

tarcieri Dec 2, 2023

fjarri commented Sep 14, 2023

HastD commented Sep 14, 2023

fjarri commented Sep 14, 2023

fjarri commented Sep 14, 2023

HastD commented Sep 14, 2023

HastD commented Sep 20, 2023

fjarri Oct 10, 2023

fjarri Oct 10, 2023

fjarri Oct 10, 2023

fjarri Oct 10, 2023

fjarri Oct 10, 2023

fjarri Oct 10, 2023

fjarri commented Oct 10, 2023

tarcieri commented Oct 10, 2023

fjarri Oct 10, 2023

tarcieri commented Dec 3, 2023

		@@ -56,29 +64,26 @@ impl<const LIMBS: usize> Uint<LIMBS> {
		Self::ct_select(&Self::ZERO, &guess, self.ct_is_nonzero())

Constant-time square root and division #277

Constant-time square root and division #277

Conversation

HastD commented Sep 13, 2023 • edited Loading

tarcieri commented Sep 13, 2023

HastD commented Sep 13, 2023

tarcieri commented Sep 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fjarri commented Sep 14, 2023

HastD commented Sep 14, 2023

fjarri commented Sep 14, 2023

fjarri commented Sep 14, 2023

HastD commented Sep 14, 2023

HastD commented Sep 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fjarri commented Oct 10, 2023

tarcieri commented Oct 10, 2023

Choose a reason for hiding this comment

tarcieri commented Dec 3, 2023

HastD commented Sep 13, 2023 •

edited

Loading