From 47a07d41d25825a702aed5a356b3ea4c4a3140a5 Mon Sep 17 00:00:00 2001 From: Yosuke Shimizu Date: Tue, 12 May 2026 13:19:57 +0900 Subject: [PATCH] Fix minor issues and Add unit test for wrong padded OpenSSH key --- apps/wolfsshd/configuration.c | 2 +- src/internal.c | 2 ++ src/keygen.c | 15 +++++++++----- tests/api.c | 39 +++++++++++++++++++++++++++++++++++ 4 files changed, 52 insertions(+), 6 deletions(-) diff --git a/apps/wolfsshd/configuration.c b/apps/wolfsshd/configuration.c index 68024e6bc..a06d7803c 100644 --- a/apps/wolfsshd/configuration.c +++ b/apps/wolfsshd/configuration.c @@ -1120,7 +1120,7 @@ WOLFSSHD_STATIC int ParseConfigLine(WOLFSSHD_CONFIG** conf, const char* l, */ idx = sz; idx += CountWhitespace(l + idx, lSz - sz, 0); - sz = CountWhitespace(l + idx, lSz - sz, 1); + sz = CountWhitespace(l + idx, lSz - idx, 1); if (sz >= MAX_FILENAME_SZ) { wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Filename too long."); ret = WS_FATAL_ERROR; diff --git a/src/internal.c b/src/internal.c index e50693795..505711ed3 100644 --- a/src/internal.c +++ b/src/internal.c @@ -1990,6 +1990,8 @@ static int GetOpenSshKey(WS_KeySignature *key, check1++, subIdx++) { if (check1 != str[subIdx]) { /* Bad pad value. */ + ret = WS_KEY_FORMAT_E; + break; } } } diff --git a/src/keygen.c b/src/keygen.c index cad01ed4d..4baee3ace 100644 --- a/src/keygen.c +++ b/src/keygen.c @@ -102,12 +102,14 @@ int wolfSSH_MakeRsaKey(byte* out, word32 outSz, word32 size, word32 e) if (wc_FreeRsaKey(&key) != 0) { WLOG(WS_LOG_DEBUG, "RSA key free failed"); - ret = WS_CRYPTO_FAILED; + if (ret >= 0) + ret = WS_CRYPTO_FAILED; } if (wc_FreeRng(&rng) != 0) { WLOG(WS_LOG_DEBUG, "Couldn't free RNG"); - ret = WS_CRYPTO_FAILED; + if (ret >= 0) + ret = WS_CRYPTO_FAILED; } } @@ -167,12 +169,14 @@ int wolfSSH_MakeEcdsaKey(byte* out, word32 outSz, word32 size) if (wc_ecc_free(&key) != 0) { WLOG(WS_LOG_DEBUG, "ECDSA key free failed"); - ret = WS_CRYPTO_FAILED; + if (ret >= 0) + ret = WS_CRYPTO_FAILED; } if (wc_FreeRng(&rng) != 0) { WLOG(WS_LOG_DEBUG, "Couldn't free RNG"); - ret = WS_CRYPTO_FAILED; + if (ret >= 0) + ret = WS_CRYPTO_FAILED; } } @@ -234,7 +238,8 @@ int wolfSSH_MakeEd25519Key(byte* out, word32 outSz, word32 size) if (wc_FreeRng(&rng) != 0) { WLOG(WS_LOG_DEBUG, "Couldn't free RNG"); - ret = WS_CRYPTO_FAILED; + if (ret >= 0) + ret = WS_CRYPTO_FAILED; } } diff --git a/tests/api.c b/tests/api.c index 734dd573a..e2f8ad5fb 100644 --- a/tests/api.c +++ b/tests/api.c @@ -733,6 +733,19 @@ const char id_ecdsa_pub[] = "BMCp0GAKnxthKraRBDjz9R3wjLoyOdv9+kHct9IT/WTH1VpoTgUveL0aDa8NXR4sYzc9aSwU" "0+FQvG1xgnKNoGM= bob@localhost\n"; +/* Same as id_ecdsa but with the last pad byte changed from 0x03 to 0x04, + * so the padding sequence 1,2,3 is broken at position 3. */ +const char id_ecdsa_bad_pad[] = + "-----BEGIN OPENSSH PRIVATE KEY-----\n" + "b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAaAAAABNlY2RzYS\n" + "1zaGEyLW5pc3RwMjU2AAAACG5pc3RwMjU2AAAAQQTAqdBgCp8bYSq2kQQ48/Ud8Iy6Mjnb\n" + "/fpB3LfSE/1kx9VaaE4FL3i9Gg2vDV0eLGM3PWksFNPhULxtcYJyjaBjAAAAqJAeleSQHp\n" + "XkAAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBMCp0GAKnxthKraR\n" + "BDjz9R3wjLoyOdv9+kHct9IT/WTH1VpoTgUveL0aDa8NXR4sYzc9aSwU0+FQvG1xgnKNoG\n" + "MAAAAgPrOgktioNqad/wHNC/rt/zVrpNqDnOwg9tNDFMOTwo8AAAANYm9iQGxvY2FsaG9z\n" + "dAECBA==\n" + "-----END OPENSSH PRIVATE KEY-----\n"; + #endif /* WOLFSSH_NO_ECDSA_SHA2_NISTP256 */ static void test_wolfSSH_ReadKey(void) @@ -870,6 +883,31 @@ static void test_wolfSSH_ReadKey(void) } +static void test_wolfSSH_ReadKey_badPad(void) +{ +#ifndef WOLFSSH_NO_ECDSA_SHA2_NISTP256 + byte* key = NULL; + word32 keySz = 0; + const byte* keyType = NULL; + word32 keyTypeSz = 0; + int ret; + + ret = wolfSSH_ReadKey_buffer((const byte*)id_ecdsa_bad_pad, + (word32)WSTRLEN(id_ecdsa_bad_pad), WOLFSSH_FORMAT_OPENSSH, + &key, &keySz, &keyType, &keyTypeSz, NULL); + AssertIntEQ(ret, WS_KEY_FORMAT_E); + /* DoOpenSshKey never assigns *outSz, *outType, or *outTypeSz + * on the error branch (only on success), + * these assertions will catch any future regression + * where the API partially writes output before failing. */ + AssertNull(key); + AssertIntEQ(keySz, 0); + AssertNull(keyType); + AssertIntEQ(keyTypeSz, 0); +#endif +} + + #ifdef WOLFSSH_SCP static int my_ScpRecv(WOLFSSH* ssh, int state, const char* basePath, @@ -2079,6 +2117,7 @@ int wolfSSH_ApiTest(int argc, char** argv) test_wolfSSH_CTX_UsePrivateKey_buffer_pem(); test_wolfSSH_CertMan(); test_wolfSSH_ReadKey(); + test_wolfSSH_ReadKey_badPad(); test_wolfSSH_QueryAlgoList(); test_wolfSSH_SetAlgoList(); #ifdef WOLFSSH_AGENT