Skip to content

Commit 3f7e136

Browse files
igus68jogme
igus68
authored and
jogme
committed
ec: make ossl_ec_scalar_mul_ladder() scalar padding constant time
Fixes CVE-2026-54872 Assisted-by: Claude:claude-opus-4-8 Reviewed-by: Alicja Kario <hkario@redhat.com> Reviewed-by: Viktor Dukhovni <viktor@openssl.org> Merge-date: Tue Sep 29 11:26:21 2026
1 parent a4da2a3 commit 3f7e136

3 files changed

Lines changed: 77 additions & 0 deletions

File tree

‎crypto/bn/bn_intern.c‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
#include "internal/cryptlib.h"
1111
#include "bn_local.h"
12+
#include "internal/constant_time.h"
1213

1314
/*
1415
* Determine the modified width-(w+1) Non-Adjacent Form (wNAF) of 'scalar'.
@@ -152,6 +153,43 @@ void bn_set_all_zero(BIGNUM *a)
152153
a->d[i] = 0;
153154
}
154155

156+
/*
157+
* Zero-extend |a| so that it occupies exactly |words| words, flag it
158+
* BN_FLG_FIXED_TOP and leave its numeric value unchanged.
159+
*
160+
* This is a companion to bn_correct_top(): where the latter minimises the top
161+
* of a BIGNUM, this one pins the top to a caller-chosen, value-independent
162+
* width. Constant-time code uses it to make the cost of subsequent word-wise
163+
* operations (e.g. BN_uadd()/BN_add()) independent of the magnitude of a
164+
* secret value. |words| must be greater than or equal to the current top.
165+
*
166+
* The routine is itself constant time with respect to the current a->top: it
167+
* always sweeps a fixed |words| iterations and selects value-or-zero per word
168+
* with an arithmetic mask, rather than looping over the (possibly secret)
169+
* a->top..words range. Masking the high words with zero also launders any
170+
* uninitialised padding, so it is safe for the memory sanitiser.
171+
*/
172+
int bn_set_top_fixed(BIGNUM *a, int words)
173+
{
174+
size_t i, n = (size_t)words;
175+
BN_ULONG mask;
176+
177+
if (words < a->top)
178+
return 0;
179+
if (bn_wexpand(a, words) == NULL) {
180+
ERR_raise(ERR_LIB_BN, ERR_R_BN_LIB);
181+
return 0;
182+
}
183+
for (i = 0; i < n; i++) {
184+
/* mask = all ones iff i < a->top, else all zeros */
185+
mask = value_barrier_bn((BN_ULONG)0 - ((i - a->top) >> (8 * sizeof(i) - 1)));
186+
a->d[i] &= mask;
187+
}
188+
a->top = words;
189+
a->flags |= BN_FLG_FIXED_TOP;
190+
return 1;
191+
}
192+
155193
int bn_copy_words(BN_ULONG *out, const BIGNUM *in, int size)
156194
{
157195
if (in->top > size)

‎crypto/ec/ec_mult.c‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,18 @@ int ossl_ec_scalar_mul_ladder(const EC_GROUP *group, EC_POINT *r,
213213
goto err;
214214
}
215215

216+
/*
217+
* Constant-timeness of this copy depends on the caller: BN_copy() moves
218+
* scalar->top words unless |scalar| is flagged BN_FLG_CONSTTIME (in which
219+
* case scalar->dmax words are moved). Secret scalars are therefore
220+
* expected to arrive either BN_FLG_CONSTTIME or fixed-top, so that their
221+
* top is a public, value-independent width and the copy length does not
222+
* leak their magnitude. ECDSA satisfies this via
223+
* ossl_bn_priv_rand_range_fixed_top(); the generic SM2 signing path does
224+
* not yet (see the sm2_sig_gen() hardening tracked separately). The
225+
* fixed-top pinning below makes the subsequent arithmetic constant time
226+
* regardless, but cannot retroactively fix the copy length here.
227+
*/
216228
if (!BN_copy(k, scalar)) {
217229
ERR_raise(ERR_LIB_EC, ERR_R_BN_LIB);
218230
goto err;
@@ -231,10 +243,36 @@ int ossl_ec_scalar_mul_ladder(const EC_GROUP *group, EC_POINT *r,
231243
}
232244
}
233245

246+
/*
247+
* |k| may still carry a top that depends on the value of the secret
248+
* scalar: callers pass either a fixed-top BIGNUM (e.g. the ECDSA nonce)
249+
* or a minimal-top one (e.g. SM2), and BN_copy() above preserves that
250+
* top. Pin |k| to a fixed number of words (matching the group
251+
* cardinality) so that the additions below run in constant time,
252+
* independently of the bit length of the scalar. Otherwise the work
253+
* done by BN_add()/BN_uadd() depends on the operand tops and leaks the
254+
* magnitude of the secret scalar.
255+
*/
256+
if (!bn_set_top_fixed(k, group_top)) {
257+
ERR_raise(ERR_LIB_EC, ERR_R_BN_LIB);
258+
goto err;
259+
}
260+
234261
if (!BN_add(lambda, k, cardinality)) {
235262
ERR_raise(ERR_LIB_EC, ERR_R_BN_LIB);
236263
goto err;
237264
}
265+
/*
266+
* |lambda| = scalar + cardinality may or may not have produced a carry
267+
* into an extra word depending on the secret scalar. Pin its top to one
268+
* word above the group top so that the second addition, which consumes
269+
* |lambda|, is likewise constant time and so that the BN_is_bit_set()
270+
* below always inspects a defined word.
271+
*/
272+
if (!bn_set_top_fixed(lambda, group_top + 1)) {
273+
ERR_raise(ERR_LIB_EC, ERR_R_BN_LIB);
274+
goto err;
275+
}
238276
BN_set_flags(lambda, BN_FLG_CONSTTIME);
239277
if (!BN_add(k, lambda, cardinality)) {
240278
ERR_raise(ERR_LIB_EC, ERR_R_BN_LIB);

‎include/crypto/bn.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ BIGNUM *bn_wexpand(BIGNUM *a, int words);
1818
BIGNUM *bn_expand2(BIGNUM *a, int words);
1919

2020
void bn_correct_top(BIGNUM *a);
21+
int bn_set_top_fixed(BIGNUM *a, int words);
2122

2223
/*
2324
* Determine the modified width-(w+1) Non-Adjacent Form (wNAF) of 'scalar'.

0 commit comments

Comments
 (0)