diff --git a/src/sign-verify/clu_sign.c b/src/sign-verify/clu_sign.c index 6699d850..db4e2e4e 100644 --- a/src/sign-verify/clu_sign.c +++ b/src/sign-verify/clu_sign.c @@ -205,11 +205,23 @@ int wolfCLU_sign_data_rsa(byte* data, char* out, word32 dataSz, char* privKey, } } if (ret == 0) { - XFSEEK(privKeyFile, 0, SEEK_END); + if (XFSEEK(privKeyFile, 0, SEEK_END) != 0) { + wolfCLU_LogError("Failed to seek to end of file."); + ret = WOLFCLU_FATAL_ERROR; + } + } + if (ret == 0) { privFileSz = (int)XFTELL(privKeyFile); - keyBuf = (byte*)XMALLOC(privFileSz+1, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - if (keyBuf == NULL) { - ret = MEMORY_E; + if (privFileSz > 0 && privFileSz <= (RSA_MAX_SIZE / 8 * 16)) { + keyBuf = (byte*)XMALLOC(privFileSz+1, HEAP_HINT, + DYNAMIC_TYPE_TMP_BUFFER); + if (keyBuf == NULL) { + ret = MEMORY_E; + } + } + else { + wolfCLU_LogError("Incorrect private key file size: %d", privFileSz); + ret = WOLFCLU_FATAL_ERROR; } } if (ret == 0) { @@ -268,7 +280,9 @@ int wolfCLU_sign_data_rsa(byte* data, char* out, word32 dataSz, char* privKey, ret = BAD_FUNC_ARG; } else { - XFWRITE(outBuf, 1, outBufSz, s); + if ((int)XFWRITE(outBuf, 1, outBufSz, s) <= 0) { + ret = OUTPUT_FILE_ERROR; + } XFCLOSE(s); } } @@ -344,11 +358,23 @@ int wolfCLU_sign_data_ecc(byte* data, char* out, word32 fSz, char* privKey, } } if (ret == 0) { - XFSEEK(privKeyFile, 0, SEEK_END); + if (XFSEEK(privKeyFile, 0, SEEK_END) != 0) { + wolfCLU_LogError("Failed to seek to end of file."); + ret = WOLFCLU_FATAL_ERROR; + } + } + if (ret == 0) { privFileSz = (int)XFTELL(privKeyFile); - keyBuf = (byte*)XMALLOC(privFileSz+1, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - if (keyBuf == NULL) { - ret = MEMORY_E; + if (privFileSz > 0 && privFileSz <= (MAX_ECC_BITS_NEEDED / 8 * 16)) { + keyBuf = (byte*)XMALLOC(privFileSz+1, HEAP_HINT, + DYNAMIC_TYPE_TMP_BUFFER); + if (keyBuf == NULL) { + ret = MEMORY_E; + } + } + else { + wolfCLU_LogError("Incorrect private key file size: %d", privFileSz); + ret = WOLFCLU_FATAL_ERROR; } } if (ret == 0) { @@ -431,7 +457,9 @@ int wolfCLU_sign_data_ecc(byte* data, char* out, word32 fSz, char* privKey, ret = BAD_FUNC_ARG; } else { - XFWRITE(outBuf, 1, outLen, s); + if ((int)XFWRITE(outBuf, 1, outLen, s) <= 0) { + ret = OUTPUT_FILE_ERROR; + } XFCLOSE(s); } } @@ -506,11 +534,23 @@ int wolfCLU_sign_data_ed25519 (byte* data, char* out, word32 fSz, char* privKey, } } if (ret == 0) { - XFSEEK(privKeyFile, 0, SEEK_END); + if (XFSEEK(privKeyFile, 0, SEEK_END) != 0) { + wolfCLU_LogError("Failed to seek to end of file."); + ret = WOLFCLU_FATAL_ERROR; + } + } + if (ret == 0) { privFileSz = (int)XFTELL(privKeyFile); - keyBuf = (byte*)XMALLOC(privFileSz+1, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - if (keyBuf == NULL) { - ret = MEMORY_E; + if (privFileSz > 0 && privFileSz <= (ED25519_PRV_KEY_SIZE * 16)) { + keyBuf = (byte*)XMALLOC(privFileSz+1, HEAP_HINT, + DYNAMIC_TYPE_TMP_BUFFER); + if (keyBuf == NULL) { + ret = MEMORY_E; + } + } + else { + wolfCLU_LogError("Incorrect private key file size: %d", privFileSz); + ret = WOLFCLU_FATAL_ERROR; } } if (ret == 0) { @@ -582,7 +622,9 @@ int wolfCLU_sign_data_ed25519 (byte* data, char* out, word32 fSz, char* privKey, ret = BAD_FUNC_ARG; } else { - XFWRITE(outBuf, 1, outLen, s); + if ((int)XFWRITE(outBuf, 1, outLen, s) <= 0) { + ret = OUTPUT_FILE_ERROR; + } XFCLOSE(s); } } @@ -618,16 +660,18 @@ int wolfCLU_sign_data_dilithium (byte* data, char* out, word32 dataSz, char* pri { #ifdef HAVE_DILITHIUM int ret = 0; + int privFileSz = 0; + word32 index = 0; + XFILE privKeyFile = NULL; byte* privBuf = NULL; - int privFileSz = 0; + word32 privBufSz = 0; - word32 index = 0; byte* outBuf = NULL; word32 outBufSz = 0; WC_RNG rng; - XMEMSET(&rng, 0, sizeof(rng)); + #ifdef WOLFSSL_SMALL_STACK dilithium_key* key; @@ -641,6 +685,7 @@ int wolfCLU_sign_data_dilithium (byte* data, char* out, word32 dataSz, char* pri #endif /* zero before init for defensive security */ + XMEMSET(&rng, 0, sizeof(rng)); XMEMSET(key, 0, sizeof(dilithium_key)); /* init the dilithium key */ @@ -650,146 +695,137 @@ int wolfCLU_sign_data_dilithium (byte* data, char* out, word32 dataSz, char* pri #ifdef WOLFSSL_SMALL_STACK XFREE(key, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); #endif - return WOLFCLU_FAILURE; + return ret; } - if (wc_InitRng(&rng) != 0) { - wolfCLU_LogError("Failed to initialize rng.\nRET: %d", ret); - wc_dilithium_free(key); - #ifdef WOLFSSL_SMALL_STACK - XFREE(key, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - #endif - return WOLFCLU_FAILURE; + /* initialize RNG */ + if (ret == 0) { + ret = wc_InitRng(&rng); + if (ret != 0) { + wolfCLU_LogError("Failed to initialize rng.\nRET: %d", ret); + } } /* open and read private key */ - privKeyFile = XFOPEN(privKey, "rb"); - if (privKeyFile == NULL) { - wolfCLU_LogError("Faild to open Private key FILE."); - wc_FreeRng(&rng); - wc_dilithium_free(key); - #ifdef WOLFSSL_SMALL_STACK - XFREE(key, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - #endif - return WOLFCLU_FATAL_ERROR; + if (ret == 0) { + privKeyFile = XFOPEN(privKey, "rb"); + if (privKeyFile == NULL) { + wolfCLU_LogError("Failed to open Private key FILE."); + ret = BAD_FUNC_ARG; + } } - - XFSEEK(privKeyFile, 0, SEEK_END); - privFileSz = (int)XFTELL(privKeyFile); - privBuf = (byte*)XMALLOC(privFileSz+1, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - if (privBuf == NULL) { - XFCLOSE(privKeyFile); - wc_FreeRng(&rng); - wc_dilithium_free(key); - #ifdef WOLFSSL_SMALL_STACK - XFREE(key, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - #endif - return MEMORY_E; + if (ret == 0) { + if (XFSEEK(privKeyFile, 0, SEEK_END) != 0) { + wolfCLU_LogError("Failed to seek to end of file."); + ret = WOLFCLU_FATAL_ERROR; + } + if (ret == 0) { + privFileSz = (int)XFTELL(privKeyFile); + if (privFileSz > 0 && + privFileSz <= DILITHIUM_MAX_BOTH_KEY_PEM_SIZE) { + privBuf = (byte*)XMALLOC(privFileSz+1, HEAP_HINT, + DYNAMIC_TYPE_TMP_BUFFER); + if (privBuf == NULL) { + ret = MEMORY_E; + } + } else { + wolfCLU_LogError("Incorrect private key file size: %d", + privFileSz); + ret = WOLFCLU_FATAL_ERROR; + } + } } - - XMEMSET(privBuf, 0, privFileSz+1); - privBufSz = privFileSz; - XFSEEK(privKeyFile, 0, SEEK_SET); - if (XFREAD(privBuf, 1, privBufSz, privKeyFile) != privBufSz) { - wolfCLU_Log(WOLFCLU_L0, "incorecct size: %d", privFileSz); - XFREE(privBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - wc_FreeRng(&rng); - wc_dilithium_free(key); - #ifdef WOLFSSL_SMALL_STACK - XFREE(key, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - #endif - return WOLFCLU_FATAL_ERROR; + if (ret == 0) { + XMEMSET(privBuf, 0, privFileSz+1); + privBufSz = privFileSz; + if (XFSEEK(privKeyFile, 0, SEEK_SET) != 0 || + (int)XFREAD(privBuf, 1, privFileSz, privKeyFile) != privFileSz) { + wolfCLU_LogError("Failed to read private key file."); + ret = WOLFCLU_FATAL_ERROR; + } } - XFCLOSE(privKeyFile); /* convert PEM to DER if necessary */ - if (inForm == PEM_FORM) { + if (inForm == PEM_FORM && ret == 0) { ret = wolfCLU_KeyPemToDer(&privBuf, privFileSz, 0); if (ret < 0) { wolfCLU_LogError("Failed to convert PEM to DER.\nRET: %d", ret); - XFREE(privBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - wc_FreeRng(&rng); - wc_dilithium_free(key); - #ifdef WOLFSSL_SMALL_STACK - XFREE(key, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - #endif - return ret; } else { - privFileSz = ret; + /* update privBuf and privFileSz with the converted DER data */ + privBufSz = privFileSz = ret; + ret = 0; } } - /* retrieving private key and staoring in the Dilithium key */ - ret = wc_Dilithium_PrivateKeyDecode(privBuf, &index, key, privBufSz); - XFREE(privBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - if (ret != 0) { - wolfCLU_LogError("Failed to decode private key.\nRET: %d", ret); - wc_FreeRng(&rng); - wc_dilithium_free(key); - #ifdef WOLFSSL_SMALL_STACK - XFREE(key, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - #endif - return ret; + /* retrieving private key and storing in the Dilithium key */ + if (ret == 0) { + ret = wc_Dilithium_PrivateKeyDecode(privBuf, &index, key, privBufSz); + if (ret != 0) { + wolfCLU_LogError("Failed to decode private key.\nRET: %d", ret); + } } - /* malloc signature buffer */ - outBufSz = wc_dilithium_sig_size(key); - outBuf = (byte*)XMALLOC(outBufSz, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - if (outBuf == NULL) { - wc_FreeRng(&rng); - wc_dilithium_free(key); - #ifdef WOLFSSL_SMALL_STACK - XFREE(key, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - #endif - return MEMORY_E; + if (ret == 0) { + outBufSz = wc_dilithium_sig_size(key); + outBuf = (byte*)XMALLOC(outBufSz, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + if (outBuf == NULL) { + ret = MEMORY_E; + wolfCLU_LogError("Failed to allocate signature" + " buffer.\nRET: %d", ret); + } } - /* sign the message usign Dilithium private key. Note that the context is * empty. This is for interoperability. */ - ret = wc_dilithium_sign_ctx_msg(NULL, 0, data, dataSz, outBuf, &outBufSz, - key, &rng); - if (ret != 0) { - wolfCLU_LogError("Failed to sign data with Dilithium private key.\nRET: %d", ret); - XFREE(outBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - wc_FreeRng(&rng); - wc_dilithium_free(key); - #ifdef WOLFSSL_SMALL_STACK - XFREE(key, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - #endif - return ret; + if (ret == 0) { + ret = wc_dilithium_sign_ctx_msg(NULL, 0, data, dataSz, outBuf, + &outBufSz, key, &rng); + if (ret != 0) { + wolfCLU_LogError("Failed to sign data with" + " Dilithium private key.\nRET: %d", ret); + } } - else { + + if (ret == 0) { XFILE outFile; outFile = XFOPEN(out, "wb"); + if (outFile == NULL) { wolfCLU_LogError("Failed to open output file %s", out); - XFREE(outBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - wc_FreeRng(&rng); - wc_dilithium_free(key); - #ifdef WOLFSSL_SMALL_STACK - XFREE(key, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - #endif - return BAD_FUNC_ARG; - } - XFWRITE(outBuf, 1, outBufSz, outFile); - XFCLOSE(outFile); + ret = BAD_FUNC_ARG; + } else { + if ((int)XFWRITE(outBuf, 1, outBufSz, outFile) <= 0) { + ret = OUTPUT_FILE_ERROR; + } + XFCLOSE(outFile); + } } - XFREE(outBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); - wc_FreeRng(&rng); - wc_dilithium_free(key); + /* cleanup allocated resources */ + if (privKeyFile != NULL) + XFCLOSE(privKeyFile); + if (privBuf != NULL) { + XFREE(privBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + } + + if (outBuf != NULL) { + XFREE(outBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + } + + wc_dilithium_free(key); + wc_FreeRng(&rng); #ifdef WOLFSSL_SMALL_STACK XFREE(key, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); #endif - return WOLFCLU_SUCCESS; + /* expected ret == WOLFCLU_SUCCESS */ + return (ret >= 0) ? WOLFCLU_SUCCESS : ret; #else (void)data; (void)out; (void)dataSz; - (void) privKey; + (void)privKey; (void)inForm; return NOT_COMPILED_IN; diff --git a/src/sign-verify/clu_verify.c b/src/sign-verify/clu_verify.c index b9391716..11a25f20 100644 --- a/src/sign-verify/clu_verify.c +++ b/src/sign-verify/clu_verify.c @@ -450,13 +450,16 @@ int wolfCLU_verify_signature_rsa(byte* sig, char* out, int sigSz, char* keyPath, /* write the output to the specified file */ if (ret > 0) { + int writeSz = ret; XFILE s = XFOPEN(out, "wb"); if (s == NULL) { wolfCLU_LogError("Unable to open file %s", out); ret = BAD_FUNC_ARG; } else { - XFWRITE(outBuf, 1, ret, s); + if ((int)XFWRITE(outBuf, 1, writeSz, s) <= 0) { + ret = OUTPUT_FILE_ERROR; + } XFCLOSE(s); } } @@ -814,7 +817,7 @@ int wolfCLU_verify_signature_dilithium(byte* sig, int sigSz, byte* msg, /* open and read public key */ keyFile = XFOPEN(keyPath, "rb"); if (keyFile == NULL) { - wolfCLU_LogError("Faild to open Private key FILE."); + wolfCLU_LogError("Failed to open public key FILE."); wc_dilithium_free(key); #ifdef WOLFSSL_SMALL_STACK XFREE(key, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); @@ -951,7 +954,7 @@ int wolfCLU_verify_signature_xmss(byte* sig, int sigSz, keyFile = XFOPEN(pubKey, "rb"); if (keyFile == NULL) { ret = OUTPUT_FILE_ERROR; - wolfCLU_LogError("Faild to open Public key FILE."); + wolfCLU_LogError("Failed to open Public key FILE."); } } @@ -1107,7 +1110,7 @@ int wolfCLU_verify_signature_xmssmt(byte* sig, int sigSz, keyFile = XFOPEN(pubKey, "rb"); if (keyFile == NULL) { ret = OUTPUT_FILE_ERROR; - wolfCLU_LogError("Faild to open Public key FILE."); + wolfCLU_LogError("Failed to open Public key FILE."); } } diff --git a/tests/genkey_sign_ver/genkey-sign-ver-test.py b/tests/genkey_sign_ver/genkey-sign-ver-test.py index eb4b856e..0757cb03 100644 --- a/tests/genkey_sign_ver/genkey-sign-ver-test.py +++ b/tests/genkey_sign_ver/genkey-sign-ver-test.py @@ -337,6 +337,37 @@ def test_sign_nonexistent_key_fails(self): "Dilithium sign with nonexistent key should have " "failed") + def test_sign_with_pub_key_fails(self): + """Signing with a public key instead of private key must fail gracefully.""" + priv, pub = self._genkey("dilithium", "mldsakey", "der", ["-level", "2"]) + bad_sig = "mldsa_bad_pubkey.sig" + self._track(bad_sig) + r = run_wolfssl("-dilithium", "-sign", "-inkey", pub, + "-inform", "der", "-in", self.SIGN_FILE, + "-out", bad_sig) + self.assertNotEqual(r.returncode, 0, + "dilithium sign with public key should have failed") + self.assertFalse(os.path.exists(bad_sig), + "output file must not be created when signing with " + "public key") + + def test_sign_corrupted_key_fails(self): + """Signing with a corrupted private key must fail gracefully.""" + corrupt_key = "mldsakey_corrupt.priv" + bad_sig = "mldsa_bad_corrupt.sig" + self._track(corrupt_key, bad_sig) + with open(corrupt_key, "wb") as f: + f.write(b"INVALID KEY DATA") + r = run_wolfssl("-dilithium", "-sign", "-inkey", corrupt_key, + "-inform", "der", "-in", self.SIGN_FILE, + "-out", bad_sig) + self.assertNotEqual(r.returncode, 0, + "dilithium sign with corrupted key should have " + "failed") + self.assertFalse(os.path.exists(bad_sig), + "output file must not be created when signing with " + "corrupted key") + @unittest.skipUnless(_has_algorithm("xmss"), "xmss not available") class XmssTest(_GenkeySignVerifyBase):