Skip to content

Commit 5b32602

Browse files
sipasipa
committed
Split fe_set_b32 into reducing and normalizing variants
1 parent 006ddc1 commit 5b32602

16 files changed

+68
-39
lines changed

‎src/bench_internal.c

+4-4
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,10 @@ static void bench_setup(void* arg) {
6565

6666
secp256k1_scalar_set_b32(&data->scalar[0], init[0], NULL);
6767
secp256k1_scalar_set_b32(&data->scalar[1], init[1], NULL);
68-
secp256k1_fe_set_b32(&data->fe[0], init[0]);
69-
secp256k1_fe_set_b32(&data->fe[1], init[1]);
70-
secp256k1_fe_set_b32(&data->fe[2], init[2]);
71-
secp256k1_fe_set_b32(&data->fe[3], init[3]);
68+
secp256k1_fe_set_b32_limit(&data->fe[0], init[0]);
69+
secp256k1_fe_set_b32_limit(&data->fe[1], init[1]);
70+
secp256k1_fe_set_b32_limit(&data->fe[2], init[2]);
71+
secp256k1_fe_set_b32_limit(&data->fe[3], init[3]);
7272
CHECK(secp256k1_ge_set_xo_var(&data->ge[0], &data->fe[0], 0));
7373
CHECK(secp256k1_ge_set_xo_var(&data->ge[1], &data->fe[1], 1));
7474
secp256k1_gej_set_ge(&data->gej[0], &data->ge[0]);

‎src/ecdsa_impl.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,8 @@ static int secp256k1_ecdsa_sig_verify(const secp256k1_scalar *sigr, const secp25
239239
}
240240
#else
241241
secp256k1_scalar_get_b32(c, sigr);
242-
secp256k1_fe_set_b32(&xr, c);
242+
/* we can ignore the fe_set_b32_limit return value, because we know the input is in range */
243+
(void)secp256k1_fe_set_b32_limit(&xr, c);
243244

244245
/** We now have the recomputed R point in pr, and its claimed x coordinate (modulo n)
245246
* in xr. Naively, we would extract the x coordinate from pr (requiring a inversion modulo p),

‎src/eckey_impl.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@
1717
static int secp256k1_eckey_pubkey_parse(secp256k1_ge *elem, const unsigned char *pub, size_t size) {
1818
if (size == 33 && (pub[0] == SECP256K1_TAG_PUBKEY_EVEN || pub[0] == SECP256K1_TAG_PUBKEY_ODD)) {
1919
secp256k1_fe x;
20-
return secp256k1_fe_set_b32(&x, pub+1) && secp256k1_ge_set_xo_var(elem, &x, pub[0] == SECP256K1_TAG_PUBKEY_ODD);
20+
return secp256k1_fe_set_b32_limit(&x, pub+1) && secp256k1_ge_set_xo_var(elem, &x, pub[0] == SECP256K1_TAG_PUBKEY_ODD);
2121
} else if (size == 65 && (pub[0] == SECP256K1_TAG_PUBKEY_UNCOMPRESSED || pub[0] == SECP256K1_TAG_PUBKEY_HYBRID_EVEN || pub[0] == SECP256K1_TAG_PUBKEY_HYBRID_ODD)) {
2222
secp256k1_fe x, y;
23-
if (!secp256k1_fe_set_b32(&x, pub+1) || !secp256k1_fe_set_b32(&y, pub+33)) {
23+
if (!secp256k1_fe_set_b32_limit(&x, pub+1) || !secp256k1_fe_set_b32_limit(&y, pub+33)) {
2424
return 0;
2525
}
2626
secp256k1_ge_set_xy(elem, &x, &y);

‎src/ecmult_gen_compute_table_impl.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ static void secp256k1_ecmult_gen_compute_table(secp256k1_ge_storage* table, cons
3131
secp256k1_fe nums_x;
3232
secp256k1_ge nums_ge;
3333
int r;
34-
r = secp256k1_fe_set_b32(&nums_x, nums_b32);
34+
r = secp256k1_fe_set_b32_limit(&nums_x, nums_b32);
3535
(void)r;
3636
VERIFY_CHECK(r);
3737
r = secp256k1_ge_set_xo_var(&nums_ge, &nums_x, 0);

‎src/ecmult_gen_impl.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const
108108
memset(keydata, 0, sizeof(keydata));
109109
/* Accept unobservably small non-uniformity. */
110110
secp256k1_rfc6979_hmac_sha256_generate(&rng, nonce32, 32);
111-
overflow = !secp256k1_fe_set_b32(&s, nonce32);
111+
overflow = !secp256k1_fe_set_b32_limit(&s, nonce32);
112112
overflow |= secp256k1_fe_is_zero(&s);
113113
secp256k1_fe_cmov(&s, &secp256k1_fe_one, overflow);
114114
/* Randomize the projection to defend against multiplier sidechannels.

‎src/field.h

+12-7
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,8 @@ static const secp256k1_fe secp256k1_const_beta = SECP256K1_FE_CONST(
8585
# define secp256k1_fe_is_zero secp256k1_fe_impl_is_zero
8686
# define secp256k1_fe_is_odd secp256k1_fe_impl_is_odd
8787
# define secp256k1_fe_cmp_var secp256k1_fe_impl_cmp_var
88-
# define secp256k1_fe_set_b32 secp256k1_fe_impl_set_b32
88+
# define secp256k1_fe_set_b32_mod secp256k1_fe_impl_set_b32_mod
89+
# define secp256k1_fe_set_b32_limit secp256k1_fe_impl_set_b32_limit
8990
# define secp256k1_fe_get_b32 secp256k1_fe_impl_get_b32
9091
# define secp256k1_fe_negate secp256k1_fe_impl_negate
9192
# define secp256k1_fe_mul_int secp256k1_fe_impl_mul_int
@@ -189,16 +190,20 @@ static int secp256k1_fe_equal_var(const secp256k1_fe *a, const secp256k1_fe *b);
189190
*/
190191
static int secp256k1_fe_cmp_var(const secp256k1_fe *a, const secp256k1_fe *b);
191192

192-
/** Set a field element equal to a provided 32-byte big endian value.
193+
/** Set a field element equal to a provided 32-byte big endian value, reducing it.
193194
*
194195
* On input, r does not need to be initalized. a must be a pointer to an initialized 32-byte array.
195-
* On output, r = a (mod p). It will have magnitude 1, and if (a < p), it will be normalized.
196-
* If not, it will only be weakly normalized. Returns whether (a < p).
196+
* On output, r = a (mod p). It will have magnitude 1, and not be normalized.
197+
*/
198+
static void secp256k1_fe_set_b32_mod(secp256k1_fe *r, const unsigned char *a);
199+
200+
/** Set a field element equal to a provided 32-byte big endian value, checking for overflow.
197201
*
198-
* Note that this function is unusual in that the normalization of the output depends on the
199-
* run-time value of a.
202+
* On input, r does not need to be initalized. a must be a pointer to an initialized 32-byte array.
203+
* On output, r = a if (a < p), it will be normalized with magnitude 1, and 1 is returned.
204+
* If a >= p, 0 is returned, and r will be made invalid (and must not be used without overwriting).
200205
*/
201-
static int secp256k1_fe_set_b32(secp256k1_fe *r, const unsigned char *a);
206+
static int secp256k1_fe_set_b32_limit(secp256k1_fe *r, const unsigned char *a);
202207

203208
/** Convert a field element to 32-byte big endian byte array.
204209
* On input, a must be a valid normalized field element, and r a pointer to a 32-byte array.

‎src/field_10x26_impl.h

+4-1
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ static int secp256k1_fe_impl_cmp_var(const secp256k1_fe *a, const secp256k1_fe *
290290
return 0;
291291
}
292292

293-
static int secp256k1_fe_impl_set_b32(secp256k1_fe *r, const unsigned char *a) {
293+
static void secp256k1_fe_impl_set_b32_mod(secp256k1_fe *r, const unsigned char *a) {
294294
r->n[0] = (uint32_t)a[31] | ((uint32_t)a[30] << 8) | ((uint32_t)a[29] << 16) | ((uint32_t)(a[28] & 0x3) << 24);
295295
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);
296296
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);
@@ -301,7 +301,10 @@ static int secp256k1_fe_impl_set_b32(secp256k1_fe *r, const unsigned char *a) {
301301
r->n[7] = (uint32_t)((a[9] >> 6) & 0x3) | ((uint32_t)a[8] << 2) | ((uint32_t)a[7] << 10) | ((uint32_t)a[6] << 18);
302302
r->n[8] = (uint32_t)a[5] | ((uint32_t)a[4] << 8) | ((uint32_t)a[3] << 16) | ((uint32_t)(a[2] & 0x3) << 24);
303303
r->n[9] = (uint32_t)((a[2] >> 2) & 0x3f) | ((uint32_t)a[1] << 6) | ((uint32_t)a[0] << 14);
304+
}
304305

306+
static int secp256k1_fe_impl_set_b32_limit(secp256k1_fe *r, const unsigned char *a) {
307+
secp256k1_fe_impl_set_b32_mod(r, a);
305308
return !((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));
306309
}
307310

‎src/field_5x52_impl.h

+5-1
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ static int secp256k1_fe_impl_cmp_var(const secp256k1_fe *a, const secp256k1_fe *
236236
return 0;
237237
}
238238

239-
static int secp256k1_fe_impl_set_b32(secp256k1_fe *r, const unsigned char *a) {
239+
static void secp256k1_fe_impl_set_b32_mod(secp256k1_fe *r, const unsigned char *a) {
240240
r->n[0] = (uint64_t)a[31]
241241
| ((uint64_t)a[30] << 8)
242242
| ((uint64_t)a[29] << 16)
@@ -271,6 +271,10 @@ static int secp256k1_fe_impl_set_b32(secp256k1_fe *r, const unsigned char *a) {
271271
| ((uint64_t)a[2] << 24)
272272
| ((uint64_t)a[1] << 32)
273273
| ((uint64_t)a[0] << 40);
274+
}
275+
276+
static int secp256k1_fe_impl_set_b32_limit(secp256k1_fe *r, const unsigned char *a) {
277+
secp256k1_fe_impl_set_b32_mod(r, a);
274278
return !((r->n[4] == 0x0FFFFFFFFFFFFULL) & ((r->n[3] & r->n[2] & r->n[1]) == 0xFFFFFFFFFFFFFULL) & (r->n[0] >= 0xFFFFEFFFFFC2FULL));
275279
}
276280

‎src/field_impl.h

+18-5
Original file line numberDiff line numberDiff line change
@@ -260,13 +260,26 @@ SECP256K1_INLINE static int secp256k1_fe_cmp_var(const secp256k1_fe *a, const se
260260
return secp256k1_fe_impl_cmp_var(a, b);
261261
}
262262

263-
static int secp256k1_fe_impl_set_b32(secp256k1_fe *r, const unsigned char *a);
264-
SECP256K1_INLINE static int secp256k1_fe_set_b32(secp256k1_fe *r, const unsigned char *a) {
265-
int ret = secp256k1_fe_impl_set_b32(r, a);
263+
static void secp256k1_fe_impl_set_b32_mod(secp256k1_fe *r, const unsigned char *a);
264+
SECP256K1_INLINE static void secp256k1_fe_set_b32_mod(secp256k1_fe *r, const unsigned char *a) {
265+
secp256k1_fe_impl_set_b32_mod(r, a);
266266
r->magnitude = 1;
267-
r->normalized = ret;
267+
r->normalized = 0;
268268
secp256k1_fe_verify(r);
269-
return ret;
269+
}
270+
271+
static int secp256k1_fe_impl_set_b32_limit(secp256k1_fe *r, const unsigned char *a);
272+
SECP256K1_INLINE static int secp256k1_fe_set_b32_limit(secp256k1_fe *r, const unsigned char *a) {
273+
if (secp256k1_fe_impl_set_b32_limit(r, a)) {
274+
r->magnitude = 1;
275+
r->normalized = 1;
276+
secp256k1_fe_verify(r);
277+
return 1;
278+
} else {
279+
/* Mark the output field element as invalid. */
280+
r->magnitude = -1;
281+
return 0;
282+
}
270283
}
271284

