diff options
author | Matt Caswell <matt@openssl.org> | 2018-05-18 09:07:42 +0100 |
---|---|---|
committer | Matt Caswell <matt@openssl.org> | 2018-06-11 15:46:21 +0100 |
commit | 4aa5a5669c69a66fbd8b31c52014356f1e960501 (patch) | |
tree | 11ac26af3fbeaf80f1b2f4dd7b0c7cf28ef70180 /ssl | |
parent | 2285c0f624b2f5fd16b590511dc35f427053f89f (diff) |
Fix TLSv1.3 alert handling
In TLSv1.3 we should ignore the severity level of an alert according to
the spec.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6370)
Diffstat (limited to 'ssl')
-rw-r--r-- | ssl/record/rec_layer_s3.c | 57 |
1 files changed, 31 insertions, 26 deletions
diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c index 61010f4e72..a3dee2de81 100644 --- a/ssl/record/rec_layer_s3.c +++ b/ssl/record/rec_layer_s3.c @@ -1497,6 +1497,7 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, if (SSL3_RECORD_get_type(rr) == SSL3_RT_ALERT) { unsigned int alert_level, alert_descr; + int tls13_user_cancelled; unsigned char *alert_bytes = SSL3_RECORD_get_data(rr) + SSL3_RECORD_get_off(rr); PACKET alert; @@ -1524,7 +1525,9 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, cb(s, SSL_CB_READ_ALERT, j); } - if (alert_level == SSL3_AL_WARNING) { + tls13_user_cancelled = SSL_IS_TLS13(s) + && alert_descr == SSL_AD_USER_CANCELLED; + if (alert_level == SSL3_AL_WARNING || tls13_user_cancelled) { s->s3->warn_alert = alert_descr; SSL3_RECORD_set_read(rr); @@ -1535,33 +1538,37 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, return -1; } - if (alert_descr == SSL_AD_CLOSE_NOTIFY) { - s->shutdown |= SSL_RECEIVED_SHUTDOWN; - return 0; - } /* * Apart from close_notify the only other warning alert in TLSv1.3 * is user_cancelled - which we just ignore. */ - if (SSL_IS_TLS13(s) && alert_descr != SSL_AD_USER_CANCELLED) { - SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_SSL3_READ_BYTES, - SSL_R_UNKNOWN_ALERT_TYPE); - return -1; - } - /* - * This is a warning but we receive it if we requested - * renegotiation and the peer denied it. Terminate with a fatal - * alert because if application tried to renegotiate it - * presumably had a good reason and expects it to succeed. In - * future we might have a renegotiation where we don't care if - * the peer refused it where we carry on. - */ - if (alert_descr == SSL_AD_NO_RENEGOTIATION) { - SSLfatal(s, SSL_AD_HANDSHAKE_FAILURE, SSL_F_SSL3_READ_BYTES, - SSL_R_NO_RENEGOTIATION); - return -1; - } - } else if (alert_level == SSL3_AL_FATAL) { + if (tls13_user_cancelled) + goto start; + } + + if (alert_descr == SSL_AD_CLOSE_NOTIFY + && (SSL_IS_TLS13(s) || alert_level == SSL3_AL_WARNING)) { + s->shutdown |= SSL_RECEIVED_SHUTDOWN; + return 0; + } + + /* + * This is a warning but we receive it if we requested + * renegotiation and the peer denied it. Terminate with a fatal + * alert because if application tried to renegotiate it + * presumably had a good reason and expects it to succeed. In + * future we might have a renegotiation where we don't care if + * the peer refused it where we carry on. + */ + if (alert_descr == SSL_AD_NO_RENEGOTIATION + && !SSL_IS_TLS13(s) + && alert_level == SSL3_AL_WARNING) { + SSLfatal(s, SSL_AD_HANDSHAKE_FAILURE, SSL_F_SSL3_READ_BYTES, + SSL_R_NO_RENEGOTIATION); + return -1; + } + + if (alert_level == SSL3_AL_FATAL || SSL_IS_TLS13(s)) { char tmp[16]; s->rwstate = SSL_NOTHING; @@ -1579,8 +1586,6 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, SSL_R_UNKNOWN_ALERT_TYPE); return -1; } - - goto start; } if (s->shutdown & SSL_SENT_SHUTDOWN) { /* but we have not received a |