Skip to content

Commit eedd781

Browse files
real-or-randomreal-or-random
committed
Merge #1348: tighten group magnitude limits, save normalize_weak calls in group add methods (revival of #1032)
b7c685e Save _normalize_weak calls in group add methods (Peter Dettman) c83afa6 Tighten group magnitude limits (Peter Dettman) 173e8d0 Implement current magnitude assumptions (Peter Dettman) 49afd2f Take use of _fe_verify_magnitude in field_impl.h (Sebastian Falbesoner) 4e9661f Add _fe_verify_magnitude (no-op unless VERIFY is enabled) (Peter Dettman) 690b0fc add missing group element invariant checks (Sebastian Falbesoner) Pull request description: This PR picks up #1032 by peterdettman. It's essentially a rebase on master; the original first commit (09dbba5) which introduced group verification methods has mostly been replaced by PR #1299 (commit f202667) and what remains now is only adding a few missing checks at some places. The remaining commits are unchanged, though some (easy-to-solve) conflicts appeared through cherry-picking. The last commit which actually removes the `normalize_weak` calls is obviously the critical one and needs the most attention for review. ACKs for top commit: sipa: utACK b7c685e real-or-random: ACK b7c685e jonasnick: ACK b7c685e Tree-SHA512: f15167eff7ef6ed971c726a4d738de9a15be95b0c947d7e38329e7b16656202b7113497d36625304e784866349f2293f6f1d8cb97df35393af9ea465a4156da3
2 parents b2f6712 + b7c685e commit eedd781

File tree

5 files changed

+182
-73
lines changed

5 files changed

+182
-73
lines changed

‎src/field.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,4 +352,7 @@ static int secp256k1_fe_is_square_var(const secp256k1_fe *a);
352352
/** Check invariants on a field element (no-op unless VERIFY is enabled). */
353353
static void secp256k1_fe_verify(const secp256k1_fe *a);
354354

355+
/** Check that magnitude of a is at most m (no-op unless VERIFY is enabled). */
356+
static void secp256k1_fe_verify_magnitude(const secp256k1_fe *a, int m);
357+
355358
#endif /* SECP256K1_FIELD_H */

