Merge pull request #10302 from JacobBarthelmeh/ecc

additional sanity checks on invalid input

Author: dgarske

.github/workflows/os-check.yml

@@ -106,6 +106,7 @@ jobs:
           'CPPFLAGS=-DNO_WOLFSSL_CLIENT',
           'CPPFLAGS=-DNO_WOLFSSL_SERVER',
           '--enable-lms=small,verify-only --enable-xmss=small,verify-only',
+          '--enable-opensslall --enable-ecc CPPFLAGS="-DWC_ALLOW_ECC_ZERO_HASH"',
           '--enable-curve25519=nonblock --enable-ecc=nonblock --enable-sp=yes,nonblock CPPFLAGS="-DWOLFSSL_PUBLIC_MP -DWOLFSSL_DEBUG_NONBLOCK"',
           '--enable-certreq --enable-certext --enable-certgen --disable-secure-renegotiation-info CPPFLAGS="-DNO_TLS"',
           # Minimal DTLS 1.3 client-only build. The SHA-224/384/512/3

.wolfssl_known_macro_extras

@@ -623,6 +623,7 @@ USS_API
 WC_AESXTS_STREAM_NO_REQUEST_ACCOUNTING
 WC_AES_BS_WORD_SIZE
 WC_AES_GCM_DEC_AUTH_EARLY
+WC_ALLOW_ECC_ZERO_HASH
 WC_ASN_HASH_SHA256
 WC_ASN_RUNTIME_DATE_CHECK_CONTROL
 WC_ASYNC_ENABLE_ECC_KEYGEN

tests/api/test_evp_cipher.c

@@ -2195,7 +2195,7 @@ int test_wolfssl_EVP_sm4_ecb(void)
     };
     byte cipherText[sizeof(plainText) + SM4_BLOCK_SIZE];
     byte decryptedText[sizeof(plainText) + SM4_BLOCK_SIZE];
-    EVP_CIPHER_CTX* ctx;
+    EVP_CIPHER_CTX* ctx = NULL;
     int outSz;
 
     XMEMSET(key, 0, sizeof(key));
@@ -2251,7 +2251,7 @@ int test_wolfssl_EVP_sm4_cbc(void)
     };
     byte cipherText[sizeof(plainText) + SM4_BLOCK_SIZE];
     byte decryptedText[sizeof(plainText) + SM4_BLOCK_SIZE];
-    EVP_CIPHER_CTX* ctx;
+    EVP_CIPHER_CTX* ctx = NULL;
     int outSz;
 
     XMEMSET(key, 0, sizeof(key));
@@ -2319,7 +2319,7 @@ int test_wolfssl_EVP_sm4_ctr(void)
     byte plainText[] = {0xDE, 0xAD, 0xBE, 0xEF};
     byte cipherText[sizeof(plainText)];
     byte decryptedText[sizeof(plainText)];
-    EVP_CIPHER_CTX* ctx;
+    EVP_CIPHER_CTX* ctx = NULL;
     int outSz;
 
     XMEMSET(key, 0, sizeof(key));

tests/api/test_evp_pkey.c

@@ -1592,9 +1592,25 @@ static int test_wolfSSL_EVP_PKEY_sign_verify(int keyType)
     ExpectIntEQ(EVP_PKEY_verify(
         ctx_verify, sig, siglen, hash, SHA256_DIGEST_LENGTH),
         WOLFSSL_SUCCESS);
