Fix private scalar handling under ECC key blinding and add CI coverage

Author: dgarske

.github/configs/os-check-linux.json

@@ -28,6 +28,10 @@
 {"name": "all-dtls13-frag-ch-no-mlkem", "minutes": 8.2,
  "configure": ["--enable-all", "--enable-dtls13", "--enable-dtls-frag-ch",
   "--disable-mlkem"]},
+{"name": "all-ecc-blind-k", "minutes": 8.0,
+ "comment": "Only entry that sets WOLFSSL_ECC_BLIND_K (the blind-private-key entry sets WOLFSSL_BLIND_PRIVATE_KEY, which does not imply it). Keeps the read-only wc_ecc_key_get_priv() contract exercised in CI. pkcs11 is on because wc_pkcs11.c is not compiled anywhere else in this matrix.",
+ "configure": ["--enable-all", "--enable-pkcs11",
+  "CPPFLAGS=-DWOLFSSL_ECC_BLIND_K"]},
 {"name": "all-check-mem-zero", "minutes": 7.9,
  "configure": ["--enable-all", "CPPFLAGS=-DWOLFSSL_CHECK_MEM_ZERO"]},
 {"name": "all-faultharden-pk-privkey", "minutes": 7.8,

src/pk_ec.c

@@ -37,7 +37,9 @@
 #endif
 #ifndef WOLFSSL_HAVE_ECC_KEY_GET_PRIV
     /* FIPS build has replaced ecc.h. */
-    #define wc_ecc_key_get_priv(key) (&((key)->k))
+    #define wc_ecc_key_get_priv(key)  (&((key)->k))
+    #define ecc_get_k_raw(key)        (&((key)->k))
+    #define ecc_blind_k_rng(key, rng) 0
     #define WOLFSSL_HAVE_ECC_KEY_GET_PRIV
 #endif
 
@@ -3161,9 +3163,15 @@ static int wolfssl_ec_key_int_copy(ecc_key* dst, const ecc_key* src)
     }
 
     if (ret == 0) {
-        /* Copy private key. */
-        ret = mp_copy(wc_ecc_key_get_priv((ecc_key*)src),
-            wc_ecc_key_get_priv(dst));
+        /* Copy the stored private scalar, and its blind where the build
+         * keeps one. The wc_ecc_key_get_priv() accessor cannot be used
+         * here: it is read-only, and reading needs dst->dp, not set yet. */
+        ret = mp_copy(ecc_get_k_raw((ecc_key*)src), ecc_get_k_raw(dst));
+    #ifdef WOLFSSL_ECC_BLIND_K
+        if (ret == MP_OKAY) {
+            ret = mp_copy(((ecc_key*)src)->kb, dst->kb);
+        }
+    #endif
         if (ret != MP_OKAY) {
             WOLFSSL_MSG("mp_copy error");
         }
@@ -4446,11 +4454,17 @@ int SetECKeyInternal(WOLFSSL_EC_KEY* eckey)
 
         /* set privkey */
         if ((ret == 1) && (eckey->priv_key != NULL)) {
+            /* Write the stored scalar, then install a fresh blind so any
+             * blind left from a previous use of this key is replaced. */
             if (wolfssl_bn_get_value(eckey->priv_key,
-                    wc_ecc_key_get_priv(key)) != 1) {
+                    ecc_get_k_raw(key)) != 1) {
                 WOLFSSL_MSG("ec key priv error");
                 ret = WOLFSSL_FATAL_ERROR;
             }
+            if ((ret == 1) && (ecc_blind_k_rng(key, NULL) != 0)) {
+                WOLFSSL_MSG("ec key priv blind error");
+                ret = WOLFSSL_FATAL_ERROR;
+            }
             /* private key */
             if ((ret == 1) && (!mp_iszero(wc_ecc_key_get_priv(key)))) {
                 if (pubSet) {

wolfcrypt/src/ecc.c

@@ -378,15 +378,33 @@ ECC Curve Sizes:
 #endif
 
 #ifdef WOLFSSL_ECC_BLIND_K
+/* Number of digits covered by the fixed-width XORs below. */
+#define ECC_BLIND_K_DIGITS(key) \
+    ((int)(((key)->dp->size + sizeof(mp_digit) - 1) / sizeof(mp_digit)))
+
+/* The XORs read this many whole digits regardless of each operand's current
+ * length, so operands written at partial width (e.g. by mp_copy()) must be
+ * zero-extended first or stale digits fold into the value. mp_grow() cannot
+ * fail for a curve-sized key; fail closed if it ever does. */
 mp_int* ecc_get_k(ecc_key* key)
 {
-    mp_xor_ct(key->k, key->kb, key->dp->size, key->ku);
+    if ((mp_grow(key->k, ECC_BLIND_K_DIGITS(key)) != MP_OKAY) ||
+        (mp_grow(key->kb, ECC_BLIND_K_DIGITS(key)) != MP_OKAY)) {
+        mp_forcezero(key->ku);
+    }
+    else {
+        mp_xor_ct(key->k, key->kb, key->dp->size, key->ku);
+    }
     return key->ku;
 }
 void ecc_blind_k(ecc_key* key, mp_int* b)
 {
-    mp_xor_ct(key->k, b, key->dp->size, key->k);
-    mp_xor_ct(key->kb, b, key->dp->size, key->kb);
+    if ((mp_grow(key->k, ECC_BLIND_K_DIGITS(key)) == MP_OKAY) &&
+        (mp_grow(key->kb, ECC_BLIND_K_DIGITS(key)) == MP_OKAY) &&
+        (mp_grow(b, ECC_BLIND_K_DIGITS(key)) == MP_OKAY)) {
+        mp_xor_ct(key->k, b, key->dp->size, key->k);
+        mp_xor_ct(key->kb, b, key->dp->size, key->kb);
+    }
 }
 int ecc_blind_k_rng(ecc_key* key, WC_RNG* rng)
 {
@@ -405,11 +423,17 @@ int ecc_blind_k_rng(ecc_key* key, WC_RNG* rng)
         }
     }
     if (ret == 0) {
-        ret = mp_rand(key->kb, (key->dp->size + sizeof(mp_digit) - 1) /
-            sizeof(mp_digit), rng);
+        ret = mp_rand(key->kb, ECC_BLIND_K_DIGITS(key), rng);
+        if (ret == 0) {
+            ret = mp_grow(key->k, ECC_BLIND_K_DIGITS(key));
+        }
         if (ret == 0) {
             mp_xor_ct(key->k, key->kb, key->dp->size, key->k);
         }
+        else {
+            /* No blind installed - keep the stored pair consistent. */
+            mp_forcezero(key->kb);
+        }
     }
 
     if (rng == &local_rng) {
@@ -418,6 +442,13 @@ int ecc_blind_k_rng(ecc_key* key, WC_RNG* rng)
     return ret;
 }
 
+void ecc_forcezero_k(ecc_key* key)
+{
+    mp_forcezero(key->k);
+    mp_forcezero(key->kb);
+    mp_forcezero(key->ku);
+}
+
 mp_int* wc_ecc_key_get_priv(ecc_key* key)
 {
     return ecc_get_k(key);

wolfcrypt/src/eccsi.c

@@ -38,7 +38,10 @@
 
 #ifndef WOLFSSL_HAVE_ECC_KEY_GET_PRIV
     /* FIPS build has replaced ecc.h. */
-    #define wc_ecc_key_get_priv(key) (&((key)->k))
+    #define wc_ecc_key_get_priv(key)  (&((key)->k))
+    #define ecc_get_k_raw(key)        (&((key)->k))
+    #define ecc_blind_k_rng(key, rng) 0
+    #define ecc_forcezero_k(key)      mp_forcezero(&((key)->k))
     #define WOLFSSL_HAVE_ECC_KEY_GET_PRIV
 #endif
 
@@ -679,8 +682,11 @@ static int eccsi_decode_key(EccsiKey* key, const byte* data)
     int err;
 
     /* Read the secret value from key size bytes. */
-    err = mp_read_unsigned_bin(wc_ecc_key_get_priv(&key->ecc), data,
+    err = mp_read_unsigned_bin(ecc_get_k_raw(&key->ecc), data,
         (word32)key->ecc.dp->size);
+    if (err == 0) {
+        err = ecc_blind_k_rng(&key->ecc, NULL);
+    }
     if (err == 0) {
         data += key->ecc.dp->size;
         /* Read public key. */
@@ -809,9 +815,12 @@ int wc_ImportEccsiPrivateKey(EccsiKey* key, const byte* data, word32 sz)
     }
 
     if (err == 0) {
-        err = mp_read_unsigned_bin(wc_ecc_key_get_priv(&key->ecc), data,
+        err = mp_read_unsigned_bin(ecc_get_k_raw(&key->ecc), data,
             (word32)key->ecc.dp->size);
     }
+    if (err == 0) {
+        err = ecc_blind_k_rng(&key->ecc, NULL);
+    }
 
     return err;
 }
@@ -926,7 +935,7 @@ static int eccsi_make_pair(EccsiKey* key, WC_RNG* rng,
     /* Step 5: ensure SSK and HS are non-zero (code lines above) */
 
     /* Step 6: Copy out SSK (done during calc) and PVT. Erase v */
-    mp_forcezero(wc_ecc_key_get_priv(&key->pubkey));
+    ecc_forcezero_k(&key->pubkey);
 
     return err;
 }
@@ -2010,10 +2019,10 @@ int wc_SignEccsiHash(EccsiKey* key, WC_RNG* rng, enum wc_HashType hashType,
     if (err == 0) {
         j = wc_ecc_key_get_priv(&key->pubkey);
         err = mp_mulmod(s, j, &key->params.order, s);
+        /* Erase j on the failure path too. */
+        ecc_forcezero_k(&key->pubkey);
     }
     if (err == 0) {
-        mp_forcezero(j);
-
         /* Step 6: s = s' fitted */
         err = eccsi_fit_to_octets(s, &key->params.order, (int)sz, s);
     }

wolfcrypt/src/port/silabs/silabs_ecc.c

@@ -40,7 +40,9 @@ static sl_se_key_descriptor_t private_device_key =
 
 #ifndef WOLFSSL_HAVE_ECC_KEY_GET_PRIV
     /* FIPS build has replaced ecc.h. */
-    #define wc_ecc_key_get_priv(key) (&((key)->k))
+    #define wc_ecc_key_get_priv(key)  (&((key)->k))
+    #define ecc_get_k_raw(key)        (&((key)->k))
+    #define ecc_blind_k_rng(key, rng) 0
     #define WOLFSSL_HAVE_ECC_KEY_GET_PRIV
 #endif
 
@@ -209,12 +211,18 @@ int silabs_ecc_make_key(ecc_key* key, int keysize)
         key->type = ECC_PRIVATEKEY;
 
         /* copy key to mp components */
-        mp_read_unsigned_bin(key->pubkey.x,
-            key->key.storage.location.buffer.pointer, keysize);
-        mp_read_unsigned_bin(key->pubkey.y,
-            key->key.storage.location.buffer.pointer  + keysize, keysize);
-        mp_read_unsigned_bin(wc_ecc_key_get_priv(key),
-            key->key.storage.location.buffer.pointer + (2 * keysize), keysize);
+        if ((mp_read_unsigned_bin(key->pubkey.x,
+                key->key.storage.location.buffer.pointer,
+                keysize) != MP_OKAY) ||
+            (mp_read_unsigned_bin(key->pubkey.y,
+                key->key.storage.location.buffer.pointer + keysize,
+                keysize) != MP_OKAY) ||
+            (mp_read_unsigned_bin(ecc_get_k_raw(key),
+                key->key.storage.location.buffer.pointer + (2 * keysize),
+                keysize) != MP_OKAY) ||
+            (ecc_blind_k_rng(key, NULL) != 0)) {
+            return WC_HW_E;
+        }
     }
 
     return (sl_stat == SL_STATUS_OK) ? 0 : WC_HW_E;

wolfcrypt/src/sakke.c

@@ -39,7 +39,9 @@
 
 #ifndef WOLFSSL_HAVE_ECC_KEY_GET_PRIV
     /* FIPS build has replaced ecc.h. */
-    #define wc_ecc_key_get_priv(key) (&((key)->k))
+    #define wc_ecc_key_get_priv(key)  (&((key)->k))
+    #define ecc_get_k_raw(key)        (&((key)->k))
+    #define ecc_blind_k_rng(key, rng) 0
     #define WOLFSSL_HAVE_ECC_KEY_GET_PRIV
 #endif
 
@@ -533,14 +535,18 @@ int wc_MakeSakkeKey(SakkeKey* key, WC_RNG* rng)
                 err = RNG_FAILURE_E;
             }
             if (err == 0) {
-                err = mp_rand(wc_ecc_key_get_priv(&key->ecc), digits, rng);
+                err = mp_rand(ecc_get_k_raw(&key->ecc), digits, rng);
             }
             if (err == 0) {
-                err = mp_mod(wc_ecc_key_get_priv(&key->ecc), &key->params.q,
-                    wc_ecc_key_get_priv(&key->ecc));
+                err = mp_mod(ecc_get_k_raw(&key->ecc), &key->params.q,
+                    ecc_get_k_raw(&key->ecc));
             }
         }
-        while ((err == 0) && mp_iszero(wc_ecc_key_get_priv(&key->ecc)));
+        while ((err == 0) && mp_iszero(ecc_get_k_raw(&key->ecc)));
+
+        if (err == 0) {
+            err = ecc_blind_k_rng(&key->ecc, rng);
+        }
     }
     if (err == 0) {
         /* Calculate public key by multiply master secret by base point. */
@@ -672,9 +678,12 @@ int wc_ImportSakkeKey(SakkeKey* key, const byte* data, word32 sz)
 
     if (err == 0) {
         /* Read the secret value from key size bytes. */
-        err = mp_read_unsigned_bin(wc_ecc_key_get_priv(&key->ecc), data,
+        err = mp_read_unsigned_bin(ecc_get_k_raw(&key->ecc), data,
             (word32)key->ecc.dp->size);
     }
+    if (err == 0) {
+        err = ecc_blind_k_rng(&key->ecc, NULL);
+    }
     if (err == 0) {
         data += key->ecc.dp->size;
         /* Read the public key point's x value from key size bytes. */
@@ -770,9 +779,12 @@ int wc_ImportSakkePrivateKey(SakkeKey* key, const byte* data, word32 sz)
 
     if (err == 0) {
         /* Read the secret value from key size bytes. */
-        err = mp_read_unsigned_bin(wc_ecc_key_get_priv(&key->ecc), data,
+        err = mp_read_unsigned_bin(ecc_get_k_raw(&key->ecc), data,
             (word32)key->ecc.dp->size);
     }
+    if (err == 0) {
+        err = ecc_blind_k_rng(&key->ecc, NULL);
+    }
 
     return err;
 }

wolfcrypt/src/wc_pkcs11.c

@@ -41,7 +41,8 @@
 
 #ifndef WOLFSSL_HAVE_ECC_KEY_GET_PRIV
     /* FIPS build has replaced ecc.h. */
-    #define wc_ecc_key_get_priv(key) (&((key)->k))
+    #define wc_ecc_key_get_priv(key)  (&((key)->k))
+    #define ecc_forcezero_k(key)      mp_forcezero(&((key)->k))
     #define WOLFSSL_HAVE_ECC_KEY_GET_PRIV
 #endif
 
@@ -2186,7 +2187,7 @@ int wc_Pkcs11StoreKey(Pkcs11Token* token, int type, int clear, void* key)
                         ret = ret2;
                 }
                 if (ret == 0 && clear)
-                    mp_forcezero(wc_ecc_key_get_priv(eccKey));
+                    ecc_forcezero_k(eccKey);
                 break;
             }
     #endif

wolfssl/wolfcrypt/ecc.h

@@ -687,15 +687,23 @@ struct ecc_key {
 #define ecc_get_k(key)              (key)->k
 #define ecc_blind_k(key, b)         (void)b
 #define ecc_blind_k_rng(key, rng)   0
+#define ecc_forcezero_k(key)        mp_forcezero((key)->k)
 
 #define wc_ecc_key_get_priv(key)    (key)->k
 #else
 mp_int* ecc_get_k(ecc_key* key);
 void ecc_blind_k(ecc_key* key, mp_int* b);
 int ecc_blind_k_rng(ecc_key* key, WC_RNG* rng);
+WOLFSSL_LOCAL void ecc_forcezero_k(ecc_key* key);
 
 WOLFSSL_API mp_int* wc_ecc_key_get_priv(ecc_key* key);
 #endif
+/* Writable handle on the stored private scalar. With blinding enabled,
+ * wc_ecc_key_get_priv() returns a value regenerated into scratch, so it is
+ * read-only: writes through it are discarded and erasing it leaves the
+ * secret in place. Write a new scalar through this instead, then install a
+ * fresh blind with ecc_blind_k_rng(); erase with ecc_forcezero_k(). */
+#define ecc_get_k_raw(key)          (key)->k
 
 #define WOLFSSL_HAVE_ECC_KEY_GET_PRIV