Skip to content

Commit cfdd02e

Browse files
committed
rekey: trigger highwater on per-key packet count
- Track txMsgCount/rxMsgCount per key epoch and reset on NEW_KEYS; seq/peerSeq still wrap freely per RFC 4253 Sec 6.4. - Extend HighwaterCheck to fire highwaterCb when packet count crosses msgHighwaterMark (default 2^31, RFC 4344 Sec 3.1). - Add wolfSSH_CTX_SetMsgHighwater / SetMsgHighwater / GetMsgHighwater. - Consolidate receive-path HighwaterCheck into DoPacket so byte- and packet-count thresholds share a single canonical fire site. Issue: F-246
1 parent 834e60f commit cfdd02e

5 files changed

Lines changed: 245 additions & 32 deletions

File tree

src/internal.c

Lines changed: 53 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,9 @@
188188
Set the number of Miller-Rabin rounds used when the client checks the
189189
server's prime group when using GEX key exchange. The default is 8. More
190190
rounds are better, but also takes a lot longer.
191+
WOLFSSH_DEFAULT_MSG_HIGHWATER_MARK
192+
Set the default value for the number of messages to send or receive before
193+
calling the highwater callback function. By default this forces a rekey.
191194
*/
192195

193196
static const char sshProtoIdStr[] = "SSH-2.0-wolfSSHv"
@@ -544,7 +547,10 @@ static int HashUpdate(wc_HashAlg* hash, enum wc_HashType type,
544547
static INLINE int HighwaterCheck(WOLFSSH* ssh, byte side)
545548
{
546549
int ret = WS_SUCCESS;
550+
int fire = 0;
547551

552+
/* RFC 4253 Sec 9: bound bytes per key (txCount/rxCount reset on rekey)
553+
* to limit cipher keystream/IV exhaustion under a single key. */
548554
if (!ssh->highwaterFlag && ssh->highwaterMark &&
549555
(ssh->txCount >= ssh->highwaterMark ||
550556
ssh->rxCount >= ssh->highwaterMark)) {
@@ -553,10 +559,28 @@ static INLINE int HighwaterCheck(WOLFSSH* ssh, byte side)
553559
(side == WOLFSSH_HWSIDE_TRANSMIT) ? "Transmit" : "Receive");
554560

555561
ssh->highwaterFlag = 1;
562+
fire = 1;
563+
}
564+
565+
/* RFC 4344 Sec 3.1: rekey before the 32-bit SSH sequence number wraps
566+
* to prevent MAC/nonce reuse. Counter is per-key (resets on rekey),
567+
* not the absolute ssh->seq (which does not reset); default 2^31
568+
* keeps each epoch comfortably under the 2^32 wrap. */
569+
if (!ssh->msgHighwaterFlag && ssh->msgHighwaterMark &&
570+
(ssh->txMsgCount >= ssh->msgHighwaterMark ||
571+
ssh->rxMsgCount >= ssh->msgHighwaterMark)) {
572+
573+
WLOG(WS_LOG_DEBUG, "%s over msg high water mark",
574+
(side == WOLFSSH_HWSIDE_TRANSMIT) ? "Transmit" : "Receive");
556575

557-
if (ssh->ctx->highwaterCb)
558-
ret = ssh->ctx->highwaterCb(side, ssh->highwaterCtx);
576+
ssh->msgHighwaterFlag = 1;
577+
fire = 1;
559578
}
579+
580+
581+
if (fire && ssh->ctx->highwaterCb)
582+
ret = ssh->ctx->highwaterCb(side, ssh->highwaterCtx);
583+
560584
return ret;
561585
}
562586