272285
static void secp256k1_fe_impl_get_b32(unsigned char *r, const secp256k1_fe *a);

‎src/modules/extrakeys/main_impl.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ int secp256k1_xonly_pubkey_parse(const secp256k1_context* ctx, secp256k1_xonly_p
2828
memset(pubkey, 0, sizeof(*pubkey));
2929
ARG_CHECK(input32 != NULL);
3030

31-
if (!secp256k1_fe_set_b32(&x, input32)) {
31+
if (!secp256k1_fe_set_b32_limit(&x, input32)) {
3232
return 0;
3333
}
3434
if (!secp256k1_ge_set_xo_var(&pk, &x, 0)) {

‎src/modules/extrakeys/tests_exhaustive_impl.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ static void test_exhaustive_extrakeys(const secp256k1_context *ctx, const secp25
4747
CHECK(secp256k1_memcmp_var(xonly_pubkey_bytes[i - 1], buf, 32) == 0);
4848

4949
/* Compare the xonly_pubkey bytes against the precomputed group. */
50-
secp256k1_fe_set_b32(&fe, xonly_pubkey_bytes[i - 1]);
50+
secp256k1_fe_set_b32_mod(&fe, xonly_pubkey_bytes[i - 1]);
5151
CHECK(secp256k1_fe_equal_var(&fe, &group[i].x));
5252

5353
/* Check the parity against the precomputed group. */

‎src/modules/recovery/main_impl.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ static int secp256k1_ecdsa_sig_recover(const secp256k1_scalar *sigr, const secp2
9898
}
9999

100100
secp256k1_scalar_get_b32(brx, sigr);
101-
r = secp256k1_fe_set_b32(&fx, brx);
101+
r = secp256k1_fe_set_b32_limit(&fx, brx);
102102
(void)r;
103103
VERIFY_CHECK(r); /* brx comes from a scalar, so is less than the order; certainly less than p */
104104
if (recid & 2) {

‎src/modules/schnorrsig/main_impl.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ int secp256k1_schnorrsig_verify(const secp256k1_context* ctx, const unsigned cha
232232
ARG_CHECK(msg != NULL || msglen == 0);
233233
ARG_CHECK(pubkey != NULL);
234234

235-
if (!secp256k1_fe_set_b32(&rx, &sig64[0])) {
235+
if (!secp256k1_fe_set_b32_limit(&rx, &sig64[0])) {
236236
return 0;
237237
}
238238

‎src/secp256k1.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -247,8 +247,8 @@ static int secp256k1_pubkey_load(const secp256k1_context* ctx, secp256k1_ge* ge,
247247
} else {
248248
/* Otherwise, fall back to 32-byte big endian for X and Y. */
249249
secp256k1_fe x, y;
250-
secp256k1_fe_set_b32(&x, pubkey->data);
251-
secp256k1_fe_set_b32(&y, pubkey->data + 32);
250+
secp256k1_fe_set_b32_mod(&x, pubkey->data);
251+
secp256k1_fe_set_b32_mod(&y, pubkey->data + 32);
252252
secp256k1_ge_set_xy(ge, &x, &y);
253253
}
254254
ARG_CHECK(!secp256k1_fe_is_zero(&ge->x));

‎src/tests.c

+12-9
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ static void random_field_element_test(secp256k1_fe *fe) {
9090
do {
9191
unsigned char b32[32];
9292
secp256k1_testrand256_test(b32);
93-
if (secp256k1_fe_set_b32(fe, b32)) {
93+
if (secp256k1_fe_set_b32_limit(fe, b32)) {
9494
break;
9595
}
9696
} while(1);
@@ -2957,7 +2957,7 @@ static void random_fe(secp256k1_fe *x) {
29572957
unsigned char bin[32];
29582958
do {
29592959
secp256k1_testrand256(bin);
2960-
if (secp256k1_fe_set_b32(x, bin)) {
2960+
if (secp256k1_fe_set_b32_limit(x, bin)) {
29612961
return;
29622962
}
29632963
} while(1);
@@ -2967,7 +2967,7 @@ static void random_fe_test(secp256k1_fe *x) {
29672967
unsigned char bin[32];
29682968
do {
29692969
secp256k1_testrand256_test(bin);
2970-
if (secp256k1_fe_set_b32(x, bin)) {
2970+
if (secp256k1_fe_set_b32_limit(x, bin)) {
29712971
return;
29722972
}
29732973
} while(1);
@@ -3021,7 +3021,7 @@ static void run_field_convert(void) {
30213021
unsigned char b322[32];
30223022
secp256k1_fe_storage fes2;
30233023
/* Check conversions to fe. */
3024-
CHECK(secp256k1_fe_set_b32(&fe2, b32));
3024+
CHECK(secp256k1_fe_set_b32_limit(&fe2, b32));
30253025
CHECK(secp256k1_fe_equal_var(&fe, &fe2));
30263026
secp256k1_fe_from_storage(&fe2, &fes);
30273027
CHECK(secp256k1_fe_equal_var(&fe, &fe2));
@@ -3043,7 +3043,8 @@ static void run_field_be32_overflow(void) {
30433043
static const unsigned char zero[32] = { 0x00 };
30443044
unsigned char out[32];
30453045
secp256k1_fe fe;
3046-
CHECK(secp256k1_fe_set_b32(&fe, zero_overflow) == 0);
3046+
CHECK(secp256k1_fe_set_b32_limit(&fe, zero_overflow) == 0);
3047+
secp256k1_fe_set_b32_mod(&fe, zero_overflow);
30473048
CHECK(secp256k1_fe_normalizes_to_zero(&fe) == 1);
30483049
secp256k1_fe_normalize(&fe);
30493050
CHECK(secp256k1_fe_is_zero(&fe) == 1);
@@ -3065,7 +3066,8 @@ static void run_field_be32_overflow(void) {
30653066
};
30663067
unsigned char out[32];
30673068
secp256k1_fe fe;
3068-
CHECK(secp256k1_fe_set_b32(&fe, one_overflow) == 0);
3069+
CHECK(secp256k1_fe_set_b32_limit(&fe, one_overflow) == 0);
3070+
secp256k1_fe_set_b32_mod(&fe, one_overflow);
30693071
secp256k1_fe_normalize(&fe);
30703072
CHECK(secp256k1_fe_cmp_var(&fe, &secp256k1_fe_one) == 0);
30713073
secp256k1_fe_get_b32(out, &fe);
@@ -3087,7 +3089,8 @@ static void run_field_be32_overflow(void) {
30873089
unsigned char out[32];
30883090
secp256k1_fe fe;
30893091
const secp256k1_fe fe_ff = SECP256K1_FE_CONST(0, 0, 0, 0, 0, 0, 0x01, 0x000003d0);
3090-
CHECK(secp256k1_fe_set_b32(&fe, ff_overflow) == 0);
3092+
CHECK(secp256k1_fe_set_b32_limit(&fe, ff_overflow) == 0);
3093+
secp256k1_fe_set_b32_mod(&fe, ff_overflow);
30913094
secp256k1_fe_normalize(&fe);
30923095
CHECK(secp256k1_fe_cmp_var(&fe, &fe_ff) == 0);
30933096
secp256k1_fe_get_b32(out, &fe);
@@ -3673,7 +3676,7 @@ static void run_inverse_tests(void)
36733676
b32[31] = i & 0xff;
36743677
b32[30] = (i >> 8) & 0xff;
36753678
secp256k1_scalar_set_b32(&x_scalar, b32, NULL);
3676-
secp256k1_fe_set_b32(&x_fe, b32);
3679+
secp256k1_fe_set_b32_mod(&x_fe, b32);
36773680
for (var = 0; var <= 1; ++var) {
36783681
test_inverse_scalar(NULL, &x_scalar, var);
36793682
test_inverse_field(NULL, &x_fe, var);
@@ -3690,7 +3693,7 @@ static void run_inverse_tests(void)
36903693
for (i = 0; i < 64 * COUNT; ++i) {
36913694
(testrand ? secp256k1_testrand256_test : secp256k1_testrand256)(b32);
36923695
secp256k1_scalar_set_b32(&x_scalar, b32, NULL);
3693-
secp256k1_fe_set_b32(&x_fe, b32);
3696+
secp256k1_fe_set_b32_mod(&x_fe, b32);
36943697
for (var = 0; var <= 1; ++var) {
36953698
test_inverse_scalar(NULL, &x_scalar, var);
36963699
test_inverse_field(NULL, &x_fe, var);

‎src/tests_exhaustive.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ static void random_fe(secp256k1_fe *x) {
6060
unsigned char bin[32];
6161
do {
6262
secp256k1_testrand256(bin);
63-
if (secp256k1_fe_set_b32(x, bin)) {
63+
if (secp256k1_fe_set_b32_limit(x, bin)) {
6464
return;
6565
}
6666
} while(1);

0 commit comments

Comments
 (0)