Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions src/internal.c
Original file line number Diff line number Diff line change
Expand Up @@ -15194,6 +15194,7 @@ int SendKexInit(WOLFSSH* ssh)
macAlgoNamesSz = 0, noneNamesSz = 0;

int ret = WS_SUCCESS;
int delivered = 0;

WLOG(WS_LOG_DEBUG, "Entering SendKexInit()");

Expand All @@ -15220,8 +15221,6 @@ int SendKexInit(WOLFSSH* ssh)
}

if (ret == WS_SUCCESS) {
/* Set self is keying flag since we started sending the KEX init msg */
ssh->isKeying |= WOLFSSH_SELF_IS_KEYING;
if (ssh->handshake == NULL) {
ssh->handshake = HandshakeInfoNew(ssh->ctx->heap);
if (ssh->handshake == NULL) {
Expand Down Expand Up @@ -15349,11 +15348,19 @@ int SendKexInit(WOLFSSH* ssh)
}

if (ret == WS_SUCCESS) {
word32 flushes = ssh->txFlushCount;

ret = wolfSSH_SendPacket(ssh);
delivered = SendPacketDelivered(ssh, flushes, ret);
}

if (ret != WS_WANT_WRITE && ret != WS_SUCCESS)
if (delivered) {
/* Set self is keying flag now the KEX init msg is away */
ssh->isKeying |= WOLFSSH_SELF_IS_KEYING;
}
else {
PurgePacket(ssh);
}

WLOG(WS_LOG_DEBUG, "Leaving SendKexInit(), ret = %d", ret);
return ret;
Expand Down
6 changes: 6 additions & 0 deletions src/ssh.c
Original file line number Diff line number Diff line change
Expand Up @@ -4715,6 +4715,12 @@ int wolfSSH_OutputPending(const WOLFSSH* ssh)
}


int wolfSSH_RekeyPending(const WOLFSSH* ssh)
{
return (ssh != NULL && ssh->isKeying != 0);
}


#ifdef WOLFSSH_FWD

int wolfSSH_CTX_SetFwdCb(WOLFSSH_CTX* ctx,
Expand Down
34 changes: 34 additions & 0 deletions tests/regress.c
Original file line number Diff line number Diff line change
Expand Up @@ -5893,6 +5893,39 @@ static void TestChannelGetSessionGrantedAccessor(void)
}


/* Covers each keying bit alone, both together, and a NULL session. */
static void TestRekeyPendingAccessor(void)
{
WOLFSSH_CTX* ctx;
WOLFSSH* ssh;

AssertIntEQ(wolfSSH_RekeyPending(NULL), 0);
AssertIntEQ(wolfSSH_OutputPending(NULL), 0);

ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL);
AssertNotNull(ctx);
ssh = wolfSSH_new(ctx);
AssertNotNull(ssh);

AssertIntEQ(wolfSSH_RekeyPending(ssh), 0);

ssh->isKeying = WOLFSSH_PEER_IS_KEYING;
AssertTrue(wolfSSH_RekeyPending(ssh) != 0);

ssh->isKeying = WOLFSSH_SELF_IS_KEYING;
AssertTrue(wolfSSH_RekeyPending(ssh) != 0);

ssh->isKeying = WOLFSSH_SELF_IS_KEYING | WOLFSSH_PEER_IS_KEYING;
AssertTrue(wolfSSH_RekeyPending(ssh) != 0);

ssh->isKeying = 0;
AssertIntEQ(wolfSSH_RekeyPending(ssh), 0);

wolfSSH_free(ssh);
wolfSSH_CTX_free(ctx);
}


