Skip to content

Commit

Permalink
DANGER: refactor range checks incl. private key validity
Browse files Browse the repository at this point in the history
  • Loading branch information
@paulmillr
paulmillr committed Aug 4, 2024
1 parent eaf64e3 commit 3ed792f
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 72 deletions.
30 changes: 12 additions & 18 deletions src/abstract/edwards.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,6 @@ function validateOpts(curve: CurveType) {
return Object.freeze({ ...opts } as const);
}

const isBig = (n: bigint) => typeof n === 'bigint' && _0n <= n; // n in [1..]

/**
* Asserts min <= n < max
*/
function assertInRange(title: string, n: bigint, min: bigint, max: bigint) {
if (isBig(n) && isBig(min) && isBig(max) && min <= n && n < max) return;
throw new Error(`expected valid ${title}: ${min} <= coord < ${max}, got ${typeof n} ${n}`);
}

// Instance of Extended Point with coordinates in X, Y, Z, T
export interface ExtPointType extends Group<ExtPointType> {
readonly ex: bigint;
Expand All @@ -82,6 +72,10 @@ export interface ExtPointConstructor extends GroupConstructor<ExtPointType> {
fromPrivateKey(privateKey: Hex): ExtPointType;
}

/**
* Edwards Curve interface.
* Main methods: `getPublicKey(priv)`, `sign(msg, priv)`, `verify(sig, msg, pub)`.
*/
export type CurveFn = {
CURVE: ReturnType<typeof validateOpts>;
getPublicKey: (privateKey: Hex) => Uint8Array;
Expand Down Expand Up @@ -332,8 +326,8 @@ export function twistedEdwards(curveDef: CurveType): CurveFn {

// Constant-time multiplication.
multiply(scalar: bigint): Point {
let n = scalar;
assertInRange('scalar', n, _1n, CURVE_ORDER);
const n = scalar;
ut.aInRange('scalar', n, _1n, CURVE_ORDER); // 1 <= scalar < L
const { p, f } = this.wNAF(n);
return Point.normalizeZ([p, f])[0];
}
Expand All @@ -343,8 +337,8 @@ export function twistedEdwards(curveDef: CurveType): CurveFn {
// an exposed private key e.g. sig verification.
// Does NOT allow scalars higher than CURVE.n.
multiplyUnsafe(scalar: bigint): Point {
let n = scalar;
assertInRange('scalar', n, _0n, CURVE_ORDER); // 0 <= scalar < l
const n = scalar;
ut.aInRange('scalar', n, _0n, CURVE_ORDER); // 0 <= scalar < L
if (n === _0n) return I;
if (this.equals(I) || n === _1n) return this;
if (this.equals(G)) return this.wNAF(n).p;
Expand Down Expand Up @@ -390,10 +384,10 @@ export function twistedEdwards(curveDef: CurveType): CurveFn {
const y = ut.bytesToNumberLE(normed);

// RFC8032 prohibits >= p, but ZIP215 doesn't
// zip215=true: 1 <= y < MASK (2^256 for ed25519)
// zip215=false: 1 <= y < P (2^255-19 for ed25519)
// zip215=true: 0 <= y < MASK (2^256 for ed25519)
// zip215=false: 0 <= y < P (2^255-19 for ed25519)
const max = zip215 ? MASK : Fp.ORDER;
assertInRange('pointHex.y', y, _0n, max);
ut.aInRange('pointHex.y', y, _0n, max);

// Ed25519: x² = (y²-1)/(dy²+1) mod p. Ed448: x² = (y²-1)/(dy²-1) mod p. Generic case:
// ax²+y²=1+dx²y² => y²-1=dx²y²-ax² => y²-1=x²(dy²-a) => x²=(y²-1)/(dy²-a)
Expand Down Expand Up @@ -469,7 +463,7 @@ export function twistedEdwards(curveDef: CurveType): CurveFn {
const R = G.multiply(r).toRawBytes(); // R = rG
const k = hashDomainToScalar(options.context, R, pointBytes, msg); // R || A || PH(M)
const s = modN(r + k * scalar); // S = (r + k * s) mod L
assertInRange('signature.s', s, _0n, CURVE_ORDER); // 0 <= s < l
ut.aInRange('signature.s', s, _0n, CURVE_ORDER); // 0 <= s < l
const res = ut.concatBytes(R, ut.numberToBytesLE(s, Fp.BYTES));
return ensureBytes('result', res, nByteLength * 2); // 64-byte signature
}
Expand Down
21 changes: 11 additions & 10 deletions src/abstract/montgomery.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
/*! noble-curves - MIT License (c) 2022 Paul Miller (paulmillr.com) */
import { mod, pow } from './modular.js';
import { bytesToNumberLE, ensureBytes, numberToBytesLE, validateObject } from './utils.js';
import {
aInRange,
bytesToNumberLE,
ensureBytes,
numberToBytesLE,
validateObject,
} from './utils.js';

const _0n = BigInt(0);
const _1n = BigInt(1);
Expand Down Expand Up @@ -75,12 +81,6 @@ export function montgomery(curveDef: CurveType): CurveFn {
return [x_2, x_3];
}

// Accepts 0 as well
function assertFieldElement(n: bigint): bigint {
if (typeof n === 'bigint' && _0n <= n && n < P) return n;
throw new Error('Expected valid scalar 0 < scalar < CURVE.P');
}

// x25519 from 4
// The constant a24 is (486662 - 2) / 4 = 121665 for curve25519/X25519
const a24 = (CURVE.a - BigInt(2)) / BigInt(4);
Expand All @@ -90,11 +90,12 @@ export function montgomery(curveDef: CurveType): CurveFn {
* @param scalar by which the point would be multiplied
* @returns new Point on Montgomery curve
*/
function montgomeryLadder(pointU: bigint, scalar: bigint): bigint {
const u = assertFieldElement(pointU);
function montgomeryLadder(u: bigint, scalar: bigint): bigint {
aInRange('u', u, _0n, P);
aInRange('scalar', scalar, _0n, P);
// Section 5: Implementations MUST accept non-canonical values and process them as
// if they had been reduced modulo the field prime.
const k = assertFieldElement(scalar);
const k = scalar;
const x_1 = u;
let x_2 = _1n;
let z_2 = _0n;
Expand Down
27 changes: 27 additions & 0 deletions src/abstract/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ export function abytes(item: unknown): void {
if (!isBytes(item)) throw new Error('Uint8Array expected');
}

export function abool(title: string, value: boolean): void {
if (typeof value !== 'boolean')
throw new Error(`${title} must be valid boolean, got "${value}".`);
}

// Array where index 0xf0 (240) is mapped to string 'f0'
const hexes = /* @__PURE__ */ Array.from({ length: 256 }, (_, i) =>
i.toString(16).padStart(2, '0')
Expand Down Expand Up @@ -174,6 +179,28 @@ export function utf8ToBytes(str: string): Uint8Array {
return new Uint8Array(new TextEncoder().encode(str)); // https://bugzil.la/1681809
}

// Is positive bigint
const isPosBig = (n: bigint) => typeof n === 'bigint' && _0n <= n;

export function inRange(n: bigint, min: bigint, max: bigint) {
return isPosBig(n) && isPosBig(min) && isPosBig(max) && min <= n && n < max;
}

/**
* Asserts min <= n < max. NOTE: It's < max and not <= max.
* @example
* aInRange('x', x, 1n, 256n); // would assume x is in (1n..255n)
*/
export function aInRange(title: string, n: bigint, min: bigint, max: bigint) {
// Why min <= n < max and not a (min < n < max) OR b (min <= n <= max)?
// consider P=256n, min=0n, max=P
// - a for min=0 would require -1: `inRange('x', x, -1n, P)`
// - b would commonly require subtraction: `inRange('x', x, 0n, P - 1n)`
// - our way is the cleanest: `inRange('x', x, 0n, P)
if (!inRange(n, min, max))
throw new Error(`expected valid ${title}: ${min} <= n < ${max}, got ${typeof n} ${n}`);
}

// Bit operations

/**
Expand Down
50 changes: 20 additions & 30 deletions src/abstract/weierstrass.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,15 +233,12 @@ export function weierstrassPoints<T>(opts: CurvePointsType<T>): CurvePointsRes<T

// Valid group elements reside in range 1..n-1
function isWithinCurveOrder(num: bigint): boolean {
return typeof num === 'bigint' && _0n < num && num < CURVE.n;
}
function assertGE(num: bigint) {
if (!isWithinCurveOrder(num)) throw new Error('Expected valid bigint: 0 < bigint < curve.n');
return ut.inRange(num, _1n, CURVE.n);
}
// Validates if priv key is valid and converts it to bigint.
// Supports options allowedPrivateKeyLengths and wrapPrivateKey.
function normPrivateKeyToScalar(key: PrivKey): bigint {
const { allowedPrivateKeyLengths: lengths, nByteLength, wrapPrivateKey, n } = CURVE;
const { allowedPrivateKeyLengths: lengths, nByteLength, wrapPrivateKey, n: N } = CURVE;
if (lengths && typeof key !== 'bigint') {
if (ut.isBytes(key)) key = ut.bytesToHex(key);
// Normalize to hex string, pad. E.g. P521 would norm 130-132 char hex to 132-char bytes
Expand All @@ -257,16 +254,16 @@ export function weierstrassPoints<T>(opts: CurvePointsType<T>): CurvePointsRes<T
} catch (error) {
throw new Error(`private key must be ${nByteLength} bytes, hex or bigint, not ${typeof key}`);
}
if (wrapPrivateKey) num = mod.mod(num, n); // disabled by default, enabled for BLS
assertGE(num); // num in range [1..N-1]
if (wrapPrivateKey) num = mod.mod(num, N); // disabled by default, enabled for BLS
ut.aInRange('private key', num, _1n, N); // num in range [1..N-1]
return num;
}

function assertPrjPoint(other: unknown) {
if (!(other instanceof Point)) throw new Error('ProjectivePoint expected');
}
// NOTE: these are pretty heavy function which used a lot of times, but since we have completely immutable points,
// we can cache them.

// Memoized toAffine / validity check. They are heavy. Points are immutable.

// Converts Projective point to affine (x, y) coordinates.
// Can accept precomputed Z^-1 - for example, from invertBatch.
Expand Down Expand Up @@ -520,16 +517,16 @@ export function weierstrassPoints<T>(opts: CurvePointsType<T>): CurvePointsRes<T
* It's faster, but should only be used when you don't care about
* an exposed private key e.g. sig verification, which works over *public* keys.
*/
multiplyUnsafe(n: bigint): Point {
multiplyUnsafe(sc: bigint): Point {
ut.aInRange('scalar', sc, _0n, CURVE.n);
const I = Point.ZERO;
if (n === _0n) return I;
assertGE(n); // Will throw on 0
if (n === _1n) return this;
if (sc === _0n) return I;
if (sc === _1n) return this;
const { endo } = CURVE;
if (!endo) return wnaf.unsafeLadder(this, n);
if (!endo) return wnaf.unsafeLadder(this, sc);

// Apply endomorphism
let { k1neg, k1, k2neg, k2 } = endo.splitScalar(n);
let { k1neg, k1, k2neg, k2 } = endo.splitScalar(sc);
let k1p = I;
let k2p = I;
let d: Point = this;
Expand All @@ -556,12 +553,11 @@ export function weierstrassPoints<T>(opts: CurvePointsType<T>): CurvePointsRes<T
* @returns New point
*/
multiply(scalar: bigint): Point {
assertGE(scalar);
let n = scalar;
const { endo, n: N } = CURVE;
ut.aInRange('scalar', scalar, _1n, N);
let point: Point, fake: Point; // Fake point is used to const-time mult
const { endo } = CURVE;
if (endo) {
const { k1neg, k1, k2neg, k2 } = endo.splitScalar(n);
const { k1neg, k1, k2neg, k2 } = endo.splitScalar(scalar);
let { p: k1p, f: f1p } = this.wNAF(k1);
let { p: k2p, f: f2p } = this.wNAF(k2);
k1p = wnaf.constTimeNegate(k1neg, k1p);
Expand All @@ -570,7 +566,7 @@ export function weierstrassPoints<T>(opts: CurvePointsType<T>): CurvePointsRes<T
point = k1p.add(k2p);
fake = f1p.add(f2p);
} else {
const { p, f } = this.wNAF(n);
const { p, f } = this.wNAF(scalar);
point = p;
fake = f;
}
Expand Down Expand Up @@ -721,9 +717,6 @@ export function weierstrass(curveDef: CurveType): CurveFn {
const compressedLen = Fp.BYTES + 1; // e.g. 33 for 32
const uncompressedLen = 2 * Fp.BYTES + 1; // e.g. 65 for 32

function isValidFieldElement(num: bigint): boolean {
return _0n < num && num < Fp.ORDER; // 0 is banned since it's not invertible FE
}
function modN(a: bigint) {
return mod.mod(a, CURVE_ORDER);
}
Expand Down Expand Up @@ -756,7 +749,7 @@ export function weierstrass(curveDef: CurveType): CurveFn {
// this.assertValidity() is done inside of fromHex
if (len === compressedLen && (head === 0x02 || head === 0x03)) {
const x = ut.bytesToNumberBE(tail);
if (!isValidFieldElement(x)) throw new Error('Point is not on curve');
if (!ut.inRange(x, _1n, Fp.ORDER)) throw new Error('Point is not on curve');
const y2 = weierstrassEquation(x); // y² = x³ + ax + b
let y: bigint;
try {
Expand Down Expand Up @@ -822,9 +815,8 @@ export function weierstrass(curveDef: CurveType): CurveFn {
}

assertValidity(): void {
// can use assertGE here
if (!isWithinCurveOrder(this.r)) throw new Error('r must be 0 < r < CURVE.n');
if (!isWithinCurveOrder(this.s)) throw new Error('s must be 0 < s < CURVE.n');
ut.aInRange('r', this.r, _1n, CURVE_ORDER); // r in [1..N]
ut.aInRange('s', this.s, _1n, CURVE_ORDER); // s in [1..N]
}

addRecoveryBit(recovery: number): RecoveredSignature {
Expand Down Expand Up @@ -974,9 +966,7 @@ export function weierstrass(curveDef: CurveType): CurveFn {
* Converts to bytes. Checks if num in `[0..ORDER_MASK-1]` e.g.: `[0..2^256-1]`.
*/
function int2octets(num: bigint): Uint8Array {
if (typeof num !== 'bigint') throw new Error('bigint expected');
if (!(_0n <= num && num < ORDER_MASK))
throw new Error(`bigint expected < 2^${CURVE.nBitLength}`);
ut.aInRange(`num < 2^${CURVE.nBitLength}`, num, _0n, ORDER_MASK);
// works with order, can have different size than numToField!
return ut.numberToBytesBE(num, CURVE.nByteLength);
}
Expand Down
7 changes: 5 additions & 2 deletions src/ed25519.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import { sha512 } from '@noble/hashes/sha512';
import { concatBytes, randomBytes, utf8ToBytes } from '@noble/hashes/utils';
import { AffinePoint, Group } from './abstract/curve.js';
import { ExtPointType, twistedEdwards } from './abstract/edwards.js';
import { CurveFn, ExtPointType, twistedEdwards } from './abstract/edwards.js';
import { createHasher, expand_message_xmd, htfBasicOpts } from './abstract/hash-to-curve.js';
import { Field, FpSqrtEven, isNegativeLE, mod, pow2 } from './abstract/modular.js';
import { montgomery } from './abstract/montgomery.js';
Expand Down Expand Up @@ -126,7 +126,10 @@ const ed25519Defaults = /* @__PURE__ */ (() =>
uvRatio,
}) as const)();

export const ed25519 = /* @__PURE__ */ (() => twistedEdwards(ed25519Defaults))();
/**
* ed25519 curve with EdDSA signatures.
*/
export const ed25519: CurveFn = /* @__PURE__ */ (() => twistedEdwards(ed25519Defaults))();

function ed25519_domain(data: Uint8Array, ctx: Uint8Array, phflag: boolean) {
if (ctx.length > 255) throw new Error('Context is too big');
Expand Down
Loading

0 comments on commit 3ed792f

Please sign in to comment.