Don't allow too many consecutive warning alerts
authorMatt Caswell <matt@openssl.org>
Wed, 21 Sep 2016 13:07:31 +0000 (14:07 +0100)
committerMatt Caswell <matt@openssl.org>
Wed, 21 Sep 2016 19:21:57 +0000 (20:21 +0100)
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)

include/openssl/ssl.h
ssl/record/rec_layer_d1.c
ssl/record/rec_layer_s3.c
ssl/record/record.h
ssl/record/record_locl.h
ssl/ssl_err.c

index 41cb36e9438e1debf4f1abba47f2d8d273883ffa..440b9a0d74bf7b55294418eabb455207e4733379 100644 (file)
@@ -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
index cd582f32225b15be617c3ce6a484dfcf8dde7d64..2455c2bd124affcfcf606869a7572a02dd1c90d4 100644 (file)
@@ -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
                 /*
index 46870c054b82b9ba1ac2365bed6d1cb1d36e72cf..abde9d4a73447da0142ae4d4d04facfe55db79bc 100644 (file)
@@ -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);
index a093aed48ff3aab1174ed7d6cb18e4a4ced189bf..3e1530f1391012d2a4b662c26d3fcbdebdb45b92 100644 (file)
@@ -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;
 
index 52e59e46d53beb3cd6c86c5d9570bf167a307394..b69afd800237a59f1198e7cef9d0228562d76583 100644 (file)
@@ -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)
index 1fddda612a813558c7b76467c52adb2fe58f6c44..85cb489c9d36a1e01f6c60853338787d8ea1397a 100644 (file)
@@ -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),