mirror of https://github.com/wolfSSL/wolfssh.git
Bound channel maxPacketSz below the wire limit
MAX_PACKET_SZ caps the whole SSH binary packet, but the channel maxPacketSz it was compared against counts only channel payload. A peer honoring the advertised 35000 overruns the receiver's own check. - Derive MAX_CHANNEL_PACKET_SZ in internal.h: MAX_PACKET_SZ less the transport framing, the CHANNEL_EXTENDED_DATA header, the worst-case padding BundlePacket() picks, and MAX_HMAC_SZ. 34899 by default. - Name that overhead twice, once for the compiler and once as a literal for the preprocessor, which reads the wolfCrypt enum constants in the first form as zero. The #error guarding DEFAULT_MAX_PACKET_SZ uses the second rather than its own copy. - MAX_CHANNEL_PACKET_SZ is derived rather than a tunable, so it is not overridable; an override defeated the bound it enforces. - wolfSSH_CTX_SetWindowPacketSize() bounds maxPacketSz against that instead of MAX_PACKET_SZ; DEFAULT_MAX_PACKET_SZ is unaffected. - api.c tests the new edge and that MAX_PACKET_SZ is now rejected. Issue: F-8835pull/1171/head
parent
8e8b62d358
commit
050dee0fbf
|
|
@ -3314,7 +3314,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;
|
||||
|
|
|
|||
14
tests/api.c
14
tests/api.c
|
|
@ -1308,14 +1308,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,
|
||||
|
|
|
|||
|
|
@ -740,6 +740,35 @@ WOLFSSH_LOCAL int CheckAlgoList(const char* list, byte type);
|
|||
* block size first to decrypt to find the size of
|
||||
* the rest of the data. */
|
||||
|
||||
/* What a channel data packet carries besides its payload: the transport
|
||||
* framing, the larger of the two channel data headers (CHANNEL_EXTENDED_DATA),
|
||||
* the worst-case padding BundlePacket() can pick, and the largest MAC.
|
||||
* The MAC term is the one that varies, so it is spelled twice: once with
|
||||
* MAX_HMAC_SZ for the compiler, and once as a literal 64, the largest
|
||||
* wolfCrypt digest, for the preprocessor. AES_BLOCK_SIZE and MAX_HMAC_SZ are
|
||||
* wolfCrypt enum constants, which #if reads as zero, so the #error below
|
||||
* cannot use the macro form. Both come to 4+1+1+8+4+19+64 = 101. */
|
||||
#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. Comes to 35000 - 101 = 34899. Derived, not
|
||||
* a tunable, so it is deliberately not overridable. */
|
||||
#define MAX_CHANNEL_PACKET_SZ (MAX_PACKET_SZ - CHANNEL_PACKET_OVERHEAD_SZ)
|
||||
|
||||
/* wolfSSH_CTX_SetWindowPacketSize() bounds an explicit size by
|
||||
* MAX_CHANNEL_PACKET_SZ, but a zero there and CtxInit() both take
|
||||
* DEFAULT_MAX_PACKET_SZ unchecked, so assert the default holds too. Both
|
||||
* MAX_PACKET_SZ and DEFAULT_MAX_PACKET_SZ are overridable and the default
|
||||
* path is the common one. */
|
||||
#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 */
|
||||
|
|
|
|||
Loading…
Reference in New Issue