ecc.c and test.c changes to add support in ecc_sign_determinsitic.c

[aidangarske]

Description

Added support for ECDSA deterministric K signing for all curves with SHA256, SHA384, and SHA512.

The tests for ecc_test_deterministic_k, ecc384_test_deterministic_k, and ecc521_test_deterministic_k were not run because FIPS_VERSION_GE was always defined.

Added wc_ecc_set_deterministic_ex to support custom hash type for deterministic sign or verify. By default it will detect hash type based on input hash size.

Feature request for ZD 14235

Testing

./configure CFLAGS="-DWOLFSSL_PUBLIC_MP -DWOLFSSL_ECDSA_DETERMINISTIC_K" --enable-debug && make
./wolfcrypt/test/testwolfcrypt 

[SparkiDev]

Need a new ECDSA signing API that will take the hash algorithm.
The original will call the new with WC_HASH_TYPE_NONE.
The new will pass the hash algorithm in.
The code to guess the hash algorithm is fine.

[dgarske]

@SparkiDev ,

I would prefer not to add sign/verify variants. What about just extending the set_derministic API to accept a hash type?

Suggested patch:

diff --git a/wolfcrypt/src/ecc.c b/wolfcrypt/src/ecc.c
index e0ce9ec20..b4e891334 100644
--- a/wolfcrypt/src/ecc.c
+++ b/wolfcrypt/src/ecc.c
@@ -6847,7 +6847,7 @@ static int deterministic_sign_helper(const byte* in, word32 inlen, ecc_key* key)
         }
         if (key->sign_k != NULL) {
             if (wc_ecc_gen_deterministic_k(in, inlen,
-                        WC_HASH_TYPE_NONE, ecc_get_k(key), key->sign_k,
+                        key->hashType, ecc_get_k(key), key->sign_k,
                         curve->order, key->heap) != 0) {
                 mp_free(key->sign_k);
                 XFREE(key->sign_k, key->heap, DYNAMIC_TYPE_ECC);
@@ -6865,7 +6865,7 @@ static int deterministic_sign_helper(const byte* in, word32 inlen, ecc_key* key)
         }
     #else
         key->sign_k_set = 0;
-        if (wc_ecc_gen_deterministic_k(in, inlen, WC_HASH_TYPE_NONE,
+        if (wc_ecc_gen_deterministic_k(in, inlen, key->hashType,
                 ecc_get_k(key), key->sign_k, curve->order, key->heap) != 0) {
             err = ECC_PRIV_KEY_E;
         }
@@ -7561,6 +7561,14 @@ int wc_ecc_gen_deterministic_k(const byte* hash, word32 hashSz,
         }
     }
 
+    /* For deterministic k only SHA2-256, SHA2-384 and SHA2-512 are supported */
+    if ( hashType != WC_HASH_TYPE_SHA256 &&
+         hashType != WC_HASH_TYPE_SHA384 &&
+         hashType != WC_HASH_TYPE_SHA512) {
+        WOLFSSL_MSG("Invalid deterministic hash type");
+        return BAD_FUNC_ARG;
+    }
+
     if (mp_unsigned_bin_size(priv) > MAX_ECC_BYTES) {
         WOLFSSL_MSG("private key larger than max expected!");
         return BAD_FUNC_ARG;
@@ -7775,15 +7783,22 @@ int wc_ecc_gen_deterministic_k(const byte* hash, word32 hashSz,
 /* Sets the deterministic flag for 'k' generation with sign.
  * returns 0 on success
  */
-int wc_ecc_set_deterministic(ecc_key* key, byte flag)
+int wc_ecc_set_deterministic_ex(ecc_key* key, byte flag, int hashType)
 {
     if (key == NULL) {
         return BAD_FUNC_ARG;
     }
 
     key->deterministic = flag ? 1 : 0;
+    key->hashType = hashType;
     return 0;
 }
+
+int wc_ecc_set_deterministic(ecc_key* key, byte flag)
+{
+    return wc_ecc_set_deterministic_ex(key, flag, WC_HASH_TYPE_NONE);
+}
+
 #endif /* end sign_ex and deterministic sign */
 
 
diff --git a/wolfssl/wolfcrypt/ecc.h b/wolfssl/wolfcrypt/ecc.h
index 4a198a6b0..aa23e2530 100644
--- a/wolfssl/wolfcrypt/ecc.h
+++ b/wolfssl/wolfcrypt/ecc.h
@@ -595,6 +595,7 @@ struct ecc_key {
 #if defined(WOLFSSL_ECDSA_DETERMINISTIC_K) || \
     defined(WOLFSSL_ECDSA_DETERMINISTIC_K_VARIANT)
     byte deterministic:1;
+    int hashType;
 #endif
 
 #if defined(WOLFSSL_SMALL_STACK_CACHE) && !defined(WOLFSSL_ECC_NO_SMALL_STACK)
@@ -719,6 +720,8 @@ int wc_ecc_sign_hash_ex(const byte* in, word32 inlen, WC_RNG* rng,
 WOLFSSL_API
 int wc_ecc_set_deterministic(ecc_key* key, byte flag);
 WOLFSSL_API
+int wc_ecc_set_deterministic_ex(ecc_key* key, byte flag, int hashType);
+WOLFSSL_API
 int wc_ecc_gen_deterministic_k(const byte* hash, word32 hashSz,
         enum wc_HashType hashType, mp_int* priv, mp_int* k, mp_int* order,
         void* heap);

[SparkiDev]

I'm happy with that change.
It makes sense to only set the hash when doing deterministic k.

[dgarske]

@SparkiDev do we need this restriction? Seems like any supported HMAC algo would be fine. As long as we properly fail later if the hash type is unsupported or invalid. Thoughts?

[SparkiDev]

Yep. We can remove this now.

[SparkiDev]

retest this please