Fix for ecc_check_privkey_gen with KCAPI. Fix KCAPI ECDSA to ensure we don't leak handle for multiple sign/verify calls.

wolfcrypt/src/ecc.c

@@ -8604,10 +8604,6 @@ static int ecc_check_privkey_gen(ecc_key* key, mp_int* a, mp_int* prime)
     int        err;
     ecc_point* base = NULL;
     ecc_point* res  = NULL;
-#ifdef WOLFSSL_KCAPI_ECC
-    ecc_key pubKey;
-    word32 len;
-#endif
 #ifdef WOLFSSL_NO_MALLOC
     ecc_point lcl_base;
     ecc_point lcl_res;
@@ -8672,7 +8668,27 @@ static int ecc_check_privkey_gen(ecc_key* key, mp_int* a, mp_int* prime)
         if (err == MP_OKAY)
             err = mp_set(base->z, 1);
 
-#ifndef WOLFSSL_KCAPI_ECC
+#ifdef WOLFSSL_KCAPI_ECC
+        if (err == MP_OKAY) {
+            byte   pubkey_raw[MAX_ECC_BYTES * 2];
+            word32 pubkey_sz = (word32)sizeof(pubkey_raw);
+
+            err = KcapiEcc_LoadKey(key, pubkey_raw, &pubkey_sz);
+            if (err == 0) {
+                err = mp_read_unsigned_bin(res->x, pubkey_raw,
+                                           pubkey_sz/2);
+            }
+            if (err == MP_OKAY) {
+                err = mp_read_unsigned_bin(res->y, pubkey_raw + pubkey_sz/2,
+                                           pubkey_sz/2);
+            }
+            if (err == MP_OKAY) {
+                err = mp_set(res->z, 1);
+            }
+        }
+        (void)a;
+        (void)prime;
+#else
 #ifdef ECC_TIMING_RESISTANT
         if (err == MP_OKAY)
             err = wc_ecc_mulmod_ex2(&key->k, base, res, a, prime, curve->order,
@@ -8682,29 +8698,10 @@ static int ecc_check_privkey_gen(ecc_key* key, mp_int* a, mp_int* prime)
             err = wc_ecc_mulmod_ex2(&key->k, base, res, a, prime, curve->order,
                                                             NULL, 1, key->heap);
 #endif
-#else
+#endif /* WOLFSSL_KCAPI_ECC */
-        /* Using Shared Secret to perform scalar multiplication
-         * Calculates the x ordinidate only.
-         */
-        if (err == MP_OKAY) {
-            err = wc_ecc_init(&pubKey);
-            if (err == MP_OKAY) {
-                wc_ecc_copy_point(base, &pubKey.pubkey);
-            }
-            if (err == MP_OKAY) {
-                err = KcapiEcc_SetPubKey(&pubKey);
-            }
-            if (err == MP_OKAY) {
-                len = key->dp->size;
-                err = KcapiEcc_SharedSecret(key, &pubKey, pubKey.pubkey_raw,
-                                                                          &len);
-            }
-        }
-#endif /* !WOLFSSL_KCAPI_ECC */
     }
 
     if (err == MP_OKAY) {
-#ifndef WOLFSSL_KCAPI_ECC
         /* compare result to public key */
         if (mp_cmp(res->x, key->pubkey.x) != MP_EQ ||
             mp_cmp(res->y, key->pubkey.y) != MP_EQ ||
@@ -8712,17 +8709,6 @@ static int ecc_check_privkey_gen(ecc_key* key, mp_int* a, mp_int* prime)
             /* didn't match */
             err = ECC_PRIV_KEY_E;
         }
-#else
-        err = mp_read_unsigned_bin(pubKey.pubkey.x, pubKey.pubkey_raw, len);
-        /* compare result to public key - y can be negative! */
-        if (err == MP_OKAY && ((!mp_isone(res->z)) ||
-            mp_cmp(pubKey.pubkey.x, key->pubkey.x) != MP_EQ)) {
-            /* didn't match */
-            err = ECC_PRIV_KEY_E;
-        }
-        (void)a;
-        (void)prime;
-#endif
     }
 
     wc_ecc_curve_free(curve);