/* A username change after the first userauth request must end the session. */
static void TestUsernameChangeDisconnects(void)
{
Expand Down Expand Up @@ -16038,6 +16071,7 @@ int main(int argc, char** argv)
TestChannelReqSubsysCallbackRuns();
TestSessionReqCallbackSeesCommandSz();
TestChannelGetSessionGrantedAccessor();
TestRekeyPendingAccessor();
TestMalformedSessionRequestSkipsCallback();
TestSessionReqCallbackMayFreeChannel();
TestAppChannelsAcceptKeepsStopWithPendingOutput();
Expand Down
7 changes: 0 additions & 7 deletions tests/testsuite.c
Original file line number Diff line number Diff line change
Expand Up @@ -241,13 +241,6 @@ int wolfSSH_TestsuiteTest(int argc, char** argv)

wolfSSH_Init();

/* Linked against the installed library, so this also proves
* wolfSSH_OutputPending() is exported and not hidden. */
if (wolfSSH_OutputPending(NULL) != 0) {
fprintf(stderr, "wolfSSH_OutputPending(NULL) was not zero\n");
return EXIT_FAILURE;
}

#if defined(FIPS_VERSION_GE) && FIPS_VERSION_GE(5,2)
{
int i;
Expand Down
96 changes: 89 additions & 7 deletions tests/unit.c
Original file line number Diff line number Diff line change
Expand Up @@ -4565,6 +4565,13 @@ static WS_MAYBE_UNUSED int OobIoSend(WOLFSSH* ssh, void* buf, word32 sz,
return (int)sz + 1;
}

/* Fires once the message highwater mark is crossed and reports an error. */
static WS_MAYBE_UNUSED int FailHighwater(byte side, void* ctx)
{
(void)side; (void)ctx;
return WS_FATAL_ERROR;
}

static int test_DoChannelExtendedData_overflow(void)
{
WOLFSSH_CTX* ctx = NULL;
Expand Down Expand Up @@ -5705,13 +5712,6 @@ static int test_ChannelExtDataBufferGrowth(void)

#ifndef NO_WOLFSSH_SERVER

/* Fires once the message highwater mark is crossed and reports an error. */
static int FailHighwater(byte side, void* ctx)
{
(void)side; (void)ctx;
return WS_FATAL_ERROR;
}

/* wolfSSH_SendPacket() runs the highwater check after the packet is on the wire
* and returns the highwater callback's status, so a failing callback makes a
* delivered WINDOW_ADJUST look like a failed send. Credit re-parked then is
Expand Down Expand Up @@ -7189,6 +7189,8 @@ static int test_WorkerKeyingReportsRekey(void)
if (reportedId != ch->channel) { result = -1818; goto done; }
/* The flush ran and drained, which the rekey report is gated on. */
if (ssh->outputBuffer.length != 0) { result = -1817; goto done; }
/* The predicate answers the same pass the status reports. */
if (!wolfSSH_RekeyPending(ssh)) { result = -1819; goto done; }

done:
s_recvPkt = NULL;
Expand Down Expand Up @@ -8756,6 +8758,81 @@ static int test_TriggerKeyExchangeKeepsError(void)
wolfSSH_CTX_free(ctx);
return result;
}


/* Covers a KEX init whose send fails outright and one that short-writes. */
static int test_KexInitSendAwayGatesKeying(void)
{
WOLFSSH_CTX* ctx = NULL;
WOLFSSH* ssh = NULL;
int result = 0;
int ret;

ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL);
if (ctx == NULL)
return -1897;
/* No refusals, so the first write resets the socket. */
s_sendRefusals = 0;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New rekey test breaks client-only builds · Logic errors

test_KexInitSendAwayGatesKeying() is compiled when the client is enabled, but s_sendRefusals and RefuseThenResetIoSend are server-only. Defining NO_WOLFSSH_SERVER makes tests/unit.c fail to compile.

Related known finding #10542 (similar but distinct): Both are build-configuration defects, but #10542 incorrectly gates public-key authentication in DoUserAuthFailure/GetAllowedAuth, whereas this test references server-only symbols in a client-enabled compilation path. The faulting operations, root causes, locations, and required patches differ.

Suggested fix: Move the shared refusal state and callback outside the server-only block, marking the callback unused where necessary.
Basis: ISO C17 §6.5.1 requires identifiers used as primary expressions to designate declared objects or functions.

wolfSSH_SetIOSend(ctx, RefuseThenResetIoSend);
wolfSSH_SetIORecv(ctx, PacketIoRecv);

ssh = wolfSSH_new(ctx);
if (ssh == NULL) { result = -1898; goto done; }

ret = wolfSSH_TriggerKeyExchange(ssh);
if (ret == WS_SUCCESS || ret == WS_WANT_WRITE) {
result = -1899;
goto done;
}
if (wolfSSH_RekeyPending(ssh)) { result = -1900; goto done; }

wolfSSH_free(ssh);

/* One refusal short-writes instead. */
s_sendRefusals = 1;
ssh = wolfSSH_new(ctx);
if (ssh == NULL) { result = -1903; goto done; }

ret = wolfSSH_TriggerKeyExchange(ssh);
if (ret != WS_SUCCESS && ret != WS_WANT_WRITE) {
result = -1904;
goto done;
}
if (!wolfSSH_RekeyPending(ssh)) { result = -1905; goto done; }

wolfSSH_free(ssh);
ssh = NULL;
wolfSSH_CTX_free(ctx);
ctx = NULL;

/* The transport takes the whole packet and the highwater callback then
* fails, so the error arrives with the KEX init already sent. */
ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL);
if (ctx == NULL) { result = -1906; goto done; }
wolfSSH_SetIOSend(ctx, DiscardIoSend);
wolfSSH_SetIORecv(ctx, PacketIoRecv);
wolfSSH_SetHighwaterCb(ctx, 1, FailHighwater);

ssh = wolfSSH_new(ctx);
if (ssh == NULL) { result = -1907; goto done; }
if (wolfSSH_SetHighwater(ssh, 1) != WS_SUCCESS) {
result = -1908;
goto done;
}

ret = wolfSSH_TriggerKeyExchange(ssh);
if (ret == WS_SUCCESS) { result = -1909; goto done; }
if (!wolfSSH_RekeyPending(ssh)) { result = -1910; goto done; }

done:
s_sendRefusals = 0;
s_recvPkt = NULL;
s_recvPktSz = 0;
s_recvPktOff = 0;
wolfSSH_free(ssh);
wolfSSH_CTX_free(ctx);
return result;
}
#endif /* NO_WOLFSSH_CLIENT */


