Fix minor issues and add unit test for IdentifyAsn1Key

Author: yosuke-wolfssl

src/internal.c

@@ -1501,6 +1501,8 @@ int IdentifyAsn1Key(const byte* in, word32 inSz, int isPrivate, void* heap,
                 /* If decode was successful, this is an RSA key. */
                 if (ret == 0) {
                     key->keyId = ID_SSH_RSA;
+                } else {
+                    wc_FreeRsaKey(&key->ks.rsa.key);
                 }
             }
         }
@@ -1533,7 +1535,12 @@ int IdentifyAsn1Key(const byte* in, word32 inSz, int isPrivate, void* heap,
                         case ECC_SECP521R1:
                             key->keyId = ID_ECDSA_SHA2_NISTP521;
                             break;
+                        default:
+                            /* Not a supported curve, so free the key */
+                            wc_ecc_free(&key->ks.ecc.key);
                     }
+                } else {
+                    wc_ecc_free(&key->ks.ecc.key);
                 }
             }
         }
@@ -1552,11 +1559,14 @@ int IdentifyAsn1Key(const byte* in, word32 inSz, int isPrivate, void* heap,
                     ret = wc_Ed25519PublicKeyDecode(in, &idx,
                             &key->ks.ed25519.key, inSz);
                 }
-            }
 
-            /* If decode was successful, this is a Ed25519 key. */
-            if (ret == 0)
-                key->keyId = ID_ED25519;
+                /* If decode was successful, this is a Ed25519 key. */
+                if (ret == 0) {
+                    key->keyId = ID_ED25519;
+                } else {
+                    wc_ed25519_free(&key->ks.ed25519.key);
+                }
+            }
         }
 #endif /* WOLFSSH_NO_ED25519 */
 
@@ -13052,7 +13062,7 @@ int SendKexDhReply(WOLFSSH* ssh)
             else if (useEcc) {
                 ret = KeyAgreeEcdh_server(ssh, hashId, f_ptr, &fSz);
             }
