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
134 changes: 109 additions & 25 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 @@ -4854,6 +4861,26 @@ static WS_MAYBE_UNUSED int PacketIoRecv(WOLFSSH* ssh, void* buf, word32 sz, void
return (int)n;
}

/* Write budget for the IOSend mocks: that many writes are refused with a
* would-block before the mock acts. */
static int s_sendRefusals = 0;

/* Refuses the first s_sendRefusals writes, then resets the socket. */
static WS_MAYBE_UNUSED int RefuseThenResetIoSend(WOLFSSH* ssh, void* buf,
word32 sz, void* ctx)
{
WOLFSSH_UNUSED(ssh);
WOLFSSH_UNUSED(buf);
WOLFSSH_UNUSED(sz);
WOLFSSH_UNUSED(ctx);

if (s_sendRefusals > 0) {
s_sendRefusals--;
return WS_CBIO_ERR_WANT_WRITE;
}
return WS_CBIO_ERR_CONN_RST;
}

/* Builds a plaintext CHANNEL_EXTENDED_DATA (stderr) SSH packet addressed to
* channelId, carrying 10 bytes of payload set to fill, into pkt (needs 32
* bytes) and returns its size. A bare session negotiates no cipher
Expand Down Expand Up @@ -5705,13 +5732,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 +7209,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 @@ -7874,7 +7896,6 @@ static int test_StreamReadEofOtherChannel(void)

static byte s_sentBuf[512];
static word32 s_sentSz = 0;
static int s_sendRefusals = 0;

/* Refuses the first s_sendRefusals writes with a would-block, then takes
* everything and keeps a copy of what reached the transport. */
Expand All @@ -7896,23 +7917,6 @@ static int RefuseThenCaptureIoSend(WOLFSSH* ssh, void* buf, word32 sz,
}


/* Refuses the sends DoChannelClose() makes, then resets the socket under the
* worker's flush. */
static int RefuseThenResetIoSend(WOLFSSH* ssh, void* buf, word32 sz, void* ctx)
{
WOLFSSH_UNUSED(ssh);
WOLFSSH_UNUSED(buf);
WOLFSSH_UNUSED(sz);
WOLFSSH_UNUSED(ctx);

if (s_sendRefusals > 0) {
s_sendRefusals--;
return WS_CBIO_ERR_WANT_WRITE;
}
return WS_CBIO_ERR_CONN_RST;
}


/* DoPacket() consumes the peer's CHANNEL_CLOSE whatever DoChannelClose()
* returns, so the reply gets one chance to be built. A blocked socket must not
* cost it: the EOF and the close both have to be bundled, and the channel
Expand Down Expand Up @@ -8756,6 +8760,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;
Comment thread
ejohnstown marked this conversation as resolved.
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 +22875,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