refactor: more centralized extra alerts

on handshake messages' errors:

- don't send alerts on WANT_READ, WANT_WRITE and WC_PENDING_E "errors"
- use return error code to decide which alert description
  to send
- use alert description handshake_failure in the general case
- if a fatal alert was already sent, do not send any new alerts. This allow
  a more specific alert description in case the exact description can't be
  derived from the return code
This commit is contained in:
Marco Oliverio
2023-03-06 16:47:57 +00:00
parent f666a7d4b7
commit 7b53baea62
2 changed files with 63 additions and 41 deletions

View File

@ -14728,10 +14728,6 @@ static int DoCertificate(WOLFSSL* ssl, byte* input, word32* inOutIdx,
#endif /* SESSION_CERTS */
ret = ProcessPeerCerts(ssl, input, inOutIdx, size);
#ifdef WOLFSSL_EXTRA_ALERTS
if (ret == BUFFER_ERROR || ret == ASN_PARSE_E)
SendAlert(ssl, alert_fatal, decode_error);
#endif
#ifdef OPENSSL_EXTRA
ssl->options.serverState = SERVER_CERT_COMPLETE;
@ -15008,9 +15004,6 @@ int DoFinished(WOLFSSL* ssl, const byte* input, word32* inOutIdx, word32 size,
if (sniff == NO_SNIFF) {
if (XMEMCMP(input + *inOutIdx, &ssl->hsHashes->verifyHashes,size) != 0){
WOLFSSL_MSG("Verify finished error on hashes");
#ifdef WOLFSSL_EXTRA_ALERTS
SendAlert(ssl, alert_fatal, decrypt_error);
#endif
WOLFSSL_ERROR_VERBOSE(VERIFY_FINISHED_ERROR);
return VERIFY_FINISHED_ERROR;
}
@ -15134,9 +15127,6 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
#endif
if (ssl->msgsReceived.got_client_hello) {
WOLFSSL_MSG("Duplicate ClientHello received");
#ifdef WOLFSSL_EXTRA_ALERTS
SendAlert(ssl, alert_fatal, unexpected_message);
#endif
WOLFSSL_ERROR_VERBOSE(DUPLICATE_MSG_E);
return DUPLICATE_MSG_E;
}
@ -15428,9 +15418,6 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
#endif
if (ssl->msgsReceived.got_client_key_exchange) {
WOLFSSL_MSG("Duplicate ClientKeyExchange received");
#ifdef WOLFSSL_EXTRA_ALERTS
SendAlert(ssl, alert_fatal, unexpected_message);
#endif
WOLFSSL_ERROR_VERBOSE(DUPLICATE_MSG_E);
return DUPLICATE_MSG_E;
}
@ -15463,9 +15450,6 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
if (ssl->msgsReceived.got_change_cipher == 0) {
WOLFSSL_MSG("Finished received before ChangeCipher");
#ifdef WOLFSSL_EXTRA_ALERTS
SendAlert(ssl, alert_fatal, unexpected_message);
#endif
WOLFSSL_ERROR_VERBOSE(NO_CHANGE_CIPHER_E);
return NO_CHANGE_CIPHER_E;
}
@ -15519,9 +15503,6 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
if (!ssl->options.resuming &&
ssl->msgsReceived.got_client_key_exchange == 0) {
WOLFSSL_MSG("No ClientKeyExchange before ChangeCipher");
#ifdef WOLFSSL_EXTRA_ALERTS
SendAlert(ssl, alert_fatal, unexpected_message);
#endif
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
return OUT_OF_ORDER_E;
}
@ -16052,6 +16033,61 @@ static int DoHandShakeMsg(WOLFSSL* ssl, byte* input, word32* inOutIdx,
#endif /* !WOLFSSL_NO_TLS12 */
#ifdef WOLFSSL_EXTRA_ALERTS
void SendFatalAlertOnly(WOLFSSL *ssl, int error)
{
int why;
/* already sent a more specific fatal alert */
if (ssl->alert_history.last_tx.level == alert_fatal)
return;
switch (error) {
/* not fatal errors */
case WANT_WRITE:
case WANT_READ:
case ZERO_RETURN:
#ifdef WOLFSSL_ASYNC
case WC_PENGIND_E:
#endif
return;
case BUFFER_ERROR:
case ASN_PARSE_E:
case COMPRESSION_ERROR:
why = decode_error;
break;
case MATCH_SUITE_ERROR:
why = illegal_parameter;
break;
case VERIFY_FINISHED_ERROR:
case SIG_VERIFY_E:
why = decrypt_error;
break;
case DUPLICATE_MSG_E:
case NO_CHANGE_CIPHER_E:
case OUT_OF_ORDER_E:
why = unexpected_message;
break;
case ECC_OUT_OF_RANGE_E:
why = bad_record_mac;
break;
case VERSION_ERROR:
default:
why = handshake_failure;
break;
}
SendAlert(ssl, alert_fatal, why);
}
#else
void SendFatalAlertOnly(WOLFSSL *ssl, int error)
{
(void)ssl;
(void)error;
/* no op */
}
#endif /* WOLFSSL_EXTRA_ALERTS */
#ifdef WOLFSSL_DTLS
static int _DtlsCheckWindow(WOLFSSL* ssl)
@ -16456,7 +16492,6 @@ static WC_INLINE int Dtls13UpdateWindow(WOLFSSL* ssl)
}
#endif /* WOLFSSL_DTLS13 */
int DtlsMsgDrain(WOLFSSL* ssl)
{
DtlsMsg* item = ssl->dtls_rx_msg_list;
@ -16482,6 +16517,9 @@ int DtlsMsgDrain(WOLFSSL* ssl)
if (ret == 0) {
DtlsTxMsgListClean(ssl);
}
else if (!IsAtLeastTLSv1_3(ssl->version)) {
SendFatalAlertOnly(ssl, ret);
}
#ifdef WOLFSSL_ASYNC_CRYPT
if (ret == WC_PENDING_E) {
break;
@ -19784,6 +19822,8 @@ int ProcessReplyEx(WOLFSSL* ssl, int allowSocketErr)
ssl->buffers.inputBuffer.buffer,
&ssl->buffers.inputBuffer.idx,
ssl->buffers.inputBuffer.length);
if (ret != 0)
SendFatalAlertOnly(ssl, ret);
}
#endif
#ifdef WOLFSSL_DTLS13
@ -19820,6 +19860,8 @@ int ProcessReplyEx(WOLFSSL* ssl, int allowSocketErr)
ssl->buffers.inputBuffer.buffer,
&ssl->buffers.inputBuffer.idx,
ssl->buffers.inputBuffer.length);
if (ret != 0)
SendFatalAlertOnly(ssl, ret);
#else
ret = BUFFER_ERROR;
#endif
@ -33226,9 +33268,6 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
if (!ssl->options.downgrade) {
WOLFSSL_MSG("Client trying to connect with lesser version");
#if defined(WOLFSSL_EXTRA_ALERTS) || defined(OPENSSL_EXTRA)
SendAlert(ssl, alert_fatal, handshake_failure);
#endif
ret = VERSION_ERROR;
goto out;
}
@ -33242,9 +33281,6 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
if (belowMinDowngrade) {
WOLFSSL_MSG("\tversion below minimum allowed, fatal error");
#if defined(WOLFSSL_EXTRA_ALERTS) || defined(OPENSSL_EXTRA)
SendAlert(ssl, alert_fatal, handshake_failure);
#endif
ret = VERSION_ERROR;
goto out;
}
@ -33742,12 +33778,6 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
if (ret == 0)
ret = MatchSuite(ssl, clSuites);
#ifdef WOLFSSL_EXTRA_ALERTS
if (ret == BUFFER_ERROR)
SendAlert(ssl, alert_fatal, decode_error);
else if (ret < 0)
SendAlert(ssl, alert_fatal, handshake_failure);
#endif
#if defined(HAVE_TLS_EXTENSIONS) && defined(HAVE_ENCRYPT_THEN_MAC) && \
!defined(WOLFSSL_AEAD_ONLY)
if (ret == 0 && ssl->options.encThenMac &&
@ -35919,18 +35949,12 @@ static int DefTicketEncCb(WOLFSSL* ssl, byte key_name[WOLFSSL_TICKET_NAME_SZ],
/* import peer ECC key */
if ((args->idx - args->begin) + OPAQUE8_LEN > size) {
#ifdef WOLFSSL_EXTRA_ALERTS
SendAlert(ssl, alert_fatal, decode_error);
#endif
ERROR_OUT(BUFFER_ERROR, exit_dcke);
}
args->length = input[args->idx++];
if ((args->idx - args->begin) + args->length > size) {
#ifdef WOLFSSL_EXTRA_ALERTS
SendAlert(ssl, alert_fatal, decode_error);
#endif
ERROR_OUT(BUFFER_ERROR, exit_dcke);
}
@ -36115,9 +36139,6 @@ static int DefTicketEncCb(WOLFSSL* ssl, byte key_name[WOLFSSL_TICKET_NAME_SZ],
args->idx += OPAQUE16_LEN;
if ((args->idx - args->begin) + clientPubSz > size) {
#ifdef WOLFSSL_EXTRA_ALERTS
SendAlert(ssl, alert_fatal, decode_error);
#endif
ERROR_OUT(BUFFER_ERROR, exit_dcke);
}

View File

@ -5685,6 +5685,7 @@ WOLFSSL_LOCAL int ReceiveData(WOLFSSL* ssl, byte* output, int sz, int peek);
WOLFSSL_LOCAL int SendFinished(WOLFSSL* ssl);
WOLFSSL_LOCAL int RetrySendAlert(WOLFSSL* ssl);
WOLFSSL_LOCAL int SendAlert(WOLFSSL* ssl, int severity, int type);
WOLFSSL_LOCAL void SendFatalAlertOnly(WOLFSSL *ssl, int error);
WOLFSSL_LOCAL int ProcessReply(WOLFSSL* ssl);
WOLFSSL_LOCAL int ProcessReplyEx(WOLFSSL* ssl, int allowSocketErr);