Skip to content

Commit 0ce80ef

Browse files
committed
Merge pull request #94
da55986 Label variable-time functions correctly and don't use those in sign (Pieter Wuille)
2 parents 784e62f + da55986 commit 0ce80ef

File tree

5 files changed

+56
-41
lines changed

5 files changed

+56
-41
lines changed

src/ecdsa_impl.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ int static secp256k1_ecdsa_sig_recompute(secp256k1_num_t *r2, const secp256k1_ec
7070
secp256k1_gej_t pubkeyj; secp256k1_gej_set_ge(&pubkeyj, pubkey);
7171
secp256k1_gej_t pr; secp256k1_ecmult(&pr, &pubkeyj, &u2, &u1);
7272
if (!secp256k1_gej_is_infinity(&pr)) {
73-
secp256k1_fe_t xr; secp256k1_gej_get_x(&xr, &pr);
73+
secp256k1_fe_t xr; secp256k1_gej_get_x_var(&xr, &pr);
7474
secp256k1_fe_normalize(&xr);
7575
unsigned char xrb[32]; secp256k1_fe_get_b32(xrb, &xr);
7676
secp256k1_num_set_bin(r2, xrb, 32);
@@ -121,7 +121,7 @@ int static secp256k1_ecdsa_sig_recover(const secp256k1_ecdsa_sig_t *sig, secp256
121121
secp256k1_num_mod_mul(&u2, &rn, &sig->s, &c->order);
122122
secp256k1_gej_t qj;
123123
secp256k1_ecmult(&qj, &xj, &u2, &u1);
124-
secp256k1_ge_set_gej(pubkey, &qj);
124+
secp256k1_ge_set_gej_var(pubkey, &qj);
125125
secp256k1_num_free(&rn);
126126
secp256k1_num_free(&u1);
127127
secp256k1_num_free(&u2);

src/ecmult_gen_impl.h

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ static void secp256k1_ecmult_gen_start(void) {
4949
VERIFY_CHECK(secp256k1_ge_set_xo(&nums_ge, &nums_x, 0));
5050
secp256k1_gej_set_ge(&nums_gej, &nums_ge);
5151
// Add G to make the bits in x uniformly distributed.
52-
secp256k1_gej_add_ge(&nums_gej, &nums_gej, g);
52+
secp256k1_gej_add_ge_var(&nums_gej, &nums_gej, g);
5353
}
5454

5555
// compute prec.
@@ -63,21 +63,21 @@ static void secp256k1_ecmult_gen_start(void) {
6363
// Set precj[j*16 .. j*16+15] to (numsbase, numsbase + gbase, ..., numsbase + 15*gbase).
6464
precj[j*16] = numsbase;
6565
for (int i=1; i<16; i++) {
66-
secp256k1_gej_add(&precj[j*16 + i], &precj[j*16 + i - 1], &gbase);
66+
secp256k1_gej_add_var(&precj[j*16 + i], &precj[j*16 + i - 1], &gbase);
6767
}
6868
// Multiply gbase by 16.
6969
for (int i=0; i<4; i++) {
70-
secp256k1_gej_double(&gbase, &gbase);
70+
secp256k1_gej_double_var(&gbase, &gbase);
7171
}
7272
// Multiply numbase by 2.
73-
secp256k1_gej_double(&numsbase, &numsbase);
73+
secp256k1_gej_double_var(&numsbase, &numsbase);
7474
if (j == 62) {
7575
// In the last iteration, numsbase is (1 - 2^j) * nums instead.
7676
secp256k1_gej_neg(&numsbase, &numsbase);
77-
secp256k1_gej_add(&numsbase, &numsbase, &nums_gej);
77+
secp256k1_gej_add_var(&numsbase, &numsbase, &nums_gej);
7878
}
7979
}
80-
secp256k1_ge_set_all_gej(1024, prec, precj);
80+
secp256k1_ge_set_all_gej_var(1024, prec, precj);
8181
}
8282
for (int j=0; j<64; j++) {
8383
for (int i=0; i<16; i++) {
@@ -109,7 +109,10 @@ void static secp256k1_ecmult_gen(secp256k1_gej_t *r, const secp256k1_scalar_t *g
109109
bits = secp256k1_scalar_get_bits(gn, j * 4, 4);
110110
for (int k=0; k<sizeof(secp256k1_ge_t); k++)
111111
((unsigned char*)(&add))[k] = c->prec[j][k][bits];
112-
secp256k1_gej_add_ge(r, r, &add);
112+
// Note that the next line uses a variable-time addition function, which
113+
// is fine, as the inputs are blinded (they have no known corresponding
114+
// private key).
115+
secp256k1_gej_add_ge_var(r, r, &add);
113116
}
114117
bits = 0;
115118
secp256k1_ge_clear(&add);

src/ecmult_impl.h

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -29,22 +29,22 @@
2929
* To compute a*P + b*G, we use the jacobian version for P, and the affine version for G, as
3030
* G is constant, so it only needs to be done once in advance.
3131
*/
32-
void static secp256k1_ecmult_table_precomp_gej(secp256k1_gej_t *pre, const secp256k1_gej_t *a, int w) {
32+
void static secp256k1_ecmult_table_precomp_gej_var(secp256k1_gej_t *pre, const secp256k1_gej_t *a, int w) {
3333
pre[0] = *a;
34-
secp256k1_gej_t d; secp256k1_gej_double(&d, &pre[0]);
34+
secp256k1_gej_t d; secp256k1_gej_double_var(&d, &pre[0]);
3535
for (int i=1; i<(1 << (w-2)); i++)
36-
secp256k1_gej_add(&pre[i], &d, &pre[i-1]);
36+
secp256k1_gej_add_var(&pre[i], &d, &pre[i-1]);
3737
}
3838

39-
void static secp256k1_ecmult_table_precomp_ge(secp256k1_ge_t *pre, const secp256k1_gej_t *a, int w) {
39+
void static secp256k1_ecmult_table_precomp_ge_var(secp256k1_ge_t *pre, const secp256k1_gej_t *a, int w) {
4040
const int table_size = 1 << (w-2);
4141
secp256k1_gej_t prej[table_size];
4242
prej[0] = *a;
43-
secp256k1_gej_t d; secp256k1_gej_double(&d, a);
43+
secp256k1_gej_t d; secp256k1_gej_double_var(&d, a);
4444
for (int i=1; i<table_size; i++) {
45-
secp256k1_gej_add(&prej[i], &d, &prej[i-1]);
45+
secp256k1_gej_add_var(&prej[i], &d, &prej[i-1]);
4646
}
47-
secp256k1_ge_set_all_gej(table_size, pre, prej);
47+
secp256k1_ge_set_all_gej_var(table_size, pre, prej);
4848
}
4949

5050
/** The number of entries a table with precomputed multiples needs to have. */
@@ -87,11 +87,11 @@ static void secp256k1_ecmult_start(void) {
8787
// calculate 2^128*generator
8888
secp256k1_gej_t g_128j = gj;
8989
for (int i=0; i<128; i++)
90-
secp256k1_gej_double(&g_128j, &g_128j);
90+
secp256k1_gej_double_var(&g_128j, &g_128j);
9191

9292
// precompute the tables with odd multiples
93-
secp256k1_ecmult_table_precomp_ge(ret->pre_g, &gj, WINDOW_G);
94-
secp256k1_ecmult_table_precomp_ge(ret->pre_g_128, &g_128j, WINDOW_G);
93+
secp256k1_ecmult_table_precomp_ge_var(ret->pre_g, &gj, WINDOW_G);
94+
secp256k1_ecmult_table_precomp_ge_var(ret->pre_g_128, &g_128j, WINDOW_G);
9595

9696
// Set the global pointer to the precomputation table.
9797
secp256k1_ecmult_consts = ret;
@@ -150,7 +150,7 @@ void static secp256k1_ecmult(secp256k1_gej_t *r, const secp256k1_gej_t *a, const
150150
#ifdef USE_ENDOMORPHISM
151151
secp256k1_num_t na_1, na_lam;
152152
// split na into na_1 and na_lam (where na = na_1 + na_lam*lambda, and na_1 and na_lam are ~128 bit)
153-
secp256k1_gej_split_exp(&na_1, &na_lam, na);
153+
secp256k1_gej_split_exp_var(&na_1, &na_lam, na);
154154

155155
// build wnaf representation for na_1 and na_lam.
156156
int wnaf_na_1[129]; int bits_na_1 = secp256k1_ecmult_wnaf(wnaf_na_1, &na_1, WINDOW_A);
@@ -165,7 +165,7 @@ void static secp256k1_ecmult(secp256k1_gej_t *r, const secp256k1_gej_t *a, const
165165

166166
// calculate odd multiples of a
167167
secp256k1_gej_t pre_a[ECMULT_TABLE_SIZE(WINDOW_A)];
168-
secp256k1_ecmult_table_precomp_gej(pre_a, a, WINDOW_A);
168+
secp256k1_ecmult_table_precomp_gej_var(pre_a, a, WINDOW_A);
169169

170170
#ifdef USE_ENDOMORPHISM
171171
secp256k1_gej_t pre_a_lam[ECMULT_TABLE_SIZE(WINDOW_A)];
@@ -190,30 +190,30 @@ void static secp256k1_ecmult(secp256k1_gej_t *r, const secp256k1_gej_t *a, const
190190
secp256k1_ge_t tmpa;
191191

192192
for (int i=bits-1; i>=0; i--) {
193-
secp256k1_gej_double(r, r);
193+
secp256k1_gej_double_var(r, r);
194194
int n;
195195
#ifdef USE_ENDOMORPHISM
196196
if (i < bits_na_1 && (n = wnaf_na_1[i])) {
197197
ECMULT_TABLE_GET_GEJ(&tmpj, pre_a, n, WINDOW_A);
198-
secp256k1_gej_add(r, r, &tmpj);
198+
secp256k1_gej_add_var(r, r, &tmpj);
199199
}
200200
if (i < bits_na_lam && (n = wnaf_na_lam[i])) {
201201
ECMULT_TABLE_GET_GEJ(&tmpj, pre_a_lam, n, WINDOW_A);
202-
secp256k1_gej_add(r, r, &tmpj);
202+
secp256k1_gej_add_var(r, r, &tmpj);
203203
}
204204
#else
205205
if (i < bits_na && (n = wnaf_na[i])) {
206206
ECMULT_TABLE_GET_GEJ(&tmpj, pre_a, n, WINDOW_A);
207-
secp256k1_gej_add(r, r, &tmpj);
207+
secp256k1_gej_add_var(r, r, &tmpj);
208208
}
209209
#endif
210210
if (i < bits_ng_1 && (n = wnaf_ng_1[i])) {
211211
ECMULT_TABLE_GET_GE(&tmpa, c->pre_g, n, WINDOW_G);
212-
secp256k1_gej_add_ge(r, r, &tmpa);
212+
secp256k1_gej_add_ge_var(r, r, &tmpa);
213213
}
214214
if (i < bits_ng_128 && (n = wnaf_ng_128[i])) {
215215
ECMULT_TABLE_GET_GE(&tmpa, c->pre_g_128, n, WINDOW_G);
216-
secp256k1_gej_add_ge(r, r, &tmpa);
216+
secp256k1_gej_add_ge_var(r, r, &tmpa);
217217
}
218218
}
219219
}

src/group.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ void static secp256k1_ge_get_hex(char *r, int *rlen, const secp256k1_ge_t *a);
6969
void static secp256k1_ge_set_gej(secp256k1_ge_t *r, secp256k1_gej_t *a);
7070

7171
/** Set a batch of group elements equal to the inputs given in jacobian coordinates */
72-
void static secp256k1_ge_set_all_gej(size_t len, secp256k1_ge_t r[len], const secp256k1_gej_t a[len]);
72+
void static secp256k1_ge_set_all_gej_var(size_t len, secp256k1_ge_t r[len], const secp256k1_gej_t a[len]);
7373

7474

7575
/** Set a group element (jacobian) equal to the point at infinity. */
@@ -82,7 +82,7 @@ void static secp256k1_gej_set_xy(secp256k1_gej_t *r, const secp256k1_fe_t *x, co
8282
void static secp256k1_gej_set_ge(secp256k1_gej_t *r, const secp256k1_ge_t *a);
8383

8484
/** Get the X coordinate of a group element (jacobian). */
85-
void static secp256k1_gej_get_x(secp256k1_fe_t *r, const secp256k1_gej_t *a);
85+
void static secp256k1_gej_get_x_var(secp256k1_fe_t *r, const secp256k1_gej_t *a);
8686

8787
/** Set r equal to the inverse of a (i.e., mirrored around the X axis) */
8888
void static secp256k1_gej_neg(secp256k1_gej_t *r, const secp256k1_gej_t *a);
@@ -91,14 +91,14 @@ void static secp256k1_gej_neg(secp256k1_gej_t *r, const secp256k1_gej_t *a);
9191
int static secp256k1_gej_is_infinity(const secp256k1_gej_t *a);
9292

9393
/** Set r equal to the double of a. */
94-
void static secp256k1_gej_double(secp256k1_gej_t *r, const secp256k1_gej_t *a);
94+
void static secp256k1_gej_double_var(secp256k1_gej_t *r, const secp256k1_gej_t *a);
9595

9696
/** Set r equal to the sum of a and b. */
97-
void static secp256k1_gej_add(secp256k1_gej_t *r, const secp256k1_gej_t *a, const secp256k1_gej_t *b);
97+
void static secp256k1_gej_add_var(secp256k1_gej_t *r, const secp256k1_gej_t *a, const secp256k1_gej_t *b);
9898

9999
/** Set r equal to the sum of a and b (with b given in affine coordinates). This is more efficient
100100
than secp256k1_gej_add. */
101-
void static secp256k1_gej_add_ge(secp256k1_gej_t *r, const secp256k1_gej_t *a, const secp256k1_ge_t *b);
101+
void static secp256k1_gej_add_ge_var(secp256k1_gej_t *r, const secp256k1_gej_t *a, const secp256k1_ge_t *b);
102102

103103
/** Get a hex representation of a point. *rlen will be overwritten with the real length. */
104104
void static secp256k1_gej_get_hex(char *r, int *rlen, const secp256k1_gej_t *a);
@@ -109,7 +109,7 @@ void static secp256k1_gej_mul_lambda(secp256k1_gej_t *r, const secp256k1_gej_t *
109109

110110
/** Find r1 and r2 such that r1+r2*lambda = a, and r1 and r2 are maximum 128 bits long (given that a is
111111
not more than 256 bits). */
112-
void static secp256k1_gej_split_exp(secp256k1_num_t *r1, secp256k1_num_t *r2, const secp256k1_num_t *a);
112+
void static secp256k1_gej_split_exp_var(secp256k1_num_t *r1, secp256k1_num_t *r2, const secp256k1_num_t *a);
113113
#endif
114114

115115
/** Clear a secp256k1_gej_t to prevent leaking sensitive information. */

src/group_impl.h

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,18 @@ void static secp256k1_ge_get_hex(char *r, int *rlen, const secp256k1_ge_t *a) {
5555
}
5656

5757
void static secp256k1_ge_set_gej(secp256k1_ge_t *r, secp256k1_gej_t *a) {
58+
r->infinity = a->infinity;
59+
secp256k1_fe_inv(&a->z, &a->z);
60+
secp256k1_fe_t z2; secp256k1_fe_sqr(&z2, &a->z);
61+
secp256k1_fe_t z3; secp256k1_fe_mul(&z3, &a->z, &z2);
62+
secp256k1_fe_mul(&a->x, &a->x, &z2);
63+
secp256k1_fe_mul(&a->y, &a->y, &z3);
64+
secp256k1_fe_set_int(&a->z, 1);
65+
r->x = a->x;
66+
r->y = a->y;
67+
}
68+
69+
void static secp256k1_ge_set_gej_var(secp256k1_ge_t *r, secp256k1_gej_t *a) {
5870
r->infinity = a->infinity;
5971
if (a->infinity) {
6072
return;
@@ -69,7 +81,7 @@ void static secp256k1_ge_set_gej(secp256k1_ge_t *r, secp256k1_gej_t *a) {
6981
r->y = a->y;
7082
}
7183

72-
void static secp256k1_ge_set_all_gej(size_t len, secp256k1_ge_t r[len], const secp256k1_gej_t a[len]) {
84+
void static secp256k1_ge_set_all_gej_var(size_t len, secp256k1_ge_t r[len], const secp256k1_gej_t a[len]) {
7385
int count = 0;
7486
secp256k1_fe_t az[len];
7587
for (int i=0; i<len; i++) {
@@ -140,7 +152,7 @@ void static secp256k1_gej_set_ge(secp256k1_gej_t *r, const secp256k1_ge_t *a) {
140152
secp256k1_fe_set_int(&r->z, 1);
141153
}
142154

143-
void static secp256k1_gej_get_x(secp256k1_fe_t *r, const secp256k1_gej_t *a) {
155+
void static secp256k1_gej_get_x_var(secp256k1_fe_t *r, const secp256k1_gej_t *a) {
144156
secp256k1_fe_t zi2; secp256k1_fe_inv_var(&zi2, &a->z); secp256k1_fe_sqr(&zi2, &zi2);
145157
secp256k1_fe_mul(r, &a->x, &zi2);
146158
}
@@ -189,7 +201,7 @@ int static secp256k1_ge_is_valid(const secp256k1_ge_t *a) {
189201
return secp256k1_fe_equal(&y2, &x3);
190202
}
191203

192-
void static secp256k1_gej_double(secp256k1_gej_t *r, const secp256k1_gej_t *a) {
204+
void static secp256k1_gej_double_var(secp256k1_gej_t *r, const secp256k1_gej_t *a) {
193205
if (a->infinity) {
194206
r->infinity = 1;
195207
return;
@@ -226,7 +238,7 @@ void static secp256k1_gej_double(secp256k1_gej_t *r, const secp256k1_gej_t *a) {
226238
r->infinity = 0;
227239
}
228240

229-
void static secp256k1_gej_add(secp256k1_gej_t *r, const secp256k1_gej_t *a, const secp256k1_gej_t *b) {
241+
void static secp256k1_gej_add_var(secp256k1_gej_t *r, const secp256k1_gej_t *a, const secp256k1_gej_t *b) {
230242
if (a->infinity) {
231243
*r = *b;
232244
return;
@@ -248,7 +260,7 @@ void static secp256k1_gej_add(secp256k1_gej_t *r, const secp256k1_gej_t *a, cons
248260
secp256k1_fe_normalize(&s1);
249261
secp256k1_fe_normalize(&s2);
250262
if (secp256k1_fe_equal(&s1, &s2)) {
251-
secp256k1_gej_double(r, a);
263+
secp256k1_gej_double_var(r, a);
252264
} else {
253265
r->infinity = 1;
254266
}
@@ -267,7 +279,7 @@ void static secp256k1_gej_add(secp256k1_gej_t *r, const secp256k1_gej_t *a, cons
267279
secp256k1_fe_add(&r->y, &h3);
268280
}
269281

270-
void static secp256k1_gej_add_ge(secp256k1_gej_t *r, const secp256k1_gej_t *a, const secp256k1_ge_t *b) {
282+
void static secp256k1_gej_add_ge_var(secp256k1_gej_t *r, const secp256k1_gej_t *a, const secp256k1_ge_t *b) {
271283
if (a->infinity) {
272284
r->infinity = b->infinity;
273285
r->x = b->x;
@@ -291,7 +303,7 @@ void static secp256k1_gej_add_ge(secp256k1_gej_t *r, const secp256k1_gej_t *a, c
291303
secp256k1_fe_normalize(&s1);
292304
secp256k1_fe_normalize(&s2);
293305
if (secp256k1_fe_equal(&s1, &s2)) {
294-
secp256k1_gej_double(r, a);
306+
secp256k1_gej_double_var(r, a);
295307
} else {
296308
r->infinity = 1;
297309
}
@@ -323,7 +335,7 @@ void static secp256k1_gej_mul_lambda(secp256k1_gej_t *r, const secp256k1_gej_t *
323335
secp256k1_fe_mul(&r->x, &r->x, beta);
324336
}
325337

326-
void static secp256k1_gej_split_exp(secp256k1_num_t *r1, secp256k1_num_t *r2, const secp256k1_num_t *a) {
338+
void static secp256k1_gej_split_exp_var(secp256k1_num_t *r1, secp256k1_num_t *r2, const secp256k1_num_t *a) {
327339
const secp256k1_ge_consts_t *c = secp256k1_ge_consts;
328340
secp256k1_num_t bnc1, bnc2, bnt1, bnt2, bnn2;
329341

0 commit comments

Comments
 (0)