-            if (useCurve25519) {
+            else if (useCurve25519) {
                 ret = KeyAgreeCurve25519_server(ssh, hashId, f_ptr, &fSz);
             }
             else if (useEccMlKem) {

src/wolfsftp.c

@@ -2775,7 +2775,6 @@ static int SFTP_CreateLongName(WS_SFTPNAME* name)
 
     name->lName = (char*)WMALLOC(totalSz + 1, name->heap, DYNTYPE_SFTP);
     if (name->lName == NULL) {
-        WFREE(name->lName, name->heap, DYNTYPE_SFTP);
         return WS_MEMORY_E;
     }
     name->lSz = totalSz;
@@ -3987,6 +3986,7 @@ int wolfSSH_SFTP_RecvRead(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
     if (res != NULL) {
         if (wolfSSH_SFTP_CreateStatus(ssh, type, reqId, res, "English", NULL,
                 &outSz) != WS_SIZE_ONLY) {
+            WFREE(out, ssh->ctx->heap, DYNTYPE_BUFFER);
             return WS_FATAL_ERROR;
         }
         if (outSz > strSz) {
@@ -5282,6 +5282,7 @@ int wolfSSH_SFTP_RecvFSTAT(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
     if (ret == WS_SUCCESS) {
         if (SFTP_SetHeader(ssh, reqId, WOLFSSH_FTP_ATTRS, sz, out)
                 != WS_SUCCESS) {
+            WFREE(out, ssh->ctx->heap, DYNTYPE_BUFFER);
             return WS_FATAL_ERROR;
         }
         SFTP_SetAttributes(ssh, out + WOLFSSH_SFTP_HEADER, sz, &atr);

tests/unit.c

@@ -1485,7 +1485,7 @@ static int CaptureIoSendAuthSvc(WOLFSSH* ssh, void* buf, word32 sz, void* ctx)
  *
  * For invalid-service cases the auth-method field is intentionally omitted
  * from the payload.  DoUserAuthRequest must short-circuit at the service-name
- * check and still satisfy all three assertions — proving it never tries to
+ * check and still satisfy all three assertions - proving it never tries to
  * parse the missing auth-method field.  If the short-circuit were absent,
  * GetSize() for authNameSz would hit end-of-buffer and return WS_BUFFER_E,
  * failing assertion 1.
@@ -1750,7 +1750,120 @@ static const byte unitTestRsaPrivKey[] = {
 };
 static const word32 unitTestRsaPrivKeySz =
         (word32)sizeof(unitTestRsaPrivKey);
+#endif /* WOLFSSH_NO_RSA */
+
+/* Keys for test_IdentifyAsn1Key: inline DER so the test is self-contained
+ * in both filesystem and NO_FILESYSTEM builds.  Each array matches the
+ * corresponding file under keys/. */
+#ifndef WOLFSSH_NO_ECDSA_SHA2_NISTP256
+/* keys/server-key-ecc.der - P-256 RFC-5915 ECPrivateKey */
+static const byte unitTestEcc256PrivKey[] = {
+    0x30, 0x77, 0x02, 0x01, 0x01, 0x04, 0x20, 0x61, 0x09, 0x99,
+    0x0B, 0x79, 0xD2, 0x5F, 0x28, 0x5A, 0x0F, 0x5D, 0x15, 0xCC,
+    0xA1, 0x56, 0x54, 0xF9, 0x2B, 0x39, 0x87, 0x21, 0x2D, 0xA7,
+    0x7D, 0x85, 0x7B, 0xB8, 0x7F, 0x38, 0xC6, 0x6D, 0xD5, 0xA0,
+    0x0A, 0x06, 0x08, 0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x03, 0x01,
+    0x07, 0xA1, 0x44, 0x03, 0x42, 0x00, 0x04, 0x81, 0x13, 0xFF,
+    0xA4, 0x2B, 0xB7, 0x9C, 0x45, 0x74, 0x7A, 0x83, 0x4C, 0x61,
+    0xF3, 0x3F, 0xAD, 0x26, 0xCF, 0x22, 0xCD, 0xA9, 0xA3, 0xBC,
+    0xA5, 0x61, 0xB4, 0x7C, 0xE6, 0x62, 0xD4, 0xC2, 0xF7, 0x55,
+    0x43, 0x9A, 0x31, 0xFB, 0x80, 0x11, 0x20, 0xB5, 0x12, 0x4B,
+    0x24, 0xF5, 0x78, 0xD7, 0xFD, 0x22, 0xEF, 0x46, 0x35, 0xF0,
+    0x05, 0x58, 0x6B, 0x5F, 0x63, 0xC8, 0xDA, 0x1B, 0xC4, 0xF5,
+    0x69
+};
+#endif /* WOLFSSH_NO_ECDSA_SHA2_NISTP256 */
+
+#ifndef WOLFSSH_NO_ECDSA_SHA2_NISTP384
+/* keys/server-key-ecc-384.der - P-384 RFC-5915 ECPrivateKey */
+static const byte unitTestEcc384PrivKey[] = {
+    0x30, 0x81, 0xA4, 0x02, 0x01, 0x01, 0x04, 0x30, 0x3E, 0xAD,
+    0xD2, 0xBB, 0xBF, 0x05, 0xA7, 0xBE, 0x3A, 0x3F, 0x7C, 0x28,
+    0x15, 0x12, 0x89, 0xDE, 0x5B, 0xB3, 0x64, 0x4D, 0x70, 0x11,
+    0x76, 0x1D, 0xB5, 0x6F, 0x2A, 0x03, 0x62, 0xFB, 0xA6, 0x4F,
+    0x98, 0xE6, 0x4F, 0xF9, 0x86, 0xDC, 0x4F, 0xB8, 0xEF, 0xDB,
+    0x2D, 0x6B, 0x8D, 0xA5, 0x71, 0x42, 0xA0, 0x07, 0x06, 0x05,
+    0x2B, 0x81, 0x04, 0x00, 0x22, 0xA1, 0x64, 0x03, 0x62, 0x00,
+    0x04, 0x38, 0xD6, 0x2B, 0xE4, 0x18, 0xFF, 0x57, 0x3F, 0xD0,
+    0xE0, 0x20, 0xD4, 0x88, 0x76, 0xC4, 0xE1, 0x12, 0x1D, 0xFB,
+    0x2D, 0x6E, 0xBE, 0xE4, 0x89, 0x5D, 0x77, 0x24, 0x31, 0x6D,
+    0x46, 0xA2, 0x31, 0x05, 0x87, 0x3F, 0x29, 0x86, 0xD5, 0xC7,
+    0x12, 0x80, 0x3A, 0x6F, 0x47, 0x1A, 0xB8, 0x68, 0x50, 0xEB,
+    0x06, 0x3E, 0x10, 0x89, 0x61, 0x34, 0x9C, 0xF8, 0xB4, 0xC6,
+    0xA4, 0xCF, 0x5E, 0x97, 0xBD, 0x7E, 0x51, 0xE9, 0x75, 0xE3,
+    0xE9, 0x21, 0x72, 0x61, 0x50, 0x6E, 0xB9, 0xCF, 0x3C, 0x49,
+    0x3D, 0x3E, 0xB8, 0x8D, 0x46, 0x7B, 0x5F, 0x27, 0xEB, 0xAB,
+    0x21, 0x61, 0xC0, 0x00, 0x66, 0xFE, 0xBD
+};
+#endif /* WOLFSSH_NO_ECDSA_SHA2_NISTP384 */
+
+#ifndef WOLFSSH_NO_ECDSA_SHA2_NISTP521
+/* keys/server-key-ecc-521.der - P-521 RFC-5915 ECPrivateKey */
+static const byte unitTestEcc521PrivKey[] = {
+    0x30, 0x81, 0xDC, 0x02, 0x01, 0x01, 0x04, 0x42, 0x00, 0x4C,
+    0xA4, 0xD8, 0x64, 0x28, 0xD9, 0x40, 0x0E, 0x7B, 0x2D, 0xF3,
+    0x91, 0x2E, 0xB9, 0x96, 0xC1, 0x95, 0x89, 0x50, 0x43, 0xAF,
+    0x92, 0xE8, 0x6D, 0xE7, 0x0A, 0xE4, 0xDF, 0x46, 0xF2, 0x2A,
+    0x29, 0x1A, 0x6B, 0xB2, 0x74, 0x8A, 0xAE, 0x82, 0x58, 0x0D,
+    0xF6, 0xC3, 0x9F, 0x49, 0xB3, 0xED, 0x82, 0xF1, 0x78, 0x9E,
+    0xCE, 0x1B, 0x65, 0x7D, 0x45, 0x43, 0x8C, 0xFF, 0x15, 0x65,
+    0x34, 0x35, 0x45, 0x75, 0xA0, 0x07, 0x06, 0x05, 0x2B, 0x81,
+    0x04, 0x00, 0x23, 0xA1, 0x81, 0x89, 0x03, 0x81, 0x86, 0x00,
+    0x04, 0x01, 0xF8, 0xD0, 0xA7, 0xC3, 0xC5, 0x8D, 0x84, 0x19,
+    0x57, 0x96, 0x9F, 0x21, 0x3A, 0x94, 0xF3, 0xDA, 0x55, 0x0E,
+    0xDF, 0x76, 0xD8, 0xDD, 0x17, 0x15, 0x31, 0xF3, 0x5B, 0xB0,
+    0x69, 0xC8, 0xBC, 0x30, 0x0D, 0x6F, 0x6B, 0x37, 0xD1, 0x80,
+    0x46, 0xA9, 0x71, 0x7F, 0x2C, 0x6F, 0x59, 0x51, 0x9C, 0x82,
+    0x70, 0x95, 0xB2, 0x9A, 0x63, 0x13, 0x30, 0x62, 0x18, 0xC2,
+    0x35, 0x76, 0x94, 0x00, 0xD0, 0xF9, 0x6D, 0x00, 0x0A, 0x19,
+    0x3B, 0xA3, 0x46, 0x65, 0x2B, 0xEB, 0x40, 0x9A, 0x9A, 0x45,
+    0xC5, 0x97, 0xA3, 0xED, 0x93, 0x2D, 0xD5, 0xAA, 0xAE, 0x96,
+    0xBF, 0x2F, 0x31, 0x7E, 0x5A, 0x7A, 0xC7, 0x45, 0x8B, 0x3C,
+    0x6C, 0xDB, 0xAA, 0x90, 0xC3, 0x55, 0x38, 0x2C, 0xDF, 0xCD,
+    0xCA, 0x73, 0x77, 0xD9, 0x2E, 0xB2, 0x0A, 0x5E, 0x8C, 0x74,
+    0x23, 0x7C, 0xA5, 0xA3, 0x45, 0xB1, 0x9E, 0x3F, 0x1A, 0x22,
+    0x90, 0xB1, 0x54
+};
+#endif /* WOLFSSH_NO_ECDSA_SHA2_NISTP521 */
+
+#if !defined(WOLFSSH_NO_ED25519)
+/* keys/server-key-ed25519.der - Ed25519 OneAsymmetricKey (RFC 8410) */
+static const byte unitTestEd25519PrivKey[] = {
+    0x30, 0x50, 0x02, 0x01, 0x00, 0x30, 0x05, 0x06, 0x03, 0x2b,
+    0x65, 0x70, 0x04, 0x22, 0x04, 0x20, 0x6a, 0x67, 0xf3, 0x0e,
+    0x64, 0xea, 0x52, 0xfe, 0xf4, 0xad, 0x65, 0x4d, 0x45, 0x60,
+    0x61, 0x38, 0x58, 0x11, 0x07, 0x84, 0xf0, 0x03, 0x94, 0x93,
+    0x14, 0x7b, 0x7b, 0x33, 0x1a, 0xba, 0xf6, 0x19, 0x81, 0x20,
+    0x0f, 0x56, 0x0c, 0x9f, 0x7d, 0x7a, 0x62, 0x87, 0xf0, 0x26,
+    0x16, 0x19, 0x31, 0xe4, 0xb2, 0x1d, 0xe9, 0xbd, 0xee, 0x4a,
+    0x7f, 0x55, 0xae, 0x26, 0x2d, 0xa1, 0x25, 0xe4, 0xee, 0x4a,
+    0x51, 0x00
+};
+#endif /* !WOLFSSH_NO_ED25519 */
+
+#ifndef WOLFSSH_NO_ECDSA
+/* P-256 DER with the OID last byte changed 0x07 -> 0x01 (secp192r1).
+ * Forces wc_EccPrivateKeyDecode to fail or to return an unsupported curve id,
+ * exercising the wc_ecc_free cleanup in both the default: and else paths. */
+static const byte unitTestEccUnsupportedCurveKey[] = {
+    0x30, 0x77, 0x02, 0x01, 0x01, 0x04, 0x20, 0x61, 0x09, 0x99,
+    0x0B, 0x79, 0xD2, 0x5F, 0x28, 0x5A, 0x0F, 0x5D, 0x15, 0xCC,
+    0xA1, 0x56, 0x54, 0xF9, 0x2B, 0x39, 0x87, 0x21, 0x2D, 0xA7,
+    0x7D, 0x85, 0x7B, 0xB8, 0x7F, 0x38, 0xC6, 0x6D, 0xD5, 0xA0,
+    0x0A, 0x06, 0x08, 0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x03, 0x01,
+    0x01, /* 0x07 (secp256r1) changed to 0x01 (secp192r1) */
+    0xA1, 0x44, 0x03, 0x42, 0x00, 0x04, 0x81, 0x13, 0xFF,
+    0xA4, 0x2B, 0xB7, 0x9C, 0x45, 0x74, 0x7A, 0x83, 0x4C, 0x61,
+    0xF3, 0x3F, 0xAD, 0x26, 0xCF, 0x22, 0xCD, 0xA9, 0xA3, 0xBC,
+    0xA5, 0x61, 0xB4, 0x7C, 0xE6, 0x62, 0xD4, 0xC2, 0xF7, 0x55,
+    0x43, 0x9A, 0x31, 0xFB, 0x80, 0x11, 0x20, 0xB5, 0x12, 0x4B,
+    0x24, 0xF5, 0x78, 0xD7, 0xFD, 0x22, 0xEF, 0x46, 0x35, 0xF0,
+    0x05, 0x58, 0x6B, 0x5F, 0x63, 0xC8, 0xDA, 0x1B, 0xC4, 0xF5,
+    0x69
+};
+#endif /* WOLFSSH_NO_ECDSA */
 
+#ifndef WOLFSSH_NO_RSA
 /* wolfSSH_RsaVerify unit test
  *
  * Verifies that wolfSSH_RsaVerify returns WS_RSA_E when given a signature
@@ -2034,6 +2147,98 @@ static int test_DoUserAuthRequestEd25519(void)
 
 #endif /* Ed25519 verify test guards */
 
+/* IdentifyAsn1Key unit test
+ *
+ * Exercises every new wc_Free* error-path added in IdentifyAsn1Key:
+ *  - wc_FreeRsaKey on RSA decode failure
+ *  - wc_ecc_free on ECC decode failure
+ *  - wc_ecc_free in the default: branch (unsupported curve)
+ *  - wc_ed25519_free on Ed25519 decode failure
+ * Each happy-path call implicitly exercises the failure-path frees for the
+ * other key types.
+ */
+static int test_IdentifyAsn1Key(void)
+{
+    int result = 0;
+    int ret;
+
+#ifndef WOLFSSH_NO_RSA
+    ret = IdentifyAsn1Key(unitTestRsaPrivKey, unitTestRsaPrivKeySz,
+                          1, NULL, NULL);
+    if (ret != ID_SSH_RSA) {
+        printf("IdentifyAsn1Key: RSA priv failed, ret=%d\n", ret);
+        result = -600; goto done;
+    }
+#endif
+
+#ifndef WOLFSSH_NO_ECDSA_SHA2_NISTP256
+    ret = IdentifyAsn1Key(unitTestEcc256PrivKey,
+                          (word32)sizeof(unitTestEcc256PrivKey),
+                          1, NULL, NULL);
+    if (ret != ID_ECDSA_SHA2_NISTP256) {
+        printf("IdentifyAsn1Key: ECC P-256 priv failed, ret=%d\n", ret);
+        result = -601; goto done;
+    }
+#endif
+
+#ifndef WOLFSSH_NO_ECDSA_SHA2_NISTP384
+    ret = IdentifyAsn1Key(unitTestEcc384PrivKey,
+                          (word32)sizeof(unitTestEcc384PrivKey),
+                          1, NULL, NULL);
+    if (ret != ID_ECDSA_SHA2_NISTP384) {
+        printf("IdentifyAsn1Key: ECC P-384 priv failed, ret=%d\n", ret);
+        result = -602; goto done;
+    }
+#endif
+
+#ifndef WOLFSSH_NO_ECDSA_SHA2_NISTP521
+    ret = IdentifyAsn1Key(unitTestEcc521PrivKey,
+                          (word32)sizeof(unitTestEcc521PrivKey),
+                          1, NULL, NULL);
+    if (ret != ID_ECDSA_SHA2_NISTP521) {
+        printf("IdentifyAsn1Key: ECC P-521 priv failed, ret=%d\n", ret);
+        result = -603; goto done;
+    }
+#endif
+
+#if !defined(WOLFSSH_NO_ED25519)
+    ret = IdentifyAsn1Key(unitTestEd25519PrivKey,
+                          (word32)sizeof(unitTestEd25519PrivKey),
+                          1, NULL, NULL);
+    if (ret != ID_ED25519) {
+        printf("IdentifyAsn1Key: Ed25519 priv failed, ret=%d\n", ret);
+        result = -604; goto done;
+    }
+#endif
+
+    /* Unsupported ECC curve: triggers wc_ecc_free in the default: branch
+     * (wolfSSL has P-192) or the else branch (wolfSSL lacks P-192).
+     * Either way the key must be freed and WS_UNIMPLEMENTED_E returned. */
+#ifndef WOLFSSH_NO_ECDSA
+    ret = IdentifyAsn1Key(unitTestEccUnsupportedCurveKey,
+                          (word32)sizeof(unitTestEccUnsupportedCurveKey),
+                          1, NULL, NULL);
+    if (ret != WS_UNIMPLEMENTED_E) {
+        printf("IdentifyAsn1Key: unsupported ECC curve expected "
+               "WS_UNIMPLEMENTED_E, got %d\n", ret);
+        result = -605; goto done;
+    }
+#endif
+
+    /* Garbage: all decode attempts fail, all wc_Free* cleanup paths hit */
+    {
+        static const byte garbage[] = {0x00, 0x01, 0x02, 0x03};
+        ret = IdentifyAsn1Key(garbage, (word32)sizeof(garbage), 1, NULL, NULL);
+        if (ret != WS_UNIMPLEMENTED_E) {
+            printf("IdentifyAsn1Key: garbage expected WS_UNIMPLEMENTED_E, "
+                   "got %d\n", ret);
+            result = -606; goto done;
+        }
+    }
+
+done:
+    return result;
+}
 #endif /* WOLFSSH_TEST_INTERNAL */
 
 /* Error Code And Message Test */
@@ -2165,6 +2370,10 @@ int wolfSSH_UnitTest(int argc, char** argv)
            (unitResult == 0 ? "SUCCESS" : "FAILED"));
     testResult = testResult || unitResult;
 
+    unitResult = test_IdentifyAsn1Key();
+    printf("IdentifyAsn1Key: %s\n", (unitResult == 0 ? "SUCCESS" : "FAILED"));
+    testResult = testResult || unitResult;
+
 #endif
 
 #ifdef WOLFSSH_KEYGEN