From 7d83bc7764999dfd91b83b4f0815b45390422afd Mon Sep 17 00:00:00 2001
From: Igor Ustinov <igus@openssl.foundation>
Date: Tue, 28 Jul 2026 22:09:29 +0200
Subject: 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:39 2026
---
 crypto/bn/bn_intern.c | 38 ++++++++++++++++++++++++++++++++++++++
 crypto/ec/ec_mult.c   | 38 ++++++++++++++++++++++++++++++++++++++
 include/crypto/bn.h   |  1 +
 3 files changed, 77 insertions(+)

diff --git a/crypto/bn/bn_intern.c b/crypto/bn/bn_intern.c
index bd299cd1442..6cf72c01568 100644
--- a/crypto/bn/bn_intern.c
+++ b/crypto/bn/bn_intern.c
@@ -9,6 +9,7 @@
 
 #include "internal/cryptlib.h"
 #include "bn_local.h"
+#include "internal/constant_time.h"
 
 /*
  * Determine the modified width-(w+1) Non-Adjacent Form (wNAF) of 'scalar'.
@@ -152,6 +153,43 @@ void bn_set_all_zero(BIGNUM *a)
         a->d[i] = 0;
 }
 
+/*
+ * Zero-extend |a| so that it occupies exactly |words| words, flag it
+ * BN_FLG_FIXED_TOP and leave its numeric value unchanged.
+ *
+ * This is a companion to bn_correct_top(): where the latter minimises the top
+ * of a BIGNUM, this one pins the top to a caller-chosen, value-independent
+ * width.  Constant-time code uses it to make the cost of subsequent word-wise
+ * operations (e.g. BN_uadd()/BN_add()) independent of the magnitude of a
+ * secret value.  |words| must be greater than or equal to the current top.
+ *
+ * The routine is itself constant time with respect to the current a->top: it
+ * always sweeps a fixed |words| iterations and selects value-or-zero per word
+ * with an arithmetic mask, rather than looping over the (possibly secret)
+ * a->top..words range.  Masking the high words with zero also launders any
+ * uninitialised padding, so it is safe for the memory sanitiser.
+ */
+int bn_set_top_fixed(BIGNUM *a, int words)
+{
+    size_t i, n = (size_t)words;
+    BN_ULONG mask;
+
+    if (words < a->top)
+        return 0;
+    if (bn_wexpand(a, words) == NULL) {
+        ERR_raise(ERR_LIB_BN, ERR_R_BN_LIB);
+        return 0;
+    }
+    for (i = 0; i < n; i++) {
+        /* mask = all ones iff i < a->top, else all zeros */
+        mask = value_barrier_bn((BN_ULONG)0 - ((i - a->top) >> (8 * sizeof(i) - 1)));
+        a->d[i] &= mask;
+    }
+    a->top = words;
+    a->flags |= BN_FLG_FIXED_TOP;
+    return 1;
+}
+
 int bn_copy_words(BN_ULONG *out, const BIGNUM *in, int size)
 {
     if (in->top > size)
diff --git a/crypto/ec/ec_mult.c b/crypto/ec/ec_mult.c
index 18f3d47d936..94208a7c6a4 100644
--- a/crypto/ec/ec_mult.c
+++ b/crypto/ec/ec_mult.c
@@ -213,6 +213,18 @@ int ossl_ec_scalar_mul_ladder(const EC_GROUP *group, EC_POINT *r,
         goto err;
     }
 
+    /*
+     * Constant-timeness of this copy depends on the caller: BN_copy() moves
+     * scalar->top words unless |scalar| is flagged BN_FLG_CONSTTIME (in which
+     * case scalar->dmax words are moved).  Secret scalars are therefore
+     * 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.
+     */
     if (!BN_copy(k, scalar)) {
         ERR_raise(ERR_LIB_EC, ERR_R_BN_LIB);
         goto err;
@@ -231,10 +243,36 @@ int ossl_ec_scalar_mul_ladder(const EC_GROUP *group, EC_POINT *r,
         }
     }
 
+    /*
+     * |k| may still carry a top that depends on the value of the secret
+     * scalar: callers pass either a fixed-top BIGNUM (e.g. the ECDSA nonce)
+     * or a minimal-top one (e.g. SM2), and BN_copy() above preserves that
+     * top.  Pin |k| to a fixed number of words (matching the group
+     * cardinality) so that the additions below run in constant time,
+     * independently of the bit length of the scalar.  Otherwise the work
+     * done by BN_add()/BN_uadd() depends on the operand tops and leaks the
+     * magnitude of the secret scalar.
+     */
+    if (!bn_set_top_fixed(k, group_top)) {
+        ERR_raise(ERR_LIB_EC, ERR_R_BN_LIB);
+        goto err;
+    }
+
     if (!BN_add(lambda, k, cardinality)) {
         ERR_raise(ERR_LIB_EC, ERR_R_BN_LIB);
         goto err;
     }
+    /*
+     * |lambda| = scalar + cardinality may or may not have produced a carry
+     * into an extra word depending on the secret scalar.  Pin its top to one
+     * word above the group top so that the second addition, which consumes
+     * |lambda|, is likewise constant time and so that the BN_is_bit_set()
+     * below always inspects a defined word.
+     */
+    if (!bn_set_top_fixed(lambda, group_top + 1)) {
+        ERR_raise(ERR_LIB_EC, ERR_R_BN_LIB);
+        goto err;
+    }
     BN_set_flags(lambda, BN_FLG_CONSTTIME);
     if (!BN_add(k, lambda, cardinality)) {
         ERR_raise(ERR_LIB_EC, ERR_R_BN_LIB);
diff --git a/include/crypto/bn.h b/include/crypto/bn.h
index 952840be5e6..dd6fd89ec8a 100644
--- a/include/crypto/bn.h
+++ b/include/crypto/bn.h
@@ -18,6 +18,7 @@ BIGNUM *bn_wexpand(BIGNUM *a, int words);
 BIGNUM *bn_expand2(BIGNUM *a, int words);
 
 void bn_correct_top(BIGNUM *a);
+int bn_set_top_fixed(BIGNUM *a, int words);
 
 /*
  * Determine the modified width-(w+1) Non-Adjacent Form (wNAF) of 'scalar'.
-- 
cgit