Remodel the if sequence for handling alerts

Reviewed-by: Andy Polyakov <appro@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6370)
This commit is contained in:
Matt Caswell 2018-06-08 17:18:03 +01:00
parent fb62e47c78
commit bcf2907c68

View file

@ -1209,6 +1209,7 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
SSL3_RECORD *rr;
SSL3_BUFFER *rbuf;
void (*cb) (const SSL *ssl, int type2, int val) = NULL;
int is_tls13 = SSL_IS_TLS13(s);
rbuf = &s->rlayer.rbuf;
@ -1340,7 +1341,7 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
if (type == SSL3_RECORD_get_type(rr)
|| (SSL3_RECORD_get_type(rr) == SSL3_RT_CHANGE_CIPHER_SPEC
&& type == SSL3_RT_HANDSHAKE && recvd_type != NULL
&& !SSL_IS_TLS13(s))) {
&& !is_tls13)) {
/*
* SSL3_RT_APPLICATION_DATA or
* SSL3_RT_HANDSHAKE or
@ -1497,7 +1498,6 @@ 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;
@ -1525,9 +1525,8 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
cb(s, SSL_CB_READ_ALERT, j);
}
tls13_user_cancelled = SSL_IS_TLS13(s)
&& alert_descr == SSL_AD_USER_CANCELLED;
if (alert_level == SSL3_AL_WARNING || tls13_user_cancelled) {
if (alert_level == SSL3_AL_WARNING
|| (is_tls13 && alert_descr == SSL_AD_USER_CANCELLED)) {
s->s3->warn_alert = alert_descr;
SSL3_RECORD_set_read(rr);
@ -1537,38 +1536,19 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
SSL_R_TOO_MANY_WARN_ALERTS);
return -1;
}
/*
* Apart from close_notify the only other warning alert in TLSv1.3
* is user_cancelled - which we just ignore.
*/
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.
* Apart from close_notify the only other warning alert in TLSv1.3
* is user_cancelled - which we just ignore.
*/
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)) {
if (is_tls13 && alert_descr == SSL_AD_USER_CANCELLED) {
goto start;
} else if (alert_descr == SSL_AD_CLOSE_NOTIFY
&& (is_tls13 || alert_level == SSL3_AL_WARNING)) {
s->shutdown |= SSL_RECEIVED_SHUTDOWN;
return 0;
} else if (alert_level == SSL3_AL_FATAL || is_tls13) {
char tmp[16];
s->rwstate = SSL_NOTHING;
@ -1581,11 +1561,23 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
SSL3_RECORD_set_read(rr);
SSL_CTX_remove_session(s->session_ctx, s->session);
return 0;
} else {
SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_SSL3_READ_BYTES,
SSL_R_UNKNOWN_ALERT_TYPE);
} else if (alert_descr == SSL_AD_NO_RENEGOTIATION) {
/*
* 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.
*/
SSLfatal(s, SSL_AD_HANDSHAKE_FAILURE, SSL_F_SSL3_READ_BYTES,
SSL_R_NO_RENEGOTIATION);
return -1;
}
SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_SSL3_READ_BYTES,
SSL_R_UNKNOWN_ALERT_TYPE);
return -1;
}
if (s->shutdown & SSL_SENT_SHUTDOWN) { /* but we have not received a