Skip to content

Commit 1464b93

Browse files
peterdettmansipa
authored andcommitted
Optimization: use Nx32 representation for recoded bits
The existing code needs to deal with the edge case that bit_pos >= 256, which would lead to an out-of-bounds read from secp256k1_scalar. Instead, recode the scalar into an array of uint32_t with enough zero padding at the end to alleviate the issue. This also simplifies the code, and is necessary for a security improvement in a follow-up commit. Original code by Peter Dettman, with modifications by Pieter Wuille.
1 parent eab993a commit 1464b93

File tree

1 file changed

+19
-10
lines changed

1 file changed

+19
-10
lines changed

src/ecmult_gen_impl.h

+19-10
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,12 @@ static void secp256k1_ecmult_gen(const secp256k1_ecmult_gen_context *ctx, secp25
3434
secp256k1_ge add;
3535
secp256k1_fe neg;
3636
secp256k1_ge_storage adds;
37-
secp256k1_scalar recoded;
38-
int first = 1;
37+
secp256k1_scalar tmp;
38+
/* Array of uint32_t values large enough to store COMB_BITS bits. Only the bottom
39+
* 8 are ever nonzero, but having the zero padding at the end if COMB_BITS>256
40+
* avoids the need to deal with out-of-bounds reads from a scalar. */
41+
uint32_t recoded[(COMB_BITS + 31) >> 5] = {0};
42+
int first = 1, i;
3943

4044
memset(&adds, 0, sizeof(adds));
4145

@@ -44,17 +48,22 @@ static void secp256k1_ecmult_gen(const secp256k1_ecmult_gen_context *ctx, secp25
4448
* To blind the scalar used in the computation, we rewrite this to be R = (gn-b)*G + b*G.
4549
*
4650
* Next, we write (gn-b)*G as a sum of values (2*bit_i-1) * 2^i * G, for i=0..COMB_BITS-1.
47-
* The values bit_i can be found as the binary representation of recoded =
48-
* (gn + 2^COMB_BITS - 1 - b)/2 (mod order).
51+
* The values bit_i can be found as the binary representation of
52+
* (gn + 2^COMB_BITS - 1 - b)/2 (mod order), stored in recoded.
4953
*
5054
* The value (2^COMB_BITS - 1 - b) is precomputed as ctx->scalar_offset, and bG is
5155
* precomputed as ctx->final_point_add. Thus recoded can be written as
5256
* recoded = (gn + scalar_offset)/2, and R becomes the sum of (2*bit_i-1)*2^i*G
5357
* values plus final_point_add. */
5458

55-
/* Compute the recoded value as a scalar. */
56-
secp256k1_scalar_add(&recoded, gn, &ctx->scalar_offset);
57-
secp256k1_scalar_half(&recoded, &recoded);
59+
/* Compute (gn + 2^COMB_BITS - 1 - 1)/2 value as a scalar. */
60+
secp256k1_scalar_add(&tmp, gn, &ctx->scalar_offset);
61+
secp256k1_scalar_half(&tmp, &tmp);
62+
/* Convert to recoded array. */
63+
for (i = 0; i < 8; ++i) {
64+
recoded[i] = secp256k1_scalar_get_bits(&tmp, 32 * i, 32);
65+
}
66+
secp256k1_scalar_clear(&tmp);
5867

5968
/* In secp256k1_ecmult_gen_prec_table we have precomputed sums of the
6069
* (2*bit_i-1) * 2^i * G points, for various combinations of i positions.
@@ -122,8 +131,8 @@ static void secp256k1_ecmult_gen(const secp256k1_ecmult_gen_context *ctx, secp25
122131
* together: bit (tooth) of bits = bit
123132
* ((block*COMB_TEETH + tooth)*COMB_SPACING + comb_off) of recoded. */
124133
uint32_t bits = 0, sign, abs, index, tooth;
125-
for (tooth = 0; tooth < COMB_TEETH && bit_pos < 256; ++tooth) {
126-
uint32_t bit = secp256k1_scalar_get_bits(&recoded, bit_pos, 1);
134+
for (tooth = 0; tooth < COMB_TEETH; ++tooth) {
135+
uint32_t bit = (recoded[bit_pos >> 5] >> (bit_pos & 0x1f)) & 1;
127136
bits |= bit << tooth;
128137
bit_pos += COMB_SPACING;
129138
}
@@ -178,7 +187,7 @@ static void secp256k1_ecmult_gen(const secp256k1_ecmult_gen_context *ctx, secp25
178187
secp256k1_fe_clear(&neg);
179188
secp256k1_ge_clear(&add);
180189
memset(&adds, 0, sizeof(adds));
181-
secp256k1_scalar_clear(&recoded);
190+
memset(&recoded, 0, sizeof(recoded));
182191
}
183192

184193
/* Setup blinding values for secp256k1_ecmult_gen. */

0 commit comments

Comments
 (0)