From a2d4f7e1533adf291cbe23ff62900d5ce9097631 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 21 Oct 2026 09:29:28 -0700 Subject: [PATCH 1/2] echoserver: wipe key load buffers in all builds keyLoadBuf held host private keys but was wiped only under WOLFSSH_SMALL_STACK. KeyLoadBufFree() now wipes it in every build and frees it only when it came from the heap. - Route every keyLoadBuf exit through KeyLoadBufFree() - Wipe the ML-DSA key buffer in LoadMlDsaHostKeys() before each free Issue: F-8859, F-10605 --- examples/echoserver/echoserver.c | 120 ++++++++++++------------------- 1 file changed, 45 insertions(+), 75 deletions(-) diff --git a/examples/echoserver/echoserver.c b/examples/echoserver/echoserver.c index ca45977b0..e50977509 100644 --- a/examples/echoserver/echoserver.c +++ b/examples/echoserver/echoserver.c @@ -2638,6 +2638,7 @@ static int LoadMlDsaHostKeys(WOLFSSH_CTX* ctx, const char* keyList) { byte* mldsaBuf; int loaded = 0; + int ret = 0; mldsaBuf = (byte*)WMALLOC(MLDSA_MAX_BOTH_KEY_DER_SIZE, NULL, 0); if (mldsaBuf == NULL) { @@ -2650,14 +2651,14 @@ static int LoadMlDsaHostKeys(WOLFSSH_CTX* ctx, const char* keyList) int mldsaSz = load_key_mldsa44(mldsaBuf, MLDSA_MAX_BOTH_KEY_DER_SIZE); if (mldsaSz <= 0) { fprintf(stderr, "Couldn't load ML-DSA-44 key file.\n"); - WFREE(mldsaBuf, NULL, 0); - return -1; + ret = -1; + goto cleanup; } if (wolfSSH_CTX_UsePrivateKey_buffer(ctx, mldsaBuf, (word32)mldsaSz, WOLFSSH_FORMAT_ASN1) < 0) { fprintf(stderr, "Couldn't use ML-DSA-44 key buffer.\n"); - WFREE(mldsaBuf, NULL, 0); - return -1; + ret = -1; + goto cleanup; } loaded++; } @@ -2668,14 +2669,14 @@ static int LoadMlDsaHostKeys(WOLFSSH_CTX* ctx, const char* keyList) int mldsaSz = load_key_mldsa65(mldsaBuf, MLDSA_MAX_BOTH_KEY_DER_SIZE); if (mldsaSz <= 0) { fprintf(stderr, "Couldn't load ML-DSA-65 key file.\n"); - WFREE(mldsaBuf, NULL, 0); - return -1; + ret = -1; + goto cleanup; } if (wolfSSH_CTX_UsePrivateKey_buffer(ctx, mldsaBuf, (word32)mldsaSz, WOLFSSH_FORMAT_ASN1) < 0) { fprintf(stderr, "Couldn't use ML-DSA-65 key buffer.\n"); - WFREE(mldsaBuf, NULL, 0); - return -1; + ret = -1; + goto cleanup; } loaded++; } @@ -2686,26 +2687,29 @@ static int LoadMlDsaHostKeys(WOLFSSH_CTX* ctx, const char* keyList) int mldsaSz = load_key_mldsa87(mldsaBuf, MLDSA_MAX_BOTH_KEY_DER_SIZE); if (mldsaSz <= 0) { fprintf(stderr, "Couldn't load ML-DSA-87 key file.\n"); - WFREE(mldsaBuf, NULL, 0); - return -1; + ret = -1; + goto cleanup; } if (wolfSSH_CTX_UsePrivateKey_buffer(ctx, mldsaBuf, (word32)mldsaSz, WOLFSSH_FORMAT_ASN1) < 0) { fprintf(stderr, "Couldn't use ML-DSA-87 key buffer.\n"); - WFREE(mldsaBuf, NULL, 0); - return -1; + ret = -1; + goto cleanup; } loaded++; } #endif /* WOLFSSH_NO_MLDSA87 */ - WFREE(mldsaBuf, NULL, 0); if (loaded == 0) { fprintf(stderr, "ML-DSA key list '%s' matched no supported level.\n", keyList); - return -1; + ret = -1; } - return 0; + +cleanup: + wc_ForceZero(mldsaBuf, MLDSA_MAX_BOTH_KEY_DER_SIZE); + WFREE(mldsaBuf, NULL, 0); + return ret; } #endif /* WOLFSSH_NO_MLDSA */ @@ -3648,6 +3652,17 @@ static int SetDefaultSftpPath(WOLFSSH* ssh, const char* defaultSftpPath, #endif +/* Wipes the key load buffer, which held host private keys, and frees it + * when it came from the heap. */ +static void KeyLoadBufFree(byte* buf) +{ + wc_ForceZero(buf, EXAMPLE_KEYLOAD_BUFFER_SZ); +#ifdef WOLFSSH_SMALL_STACK + WFREE(buf, NULL, 0); +#endif +} + + static void ShowUsage(void) { printf("echoserver %s linked with wolfSSL %s\n", LIBWOLFSSH_VERSION_STRING, @@ -4253,10 +4268,7 @@ 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 + KeyLoadBufFree(keyLoadBuf); ES_ERROR("Couldn't load TPM host key from %s.\n", tpmHostKeyPath); } @@ -4289,10 +4301,7 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args) } } if (storeErr != NULL) { - #ifdef WOLFSSH_SMALL_STACK - wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); - WFREE(keyLoadBuf, NULL, 0); - #endif + KeyLoadBufFree(keyLoadBuf); #ifdef WOLFSSH_KEYBOARD_INTERACTIVE if (kbAuthData.promptCount > 0) { WFREE(kbAuthData.promptLengths, NULL, 0); @@ -4313,18 +4322,12 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args) #if !defined(WOLFSSH_NO_RSA) || !defined(WOLFSSH_NO_ECDSA) 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 + KeyLoadBufFree(keyLoadBuf); 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 + KeyLoadBufFree(keyLoadBuf); ES_ERROR("Couldn't use first key buffer.\n"); } @@ -4334,18 +4337,12 @@ 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 + KeyLoadBufFree(keyLoadBuf); 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 + KeyLoadBufFree(keyLoadBuf); ES_ERROR("Couldn't use second key buffer.\n"); } #endif @@ -4355,18 +4352,12 @@ 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 + KeyLoadBufFree(keyLoadBuf); 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 + KeyLoadBufFree(keyLoadBuf); ES_ERROR("Couldn't use Ed25519 key buffer.\n"); } #endif /* WOLFSSH_NO_ED25519 */ @@ -4391,10 +4382,7 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args) } if (mldsaErr) { - #ifdef WOLFSSH_SMALL_STACK - wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); - WFREE(keyLoadBuf, NULL, 0); - #endif + KeyLoadBufFree(keyLoadBuf); ES_ERROR("Error loading ML-DSA host keys.\n"); } else { @@ -4410,10 +4398,7 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args) mldsaMatched = 1; if (LoadMlDsaCompositeHostKey(ctx, entry->fileName, entry->label) != 0) { - #ifdef WOLFSSH_SMALL_STACK - wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); - WFREE(keyLoadBuf, NULL, 0); - #endif + KeyLoadBufFree(keyLoadBuf); ES_ERROR("Error loading %s host key.\n", entry->label); } @@ -4421,10 +4406,7 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args) } if (!mldsaMatched) { - #ifdef WOLFSSH_SMALL_STACK - wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); - WFREE(keyLoadBuf, NULL, 0); - #endif + KeyLoadBufFree(keyLoadBuf); ES_ERROR("ML-DSA key list '%s' matched no supported " "level.\n", keyList); } @@ -4442,19 +4424,13 @@ 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 + KeyLoadBufFree(keyLoadBuf); 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 + KeyLoadBufFree(keyLoadBuf); ES_ERROR("WMALLOC failed\n"); } load_file(userPubKey, userBuf, &userBufSz); @@ -4468,10 +4444,7 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args) if (caCert) { /* PEM or DER is detected from the file's content. */ if (wolfSSH_CTX_AddRootCert_file(ctx, caCert) != WS_SUCCESS) { - #ifdef WOLFSSH_SMALL_STACK - wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); - WFREE(keyLoadBuf, NULL, 0); - #endif + KeyLoadBufFree(keyLoadBuf); ES_ERROR("Couldn't add root cert\n"); } } @@ -4518,10 +4491,7 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args) LoadNoneBuffer(keyLoadBuf, bufSz, &pwMapList); #endif /* WOLFSSH_ALLOW_USERAUTH_NONE */ - #ifdef WOLFSSH_SMALL_STACK - wc_ForceZero(keyLoadBuf, EXAMPLE_KEYLOAD_BUFFER_SZ); - WFREE(keyLoadBuf, NULL, 0); - #endif + KeyLoadBufFree(keyLoadBuf); } #ifdef WOLFSSL_NUCLEUS { From 852495741409a21b4f64ac662a03323c072d896a Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 21 Oct 2026 09:29:28 -0700 Subject: [PATCH 2/2] echoserver: bound the key list parse and copies LoadPublicKeyBuffer() searches for its delimiters only up to the end of the buffer, so a -j file without a final newline is rejected instead of read past. The sample password, public key, and none lists are copied into keyLoadBuf only when they fit. - Add FindDelimiter() in place of the unbounded WSTRCHR() calls - Add CopyToKeyLoadBuf(), which refuses a string with no room for its terminator and still accepts an empty one - Warn when a -j file's keys are not all loaded - Fix the comment claiming every sample key is ssh-rsa Issue: F-10600, F-10565, F-14765 --- examples/echoserver/echoserver.c | 67 +++++++++++++++++++++++--------- 1 file changed, 48 insertions(+), 19 deletions(-) diff --git a/examples/echoserver/echoserver.c b/examples/echoserver/echoserver.c index e50977509..c82ca2c85 100644 --- a/examples/echoserver/echoserver.c +++ b/examples/echoserver/echoserver.c @@ -2971,6 +2971,17 @@ static int LoadPasswordBuffer(byte* buf, word32 bufSz, PwMapList* list) } +/* Finds c in [str, end). Returns NULL when it is not there. */ +static char* FindDelimiter(char* str, const char* end, char c) +{ + for (; str < end; str++) { + if (*str == c) + return str; + } + return NULL; +} + + static int LoadPublicKeyBuffer(byte* buf, word32 bufSz, PwMapList* list) { char* str = (char*)buf; @@ -2985,7 +2996,8 @@ static int LoadPublicKeyBuffer(byte* buf, word32 bufSz, PwMapList* list) /* Each line of passwd.txt is in the format * ssh-rsa AAAB3BASE64ENCODEDPUBLICKEYBLOB username\n - * This function modifies the passed-in buffer. */ + * This function modifies the passed-in buffer, which need not be + * NUL-terminated. */ if (list == NULL) return -1; @@ -2993,25 +3005,22 @@ static int LoadPublicKeyBuffer(byte* buf, word32 bufSz, PwMapList* list) return 0; while (str < end && *str != 0) { - /* Skip the public key type. This example will always be ssh-rsa. */ - delimiter = WSTRCHR(str, ' '); + /* Skip the OpenSSH public-key type; the decoded blob carries the + * algorithm. */ + delimiter = FindDelimiter(str, end, ' '); if (delimiter == NULL) { return -1; } - if (str >= end) - break; str = delimiter + 1; - delimiter = WSTRCHR(str, ' '); + delimiter = FindDelimiter(str, end, ' '); if (delimiter == NULL) { return -1; } publicKey64 = (byte*)str; *delimiter = 0; publicKey64Sz = (word32)(delimiter - str); - if (str >= end) - break; str = delimiter + 1; - delimiter = WSTRCHR(str, '\n'); + delimiter = FindDelimiter(str, end, '\n'); if (delimiter == NULL) { return -1; } @@ -3663,6 +3672,20 @@ static void KeyLoadBufFree(byte* buf) } +/* Copies str and its terminator into the key load buffer and sets *sz to + * the length of str. Returns 0, or -1 when it does not fit. */ +static int CopyToKeyLoadBuf(byte* buf, const char* str, word32* sz) +{ + word32 len = (word32)WSTRLEN(str); + + if (len >= EXAMPLE_KEYLOAD_BUFFER_SZ) + return -1; + WMEMCPY(buf, str, len + 1); + *sz = len; + return 0; +} + + static void ShowUsage(void) { printf("echoserver %s linked with wolfSSL %s\n", LIBWOLFSSH_VERSION_STRING, @@ -4434,7 +4457,10 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args) ES_ERROR("WMALLOC failed\n"); } load_file(userPubKey, userBuf, &userBufSz); - LoadPublicKeyBuffer(userBuf, userBufSz, &pwMapList); + if (LoadPublicKeyBuffer(userBuf, userBufSz, &pwMapList) != 0) { + fprintf(stderr, "Couldn't load every key in %s.\n", + userPubKey); + } WFREE(userBuf, NULL, 0); } #endif @@ -4450,9 +4476,10 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args) } #endif - bufSz = (word32)WSTRLEN(samplePasswordBuffer); - WMEMCPY(keyLoadBuf, samplePasswordBuffer, bufSz); - keyLoadBuf[bufSz] = 0; + if (CopyToKeyLoadBuf(keyLoadBuf, samplePasswordBuffer, &bufSz) != 0) { + KeyLoadBufFree(keyLoadBuf); + ES_ERROR("Sample password list doesn't fit.\n"); + } LoadPasswordBuffer(keyLoadBuf, bufSz, &pwMapList); #if defined(WOLFSSH_NO_RSA) && defined(WOLFSSH_NO_ECDSA) @@ -4478,16 +4505,18 @@ THREAD_RETURN WOLFSSH_THREAD echoserver_test(void* args) } #endif if (bufName != NULL) { - bufSz = (word32)WSTRLEN(bufName); - WMEMCPY(keyLoadBuf, bufName, bufSz); - keyLoadBuf[bufSz] = 0; + if (CopyToKeyLoadBuf(keyLoadBuf, bufName, &bufSz) != 0) { + KeyLoadBufFree(keyLoadBuf); + ES_ERROR("Sample public key list doesn't fit.\n"); + } LoadPublicKeyBuffer(keyLoadBuf, bufSz, &pwMapList); } #ifdef WOLFSSH_ALLOW_USERAUTH_NONE - bufSz = (word32)WSTRLEN(sampleNoneBuffer); - WMEMCPY(keyLoadBuf, sampleNoneBuffer, bufSz); - keyLoadBuf[bufSz] = 0; + if (CopyToKeyLoadBuf(keyLoadBuf, sampleNoneBuffer, &bufSz) != 0) { + KeyLoadBufFree(keyLoadBuf); + ES_ERROR("Sample none list doesn't fit.\n"); + } LoadNoneBuffer(keyLoadBuf, bufSz, &pwMapList); #endif /* WOLFSSH_ALLOW_USERAUTH_NONE */