Optimize Scalar constant time canonical check #384
Conversation
|
Note: pub fn is_canonical(&self) -> Choice {
let mut over = 0;
let mut under = 0;
for (this, l) in self.unpack().0.iter().zip(&constants::L.0).rev() {
let gt = (this > l) as u8;
let eq = (this == l) as u8;
under |= (!gt & !eq) &!over;
over |= gt;
}
Choice::from(under & 1)
} |
elichai
force-pushed
the
ct_canonical
branch
from
988f281
to
3e73ba3
Compare
|
Yet another option, is using https://github.com/veorq/cryptocoding for the eq/gt code, and doing it like this: pub fn is_canonical(&self) -> Choice {
let mut over = 0;
let mut under = 0;
for (&this, &l) in self.unpack().0.iter().zip(&constants::L.0).rev() {
let gt = (l ^ ((l ^ this) | (l.wrapping_sub(this) ^ this))) >> 63;
let eq = 1 ^ ((this.wrapping_sub(l) | l.wrapping_sub(this)) >> 63);
under |= (!gt & !eq) & !over;
over |= gt;
}
Choice::from((under & 1) as u8)
}With the following ASM: https://godbolt.org/z/r4zvrG9ex |
elichai
changed the title
|
I rebased the branch on top of #472 and updated the benchmarks, description and topic But now that it's only 20% improvement it's also not as interesting, unless we go with one of the more performant impls which uses |
elichai
force-pushed
the
ct_canonical
branch
from
3e73ba3
to
1e4f0d4
Compare
tarcieri
changed the title
src/scalar.rs
Outdated
| pub fn is_canonical(&self) -> Choice { | ||
| self.ct_eq(&self.reduce()) | ||
| let mut over = Choice::from(0); | ||
| let mut under = Choice::from(0); | ||
| for (this, l) in self.unpack().0.iter().zip(&constants::L.0).rev() { | ||
| let gt = this.ct_gt(l); | ||
| let eq = this.ct_eq(l); | ||
| under |= (!gt & !eq) & !over; | ||
| over |= gt; | ||
| } | ||
| under | ||
| } |
There was a problem hiding this comment.
This seems like it should be fine.
Another way to do this check is to subtract the modulus and check for underflow, although I don't think that's going to be straightforward with UnpackedScalar::sub.
There was a problem hiding this comment.
That's an interesting idea!
I hacked your idea right now, and it seems to be the fastest yet :)
it drops to ~40ns
I'll verify the code by hand and figure out what's the best place/naming for it but that's the logic I have in mind (and it passes the tests) https://godbolt.org/z/1Pz9a56nv (This code is incorrect, shouldn't write cryptography at night)
(sadly this would need to be added to the backends)
elichai
changed the title
|
Rebased ontop of |
Edited after the merge of #472
Currently, the
is_canonicalcheck fully reduces the scalar and only then does a constant time comparison.This isn't really necessary and instead, we can check if it's smaller than L, this implements the direct comparison without the need to reduce the scalar
Benchmarks from my machine (i9-9980HK @ 2.4GHz turbo off)
Before:
After: