Add digest size checks for ECC sign/verify

Author: ColtonWilley

src/wp_ecdsa_sig.c

@@ -32,6 +32,9 @@
 
 #ifdef WP_HAVE_ECDSA
 
+/* SHA-1 digest size; literal because WC_SHA_DIGEST_SIZE is !NO_SHA-gated. */
+#define WP_ECDSA_MIN_HASH_LEN 20
+
 /**
  * ECDSA signature context.
  *
@@ -271,14 +274,20 @@ static int wp_ecdsa_sign(wp_EcdsaSigCtx *ctx, unsigned char *sig,
         *sigLen = wc_ecc_sig_size(wp_ecc_get_key(ctx->ecc));
     }
     else {
+        /* Enforce digest-size invariants the FIPS-validated
+         * wc_ecc_sign_hash boundary cannot. */
 #if LIBWOLFSSL_VERSION_HEX >= 0x05007004
-        if ((ctx->hash.type != WC_HASH_TYPE_NONE) &&
-            (tbsLen != (size_t)wc_HashGetDigestSize(ctx->hash.type)))
+        enum wc_HashType hashType = ctx->hash.type;
 #else
-        if ((ctx->hashType != WC_HASH_TYPE_NONE) &&
-            (tbsLen != (size_t)wc_HashGetDigestSize(ctx->hashType)))
+        enum wc_HashType hashType = ctx->hashType;
 #endif
-        {
+        int digestSize = wc_HashGetDigestSize(hashType);
+        if ((hashType != WC_HASH_TYPE_NONE) &&
+            ((digestSize < 0) || (tbsLen != (size_t)digestSize))) {
+            ok = 0;
+        }
+        else if ((hashType == WC_HASH_TYPE_NONE) &&
+                 (tbsLen < WP_ECDSA_MIN_HASH_LEN)) {
             ok = 0;
         }
         else if ((ok = wp_ecc_check_usage(ctx->ecc))) {
@@ -362,17 +371,35 @@ static int wp_ecdsa_verify(wp_EcdsaSigCtx *ctx, const unsigned char *sig,
         ok = 0;
     }
     else {
-        int res;
-        int rc = wc_ecc_verify_hash(sig, (word32)sigLen, tbs, (word32)tbsLen,
-            &res, wp_ecc_get_key(ctx->ecc));
-        if (rc != 0) {
-            WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_ecc_verify_hash", rc);
+        /* Enforce digest-size invariants the FIPS-validated
+         * wc_ecc_verify_hash boundary cannot. */
+#if LIBWOLFSSL_VERSION_HEX >= 0x05007004
+        enum wc_HashType hashType = ctx->hash.type;
+#else
+        enum wc_HashType hashType = ctx->hashType;
+#endif
+        int digestSize = wc_HashGetDigestSize(hashType);
+        if ((hashType != WC_HASH_TYPE_NONE) &&
+            ((digestSize < 0) || (tbsLen != (size_t)digestSize))) {
             ok = 0;
         }
-        if (res == 0) {
-            WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "Signature verification", rc);
+        else if ((hashType == WC_HASH_TYPE_NONE) &&
+                 (tbsLen < WP_ECDSA_MIN_HASH_LEN)) {
             ok = 0;
         }
+        else {
+            int res;
+            int rc = wc_ecc_verify_hash(sig, (word32)sigLen, tbs, (word32)tbsLen,
+                &res, wp_ecc_get_key(ctx->ecc));
+            if (rc != 0) {
+                WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_ecc_verify_hash", rc);
+                ok = 0;
+            }
+            else if (res == 0) {
+                WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "Signature verification", rc);
+                ok = 0;
+            }
+        }
     }
 
     WOLFPROV_LEAVE(WP_LOG_COMP_ECDSA, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__), ok);

test/test_ecc.c

@@ -990,6 +990,36 @@ static int test_pkey_verify_ecc(EVP_PKEY *pkey, OSSL_LIB_CTX* libCtx,
     return test_pkey_verify(pkey, libCtx, hash, hashLen, sig, sigLen, 0, NULL, NULL);
 }
 
+/* As test_pkey_verify_ecc, but sets signature_md before verifying. */
+static int test_pkey_verify_ecc_md(EVP_PKEY *pkey, OSSL_LIB_CTX* libCtx,
+    const EVP_MD *md, unsigned char *hash, size_t hashLen,
+    unsigned char *sig, size_t sigLen)
+{
+    int err;
+    EVP_PKEY_CTX *ctx = NULL;
+
+    err = (ctx = EVP_PKEY_CTX_new_from_pkey(libCtx, pkey, NULL)) == NULL;
+    if (err == 0) {
+        err = EVP_PKEY_verify_init(ctx) != 1;
+    }
+    if (err == 0) {
+        err = EVP_PKEY_CTX_set_signature_md(ctx, md) <= 0;
+    }
+    if (err == 0) {
+        err = EVP_PKEY_verify(ctx, sig, sigLen, hash, hashLen) != 1;
+    }
+    if (err == 0) {
+        PRINT_MSG("Signature verified");
+    }
+    else {
+        PRINT_MSG("Signature not verified");
+    }
+
+    EVP_PKEY_CTX_free(ctx);
+
+    return err;
+}
+
 #ifdef WP_HAVE_EC_P192
 int test_ecdsa_p192_pkey(void *data)
 {
@@ -1165,6 +1195,94 @@ int test_ecdsa_p256_pkey(void *data)
 
     return err;
 }
+
+/* Raw EVP_PKEY_verify must reject sub-SHA-1 inputs. */
+int test_ecdsa_verify_undersized_hash(void *data)
+{
+    static const size_t sizes[]      = { 0, 19, 20, 32 };
+    static const int    expectFail[] = { 1,  1,  0,  0 };
+    int err;
+    size_t i;
+    EVP_PKEY *pkey = NULL;
+    unsigned char ecdsaSig[80];
+    size_t ecdsaSigLen;
+    unsigned char buf[32];
+    const unsigned char *p = ecc_key_der_256;
+
+    (void)data;
+
+    err = RAND_bytes(buf, sizeof(buf)) == 0;
+    if (err == 0) {
+        pkey = d2i_PrivateKey(EVP_PKEY_EC, NULL, &p, sizeof(ecc_key_der_256));
+        err = pkey == NULL;
+    }
+
+    for (i = 0; (err == 0) && (i < sizeof(sizes)/sizeof(sizes[0])); i++) {
+        PRINT_MSG("verify tbsLen=%zu", sizes[i]);
+        ecdsaSigLen = sizeof(ecdsaSig);
+        err = test_pkey_sign_ecc(pkey, osslLibCtx, buf, sizes[i], ecdsaSig,
+                                 &ecdsaSigLen);
+        if (err == 0) {
+            int verifyErr = test_pkey_verify_ecc(pkey, wpLibCtx, buf, sizes[i],
+                                                 ecdsaSig, ecdsaSigLen);
+            err = (verifyErr != 0) != expectFail[i];
+        }
+    }
+
+    /* Same minimum applies on the sign side: wolfProvider must refuse to
+     * produce a raw signature it would later refuse to verify. */
+    for (i = 0; (err == 0) && (i < sizeof(sizes)/sizeof(sizes[0])); i++) {
+        int signErr;
+        PRINT_MSG("sign tbsLen=%zu", sizes[i]);
+        ecdsaSigLen = sizeof(ecdsaSig);
+        signErr = test_pkey_sign_ecc(pkey, wpLibCtx, buf, sizes[i], ecdsaSig,
+                                     &ecdsaSigLen);
+        err = (signErr != 0) != expectFail[i];
+    }
+
+    EVP_PKEY_free(pkey);
+
+    return err;
+}
+
+/* EVP_PKEY_verify with signature_md=SHA-256 must require tbsLen == 32. */
+int test_ecdsa_verify_md_len_mismatch(void *data)
+{
+    static const size_t sizes[]      = { 19, 31, 32, 33 };
+    static const int    expectFail[] = {  1,  1,  0,  1 };
+    int err;
+    size_t i;
+    EVP_PKEY *pkey = NULL;
+    unsigned char ecdsaSig[80];
+    size_t ecdsaSigLen;
+    unsigned char buf[64];
+    const unsigned char *p = ecc_key_der_256;
+
+    (void)data;
+
+    err = RAND_bytes(buf, sizeof(buf)) == 0;
+    if (err == 0) {
+        pkey = d2i_PrivateKey(EVP_PKEY_EC, NULL, &p, sizeof(ecc_key_der_256));
+        err = pkey == NULL;
+    }
+
+    for (i = 0; (err == 0) && (i < sizeof(sizes)/sizeof(sizes[0])); i++) {
+        PRINT_MSG("tbsLen=%zu", sizes[i]);
+        ecdsaSigLen = sizeof(ecdsaSig);
+        err = test_pkey_sign_ecc(pkey, osslLibCtx, buf, sizes[i], ecdsaSig,
+                                 &ecdsaSigLen);
+        if (err == 0) {
+            int verifyErr = test_pkey_verify_ecc_md(pkey, wpLibCtx, EVP_sha256(),
+                                                    buf, sizes[i], ecdsaSig,
+                                                    ecdsaSigLen);
+            err = (verifyErr != 0) != expectFail[i];
+        }
+    }
+
+    EVP_PKEY_free(pkey);
+
+    return err;
+}
 #endif /* WP_HAVE_EC_P256 */
 
 #ifdef WP_HAVE_EC_P384

test/unit.c

@@ -362,6 +362,8 @@ TEST_CASE test_case[] = {
     #ifdef WP_HAVE_ECDSA
         TEST_DECL(test_ecdsa_p256_pkey, NULL),
         TEST_DECL(test_ecdsa_p256, NULL),
+        TEST_DECL(test_ecdsa_verify_undersized_hash, NULL),
+        TEST_DECL(test_ecdsa_verify_md_len_mismatch, NULL),
     #endif
     TEST_DECL(test_ec_decode, NULL),
     TEST_DECL(test_ec_import, NULL),

test/unit.h

@@ -396,6 +396,8 @@ int test_ecdsa_p521_pkey(void *data);
 #ifdef WP_HAVE_EC_P256
 int test_ecdsa_p256_pkey(void *data);
 int test_ecdsa_p256(void *data);
+int test_ecdsa_verify_undersized_hash(void *data);
+int test_ecdsa_verify_md_len_mismatch(void *data);
 #endif /* WP_HAVE_EC_P256 */
 
 #ifdef WP_HAVE_EC_P384