Fix copilot, fenrir, internal review

  - Copilot: BN_bn2hex NULL guard — Added NULL check on num before calling wolfSSL_BN_bn2hex
  - Copilot: return 0 on missing args — Changed return ret to return USER_INPUT_ERROR at lines 118 and 194
  - Copilot: SHA-224 test assertion — Test now fails if sha224 is NOT found (not just if sha256 is)
  - Copilot: dilithium_init return value — Capture into ret for proper error logging
  - Security review: Missing ForceZero on keyBuf — Added ForceZero before XFREE on all keyBuf free paths in both certgen files

Author: aidangarske

src/certgen/clu_certgen_ed25519.c

@@ -62,6 +62,7 @@ int make_self_signed_ed25519_certificate(char* keyPath, char* certOut)
     }
     if (XFSEEK(keyFile, 0, SEEK_SET) != 0 || (int)XFREAD(keyBuf, 1, keyFileSz, keyFile) != keyFileSz) {
         XFCLOSE(keyFile);
+        wolfCLU_ForceZero(keyBuf, keyFileSz);
         XFREE(keyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
         return WOLFCLU_FAILURE;
     }
@@ -85,6 +86,7 @@ int make_self_signed_ed25519_certificate(char* keyPath, char* certOut)
                                         ED25519_KEY_SIZE,
                                         keyBuf + ED25519_KEY_SIZE,
                                         ED25519_KEY_SIZE, &key);
+    wolfCLU_ForceZero(keyBuf, keyFileSz);
     XFREE(keyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
     if (ret != 0 ) {
         wolfCLU_LogError("Failed to decode private key.\nRET: %d", ret);

src/certgen/clu_certgen_rsa.c

@@ -63,6 +63,7 @@ int make_self_signed_rsa_certificate(char* keyPath, char* certOut, int oid)
     }
     if (XFSEEK(keyFile, 0, SEEK_SET) != 0 || (int)XFREAD(keyBuf, 1, keyFileSz, keyFile) != keyFileSz) {
         XFCLOSE(keyFile);
+        wolfCLU_ForceZero(keyBuf, keyFileSz);
         XFREE(keyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
         return WOLFCLU_FAILURE;
     }
@@ -86,6 +87,7 @@ int make_self_signed_rsa_certificate(char* keyPath, char* certOut, int oid)
     rngInit = 1;
 
     ret = wc_RsaPrivateKeyDecode(keyBuf, &index, &key, keyFileSz);
+    wolfCLU_ForceZero(keyBuf, keyFileSz);
     XFREE(keyBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
     if (ret != 0 ) {
         wolfCLU_LogError("Failed to decode private key.\nRET: %d", ret);

src/sign-verify/clu_sign.c

@@ -612,7 +612,8 @@ int wolfCLU_sign_data_dilithium (byte* data, char* out, word32 dataSz, char* pri
     XMEMSET(key, 0, sizeof(dilithium_key));
 
     /* init the dilithium key */
-    if (wc_dilithium_init(key) != 0) {
+    ret = wc_dilithium_init(key);
+    if (ret != 0) {
         wolfCLU_LogError("Failed to initialize Dilithium Key.\nRET: %d", ret);
     #ifdef WOLFSSL_SMALL_STACK
         XFREE(key, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);

src/sign-verify/clu_sign_verify_setup.c

@@ -115,7 +115,7 @@ int wolfCLU_sign_verify_setup(int argc, char** argv)
                "signing or verifying.");
         wolfCLU_signHelp(algCheck);
         wolfCLU_verifyHelp(algCheck);
-        return ret;
+        return USER_INPUT_ERROR;
     }
 
     ret = wolfCLU_checkForArg("-inform", 7, argc, argv);
@@ -191,7 +191,7 @@ int wolfCLU_sign_verify_setup(int argc, char** argv)
             XFREE(in, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
         if (sig)
             XFREE(sig, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER);
-        return ret;
+        return USER_INPUT_ERROR;
     }
 
     ret = wolfCLU_checkForArg("-out", 4, argc, argv);

src/x509/clu_cert_setup.c

@@ -759,7 +759,12 @@ int wolfCLU_certSetup(int argc, char** argv)
                     if (rsa != NULL) {
                         wolfSSL_RSA_get0_key(rsa, &num, NULL, NULL);
                     }
-                    hex = wolfSSL_BN_bn2hex(num);
+                    if (num == NULL) {
+                        wolfCLU_LogError("Modulus=unavailable");
+                        ret = WOLFCLU_FATAL_ERROR;
+                    }
+                    hex = (num != NULL) ?
+                        wolfSSL_BN_bn2hex(num) : NULL;
 
                     if (hex != NULL) {
                         if (wolfSSL_BIO_write(out, "Modulus=", (int)XSTRLEN("Modulus="))

tests/x509/x509-req-test.sh

@@ -181,15 +181,12 @@ rm -f tmp.cert
 run_success "x509 -req -in tmp.csr -days 3650 -sha1 -signkey ./certs/server-key.pem -out tmp.cert"
 rm -f tmp.cert
 run_success "x509 -req -in tmp.csr -days 3650 -sha224 -signkey ./certs/server-key.pem -out tmp.cert"
-# Verify SHA-224 cert uses sha224 signature algorithm, not sha256
+# Verify SHA-224 cert uses sha224 signature algorithm
 SIGALG=`./wolfssl x509 -in tmp.cert -text -noout 2>&1`
 echo "$SIGALG" | grep -i "sha224"
 if [ $? -ne 0 ]; then
-    echo "$SIGALG" | grep -i "sha256"
-    if [ $? -eq 0 ]; then
-        echo "SHA-224 cert incorrectly uses SHA-256 signature algorithm"
-        exit 99
-    fi
+    echo "SHA-224 cert does not report SHA-224 signature algorithm"
+    exit 99
 fi
 rm -f tmp.cert
 run_success "x509 -req -in tmp.csr -days 3650 -sha256 -signkey ./certs/server-key.pem -out tmp.cert"