[openssl-commits] [openssl] master update
Matt Caswell
matt at openssl.org
Mon Jun 11 14:53:17 UTC 2018
The branch master has been updated
via bcf2907c685cf1bde9eb92928fad5e85c483563b (commit)
via fb62e47c782397cadf607b92ce50f2bbe250d12e (commit)
via 4aa5a5669c69a66fbd8b31c52014356f1e960501 (commit)
from 2285c0f624b2f5fd16b590511dc35f427053f89f (commit)
- Log -----------------------------------------------------------------
commit bcf2907c685cf1bde9eb92928fad5e85c483563b
Author: Matt Caswell <matt at openssl.org>
Date: Fri Jun 8 17:18:03 2018 +0100
Remodel the if sequence for handling alerts
Reviewed-by: Andy Polyakov <appro at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6370)
commit fb62e47c782397cadf607b92ce50f2bbe250d12e
Author: Matt Caswell <matt at openssl.org>
Date: Fri May 18 09:08:19 2018 +0100
Don't send a warning alert in TLSv1.3
TLSv1.3 ignores the alert level, so we should suppress sending of
warning only alerts.
Fixes #6211
Reviewed-by: Andy Polyakov <appro at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6370)
commit 4aa5a5669c69a66fbd8b31c52014356f1e960501
Author: Matt Caswell <matt at openssl.org>
Date: Fri May 18 09:07:42 2018 +0100
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 at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6370)
-----------------------------------------------------------------------
Summary of changes:
ssl/record/rec_layer_s3.c | 63 ++++++++++++++++++++++-------------------------
ssl/statem/extensions.c | 4 ++-
2 files changed, 33 insertions(+), 34 deletions(-)
diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index 61010f4..75b506b 100644
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -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
@@ -1524,7 +1525,8 @@ 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) {
+ 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);
@@ -1534,34 +1536,19 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
SSL_R_TOO_MANY_WARN_ALERTS);
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) {
+ /*
+ * Apart from close_notify the only other warning alert in TLSv1.3
+ * is user_cancelled - which we just ignore.
+ */
+ 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;
@@ -1574,13 +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;
}
- goto start;
+ 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
diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index 8885e5e..496039e 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -984,7 +984,9 @@ static int final_server_name(SSL *s, unsigned int context, int sent)
return 0;
case SSL_TLSEXT_ERR_ALERT_WARNING:
- ssl3_send_alert(s, SSL3_AL_WARNING, altmp);
+ /* TLSv1.3 doesn't have warning alerts so we suppress this */
+ if (!SSL_IS_TLS13(s))
+ ssl3_send_alert(s, SSL3_AL_WARNING, altmp);
return 1;
case SSL_TLSEXT_ERR_NOACK:
More information about the openssl-commits
mailing list