From ef5510cbcce18b9695039ff178a9cc2bb3c36ca5 Mon Sep 17 00:00:00 2001 From: Hayden Roche Date: Fri, 6 Aug 2021 09:49:26 -0700 Subject: [PATCH] During the handshake, make sure alerts are getting read on the client side in the event of an error. --- src/internal.c | 215 ++++++++++++++++++++++++++++++++++++++++----- src/ssl.c | 174 ++---------------------------------- src/tls13.c | 3 + wolfssl/internal.h | 3 + 4 files changed, 207 insertions(+), 188 deletions(-) diff --git a/src/internal.c b/src/internal.c index 5d6a8fe3a..d8235521a 100644 --- a/src/internal.c +++ b/src/internal.c @@ -12189,21 +12189,13 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx, "\tCallback override available, will continue"); /* check if fatal error */ args->fatal = (args->verifyErr) ? 1 : 0; - #if defined(WOLFSSL_EXTRA_ALERTS) || \ - defined(OPENSSL_EXTRA) || \ - defined(OPENSSL_EXTRA_X509_SMALL) if (args->fatal) DoCertFatalAlert(ssl, ret); - #endif } else { WOLFSSL_MSG("\tNo callback override available, fatal"); args->fatal = 1; - #if defined(WOLFSSL_EXTRA_ALERTS) || \ - defined(OPENSSL_EXTRA) || \ - defined(OPENSSL_EXTRA_X509_SMALL) DoCertFatalAlert(ssl, ret); - #endif } } @@ -15924,6 +15916,187 @@ int DoApplicationData(WOLFSSL* ssl, byte* input, word32* inOutIdx) return 0; } +const char* AlertTypeToString(int type) +{ + switch (type) { + case close_notify: + { + static const char close_notify_str[] = + "close_notify"; + return close_notify_str; + } + + case unexpected_message: + { + static const char unexpected_message_str[] = + "unexpected_message"; + return unexpected_message_str; + } + + case bad_record_mac: + { + static const char bad_record_mac_str[] = + "bad_record_mac"; + return bad_record_mac_str; + } + + case record_overflow: + { + static const char record_overflow_str[] = + "record_overflow"; + return record_overflow_str; + } + + case decompression_failure: + { + static const char decompression_failure_str[] = + "decompression_failure"; + return decompression_failure_str; + } + + case handshake_failure: + { + static const char handshake_failure_str[] = + "handshake_failure"; + return handshake_failure_str; + } + + case no_certificate: + { + static const char no_certificate_str[] = + "no_certificate"; + return no_certificate_str; + } + + case bad_certificate: + { + static const char bad_certificate_str[] = + "bad_certificate"; + return bad_certificate_str; + } + + case unsupported_certificate: + { + static const char unsupported_certificate_str[] = + "unsupported_certificate"; + return unsupported_certificate_str; + } + + case certificate_revoked: + { + static const char certificate_revoked_str[] = + "certificate_revoked"; + return certificate_revoked_str; + } + + case certificate_expired: + { + static const char certificate_expired_str[] = + "certificate_expired"; + return certificate_expired_str; + } + + case certificate_unknown: + { + static const char certificate_unknown_str[] = + "certificate_unknown"; + return certificate_unknown_str; + } + + case illegal_parameter: + { + static const char illegal_parameter_str[] = + "illegal_parameter"; + return illegal_parameter_str; + } + + case unknown_ca: + { + static const char unknown_ca_str[] = + "unknown_ca"; + return unknown_ca_str; + } + + case decode_error: + { + static const char decode_error_str[] = + "decode_error"; + return decode_error_str; + } + + case decrypt_error: + { + static const char decrypt_error_str[] = + "decrypt_error"; + return decrypt_error_str; + } + + #ifdef WOLFSSL_MYSQL_COMPATIBLE + /* catch name conflict for enum protocol with MYSQL build */ + case wc_protocol_version: + { + static const char wc_protocol_version_str[] = + "wc_protocol_version"; + return wc_protocol_version_str; + } + + #else + case protocol_version: + { + static const char protocol_version_str[] = + "protocol_version"; + return protocol_version_str; + } + + #endif + case no_renegotiation: + { + static const char no_renegotiation_str[] = + "no_renegotiation"; + return no_renegotiation_str; + } + + case unrecognized_name: + { + static const char unrecognized_name_str[] = + "unrecognized_name"; + return unrecognized_name_str; + } + + case bad_certificate_status_response: + { + static const char bad_certificate_status_response_str[] = + "bad_certificate_status_response"; + return bad_certificate_status_response_str; + } + + case no_application_protocol: + { + static const char no_application_protocol_str[] = + "no_application_protocol"; + return no_application_protocol_str; + } + + default: + WOLFSSL_MSG("Unknown Alert"); + return NULL; + } +} + +static void LogAlert(int type) +{ + (void)type; +#ifdef DEBUG_WOLFSSL + const char* typeStr; + char buff[60]; + + typeStr = AlertTypeToString(type); + if (typeStr != NULL) { + XSNPRINTF(buff, sizeof(buff), "Alert type: %s", typeStr); + WOLFSSL_MSG(buff); + } +#endif /* DEBUG_WOLFSSL */ +} /* process alert, return level */ static int DoAlert(WOLFSSL* ssl, byte* input, word32* inOutIdx, int* type, @@ -15977,20 +16150,12 @@ static int DoAlert(WOLFSSL* ssl, byte* input, word32* inOutIdx, int* type, return ALERT_COUNT_E; } - WOLFSSL_MSG("Got alert"); + LogAlert(*type); if (*type == close_notify) { - WOLFSSL_MSG("\tclose notify"); ssl->options.closeNotify = 1; } -#ifdef WOLFSSL_TLS13 - if (*type == decode_error) { - WOLFSSL_MSG("\tdecode error"); - } - if (*type == illegal_parameter) { - WOLFSSL_MSG("\tillegal parameter"); - } -#endif WOLFSSL_ERROR(*type); + if (IsEncryptionOn(ssl, 0)) { *inOutIdx += ssl->keys.padSz; #if defined(HAVE_ENCRYPT_THEN_MAC) && !defined(WOLFSSL_AEAD_ONLY) @@ -16183,10 +16348,16 @@ static WC_INLINE int VerifyMac(WOLFSSL* ssl, const byte* input, word32 msgSz, return 0; } - -/* process input requests, return 0 is done, 1 is call again to complete, and - negative number is error */ int ProcessReply(WOLFSSL* ssl) +{ + return ProcessReplyEx(ssl, 0); +} + +/* Process input requests. Return 0 is done, 1 is call again to complete, and + negative number is error. If allowSocketErr is set, SOCKET_ERROR_E in + ssl->error will be whitelisted. This is useful when the connection has been + closed and the endpoint wants to check for an alert sent by the other end. */ +int ProcessReplyEx(WOLFSSL* ssl, int allowSocketErr) { int ret = 0, type, readSz; int atomicUser = 0; @@ -16210,6 +16381,7 @@ int ProcessReply(WOLFSSL* ssl) #ifdef WOLFSSL_NONBLOCK_OCSP && ssl->error != OCSP_WANT_READ #endif + && (allowSocketErr != 1 || ssl->error != SOCKET_ERROR_E) ) { WOLFSSL_MSG("ProcessReply retry in error state, not allowed"); return ssl->error; @@ -17088,7 +17260,6 @@ int ProcessReply(WOLFSSL* ssl) } } - #if !defined(WOLFSSL_NO_TLS12) || !defined(NO_OLD_TLS) || \ (defined(WOLFSSL_TLS13) && defined(WOLFSSL_TLS13_MIDDLEBOX_COMPAT)) int SendChangeCipher(WOLFSSL* ssl) diff --git a/src/ssl.c b/src/ssl.c index 5cb2ff162..47777744b 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -13258,6 +13258,7 @@ int wolfSSL_DTLS_SetCookieSecret(WOLFSSL* ssl, #endif if (ssl->options.sendVerify) { if ( (ssl->error = SendCertificate(ssl)) != 0) { + ProcessReplyEx(ssl, 1); /* See if an alert was sent. */ WOLFSSL_ERROR(ssl->error); return WOLFSSL_FATAL_ERROR; } @@ -13276,6 +13277,7 @@ int wolfSSL_DTLS_SetCookieSecret(WOLFSSL* ssl, #endif if (!ssl->options.resuming) { if ( (ssl->error = SendClientKeyExchange(ssl)) != 0) { + ProcessReplyEx(ssl, 1); /* See if an alert was sent. */ WOLFSSL_ERROR(ssl->error); return WOLFSSL_FATAL_ERROR; } @@ -13291,6 +13293,7 @@ int wolfSSL_DTLS_SetCookieSecret(WOLFSSL* ssl, #if !defined(NO_CERTS) && !defined(WOLFSSL_NO_CLIENT_AUTH) if (ssl->options.sendVerify) { if ( (ssl->error = SendCertificateVerify(ssl)) != 0) { + ProcessReplyEx(ssl, 1); /* See if an alert was sent. */ WOLFSSL_ERROR(ssl->error); return WOLFSSL_FATAL_ERROR; } @@ -13303,6 +13306,7 @@ int wolfSSL_DTLS_SetCookieSecret(WOLFSSL* ssl, case FIRST_REPLY_THIRD : if ( (ssl->error = SendChangeCipher(ssl)) != 0) { + ProcessReplyEx(ssl, 1); /* See if an alert was sent. */ WOLFSSL_ERROR(ssl->error); return WOLFSSL_FATAL_ERROR; } @@ -13313,6 +13317,7 @@ int wolfSSL_DTLS_SetCookieSecret(WOLFSSL* ssl, case FIRST_REPLY_FOURTH : if ( (ssl->error = SendFinished(ssl)) != 0) { + ProcessReplyEx(ssl, 1); /* See if an alert was sent. */ WOLFSSL_ERROR(ssl->error); return WOLFSSL_FATAL_ERROR; } @@ -25728,7 +25733,6 @@ int wolfSSL_ERR_GET_REASON(unsigned long err) return ret; } - /* returns a string that describes the alert * * alertID the alert value to look up @@ -25737,178 +25741,16 @@ const char* wolfSSL_alert_type_string_long(int alertID) { WOLFSSL_ENTER("wolfSSL_alert_type_string_long"); - switch (alertID) { - case close_notify: - { - static const char close_notify_str[] = - "close_notify"; - return close_notify_str; - } - - case unexpected_message: - { - static const char unexpected_message_str[] = - "unexpected_message"; - return unexpected_message_str; - } - - case bad_record_mac: - { - static const char bad_record_mac_str[] = - "bad_record_mac"; - return bad_record_mac_str; - } - - case record_overflow: - { - static const char record_overflow_str[] = - "record_overflow"; - return record_overflow_str; - } - - case decompression_failure: - { - static const char decompression_failure_str[] = - "decompression_failure"; - return decompression_failure_str; - } - - case handshake_failure: - { - static const char handshake_failure_str[] = - "handshake_failure"; - return handshake_failure_str; - } - - case no_certificate: - { - static const char no_certificate_str[] = - "no_certificate"; - return no_certificate_str; - } - - case bad_certificate: - { - static const char bad_certificate_str[] = - "bad_certificate"; - return bad_certificate_str; - } - - case unsupported_certificate: - { - static const char unsupported_certificate_str[] = - "unsupported_certificate"; - return unsupported_certificate_str; - } - - case certificate_revoked: - { - static const char certificate_revoked_str[] = - "certificate_revoked"; - return certificate_revoked_str; - } - - case certificate_expired: - { - static const char certificate_expired_str[] = - "certificate_expired"; - return certificate_expired_str; - } - - case certificate_unknown: - { - static const char certificate_unknown_str[] = - "certificate_unknown"; - return certificate_unknown_str; - } - - case illegal_parameter: - { - static const char illegal_parameter_str[] = - "illegal_parameter"; - return illegal_parameter_str; - } - - case unknown_ca: - { - static const char unknown_ca_str[] = - "unknown_ca"; - return unknown_ca_str; - } - - case decode_error: - { - static const char decode_error_str[] = - "decode_error"; - return decode_error_str; - } - - case decrypt_error: - { - static const char decrypt_error_str[] = - "decrypt_error"; - return decrypt_error_str; - } - - #ifdef WOLFSSL_MYSQL_COMPATIBLE - /* catch name conflict for enum protocol with MYSQL build */ - case wc_protocol_version: - { - static const char wc_protocol_version_str[] = - "wc_protocol_version"; - return wc_protocol_version_str; - } - - #else - case protocol_version: - { - static const char protocol_version_str[] = - "protocol_version"; - return protocol_version_str; - } - - #endif - case no_renegotiation: - { - static const char no_renegotiation_str[] = - "no_renegotiation"; - return no_renegotiation_str; - } - - case unrecognized_name: - { - static const char unrecognized_name_str[] = - "unrecognized_name"; - return unrecognized_name_str; - } - - case bad_certificate_status_response: - { - static const char bad_certificate_status_response_str[] = - "bad_certificate_status_response"; - return bad_certificate_status_response_str; - } - - case no_application_protocol: - { - static const char no_application_protocol_str[] = - "no_application_protocol"; - return no_application_protocol_str; - } - - default: - WOLFSSL_MSG("Unknown Alert"); - return NULL; - } + return AlertTypeToString(alertID); } const char* wolfSSL_alert_desc_string_long(int alertID) { WOLFSSL_ENTER("wolfSSL_alert_desc_string_long"); - return wolfSSL_alert_type_string_long(alertID); -} + return AlertTypeToString(alertID); +} /* Gets the current state of the WOLFSSL structure * diff --git a/src/tls13.c b/src/tls13.c index 524b751b4..c877a9421 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -8471,6 +8471,7 @@ int wolfSSL_connect_TLSv13(WOLFSSL* ssl) if (!ssl->options.resuming && ssl->options.sendVerify) { ssl->error = SendTls13Certificate(ssl); if (ssl->error != 0) { + ProcessReplyEx(ssl, 1); /* See if an alert was sent. */ WOLFSSL_ERROR(ssl->error); return WOLFSSL_FATAL_ERROR; } @@ -8489,6 +8490,7 @@ int wolfSSL_connect_TLSv13(WOLFSSL* ssl) if (!ssl->options.resuming && ssl->options.sendVerify) { ssl->error = SendTls13CertificateVerify(ssl); if (ssl->error != 0) { + ProcessReplyEx(ssl, 1); /* See if an alert was sent. */ WOLFSSL_ERROR(ssl->error); return WOLFSSL_FATAL_ERROR; } @@ -8502,6 +8504,7 @@ int wolfSSL_connect_TLSv13(WOLFSSL* ssl) case FIRST_REPLY_FOURTH: if ((ssl->error = SendTls13Finished(ssl)) != 0) { + ProcessReplyEx(ssl, 1); /* See if an alert was sent. */ WOLFSSL_ERROR(ssl->error); return WOLFSSL_FATAL_ERROR; } diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 60ff15ab2..9e915893d 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -4635,6 +4635,9 @@ WOLFSSL_LOCAL int ReceiveData(WOLFSSL*, byte*, int, int); WOLFSSL_LOCAL int SendFinished(WOLFSSL*); WOLFSSL_LOCAL int SendAlert(WOLFSSL*, int, int); WOLFSSL_LOCAL int ProcessReply(WOLFSSL*); +WOLFSSL_LOCAL int ProcessReplyEx(WOLFSSL*, int); + +WOLFSSL_LOCAL const char* AlertTypeToString(int); WOLFSSL_LOCAL int SetCipherSpecs(WOLFSSL*); WOLFSSL_LOCAL int MakeMasterSecret(WOLFSSL*);