From 1edee3ca2be7d3388f7485e2bd96cd15fae2b962 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 24 Sep 2026 15:55:24 -0700 Subject: [PATCH] Wipe the KEX shared secret once keys are derived DoKexDhReply() and SendKexDhReply() zero all of ssh->k and reset kSz on every exit, so K does not outlive the key exchange. GenerateKeys() zeroes both handshake key sets when a derivation fails. - the full-buffer wipe clears CreateMpint and ML-KEM hybrid leftovers - tests/regress.c checks K after each KEX, the hybrids included - tests/regress.c checks a failed derivation leaves no keys Issue: F-8845, F-11680, F-11681, F-8843, F-11682, F-14001 --- src/internal.c | 16 ++++++++++++ tests/regress.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+) diff --git a/src/internal.c b/src/internal.c index 3416a7871..b9b324267 100644 --- a/src/internal.c +++ b/src/internal.c @@ -3700,6 +3700,12 @@ static int GenerateKeys(WOLFSSH* ssh, byte hashId, byte doKeyPad) } #endif /* SHOW_SECRETS */ + /* Do not keep a partial key set from a failed derivation. */ + if (ret != WS_SUCCESS && cK != NULL) { + WS_FORCEZERO(cK, sizeof(Keys)); + WS_FORCEZERO(sK, sizeof(Keys)); + } + return ret; } @@ -8914,6 +8920,10 @@ static int DoKexDhReply(WOLFSSH* ssh, byte* buf, word32 len, word32* idx) ret = SendNewKeys(ssh); } + /* K is only needed to derive the keys. Wipe all of it, success or not. */ + WS_FORCEZERO(ssh->k, sizeof(ssh->k)); + ssh->kSz = 0; + if (sigKeyBlock_ptr) WFREE(sigKeyBlock_ptr, ssh->ctx->heap, DYNTYPE_PRIVKEY); /* RFC 4253 11.1: WS_PUBKEY_REJECTED_E here is only the host key check, @@ -18675,6 +18685,12 @@ int SendKexDhReply(WOLFSSH* ssh) if (ret != WS_WANT_WRITE && ret != WS_SUCCESS) PurgePacket(ssh); + /* K is only needed to derive the keys. Wipe all of it, success or not. */ + if (ssh != NULL) { + WS_FORCEZERO(ssh->k, sizeof(ssh->k)); + ssh->kSz = 0; + } + WLOG(WS_LOG_DEBUG, "Leaving SendKexDhReply(), ret = %d", ret); if (sigKeyBlock_ptr) { WS_FORCEZERO(sigKeyBlock_ptr, sizeof(struct wolfSSH_sigKeyBlockFull)); diff --git a/tests/regress.c b/tests/regress.c index 991d03052..997c514d2 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -1500,6 +1500,17 @@ static void RunKexReplyHandshake(KexReplyHarness* harness, result->steps = REGRESS_MAX_HANDSHAKE_STEPS; } +/* The shared secret K does not outlive the key exchange. */ +static void AssertKexSecretWiped(const WOLFSSH* ssh) +{ + word32 i; + + AssertIntEQ(ssh->kSz, 0); + for (i = 0; i < (word32)sizeof(ssh->k); i++) { + AssertIntEQ(ssh->k[i], 0); + } +} + static void AssertHandshakeSucceeds(const char* keyAlgo, const char* keyPath) { KexReplyHarness harness; @@ -1511,6 +1522,8 @@ static void AssertHandshakeSucceeds(const char* keyAlgo, const char* keyPath) AssertTrue(result.clientSuccess); AssertTrue(result.serverSuccess); + AssertKexSecretWiped(harness.client); + AssertKexSecretWiped(harness.server); AssertIntEQ(harness.mutator.mutatedPackets, 0); AssertIntEQ(harness.client->connectState, CONNECT_SERVER_CHANNEL_REQUEST_DONE); AssertIntEQ(harness.server->acceptState, ACCEPT_CLIENT_SESSION_ESTABLISHED); @@ -1919,10 +1932,55 @@ static void AssertHandshakeRejectsCorruptedSig(const char* keyAlgo, AssertTrue(result.clientErr != WS_WANT_READ && result.clientErr != WS_WANT_WRITE); AssertIntEQ(result.clientErr, expectedErr); + /* K was agreed before the verify failed. */ + AssertKexSecretWiped(harness.client); FreeKexReplyHarness(&harness); } +/* Every key agreement, the ML-KEM hybrids included, wipes K on both sides. */ +static void TestKexSecretWipedPerKex(void) +{ + static const char* kexAlgos[] = { +#ifndef WOLFSSH_NO_DH_GROUP14_SHA256 + "diffie-hellman-group14-sha256", +#endif +#ifndef WOLFSSH_NO_ECDH_SHA2_NISTP256 + "ecdh-sha2-nistp256", +#endif +#ifndef WOLFSSH_NO_CURVE25519_SHA256 + "curve25519-sha256", +#endif +#ifndef WOLFSSH_NO_NISTP256_MLKEM768_SHA256 + "mlkem768nistp256-sha256", +#endif +#ifndef WOLFSSH_NO_NISTP384_MLKEM1024_SHA384 + "mlkem1024nistp384-sha384", +#endif +#ifndef WOLFSSH_NO_CURVE25519_MLKEM768_SHA256 + "mlkem768x25519-sha256", +#endif + NULL + }; + KexReplyHarness harness; + KexReplyRunResult result; + word32 i; + + for (i = 0; kexAlgos[i] != NULL; i++) { + InitKexReplyHarnessKex(&harness, kexAlgos[i], + REGRESS_DEFAULT_KEY_ALGO, REGRESS_DEFAULT_KEY_PATH, 0, + REGRESS_MUTATE_SIG_NAME, NULL, 0); + RunKexReplyHandshake(&harness, &result); + + AssertTrue(result.clientSuccess); + AssertTrue(result.serverSuccess); + AssertKexSecretWiped(harness.client); + AssertKexSecretWiped(harness.server); + + FreeKexReplyHarness(&harness); + } +} + #ifndef WOLFSSH_NO_RSA_SHA2_256 static void TestKexDhReplyRejectsRsaSha2_256CorruptSig(void) { @@ -15674,6 +15732,7 @@ static void TestGenerateKeysSplit(void) word32 payloadSz; word32 idx; byte zeros[AES_256_KEY_SIZE]; + Keys zeroKeys; WMEMSET(zeros, 0, sizeof(zeros)); @@ -15741,6 +15800,13 @@ static void TestGenerateKeysSplit(void) AssertTrue(WMEMCMP(ssh->handshake->peerKeys.encKey, ssh->handshake->keys.encKey, AES_128_KEY_SIZE) != 0); + /* A failed derivation leaves no key material behind. */ + AssertTrue(wolfSSH_TestGenerateKeys(ssh, WC_HASH_TYPE_NONE) != WS_SUCCESS); + WMEMSET(&zeroKeys, 0, sizeof(zeroKeys)); + AssertTrue(WMEMCMP(&ssh->handshake->keys, &zeroKeys, sizeof(Keys)) == 0); + AssertTrue(WMEMCMP(&ssh->handshake->peerKeys, &zeroKeys, + sizeof(Keys)) == 0); + wolfSSH_free(ssh); #ifndef WOLFSSH_NO_AES_GCM @@ -17406,6 +17472,7 @@ int main(int argc, char** argv) TestKexDhReplyRejectsEd25519CorruptSig(); #endif TestKexDhReplyRejectsSigNameOverrun(); + TestKexSecretWipedPerKex(); #ifdef REGRESS_TRUNC_KEX_ALGO TestKexDhReplyTruncatedFSendsDisconnect(); TestKexDhInitTruncatedESendsDisconnect();