‎src/field_impl.h

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ SECP256K1_INLINE static int secp256k1_fe_equal(const secp256k1_fe *a, const secp
2323
#ifdef VERIFY
2424
secp256k1_fe_verify(a);
2525
secp256k1_fe_verify(b);
26-
VERIFY_CHECK(a->magnitude <= 1);
27-
VERIFY_CHECK(b->magnitude <= 31);
26+
secp256k1_fe_verify_magnitude(a, 1);
27+
secp256k1_fe_verify_magnitude(b, 31);
2828
#endif
2929
secp256k1_fe_negate(&na, a, 1);
3030
secp256k1_fe_add(&na, b);
@@ -36,8 +36,8 @@ SECP256K1_INLINE static int secp256k1_fe_equal_var(const secp256k1_fe *a, const
3636
#ifdef VERIFY
3737
secp256k1_fe_verify(a);
3838
secp256k1_fe_verify(b);
39-
VERIFY_CHECK(a->magnitude <= 1);
40-
VERIFY_CHECK(b->magnitude <= 31);
39+
secp256k1_fe_verify_magnitude(a, 1);
40+
secp256k1_fe_verify_magnitude(b, 31);
4141
#endif
4242
secp256k1_fe_negate(&na, a, 1);
4343
secp256k1_fe_add(&na, b);
@@ -60,7 +60,7 @@ static int secp256k1_fe_sqrt(secp256k1_fe * SECP256K1_RESTRICT r, const secp256k
6060
#ifdef VERIFY
6161
VERIFY_CHECK(r != a);
6262
secp256k1_fe_verify(a);
63-
VERIFY_CHECK(a->magnitude <= 8);
63+
secp256k1_fe_verify_magnitude(a, 8);
6464
#endif
6565

6666
/** The binary representation of (p + 1)/4 has 3 blocks of 1s, with lengths in
@@ -159,19 +159,26 @@ static int secp256k1_fe_sqrt(secp256k1_fe * SECP256K1_RESTRICT r, const secp256k
159159

160160
#ifndef VERIFY
161161
static void secp256k1_fe_verify(const secp256k1_fe *a) { (void)a; }
162+
static void secp256k1_fe_verify_magnitude(const secp256k1_fe *a, int m) { (void)a; (void)m; }
162163
#else
163164
static void secp256k1_fe_impl_verify(const secp256k1_fe *a);
164165
static void secp256k1_fe_verify(const secp256k1_fe *a) {
165166
/* Magnitude between 0 and 32. */
166-
VERIFY_CHECK((a->magnitude >= 0) && (a->magnitude <= 32));
167+
secp256k1_fe_verify_magnitude(a, 32);
167168
/* Normalized is 0 or 1. */
168169
VERIFY_CHECK((a->normalized == 0) || (a->normalized == 1));
169170
/* If normalized, magnitude must be 0 or 1. */
170-
if (a->normalized) VERIFY_CHECK(a->magnitude <= 1);
171+
if (a->normalized) secp256k1_fe_verify_magnitude(a, 1);
171172
/* Invoke implementation-specific checks. */
172173
secp256k1_fe_impl_verify(a);
173174
}
174175

176+
static void secp256k1_fe_verify_magnitude(const secp256k1_fe *a, int m) {
177+
VERIFY_CHECK(m >= 0);
178+
VERIFY_CHECK(m <= 32);
179+
VERIFY_CHECK(a->magnitude <= m);
180+
}
181+
175182
static void secp256k1_fe_impl_normalize(secp256k1_fe *r);
176183
SECP256K1_INLINE static void secp256k1_fe_normalize(secp256k1_fe *r) {
177184
secp256k1_fe_verify(r);
@@ -293,7 +300,7 @@ static void secp256k1_fe_impl_negate_unchecked(secp256k1_fe *r, const secp256k1_
293300
SECP256K1_INLINE static void secp256k1_fe_negate_unchecked(secp256k1_fe *r, const secp256k1_fe *a, int m) {
294301
secp256k1_fe_verify(a);
295302
VERIFY_CHECK(m >= 0 && m <= 31);
296-
VERIFY_CHECK(a->magnitude <= m);
303+
secp256k1_fe_verify_magnitude(a, m);
297304
secp256k1_fe_impl_negate_unchecked(r, a, m);
298305
r->magnitude = m + 1;
299306
r->normalized = 0;
@@ -326,8 +333,8 @@ static void secp256k1_fe_impl_mul(secp256k1_fe *r, const secp256k1_fe *a, const
326333
SECP256K1_INLINE static void secp256k1_fe_mul(secp256k1_fe *r, const secp256k1_fe *a, const secp256k1_fe * SECP256K1_RESTRICT b) {
327334
secp256k1_fe_verify(a);
328335
secp256k1_fe_verify(b);
329-
VERIFY_CHECK(a->magnitude <= 8);
330-
VERIFY_CHECK(b->magnitude <= 8);
336+
secp256k1_fe_verify_magnitude(a, 8);
337+
secp256k1_fe_verify_magnitude(b, 8);
331338
VERIFY_CHECK(r != b);
332339
VERIFY_CHECK(a != b);
333340
secp256k1_fe_impl_mul(r, a, b);
@@ -339,7 +346,7 @@ SECP256K1_INLINE static void secp256k1_fe_mul(secp256k1_fe *r, const secp256k1_f
339346
static void secp256k1_fe_impl_sqr(secp256k1_fe *r, const secp256k1_fe *a);
340347
SECP256K1_INLINE static void secp256k1_fe_sqr(secp256k1_fe *r, const secp256k1_fe *a) {
341348
secp256k1_fe_verify(a);
342-
VERIFY_CHECK(a->magnitude <= 8);
349+
secp256k1_fe_verify_magnitude(a, 8);
343350
secp256k1_fe_impl_sqr(r, a);
344351
r->magnitude = 1;
345352
r->normalized = 0;
@@ -418,7 +425,7 @@ SECP256K1_INLINE static void secp256k1_fe_get_bounds(secp256k1_fe* r, int m) {
418425
static void secp256k1_fe_impl_half(secp256k1_fe *r);
419426
SECP256K1_INLINE static void secp256k1_fe_half(secp256k1_fe *r) {
420427
secp256k1_fe_verify(r);
421-
VERIFY_CHECK(r->magnitude < 32);
428+
secp256k1_fe_verify_magnitude(r, 31);
422429
secp256k1_fe_impl_half(r);
423430
r->magnitude = (r->magnitude >> 1) + 1;
424431
r->normalized = 0;

‎src/group.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,14 @@ typedef struct {
4444

4545
#define SECP256K1_GE_STORAGE_CONST_GET(t) SECP256K1_FE_STORAGE_CONST_GET(t.x), SECP256K1_FE_STORAGE_CONST_GET(t.y)
4646

47+
/** Maximum allowed magnitudes for group element coordinates
48+
* in affine (x, y) and jacobian (x, y, z) representation. */
49+
#define SECP256K1_GE_X_MAGNITUDE_MAX 4
50+
#define SECP256K1_GE_Y_MAGNITUDE_MAX 3
51+
#define SECP256K1_GEJ_X_MAGNITUDE_MAX 4
52+
#define SECP256K1_GEJ_Y_MAGNITUDE_MAX 4
53+
#define SECP256K1_GEJ_Z_MAGNITUDE_MAX 1
54+
4755
/** Set a group element equal to the point with given X and Y coordinates */
4856
static void secp256k1_ge_set_xy(secp256k1_ge *r, const secp256k1_fe *x, const secp256k1_fe *y);
4957

0 commit comments

Comments
 (0)