From 5ebd7f83acf53d9c5595637e55d29e28159b8f01 Mon Sep 17 00:00:00 2001 From: Emma Stensland Date: Mon, 29 Jun 2026 15:41:59 -0600 Subject: [PATCH] SSHD/Echoserver: Fix memory leaks and public-key lookup --- apps/wolfsshd/auth.c | 49 ++- apps/wolfsshd/auth.h | 10 + apps/wolfsshd/test/test_configuration.c | 282 ++++++++++++++++++ examples/client/common.c | 2 + examples/echoserver/echoserver.c | 76 ++++- .../wolfssh_echoserver/main/echoserver.c | 56 +++- src/internal.c | 40 +-- tests/auth.c | 3 + tests/testsuite.c | 76 ++++- 9 files changed, 543 insertions(+), 51 deletions(-) diff --git a/apps/wolfsshd/auth.c b/apps/wolfsshd/auth.c index 793ee2dcc..7ae82484a 100644 --- a/apps/wolfsshd/auth.c +++ b/apps/wolfsshd/auth.c @@ -95,6 +95,10 @@ #if defined(WOLFSSHD_UNIT_TEST) && !defined(_WIN32) int (*wsshd_setregid_cb)(WGID_T, WGID_T) = setregid; int (*wsshd_setreuid_cb)(WUID_T, WUID_T) = setreuid; +int (*wsshd_setegid_cb)(WGID_T) = setegid; +int (*wsshd_seteuid_cb)(WUID_T) = seteuid; +struct passwd* (*wsshd_getpwnam_cb)(const char*) = getpwnam; +#define getpwnam wsshd_getpwnam_cb #endif struct WOLFSSHD_AUTH { @@ -1835,13 +1839,8 @@ static int SetDefaultPublicKeyCheck(WOLFSSHD_AUTH* auth) return ret; } -#ifndef WOLFSSH_SSHD_USER - #define WOLFSSH_SSHD_USER sshd -#endif -#define WOLFSSH_USER_GET_STRING(x) #x -#define WOLFSSH_USER_STRING(x) WOLFSSH_USER_GET_STRING(x) -static int SetDefualtUserID(WOLFSSHD_AUTH* auth) +static int SetDefaultUserID(WOLFSSHD_AUTH* auth) { #ifdef _WIN32 /* TODO: Implement for Windows. */ @@ -1850,6 +1849,15 @@ static int SetDefualtUserID(WOLFSSHD_AUTH* auth) struct passwd* pwInfo; int ret = WS_SUCCESS; + if (wolfSSHD_ConfigGetPrivilegeSeparation(auth->conf) == + WOLFSSHD_PRIV_OFF) { + auth->gid = getgid(); + auth->uid = getuid(); + auth->sGid = auth->gid; + auth->sUid = auth->uid; + return WS_SUCCESS; + } + pwInfo = getpwnam(WOLFSSH_USER_STRING(WOLFSSH_SSHD_USER)); if (pwInfo == NULL) { /* user name not found on system */ @@ -1910,7 +1918,7 @@ WOLFSSHD_AUTH* wolfSSHD_AuthCreateUser(void* heap, const WOLFSSHD_CONFIG* conf) } if (ret == WS_SUCCESS) { - ret = SetDefualtUserID(auth); + ret = SetDefaultUserID(auth); if (ret != WS_SUCCESS) { wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error setting default " "user ID."); @@ -1942,24 +1950,37 @@ int wolfSSHD_AuthFreeUser(WOLFSSHD_AUTH* auth) /* return WS_SUCCESS on success */ int wolfSSHD_AuthRaisePermissions(WOLFSSHD_AUTH* auth) { - int ret = 0; + int ret = WS_SUCCESS; - wolfSSH_Log(WS_LOG_INFO, "[SSHD] Attempting to raise permissions level"); #ifndef WIN32 - if (auth) { + byte flag = 0; + + if (auth == NULL) { + return WS_BAD_ARGUMENT; + } + + flag = wolfSSHD_ConfigGetPrivilegeSeparation(auth->conf); + if (flag == WOLFSSHD_PRIV_SEPARAT || flag == WOLFSSHD_PRIV_SANDBOX) { + wolfSSH_Log(WS_LOG_INFO, + "[SSHD] Attempting to raise permissions level"); +#ifdef WOLFSSHD_UNIT_TEST + if (wsshd_setegid_cb(auth->sGid) != 0) { +#else if (setegid(auth->sGid) != 0) { +#endif wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error raising gid"); ret = WS_FATAL_ERROR; } - if (seteuid(auth->sUid) != 0) { +#ifdef WOLFSSHD_UNIT_TEST + if (ret == WS_SUCCESS && wsshd_seteuid_cb(auth->sUid) != 0) { +#else + if (ret == WS_SUCCESS && seteuid(auth->sUid) != 0) { +#endif wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error raising uid"); ret = WS_FATAL_ERROR; } } - else { - ret = WS_BAD_ARGUMENT; - } #endif return ret; diff --git a/apps/wolfsshd/auth.h b/apps/wolfsshd/auth.h index c487a68ca..ca8e7af6f 100644 --- a/apps/wolfsshd/auth.h +++ b/apps/wolfsshd/auth.h @@ -21,7 +21,14 @@ #ifndef WOLFAUTH_H #define WOLFAUTH_H +#ifndef WOLFSSH_SSHD_USER + #define WOLFSSH_SSHD_USER sshd +#endif +#define WOLFSSH_USER_GET_STRING(x) #x +#define WOLFSSH_USER_STRING(x) WOLFSSH_USER_GET_STRING(x) + #if 0 + typedef struct USER_NODE USER_NODE; USER_NODE* AddNewUser(USER_NODE* list, byte type, const byte* username, @@ -90,6 +97,9 @@ int wolfSSHD_OpenSecureFile(const char* path, WUID_T ownerUid, #ifndef _WIN32 extern int (*wsshd_setregid_cb)(WGID_T, WGID_T); extern int (*wsshd_setreuid_cb)(WUID_T, WUID_T); +extern int (*wsshd_setegid_cb)(WGID_T); +extern int (*wsshd_seteuid_cb)(WUID_T); +extern struct passwd* (*wsshd_getpwnam_cb)(const char*); int wolfSSHD_GetUserGroupNames(void* heap, const char* usr, WGID_T primaryGid, char*** outNames, word32* outCount); void wolfSSHD_FreeUserGroupNames(void* heap, char** names, word32 count); diff --git a/apps/wolfsshd/test/test_configuration.c b/apps/wolfsshd/test/test_configuration.c index aa5c410de..fd61520a1 100644 --- a/apps/wolfsshd/test/test_configuration.c +++ b/apps/wolfsshd/test/test_configuration.c @@ -1559,6 +1559,282 @@ static int test_AuthReducePermissionsUser_uid_fail(void) wsshd_setreuid_cb = savedReuid; return ret; } + +static WGID_T s_setegid_arg; +static WUID_T s_seteuid_arg; +static int s_setegid_ret; +static int s_seteuid_ret; +static int s_setegid_called; +static int s_seteuid_called; + +static int stub_setegid(WGID_T egid) +{ + s_setegid_called = 1; + s_setegid_arg = egid; + return s_setegid_ret; +} + +static int stub_seteuid(WUID_T euid) +{ + s_seteuid_called = 1; + s_seteuid_arg = euid; + return s_seteuid_ret; +} + +static void InstallPrivRaiseStubs(int egidRet, int euidRet, + int (**savedEgid)(WGID_T), int (**savedEuid)(WUID_T)) +{ + *savedEgid = wsshd_setegid_cb; + *savedEuid = wsshd_seteuid_cb; + wsshd_setegid_cb = stub_setegid; + wsshd_seteuid_cb = stub_seteuid; + s_setegid_ret = egidRet; + s_seteuid_ret = euidRet; + s_setegid_called = 0; + s_seteuid_called = 0; + s_setegid_arg = 0; + s_seteuid_arg = 0; +} + +/* Synthetic "sshd" account used so privilege-separation tests don't depend + * on the host actually having an sshd system user configured. */ +static struct passwd stub_sshd_pw; + +static struct passwd* stub_getpwnam(const char* name) +{ + if (name != NULL && WSTRCMP(name, WOLFSSH_USER_STRING(WOLFSSH_SSHD_USER)) == 0) { + WMEMSET(&stub_sshd_pw, 0, sizeof(stub_sshd_pw)); + stub_sshd_pw.pw_name = (char*)WOLFSSH_USER_STRING(WOLFSSH_SSHD_USER); + stub_sshd_pw.pw_uid = 1000; + stub_sshd_pw.pw_gid = 1000; + return &stub_sshd_pw; + } + return NULL; +} + +static void InstallGetpwnamStub(struct passwd* (**savedGetpwnam)(const char*)) +{ + *savedGetpwnam = wsshd_getpwnam_cb; + wsshd_getpwnam_cb = stub_getpwnam; +} + +/* UsePrivilegeSeparation no must let SetDefaultUserID succeed without a + * configured sshd system user, since no uid/gid switching will ever happen. */ +static int test_AuthCreateUser_privSepOff(void) +{ + int ret = WS_SUCCESS; + WOLFSSHD_CONFIG* conf; + WOLFSSHD_AUTH* auth; + static const char line[] = "UsePrivilegeSeparation no"; + + conf = wolfSSHD_ConfigNew(NULL); + if (conf == NULL) { + return WS_MEMORY_E; + } + + if (ParseConfigLine(&conf, line, (int)WSTRLEN(line), 0) != WS_SUCCESS) { + ret = WS_FATAL_ERROR; + } + + if (ret == WS_SUCCESS) { + auth = wolfSSHD_AuthCreateUser(NULL, conf); + if (auth == NULL) { + ret = WS_FATAL_ERROR; + } + else { + wolfSSHD_AuthFreeUser(auth); + } + } + + wolfSSHD_ConfigFree(conf); + return ret; +} + +/* wolfSSHD_AuthRaisePermissions must not touch setegid/seteuid at all when + * privilege separation is off, since the process never dropped privileges. */ +static int test_AuthRaisePermissions_offSkipsSyscalls(void) +{ + int ret = WS_SUCCESS; + WOLFSSHD_CONFIG* conf; + WOLFSSHD_AUTH* auth; + int (*savedEgid)(WGID_T); + int (*savedEuid)(WUID_T); + static const char line[] = "UsePrivilegeSeparation no"; + + conf = wolfSSHD_ConfigNew(NULL); + if (conf == NULL) { + return WS_MEMORY_E; + } + + if (ParseConfigLine(&conf, line, (int)WSTRLEN(line), 0) != WS_SUCCESS) { + ret = WS_FATAL_ERROR; + } + + if (ret == WS_SUCCESS) { + auth = wolfSSHD_AuthCreateUser(NULL, conf); + if (auth == NULL) { + ret = WS_FATAL_ERROR; + } + else { + InstallPrivRaiseStubs(0, 0, &savedEgid, &savedEuid); + + if (wolfSSHD_AuthRaisePermissions(auth) != WS_SUCCESS) + ret = WS_FATAL_ERROR; + if (ret == WS_SUCCESS + && (s_setegid_called || s_seteuid_called)) + ret = WS_FATAL_ERROR; + + wsshd_setegid_cb = savedEgid; + wsshd_seteuid_cb = savedEuid; + wolfSSHD_AuthFreeUser(auth); + } + } + + wolfSSHD_ConfigFree(conf); + return ret; +} + +/* With privilege separation on, wolfSSHD_AuthRaisePermissions restores the + * uid/gid the process started with (captured at AuthCreateUser time). Uses a + * stubbed getpwnam("sshd") so the test doesn't depend on the environment + * actually having an sshd system user configured. */ +static int test_AuthRaisePermissions_separateCallsSyscalls(void) +{ + int ret = WS_SUCCESS; + WOLFSSHD_CONFIG* conf; + WOLFSSHD_AUTH* auth; + int (*savedEgid)(WGID_T); + int (*savedEuid)(WUID_T); + struct passwd* (*savedGetpwnam)(const char*); + + conf = wolfSSHD_ConfigNew(NULL); + if (conf == NULL) { + return WS_MEMORY_E; + } + + InstallGetpwnamStub(&savedGetpwnam); + + /* privilege separation defaults to on */ + auth = wolfSSHD_AuthCreateUser(NULL, conf); + if (auth == NULL) { + ret = WS_FATAL_ERROR; + } + else { + InstallPrivRaiseStubs(0, 0, &savedEgid, &savedEuid); + + if (wolfSSHD_AuthRaisePermissions(auth) != WS_SUCCESS) + ret = WS_FATAL_ERROR; + if (ret == WS_SUCCESS && (!s_setegid_called || !s_seteuid_called)) + ret = WS_FATAL_ERROR; + if (ret == WS_SUCCESS && s_setegid_arg != getgid()) + ret = WS_FATAL_ERROR; + if (ret == WS_SUCCESS && s_seteuid_arg != getuid()) + ret = WS_FATAL_ERROR; + + wsshd_setegid_cb = savedEgid; + wsshd_seteuid_cb = savedEuid; + wolfSSHD_AuthFreeUser(auth); + } + + wsshd_getpwnam_cb = savedGetpwnam; + wolfSSHD_ConfigFree(conf); + return ret; +} + +/* wolfSSHD_AuthRaisePermissions must reject a NULL auth argument instead of + * dereferencing it. */ +static int test_AuthRaisePermissions_nullArg(void) +{ + if (wolfSSHD_AuthRaisePermissions(NULL) != WS_BAD_ARGUMENT) + return WS_FATAL_ERROR; + return WS_SUCCESS; +} + +/* When setegid fails, wolfSSHD_AuthRaisePermissions must report the failure + * and short-circuit seteuid rather than attempting it anyway. */ +static int test_AuthRaisePermissions_gidFailSkipsUid(void) +{ + int ret = WS_SUCCESS; + WOLFSSHD_CONFIG* conf; + WOLFSSHD_AUTH* auth; + int (*savedEgid)(WGID_T); + int (*savedEuid)(WUID_T); + struct passwd* (*savedGetpwnam)(const char*); + + conf = wolfSSHD_ConfigNew(NULL); + if (conf == NULL) { + return WS_MEMORY_E; + } + + InstallGetpwnamStub(&savedGetpwnam); + + /* privilege separation defaults to on */ + auth = wolfSSHD_AuthCreateUser(NULL, conf); + if (auth == NULL) { + ret = WS_FATAL_ERROR; + } + else { + InstallPrivRaiseStubs(-1, 0, &savedEgid, &savedEuid); + + if (wolfSSHD_AuthRaisePermissions(auth) != WS_FATAL_ERROR) + ret = WS_FATAL_ERROR; + if (ret == WS_SUCCESS && !s_setegid_called) + ret = WS_FATAL_ERROR; + if (ret == WS_SUCCESS && s_seteuid_called) + ret = WS_FATAL_ERROR; + + wsshd_setegid_cb = savedEgid; + wsshd_seteuid_cb = savedEuid; + wolfSSHD_AuthFreeUser(auth); + } + + wsshd_getpwnam_cb = savedGetpwnam; + wolfSSHD_ConfigFree(conf); + return ret; +} + +/* When setegid succeeds but seteuid fails, wolfSSHD_AuthRaisePermissions must + * still report the failure. */ +static int test_AuthRaisePermissions_uidFail(void) +{ + int ret = WS_SUCCESS; + WOLFSSHD_CONFIG* conf; + WOLFSSHD_AUTH* auth; + int (*savedEgid)(WGID_T); + int (*savedEuid)(WUID_T); + struct passwd* (*savedGetpwnam)(const char*); + + conf = wolfSSHD_ConfigNew(NULL); + if (conf == NULL) { + return WS_MEMORY_E; + } + + InstallGetpwnamStub(&savedGetpwnam); + + /* privilege separation defaults to on */ + auth = wolfSSHD_AuthCreateUser(NULL, conf); + if (auth == NULL) { + ret = WS_FATAL_ERROR; + } + else { + InstallPrivRaiseStubs(0, -1, &savedEgid, &savedEuid); + + if (wolfSSHD_AuthRaisePermissions(auth) != WS_FATAL_ERROR) + ret = WS_FATAL_ERROR; + if (ret == WS_SUCCESS && !s_setegid_called) + ret = WS_FATAL_ERROR; + if (ret == WS_SUCCESS && !s_seteuid_called) + ret = WS_FATAL_ERROR; + + wsshd_setegid_cb = savedEgid; + wsshd_seteuid_cb = savedEuid; + wolfSSHD_AuthFreeUser(auth); + } + + wsshd_getpwnam_cb = savedGetpwnam; + wolfSSHD_ConfigFree(conf); + return ret; +} #endif /* !_WIN32 */ /* Locks in the NULL-safe comparison used by RequestAuthentication to fail @@ -1929,6 +2205,12 @@ const TEST_CASE testCases[] = { TEST_DECL(test_AuthReducePermissionsUser_ok), TEST_DECL(test_AuthReducePermissionsUser_gid_fail), TEST_DECL(test_AuthReducePermissionsUser_uid_fail), + TEST_DECL(test_AuthCreateUser_privSepOff), + TEST_DECL(test_AuthRaisePermissions_offSkipsSyscalls), + TEST_DECL(test_AuthRaisePermissions_separateCallsSyscalls), + TEST_DECL(test_AuthRaisePermissions_nullArg), + TEST_DECL(test_AuthRaisePermissions_gidFailSkipsUid), + TEST_DECL(test_AuthRaisePermissions_uidFail), #endif #if defined(WOLFSSH_HAVE_LIBCRYPT) || defined(WOLFSSH_HAVE_LIBLOGIN) TEST_DECL(test_CheckPasswordHashUnix), diff --git a/examples/client/common.c b/examples/client/common.c index ed7e33d7d..ba0205022 100644 --- a/examples/client/common.c +++ b/examples/client/common.c @@ -1138,6 +1138,7 @@ void ClientFreeBuffers(const char* pubKeyName, const char* privKeyName, userPrivateKey = userPrivateKeyBuf; userPrivateKeyAlloc = 0; } + userPrivateKeySz = 0; } #ifdef WOLFSSH_KEYBOARD_INTERACTIVE @@ -1164,4 +1165,5 @@ void ClientFreeBuffers(const char* pubKeyName, const char* privKeyName, wc_ForceZero(userPrivateKeyBuf, sizeof(userPrivateKeyBuf)); userPrivateKeySz = 0; wc_ForceZero(userPassword, sizeof(userPassword)); + pubKeyLoaded = 0; } diff --git a/examples/echoserver/echoserver.c b/examples/echoserver/echoserver.c index f86fbc75f..20f4eb27c 100644 --- a/examples/echoserver/echoserver.c +++ b/examples/echoserver/echoserver.c @@ -1785,7 +1785,6 @@ static int load_key(byte isEcc, byte* buf, word32 bufSz) return sz; } - #ifndef WOLFSSH_NO_ED25519 /* returns buffer size on success */ static int load_key_ed25519(byte* buf, word32 bufSz) @@ -1922,7 +1921,6 @@ static int LoadMlDsaHostKeys(WOLFSSH_CTX* ctx, const char* keyList) } #endif /* WOLFSSH_NO_MLDSA */ - typedef struct StrList { const char* str; struct StrList* next; @@ -2475,7 +2473,7 @@ static int EchoserverInitTpmHostKey(WOLFSSH_CTX* ctx, const char* keyFile, wolfTPM2_Cleanup(&tpmHostDev); } - /* keyBlob holds the private blob and key auth; the session may hold auth. */ + /* zeroize key material; session may also hold auth data */ wc_ForceZero(&keyBlob, sizeof(keyBlob)); wc_ForceZero(&tpmSession, sizeof(tpmSession)); #ifndef NO_FILESYSTEM @@ -2556,6 +2554,7 @@ static int wsUserAuth(byte authType, PwMapList* list; PwMap* map; byte authHash[WC_SHA256_DIGEST_SIZE] = {0}; + int userFound = 0; if (ctx == NULL) { fprintf(stderr, "wsUserAuth: ctx not set"); @@ -2676,12 +2675,12 @@ static int wsUserAuth(byte authType, authData->type == map->type) { if (authData->type == WOLFSSH_USERAUTH_PUBLICKEY) { + userFound = 1; if (WMEMCMP(map->p, authHash, WC_SHA256_DIGEST_SIZE) == 0) { return WOLFSSH_USERAUTH_SUCCESS; } - else { - return WOLFSSH_USERAUTH_INVALID_PUBLICKEY; - } + /* Hash mismatch: continue checking other registered keys + * for this user (a user may have multiple public keys). */ } else if (authData->type == WOLFSSH_USERAUTH_PASSWORD) { if (WMEMCMP(map->p, authHash, WC_SHA256_DIGEST_SIZE) == 0) { @@ -2724,6 +2723,8 @@ static int wsUserAuth(byte authType, map = map->next; } + if (userFound) + return WOLFSSH_USERAUTH_INVALID_PUBLICKEY; return WOLFSSH_USERAUTH_INVALID_USER; } @@ -2807,7 +2808,8 @@ static void ShowUsage(void) " load in a SSH public key to accept from peer\n"); printf(" -s load in a TPM public key file to replace default hansel key\n"); #ifdef WOLFSSH_TPM - printf(" -G load an ECC/RSA host key blob from the TPM (private key stays in the TPM)\n"); + printf(" -G load ECC/RSA host key blob from TPM" + " (private key stays in TPM)\n"); #endif printf(" -J :\n" " load in an X.509 PEM cert to accept from peer\n"); @@ -3213,14 +3215,17 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args) if (kbAuthData.prompts == NULL) { ES_ERROR("Error allocating prompts"); } - kbAuthData.prompts[0] = (byte*)"KB Auth Password: "; kbAuthData.promptLengths = (word32*)WMALLOC(sizeof(word32), NULL, 0); - if (kbAuthData.prompts == NULL) { + if (kbAuthData.promptLengths == NULL) { + WFREE(kbAuthData.prompts, NULL, 0); ES_ERROR("Error allocating promptLengths"); } + kbAuthData.prompts[0] = (byte*)"KB Auth Password: "; kbAuthData.promptLengths[0] = 18; kbAuthData.promptEcho = (byte*)WMALLOC(sizeof(byte), NULL, 0); - if (kbAuthData.prompts == NULL) { + if (kbAuthData.promptEcho == NULL) { + WFREE(kbAuthData.prompts, NULL, 0); + WFREE(kbAuthData.promptLengths, NULL, 0); ES_ERROR("Error allocating promptEcho"); } kbAuthData.promptEcho[0] = 0; @@ -3254,6 +3259,10 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args) if (tpmHostKeyPath != NULL) { if (EchoserverInitTpmHostKey(ctx, tpmHostKeyPath, ECHOSERVER_TPM_KEY_AUTH_DEFAULT) != 0) { + #ifdef WOLFSSH_SMALL_STACK + wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); + WFREE(keyLoadBuf, NULL, 0); + #endif ES_ERROR("Couldn't load TPM host key from %s.\n", tpmHostKeyPath); } @@ -3264,10 +3273,18 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args) if (loadDefaultHostKeys) { bufSz = load_key(peerEcc, keyLoadBuf, bufSz); if (bufSz == 0) { + #ifdef WOLFSSH_SMALL_STACK + wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); + WFREE(keyLoadBuf, NULL, 0); + #endif ES_ERROR("Couldn't load first key file.\n"); } if (wolfSSH_CTX_UsePrivateKey_buffer(ctx, keyLoadBuf, bufSz, WOLFSSH_FORMAT_ASN1) < 0) { + #ifdef WOLFSSH_SMALL_STACK + wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); + WFREE(keyLoadBuf, NULL, 0); + #endif ES_ERROR("Couldn't use first key buffer.\n"); } @@ -3277,10 +3294,18 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args) bufSz = load_key(peerEcc, keyLoadBuf, bufSz); if (bufSz == 0) { + #ifdef WOLFSSH_SMALL_STACK + wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); + WFREE(keyLoadBuf, NULL, 0); + #endif ES_ERROR("Couldn't load second key file.\n"); } if (wolfSSH_CTX_UsePrivateKey_buffer(ctx, keyLoadBuf, bufSz, WOLFSSH_FORMAT_ASN1) < 0) { + #ifdef WOLFSSH_SMALL_STACK + wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); + WFREE(keyLoadBuf, NULL, 0); + #endif ES_ERROR("Couldn't use second key buffer.\n"); } #endif @@ -3289,10 +3314,18 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args) bufSz = EXAMPLE_KEYLOAD_BUFFER_SZ; bufSz = load_key_ed25519(keyLoadBuf, bufSz); if (bufSz == 0) { + #ifdef WOLFSSH_SMALL_STACK + wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); + WFREE(keyLoadBuf, NULL, 0); + #endif ES_ERROR("Couldn't load Ed25519 key file.\n"); } if (wolfSSH_CTX_UsePrivateKey_buffer(ctx, keyLoadBuf, bufSz, WOLFSSH_FORMAT_ASN1) < 0) { + #ifdef WOLFSSH_SMALL_STACK + wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); + WFREE(keyLoadBuf, NULL, 0); + #endif ES_ERROR("Couldn't use Ed25519 key buffer.\n"); } #endif /* WOLFSSH_NO_ED25519 */ @@ -3304,6 +3337,7 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args) if (keyList != NULL && WSTRSTR(keyList, "mldsa") != NULL) { if (LoadMlDsaHostKeys(ctx, keyList) != 0) { #ifdef WOLFSSH_SMALL_STACK + wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); WFREE(keyLoadBuf, NULL, 0); #endif ES_ERROR("Error loading ML-DSA host keys.\n"); @@ -3321,11 +3355,19 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args) /* create temp buffer and load in file */ if (userBufSz == 0) { + #ifdef WOLFSSH_SMALL_STACK + wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); + WFREE(keyLoadBuf, NULL, 0); + #endif ES_ERROR("Couldn't find size of file %s.\n", userPubKey); } userBuf = (byte*)WMALLOC(userBufSz, NULL, 0); if (userBuf == NULL) { + #ifdef WOLFSSH_SMALL_STACK + wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); + WFREE(keyLoadBuf, NULL, 0); + #endif ES_ERROR("WMALLOC failed\n"); } load_file(userPubKey, userBuf, &userBufSz); @@ -3343,17 +3385,30 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args) load_file(caCert, NULL, &certBufSz); if (certBufSz == 0) { + #ifdef WOLFSSH_SMALL_STACK + wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); + WFREE(keyLoadBuf, NULL, 0); + #endif ES_ERROR("Couldn't find size of file %s.\n", caCert); } certBuf = (byte*)WMALLOC(certBufSz, NULL, 0); if (certBuf == NULL) { + #ifdef WOLFSSH_SMALL_STACK + wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); + WFREE(keyLoadBuf, NULL, 0); + #endif ES_ERROR("WMALLOC failed\n"); } load_file(caCert, certBuf, &certBufSz); ret = wolfSSH_CTX_AddRootCert_buffer(ctx, certBuf, certBufSz, WOLFSSH_FORMAT_PEM); if (ret != 0) { + #ifdef WOLFSSH_SMALL_STACK + wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); + WFREE(keyLoadBuf, NULL, 0); + #endif + WFREE(certBuf, NULL, 0); ES_ERROR("Couldn't add root cert\n"); } WFREE(certBuf, NULL, 0); @@ -3394,6 +3449,7 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args) #endif /* WOLFSSH_ALLOW_USERAUTH_NONE */ #ifdef WOLFSSH_SMALL_STACK + wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); WFREE(keyLoadBuf, NULL, 0); #endif } diff --git a/ide/Espressif/ESP-IDF/examples/wolfssh_echoserver/main/echoserver.c b/ide/Espressif/ESP-IDF/examples/wolfssh_echoserver/main/echoserver.c index 50321ac8c..2f4076566 100644 --- a/ide/Espressif/ESP-IDF/examples/wolfssh_echoserver/main/echoserver.c +++ b/ide/Espressif/ESP-IDF/examples/wolfssh_echoserver/main/echoserver.c @@ -2221,6 +2221,7 @@ static int wsUserAuth(byte authType, PwMapList* list; PwMap* map; byte authHash[WC_SHA256_DIGEST_SIZE]; + int userFound = 0; if (ctx == NULL) { fprintf(stderr, "wsUserAuth: ctx not set"); @@ -2342,12 +2343,12 @@ static int wsUserAuth(byte authType, authData->type == map->type) { if (authData->type == WOLFSSH_USERAUTH_PUBLICKEY) { + userFound = 1; if (WMEMCMP(map->p, authHash, WC_SHA256_DIGEST_SIZE) == 0) { return WOLFSSH_USERAUTH_SUCCESS; } - else { - return WOLFSSH_USERAUTH_INVALID_PUBLICKEY; - } + /* Hash mismatch: continue checking other registered keys + * for this user (a user may have multiple public keys). */ } else if (authData->type == WOLFSSH_USERAUTH_PASSWORD) { if (WMEMCMP(map->p, authHash, WC_SHA256_DIGEST_SIZE) == 0) { @@ -2382,6 +2383,8 @@ static int wsUserAuth(byte authType, map = map->next; } + if (userFound) + return WOLFSSH_USERAUTH_INVALID_PUBLICKEY; return WOLFSSH_USERAUTH_INVALID_USER; } @@ -2861,14 +2864,17 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args) if (kbAuthData.prompts == NULL) { ES_ERROR("Error allocating prompts"); } - kbAuthData.prompts[0] = (byte*)"KB Auth Password: "; kbAuthData.promptLengths = (word32*)WMALLOC(sizeof(word32), NULL, 0); - if (kbAuthData.prompts == NULL) { + if (kbAuthData.promptLengths == NULL) { + WFREE(kbAuthData.prompts, NULL, 0); ES_ERROR("Error allocating promptLengths"); } + kbAuthData.prompts[0] = (byte*)"KB Auth Password: "; kbAuthData.promptLengths[0] = 18; kbAuthData.promptEcho = (byte*)WMALLOC(sizeof(byte), NULL, 0); - if (kbAuthData.prompts == NULL) { + if (kbAuthData.promptEcho == NULL) { + WFREE(kbAuthData.prompts, NULL, 0); + WFREE(kbAuthData.promptLengths, NULL, 0); ES_ERROR("Error allocating promptEcho"); } kbAuthData.promptEcho[0] = 0; @@ -2897,10 +2903,18 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args) bufSz = load_key(peerEcc, keyLoadBuf, bufSz); if (bufSz == 0) { + #ifdef WOLFSSH_SMALL_STACK + wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); + WFREE(keyLoadBuf, NULL, 0); + #endif ES_ERROR("Couldn't load first key file.\n"); } if (wolfSSH_CTX_UsePrivateKey_buffer(ctx, keyLoadBuf, bufSz, WOLFSSH_FORMAT_ASN1) < 0) { + #ifdef WOLFSSH_SMALL_STACK + wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); + WFREE(keyLoadBuf, NULL, 0); + #endif ES_ERROR("Couldn't use first key buffer.\n"); } @@ -2910,10 +2924,18 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args) bufSz = load_key(peerEcc, keyLoadBuf, bufSz); if (bufSz == 0) { + #ifdef WOLFSSH_SMALL_STACK + wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); + WFREE(keyLoadBuf, NULL, 0); + #endif ES_ERROR("Couldn't load second key file.\n"); } if (wolfSSH_CTX_UsePrivateKey_buffer(ctx, keyLoadBuf, bufSz, WOLFSSH_FORMAT_ASN1) < 0) { + #ifdef WOLFSSH_SMALL_STACK + wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); + WFREE(keyLoadBuf, NULL, 0); + #endif ES_ERROR("Couldn't use second key buffer.\n"); } #endif @@ -2928,11 +2950,19 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args) /* create temp buffer and load in file */ if (userBufSz == 0) { + #ifdef WOLFSSH_SMALL_STACK + wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); + WFREE(keyLoadBuf, NULL, 0); + #endif ES_ERROR("Couldn't find size of file %s.\n", userPubKey); } userBuf = (byte*)WMALLOC(userBufSz, NULL, 0); if (userBuf == NULL) { + #ifdef WOLFSSH_SMALL_STACK + wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); + WFREE(keyLoadBuf, NULL, 0); + #endif ES_ERROR("WMALLOC failed\n"); } load_file(userPubKey, userBuf, &userBufSz); @@ -2950,17 +2980,30 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args) load_file(caCert, NULL, &certBufSz); if (certBufSz == 0) { + #ifdef WOLFSSH_SMALL_STACK + wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); + WFREE(keyLoadBuf, NULL, 0); + #endif ES_ERROR("Couldn't find size of file %s.\n", caCert); } certBuf = (byte*)WMALLOC(certBufSz, NULL, 0); if (certBuf == NULL) { + #ifdef WOLFSSH_SMALL_STACK + wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); + WFREE(keyLoadBuf, NULL, 0); + #endif ES_ERROR("WMALLOC failed\n"); } load_file(caCert, certBuf, &certBufSz); ret = wolfSSH_CTX_AddRootCert_buffer(ctx, certBuf, certBufSz, WOLFSSH_FORMAT_PEM); if (ret != 0) { + #ifdef WOLFSSH_SMALL_STACK + wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); + WFREE(keyLoadBuf, NULL, 0); + #endif + WFREE(certBuf, NULL, 0); ES_ERROR("Couldn't add root cert\n"); } WFREE(certBuf, NULL, 0); @@ -3001,6 +3044,7 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args) #endif /* WOLFSSH_ALLOW_USERAUTH_NONE */ #ifdef WOLFSSH_SMALL_STACK + wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); WFREE(keyLoadBuf, NULL, 0); #endif } diff --git a/src/internal.c b/src/internal.c index 8b7fd28cc..f4e052c76 100644 --- a/src/internal.c +++ b/src/internal.c @@ -12835,7 +12835,9 @@ static int SendKexGetSigningKey(WOLFSSH* ssh, ssh->handshake->useTpm = ssh->ctx->privateKey[keyIdx].isTpm; #endif - switch (sigKeyBlock_ptr->pubKeyId) { + /* Dispatches on pubKeyFmtId to sync with SendKexDhReply's free chain. + * ID_RSA_SHA2_256/512 already collapse to ID_SSH_RSA. */ + switch (sigKeyBlock_ptr->pubKeyFmtId) { #ifndef WOLFSSH_NO_RSA #ifdef WOLFSSH_CERTS case ID_X509V3_SSH_RSA: @@ -12843,8 +12845,6 @@ static int SendKexGetSigningKey(WOLFSSH* ssh, FALL_THROUGH; #endif case ID_SSH_RSA: - case ID_RSA_SHA2_256: - case ID_RSA_SHA2_512: /* Decode the user-configured RSA private key. */ sigKeyBlock_ptr->sk.rsa.eSz = (word32)sizeof(sigKeyBlock_ptr->sk.rsa.e); @@ -14594,44 +14594,44 @@ int SendKexDhReply(WOLFSSH* ssh) ret = SignH(ssh, sig_ptr, &sigSz, sigKeyBlock_ptr); } + /* Note: this free chain uses pubKeyFmtId, but it must structurally mirror + * the initialization switch in SendKexGetSigningKey which dispatches on pubKeyFmtId. */ if (sigKeyBlock_ptr != NULL) { - if (sigKeyBlock_ptr->pubKeyId == ID_SSH_RSA - || sigKeyBlock_ptr->pubKeyId == ID_RSA_SHA2_256 - || sigKeyBlock_ptr->pubKeyId == ID_RSA_SHA2_512 + if (sigKeyBlock_ptr->pubKeyFmtId == ID_SSH_RSA #ifdef WOLFSSH_CERTS - || sigKeyBlock_ptr->pubKeyId == ID_X509V3_SSH_RSA + || sigKeyBlock_ptr->pubKeyFmtId == ID_X509V3_SSH_RSA #endif ) { #ifndef WOLFSSH_NO_RSA wc_FreeRsaKey(&sigKeyBlock_ptr->sk.rsa.key); #endif } - else if (sigKeyBlock_ptr->pubKeyId == ID_ECDSA_SHA2_NISTP256 - || sigKeyBlock_ptr->pubKeyId == ID_ECDSA_SHA2_NISTP384 - || sigKeyBlock_ptr->pubKeyId == ID_ECDSA_SHA2_NISTP521 + else if (sigKeyBlock_ptr->pubKeyFmtId == ID_ECDSA_SHA2_NISTP256 + || sigKeyBlock_ptr->pubKeyFmtId == ID_ECDSA_SHA2_NISTP384 + || sigKeyBlock_ptr->pubKeyFmtId == ID_ECDSA_SHA2_NISTP521 #ifdef WOLFSSH_CERTS - || sigKeyBlock_ptr->pubKeyId == ID_X509V3_ECDSA_SHA2_NISTP256 - || sigKeyBlock_ptr->pubKeyId == ID_X509V3_ECDSA_SHA2_NISTP384 - || sigKeyBlock_ptr->pubKeyId == ID_X509V3_ECDSA_SHA2_NISTP521 + || sigKeyBlock_ptr->pubKeyFmtId == ID_X509V3_ECDSA_SHA2_NISTP256 + || sigKeyBlock_ptr->pubKeyFmtId == ID_X509V3_ECDSA_SHA2_NISTP384 + || sigKeyBlock_ptr->pubKeyFmtId == ID_X509V3_ECDSA_SHA2_NISTP521 #endif ) { #ifndef WOLFSSH_NO_ECDSA wc_ecc_free(&sigKeyBlock_ptr->sk.ecc.key); #endif } - else if (sigKeyBlock_ptr->pubKeyId == ID_ED25519) { + else if (sigKeyBlock_ptr->pubKeyFmtId == ID_ED25519) { #if !defined(WOLFSSH_NO_ED25519) wc_ed25519_free(&sigKeyBlock_ptr->sk.ed.key); #endif } #if !defined(WOLFSSH_NO_MLDSA) - else if (sigKeyBlock_ptr->pubKeyId == ID_MLDSA44 || - sigKeyBlock_ptr->pubKeyId == ID_MLDSA65 || - sigKeyBlock_ptr->pubKeyId == ID_MLDSA87 + else if (sigKeyBlock_ptr->pubKeyFmtId == ID_MLDSA44 || + sigKeyBlock_ptr->pubKeyFmtId == ID_MLDSA65 || + sigKeyBlock_ptr->pubKeyFmtId == ID_MLDSA87 #ifdef WOLFSSH_CERTS - || sigKeyBlock_ptr->pubKeyId == ID_X509V3_MLDSA44 - || sigKeyBlock_ptr->pubKeyId == ID_X509V3_MLDSA65 - || sigKeyBlock_ptr->pubKeyId == ID_X509V3_MLDSA87 + || sigKeyBlock_ptr->pubKeyFmtId == ID_X509V3_MLDSA44 + || sigKeyBlock_ptr->pubKeyFmtId == ID_X509V3_MLDSA65 + || sigKeyBlock_ptr->pubKeyFmtId == ID_X509V3_MLDSA87 #endif ) { wc_MlDsaKey_Free(&sigKeyBlock_ptr->sk.mldsa.key); diff --git a/tests/auth.c b/tests/auth.c index ec2890bde..83b1dd4c6 100644 --- a/tests/auth.c +++ b/tests/auth.c @@ -1602,11 +1602,14 @@ static THREAD_RETURN WOLFSSH_THREAD server_thread(void* args) promptData.promptLengths = (word32*)WMALLOC(sizeof(word32) * kbResponseCount, NULL, 0); if (promptData.promptLengths == NULL) { + WFREE(promptData.prompts, NULL, 0); ES_ERROR("Could not allocate promptLengths"); } promptData.promptEcho = (byte*)WMALLOC(sizeof(byte) * kbResponseCount, NULL, 0); if (promptData.promptEcho == NULL) { + WFREE(promptData.prompts, NULL, 0); + WFREE(promptData.promptLengths, NULL, 0); ES_ERROR("Could not allocate promptEcho"); } for (word32 prompt = 0; prompt < kbResponseCount; prompt++) { diff --git a/tests/testsuite.c b/tests/testsuite.c index f7111920d..68fd73ecb 100644 --- a/tests/testsuite.c +++ b/tests/testsuite.c @@ -91,7 +91,7 @@ static int tsClientUserAuth(byte authType, WS_UserAuthData* authData, void* ctx) #define NUMARGS 5 -#define ARGLEN 32 +#define ARGLEN 64 static void wolfSSH_EchoTest(void) { @@ -150,6 +150,79 @@ static void wolfSSH_EchoTest(void) FreeTcpReady(&ready); } + +static void wolfSSH_EchoTest_MultiPubKey(void) +{ + tcp_ready ready; + THREAD_TYPE serverThread; + func_args serverArgs; + func_args clientArgs; + char sA[15][ARGLEN]; + char *serverArgv[15] = + { sA[0], sA[1], sA[2], sA[3], sA[4], sA[5], sA[6], sA[7], sA[8], sA[9], sA[10] }; + char cA[15][ARGLEN]; + char *clientArgv[15] = + { cA[0], cA[1], cA[2], cA[3], cA[4], cA[5], cA[6], cA[7], cA[8], cA[9], cA[10], cA[11], cA[12] }; + int serverArgc = 0; + int clientArgc = 0; + + InitTcpReady(&ready); + + WSTRNCPY(serverArgv[serverArgc++], "echoserver", ARGLEN); + WSTRNCPY(serverArgv[serverArgc++], "-1", ARGLEN); + WSTRNCPY(serverArgv[serverArgc++], "-f", ARGLEN); + #if !defined(USE_WINDOWS_API) && !defined(WOLFSSH_ZEPHYR) + WSTRNCPY(serverArgv[serverArgc++], "-p", ARGLEN); + WSTRNCPY(serverArgv[serverArgc++], "-0", ARGLEN); + #endif + WSTRNCPY(serverArgv[serverArgc++], "-I", ARGLEN); + WSTRNCPY(serverArgv[serverArgc++], "hansel:./keys/hansel-key-rsa.pub", ARGLEN); + WSTRNCPY(serverArgv[serverArgc++], "-I", ARGLEN); + WSTRNCPY(serverArgv[serverArgc++], "hansel:./keys/hansel-key-ecc.pub", ARGLEN); + + serverArgs.argc = serverArgc; + serverArgs.argv = serverArgv; + serverArgs.return_code = EXIT_SUCCESS; + serverArgs.signal = &ready; + serverArgs.user_auth = NULL; + ThreadStart(echoserver_test, &serverArgs, &serverThread); + WaitTcpReady(&ready); + + WSTRNCPY(cA[clientArgc++], "client", ARGLEN); + WSTRNCPY(cA[clientArgc++], "-u", ARGLEN); + WSTRNCPY(cA[clientArgc++], "hansel", ARGLEN); + WSTRNCPY(cA[clientArgc++], "-j", ARGLEN); + WSTRNCPY(cA[clientArgc++], "./keys/hansel-key-ecc.pub", ARGLEN); + WSTRNCPY(cA[clientArgc++], "-i", ARGLEN); + WSTRNCPY(cA[clientArgc++], "./keys/hansel-key-ecc.der", ARGLEN); + #if !defined(USE_WINDOWS_API) && !defined(WOLFSSH_ZEPHYR) + WSTRNCPY(cA[clientArgc++], "-p", ARGLEN); + WSNPRINTF(cA[clientArgc++], ARGLEN, "%d", ready.port); + #endif + + clientArgs.argc = clientArgc; + clientArgs.argv = clientArgv; + clientArgs.return_code = EXIT_SUCCESS; + clientArgs.signal = &ready; + clientArgs.user_auth = NULL; + + client_test(&clientArgs); + if (clientArgs.return_code != EXIT_SUCCESS) { + err_sys("client_test failed"); + } + +#ifdef WOLFSSH_ZEPHYR + /* Weird deadlock without this sleep */ + k_sleep(Z_TIMEOUT_TICKS(100)); +#endif + ThreadJoin(serverThread); + if (serverArgs.return_code != EXIT_SUCCESS) { + err_sys("server_test failed"); + } + + FreeTcpReady(&ready); +} + #endif /* WOLFSSH_SHELL */ @@ -181,6 +254,7 @@ int wolfSSH_TestsuiteTest(int argc, char** argv) #ifdef WOLFSSH_SHELL wolfSSH_EchoTest(); + wolfSSH_EchoTest_MultiPubKey(); #endif wolfSSH_Cleanup();