Skip to content

Commit 60de6ff

Browse files
ejohnstownpadelsbach
authored andcommitted
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 978ef5c commit 60de6ff

5 files changed

Lines changed: 247 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

@@ -1023,6 +1047,7 @@ WOLFSSH_CTX* CtxInit(WOLFSSH_CTX* ctx, byte side, void* heap)
10231047
ctx->ioSendCb = wsEmbedSend;
10241048
#endif /* WOLFSSH_USER_IO */
10251049
ctx->highwaterMark = DEFAULT_HIGHWATER_MARK;
1050+
ctx->msgHighwaterMark = WOLFSSH_DEFAULT_MSG_HIGHWATER_MARK;
10261051
ctx->highwaterCb = wsHighwater;
10271052
#if defined(WOLFSSH_SCP) && !defined(WOLFSSH_SCP_USER_CALLBACKS)
10281053
ctx->scpRecvCb = wsScpRecvCallback;
@@ -1225,6 +1250,7 @@ WOLFSSH* SshInit(WOLFSSH* ssh, WOLFSSH_CTX* ctx)
12251250
ssh->ioReadCtx = &ssh->rfd; /* prevent invalid access if not correctly */
12261251
ssh->ioWriteCtx = &ssh->wfd; /* set */
12271252
ssh->highwaterMark = ctx->highwaterMark;
1253+
ssh->msgHighwaterMark = ctx->msgHighwaterMark;
12281254
ssh->highwaterCtx = (void*)ssh;
12291255
ssh->reqSuccessCtx = (void*)ssh;
12301256
ssh->fs = NULL;
@@ -6339,7 +6365,9 @@ static int DoNewKeys(WOLFSSH* ssh, byte* buf, word32 len, word32* idx)
63396365

63406366
if (ret == WS_SUCCESS) {
63416367
ssh->rxCount = 0;
6368+
ssh->rxMsgCount = 0;
63426369
ssh->highwaterFlag = 0;
6370+
ssh->msgHighwaterFlag = 0;
63436371

63446372
/* Clear peer is keying flag */
63456373
ssh->isKeying &= ~WOLFSSH_PEER_IS_KEYING;
@@ -10226,7 +10254,23 @@ static int DoPacket(WOLFSSH* ssh, byte* bufferConsumed)
1022610254
idx += padSz;
1022710255
ssh->inputBuffer.idx = idx;
1022810256
ssh->peerSeq++;
10257+
ssh->rxMsgCount++;
1022910258
*bufferConsumed = 1;
10259+
10260+
/* Canonical receive-path highwater check. rxCount was advanced by
10261+
* Decrypt/DecryptAead in DoReceive and rxMsgCount just above, so
10262+
* either threshold can fire here on the crossing packet. Run the
10263+
* check unconditionally so the flag-set side effect and callback
10264+
* fire on data packets (DoChannelData et al. return informational
10265+
* status like WS_CHAN_RXD, not WS_SUCCESS). Only fold the result
10266+
* into ret when ret is currently WS_SUCCESS, so a rekey-trigger
10267+
* return (e.g. WS_WANT_WRITE from SendKexInit) does not mask a
10268+
* real packet error or informational status already in ret. */
10269+
{
10270+
int hwRet = HighwaterCheck(ssh, WOLFSSH_HWSIDE_RECEIVE);
10271+
if (ret == WS_SUCCESS)
10272+
ret = hwRet;
10273+
}
1023010274
}
1023110275

1023210276
return ret;
@@ -10666,14 +10710,6 @@ int DoReceive(WOLFSSH* ssh)
1066610710
ssh->error = ret;
1066710711
return WS_FATAL_ERROR;
1066810712
}
10669-
10670-
ret = HighwaterCheck(ssh, WOLFSSH_HWSIDE_RECEIVE);
10671-
if (ret != WS_SUCCESS) {
10672-
WLOG(WS_LOG_DEBUG, "PR: First HighwaterCheck fail");
10673-
ssh->error = ret;
10674-
ret = WS_FATAL_ERROR;
10675-
break;
10676-
}
1067710713
}
1067810714
FALL_THROUGH;
1067910715

@@ -10758,14 +10794,6 @@ int DoReceive(WOLFSSH* ssh)
1075810794
}
1075910795
}
1076010796
ssh->processReplyState = PROCESS_PACKET;
10761-
10762-
ret = HighwaterCheck(ssh, WOLFSSH_HWSIDE_RECEIVE);
10763-
if (ret != WS_SUCCESS) {
10764-
WLOG(WS_LOG_DEBUG, "PR: HighwaterCheck fail");
10765-
ssh->error = ret;
10766-
ret = WS_FATAL_ERROR;
10767-
break;
10768-
}
1076910797
FALL_THROUGH;
1077010798

1077110799
case PROCESS_PACKET:
@@ -11055,6 +11083,7 @@ static int BundlePacket(WOLFSSH* ssh)
1105511083

1105611084
if (ret == WS_SUCCESS) {
1105711085
ssh->seq++;
11086+
ssh->txMsgCount++;
1105811087
ssh->outputBuffer.length = idx;
1105911088
}
1106011089
else {
@@ -13365,6 +13394,7 @@ int SendNewKeys(WOLFSSH* ssh)
1336513394

1336613395
if (ret == WS_SUCCESS) {
1336713396
ssh->txCount = 0;
13397+
ssh->txMsgCount = 0;
1336813398
}
1336913399

1337013400
if (ret == WS_SUCCESS) {
@@ -18065,6 +18095,11 @@ int wolfSSH_TestDoUserAuthRequest(WOLFSSH* ssh, byte* buf, word32 len,
1806518095
return DoUserAuthRequest(ssh, buf, len, idx);
1806618096
}
1806718097

18098+
int wolfSSH_TestHighwaterCheck(WOLFSSH* ssh, byte side)
18099+
{
18100+
return HighwaterCheck(ssh, side);
18101+
}
18102+
1806818103
#ifndef WOLFSSH_NO_DH_GEX_SHA256
1806918104

1807018105
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: 139 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
/* Plaintext SSH packet from IoSend (before encryption/MAC): LENGTH_SZ,
11851318
* PAD_LENGTH_SZ, then payload starting with the message ID (RFC 4253;
11861319
* wolfSSH PreparePacket/BundlePacket). Not for encrypted payloads or
@@ -1507,6 +1640,7 @@ static int test_DoUserAuthRequest_serviceName(void)
15071640
return result;
15081641
}
15091642

1643+
15101644
#if !defined(WOLFSSH_NO_RSA)
15111645

15121646
/* 2048-bit RSA private key (PKCS#1 DER).
@@ -1806,10 +1940,15 @@ int wolfSSH_UnitTest(int argc, char** argv)
18061940
printf("ChannelPutData: %s\n", (unitResult == 0 ? "SUCCESS" : "FAILED"));
18071941
testResult = testResult || unitResult;
18081942

1943+
unitResult = test_MsgHighwater();
1944+
printf("MsgHighwater: %s\n", (unitResult == 0 ? "SUCCESS" : "FAILED"));
1945+
testResult = testResult || unitResult;
1946+
18091947
unitResult = test_DoUserAuthRequest_serviceName();
18101948
printf("DoUserAuthRequest_serviceName: %s\n",
18111949
(unitResult == 0 ? "SUCCESS" : "FAILED"));
18121950
testResult = testResult || unitResult;
1951+
18131952
#endif
18141953

18151954
#ifdef WOLFSSH_KEYGEN

0 commit comments

Comments
 (0)