commit 32635b828fff10a92441db25ef2341768c22a103
author Steven Valdez <svaldez@google.com> Tue Aug 16 11:25:03 2016 -0400
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Thu Aug 18 23:43:12 2016 +0000
tree 2406ce09f556d1797f15e00f8b91dd2ce89cf5b2
parent dd634ebebd77e0822b26b4273c33e7a08f0b5e1e

Add limit for consecutive KeyUpdate messages.

Change-Id: I2e1ee319bb9852b9c686f2f297c470db54f72279
Reviewed-on: https://boringssl-review.googlesource.com/10370
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

ssl/ssl_lib.c

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index e7830a1..224cae8 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -722,6 +722,7 @@
     int got_handshake;
     int ret = ssl->method->read_app_data(ssl, &got_handshake, buf, num, peek);
     if (ret > 0 || !got_handshake) {
+      ssl->s3->key_update_count = 0;
       return ret;
     }
 

ssl/test/runner/runner.go

diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 3e4ba2e..19b8ee7 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -353,6 +353,9 @@
 	// sendWarningAlerts is the number of consecutive warning alerts to send
 	// before and after the test message.
 	sendWarningAlerts int
+	// sendKeyUpdates is the number of consecutive key updates to send
+	// before and after the test message.
+	sendKeyUpdates int
 	// expectMessageDropped, if true, means the test message is expected to
 	// be dropped by the client rather than echoed back.
 	expectMessageDropped bool
@@ -615,6 +618,10 @@
 		}
 	}
 
+	for i := 0; i < test.sendKeyUpdates; i++ {
+		tlsConn.SendKeyUpdate()
+	}
+
 	for i := 0; i < test.sendEmptyRecords; i++ {
 		tlsConn.Write(nil)
 	}
@@ -671,6 +678,10 @@
 		}
 		tlsConn.Write(testMessage)
 
+		for i := 0; i < test.sendKeyUpdates; i++ {
+			tlsConn.SendKeyUpdate()
+		}
+
 		for i := 0; i < test.sendEmptyRecords; i++ {
 			tlsConn.Write(nil)
 		}
@@ -1993,6 +2004,15 @@
 			expectedError:     ":TOO_MANY_WARNING_ALERTS:",
 		},
 		{
+			name: "SendKeyUpdates",
+			config: Config{
+				MaxVersion: VersionTLS13,
+			},
+			sendKeyUpdates: 33,
+			shouldFail:     true,
+			expectedError:  ":TOO_MANY_KEY_UPDATES:",
+		},
+		{
 			name: "EmptySessionID",
 			config: Config{
 				MaxVersion:             VersionTLS12,

ssl/tls13_both.c

diff --git a/ssl/tls13_both.c b/ssl/tls13_both.c
index e18f246..8400c60 100644
--- a/ssl/tls13_both.c
+++ b/ssl/tls13_both.c
@@ -28,6 +28,11 @@
 #include "internal.h"
 
 
+/* kMaxKeyUpdates is the number of consecutive KeyUpdates that will be
+ * processed. Without this limit an attacker could force unbounded processing
+ * without being able to return application data. */
+static const uint8_t kMaxKeyUpdates = 32;
+
 int tls13_handshake(SSL *ssl) {
   SSL_HANDSHAKE *hs = ssl->s3->hs;
 
@@ -403,9 +408,18 @@
 
 int tls13_post_handshake(SSL *ssl) {
   if (ssl->s3->tmp.message_type == SSL3_MT_KEY_UPDATE) {
+    ssl->s3->key_update_count++;
+    if (ssl->s3->key_update_count > kMaxKeyUpdates) {
+      OPENSSL_PUT_ERROR(SSL, SSL_R_TOO_MANY_KEY_UPDATES);
+      ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_UNEXPECTED_MESSAGE);
+      return 0;
+    }
+
     return tls13_receive_key_update(ssl);
   }
 
+  ssl->s3->key_update_count = 0;
+
   if (ssl->s3->tmp.message_type == SSL3_MT_NEW_SESSION_TICKET &&
       !ssl->server) {
     return tls13_process_new_session_ticket(ssl);