diff --git a/src/internal.c b/src/internal.c index 77382f50c..90eb3800e 100644 --- a/src/internal.c +++ b/src/internal.c @@ -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()"); @@ -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) { @@ -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; diff --git a/src/ssh.c b/src/ssh.c index 2afd0cab4..98538e465 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -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, diff --git a/tests/regress.c b/tests/regress.c index 3c638fe81..34324dc66 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -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) { @@ -16038,6 +16071,7 @@ int main(int argc, char** argv) TestChannelReqSubsysCallbackRuns(); TestSessionReqCallbackSeesCommandSz(); TestChannelGetSessionGrantedAccessor(); + TestRekeyPendingAccessor(); TestMalformedSessionRequestSkipsCallback(); TestSessionReqCallbackMayFreeChannel(); TestAppChannelsAcceptKeepsStopWithPendingOutput(); diff --git a/tests/testsuite.c b/tests/testsuite.c index 48ed59532..77764bdba 100644 --- a/tests/testsuite.c +++ b/tests/testsuite.c @@ -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; diff --git a/tests/unit.c b/tests/unit.c index e8ddc1d2a..b421068f5 100644 --- a/tests/unit.c +++ b/tests/unit.c @@ -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; @@ -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 @@ -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; @@ -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; + 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 */ @@ -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 diff --git a/wolfssh/internal.h b/wolfssh/internal.h index d41e93309..6682661b3 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -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 diff --git a/wolfssh/ssh.h b/wolfssh/ssh.h index 374ecc0d8..44d80f46c 100644 --- a/wolfssh/ssh.h +++ b/wolfssh/ssh.h @@ -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 @@ -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);