@@ -1025,6 +1049,7 @@ WOLFSSH_CTX* CtxInit(WOLFSSH_CTX* ctx, byte side, void* heap)
10251049
ctx->ioSendCb = wsEmbedSend;
10261050
#endif /* WOLFSSH_USER_IO */
10271051
ctx->highwaterMark = DEFAULT_HIGHWATER_MARK;
1052+
ctx->msgHighwaterMark = WOLFSSH_DEFAULT_MSG_HIGHWATER_MARK;
10281053
ctx->highwaterCb = wsHighwater;
10291054
#if defined(WOLFSSH_SCP) && !defined(WOLFSSH_SCP_USER_CALLBACKS)
10301055
ctx->scpRecvCb = wsScpRecvCallback;
@@ -1227,6 +1252,7 @@ WOLFSSH* SshInit(WOLFSSH* ssh, WOLFSSH_CTX* ctx)
12271252
ssh->ioReadCtx = &ssh->rfd; /* prevent invalid access if not correctly */
12281253
ssh->ioWriteCtx = &ssh->wfd; /* set */
12291254
ssh->highwaterMark = ctx->highwaterMark;
1255+
ssh->msgHighwaterMark = ctx->msgHighwaterMark;
12301256
ssh->highwaterCtx = (void*)ssh;
12311257
ssh->reqSuccessCtx = (void*)ssh;
12321258
ssh->fs = NULL;
@@ -6284,7 +6310,9 @@ static int DoNewKeys(WOLFSSH* ssh, byte* buf, word32 len, word32* idx)
62846310

62856311
if (ret == WS_SUCCESS) {
62866312
ssh->rxCount = 0;
6313+
ssh->rxMsgCount = 0;
62876314
ssh->highwaterFlag = 0;
6315+
ssh->msgHighwaterFlag = 0;
62886316

62896317
/* Clear peer is keying flag */
62906318
ssh->isKeying &= ~WOLFSSH_PEER_IS_KEYING;
@@ -10162,7 +10190,23 @@ static int DoPacket(WOLFSSH* ssh, byte* bufferConsumed)
1016210190
idx += padSz;
1016310191
ssh->inputBuffer.idx = idx;
1016410192
ssh->peerSeq++;
10193+
ssh->rxMsgCount++;
1016510194
*bufferConsumed = 1;
10195+
10196+
/* Canonical receive-path highwater check. rxCount was advanced by
10197+
* Decrypt/DecryptAead in DoReceive and rxMsgCount just above, so
10198+
* either threshold can fire here on the crossing packet. Run the
10199+
* check unconditionally so the flag-set side effect and callback
10200+
* fire on data packets (DoChannelData et al. return informational
10201+
* status like WS_CHAN_RXD, not WS_SUCCESS). Only fold the result
10202+
* into ret when ret is currently WS_SUCCESS, so a rekey-trigger
10203+
* return (e.g. WS_WANT_WRITE from SendKexInit) does not mask a
10204+
* real packet error or informational status already in ret. */
10205+
{
10206+
int hwRet = HighwaterCheck(ssh, WOLFSSH_HWSIDE_RECEIVE);
10207+
if (ret == WS_SUCCESS)
10208+
ret = hwRet;
10209+
}
1016610210
}
1016710211

1016810212
return ret;
@@ -10602,14 +10646,6 @@ int DoReceive(WOLFSSH* ssh)
1060210646
ssh->error = ret;
1060310647
return WS_FATAL_ERROR;
1060410648
}
10605-
10606-
ret = HighwaterCheck(ssh, WOLFSSH_HWSIDE_RECEIVE);
10607-
if (ret != WS_SUCCESS) {
10608-
WLOG(WS_LOG_DEBUG, "PR: First HighwaterCheck fail");
10609-
ssh->error = ret;
10610-
ret = WS_FATAL_ERROR;
10611-
break;
10612-
}
1061310649
}
1061410650
FALL_THROUGH;
1061510651

