From d40ea5f78e77457eb3b915d884bdea24168ee6cf Mon Sep 17 00:00:00 2001 From: siv2r Date: Fri, 14 Jan 2022 09:16:17 +0530 Subject: [PATCH] field: add magnitude check and _fe_verify check for internal field APIs 1. _fe_verify on inputs of the functions: 1. _fe_normalizes_to_zero, _fe_normalizes_to_zero_var 2. _fe_to_storage 3. _fe_inv,_fe_inv_var 4. _fe_equal, _fe_equal_var 5. _fe_sqrt 2. _fe_verify on the result calculated in the functions: 1. _fe_inv,_fe_inv_var 2. _fe_sqrt 3. magnitude checks for inputs of the functions: 1. _fe_inv,_fe_inv_var 2. _fe_equal, _fe_equal_var 3. _fe_sqrt 4. move a VERIFY_CHECK call (in _fe_inv, _fe_inv_var) out of #ifdef VERIFY in `field_5x52_impl.h` 5. move _fe_verify call after else block in _fe_setb32 --- src/field_10x26_impl.h | 29 ++++++++++++++++++++++------- src/field_5x52_impl.h | 29 ++++++++++++++++++++--------- src/field_impl.h | 20 +++++++++++++++++--- 3 files changed, 59 insertions(+), 19 deletions(-) diff --git a/src/field_10x26_impl.h b/src/field_10x26_impl.h index aecb6117e7..b15c283c89 100644 --- a/src/field_10x26_impl.h +++ b/src/field_10x26_impl.h @@ -200,7 +200,9 @@ static int secp256k1_fe_normalizes_to_zero(const secp256k1_fe *r) { /* Reduce t9 at the start so there will be at most a single carry from the first pass */ uint32_t x = t9 >> 22; t9 &= 0x03FFFFFUL; - +#ifdef VERIFY + secp256k1_fe_verify(r); +#endif /* The first pass ensures the magnitude is 1, ... */ t0 += x * 0x3D1UL; t1 += (x << 6); t1 += (t0 >> 26); t0 &= 0x3FFFFFFUL; z0 = t0; z1 = t0 ^ 0x3D0UL; @@ -224,7 +226,9 @@ static int secp256k1_fe_normalizes_to_zero_var(const secp256k1_fe *r) { uint32_t t0, t1, t2, t3, t4, t5, t6, t7, t8, t9; uint32_t z0, z1; uint32_t x; - +#ifdef VERIFY + secp256k1_fe_verify(r); +#endif t0 = r->n[0]; t9 = r->n[9]; @@ -348,10 +352,10 @@ static int secp256k1_fe_set_b32(secp256k1_fe *r, const unsigned char *a) { r->magnitude = 1; if (ret) { r->normalized = 1; - secp256k1_fe_verify(r); } else { r->normalized = 0; } + secp256k1_fe_verify(r); #endif return ret; } @@ -1151,6 +1155,7 @@ static SECP256K1_INLINE void secp256k1_fe_storage_cmov(secp256k1_fe_storage *r, static void secp256k1_fe_to_storage(secp256k1_fe_storage *r, const secp256k1_fe *a) { #ifdef VERIFY VERIFY_CHECK(a->normalized); + secp256k1_fe_verify(a); #endif r->n[0] = a->n[0] | a->n[1] << 26; r->n[1] = a->n[1] >> 6 | a->n[2] << 20; @@ -1245,26 +1250,36 @@ static const secp256k1_modinv32_modinfo secp256k1_const_modinfo_fe = { static void secp256k1_fe_inv(secp256k1_fe *r, const secp256k1_fe *x) { secp256k1_fe tmp; secp256k1_modinv32_signed30 s; - +#ifdef VERIFY + VERIFY_CHECK(x->magnitude <= 8); + secp256k1_fe_verify(x); +#endif tmp = *x; secp256k1_fe_normalize(&tmp); secp256k1_fe_to_signed30(&s, &tmp); secp256k1_modinv32(&s, &secp256k1_const_modinfo_fe); secp256k1_fe_from_signed30(r, &s); - +#ifdef VERIFY + secp256k1_fe_verify(r); +#endif VERIFY_CHECK(secp256k1_fe_normalizes_to_zero(r) == secp256k1_fe_normalizes_to_zero(&tmp)); } static void secp256k1_fe_inv_var(secp256k1_fe *r, const secp256k1_fe *x) { secp256k1_fe tmp; secp256k1_modinv32_signed30 s; - +#ifdef VERIFY + VERIFY_CHECK(x->magnitude <= 8); + secp256k1_fe_verify(x); +#endif tmp = *x; secp256k1_fe_normalize_var(&tmp); secp256k1_fe_to_signed30(&s, &tmp); secp256k1_modinv32_var(&s, &secp256k1_const_modinfo_fe); secp256k1_fe_from_signed30(r, &s); - +#ifdef VERIFY + secp256k1_fe_verify(r); +#endif VERIFY_CHECK(secp256k1_fe_normalizes_to_zero(r) == secp256k1_fe_normalizes_to_zero(&tmp)); } diff --git a/src/field_5x52_impl.h b/src/field_5x52_impl.h index 9b824b9204..e34003fee2 100644 --- a/src/field_5x52_impl.h +++ b/src/field_5x52_impl.h @@ -177,7 +177,9 @@ static int secp256k1_fe_normalizes_to_zero(const secp256k1_fe *r) { /* Reduce t4 at the start so there will be at most a single carry from the first pass */ uint64_t x = t4 >> 48; t4 &= 0x0FFFFFFFFFFFFULL; - +#ifdef VERIFY + secp256k1_fe_verify(r); +#endif /* The first pass ensures the magnitude is 1, ... */ t0 += x * 0x1000003D1ULL; t1 += (t0 >> 52); t0 &= 0xFFFFFFFFFFFFFULL; z0 = t0; z1 = t0 ^ 0x1000003D0ULL; @@ -196,7 +198,9 @@ static int secp256k1_fe_normalizes_to_zero_var(const secp256k1_fe *r) { uint64_t t0, t1, t2, t3, t4; uint64_t z0, z1; uint64_t x; - +#ifdef VERIFY + secp256k1_fe_verify(r); +#endif t0 = r->n[0]; t4 = r->n[4]; @@ -332,10 +336,10 @@ static int secp256k1_fe_set_b32(secp256k1_fe *r, const unsigned char *a) { r->magnitude = 1; if (ret) { r->normalized = 1; - secp256k1_fe_verify(r); } else { r->normalized = 0; } + secp256k1_fe_verify(r); #endif return ret; } @@ -491,6 +495,7 @@ static SECP256K1_INLINE void secp256k1_fe_storage_cmov(secp256k1_fe_storage *r, static void secp256k1_fe_to_storage(secp256k1_fe_storage *r, const secp256k1_fe *a) { #ifdef VERIFY VERIFY_CHECK(a->normalized); + secp256k1_fe_verify(a); #endif r->n[0] = a->n[0] | a->n[1] << 52; r->n[1] = a->n[1] >> 12 | a->n[2] << 40; @@ -560,31 +565,37 @@ static const secp256k1_modinv64_modinfo secp256k1_const_modinfo_fe = { static void secp256k1_fe_inv(secp256k1_fe *r, const secp256k1_fe *x) { secp256k1_fe tmp; secp256k1_modinv64_signed62 s; - +#ifdef VERIFY + VERIFY_CHECK(x->magnitude <= 8); + secp256k1_fe_verify(x); +#endif tmp = *x; secp256k1_fe_normalize(&tmp); secp256k1_fe_to_signed62(&s, &tmp); secp256k1_modinv64(&s, &secp256k1_const_modinfo_fe); secp256k1_fe_from_signed62(r, &s); - #ifdef VERIFY - VERIFY_CHECK(secp256k1_fe_normalizes_to_zero(r) == secp256k1_fe_normalizes_to_zero(&tmp)); + secp256k1_fe_verify(r); #endif + VERIFY_CHECK(secp256k1_fe_normalizes_to_zero(r) == secp256k1_fe_normalizes_to_zero(&tmp)); } static void secp256k1_fe_inv_var(secp256k1_fe *r, const secp256k1_fe *x) { secp256k1_fe tmp; secp256k1_modinv64_signed62 s; - +#ifdef VERIFY + VERIFY_CHECK(x->magnitude <= 8); + secp256k1_fe_verify(x); +#endif tmp = *x; secp256k1_fe_normalize_var(&tmp); secp256k1_fe_to_signed62(&s, &tmp); secp256k1_modinv64_var(&s, &secp256k1_const_modinfo_fe); secp256k1_fe_from_signed62(r, &s); - #ifdef VERIFY - VERIFY_CHECK(secp256k1_fe_normalizes_to_zero(r) == secp256k1_fe_normalizes_to_zero(&tmp)); + secp256k1_fe_verify(r); #endif + VERIFY_CHECK(secp256k1_fe_normalizes_to_zero(r) == secp256k1_fe_normalizes_to_zero(&tmp)); } #endif /* SECP256K1_FIELD_REPR_IMPL_H */ diff --git a/src/field_impl.h b/src/field_impl.h index ca04c2d04b..537d1c4340 100644 --- a/src/field_impl.h +++ b/src/field_impl.h @@ -23,6 +23,11 @@ SECP256K1_INLINE static int secp256k1_fe_equal(const secp256k1_fe *a, const secp256k1_fe *b) { secp256k1_fe na; +#ifdef VERIFY + VERIFY_CHECK(a->magnitude == 1); + secp256k1_fe_verify(a); + secp256k1_fe_verify(b); +#endif secp256k1_fe_negate(&na, a, 1); secp256k1_fe_add(&na, b); return secp256k1_fe_normalizes_to_zero(&na); @@ -30,6 +35,11 @@ SECP256K1_INLINE static int secp256k1_fe_equal(const secp256k1_fe *a, const secp SECP256K1_INLINE static int secp256k1_fe_equal_var(const secp256k1_fe *a, const secp256k1_fe *b) { secp256k1_fe na; +#ifdef VERIFY + VERIFY_CHECK(a->magnitude == 1); + secp256k1_fe_verify(a); + secp256k1_fe_verify(b); +#endif secp256k1_fe_negate(&na, a, 1); secp256k1_fe_add(&na, b); return secp256k1_fe_normalizes_to_zero_var(&na); @@ -47,7 +57,10 @@ static int secp256k1_fe_sqrt(secp256k1_fe *r, const secp256k1_fe * SECP256K1_RES */ secp256k1_fe x2, x3, x6, x9, x11, x22, x44, x88, x176, x220, x223, t1; int j; - +#ifdef VERIFY + VERIFY_CHECK(a->magnitude <= 8); + secp256k1_fe_verify(a); +#endif VERIFY_CHECK(r != a); /** The binary representation of (p + 1)/4 has 3 blocks of 1s, with lengths in @@ -128,9 +141,10 @@ static int secp256k1_fe_sqrt(secp256k1_fe *r, const secp256k1_fe * SECP256K1_RES secp256k1_fe_mul(&t1, &t1, &x2); secp256k1_fe_sqr(&t1, &t1); secp256k1_fe_sqr(r, &t1); - +#ifdef VERIFY + secp256k1_fe_verify(r); +#endif /* Check that a square root was actually calculated */ - secp256k1_fe_sqr(&t1, r); return secp256k1_fe_equal(&t1, a); }