Skip to content

Commit 419f5cb

Browse files
igus68jogme
igus68
authored and
jogme
committed
sm2: make sm2_sig_gen() constant time
After fixing CVE-2025-9231, sm2_sig_gen() still used variable-time BN_mod_mul() and BN_sub() on private key and nonce material. This commit makes it fully constant time. Fixes CVE-2026-77696 Co-authored-by: Viktor Dukhovni <viktor@openssl.org> 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:40 2026
1 parent 7d83bc7 commit 419f5cb

2 files changed

Lines changed: 55 additions & 13 deletions

File tree

‎crypto/ec/ec_mult.c‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -220,10 +220,9 @@ int ossl_ec_scalar_mul_ladder(const EC_GROUP *group, EC_POINT *r,
220220
* expected to arrive either BN_FLG_CONSTTIME or fixed-top, so that their
221221
* top is a public, value-independent width and the copy length does not
222222
* 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.
223+
* ossl_bn_priv_rand_range_fixed_top(). The fixed-top pinning below makes
224+
* the subsequent arithmetic constant time regardless, but cannot
225+
* retroactively fix the copy length here.
227226
*/
228227
if (!BN_copy(k, scalar)) {
229228
ERR_raise(ERR_LIB_EC, ERR_R_BN_LIB);

‎crypto/sm2/sm2_sign.c‎

Lines changed: 52 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "crypto/sm2.h"
1515
#include "crypto/sm2err.h"
1616
#include "crypto/ec.h" /* ossl_ec_group_do_inverse_ord() */
17+
#include "crypto/bn.h" /* fixed-top / Montgomery constant-time BN helpers */
1718
#include "internal/numbers.h"
1819
#include <openssl/err.h>
1920
#include <openssl/evp.h>
@@ -215,17 +216,22 @@ static ECDSA_SIG *sm2_sig_gen(const EC_KEY *key, const BIGNUM *e)
215216
EC_POINT *kG = NULL;
216217
BN_CTX *ctx = NULL;
217218
BIGNUM *k = NULL;
218-
BIGNUM *rk = NULL;
219219
BIGNUM *r = NULL;
220220
BIGNUM *s = NULL;
221221
BIGNUM *x1 = NULL;
222222
BIGNUM *tmp = NULL;
223+
BN_MONT_CTX *mont = EC_GROUP_get_mont_data(group);
223224
OSSL_LIB_CTX *libctx = ossl_ec_key_get_libctx(key);
224225

225226
if (dA == NULL) {
226227
ERR_raise(ERR_LIB_SM2, SM2_R_INVALID_PRIVATE_KEY);
227228
goto done;
228229
}
230+
231+
if (mont == NULL) {
232+
ERR_raise(ERR_LIB_SM2, ERR_R_EC_LIB);
233+
goto done;
234+
}
229235
kG = EC_POINT_new(group);
230236
if (kG == NULL) {
231237
ERR_raise(ERR_LIB_SM2, ERR_R_EC_LIB);
@@ -239,7 +245,6 @@ static ECDSA_SIG *sm2_sig_gen(const EC_KEY *key, const BIGNUM *e)
239245

240246
BN_CTX_start(ctx);
241247
k = BN_CTX_get(ctx);
242-
rk = BN_CTX_get(ctx);
243248
x1 = BN_CTX_get(ctx);
244249
tmp = BN_CTX_get(ctx);
245250
if (tmp == NULL) {
@@ -273,6 +278,18 @@ static ECDSA_SIG *sm2_sig_gen(const EC_KEY *key, const BIGNUM *e)
273278
ERR_raise(ERR_LIB_SM2, ERR_R_INTERNAL_ERROR);
274279
goto done;
275280
}
281+
/*
282+
* Pin the nonce to a fixed, value-independent width and flag it
283+
* BN_FLG_CONSTTIME, so its magnitude does not leak through operand
284+
* lengths in the scalar copy inside the ladder or in the arithmetic
285+
* below. BN_priv_rand_range_ex() is kept so the nonce value itself
286+
* is unchanged; only its representation is pinned.
287+
*/
288+
BN_set_flags(k, BN_FLG_CONSTTIME);
289+
if (!bn_set_top_fixed(k, bn_get_top(order))) {
290+
ERR_raise(ERR_LIB_SM2, ERR_R_BN_LIB);
291+
goto done;
292+
}
276293

277294
if (!EC_POINT_mul(group, kG, k, NULL, NULL, ctx)
278295
|| !EC_POINT_get_affine_coordinates(group, kG, x1, NULL,
@@ -282,23 +299,49 @@ static ECDSA_SIG *sm2_sig_gen(const EC_KEY *key, const BIGNUM *e)
282299
goto done;
283300
}
284301

285-
/* try again if r == 0 or r+k == n */
302+
/* try again if r == 0 or r + k == n */
286303
if (BN_is_zero(r))
287304
continue;
288305

289-
if (!BN_add(rk, r, k)) {
290-
ERR_raise(ERR_LIB_SM2, ERR_R_INTERNAL_ERROR);
306+
/*
307+
* Since 0 < r < n and 0 < k < n, r + k == n is the same as
308+
* k == n - r. Both operands of the subtraction are public, so
309+
* compute it in the open and then compare against the nonce with a
310+
* fixed-width constant-time comparison. A BN_cmp() on r + k would
311+
* branch on whether the sum carried into an extra word, which
312+
* depends on the value of k.
313+
*/
314+
if (!BN_sub(tmp, order, r)
315+
|| !bn_set_top_fixed(tmp, bn_get_top(order))) {
316+
ERR_raise(ERR_LIB_SM2, ERR_R_BN_LIB);
291317
goto done;
292318
}
293319

294-
if (BN_cmp(rk, order) == 0)
320+
if (CRYPTO_memcmp(bn_get_words(k), bn_get_words(tmp),
321+
bn_get_top(order) * sizeof(BN_ULONG))
322+
== 0)
295323
continue;
296324

325+
/*
326+
* s = ((1 + dA)^-1 * (k - r * dA)) mod order
327+
*
328+
* Computed with fixed-top / Montgomery constant-time primitives, so
329+
* that the running time does not depend on the secret k or dA (the
330+
* generic BN_mod_mul()/BN_sub() used previously reduce via BN_div(),
331+
* whose timing is value dependent). This mirrors the ECDSA path.
332+
*
333+
* s holds (1 + dA)^-1 throughout; the (k - r * dA) term is built in
334+
* tmp. bn_mul_mont_fixed_top() with one operand in the Montgomery
335+
* domain yields the plain product, and the final
336+
* BN_mod_mul_montgomery() returns the user-visible, normalised value.
337+
*/
297338
if (!BN_add(s, dA, BN_value_one())
298339
|| !ossl_ec_group_do_inverse_ord(group, s, s, ctx)
299-
|| !BN_mod_mul(tmp, dA, r, order, ctx)
300-
|| !BN_sub(tmp, k, tmp)
301-
|| !BN_mod_mul(s, s, tmp, order, ctx)) {
340+
|| !bn_to_mont_fixed_top(tmp, r, mont, ctx)
341+
|| !bn_mul_mont_fixed_top(tmp, tmp, dA, mont, ctx)
342+
|| !bn_mod_sub_fixed_top(tmp, k, tmp, order)
343+
|| !bn_to_mont_fixed_top(tmp, tmp, mont, ctx)
344+
|| !BN_mod_mul_montgomery(s, tmp, s, mont, ctx)) {
302345
ERR_raise(ERR_LIB_SM2, ERR_R_BN_LIB);
303346
goto done;
304347
}

0 commit comments

Comments
 (0)