wolfcrypt/src/port/kcapi/kcapi_ecc.c

@@ -34,6 +34,9 @@
 #include <wolfssl/wolfcrypt/port/kcapi/kcapi_ecc.h>
 #include <wolfssl/wolfcrypt/ecc.h>
 
+#ifndef ECC_CURVE_NIST_P256
+#define ECC_CURVE_NIST_P256     2
+#endif
 #ifndef ECC_CURVE_NIST_P384
 #define ECC_CURVE_NIST_P384     3
 #endif
@@ -79,70 +82,96 @@ static int KcapiEcc_CurveId(int curve_id, word32* kcapiCurveId)
      return ret;
 }
 
-int KcapiEcc_MakeKey(ecc_key* key, int keysize, int curve_id)
+int KcapiEcc_LoadKey(ecc_key* key, byte* pubkey_raw, word32* pubkey_sz)
 {
     int ret = 0;
-    int sz = 0;
+    word32 kcapiCurveId = 0;
-    word32 kcapiCurveId;
 
-    if (curve_id == ECC_CURVE_DEF) {
+    if (key == NULL || key->dp == NULL) {
-        switch (keysize) {
+        ret = BAD_FUNC_ARG;
-            case 32:
-                curve_id = ECC_SECP256R1;
-                break;
-            case 48:
-                curve_id = ECC_SECP384R1;
-                break;
-            case 66:
-                curve_id = ECC_SECP521R1;
-                break;
-            default:
-                ret = BAD_FUNC_ARG;
-                break;
-        }
-    }
-    if (ret == 0) {
-        ret = KcapiEcc_CurveId(curve_id, &kcapiCurveId);
     }
 
-    if (key->handle != NULL) {
-        kcapi_kpp_destroy(key->handle);
-        key->handle = NULL;
-    }
     if (ret == 0) {
+        ret = KcapiEcc_CurveId(key->dp->id, &kcapiCurveId);
+    }
+
+    /* if handle doesn't exist create one */
+    if (ret == 0 && key->handle == NULL) {
         ret = kcapi_kpp_init(&key->handle, WC_NAME_ECDH, 0);
-        if (ret != 0) {
+        if (ret == 0) {
-            WOLFSSL_MSG("KcapiEcc_MakeKey: Failed to initialize");
+            ret = kcapi_kpp_ecdh_setcurve(key->handle, kcapiCurveId);
+            if (ret >= 0) {
+                ret = 0;
+            }
         }
     }
-    if (ret == 0) {
+
-        ret = kcapi_kpp_ecdh_setcurve(key->handle, kcapiCurveId);
+    /* if a private key value is set, load and use it.
+     * otherwise use existing key->handle */
+    if (ret == 0 && mp_iszero(&key->k) != MP_YES) {
+        byte priv[MAX_ECC_BYTES];
+        word32 keySz = key->dp->size;
+        ret = wc_export_int(&key->k, priv, &keySz, keySz, WC_TYPE_UNSIGNED_BIN);
+        if (ret == 0) {
+            ret = kcapi_kpp_setkey(key->handle, priv, keySz);
+            if (ret >= 0) {
+                ret = 0;
+            }
+        }
     }
-    if (ret == 0) {
+
-        ret = kcapi_kpp_setkey(key->handle, NULL, 0);
+    /* optionally load public key */
+    if (ret == 0 && pubkey_raw != NULL && pubkey_sz != NULL) {
+        ret = (int)kcapi_kpp_keygen(key->handle, pubkey_raw, *pubkey_sz,
+            KCAPI_ACCESS_HEURISTIC);
         if (ret >= 0) {
+            *pubkey_sz = ret;
             ret = 0;
         }
     }
-    if (ret == 0) {
+
-        sz = ret = (int)kcapi_kpp_keygen(key->handle, key->pubkey_raw,
+    return ret;    
-                               sizeof(key->pubkey_raw), KCAPI_ACCESS_HEURISTIC);
+}
+
+int KcapiEcc_MakeKey(ecc_key* key, int keysize, int curve_id)
+{
+    int ret = 0;
+    word32 pubkey_sz = (word32)sizeof(key->pubkey_raw);
+
+    /* free existing handle */
+    if (key != NULL && key->handle != NULL) {
+        kcapi_kpp_destroy(key->handle);
+        key->handle = NULL;
     }
-    if (ret >= 0) {
+
-        ret = mp_read_unsigned_bin(key->pubkey.x, key->pubkey_raw, sz / 2);
+    /* check arguments */
+    if (key == NULL || key->dp == NULL) {
+        ret = BAD_FUNC_ARG;
+    }
+
+    ret = KcapiEcc_LoadKey(key, key->pubkey_raw, &pubkey_sz);
+    if (ret == 0) {
+        ret = mp_read_unsigned_bin(key->pubkey.x,
+            key->pubkey_raw, pubkey_sz / 2);
     }
     if (ret == 0) {
-        ret = mp_read_unsigned_bin(key->pubkey.y, key->pubkey_raw + sz / 2,
+        ret = mp_read_unsigned_bin(key->pubkey.y,
-                                   sz / 2);
+            key->pubkey_raw + pubkey_sz / 2, pubkey_sz / 2);
     }
     if (ret == 0) {
         key->type = ECC_PRIVATEKEY;
     }
-    if ((ret != 0) && (key->handle != NULL)) {
+
+    /* if error release handle now */
+    if (ret != 0 && key->handle != NULL) {
         kcapi_kpp_destroy(key->handle);
         key->handle = NULL;
     }
 
+    /* These are not used. The key->dp is set */
+    (void)keysize;
+    (void)curve_id;
+
     return ret;
 }
 
@@ -163,11 +192,14 @@ int KcapiEcc_SharedSecret(ecc_key* private_key, ecc_key* public_key, byte* out,
         ret = kcapi_kpp_init(&private_key->handle, WC_NAME_ECDH, 0);
         if (ret == 0) {
             ret = kcapi_kpp_ecdh_setcurve(private_key->handle, kcapiCurveId);
+            if (ret >= 0) {
+                ret = 0;
+            }
         }
     }
 
     /* if a private key value is set, load and use it */
-    if (mp_iszero(&private_key->k) != MP_YES) {
+    if (ret == 0 && mp_iszero(&private_key->k) != MP_YES) {
         byte priv[MAX_ECC_BYTES];
         word32 keySz = private_key->dp->size;
         ret = wc_export_int(&private_key->k, priv, &keySz, keySz,
@@ -254,6 +286,7 @@ int KcapiEcc_Sign(ecc_key* key, const byte* hash, word32 hashLen, byte* sig,
     byte* hash_aligned = NULL;
     byte* sig_aligned = NULL;
     size_t pageSz = (size_t)sysconf(_SC_PAGESIZE);
+    int handleInit = 0;
 
     if (key->handle == NULL) {
         ret = kcapi_akcipher_init(&key->handle, WC_NAME_ECDSA, 0);
@@ -261,6 +294,7 @@ int KcapiEcc_Sign(ecc_key* key, const byte* hash, word32 hashLen, byte* sig,
             WOLFSSL_MSG("KcapiEcc_Sign: Failed to initialize");
         }
         if (ret == 0) {
+            handleInit = 1;
             ret = KcapiEcc_SetPrivKey(key);
         }
     }
@@ -282,8 +316,7 @@ int KcapiEcc_Sign(ecc_key* key, const byte* hash, word32 hashLen, byte* sig,
             XMEMCPY(hash_aligned, hash, hashLen);
         }
         ret = (int)kcapi_akcipher_sign(key->handle, hash_aligned, hashLen,
-                                       sig_aligned, *sigLen,
+            sig_aligned, *sigLen, KCAPI_ACCESS_HEURISTIC);
-                                       KCAPI_ACCESS_HEURISTIC);
         if (ret >= 0) {
             *sigLen = ret;
             ret = 0;
@@ -298,6 +331,11 @@ int KcapiEcc_Sign(ecc_key* key, const byte* hash, word32 hashLen, byte* sig,
         free(buf_aligned);
     }
 
+    if (handleInit) {
+        kcapi_kpp_destroy(key->handle);
+        key->handle = NULL;
+    }
+
     return ret;
 }
 #endif
@@ -330,17 +368,18 @@ int KcapiEcc_Verify(ecc_key* key, const byte* hash, word32 hashLen, byte* sig,
     int ret = 0;
     byte* sigHash_aligned = NULL;
     size_t pageSz = (size_t)sysconf(_SC_PAGESIZE);
+    int handleInit = 0;
 
     if (key->handle == NULL) {
         ret = kcapi_akcipher_init(&key->handle, WC_NAME_ECDSA, 0);
         if (ret != 0) {
             WOLFSSL_MSG("KcapiEcc_Verify: Failed to initialize");
         }
+        if (ret == 0) {
+            handleInit = 1;
+            ret = KcapiEcc_SetPubKey(key);
+        }
     }
-    if (ret == 0) {
-        ret = KcapiEcc_SetPubKey(key);
-    }
-
     if (ret == 0) {
         ret = posix_memalign((void*)&sigHash_aligned, pageSz, sigLen + hashLen);
         if (ret < 0) {
@@ -363,6 +402,11 @@ int KcapiEcc_Verify(ecc_key* key, const byte* hash, word32 hashLen, byte* sig,
     if (sigHash_aligned != NULL) {
         free(sigHash_aligned);
     }
+    
+    if (handleInit) {
+        kcapi_kpp_destroy(key->handle);
+        key->handle = NULL;
+    }
     return ret;
 }
 #endif

wolfssl/wolfcrypt/port/kcapi/kcapi_ecc.h

@@ -36,16 +36,18 @@
     #define WC_ECCKEY_TYPE_DEFINED
 #endif
 
-int KcapiEcc_SetPubKey(ecc_key* key);
+WOLFSSL_LOCAL int KcapiEcc_SetPubKey(ecc_key* key);
 
-void KcapiEcc_Free(ecc_key* key);
+WOLFSSL_LOCAL void KcapiEcc_Free(ecc_key* key);
-int KcapiEcc_MakeKey(ecc_key* key, int keysize, int curve_id);
+WOLFSSL_LOCAL int KcapiEcc_MakeKey(ecc_key* key, int keysize, int curve_id);
-int KcapiEcc_SharedSecret(ecc_key* private_key, ecc_key* public_key, byte* out,
+WOLFSSL_LOCAL int KcapiEcc_LoadKey(ecc_key* key, byte* pubkey_raw,
-                          word32* outlen);
+                  word32* pubkey_sz);
-int KcapiEcc_Sign(ecc_key* key, const byte* hash, word32 hashLen, byte* sig,
+WOLFSSL_LOCAL int KcapiEcc_SharedSecret(ecc_key* private_key,
-                  word32* sigLen);
+                  ecc_key* public_key, byte* out, word32* outlen);
-int KcapiEcc_Verify(ecc_key* key, const byte* hash, word32 hashLen, byte* sig,
+WOLFSSL_LOCAL int KcapiEcc_Sign(ecc_key* key, const byte* hash, word32 hashLen,
-                  word32 sigLen);
+                  byte* sig, word32* sigLen);
+WOLFSSL_LOCAL int KcapiEcc_Verify(ecc_key* key, const byte* hash, word32 hashLen,
+                  byte* sig, word32 sigLen);
 
 #endif /* WOLF_CRYPT_KCAPI_ECC_H  */