diff --git a/src/internal.c b/src/internal.c index f19d8fa10d..c7656e0baa 100644 --- a/src/internal.c +++ b/src/internal.c @@ -11905,10 +11905,7 @@ int MsgCheckEncryption(WOLFSSL* ssl, byte type, byte encrypted) static WC_INLINE int isLastMsg(const WOLFSSL* ssl, word32 msgSz) { - word32 extra = 0; - if (IsEncryptionOn(ssl, 0)) - extra = ssl->keys.padSz; - return (ssl->buffers.inputBuffer.idx - ssl->curStartIdx) + msgSz + extra + return (ssl->buffers.inputBuffer.idx - ssl->curStartIdx) + msgSz == ssl->curSize; } @@ -17698,9 +17695,6 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx, ssl->options.serverState = SERVER_CERT_COMPLETE; } - if (IsEncryptionOn(ssl, 0)) - args->idx += ssl->keys.padSz; - /* Advance state and proceed */ ssl->options.asyncState = TLS_ASYNC_END; } /* case TLS_ASYNC_FINALIZE */ @@ -17964,12 +17958,6 @@ static int DoCertificateStatus(WOLFSSL* ssl, byte* input, word32* inOutIdx, SendAlert(ssl, alert_fatal, bad_certificate_status_response); } - if (IsEncryptionOn(ssl, 0)) { - if (*inOutIdx + ssl->keys.padSz > size) - return BUFFER_E; - *inOutIdx += ssl->keys.padSz; - } - WOLFSSL_LEAVE("DoCertificateStatus", ret); WOLFSSL_END(WC_FUNC_CERTIFICATE_STATUS_DO); @@ -17984,10 +17972,9 @@ static int DoCertificateStatus(WOLFSSL* ssl, byte* input, word32* inOutIdx, #if !defined(NO_TLS) && !defined(WOLFSSL_NO_TLS12) -static int DoHelloRequest(WOLFSSL* ssl, const byte* input, word32* inOutIdx, - word32 size, word32 totalSz) +static int DoHelloRequest(WOLFSSL* ssl, word32 size) { - (void)input; + (void)size; WOLFSSL_START(WC_FUNC_HELLO_REQUEST_DO); WOLFSSL_ENTER("DoHelloRequest"); @@ -17995,17 +17982,6 @@ static int DoHelloRequest(WOLFSSL* ssl, const byte* input, word32* inOutIdx, if (size) /* must be 0 */ return BUFFER_ERROR; - if (IsEncryptionOn(ssl, 0)) { - /* If size == totalSz then we are in DtlsMsgDrain so no need to worry - * about padding */ - /* access beyond input + size should be checked against totalSz */ - if (size != totalSz && - *inOutIdx + ssl->keys.padSz > totalSz) - return BUFFER_E; - - *inOutIdx += ssl->keys.padSz; - } - if (ssl->options.side == WOLFSSL_SERVER_END) { SendAlert(ssl, alert_fatal, unexpected_message); /* try */ WOLFSSL_ERROR_VERBOSE(FATAL_ERROR); @@ -18040,7 +18016,7 @@ int DoFinished(WOLFSSL* ssl, const byte* input, word32* inOutIdx, word32 size, * If size == totalSz then we are in DtlsMsgDrain so no need to worry about * padding */ if (size != totalSz) { - if (*inOutIdx + size + ssl->keys.padSz > totalSz) + if (*inOutIdx + size > totalSz) return BUFFER_E; } @@ -18083,8 +18059,7 @@ int DoFinished(WOLFSSL* ssl, const byte* input, word32* inOutIdx, word32 size, } #endif - /* force input exhaustion at ProcessReply consuming padSz */ - *inOutIdx += size + ssl->keys.padSz; + *inOutIdx += size; if (ssl->options.side == WOLFSSL_CLIENT_END) { ssl->options.serverState = SERVER_FINISHED_COMPLETE; @@ -18737,8 +18712,7 @@ int DoHandShakeMsgType(WOLFSSL* ssl, byte* input, word32* inOutIdx, return INCOMPLETE_DATA; } - expectedIdx = *inOutIdx + size + - (ssl->keys.encryptionOn ? ssl->keys.padSz : 0); + expectedIdx = *inOutIdx + size; #if !defined(NO_WOLFSSL_SERVER) && \ defined(HAVE_SECURE_RENEGOTIATION) && \ @@ -18903,21 +18877,13 @@ int DoHandShakeMsgType(WOLFSSL* ssl, byte* input, word32* inOutIdx, case hello_request: WOLFSSL_MSG("processing hello request"); - ret = DoHelloRequest(ssl, input, inOutIdx, size, totalSz); + ret = DoHelloRequest(ssl, size); break; #ifndef NO_WOLFSSL_CLIENT case hello_verify_request: WOLFSSL_MSG("processing hello verify request"); ret = DoHelloVerifyRequest(ssl, input,inOutIdx, size); - if (IsEncryptionOn(ssl, 0)) { - /* access beyond input + size should be checked against totalSz - */ - if (*inOutIdx + ssl->keys.padSz > totalSz) - return BUFFER_E; - - *inOutIdx += ssl->keys.padSz; - } break; case server_hello: @@ -18989,8 +18955,6 @@ int DoHandShakeMsgType(WOLFSSL* ssl, byte* input, word32* inOutIdx, AddLateName("ServerHelloDone", &ssl->timeoutInfo); #endif ssl->options.serverState = SERVER_HELLODONE_COMPLETE; - if (IsEncryptionOn(ssl, 0)) - *inOutIdx += ssl->keys.padSz; break; case finished: @@ -19022,16 +18986,6 @@ int DoHandShakeMsgType(WOLFSSL* ssl, byte* input, word32* inOutIdx, } } #endif - /* If size == totalSz then we are in DtlsMsgDrain so no need to worry - * about padding */ - if (IsEncryptionOn(ssl, 0)) { - /* access beyond input + size should be checked against totalSz - */ - if (size != totalSz && - *inOutIdx + ssl->keys.padSz > totalSz) - return BUFFER_E; - *inOutIdx += ssl->keys.padSz; - } break; case client_key_exchange: @@ -19133,6 +19087,8 @@ static int DoHandShakeMsg(WOLFSSL* ssl, byte* input, word32* inOutIdx, } inputLength = ssl->buffers.inputBuffer.length - *inOutIdx; + if (IsEncryptionOn(ssl, 0)) + inputLength -= ssl->keys.padSz; /* If there is a pending fragmented handshake message, * pending message size will be non-zero. */ @@ -19893,11 +19849,6 @@ static int DoDtlsHandShakeMsg(WOLFSSL* ssl, byte* input, word32* inOutIdx, input + *inOutIdx, size, type, fragOffset, fragSz, ssl->heap); *inOutIdx += fragSz; - if (*inOutIdx + ssl->keys.padSz > totalSz) { - WOLFSSL_ERROR(BUFFER_E); - return BUFFER_E; - } - *inOutIdx += ssl->keys.padSz; ret = 0; #ifndef WOLFSSL_DTLS_RESEND_ONLY_TIMEOUT /* If we receive an out of order last flight msg then retransmit */ @@ -19936,10 +19887,6 @@ static int DoDtlsHandShakeMsg(WOLFSSL* ssl, byte* input, word32* inOutIdx, /* Already saw this message and processed it. It can be ignored. */ WOLFSSL_MSG("Already saw this message and processed it"); *inOutIdx += fragSz; - if (*inOutIdx + ssl->keys.padSz > totalSz) { - WOLFSSL_ERROR(BUFFER_E); - return BUFFER_E; - } #ifndef WOLFSSL_DTLS_RESEND_ONLY_TIMEOUT if (IsDtlsNotSctpMode(ssl) && VerifyForDtlsMsgPoolSend(ssl, type, fragOffset)) { @@ -19947,7 +19894,6 @@ static int DoDtlsHandShakeMsg(WOLFSSL* ssl, byte* input, word32* inOutIdx, ret = DtlsMsgPoolSend(ssl, 0); } #endif - *inOutIdx += ssl->keys.padSz; } else if (fragSz < size) { /* Since this branch is in order, but fragmented, dtls_rx_msg_list will @@ -19971,11 +19917,6 @@ static int DoDtlsHandShakeMsg(WOLFSSL* ssl, byte* input, word32* inOutIdx, input + *inOutIdx, size, type, fragOffset, fragSz, ssl->heap); *inOutIdx += fragSz; - if (*inOutIdx + ssl->keys.padSz > totalSz) { - WOLFSSL_ERROR(BUFFER_E); - return BUFFER_E; - } - *inOutIdx += ssl->keys.padSz; ret = 0; if (ssl->dtls_rx_msg_list != NULL && ssl->dtls_rx_msg_list->ready) ret = DtlsMsgDrain(ssl); @@ -19992,9 +19933,9 @@ static int DoDtlsHandShakeMsg(WOLFSSL* ssl, byte* input, word32* inOutIdx, WOLFSSL_ERROR(BUFFER_ERROR); return BUFFER_ERROR; } - if (idx + fragSz + ssl->keys.padSz > totalSz) + if (idx + fragSz > totalSz) return BUFFER_E; - *inOutIdx = idx + fragSz + ssl->keys.padSz; + *inOutIdx = idx + fragSz; /* In async mode always store the message and process it with * DtlsMsgDrain because in case of a WC_PENDING_E it will be * easier this way. */ @@ -22147,7 +22088,7 @@ int DoApplicationData(WOLFSSL* ssl, byte* input, word32* inOutIdx, int sniff) } #endif - dataSz = (int)(msgSz - ssl->keys.padSz); + dataSz = (int)msgSz; if (dataSz < 0) { WOLFSSL_MSG("App data buffer error, malicious input?"); if (sniff == NO_SNIFF) { @@ -22202,8 +22143,6 @@ int DoApplicationData(WOLFSSL* ssl, byte* input, word32* inOutIdx, int sniff) ssl->buffers.clearOutputBuffer.length = (unsigned int)dataSz; } - idx += ssl->keys.padSz; - #ifdef HAVE_LIBZ /* decompress could be bigger, overwrite after verify */ if (ssl->options.usingCompression) @@ -22494,9 +22433,6 @@ static int DoAlert(WOLFSSL* ssl, byte* input, word32* inOutIdx, int* type) } #endif - if (IsEncryptionOn(ssl, 0)) - dataSz -= ssl->keys.padSz; - /* make sure can read the message */ if (dataSz != ALERT_SIZE) { #ifdef WOLFSSL_EXTRA_ALERTS @@ -22565,9 +22501,6 @@ static int DoAlert(WOLFSSL* ssl, byte* input, word32* inOutIdx, int* type) WOLFSSL_ERROR(*type); } } - if (IsEncryptionOn(ssl, 0)) - *inOutIdx += ssl->keys.padSz; - return level; } @@ -23575,6 +23508,11 @@ default: } + /* Reduce curSize to content only, excluding padding/MAC overhead. + * padSz is accounted for once at the end of record processing. */ + if (IsEncryptionOn(ssl, 0)) + ssl->curSize -= (word16)ssl->keys.padSz; + /* in case > 1 msg per record */ ssl->curStartIdx = ssl->buffers.inputBuffer.idx; @@ -23632,7 +23570,7 @@ default: } #if defined(HAVE_ENCRYPT_THEN_MAC) && !defined(WOLFSSL_AEAD_ONLY) if (IsEncryptionOn(ssl, 0) && ssl->options.startedETMRead) { - if ((ssl->curSize - ssl->keys.padSz > MAX_PLAINTEXT_SZ) + if ((ssl->curSize > MAX_PLAINTEXT_SZ) #ifdef WOLFSSL_ASYNC_CRYPT && ssl->buffers.inputBuffer.length != ssl->buffers.inputBuffer.idx @@ -23650,7 +23588,7 @@ default: #endif /* TLS13 plaintext limit is checked earlier before decryption */ if (!IsAtLeastTLSv1_3(ssl->version) - && ssl->curSize - ssl->keys.padSz > MAX_PLAINTEXT_SZ + && ssl->curSize > MAX_PLAINTEXT_SZ #ifdef WOLFSSL_ASYNC_CRYPT && ssl->buffers.inputBuffer.length != ssl->buffers.inputBuffer.idx @@ -23855,11 +23793,6 @@ default: return LENGTH_ERROR; } - if (IsEncryptionOn(ssl, 0) && ssl->options.handShakeDone) { - ssl->buffers.inputBuffer.idx += ssl->keys.padSz; - ssl->curSize -= (word16)ssl->keys.padSz; - } - if (ssl->curSize != 1) { WOLFSSL_MSG("Malicious or corrupted ChangeCipher msg"); WOLFSSL_ERROR_VERBOSE(LENGTH_ERROR); @@ -24027,7 +23960,6 @@ default: ssl->buffers.inputBuffer.idx - ssl->keys.padSz, &processedSize); ssl->buffers.inputBuffer.idx += processedSize; - ssl->buffers.inputBuffer.idx += ssl->keys.padSz; if (ret != 0) return ret; break; @@ -24041,53 +23973,44 @@ default: ssl->options.processReply = doProcessInit; - /* input exhausted */ - if (ssl->buffers.inputBuffer.idx >= ssl->buffers.inputBuffer.length -#ifdef WOLFSSL_DTLS - || (ssl->options.dtls && - /* If app data was processed then return now to avoid - * dropping any app data. */ - (ssl->curRL.type == application_data || - /* client: if we processed a finished message, return to - * allow higher layers to establish the crypto - * parameters of the connection. The remaining data - * may be app data that we would drop without the - * crypto setup. */ - (ssl->options.side == WOLFSSL_CLIENT_END && - ssl->options.serverState == SERVER_FINISHED_COMPLETE && - ssl->options.handShakeState != HANDSHAKE_DONE))) -#endif - ) { - /* Shrink input buffer when we successfully finish record - * processing */ - if ((ret == 0) && ssl->buffers.inputBuffer.dynamicFlag) - ShrinkInputBuffer(ssl, NO_FORCED_FREE); - return ret; - } /* more messages per record */ - else if ((ssl->buffers.inputBuffer.idx - ssl->curStartIdx) + if ((ssl->buffers.inputBuffer.idx - ssl->curStartIdx) < ssl->curSize) { WOLFSSL_MSG("More messages in record"); ssl->options.processReply = runProcessingOneMessage; - - if (IsEncryptionOn(ssl, 0)) { - /* With encryption on, we advance the index by the value - * of ssl->keys.padSz. Since padding only appears once, we - * only can do this at the end of record parsing. We have to - * reset the index to the start of the next message here. */ - if (ssl->buffers.inputBuffer.idx >= ssl->keys.padSz) { - ssl->buffers.inputBuffer.idx -= ssl->keys.padSz; - } - else { - WOLFSSL_MSG("\tBuffer advanced not enough error"); - WOLFSSL_ERROR_VERBOSE(FATAL_ERROR); - return FATAL_ERROR; - } - } } - /* more records */ else { + /* Done with this record. Advance past padding/MAC. */ + if (IsEncryptionOn(ssl, 0)) + ssl->buffers.inputBuffer.idx += ssl->keys.padSz; + + /* input exhausted */ + if (ssl->buffers.inputBuffer.idx >= + ssl->buffers.inputBuffer.length +#ifdef WOLFSSL_DTLS + || (ssl->options.dtls && + /* If app data was processed then return now to avoid + * dropping any app data. */ + (ssl->curRL.type == application_data || + /* client: if we processed a finished message, return + * to allow higher layers to establish the + * crypto parameters of the connection. The + * remaining data may be app data that we would + * drop without the crypto setup. */ + (ssl->options.side == WOLFSSL_CLIENT_END && + ssl->options.serverState == + SERVER_FINISHED_COMPLETE && + ssl->options.handShakeState != HANDSHAKE_DONE))) +#endif + ) { + /* Shrink input buffer when we successfully finish record + * processing */ + if ((ret == 0) && ssl->buffers.inputBuffer.dynamicFlag) + ShrinkInputBuffer(ssl, NO_FORCED_FREE); + return ret; + } + /* more records */ WOLFSSL_MSG("More records in input"); } #ifdef WOLFSSL_ASYNC_CRYPT @@ -24102,10 +24025,14 @@ default: if (ret == WC_NO_ERR_TRACE(APP_DATA_READY)) return ret; #endif - /* It is safe to shrink the input buffer here now. local vars will - * be reset to the new starting value. */ - if (ret == 0 && ssl->buffers.inputBuffer.dynamicFlag) + /* It is safe to shrink the input buffer here now, but only + * when done with the current record. During multi-message + * record processing, shrinking would invalidate curStartIdx + * and curSize. */ + if (ssl->options.processReply != runProcessingOneMessage + && ret == 0 && ssl->buffers.inputBuffer.dynamicFlag) { ShrinkInputBuffer(ssl, NO_FORCED_FREE); + } continue; default: WOLFSSL_MSG("Bad process input state, programming error"); @@ -32163,9 +32090,6 @@ static void MakePSKPreMasterSecret(Arrays* arrays, byte use_psk_key) ssl->options.serverState = SERVER_HELLO_COMPLETE; - if (IsEncryptionOn(ssl, 0)) - *inOutIdx += ssl->keys.padSz; - #ifdef HAVE_SECRET_CALLBACK if (ssl->sessionSecretCb != NULL #ifdef HAVE_SESSION_TICKET @@ -32497,9 +32421,6 @@ static void MakePSKPreMasterSecret(Arrays* arrays, byte use_psk_key) ssl->options.sendVerify = SEND_BLANK_CERT; } - if (IsEncryptionOn(ssl, 0)) - *inOutIdx += ssl->keys.padSz; - WOLFSSL_LEAVE("DoCertificateRequest", 0); WOLFSSL_END(WC_FUNC_CERTIFICATE_REQUEST_DO); @@ -33739,9 +33660,6 @@ static int DoServerKeyExchange(WOLFSSL* ssl, const byte* input, case TLS_ASYNC_FINALIZE: { - if (IsEncryptionOn(ssl, 0)) - args->idx += ssl->keys.padSz; - /* Advance state and proceed */ ssl->options.asyncState = TLS_ASYNC_END; } /* case TLS_ASYNC_FINALIZE */ @@ -35509,9 +35427,6 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, #endif } - if (IsEncryptionOn(ssl, 0)) - *inOutIdx += ssl->keys.padSz; - ssl->expect_session_ticket = 0; return 0; @@ -39340,9 +39255,6 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl) case TLS_ASYNC_FINALIZE: { - if (IsEncryptionOn(ssl, 0)) - args->idx += ssl->keys.padSz; - ssl->options.havePeerVerify = 1; /* Set final index */ @@ -42404,9 +42316,6 @@ static int DefTicketEncCb(WOLFSSL* ssl, byte key_name[WOLFSSL_TICKET_NAME_SZ], case TLS_ASYNC_FINALIZE: { - if (IsEncryptionOn(ssl, 0)) - args->idx += ssl->keys.padSz; - ret = MakeMasterSecret(ssl); /* Check for error */ diff --git a/src/tls13.c b/src/tls13.c index 1d7e0fd3e1..2b8cdfe189 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -6005,9 +6005,6 @@ static int DoTls13EncryptedExtensions(WOLFSSL* ssl, const byte* input, /* Move index to byte after message. */ *inOutIdx = i + totalExtSz; - /* Always encrypted. */ - *inOutIdx += ssl->keys.padSz; - #ifdef WOLFSSL_EARLY_DATA if (ssl->earlyData != no_early_data) { TLSX* ext = TLSX_Find(ssl->extensions, TLSX_EARLY_DATA); @@ -6148,9 +6145,6 @@ static int DoTls13CertificateRequest(WOLFSSL* ssl, const byte* input, #endif } - /* This message is always encrypted so add encryption padding. */ - *inOutIdx += ssl->keys.padSz; - #ifdef WOLFSSL_POST_HANDSHAKE_AUTH { /* CertReqCtx has one byte at end for context value. @@ -11619,9 +11613,6 @@ static int DoTls13CertificateVerify(WOLFSSL* ssl, byte* input, args->idx += args->sz; *inOutIdx = args->idx; - /* Encryption is always on: add padding */ - *inOutIdx += ssl->keys.padSz; - /* Advance state and proceed */ ssl->options.asyncState = TLS_ASYNC_END; @@ -11819,8 +11810,7 @@ int DoTls13Finished(WOLFSSL* ssl, const byte* input, word32* inOutIdx, } } - /* Force input exhaustion at ProcessReply by consuming padSz. */ - *inOutIdx += size + ssl->keys.padSz; + *inOutIdx += size; #ifndef NO_WOLFSSL_SERVER if (ssl->options.side == WOLFSSL_SERVER_END && @@ -12273,8 +12263,6 @@ static int DoTls13KeyUpdate(WOLFSSL* ssl, const byte* input, word32* inOutIdx, /* Move index to byte after message. */ *inOutIdx += totalSz; - /* Always encrypted. */ - *inOutIdx += ssl->keys.padSz; /* Future traffic uses new decryption keys. */ if ((ret = DeriveTls13Keys(ssl, update_traffic_key, DECRYPT_SIDE_ONLY, 1)) @@ -12425,9 +12413,6 @@ static int DoTls13EndOfEarlyData(WOLFSSL* ssl, const byte* input, ssl->earlyData = done_early_data; - /* Always encrypted. */ - *inOutIdx += ssl->keys.padSz; - ret = SetKeysSide(ssl, DECRYPT_SIDE_ONLY); WOLFSSL_LEAVE("DoTls13EndOfEarlyData", ret); @@ -12599,9 +12584,6 @@ static int DoTls13NewSessionTicket(WOLFSSL* ssl, const byte* input, AddSession(ssl); #endif - /* Always encrypted. */ - *inOutIdx += ssl->keys.padSz; - ssl->expect_session_ticket = 0; #else (void)ssl; @@ -12609,7 +12591,7 @@ static int DoTls13NewSessionTicket(WOLFSSL* ssl, const byte* input, WOLFSSL_ENTER("DoTls13NewSessionTicket"); - *inOutIdx += size + ssl->keys.padSz; + *inOutIdx += size; #endif /* HAVE_SESSION_TICKET */ WOLFSSL_LEAVE("DoTls13NewSessionTicket", 0); @@ -13994,7 +13976,7 @@ int DoTls13HandShakeMsg(WOLFSSL* ssl, byte* input, word32* inOutIdx, input + *inOutIdx - HANDSHAKE_HEADER_SZ, inputLength); ssl->arrays->pendingMsgOffset = inputLength; - *inOutIdx += inputLength + ssl->keys.padSz - HANDSHAKE_HEADER_SZ; + *inOutIdx += inputLength - HANDSHAKE_HEADER_SZ; return 0; } @@ -14018,7 +14000,7 @@ int DoTls13HandShakeMsg(WOLFSSL* ssl, byte* input, word32* inOutIdx, XMEMCPY(ssl->arrays->pendingMsg + ssl->arrays->pendingMsgOffset, input + *inOutIdx, inputLength); ssl->arrays->pendingMsgOffset += inputLength; - *inOutIdx += inputLength + ssl->keys.padSz; + *inOutIdx += inputLength; if (ssl->arrays->pendingMsgOffset == ssl->arrays->pendingMsgSz) { @@ -14033,7 +14015,7 @@ int DoTls13HandShakeMsg(WOLFSSL* ssl, byte* input, word32* inOutIdx, ret == WC_NO_ERR_TRACE(OCSP_WANT_READ)) { /* setup to process fragment again */ ssl->arrays->pendingMsgOffset -= inputLength; - *inOutIdx -= inputLength + ssl->keys.padSz; + *inOutIdx -= inputLength; } else #endif