diff --git a/apps/wolfssh/wolfssh.c b/apps/wolfssh/wolfssh.c index 9702122e5..14a63ffe6 100644 --- a/apps/wolfssh/wolfssh.c +++ b/apps/wolfssh/wolfssh.c @@ -728,8 +728,8 @@ static int wolfSSH_AGENT_IO_Cb(WS_AgentIoCbAction action, /* Mirrors the library's channel name limit. wolfSSH_SetChannelType() - * discards a longer command and still returns WS_SUCCESS, which would - * send an exec request with no command string. */ + * rejects a longer command with WS_BAD_ARGUMENT; checking it here reports + * it before a connection is attempted. */ #ifndef WOLFSSH_MAX_CHN_NAMESZ #define WOLFSSH_MAX_CHN_NAMESZ 4096 #endif diff --git a/src/internal.c b/src/internal.c index 316397e69..fa72a691f 100644 --- a/src/internal.c +++ b/src/internal.c @@ -98,6 +98,14 @@ #include + +/* The #error in internal.h can't compare the two: the expression's terms are + * enum constants that #if reads as zero. Here both are ordinary constant + * expressions, so a term added without bumping the literal fails the build. */ +typedef char wolfSSH_channel_overhead_check[ + (CHANNEL_PACKET_OVERHEAD_SZ <= CHANNEL_PACKET_OVERHEAD_MAX) ? 1 : -1]; + + /* Flags: HAVE_WC_ECC_SET_RNG @@ -11611,6 +11619,23 @@ static int ChannelRequestIs(const char* type, word32 typeSz, const char* name) } +#ifdef WOLFSSH_TERM +/* Store the terminal size a pty-req or window-change carried. All four + * are taken as sent, zero included, as others do; a zero is how a peer + * reports a dimension it has no value for. The clamp is for the + * consumers, which copy these into the unsigned short fields of a + * struct winsize, and for termResizeCb, which sees them first. */ +static void SetTerminalSize(WOLFSSH* ssh, word32 widthChar, word32 heightRows, + word32 widthPixels, word32 heightPixels) +{ + ssh->widthChar = min(widthChar, TERMINAL_DIMENSION_MAX); + ssh->heightRows = min(heightRows, TERMINAL_DIMENSION_MAX); + ssh->widthPixels = min(widthPixels, TERMINAL_DIMENSION_MAX); + ssh->heightPixels = min(heightPixels, TERMINAL_DIMENSION_MAX); +} +#endif /* WOLFSSH_TERM */ + + static int DoChannelRequest(WOLFSSH* ssh, byte* buf, word32 len, word32* idx) { @@ -11698,23 +11723,26 @@ static int DoChannelRequest(WOLFSSH* ssh, else if (ChannelRequestIs(type, typeSz, "pty-req")) { char term[32]; word32 termSz; + word32 widthChar, heightRows, widthPixels, heightPixels; - channel->ptyReq = 1; /* received a pty request */ termSz = (word32)sizeof(term); ret = GetString(term, &termSz, buf, len, &begin); if (ret == WS_SUCCESS) - ret = GetUint32(&ssh->widthChar, buf, len, &begin); + ret = GetUint32(&widthChar, buf, len, &begin); if (ret == WS_SUCCESS) - ret = GetUint32(&ssh->heightRows, buf, len, &begin); + ret = GetUint32(&heightRows, buf, len, &begin); if (ret == WS_SUCCESS) - ret = GetUint32(&ssh->widthPixels, buf, len, &begin); + ret = GetUint32(&widthPixels, buf, len, &begin); if (ret == WS_SUCCESS) - ret = GetUint32(&ssh->heightPixels, buf, len, &begin); + ret = GetUint32(&heightPixels, buf, len, &begin); if (ret == WS_SUCCESS) ret = GetStringAlloc(ssh->ctx->heap, (char**)&ssh->modes, &ssh->modesSz, buf, len, &begin); if (ret == WS_SUCCESS) { + channel->ptyReq = 1; /* only on a fully parsed request */ + SetTerminalSize(ssh, widthChar, heightRows, + widthPixels, heightPixels); WLOG(WS_LOG_DEBUG, " term = %s", term); WLOG(WS_LOG_DEBUG, " widthChar = %u", ssh->widthChar); WLOG(WS_LOG_DEBUG, " heightRows = %u", ssh->heightRows); @@ -11745,18 +11773,23 @@ static int DoChannelRequest(WOLFSSH* ssh, if (ret == WS_SUCCESS) ret = GetUint32(&heightPixels, buf, len, &begin); - if (ret == WS_SUCCESS) { - WLOG(WS_LOG_DEBUG, " widthChar = %u", widthChar); - WLOG(WS_LOG_DEBUG, " heightRows = %u", heightRows); - WLOG(WS_LOG_DEBUG, " widthPixels = %u", widthPixels); - WLOG(WS_LOG_DEBUG, " heightPixels = %u", heightPixels); - ssh->widthChar = widthChar; - ssh->heightRows = heightRows; - ssh->widthPixels = widthPixels; - ssh->heightPixels = heightPixels; + if (ret == WS_SUCCESS && !channel->ptyReq) { + /* Nothing to resize without a pty on this channel. Dropbear + * refuses the same request for the same reason. */ + WLOG(WS_LOG_DEBUG, " no pty on this channel, rejecting."); + rej = 1; + } + else if (ret == WS_SUCCESS) { + SetTerminalSize(ssh, widthChar, heightRows, + widthPixels, heightPixels); + WLOG(WS_LOG_DEBUG, " widthChar = %u", ssh->widthChar); + WLOG(WS_LOG_DEBUG, " heightRows = %u", ssh->heightRows); + WLOG(WS_LOG_DEBUG, " widthPixels = %u", ssh->widthPixels); + WLOG(WS_LOG_DEBUG, " heightPixels = %u", ssh->heightPixels); if (ssh->termResizeCb) { - if (ssh->termResizeCb(ssh, widthChar, heightRows, - widthPixels, heightPixels, + if (ssh->termResizeCb(ssh, + ssh->widthChar, ssh->heightRows, + ssh->widthPixels, ssh->heightPixels, ssh->termCtx) != WS_SUCCESS) { ret = WS_FATAL_ERROR; } @@ -12109,7 +12142,9 @@ static int DoChannelExtendedData(WOLFSSH* ssh, static int DoPacket(WOLFSSH* ssh, byte* bufferConsumed) { byte* buf = (byte*)ssh->inputBuffer.buffer; - word32 idx = ssh->inputBuffer.idx; + word32 pktStart = ssh->inputBuffer.idx; + word32 pktSz = ssh->curSz; + word32 idx = pktStart; word32 len = ssh->inputBuffer.length; word32 payloadSz; byte padSz; @@ -12132,11 +12167,11 @@ static int DoPacket(WOLFSSH* ssh, byte* bufferConsumed) } /* check for underflow */ - if ((word32)(PAD_LENGTH_SZ + padSz + MSG_ID_SZ) > ssh->curSz) { + if ((word32)(PAD_LENGTH_SZ + padSz + MSG_ID_SZ) > pktSz) { return WS_OVERFLOW_E; } - payloadSz = ssh->curSz - PAD_LENGTH_SZ - padSz - MSG_ID_SZ; + payloadSz = pktSz - PAD_LENGTH_SZ - padSz - MSG_ID_SZ; msg = buf[idx++]; /* At this point, payload starts at "buf + idx". */ @@ -12365,15 +12400,17 @@ static int DoPacket(WOLFSSH* ssh, byte* bufferConsumed) /* if the auth is still pending, don't discard the packet data */ if (ret != WS_AUTH_PENDING) { - if (payloadSz > 0) { - idx += payloadIdx; - if (idx + padSz > len) { - WLOG(WS_LOG_DEBUG, "Not enough data in buffer for pad."); - ret = WS_BUFFER_E; - } + /* Step over the packet using the length DoReceive already validated, + * not payloadIdx. A handler is free to read less than the payload it + * was handed -- the default case above reads none of it -- and that + * must not decide where the next packet begins. pktStart and pktSz + * are both from entry, so a handler cannot move the frame either. */ + idx = pktStart + UINT32_SZ + pktSz; + if (idx > len) { + WLOG(WS_LOG_DEBUG, "Not enough data in buffer for packet."); + ret = WS_BUFFER_E; + idx = len; } - - idx += padSz; ssh->inputBuffer.idx = idx; ssh->peerSeq++; ssh->rxMsgCount++; @@ -22891,6 +22928,11 @@ int wolfSSH_TestDoReceive(WOLFSSH* ssh) return DoReceive(ssh); } +int wolfSSH_TestDoPacket(WOLFSSH* ssh, byte* bufferConsumed) +{ + return DoPacket(ssh, bufferConsumed); +} + int wolfSSH_TestDoUserAuthBanner(WOLFSSH* ssh, byte* buf, word32 len, word32* idx) { diff --git a/src/ssh.c b/src/ssh.c index 1f3f7cc49..1ad18acca 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -1645,7 +1645,18 @@ int wolfSSH_SetExitStatus(WOLFSSH* ssh, word32 exitStatus) * name name or command in the case of subsystem and exec channel types * nameSz size of name buffer * + * Exec and subsystem carry a name string the peer requires, so one must be + * available. Passing none keeps the name an earlier call stored; with + * nothing stored the call is refused rather than sending a request the peer + * reads as malformed. Shell and terminal take no name and drop any stored + * one. A refused call changes nothing, the selected type included. + * * returns WS_SUCCESS on success + * returns WS_BAD_ARGUMENT for a NULL ssh or an unknown type, for exec on + * the server side, for a name at or above WOLFSSH_MAX_CHN_NAMESZ, for a + * nameSz with no name behind it, and for exec or subsystem with no name + * given and none stored + * returns WS_MEMORY_E if the name cannot be allocated */ int wolfSSH_SetChannelType(WOLFSSH* ssh, byte type, byte* name, word32 nameSz) { @@ -1674,7 +1685,18 @@ int wolfSSH_SetChannelType(WOLFSSH* ssh, byte type, byte* name, word32 nameSz) case WOLFSSH_SESSION_SUBSYSTEM: { byte* newName; - if (name != NULL && nameSz > 0 && nameSz < WOLFSSH_MAX_CHN_NAMESZ) { + if (name == NULL && nameSz > 0) { + WLOG(WS_LOG_DEBUG, "Channel name size without a name"); + return WS_BAD_ARGUMENT; + } + if (name != NULL && nameSz >= WOLFSSH_MAX_CHN_NAMESZ) { + /* Report it. Dropping the name sends a request with no name + * string, which the peer rejects as malformed. */ + WLOG(WS_LOG_DEBUG, "Channel name too large"); + return WS_BAD_ARGUMENT; + } + + if (name != NULL && nameSz > 0) { /* only (re)allocate when the name changed; SFTP/SCP retry * loops re-set the same name on every poll */ if (ssh->channelName == NULL || ssh->channelNameSz != nameSz || @@ -1693,9 +1715,18 @@ int wolfSSH_SetChannelType(WOLFSSH* ssh, byte type, byte* name, word32 nameSz) ssh->channelNameSz = nameSz; } } + else if (ssh->channelName == NULL) { + /* No name now and none from an earlier call. Same reason as + * the oversize case: exec and subsystem both carry a + * required name string, and SendChannelRequest() leaves the + * field out entirely when it has nothing to put there. */ + WLOG(WS_LOG_DEBUG, "No channel name to send"); + return WS_BAD_ARGUMENT; + } else { - /* invalid name ignored; type set but WS_SUCCESS returned */ - WLOG(WS_LOG_DEBUG, "No subsystem name or name was too large"); + /* keep the name an earlier call stored; SFTP/SCP retry + * loops re-enter with nothing to say */ + WLOG(WS_LOG_DEBUG, "Keeping the stored channel name"); } ssh->connectChannelId = type; break; @@ -3314,7 +3345,7 @@ int wolfSSH_CTX_SetWindowPacketSize(WOLFSSH_CTX* ctx, return WS_BAD_ARGUMENT; if (windowSz == 0) windowSz = DEFAULT_WINDOW_SZ; - if (maxPacketSz != 0 && maxPacketSz > MAX_PACKET_SZ) + if (maxPacketSz != 0 && maxPacketSz > MAX_CHANNEL_PACKET_SZ) return WS_BAD_ARGUMENT; if (maxPacketSz == 0) maxPacketSz = DEFAULT_MAX_PACKET_SZ; diff --git a/tests/api.c b/tests/api.c index e432fdc2a..2ac3fec21 100644 --- a/tests/api.c +++ b/tests/api.c @@ -299,6 +299,7 @@ static void test_wolfSSH_SetChannelType(void) const byte sub1[] = "sftp"; const byte sub2[] = "a-longer-subsystem-name"; byte* prevName; + byte* maxName; AssertIntNE(WS_SUCCESS, wolfSSH_SetChannelType(NULL, WOLFSSH_SESSION_SHELL, NULL, 0)); @@ -311,13 +312,25 @@ static void test_wolfSSH_SetChannelType(void) AssertNull(ssh->channelName); AssertIntEQ(0, ssh->channelNameSz); - AssertIntEQ(WS_SUCCESS, wolfSSH_SetChannelType(ssh, + /* subsystem carries a required name string, so with none stored and + * none given the request would go out without one */ + AssertIntEQ(WS_BAD_ARGUMENT, wolfSSH_SetChannelType(ssh, WOLFSSH_SESSION_SUBSYSTEM, NULL, 0)); AssertNull(ssh->channelName); + /* a refused call leaves the selected type alone, not just the name */ + AssertIntEQ(WOLFSSH_SESSION_SHELL, ssh->connectChannelId); - AssertIntEQ(WS_SUCCESS, wolfSSH_SetChannelType(ssh, + /* likewise for a size with no name behind it */ + AssertIntEQ(WS_BAD_ARGUMENT, wolfSSH_SetChannelType(ssh, + WOLFSSH_SESSION_SUBSYSTEM, NULL, 4)); + AssertNull(ssh->channelName); + AssertIntEQ(WOLFSSH_SESSION_SHELL, ssh->connectChannelId); + + /* an oversized name is reported, not silently dropped */ + AssertIntEQ(WS_BAD_ARGUMENT, wolfSSH_SetChannelType(ssh, WOLFSSH_SESSION_SUBSYSTEM, (byte*)sub1, WOLFSSH_MAX_CHN_NAMESZ)); AssertNull(ssh->channelName); + AssertIntEQ(WOLFSSH_SESSION_SHELL, ssh->connectChannelId); AssertIntEQ(WS_SUCCESS, wolfSSH_SetChannelType(ssh, WOLFSSH_SESSION_SUBSYSTEM, (byte*)sub1, @@ -335,9 +348,10 @@ static void test_wolfSSH_SetChannelType(void) AssertIntEQ(1, ssh->channelName == prevName); /* a rejected (oversize) name must leave the previous buffer intact */ - AssertIntEQ(WS_SUCCESS, wolfSSH_SetChannelType(ssh, + AssertIntEQ(WS_BAD_ARGUMENT, wolfSSH_SetChannelType(ssh, WOLFSSH_SESSION_SUBSYSTEM, (byte*)sub1, WOLFSSH_MAX_CHN_NAMESZ)); AssertIntEQ(1, ssh->channelName == prevName); + AssertIntEQ(WOLFSSH_SESSION_SUBSYSTEM, ssh->connectChannelId); AssertIntEQ((int)(sizeof(sub1) - 1), (int)ssh->channelNameSz); AssertIntEQ(0, strcmp((const char*)ssh->channelName, (const char*)sub1)); @@ -347,6 +361,19 @@ static void test_wolfSSH_SetChannelType(void) AssertIntEQ(1, ssh->channelName == prevName); AssertIntEQ((int)(sizeof(sub1) - 1), (int)ssh->channelNameSz); + /* the largest name the limit still admits is stored, pinning the + * other side of the boundary the oversize checks above cover */ + AssertNotNull(maxName = (byte*)malloc(WOLFSSH_MAX_CHN_NAMESZ - 1)); + memset(maxName, 'a', WOLFSSH_MAX_CHN_NAMESZ - 1); + AssertIntEQ(WS_SUCCESS, wolfSSH_SetChannelType(ssh, + WOLFSSH_SESSION_SUBSYSTEM, maxName, + WOLFSSH_MAX_CHN_NAMESZ - 1)); + AssertIntEQ(WOLFSSH_MAX_CHN_NAMESZ - 1, (int)ssh->channelNameSz); + AssertIntEQ(0, memcmp(ssh->channelName, maxName, + WOLFSSH_MAX_CHN_NAMESZ - 1)); + AssertIntEQ(0, ssh->channelName[ssh->channelNameSz]); /* NUL terminated */ + free(maxName); + /* repeated set frees the previous buffer before replacing it */ AssertIntEQ(WS_SUCCESS, wolfSSH_SetChannelType(ssh, WOLFSSH_SESSION_SUBSYSTEM, (byte*)sub2, @@ -1308,14 +1335,18 @@ static void test_wolfSSH_CTX_SetWindowPacketSize(void) wolfSSH_CTX_SetWindowPacketSize(ctx, WINDOW_SZ_UPPER_BOUND + 1, 0)); - /* maxPacketSz exactly at transport limit: must succeed and be stored. */ + /* maxPacketSz exactly at the channel limit: must succeed and be stored. */ AssertIntEQ(WS_SUCCESS, - wolfSSH_CTX_SetWindowPacketSize(ctx, 0, MAX_PACKET_SZ)); - AssertIntEQ(MAX_PACKET_SZ, (int)ctx->maxPacketSz); + wolfSSH_CTX_SetWindowPacketSize(ctx, 0, MAX_CHANNEL_PACKET_SZ)); + AssertIntEQ(MAX_CHANNEL_PACKET_SZ, (int)ctx->maxPacketSz); - /* maxPacketSz one above transport limit: must fail. */ + /* maxPacketSz one above the channel limit: must fail. */ AssertIntEQ(WS_BAD_ARGUMENT, - wolfSSH_CTX_SetWindowPacketSize(ctx, 0, MAX_PACKET_SZ + 1)); + wolfSSH_CTX_SetWindowPacketSize(ctx, 0, MAX_CHANNEL_PACKET_SZ + 1)); + + /* The transport limit itself does not fit once framing is added. */ + AssertIntEQ(WS_BAD_ARGUMENT, + wolfSSH_CTX_SetWindowPacketSize(ctx, 0, MAX_PACKET_SZ)); /* Both valid non-zero values: must succeed and be stored. */ AssertIntEQ(WS_SUCCESS, diff --git a/tests/unit.c b/tests/unit.c index e1eff261c..269a8d75f 100644 --- a/tests/unit.c +++ b/tests/unit.c @@ -1267,6 +1267,140 @@ static word32 BuildMacTestPacketPrefix(byte msgId, byte* out, word32 outSz) #endif +/* The test below drives a server-side session. With NO_WOLFSSH_SERVER the + * message filter has no server branch, so every message on such a session is + * refused and the test cannot run. */ +#if defined(WOLFSSH_TEST_INTERNAL) && !defined(NO_WOLFSSH_SERVER) + +/* Swallow the SSH_MSG_UNIMPLEMENTED that DoPacket sends back, so its send + * does not decide the result of the test. */ +static int SinkIoSendUnimplemented(WOLFSSH* ssh, void* buf, word32 sz, + void* ctx) +{ + (void)ssh; (void)buf; (void)ctx; + return (int)sz; +} + +/* Two back-to-back packets in one buffer. DoPacket() is driven directly: + * DoReceive() force-frees the buffer after every packet, zeroing the cursor + * under test. */ +static const byte s_unimplStream[] = { + /* unimplemented message, 8 bytes of payload that no handler reads */ + 0x00, 0x00, 0x00, 0x10, /* packetSz = 16 */ + 0x06, /* padSz = 6 */ + 0x0A, /* msgId = 10 */ + 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, /* payload */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* padding */ + /* MSGID_IGNORE carrying a zero-length string */ + 0x00, 0x00, 0x00, 0x0C, /* packetSz = 12 */ + 0x06, /* padSz = 6 */ + 0x02, /* msgId = IGNORE */ + 0x00, 0x00, 0x00, 0x00, /* string, len 0 */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 /* padding */ +}; + +/* Message ID 10 is unassigned and always allowed, so it reaches DoPacket's + * default case, which reads none of its 8 payload bytes. DoPacket must step + * over them itself. Checked by the cursor it leaves, and by the length peeked + * there being the next packet's. Framing from payloadIdx lands 8 short. */ +static int test_DoPacket_UnimplementedConsumesPayload(void) +{ + WOLFSSH_CTX* ctx = NULL; + WOLFSSH* ssh = NULL; + int result = 0; + int ret; + int i; + /* packetSz of each packet in s_unimplStream, in order */ + static const word32 pktSizes[] = { 16, 12 }; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + if (ctx == NULL) + return -220; + wolfSSH_SetIOSend(ctx, SinkIoSendUnimplemented); + + ssh = wolfSSH_new(ctx); + if (ssh == NULL) { result = -221; goto done; } + + /* Past user auth, so the connection-layer message IDs are allowed. */ + ssh->acceptState = ACCEPT_SERVER_USERAUTH_SENT; + ssh->peerEncryptId = ID_NONE; + ssh->peerMacId = ID_NONE; + ssh->peerAeadMode = 0; + ssh->peerBlockSz = MIN_BLOCK_SZ; + ssh->peerMacSz = 0; + ssh->peerSeq = 0; + ssh->error = 0; + + /* Both packets in the buffer at once. */ + ShrinkBuffer(&ssh->inputBuffer, 1); + if (GrowBuffer(&ssh->inputBuffer, (word32)sizeof(s_unimplStream)) + != WS_SUCCESS) { + result = -222; + goto done; + } + WMEMCPY(ssh->inputBuffer.buffer, s_unimplStream, sizeof(s_unimplStream)); + ssh->inputBuffer.length = (word32)sizeof(s_unimplStream); + ssh->inputBuffer.idx = 0; + + for (i = 0; i < (int)(sizeof(pktSizes) / sizeof(pktSizes[0])); i++) { + word32 pktStart = ssh->inputBuffer.idx; + const byte* lenField = ssh->inputBuffer.buffer + pktStart; + word32 curSz; + byte bufferConsumed = 0; + + /* The peek DoReceive does; a short cursor reads leftover payload. */ + curSz = ((word32)lenField[0] << 24) | ((word32)lenField[1] << 16) + | ((word32)lenField[2] << 8) | (word32)lenField[3]; + if (curSz != pktSizes[i]) { + printf("DoPacket[%d]: packetSz=%u at cursor %u, expected %u\n", + i, curSz, pktStart, pktSizes[i]); + result = -223; + goto done; + } + ssh->curSz = curSz; + + ret = wolfSSH_TestDoPacket(ssh, &bufferConsumed); + if (ret != WS_SUCCESS) { + printf("DoPacket[%d]: ret=%d, error=%d\n", i, ret, ssh->error); + result = -224; + goto done; + } + if (!bufferConsumed) { + printf("DoPacket[%d]: packet not consumed\n", i); + result = -225; + goto done; + } + /* The whole packet, no more and no less. */ + if (ssh->inputBuffer.idx != pktStart + UINT32_SZ + curSz) { + printf("DoPacket[%d]: cursor at %u, expected %u\n", i, + ssh->inputBuffer.idx, pktStart + UINT32_SZ + curSz); + result = -226; + goto done; + } + if (ssh->peerSeq != (word32)(i + 1)) { + printf("DoPacket[%d]: peerSeq=%u, expected %d\n", i, + ssh->peerSeq, i + 1); + result = -227; + goto done; + } + } + + /* Buffer exactly spent. */ + if (ssh->inputBuffer.idx != (word32)sizeof(s_unimplStream)) { + printf("DoPacket: %u of %u bytes consumed\n", ssh->inputBuffer.idx, + (word32)sizeof(s_unimplStream)); + result = -228; + goto done; + } + +done: + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); + return result; +} +#endif /* WOLFSSH_TEST_INTERNAL && !NO_WOLFSSH_SERVER */ + + #if defined(WOLFSSH_TEST_INTERNAL) && \ (!defined(WOLFSSH_NO_HMAC_SHA1) || \ !defined(WOLFSSH_NO_HMAC_SHA1_96) || \ @@ -6596,7 +6730,8 @@ static int test_DoChannelRequest(void) #endif /* WOLFSSH_TERM || WOLFSSH_SHELL */ /* RFC 4254 sec 6.7: window-change must not send a reply even if the - * wire wantReply byte is 1. */ + * wire wantReply byte is 1. No pty-req has been seen on this channel + * yet, so the request is also rejected and the size stays put. */ #if defined(WOLFSSH_SHELL) && defined(WOLFSSH_TERM) { static const byte payWindowChange[] = { @@ -6630,6 +6765,151 @@ static int test_DoChannelRequest(void) result = -451; goto done; } + if (ssh->widthChar != 0 || ssh->heightRows != 0) { + printf("DoChannelRequest[window-change]: applied %ux%u without " + "a pty\n", ssh->widthChar, ssh->heightRows); + result = -452; + goto done; + } + } + + /* A pty-req establishes the pty and the size, and runs the same + * clamping the window-change branch does. */ + { + static byte payPtyReq[] = { + 0x00,0x00,0x00,0x00, /* channelId = 0 */ + 0x00,0x00,0x00,0x07, /* typeSz = 7 */ + 0x70,0x74,0x79,0x2D,0x72,0x65,0x71, /* "pty-req" */ + 0x00, /* wantReply = 0 */ + 0x00,0x00,0x00,0x05, /* termSz = 5 */ + 0x76,0x74,0x31,0x30,0x30, /* "vt100" */ + 0x00,0x00,0x00,0x50, /* widthChar = 80 */ + 0x00,0x00,0x00,0x18, /* heightRows = 24 */ + 0x00,0x00,0x02,0x80, /* widthPixels = 640 */ + 0x00,0x00,0x01,0xE0, /* heightPixels = 480 */ + 0x00,0x00,0x00,0x01, /* modesSz = 1 */ + 0x00 /* TTY_OP_END */ + }; + /* offsets of the four dimensions within the payload above */ + const word32 pwcOff = 25, phrOff = 29, pwpOff = 33, phpOff = 37; + word32 idx2 = 0; + int ret2; + + ret2 = wolfSSH_TestDoChannelRequest(ssh, payPtyReq, + (word32)sizeof(payPtyReq), &idx2); + if (ret2 != WS_SUCCESS) { + printf("DoChannelRequest[pty-req]: ret=%d, expected=%d\n", + ret2, WS_SUCCESS); + result = -453; + goto done; + } + if (ssh->widthChar != 80 || ssh->heightRows != 24 || + ssh->widthPixels != 640 || ssh->heightPixels != 480) { + printf("DoChannelRequest[pty-req]: got %ux%u %ux%u, " + "expected 80x24 640x480\n", ssh->widthChar, + ssh->heightRows, ssh->widthPixels, ssh->heightPixels); + result = -454; + goto done; + } + + /* the clamp is on the pty-req path too, pixels included */ + PutU32BE(payPtyReq + pwcOff, 0x10000); + PutU32BE(payPtyReq + phrOff, 0x10000); + PutU32BE(payPtyReq + pwpOff, 0xFFFFFFFF); + PutU32BE(payPtyReq + phpOff, 0xFFFFFFFF); + idx2 = 0; + ret2 = wolfSSH_TestDoChannelRequest(ssh, payPtyReq, + (word32)sizeof(payPtyReq), &idx2); + if (ret2 != WS_SUCCESS) { + printf("DoChannelRequest[pty-req clamp]: ret=%d, expected=%d\n", + ret2, WS_SUCCESS); + result = -455; + goto done; + } + if (ssh->widthChar != 65535 || ssh->heightRows != 65535 || + ssh->widthPixels != 65535 || ssh->heightPixels != 65535) { + printf("DoChannelRequest[pty-req clamp]: got %ux%u %ux%u, " + "expected 65535 throughout\n", ssh->widthChar, + ssh->heightRows, ssh->widthPixels, ssh->heightPixels); + result = -456; + goto done; + } + } + + /* Dimensions are stored as sent, zero included, as others do. + * Oversized ones are clamped to what a struct winsize holds. */ + { + static byte payWindowChange[] = { + 0x00,0x00,0x00,0x00, /* channelId = 0 */ + 0x00,0x00,0x00,0x0D, /* typeSz = 13 */ + 0x77,0x69,0x6E,0x64,0x6F,0x77,0x2D, /* "window-" */ + 0x63,0x68,0x61,0x6E,0x67,0x65, /* "change" */ + 0x00, /* wantReply = 0 */ + 0x00,0x00,0x00,0x00, /* widthChar */ + 0x00,0x00,0x00,0x00, /* heightRows */ + 0x00,0x00,0x00,0x00, /* widthPixels */ + 0x00,0x00,0x00,0x00 /* heightPixels */ + }; + /* offsets of the four dimensions within the payload above */ + const word32 wcOff = 22, hrOff = 26, wpOff = 30, hpOff = 34; + word32 idx2; + int ret2; + int d; + struct { + const char* label; + word32 widthChar; + word32 heightRows; + word32 widthPixels; + word32 heightPixels; + word32 expectWidth; + word32 expectRows; + word32 expectPixWidth; + word32 expectPixHeight; + int errBase; + } dimCases[] = { + /* establish a known size first */ + { "baseline", 120, 40, 960, 640, 120, 40, 960, 640, -1600 }, + /* zeros are stored as sent, not merged with the previous size */ + { "zeroes", 0, 0, 0, 0, 0, 0, 0, 0, -1602 }, + /* including a zero in one dimension only */ + { "zeroWidth", 0, 50, 0, 640, 0, 50, 0, 640, -1604 }, + /* out of range is clamped, not wrapped to zero */ + { "wrapping", 0x10000, 0x10000, 0x10000, 0x10000, + 65535, 65535, 65535, 65535, -1606 }, + { "huge", 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, + 65535, 65535, 65535, 65535, -1608 } + }; + + for (d = 0; d < (int)(sizeof(dimCases) / sizeof(dimCases[0])); d++) { + PutU32BE(payWindowChange + wcOff, dimCases[d].widthChar); + PutU32BE(payWindowChange + hrOff, dimCases[d].heightRows); + PutU32BE(payWindowChange + wpOff, dimCases[d].widthPixels); + PutU32BE(payWindowChange + hpOff, dimCases[d].heightPixels); + + idx2 = 0; + ret2 = wolfSSH_TestDoChannelRequest(ssh, payWindowChange, + (word32)sizeof(payWindowChange), &idx2); + if (ret2 != WS_SUCCESS) { + printf("DoChannelRequest[%s]: ret=%d, expected=%d\n", + dimCases[d].label, ret2, WS_SUCCESS); + result = dimCases[d].errBase; + goto done; + } + if (ssh->widthChar != dimCases[d].expectWidth || + ssh->heightRows != dimCases[d].expectRows || + ssh->widthPixels != dimCases[d].expectPixWidth || + ssh->heightPixels != dimCases[d].expectPixHeight) { + printf("DoChannelRequest[%s]: got %ux%u %ux%u, " + "expected %ux%u %ux%u\n", dimCases[d].label, + ssh->widthChar, ssh->heightRows, + ssh->widthPixels, ssh->heightPixels, + dimCases[d].expectWidth, dimCases[d].expectRows, + dimCases[d].expectPixWidth, + dimCases[d].expectPixHeight); + result = dimCases[d].errBase - 1; + goto done; + } + } } #endif /* WOLFSSH_SHELL && WOLFSSH_TERM */ @@ -16127,6 +16407,13 @@ int wolfSSH_UnitTest(int argc, char** argv) testResult = testResult || unitResult; #endif +#if defined(WOLFSSH_TEST_INTERNAL) && !defined(NO_WOLFSSH_SERVER) + unitResult = test_DoPacket_UnimplementedConsumesPayload(); + printf("DoPacketUnimplemented: %s\n", + (unitResult == 0 ? "SUCCESS" : "FAILED")); + testResult = testResult || unitResult; +#endif + #if defined(WOLFSSH_TEST_INTERNAL) && \ (!defined(WOLFSSH_NO_HMAC_SHA1) || \ !defined(WOLFSSH_NO_HMAC_SHA1_96) || \ diff --git a/wolfssh/internal.h b/wolfssh/internal.h index c40d78edc..32714c14f 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -567,6 +567,7 @@ enum NameIdType { #define TERMINAL_MODES_MAX_SZ 4096 #define TERMINAL_WIDTH_DEFAULT 80 /* used when there is no terminal */ #define TERMINAL_HEIGHT_DEFAULT 24 +#define TERMINAL_DIMENSION_MAX 65535 /* what struct winsize can hold */ #define AEAD_IMP_IV_SZ 4 #define AEAD_EXP_IV_SZ 8 #define AEAD_NONCE_SZ (AEAD_IMP_IV_SZ+AEAD_EXP_IV_SZ) @@ -735,6 +736,29 @@ WOLFSSH_LOCAL int CheckAlgoList(const char* list, byte type); * block size first to decrypt to find the size of * the rest of the data. */ +/* Channel data packet overhead: transport framing, the larger channel data + * header (CHANNEL_EXTENDED_DATA), worst-case BundlePacket() padding, and the + * largest MAC. Spelled twice because AES_BLOCK_SIZE and MAX_HMAC_SZ are enum + * constants that #if reads as zero. The literal is a ceiling: 101 with a + * 64-byte MAC, less with a smaller digest. */ +#define CHANNEL_PACKET_OVERHEAD_SZ \ + (LENGTH_SZ + PAD_LENGTH_SZ \ + + MSG_ID_SZ + (UINT32_SZ * 2) + LENGTH_SZ \ + + (AES_BLOCK_SIZE + MIN_PAD_LENGTH - 1) \ + + MAX_HMAC_SZ) +#define CHANNEL_PACKET_OVERHEAD_MAX 101 + +/* Largest channel payload that still fits MAX_PACKET_SZ on the wire, which + * bounds the whole binary packet. At most 35000 - 101 = 34899. Derived, not + * a tunable, so deliberately not overridable. */ +#define MAX_CHANNEL_PACKET_SZ (MAX_PACKET_SZ - CHANNEL_PACKET_OVERHEAD_SZ) + +/* Both sizes are overridable, and CtxInit() takes DEFAULT_MAX_PACKET_SZ + * unchecked, so assert the default frames too. */ +#if DEFAULT_MAX_PACKET_SZ > (MAX_PACKET_SZ - CHANNEL_PACKET_OVERHEAD_MAX) + #error "DEFAULT_MAX_PACKET_SZ too large to frame inside MAX_PACKET_SZ" +#endif + typedef struct WOLFSSH_BUFFER { void* heap; /* Heap for allocations */ @@ -1776,6 +1800,8 @@ enum WS_MessageIdLimits { WOLFSSH_API int wolfSSH_TestIsMessageAllowed(WOLFSSH* ssh, byte msg, byte state); WOLFSSH_API int wolfSSH_TestDoReceive(WOLFSSH* ssh); + WOLFSSH_API int wolfSSH_TestDoPacket(WOLFSSH* ssh, + byte* bufferConsumed); WOLFSSH_API int wolfSSH_TestDoUserAuthBanner(WOLFSSH* ssh, byte* buf, word32 len, word32* idx); WOLFSSH_API int wolfSSH_TestPrepareUserAuthRequestPassword(WOLFSSH* ssh,