-    ExpectIntEQ(EVP_PKEY_verify(
-        ctx_verify, sig, siglen, zero, SHA256_DIGEST_LENGTH),
-        WC_NO_ERR_TRACE(WOLFSSL_FAILURE));
+
+    if (keyType == EVP_PKEY_EC) {
+#ifdef WC_TEST_NO_ECC_SIGN_VERIFY_ZERO_DIGEST
+        /* wolfSSL differs from OpenSSL in that it treats a hash of all 0's as a
+         * fatal error and does not attempt to verify */
+        ExpectIntEQ(EVP_PKEY_verify(
+            ctx_verify, sig, siglen, zero, SHA256_DIGEST_LENGTH),
+            WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR));
+#else
+        ExpectIntEQ(EVP_PKEY_verify(
+            ctx_verify, sig, siglen, zero, SHA256_DIGEST_LENGTH),
+            WC_NO_ERR_TRACE(WOLFSSL_FAILURE));
+#endif
+    }
+    else {
+        ExpectIntEQ(EVP_PKEY_verify(
+            ctx_verify, sig, siglen, zero, SHA256_DIGEST_LENGTH),
+            WC_NO_ERR_TRACE(WOLFSSL_FAILURE));
+    }
 
 #if defined(OPENSSL_EXTRA) && !defined(NO_RSA) && defined(WOLFSSL_KEY_GEN) && \
     !defined(HAVE_SELFTEST)

wolfcrypt/src/ecc.c