Expand Down Expand Up @@ -22796,6 +22873,11 @@ int wolfSSH_UnitTest(int argc, char** argv)
printf("TriggerKeyExchangeKeepsError: %s\n",
(unitResult == 0 ? "SUCCESS" : "FAILED"));
testResult = testResult || unitResult;

unitResult = test_KexInitSendAwayGatesKeying();
printf("KexInitSendAwayGatesKeying: %s\n",
(unitResult == 0 ? "SUCCESS" : "FAILED"));
testResult = testResult || unitResult;
#endif


Expand Down
2 changes: 1 addition & 1 deletion wolfssh/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -736,7 +736,7 @@ enum NameIdType {
#define WOLFSSH_PROTOID_LIMIT 255

/* Keep track of keying state for both sides of the connection.
* WOLFSSH_SELF_IS_KEYING gets set on sending KEX init and
* WOLFSSH_SELF_IS_KEYING gets set once the KEX init is sent or queued and
* WOLFSSH_PEER_IS_KEYING gets set on receiving KEX init */
#define WOLFSSH_PEER_IS_KEYING 0x01
#define WOLFSSH_SELF_IS_KEYING 0x02
Expand Down
7 changes: 7 additions & 0 deletions wolfssh/ssh.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ WOLFSSH_API void wolfSSH_free(WOLFSSH* ssh);
* the peer's disconnect, which is how most sessions end.
* To ask whether a write is still owed, call wolfSSH_OutputPending() rather
* than reading a status: it answers after any return, including a success.
* To ask whether a key exchange is in flight, call wolfSSH_RekeyPending()
* rather than reading a status: it answers after any return.
*
* For WS_CHAN_RXD, WS_EXTDATA, WS_EOF, WS_SUCCESS and a WS_REKEYING that
* displaced one of those, channelId (when not NULL) names the channel the
Expand All @@ -118,6 +120,11 @@ WOLFSSH_API int wolfSSH_GetLastRxId(WOLFSSH* ssh, word32* channelId);
/* Returns nonzero if a write is still owed. Session state */
WOLFSSH_API int wolfSSH_OutputPending(const WOLFSSH* ssh);

/* Returns nonzero while a key exchange is in flight, and 0 otherwise,
* including when ssh is NULL. Only NEWKEYS from both sides clears it, so a
* peer that abandons the exchange leaves it set. */
WOLFSSH_API int wolfSSH_RekeyPending(const WOLFSSH* ssh);

WOLFSSH_API int wolfSSH_set_fd(WOLFSSH* ssh, WS_SOCKET_T fd);
WOLFSSH_API WS_SOCKET_T wolfSSH_get_fd(const WOLFSSH* ssh);

Expand Down
Loading