diff --git a/src/bench_internal.c b/src/bench_internal.c index e1ef14fdb0..362737f706 100644 --- a/src/bench_internal.c +++ b/src/bench_internal.c @@ -65,10 +65,10 @@ static void bench_setup(void* arg) { secp256k1_scalar_set_b32(&data->scalar[0], init[0], NULL); secp256k1_scalar_set_b32(&data->scalar[1], init[1], NULL); - secp256k1_fe_set_b32(&data->fe[0], init[0]); - secp256k1_fe_set_b32(&data->fe[1], init[1]); - secp256k1_fe_set_b32(&data->fe[2], init[2]); - secp256k1_fe_set_b32(&data->fe[3], init[3]); + secp256k1_fe_set_b32_limit(&data->fe[0], init[0]); + secp256k1_fe_set_b32_limit(&data->fe[1], init[1]); + secp256k1_fe_set_b32_limit(&data->fe[2], init[2]); + secp256k1_fe_set_b32_limit(&data->fe[3], init[3]); CHECK(secp256k1_ge_set_xo_var(&data->ge[0], &data->fe[0], 0)); CHECK(secp256k1_ge_set_xo_var(&data->ge[1], &data->fe[1], 1)); secp256k1_gej_set_ge(&data->gej[0], &data->ge[0]); diff --git a/src/ecdsa_impl.h b/src/ecdsa_impl.h index 90b4b22b77..48e30851b5 100644 --- a/src/ecdsa_impl.h +++ b/src/ecdsa_impl.h @@ -239,7 +239,8 @@ static int secp256k1_ecdsa_sig_verify(const secp256k1_scalar *sigr, const secp25 } #else secp256k1_scalar_get_b32(c, sigr); - secp256k1_fe_set_b32(&xr, c); + /* we can ignore the fe_set_b32_limit return value, because we know the input is in range */ + (void)secp256k1_fe_set_b32_limit(&xr, c); /** We now have the recomputed R point in pr, and its claimed x coordinate (modulo n) * in xr. Naively, we would extract the x coordinate from pr (requiring a inversion modulo p), diff --git a/src/eckey_impl.h b/src/eckey_impl.h index e0506d3e2b..b2fe36fe93 100644 --- a/src/eckey_impl.h +++ b/src/eckey_impl.h @@ -17,10 +17,10 @@ static int secp256k1_eckey_pubkey_parse(secp256k1_ge *elem, const unsigned char *pub, size_t size) { if (size == 33 && (pub[0] == SECP256K1_TAG_PUBKEY_EVEN || pub[0] == SECP256K1_TAG_PUBKEY_ODD)) { secp256k1_fe x; - return secp256k1_fe_set_b32(&x, pub+1) && secp256k1_ge_set_xo_var(elem, &x, pub[0] == SECP256K1_TAG_PUBKEY_ODD); + return secp256k1_fe_set_b32_limit(&x, pub+1) && secp256k1_ge_set_xo_var(elem, &x, pub[0] == SECP256K1_TAG_PUBKEY_ODD); } else if (size == 65 && (pub[0] == SECP256K1_TAG_PUBKEY_UNCOMPRESSED || pub[0] == SECP256K1_TAG_PUBKEY_HYBRID_EVEN || pub[0] == SECP256K1_TAG_PUBKEY_HYBRID_ODD)) { secp256k1_fe x, y; - if (!secp256k1_fe_set_b32(&x, pub+1) || !secp256k1_fe_set_b32(&y, pub+33)) { + if (!secp256k1_fe_set_b32_limit(&x, pub+1) || !secp256k1_fe_set_b32_limit(&y, pub+33)) { return 0; } secp256k1_ge_set_xy(elem, &x, &y); diff --git a/src/ecmult_gen_compute_table_impl.h b/src/ecmult_gen_compute_table_impl.h index ff6a2992dc..7d672b9950 100644 --- a/src/ecmult_gen_compute_table_impl.h +++ b/src/ecmult_gen_compute_table_impl.h @@ -31,7 +31,7 @@ static void secp256k1_ecmult_gen_compute_table(secp256k1_ge_storage* table, cons secp256k1_fe nums_x; secp256k1_ge nums_ge; int r; - r = secp256k1_fe_set_b32(&nums_x, nums_b32); + r = secp256k1_fe_set_b32_limit(&nums_x, nums_b32); (void)r; VERIFY_CHECK(r); r = secp256k1_ge_set_xo_var(&nums_ge, &nums_x, 0); diff --git a/src/ecmult_gen_impl.h b/src/ecmult_gen_impl.h index 4f5ea9f3c0..deb0323b7a 100644 --- a/src/ecmult_gen_impl.h +++ b/src/ecmult_gen_impl.h @@ -108,7 +108,7 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const memset(keydata, 0, sizeof(keydata)); /* Accept unobservably small non-uniformity. */ secp256k1_rfc6979_hmac_sha256_generate(&rng, nonce32, 32); - overflow = !secp256k1_fe_set_b32(&s, nonce32); + overflow = !secp256k1_fe_set_b32_limit(&s, nonce32); overflow |= secp256k1_fe_is_zero(&s); secp256k1_fe_cmov(&s, &secp256k1_fe_one, overflow); /* Randomize the projection to defend against multiplier sidechannels. diff --git a/src/field.h b/src/field.h index d02a4d9bed..9707c5a421 100644 --- a/src/field.h +++ b/src/field.h @@ -75,10 +75,14 @@ static int secp256k1_fe_equal_var(const secp256k1_fe *a, const secp256k1_fe *b); /** Compare two field elements. Requires both inputs to be normalized */ static int secp256k1_fe_cmp_var(const secp256k1_fe *a, const secp256k1_fe *b); -/** Set a field element equal to 32-byte big endian value. - * Returns 1 if no overflow occurred, and then the output is normalized. - * Returns 0 if overflow occurred, and then the output is only weakly normalized. */ -static int secp256k1_fe_set_b32(secp256k1_fe *r, const unsigned char *a); +/** Set a field element equal to 32-byte big endian value, reducing modulo the curve + * order if need be. The output is weakly normalized. */ +static void secp256k1_fe_set_b32_mod(secp256k1_fe *r, const unsigned char *a); + +/** Set a field element equal to 32-byte big endian value. If the input is >= the + * curve order, r is set to an indeterminate value and 0 is returned. Otherwise, + * 1 is returned and the output is normalized. */ +static int secp256k1_fe_set_b32_limit(secp256k1_fe *r, const unsigned char *a); /** Convert a field element to a 32-byte big endian value. Requires the input to be normalized */ static void secp256k1_fe_get_b32(unsigned char *r, const secp256k1_fe *a); diff --git a/src/field_10x26_impl.h b/src/field_10x26_impl.h index 9fd8de808d..00631643b0 100644 --- a/src/field_10x26_impl.h +++ b/src/field_10x26_impl.h @@ -351,8 +351,7 @@ static int secp256k1_fe_cmp_var(const secp256k1_fe *a, const secp256k1_fe *b) { return 0; } -static int secp256k1_fe_set_b32(secp256k1_fe *r, const unsigned char *a) { - int ret; +static void secp256k1_fe_set_b32_mod(secp256k1_fe *r, const unsigned char *a) { r->n[0] = (uint32_t)a[31] | ((uint32_t)a[30] << 8) | ((uint32_t)a[29] << 16) | ((uint32_t)(a[28] & 0x3) << 24); r->n[1] = (uint32_t)((a[28] >> 2) & 0x3f) | ((uint32_t)a[27] << 6) | ((uint32_t)a[26] << 14) | ((uint32_t)(a[25] & 0xf) << 22); r->n[2] = (uint32_t)((a[25] >> 4) & 0xf) | ((uint32_t)a[24] << 4) | ((uint32_t)a[23] << 12) | ((uint32_t)(a[22] & 0x3f) << 20); @@ -364,12 +363,32 @@ static int secp256k1_fe_set_b32(secp256k1_fe *r, const unsigned char *a) { r->n[8] = (uint32_t)a[5] | ((uint32_t)a[4] << 8) | ((uint32_t)a[3] << 16) | ((uint32_t)(a[2] & 0x3) << 24); r->n[9] = (uint32_t)((a[2] >> 2) & 0x3f) | ((uint32_t)a[1] << 6) | ((uint32_t)a[0] << 14); - ret = !((r->n[9] == 0x3FFFFFUL) & ((r->n[8] & r->n[7] & r->n[6] & r->n[5] & r->n[4] & r->n[3] & r->n[2]) == 0x3FFFFFFUL) & ((r->n[1] + 0x40UL + ((r->n[0] + 0x3D1UL) >> 26)) > 0x3FFFFFFUL)); #ifdef VERIFY r->magnitude = 1; - r->normalized = ret; + r->normalized = 0; secp256k1_fe_verify(r); #endif +} + +static int secp256k1_fe_set_b32_limit(secp256k1_fe *r, const unsigned char *a) { + int ret; + + secp256k1_fe_set_b32_mod(r, a); + ret = !((r->n[9] == 0x3FFFFFUL) & ((r->n[8] & r->n[7] & r->n[6] & r->n[5] & r->n[4] & r->n[3] & r->n[2]) == 0x3FFFFFFUL) & ((r->n[1] + 0x40UL + ((r->n[0] + 0x3D1UL) >> 26)) > 0x3FFFFFFUL)); + +#ifdef VERIFY + if (ret) { + r->magnitude = 1; + r->normalized = 1; + secp256k1_fe_verify(r); + } else { + /* The input is out of range, so the output will have an indeterminate value. + * In VERIFY mode, explicitly mark its magnitude as something invalid so using it + * anywhere will trigger an error. */ + r->magnitude = -1; + } +#endif + return ret; } diff --git a/src/field_5x52_impl.h b/src/field_5x52_impl.h index 4262542fc1..9ad0c213cc 100644 --- a/src/field_5x52_impl.h +++ b/src/field_5x52_impl.h @@ -303,8 +303,7 @@ static int secp256k1_fe_cmp_var(const secp256k1_fe *a, const secp256k1_fe *b) { return 0; } -static int secp256k1_fe_set_b32(secp256k1_fe *r, const unsigned char *a) { - int ret; +static void secp256k1_fe_set_b32_mod(secp256k1_fe *r, const unsigned char *a) { r->n[0] = (uint64_t)a[31] | ((uint64_t)a[30] << 8) | ((uint64_t)a[29] << 16) @@ -339,12 +338,33 @@ static int secp256k1_fe_set_b32(secp256k1_fe *r, const unsigned char *a) { | ((uint64_t)a[2] << 24) | ((uint64_t)a[1] << 32) | ((uint64_t)a[0] << 40); - ret = !((r->n[4] == 0x0FFFFFFFFFFFFULL) & ((r->n[3] & r->n[2] & r->n[1]) == 0xFFFFFFFFFFFFFULL) & (r->n[0] >= 0xFFFFEFFFFFC2FULL)); + #ifdef VERIFY r->magnitude = 1; - r->normalized = ret; + r->normalized = 0; secp256k1_fe_verify(r); #endif +} + +static int secp256k1_fe_set_b32_limit(secp256k1_fe *r, const unsigned char *a) { + int ret; + + secp256k1_fe_set_b32_mod(r, a); + ret = !((r->n[4] == 0x0FFFFFFFFFFFFULL) & ((r->n[3] & r->n[2] & r->n[1]) == 0xFFFFFFFFFFFFFULL) & (r->n[0] >= 0xFFFFEFFFFFC2FULL)); + +#ifdef VERIFY + if (ret) { + r->magnitude = 1; + r->normalized = 1; + secp256k1_fe_verify(r); + } else { + /* The input is out of range, so the output will have an indeterminate value. + * In VERIFY mode, explicitly mark its magnitude as something invalid so using it + * anywhere will trigger an error. */ + r->magnitude = -1; + } +#endif + return ret; } diff --git a/src/modules/extrakeys/main_impl.h b/src/modules/extrakeys/main_impl.h index e1003052f4..cdd65c744c 100644 --- a/src/modules/extrakeys/main_impl.h +++ b/src/modules/extrakeys/main_impl.h @@ -27,7 +27,7 @@ int secp256k1_xonly_pubkey_parse(const secp256k1_context* ctx, secp256k1_xonly_p memset(pubkey, 0, sizeof(*pubkey)); ARG_CHECK(input32 != NULL); - if (!secp256k1_fe_set_b32(&x, input32)) { + if (!secp256k1_fe_set_b32_limit(&x, input32)) { return 0; } if (!secp256k1_ge_set_xo_var(&pk, &x, 0)) { diff --git a/src/modules/extrakeys/tests_exhaustive_impl.h b/src/modules/extrakeys/tests_exhaustive_impl.h index 5ecc90d50f..d3d817a131 100644 --- a/src/modules/extrakeys/tests_exhaustive_impl.h +++ b/src/modules/extrakeys/tests_exhaustive_impl.h @@ -47,7 +47,7 @@ static void test_exhaustive_extrakeys(const secp256k1_context *ctx, const secp25 CHECK(secp256k1_memcmp_var(xonly_pubkey_bytes[i - 1], buf, 32) == 0); /* Compare the xonly_pubkey bytes against the precomputed group. */ - secp256k1_fe_set_b32(&fe, xonly_pubkey_bytes[i - 1]); + secp256k1_fe_set_b32_mod(&fe, xonly_pubkey_bytes[i - 1]); CHECK(secp256k1_fe_equal_var(&fe, &group[i].x)); /* Check the parity against the precomputed group. */ diff --git a/src/modules/recovery/main_impl.h b/src/modules/recovery/main_impl.h index e7906eb62e..76a005e017 100644 --- a/src/modules/recovery/main_impl.h +++ b/src/modules/recovery/main_impl.h @@ -98,7 +98,7 @@ static int secp256k1_ecdsa_sig_recover(const secp256k1_scalar *sigr, const secp2 } secp256k1_scalar_get_b32(brx, sigr); - r = secp256k1_fe_set_b32(&fx, brx); + r = secp256k1_fe_set_b32_limit(&fx, brx); (void)r; VERIFY_CHECK(r); /* brx comes from a scalar, so is less than the order; certainly less than p */ if (recid & 2) { diff --git a/src/modules/schnorrsig/main_impl.h b/src/modules/schnorrsig/main_impl.h index cd651591c4..4e7b45a045 100644 --- a/src/modules/schnorrsig/main_impl.h +++ b/src/modules/schnorrsig/main_impl.h @@ -232,7 +232,7 @@ int secp256k1_schnorrsig_verify(const secp256k1_context* ctx, const unsigned cha ARG_CHECK(msg != NULL || msglen == 0); ARG_CHECK(pubkey != NULL); - if (!secp256k1_fe_set_b32(&rx, &sig64[0])) { + if (!secp256k1_fe_set_b32_limit(&rx, &sig64[0])) { return 0; } diff --git a/src/secp256k1.c b/src/secp256k1.c index 7af333ca90..bdbd97cc40 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -247,8 +247,8 @@ static int secp256k1_pubkey_load(const secp256k1_context* ctx, secp256k1_ge* ge, } else { /* Otherwise, fall back to 32-byte big endian for X and Y. */ secp256k1_fe x, y; - secp256k1_fe_set_b32(&x, pubkey->data); - secp256k1_fe_set_b32(&y, pubkey->data + 32); + secp256k1_fe_set_b32_mod(&x, pubkey->data); + secp256k1_fe_set_b32_mod(&y, pubkey->data + 32); secp256k1_ge_set_xy(ge, &x, &y); } ARG_CHECK(!secp256k1_fe_is_zero(&ge->x)); diff --git a/src/tests.c b/src/tests.c index 5440562ad6..ca56ee7795 100644 --- a/src/tests.c +++ b/src/tests.c @@ -100,7 +100,7 @@ static void random_field_element_test(secp256k1_fe *fe) { do { unsigned char b32[32]; secp256k1_testrand256_test(b32); - if (secp256k1_fe_set_b32(fe, b32)) { + if (secp256k1_fe_set_b32_limit(fe, b32)) { break; } } while(1); @@ -2901,7 +2901,7 @@ static void random_fe(secp256k1_fe *x) { unsigned char bin[32]; do { secp256k1_testrand256(bin); - if (secp256k1_fe_set_b32(x, bin)) { + if (secp256k1_fe_set_b32_limit(x, bin)) { return; } } while(1); @@ -2911,7 +2911,7 @@ static void random_fe_test(secp256k1_fe *x) { unsigned char bin[32]; do { secp256k1_testrand256_test(bin); - if (secp256k1_fe_set_b32(x, bin)) { + if (secp256k1_fe_set_b32_limit(x, bin)) { return; } } while(1); @@ -2965,7 +2965,7 @@ static void run_field_convert(void) { unsigned char b322[32]; secp256k1_fe_storage fes2; /* Check conversions to fe. */ - CHECK(secp256k1_fe_set_b32(&fe2, b32)); + CHECK(secp256k1_fe_set_b32_limit(&fe2, b32)); CHECK(secp256k1_fe_equal_var(&fe, &fe2)); secp256k1_fe_from_storage(&fe2, &fes); CHECK(secp256k1_fe_equal_var(&fe, &fe2)); @@ -2992,7 +2992,8 @@ static void run_field_be32_overflow(void) { }; unsigned char out[32]; secp256k1_fe fe; - CHECK(secp256k1_fe_set_b32(&fe, zero_overflow) == 0); + CHECK(secp256k1_fe_set_b32_limit(&fe, zero_overflow) == 0); + secp256k1_fe_set_b32_mod(&fe, zero_overflow); CHECK(secp256k1_fe_normalizes_to_zero(&fe) == 1); secp256k1_fe_normalize(&fe); CHECK(secp256k1_fe_is_zero(&fe) == 1); @@ -3014,7 +3015,8 @@ static void run_field_be32_overflow(void) { }; unsigned char out[32]; secp256k1_fe fe; - CHECK(secp256k1_fe_set_b32(&fe, one_overflow) == 0); + CHECK(secp256k1_fe_set_b32_limit(&fe, one_overflow) == 0); + secp256k1_fe_set_b32_mod(&fe, one_overflow); secp256k1_fe_normalize(&fe); CHECK(secp256k1_fe_cmp_var(&fe, &secp256k1_fe_one) == 0); secp256k1_fe_get_b32(out, &fe); @@ -3036,7 +3038,8 @@ static void run_field_be32_overflow(void) { unsigned char out[32]; secp256k1_fe fe; const secp256k1_fe fe_ff = SECP256K1_FE_CONST(0, 0, 0, 0, 0, 0, 0x01, 0x000003d0); - CHECK(secp256k1_fe_set_b32(&fe, ff_overflow) == 0); + CHECK(secp256k1_fe_set_b32_limit(&fe, ff_overflow) == 0); + secp256k1_fe_set_b32_mod(&fe, ff_overflow); secp256k1_fe_normalize(&fe); CHECK(secp256k1_fe_cmp_var(&fe, &fe_ff) == 0); secp256k1_fe_get_b32(out, &fe); @@ -3611,7 +3614,7 @@ static void run_inverse_tests(void) b32[31] = i & 0xff; b32[30] = (i >> 8) & 0xff; secp256k1_scalar_set_b32(&x_scalar, b32, NULL); - secp256k1_fe_set_b32(&x_fe, b32); + secp256k1_fe_set_b32_mod(&x_fe, b32); for (var = 0; var <= 1; ++var) { test_inverse_scalar(NULL, &x_scalar, var); test_inverse_field(NULL, &x_fe, var); @@ -3628,7 +3631,7 @@ static void run_inverse_tests(void) for (i = 0; i < 64 * COUNT; ++i) { (testrand ? secp256k1_testrand256_test : secp256k1_testrand256)(b32); secp256k1_scalar_set_b32(&x_scalar, b32, NULL); - secp256k1_fe_set_b32(&x_fe, b32); + secp256k1_fe_set_b32_mod(&x_fe, b32); for (var = 0; var <= 1; ++var) { test_inverse_scalar(NULL, &x_scalar, var); test_inverse_field(NULL, &x_fe, var); diff --git a/src/tests_exhaustive.c b/src/tests_exhaustive.c index 86b9334cae..1b9e4463db 100644 --- a/src/tests_exhaustive.c +++ b/src/tests_exhaustive.c @@ -54,7 +54,7 @@ static void random_fe(secp256k1_fe *x) { unsigned char bin[32]; do { secp256k1_testrand256(bin); - if (secp256k1_fe_set_b32(x, bin)) { + if (secp256k1_fe_set_b32_limit(x, bin)) { return; } } while(1);