Don't allow too many consecutive warning alerts

Certain warning alerts are ignored if they are received. This can mean that
no progress will be made if one peer continually sends those warning alerts.
Implement a count so that we abort the connection if we receive too many.

Issue reported by Shi Lei.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit af58be768ebb690f78530f796e92b8ae5c9a4401)

diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 41cb36e9438e1debf4f1abba47f2d8d273883ffa..440b9a0d74bf7b55294418eabb455207e4733379 100644 (file)
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -2482,6 +2482,7 @@ int ERR_load_SSL_strings(void);
 # define SSL_R_TLS_HEARTBEAT_PENDING                      366
 # define SSL_R_TLS_ILLEGAL_EXPORTER_LABEL                 367
 # define SSL_R_TLS_INVALID_ECPOINTFORMAT_LIST             157
+# define SSL_R_TOO_MANY_WARN_ALERTS                       409
 # define SSL_R_UNABLE_TO_FIND_ECDH_PARAMETERS             314
 # define SSL_R_UNABLE_TO_FIND_PUBLIC_KEY_PARAMETERS       239
 # define SSL_R_UNABLE_TO_LOAD_SSL3_MD5_ROUTINES           242

diff --git a/ssl/record/rec_layer_d1.c b/ssl/record/rec_layer_d1.c
index cd582f32225b15be617c3ce6a484dfcf8dde7d64..2455c2bd124affcfcf606869a7572a02dd1c90d4 100644 (file)
--- a/ssl/record/rec_layer_d1.c
+++ b/ssl/record/rec_layer_d1.c
@@ -443,6 +443,14 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
         }
     }
 
+    /*
+     * Reset the count of consecutive warning alerts if we've got a non-empty
+     * record that isn't an alert.
+     */
+    if (SSL3_RECORD_get_type(rr) != SSL3_RT_ALERT
+            && SSL3_RECORD_get_length(rr) != 0)
+        s->rlayer.alert_count = 0;
+
     /* we now have a packet which can be read and processed */
 
     if (s->s3->change_cipher_spec /* set when we receive ChangeCipherSpec,
@@ -722,6 +730,14 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
 
         if (alert_level == SSL3_AL_WARNING) {
             s->s3->warn_alert = alert_descr;
+
+            s->rlayer.alert_count++;
+            if (s->rlayer.alert_count == MAX_WARN_ALERT_COUNT) {
+                al = SSL_AD_UNEXPECTED_MESSAGE;
+                SSLerr(SSL_F_DTLS1_READ_BYTES, SSL_R_TOO_MANY_WARN_ALERTS);
+                goto f_err;
+            }
+
             if (alert_descr == SSL_AD_CLOSE_NOTIFY) {
 #ifndef OPENSSL_NO_SCTP
                 /*

diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index 46870c054b82b9ba1ac2365bed6d1cb1d36e72cf..abde9d4a73447da0142ae4d4d04facfe55db79bc 100644 (file)
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -1063,6 +1063,14 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
     } while (num_recs == 0);
     rr = &rr[curr_rec];
 
+    /*
+     * Reset the count of consecutive warning alerts if we've got a non-empty
+     * record that isn't an alert.
+     */
+    if (SSL3_RECORD_get_type(rr) != SSL3_RT_ALERT
+            && SSL3_RECORD_get_length(rr) != 0)
+        s->rlayer.alert_count = 0;
+
     /* we now have a packet which can be read and processed */
 
     if (s->s3->change_cipher_spec /* set when we receive ChangeCipherSpec,
@@ -1333,6 +1341,14 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
         if (alert_level == SSL3_AL_WARNING) {
             s->s3->warn_alert = alert_descr;
             SSL3_RECORD_set_read(rr);
+
+            s->rlayer.alert_count++;
+            if (s->rlayer.alert_count == MAX_WARN_ALERT_COUNT) {
+                al = SSL_AD_UNEXPECTED_MESSAGE;
+                SSLerr(SSL_F_SSL3_READ_BYTES, SSL_R_TOO_MANY_WARN_ALERTS);
+                goto f_err;
+            }
+
             if (alert_descr == SSL_AD_CLOSE_NOTIFY) {
                 s->shutdown |= SSL_RECEIVED_SHUTDOWN;
                 return (0);

diff --git a/ssl/record/record.h b/ssl/record/record.h
index a093aed48ff3aab1174ed7d6cb18e4a4ced189bf..3e1530f1391012d2a4b662c26d3fcbdebdb45b92 100644 (file)
--- a/ssl/record/record.h
+++ b/ssl/record/record.h
@@ -178,6 +178,8 @@ typedef struct record_layer_st {
     unsigned char write_sequence[SEQ_NUM_SIZE];
     /* Set to true if this is the first record in a connection */
     unsigned int is_first_record;
+    /* Count of the number of consecutive warning alerts received */
+    unsigned int alert_count;
     DTLS_RECORD_LAYER *d;
 } RECORD_LAYER;
 

diff --git a/ssl/record/record_locl.h b/ssl/record/record_locl.h
index 52e59e46d53beb3cd6c86c5d9570bf167a307394..b69afd800237a59f1198e7cef9d0228562d76583 100644 (file)
--- a/ssl/record/record_locl.h
+++ b/ssl/record/record_locl.h
@@ -14,6 +14,8 @@
  *                                                                           *
  *****************************************************************************/
 
+#define MAX_WARN_ALERT_COUNT    5
+
 /* Functions/macros provided by the RECORD_LAYER component */
 
 #define RECORD_LAYER_get_rbuf(rl)               (&(rl)->rbuf)

diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index 1fddda612a813558c7b76467c52adb2fe58f6c44..85cb489c9d36a1e01f6c60853338787d8ea1397a 100644 (file)
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -614,6 +614,7 @@ static ERR_STRING_DATA SSL_str_reasons[] = {
      "tls illegal exporter label"},
     {ERR_REASON(SSL_R_TLS_INVALID_ECPOINTFORMAT_LIST),
      "tls invalid ecpointformat list"},
+    {ERR_REASON(SSL_R_TOO_MANY_WARN_ALERTS), "too many warn alerts"},
     {ERR_REASON(SSL_R_UNABLE_TO_FIND_ECDH_PARAMETERS),
      "unable to find ecdh parameters"},
     {ERR_REASON(SSL_R_UNABLE_TO_FIND_PUBLIC_KEY_PARAMETERS),