From c685862918e16a9c885fc554b18c7b1ef5816bf7 Mon Sep 17 00:00:00 2001
From: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com>
Date: Thu, 18 May 2023 19:46:07 -0500
Subject: [PATCH] fix(ecdsa): allow u1*G == u2*PK case (#36)
NOTE: current ecdsa requires `r, s` to be given as proper CRT integers
TODO: newtypes to guard this assumption
---
halo2-ecc/src/ecc/ecdsa.rs | 43 +++++++++++++-------------
halo2-ecc/src/fields/fp.rs | 4 +--
halo2-ecc/src/secp256k1/tests/ecdsa.rs | 2 +-
3 files changed, 25 insertions(+), 24 deletions(-)
diff --git a/halo2-ecc/src/ecc/ecdsa.rs b/halo2-ecc/src/ecc/ecdsa.rs
index 874c185f..56587d0e 100644
--- a/halo2-ecc/src/ecc/ecdsa.rs
+++ b/halo2-ecc/src/ecc/ecdsa.rs
@@ -1,20 +1,19 @@
+use halo2_base::{gates::GateInstructions, utils::CurveAffineExt, AssignedValue, Context};
+
use crate::bigint::{big_less_than, CRTInteger};
use crate::fields::{fp::FpChip, FieldChip, PrimeField};
-use halo2_base::{
- gates::{GateInstructions, RangeInstructions},
- utils::CurveAffineExt,
- AssignedValue, Context,
-};
-use super::fixed_base;
-use super::{ec_add_unequal, scalar_multiply, EcPoint};
+use super::{fixed_base, EccChip};
+use super::{scalar_multiply, EcPoint};
// CF is the coordinate field of GA
// SF is the scalar field of GA
// p = coordinate field modulus
// n = scalar field modulus
// Only valid when p is very close to n in size (e.g. for Secp256k1)
+// Assumes `r, s` are proper CRT integers
+/// **WARNING**: Only use this function if `1 / (p - n)` is very small (e.g., < 2-100)
pub fn ecdsa_verify_no_pubkey_check(
- base_chip: &FpChip,
+ chip: &EccChip>,
ctx: &mut Context,
pubkey: &EcPoint as FieldChip>::FieldPoint>,
r: &CRTInteger,
@@ -26,6 +25,8 @@ pub fn ecdsa_verify_no_pubkey_check,
{
+ // Following https://en.wikipedia.org/wiki/Elliptic_Curve_Digital_Signature_Algorithm
+ let base_chip = chip.field_chip;
let scalar_chip =
FpChip::::new(base_chip.range, base_chip.limb_bits, base_chip.num_limbs);
let n = scalar_chip.load_constant(ctx, scalar_chip.p.to_biguint().unwrap());
@@ -38,8 +39,6 @@ where
let u1 = scalar_chip.divide(ctx, msghash, s);
let u2 = scalar_chip.divide(ctx, r, s);
- //let r_crt = scalar_chip.to_crt(ctx, r)?;
-
// compute u1 * G and u2 * pubkey
let u1_mul = fixed_base::scalar_multiply::(
base_chip,
@@ -58,21 +57,23 @@ where
var_window_bits,
);
- // check u1 * G and u2 * pubkey are not negatives and not equal
- // TODO: Technically they could be equal for a valid signature, but this happens with vanishing probability
- // for an ECDSA signature constructed in a standard way
+ // check u1 * G != -(u2 * pubkey) but allow u1 * G == u2 * pubkey
+ // check (u1 * G).x != (u2 * pubkey).x or (u1 * G).y == (u2 * pubkey).y
// coordinates of u1_mul and u2_mul are in proper bigint form, and lie in but are not constrained to [0, n)
// we therefore need hard inequality here
- let u1_u2_x_eq = base_chip.is_equal(ctx, &u1_mul.x, &u2_mul.x);
- let u1_u2_not_neg = base_chip.range.gate().not(ctx, u1_u2_x_eq);
+ let x_eq = base_chip.is_equal(ctx, &u1_mul.x, &u2_mul.x);
+ let x_neq = base_chip.gate().not(ctx, x_eq);
+ let y_eq = base_chip.is_equal(ctx, &u1_mul.y, &u2_mul.y);
+ let u1g_u2pk_not_neg = base_chip.gate().or(ctx, x_neq, y_eq);
// compute (x1, y1) = u1 * G + u2 * pubkey and check (r mod n) == x1 as integers
+ // because it is possible for u1 * G == u2 * pubkey, we must use `EccChip::sum`
+ let sum = chip.sum::(ctx, [u1_mul, u2_mul].iter());
// WARNING: For optimization reasons, does not reduce x1 mod n, which is
// invalid unless p is very close to n in size.
- base_chip.enforce_less_than_p(ctx, u1_mul.x());
- base_chip.enforce_less_than_p(ctx, u2_mul.x());
- let sum = ec_add_unequal(base_chip, ctx, &u1_mul, &u2_mul, false);
- let equal_check = base_chip.is_equal(ctx, &sum.x, r);
+ // enforce x1 < n
+ scalar_chip.enforce_less_than_p(ctx, &sum.x);
+ let equal_check = scalar_chip.is_equal_unenforced(ctx, &sum.x, r);
// TODO: maybe the big_less_than is optional?
let u1_small = big_less_than::assign::(
@@ -92,11 +93,11 @@ where
base_chip.limb_bases[1],
);
- // check (r in [1, n - 1]) and (s in [1, n - 1]) and (u1_mul != - u2_mul) and (r == x1 mod n)
+ // check (r in [1, n - 1]) and (s in [1, n - 1]) and (u1 * G != - u2 * pubkey) and (r == x1 mod n)
let res1 = base_chip.gate().and(ctx, r_valid, s_valid);
let res2 = base_chip.gate().and(ctx, res1, u1_small);
let res3 = base_chip.gate().and(ctx, res2, u2_small);
- let res4 = base_chip.gate().and(ctx, res3, u1_u2_not_neg);
+ let res4 = base_chip.gate().and(ctx, res3, u1g_u2pk_not_neg);
let res5 = base_chip.gate().and(ctx, res4, equal_check);
res5
}
diff --git a/halo2-ecc/src/fields/fp.rs b/halo2-ecc/src/fields/fp.rs
index 6099a147..1ae6e002 100644
--- a/halo2-ecc/src/fields/fp.rs
+++ b/halo2-ecc/src/fields/fp.rs
@@ -338,9 +338,9 @@ impl<'range, F: PrimeField, Fp: PrimeField> FieldChip for FpChip<'range, F, F
let (_, underflow) =
sub::crt::(self.range(), ctx, a, &p, self.limb_bits, self.limb_bases[1]);
let is_underflow_zero = self.gate().is_zero(ctx, underflow);
- let range_check = self.gate().not(ctx, is_underflow_zero);
+ let no_underflow = self.gate().not(ctx, is_underflow_zero);
- self.gate().and(ctx, is_nonzero, range_check)
+ self.gate().and(ctx, is_nonzero, no_underflow)
}
// assuming `a` has been range checked to be a proper BigInt
diff --git a/halo2-ecc/src/secp256k1/tests/ecdsa.rs b/halo2-ecc/src/secp256k1/tests/ecdsa.rs
index 739bffc7..eb5ab113 100644
--- a/halo2-ecc/src/secp256k1/tests/ecdsa.rs
+++ b/halo2-ecc/src/secp256k1/tests/ecdsa.rs
@@ -69,7 +69,7 @@ fn ecdsa_test(
let pk = ecc_chip.load_private(ctx, (pk.x, pk.y));
// test ECDSA
let res = ecdsa_verify_no_pubkey_check::(
- &fp_chip, ctx, &pk, &r, &s, &m, 4, 4,
+ &ecc_chip, ctx, &pk, &r, &s, &m, 4, 4,
);
assert_eq!(res.value(), &F::one());
}