Skip to content

Commit 20b2062

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:25:42 2026
1 parent 8166827 commit 20b2062

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>
@@ -235,17 +236,22 @@ static ECDSA_SIG *sm2_sig_gen(const EC_KEY *key, const BIGNUM *e)
235236
EC_POINT *kG = NULL;
236237
BN_CTX *ctx = NULL;
237238
BIGNUM *k = NULL;
238-
BIGNUM *rk = NULL;
239239
BIGNUM *r = NULL;
240240
BIGNUM *s = NULL;
241241
BIGNUM *x1 = NULL;
242242
BIGNUM *tmp = NULL;
243+
BN_MONT_CTX *mont = EC_GROUP_get_mont_data(group);
243244
OSSL_LIB_CTX *libctx = ossl_ec_key_get_libctx(key);
244245

245246
if (dA == NULL) {
246247
ERR_raise(ERR_LIB_SM2, SM2_R_INVALID_PRIVATE_KEY);
247248
goto done;
248249
}
250+
251+
if (mont == NULL) {
252+
ERR_raise(ERR_LIB_SM2, ERR_R_EC_LIB);
253+
goto done;
254+
}
249255
kG = EC_POINT_new(group);
250256
if (kG == NULL) {
251257
ERR_raise(ERR_LIB_SM2, ERR_R_EC_LIB);
@@ -259,7 +265,6 @@ static ECDSA_SIG *sm2_sig_gen(const EC_KEY *key, const BIGNUM *e)
259265

260266
BN_CTX_start(ctx);
261267
k = BN_CTX_get(ctx);
262-
rk = BN_CTX_get(ctx);
263268
x1 = BN_CTX_get(ctx);
264269
tmp = BN_CTX_get(ctx);
265270
if (tmp == NULL) {
@@ -293,6 +298,18 @@ static ECDSA_SIG *sm2_sig_gen(const EC_KEY *key, const BIGNUM *e)
293298
ERR_raise(ERR_LIB_SM2, ERR_R_INTERNAL_ERROR);
294299
goto done;
295300
}
301+
/*
302+
* Pin the nonce to a fixed, value-independent width and flag it
303+
* BN_FLG_CONSTTIME, so its magnitude does not leak through operand
304+
* lengths in the scalar copy inside the ladder or in the arithmetic
305+
* below. BN_priv_rand_range_ex() is kept so the nonce value itself
306+
* is unchanged; only its representation is pinned.
307+
*/
308+
BN_set_flags(k, BN_FLG_CONSTTIME);
309+
if (!bn_set_top_fixed(k, bn_get_top(order))) {
310+
ERR_raise(ERR_LIB_SM2, ERR_R_BN_LIB);
311+
goto done;
312+
}
296313

297314
if (!EC_POINT_mul(group, kG, k, NULL, NULL, ctx)
298315
|| !EC_POINT_get_affine_coordinates(group, kG, x1, NULL,
@@ -302,23 +319,49 @@ static ECDSA_SIG *sm2_sig_gen(const EC_KEY *key, const BIGNUM *e)
302319
goto done;
303320
}
304321

305-
/* try again if r == 0 or r+k == n */
322+
/* try again if r == 0 or r + k == n */
306323
if (BN_is_zero(r))
307324
continue;
308325

309-
if (!BN_add(rk, r, k)) {
310-
ERR_raise(ERR_LIB_SM2, ERR_R_INTERNAL_ERROR);
326+
/*
327+
* Since 0 < r < n and 0 < k < n, r + k == n is the same as
328+
* k == n - r. Both operands of the subtraction are public, so
329+
* compute it in the open and then compare against the nonce with a
330+
* fixed-width constant-time comparison. A BN_cmp() on r + k would
331+
* branch on whether the sum carried into an extra word, which
332+
* depends on the value of k.
333+
*/
334+
if (!BN_sub(tmp, order, r)
335+
|| !bn_set_top_fixed(tmp, bn_get_top(order))) {
336+
ERR_raise(ERR_LIB_SM2, ERR_R_BN_LIB);
311337
goto done;
312338
}
313339

314-
if (BN_cmp(rk, order) == 0)
340+
if (CRYPTO_memcmp(bn_get_words(k), bn_get_words(tmp),
341+
bn_get_top(order) * sizeof(BN_ULONG))
342+
== 0)
315343
continue;
316344

345+
/*
346+
* s = ((1 + dA)^-1 * (k - r * dA)) mod order
347+
*
348+
* Computed with fixed-top / Montgomery constant-time primitives, so
349+
* that the running time does not depend on the secret k or dA (the
350+
* generic BN_mod_mul()/BN_sub() used previously reduce via BN_div(),
351+
* whose timing is value dependent). This mirrors the ECDSA path.
352+
*
353+
* s holds (1 + dA)^-1 throughout; the (k - r * dA) term is built in
354+
* tmp. bn_mul_mont_fixed_top() with one operand in the Montgomery
355+
* domain yields the plain product, and the final
356+
* BN_mod_mul_montgomery() returns the user-visible, normalised value.
357+
*/
317358
if (!BN_add(s, dA, BN_value_one())
318359
|| !ossl_ec_group_do_inverse_ord(group, s, s, ctx)
319-
|| !BN_mod_mul(tmp, dA, r, order, ctx)
320-
|| !BN_sub(tmp, k, tmp)
321-
|| !BN_mod_mul(s, s, tmp, order, ctx)) {
360+
|| !bn_to_mont_fixed_top(tmp, r, mont, ctx)
361+
|| !bn_mul_mont_fixed_top(tmp, tmp, dA, mont, ctx)
362+
|| !bn_mod_sub_fixed_top(tmp, k, tmp, order)
363+
|| !bn_to_mont_fixed_top(tmp, tmp, mont, ctx)
364+
|| !BN_mod_mul_montgomery(s, tmp, s, mont, ctx)) {
322365
ERR_raise(ERR_LIB_SM2, ERR_R_BN_LIB);
323366
goto done;
324367
}

0 commit comments

Comments
 (0)