diff --git a/src/internal.c b/src/internal.c index 3416a7871..67bae5172 100644 --- a/src/internal.c +++ b/src/internal.c @@ -14601,6 +14601,64 @@ static INLINE int DecryptAead(WOLFSSH* ssh, byte* plain, #endif /* WOLFSSH_NO_AEAD */ +#ifndef WOLFSSH_NO_AES_CBC +/* Reads a rejected packet out to MAX_PACKET_SZ bytes and runs the MAC over + * the rest before failing. The byte count at failure is fixed, and so is the + * MAC work for each kind of rejection, whatever the decrypted length. */ +static int DiscardPacket(WOLFSSH* ssh) +{ + byte mac[MAX_HMAC_SZ]; + int ret; + + ret = GetInputData(ssh, MAX_PACKET_SZ); + if (ret < 0) { + /* Past a want-read the discard is over; report the rejection. */ + if (ssh->error != WS_WANT_READ) + ssh->error = ssh->discardError; + return ret; + } + + WMEMSET(mac, 0, sizeof(mac)); + (void)VerifyMac(ssh, + ssh->inputBuffer.buffer + ssh->inputBuffer.idx + ssh->discardSz, + MAX_PACKET_SZ - ssh->discardSz, mac); + + WLOG(WS_LOG_DEBUG, "Discarded rejected packet"); + ssh->error = ssh->discardError; + return WS_FATAL_ERROR; +} +#endif /* WOLFSSH_NO_AES_CBC */ + + +/* Fails a packet with error. Under CBC the length check and the MAC are an + * oracle on the decrypted length (Albrecht, Paterson and Watson, 2009), so + * the packet is discarded as OpenSSH does. doneSz is how far into the packet + * the discard's MAC starts: the first block, or the whole packet once its + * MAC has run. */ +static int RejectPacket(WOLFSSH* ssh, word32 doneSz, int error) +{ +#ifndef WOLFSSH_NO_AES_CBC + if (ssh->peerEncryptId == ID_AES128_CBC + || ssh->peerEncryptId == ID_AES192_CBC + || ssh->peerEncryptId == ID_AES256_CBC) { + if (doneSz > (word32)MAX_PACKET_SZ) + doneSz = (word32)MAX_PACKET_SZ; + WLOG(WS_LOG_DEBUG, "Discarding rejected packet, error %d", error); + /* ssh->error carries want-read during the discard. */ + ssh->discardSz = doneSz; + ssh->discardError = error; + ssh->processReplyState = PROCESS_DISCARD; + return DiscardPacket(ssh); + } +#else + WOLFSSH_UNUSED(doneSz); +#endif + + ssh->error = error; + return WS_FATAL_ERROR; +} + + int DoReceive(WOLFSSH* ssh) { int ret = WS_SUCCESS; @@ -14647,8 +14705,7 @@ int DoReceive(WOLFSSH* ssh) if (ssh->curSz > MAX_PACKET_SZ - (word32)peerMacSz - UINT32_SZ) { WLOG(WS_LOG_DEBUG, "Packet length overflow: size = %u", ssh->curSz); - ssh->error = WS_OVERFLOW_E; - return WS_FATAL_ERROR; + return RejectPacket(ssh, peerBlockSz, WS_OVERFLOW_E); } /* RFC 4253 section 6 aligns packet_length through the padding @@ -14662,8 +14719,7 @@ int DoReceive(WOLFSSH* ssh) "Packet not block aligned: aligned size = %u, " "block = %u, aead = %u", alignSz, (word32)alignBlockSz, (word32)aeadMode); - ssh->error = WS_BUFFER_E; - return WS_FATAL_ERROR; + return RejectPacket(ssh, peerBlockSz, WS_BUFFER_E); } ssh->processReplyState = PROCESS_PACKET_FINISH; FALL_THROUGH; @@ -14706,8 +14762,7 @@ int DoReceive(WOLFSSH* ssh) } if (verifyResult != WS_SUCCESS) { WLOG(WS_LOG_DEBUG, "PR: VerifyMac fail"); - ssh->error = verifyResult; - return WS_FATAL_ERROR; + return RejectPacket(ssh, readSz, verifyResult); } } else { @@ -14745,6 +14800,11 @@ int DoReceive(WOLFSSH* ssh) } break; +#ifndef WOLFSSH_NO_AES_CBC + case PROCESS_DISCARD: + return DiscardPacket(ssh); +#endif + default: WLOG(WS_LOG_DEBUG, "Bad process input state, program error"); ssh->error = WS_INPUT_CASE_E; diff --git a/tests/unit.c b/tests/unit.c index e98000624..45ae773f0 100644 --- a/tests/unit.c +++ b/tests/unit.c @@ -2183,9 +2183,20 @@ static int test_DoReceive_RejectsMisalignedPacket(void) #if defined(WOLFSSH_TEST_INTERNAL) && !defined(WOLFSSH_NO_AES_CTR) +/* Counts receive calls. */ +static int CountRecv(WOLFSSH* ssh, void* buf, word32 sz, void* ctx) +{ + WOLFSSH_UNUSED(ssh); + WOLFSSH_UNUSED(buf); + WOLFSSH_UNUSED(sz); + (*(word32*)ctx)++; + return WS_CBIO_ERR_WANT_READ; +} + /* Verify DoReceive rejects an AES-CTR packet aligned as a body alone but * misaligned with the length field counted. RFC 4253 section 6 covers - * packet_length too, so a packet_length of 16 is a 20-byte total. */ + * packet_length too, so a packet_length of 16 is a 20-byte total. It fails + * at once, with no CBC-style discard. */ static int test_DoReceive_RejectsMisalignedCtr(void) { WOLFSSH_CTX* ctx = NULL; @@ -2202,15 +2213,18 @@ static int test_DoReceive_RejectsMisalignedCtr(void) byte pkt[UINT32_SZ + 16]; byte record[UINT32_SZ + 16]; word32 totalLen = (word32)sizeof(record); + word32 recvCalls = 0; ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL); if (ctx == NULL) return -800; + wolfSSH_SetIORecv(ctx, CountRecv); ssh = wolfSSH_new(ctx); if (ssh == NULL) { wolfSSH_CTX_free(ctx); return -801; } + wolfSSH_SetIOReadCtx(ssh, &recvCalls); WMEMSET(key, 0x4B, sizeof(key)); WMEMSET(iv, 0x27, sizeof(iv)); @@ -2272,6 +2286,10 @@ static int test_DoReceive_RejectsMisalignedCtr(void) result = -808; goto done5; } + if (recvCalls != 0) { + result = -809; + goto done5; + } done5: if (aesInited) @@ -2283,6 +2301,192 @@ static int test_DoReceive_RejectsMisalignedCtr(void) #endif /* WOLFSSH_TEST_INTERNAL && !WOLFSSH_NO_AES_CTR */ +#if defined(WOLFSSH_TEST_INTERNAL) && !defined(WOLFSSH_NO_AES_CBC) && \ + !defined(WOLFSSH_NO_HMAC_SHA2_256) +typedef struct { + word32 fed; + word32 calls; + word32 closeAt; +} CbcDiscardIo; + +/* Feeds filler in short reads, with a WS_WANT_READ before each one. Closes + * the connection once closeAt bytes are fed, if closeAt is set. */ +static int CbcDiscardRecv(WOLFSSH* ssh, void* buf, word32 sz, void* ctx) +{ + CbcDiscardIo* io = (CbcDiscardIo*)ctx; + + WOLFSSH_UNUSED(ssh); + if (io->closeAt != 0 && io->fed >= io->closeAt) + return WS_CBIO_ERR_CONN_CLOSE; + if ((io->calls++ & 1) == 0) + return WS_CBIO_ERR_WANT_READ; + if (sz > 1000) + sz = 1000; + WMEMSET(buf, 0x5A, sz); + io->fed += sz; + return (int)sz; +} + +/* Verify a rejected AES-CBC packet is read out to MAX_PACKET_SZ bytes + * before DoReceive fails, whether the length check or the MAC rejects it, + * so the failure point does not reveal the decrypted length. A discard cut + * short by the peer closing still reports the rejection, and calling again + * after the discard reads nothing more. */ +static int test_DoReceive_CbcRejectDiscards(void) +{ + WOLFSSH_CTX* ctx = NULL; + WOLFSSH* ssh = NULL; + Aes encAes; + CbcDiscardIo io; + int ret; + int result = 0; + int aesInited = 0; + int i; + word32 step; + word32 fed; + byte key[AES_128_KEY_SIZE]; + byte iv[AES_BLOCK_SIZE]; + /* Two blocks of packet then a MAC: packet_length=28, padding_length=22, + * msgId, uint32 string_len=0, 22 pad bytes. */ + byte pkt[2 * AES_BLOCK_SIZE]; + byte record[2 * AES_BLOCK_SIZE + WC_SHA256_DIGEST_SIZE]; + struct { + word32 packetLen; + word32 readSz; + int error; + word32 lead; /* bytes ahead of the packet in inputBuffer */ + word32 closeAt; /* peer closes after this many filler bytes */ + } cases[] = { + { 20, AES_BLOCK_SIZE, WS_BUFFER_E, 0, 0 }, /* misaligned */ + { 0x7FFFFFF0, AES_BLOCK_SIZE, WS_OVERFLOW_E, 0, 0 }, /* too long */ + { 28, (word32)sizeof(record), WS_VERIFY_MAC_E, 0, 0 }, /* bad MAC */ + { 20, AES_BLOCK_SIZE, WS_BUFFER_E, 7, 0 }, /* offset start */ + { 28, (word32)sizeof(record), WS_VERIFY_MAC_E, 0, 5000 } /* closed */ + }; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL); + if (ctx == NULL) + return -840; + wolfSSH_SetIORecv(ctx, CbcDiscardRecv); + ret = wc_AesInit(&encAes, NULL, INVALID_DEVID); + if (ret != 0) { + result = -841; + goto done6; + } + aesInited = 1; + + WMEMSET(key, 0x3C, sizeof(key)); + WMEMSET(iv, 0x61, sizeof(iv)); + + for (i = 0; i < (int)(sizeof(cases) / sizeof(cases[0])); i++) { + ssh = wolfSSH_new(ctx); + if (ssh == NULL) { + result = -842; + goto done6; + } + WMEMSET(&io, 0, sizeof(io)); + io.closeAt = cases[i].closeAt; + wolfSSH_SetIOReadCtx(ssh, &io); + + WMEMSET(pkt, 0, sizeof(pkt)); + pkt[0] = (byte)(cases[i].packetLen >> 24); + pkt[1] = (byte)(cases[i].packetLen >> 16); + pkt[2] = (byte)(cases[i].packetLen >> 8); + pkt[3] = (byte)(cases[i].packetLen); + pkt[4] = 22; /* padding_length */ + pkt[5] = MSGID_IGNORE; + + if (wc_AesSetKey(&encAes, key, sizeof(key), iv, + AES_ENCRYPTION) != 0 + || wc_AesCbcEncrypt(&encAes, record, pkt, + sizeof(pkt)) != 0) { + result = -843; + goto done6; + } + /* The MAC is zeros, which is wrong for any key. */ + WMEMSET(record + sizeof(pkt), 0, WC_SHA256_DIGEST_SIZE); + + if (wc_AesInit(&ssh->decryptCipher.aes, ssh->ctx->heap, + INVALID_DEVID) != 0 + || wc_AesSetKey(&ssh->decryptCipher.aes, key, sizeof(key), + iv, AES_DECRYPTION) != 0) { + result = -844; + goto done6; + } + ssh->decryptCipher.isInit = 1; + ssh->decryptCipher.cipherType = ID_AES128_CBC; + ssh->peerEncryptId = ID_AES128_CBC; + ssh->peerAeadMode = 0; + ssh->peerBlockSz = AES_BLOCK_SIZE; + ssh->peerMacId = ID_HMAC_SHA2_256; + ssh->peerMacSz = WC_SHA256_DIGEST_SIZE; + WMEMSET(ssh->peerKeys.macKey, 0xA5, WC_SHA256_DIGEST_SIZE); + ssh->peerKeys.macKeySz = WC_SHA256_DIGEST_SIZE; + ssh->peerSeq = 0; + ssh->curSz = 0; + ssh->processReplyState = PROCESS_INIT; + ssh->error = 0; + + /* Preload what DoReceive reads before it rejects the packet. */ + ShrinkBuffer(&ssh->inputBuffer, 1); + if (GrowBuffer(&ssh->inputBuffer, cases[i].lead + cases[i].readSz) + != WS_SUCCESS) { + result = -845; + goto done6; + } + WMEMSET(ssh->inputBuffer.buffer, 0xEE, cases[i].lead); + WMEMCPY(ssh->inputBuffer.buffer + cases[i].lead, record, + cases[i].readSz); + ssh->inputBuffer.length = cases[i].lead + cases[i].readSz; + ssh->inputBuffer.idx = cases[i].lead; + + step = 0; + do { + ret = wolfSSH_TestDoReceive(ssh); + } while (ret == WS_FATAL_ERROR && ssh->error == WS_WANT_READ + && ++step < 1000); + + if (ret != WS_FATAL_ERROR) { + result = -846; + goto done6; + } + if (ssh->error != cases[i].error) { + result = -847; + goto done6; + } + fed = cases[i].closeAt != 0 ? + cases[i].closeAt : MAX_PACKET_SZ - cases[i].readSz; + if (io.fed != fed) { + result = -848; + goto done6; + } + + /* Again after the discard: same error, nothing more read. */ + if (ssh->processReplyState != PROCESS_DISCARD) { + result = -849; + goto done6; + } + ret = wolfSSH_TestDoReceive(ssh); + if (ret != WS_FATAL_ERROR || ssh->error != cases[i].error + || io.fed != fed) { + result = -850; + goto done6; + } + + wolfSSH_free(ssh); + ssh = NULL; + } + +done6: + if (aesInited) + wc_AesFree(&encAes); + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); + return result; +} +#endif /* WOLFSSH_TEST_INTERNAL && !WOLFSSH_NO_AES_CBC && HMAC-SHA2-256 */ + + #if defined(WOLFSSH_TEST_INTERNAL) && !defined(WOLFSSH_NO_AES_GCM) /* Verify DoReceive rejects an AES-GCM record whose length field is not a * multiple of the cipher block size. RFC 5647 aligns the body alone, so a @@ -22700,6 +22904,14 @@ int wolfSSH_UnitTest(int argc, char** argv) testResult = testResult || unitResult; #endif +#if defined(WOLFSSH_TEST_INTERNAL) && !defined(WOLFSSH_NO_AES_CBC) && \ + !defined(WOLFSSH_NO_HMAC_SHA2_256) + unitResult = test_DoReceive_CbcRejectDiscards(); + printf("DoReceiveCbcRejectDiscards: %s\n", + (unitResult == 0 ? "SUCCESS" : "FAILED")); + testResult = testResult || unitResult; +#endif + #if defined(WOLFSSH_TEST_INTERNAL) && !defined(WOLFSSH_NO_AES_GCM) unitResult = test_DoReceive_RejectsMisalignedAead(); printf("DoReceiveRejectsMisalignedAead: %s\n", diff --git a/wolfssh/internal.h b/wolfssh/internal.h index d0b39c7ee..e516c51c3 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -1204,6 +1204,8 @@ struct WOLFSSH { void* channelCloseCtx; /* Channel Close CB context */ void* fs; /* File system handle */ word32 curSz; + word32 discardSz; /* offset of a rejected packet's discard MAC */ + int discardError; /* error reported when the discard ends */ word32 seq; word32 peerSeq; word32 packetStartIdx; /* Current send packet start index */ @@ -1932,7 +1934,8 @@ enum ProcessReplyStates { PROCESS_INIT, PROCESS_PACKET_LENGTH, PROCESS_PACKET_FINISH, - PROCESS_PACKET + PROCESS_PACKET, + PROCESS_DISCARD };