-
Notifications
You must be signed in to change notification settings - Fork 92
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.
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
Constant-time EC point multiplication (Montgomery ladder) implementation #325
base: main
Are you sure you want to change the base?
Changes from 18 commits
130b977
6049ea4
e00a891
3e9e54b
cd3156e
06e941c
4b51e44
29638d5
880812c
681239e
1bfffd4
bd67b35
c5e836d
6270ad5
fe22dac
6665e36
fc0f3b8
8127d10
3c122c5
56fcd9f
b7dec5b
f0ad8e1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -65,25 +65,43 @@ export function addPoint(p1: Point<bigint>, p2: Point<bigint>): Point<bigint> { | |
/** | ||
* Performs a scalar multiplication by starting from the 'base' point and 'adding' | ||
* it to itself 'e' times. | ||
* This works given the following invariant: At each step, R0 will be r_0*base where r_0 is the prefix of e | ||
* written in binary and R1 will be (r_0+1)*base. In other words: at iteration i of the loop, r_0's binary | ||
* representation will be the first i+1 most significant bits of e. If the upcoming bit is a 0, we just have to | ||
* double R0 and add R0 to R1 to maintain the invariant. If it is a 1, we have to double R0 and add 1*base | ||
* (or add R1, which is the same as (r_0+1)*base), and double R1 to maintain the invariant. | ||
* @param base The base point used as a starting point. | ||
* @param e A secret number representing the private key. | ||
* @returns The resulting point representing the public key. | ||
*/ | ||
export function mulPointEscalar(base: Point<bigint>, e: bigint): Point<bigint> { | ||
let res: Point<bigint> = [Fr.e(BigInt(0)), Fr.e(BigInt(1))] | ||
let rem: bigint = e | ||
let exp: Point<bigint> = base | ||
|
||
while (!scalar.isZero(rem)) { | ||
if (scalar.isOdd(rem)) { | ||
res = addPoint(res, exp) | ||
if (scalar.isZero(e)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't constant time if e is zero. Is that okay? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think although the exponent is marked as secret, it may be considered public if it's set to 0. The rationale in my mind is "no private key would ever be selected as the 0 value". That said, I don't think it's a good idea to short circuit here because it breaks the comment above of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I also thought initially that we shouldn't worry too much about the zero case since it is irrelevant cryptographically. But I agree to change this to make the method more consistent. |
||
return [BigInt(0), BigInt(1)] | ||
} | ||
const eBits: Array<boolean> = [] | ||
while (!scalar.isZero(e)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the goal constant time? This loop doesn't seem constant time to me, since it'll exit early if high bits of the input are zero. As a result the length of eBits is also variable. If you want constant time, it seems like there has to be a hard-coded max bit length (254 I think, but someone should double-check me) somewhere. If you wish, I think this loop can probably be skipped entirely with some bigint bitwise operations below. Something like this: for (let bitMask = 1n, bitmask < 1n<<254n, bitmask <<= 1n) {
if (e & bitMask != 0) {
// ... I'm not sure of the relative efficiency, but I'd guess that the bitmasks are cheaper than allocating memory for an array. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes this sounds correct to me. If Hardcoding an iteration count is a good approach, but we'll still have timing vulnerabilities due to the use of the javascript It's also unclear if the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with this point. I changed the loop with a hardcoded for. |
||
if (scalar.isOdd(e)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unclear if |
||
eBits.push(true) | ||
} else { | ||
eBits.push(false) | ||
} | ||
e = scalar.shiftRight(e, BigInt(1)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A right shift on a |
||
} | ||
|
||
exp = addPoint(exp, exp) | ||
rem = scalar.shiftRight(rem, BigInt(1)) | ||
let R0: Point<bigint> = base | ||
let R1: Point<bigint> = addPoint(base, base) | ||
|
||
for (const bit of eBits.slice(0, -1).reverse()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
nvm, it returns all elements but the last one. |
||
if (bit) { | ||
R0 = addPoint(R0, R1) | ||
R1 = addPoint(R1, R1) | ||
} else { | ||
R1 = addPoint(R0, R1) | ||
R0 = addPoint(R0, R0) | ||
} | ||
} | ||
|
||
return res | ||
return R0 | ||
} | ||
|
||
/** | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest additionally referring to the named algorithm being used here (Montgomery Ladder) perhaps with a link to a description of the algorithm (Wikipedia or some other source).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure! Will do.