From 1752b5e5c0d97ed84b0ab913c45137740810fa70 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 16 Jul 2026 10:05:47 -0700 Subject: [PATCH] Use wolfCrypt's wc_ForceZero() - Keep a gated fallback, wolfSSH_ForceZero(). - Add macro WS_FORCEZERO gated between wc_ForceZero() or wolfSSH_ForceZero(). - Use WS_FORCEZERO() in place of the local ForceZero(). - Change the fallback's length parameter from word32 to size_t to match wc_ForceZero()'s signature. --- apps/wolfsshd/auth.c | 4 +-- src/agent.c | 4 +-- src/internal.c | 60 ++++++++++++++++++++++---------------------- src/misc.c | 6 +++-- src/ssh.c | 6 ++--- src/wolfsftp.c | 6 ++--- tests/unit.c | 16 ++++++------ wolfssh/misc.h | 20 ++++++++++++++- 8 files changed, 71 insertions(+), 51 deletions(-) diff --git a/apps/wolfsshd/auth.c b/apps/wolfsshd/auth.c index 793ee2dcc..a72873d1f 100644 --- a/apps/wolfsshd/auth.c +++ b/apps/wolfsshd/auth.c @@ -487,11 +487,11 @@ static int CheckPasswordUnix(const char* usr, const byte* pw, word32 pwSz, WOLFS } if (pwStr != NULL) { - ForceZero(pwStr, pwSz + 1); + WS_FORCEZERO(pwStr, pwSz + 1); WFREE(pwStr, NULL, DYNTYPE_STRING); } if (storedHashCpy != NULL) { - ForceZero(storedHashCpy, (word32)WSTRLEN(storedHashCpy) + 1); + WS_FORCEZERO(storedHashCpy, (word32)WSTRLEN(storedHashCpy) + 1); WFREE(storedHashCpy, NULL, DYNTYPE_STRING); } diff --git a/src/agent.c b/src/agent.c index 85766fb21..b83c2670a 100644 --- a/src/agent.c +++ b/src/agent.c @@ -392,7 +392,7 @@ static int PostLock(WOLFSSH_AGENT_CTX* agent, #else WLOG(WS_LOG_AGENT, "Locking with passphrase"); #endif - ForceZero(pp, passphraseSz); + WS_FORCEZERO(pp, passphraseSz); WFREE(pp, agent->heap, DYNTYPE_STRING); } @@ -421,7 +421,7 @@ static int PostUnlock(WOLFSSH_AGENT_CTX* agent, #else WLOG(WS_LOG_AGENT, "Unlocking with passphrase"); #endif - ForceZero(pp, passphraseSz); + WS_FORCEZERO(pp, passphraseSz); WFREE(pp, agent->heap, DYNTYPE_STRING); } diff --git a/src/internal.c b/src/internal.c index e2e73a73e..bb9d9a186 100644 --- a/src/internal.c +++ b/src/internal.c @@ -659,7 +659,7 @@ static void HandshakeInfoFree(HandshakeInfo* hs, void* heap) if (hs->kexHashId != WC_HASH_TYPE_NONE) { wc_HashFree(&hs->kexHash, (enum wc_HashType)hs->kexHashId); } - ForceZero(hs, sizeof(HandshakeInfo)); + WS_FORCEZERO(hs, sizeof(HandshakeInfo)); WFREE(hs, heap, DYNTYPE_HS); } } @@ -1160,7 +1160,7 @@ void CtxResourceFree(WOLFSSH_CTX* ctx) for (i = 0; i < ctx->privateKeyCount; i++) { if (ctx->privateKey[i].key != NULL) { - ForceZero(ctx->privateKey[i].key, ctx->privateKey[i].keySz); + WS_FORCEZERO(ctx->privateKey[i].key, ctx->privateKey[i].keySz); WFREE(ctx->privateKey[i].key, ctx->heap, DYNTYPE_PRIVKEY); ctx->privateKey[i].key = NULL; ctx->privateKey[i].keySz = 0; @@ -1554,10 +1554,10 @@ void SshResourceFree(WOLFSSH* ssh, void* heap) ShrinkBuffer(&ssh->inputBuffer, 1); ShrinkBuffer(&ssh->outputBuffer, 1); ShrinkBuffer(&ssh->extDataBuffer, 1); - ForceZero(ssh->k, ssh->kSz); + WS_FORCEZERO(ssh->k, ssh->kSz); HandshakeInfoFree(ssh->handshake, heap); - ForceZero(&ssh->keys, sizeof(Keys)); - ForceZero(&ssh->peerKeys, sizeof(Keys)); + WS_FORCEZERO(&ssh->keys, sizeof(Keys)); + WS_FORCEZERO(&ssh->peerKeys, sizeof(Keys)); if (ssh->rng) { wc_FreeRng(ssh->rng); WFREE(ssh->rng, heap, DYNTYPE_RNG); @@ -1590,7 +1590,7 @@ void SshResourceFree(WOLFSSH* ssh, void* heap) ssh->scpConfirmMsgSz = 0; } if (ssh->scpFileBuffer) { - ForceZero(ssh->scpFileBuffer, ssh->scpFileBufferSz); + WS_FORCEZERO(ssh->scpFileBuffer, ssh->scpFileBufferSz); WFREE(ssh->scpFileBuffer, heap, DYNTYPE_BUFFER); ssh->scpFileBuffer = NULL; ssh->scpFileBufferSz = 0; @@ -2674,7 +2674,7 @@ static int UpdateHostCertificates(WOLFSSH_CTX* ctx, * software key on the certificate slot and mark it TPM-backed. */ if (keyIsTpm) { if (ctx->privateKey[certHint].key != NULL) { - ForceZero(ctx->privateKey[certHint].key, + WS_FORCEZERO(ctx->privateKey[certHint].key, ctx->privateKey[certHint].keySz); WFREE(ctx->privateKey[certHint].key, ctx->heap, DYNTYPE_PRIVKEY); @@ -2698,7 +2698,7 @@ static int UpdateHostCertificates(WOLFSSH_CTX* ctx, WMEMCPY(key, ctx->privateKey[keyHint].key, keySz); if (ctx->privateKey[certHint].key != NULL) { - ForceZero(ctx->privateKey[certHint].key, + WS_FORCEZERO(ctx->privateKey[certHint].key, ctx->privateKey[certHint].keySz); WFREE(ctx->privateKey[certHint].key, ctx->heap, DYNTYPE_PRIVKEY); @@ -2801,7 +2801,7 @@ static int SetHostPrivateKey(WOLFSSH_CTX* ctx, if (pvtKey->publicKeyFmt == keyId) { if (pvtKey->key != NULL) { - ForceZero(pvtKey->key, pvtKey->keySz); + WS_FORCEZERO(pvtKey->key, pvtKey->keySz); WFREE(pvtKey->key, ctx->heap, dynamicType); } } @@ -2859,7 +2859,7 @@ int wolfSSH_SetHostTpmKey(WOLFSSH_CTX* ctx, byte keyId) pvtKey->publicKeyFmt = keyId; } else if (pvtKey->key != NULL) { - ForceZero(pvtKey->key, pvtKey->keySz); + WS_FORCEZERO(pvtKey->key, pvtKey->keySz); WFREE(pvtKey->key, ctx->heap, DYNTYPE_PRIVKEY); } @@ -2874,7 +2874,7 @@ int wolfSSH_SetHostTpmKey(WOLFSSH_CTX* ctx, byte keyId) for (certIdx = 0; certIdx < ctx->privateKeyCount; certIdx++) { if (ctx->privateKey[certIdx].publicKeyFmt == certId) { if (ctx->privateKey[certIdx].key != NULL) { - ForceZero(ctx->privateKey[certIdx].key, + WS_FORCEZERO(ctx->privateKey[certIdx].key, ctx->privateKey[certIdx].keySz); WFREE(ctx->privateKey[certIdx].key, ctx->heap, DYNTYPE_PRIVKEY); @@ -2961,7 +2961,7 @@ int wolfSSH_ProcessBuffer(WOLFSSH_CTX* ctx, if (type == BUFTYPE_PRIVKEY) { /* wc_KeyPemToDer may have written partial key material; * zeroize before free on the private-key path. */ - ForceZero(der, inSz); + WS_FORCEZERO(der, inSz); } WFREE(der, heap, dynamicType); return WS_BAD_FILE_E; @@ -2979,7 +2979,7 @@ int wolfSSH_ProcessBuffer(WOLFSSH_CTX* ctx, ret = IdentifyAsn1Key(der, derSz, 1, ctx->heap, NULL); if (ret < 0) { if (der != NULL) { - ForceZero(der, derSz); + WS_FORCEZERO(der, derSz); WFREE(der, heap, dynamicType); } return ret; @@ -3108,7 +3108,7 @@ int GenerateKey(byte hashId, byte keyId, ret = wc_HashFinal(&hash, enmhashId, lastBlock); if (ret == WS_SUCCESS) WMEMCPY(key, lastBlock, remainder); - ForceZero(lastBlock, sizeof(lastBlock)); + WS_FORCEZERO(lastBlock, sizeof(lastBlock)); } } else { @@ -3154,7 +3154,7 @@ int GenerateKey(byte hashId, byte keyId, ret = wc_HashFinal(&hash, enmhashId, lastBlock); if (ret == WS_SUCCESS) WMEMCPY(key + runningKeySz, lastBlock, remainder); - ForceZero(lastBlock, sizeof(lastBlock)); + WS_FORCEZERO(lastBlock, sizeof(lastBlock)); } } } @@ -6315,7 +6315,7 @@ static int KeyAgreeDh_client(WOLFSSH* ssh, byte hashId, ret = WS_CRYPTO_FAILED; } } - ForceZero(ssh->handshake->x, ssh->handshake->xSz); + WS_FORCEZERO(ssh->handshake->x, ssh->handshake->xSz); wc_FreeDhKey(&ssh->handshake->privKey.dh); WLOG(WS_LOG_DEBUG, "Leaving KeyAgreeDh_client(), ret = %d", ret); @@ -6709,7 +6709,7 @@ static int KeyAgreeEcdhMlKem_client(WOLFSSH* ssh, byte hashId, } if (sharedSecretHash) { - ForceZero(sharedSecretHash, sharedSecretHashSz); + WS_FORCEZERO(sharedSecretHash, sharedSecretHashSz); WFREE(sharedSecretHash, ssh->ctx->heap, DYNTYPE_PRIVKEY); } @@ -8980,7 +8980,7 @@ static int DoUserAuthRequestMlDsa(WOLFSSH* ssh, } if (checkData != NULL) { - ForceZero(checkData, checkDataSz); + WS_FORCEZERO(checkData, checkDataSz); WFREE(checkData, ssh->ctx->heap, DYNTYPE_TEMP); } @@ -11541,7 +11541,7 @@ static int DoPacket(WOLFSSH* ssh, byte* bufferConsumed) in += AES_BLOCK_SIZE; sz -= AES_BLOCK_SIZE; } - ForceZero(scratch, sizeof(scratch)); + WS_FORCEZERO(scratch, sizeof(scratch)); return ret; } @@ -13612,7 +13612,7 @@ static int KeyAgreeDh_server(WOLFSSH* ssh, byte hashId, byte* f, word32* fSz) ssh->handshake->e, ssh->handshake->eSz); PRIVATE_KEY_LOCK(); } - ForceZero(y_ptr, ySz); + WS_FORCEZERO(y_ptr, ySz); if (keyInited) wc_FreeDhKey(privKey); } @@ -14081,7 +14081,7 @@ static int KeyAgreeEcdhMlKem_server(WOLFSSH* ssh, byte hashId, } if (sharedSecretHash) { - ForceZero(sharedSecretHash, sharedSecretHashSz); + WS_FORCEZERO(sharedSecretHash, sharedSecretHashSz); WFREE(sharedSecretHash, ssh->ctx->heap, DYNTYPE_PRIVKEY); } @@ -14968,7 +14968,7 @@ int SendKexDhReply(WOLFSSH* ssh) WLOG(WS_LOG_DEBUG, "Leaving SendKexDhReply(), ret = %d", ret); if (sigKeyBlock_ptr) { - ForceZero(sigKeyBlock_ptr, sizeof(struct wolfSSH_sigKeyBlockFull)); + WS_FORCEZERO(sigKeyBlock_ptr, sizeof(struct wolfSSH_sigKeyBlockFull)); WFREE(sigKeyBlock_ptr, heap, DYNTYPE_PRIVKEY); } #ifdef WOLFSSH_SMALL_STACK @@ -16414,7 +16414,7 @@ static int BuildUserAuthRequestRsa(WOLFSSH* ssh, *idx = begin; if (checkData != NULL) { - ForceZero(checkData, checkDataSz); + WS_FORCEZERO(checkData, checkDataSz); WFREE(checkData, ssh->ctx->heap, DYNTYPE_TEMP); } @@ -16594,7 +16594,7 @@ static int BuildUserAuthRequestRsaCert(WOLFSSH* ssh, *idx = begin; if (checkData != NULL) { - ForceZero(checkData, checkDataSz); + WS_FORCEZERO(checkData, checkDataSz); WFREE(checkData, ssh->ctx->heap, DYNTYPE_TEMP); } @@ -16861,7 +16861,7 @@ static int BuildUserAuthRequestEcc(WOLFSSH* ssh, *idx = begin; if (checkData != NULL) { - ForceZero(checkData, checkDataSz); + WS_FORCEZERO(checkData, checkDataSz); WFREE(checkData, ssh->ctx->heap, DYNTYPE_TEMP); } @@ -17121,7 +17121,7 @@ static int BuildUserAuthRequestEccCert(WOLFSSH* ssh, *idx = begin; if (checkData != NULL) { - ForceZero(checkData, checkDataSz); + WS_FORCEZERO(checkData, checkDataSz); WFREE(checkData, ssh->ctx->heap, DYNTYPE_TEMP); } @@ -17276,7 +17276,7 @@ static int BuildUserAuthRequestEd25519(WOLFSSH* ssh, *idx = begin; if (checkData != NULL) { - ForceZero(checkData, checkDataSz); + WS_FORCEZERO(checkData, checkDataSz); WFREE(checkData, keySig->heap, DYNTYPE_TEMP); } @@ -17523,7 +17523,7 @@ static int BuildUserAuthRequestMlDsa(WOLFSSH* ssh, *idx = begin; if (checkData != NULL) { - ForceZero(checkData, checkDataSz); + WS_FORCEZERO(checkData, checkDataSz); WFREE(checkData, keySig->heap, DYNTYPE_TEMP); } @@ -17936,7 +17936,7 @@ int SendUserAuthKeyboardResponse(WOLFSSH* ssh) if (ret != WS_WANT_WRITE && ret != WS_SUCCESS) PurgePacket(ssh); - ForceZero(&authData, sizeof(WS_UserAuthData)); + WS_FORCEZERO(&authData, sizeof(WS_UserAuthData)); WLOG(WS_LOG_DEBUG, "Leaving SendUserAuthKeyboardResponse(), ret = %d", ret); @@ -18145,7 +18145,7 @@ int SendUserAuthRequest(WOLFSSH* ssh, byte authType, int addSig) if (ret != WS_WANT_WRITE && ret != WS_SUCCESS) PurgePacket(ssh); - ForceZero(&authData, sizeof(WS_UserAuthData)); + WS_FORCEZERO(&authData, sizeof(WS_UserAuthData)); WLOG(WS_LOG_DEBUG, "Leaving SendUserAuthRequest(), ret = %d", ret); if (keySig_ptr) diff --git a/src/misc.c b/src/misc.c index 6af986701..c22bc929e 100644 --- a/src/misc.c +++ b/src/misc.c @@ -98,13 +98,15 @@ STATIC INLINE void c32toa(word32 u32, byte* c) } +#ifdef WOLFSSH_NO_FORCEZERO /* Make sure compiler doesn't skip */ -STATIC INLINE void ForceZero(void* mem, word32 length) +STATIC INLINE void wolfSSH_ForceZero(void* mem, size_t len) { volatile byte* z = (volatile byte*)mem; - while (length--) *z++ = 0; + while (len--) *z++ = 0; } +#endif /* check all length bytes for equality, return 0 on success */ diff --git a/src/ssh.c b/src/ssh.c index 4d8bd60b9..bfb8992b9 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -2054,7 +2054,7 @@ static int DoPemKey(const byte* in, word32 inSz, byte** out, } else { WLOG(WS_LOG_DEBUG, "Unable to identify PEM key"); - ForceZero(newKey, newKeySz); + WS_FORCEZERO(newKey, newKeySz); if (*out == NULL) { WFREE(newKey, heap, DYNTYPE_PRIVKEY); } @@ -2138,7 +2138,7 @@ static int DoOpenSshKey(const byte* in, word32 inSz, byte** out, } else { WLOG(WS_LOG_DEBUG, "Unable to identify key"); - ForceZero(newKey, newKeySz); + WS_FORCEZERO(newKey, newKeySz); if (*out == NULL) { WFREE(newKey, heap, DYNTYPE_PRIVKEY); } @@ -2295,7 +2295,7 @@ int wolfSSH_ReadKey_file(const char* name, } WFCLOSE(NULL, file); - ForceZero(in, inSz); + WS_FORCEZERO(in, inSz); WFREE(in, heap, DYNTYPE_FILE); return ret; diff --git a/src/wolfsftp.c b/src/wolfsftp.c index 9d4671834..b66389bc2 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -473,7 +473,7 @@ static int wolfSSH_SFTP_buffer_set_size(WS_SFTP_BUFFER* buffer, word32 sz) #ifndef WOLFSSH_NO_SFTP_BUFFER_ZERO /* wipe any payload in the region being trimmed off before shrinking */ if (buffer->data != NULL && sz < buffer->sz) { - ForceZero(buffer->data + sz, buffer->sz - sz); + WS_FORCEZERO(buffer->data + sz, buffer->sz - sz); } #endif buffer->sz = sz; @@ -801,7 +801,7 @@ static void wolfSSH_SFTP_buffer_free(WOLFSSH* ssh, WS_SFTP_BUFFER* buffer) if (ssh != NULL && buffer != NULL) { if (buffer->data != NULL) { #ifndef WOLFSSH_NO_SFTP_BUFFER_ZERO - ForceZero(buffer->data, buffer->sz); + WS_FORCEZERO(buffer->data, buffer->sz); #endif WFREE(buffer->data, ssh->ctx->heap, DYNTYPE_BUFFER); buffer->data = NULL; @@ -1434,7 +1434,7 @@ static void wolfSSH_SFTP_RecvSetSend(WOLFSSH* ssh, byte* buf, int sz) /* free up existing data if needed */ if (buf != state->buffer.data && state->buffer.data != NULL) { #ifndef WOLFSSH_NO_SFTP_BUFFER_ZERO - ForceZero(state->buffer.data, state->buffer.sz); + WS_FORCEZERO(state->buffer.data, state->buffer.sz); #endif WFREE(state->buffer.data, ssh->ctx->heap, DYNTYPE_BUFFER); state->buffer.data = NULL; diff --git a/tests/unit.c b/tests/unit.c index 97ef16205..c6601d755 100644 --- a/tests/unit.c +++ b/tests/unit.c @@ -6398,7 +6398,7 @@ static void DrainRetained(void) * - ssh->k: the DH/ECDH shared secret * - ssh->keys: active session encryption + MAC keys (our direction) * - ssh->peerKeys: active session encryption + MAC keys (peer direction) - * Mutation testing flagged each ForceZero in SshResourceFree as having no + * Mutation testing flagged each WS_FORCEZERO in SshResourceFree as having no * coverage; removing any of them would leave key material in heap memory * after wolfSSH_free. To inspect the bytes after free without touching * freed memory, the test installs the retain-on-free allocator just @@ -6485,7 +6485,7 @@ static int test_SshResourceFree_zeroesSecrets(void) #ifndef WOLFSSH_NO_DH /* Verify KeyAgreeDh_client zeroes the ephemeral DH private key - * ssh->handshake->x before returning. The ForceZero is unconditional in + * ssh->handshake->x before returning. The WS_FORCEZERO is unconditional in * the function (runs even if wc_DhAgree fails), so the test does not need * to feed a valid peer public key - it just needs to observe that x is * wiped after the call returns. The test hook wolfSSH_TestKeyAgreeDh_client @@ -6529,7 +6529,7 @@ static int test_KeyAgreeDh_client_zeroesEphemeralPrivKey(void) hs->xSz = markedSz; /* No prime group is set, so wc_DhCheckPubKey fails before wc_DhAgree is - * reached. The ForceZero on x is unconditional and runs regardless. */ + * reached. The WS_FORCEZERO on x is unconditional and runs regardless. */ WMEMSET(bogusF, 0xCC, sizeof(bogusF)); (void)wolfSSH_TestKeyAgreeDh_client(ssh, WC_HASH_TYPE_SHA256, bogusF, (word32)sizeof(bogusF)); @@ -6765,12 +6765,12 @@ static void DrainCaptured(void) * * wc_DhGenerateKeyPair writes only the leading ySz bytes of the * MAX_KEX_KEY_SZ allocation (ySz is typically the prime-group size, well - * below MAX_KEX_KEY_SZ), and ForceZero only wipes those same ySz bytes - + * below MAX_KEX_KEY_SZ), and WS_FORCEZERO only wipes those same ySz bytes - * so the tail of the buffer is never written by the function under test. * The capture allocator stamps every fresh allocation with 0xCC so that * after the call: - * - present ForceZero -> [0x00 * ySz] [0xCC * (MAX - ySz)] - * - removed ForceZero -> [priv-key * ySz] [0xCC * (MAX - ySz)] + * - present WS_FORCEZERO -> [0x00 * ySz] [0xCC * (MAX - ySz)] + * - removed WS_FORCEZERO -> [priv-key * ySz] [0xCC * (MAX - ySz)] * The check requires a captured MAX_KEX_KEY_SZ buffer whose bytes are all * either 0x00 or 0xCC AND that contains at least one 0x00. The DH private * key emitted by wc_DhGenerateKeyPair is overwhelmingly unlikely to be @@ -6779,7 +6779,7 @@ static void DrainCaptured(void) * * The peer value ssh->handshake->e is left zero, so the new * wc_DhCheckPubKey now fails between wc_DhGenerateKeyPair and wc_DhAgree; - * the ForceZero on y_ptr is unconditional and still runs, so the buffer + * the WS_FORCEZERO on y_ptr is unconditional and still runs, so the buffer * assertion above is unaffected. */ static int test_KeyAgreeDh_server_zeroesEphemeralPrivKey(void) { @@ -6978,7 +6978,7 @@ static int test_KeyAgreeDh_client_rejectsOutOfRangePeer(void) result = -754; break; } - /* x is unused on the reject path but ForceZero still runs over it. */ + /* x is unused on the reject path but WS_FORCEZERO still runs over it. */ hs->xSz = 0; badF[0] = badVals[c]; ret = wolfSSH_TestKeyAgreeDh_client(ssh, WC_HASH_TYPE_SHA256, diff --git a/wolfssh/misc.h b/wolfssh/misc.h index 3938fe7ea..3ec47b9dd 100644 --- a/wolfssh/misc.h +++ b/wolfssh/misc.h @@ -29,10 +29,26 @@ #include +#include +#include #include #include +#define WOLFSSL_V5_8_4 0x05008004 + +#if (LIBWOLFSSL_VERSION_HEX < WOLFSSL_V5_8_4) || \ + defined(WOLFSSL_NO_FORCE_ZERO) + #define WOLFSSH_NO_FORCEZERO +#endif + +#ifdef WOLFSSH_NO_FORCEZERO + #define WS_FORCEZERO(mem, len) wolfSSH_ForceZero((mem), (len)) +#else + #define WS_FORCEZERO(mem, len) wc_ForceZero((mem), (len)) +#endif + + #ifdef NO_INLINE @@ -42,8 +58,10 @@ WOLFSSH_LOCAL word32 min(word32 a, word32 b); WOLFSSH_LOCAL void ato32(const byte* c, word32* u32); WOLFSSH_LOCAL void c32toa(word32 u32, byte* c); -WOLFSSH_LOCAL void ForceZero(void* mem, word32 length); WOLFSSH_LOCAL int ConstantCompare(const byte* a, const byte* b, word32 length); +#ifdef WOLFSSH_NO_FORCEZERO +WOLFSSH_LOCAL void wolfSSH_ForceZero(void* mem, size_t len); +#endif #endif /* NO_INLINE */