From 8d261c0f28bd1fae98c7c771b7e24d8619e34db9 Mon Sep 17 00:00:00 2001 From: Igor Ustinov Date: Fri, 31 Jul 2026 17:18:41 +0200 Subject: [PATCH 1/2] 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 Assisted-by: Claude:claude-opus-4-8 Reviewed-by: Alicja Kario Reviewed-by: Viktor Dukhovni Merge-date: Tue Sep 29 11:26:40 2026 --- crypto/ec/ec_mult.c | 7 +++-- crypto/sm2/sm2_sign.c | 61 ++++++++++++++++++++++++++++++++++++------- 2 files changed, 55 insertions(+), 13 deletions(-) diff --git a/crypto/ec/ec_mult.c b/crypto/ec/ec_mult.c index 94208a7c6a..d38368c403 100644 --- a/crypto/ec/ec_mult.c +++ b/crypto/ec/ec_mult.c @@ -220,10 +220,9 @@ int ossl_ec_scalar_mul_ladder(const EC_GROUP *group, EC_POINT *r, * expected to arrive either BN_FLG_CONSTTIME or fixed-top, so that their * top is a public, value-independent width and the copy length does not * leak their magnitude. ECDSA satisfies this via - * ossl_bn_priv_rand_range_fixed_top(); the generic SM2 signing path does - * not yet (see the sm2_sig_gen() hardening tracked separately). The - * fixed-top pinning below makes the subsequent arithmetic constant time - * regardless, but cannot retroactively fix the copy length here. + * ossl_bn_priv_rand_range_fixed_top(). The fixed-top pinning below makes + * the subsequent arithmetic constant time regardless, but cannot + * retroactively fix the copy length here. */ if (!BN_copy(k, scalar)) { ERR_raise(ERR_LIB_EC, ERR_R_BN_LIB); diff --git a/crypto/sm2/sm2_sign.c b/crypto/sm2/sm2_sign.c index 5157abc9b8..40aa7ba51d 100644 --- a/crypto/sm2/sm2_sign.c +++ b/crypto/sm2/sm2_sign.c @@ -14,6 +14,7 @@ #include "crypto/sm2.h" #include "crypto/sm2err.h" #include "crypto/ec.h" /* ossl_ec_group_do_inverse_ord() */ +#include "crypto/bn.h" /* fixed-top / Montgomery constant-time BN helpers */ #include "internal/numbers.h" #include #include @@ -213,17 +214,22 @@ static ECDSA_SIG *sm2_sig_gen(const EC_KEY *key, const BIGNUM *e) EC_POINT *kG = NULL; BN_CTX *ctx = NULL; BIGNUM *k = NULL; - BIGNUM *rk = NULL; BIGNUM *r = NULL; BIGNUM *s = NULL; BIGNUM *x1 = NULL; BIGNUM *tmp = NULL; + BN_MONT_CTX *mont = EC_GROUP_get_mont_data(group); OSSL_LIB_CTX *libctx = ossl_ec_key_get_libctx(key); if (dA == NULL) { ERR_raise(ERR_LIB_SM2, SM2_R_INVALID_PRIVATE_KEY); goto done; } + + if (mont == NULL) { + ERR_raise(ERR_LIB_SM2, ERR_R_EC_LIB); + goto done; + } kG = EC_POINT_new(group); if (kG == NULL) { ERR_raise(ERR_LIB_SM2, ERR_R_EC_LIB); @@ -237,7 +243,6 @@ static ECDSA_SIG *sm2_sig_gen(const EC_KEY *key, const BIGNUM *e) BN_CTX_start(ctx); k = BN_CTX_get(ctx); - rk = BN_CTX_get(ctx); x1 = BN_CTX_get(ctx); tmp = BN_CTX_get(ctx); if (tmp == NULL) { @@ -271,6 +276,18 @@ static ECDSA_SIG *sm2_sig_gen(const EC_KEY *key, const BIGNUM *e) ERR_raise(ERR_LIB_SM2, ERR_R_INTERNAL_ERROR); goto done; } + /* + * Pin the nonce to a fixed, value-independent width and flag it + * BN_FLG_CONSTTIME, so its magnitude does not leak through operand + * lengths in the scalar copy inside the ladder or in the arithmetic + * below. BN_priv_rand_range_ex() is kept so the nonce value itself + * is unchanged; only its representation is pinned. + */ + BN_set_flags(k, BN_FLG_CONSTTIME); + if (!bn_set_top_fixed(k, bn_get_top(order))) { + ERR_raise(ERR_LIB_SM2, ERR_R_BN_LIB); + goto done; + } if (!EC_POINT_mul(group, kG, k, NULL, NULL, ctx) || !EC_POINT_get_affine_coordinates(group, kG, x1, NULL, @@ -280,23 +297,49 @@ static ECDSA_SIG *sm2_sig_gen(const EC_KEY *key, const BIGNUM *e) goto done; } - /* try again if r == 0 or r+k == n */ + /* try again if r == 0 or r + k == n */ if (BN_is_zero(r)) continue; - if (!BN_add(rk, r, k)) { - ERR_raise(ERR_LIB_SM2, ERR_R_INTERNAL_ERROR); + /* + * Since 0 < r < n and 0 < k < n, r + k == n is the same as + * k == n - r. Both operands of the subtraction are public, so + * compute it in the open and then compare against the nonce with a + * fixed-width constant-time comparison. A BN_cmp() on r + k would + * branch on whether the sum carried into an extra word, which + * depends on the value of k. + */ + if (!BN_sub(tmp, order, r) + || !bn_set_top_fixed(tmp, bn_get_top(order))) { + ERR_raise(ERR_LIB_SM2, ERR_R_BN_LIB); goto done; } - if (BN_cmp(rk, order) == 0) + if (CRYPTO_memcmp(bn_get_words(k), bn_get_words(tmp), + bn_get_top(order) * sizeof(BN_ULONG)) + == 0) continue; + /* + * s = ((1 + dA)^-1 * (k - r * dA)) mod order + * + * Computed with fixed-top / Montgomery constant-time primitives, so + * that the running time does not depend on the secret k or dA (the + * generic BN_mod_mul()/BN_sub() used previously reduce via BN_div(), + * whose timing is value dependent). This mirrors the ECDSA path. + * + * s holds (1 + dA)^-1 throughout; the (k - r * dA) term is built in + * tmp. bn_mul_mont_fixed_top() with one operand in the Montgomery + * domain yields the plain product, and the final + * BN_mod_mul_montgomery() returns the user-visible, normalised value. + */ if (!BN_add(s, dA, BN_value_one()) || !ossl_ec_group_do_inverse_ord(group, s, s, ctx) - || !BN_mod_mul(tmp, dA, r, order, ctx) - || !BN_sub(tmp, k, tmp) - || !BN_mod_mul(s, s, tmp, order, ctx)) { + || !bn_to_mont_fixed_top(tmp, r, mont, ctx) + || !bn_mul_mont_fixed_top(tmp, tmp, dA, mont, ctx) + || !bn_mod_sub_fixed_top(tmp, k, tmp, order) + || !bn_to_mont_fixed_top(tmp, tmp, mont, ctx) + || !BN_mod_mul_montgomery(s, tmp, s, mont, ctx)) { ERR_raise(ERR_LIB_SM2, ERR_R_BN_LIB); goto done; } -- 2.55.0 From 179bd4ca1eb4951cc503e64a5451f1d5538b1837 Mon Sep 17 00:00:00 2001 From: Lukas Gerlach Date: Sun, 5 May 2024 14:27:26 +0300 Subject: [PATCH 2/2] include/internal/constant_time.h: add value_barrier_bn() A build breakage has been introduced on openssl-3.0 by commits 38d6c2d5fd6c "Add value_barrier in MOD_EXP_CTIME_COPY_FROM_PREBUF", 0c306f0ad0a8 "Add value_barrier in BN_consttime_swap", and a50aa72aba6c "Apply suggestion from @andrewkdinh" due to missing value_barrier_bn(), originally introduced in commit aaa1bda7187c "Optimizated calculation of shared power of 2 in bn_gcd". Cherry-pick the part of the commit that contains the function definition. Fixes: 38d6c2d5fd6c "Add value_barrier in MOD_EXP_CTIME_COPY_FROM_PREBUF" Fixes: 0c306f0ad0a8 "Add value_barrier in BN_consttime_swap" Fixes: a50aa72aba6c "Apply suggestion from @andrewkdinh" References: aaa1bda7187c "Optimizated calculation of shared power of 2 in bn_gcd" Signed-off-by: Eugene Syromiatnikov Reviewed-by: Milan Broz Reviewed-by: Norbert Pocs MergeDate: Thu Jul 30 12:17:44 2026 (Merged from https://github.com/openssl/openssl/pull/32118) --- include/internal/constant_time.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/include/internal/constant_time.h b/include/internal/constant_time.h index 3a634c5617..0f8540bc74 100644 --- a/include/internal/constant_time.h +++ b/include/internal/constant_time.h @@ -139,6 +139,17 @@ static ossl_inline uint64_t constant_time_lt_64(uint64_t a, uint64_t b) } #ifdef BN_ULONG +static ossl_inline BN_ULONG value_barrier_bn(BN_ULONG a) +{ +#if !defined(OPENSSL_NO_ASM) && defined(__GNUC__) + BN_ULONG r; + __asm__("" : "=r"(r) : "0"(a)); +#else + volatile BN_ULONG r = a; +#endif + return r; +} + static ossl_inline BN_ULONG constant_time_msb_bn(BN_ULONG a) { return 0 - (a >> (sizeof(a) * 8 - 1)); -- 2.55.0