Commander1024/ESP32_Serial_Swiss_Army_Knife
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
bea33e1c955d8eb18a520913a485d8ae180f0e04
ESP32_Serial_Swiss_Army_Knife/tools/wolfssh_order/855.patch
T
Commander1024 bea33e1c95 Add restricted wolfSSH ordering fix 
Apply hash-pinned generated edits for CVE-2025-14942 while keeping
wolfSSH 1.4.20 managed sources unchanged. Add the ABI header overlay,
provenance records, and real state-machine interoperability contracts.
2026-09-16 14:04:34 +02:00

1710 lines
57 KiB
Diff
Raw Blame History

From 73b165651c80a20047d913d32b0ffac47cb99ef6 Mon Sep 17 00:00:00 2001
From: John Safranek <john@wolfssl.com>
Date: Mon, 17 Nov 2025 09:50:48 -0800
Subject: [PATCH 1/7] Client Out Of Order Messaging Checking
1. Add macro for logging an expected message.
2. Add an expected message ID to the HandshakeInfo.
3. Add a message ID for "none (0)".
4. Add a check in IsMessageAllowedClient() for the expected message ID.
Clear it if successful.
5. The KEXDH messages sent to the server have expected responses. Set
them if sending the message is successful.
6. Add the set of message ID ranges and macros for testing if a message
ID is in a specific range.
7. Add flags for having sent the kexinit message and received it. Tweak
the checks for isKeying and these flags.
8. IsMessageAllowedClient() to check for appropriate messages at the
appropriate time during the connect.
---
src/internal.c | 158 +++++++++++++++++++++++++++++++++++----------
wolfssh/internal.h | 64 ++++++++++++++----
wolfssh/log.h | 2 +
3 files changed, 178 insertions(+), 46 deletions(-)
diff --git a/src/internal.c b/src/internal.c
index 5c553c6b7..3b4fe5f30 100644
--- a/src/internal.c
+++ b/src/internal.c
@@ -612,8 +612,8 @@ INLINE static int IsMessageAllowedKeying(WOLFSSH *ssh, byte msg)
return 0;
}
- /* case of resending SSH_MSG_KEXINIT */
- if (msg == MSGID_KEXINIT) {
+ /* case of peer resending SSH_MSG_KEXINIT */
+ if ((ssh->isKeying & WOLFSSH_PEER_IS_KEYING) && msg == MSGID_KEXINIT) {
WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by during rekeying", msg);
ssh->error = WS_REKEYING;
return 0;
@@ -632,34 +632,50 @@ INLINE static int IsMessageAllowedKeying(WOLFSSH *ssh, byte msg)
#ifndef NO_WOLFSSH_SERVER
INLINE static int IsMessageAllowedServer(WOLFSSH *ssh, byte msg)
{
+ /* Only the server should send these messages, never receive. */
+ if (msg == MSGID_SERVICE_ACCEPT) {
+ WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by %s %s",
+ msg, "client", "ever");
+ return 0;
+ }
+
+ /* Transport Layer Generic messages are always allowed. */
+ if (MSGIDLIMIT_TRANS_GEN(msg)) {
+ return 1;
+ }
+
/* Has client userauth started? */
+ /* Allows the server to receive up to KEXDH GEX Request during KEX. */
if (ssh->acceptState < ACCEPT_KEYED) {
- if (msg > MSGID_KEXDH_LIMIT) {
+ if (msg > MSGID_KEXDH_GEX_REQUEST) {
return 0;
}
}
/* Is server userauth complete? */
if (ssh->acceptState < ACCEPT_SERVER_USERAUTH_SENT) {
+ /* The server should only receive the user auth request message,
+ * it should not accept the other user auth messages, it sends
+ * them. (>50) */
/* Explicitly check for messages not allowed before user
* authentication has comleted. */
- if (msg >= MSGID_USERAUTH_LIMIT) {
- WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by server "
- "before user authentication is complete", msg);
+ if (MSGIDLIMIT_POST_USERAUTH(msg)) {
+ WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by %s %s",
+ msg, "server", "before user authentication is complete");
return 0;
}
/* Explicitly check for the user authentication messages that
* only the server sends, it shouldn't receive them. */
- if ((msg > MSGID_USERAUTH_RESTRICT) &&
+ if ((msg > MSGID_USERAUTH_REQUEST) &&
(msg != MSGID_USERAUTH_INFO_RESPONSE)) {
- WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by server "
- "during user authentication", msg);
+ WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by %s %s",
+ msg, "server", "during user authentication");
return 0;
}
}
else {
- if (msg >= MSGID_USERAUTH_RESTRICT && msg < MSGID_USERAUTH_LIMIT) {
- WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by server "
- "after user authentication", msg);
+ if (msg >= MSGID_USERAUTH_REQUEST && msg < MSGID_GLOBAL_REQUEST) {
+ WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by %s %s",
+ msg, "server", "after user authentication");
return 0;
}
}
@@ -672,37 +688,95 @@ INLINE static int IsMessageAllowedServer(WOLFSSH *ssh, byte msg)
#ifndef NO_WOLFSSH_CLIENT
INLINE static int IsMessageAllowedClient(WOLFSSH *ssh, byte msg)
{
- /* Has client userauth started? */
- if (ssh->connectState < CONNECT_CLIENT_KEXDH_INIT_SENT) {
- if (msg >= MSGID_KEXDH_LIMIT) {
+ /* Only the client should send these messages, never receive. */
+ if (msg == MSGID_SERVICE_REQUEST || msg == MSGID_USERAUTH_REQUEST) {
+ WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by %s %s",
+ msg, "client", "ever");
+ return 0;
+ }
+
+ if (msg == MSGID_SERVICE_ACCEPT) {
+ if (ssh->connectState == CONNECT_CLIENT_USERAUTH_REQUEST_SENT) {
+ return 1;
+ }
+ else {
+ WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by %s %s",
+ msg, "client", "after starting user auth");
return 0;
}
}
- /* Is client userauth complete? */
- if (ssh->connectState < CONNECT_SERVER_USERAUTH_ACCEPT_DONE) {
- /* Explicitly check for messages not allowed before user
- * authentication has comleted. */
- if (msg >= MSGID_USERAUTH_LIMIT) {
- WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by client "
- "before user authentication is complete", msg);
+
+ /* Transport Layer Generic messages are always allowed. */
+ if (MSGIDLIMIT_TRANS_GEN(msg)) {
+ return 1;
+ }
+
+ /* Is KEX complete? */
+ if (MSGIDLIMIT_TRANS(msg)) {
+ if (ssh->isKeying & WOLFSSH_PEER_IS_KEYING) {
+ /* MSGID_KEXINIT not allowed when keying. */
+ if (msg == MSGID_KEXINIT) {
+ WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by %s %s",
+ msg, "client", "when keying");
+ return 0;
+ }
+
+ /* Error if expecting a specific message and didn't receive. */
+ if (ssh->handshake && ssh->handshake->expectMsgId != MSGID_NONE) {
+ if (msg != ssh->handshake->expectMsgId) {
+ WLOG(WS_LOG_DEBUG,
+ "Message ID %u not the expected message %u",
+ msg, ssh->handshake->expectMsgId);
+ return 0;
+ }
+ else {
+ /* Got the expected message, clear expectation. */
+ ssh->handshake->expectMsgId = MSGID_NONE;
+ return 1;
+ }
+ }
+ }
+ else {
+ /* MSGID_KEXINIT only allowed when not keying. */
+ if (msg == MSGID_KEXINIT) {
+ return 1;
+ }
+
+ /* All other transport KEX and ALGO messages are not allowed
+ * when not keying. */
+ WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by %s %s",
+ msg, "client", "when not keying");
return 0;
}
- /* Explicitly check for the user authentication message that
- * only the client sends, it shouldn't receive it. */
- if (msg == MSGID_USERAUTH_RESTRICT) {
- WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by client "
- "during user authentication", msg);
+ }
+
+ /* Is client userauth complete? */
+ if (ssh->connectState >= CONNECT_KEYED
+ && ssh->connectState < CONNECT_SERVER_USERAUTH_ACCEPT_DONE) {
+ /* The endpoints should not allow message IDs greater than or
+ * equal to msgid 80 before user authentication is complete.
+ * Per RFC 4252 section 6. */
+ if (MSGIDLIMIT_POST_USERAUTH(msg)) {
+ WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by %s %s",
+ msg, "client", "before user authentication is complete");
return 0;
}
+ else if (MSGIDLIMIT_AUTH(msg)) {
+ return 1;
+ }
}
else {
- if (msg >= MSGID_USERAUTH_RESTRICT && msg < MSGID_USERAUTH_LIMIT) {
- WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by client "
- "after user authentication", msg);
+ if (MSGIDLIMIT_POST_USERAUTH(msg)) {
+ return 1;
+ }
+ else if (MSGIDLIMIT_AUTH(msg)) {
+ WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by %s %s",
+ msg, "client", "after user authentication");
return 0;
}
}
- return 1;
+
+ return 0;
}
#endif /* NO_WOLFSSH_CLIENT */
@@ -711,7 +785,7 @@ INLINE static int IsMessageAllowedClient(WOLFSSH *ssh, byte msg)
* Returns 1 if allowed 0 if not allowed. */
INLINE static int IsMessageAllowed(WOLFSSH *ssh, byte msg, byte state)
{
- if (state == WS_MSG_SEND && !IsMessageAllowedKeying(ssh, msg)) {
+ if (!IsMessageAllowedKeying(ssh, msg)) {
return 0;
}
@@ -725,6 +799,7 @@ INLINE static int IsMessageAllowed(WOLFSSH *ssh, byte msg, byte state)
return IsMessageAllowedClient(ssh, msg);
}
#endif /* NO_WOLFSSH_CLIENT */
+ (void)state;
return 0;
}
@@ -5872,8 +5947,10 @@ static int DoKexDhReply(WOLFSSH* ssh, byte* buf, word32 len, word32* idx)
ret = GenerateKeys(ssh, hashId, !ssh->handshake->useEccMlKem);
}
- if (ret == WS_SUCCESS)
+ if (ret == WS_SUCCESS) {
ret = SendNewKeys(ssh);
+ ssh->handshake->expectMsgId = MSGID_NEWKEYS;
+ }
if (sigKeyBlock_ptr)
WFREE(sigKeyBlock_ptr, ssh->ctx->heap, DYNTYPE_PRIVKEY);
@@ -10648,8 +10725,9 @@ int SendKexInit(WOLFSSH* ssh)
ret = BundlePacket(ssh);
}
- if (ret == WS_SUCCESS)
+ if (ret == WS_SUCCESS) {
ret = wolfSSH_SendPacket(ssh);
+ }
if (ret != WS_WANT_WRITE && ret != WS_SUCCESS)
PurgePacket(ssh);
@@ -12613,6 +12691,11 @@ int SendKexDhGexRequest(WOLFSSH* ssh)
if (ret == WS_SUCCESS)
ret = wolfSSH_SendPacket(ssh);
+ if (ret == WS_SUCCESS) {
+ WLOG_EXPECT_MSGID(MSGID_KEXDH_GEX_GROUP);
+ ssh->handshake->expectMsgId = MSGID_KEXDH_GEX_GROUP;
+ }
+
WLOG(WS_LOG_DEBUG, "Leaving SendKexDhGexRequest(), ret = %d", ret);
return ret;
}
@@ -12701,6 +12784,7 @@ int SendKexDhInit(WOLFSSH* ssh)
#endif
int ret = WS_SUCCESS;
byte msgId = MSGID_KEXDH_INIT;
+ byte expectMsgId = MSGID_KEXDH_REPLY;
byte e[MAX_KEX_KEY_SZ+1]; /* plus 1 in case of padding. */
word32 eSz = (word32)sizeof(e);
byte ePad = 0;
@@ -12752,6 +12836,7 @@ int SendKexDhInit(WOLFSSH* ssh)
generator = ssh->handshake->generator;
generatorSz = ssh->handshake->generatorSz;
msgId = MSGID_KEXDH_GEX_INIT;
+ expectMsgId = MSGID_KEXDH_GEX_REPLY;
break;
#endif
#ifndef WOLFSSH_NO_ECDH_SHA2_NISTP256
@@ -12963,6 +13048,11 @@ int SendKexDhInit(WOLFSSH* ssh)
if (ret == WS_SUCCESS)
ret = wolfSSH_SendPacket(ssh);
+ if (ret == WS_SUCCESS) {
+ WLOG_EXPECT_MSGID(expectMsgId);
+ ssh->handshake->expectMsgId = expectMsgId;
+ }
+
WLOG(WS_LOG_DEBUG, "Leaving SendKexDhInit(), ret = %d", ret);
return ret;
}
diff --git a/wolfssh/internal.h b/wolfssh/internal.h
index 6df5f1147..9bba76367 100644
--- a/wolfssh/internal.h
+++ b/wolfssh/internal.h
@@ -607,6 +607,7 @@ typedef struct Keys {
typedef struct HandshakeInfo {
+ byte expectMsgId;
byte kexId;
byte kexIdGuess;
byte kexHashId;
@@ -1183,6 +1184,8 @@ enum ProcessReplyStates {
enum WS_MessageIds {
+ MSGID_NONE = 0,
+
MSGID_DISCONNECT = 1,
MSGID_IGNORE = 2,
MSGID_UNIMPLEMENTED = 3,
@@ -1238,19 +1241,56 @@ enum WS_MessageIds {
};
-/* Allows the server to receive up to KEXDH GEX Request during KEX. */
-#define MSGID_KEXDH_LIMIT MSGID_KEXDH_GEX_REQUEST
-
-/* The endpoints should not allow message IDs greater than or
- * equal to msgid 80 before user authentication is complete.
- * Per RFC 4252 section 6. */
-#define MSGID_USERAUTH_LIMIT 80
+/* The following message ID ranges are described in RFC 5251, section 7. */
+enum WS_MessageIdLimits {
+/* Transport Layer Protocol: */
+ MSGIDLIMIT_TRANS_MIN = 1,
+ MSGIDLIMIT_TRANS_GEN_MIN = 1,
+ MSGIDLIMIT_TRANS_GEN_MAX = 19,
+ MSGIDLIMIT_TRANS_ALGO_MIN = 20,
+ MSGIDLIMIT_TRANS_ALGO_MAX = 29,
+ MSGIDLIMIT_TRANS_KEX_MIN = 30,
+ MSGIDLIMIT_TRANS_KEX_MAX = 49,
+ MSGIDLIMIT_TRANS_MAX = 49,
+/* User Authentication Protocol: */
+ MSGIDLIMIT_AUTH_MIN = 50,
+ MSGIDLIMIT_AUTH_GEN_MIN = 50,
+ MSGIDLIMIT_AUTH_GEN_MAX = 59,
+ MSGIDLIMIT_AUTH_METH_MIN = 60,
+ MSGIDLIMIT_AUTH_METH_MAX = 79,
+ MSGIDLIMIT_AUTH_MAX = 79,
+/* Connection Protocol: */
+ MSGIDLIMIT_CONN_MIN = 80,
+ MSGIDLIMIT_CONN_GEN_MIN = 80,
+ MSGIDLIMIT_CONN_GEN_MAX = 89,
+ MSGIDLIMIT_CONN_CHAN_MIN = 90,
+ MSGIDLIMIT_CONN_CHAN_MAX = 127,
+ MSGIDLIMIT_CONN_MAX = 127,
+/* Reserved For Client Protocols: */
+ MSGIDLIMIT_RESERVED_MIN = 128,
+ MSGIDLIMIT_RESERVED_MAX = 191,
+/* Local Extensions: */
+ MSGIDLIMIT_EXTENDED_MIN = 192,
+ MSGIDLIMIT_EXTENDED_MAX = 255,
+};
-/* The client should only send the user auth request message
- * (50), it should not accept it. The server should only receive
- * the user auth request message, it should not accept the other
- * user auth messages, it sends them. (>50) */
-#define MSGID_USERAUTH_RESTRICT 50
+/* Message ID bounds checking. */
+#define MSGIDLIMIT_BOUND(x,y,z) ((x) >= (y) && (x) <= (z))
+#define MSGIDLIMIT_COMP(x,name) \
+ MSGIDLIMIT_BOUND((x),MSGIDLIMIT_##name##_MIN,MSGIDLIMIT_##name##_MAX)
+#define MSGIDLIMIT_TRANS(x) MSGIDLIMIT_COMP((x),TRANS)
+#define MSGIDLIMIT_TRANS_GEN(x) MSGIDLIMIT_COMP((x),TRANS_GEN)
+#define MSGIDLIMIT_TRANS_ALGO(x) MSGIDLIMIT_COMP((x),TRANS_ALGO)
+#define MSGIDLIMIT_TRANS_KEX(x) MSGIDLIMIT_COMP((x),TRANS_KEX)
+#define MSGIDLIMIT_AUTH(x) MSGIDLIMIT_COMP((x),AUTH)
+#define MSGIDLIMIT_AUTH_GEN(x) MSGIDLIMIT_COMP((x),AUTH_GEN)
+#define MSGIDLIMIT_AUTH_METH(x) MSGIDLIMIT_COMP((x),AUTH_METH)
+#define MSGIDLIMIT_CONN(x) MSGIDLIMIT_COMP((x),CONN)
+#define MSGIDLIMIT_CONN_GEN(x) MSGIDLIMIT_COMP((x),CONN_GEN)
+#define MSGIDLIMIT_CONN_CHAN(x) MSGIDLIMIT_COMP((x),CONN_CHAN)
+#define MSGIDLIMIT_RESERVED(x) MSGIDLIMIT_COMP((x),RESERVED)
+#define MSGIDLIMIT_EXTENDED(x) MSGIDLIMIT_COMP((x),EXTENDED)
+#define MSGIDLIMIT_POST_USERAUTH(x) ((x) >= MSGIDLIMIT_CONN_MIN)
#define CHANNEL_EXTENDED_DATA_STDERR WOLFSSH_EXT_DATA_STDERR
diff --git a/wolfssh/log.h b/wolfssh/log.h
index 8758e7d43..89bd93827 100644
--- a/wolfssh/log.h
+++ b/wolfssh/log.h
@@ -81,6 +81,8 @@ WOLFSSH_API void wolfSSH_Log(enum wolfSSH_LogLevel,
#define WLOG(...) WC_DO_NOTHING
#endif
+#define WLOG_EXPECT_MSGID(x) WLOG(WS_LOG_DEBUG, "Expecting message %d", (x))
+
#ifdef __cplusplus
}
#endif
From a87ab400b3900d1e7fdda33c898094d6e3ada21d Mon Sep 17 00:00:00 2001
From: John Safranek <john@wolfssl.com>
Date: Mon, 24 Nov 2025 09:15:29 -0800
Subject: [PATCH 2/7] Guard out IsKeyingAllowed() as it is stepping on the
wrong messages.
---
src/internal.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/internal.c b/src/internal.c
index 3b4fe5f30..0a7a2ad00 100644
--- a/src/internal.c
+++ b/src/internal.c
@@ -595,6 +595,7 @@ static void HandshakeInfoFree(HandshakeInfo* hs, void* heap)
}
+#if 0
/* RFC 4253 section 7.1, Once having sent SSH_MSG_KEXINIT the only messages
* that can be sent are 1-19 (except SSH_MSG_SERVICE_REQUEST and
* SSH_MSG_SERVICE_ACCEPT), 20-29 (except SSH_MSG_KEXINIT again), and 30-49
@@ -627,6 +628,7 @@ INLINE static int IsMessageAllowedKeying(WOLFSSH *ssh, byte msg)
}
return 1;
}
+#endif
#ifndef NO_WOLFSSH_SERVER
@@ -785,9 +787,11 @@ INLINE static int IsMessageAllowedClient(WOLFSSH *ssh, byte msg)
* Returns 1 if allowed 0 if not allowed. */
INLINE static int IsMessageAllowed(WOLFSSH *ssh, byte msg, byte state)
{
+#if 0
if (!IsMessageAllowedKeying(ssh, msg)) {
return 0;
}
+#endif
#ifndef NO_WOLFSSH_SERVER
if (ssh->ctx->side == WOLFSSH_ENDPOINT_SERVER) {
From 5ae5c250e2d9b3bfdaea4f9c73cdf4b9f2592daa Mon Sep 17 00:00:00 2001
From: Andrew Hutchings <andrew@wolfssl.com>
Date: Mon, 1 Dec 2025 15:03:53 +0000
Subject: [PATCH 3/7] Add tests and fix issues
---
Makefile.am | 1 +
src/include.am | 12 ++
src/internal.c | 62 +++----
tests/include.am | 8 +-
tests/regress.c | 401 +++++++++++++++++++++++++++++++++++++++++++++
wolfssh/internal.h | 5 +
6 files changed, 446 insertions(+), 43 deletions(-)
create mode 100644 tests/regress.c
diff --git a/Makefile.am b/Makefile.am
index e729d5150..d5db195d5 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2,6 +2,7 @@
bin_PROGRAMS =
noinst_HEADERS =
lib_LTLIBRARIES =
+noinst_LTLIBRARIES =
noinst_PROGRAMS =
nobase_include_HEADERS =
check_PROGRAMS =
diff --git a/src/include.am b/src/include.am
index 8b70bdccd..88e535610 100644
--- a/src/include.am
+++ b/src/include.am
@@ -11,30 +11,42 @@ src_libwolfssh_la_SOURCES = src/ssh.c \
src_libwolfssh_la_CPPFLAGS = -DBUILDING_WOLFSSH ${AM_CPPFLAGS}
src_libwolfssh_la_LDFLAGS = -no-undefined -version-info ${WOLFSSH_LIBRARY_VERSION}
+noinst_LTLIBRARIES += src/libwolfssh_test.la
+src_libwolfssh_test_la_SOURCES = $(src_libwolfssh_la_SOURCES)
+src_libwolfssh_test_la_CPPFLAGS = -DBUILDING_WOLFSSH -DWOLFSSH_TEST_INTERNAL ${AM_CPPFLAGS}
+src_libwolfssh_test_la_LDFLAGS = -no-undefined
+
if !BUILD_INLINE
src_libwolfssh_la_SOURCES += src/misc.c
+src_libwolfssh_test_la_SOURCES += src/misc.c
endif
if BUILD_KEYGEN
src_libwolfssh_la_SOURCES += src/keygen.c
+src_libwolfssh_test_la_SOURCES += src/keygen.c
endif
if BUILD_SCP
src_libwolfssh_la_SOURCES += src/wolfscp.c
+src_libwolfssh_test_la_SOURCES += src/wolfscp.c
endif
if BUILD_SFTP
src_libwolfssh_la_SOURCES += src/wolfsftp.c
+src_libwolfssh_test_la_SOURCES += src/wolfsftp.c
endif
if BUILD_TERM
src_libwolfssh_la_SOURCES += src/wolfterm.c
+src_libwolfssh_test_la_SOURCES += src/wolfterm.c
endif
if BUILD_AGENT
src_libwolfssh_la_SOURCES += src/agent.c
+src_libwolfssh_test_la_SOURCES += src/agent.c
endif
if BUILD_CERTS
src_libwolfssh_la_SOURCES += src/certman.c
+src_libwolfssh_test_la_SOURCES += src/certman.c
endif
diff --git a/src/internal.c b/src/internal.c
index 0a7a2ad00..dfadcedea 100644
--- a/src/internal.c
+++ b/src/internal.c
@@ -595,42 +595,6 @@ static void HandshakeInfoFree(HandshakeInfo* hs, void* heap)
}
-#if 0
-/* RFC 4253 section 7.1, Once having sent SSH_MSG_KEXINIT the only messages
-* that can be sent are 1-19 (except SSH_MSG_SERVICE_REQUEST and
-* SSH_MSG_SERVICE_ACCEPT), 20-29 (except SSH_MSG_KEXINIT again), and 30-49
-*/
-INLINE static int IsMessageAllowedKeying(WOLFSSH *ssh, byte msg)
-{
- if (ssh->isKeying == 0) {
- return 1;
- }
-
- /* case of service request or accept in 1-19 */
- if (msg == MSGID_SERVICE_REQUEST || msg == MSGID_SERVICE_ACCEPT) {
- WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by during rekeying", msg);
- ssh->error = WS_REKEYING;
- return 0;
- }
-
- /* case of peer resending SSH_MSG_KEXINIT */
- if ((ssh->isKeying & WOLFSSH_PEER_IS_KEYING) && msg == MSGID_KEXINIT) {
- WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by during rekeying", msg);
- ssh->error = WS_REKEYING;
- return 0;
- }
-
- /* case where message id greater than 49 */
- if (msg >= MSGID_USERAUTH_REQUEST) {
- WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by during rekeying", msg);
- ssh->error = WS_REKEYING;
- return 0;
- }
- return 1;
-}
-#endif
-
-
#ifndef NO_WOLFSSH_SERVER
INLINE static int IsMessageAllowedServer(WOLFSSH *ssh, byte msg)
{
@@ -694,6 +658,7 @@ INLINE static int IsMessageAllowedClient(WOLFSSH *ssh, byte msg)
if (msg == MSGID_SERVICE_REQUEST || msg == MSGID_USERAUTH_REQUEST) {
WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by %s %s",
msg, "client", "ever");
+ ssh->error = WS_MSGID_NOT_ALLOWED_E;
return 0;
}
@@ -720,6 +685,7 @@ INLINE static int IsMessageAllowedClient(WOLFSSH *ssh, byte msg)
if (msg == MSGID_KEXINIT) {
WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by %s %s",
msg, "client", "when keying");
+ ssh->error = WS_REKEYING;
return 0;
}
@@ -729,6 +695,7 @@ INLINE static int IsMessageAllowedClient(WOLFSSH *ssh, byte msg)
WLOG(WS_LOG_DEBUG,
"Message ID %u not the expected message %u",
msg, ssh->handshake->expectMsgId);
+ ssh->error = WS_REKEYING;
return 0;
}
else {
@@ -748,6 +715,7 @@ INLINE static int IsMessageAllowedClient(WOLFSSH *ssh, byte msg)
* when not keying. */
WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by %s %s",
msg, "client", "when not keying");
+ ssh->error = WS_MSGID_NOT_ALLOWED_E;
return 0;
}
}
@@ -761,9 +729,17 @@ INLINE static int IsMessageAllowedClient(WOLFSSH *ssh, byte msg)
if (MSGIDLIMIT_POST_USERAUTH(msg)) {
WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by %s %s",
msg, "client", "before user authentication is complete");
+ ssh->error = WS_MSGID_NOT_ALLOWED_E;
return 0;
}
else if (MSGIDLIMIT_AUTH(msg)) {
+ /* Do not accept any userauth messages until we've asked for auth. */
+ if (ssh->connectState < CONNECT_CLIENT_USERAUTH_REQUEST_SENT) {
+ WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by %s %s",
+ msg, "client", "before sending userauth request");
+ ssh->error = WS_MSGID_NOT_ALLOWED_E;
+ return 0;
+ }
return 1;
}
}
@@ -774,6 +750,7 @@ INLINE static int IsMessageAllowedClient(WOLFSSH *ssh, byte msg)
else if (MSGIDLIMIT_AUTH(msg)) {
WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by %s %s",
msg, "client", "after user authentication");
+ ssh->error = WS_MSGID_NOT_ALLOWED_E;
return 0;
}
}
@@ -787,12 +764,6 @@ INLINE static int IsMessageAllowedClient(WOLFSSH *ssh, byte msg)
* Returns 1 if allowed 0 if not allowed. */
INLINE static int IsMessageAllowed(WOLFSSH *ssh, byte msg, byte state)
{
-#if 0
- if (!IsMessageAllowedKeying(ssh, msg)) {
- return 0;
- }
-#endif
-
#ifndef NO_WOLFSSH_SERVER
if (ssh->ctx->side == WOLFSSH_ENDPOINT_SERVER) {
return IsMessageAllowedServer(ssh, msg);
@@ -807,6 +778,13 @@ INLINE static int IsMessageAllowed(WOLFSSH *ssh, byte msg, byte state)
return 0;
}
+#ifdef WOLFSSH_TEST_INTERNAL
+int wolfSSH_TestIsMessageAllowed(WOLFSSH* ssh, byte msg, byte state)
+{
+ return IsMessageAllowed(ssh, msg, state);
+}
+#endif
+
static const char cannedKexAlgoNames[] =
#if !defined(WOLFSSH_NO_NISTP256_MLKEM768_SHA256)
diff --git a/tests/include.am b/tests/include.am
index f14b3007a..a9aed41c0 100644
--- a/tests/include.am
+++ b/tests/include.am
@@ -3,7 +3,8 @@
# All paths should be given relative to the root
check_PROGRAMS += tests/unit.test tests/api.test \
- tests/testsuite.test tests/kex.test
+ tests/testsuite.test tests/kex.test \
+ tests/regress.test
tests_unit_test_SOURCES = tests/unit.c tests/unit.h
tests_unit_test_CPPFLAGS = -DNO_MAIN_DRIVER $(AM_CPPFLAGS)
@@ -43,3 +44,8 @@ tests_kex_test_SOURCES = tests/kex.c tests/kex.h \
tests_kex_test_CPPFLAGS = -DNO_MAIN_DRIVER $(AM_CPPFLAGS)
tests_kex_test_LDADD = src/libwolfssh.la
tests_kex_test_DEPENDENCIES = src/libwolfssh.la
+
+tests_regress_test_SOURCES = tests/regress.c
+tests_regress_test_CPPFLAGS = -DNO_MAIN_DRIVER -DWOLFSSH_TEST_INTERNAL $(AM_CPPFLAGS)
+tests_regress_test_LDADD = src/libwolfssh_test.la
+tests_regress_test_DEPENDENCIES = src/libwolfssh_test.la
diff --git a/tests/regress.c b/tests/regress.c
new file mode 100644
index 000000000..fbd6032e5
--- /dev/null
+++ b/tests/regress.c
@@ -0,0 +1,401 @@
+/* regress.c
+ *
+ * Regression coverage for message ordering / keying state handling.
+ *
+ * Copyright (C) 2025 wolfSSL Inc.
+ */
+
+#ifdef HAVE_CONFIG_H
+ #include <config.h>
+#endif
+
+#ifdef WOLFSSL_USER_SETTINGS
+ #include <wolfssl/wolfcrypt/settings.h>
+#else
+ #include <wolfssl/options.h>
+#endif
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <arpa/inet.h>
+#include <string.h>
+
+#include <wolfssh/port.h>
+#include <wolfssh/ssh.h>
+#include <wolfssh/internal.h>
+
+#ifndef WOLFSSH_NO_ABORT
+ #define WABORT() abort()
+#else
+ #define WABORT()
+#endif
+
+#define PrintError(description, result) do { \
+ printf("\nERROR - %s line %d failed with:", __FILE__, __LINE__); \
+ printf("\n expected: "); printf description; \
+ printf("\n result: "); printf result; printf("\n\n"); \
+} while(0)
+
+#define Fail(description, result) do { \
+ PrintError(description, result); \
+ WABORT(); \
+} while(0)
+
+#define Assert(test, description, result) if (!(test)) Fail(description, result)
+
+#define AssertTrue(x) Assert((x), ("%s is true", #x), (#x " => FALSE"))
+#define AssertFalse(x) Assert(!(x), ("%s is false", #x), (#x " => TRUE"))
+#define AssertNotNull(x) Assert((x), ("%s is not null", #x), (#x " => NULL"))
+#define AssertIntEQ(x, y) do { int _x = (int)(x); int _y = (int)(y); \
+ Assert(_x == _y, ("%s == %s", #x, #y), ("%d != %d", _x, _y)); } while (0)
+
+
+static void ResetSession(WOLFSSH* ssh)
+{
+ if (ssh->handshake != NULL) {
+ WFREE(ssh->handshake, ssh->ctx->heap, DYNTYPE_HS);
+ ssh->handshake = NULL;
+ }
+ ssh->isKeying = 0;
+ ssh->connectState = CONNECT_BEGIN;
+ ssh->error = 0;
+}
+
+
+static HandshakeInfo* AllocHandshake(WOLFSSH* ssh)
+{
+ HandshakeInfo* hs;
+
+ hs = (HandshakeInfo*)WMALLOC(sizeof(HandshakeInfo), ssh->ctx->heap,
+ DYNTYPE_HS);
+ AssertNotNull(hs);
+ WMEMSET(hs, 0, sizeof(HandshakeInfo));
+ hs->blockSz = MIN_BLOCK_SZ;
+ hs->eSz = (word32)sizeof(hs->e);
+ hs->xSz = (word32)sizeof(hs->x);
+
+ return hs;
+}
+
+/* Build a minimal SSH binary packet carrying only a message ID.
+ * Layout: uint32 packetLen, byte padLen, payload[msgId], pad[padLen].
+ * Choose padLen so total is 8-byte aligned for the clear transport case. */
+static word32 BuildPacket(byte msgId, byte* out, word32 outSz)
+{
+ byte padLen = 6; /* 1 (msgId) +1 (padLen) +6 = 8 */
+ word32 packetLen = 1 + 1 + padLen; /* payload + padLen field + pad */
+ word32 need = 4 + packetLen;
+
+ AssertTrue(outSz >= need);
+ out[0] = (byte)(packetLen >> 24);
+ out[1] = (byte)(packetLen >> 16);
+ out[2] = (byte)(packetLen >> 8);
+ out[3] = (byte)(packetLen);
+ out[4] = padLen;
+ out[5] = msgId;
+ WMEMSET(out + 6, 0, padLen);
+ return need;
+}
+
+static byte ParseMsgId(const byte* pkt, word32 sz)
+{
+ AssertTrue(sz >= 6);
+ return pkt[5];
+}
+
+/* Simple in-memory transport harness */
+typedef struct {
+ byte* in; /* data to feed into client */
+ word32 inSz;
+ word32 inOff;
+ byte* out; /* data written by client */
+ word32 outSz;
+ word32 outCap;
+} MemIo;
+
+/* Minimal send/recv helpers for future transport-level tests; keep them static
+ * and unused for now to avoid warnings when Werror is on. */
+#ifdef WOLFSSH_TEST_MEMIO
+static int MemRecv(WOLFSSH* ssh, void* buf, word32 sz, void* ctx)
+{
+ (void)ssh;
+ MemIo* io = (MemIo*)ctx;
+ word32 remain = io->inSz - io->inOff;
+ if (remain == 0)
+ return WS_CBIO_ERR_WANT_READ;
+ if (sz > remain)
+ sz = remain;
+ WMEMCPY(buf, io->in + io->inOff, sz);
+ io->inOff += sz;
+ return (int)sz;
+}
+
+static int MemSend(WOLFSSH* ssh, void* buf, word32 sz, void* ctx)
+{
+ (void)ssh;
+ MemIo* io = (MemIo*)ctx;
+ if (io->outSz + sz > io->outCap) {
+ return WS_CBIO_ERR_GENERAL;
+ }
+ WMEMCPY(io->out + io->outSz, buf, sz);
+ io->outSz += sz;
+ return (int)sz;
+}
+
+static void MemIoInit(MemIo* io, byte* in, word32 inSz, byte* out, word32 outCap)
+{
+ io->in = in;
+ io->inSz = inSz;
+ io->inOff = 0;
+ io->out = out;
+ io->outSz = 0;
+ io->outCap = outCap;
+}
+#endif
+
+
+/* Reject auth messages while the peer is still keying and the client
+ * expects the KEX reply. */
+static void TestAuthMessageBlockedDuringKeying(WOLFSSH* ssh)
+{
+ int allowed;
+
+ ResetSession(ssh);
+ ssh->isKeying = WOLFSSH_PEER_IS_KEYING;
+ ssh->connectState = CONNECT_CLIENT_KEXDH_INIT_SENT;
+ ssh->handshake = AllocHandshake(ssh);
+ ssh->handshake->expectMsgId = MSGID_KEXDH_REPLY;
+
+ allowed = wolfSSH_TestIsMessageAllowed(ssh, MSGID_USERAUTH_FAILURE,
+ WS_MSG_RECV);
+ AssertFalse(allowed);
+
+ /* The expected message must be allowed and clear the expectation. */
+ allowed = wolfSSH_TestIsMessageAllowed(ssh, MSGID_KEXDH_REPLY,
+ WS_MSG_RECV);
+ AssertTrue(allowed);
+ AssertIntEQ(ssh->handshake->expectMsgId, MSGID_NONE);
+}
+
+/* Reject USERAUTH_FAILURE with password list during keying (password-leak PoC). */
+static void TestUserauthFailureDuringKeying(WOLFSSH* ssh)
+{
+ byte buf[32];
+ word32 sz;
+ int allowed;
+
+ ResetSession(ssh);
+ ssh->isKeying = WOLFSSH_PEER_IS_KEYING;
+ ssh->connectState = CONNECT_CLIENT_KEXDH_INIT_SENT;
+ ssh->handshake = AllocHandshake(ssh);
+ ssh->handshake->expectMsgId = MSGID_KEXDH_REPLY;
+
+ sz = BuildPacket(MSGID_USERAUTH_FAILURE, buf, sizeof(buf));
+ allowed = wolfSSH_TestIsMessageAllowed(ssh, ParseMsgId(buf, sz),
+ WS_MSG_RECV);
+ AssertFalse(allowed);
+}
+
+
+/* Expect an abort/error to be set when password-leak sequence hits during keying. */
+static void TestPasswordLeakAborts(WOLFSSH* ssh)
+{
+ byte buf[32];
+ word32 sz;
+ int allowed;
+
+ ResetSession(ssh);
+ ssh->isKeying = WOLFSSH_PEER_IS_KEYING;
+ ssh->connectState = CONNECT_CLIENT_KEXDH_INIT_SENT;
+ ssh->handshake = AllocHandshake(ssh);
+ ssh->handshake->expectMsgId = MSGID_KEXDH_REPLY;
+
+ sz = BuildPacket(MSGID_USERAUTH_FAILURE, buf, sizeof(buf));
+ allowed = wolfSSH_TestIsMessageAllowed(ssh, ParseMsgId(buf, sz),
+ WS_MSG_RECV);
+ AssertFalse(allowed);
+ AssertTrue(ssh->error != 0); /* should set an error / abort path */
+}
+
+
+/* Reject USERAUTH_SUCCESS before the client has even sent a userauth request. */
+static void TestPrematureUserauthSuccess(WOLFSSH* ssh)
+{
+ int allowed;
+
+ ResetSession(ssh);
+ ssh->connectState = CONNECT_KEYED;
+
+ allowed = wolfSSH_TestIsMessageAllowed(ssh, MSGID_USERAUTH_SUCCESS,
+ WS_MSG_RECV);
+ AssertFalse(allowed);
+}
+
+
+/* Reject a spoofed sequence: bogus USERAUTH_SUCCESS followed by channel msgs. */
+static void TestChannelSpoofSequence(WOLFSSH* ssh)
+{
+ byte buf[32];
+ word32 sz;
+ int allowed;
+
+ ResetSession(ssh);
+ ssh->connectState = CONNECT_KEYED;
+
+ sz = BuildPacket(MSGID_USERAUTH_SUCCESS, buf, sizeof(buf));
+ allowed = wolfSSH_TestIsMessageAllowed(ssh, ParseMsgId(buf, sz),
+ WS_MSG_RECV);
+ AssertFalse(allowed);
+
+ sz = BuildPacket(MSGID_CHANNEL_OPEN_CONF, buf, sizeof(buf));
+ allowed = wolfSSH_TestIsMessageAllowed(ssh, ParseMsgId(buf, sz),
+ WS_MSG_RECV);
+ AssertFalse(allowed);
+
+ sz = BuildPacket(MSGID_CHANNEL_SUCCESS, buf, sizeof(buf));
+ allowed = wolfSSH_TestIsMessageAllowed(ssh, ParseMsgId(buf, sz),
+ WS_MSG_RECV);
+ AssertFalse(allowed);
+
+ sz = BuildPacket(MSGID_CHANNEL_DATA, buf, sizeof(buf));
+ allowed = wolfSSH_TestIsMessageAllowed(ssh, ParseMsgId(buf, sz),
+ WS_MSG_RECV);
+ AssertFalse(allowed);
+}
+
+/* Expect abort/error on spoofed auth+channel sequence. */
+static void TestChannelSpoofAborts(WOLFSSH* ssh)
+{
+ byte buf[32];
+ word32 sz;
+ int allowed;
+
+ ResetSession(ssh);
+ ssh->connectState = CONNECT_KEYED;
+
+ sz = BuildPacket(MSGID_USERAUTH_SUCCESS, buf, sizeof(buf));
+ allowed = wolfSSH_TestIsMessageAllowed(ssh, ParseMsgId(buf, sz),
+ WS_MSG_RECV);
+ AssertFalse(allowed);
+
+ sz = BuildPacket(MSGID_CHANNEL_OPEN_CONF, buf, sizeof(buf));
+ allowed = wolfSSH_TestIsMessageAllowed(ssh, ParseMsgId(buf, sz),
+ WS_MSG_RECV);
+ AssertFalse(allowed);
+
+ AssertTrue(ssh->error != 0);
+}
+
+
+/* Reject USERAUTH_FAILURE(publickey) before any auth request (static-signature PoC). */
+static void TestPublicKeyFailureBeforeRequest(WOLFSSH* ssh)
+{
+ byte buf[32];
+ word32 sz;
+ int allowed;
+
+ ResetSession(ssh);
+ ssh->connectState = CONNECT_KEYED;
+
+ sz = BuildPacket(MSGID_USERAUTH_FAILURE, buf, sizeof(buf));
+ allowed = wolfSSH_TestIsMessageAllowed(ssh, ParseMsgId(buf, sz),
+ WS_MSG_RECV);
+ AssertFalse(allowed);
+}
+
+/* Expect abort/error when publickey failure arrives before any request. */
+static void TestPublicKeyFailureAborts(WOLFSSH* ssh)
+{
+ byte buf[32];
+ word32 sz;
+ int allowed;
+
+ ResetSession(ssh);
+ ssh->connectState = CONNECT_KEYED;
+
+ sz = BuildPacket(MSGID_USERAUTH_FAILURE, buf, sizeof(buf));
+ allowed = wolfSSH_TestIsMessageAllowed(ssh, ParseMsgId(buf, sz),
+ WS_MSG_RECV);
+ AssertFalse(allowed);
+ AssertTrue(ssh->error != 0);
+}
+
+
+/* Reject channel messages before user authentication completes. */
+static void TestChannelBlockedBeforeAuth(WOLFSSH* ssh)
+{
+ int allowed;
+
+ ResetSession(ssh);
+ ssh->connectState = CONNECT_KEYED;
+
+ allowed = wolfSSH_TestIsMessageAllowed(ssh, MSGID_CHANNEL_OPEN,
+ WS_MSG_RECV);
+ AssertFalse(allowed);
+}
+
+
+/* Allow channel messages after user authentication completes. */
+static void TestChannelAllowedAfterAuth(WOLFSSH* ssh)
+{
+ int allowed;
+
+ ResetSession(ssh);
+ ssh->connectState = CONNECT_SERVER_USERAUTH_ACCEPT_DONE;
+
+ allowed = wolfSSH_TestIsMessageAllowed(ssh, MSGID_CHANNEL_OPEN,
+ WS_MSG_RECV);
+ AssertTrue(allowed);
+}
+
+
+/* Reject a peer KEXINIT once keying is in progress. */
+static void TestKexInitRejectedWhenKeying(WOLFSSH* ssh)
+{
+ int allowed;
+
+ ResetSession(ssh);
+ ssh->isKeying = WOLFSSH_PEER_IS_KEYING;
+ ssh->connectState = CONNECT_SERVER_KEXINIT_DONE;
+
+ allowed = wolfSSH_TestIsMessageAllowed(ssh, MSGID_KEXINIT, WS_MSG_RECV);
+ AssertFalse(allowed);
+}
+
+
+int main(int argc, char** argv)
+{
+ WOLFSSH_CTX* ctx;
+ WOLFSSH* ssh;
+
+ (void)argc;
+ (void)argv;
+
+ wolfSSH_Init();
+
+ ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL);
+ AssertNotNull(ctx);
+
+ ssh = wolfSSH_new(ctx);
+ AssertNotNull(ssh);
+
+ TestAuthMessageBlockedDuringKeying(ssh);
+ TestUserauthFailureDuringKeying(ssh);
+ TestPasswordLeakAborts(ssh);
+ TestPrematureUserauthSuccess(ssh);
+ TestChannelSpoofSequence(ssh);
+ TestChannelSpoofAborts(ssh);
+ TestPublicKeyFailureBeforeRequest(ssh);
+ TestPublicKeyFailureAborts(ssh);
+ TestChannelBlockedBeforeAuth(ssh);
+ TestChannelAllowedAfterAuth(ssh);
+ TestKexInitRejectedWhenKeying(ssh);
+
+ ResetSession(ssh);
+ wolfSSH_free(ssh);
+ wolfSSH_CTX_free(ctx);
+ wolfSSH_Cleanup();
+
+ printf("regress: PASS\n");
+ return 0;
+}
diff --git a/wolfssh/internal.h b/wolfssh/internal.h
index 9bba76367..fa3a2d590 100644
--- a/wolfssh/internal.h
+++ b/wolfssh/internal.h
@@ -1300,6 +1300,11 @@ enum WS_MessageIdLimits {
#define WS_MSG_SEND 1
#define WS_MSG_RECV 2
+#ifdef WOLFSSH_TEST_INTERNAL
+ WOLFSSH_API int wolfSSH_TestIsMessageAllowed(WOLFSSH* ssh, byte msg,
+ byte state);
+#endif
+
/* dynamic memory types */
enum WS_DynamicTypes {
DYNTYPE_STRING = 500,
From dee1c59f263220ecd05a88a72da104c22bef0598 Mon Sep 17 00:00:00 2001
From: Andrew Hutchings <andrew@wolfssl.com>
Date: Mon, 1 Dec 2025 15:41:13 +0000
Subject: [PATCH 4/7] Fix double-free crash and socket-close spin
---
apps/wolfssh/common.c | 35 +++++++++++++++++--
apps/wolfssh/wolfssh.c | 79 ++++++++++++++++++++++++++++--------------
src/internal.c | 2 ++
tests/include.am | 3 +-
tests/regress.c | 47 +++++++++++++++++++++++++
5 files changed, 137 insertions(+), 29 deletions(-)
diff --git a/apps/wolfssh/common.c b/apps/wolfssh/common.c
index 283708cc1..e8e347a40 100644
--- a/apps/wolfssh/common.c
+++ b/apps/wolfssh/common.c
@@ -45,10 +45,12 @@ static byte* userPublicKey = userPublicKeyBuf;
static const byte* userPublicKeyType = NULL;
static byte userPassword[256];
static const byte* userPrivateKeyType = NULL;
+static byte userPublicKeyAlloc = 0;
static word32 userPublicKeySz = 0;
static byte pubKeyLoaded = 0; /* was a public key loaded */
static byte userPrivateKeyBuf[1191];
static byte* userPrivateKey = userPrivateKeyBuf;
+static byte userPrivateKeyAlloc = 0;
static word32 userPublicKeyTypeSz = 0;
static word32 userPrivateKeySz = sizeof(userPrivateKeyBuf);
static word32 userPrivateKeyTypeSz = 0;
@@ -670,6 +672,13 @@ int ClientUseCert(const char* certName)
userPublicKeyType = publicKeyType;
userPublicKeyTypeSz = (word32)WSTRLEN((const char*)publicKeyType);
pubKeyLoaded = 1;
+ userPublicKeyAlloc = 1;
+ }
+ else {
+ userPublicKey = userPublicKeyBuf;
+ userPublicKeySz = 0;
+ userPublicKeyType = NULL;
+ userPublicKeyAlloc = 0;
}
#else
fprintf(stderr, "Certificate support not compiled in");
@@ -687,12 +696,22 @@ int ClientSetPrivateKey(const char* privKeyName)
{
int ret;
+ userPrivateKeyAlloc = 0;
userPrivateKey = NULL; /* create new buffer based on parsed input */
ret = wolfSSH_ReadKey_file(privKeyName,
(byte**)&userPrivateKey, &userPrivateKeySz,
(const byte**)&userPrivateKeyType, &userPrivateKeyTypeSz,
&isPrivate, NULL);
+ if (ret == 0) {
+ userPrivateKeyAlloc = 1;
+ }
+ else {
+ userPrivateKey = userPrivateKeyBuf;
+ userPrivateKeySz = sizeof(userPrivateKeyBuf);
+ userPrivateKeyType = NULL;
+ }
+
return ret;
}
@@ -703,6 +722,7 @@ int ClientUsePubKey(const char* pubKeyName)
{
int ret;
+ userPublicKeyAlloc = 0;
userPublicKey = NULL; /* create new buffer based on parsed input */
ret = wolfSSH_ReadKey_file(pubKeyName,
&userPublicKey, &userPublicKeySz,
@@ -711,6 +731,11 @@ int ClientUsePubKey(const char* pubKeyName)
if (ret == 0) {
pubKeyLoaded = 1;
+ userPublicKeyAlloc = 1;
+ }
+ else {
+ userPublicKey = userPublicKeyBuf;
+ userPublicKeySz = 0;
}
return ret;
@@ -747,11 +772,17 @@ int ClientLoadCA(WOLFSSH_CTX* ctx, const char* caCert)
void ClientFreeBuffers(void)
{
- if (userPublicKey != userPublicKeyBuf) {
+ if (userPublicKeyAlloc && userPublicKey != NULL) {
WFREE(userPublicKey, NULL, DYNTYPE_PRIVKEY);
+ userPublicKey = userPublicKeyBuf;
+ userPublicKeySz = 0;
+ userPublicKeyAlloc = 0;
}
- if (userPrivateKey != userPrivateKeyBuf) {
+ if (userPrivateKeyAlloc && userPrivateKey != NULL) {
WFREE(userPrivateKey, NULL, DYNTYPE_PRIVKEY);
+ userPrivateKey = userPrivateKeyBuf;
+ userPrivateKeySz = sizeof(userPrivateKeyBuf);
+ userPrivateKeyAlloc = 0;
}
}
diff --git a/apps/wolfssh/wolfssh.c b/apps/wolfssh/wolfssh.c
index dc2d83bc9..ca405d41b 100644
--- a/apps/wolfssh/wolfssh.c
+++ b/apps/wolfssh/wolfssh.c
@@ -220,6 +220,7 @@ typedef struct thread_args {
wolfSSL_Mutex lock;
byte rawMode;
byte quit;
+ int readError;
} thread_args;
#ifdef _POSIX_THREADS
@@ -390,6 +391,7 @@ static THREAD_RET readPeer(void* in)
int bufSz = sizeof(buf);
thread_args* args = (thread_args*)in;
int ret = 0;
+ int stop = 0;
int fd = wolfSSH_get_fd(args->ssh);
word32 bytes;
#ifdef USE_WINDOWS_API
@@ -398,11 +400,6 @@ static THREAD_RET readPeer(void* in)
fd_set readSet;
fd_set errSet;
- FD_ZERO(&readSet);
- FD_ZERO(&errSet);
- FD_SET(fd, &readSet);
- FD_SET(fd, &errSet);
-
#ifdef USE_WINDOWS_API
if (args->rawMode == 0) {
DWORD wrd;
@@ -431,9 +428,13 @@ static THREAD_RET readPeer(void* in)
#endif
while (ret >= 0) {
- #if defined(WOLFSSH_TERM) && defined(USE_WINDOWS_API)
+#if defined(WOLFSSH_TERM) && defined(USE_WINDOWS_API)
(void)windowMonitor(args);
- #endif
+#endif
+ FD_ZERO(&readSet);
+ FD_ZERO(&errSet);
+ FD_SET(fd, &readSet);
+ FD_SET(fd, &errSet);
bytes = select(fd + 1, &readSet, NULL, &errSet, NULL);
wc_LockMutex(&args->lock);
@@ -458,18 +459,18 @@ static THREAD_RET readPeer(void* in)
} while (ret > 0);
}
else if (ret <= 0) {
- if (ret == WS_FATAL_ERROR) {
- ret = wolfSSH_get_error(args->ssh);
- if (ret == WS_WANT_READ) {
- continue;
- }
- #ifdef WOLFSSH_AGENT
- else if (ret == WS_CHAN_RXD) {
- byte agentBuf[512];
- int rxd, txd;
- word32 channel = 0;
+ int err = (ret == WS_FATAL_ERROR) ?
+ wolfSSH_get_error(args->ssh) : ret;
+ if (err == WS_WANT_READ) {
+ bytes = 0;
+ }
+#ifdef WOLFSSH_AGENT
+ else if (err == WS_CHAN_RXD) {
+ byte agentBuf[512];
+ int rxd, txd;
+ word32 channel = 0;
- wolfSSH_GetLastRxId(args->ssh, &channel);
+ wolfSSH_GetLastRxId(args->ssh, &channel);
rxd = wolfSSH_ChannelIdRead(args->ssh, channel,
agentBuf, sizeof(agentBuf));
if (rxd > 4) {
@@ -495,9 +496,17 @@ static THREAD_RET readPeer(void* in)
WMEMSET(agentBuf, 0, sizeof(agentBuf));
continue;
}
- #endif /* WOLFSSH_AGENT */
+#endif /* WOLFSSH_AGENT */
+ else if (err == WS_CBIO_ERR_CONN_CLOSE ||
+ err == WS_SOCKET_ERROR_E ||
+ err == WS_MSGID_NOT_ALLOWED_E) {
+ args->readError = err;
+ ret = err;
+ stop = 1;
+ bytes = 0;
}
- else if (ret != WS_EOF) {
+ else if (err != WS_EOF) {
+ wc_UnLockMutex(&args->lock);
err_sys("Stream read failed.");
}
}
@@ -517,12 +526,16 @@ static THREAD_RET readPeer(void* in)
}
#endif
}
- ret = wolfSSH_stream_peek(args->ssh, buf, bufSz);
- if (ret <= 0) {
- bytes = 0; /* read it all */
+ if (!stop) {
+ ret = wolfSSH_stream_peek(args->ssh, buf, bufSz);
+ if (ret <= 0) {
+ bytes = 0; /* read it all */
+ }
}
}
wc_UnLockMutex(&args->lock);
+ if (stop)
+ break;
}
#if !defined(WOLFSSH_NO_ECC) && defined(FP_ECC) && defined(HAVE_THREAD_LS)
wc_ecc_fp_free(); /* free per thread cache */
@@ -791,7 +804,8 @@ static int config_parse_command_line(struct config* config,
if (found != NULL) {
*found = '\0';
if (config->user) {
- free(config->user);
+ WFREE(config->user, NULL, 0);
+ config->user = NULL;
}
sz = WSTRLEN(cursor);
config->user = (char*)WMALLOC(sz + 1, NULL, 0);
@@ -818,7 +832,7 @@ static int config_parse_command_line(struct config* config,
strcpy(config->hostname, cursor);
}
- free(dest);
+ WFREE(dest, NULL, 0);
myoptind++;
}
@@ -874,18 +888,23 @@ static int config_cleanup(struct config* config)
{
if (config->user) {
WFREE(config->user, NULL, 0);
+ config->user = NULL;
}
if (config->hostname) {
WFREE(config->hostname, NULL, 0);
+ config->hostname = NULL;
}
if (config->keyFile) {
WFREE(config->keyFile, NULL, 0);
+ config->keyFile = NULL;
}
if (config->pubKeyFile) {
WFREE(config->pubKeyFile, NULL, 0);
+ config->pubKeyFile = NULL;
}
if (config->command) {
WFREE(config->command, NULL, 0);
+ config->command = NULL;
}
return 0;
@@ -900,6 +919,7 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args)
SOCKADDR_IN_T clientAddr;
socklen_t clientAddrSz = sizeof(clientAddr);
int ret = 0;
+ int ioErr = 0;
byte keepOpen = 1;
#ifdef USE_WINDOWS_API
byte rawMode = 0;
@@ -1037,6 +1057,7 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args)
wc_InitMutex(&arg.lock);
arg.ssh = ssh;
+ arg.readError = 0;
#ifdef WOLFSSH_TERM
arg.quit = 0;
#if (defined(__OSX__) || defined(__APPLE__))
@@ -1082,12 +1103,14 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args)
sem_destroy(&windowSem);
#endif
#endif /* WOLFSSH_TERM */
+ ioErr = arg.readError;
#elif defined(_MSC_VER)
thread_args arg;
HANDLE thread[2];
arg.ssh = ssh;
arg.rawMode = rawMode;
+ arg.readError = 0;
wc_InitMutex(&arg.lock);
if (config.command) {
@@ -1107,6 +1130,7 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args)
WaitForSingleObject(thread[1], INFINITE);
CloseHandle(thread[0]);
CloseHandle(thread[1]);
+ ioErr = arg.readError;
#else
err_sys("No threading to use");
#endif
@@ -1139,10 +1163,13 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args)
#if defined(WOLFSSH_TERM) || defined(WOLFSSH_SHELL)
((func_args*)args)->return_code = wolfSSH_GetExitStatus(ssh);
#endif
+ if (ioErr != 0 && ((func_args*)args)->return_code == 0) {
+ ((func_args*)args)->return_code = 1;
+ }
wolfSSH_free(ssh);
wolfSSH_CTX_free(ctx);
- if (ret != WS_SUCCESS && ret != WS_SOCKET_ERROR_E) {
+ if ((ret != WS_SUCCESS && ret != WS_SOCKET_ERROR_E) || ioErr != 0) {
WLOG(WS_LOG_DEBUG, "Closing client stream failed");
#if defined(WOLFSSH_TERM) || defined(WOLFSSH_SHELL)
/* override return value, do not want to return success if connection
diff --git a/src/internal.c b/src/internal.c
index dfadcedea..5a162f3ce 100644
--- a/src/internal.c
+++ b/src/internal.c
@@ -691,6 +691,8 @@ INLINE static int IsMessageAllowedClient(WOLFSSH *ssh, byte msg)
/* Error if expecting a specific message and didn't receive. */
if (ssh->handshake && ssh->handshake->expectMsgId != MSGID_NONE) {
+ /* The explicit expectMsgId check supersedes the old
+ * IsMessageAllowedKeying() stub for rekey filtering. */
if (msg != ssh->handshake->expectMsgId) {
WLOG(WS_LOG_DEBUG,
"Message ID %u not the expected message %u",
diff --git a/tests/include.am b/tests/include.am
index a9aed41c0..644b4af59 100644
--- a/tests/include.am
+++ b/tests/include.am
@@ -45,7 +45,8 @@ tests_kex_test_CPPFLAGS = -DNO_MAIN_DRIVER $(AM_CPPFLAGS)
tests_kex_test_LDADD = src/libwolfssh.la
tests_kex_test_DEPENDENCIES = src/libwolfssh.la
-tests_regress_test_SOURCES = tests/regress.c
+tests_regress_test_SOURCES = tests/regress.c apps/wolfssh/common.c \
+ apps/wolfssh/common.h
tests_regress_test_CPPFLAGS = -DNO_MAIN_DRIVER -DWOLFSSH_TEST_INTERNAL $(AM_CPPFLAGS)
tests_regress_test_LDADD = src/libwolfssh_test.la
tests_regress_test_DEPENDENCIES = src/libwolfssh_test.la
diff --git a/tests/regress.c b/tests/regress.c
index fbd6032e5..0fe4218f2 100644
--- a/tests/regress.c
+++ b/tests/regress.c
@@ -19,10 +19,13 @@
#include <stdlib.h>
#include <arpa/inet.h>
#include <string.h>
+#include <unistd.h>
+#include <fcntl.h>
#include <wolfssh/port.h>
#include <wolfssh/ssh.h>
#include <wolfssh/internal.h>
+#include "apps/wolfssh/common.h"
#ifndef WOLFSSH_NO_ABORT
#define WABORT() abort()
@@ -362,6 +365,44 @@ static void TestKexInitRejectedWhenKeying(WOLFSSH* ssh)
AssertFalse(allowed);
}
+/* Ensure client buffer cleanup tolerates multiple invocations after allocs. */
+static void TestClientBuffersIdempotent(void)
+{
+ int ret;
+
+ ret = ClientUsePubKey("keys/gretel-key-rsa.pub");
+ AssertIntEQ(ret, 0);
+ ret = ClientSetPrivateKey("keys/gretel-key-rsa.pem");
+ AssertIntEQ(ret, 0);
+
+ ClientFreeBuffers();
+ /* Should be safe to call again without double free. */
+ ClientFreeBuffers();
+}
+
+/* Simulate Ctrl+D (stdin EOF) during password prompt; expect failure but no crash. */
+static void TestPasswordEofNoCrash(void)
+{
+ WS_UserAuthData auth;
+ int savedStdin, devNull, ret;
+
+ WMEMSET(&auth, 0, sizeof(auth));
+
+ savedStdin = dup(STDIN_FILENO);
+ devNull = open("/dev/null", O_RDONLY);
+ AssertTrue(devNull >= 0);
+ AssertTrue(dup2(devNull, STDIN_FILENO) >= 0);
+
+ ret = ClientUserAuth(WOLFSSH_USERAUTH_PASSWORD, &auth, NULL);
+ AssertIntEQ(ret, WOLFSSH_USERAUTH_FAILURE);
+
+ close(devNull);
+ dup2(savedStdin, STDIN_FILENO);
+ close(savedStdin);
+
+ ClientFreeBuffers();
+}
+
int main(int argc, char** argv)
{
@@ -390,6 +431,12 @@ int main(int argc, char** argv)
TestChannelBlockedBeforeAuth(ssh);
TestChannelAllowedAfterAuth(ssh);
TestKexInitRejectedWhenKeying(ssh);
+ TestClientBuffersIdempotent();
+ TestPasswordEofNoCrash();
+
+ /* TODO: add app-level regressions that simulate stdin EOF/password
+ * prompts and mid-session socket closes once the test harness can
+ * drive the wolfssh client without real sockets/tty. */
ResetSession(ssh);
wolfSSH_free(ssh);
From 2e5484f36f279484ca06e02ef34ddd420fc0cf87 Mon Sep 17 00:00:00 2001
From: John Safranek <john@wolfssl.com>
Date: Thu, 4 Dec 2025 15:52:39 -0800
Subject: [PATCH 5/7] Server Out of Order Message Handling
1. Updated the checking for the server to be more like the client's
checking.
---
src/internal.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 58 insertions(+), 1 deletion(-)
diff --git a/src/internal.c b/src/internal.c
index 5a162f3ce..c3e6c41e3 100644
--- a/src/internal.c
+++ b/src/internal.c
@@ -601,15 +601,71 @@ INLINE static int IsMessageAllowedServer(WOLFSSH *ssh, byte msg)
/* Only the server should send these messages, never receive. */
if (msg == MSGID_SERVICE_ACCEPT) {
WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by %s %s",
- msg, "client", "ever");
+ msg, "server", "ever");
+ ssh->error = WS_MSGID_NOT_ALLOWED_E;
return 0;
}
+ if (msg == MSGID_SERVICE_REQUEST) {
+ if (ssh->acceptState == ACCEPT_KEYED) {
+ return 1;
+ }
+ else {
+ WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by %s %s",
+ msg, "server", "after starting user auth");
+ return 0;
+ }
+ }
+
/* Transport Layer Generic messages are always allowed. */
if (MSGIDLIMIT_TRANS_GEN(msg)) {
return 1;
}
+ /* Is KEX complete? */
+ if (MSGIDLIMIT_TRANS(msg)) {
+ if (ssh->isKeying & WOLFSSH_PEER_IS_KEYING) {
+ /* MSGID_KEXINIT not allowed when keying. */
+ if (msg == MSGID_KEXINIT) {
+ WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by %s %s",
+ msg, "server", "when keying");
+ ssh->error = WS_REKEYING;
+ return 0;
+ }
+
+ /* Error if expecting a specific message and didn't receive. */
+ if (ssh->handshake && ssh->handshake->expectMsgId != MSGID_NONE) {
+ /* The explicit expectMsgId check supersedes the old
+ * IsMessageAllowedKeying() stub for rekey filtering. */
+ if (msg != ssh->handshake->expectMsgId) {
+ WLOG(WS_LOG_DEBUG,
+ "Message ID %u not the expected message %u",
+ msg, ssh->handshake->expectMsgId);
+ ssh->error = WS_REKEYING;
+ return 0;
+ }
+ else {
+ /* Got the expected message, clear expectation. */
+ ssh->handshake->expectMsgId = MSGID_NONE;
+ return 1;
+ }
+ }
+ }
+ else {
+ /* MSGID_KEXINIT only allowed when not keying. */
+ if (msg == MSGID_KEXINIT) {
+ return 1;
+ }
+
+ /* All other transport KEX and ALGO messages are not allowed
+ * when not keying. */
+ WLOG(WS_LOG_DEBUG, "Message ID %u not allowed by %s %s",
+ msg, "server", "when not keying");
+ ssh->error = WS_MSGID_NOT_ALLOWED_E;
+ return 0;
+ }
+ }
+
/* Has client userauth started? */
/* Allows the server to receive up to KEXDH GEX Request during KEX. */
if (ssh->acceptState < ACCEPT_KEYED) {
@@ -617,6 +673,7 @@ INLINE static int IsMessageAllowedServer(WOLFSSH *ssh, byte msg)
return 0;
}
}
+
/* Is server userauth complete? */
if (ssh->acceptState < ACCEPT_SERVER_USERAUTH_SENT) {
/* The server should only receive the user auth request message,
From 03ca9221a36a2a7160a99aeee7bc7ea8368013b5 Mon Sep 17 00:00:00 2001
From: John Safranek <john@wolfssl.com>
Date: Tue, 9 Dec 2025 09:17:47 -0800
Subject: [PATCH 6/7] Zephyr
1. Exclude the file regress.c from the Zephyr testing sample. The test
is covered in many other environments already. The test needs some
retooling to fit in with the Zephyr build, as it is a standalone
application with a main() function and it depends on a testing build
of libwolfssh.
2. Whitespace.
---
zephyr/samples/tests/CMakeLists.txt | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/zephyr/samples/tests/CMakeLists.txt b/zephyr/samples/tests/CMakeLists.txt
index 4f563b495..3de00b745 100644
--- a/zephyr/samples/tests/CMakeLists.txt
+++ b/zephyr/samples/tests/CMakeLists.txt
@@ -4,8 +4,12 @@ find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE})
project(wolfssl_tests)
FILE(GLOB app_sources ../../../tests/*.c ../../../examples/client/client.c
- ../../../examples/client/common.c ../../../examples/echoserver/echoserver.c
- ../../../examples/sftpclient/sftpclient.c tests.c)
+ ../../../examples/client/common.c ../../../examples/echoserver/echoserver.c
+ ../../../examples/sftpclient/sftpclient.c tests.c)
+# Remove the file regress.c from the list of test app sources. The regression
+# test is covered in many other environments.
+list(REMOVE_ITEM app_sources
+ "${CMAKE_CURRENT_SOURCE_DIR}/../../../tests/regress.c")
target_sources(app PRIVATE ${app_sources})
add_definitions(-DWOLFSSL_ZEPHYR)
add_definitions(-DWOLFSSL_USER_SETTINGS)
From 2086f34ff37911a1fa388f843ee9d336e740bf44 Mon Sep 17 00:00:00 2001
From: John Safranek <john@wolfssl.com>
Date: Thu, 11 Dec 2025 10:43:35 -0800
Subject: [PATCH 7/7] Out of Order Message Handling
1. Always set the expected message right before sending. If the send
fails, it is either because the socket is closing, or it is wanting
to block. If it is wanting to block, we still want to check the next
message as expected.
---
src/internal.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/src/internal.c b/src/internal.c
index c3e6c41e3..fb67a53c8 100644
--- a/src/internal.c
+++ b/src/internal.c
@@ -5989,8 +5989,9 @@ static int DoKexDhReply(WOLFSSH* ssh, byte* buf, word32 len, word32* idx)
}
if (ret == WS_SUCCESS) {
- ret = SendNewKeys(ssh);
ssh->handshake->expectMsgId = MSGID_NEWKEYS;
+ WLOG_EXPECT_MSGID(ssh->handshake->expectMsgId);
+ ret = SendNewKeys(ssh);
}
if (sigKeyBlock_ptr)
@@ -12729,12 +12730,10 @@ int SendKexDhGexRequest(WOLFSSH* ssh)
ret = BundlePacket(ssh);
}
- if (ret == WS_SUCCESS)
- ret = wolfSSH_SendPacket(ssh);
-
if (ret == WS_SUCCESS) {
WLOG_EXPECT_MSGID(MSGID_KEXDH_GEX_GROUP);
ssh->handshake->expectMsgId = MSGID_KEXDH_GEX_GROUP;
+ ret = wolfSSH_SendPacket(ssh);
}
WLOG(WS_LOG_DEBUG, "Leaving SendKexDhGexRequest(), ret = %d", ret);
@@ -13086,12 +13085,10 @@ int SendKexDhInit(WOLFSSH* ssh)
ret = BundlePacket(ssh);
}
- if (ret == WS_SUCCESS)
- ret = wolfSSH_SendPacket(ssh);
-
if (ret == WS_SUCCESS) {
WLOG_EXPECT_MSGID(expectMsgId);
ssh->handshake->expectMsgId = expectMsgId;
+ ret = wolfSSH_SendPacket(ssh);
}
WLOG(WS_LOG_DEBUG, "Leaving SendKexDhInit(), ret = %d", ret);
Reference in New Issue View Git Blame Copy Permalink