@@ -7272,6 +7272,10 @@ int wc_ecc_sign_hash_ex(const byte* in, word32 inlen, WC_RNG* rng,
                      ecc_key* key, mp_int *r, mp_int *s)
 {
    int    err = 0;
+#ifndef WC_ALLOW_ECC_ZERO_HASH
+   byte hashIsZero = 0;
+   word32 zIdx;
+#endif
 #if !defined(WOLFSSL_SP_MATH)
    mp_int* e;
 #if !defined(WOLFSSL_ASYNC_CRYPT) || !defined(HAVE_CAVIUM_V)
@@ -7298,6 +7302,14 @@ int wc_ecc_sign_hash_ex(const byte* in, word32 inlen, WC_RNG* rng,
        return BAD_LENGTH_E;
    }
 
+#ifndef WC_ALLOW_ECC_ZERO_HASH
+   /* reject all 0's hash */
+   for (zIdx = 0; zIdx < inlen; zIdx++)
+       hashIsZero |= in[zIdx];
+   if (hashIsZero == 0)
+       return ECC_BAD_ARG_E;
+#endif
+
    /* is this a private key? */
    if (key->type != ECC_PRIVATEKEY && key->type != ECC_PRIVATEKEY_ONLY) {
       return ECC_BAD_ARG_E;
@@ -9274,6 +9286,10 @@ int wc_ecc_verify_hash_ex(mp_int *r, mp_int *s, const byte* hash,
 #else
    int           err;
    word32        keySz = 0;
+#ifndef WC_ALLOW_ECC_ZERO_HASH
+   byte hashIsZero = 0;
+   word32 zIdx;
+#endif
 #if defined(WOLFSSL_ATECC508A) || defined(WOLFSSL_ATECC608A)
    byte sigRS[ATECC_KEY_SIZE*2];
 #elif defined(WOLFSSL_CRYPTOCELL)
@@ -9303,6 +9319,14 @@ int wc_ecc_verify_hash_ex(mp_int *r, mp_int *s, const byte* hash,
         return BAD_LENGTH_E;
     }
 
+#ifndef WC_ALLOW_ECC_ZERO_HASH
+    /* reject all 0's hash */
+    for (zIdx = 0; zIdx < hashlen; zIdx++)
+        hashIsZero |= hash[zIdx];
+    if (hashIsZero == 0)
+        return ECC_BAD_ARG_E;
+#endif
+
    /* default to invalid signature */
    *res = 0;
 

wolfcrypt/src/port/atmel/atmel.c

@@ -469,6 +469,16 @@ int atmel_ecc_create_key(int slotId, byte* peerKey)
 int atmel_ecc_sign(int slotId, const byte* message, byte* signature)
 {
     int ret;
+#ifndef WC_ALLOW_ECC_ZERO_HASH
+    byte hashIsZero = 0;
+    word32 zIdx;
+
+    /* defensive sanity check on all 0's hash */
+    for (zIdx = 0; zIdx < ATECC_KEY_SIZE; zIdx++)
+        hashIsZero |= message[zIdx];
+    if (hashIsZero == 0)
+        return ECC_BAD_ARG_E;
+#endif
 
     ret = atcab_sign(slotId, message, signature);
     ret = atmel_ecc_translate_err(ret);

wolfcrypt/src/port/nxp/se050_port.c

@@ -2113,6 +2113,10 @@ int se050_ecc_sign_hash_ex(const byte* in, word32 inLen, MATH_INT_T* r, MATH_INT
     size_t sigSz = sizeof(sigBuf);
     word32 rLen = 0;
     word32 sLen = 0;
+#ifndef WC_ALLOW_ECC_ZERO_HASH
+    byte hashIsZero = 0;
+    word32 zIdx;
+#endif
 
 #ifdef SE050_DEBUG
     printf("se050_ecc_sign_hash_ex: key %p, in %p (%d), out %p (%d), "
@@ -2124,6 +2128,15 @@ int se050_ecc_sign_hash_ex(const byte* in, word32 inLen, MATH_INT_T* r, MATH_INT
         return BAD_FUNC_ARG;
     }
 
+#ifndef WC_ALLOW_ECC_ZERO_HASH
+    /* SE050 hardware does not reject all-zero digests; mirror the
+     * software path's check so behavior is consistent. */
+    for (zIdx = 0; zIdx < inLen; zIdx++)
+        hashIsZero |= in[zIdx];
+    if (hashIsZero == 0)
+        return ECC_BAD_ARG_E;
+#endif
+
     if (cfg_se050_i2c_pi == NULL) {
         return WC_HW_E;
     }

wolfcrypt/test/test.c

@@ -36876,9 +36876,12 @@ static wc_test_ret_t ecc_test_curve_size(WC_RNG* rng, int keySize, int testVerif
 #if !defined(ECC_TIMING_RESISTANT) || (defined(ECC_TIMING_RESISTANT) && \
     !defined(WC_NO_RNG) && !defined(WOLFSSL_KCAPI_ECC))
 #ifdef HAVE_ECC_SIGN
-    /* some hardware doesn't support sign/verify of all zero digest */
-#if !defined(WC_TEST_NO_ECC_SIGN_VERIFY_ZERO_DIGEST)
-    /* test DSA sign hash with zeros */
+    /* WC_TEST_NO_ECC_SIGN_VERIFY_ZERO_DIGEST: build rejects all-zero digest,
+     * so test expects failure. */
+#ifdef WOLFSSL_SM2
+    if (curve_id != ECC_SM2P256V1)
+#endif
+    {
     for (i = 0; i < (int)ECC_DIGEST_SIZE; i++) {
         digest[i] = 0;
     }
@@ -36892,29 +36895,47 @@ static wc_test_ret_t ecc_test_curve_size(WC_RNG* rng, int keySize, int testVerif
             ret = wc_ecc_sign_hash(digest, ECC_DIGEST_SIZE, sig, &x, rng,
                                                                         userA);
     } while (ret == WC_NO_ERR_TRACE(WC_PENDING_E));
+#ifdef WC_TEST_NO_ECC_SIGN_VERIFY_ZERO_DIGEST
+    if (ret == 0) {
+        ERROR_OUT(WC_TEST_RET_ENC_EC(ret), done);
+    }
+    else {
+        ret = 0;
+    }
+#else
     if (ret != 0)
         ERROR_OUT(WC_TEST_RET_ENC_EC(ret), done);
+#endif
     TEST_SLEEP();
 
 #ifdef HAVE_ECC_VERIFY
-    for (i=0; i<testVerifyCount; i++) {
-        verify = 0;
-        do {
-        #if defined(WOLFSSL_ASYNC_CRYPT)
-            ret = wc_AsyncWait(ret, &userA->asyncDev, WC_ASYNC_FLAG_CALL_AGAIN);
-        #endif
-            if (ret == 0)
-                ret = wc_ecc_verify_hash(sig, x, digest, ECC_DIGEST_SIZE,
-                                                               &verify, userA);
-        } while (ret == WC_NO_ERR_TRACE(WC_PENDING_E));
-        if (ret != 0)
-            ERROR_OUT(WC_TEST_RET_ENC_EC(ret), done);
-        if (verify != 1)
-            ERROR_OUT(WC_TEST_RET_ENC_NC, done);
-        TEST_SLEEP();
+    verify = 0;
+    do {
+    #if defined(WOLFSSL_ASYNC_CRYPT)
+        ret = wc_AsyncWait(ret, &userA->asyncDev, WC_ASYNC_FLAG_CALL_AGAIN);
+    #endif
+        if (ret == 0)
+            ret = wc_ecc_verify_hash(sig, x, digest, ECC_DIGEST_SIZE,
+                                                           &verify, userA);
+    } while (ret == WC_NO_ERR_TRACE(WC_PENDING_E));
+#ifdef WC_TEST_NO_ECC_SIGN_VERIFY_ZERO_DIGEST
+    if (ret == 0) {
+        ERROR_OUT(WC_TEST_RET_ENC_EC(ret), done);
     }
+    else {
+        ret = 0;
+    }
+    if (verify == 1)
+        ERROR_OUT(WC_TEST_RET_ENC_NC, done);
+#else
+    if (ret != 0)
+        ERROR_OUT(WC_TEST_RET_ENC_EC(ret), done);
+    if (verify != 1)
+        ERROR_OUT(WC_TEST_RET_ENC_NC, done);
+#endif
+    TEST_SLEEP();
 #endif /* HAVE_ECC_VERIFY */
-#endif /* !WC_TEST_NO_ECC_SIGN_VERIFY_ZERO_DIGEST */
+    }
 
     /* test DSA sign hash with sequence (0,1,2,3,4,...) */
     for (i = 0; i < (int)ECC_DIGEST_SIZE; i++) {

wolfssl/wolfcrypt/settings.h

@@ -3518,13 +3518,18 @@ extern void uITRON4_free(void *p) ;
     #undef NO_DH
 #endif
 
-/* CryptoCell defines */
-#ifdef WOLFSSL_CRYPTOCELL
-    #if defined(HAVE_ECC) && defined(HAVE_ECC_SIGN)
-        /* Don't attempt to sign/verify an all-zero digest in wolfCrypt tests */
+#ifdef HAVE_ECC
+    /* defined for all ECC non FIPS builds and for FIPS v7+ (including
+     * fips-ready/fips-dev which track the latest in-development source),
+     * unless the user explicitly opts in to allowing an all-zero digest with
+     * WC_ALLOW_ECC_ZERO_HASH or is building with HAVE_SELFTEST */
+    #if (!defined(HAVE_FIPS) || FIPS_VERSION_GT(7,0) || \
+         defined(WOLFSSL_FIPS_READY) || defined(WOLFSSL_FIPS_DEV)) && \
+        !defined(HAVE_SELFTEST) && !defined(WC_ALLOW_ECC_ZERO_HASH)
+        /* sign/verify of an all-zero digest in wolfCrypt rejected */
         #define WC_TEST_NO_ECC_SIGN_VERIFY_ZERO_DIGEST
-    #endif /* HAVE_ECC && HAVE_ECC_SIGN */
-#endif
+    #endif
+#endif /* HAVE_ECC */
 
 /* Asynchronous Crypto */
 #ifdef WOLFSSL_ASYNC_CRYPT
@@ -3551,11 +3556,6 @@ extern void uITRON4_free(void *p) ;
         #define ECC_CACHE_CURVE
     #endif
 
-    #if defined(HAVE_ECC) && defined(HAVE_ECC_SIGN)
-        /* Don't attempt to sign/verify an all-zero digest in wolfCrypt tests */
-        #define WC_TEST_NO_ECC_SIGN_VERIFY_ZERO_DIGEST
-    #endif /* HAVE_ECC && HAVE_ECC_SIGN */
-
 #endif /* WOLFSSL_ASYNC_CRYPT */
 #ifndef WC_ASYNC_DEV_SIZE
     #define WC_ASYNC_DEV_SIZE 0