@@ -10694,14 +10730,6 @@ int DoReceive(WOLFSSH* ssh)
1069410730
}
1069510731
}
1069610732
ssh->processReplyState = PROCESS_PACKET;
10697-
10698-
ret = HighwaterCheck(ssh, WOLFSSH_HWSIDE_RECEIVE);
10699-
if (ret != WS_SUCCESS) {
10700-
WLOG(WS_LOG_DEBUG, "PR: HighwaterCheck fail");
10701-
ssh->error = ret;
10702-
ret = WS_FATAL_ERROR;
10703-
break;
10704-
}
1070510733
FALL_THROUGH;
1070610734

1070710735
case PROCESS_PACKET:
@@ -10991,6 +11019,7 @@ static int BundlePacket(WOLFSSH* ssh)
1099111019

1099211020
if (ret == WS_SUCCESS) {
1099311021
ssh->seq++;
11022+
ssh->txMsgCount++;
1099411023
ssh->outputBuffer.length = idx;
1099511024
}
1099611025
else {
@@ -13301,6 +13330,7 @@ int SendNewKeys(WOLFSSH* ssh)
1330113330

1330213331
if (ret == WS_SUCCESS) {
1330313332
ssh->txCount = 0;
13333+
ssh->txMsgCount = 0;
1330413334
}
1330513335

1330613336
if (ret == WS_SUCCESS) {
@@ -17976,6 +18006,11 @@ int wolfSSH_TestChannelPutData(WOLFSSH_CHANNEL* channel, byte* data,
1797618006
return ChannelPutData(channel, data, dataSz);
1797718007
}
1797818008

18009+
int wolfSSH_TestHighwaterCheck(WOLFSSH* ssh, byte side)
18010+
{
18011+
return HighwaterCheck(ssh, side);
18012+
}
18013+
1797918014
#ifndef WOLFSSH_NO_DH_GEX_SHA256
1798018015

1798118016
int wolfSSH_TestDoKexDhGexRequest(WOLFSSH* ssh, byte* buf, word32 len,

src/ssh.c

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -242,12 +242,12 @@ void* wolfSSH_GetFilesystemHandle(WOLFSSH* ssh)
242242
}
243243

244244

245-
int wolfSSH_SetHighwater(WOLFSSH* ssh, word32 highwater)
245+
int wolfSSH_SetHighwater(WOLFSSH* ssh, word32 level)
246246
{
247247
WLOG(WS_LOG_DEBUG, "Entering wolfSSH_SetHighwater()");
248248

249249
if (ssh) {
250-
ssh->highwaterMark = highwater;
250+
ssh->highwaterMark = level;
251251

252252
return WS_SUCCESS;
253253
}
@@ -267,13 +267,42 @@ word32 wolfSSH_GetHighwater(WOLFSSH* ssh)
267267
}
268268

269269

270-
void wolfSSH_SetHighwaterCb(WOLFSSH_CTX* ctx, word32 highwater,
271-
WS_CallbackHighwater cb)
270+
void wolfSSH_CTX_SetMsgHighwater(WOLFSSH_CTX* ctx, word32 level)
271+
{
272+
WLOG(WS_LOG_DEBUG, "Entering wolfSSH_CTX_SetMsgHighwater()");
273+
274+
if (ctx)
275+
ctx->msgHighwaterMark = level;
276+
}
277+
278+
279+
void wolfSSH_SetMsgHighwater(WOLFSSH* ssh, word32 level)
280+
{
281+
WLOG(WS_LOG_DEBUG, "Entering wolfSSH_SetMsgHighwater()");
282+
283+
if (ssh)
284+
ssh->msgHighwaterMark = level;
285+
}
286+
287+
288+
word32 wolfSSH_GetMsgHighwater(WOLFSSH* ssh)
289+
{
290+
WLOG(WS_LOG_DEBUG, "Entering wolfSSH_GetMsgHighwater()");
291+
292+
if (ssh)
293+
return ssh->msgHighwaterMark;
294+
295+
return 0;
296+
}
297+
298+
299+
void wolfSSH_SetHighwaterCb(WOLFSSH_CTX* ctx, word32 level,
300+
WS_CallbackHighwater cb)
272301
{
273302
WLOG(WS_LOG_DEBUG, "Entering wolfSSH_SetHighwaterCb()");
274303

275304
if (ctx) {
276-
ctx->highwaterMark = highwater;
305+
ctx->highwaterMark = level;
277306
ctx->highwaterCb = cb;
278307
}
279308
}

tests/unit.c

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1181,6 +1181,139 @@ static int test_ChannelPutData(void)
11811181
return result;
11821182
}
11831183

1184+
/* Counter callback for test_MsgHighwater. Records each invocation without
1185+
* triggering wolfSSH_TriggerKeyExchange (which needs a live session). */
1186+
typedef struct HwTestCtx {
1187+
int count;
1188+
byte lastSide;
1189+
} HwTestCtx;
1190+
1191+
static int HwTestCb(byte side, void* ctx)
1192+
{
1193+
HwTestCtx* hc = (HwTestCtx*)ctx;
1194+
if (hc != NULL) {
1195+
hc->count++;
1196+
hc->lastSide = side;
1197+
}
1198+
return WS_SUCCESS;
1199+
}
1200+
1201+
/* Exercise the wolfSSH_*MsgHighwater APIs and the per-key packet-count
1202+
* threshold path inside HighwaterCheck. Covers:
1203+
* - NULL safety on getters/setters
1204+
* - CTX default value matches WOLFSSH_DEFAULT_MSG_HIGHWATER_MARK
1205+
* - CTX/SSH setter round-trip and CTX -> SSH inheritance on wolfSSH_new
1206+
* - SSH setter does not bleed back into the CTX
1207+
* - Threshold crossing fires the highwater callback exactly once per epoch
1208+
* (msgHighwaterFlag gates re-firing under the same keys)
1209+
* - Receive side fires independently of the transmit side
1210+
* - msgHighwaterMark == 0 disables the per-key packet check */
1211+
static int test_MsgHighwater(void)
1212+
{
1213+
WOLFSSH_CTX* ctx = NULL;
1214+
WOLFSSH* ssh = NULL;
1215+
HwTestCtx hc;
1216+
int result = 0;
1217+
1218+
if (wolfSSH_GetMsgHighwater(NULL) != 0)
1219+
return -800;
1220+
wolfSSH_CTX_SetMsgHighwater(NULL, 1234);
1221+
wolfSSH_SetMsgHighwater(NULL, 1234);
1222+
1223+
ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL);
1224+
if (ctx == NULL)
1225+
return -801;
1226+
1227+
if (ctx->msgHighwaterMark != WOLFSSH_DEFAULT_MSG_HIGHWATER_MARK) {
1228+
result = -802;
1229+
goto done;
1230+
}
1231+
1232+
wolfSSH_CTX_SetMsgHighwater(ctx, 4096);
1233+
if (ctx->msgHighwaterMark != 4096) {
1234+
result = -803;
1235+
goto done;
1236+
}
1237+
1238+
ssh = wolfSSH_new(ctx);
1239+
if (ssh == NULL) {
1240+
result = -804;
1241+
goto done;
1242+
}
1243+
if (wolfSSH_GetMsgHighwater(ssh) != 4096) {
1244+
result = -805;
1245+
goto done;
1246+
}
1247+
1248+
wolfSSH_SetMsgHighwater(ssh, 16);
1249+
if (wolfSSH_GetMsgHighwater(ssh) != 16) {
1250+
result = -806;
1251+
goto done;
1252+
}
1253+
if (ctx->msgHighwaterMark != 4096) {
1254+
result = -807;
1255+
goto done;
1256+
}
1257+
1258+
/* Install a counter callback. ssh->highwaterMark stays at the inherited
1259+
* default (~1 GiB) and txCount/rxCount are not touched, so the byte-count
1260+
* branch cannot fire and only the packet-count branch is under test. */
1261+
WMEMSET(&hc, 0, sizeof(hc));
1262+
wolfSSH_SetHighwaterCb(ctx, ctx->highwaterMark, HwTestCb);
1263+
wolfSSH_SetHighwaterCtx(ssh, &hc);
1264+
wolfSSH_SetMsgHighwater(ssh, 8);
1265+
1266+
ssh->txMsgCount = 7;
1267+
if (wolfSSH_TestHighwaterCheck(ssh, WOLFSSH_HWSIDE_TRANSMIT) != WS_SUCCESS
1268+
|| hc.count != 0) {
1269+
result = -808;
1270+
goto done;
1271+
}
1272+
1273+
ssh->txMsgCount = 8;
1274+
if (wolfSSH_TestHighwaterCheck(ssh, WOLFSSH_HWSIDE_TRANSMIT) != WS_SUCCESS
1275+
|| hc.count != 1
1276+
|| hc.lastSide != WOLFSSH_HWSIDE_TRANSMIT) {
1277+
result = -809;
1278+
goto done;
1279+
}
1280+
1281+
/* Flag-gated: further packets in the same epoch must not re-fire. */
1282+
ssh->txMsgCount = 100;
1283+
if (wolfSSH_TestHighwaterCheck(ssh, WOLFSSH_HWSIDE_TRANSMIT) != WS_SUCCESS
1284+
|| hc.count != 1) {
1285+
result = -810;
1286+
goto done;
1287+
}
1288+
1289+
/* Simulate a fresh key epoch (msgHighwaterFlag and rx/txMsgCount are
1290+
* reset by DoNewKeys/SendNewKeys) and verify the receive side fires. */
1291+
ssh->msgHighwaterFlag = 0;
1292+
ssh->rxMsgCount = 8;
1293+
if (wolfSSH_TestHighwaterCheck(ssh, WOLFSSH_HWSIDE_RECEIVE) != WS_SUCCESS
1294+
|| hc.count != 2
1295+
|| hc.lastSide != WOLFSSH_HWSIDE_RECEIVE) {
1296+
result = -811;
1297+
goto done;
1298+
}
1299+
1300+
/* mark == 0 disables the per-key packet check entirely. */
1301+
wolfSSH_SetMsgHighwater(ssh, 0);
1302+
ssh->msgHighwaterFlag = 0;
1303+
ssh->txMsgCount = 0xFFFFFFFFu;
1304+
ssh->rxMsgCount = 0xFFFFFFFFu;
1305+
if (wolfSSH_TestHighwaterCheck(ssh, WOLFSSH_HWSIDE_TRANSMIT) != WS_SUCCESS
1306+
|| hc.count != 2) {
1307+
result = -812;
1308+
goto done;
1309+
}
1310+
1311+
done:
1312+
wolfSSH_free(ssh);
1313+
wolfSSH_CTX_free(ctx);
1314+
return result;
1315+
}
1316+
11841317
/* Verify DoChannelRequest sends CHANNEL_SUCCESS for known types and
11851318
* CHANNEL_FAILURE for unrecognized ones (RFC 4254 Section 5.4).
11861319
*
@@ -1609,6 +1742,10 @@ int wolfSSH_UnitTest(int argc, char** argv)
16091742
unitResult = test_ChannelPutData();
16101743
printf("ChannelPutData: %s\n", (unitResult == 0 ? "SUCCESS" : "FAILED"));
16111744
testResult = testResult || unitResult;
1745+
1746+
unitResult = test_MsgHighwater();
1747+
printf("MsgHighwater: %s\n", (unitResult == 0 ? "SUCCESS" : "FAILED"));
1748+
testResult = testResult || unitResult;
16121749
#endif
16131750

16141751
#ifdef WOLFSSH_KEYGEN

0 commit comments

Comments
 (0)