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>
This commit is contained in:
Steven Valdez 2016-08-16 11:25:03 -04:00 committed by CQ bot account: commit-bot@chromium.org
parent dd634ebebd
commit 32635b828f
5 changed files with 40 additions and 0 deletions

View File

@ -157,6 +157,7 @@ SSL,1110,TLSV1_UNSUPPORTED_EXTENSION
SSL,217,TLS_PEER_DID_NOT_RESPOND_WITH_CERTIFICATE_LIST
SSL,218,TLS_RSA_ENCRYPTED_VALUE_LENGTH_IS_WRONG
SSL,219,TOO_MANY_EMPTY_FRAGMENTS
SSL,260,TOO_MANY_KEY_UPDATES
SSL,220,TOO_MANY_WARNING_ALERTS
SSL,221,UNABLE_TO_FIND_ECDH_PARAMETERS
SSL,222,UNEXPECTED_EXTENSION

View File

@ -4290,6 +4290,9 @@ typedef struct ssl3_state_st {
* received. */
uint8_t warning_alert_count;
/* key_update_count is the number of consecutive KeyUpdates received. */
uint8_t key_update_count;
/* aead_read_ctx is the current read cipher state. */
SSL_AEAD_CTX *aead_read_ctx;
@ -4807,6 +4810,7 @@ OPENSSL_EXPORT int SSL_set_ssl_method(SSL *s, const SSL_METHOD *method);
#define SSL_R_DUPLICATE_EXTENSION 257
#define SSL_R_MISSING_KEY_SHARE 258
#define SSL_R_INVALID_ALPN_PROTOCOL 259
#define SSL_R_TOO_MANY_KEY_UPDATES 260
#define SSL_R_SSLV3_ALERT_CLOSE_NOTIFY 1000
#define SSL_R_SSLV3_ALERT_UNEXPECTED_MESSAGE 1010
#define SSL_R_SSLV3_ALERT_BAD_RECORD_MAC 1020

View File

@ -722,6 +722,7 @@ static int ssl_read_impl(SSL *ssl, void *buf, int num, int peek) {
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;
}

View File

@ -353,6 +353,9 @@ type testCase struct {
// 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 @@ func doExchange(test *testCase, config *Config, conn net.Conn, isResume bool) er
}
}
for i := 0; i < test.sendKeyUpdates; i++ {
tlsConn.SendKeyUpdate()
}
for i := 0; i < test.sendEmptyRecords; i++ {
tlsConn.Write(nil)
}
@ -671,6 +678,10 @@ func doExchange(test *testCase, config *Config, conn net.Conn, isResume bool) er
}
tlsConn.Write(testMessage)
for i := 0; i < test.sendKeyUpdates; i++ {
tlsConn.SendKeyUpdate()
}
for i := 0; i < test.sendEmptyRecords; i++ {
tlsConn.Write(nil)
}
@ -1992,6 +2003,15 @@ func addBasicTests() {
shouldFail: true,
expectedError: ":TOO_MANY_WARNING_ALERTS:",
},
{
name: "SendKeyUpdates",
config: Config{
MaxVersion: VersionTLS13,
},
sendKeyUpdates: 33,
shouldFail: true,
expectedError: ":TOO_MANY_KEY_UPDATES:",
},
{
name: "EmptySessionID",
config: Config{

View File

@ -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 @@ static int tls13_receive_key_update(SSL *ssl) {
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);