Skip to content

Commit 2f6c801

Browse files
gmaxwellgmaxwell
committed
Try to not leave secret data on the stack or heap.
This makes a basic effort and has not been audited. Doesn't appear to have a measurable performance impact on bench. It also adds a secp256k1_num_free to secp256k1_ecdsa_pubkey_create.
1 parent 13e44df commit 2f6c801

File tree

11 files changed

+84
-3
lines changed

11 files changed

+84
-3
lines changed

‎src/ecdsa_impl.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,10 @@ int static secp256k1_ecdsa_sig_sign(secp256k1_ecdsa_sig_t *sig, const secp256k1_
186186
secp256k1_num_mod(&n, &c->order);
187187
secp256k1_num_mod_inverse(&sig->s, nonce, &c->order);
188188
secp256k1_num_mod_mul(&sig->s, &sig->s, &n, &c->order);
189+
secp256k1_num_clear(&n);
189190
secp256k1_num_free(&n);
191+
secp256k1_gej_clear(&rp);
192+
secp256k1_ge_clear(&r);
190193
if (secp256k1_num_is_zero(&sig->s))
191194
return 0;
192195
if (secp256k1_num_cmp(&sig->s, &c->half_order) > 0) {

‎src/ecmult_impl.h‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,13 +190,17 @@ void static secp256k1_ecmult_gen(secp256k1_gej_t *r, const secp256k1_num_t *gn)
190190
secp256k1_num_copy(&n, gn);
191191
const secp256k1_ecmult_consts_t *c = secp256k1_ecmult_consts;
192192
secp256k1_gej_set_infinity(r);
193+
secp256k1_ge_t add;
194+
int bits;
193195
for (int j=0; j<64; j++) {
194-
secp256k1_ge_t add;
195-
int bits = secp256k1_num_shift(&n, 4);
196+
bits = secp256k1_num_shift(&n, 4);
196197
for (int k=0; k<sizeof(secp256k1_ge_t); k++)
197198
((unsigned char*)(&add))[k] = c->prec[j][k][bits];
198199
secp256k1_gej_add_ge(r, r, &add);
199200
}
201+
bits = 0;
202+
secp256k1_ge_clear(&add);
203+
secp256k1_num_clear(&n);
200204
secp256k1_num_free(&n);
201205
secp256k1_gej_add_ge(r, r, &c->fin);
202206
}

‎src/field_10x26_impl.h‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,16 @@ int static inline secp256k1_fe_is_odd(const secp256k1_fe_t *a) {
9292
return a->n[0] & 1;
9393
}
9494

95+
void static inline secp256k1_fe_clear(secp256k1_fe_t *a) {
96+
#ifdef VERIFY
97+
a->normalized = 0;
98+
a->magnitude = 0;
99+
#endif
100+
for (int i=0; i<10; i++) {
101+
a->n[i] = 0;
102+
}
103+
}
104+
95105
// TODO: not constant time!
96106
int static inline secp256k1_fe_equal(const secp256k1_fe_t *a, const secp256k1_fe_t *b) {
97107
#ifdef VERIFY

‎src/field_5x52_impl.h‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,16 @@ int static inline secp256k1_fe_is_odd(const secp256k1_fe_t *a) {
124124
return a->n[0] & 1;
125125
}
126126

127+
void static inline secp256k1_fe_clear(secp256k1_fe_t *a) {
128+
#ifdef VERIFY
129+
a->magnitude = 0;
130+
a->normalized = 0;
131+
#endif
132+
for (int i=0; i<5; i++) {
133+
a->n[i] = 0;
134+
}
135+
}
136+
127137
// TODO: not constant time!
128138
int static inline secp256k1_fe_equal(const secp256k1_fe_t *a, const secp256k1_fe_t *b) {
129139
#ifdef VERIFY

‎src/field_gmp_impl.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,11 @@ void static inline secp256k1_fe_set_int(secp256k1_fe_t *r, int a) {
5050
r->n[i] = 0;
5151
}
5252

53+
void static inline secp256k1_fe_clear(secp256k1_fe_t *r) {
54+
for (int i=0; i<FIELD_LIMBS+1; i++)
55+
r->n[i] = 0;
56+
}
57+
5358
int static inline secp256k1_fe_is_zero(const secp256k1_fe_t *a) {
5459
int ret = 1;
5560
for (int i=0; i<FIELD_LIMBS+1; i++)

‎src/group.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,4 +112,11 @@ void static secp256k1_gej_mul_lambda(secp256k1_gej_t *r, const secp256k1_gej_t *
112112
void static secp256k1_gej_split_exp(secp256k1_num_t *r1, secp256k1_num_t *r2, const secp256k1_num_t *a);
113113
#endif
114114

115+
/** Clear a secp256k1_gej_t to prevent leaking sensitive information. */
116+
void static secp256k1_gej_clear(secp256k1_gej_t *r);
117+
118+
/** Clear a secp256k1_ge_t to prevent leaking sensitive information. */
119+
void static secp256k1_ge_clear(secp256k1_ge_t *r);
120+
121+
115122
#endif

‎src/group_impl.h‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,19 @@ void static secp256k1_gej_set_xy(secp256k1_gej_t *r, const secp256k1_fe_t *x, co
102102
secp256k1_fe_set_int(&r->z, 1);
103103
}
104104

105+
void static secp256k1_gej_clear(secp256k1_gej_t *r) {
106+
r->infinity = 0;
107+
secp256k1_fe_clear(&r->x);
108+
secp256k1_fe_clear(&r->y);
109+
secp256k1_fe_clear(&r->z);
110+
}
111+
112+
void static secp256k1_ge_clear(secp256k1_ge_t *r) {
113+
r->infinity = 0;
114+
secp256k1_fe_clear(&r->x);
115+
secp256k1_fe_clear(&r->y);
116+
}
117+
105118
int static secp256k1_ge_set_xo(secp256k1_ge_t *r, const secp256k1_fe_t *x, int odd) {
106119
r->x = *x;
107120
secp256k1_fe_t x2; secp256k1_fe_sqr(&x2, x);

‎src/num.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@
2020
/** Initialize a number. */
2121
void static secp256k1_num_init(secp256k1_num_t *r);
2222

23+
/** Clear a number to prevent the leak of sensitive data. */
24+
void static secp256k1_num_clear(secp256k1_num_t *r);
25+
2326
/** Free a number. */
2427
void static secp256k1_num_free(secp256k1_num_t *r);
2528

‎src/num_gmp_impl.h‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ void static secp256k1_num_init(secp256k1_num_t *r) {
2626
r->data[0] = 0;
2727
}
2828

29+
void static secp256k1_num_clear(secp256k1_num_t *r) {
30+
memset(r, 0, sizeof(*r));
31+
}
32+
2933
void static secp256k1_num_free(secp256k1_num_t *r) {
3034
}
3135

@@ -54,8 +58,10 @@ void static secp256k1_num_get_bin(unsigned char *r, unsigned int rlen, const sec
5458
while (shift < len && tmp[shift] == 0) shift++;
5559
assert(len-shift <= rlen);
5660
memset(r, 0, rlen - len + shift);
57-
if (len > shift)
61+
if (len > shift) {
5862
memcpy(r + rlen - len + shift, tmp + shift, len - shift);
63+
}
64+
memset(tmp, 0, sizeof(tmp));
5965
}
6066

6167
void static secp256k1_num_set_bin(secp256k1_num_t *r, const unsigned char *a, unsigned int alen) {
@@ -97,6 +103,7 @@ void static secp256k1_num_mod(secp256k1_num_t *r, const secp256k1_num_t *m) {
97103
if (r->limbs >= m->limbs) {
98104
mp_limb_t t[2*NUM_LIMBS];
99105
mpn_tdiv_qr(t, r->data, 0, r->data, r->limbs, m->data, m->limbs);
106+
memset(t, 0, sizeof(t));
100107
r->limbs = m->limbs;
101108
while (r->limbs > 1 && r->data[r->limbs-1]==0) r->limbs--;
102109
}
@@ -141,6 +148,9 @@ void static secp256k1_num_mod_inverse(secp256k1_num_t *r, const secp256k1_num_t
141148
} else {
142149
r->limbs = sn;
143150
}
151+
memset(g, 0, sizeof(g));
152+
memset(u, 0, sizeof(u));
153+
memset(v, 0, sizeof(v));
144154
}
145155

146156
int static secp256k1_num_is_zero(const secp256k1_num_t *a) {
@@ -213,6 +223,7 @@ void static secp256k1_num_mul(secp256k1_num_t *r, const secp256k1_num_t *a, cons
213223
assert(r->limbs <= 2*NUM_LIMBS);
214224
mpn_copyi(r->data, tmp, r->limbs);
215225
r->neg = a->neg ^ b->neg;
226+
memset(tmp, 0, sizeof(tmp));
216227
}
217228

218229
void static secp256k1_num_div(secp256k1_num_t *r, const secp256k1_num_t *a, const secp256k1_num_t *b) {

‎src/num_openssl_impl.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@ void static secp256k1_num_free(secp256k1_num_t *r) {
2121
BN_free(&r->bn);
2222
}
2323

24+
void static secp256k1_num_clear(secp256k1_num_t *r) {
25+
BN_clear(&r->bn);
26+
}
27+
2428
void static secp256k1_num_copy(secp256k1_num_t *r, const secp256k1_num_t *a) {
2529
BN_copy(&r->bn, &a->bn);
2630
}

0 commit comments

Comments
 (0)